RP2040: track DPRAM buffer for each endpoint

[howard0su]

Describe the PR Running audio example on pico, I am still hitting the following assert: hard_assert(hw_data_offset(next_buffer_ptr) <= USB_DPRAM_MAX);

Additional context The issue is that pico implementation only reclaim the memory when all the endpoints are closed. In audio example, speaker and mic may not share the same lifecycle. This end up the crash.

I fixed this to implement a poor man allocation algorithm to track 64 of the blocks with 64 bytes. Since there are only 64 blocks, the simple algorithm works well.

Tested on Pico with UAC2 example

[kilograham]

this is a good idea; but i'd use a single uint64_t to do the tracking and some basic bit ops (you are using 8x the memory and don't seem to make use of the difference between 0xff and the item start) other than for an assertion.

[howard0su]

Update the code change to use uint64_t to track the allocation status. Calculate allocation size when free. Put the size calculation into a new static function to make sure we have the same logic when allocating and free.

[howard0su]

@kilograham can you approve the workflow? so that I can verify build

[howard0su]

please merge.

[kilograham]

i pushed a change to change optimization to -Os for hw_endpoint_init which was pointlessly being generated 3 times.

[howard0su]

Did you measure the code size difference? I believe RAM difference is only 8bytes. On RPI, it is so small to ignore.

[hathach]

hihi, thank you @howard0su for the PR, and sorry for late response. Recently, we have added a new API for ISO transfer, that would reserve the largest packet size before SET_INTERFACE (activation). This will help to solve the changing endpoint size of ISO transfer.

https://github.com/hathach/tinyusb/blob/master/src/device/dcd.h#L172-L175

The API is currently only implemented with stm32 fsdev, but will be added for rp2040 later as well when I have time. I appreciate your effort but I think reserving largest size (with endpoint with multiple size) work best as generic for most ports (some mcu has difficulty managing usb ram).

[pschatzmann]

Could you please move ahead with this merge and implement your proposed improvement later. W/o this, audio is completley broken when using it together with CDC!

I would like to provide a working Audio implementation with examples for Adafruit_TinyUSB_Arduino

[kholia]

+1 for merging this patch.

I am maintaining and using a local copy of this patch and it works brilliantly.

Without this patch, https://github.com/kholia/Global-Communications-Transceiver and other radio firmwares crash very soon. With this patch applied, the radio firmware runs 'forever' (1 week plus and counting).

Thanks @hathach for considering this PR <3

...

PS: This patch is powering my main HF rig now.

[pschatzmann]

I think you will need to update it to make sure that there are not compile warnings. I needed to comment out some defines

//#define usb_hw_set hw_set_alias(usb_hw)
//#define usb_hw_clear hw_clear_alias(usb_hw)

when I used it in Adafruit_TinyUSB_Arduino

[howard0su]

Really old patch. give me some time to setup the environment and recall the memory of this.

[HiFiPhile]

Hi,

As @hathach said last year, many things has been changed since this PR, notably the introduce of dcd_edpt_iso_alloc() and `dcd_edpt_iso_activate() to facilitate ISO endpoint FIFO management.

There is no more need to track the FIFO usage of each endpoints since the audio (also video) class will claim the maximum FIFO needed with dcd_edpt_iso_alloc() during initialization. Then dcd_edpt_iso_activate() is called instead of dcd_edpt_open() when the endpoint is opened. So only one index tracking the total RAM usage is enough.

You can check DWC2 driver for example: https://github.com/hathach/tinyusb/blob/a4fb8354e41b2604f66e7da22afa292b1a44f0a2/src/portable/synopsys/dwc2/dcd_dwc2.c#L849 https://github.com/hathach/tinyusb/blob/a4fb8354e41b2604f66e7da22afa292b1a44f0a2/src/portable/synopsys/dwc2/dcd_dwc2.c#L854

Sorry I can't review this since I don't use RP2040.

[hathach]

@HiFiPhile yeah, we should update the rp2040 driver, I will do that later on, after the hcd for dwc2 (WIP).

[pschatzmann]

Following the comments from HiFiPhile, I tried to extend the existing driver with dcd_edpt_iso_alloc() and dcd_edpt_iso_activate()

I tested it in Arduino with my audio scenarios. What would be the process to move forward ?

[hathach]

closed since a better approach with iso allocate/active is implemented https://github.com/hathach/tinyusb/pull/2847