Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#11753 - Activate CACHEALIGN macros for non-PP CPUs

Attached to Project: Rockbox
Opened by Andree Buschmann (Buschel) - Saturday, 13 November 2010, 00:28 GMT+2
Last edited by Andree Buschmann (Buschel) - Monday, 22 November 2010, 07:59 GMT+2
Task Type Patches
Category Operating System/Drivers
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

In current svn the CACHEALIGN macros are not defined except for PP CPUs. This patch enables those macros for all CPUs. For those who have CACHEALIGN_BITS defined, this value is used to calculate the alignment. For others a default alignment of 32 bytes is used.

The macros can e.g. now be used to optimized codec buffers. I have attached a patch for this as well.

align_v02.patch: Just use CACHEALIGN_BITS like defined for the CPUs in svn.

align_v03.patch: Additionally define CACHEALIGN_BITS for as3525v2/tcc78/tcc77-CPUs (5) and mcf5249/mcf5250 (4)

align_codecs_v01.patch: Replaces the hardcodec alignments with the appropriate macros.
   align_v02.patch (2.7 KiB) 
 firmware/export/system.h            |   31 +++++++++++--------------------
 firmware/export/imx31l.h            |    1 -
 firmware/target/arm/system-target.h |    4 ----
 3 files changed, 11 insertions(+), 25 deletions(-)

   align_v03.patch (4.2 KiB) 
 firmware/export/tcc77x.h            |    2 ++
 firmware/export/system.h            |   30 ++++++++++--------------------
 firmware/export/mcf5249.h           |    2 ++
 firmware/export/as3525v2.h          |    2 ++
 firmware/export/tcc780x.h           |    2 ++
 firmware/export/imx31l.h            |    1 -
 firmware/target/arm/system-target.h |    4 ----
 7 files changed, 18 insertions(+), 25 deletions(-)

   align_codecs_v01.patch (10.5 KiB) 
 apps/codecs/libcook/cook.h             |    2 +-
 apps/codecs/atrac3_rm.c                |    2 +-
 apps/codecs/libmusepack/mpc_decoder.c  |    8 ++++----
 apps/codecs/libmusepack/synth_filter.c |    3 ++-
 apps/codecs/mp3_enc.c                  |    2 +-
 apps/codecs/lib/asm_mcf5249.h          |    2 +-
 apps/codecs/libwma/wmadeci.c           |    2 +-
 apps/codecs/libatrac/atrac3.c          |   10 +++-------
 apps/codecs/libtremor/asm_mcf5249.h    |    2 +-
 apps/codecs/libtremor/oggmalloc.c      |    2 +-
 apps/codecs/libwmavoice/fft.h          |    6 +++---
 apps/codecs/libwmavoice/wmavoice.c     |    6 +++---
 apps/codecs/demac/libdemac/decoder.c   |   10 +++++-----
 13 files changed, 27 insertions(+), 30 deletions(-)

This task depends upon

Closed by  Andree Buschmann (Buschel)
Monday, 22 November 2010, 07:59 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  MEM_ALIGN_ATTR introduced with r28625. Some codec optimizations already based on this (mpc: r28627, libdemac: r28632, libmad: r28639).
Comment by Nils Wallménius (nls) - Saturday, 13 November 2010, 11:37 GMT+2
On coldfire, having 16byte alignment when loading or storing multiple in dram is preferred but this is not a cache alignment since our coldfire cores have no d-cache. So should we abuse CAHEALIGN on the coldfires too or call it something else?
Comment by MichaelGiacomelli (saratoga) - Saturday, 13 November 2010, 21:31 GMT+2
You're aligning it to some internal memory buffer, even if its not a cacheline, so CACHEALIGN makes some sense on CF. Plus consistency across targets is nice since so many of us don't have CF targets to test on.
Comment by Andree Buschmann (Buschel) - Friday, 19 November 2010, 08:32 GMT+2
The following v05-patch will only define the CACHEALIGN macros for CPUs which have HAVE_CPU_CACHE_ALIGN defined. HAVE_CPU_CACHE_ALIGN is defined for ARM CPUs only. There are three options how to proceed.

1) Coldfire CPUs will not have those generic macros defined, to use alignment for Coldfire we still need to define this locally. There is also a v05-patch containing an example (libmusepack) of how alignment can be used for codecs.

2) Of course, it would be easy to define CACHE_ALIGN_BITS and HAVE_CPU_CACHE_ALIGN for Coldfire as well to make use of the generic macros. This would of course avoid special definitions in codecs like shown in the align_codecs_v05-patch. The name of the macros is a bit misleading though...

3) Or we could define another generic macro like MEM_ALIGN_ATTR which is set to CACHEALIGN_ATTR for ARM or __attribute__((aligned(16)) for Coldfire. This would of course avoid special definitions in codecs as well.

Your opinions?
   align_v05.patch (5 KiB) 
 firmware/export/system.h            |   96 +++++++++++++++++++-----------------
 firmware/target/arm/system-target.h |    4 -
 2 files changed, 51 insertions(+), 49 deletions(-)

   align_codecs_v05.patch (3.2 KiB) 
 apps/codecs/libmusepack/mpcdec.h       |   13 +++++++++++++
 apps/codecs/libmusepack/mpc_decoder.c  |    8 ++++----
 apps/codecs/libmusepack/synth_filter.c |    3 ++-
 3 files changed, 19 insertions(+), 5 deletions(-)

Comment by Andree Buschmann (Buschel) - Sunday, 21 November 2010, 01:26 GMT+2
After some short discussions in IRC option 1 was dropped due to complexity for each codec and option 2 was dropped to avoid mis-understanding of what is happening on Coldfire CPUs. So, option 3 is the choice for further steps.

v06 patches define MEM_ALIGN_ATTR which either is defined to CACHEALIGN_ATTR for ARM or __attribute__((aligned(16)) for Coldfire or left empty for all other CPUs. All ARM CPUs which do not have any alignment size defined will use 32 byte alignment by default. This is to cover new ports with new ARM CPUs which may most probably use such alignment.
   align_v06.patch (5.4 KiB) 
 firmware/export/system.h            |  107 +++++++++++++++++++++---------------
 firmware/target/arm/system-target.h |    4 -
 2 files changed, 63 insertions(+), 48 deletions(-)

   align_codecs_v06.patch (2.4 KiB) 
 apps/codecs/libmusepack/mpc_decoder.c  |    8 ++++----
 apps/codecs/libmusepack/synth_filter.c |    3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

Comment by Andree Buschmann (Buschel) - Sunday, 21 November 2010, 15:06 GMT+2
align_v06 was submitted with r28625. align_codecs_v06 was submitted with r28627.
Task kept open for further usage/tests of MEM_ALIGN_ATTR in other codecs.

Loading...