Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#8987 - use 128KB IRAM for PP5022/PP5024

Attached to Project: Rockbox
Opened by Andree Buschmann (Buschel) - Monday, 12 May 2008, 13:10 GMT+2
Last edited by Andree Buschmann (Buschel) - Wednesday, 14 May 2008, 22:30 GMT+2
Task Type Patches
Category Operating System/Drivers
Status Closed
Assigned To No-one
Player Type PortalPlayer-based
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This patch enabled the additional 32KB IRAM for PP5022 and PP5024 processors. This is kind of a first approach and uses 64KB for core and 64KB for plugin/codec.

An additional patch for the musepack decoder takes advantage of this additional IRAM and is sped up by ~6.6%.

EDIT_1: Exchanged the iram-patch (removed the mpc-related changes).
EDIT_2: Exchanged the mpc-patch (removed iram-unrelated changed).
   iram_pp5022_24_v01.patch (1.8 KiB) 
 apps/plugins/plugin.lds             |    8 +++++++-
 firmware/target/arm/ipod/app.lds    |    8 +++++++-
 firmware/target/arm/sandisk/app.lds |    8 +++++++-
 3 files changed, 21 insertions(+), 3 deletions(-)

   mpc_large_iram_v01.patch (3.2 KiB) 
 apps/codecs/libmusepack/mpc_decoder.c |   34 +++++++++++++++++-----------------
 apps/codecs/libmusepack/musepack.h    |   10 ++++++++++
 2 files changed, 27 insertions(+), 17 deletions(-)

This task depends upon

Closed by  Andree Buschmann (Buschel)
Wednesday, 14 May 2008, 22:30 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Committed with r17506 and 17507.
Comment by MichaelGiacomelli (saratoga) - Monday, 12 May 2008, 16:26 GMT+2
I would really prefer to give the entire extra 32KB of IRAM to codecs. They can use it a lot more then core.
Comment by Andree Buschmann (Buschel) - Monday, 12 May 2008, 16:48 GMT+2
saratoga: v2 give 80KB of IRAM to plugin/codec, core's IRAM is untouched.
   iram_pp5022_24_v02.patch (0.7 KiB) 
 apps/plugins/plugin.lds |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comment by Jens Arnold (amiconn) - Wednesday, 14 May 2008, 01:59 GMT+2
I tried the patch on X5 (Coldfire MCF5250, also having 128KB or IRAM). It sped up decoding of my test track from 482% realtime to 615% realtime (~27% speedup)!

I've attached the modified patch, which also corrects the checks for PP5022 and PP5024. Those macros don't work like the CPU_* group macros. They are always defined, which would cause v1 to break on any other CPU than PP5022/PP5024. You're supposed to check CONFIG_CPU.

Regarding IRAM distribution - I'd prefer to keep even distribution between core and codecs/plugins, like it's already done in SVN for MCF5250. While IRAM might be quite useful for codecs, it might also be quite useful in the core (e.g. for better DSP code), and there are often also some target specific tasks in the core which profit from extra IRAM.
   mpc_large_iram_v02.patch (3.3 KiB) 
 apps/codecs/libmusepack/mpc_decoder.c |   34 +++++++++++++++++-----------------
 apps/codecs/libmusepack/musepack.h    |   10 ++++++++++
 2 files changed, 27 insertions(+), 17 deletions(-)

Comment by MichaelGiacomelli (saratoga) - Wednesday, 14 May 2008, 18:14 GMT+2
I don't see any point in 64/64k on PP. We're only using about 2/3 of the available IRAM as it is, and theres no indication that theres some compelling feature coming that would require as much or more IRAM as we currently use for the entire core. 48K is arguably too much already, and should be reduced in order to save battery life decoding . . .

I won't argue too much for Coldfire though, since I don't have any of those targets.
Comment by Andree Buschmann (Buschel) - Wednesday, 14 May 2008, 21:54 GMT+2
iram_v02 patch submitted with r17506, mpc_v02-patch submitted with r17507.

Loading...