dma: allow choices of CHUNK_SIZE

[liebman]

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

• I have updated existing examples or added new ones (if applicable).
• I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
• My changes were added to the CHANGELOG.md in the proper section.
• My changes are in accordance to the esp-rs API guidelines

Extra:

• I have read the CONTRIBUTING.md guide and followed its instructions.

Pull Request Details 📖

Description

If you need to DMA to/from PSRAM the descriptor buffer size (CHUNK_SIZE) needs to be a multiple of the cache block size. The default value 4092 is 4 byte aligned and not 32 (a common cache block size) or 64 (an allowable cache size). This PR adds features that change the default CHUNK_SIZE to 4064 (32 byte multiple) or 4032 (64 byte multiple).

• dma4064
• dma4032

I'm not sure that this is the best approach, it is the easiest. An alternative would be to make the DmaDescriptor or a DmaDescriptorList be CHUNK_SIZE aware.

Testing

DMA from PSRAM to hub75 panel via LCD peripheral in i8080 mode.

[Dominaezzz]

Instead of using a feature, maybe this could be a parameter to configure/configure_for_async.
Not sure if it should accept a chunk size or alignment value.

NOTE: as far as I am aware the esp32s3 is the only chip that allows DMA to/from PSRAM.

The ESP32-S2 (and the upcoming P4) also allow DMA to/from PSRAM.

[liebman]

Instead of using a feature, maybe this could be a parameter to configure/configure_for_async.

One usability issue there is that the user also has to allocate the descriptors with the same (or less) chunk size to be sure they won't run short of descriptors and panic when they attempt a transfer. (been there done that)

I think there needs to be some discussion on how descriptors should be presented. I've not tried but have been thinking something like a DescriptorList with new(size: usize, chunk_size: usize) that could use alloc (and a DescriptorListStatic<const N: usize, const C: usize> for a static allocation, both would implement traits for filling that would abide the chunk size.

[Dominaezzz]

Yeah the usability is a bit of an issue there. (Though I think features have the same problem)

I think there needs to be some discussion on how descriptors should be presented. I've not tried but have been thinking something like a DescriptorList with new(size: usize, chunk_size: usize) that could use alloc (and a DescriptorListStatic<const N: usize, const C: usize> for a static allocation, both would implement traits for filling that would abide the chunk size.

I'm currently working on something similar in #1856, the difference is the structs I've introduced contain both a descriptor and a buffer together. Eventually I was gonna throw in a DmaPsramBuf (or something like that) that would check for alignment, cache coherence, etc.

[jessebraham]

Sorry for letting this fall off the radar for so long.

I think the functionality being added is good, however I'm not sure that creating new features is the correct path forward. I think this boils down to yet another thing that is blocked by #1111, so hopefully we can make some progress there soon.

@MabezDev @bjoernQ please let me know if you disagree with this assessment.

[bjoernQ]

I'd say sooner or later we definitely want users to have more control over the descriptors including the chunk-size.

I agree a feature is easy to implement and somewhat easy to use. Downside of a feature is that this CHUNK_SIZE is then used everywhere (not a new downside - it's like that now - users just don't have a choice)

Maybe making CHUNK_SIZE a DEFAULT_CHUNK_SIZE and make the macros to allocate buffers and descriptors optionally take a chunk-size would mitigate the annoyance of having the chunk size a parameter

[liebman]

Maybe making CHUNK_SIZE a DEFAULT_CHUNK_SIZE and make the macros to allocate buffers and descriptors optionally take a chunk-size would mitigate the annoyance of having the chunk size a parameter

I like this - I'll rework the PR (or close this one and create a new one)

[jessebraham]

Just converting this to a draft until the rework is done, no rush of course :)