Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Operating System/Drivers
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.6
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Buschel - 2010-11-12
Last edited by Buschel - 2010-11-22

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

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.

Closed by  Buschel
2010-11-22 06:59
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

MEM_ALIGN_ATTR introduced with r28625. Some codec optimizations already based on this (mpc: r28627, libdemac: r28632, libmad: r28639).

nls commented on 2010-11-13 10:37

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?

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.

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 attribute1) for Coldfire. This would of course avoid special definitions in codecs as well.

Your opinions?

1) aligned(16

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 attribute1) 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.

1) aligned(16

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...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing