Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Drivers
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.1
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Frank Gevaerts - 2009-03-12
Last edited by Frank Gevaerts - 2009-03-29

FS#10015 - Use chained transfer descriptor to speed up USB transfers on PP and iMX31

Use chained transfer descriptors to allow larger blocks of data in a single transfer. This speeds up USB transfers significantly.

Closed by  Frank Gevaerts
2009-03-29 19:59
Reason for closing:  Accepted
Additional comments about closing:  

Committed as r20570

Admin
Frank Gevaerts commented on 2009-03-12 21:11

This crashes on gigabeat S for some reason. Taking out some error handling (that I’ve never seen triggered) helps. The problem is that this also takes size of transfered data, which doesn’t matter for storage, but it does for e.g. serial. This means that this patch, while good for testing, isn’t ready for commit.

Boris Gjenero commented on 2009-03-12 22:08

It seems like each transfer descriptor can handle 20K. Why are they being limited to 16K?

Admin
Frank Gevaerts commented on 2009-03-12 22:13

They can handle 20K, if the data is 4K-aligned. If not, you can only get 16K (or actually, you can get whatever fits. Just assuming 16K is safe and simple)

Przemysław Hołubowski commented on 2009-03-13 09:37

I have tested the patch on H10-20 with 60GB HDD using UDMA1 mode.

USB wite (PC→H10) speed is 9.4MB/s (about 1.5MB/s faster then using OF) without the patch and about 11.2MB/s with the patch, and USB read speed is 8MB/s (OF attains 14-18MB/s) no matter the patch is applied or not.

Tom commented on 2009-03-14 02:27

Tested on my e250v1.
3.1mb/s write, 6.1mb/s read without patch.
3.3mb/s write, 6.1mb/s read with patch.

MichaelGiacomelli commented on 2009-03-14 04:46

I tried chained-tds-2.diff on my PC that had disconnects with Rockbox’s USB mode and I saw a pretty big improvement. With this patch I didn’t get a disconnect until over 5GB had been transfered. Without it I typically got one within a couple hundred MB, with 500MB being relatively rare.

Andree Buschmann commented on 2009-03-14 13:29

Tested chained-tds-2.diff with r20323 and  FS#9708  v0.6 (UDMA1 and UDMA4) on my 5.5G.

Keeping the CPU at 80MHz I get:
PIO4: 8100 KB/s write, 4700 KB/s read
UDMA1: 7100 KB/s write, 9400 KB/s read
UDMA4: 7100 KB/s write, 11300 KB/s read

When setting the CPU to 100MHz I get:
UDMA1: 8100 KB/s write, 9400 KB/s read
UDMA4: 8100 KB/s write, 11300 KB/s read

Edit: Oops, corrected read/write.

Admin
Frank Gevaerts commented on 2009-03-29 16:44

This one adds proper length handling, but it gets data aborts on gigabeat S at 0x59a18 (see the attached elf file for context)

Edit: I forgot the buffer size change to usb_storage in this one. It needs that too of course to get proper speed benefits

Toni commented on 2009-03-29 19:06

The data abort seems to happen while accessing td→reserved in:

while(td!=QH_NEXT_TERMINATE)
{
length += 1);

td=td->next_td_ptr;

}

By the way, do ‘transfer_descriptor’ and ‘queue_head’ need to be byte packed? If the related struct members are accessed often, there might be a performance loss.

1) td→reserved & DTD_RESERVED_LENGTH_MASK) -
((td→size_ioc_sts & DTD_PACKET_SIZE) » DTD_LENGTH_BIT_POS
Admin
Frank Gevaerts commented on 2009-03-29 19:31

I don’t really see how this can happen, as td→reserved should be nicely aligned. I’ll check if it never jumps to an invalid location.

transfer_descriptor and queue_head are used directly by the hardware, so they need to be precisely packed and aligned. They only consist of 4-byte int fields though, so I don’t think it should make any difference

Admin
Frank Gevaerts commented on 2009-03-29 19:57

It was a bad pointer. Fixed now (and committed)

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing