From 306d118f4b1055269484fafceac7b80fe11834e3 Mon Sep 17 00:00:00 2001 From: liebman Date: Sun, 15 Sep 2024 12:25:07 -0700 Subject: [PATCH 01/24] support psram in DmaTxBuf --- esp-hal/src/dma/mod.rs | 103 +++++++++- esp-hal/src/soc/mod.rs | 7 + hil-test/Cargo.toml | 8 +- hil-test/tests/spi_half_duplex_write_psram.rs | 191 ++++++++++++++++++ 4 files changed, 299 insertions(+), 10 deletions(-) create mode 100644 hil-test/tests/spi_half_duplex_write_psram.rs diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 799033bddab..349564b567d 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -226,6 +226,13 @@ 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,7 +293,11 @@ use enumset::{EnumSet, EnumSetType}; pub use self::gdma::*; #[cfg(pdma)] pub use self::pdma::*; -use crate::{interrupt::InterruptHandler, soc::is_slice_in_dram, Mode}; +use crate::{ + interrupt::InterruptHandler, + soc::{is_slice_in_dram, is_slice_in_psram}, + Mode, +}; #[cfg(gdma)] mod gdma; @@ -954,6 +965,16 @@ pub enum DmaExtMemBKSize { Size64 = 2, } +impl From 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, @@ -1387,7 +1408,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); } } @@ -1704,6 +1724,7 @@ where peri: DmaPeripheral, chain: &DescriptorChain, ) -> Result<(), DmaError> { + // TODO: based on the ESP32-S3 TRM teh 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)] @@ -1729,7 +1750,11 @@ where buffer: &mut BUF, ) -> Result<(), DmaError> { let preparation = buffer.prepare(); - + #[cfg(esp32s3)] + if let Some(block_size) = preparation.block_size { + self.set_ext_mem_block_size(block_size.into()); + } + // TODO: Get burst mode from DmaBuf. self.tx_impl .prepare_transfer_without_start(preparation.start, peri) } @@ -1962,6 +1987,8 @@ 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?) + block_size: Option, // burst_mode, alignment, check_owner, etc. } @@ -2009,17 +2036,37 @@ pub enum DmaBufError { 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 <= 4092 + InvalidChunkSize, +} + +/// DMA buffer allignments +#[cfg(all(esp32s3, psram))] +#[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. +/// 4092 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], + chunk_size: usize, + block_size: Option, } impl DmaTxBuf { @@ -2029,23 +2076,50 @@ 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 { - let min_descriptors = buffer.len().div_ceil(CHUNK_SIZE); + Self::new_with_chunk_size(descriptors, buffer, CHUNK_SIZE, None) + } + + /// 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 4092 bytes worth of buffer. + /// The chunk size must be greater than 0 and less than or equal to 4092. + /// 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_chunk_size( + descriptors: &'static mut [DmaDescriptor], + buffer: &'static mut [u8], + chunk_size: usize, + block_size: Option, + ) -> Result { + if chunk_size == 0 || chunk_size > 4092 { + return Err(DmaBufError::InvalidChunkSize); + } + let min_descriptors = buffer.len().div_ceil(chunk_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); + } + + // buffer can be either DRAM or PSRAM (if supported) + if !is_slice_in_dram(buffer) && !is_slice_in_psram(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(); @@ -2054,6 +2128,8 @@ impl DmaTxBuf { let mut buf = Self { descriptors, buffer, + chunk_size, + block_size, }; buf.set_length(buf.capacity()); @@ -2092,7 +2168,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.chunk_size).max(1); let required_descriptors = &mut self.descriptors[0..descriptor_count]; // Link up the relevant descriptors. @@ -2153,8 +2229,14 @@ 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, } } @@ -2391,6 +2473,7 @@ impl DmaRxBuffer for DmaRxBuf { Preparation { start: self.descriptors.as_mut_ptr(), + block_size: None, } } @@ -2583,6 +2666,7 @@ impl DmaTxBuffer for DmaRxTxBuf { Preparation { start: self.tx_descriptors.as_mut_ptr(), + block_size: None, // TODO: support block size! } } @@ -2612,6 +2696,7 @@ impl DmaRxBuffer for DmaRxTxBuf { Preparation { start: self.rx_descriptors.as_mut_ptr(), + block_size: None, // TODO: support block size! } } diff --git a/esp-hal/src/soc/mod.rs b/esp-hal/src/soc/mod.rs index 2338237f515..a24f409bed2 100644 --- a/esp-hal/src/soc/mod.rs +++ b/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(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) diff --git a/hil-test/Cargo.toml b/hil-test/Cargo.toml index bef22fea4dc..d431ac45f21 100644 --- a/hil-test/Cargo.toml +++ b/hil-test/Cargo.toml @@ -95,6 +95,10 @@ harness = false name = "spi_half_duplex_write" harness = false +[[test]] +name = "spi_half_duplex_write_psram" +harness = false + [[test]] name = "systimer" harness = false @@ -171,6 +175,7 @@ embedded-hal = "1.0.0" embedded-hal-02 = { version = "0.2.7", package = "embedded-hal", features = ["unproven"] } embedded-hal-async = "1.0.0" embedded-hal-nb = { version = "1.0.0", optional = true } +esp-alloc = { path = "../esp-alloc", optional = true } esp-backtrace = { path = "../esp-backtrace", default-features = false, features = ["exception-handler", "panic-handler", "defmt", "semihosting"] } esp-hal = { path = "../esp-hal", features = ["defmt", "digest"], optional = true } esp-hal-embassy = { path = "../esp-hal-embassy", optional = true } @@ -199,7 +204,7 @@ esp-metadata = { version = "0.3.0", path = "../esp-metadata" } [features] default = ["embassy"] -defmt = ["dep:defmt-rtt"] +defmt = ["dep:defmt-rtt", "embedded-test/defmt"] # Device support (required!): esp32 = [ @@ -236,6 +241,7 @@ generic-queue = [ integrated-timers = [ "esp-hal-embassy/integrated-timers", ] +psram = ["esp-hal/opsram-2m", "dep:esp-alloc"] # https://doc.rust-lang.org/cargo/reference/profiles.html#test # Test and bench profiles inherit from dev and release respectively. diff --git a/hil-test/tests/spi_half_duplex_write_psram.rs b/hil-test/tests/spi_half_duplex_write_psram.rs new file mode 100644 index 00000000000..495387c8214 --- /dev/null +++ b/hil-test/tests/spi_half_duplex_write_psram.rs @@ -0,0 +1,191 @@ +//! SPI Half Duplex Write Test +//% FEATURES: psram +//% CHIPS: esp32s3 + +#![no_std] +#![no_main] +use esp_alloc as _; +use esp_hal::{ + dma::{Dma, DmaPriority, DmaTxBuf, DmaBufBlkSize}, + dma_descriptors_chunk_size, + gpio::{interconnect::InputSignal, Io}, + pcnt::{channel::EdgeMode, unit::Unit, Pcnt}, + peripherals::SPI2, + prelude::*, + spi::{ + master::{Address, Command, Spi, SpiDma}, + HalfDuplexMode, + SpiDataMode, + SpiMode, + }, + Blocking, +}; +use hil_test as _; +extern crate alloc; + +cfg_if::cfg_if! { + if #[cfg(any( + feature = "esp32", + feature = "esp32s2", + ))] { + use esp_hal::dma::Spi2DmaChannel as DmaChannel0; + } else { + use esp_hal::dma::DmaChannel0; + } +} + +macro_rules! dma_alloc_buffer { + ($size:expr, $align:expr) => {{ + let layout = core::alloc::Layout::from_size_align($size, $align).unwrap(); + unsafe { + let ptr = alloc::alloc::alloc(layout); + if ptr.is_null() { + error!("dma_alloc_buffer: alloc failed"); + alloc::alloc::handle_alloc_error(layout); + } + core::slice::from_raw_parts_mut(ptr, $size) + } + }}; +} + +struct Context { + spi: SpiDma<'static, SPI2, DmaChannel0, HalfDuplexMode, Blocking>, + pcnt_unit: Unit<'static, 0>, + pcnt_source: InputSignal, +} + +#[cfg(test)] +#[embedded_test::tests] +mod tests { + // defmt::* is load-bearing, it ensures that the assert in dma_buffers! is not + // using defmt's non-const assert. Doing so would result in a compile error. + #[allow(unused_imports)] + use defmt::{assert_eq, *}; + + use super::*; + + #[init] + fn init() -> Context { + let peripherals = esp_hal::init(esp_hal::Config::default()); + esp_alloc::psram_allocator!(peripherals.PSRAM, esp_hal::psram); + + let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); + let sclk = io.pins.gpio0; + let (mosi, _) = hil_test::common_test_pins!(io); + + let pcnt = Pcnt::new(peripherals.PCNT); + let dma = Dma::new(peripherals.DMA); + + cfg_if::cfg_if! { + if #[cfg(any(feature = "esp32", feature = "esp32s2"))] { + let dma_channel = dma.spi2channel; + } else { + let dma_channel = dma.channel0; + } + } + + let mosi_loopback = mosi.peripheral_input(); + + let spi = Spi::new_half_duplex(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) + .with_sck(sclk) + .with_mosi(mosi) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); + + Context { + spi, + pcnt_unit: pcnt.unit0, + pcnt_source: mosi_loopback, + } + } + + #[test] + #[timeout(3)] + fn test_spi_writes_are_correctly_by_pcnt(ctx: Context) { + const DMA_BUFFER_SIZE: usize = 64; + const DMA_CHUNK_SIZE: usize = 4032; // 64 byte aligned + const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size32; // matches dcache line size + + let (_, descriptors) = dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); + let buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize); + let mut dma_tx_buf = DmaTxBuf::new_with_chunk_size(descriptors, buffer, DMA_CHUNK_SIZE, Some(DMA_ALIGNMENT)).unwrap(); + + let unit = ctx.pcnt_unit; + let mut spi = ctx.spi; + + unit.channel0.set_edge_signal(ctx.pcnt_source); + unit.channel0 + .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); + + // Fill the buffer where each byte has 3 pos edges. + dma_tx_buf.fill(&[0b0110_1010; DMA_BUFFER_SIZE]); + let transfer = spi + .write( + SpiDataMode::Single, + Command::None, + Address::None, + 0, + dma_tx_buf, + ) + .map_err(|e| e.0) + .unwrap(); + (spi, dma_tx_buf) = transfer.wait(); + + assert_eq!(unit.get_value(), (3 * DMA_BUFFER_SIZE) as _); + + let transfer = spi + .write( + SpiDataMode::Single, + Command::None, + Address::None, + 0, + dma_tx_buf, + ) + .map_err(|e| e.0) + .unwrap(); + transfer.wait(); + + assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _); + } + + // TODO: when the first test is working, we can add a second one! + // #[test] + // #[timeout(3)] + // fn test_spidmabus_writes_are_correctly_by_pcnt(ctx: Context) { + // const DMA_BUFFER_SIZE: usize = 64; + + // let (rx, rxd, buffer, descriptors) = dma_buffers!(1, DMA_BUFFER_SIZE); + // let dma_rx_buf = DmaRxBuf::new(rxd, rx).unwrap(); + // let dma_tx_buf = DmaTxBuf::new(descriptors, buffer).unwrap(); + + // let unit = ctx.pcnt_unit; + // let mut spi = ctx.spi.with_buffers(dma_rx_buf, dma_tx_buf); + + // unit.channel0.set_edge_signal(ctx.pcnt_source); + // unit.channel0 + // .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); + + // let buffer = [0b0110_1010; DMA_BUFFER_SIZE]; + // // Write the buffer where each byte has 3 pos edges. + // spi.write( + // SpiDataMode::Single, + // Command::None, + // Address::None, + // 0, + // &buffer, + // ) + // .unwrap(); + + // assert_eq!(unit.get_value(), (3 * DMA_BUFFER_SIZE) as _); + + // spi.write( + // SpiDataMode::Single, + // Command::None, + // Address::None, + // 0, + // &buffer, + // ) + // .unwrap(); + + // assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _); + // } +} From 008c8cd1c78c99bbd19ccbdb5b616d6bbcad6c31 Mon Sep 17 00:00:00 2001 From: liebman Date: Sun, 15 Sep 2024 12:46:30 -0700 Subject: [PATCH 02/24] add example that sometimes works :-( --- examples/src/bin/spi_loopback_dma_psram.rs | 115 +++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 examples/src/bin/spi_loopback_dma_psram.rs diff --git a/examples/src/bin/spi_loopback_dma_psram.rs b/examples/src/bin/spi_loopback_dma_psram.rs new file mode 100644 index 00000000000..fddaf0a4649 --- /dev/null +++ b/examples/src/bin/spi_loopback_dma_psram.rs @@ -0,0 +1,115 @@ +//! SPI loopback test using DMA - send from PSRAM receive to internal RAM +//! +//! The following wiring is assumed: +//! - SCLK => GPIO42 +//! - MISO => (loopback to MOSI via peripheral_input()) +//! - MOSI => GPIO48 +//! - CS => GPIO38 +//! +//! Depending on your target and the board you are using you have to change the +//! pins. +//! +//! This example transfers data via SPI. +//! Connect MISO and MOSI pins to see the outgoing data is read as incoming +//! data. + +//% FEATURES: esp-hal/log opsram-2m +//% CHIPS: esp32s3 + +#![no_std] +#![no_main] + +use esp_backtrace as _; +use esp_hal::{ + delay::Delay, + dma::{Dma, DmaPriority, DmaRxBuf, DmaTxBuf, DmaBufBlkSize}, + gpio::Io, + prelude::*, + spi::{master::Spi, SpiMode}, +}; +extern crate alloc; +use log::*; + +macro_rules! dma_alloc_buffer { + ($size:expr, $align:expr) => {{ + let layout = core::alloc::Layout::from_size_align($size, $align).unwrap(); + unsafe { + let ptr = alloc::alloc::alloc(layout); + if ptr.is_null() { + error!("dma_alloc_buffer: alloc failed"); + alloc::alloc::handle_alloc_error(layout); + } + core::slice::from_raw_parts_mut(ptr, $size) + } + }}; +} + +// TODO: the fist transfer fails in some conditions: +// - if this is <= 8192 when chunk_size is 4032!?!? +// - varing either DMA_CHUNK_SIZE or DMA_BUFFER_SIZE seems change the behavior +const DMA_BUFFER_SIZE: usize = 16384; // the first request fails is this is <= 8192 when chunk_size is 4032!?!? +const DMA_CHUNK_SIZE: usize = 4032; // size is aligned to 64 bytes +const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size16; + +#[entry] +fn main() -> ! { + esp_println::logger::init_logger(log::LevelFilter::Info); + info!("Starting SPI loopback test"); + let peripherals = esp_hal::init(esp_hal::Config::default()); + esp_alloc::psram_allocator!(peripherals.PSRAM, esp_hal::psram); + let delay = Delay::new(); + + let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); + let sclk = io.pins.gpio42; + let mosi = io.pins.gpio48; + let miso = mosi.peripheral_input(); + let cs = io.pins.gpio38; + + let dma = Dma::new(peripherals.DMA); + let dma_channel = dma.channel0; + + let (_, tx_descriptors) = esp_hal::dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); + let tx_buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize); + info!("TX: {:p} len {} ({} descripters)", tx_buffer.as_ptr(), tx_buffer.len(), tx_descriptors.len()); + let mut dma_tx_buf = DmaTxBuf::new_with_chunk_size(tx_descriptors, tx_buffer, DMA_CHUNK_SIZE, Some(DMA_ALIGNMENT)).unwrap(); + let (rx_buffer, rx_descriptors, _, _) = esp_hal::dma_buffers!(DMA_BUFFER_SIZE, 0); + info!("RX: {:p} len {} ({} descripters)", rx_buffer.as_ptr(), rx_buffer.len(), rx_descriptors.len()); + let mut dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); + let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) + .with_pins(sclk, mosi, miso, cs) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); + + delay.delay_millis(100); // delay to let the above messages display + + for (i, v) in dma_tx_buf.as_mut_slice().iter_mut().enumerate() { + *v = (i % 256) as u8; + } + + let mut i = 0; + + loop { + dma_tx_buf.as_mut_slice()[0] = i; + *dma_tx_buf.as_mut_slice().last_mut().unwrap() = i; + i = i.wrapping_add(1); + + let transfer = spi + .dma_transfer(dma_rx_buf, dma_tx_buf) + .map_err(|e| e.0) + .unwrap(); + + (spi, (dma_rx_buf, dma_tx_buf)) = transfer.wait(); + for (i, v) in dma_tx_buf.as_mut_slice().iter_mut().enumerate() { + if dma_rx_buf.as_slice()[i] != *v { + error!("Mismatch at index {}: expected {}, got {}", i, *v, dma_rx_buf.as_slice()[i]); + break; + } + } + info!( + "{:0x?} .. {:0x?}", + &dma_rx_buf.as_slice()[..10], + &dma_rx_buf.as_slice().last_chunk::<10>().unwrap() + ); + dma_tx_buf.as_mut_slice().reverse(); + delay.delay_millis(1000); + } +} From b231c8bf1c469b50232e2b8dd6e861439c1adfa6 Mon Sep 17 00:00:00 2001 From: liebman Date: Sun, 15 Sep 2024 12:51:59 -0700 Subject: [PATCH 03/24] fmt --- esp-hal/src/dma/mod.rs | 23 +++++++++--- examples/src/bin/spi_loopback_dma_psram.rs | 36 +++++++++++++++---- hil-test/tests/spi_half_duplex_write_psram.rs | 13 ++++--- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 349564b567d..43b77e10eca 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -229,7 +229,14 @@ 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" }); + defmt::write!( + fmt, + "DmaDescriptorFlags {{ size: {}, length: {}, suc_eof: {}, owner: {} }}", + self.size(), + self.length(), + self.suc_eof(), + if self.owner() { "DMA" } else { "CPU" } + ); } } @@ -1987,7 +1994,8 @@ 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?) + /// block size for PSRAM transfers (TODO: enable burst mode for non external + /// memory?) block_size: Option, // burst_mode, alignment, check_owner, etc. } @@ -2058,8 +2066,8 @@ pub enum DmaBufBlkSize { /// DMA transmit buffer /// /// This is a contiguous buffer linked together by DMA descriptors of length -/// 4092 at most. It can only be used for transmitting data to a peripheral's FIFO. -/// See [DmaRxBuf] for receiving data. +/// 4092 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 { @@ -2231,7 +2239,12 @@ 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)}; + unsafe { + crate::soc::cache_writeback_addr( + self.buffer.as_ptr() as u32, + self.buffer.len() as u32, + ) + }; } Preparation { diff --git a/examples/src/bin/spi_loopback_dma_psram.rs b/examples/src/bin/spi_loopback_dma_psram.rs index fddaf0a4649..3c31349ed64 100644 --- a/examples/src/bin/spi_loopback_dma_psram.rs +++ b/examples/src/bin/spi_loopback_dma_psram.rs @@ -22,7 +22,7 @@ use esp_backtrace as _; use esp_hal::{ delay::Delay, - dma::{Dma, DmaPriority, DmaRxBuf, DmaTxBuf, DmaBufBlkSize}, + dma::{Dma, DmaBufBlkSize, DmaPriority, DmaRxBuf, DmaTxBuf}, gpio::Io, prelude::*, spi::{master::Spi, SpiMode}, @@ -47,7 +47,7 @@ macro_rules! dma_alloc_buffer { // TODO: the fist transfer fails in some conditions: // - if this is <= 8192 when chunk_size is 4032!?!? // - varing either DMA_CHUNK_SIZE or DMA_BUFFER_SIZE seems change the behavior -const DMA_BUFFER_SIZE: usize = 16384; // the first request fails is this is <= 8192 when chunk_size is 4032!?!? +const DMA_BUFFER_SIZE: usize = 16384; // the first request fails is this is <= 8192 when chunk_size is 4032!?!? const DMA_CHUNK_SIZE: usize = 4032; // size is aligned to 64 bytes const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size16; @@ -68,12 +68,29 @@ fn main() -> ! { let dma = Dma::new(peripherals.DMA); let dma_channel = dma.channel0; - let (_, tx_descriptors) = esp_hal::dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); + let (_, tx_descriptors) = + esp_hal::dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); let tx_buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize); - info!("TX: {:p} len {} ({} descripters)", tx_buffer.as_ptr(), tx_buffer.len(), tx_descriptors.len()); - let mut dma_tx_buf = DmaTxBuf::new_with_chunk_size(tx_descriptors, tx_buffer, DMA_CHUNK_SIZE, Some(DMA_ALIGNMENT)).unwrap(); + info!( + "TX: {:p} len {} ({} descripters)", + tx_buffer.as_ptr(), + tx_buffer.len(), + tx_descriptors.len() + ); + let mut dma_tx_buf = DmaTxBuf::new_with_chunk_size( + tx_descriptors, + tx_buffer, + DMA_CHUNK_SIZE, + Some(DMA_ALIGNMENT), + ) + .unwrap(); let (rx_buffer, rx_descriptors, _, _) = esp_hal::dma_buffers!(DMA_BUFFER_SIZE, 0); - info!("RX: {:p} len {} ({} descripters)", rx_buffer.as_ptr(), rx_buffer.len(), rx_descriptors.len()); + info!( + "RX: {:p} len {} ({} descripters)", + rx_buffer.as_ptr(), + rx_buffer.len(), + rx_descriptors.len() + ); let mut dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) .with_pins(sclk, mosi, miso, cs) @@ -100,7 +117,12 @@ fn main() -> ! { (spi, (dma_rx_buf, dma_tx_buf)) = transfer.wait(); for (i, v) in dma_tx_buf.as_mut_slice().iter_mut().enumerate() { if dma_rx_buf.as_slice()[i] != *v { - error!("Mismatch at index {}: expected {}, got {}", i, *v, dma_rx_buf.as_slice()[i]); + error!( + "Mismatch at index {}: expected {}, got {}", + i, + *v, + dma_rx_buf.as_slice()[i] + ); break; } } diff --git a/hil-test/tests/spi_half_duplex_write_psram.rs b/hil-test/tests/spi_half_duplex_write_psram.rs index 495387c8214..0a9dbe86b47 100644 --- a/hil-test/tests/spi_half_duplex_write_psram.rs +++ b/hil-test/tests/spi_half_duplex_write_psram.rs @@ -6,7 +6,7 @@ #![no_main] use esp_alloc as _; use esp_hal::{ - dma::{Dma, DmaPriority, DmaTxBuf, DmaBufBlkSize}, + dma::{Dma, DmaBufBlkSize, DmaPriority, DmaTxBuf}, dma_descriptors_chunk_size, gpio::{interconnect::InputSignal, Io}, pcnt::{channel::EdgeMode, unit::Unit, Pcnt}, @@ -107,7 +107,9 @@ mod tests { let (_, descriptors) = dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); let buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize); - let mut dma_tx_buf = DmaTxBuf::new_with_chunk_size(descriptors, buffer, DMA_CHUNK_SIZE, Some(DMA_ALIGNMENT)).unwrap(); + let mut dma_tx_buf = + DmaTxBuf::new_with_chunk_size(descriptors, buffer, DMA_CHUNK_SIZE, Some(DMA_ALIGNMENT)) + .unwrap(); let unit = ctx.pcnt_unit; let mut spi = ctx.spi; @@ -153,9 +155,10 @@ mod tests { // fn test_spidmabus_writes_are_correctly_by_pcnt(ctx: Context) { // const DMA_BUFFER_SIZE: usize = 64; - // let (rx, rxd, buffer, descriptors) = dma_buffers!(1, DMA_BUFFER_SIZE); - // let dma_rx_buf = DmaRxBuf::new(rxd, rx).unwrap(); - // let dma_tx_buf = DmaTxBuf::new(descriptors, buffer).unwrap(); + // let (rx, rxd, buffer, descriptors) = dma_buffers!(1, + // DMA_BUFFER_SIZE); let dma_rx_buf = DmaRxBuf::new(rxd, + // rx).unwrap(); let dma_tx_buf = DmaTxBuf::new(descriptors, + // buffer).unwrap(); // let unit = ctx.pcnt_unit; // let mut spi = ctx.spi.with_buffers(dma_rx_buf, dma_tx_buf); From 07f780c955ffeabcfa8de35c34f611277e6af2ef Mon Sep 17 00:00:00 2001 From: liebman Date: Sun, 15 Sep 2024 13:45:33 -0700 Subject: [PATCH 04/24] cleanups --- esp-hal/src/dma/mod.rs | 3 +-- examples/src/bin/spi_loopback_dma_psram.rs | 3 +++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 43b77e10eca..73fb4fb978d 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -1731,7 +1731,7 @@ where peri: DmaPeripheral, chain: &DescriptorChain, ) -> Result<(), DmaError> { - // TODO: based on the ESP32-S3 TRM teh alignment check is not needed for TX! + // 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)] @@ -2051,7 +2051,6 @@ pub enum DmaBufError { } /// DMA buffer allignments -#[cfg(all(esp32s3, psram))] #[derive(Debug, Clone, Copy, PartialEq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum DmaBufBlkSize { diff --git a/examples/src/bin/spi_loopback_dma_psram.rs b/examples/src/bin/spi_loopback_dma_psram.rs index 3c31349ed64..cb5aca81251 100644 --- a/examples/src/bin/spi_loopback_dma_psram.rs +++ b/examples/src/bin/spi_loopback_dma_psram.rs @@ -12,6 +12,9 @@ //! This example transfers data via SPI. //! Connect MISO and MOSI pins to see the outgoing data is read as incoming //! data. +//! +//! If your module is quad PSRAM then you need to change the `psram` feature in the +//! in the features line below to `psram-2m`. //% FEATURES: esp-hal/log opsram-2m //% CHIPS: esp32s3 From b1376b3a947ec4f68e58a57716339df31684160a Mon Sep 17 00:00:00 2001 From: liebman Date: Mon, 16 Sep 2024 10:21:17 -0700 Subject: [PATCH 05/24] allow chunk_size upto (including) 4095 --- esp-hal/src/dma/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 73fb4fb978d..64041a77c80 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -529,7 +529,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; @@ -564,7 +564,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 { @@ -2046,7 +2046,7 @@ pub enum DmaBufError { UnsupportedMemoryRegion, /// Buffer is not aligned to the required size InvalidAlignment, - /// Invalid chunk size: must be > 0 and <= 4092 + /// Invalid chunk size: must be > 0 and <= 4095 InvalidChunkSize, } @@ -2065,7 +2065,7 @@ pub enum DmaBufBlkSize { /// DMA transmit buffer /// /// This is a contiguous buffer linked together by DMA descriptors of length -/// 4092 at most. It can only be used for transmitting data to a peripheral's +/// 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))] @@ -2094,8 +2094,8 @@ impl DmaTxBuf { /// 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 4092 bytes worth of buffer. - /// The chunk size must be greater than 0 and less than or equal to 4092. + /// Each descriptor can handle at most 4095 bytes worth of buffer. + /// The chunk size must be greater than 0 and less than or equal to 4095. /// Optionally, a block size can be provided for PSRAM & Burst transfers. /// /// Both the descriptors and buffer must be in DMA-capable memory. @@ -2106,7 +2106,7 @@ impl DmaTxBuf { chunk_size: usize, block_size: Option, ) -> Result { - if chunk_size == 0 || chunk_size > 4092 { + if chunk_size == 0 || chunk_size > 4095 { return Err(DmaBufError::InvalidChunkSize); } let min_descriptors = buffer.len().div_ceil(chunk_size); From 179e926198a4d4ec0fd4a1df03498790f4fbf56d Mon Sep 17 00:00:00 2001 From: liebman Date: Mon, 16 Sep 2024 10:40:55 -0700 Subject: [PATCH 06/24] this test is passing for me now --- hil-test/tests/spi_half_duplex_write_psram.rs | 104 +++++++++--------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/hil-test/tests/spi_half_duplex_write_psram.rs b/hil-test/tests/spi_half_duplex_write_psram.rs index 0a9dbe86b47..9d028084676 100644 --- a/hil-test/tests/spi_half_duplex_write_psram.rs +++ b/hil-test/tests/spi_half_duplex_write_psram.rs @@ -6,14 +6,15 @@ #![no_main] use esp_alloc as _; use esp_hal::{ - dma::{Dma, DmaBufBlkSize, DmaPriority, DmaTxBuf}, + dma::{Dma, DmaBufBlkSize, DmaPriority, DmaRxBuf, DmaTxBuf}, + dma_buffers, dma_descriptors_chunk_size, gpio::{interconnect::InputSignal, Io}, pcnt::{channel::EdgeMode, unit::Unit, Pcnt}, peripherals::SPI2, prelude::*, spi::{ - master::{Address, Command, Spi, SpiDma}, + master::{Address, Command, HalfDuplexReadWrite, Spi, SpiDma}, HalfDuplexMode, SpiDataMode, SpiMode, @@ -76,13 +77,7 @@ mod tests { let pcnt = Pcnt::new(peripherals.PCNT); let dma = Dma::new(peripherals.DMA); - cfg_if::cfg_if! { - if #[cfg(any(feature = "esp32", feature = "esp32s2"))] { - let dma_channel = dma.spi2channel; - } else { - let dma_channel = dma.channel0; - } - } + let dma_channel = dma.channel0; let mosi_loopback = mosi.peripheral_input(); @@ -101,7 +96,7 @@ mod tests { #[test] #[timeout(3)] fn test_spi_writes_are_correctly_by_pcnt(ctx: Context) { - const DMA_BUFFER_SIZE: usize = 64; + const DMA_BUFFER_SIZE: usize = 4; const DMA_CHUNK_SIZE: usize = 4032; // 64 byte aligned const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size32; // matches dcache line size @@ -149,46 +144,51 @@ mod tests { assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _); } - // TODO: when the first test is working, we can add a second one! - // #[test] - // #[timeout(3)] - // fn test_spidmabus_writes_are_correctly_by_pcnt(ctx: Context) { - // const DMA_BUFFER_SIZE: usize = 64; - - // let (rx, rxd, buffer, descriptors) = dma_buffers!(1, - // DMA_BUFFER_SIZE); let dma_rx_buf = DmaRxBuf::new(rxd, - // rx).unwrap(); let dma_tx_buf = DmaTxBuf::new(descriptors, - // buffer).unwrap(); - - // let unit = ctx.pcnt_unit; - // let mut spi = ctx.spi.with_buffers(dma_rx_buf, dma_tx_buf); - - // unit.channel0.set_edge_signal(ctx.pcnt_source); - // unit.channel0 - // .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); - - // let buffer = [0b0110_1010; DMA_BUFFER_SIZE]; - // // Write the buffer where each byte has 3 pos edges. - // spi.write( - // SpiDataMode::Single, - // Command::None, - // Address::None, - // 0, - // &buffer, - // ) - // .unwrap(); - - // assert_eq!(unit.get_value(), (3 * DMA_BUFFER_SIZE) as _); - - // spi.write( - // SpiDataMode::Single, - // Command::None, - // Address::None, - // 0, - // &buffer, - // ) - // .unwrap(); - - // assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _); - // } + #[test] + #[timeout(3)] + fn test_spidmabus_writes_are_correctly_by_pcnt(ctx: Context) { + const DMA_BUFFER_SIZE: usize = 4; + const DMA_CHUNK_SIZE: usize = 4032; // 64 byte aligned + const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size32; // matches dcache line size + + let (_, descriptors) = dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); + let buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize); + let dma_tx_buf = + DmaTxBuf::new_with_chunk_size(descriptors, buffer, DMA_CHUNK_SIZE, Some(DMA_ALIGNMENT)) + .unwrap(); + + let (rx, rxd, _, _) = dma_buffers!(1, 0); + let dma_rx_buf = DmaRxBuf::new(rxd, rx).unwrap(); + + let unit = ctx.pcnt_unit; + let mut spi = ctx.spi.with_buffers(dma_rx_buf, dma_tx_buf); + + unit.channel0.set_edge_signal(ctx.pcnt_source); + unit.channel0 + .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); + + let buffer = [0b0110_1010; DMA_BUFFER_SIZE]; + // Write the buffer where each byte has 3 pos edges. + spi.write( + SpiDataMode::Single, + Command::None, + Address::None, + 0, + &buffer, + ) + .unwrap(); + + assert_eq!(unit.get_value(), (3 * DMA_BUFFER_SIZE) as _); + + spi.write( + SpiDataMode::Single, + Command::None, + Address::None, + 0, + &buffer, + ) + .unwrap(); + + assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _); + } } From 1e37d4fca88560e9b15274c33b888cea1b3bca12 Mon Sep 17 00:00:00 2001 From: liebman Date: Mon, 16 Sep 2024 15:57:49 -0700 Subject: [PATCH 07/24] remove chunk_size and compute based on block_size --- esp-hal/src/dma/mod.rs | 48 ++++++++++++------- examples/src/bin/spi_loopback_dma_psram.rs | 11 ++--- hil-test/tests/spi_half_duplex_write_psram.rs | 10 ++-- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 64041a77c80..91715bc6ff6 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -1757,10 +1757,17 @@ where buffer: &mut BUF, ) -> Result<(), DmaError> { let preparation = buffer.prepare(); - #[cfg(esp32s3)] - if let Some(block_size) = preparation.block_size { - self.set_ext_mem_block_size(block_size.into()); - } + 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 { + if is_slice_in_psram(self.buffer) { + return Err(DmaError::UnsupportedMemoryRegion); + } + } + ); // TODO: Get burst mode from DmaBuf. self.tx_impl .prepare_transfer_without_start(preparation.start, peri) @@ -2072,7 +2079,6 @@ pub enum DmaBufBlkSize { pub struct DmaTxBuf { descriptors: &'static mut [DmaDescriptor], buffer: &'static mut [u8], - chunk_size: usize, block_size: Option, } @@ -2088,27 +2094,27 @@ impl DmaTxBuf { descriptors: &'static mut [DmaDescriptor], buffer: &'static mut [u8], ) -> Result { - Self::new_with_chunk_size(descriptors, buffer, CHUNK_SIZE, None) + Self::new_with_block_size(descriptors, buffer, None) } /// 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. - /// The chunk size must be greater than 0 and less than or equal to 4095. /// 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_chunk_size( + pub fn new_with_block_size( descriptors: &'static mut [DmaDescriptor], buffer: &'static mut [u8], - chunk_size: usize, block_size: Option, ) -> Result { - if chunk_size == 0 || chunk_size > 4095 { - return Err(DmaBufError::InvalidChunkSize); - } + let chunk_size = match block_size { + Some(size) => 4096 - size as usize, + None => 4095, // max chunk_siize + }; + let min_descriptors = buffer.len().div_ceil(chunk_size); if descriptors.len() < min_descriptors { return Err(DmaBufError::InsufficientDescriptors); @@ -2119,9 +2125,18 @@ impl DmaTxBuf { return Err(DmaBufError::UnsupportedMemoryRegion); } - // buffer can be either DRAM or PSRAM (if supported) - if !is_slice_in_dram(buffer) && !is_slice_in_psram(buffer) { - 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); + } + } else { + // 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 @@ -2135,7 +2150,6 @@ impl DmaTxBuf { let mut buf = Self { descriptors, buffer, - chunk_size, block_size, }; buf.set_length(buf.capacity()); @@ -2175,7 +2189,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(self.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. diff --git a/examples/src/bin/spi_loopback_dma_psram.rs b/examples/src/bin/spi_loopback_dma_psram.rs index cb5aca81251..5576c105a37 100644 --- a/examples/src/bin/spi_loopback_dma_psram.rs +++ b/examples/src/bin/spi_loopback_dma_psram.rs @@ -51,8 +51,8 @@ macro_rules! dma_alloc_buffer { // - if this is <= 8192 when chunk_size is 4032!?!? // - varing either DMA_CHUNK_SIZE or DMA_BUFFER_SIZE seems change the behavior const DMA_BUFFER_SIZE: usize = 16384; // the first request fails is this is <= 8192 when chunk_size is 4032!?!? -const DMA_CHUNK_SIZE: usize = 4032; // size is aligned to 64 bytes const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size16; +const DMA_CHUNK_SIZE: usize = 4096 - DMA_ALIGNMENT as usize; #[entry] fn main() -> ! { @@ -80,13 +80,8 @@ fn main() -> ! { tx_buffer.len(), tx_descriptors.len() ); - let mut dma_tx_buf = DmaTxBuf::new_with_chunk_size( - tx_descriptors, - tx_buffer, - DMA_CHUNK_SIZE, - Some(DMA_ALIGNMENT), - ) - .unwrap(); + let mut dma_tx_buf = + DmaTxBuf::new_with_block_size(tx_descriptors, tx_buffer, Some(DMA_ALIGNMENT)).unwrap(); let (rx_buffer, rx_descriptors, _, _) = esp_hal::dma_buffers!(DMA_BUFFER_SIZE, 0); info!( "RX: {:p} len {} ({} descripters)", diff --git a/hil-test/tests/spi_half_duplex_write_psram.rs b/hil-test/tests/spi_half_duplex_write_psram.rs index 9d028084676..abeb4110dde 100644 --- a/hil-test/tests/spi_half_duplex_write_psram.rs +++ b/hil-test/tests/spi_half_duplex_write_psram.rs @@ -97,14 +97,13 @@ mod tests { #[timeout(3)] fn test_spi_writes_are_correctly_by_pcnt(ctx: Context) { const DMA_BUFFER_SIZE: usize = 4; - const DMA_CHUNK_SIZE: usize = 4032; // 64 byte aligned const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size32; // matches dcache line size + const DMA_CHUNK_SIZE: usize = 4096 - DMA_ALIGNMENT as usize; // 64 byte aligned let (_, descriptors) = dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); let buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize); let mut dma_tx_buf = - DmaTxBuf::new_with_chunk_size(descriptors, buffer, DMA_CHUNK_SIZE, Some(DMA_ALIGNMENT)) - .unwrap(); + DmaTxBuf::new_with_block_size(descriptors, buffer, Some(DMA_ALIGNMENT)).unwrap(); let unit = ctx.pcnt_unit; let mut spi = ctx.spi; @@ -148,14 +147,13 @@ mod tests { #[timeout(3)] fn test_spidmabus_writes_are_correctly_by_pcnt(ctx: Context) { const DMA_BUFFER_SIZE: usize = 4; - const DMA_CHUNK_SIZE: usize = 4032; // 64 byte aligned const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size32; // matches dcache line size + const DMA_CHUNK_SIZE: usize = 4096 - DMA_ALIGNMENT as usize; // 64 byte aligned let (_, descriptors) = dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); let buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize); let dma_tx_buf = - DmaTxBuf::new_with_chunk_size(descriptors, buffer, DMA_CHUNK_SIZE, Some(DMA_ALIGNMENT)) - .unwrap(); + DmaTxBuf::new_with_block_size(descriptors, buffer, Some(DMA_ALIGNMENT)).unwrap(); let (rx, rxd, _, _) = dma_buffers!(1, 0); let dma_rx_buf = DmaRxBuf::new(rxd, rx).unwrap(); From 5b2b4c9f82cf6f02baba2851754a5a0d3e4bfc08 Mon Sep 17 00:00:00 2001 From: liebman Date: Wed, 18 Sep 2024 14:22:05 -0700 Subject: [PATCH 08/24] return error in `prepare_transfer` if psram is found on non-esp32s3 add `dma_tx_buffer` macro --- esp-hal/src/dma/mod.rs | 75 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 67 insertions(+), 8 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 91715bc6ff6..baf4ff354e9 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -591,6 +591,42 @@ macro_rules! dma_circular_descriptors_chunk_size { }; } +/// Convenience macro to create a DmaTxBuf from buffer size and optional +/// alignment. The buffer and descriptors are statically allocated used to +/// create the `DmaTxBuf` with the passed alignment. With one ardument, the +/// alignment is set to `None`. +/// +/// ## 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, Some(DmaBufBlkSize::Size32)); +/// # } +/// ``` +#[macro_export] +macro_rules! dma_tx_buffer { + ($tx_size:expr, $tx_align:expr) => {{ + const TX_DESCRIPTOR_LEN: usize = $crate::dma::DmaTxBuf::compute_descriptor_count($tx_size, $tx_align); + static mut TX_BUFFER: [u8; $tx_size] = [0u8; $tx_size]; + static mut TX_DESCRIPTORS: [$crate::dma::DmaDescriptor; TX_DESCRIPTOR_LEN] = + [$crate::dma::DmaDescriptor::EMPTY; TX_DESCRIPTOR_LEN]; + let tx_buffer, tx_descriptors = unsafe { + ( + &mut TX_BUFFER, + &mut TX_DESCRIPTORS, + ) + }; + $crate::dma::DmaTxBuf::new_with_block_size(tx_descriptors, tx_buffer, $tx_align) + }}; + + ($tx_size:expr) => { + $crate::dma_tx_buffer!($tx_size, None) + }; +} + /// DMA Errors #[derive(Debug, Clone, Copy, PartialEq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] @@ -1763,8 +1799,17 @@ where self.set_ext_mem_block_size(block_size.into()); } } else { - if is_slice_in_psram(self.buffer) { - return Err(DmaError::UnsupportedMemoryRegion); + // we check here to insure that no descriptor points to PSRAM as + // that is only supported on esp32s3 + let mut descriptor = Some(*preparation.start); + while let Some(current) = descriptor { + if is_slice_in_psram(core::slice::from_raw_parts(current.buffer, current.len())) { + return Err(DmaError::UnsupportedMemoryRegion); + } + descriptor = match current.next { + next if !next.is_null() => Some(unsafe { *next }), + _ => None, + }; } } ); @@ -2003,6 +2048,7 @@ 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, // burst_mode, alignment, check_owner, etc. } @@ -2097,6 +2143,23 @@ impl DmaTxBuf { 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) -> usize { + match block_size { + Some(size) => 4096 - size as usize, + None => 4095, // TODO: does this need to be chip specific? + } + } + + /// 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, + ) -> 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. @@ -2110,12 +2173,8 @@ impl DmaTxBuf { buffer: &'static mut [u8], block_size: Option, ) -> Result { - let chunk_size = match block_size { - Some(size) => 4096 - size as usize, - None => 4095, // max chunk_siize - }; - - let min_descriptors = buffer.len().div_ceil(chunk_size); + 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); } From 12acacdb1e35de6cff2fa147021f169ad8ad7f43 Mon Sep 17 00:00:00 2001 From: liebman Date: Wed, 18 Sep 2024 14:34:31 -0700 Subject: [PATCH 09/24] missing parens --- esp-hal/src/dma/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index baf4ff354e9..29c75564875 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -613,7 +613,7 @@ macro_rules! dma_tx_buffer { static mut TX_BUFFER: [u8; $tx_size] = [0u8; $tx_size]; static mut TX_DESCRIPTORS: [$crate::dma::DmaDescriptor; TX_DESCRIPTOR_LEN] = [$crate::dma::DmaDescriptor::EMPTY; TX_DESCRIPTOR_LEN]; - let tx_buffer, tx_descriptors = unsafe { + let (tx_buffer, tx_descriptors) = unsafe { ( &mut TX_BUFFER, &mut TX_DESCRIPTORS, From 9e2067db719fe5ea067751d093c0b75d3a055d81 Mon Sep 17 00:00:00 2001 From: liebman Date: Wed, 18 Sep 2024 15:34:30 -0700 Subject: [PATCH 10/24] changelog --- esp-hal/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 2ed4b75424f..7b20c9826e8 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - MSRV bump to 1.79 (#2156) - 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) +- Change `DmaTxBuf` to support PSRAM on `esp32s3` (#2161) ### Fixed From 9fce2f4371f8246366435926fec17aff631d6b4d Mon Sep 17 00:00:00 2001 From: liebman Date: Wed, 18 Sep 2024 15:35:50 -0700 Subject: [PATCH 11/24] default 4092 for esp32 & fmt --- esp-hal/src/dma/mod.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 29c75564875..f44b76d44d8 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -609,16 +609,12 @@ macro_rules! dma_circular_descriptors_chunk_size { #[macro_export] macro_rules! dma_tx_buffer { ($tx_size:expr, $tx_align:expr) => {{ - const TX_DESCRIPTOR_LEN: usize = $crate::dma::DmaTxBuf::compute_descriptor_count($tx_size, $tx_align); + const TX_DESCRIPTOR_LEN: usize = + $crate::dma::DmaTxBuf::compute_descriptor_count($tx_size, $tx_align); static mut TX_BUFFER: [u8; $tx_size] = [0u8; $tx_size]; static mut TX_DESCRIPTORS: [$crate::dma::DmaDescriptor; TX_DESCRIPTOR_LEN] = [$crate::dma::DmaDescriptor::EMPTY; TX_DESCRIPTOR_LEN]; - let (tx_buffer, tx_descriptors) = unsafe { - ( - &mut TX_BUFFER, - &mut TX_DESCRIPTORS, - ) - }; + let (tx_buffer, tx_descriptors) = unsafe { (&mut TX_BUFFER, &mut TX_DESCRIPTORS) }; $crate::dma::DmaTxBuf::new_with_block_size(tx_descriptors, tx_buffer, $tx_align) }}; @@ -2147,7 +2143,10 @@ impl DmaTxBuf { pub const fn compute_chunk_size(block_size: Option) -> usize { match block_size { Some(size) => 4096 - size as usize, - None => 4095, // TODO: does this need to be chip specific? + #[cfg(esp32)] + None => 4092, // esp32 requires 4 byte alignment + #[cfg(not(esp32))] + None => 4095, } } From b264ee2118754b3dff107d97fee535a6f9b35592 Mon Sep 17 00:00:00 2001 From: liebman Date: Thu, 19 Sep 2024 09:42:42 -0700 Subject: [PATCH 12/24] no errors anymode --- examples/src/bin/spi_loopback_dma_psram.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/examples/src/bin/spi_loopback_dma_psram.rs b/examples/src/bin/spi_loopback_dma_psram.rs index 5576c105a37..6b3ebf4b4a5 100644 --- a/examples/src/bin/spi_loopback_dma_psram.rs +++ b/examples/src/bin/spi_loopback_dma_psram.rs @@ -47,11 +47,8 @@ macro_rules! dma_alloc_buffer { }}; } -// TODO: the fist transfer fails in some conditions: -// - if this is <= 8192 when chunk_size is 4032!?!? -// - varing either DMA_CHUNK_SIZE or DMA_BUFFER_SIZE seems change the behavior -const DMA_BUFFER_SIZE: usize = 16384; // the first request fails is this is <= 8192 when chunk_size is 4032!?!? -const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size16; +const DMA_BUFFER_SIZE: usize = 8192; +const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size64; const DMA_CHUNK_SIZE: usize = 4096 - DMA_ALIGNMENT as usize; #[entry] From 1e62e9157f8981ac9b6d82650559cafc7b296496 Mon Sep 17 00:00:00 2001 From: liebman Date: Thu, 19 Sep 2024 12:30:00 -0700 Subject: [PATCH 13/24] use block_size is_some to flag invalid psram in prepare_transfer --- esp-hal/src/dma/mod.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index f44b76d44d8..8ca75741945 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -1795,17 +1795,9 @@ where self.set_ext_mem_block_size(block_size.into()); } } else { - // we check here to insure that no descriptor points to PSRAM as - // that is only supported on esp32s3 - let mut descriptor = Some(*preparation.start); - while let Some(current) = descriptor { - if is_slice_in_psram(core::slice::from_raw_parts(current.buffer, current.len())) { - return Err(DmaError::UnsupportedMemoryRegion); - } - descriptor = match current.next { - next if !next.is_null() => Some(unsafe { *next }), - _ => None, - }; + // we insure that block_size is some only for PSRAM addresses + if preperation.block_size.is_some() { + return Err(DmaError::UnsupportedMemoryRegion); } } ); @@ -2211,7 +2203,10 @@ impl DmaTxBuf { 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) } From 510b392a35a26dd5f157b0d85bc251eae48928d2 Mon Sep 17 00:00:00 2001 From: liebman Date: Thu, 19 Sep 2024 12:32:43 -0700 Subject: [PATCH 14/24] drop block_size from macro, the buffer allocation was not being aligned - its not needed for dram anyway. --- esp-hal/src/dma/mod.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 8ca75741945..e9a82b667cc 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -591,10 +591,8 @@ macro_rules! dma_circular_descriptors_chunk_size { }; } -/// Convenience macro to create a DmaTxBuf from buffer size and optional -/// alignment. The buffer and descriptors are statically allocated used to -/// create the `DmaTxBuf` with the passed alignment. With one ardument, the -/// alignment is set to `None`. +/// 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 @@ -608,19 +606,15 @@ macro_rules! dma_circular_descriptors_chunk_size { /// ``` #[macro_export] macro_rules! dma_tx_buffer { - ($tx_size:expr, $tx_align:expr) => {{ + ($tx_size:expr) => {{ const TX_DESCRIPTOR_LEN: usize = - $crate::dma::DmaTxBuf::compute_descriptor_count($tx_size, $tx_align); + $crate::dma::DmaTxBuf::compute_descriptor_count($tx_size, None); static mut TX_BUFFER: [u8; $tx_size] = [0u8; $tx_size]; static mut TX_DESCRIPTORS: [$crate::dma::DmaDescriptor; TX_DESCRIPTOR_LEN] = [$crate::dma::DmaDescriptor::EMPTY; TX_DESCRIPTOR_LEN]; let (tx_buffer, tx_descriptors) = unsafe { (&mut TX_BUFFER, &mut TX_DESCRIPTORS) }; - $crate::dma::DmaTxBuf::new_with_block_size(tx_descriptors, tx_buffer, $tx_align) + $crate::dma::DmaTxBuf::new(tx_descriptors, tx_buffer) }}; - - ($tx_size:expr) => { - $crate::dma_tx_buffer!($tx_size, None) - }; } /// DMA Errors From af58a615c88353787d56a8ab5f146d719db080d2 Mon Sep 17 00:00:00 2001 From: liebman Date: Thu, 19 Sep 2024 12:34:16 -0700 Subject: [PATCH 15/24] missed macro example --- esp-hal/src/dma/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index e9a82b667cc..8599ce65c19 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -601,7 +601,7 @@ macro_rules! dma_circular_descriptors_chunk_size { /// use esp_hal::dma::DmaBufBlkSize; /// /// let tx_buf = -/// dma_tx_buffer!(32000, Some(DmaBufBlkSize::Size32)); +/// dma_tx_buffer!(32000); /// # } /// ``` #[macro_export] From 70e98d782ce9bd7888cb774f1d4a37ed5a039a55 Mon Sep 17 00:00:00 2001 From: liebman Date: Thu, 19 Sep 2024 12:42:51 -0700 Subject: [PATCH 16/24] use defmt::Format that decodes owner like Debug --- esp-hal/src/dma/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 8599ce65c19..e62c60be2b8 100644 --- a/esp-hal/src/dma/mod.rs +++ b/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; From fa7cce0c92c98b6908b04dca192f359037e732c1 Mon Sep 17 00:00:00 2001 From: liebman Date: Thu, 19 Sep 2024 13:15:44 -0700 Subject: [PATCH 17/24] fix typo --- esp-hal/src/dma/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index e62c60be2b8..4f50d378923 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -299,11 +299,9 @@ use enumset::{EnumSet, EnumSetType}; pub use self::gdma::*; #[cfg(pdma)] pub use self::pdma::*; -use crate::{ - interrupt::InterruptHandler, - soc::{is_slice_in_dram, is_slice_in_psram}, - Mode, -}; +#[cfg(esp32s3)] +use crate::soc::is_slice_in_psram; +use crate::{interrupt::InterruptHandler, soc::is_slice_in_dram, Mode}; #[cfg(gdma)] mod gdma; @@ -1789,7 +1787,7 @@ where } } else { // we insure that block_size is some only for PSRAM addresses - if preperation.block_size.is_some() { + if preparation.block_size.is_some() { return Err(DmaError::UnsupportedMemoryRegion); } } From 6c9ff4c5e285fd610fd0eb9094fcc7e620ccda75 Mon Sep 17 00:00:00 2001 From: liebman Date: Fri, 20 Sep 2024 05:10:20 -0700 Subject: [PATCH 18/24] DmaTxBuf: its an error if buffer is in psram and block_size is none --- esp-hal/src/dma/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 4f50d378923..0be2ddbfc90 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -2172,6 +2172,10 @@ impl DmaTxBuf { 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 block_size.is_none() { + return Err(DmaBufError::InvalidAlignment); + } } else { // buffer can only be DRAM if !is_slice_in_dram(buffer) { From d18fea77c73a35428e558bacc6277801bb1eae26 Mon Sep 17 00:00:00 2001 From: liebman Date: Fri, 20 Sep 2024 06:25:39 -0700 Subject: [PATCH 19/24] DmaTxBuf: its an error if buffer is in psram and block_size is none --- esp-hal/src/dma/mod.rs | 2 +- hil-test/tests/spi_half_duplex_write_psram.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 0be2ddbfc90..2caa4a29fdd 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -2173,7 +2173,7 @@ impl DmaTxBuf { return Err(DmaBufError::UnsupportedMemoryRegion); } // if its PSRAM, the block_size/alignment must be specified - if block_size.is_none() { + if is_slice_in_psram(buffer) && block_size.is_none() { return Err(DmaBufError::InvalidAlignment); } } else { diff --git a/hil-test/tests/spi_half_duplex_write_psram.rs b/hil-test/tests/spi_half_duplex_write_psram.rs index abeb4110dde..bef585923ac 100644 --- a/hil-test/tests/spi_half_duplex_write_psram.rs +++ b/hil-test/tests/spi_half_duplex_write_psram.rs @@ -97,8 +97,8 @@ mod tests { #[timeout(3)] fn test_spi_writes_are_correctly_by_pcnt(ctx: Context) { const DMA_BUFFER_SIZE: usize = 4; - const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size32; // matches dcache line size - const DMA_CHUNK_SIZE: usize = 4096 - DMA_ALIGNMENT as usize; // 64 byte aligned + const DMA_ALIGNMENT: DmaBufBlkSize = DmaBufBlkSize::Size32; + const DMA_CHUNK_SIZE: usize = 4096 - DMA_ALIGNMENT as usize; let (_, descriptors) = dma_descriptors_chunk_size!(0, DMA_BUFFER_SIZE, DMA_CHUNK_SIZE); let buffer = dma_alloc_buffer!(DMA_BUFFER_SIZE, DMA_ALIGNMENT as usize); From 37f7bff3cc3d440f8f78691daf6613a5867bb316 Mon Sep 17 00:00:00 2001 From: liebman Date: Fri, 20 Sep 2024 11:31:44 -0700 Subject: [PATCH 20/24] update for PSRAM feature changes --- examples/src/bin/spi_loopback_dma_psram.rs | 4 ++-- hil-test/Cargo.toml | 2 +- hil-test/tests/spi_half_duplex_write_psram.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/src/bin/spi_loopback_dma_psram.rs b/examples/src/bin/spi_loopback_dma_psram.rs index 6b3ebf4b4a5..250123752d5 100644 --- a/examples/src/bin/spi_loopback_dma_psram.rs +++ b/examples/src/bin/spi_loopback_dma_psram.rs @@ -14,9 +14,9 @@ //! data. //! //! If your module is quad PSRAM then you need to change the `psram` feature in the -//! in the features line below to `psram-2m`. +//! in the features line below to `quad-psram`. -//% FEATURES: esp-hal/log opsram-2m +//% FEATURES: esp-hal/log esp-hal/octal-psram //% CHIPS: esp32s3 #![no_std] diff --git a/hil-test/Cargo.toml b/hil-test/Cargo.toml index d431ac45f21..fff719374a9 100644 --- a/hil-test/Cargo.toml +++ b/hil-test/Cargo.toml @@ -241,7 +241,7 @@ generic-queue = [ integrated-timers = [ "esp-hal-embassy/integrated-timers", ] -psram = ["esp-hal/opsram-2m", "dep:esp-alloc"] +octal-psram = ["esp-hal/octal-psram", "dep:esp-alloc"] # https://doc.rust-lang.org/cargo/reference/profiles.html#test # Test and bench profiles inherit from dev and release respectively. diff --git a/hil-test/tests/spi_half_duplex_write_psram.rs b/hil-test/tests/spi_half_duplex_write_psram.rs index bef585923ac..a63d8b335e5 100644 --- a/hil-test/tests/spi_half_duplex_write_psram.rs +++ b/hil-test/tests/spi_half_duplex_write_psram.rs @@ -1,5 +1,5 @@ //! SPI Half Duplex Write Test -//% FEATURES: psram +//% FEATURES: octal-psram //% CHIPS: esp32s3 #![no_std] From 51028c664ce4baa26396d7e8ca3fc5ef441f5743 Mon Sep 17 00:00:00 2001 From: liebman Date: Tue, 24 Sep 2024 06:32:12 -0700 Subject: [PATCH 21/24] address alignment comments add simple test --- esp-hal/src/dma/mod.rs | 14 ++++++++++++-- hil-test/tests/dma_macros.rs | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 67b29c95661..7c57073a763 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -653,10 +653,11 @@ macro_rules! dma_tx_buffer { ($tx_size:expr) => {{ const TX_DESCRIPTOR_LEN: usize = $crate::dma::DmaTxBuf::compute_descriptor_count($tx_size, None); - static mut TX_BUFFER: [u8; $tx_size] = [0u8; $tx_size]; + $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, tx_descriptors) = unsafe { (&mut TX_BUFFER, &mut TX_DESCRIPTORS) }; + 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) }}; } @@ -1928,6 +1929,7 @@ 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, @@ -2034,6 +2036,14 @@ impl DmaTxBuf { 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); diff --git a/hil-test/tests/dma_macros.rs b/hil-test/tests/dma_macros.rs index e3c9502ef7d..46106e9e0f9 100644 --- a/hil-test/tests/dma_macros.rs +++ b/hil-test/tests/dma_macros.rs @@ -201,4 +201,27 @@ mod tests { compute_circular_size(TX_SIZE, CHUNK_SIZE) ); } + + #[test] + fn test_dma_tx_buffer() { + use esp_hal::dma::DmaTxBuf; + use esp_hal::dma::DmaBufError; + const TX_SIZE: usize = DATA_SIZE; + + fn check(result: Result, size: usize) { + match result { + Ok(tx_buf) => { + assert_eq!(tx_buf.len(), size); + } + Err(_) => { + panic!("Failed to create DmaTxBuf"); + } + } + } + check(esp_hal::dma_tx_buffer!(TX_SIZE), TX_SIZE); + check(esp_hal::dma_tx_buffer!(TX_SIZE+1), TX_SIZE+1); + check(esp_hal::dma_tx_buffer!(TX_SIZE+2), TX_SIZE+2); + check(esp_hal::dma_tx_buffer!(TX_SIZE+3), TX_SIZE+3); + } + } From d452ef6d002b1a67765766a79872b7781c2546c1 Mon Sep 17 00:00:00 2001 From: liebman Date: Tue, 24 Sep 2024 06:50:33 -0700 Subject: [PATCH 22/24] fmt --- hil-test/tests/dma_macros.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/hil-test/tests/dma_macros.rs b/hil-test/tests/dma_macros.rs index 46106e9e0f9..bc3a7bbf414 100644 --- a/hil-test/tests/dma_macros.rs +++ b/hil-test/tests/dma_macros.rs @@ -204,8 +204,7 @@ mod tests { #[test] fn test_dma_tx_buffer() { - use esp_hal::dma::DmaTxBuf; - use esp_hal::dma::DmaBufError; + use esp_hal::dma::{DmaBufError, DmaTxBuf}; const TX_SIZE: usize = DATA_SIZE; fn check(result: Result, size: usize) { @@ -219,9 +218,8 @@ mod tests { } } check(esp_hal::dma_tx_buffer!(TX_SIZE), TX_SIZE); - check(esp_hal::dma_tx_buffer!(TX_SIZE+1), TX_SIZE+1); - check(esp_hal::dma_tx_buffer!(TX_SIZE+2), TX_SIZE+2); - check(esp_hal::dma_tx_buffer!(TX_SIZE+3), TX_SIZE+3); + check(esp_hal::dma_tx_buffer!(TX_SIZE + 1), TX_SIZE + 1); + check(esp_hal::dma_tx_buffer!(TX_SIZE + 2), TX_SIZE + 2); + check(esp_hal::dma_tx_buffer!(TX_SIZE + 3), TX_SIZE + 3); } - } From b24f0660d1c5cfcb101e5b1b2ab9b6384dafb9b1 Mon Sep 17 00:00:00 2001 From: liebman Date: Tue, 24 Sep 2024 07:25:38 -0700 Subject: [PATCH 23/24] better alignment test --- esp-hal/src/dma/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 7c57073a763..70e92614f8e 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -2037,7 +2037,7 @@ impl DmaTxBuf { } } else { #[cfg(any(esp32,esp32s2))] - if buffer.len() % 4 != 0 && buffer.as_ptr() as usize % 4 != 0 { + if buffer.len() % 4 != 0 && buffer.as_ptr().is_aligned_to(4) { // 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 From 65adcdfb9985e57fd7b5996f332a98698bd7260d Mon Sep 17 00:00:00 2001 From: liebman Date: Tue, 24 Sep 2024 07:56:46 -0700 Subject: [PATCH 24/24] revert alignment test --- esp-hal/src/dma/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 70e92614f8e..7c57073a763 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -2037,7 +2037,7 @@ impl DmaTxBuf { } } else { #[cfg(any(esp32,esp32s2))] - if buffer.len() % 4 != 0 && buffer.as_ptr().is_aligned_to(4) { + 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