Support PSRAM in DmaTxBuf (#2161)

* support psram in DmaTxBuf

* add example that sometimes works :-(

* fmt

* cleanups

* allow chunk_size upto (including) 4095

* this test is passing for me now

* remove chunk_size and compute based on block_size

* return error in `prepare_transfer` if psram is found on non-esp32s3
add `dma_tx_buffer` macro

* missing parens

* changelog

* default 4092 for esp32 & fmt

* no errors anymode

* use block_size is_some to flag invalid psram in prepare_transfer

* drop block_size from macro, the buffer allocation was not being aligned - its not needed for dram anyway.

* missed macro example

* use defmt::Format that decodes owner like Debug

* fix typo

* DmaTxBuf: its an error if buffer is in psram and block_size is none

* DmaTxBuf: its an error if buffer is in psram and block_size is none

* update for PSRAM feature changes

* address alignment comments
add simple test

* fmt

* better alignment test

* revert alignment test

---------

Co-authored-by: Juraj Sadel <[email protected]>

esp-hal/CHANGELOG.md

@@ -53,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Allow handling interrupts while trying to lock critical section on multi-core chips. (#2197)
 - Removed the PS-RAM related features, replaced by `quad-psram`/`octal-psram`, `init_psram` takes a configuration parameter, it's now possible to auto-detect PS-RAM size (#2178)
 - `EspTwaiFrame` constructors now accept any type that converts into `esp_hal::twai::Id` (#2207)
+- Change `DmaTxBuf` to support PSRAM on `esp32s3` (#2161)
 
 ### Fixed
 

esp-hal/src/dma/mod.rs

@@ -205,7 +205,6 @@ where
 bitfield::bitfield! {
     #[doc(hidden)]
     #[derive(Clone, Copy)]
-    #[cfg_attr(feature = "defmt", derive(defmt::Format))]
     pub struct DmaDescriptorFlags(u32);
 
     u16;
@@ -226,6 +225,20 @@ impl Debug for DmaDescriptorFlags {
     }
 }
 
+#[cfg(feature = "defmt")]
+impl defmt::Format for DmaDescriptorFlags {
+    fn format(&self, fmt: defmt::Formatter<'_>) {
+        defmt::write!(
+            fmt,
+            "DmaDescriptorFlags {{ size: {}, length: {}, suc_eof: {}, owner: {} }}",
+            self.size(),
+            self.length(),
+            self.suc_eof(),
+            if self.owner() { "DMA" } else { "CPU" }
+        );
+    }
+}
+
 /// A DMA transfer descriptor.
 #[derive(Clone, Copy, Debug)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
@@ -286,6 +299,8 @@ use enumset::{EnumSet, EnumSetType};
 pub use self::gdma::*;
 #[cfg(pdma)]
 pub use self::pdma::*;
+#[cfg(esp32s3)]
+use crate::soc::is_slice_in_psram;
 use crate::{interrupt::InterruptHandler, soc::is_slice_in_dram, Mode};
 
 #[cfg(gdma)]
@@ -558,7 +573,7 @@ macro_rules! dma_circular_buffers_chunk_size {
 macro_rules! dma_descriptors_chunk_size {
     ($rx_size:expr, $tx_size:expr, $chunk_size:expr) => {{
         // these will check for size at compile time
-        const _: () = ::core::assert!($chunk_size <= 4092, "chunk size must be <= 4092");
+        const _: () = ::core::assert!($chunk_size <= 4095, "chunk size must be <= 4095");
         const _: () = ::core::assert!($chunk_size > 0, "chunk size must be > 0");
 
         static mut RX_DESCRIPTORS: [$crate::dma::DmaDescriptor;
@@ -593,7 +608,7 @@ macro_rules! dma_descriptors_chunk_size {
 macro_rules! dma_circular_descriptors_chunk_size {
     ($rx_size:expr, $tx_size:expr, $chunk_size:expr) => {{
         // these will check for size at compile time
-        const _: () = ::core::assert!($chunk_size <= 4092, "chunk size must be <= 4092");
+        const _: () = ::core::assert!($chunk_size <= 4095, "chunk size must be <= 4095");
         const _: () = ::core::assert!($chunk_size > 0, "chunk size must be > 0");
 
         const rx_descriptor_len: usize = if $rx_size > $chunk_size * 2 {
@@ -620,6 +635,33 @@ macro_rules! dma_circular_descriptors_chunk_size {
     };
 }
 
+/// Convenience macro to create a DmaTxBuf from buffer size. The buffer and
+/// descriptors are statically allocated and used to create the `DmaTxBuf`.
+///
+/// ## Usage
+/// ```rust,no_run
+#[doc = crate::before_snippet!()]
+/// use esp_hal::dma_tx_buffer;
+/// use esp_hal::dma::DmaBufBlkSize;
+///
+/// let tx_buf =
+///     dma_tx_buffer!(32000);
+/// # }
+/// ```
+#[macro_export]
+macro_rules! dma_tx_buffer {
+    ($tx_size:expr) => {{
+        const TX_DESCRIPTOR_LEN: usize =
+            $crate::dma::DmaTxBuf::compute_descriptor_count($tx_size, None);
+        $crate::declare_aligned_dma_buffer!(TX_BUFFER, $tx_size);
+        static mut TX_DESCRIPTORS: [$crate::dma::DmaDescriptor; TX_DESCRIPTOR_LEN] =
+            [$crate::dma::DmaDescriptor::EMPTY; TX_DESCRIPTOR_LEN];
+        let tx_buffer = $crate::as_mut_byte_array!(TX_BUFFER, $tx_size);
+        let tx_descriptors = unsafe { &mut TX_DESCRIPTORS };
+        $crate::dma::DmaTxBuf::new(tx_descriptors, tx_buffer)
+    }};
+}
+
 /// DMA Errors
 #[derive(Debug, Clone, Copy, PartialEq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
@@ -1001,6 +1043,16 @@ pub enum DmaExtMemBKSize {
     Size64 = 2,
 }
 
+impl From<DmaBufBlkSize> for DmaExtMemBKSize {
+    fn from(size: DmaBufBlkSize) -> Self {
+        match size {
+            DmaBufBlkSize::Size16 => DmaExtMemBKSize::Size16,
+            DmaBufBlkSize::Size32 => DmaExtMemBKSize::Size32,
+            DmaBufBlkSize::Size64 => DmaExtMemBKSize::Size64,
+        }
+    }
+}
+
 pub(crate) struct TxCircularState {
     write_offset: usize,
     write_descr_ptr: *mut DmaDescriptor,
@@ -1417,7 +1469,6 @@ where
                 if des.buffer as usize % alignment != 0 && des.size() % alignment != 0 {
                     return Err(DmaError::InvalidAlignment);
                 }
-                // TODO: make this optional?
                 crate::soc::cache_invalidate_addr(des.buffer as u32, des.size() as u32);
             }
         }
@@ -1635,6 +1686,7 @@ where
         peri: DmaPeripheral,
         chain: &DescriptorChain,
     ) -> Result<(), DmaError> {
+        // TODO: based on the ESP32-S3 TRM the alignment check is not needed for TX!
         // for esp32s3 we check each descriptor buffer that points to psram for
         // alignment and writeback the cache for that buffer
         #[cfg(esp32s3)]
@@ -1660,7 +1712,19 @@ where
         buffer: &mut BUF,
     ) -> Result<(), DmaError> {
         let preparation = buffer.prepare();
-
+        cfg_if::cfg_if!(
+            if #[cfg(esp32s3)] {
+                if let Some(block_size) = preparation.block_size {
+                    self.set_ext_mem_block_size(block_size.into());
+                }
+            } else {
+                // we insure that block_size is some only for PSRAM addresses
+                if preparation.block_size.is_some() {
+                    return Err(DmaError::UnsupportedMemoryRegion);
+                }
+            }
+        );
+        // TODO: Get burst mode from DmaBuf.
         self.tx_impl
             .prepare_transfer_without_start(preparation.start, peri)
     }
@@ -1817,6 +1881,10 @@ where
 /// Holds all the information needed to configure a DMA channel for a transfer.
 pub struct Preparation {
     start: *mut DmaDescriptor,
+    /// block size for PSRAM transfers (TODO: enable burst mode for non external
+    /// memory?)
+    #[cfg_attr(not(esp32s3), allow(dead_code))]
+    block_size: Option<DmaBufBlkSize>,
     // burst_mode, alignment, check_owner, etc.
 }
 
@@ -1859,22 +1927,41 @@ pub trait DmaRxBuffer {
 
 /// Error returned from Dma[Rx|Tx|RxTx]Buf operations.
 #[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum DmaBufError {
     /// More descriptors are needed for the buffer size
     InsufficientDescriptors,
     /// Descriptors or buffers are not located in a supported memory region
     UnsupportedMemoryRegion,
+    /// Buffer is not aligned to the required size
+    InvalidAlignment,
+    /// Invalid chunk size: must be > 0 and <= 4095
+    InvalidChunkSize,
+}
+
+/// DMA buffer allignments
+#[derive(Debug, Clone, Copy, PartialEq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum DmaBufBlkSize {
+    /// 16 bytes
+    Size16 = 16,
+    /// 32 bytes
+    Size32 = 32,
+    /// 64 bytes
+    Size64 = 64,
 }
 
 /// DMA transmit buffer
 ///
 /// This is a contiguous buffer linked together by DMA descriptors of length
-/// 4092. It can only be used for transmitting data to a peripheral's FIFO.
-/// See [DmaRxBuf] for receiving data.
+/// 4095 at most. It can only be used for transmitting data to a peripheral's
+/// FIFO. See [DmaRxBuf] for receiving data.
 #[derive(Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct DmaTxBuf {
     descriptors: &'static mut [DmaDescriptor],
     buffer: &'static mut [u8],
+    block_size: Option<DmaBufBlkSize>,
 }
 
 impl DmaTxBuf {
@@ -1884,23 +1971,87 @@ impl DmaTxBuf {
     /// Each descriptor can handle 4092 bytes worth of buffer.
     ///
     /// Both the descriptors and buffer must be in DMA-capable memory.
-    /// Only DRAM is supported.
+    /// Only DRAM is supported for descriptors.
     pub fn new(
         descriptors: &'static mut [DmaDescriptor],
         buffer: &'static mut [u8],
     ) -> Result<Self, DmaBufError> {
-        let min_descriptors = buffer.len().div_ceil(CHUNK_SIZE);
+        Self::new_with_block_size(descriptors, buffer, None)
+    }
+
+    /// Compute max chunk size based on block size
+    pub const fn compute_chunk_size(block_size: Option<DmaBufBlkSize>) -> usize {
+        match block_size {
+            Some(size) => 4096 - size as usize,
+            #[cfg(esp32)]
+            None => 4092, // esp32 requires 4 byte alignment
+            #[cfg(not(esp32))]
+            None => 4095,
+        }
+    }
+
+    /// Compute the number of descriptors required for a given block size and
+    /// buffer size
+    pub const fn compute_descriptor_count(
+        buffer_size: usize,
+        block_size: Option<DmaBufBlkSize>,
+    ) -> usize {
+        buffer_size.div_ceil(Self::compute_chunk_size(block_size))
+    }
+
+    /// Creates a new [DmaTxBuf] from some descriptors and a buffer.
+    ///
+    /// There must be enough descriptors for the provided buffer.
+    /// Each descriptor can handle at most 4095 bytes worth of buffer.
+    /// Optionally, a block size can be provided for PSRAM & Burst transfers.
+    ///
+    /// Both the descriptors and buffer must be in DMA-capable memory.
+    /// Only DRAM is supported for descriptors.
+    pub fn new_with_block_size(
+        descriptors: &'static mut [DmaDescriptor],
+        buffer: &'static mut [u8],
+        block_size: Option<DmaBufBlkSize>,
+    ) -> Result<Self, DmaBufError> {
+        let chunk_size = Self::compute_chunk_size(block_size);
+        let min_descriptors = Self::compute_descriptor_count(buffer.len(), block_size);
         if descriptors.len() < min_descriptors {
             return Err(DmaBufError::InsufficientDescriptors);
         }
 
-        if !is_slice_in_dram(descriptors) || !is_slice_in_dram(buffer) {
+        // descriptors are required to be in DRAM
+        if !is_slice_in_dram(descriptors) {
             return Err(DmaBufError::UnsupportedMemoryRegion);
         }
 
+        cfg_if::cfg_if! {
+            if #[cfg(esp32s3)] {
+                // buffer can be either DRAM or PSRAM (if supported)
+                if !is_slice_in_dram(buffer) && !is_slice_in_psram(buffer) {
+                    return Err(DmaBufError::UnsupportedMemoryRegion);
+                }
+                // if its PSRAM, the block_size/alignment must be specified
+                if is_slice_in_psram(buffer) && block_size.is_none() {
+                    return Err(DmaBufError::InvalidAlignment);
+                }
+            } else {
+                #[cfg(any(esp32,esp32s2))]
+                if buffer.len() % 4 != 0 && buffer.as_ptr() as usize % 4 != 0 {
+                    // ESP32 requires word alignment for DMA buffers.
+                    // ESP32-S2 technically supports byte-aligned DMA buffers, but the
+                    // transfer ends up writing out of bounds if the buffer's length
+                    // is 2 or 3 (mod 4).
+                    return Err(DmaBufError::InvalidAlignment);
+                }
+                // buffer can only be DRAM
+                if !is_slice_in_dram(buffer) {
+                    return Err(DmaBufError::UnsupportedMemoryRegion);
+                }
+            }
+        }
+
         // Setup size and buffer pointer as these will not change for the remainder of
         // this object's lifetime
-        let chunk_iter = descriptors.iter_mut().zip(buffer.chunks_mut(CHUNK_SIZE));
+        let chunk_iter = descriptors.iter_mut().zip(buffer.chunks_mut(chunk_size));
         for (desc, chunk) in chunk_iter {
             desc.set_size(chunk.len());
             desc.buffer = chunk.as_mut_ptr();
@@ -1909,9 +2060,13 @@ impl DmaTxBuf {
         let mut buf = Self {
             descriptors,
             buffer,
+            block_size,
         };
         buf.set_length(buf.capacity());
-
+        // no need for block size if the buffer is in DRAM
+        if is_slice_in_dram(buf.buffer) {
+            buf.block_size = None;
+        }
         Ok(buf)
     }
 
@@ -1947,7 +2102,7 @@ impl DmaTxBuf {
         assert!(len <= self.buffer.len());
 
         // Get the minimum number of descriptors needed for this length of data.
-        let descriptor_count = len.div_ceil(CHUNK_SIZE).max(1);
+        let descriptor_count = len.div_ceil(self.descriptors[0].size()).max(1);
         let required_descriptors = &mut self.descriptors[0..descriptor_count];
 
         // Link up the relevant descriptors.
@@ -2008,8 +2163,19 @@ impl DmaTxBuffer for DmaTxBuf {
             }
         }
 
+        #[cfg(esp32s3)]
+        if crate::soc::is_valid_psram_address(self.buffer.as_ptr() as u32) {
+            unsafe {
+                crate::soc::cache_writeback_addr(
+                    self.buffer.as_ptr() as u32,
+                    self.buffer.len() as u32,
+                )
+            };
+        }
+
         Preparation {
             start: self.descriptors.as_mut_ptr(),
+            block_size: self.block_size,
         }
     }
 
@@ -2246,6 +2412,7 @@ impl DmaRxBuffer for DmaRxBuf {
 
         Preparation {
             start: self.descriptors.as_mut_ptr(),
+            block_size: None,
         }
     }
 
@@ -2438,6 +2605,7 @@ impl DmaTxBuffer for DmaRxTxBuf {
 
         Preparation {
             start: self.tx_descriptors.as_mut_ptr(),
+            block_size: None, // TODO: support block size!
         }
     }
 
@@ -2467,6 +2635,7 @@ impl DmaRxBuffer for DmaRxTxBuf {
 
         Preparation {
             start: self.rx_descriptors.as_mut_ptr(),
+            block_size: None, // TODO: support block size!
         }
     }
 

esp-hal/src/soc/mod.rs

@@ -105,6 +105,13 @@ pub(crate) fn is_valid_psram_address(address: u32) -> bool {
     false
 }
 
+#[allow(unused)]
+pub(crate) fn is_slice_in_psram<T>(slice: &[T]) -> bool {
+    let start = slice.as_ptr() as u32;
+    let end = start + slice.len() as u32;
+    is_valid_psram_address(start) && is_valid_psram_address(end)
+}
+
 #[allow(unused)]
 pub(crate) fn is_valid_memory_address(address: u32) -> bool {
     is_valid_ram_address(address) || is_valid_psram_address(address)