rp2040 correct dcd_edpt_iso_activate #2937

pschatzmann · 2025-01-09T15:10:58Z

Describe the PR
I think your last redesign of my iso implementation has introduced an error. The audio is not wokring any more.
This PR resolves the issue.

Additional context
See these comments
#2838

HiFiPhile · 2025-01-15T09:46:36Z

Thank you for the fix, I did a forced push to fix the CI.

It seems strange to add hw_endpoint_init() in dcd_edpt_iso_activate() since it has already been called in dcd_edpt_iso_alloc(), beside packet size is there anything changed between these calls ?

pschatzmann · 2025-01-15T10:49:45Z

I did not double check this: I just added it back because it was also there before the change and removing it was breaking it from working properly.

Daninet · 2025-02-24T20:31:06Z

Applying this patch fixed the issue on my RP2040 with audio.

hathach · 2025-02-25T07:27:44Z

src/portable/raspberrypi/rp2040/dcd_rp2040.c

+  // init w/o allocate
+  const uint16_t mps = ep_desc->wMaxPacketSize;
+  uint16_t size = (uint16_t)tu_div_ceil(mps, 64) * 64u;
+  hw_endpoint_init(ep_addr, size, TUSB_XFER_ISOCHRONOUS);


re-added code only seems to change wMaxPacketSize to the value larger than the one specified in endpoint descriptor which is out of specs. Can anyone

provide the wMaxPacketSize of your endpoint descriptor along with your expected bitrate. Then

try to increase the value in your endpoint descriptor to higher (maybe round up to 64) to see it would work then.

I should note that this patch has saved the day on a big project, as it is the difference between working and not working for me as well.

It seems that the hw_endpoint_init() call is what is missing from the main repo. Adding it back as follows also works.

// New API: Configure and enable an ISO endpoint according to descriptor bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const * ep_desc) { (void) rhport; const uint8_t ep_addr = ep_desc->bEndpointAddress; printf("ep_desc->wMaxPacketSize: %d\r\n",ep_desc->wMaxPacketSize); // init w/o allocate (removed quantize size to 64) hw_endpoint_init(ep_addr, ep_desc->wMaxPacketSize, TUSB_XFER_ISOCHRONOUS); // Fill in endpoint control register with buffer offset struct hw_endpoint* ep = hw_endpoint_get_by_addr(ep_addr); TU_ASSERT(ep->hw_data_buf != NULL); // must be inited and buffer allocated ep->wMaxPacketSize = ep_desc->wMaxPacketSize; hw_endpoint_enable(ep); return true; }

Running this with CFG_TUSB_DEBUG=1 shows the following on some audio playback.
48000, 4 bytes per sample, 24 bits resolution

Mute: 0 ep_desc->wMaxPacketSize: 392 WARN: starting new transfer on already active ep 01 ep_desc->wMaxPacketSize: 4 led_blinking_task: USB Streaming led_blinking_task: USB Mount Clock set current freq: 48000 ep_desc->wMaxPacketSize: 392 WARN: starting new transfer on already active ep 01 ep_desc->wMaxPacketSize: 4 led_blinking_task: USB Streaming WARN: starting new transfer on already active ep 81 led_blinking_task: USB Mount

I'm not sure what the "WARN: starting new transfer on already active ep" is all about.

would it work if you increase the wMaxPacketSize to 448 in endpoint descriptor ? Also if possible please provide LOG=2 output.

Also would it work if you try this snippet instead of the patch

