Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10191 - AMSSansa Synchronous Clocking

Attached to Project: Rockbox
Opened by Jack Halpin (FlynDice) - Thursday, 07 May 2009, 07:36 GMT+2
Last edited by Rafaël Carré (funman) - Tuesday, 26 May 2009, 20:45 GMT+2
Task Type Patches
Category Operating System/Drivers
Status Closed
Assigned To No-one
Player Type Another
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

While trying to get the mmu working correctly I managed to get a synchronous clocking scheme to work and thought I would post it for comments. This is probably more of a proof of concept type of thing but I figure it can't hurt to test it out and see how it compares to the current asynchronous clocking scheme. I have left the assembler routines embedded in the .c code to simplify things and also enabled round robin caching.
   synchronous_clocking.patch (3.6 KiB) 
 firmware/target/arm/as3525/system-as3525.c |   53 ++++++++++++++++-------------
 firmware/target/arm/as3525/clock-target.h  |    4 +-
 2 files changed, 32 insertions(+), 25 deletions(-)

This task depends upon

Closed by  Rafaël Carré (funman)
Tuesday, 26 May 2009, 20:45 GMT+2
Reason for closing:  Duplicate
Additional comments about closing:   FS#10245  is more complete, and committed
Comment by Jack Halpin (FlynDice) - Sunday, 10 May 2009, 02:44 GMT+2
When I tested the synchronous clocking setup I discovered it was faster than the current svn asynchronous so I cleaned it up a bit for version 2. I get a slight (10 MHz) improvement over svn with test_codec and a pretty substantial(40%) increase in fps with test_fps.
   synchronous_clocking_2.patch (4.1 KiB) 
 firmware/target/arm/as3525/system-as3525.c |   64 ++++++++++++++---------------
 firmware/target/arm/as3525/clock-target.h  |    4 -
 2 files changed, 35 insertions(+), 33 deletions(-)

