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

Remove 4096 limit for SPI dma transfer size #377

Closed
Kezii opened this issue Feb 10, 2024 · 2 comments · Fixed by #379
Closed

Remove 4096 limit for SPI dma transfer size #377

Kezii opened this issue Feb 10, 2024 · 2 comments · Fixed by #379

Comments

@Kezii
Copy link
Contributor

Kezii commented Feb 10, 2024

Why is there a limit on the transfer size for dma?

x if x > 4096 => panic!("The max transfer size must be less than or equal to 4096"),

The esp32-s3 can supposedly go up to 32768 and previous models even higher, as referenced on espressif/esp-idf#7804

Also, the documentation doesn't mention this number, it says, for max_transfer_sz

"Maximum transfer size, in bytes. Defaults to 4092 if 0 when DMA enabled, or to SOC_SPI_MAXIMUM_BUFFER_SIZE if DMA is disabled. "

4092 is the default, and the number is wrong too

The 4096 limit is arbitrary, and should be removed

@Kezii Kezii changed the title Remove 4096 limit for dma transfer size Remove 4096 limit for SPI dma transfer size Feb 10, 2024
@Vollbrecht
Copy link
Collaborator

You are correct that this limit is set arbitrary and may not make much sense. Though your assessments is about the 32768 is also not quite right as one has to think differently between actuall dma_descriptor lengths and what the C driver allows you to put a buffer into it. We had a discussion around it in the matrix channel some time ago. But in short, just removing it doesn't make the overall situation better.

What we would need is:
a) For every esp variant a verified dma transfer with the driver given arbitrary length buffers boxed on the heap for testing, and look what work and what doesn't work.
b) check how the c spi_driver constructs the dma linked list out of the buffer we give him, and verify or deny any possible limit from the input buffer perspective.
c) Find out about possible penalty of non aligned buffers with respect to the dma descriptors itself.

Based on that testing we could make a decision how to move forward correctly. So if you want to tackle the issue this are the first steps to dive into.

@Kezii
Copy link
Contributor Author

Kezii commented Feb 12, 2024

Isn't it wiser to just let the underlying esp-idf handle this?

Right now it panics at runtime and it's not that much different than returning an error (what idf does when you remove the limit)

What you are saying is probably better but looks like a lot of work and a lot of maintenance, don't let perfect be the enemy of good

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

Successfully merging a pull request may close this issue.

2 participants