bool dcd_edpt_iso_activate(uint8_t rhport, tusb_desc_endpoint_t const * ep_desc) { (void) rhport; const uint8_t ep_addr = ep_desc->bEndpointAddress; // Fill in endpoint control register with buffer offset struct hw_endpoint* ep = hw_endpoint_get_by_addr(ep_addr); TU_ASSERT(ep->hw_data_buf != NULL); // must be inited and buffer allocated ep->wMaxPacketSize = ep_desc->wMaxPacketSize; *ep->buffer_control = 0; // new code hw_endpoint_reset_transfer(ep); // new code hw_endpoint_enable(ep); return true; }

Using the dcd_edpt_iso_activate() from current main repo, setting ep size to 448 did not change anything. No playback.

Using the snippet above with the hw_endpoint_reset_transfer() call, ep size to 448, WORKS!

Using LOG=2 output, the massive load of output messages breaks the audio playback, here is the LOG=1 output with your new snippet:

ep_desc->wMaxPacketSize: 448 ep_desc->wMaxPacketSize: 4 USB Streaming USB Mount Clock set current freq: 48000 ep_desc->wMaxPacketSize: 448 ep_desc->wMaxPacketSize: 4 USB Streaming USB Mount

Here is the LOG=2 Output at a streaming start event:

USBD Setup Received 01 0B 01 00 01 00 00 00 Set Interface AUDIO control request Set itf: 1 - alt: 1 Open EP 01 with Size = 448 ep_desc->wMaxPacketSize: 448 Clear Stall EP 01 Queue EP 01 with 448 bytes ... Open EP 81 with Size = 4 ep_desc->wMaxPacketSize: 4 Clear Stall EP 81 Set interface 1 alt 1 USB Streaming Queue EP 80 with 0 bytes ... USBD Xfer Complete on EP 84 with 8 bytes HID xfer callback USBD Xfer Complete on EP 80 with 0 bytes USBD Setup Received 01 0B 00 00 01 00 00 00 Set Interface AUDIO control request Set itf: 1 - alt: 0 USB Mount Set interface 1 alt 0 Queue EP 80 with 0 bytes ... Queue EP 84 with 8 bytes ... Queue EP 83 with 20 bytes ... USBD Xfer Complete on EP 80 with 0 bytes USBD Setup Received 21 01 00 01 00 04 04 00 AUDIO control request Queue EP 00 with 4 bytes ... USBD Xfer Complete on EP 83 with 20 bytes HID xfer callback USBD Xfer Complete on EP 84 with 8 bytes HID xfer callback USBD Xfer Complete on EP 00 with 4 bytes 0000: 80 BB 00 00 |....| AUDIO control complete Clock set current freq: 48000 Queue EP 80 with 0 bytes ... Queue EP 84 with 8 bytes ... Queue EP 83 with 20 bytes ... USBD Xfer Complete on EP 80 with 0 bytes USBD Setup Received 01 0B 01 00 01 00 00 00 Set Interface AUDIO control request Set itf: 1 - alt: 1 Open EP 01 with Size = 448 ep_desc->wMaxPacketSize: 448 Clear Stall EP 01 Queue EP 01 with 448 bytes ... Open EP 81 with Size = 4 ep_desc->wMaxPacketSize: 4 Clear Stall EP 81 Set interface 1 alt 1 USB Streaming Queue EP 80 with 0 bytes ... USBD Xfer Complete on EP 83 with 20 bytes HID xfer callback USBD Xfer Complete on EP 84 with 8 bytes HID xfer callback USBD Xfer Complete on EP 80 with 0 bytes USBD Xfer Complete on EP 01 with 384 bytes AUDIO xfer callback Queue EP 01 with 448 bytes ... Queue EP 81 with 4 bytes ... USBD Xfer Complete on EP 01 with 384 bytes AUDIO xfer callback Queue EP 01 with 448 bytes ... USBD Xfer Complete on EP 81 with 4 bytes AUDIO xfer callback Queue EP 81 with 4 bytes ... USBD Xfer Complete on EP 01 with 384 bytes AUDIO xfer callback Queue EP 01 with 448 bytes ... USBD Xfer Complete on EP 81 with 4 bytes AUDIO xfer callback Queue EP 81 with 4 bytes ... USBD Xfer Complete on EP 01 with 384 bytes AUDIO xfer callback Queue EP 01 with 448 bytes ... USBD Xfer Complete on EP 81 with 4 bytes AUDIO xfer callback Queue EP 81 with 4 bytes ... USBD Xfer Complete on EP 01 with 384 bytes AUDIO xfer callback Queue EP 01 with 448 bytes ... USBD Xfer Complete on EP 81 with 4 bytes AUDIO xfer callback Queue EP 81 with 4 bytes ... USBD Xfer Complete on EP 01 with 392 bytes AUDIO xfer callback Queue EP 01 with 448 bytes ... USBD Xfer Complete on EP 81 with 4 bytes AUDIO xfer callback Queue EP 81 with 4 bytes ... USBD Xfer Complete on EP 01 with 392 bytes AUDIO xfer callback Queue EP 01 with 448 bytes ... USBD Xfer Complete on EP 81 with 4 bytes AUDIO xfer callback Queue EP 81 with 4 bytes ... USBD Xfer Complete on EP 01 with 392 bytes AUDIO xfer callback Queue EP 01 with 448 bytes ... USBD Xfer Complete on EP 81 with 4 bytes AUDIO xfer callback Queue EP 81 with 4 bytes ...

I can also confirm that audio is working when setting the ep size back to 392.

In both cases (448 or 392, using the new snippet) the "WARN: starting new transfer on already active ep" warning has gone away as well.

Here's LOG=1 with the ep size at 392

ep_desc->wMaxPacketSize: 392 ep_desc->wMaxPacketSize: 4 USB Streaming USB Mount Clock set current freq: 48000 ep_desc->wMaxPacketSize: 392 ep_desc->wMaxPacketSize: 4 USB Streaming USB Mount

thank you for the detail, I got what is really missing from my testing. I only test with the configuraation wherre it is either streaming or off entirely. Wehereaas you switch to very low bitrate 4 <-> 384 back and forth. Look like the issue happen when switching and the iso transfer is not complete yet (therefore you got the warning previously). Let me revise this, and I think we can update and get this merge.

@FearLabsAudio based on your feedback, I have updated the PR to abort pending PR if it is active. Would you mind to test again simply with your normal packet size = 392 to see if it still works.

Great, audio should be functional on RP2040/2350 again!

I did move the ep size back to 392 and it works fine. I am using the macro in tusb_config.txt:

TUD_AUDIO_EP_SIZE(CFG_TUD_AUDIO_FUNC_1_MAX_SAMPLE_RATE, CFG_TUD_AUDIO_FUNC_1_N_BYTES_PER_SAMPLE_RX, CFG_TUD_AUDIO_FUNC_1_N_CHANNELS_RX)

Which in my case evaluates to 392

thank you for your feedback. @pschatzmann would you mind checking this to see if PR's update work for your setup. Once confirmed we can finally merge this :)

add hw_endpoint_abort_xfer()

hathach

Thank you for your PR, the issue is caused when switching interface when endpoint is transferring data. #2937 (comment) I have updated this PR to abort pending transfer if active and reset and start clean. I will merge this now since I am in the middle of updating TinyUSB Arduino adafruit/Adafruit_TinyUSB_Arduino#498 . Should it still be an issue with you, feel free to open issue/pr

rp2040 correct dcd_edpt_iso_activate

b91d5eb

HiFiPhile force-pushed the rp2040-iso branch from 491fe4c to b91d5eb Compare January 15, 2025 09:27

kilograham mentioned this pull request Feb 4, 2025

SDK 2.1.0 tinyusb panics on composite usb audio and cdc acm device raspberrypi/pico-sdk#2236

Open

hathach reviewed Feb 25, 2025

View reviewed changes

hathach added 2 commits March 5, 2025 22:25

Merge branch 'master' into fork/pschatzmann/rp2040-iso

3ea71b6

rp2040: abort transfer if active in iso_activate()

102c199

add hw_endpoint_abort_xfer()

hathach approved these changes Mar 7, 2025

View reviewed changes

hathach merged commit 334ac80 into hathach:master Mar 7, 2025
108 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rp2040 correct dcd_edpt_iso_activate #2937

rp2040 correct dcd_edpt_iso_activate #2937

pschatzmann commented Jan 9, 2025

HiFiPhile commented Jan 15, 2025 •

edited

Loading

pschatzmann commented Jan 15, 2025

Daninet commented Feb 24, 2025

hathach Feb 25, 2025 •

edited

Loading

FearLabsAudio Mar 4, 2025 •

edited

Loading

hathach Mar 4, 2025 •

edited

Loading

FearLabsAudio Mar 4, 2025

hathach Mar 5, 2025

hathach Mar 5, 2025

FearLabsAudio Mar 5, 2025 •

edited

Loading

hathach Mar 5, 2025

hathach left a comment

rp2040 correct dcd_edpt_iso_activate #2937

rp2040 correct dcd_edpt_iso_activate #2937

Conversation

pschatzmann commented Jan 9, 2025

HiFiPhile commented Jan 15, 2025 • edited Loading

pschatzmann commented Jan 15, 2025

Daninet commented Feb 24, 2025

hathach Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

FearLabsAudio Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

hathach Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

FearLabsAudio Mar 4, 2025

Choose a reason for hiding this comment

hathach Mar 5, 2025

Choose a reason for hiding this comment

hathach Mar 5, 2025

Choose a reason for hiding this comment

FearLabsAudio Mar 5, 2025 • edited Loading

Choose a reason for hiding this comment

hathach Mar 5, 2025

Choose a reason for hiding this comment

hathach left a comment

Choose a reason for hiding this comment

HiFiPhile commented Jan 15, 2025 •

edited

Loading

hathach Feb 25, 2025 •

edited

Loading

FearLabsAudio Mar 4, 2025 •

edited

Loading

hathach Mar 4, 2025 •

edited

Loading

FearLabsAudio Mar 5, 2025 •

edited

Loading