Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESP32-S3 dwc2 driver broken, FIFO size limited to 1/4 #2993

Closed
1 task done
dzingoni opened this issue Feb 18, 2025 · 4 comments
Closed
1 task done

ESP32-S3 dwc2 driver broken, FIFO size limited to 1/4 #2993

dzingoni opened this issue Feb 18, 2025 · 4 comments
Labels

Comments

@dzingoni
Copy link

Operating System

MacOS

Board

ESP32-s3 devkit

Firmware

Any firmware using ESP32-s3 usb device, I worked on usb audio.

What happened ?

Look like there is a bug in calculation of available FIFO RAM for esp32-s3 (and also for P4 as I can see).

In file dwc2_esp32.h we can find this definition:
static const dwc2_controller_t _dwc2_controller[] = {
{ .reg_base = DWC2_FS_REG_BASE, .irqnum = ETS_USB_INTR_SOURCE, .ep_count = 7, .ep_in_count = 5, .ep_fifo_size = 1024 }
};

where we see that ep_fifo_size is 1024. These are words, not bytes, as far as I know.

Now when the dwc2 driver uses this number in the routine (file dcd_dwc2.c):

static void dfifo_device_init(uint8_t rhport) {
const dwc2_controller_t* dwc2_controller = &_dwc2_controller[rhport];
dwc2_regs_t* dwc2 = DWC2_REG(rhport);
dwc2->grxfsiz = calc_device_grxfsiz(CFG_TUD_ENDPOINT0_SIZE, dwc2_controller->ep_count);

// Scatter/Gather DMA mode is not yet supported. Buffer DMA only need 1 words per endpoint direction
const bool is_dma = dma_device_enabled(dwc2);
_dcd_data.dfifo_top = dwc2_controller->ep_fifo_size/4;
if (is_dma) {

...
we can see that ep_fifo_size is divided by 4, and so _dcd_data.dfifo_top is 256 (words) and not 1024.

Now with 256 words it's impossible to go over 16 bit stereo 48000Hz with IN and OUT. 32bit is impossible and generates an ASSERT.
Even 24 bit stereo playback only at 96Khz is impossible.

From what I know the 1024 ep_fifo_size is in WORDS, not BYTES. So we should either change it to 4096 (in bytes) or remove the /4 division.

How to reproduce ?

Just try to run the uac2_headset example and set it to 48Khz, 32 bit x sample, stereo (2 channels out, 2 channels in).

Debug Log as txt file (LOG/CFG_TUSB_DEBUG=2)

No need to debug, to see the asserts just set the debugg level to 3 in tusb_config.h

Screenshots

No response

I have checked existing issues, dicussion and documentation

  • I confirm I have checked existing issues, dicussion and documentation.
@hathach
Copy link
Owner

hathach commented Feb 18, 2025

value is already correct

@hathach hathach closed this as completed Feb 18, 2025
@dzingoni
Copy link
Author

I understand that the topic has already been examined, and that the usage of a common DWC2 driver seems the best to limit effort, but in file dcd_esp32sx.c it's clear that things are different:

- first there is the following:
// FIFO size in bytes
#define EP_FIFO_SIZE 1024

so it's clear that 1024 is in bytes

- then this map:
// "USB Data FIFOs" section in reference manual
// Peripheral FIFO architecture
//
// --------------- 320 or 1024 ( 1280 or 4096 bytes )
// | IN FIFO MAX |
// ---------------
// | ... |
// --------------- y + x + 16 + GRXFSIZ
// | IN FIFO 2 |
// --------------- x + 16 + GRXFSIZ
// | IN FIFO 1 |
// --------------- 16 + GRXFSIZ
// | IN FIFO 0 |
// --------------- GRXFSIZ
// | OUT FIFO |
// | ( Shared ) |
// --------------- 0
//
SO it's also clear that we have 1280 or 4096 bytes The DWC2 has a different way to allocate the fifos, but the problem is that if we consider to have only 256 words and not 1024 for buffers on the esp32s3 the usb is very limited.

I do believe that this driver (dcd_esp32sx.c) actually behaves different from the DWC2.
I tried to modify the code of DWC2 so that 1024 are words and not bytes, and the driver actually works (at least for FIFO allocation and basic functionalities, ... but with different problems, maybe this change alone is not enoughand there no sufficient documentation about internals ...).

Can anybody tell me if it's possible to use dcd_esp32sx.c for esp32s3 instead of the DWC2 driver?

@HiFiPhile
Copy link
Collaborator

if we consider to have only 256 words and not 1024 for buffers on the esp32s3 the usb is very limited.

No, ESP32-S3 has only 256 word or 1024 bytes for USB FIFO, the value 4096 is for STM32 who has a bigger FIFO.

From reference manual:

Image

@dzingoni
Copy link
Author

OK, thank you , I had seen this ... and it's incoerent with the image in the technical reference ...

Image

Moreover I was examing the fork of Tinyusb by expressif ...
https://github.com/espressif/tinyusb/blob/82a7a644ccbe5a38c725517ca3baf0703f90b2fb/src/portable/espressif/esp32sx/dcd_esp32sx.c

I don't have access to more detailed info, but trying to understand the code and reasoning on everything I believe that 1024 is the limit for each FIFO, and not global. From the image I'd bet also that different endpoints should have fifos at specific locations in the memory map (1000, 2000 and so on ) ... but again, I miss any other technical doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants