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

[DCD_DWC2][ESP32P4][HS] Added cache synchronization (cont) #2883

Merged
merged 5 commits into from
Nov 20, 2024

Conversation

hathach
Copy link
Owner

@hathach hathach commented Nov 20, 2024

Describe the PR
Superceded #2877 since I need to make some modification to have CFG_TUSB_MEM_ALIGN=__attribute__((aligned(64))) for p4 and add generic CFG for CFG_TUD_MEM_DCACHE_ENABLE/CFG_TUD_MEM_DCACHE_LINE_SIZE

if (target STREQUAL esp32p4)
# P4 change alignment to 64 (DCache line size) for possible DMA configuration
list(APPEND compile_definitions
CFG_TUSB_MEM_ALIGN=__attribute__\(\(aligned\(64\)\)\)
Copy link
Owner Author

Choose a reason for hiding this comment

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

CFG_TUSB_MEM_ALIGN will cause class driver to put correct aligment on usb/dma buffer.

Copy link
Owner Author

@hathach hathach Nov 21, 2024

Choose a reason for hiding this comment

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

@roma-jam I just realized that we probably does not need the dma buffer to be 64 bytes aligned ? It only need to occupies the whole cache line (64 byte) right ? If you we can change it back to simply 4 bytes alignment.

PS: nevermind, I try to reduce alignment to 4 and it does not work, so 64 alignment is requierd

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately the cache line size is configurable, and can be 64 or 128 bytes :(
This might be good enough for now, just so you keep this in mind if someone reports issues...

Copy link
Owner Author

Choose a reason for hiding this comment

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

what is the default value ? it isn't an problem at all. both the CFG_TUSB_MEM_ALIGN and CFG_TUD_MEM_DCACHE_LINE_SIZE is configurable at user level (tusb_config.h). So if use change it to 128, they shoud set this accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

default is 64. OK, great!

#define CFG_TUD_MEM_DCACHE_ENABLE_DEFAULT 0
#endif

#define CFG_TUD_MEM_DCACHE_ENABLE CFG_TUD_MEM_DCACHE_ENABLE_DEFAULT
Copy link
Owner Author

Choose a reason for hiding this comment

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

@HiFiPhile CFG_TUD_MEM_DCACHE_ENABLE_DEFAULT is set to 1 when CFG_TUH_DWC2_DMA_ENABLE is enabled for required port such as P4. It is overwritable from tusb_config.h if user want to use MMU/PMA to make it non-cacheable.

#endif

#ifndef CFG_TUD_MEM_DCACHE_LINE_SIZE
#define CFG_TUD_MEM_DCACHE_LINE_SIZE 32
Copy link
Owner Author

Choose a reason for hiding this comment

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

should work with most ARM core like M7

CFG_TUD_MEM_ALIGN union {
uint32_t setup_packet[2];
#if CFG_TUD_MEM_DCACHE_ENABLE
uint8_t setup_packet_cache_padding[CFG_TUD_MEM_DCACHE_LINE_SIZE];
Copy link
Owner Author

Choose a reason for hiding this comment

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

@roma-jam we will need to do this to all buffered in the class driver, which may take a couple of follow-up PRs.

TU_LOG(DWC2_DEBUG, " TX FIFO %u: allocated %u words at offset %u\r\n", epnum, fifo_size, _dfifo_top);
TU_ASSERT(_dcd_data.dfifo_top >= fifo_size + dwc2->grxfsiz);
_dcd_data.dfifo_top -= fifo_size;
// TU_LOG(DWC2_DEBUG, " TX FIFO %u: allocated %u words at offset %u\r\n", epnum, fifo_size, dfifo_top);

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
@hathach hathach merged commit 2571889 into master Nov 20, 2024
122 of 169 checks passed
@hathach hathach deleted the feature/esp32p4_dma_cache_syncronization branch November 20, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants