Implement DMA to/from psram on esp32s3

[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

Implement DMA access to psram on the esp32s3 (TRM says its supported)

TODO:

• implement errata for cache_writeback_addr()
• enforce cacheline alignment for address and sizes for transfers to/from psram
• find the correct location for calling cache_writeback_addr()
• find the correct location for calling cache_invalidate_addr()
• add error checking for buffer alignment and size requirements
• should we provide allocation macros for dma buffers and alignment restrictions?
• confirm I do not need to call writeback/invalidate for internal ram addresses

Testing

Run dma_extmem2mem example

[liebman]

Currently running the example results with garbage in the receive buffer. :-(

I'm not getting errors reported in the interrupt raw registers and from the other registers all of the dma descriptors were processed.

Is it possible that we don't have the psram configured correctly for this to work?

[bjoernQ]

Is it possible that we don't have the psram configured correctly for this to work?

I'm not aware there is something special to do for DMA - haven't seen anything related in esp-idf

[Dominaezzz]

I haven't looked at the code yet but make sure to flush the cache before triggering DMA to read from PSRAM.

EDIT: Cache_WriteBack_Addr is the rom function you're looking for.

[liebman]

I found this in esp-idf, note that ESP_ROM_HAS_CACHE_WRITEBACK_BUG is defined for esp32s3

https://github.com/espressif/esp-idf/blob/master/components/esp_rom/patches/esp_rom_cache_esp32s2_esp32s3.c#L92-L102

[liebman]

• added cache_writeback_addr() that calls Cache_WriteBack_Addr() with TODO for the errata
• called cache_writeback_addr() in `Mem2Mem::start_transfer () for simplicity with a TODO that it's in the wrong place
• updated the example to copy from psram to internal ram AND internal ram to psram

Currently using psram as the source works (!) however using psram as the destination does not, nothing was written to psram.

[liebman]

So now transfers work both directions.

• If the source is psram call WriteBack
• if the destination is psram call Invalidate

[liebman]

I have questions about the soundness of the fix used in esp-idf. The fix is all about transfers whose start and/or end are a "partial cache line". Meaning that some memory in the cache line subject to the writeback is not owned by the caller. From my understanding there is still a short period of time after the writeback was completed where the other core, or an interrupt handler could write to that adjacent memory. The only safe way I see to implement this is to only allow DMA to/from psram where the start and size are both aligned to the size of the cache line.

[Dominaezzz]

I was wondering about this before you commented but it might be best to leave to flushing/invalidating to the caller. I imagine some folks might not even be using the cache and might only access PSRAM via DMA.

size of the cache line.

Do you happen to know how big this is?

[liebman]

size of the cache line.

Do you happen to know how big this is?

Not off-hand but for esp32 its 32 bytes and I think there is a rom function for it on the esp32s3. (https://esp32.com/viewtopic.php?t=33059)

Also DMA to psram already requires a block size alignment of 16/32/64 bytes (selectable). So requiring cachline alignment may not be an additional burden. Maybe have a with_cache_management(value: bool) or set_cache_management(value: bool) add it to the ChannelCreator::configure parameters?

and the cache line can be set to 16/32/64 bytes (from the TRM)

Cache_Get_DCache_Line_Size() says 32 on the esp32s3

[liebman]

This (#954) is related. In this PR I've enforced that the buffers and sizes be cacheline (block) aligned.

[bjoernQ]

Code looks good to me and the example is working fine!

Maybe the alignment restrictions need some documentation - I guess it's hard to figure them out as a user otherwise

[liebman]

Code looks good to me and the example is working fine!

Maybe the alignment restrictions need some documentation - I guess it's hard to figure them out as a user otherwise

Added a note

[bjoernQ]

LGTM - Thanks

[JurajSadel]

LGTM, I left a few nitpick typo suggestions in docs.

[jessebraham]

Thanks, LGTM!