Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10254 - AMSSansa Place Cache Coherence Routines in DMA Driver

Attached to Project: Rockbox
Opened by Jack Halpin (FlynDice) - Thursday, 28 May 2009, 07:06 GMT+2
Last edited by Rafaël Carré (funman) - Friday, 26 June 2009, 01:40 GMT+2
Task Type Patches
Category 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

This patch places some cache coherence routines into the dma driver dma-pl081.c. I've been using this for awhile and I believe it works well to have the cache coherence all in one place. Since the mmu is not quite working yet this is a sort of plan for the future that I'm asking for opinions on. Rockbox runs fine with these routines in place and the mmu _not_ enabled. I needed to change some code in ata_sd_as3525.c to pass a size along even though the transfer is peripheral controlled so the *_dcache_range() functions would have proper info. PCM already passes a size so no change is needed there.

I was also wondering if there is a reason we don't use the protection bits in the control register (bits 30:28 p3-23). I realize this is a different caching/buffering than the mmu cacheing but it sounds like it would be advantageous to dma transfers in general.
   dma_cache_coherence.patch (2.4 KiB) 
 firmware/target/arm/as3525/dma-pl081.c     |   16 ++++++++++++++++
 firmware/target/arm/as3525/ata_sd_as3525.c |    4 ++--
 2 files changed, 18 insertions(+), 2 deletions(-)

This task depends upon

Closed by  Rafaël Carré (funman)
Friday, 26 June 2009, 01:40 GMT+2
Reason for closing:  Out of Date
Additional comments about closing:  another solution has been adopted for SD transfers
Comment by Rafaël Carré (funman) - Thursday, 28 May 2009, 13:05 GMT+2
I have put the call to cache coherence routines into ata & pcm driver to avoid an extra check, but I guess this is not really critical to have a comparison at each dma transfer.

However I think using them while we don't have any memory region cached is useless.

Also we don't use protection bits since we run in supervisor mode (no user mode), and we can access all the memory map without restrictions.

I suggest you merge this change into  FS#10048 .
Comment by Rafaël Carré (funman) - Thursday, 28 May 2009, 13:12 GMT+2
By the way I think dump_dcache_range() is more appropriate than invalidate_dcache_range() since it will only write back cache content to memory for the boundaries not aligned on the size of a cache line
Comment by Jack Halpin (FlynDice) - Thursday, 28 May 2009, 15:27 GMT+2
"Also we don't use protection bits since we run in supervisor mode (no user mode), and we can access all the memory map without restrictions."

Yes I understand that part but it appears there are 3 protection bits, 1 for user/supervisor, 1 for bufferable/nonbufferable, and 1 for cacheable/noncacheable that are different and not related to the mmu protection bits but to how the transfer can send data to the AMBA bus.

As far as dump_dcache_range goes, have you had good results with it? I understand why that would be the appropriate choice but when I have actually used it I can't make anything work.
Comment by Rafaël Carré (funman) - Thursday, 28 May 2009, 15:39 GMT+2
If you mean AP, S, and R bits : We can only disable read/write or write support for Supervisor mode. I don't understand how they apply to AMBA bus transfers.

About dump_dcache_range() I can't say I had "good" results since all my tests showed corruption somewhere.

However I still didn't write my results, since I had no comparison. Now I think I could at least compare the display of wps background.
Comment by Jack Halpin (FlynDice) - Thursday, 28 May 2009, 20:24 GMT+2
No, not the AP,S, & R bits - those are mmu related. I'm talking about some DMA controller "protection" bits that would be set in line 105 of dma-pl081.c in the line DMAC_CH_CONTROL(channel) = control; They are 3 bits, 30:28 referenced in the pl081 docs. I'm not really interested in the user/supervisor bit but the other 2 seem interesting. I'll just experiment with them and see what happens, I was just wondering if you knew if that that would be a waste of time.... Here, I'll just paste:

[1] Bufferable or not bufferable Indicates whether the access is bufferable, or not bufferable:
0 = not bufferable
1 = bufferable.
This bit indicates that you can buffer the access. You can, for example, use this bit
to indicate to an AMBA bridge that the read can complete in zero wait states on the
source bus without waiting for it to arbitrate for the destination bus, and for the slave
to accept the data. This bit controls the AHB HPROT[2] signal.

[2] Cacheable or not cacheable Indicates whether the access is cacheable or not cacheable:
0 = not cacheable
1 = cacheable.
This indicates that the access can be cached. You can, for example, use this bit to
indicate to an AMBA bridge that when it sees the first read of a burst of eight, it can
transfer the whole burst of eight reads on the destination bus, rather than pass the
transactions through one at a time. This bit controls the AHB HPROT[3] signal.
Comment by Rafaël Carré (funman) - Thursday, 28 May 2009, 21:59 GMT+2
Ok I was reading the wrong document ;)

I don't know what exactly are these bits , but I just checked the Clip OF and those 3 bits are always set to 0
Comment by Michael Sevakis (MikeS) - Thursday, 28 May 2009, 22:51 GMT+2
If you don't return the uncached physical addresses for peaking, recording in particular can have problems with corruption. Secondly, keyclick for playback will not work well either. I encountered that on Gigabeat S and there is an API in that pcm driver (pcm-pp.c for PortalPlayer too) that gets included with the appropriate #define (no, I don't recall the names atm, but it's easy to spot). Gigabeat S was the first player to use DMA for rec and play *and* have a data cache.

Loading...