Comment by Dustin Skoracki (sko) - Sunday, 10 May 2009, 10:40 GMT+2
With the first patch I noticed that rb is not always booting (nearly every fifth boot failed) but with the new patch it works. I noticed some things:
- playing mp3 needs 40 to 50 MHz less than svn
- the "spacerocks-wheel-bug" (http://forums.rockbox.org/index.php?topic=14064.msg149120#msg149120) is gone
- playback stops when starting pictureflow plugin (in svn pictureflow works with playback)
Comment by Jack Halpin (FlynDice) - Tuesday, 12 May 2009, 06:24 GMT+2
Playback works with pictureflow now. I believe that issue was not related to this patch. I have had exactly one instance of this not booting and it was with the microsd installed. It has booted normally every other time(probably close to 30 -40 times now).
Comment by Graham Hawkins (daytona955) - Tuesday, 12 May 2009, 13:41 GMT+2
I'm probably missing something here - if so sorry :)
But if you dynamically change the PCLK frequency (in set_cpu_frequency), don't you also need to dynamically change the divisors for the clocks that are derived from PCLK (I2C for example)?
Comment by Jack Halpin (FlynDice) - Tuesday, 12 May 2009, 18:49 GMT+2
No, you are most certainly not missing anything, that was also my thought initially but when I tried to adjust the other divisors I got strange results. My thinking is that all of the peripherals that are based off of PCLK get their initial divisor based on the 62MHz in clock-target.h. and the reference frequency we give them there. So when I run PCLK at half speed, all those peripheral speeds are now half speed which I believe is still within their frequency range. I never adjust PLLA so anything that is not clocked off of PCLK or FCLK should not change. That was my reasoning at least but I surely encourage you to tear it apart and improve it!
Comment by Graham Hawkins (daytona955) - Wednesday, 13 May 2009, 12:09 GMT+2
I haven't looked at the peripheral clocks yet. I was thinking about a slightly different strategy for sync clocking. I wondered about leaving the mode as sync for both normal and boost, and using PCLK_DIV1_SEL to take the PCLK from 62MHz to 31MHz while leaving the SDRAM clock mpmc_clk running at 62MHz. I've tried it on my e260v2 and it seems to work OK. What do you think? I've attached a patch.
   synchronous_clocking_fixed_mpmc_clk.patch (3.8 KiB) 
 rockbox.20920.target/firmware/target/arm/as3525/system-as3525.c |   47 ++++++----
 rockbox.20920.target/firmware/target/arm/as3525/clock-target.h  |    8 -
 2 files changed, 34 insertions(+), 21 deletions(-)

Comment by Graham Hawkins (daytona955) - Friday, 15 May 2009, 08:46 GMT+2
I have had a chance to try your patch and my suggestion back to back. Yours seems way more responsive when the FCLK is not boosted. So maybe mine is not such a good idea.. not sure why yet.
Comment by Jack Halpin (FlynDice) - Saturday, 16 May 2009, 16:56 GMT+2
"Yours seems way more responsive when the FCLK is not boosted"

Well I figured it out.. Seems I've performed a bit of cold fusion here! I was adjusting the wrong divider for FCLK. Instead of dividing by it by 2 I was multiplying by 7/8. Hence the nice performance. Here's a patch with the correct dividers or you can just change CGU_PROC |= (1<< 2); to CGU_PROC |= (1<< 4);
   synchronous_clocking_3.patch (4.1 KiB) 
 firmware/target/arm/as3525/system-as3525.c |   64 ++++++++++++++---------------
 firmware/target/arm/as3525/clock-target.h  |    4 -
 2 files changed, 35 insertions(+), 33 deletions(-)

Comment by Graham Hawkins (daytona955) - Saturday, 16 May 2009, 17:19 GMT+2
Me also! <<5 instead of << 6 when trying to adjust PCLK_DIV1_SEL. 'Code in haste, repent at leisure' - or something...
It just crashes when I adjust PCLK_DIV1_SEL. Trying to work out why.
Comment by Graham Hawkins (daytona955) - Monday, 18 May 2009, 10:29 GMT+2
Following my last ill considered patch - here's one I thought about more carefully. I'm afraid it's a bit untidy because of the compile time macros I used to test different clock configs.
I also used FlynDice's synchronous_clocking_3.patch, and his very useful HW info patch from the forum AMS thread. I tested against SVN:20931.
I made some measurements of MP3 decode performance (boosted), and LCD update rates (unboosted). I'm interested in the latter because my e260v2 UI performance is a bit laggy, and I was interested in the least bad option.
Sync clocking seems to give ~5% increase in MP3 decode performance.
Sync clocking also seems to give ~25% better unboosted LCD update rates than fastbus, although the comparison might not be fair, since the sync option uses an FCLK/PCLK of 62/31MHz, but fastbus effectively uses the same 31Mhz for both. In anycase, it is worth changing the DBOP_CLK divisor for the unboosted state, as it gives a significant improvement to UI usability. Vanilla 20931 also gives similar UI performance, but the unboosted clock config seems to be illegal (i.e. async awith PCLK > FCLK).
My original scheme to have FCLK=MPMC_CLK=61MHz, PCLK=31MHz fails to boot. I need to read some more about the MPMC to understand why.
I have attached detailed results.
   sync_async_clk_compare.patch (6.9 KiB) 
 apps/plugins/test_disk.c                             |    2 
 apps/plugins/SOURCES                                 |    3 
 firmware/target/arm/as3525/sansa-e200v2/lcd-e200v2.c |    4 
 firmware/target/arm/as3525/system-as3525.c           |   93 +++++++++++++++----
 firmware/target/arm/as3525/clock-target.h            |    4 
 5 files changed, 86 insertions(+), 20 deletions(-)

   clocking_comparison.txt (3 KiB)
Comment by Graham Hawkins (daytona955) - Monday, 18 May 2009, 10:32 GMT+2
"My original scheme to have FCLK=MPMC_CLK=61MHz, PCLK=31MHz" should read "My original scheme to have FCLK=MPMC_CLK=_62_MHz, PCLK=31MHz"
Comment by Rafaël Carré (funman) - Monday, 18 May 2009, 16:43 GMT+2
in my previous tests, I could never boot with mpmc_clk != pclk.

It seems that the OF uses a 65MHz limit for mpmc_clk, but I'm not 100% sure since there is a lot of code related to pclk that I didn't understand.
Comment by Rafaël Carré (funman) - Tuesday, 26 May 2009, 12:39 GMT+2
Here is a patch which uses synchronous clocking when boosted, and fastbus when unboosted.

The battery bench comparison can be found at http://forums.rockbox.org/index.php?topic=14064.msg150619#msg150619

Do you have objections for it going to SVN ?
   fastbus.diff (2.3 KiB) 
 b/firmware/target/arm/as3525/clock-target.h  |    4 +--
 b/firmware/target/arm/as3525/system-as3525.c |   34 ++++++++++++++++++++-------
 2 files changed, 28 insertions(+), 10 deletions(-)

Comment by Dustin Skoracki (sko) - Tuesday, 26 May 2009, 17:28 GMT+2
The last patch is working very well:
- playing mp3 needs much less MHz (average about 75 MHz on my 128 kBit/s files and average about 115 MHz on my 320 kBit/s files)
- ui-responding is perfect

Loading...