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

feature(esp_tinyusb): Added mode configuration for dcd_dwc2 layer #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roma-jam
Copy link
Collaborator

@roma-jam roma-jam commented Nov 18, 2024

esp_tinyusb component, v1.5.0

Description

DMA support for TinyUSB device stack.

DMA (Buffer mode) is a default mode for device, meanwhile there is an option to change the mode to Single/IRQ by disabling DMA mode via menuconfig.

If (by any reason) hardware isn't support DMA (arch != GHWCFG2_ARCH_INTERNAL_DMA) then the Single/IRQ mode will be used.

Related

Testing

This PR is already compatible with the branch: https://github.com/espressif/tinyusb/tree/release/v0.17

Measurements

Measurements were made on tusb_msc example (read-write operations benchmark, SDMMC target).

ESP32S3

MSC Buffer size (bytes) Slave Mode (R/W) Buffer DMA (R/W)
512 503.4 kB/s / 187.2 kB/s 509.8 kB/s / 186.2 kB/s
8192 N/A / N/A 920.6 kB/s / 612.9 kB/s

ESP32P4

MSC Buffer size (bytes) Slave Mode (R/W) Buffer DMA (R/W)
512 976.3 kB/s / 232.7 kB/s 1.0 MB/s / 234.6 kB/s
8192* 999.4 kB/s / 237.6 kB/s 5,3 MB/s / 1,8 MB/s

*Benchmark results provided

image

image


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

@roma-jam
Copy link
Collaborator Author

@tore-espressif @peter-marcisovsky

This is the configuration changes that should be done to enable DMA on S2/S3 and P4.
Cache sync implementation are already in the upstream and our branch release/v0.17 was synchronized with the master.

So, basically, we are almost there. So the steps will be:

  • Merge this PR
  • Make a tag in the espressif/tinyusb to release the first version based on the v0.17 upstream
  • Fix the NCM class driver on S2/S3 (update the esp_tinyusb wrapper).

PTAL and share your thoughts.

Copy link
Collaborator

@peter-marcisovsky peter-marcisovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the update. LGTM.

device/esp_tinyusb/Kconfig Outdated Show resolved Hide resolved
device/esp_tinyusb/include/tusb_config.h Outdated Show resolved Hide resolved
@@ -1,7 +1,7 @@
## IDF Component Manager Manifest File
description: Espressif's additions to TinyUSB
documentation: "https://docs.espressif.com/projects/esp-idf/en/latest/esp32s2/api-reference/peripherals/usb_device.html"
version: "1.4.5"
version: "1.5.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think relase 1.5.0 should be tight to TinyUSB upgrade to v0.17.0~0.

The cache sync and DMA will not work with older versions anyways

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two ways:

  • We can publish this changes and they will work with current version based on v0.15, but only in a Single IRQ mode.
  • After tinyusb will be published, it will start to use DMA

@@ -1,3 +1,7 @@
## 1.5.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep us 'Unreleased' for now

# define CFG_TUD_MEM_DCACHE_ENABLE 1
#define CFG_TUD_MEM_DCACHE_LINE_SIZE CONFIG_CACHE_L1_CACHE_LINE_SIZE // TODO: return back after #2884
// NOTE: starting with esp-idf v5.3 there is specific attribute present: DRAM_DMA_ALIGNED_ATTR
# define CFG_TUSB_MEM_SECTION __attribute__((aligned(CONFIG_CACHE_L1_CACHE_LINE_SIZE))) DRAM_ATTR
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On P4 it does not have to be in DRAM, does it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took it from DRAM_DMA_ALIGNED_ATTR.
So, it does.

// ------------------------------------------------------------------------
#ifdef CONFIG_TINYUSB_MODE_DMA
// DMA Mode has a priority over Single/IRQ mode and will be used if hardware supports it
#define CFG_TUD_DWC2_DMA_ENABLE 1 // Enable DMA
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have CFG_TUD_DWC2_SLAVE_ENABLE and CFG_TUD_DWC2_DMA_ENABLE enabled at the same time?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If (by any reason) hardware isn't support DMA (arch != GHWCFG2_ARCH_INTERNAL_DMA) then the Single/IRQ mode will be used, as it will be selected dynamically.

Which is, IMHO, preferable way.
But (what I don't like) there will be no obvious messages about mode in the debug or any other output. Which could be tricky.

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

Successfully merging this pull request may close these issues.

3 participants