From 1837bca9c9e40daaa8dfa26184d49f0d9f82428a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 29 Apr 2026 17:34:50 +0200 Subject: [PATCH 01/15] Use DmaBuffer API for I2S --- esp-hal/src/dma/buffers/mod.rs | 241 +++- esp-hal/src/dma/mod.rs | 1068 ++---------------- esp-hal/src/i2s/master.rs | 783 ++++++------- hil-test/src/bin/embassy_timers_executors.rs | 17 +- hil-test/src/bin/i2s.rs | 171 +-- qa-test/src/bin/embassy_i2s_read.rs | 40 +- qa-test/src/bin/embassy_i2s_sound.rs | 49 +- qa-test/src/bin/embassy_wifi_i2s.rs | 75 +- 8 files changed, 787 insertions(+), 1657 deletions(-) diff --git a/esp-hal/src/dma/buffers/mod.rs b/esp-hal/src/dma/buffers/mod.rs index 856f8e6e3bb..d64bddc4f37 100644 --- a/esp-hal/src/dma/buffers/mod.rs +++ b/esp-hal/src/dma/buffers/mod.rs @@ -995,6 +995,8 @@ unsafe impl DmaRxBuffer for DmaRxTxBuf { /// /// See [DmaRxStreamBufView] for APIs available whilst a transfer is in /// progress. +#[derive(Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub struct DmaRxStreamBuf { descriptors: &'static mut [DmaDescriptor], buffer: &'static mut [u8], @@ -1015,7 +1017,9 @@ impl DmaRxStreamBuf { return Err(DmaBufError::UnsupportedMemoryRegion); } - if descriptors.is_empty() { + // see https://github.com/esp-rs/esp-hal/issues/2269#issuecomment-4397953660 + // we can lift that requirement once we sort out this issue + if descriptors.len() < 4 { return Err(DmaBufError::InsufficientDescriptors); } @@ -1238,8 +1242,9 @@ impl DmaRxStreamBufView { (&[], false) } else { let length = last_descriptor.len() - self.descriptor_offset; + let chunk_size = last_descriptor.size(); ( - &self.buf.buffer[self.buf.buffer.len() - length..], + &self.buf.buffer[self.buf.buffer.len() - chunk_size..][..length], last_descriptor.flags.suc_eof(), ) } @@ -1273,6 +1278,238 @@ impl DmaRxStreamBufView { } } +/// DMA Streaming Transmit Buffer. +/// +/// This is symmetric implementation to [DmaRxStreamBuf], used for continuously +/// streaming data to a peripheral's FIFO. +/// +/// The list starts out like so `A(full) -> B(full) -> C(full) -> D(full) -> NULL`. +/// +/// As the DMA writes to FIFO, the list progresses like so: +/// - `A(full) -> B(full) -> C(full) -> D(full) -> NULL` +/// - `A(empty) -> B(full) -> C(full) -> D(full) -> NULL` +/// - `A(empty) -> B(empty) -> C(full) -> D(full) -> NULL` +/// - `A(empty) -> B(empty) -> C(empty) -> D(full) -> NULL` +/// +/// As you call [DmaTxStreamBufView::push] the list (approximately) progresses like so: +/// - `A(empty) -> B(empty) -> C(empty) -> D(full) -> NULL` +/// - `B(empty) -> C(empty) -> D(full) -> A(full) -> NULL` +/// - `C(empty) -> D(full) -> A(full) -> B(full) -> NULL` +/// - `D(full) -> A(full) -> B(full) -> C(full) -> NULL` +/// +/// If all the descriptors run out, the [DmaTxInterrupt::TotalEof] interrupt will fire and DMA +/// will stop writing, at which point it is up to you to resume/restart the transfer. +#[derive(Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub struct DmaTxStreamBuf { + descriptors: &'static mut [DmaDescriptor], + buffer: &'static mut [u8], + burst: BurstConfig, + pre_filled: Option, +} + +impl DmaTxStreamBuf { + /// Creates a new [DmaTxStreamBuf] evenly distributing the buffer between + /// the provided descriptors. + pub fn new( + descriptors: &'static mut [DmaDescriptor], + buffer: &'static mut [u8], + ) -> Result { + match () { + _ if !is_slice_in_dram(descriptors) => Err(DmaBufError::UnsupportedMemoryRegion)?, + _ if !is_slice_in_dram(buffer) => Err(DmaBufError::UnsupportedMemoryRegion)?, + _ if descriptors.len() < 4 => { + // see https://github.com/esp-rs/esp-hal/issues/2269#issuecomment-4397953660 + // we can lift that requirement once we sort out this issue + Err(DmaBufError::InsufficientDescriptors)? + } + _ => (), + }; + + // Evenly distribute the buffer between the descriptors. + let chunk_size = Some(buffer.len() / descriptors.len()) + .filter(|x| *x <= 4095) + .ok_or(DmaBufError::InsufficientDescriptors)?; + + let mut chunks = buffer.chunks_exact_mut(chunk_size); + for (desc, chunk) in descriptors.iter_mut().zip(chunks.by_ref()) { + desc.buffer = chunk.as_mut_ptr(); + desc.set_size(chunk.len()); + desc.set_length(chunk.len()); + } + let remainder = chunks.into_remainder(); + + if !remainder.is_empty() { + // Append any excess to the last descriptor. + let last_descriptor = descriptors.last_mut().unwrap(); + let size = last_descriptor.size() + remainder.len(); + if size > 4095 { + Err(DmaBufError::InsufficientDescriptors)?; + } + last_descriptor.set_size(size); + } + + Ok(Self { + descriptors, + buffer, + burst: Default::default(), + pre_filled: None, + }) + } + + /// Consume the buf, returning the descriptors and buffer. + pub fn split(self) -> (&'static mut [DmaDescriptor], &'static mut [u8]) { + (self.descriptors, self.buffer) + } + + /// Push the buffer with the given data before DMA transfer starts. + /// + /// Otherwise the streaming buffer will transfer garbage data to + /// DMA so that CPU has enough time to fill the buffer after transfer starts. + pub fn push(&mut self, data: &[u8]) -> usize { + self.push_with(|buf| { + let len = buf.len().min(data.len()); + buf[..len].copy_from_slice(&data[..len]); + len + }) + } + + /// Push the buffer with the given data before DMA transfer starts. + /// + /// Returns the number of bytes filled. + pub fn push_with(&mut self, f: impl FnOnce(&mut [u8]) -> usize) -> usize { + let start = self.pre_filled.unwrap_or(0); + let bytes_pushed = f(&mut self.buffer[start..]); + self.pre_filled = Some(start + bytes_pushed); + bytes_pushed + } +} + +unsafe impl DmaTxBuffer for DmaTxStreamBuf { + type View = DmaTxStreamBufView; + type Final = Self; + + fn prepare(&mut self) -> Preparation { + // Link up all the descriptors (but not in a circle). + let mut next = null_mut(); + for desc in self.descriptors.iter_mut().rev() { + desc.next = next; + next = desc; + + desc.reset_for_tx(true); + } + + Preparation { + start: self.descriptors.as_mut_ptr(), + direction: TransferDirection::Out, + #[cfg(dma_can_access_psram)] + accesses_psram: false, + burst_transfer: self.burst, + + // Whilst we give ownership of the descriptors the DMA, the correctness of this buffer + // implementation doesn't rely on the DMA checking for descriptor ownership. + // No descriptor is added back to the end of the stream before it's ready for the DMA + // to consume it. + check_owner: None, + auto_write_back: true, + } + } + + fn into_view(self) -> Self::View { + // If the buffer is not pre-filled, treat it as fully filled + let pre_filled = self.pre_filled.unwrap_or(self.buffer.len()); + let mut view = DmaTxStreamBufView { + buf: self, + descriptor_idx: 0, + descriptor_offset: 0, + }; + view.advance(pre_filled); + view + } + + fn from_view(view: Self::View) -> Self { + view.buf + } +} + +/// A view into a [DmaTxStreamBuf] +pub struct DmaTxStreamBufView { + buf: DmaTxStreamBuf, + descriptor_idx: usize, + descriptor_offset: usize, +} + +impl DmaTxStreamBufView { + /// Returns the number of bytes available for writing. + pub fn available_bytes(&self) -> usize { + let (tail, head) = self.buf.descriptors.split_at(self.descriptor_idx); + head.iter() + .chain(tail) + .take_while(|d| d.owner() == Owner::Cpu) + .map(|d| d.size()) + .sum::() + - self.descriptor_offset + } + + /// Pushes a buffer into the stream buffer. + /// Returns the number of bytes pushed. + pub fn push_with(&mut self, f: impl FnOnce(&mut [u8]) -> usize) -> usize { + let chunk_size = self.buf.descriptors[0].size(); + let dma_start = self.descriptor_idx * chunk_size + self.descriptor_offset; + let dma_end = (dma_start + self.available_bytes()).min(self.buf.buffer.len()); + let bytes_pushed = f(&mut self.buf.buffer[dma_start..dma_end]); + + self.advance(bytes_pushed); + bytes_pushed + } + + /// Advances the first `n` bytes from the available data + pub fn advance(&mut self, bytes_pushed: usize) { + let mut bytes_filled = 0; + for d in (self.descriptor_idx..self.buf.descriptors.len()).chain(core::iter::once(0)) { + let desc = &mut self.buf.descriptors[d]; + let bytes_in_d = desc.size() - self.descriptor_offset; + // There is at least one byte left in `desc`. + if bytes_in_d + bytes_filled > bytes_pushed { + self.descriptor_idx = d; + self.descriptor_offset = self.descriptor_offset + bytes_pushed - bytes_filled; + break; + } + bytes_filled += bytes_in_d; + self.descriptor_offset = 0; + + // Put the current descriptor at the end of the list + desc.set_owner(Owner::Dma); + desc.set_length(desc.size()); + desc.set_suc_eof(true); + let p = d.checked_sub(1).unwrap_or(self.buf.descriptors.len() - 1); + if p != d { + let [prev, desc] = self.buf.descriptors.get_disjoint_mut([p, d]).unwrap(); + desc.next = null_mut(); + prev.next = desc; + } + } + } + + /// Pushes a buffer into the stream buffer. + /// Returns the number of bytes pushed. + pub fn push(&mut self, data: &[u8]) -> usize { + let total_len = data.len(); + let mut remaining = data; + + while self.available_bytes() >= remaining.len() && !remaining.is_empty() { + let written = self.push_with(|buffer| { + let len = usize::min(buffer.len(), remaining.len()); + buffer[..len].copy_from_slice(&remaining[..len]); + len + }); + remaining = &remaining[written..]; + } + + total_len - remaining.len() + } +} + static mut EMPTY: [DmaDescriptor; 1] = [DmaDescriptor::EMPTY]; /// An empty buffer that can be used when you don't need to transfer any data. diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 001d07f85b8..de0e1b7f57f 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -73,7 +73,7 @@ use crate::{ DriverMode, interrupt::InterruptHandler, peripherals::Interrupt, - soc::{is_slice_in_dram, is_valid_memory_address, is_valid_ram_address}, + soc::is_slice_in_dram, system::{self, Cpu}, }; @@ -780,6 +780,36 @@ macro_rules! dma_rx_stream_buffer { }}; } +#[procmacros::doc_replace] +/// Convenience macro to create a [DmaTxStreamBuf] from buffer size and +/// optional chunk size (uses max if unspecified). +/// The buffer and descriptors are statically allocated and +/// used to create the [DmaTxStreamBuf]. +/// +/// Smaller chunk sizes are recommended for lower latency. +/// +/// ## Usage +/// ```rust,no_run +/// # {before_snippet} +/// use esp_hal::dma_tx_stream_buffer; +/// +/// let buf = dma_tx_stream_buffer!(32000); +/// let buf = dma_tx_stream_buffer!(32000, 1000); +/// # {after_snippet} +/// ``` +#[macro_export] +macro_rules! dma_tx_stream_buffer { + ($tx_size:expr) => { + $crate::dma_tx_stream_buffer!($tx_size, 4095) + }; + ($tx_size:expr, $chunk_size:expr) => {{ + let (buffer, descriptors) = + $crate::dma_buffers_impl!($tx_size, $chunk_size, is_circular = false); + + $crate::dma::DmaTxStreamBuf::new(descriptors, buffer).unwrap() + }}; +} + #[procmacros::doc_replace] /// Convenience macro to create a [DmaLoopBuf] from a buffer size. /// @@ -825,9 +855,6 @@ pub enum DmaError { UnsupportedMemoryRegion, /// Invalid DMA chunk size InvalidChunkSize, - /// Indicates writing to or reading from a circular DMA transaction is done - /// too late and the DMA buffers already overrun / underrun. - Late, } impl From for DmaError { @@ -917,115 +944,6 @@ pub trait DmaEligible { fn dma_peripheral(&self) -> DmaPeripheral; } -#[doc(hidden)] -#[derive(Debug)] -pub struct DescriptorChain { - pub(crate) descriptors: &'static mut [DmaDescriptor], - chunk_size: usize, -} - -impl DescriptorChain { - pub fn new(descriptors: &'static mut [DmaDescriptor]) -> Self { - Self::new_with_chunk_size(descriptors, CHUNK_SIZE) - } - - pub fn new_with_chunk_size( - descriptors: &'static mut [DmaDescriptor], - chunk_size: usize, - ) -> Self { - Self { - descriptors, - chunk_size, - } - } - - pub fn first_mut(&mut self) -> *mut DmaDescriptor { - self.descriptors.as_mut_ptr() - } - - pub fn first(&self) -> *const DmaDescriptor { - self.descriptors.as_ptr() - } - - pub fn last_mut(&mut self) -> *mut DmaDescriptor { - self.descriptors.last_mut().unwrap() - } - - pub fn last(&self) -> *const DmaDescriptor { - self.descriptors.last().unwrap() - } - - pub fn fill_for_rx( - &mut self, - circular: bool, - data: *mut u8, - len: usize, - ) -> Result<(), DmaError> { - self.fill(circular, data, len, |desc, _| { - desc.reset_for_rx(); - // Descriptor::size has been set up by `fill` - }) - } - - pub fn fill_for_tx( - &mut self, - is_circular: bool, - data: *const u8, - len: usize, - ) -> Result<(), DmaError> { - self.fill(is_circular, data.cast_mut(), len, |desc, chunk_size| { - // In circular mode, we set the `suc_eof` bit for every buffer we send. We use - // this for I2S to track progress of a transfer by checking OUTLINK_DSCR_ADDR. - // In non-circular mode, we only set `suc_eof` for the last descriptor to signal - // the end of the transfer. - desc.reset_for_tx(desc.next.is_null() || is_circular); - desc.set_length(chunk_size); // align to 32 bits? - }) - } - - #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub fn fill( - &mut self, - circular: bool, - data: *mut u8, - len: usize, - prepare_descriptor: impl Fn(&mut DmaDescriptor, usize), - ) -> Result<(), DmaError> { - if !is_valid_ram_address(self.first() as usize) - || !is_valid_ram_address(self.last() as usize) - || !is_valid_memory_address(data as usize) - || !is_valid_memory_address(unsafe { data.add(len) } as usize) - { - return Err(DmaError::UnsupportedMemoryRegion); - } - - let max_chunk_size = if circular && len <= self.chunk_size * 2 { - if len <= 3 { - return Err(DmaError::BufferTooSmall); - } - len / 3 + len % 3 - } else { - self.chunk_size - }; - - DescriptorSet::set_up_buffer_ptrs( - unsafe { core::slice::from_raw_parts_mut(data, len) }, - self.descriptors, - max_chunk_size, - circular, - )?; - DescriptorSet::set_up_descriptors( - self.descriptors, - len, - max_chunk_size, - circular, - prepare_descriptor, - )?; - - Ok(()) - } -} - /// Computes the number of descriptors required for a given buffer size with /// a given chunk size. pub const fn descriptor_count(buffer_size: usize, chunk_size: usize, is_circular: bool) -> usize { @@ -1258,274 +1176,6 @@ impl From for DmaExtMemBKSize { } } -pub(crate) struct TxCircularState { - write_offset: usize, - write_descr_ptr: *mut DmaDescriptor, - pub(crate) available: usize, - last_seen_handled_descriptor_ptr: *mut DmaDescriptor, - buffer_start: *const u8, - buffer_len: usize, - - first_desc_ptr: *mut DmaDescriptor, -} - -impl TxCircularState { - pub(crate) fn new(chain: &mut DescriptorChain) -> Self { - Self { - write_offset: 0, - write_descr_ptr: chain.first_mut(), - available: 0, - last_seen_handled_descriptor_ptr: chain.first_mut(), - buffer_start: chain.descriptors[0].buffer as _, - buffer_len: chain.descriptors.iter().map(|d| d.len()).sum(), - - first_desc_ptr: chain.first_mut(), - } - } - - pub(crate) fn update(&mut self, channel: &ChannelTx) -> Result<(), DmaError> - where - Dm: DriverMode, - CH: DmaTxChannel, - { - if channel - .pending_out_interrupts() - .contains(DmaTxInterrupt::Eof) - { - channel.clear_out(DmaTxInterrupt::Eof); - - // check if all descriptors are owned by CPU - this indicates we failed to push - // data fast enough in future we can enable `check_owner` and check - // the interrupt instead - let mut current = self.last_seen_handled_descriptor_ptr; - loop { - let descr = unsafe { current.read_volatile() }; - if descr.owner() == Owner::Cpu { - current = descr.next; - } else { - break; - } - - if core::ptr::eq(current, self.last_seen_handled_descriptor_ptr) { - return Err(DmaError::Late); - } - } - - let descr_address = channel.last_out_dscr_address() as *mut DmaDescriptor; - - let mut ptr = self.last_seen_handled_descriptor_ptr; - if descr_address >= self.last_seen_handled_descriptor_ptr { - unsafe { - while ptr < descr_address { - let dw0 = ptr.read_volatile(); - self.available += dw0.len(); - ptr = ptr.offset(1); - } - } - } else { - unsafe { - while !((*ptr).next.is_null() - || core::ptr::eq((*ptr).next, self.first_desc_ptr)) - { - let dw0 = ptr.read_volatile(); - self.available += dw0.len(); - ptr = ptr.offset(1); - } - - // add bytes pointed to by the last descriptor - let dw0 = ptr.read_volatile(); - self.available += dw0.len(); - - // in circular mode we need to honor the now available bytes at start - if core::ptr::eq((*ptr).next, self.first_desc_ptr) { - ptr = self.first_desc_ptr; - while ptr < descr_address { - let dw0 = ptr.read_volatile(); - self.available += dw0.len(); - ptr = ptr.offset(1); - } - } - } - } - - if self.available >= self.buffer_len { - unsafe { - let dw0 = self.write_descr_ptr.read_volatile(); - let segment_len = dw0.len(); - let next_descriptor = dw0.next; - self.available -= segment_len; - self.write_offset = (self.write_offset + segment_len) % self.buffer_len; - - self.write_descr_ptr = if next_descriptor.is_null() { - self.first_desc_ptr - } else { - next_descriptor - } - } - } - - self.last_seen_handled_descriptor_ptr = descr_address; - } - - Ok(()) - } - - pub(crate) fn push(&mut self, data: &[u8]) -> Result { - let avail = self.available; - - if avail < data.len() { - return Err(DmaError::Overflow); - } - - let mut remaining = data.len(); - let mut offset = 0; - while self.available >= remaining && remaining > 0 { - let written = self.push_with(|buffer| { - let len = usize::min(buffer.len(), data.len() - offset); - buffer[..len].copy_from_slice(&data[offset..][..len]); - len - })?; - offset += written; - remaining -= written; - } - - Ok(data.len()) - } - - pub(crate) fn push_with( - &mut self, - f: impl FnOnce(&mut [u8]) -> usize, - ) -> Result { - // this might write less than available in case of a wrap around - // caller needs to check and write the remaining part - let written = unsafe { - let dst = self.buffer_start.add(self.write_offset).cast_mut(); - let block_size = usize::min(self.available, self.buffer_len - self.write_offset); - let buffer = core::slice::from_raw_parts_mut(dst, block_size); - f(buffer) - }; - - let mut forward = written; - loop { - unsafe { - let mut descr = self.write_descr_ptr.read_volatile(); - descr.set_owner(Owner::Dma); - self.write_descr_ptr.write_volatile(descr); - - let segment_len = descr.len(); - self.write_descr_ptr = if descr.next.is_null() { - self.first_desc_ptr - } else { - descr.next - }; - - if forward <= segment_len { - break; - } - - forward -= segment_len; - } - } - - self.write_offset = (self.write_offset + written) % self.buffer_len; - self.available -= written; - - Ok(written) - } -} - -pub(crate) struct RxCircularState { - read_descr_ptr: *mut DmaDescriptor, - pub(crate) available: usize, - last_seen_handled_descriptor_ptr: *mut DmaDescriptor, - last_descr_ptr: *mut DmaDescriptor, -} - -impl RxCircularState { - pub(crate) fn new(chain: &mut DescriptorChain) -> Self { - Self { - read_descr_ptr: chain.first_mut(), - available: 0, - last_seen_handled_descriptor_ptr: core::ptr::null_mut(), - last_descr_ptr: chain.last_mut(), - } - } - - pub(crate) fn update(&mut self) -> Result<(), DmaError> { - if self.last_seen_handled_descriptor_ptr.is_null() { - // initially start at last descriptor (so that next will be the first - // descriptor) - self.last_seen_handled_descriptor_ptr = self.last_descr_ptr; - } - - let mut current_in_descr_ptr = - unsafe { self.last_seen_handled_descriptor_ptr.read_volatile() }.next; - let mut current_in_descr = unsafe { current_in_descr_ptr.read_volatile() }; - - let last_seen_ptr = self.last_seen_handled_descriptor_ptr; - while current_in_descr.owner() == Owner::Cpu { - self.available += current_in_descr.len(); - self.last_seen_handled_descriptor_ptr = current_in_descr_ptr; - - current_in_descr_ptr = - unsafe { self.last_seen_handled_descriptor_ptr.read_volatile() }.next; - current_in_descr = unsafe { current_in_descr_ptr.read_volatile() }; - - if core::ptr::eq(current_in_descr_ptr, last_seen_ptr) { - return Err(DmaError::Late); - } - } - - Ok(()) - } - - pub(crate) fn pop(&mut self, data: &mut [u8]) -> Result { - let len = data.len(); - let mut avail = self.available; - - if avail > len { - return Err(DmaError::BufferTooSmall); - } - - let mut remaining_buffer = data; - let mut descr_ptr = self.read_descr_ptr; - - if descr_ptr.is_null() { - return Ok(0); - } - - let mut descr = unsafe { descr_ptr.read_volatile() }; - - while avail > 0 && !remaining_buffer.is_empty() && remaining_buffer.len() >= descr.len() { - unsafe { - let dst = remaining_buffer.as_mut_ptr(); - let src = descr.buffer; - let count = descr.len(); - core::ptr::copy_nonoverlapping(src, dst, count); - - descr.set_owner(Owner::Dma); - descr.set_suc_eof(false); - descr.set_length(0); - descr_ptr.write_volatile(descr); - - remaining_buffer = &mut remaining_buffer[count..]; - avail -= count; - descr_ptr = descr.next; - } - - if descr_ptr.is_null() { - break; - } - - descr = unsafe { descr_ptr.read_volatile() }; - } - - self.read_descr_ptr = descr_ptr; - self.available = avail; - Ok(len - remaining_buffer.len()) - } -} - #[doc(hidden)] macro_rules! impl_dma_eligible { ([$dma_ch:ident] $name:ident => $dma:ident) => { @@ -1857,60 +1507,12 @@ where Dm: DriverMode, CH: DmaRxChannel, { - // TODO: used by I2S, which should be rewritten to use the Preparation-based - // API. - pub(crate) unsafe fn prepare_transfer_without_start( - &mut self, - peri: DmaPeripheral, - chain: &DescriptorChain, - ) -> Result<(), DmaError> { - // We check each descriptor buffer that points to PSRAM for - // alignment and invalidate the cache for that buffer. - // NOTE: for RX the `buffer` and `size` need to be aligned but the `len` does - // not. TRM section 3.4.9 - // Note that DmaBuffer implementations are required to do this for us. - cfg_if::cfg_if! { - if #[cfg(dma_can_access_psram)] { - let mut uses_psram = false; - let psram_range = crate::psram::psram_range(); - for des in chain.descriptors.iter() { - // we are forcing the DMA alignment to the cache line size - // required when we are using dcache - let alignment = unsafe { crate::soc::cache_get_dcache_line_size() } as usize; - if crate::soc::addr_in_range(des.buffer as usize, psram_range.clone()) { - uses_psram = true; - // both the size and address of the buffer must be aligned - if !(des.buffer as usize).is_multiple_of(alignment) { - return Err(DmaError::InvalidAlignment(DmaAlignmentError::Address)); - } - if !des.size().is_multiple_of(alignment) { - return Err(DmaError::InvalidAlignment(DmaAlignmentError::Size)); - } - unsafe {crate::soc::cache_invalidate_addr(des.buffer as u32, des.size() as u32); } - } - } - } - } - - let preparation = Preparation { - start: chain.first().cast_mut(), - direction: TransferDirection::In, - #[cfg(dma_can_access_psram)] - accesses_psram: uses_psram, - burst_transfer: BurstConfig::default(), - check_owner: Some(false), - auto_write_back: true, - }; - self.do_prepare(preparation, peri) - } - pub(crate) unsafe fn prepare_transfer( &mut self, peri: DmaPeripheral, buffer: &mut BUF, ) -> Result<(), DmaError> { let preparation = buffer.prepare(); - self.do_prepare(preparation, peri) } @@ -2121,57 +1723,6 @@ where Dm: DriverMode, CH: DmaTxChannel, { - // TODO: used by I2S, which should be rewritten to use the Preparation-based - // API. - pub(crate) unsafe fn prepare_transfer_without_start( - &mut self, - peri: DmaPeripheral, - chain: &DescriptorChain, - ) -> Result<(), DmaError> { - // Based on the ESP32-S3 TRM the alignment check is not needed for TX - - // We check each descriptor buffer that points to PSRAM for - // alignment and writeback the cache for that buffer. - // Note that DmaBuffer implementations are required to do this for us. - #[cfg(dma_can_access_psram)] - cfg_if::cfg_if! { - if #[cfg(dma_can_access_psram)] { - let mut uses_psram = false; - let psram_range = crate::psram::psram_range(); - for des in chain.descriptors.iter() { - // we are forcing the DMA alignment to the cache line size - // required when we are using dcache - let alignment = unsafe { crate::soc::cache_get_dcache_line_size()} as usize; - if crate::soc::addr_in_range(des.buffer as usize, psram_range.clone()) { - uses_psram = true; - // both the size and address of the buffer must be aligned - if !(des.buffer as usize).is_multiple_of(alignment) { - return Err(DmaError::InvalidAlignment(DmaAlignmentError::Address)); - } - if !des.size().is_multiple_of(alignment) { - return Err(DmaError::InvalidAlignment(DmaAlignmentError::Size)); - } - unsafe { crate::soc::cache_writeback_addr(des.buffer as u32, des.size() as u32); } - } - } - } - } - - let preparation = Preparation { - start: chain.first().cast_mut(), - direction: TransferDirection::Out, - #[cfg(dma_can_access_psram)] - accesses_psram: uses_psram, - burst_transfer: BurstConfig::default(), - check_owner: Some(false), - // enable descriptor write back in circular mode - auto_write_back: !(unsafe { *chain.last() }).next.is_null(), - }; - self.do_prepare(preparation, peri)?; - - Ok(()) - } - pub(crate) unsafe fn prepare_transfer( &mut self, peri: DmaPeripheral, @@ -2459,349 +2010,6 @@ impl From> for Channel { } } -pub(crate) mod dma_private { - use super::*; - - pub trait DmaSupport { - type DriverMode: DriverMode; - - /// Wait until the transfer is done. - /// - /// Depending on the peripheral this might include checking the DMA - /// channel and/or the peripheral. - /// - /// After this all data should be processed by the peripheral - i.e. the - /// peripheral should have processed it's FIFO(s) - /// - /// Please note: This is called in the transfer's `wait` function _and_ - /// by it's [Drop] implementation. - fn peripheral_wait_dma(&mut self, is_rx: bool, is_tx: bool); - - /// Only used by circular DMA transfers in both, the `stop` function - /// _and_ it's [Drop] implementation - fn peripheral_dma_stop(&mut self); - } - - #[instability::unstable] - pub trait DmaSupportTx: DmaSupport { - type Channel: DmaTxChannel; - - fn tx(&mut self) -> &mut ChannelTx; - - fn chain(&mut self) -> &mut DescriptorChain; - } - - #[instability::unstable] - pub trait DmaSupportRx: DmaSupport { - type Channel: DmaRxChannel; - - fn rx(&mut self) -> &mut ChannelRx; - - fn chain(&mut self) -> &mut DescriptorChain; - } -} - -/// DMA transaction for TX only transfers -/// -/// # Safety -/// -/// Never use [core::mem::forget] on an in-progress transfer -#[non_exhaustive] -#[must_use] -#[cfg(i2s_driver_supported)] -pub struct DmaTransferTx<'a, I> -where - I: dma_private::DmaSupportTx, -{ - instance: &'a mut I, -} - -#[cfg(i2s_driver_supported)] -impl<'a, I> DmaTransferTx<'a, I> -where - I: dma_private::DmaSupportTx, -{ - pub(crate) fn new(instance: &'a mut I) -> Self { - Self { instance } - } - - /// Wait for the transfer to finish. - pub fn wait(self) -> Result<(), DmaError> { - self.instance.peripheral_wait_dma(false, true); - - if self - .instance - .tx() - .pending_out_interrupts() - .contains(DmaTxInterrupt::DescriptorError) - { - Err(DmaError::DescriptorError) - } else { - Ok(()) - } - } - - /// Check if the transfer is finished. - pub fn is_done(&mut self) -> bool { - self.instance.tx().is_done() - } -} - -#[cfg(i2s_driver_supported)] -impl Drop for DmaTransferTx<'_, I> -where - I: dma_private::DmaSupportTx, -{ - fn drop(&mut self) { - self.instance.peripheral_wait_dma(false, true); - } -} - -/// DMA transaction for RX only transfers -/// -/// # Safety -/// -/// Never use [core::mem::forget] on an in-progress transfer -#[non_exhaustive] -#[must_use] -#[cfg(i2s_driver_supported)] -pub struct DmaTransferRx<'a, I> -where - I: dma_private::DmaSupportRx, -{ - instance: &'a mut I, -} - -#[cfg(i2s_driver_supported)] -impl<'a, I> DmaTransferRx<'a, I> -where - I: dma_private::DmaSupportRx, -{ - pub(crate) fn new(instance: &'a mut I) -> Self { - Self { instance } - } - - /// Wait for the transfer to finish. - pub fn wait(self) -> Result<(), DmaError> { - self.instance.peripheral_wait_dma(true, false); - - if self - .instance - .rx() - .pending_in_interrupts() - .contains(DmaRxInterrupt::DescriptorError) - { - Err(DmaError::DescriptorError) - } else { - Ok(()) - } - } - - /// Check if the transfer is finished. - pub fn is_done(&mut self) -> bool { - self.instance.rx().is_done() - } -} - -#[cfg(i2s_driver_supported)] -impl Drop for DmaTransferRx<'_, I> -where - I: dma_private::DmaSupportRx, -{ - fn drop(&mut self) { - self.instance.peripheral_wait_dma(true, false); - } -} - -/// DMA transaction for TX+RX transfers -/// -/// # Safety -/// -/// Never use [core::mem::forget] on an in-progress transfer -#[non_exhaustive] -#[must_use] -pub struct DmaTransferRxTx<'a, I> -where - I: dma_private::DmaSupportTx + dma_private::DmaSupportRx, -{ - instance: &'a mut I, -} - -impl<'a, I> DmaTransferRxTx<'a, I> -where - I: dma_private::DmaSupportTx + dma_private::DmaSupportRx, -{ - #[allow(dead_code)] - pub(crate) fn new(instance: &'a mut I) -> Self { - Self { instance } - } - - /// Wait for the transfer to finish. - pub fn wait(self) -> Result<(), DmaError> { - self.instance.peripheral_wait_dma(true, true); - - if self - .instance - .tx() - .pending_out_interrupts() - .contains(DmaTxInterrupt::DescriptorError) - || self - .instance - .rx() - .pending_in_interrupts() - .contains(DmaRxInterrupt::DescriptorError) - { - Err(DmaError::DescriptorError) - } else { - Ok(()) - } - } - - /// Check if the transfer is finished. - pub fn is_done(&mut self) -> bool { - self.instance.tx().is_done() && self.instance.rx().is_done() - } -} - -impl Drop for DmaTransferRxTx<'_, I> -where - I: dma_private::DmaSupportTx + dma_private::DmaSupportRx, -{ - fn drop(&mut self) { - self.instance.peripheral_wait_dma(true, true); - } -} - -/// DMA transaction for TX only circular transfers -/// -/// # Safety -/// -/// Never use [core::mem::forget] on an in-progress transfer -#[non_exhaustive] -#[must_use] -pub struct DmaTransferTxCircular<'a, I> -where - I: dma_private::DmaSupportTx, -{ - instance: &'a mut I, - state: TxCircularState, -} - -impl<'a, I> DmaTransferTxCircular<'a, I> -where - I: dma_private::DmaSupportTx, -{ - #[allow(unused)] // currently used by peripherals not available on all chips - pub(crate) fn new(instance: &'a mut I) -> Self { - let state = TxCircularState::new(instance.chain()); - Self { instance, state } - } - - /// Amount of bytes which can be pushed. - pub fn available(&mut self) -> Result { - self.state.update(self.instance.tx())?; - Ok(self.state.available) - } - - /// Push bytes into the DMA buffer. - pub fn push(&mut self, data: &[u8]) -> Result { - self.state.update(self.instance.tx())?; - self.state.push(data) - } - - /// Push bytes into the DMA buffer via the given closure. - /// The closure *must* return the actual number of bytes written. - /// The closure *might* get called with a slice which is smaller than the - /// total available buffer. - pub fn push_with(&mut self, f: impl FnOnce(&mut [u8]) -> usize) -> Result { - self.state.update(self.instance.tx())?; - self.state.push_with(f) - } - - /// Stop the DMA transfer - #[allow(clippy::type_complexity)] - pub fn stop(self) -> Result<(), DmaError> { - self.instance.peripheral_dma_stop(); - - if self - .instance - .tx() - .pending_out_interrupts() - .contains(DmaTxInterrupt::DescriptorError) - { - Err(DmaError::DescriptorError) - } else { - Ok(()) - } - } -} - -impl Drop for DmaTransferTxCircular<'_, I> -where - I: dma_private::DmaSupportTx, -{ - fn drop(&mut self) { - self.instance.peripheral_dma_stop(); - } -} - -/// DMA transaction for RX only circular transfers -/// -/// # Safety -/// -/// Never use [core::mem::forget] on an in-progress transfer -#[non_exhaustive] -#[must_use] -pub struct DmaTransferRxCircular<'a, I> -where - I: dma_private::DmaSupportRx, -{ - instance: &'a mut I, - state: RxCircularState, -} - -impl<'a, I> DmaTransferRxCircular<'a, I> -where - I: dma_private::DmaSupportRx, -{ - #[allow(unused)] // currently used by peripherals not available on all chips - pub(crate) fn new(instance: &'a mut I) -> Self { - let state = RxCircularState::new(instance.chain()); - Self { instance, state } - } - - /// Amount of bytes which can be popped. - /// - /// It's expected to call this before trying to [DmaTransferRxCircular::pop] - /// data. - pub fn available(&mut self) -> Result { - self.state.update()?; - Ok(self.state.available) - } - - /// Get available data. - /// - /// It's expected that the amount of available data is checked before by - /// calling [DmaTransferRxCircular::available] and that the buffer can hold - /// all available data. - /// - /// Fails with [DmaError::BufferTooSmall] if the given buffer is too small - /// to hold all available data - pub fn pop(&mut self, data: &mut [u8]) -> Result { - self.state.update()?; - self.state.pop(data) - } -} - -impl Drop for DmaTransferRxCircular<'_, I> -where - I: dma_private::DmaSupportRx, -{ - fn drop(&mut self) { - self.instance.peripheral_dma_stop(); - } -} - pub(crate) mod asynch { use core::task::Poll; @@ -2815,19 +2023,34 @@ pub(crate) mod asynch { CH: DmaTxChannel, { pub(crate) tx: &'a mut ChannelTx, + success_interrupts: EnumSet, + failure_interrupts: EnumSet, } impl<'a, CH> DmaTxFuture<'a, CH> where CH: DmaTxChannel, { - const SUCCESS_INTERRUPTS: EnumSet = enum_set!(DmaTxInterrupt::TotalEof); - const FAILURE_INTERRUPTS: EnumSet = - enum_set!(DmaTxInterrupt::DescriptorError); - #[cfg_attr(esp32c2, expect(dead_code))] pub fn new(tx: &'a mut ChannelTx) -> Self { - Self { tx } + Self { + tx, + success_interrupts: enum_set!(DmaTxInterrupt::TotalEof), + failure_interrupts: enum_set!(DmaTxInterrupt::DescriptorError), + } + } + + #[cfg_attr(any(esp32c2, esp32c61), expect(dead_code))] + pub fn new_with_config( + tx: &'a mut ChannelTx, + success_interrupts: EnumSet, + failure_interrupts: EnumSet, + ) -> Self { + Self { + tx, + success_interrupts, + failure_interrupts, + } } } @@ -2842,10 +2065,10 @@ pub(crate) mod asynch { cx: &mut core::task::Context<'_>, ) -> Poll { let interrupts = self.tx.pending_out_interrupts(); - let result = if !interrupts.is_disjoint(Self::SUCCESS_INTERRUPTS) { - Ok(()) - } else if !interrupts.is_disjoint(Self::FAILURE_INTERRUPTS) { + let result = if !interrupts.is_disjoint(self.failure_interrupts) { Err(DmaError::DescriptorError) + } else if !interrupts.is_disjoint(self.success_interrupts) { + Ok(()) } else { // The interrupt may become pending before we register the waker and start // listening, but that should just trigger the interrupt handler. The only @@ -2853,7 +2076,7 @@ pub(crate) mod asynch { // listening. self.tx.waker().register(cx.waker()); self.tx - .listen_out(Self::SUCCESS_INTERRUPTS | Self::FAILURE_INTERRUPTS); + .listen_out(self.success_interrupts | self.failure_interrupts); return Poll::Pending; }; @@ -2870,7 +2093,7 @@ pub(crate) mod asynch { { fn drop(&mut self) { self.tx - .unlisten_out(Self::SUCCESS_INTERRUPTS | Self::FAILURE_INTERRUPTS); + .unlisten_out(self.success_interrupts | self.failure_interrupts); } } @@ -2880,22 +2103,36 @@ pub(crate) mod asynch { CH: DmaRxChannel, { pub(crate) rx: &'a mut ChannelRx, + success_interrupts: EnumSet, + failure_interrupts: EnumSet, } impl<'a, CH> DmaRxFuture<'a, CH> where CH: DmaRxChannel, { - const SUCCESS_INTERRUPTS: EnumSet = - enum_set!(DmaRxInterrupt::SuccessfulEof); - const FAILURE_INTERRUPTS: EnumSet = enum_set!( - DmaRxInterrupt::DescriptorError - | DmaRxInterrupt::DescriptorEmpty - | DmaRxInterrupt::ErrorEof - ); - pub fn new(rx: &'a mut ChannelRx) -> Self { - Self { rx } + Self { + rx, + success_interrupts: enum_set!(DmaRxInterrupt::SuccessfulEof), + failure_interrupts: enum_set!( + DmaRxInterrupt::DescriptorError + | DmaRxInterrupt::DescriptorEmpty + | DmaRxInterrupt::ErrorEof + ), + } + } + + pub fn new_with_config( + rx: &'a mut ChannelRx, + success_interrupts: EnumSet, + failure_interrupts: EnumSet, + ) -> Self { + Self { + rx, + success_interrupts, + failure_interrupts, + } } } @@ -2910,10 +2147,10 @@ pub(crate) mod asynch { cx: &mut core::task::Context<'_>, ) -> Poll { let interrupts = self.rx.pending_in_interrupts(); - let result = if !interrupts.is_disjoint(Self::SUCCESS_INTERRUPTS) { - Ok(()) - } else if !interrupts.is_disjoint(Self::FAILURE_INTERRUPTS) { + let result = if !interrupts.is_disjoint(self.failure_interrupts) { Err(DmaError::DescriptorError) + } else if !interrupts.is_disjoint(self.success_interrupts) { + Ok(()) } else { // The interrupt may become pending before we register the waker and start // listening, but that should just trigger the interrupt handler. The only @@ -2921,7 +2158,7 @@ pub(crate) mod asynch { // listening. self.rx.waker().register(cx.waker()); self.rx - .listen_in(Self::SUCCESS_INTERRUPTS | Self::FAILURE_INTERRUPTS); + .listen_in(self.success_interrupts | self.failure_interrupts); return Poll::Pending; }; @@ -2938,144 +2175,7 @@ pub(crate) mod asynch { { fn drop(&mut self) { self.rx - .unlisten_in(Self::SUCCESS_INTERRUPTS | Self::FAILURE_INTERRUPTS); - } - } - - // Legacy API still used by I2S - #[cfg(i2s_driver_supported)] - pub struct DmaTxDoneChFuture<'a, CH> - where - CH: DmaTxChannel, - { - pub(crate) tx: &'a mut ChannelTx, - _a: (), - } - - #[cfg(i2s_driver_supported)] - impl<'a, CH> DmaTxDoneChFuture<'a, CH> - where - CH: DmaTxChannel, - { - pub fn new(tx: &'a mut ChannelTx) -> Self { - Self { tx, _a: () } - } - } - - #[cfg(i2s_driver_supported)] - impl core::future::Future for DmaTxDoneChFuture<'_, CH> - where - CH: DmaTxChannel, - { - type Output = Result<(), DmaError>; - - fn poll( - self: core::pin::Pin<&mut Self>, - cx: &mut core::task::Context<'_>, - ) -> Poll { - if self - .tx - .pending_out_interrupts() - .contains(DmaTxInterrupt::Done) - { - self.tx.clear_out(DmaTxInterrupt::Done); - Poll::Ready(Ok(())) - } else if self - .tx - .pending_out_interrupts() - .contains(DmaTxInterrupt::DescriptorError) - { - self.tx.clear_interrupts(); - Poll::Ready(Err(DmaError::DescriptorError)) - } else { - self.tx.waker().register(cx.waker()); - self.tx - .listen_out(DmaTxInterrupt::Done | DmaTxInterrupt::DescriptorError); - Poll::Pending - } - } - } - - #[cfg(i2s_driver_supported)] - impl Drop for DmaTxDoneChFuture<'_, CH> - where - CH: DmaTxChannel, - { - fn drop(&mut self) { - self.tx - .unlisten_out(DmaTxInterrupt::Done | DmaTxInterrupt::DescriptorError); - } - } - - #[cfg(i2s_driver_supported)] - pub struct DmaRxDoneChFuture<'a, CH> - where - CH: DmaRxChannel, - { - pub(crate) rx: &'a mut ChannelRx, - _a: (), - } - - #[cfg(i2s_driver_supported)] - impl<'a, CH> DmaRxDoneChFuture<'a, CH> - where - CH: DmaRxChannel, - { - pub fn new(rx: &'a mut ChannelRx) -> Self { - Self { rx, _a: () } - } - } - - #[cfg(i2s_driver_supported)] - impl core::future::Future for DmaRxDoneChFuture<'_, CH> - where - CH: DmaRxChannel, - { - type Output = Result<(), DmaError>; - - fn poll( - self: core::pin::Pin<&mut Self>, - cx: &mut core::task::Context<'_>, - ) -> Poll { - if self - .rx - .pending_in_interrupts() - .contains(DmaRxInterrupt::Done) - { - self.rx.clear_in(DmaRxInterrupt::Done); - Poll::Ready(Ok(())) - } else if !self.rx.pending_in_interrupts().is_disjoint( - DmaRxInterrupt::DescriptorError - | DmaRxInterrupt::DescriptorEmpty - | DmaRxInterrupt::ErrorEof, - ) { - self.rx.clear_interrupts(); - Poll::Ready(Err(DmaError::DescriptorError)) - } else { - self.rx.waker().register(cx.waker()); - self.rx.listen_in( - DmaRxInterrupt::Done - | DmaRxInterrupt::DescriptorError - | DmaRxInterrupt::DescriptorEmpty - | DmaRxInterrupt::ErrorEof, - ); - Poll::Pending - } - } - } - - #[cfg(i2s_driver_supported)] - impl Drop for DmaRxDoneChFuture<'_, CH> - where - CH: DmaRxChannel, - { - fn drop(&mut self) { - self.rx.unlisten_in( - DmaRxInterrupt::Done - | DmaRxInterrupt::DescriptorError - | DmaRxInterrupt::DescriptorEmpty - | DmaRxInterrupt::ErrorEof, - ); + .unlisten_in(self.success_interrupts | self.failure_interrupts); } } diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs index 2e8ac6d1402..72b733498e6 100644 --- a/esp-hal/src/i2s/master.rs +++ b/esp-hal/src/i2s/master.rs @@ -76,8 +76,8 @@ //! ```rust, no_run //! # {before_snippet} //! # use esp_hal::i2s::master::{I2s, Channels, DataFormat, Config}; -//! # use esp_hal::dma_buffers; -//! let (mut rx_buffer, rx_descriptors, _, _) = dma_buffers!(4 * 4092, 0); +//! # use esp_hal::dma_rx_stream_buffer; +//! let rx_buffer = dma_rx_stream_buffer!(4 * 4092, 1024); //! //! let i2s = I2s::new( //! peripherals.I2S0, @@ -93,16 +93,16 @@ //! .with_bclk(peripherals.GPIO1) //! .with_ws(peripherals.GPIO2) //! .with_din(peripherals.GPIO5) -//! .build(rx_descriptors); +//! .build(); //! -//! let mut transfer = i2s_rx.read_dma_circular(&mut rx_buffer)?; +//! let mut transfer = i2s_rx.read(rx_buffer)?; //! //! loop { -//! let avail = transfer.available()?; +//! let avail = transfer.available_bytes(); //! //! if avail > 0 { //! let mut rcv = [0u8; 5000]; -//! transfer.pop(&mut rcv[..avail])?; +//! transfer.pop(&mut rcv[..avail]); //! } //! } //! # } @@ -112,7 +112,9 @@ //! //! - Only TDM mode is supported. -use enumset::{EnumSet, EnumSetType}; +use core::mem::ManuallyDrop; + +use enumset::{EnumSet, EnumSetType, enum_set}; use private::*; use crate::{ @@ -124,19 +126,16 @@ use crate::{ Channel, ChannelRx, ChannelTx, - DescriptorChain, DmaChannelFor, DmaEligible, DmaError, - DmaTransferRx, - DmaTransferRxCircular, - DmaTransferTx, - DmaTransferTxCircular, + DmaRxBuffer, + DmaRxInterrupt, + DmaTxBuffer, + DmaTxInterrupt, PeripheralRxChannel, PeripheralTxChannel, - ReadBuffer, - WriteBuffer, - dma_private::{DmaSupport, DmaSupportRx, DmaSupportTx}, + asynch::{DmaRxFuture, DmaTxFuture}, }, gpio::{OutputConfig, interconnect::PeripheralOutput}, i2s::AnyI2s, @@ -165,14 +164,237 @@ pub(crate) const I2S_LL_MCLK_DIVIDER_BIT_WIDTH: usize = property!("i2s.mclk_divi pub(crate) const I2S_LL_MCLK_DIVIDER_MAX: usize = (1 << I2S_LL_MCLK_DIVIDER_BIT_WIDTH) - 1; -/// Data types that the I2S peripheral can work with. -pub trait AcceptedWord: crate::private::Sealed {} -impl AcceptedWord for u8 {} -impl AcceptedWord for u16 {} -impl AcceptedWord for u32 {} -impl AcceptedWord for i8 {} -impl AcceptedWord for i16 {} -impl AcceptedWord for i32 {} +/// A structure representing a DMA transfer. +/// +/// This structure holds references to the driver instance, DMA buffers, and +/// transfer status. +#[instability::unstable] +pub struct I2sTxDmaTransfer<'d, Dm, Buf> +where + Dm: DriverMode, + Buf: DmaTxBuffer, +{ + i2s_tx: ManuallyDrop>, + buffer_view: ManuallyDrop, +} + +impl<'d, Dm, Buf> I2sTxDmaTransfer<'d, Dm, Buf> +where + Dm: DriverMode, + Buf: DmaTxBuffer, +{ + /// Returns true when [Self::wait] will not block. + pub fn is_done(&self) -> bool { + self.i2s_tx.i2s.is_tx_done() + } + + /// Waits for the transfer to finish and returns the peripheral and buffer. + pub fn wait(mut self) -> Result<(I2sTx<'d, Dm>, Buf::Final), DmaError> { + while !self.is_done() {} + + self.i2s_tx.tx_channel.stop_transfer(); + self.i2s_tx.i2s.tx_stop(); + + let (i2s_tx, buf) = self.release(); + + if i2s_tx.tx_channel.has_error() { + Err(DmaError::DescriptorError) + } else { + Ok((i2s_tx, Buf::from_view(buf))) + } + } + + fn release(mut self) -> (I2sTx<'d, Dm>, Buf::View) { + let (i2s_tx, buffer_view) = unsafe { + ( + ManuallyDrop::take(&mut self.i2s_tx), + ManuallyDrop::take(&mut self.buffer_view), + ) + }; + + core::mem::forget(self); + (i2s_tx, buffer_view) + } +} + +impl<'d, Buf> I2sTxDmaTransfer<'d, Async, Buf> +where + Buf: DmaTxBuffer, +{ + /// Waits for the transfer to finish and returns the peripheral and buffer. + pub async fn wait_async(mut self) -> Result<(I2sTx<'d, Async>, Buf::Final), DmaError> { + DmaTxFuture::new(&mut self.i2s_tx.tx_channel).await?; + + self.i2s_tx.tx_channel.stop_transfer(); + self.i2s_tx.i2s.tx_stop(); + + let (i2s_tx, buf) = self.release(); + + if i2s_tx.tx_channel.has_error() { + Err(DmaError::DescriptorError) + } else { + Ok((i2s_tx, Buf::from_view(buf))) + } + } + + /// Waits for a condition that might indicate more data is available. + pub async fn wait_for_available_async(&mut self) -> Result<(), DmaError> { + DmaTxFuture::new_with_config( + &mut self.i2s_tx.tx_channel, + enum_set!(DmaTxInterrupt::Eof), + enum_set!(DmaTxInterrupt::DescriptorError | DmaTxInterrupt::TotalEof), + ) + .await + } +} + +impl core::ops::Deref for I2sTxDmaTransfer<'_, Dm, BUF> { + type Target = BUF::View; + + fn deref(&self) -> &Self::Target { + &self.buffer_view + } +} + +impl core::ops::DerefMut for I2sTxDmaTransfer<'_, Dm, BUF> { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.buffer_view + } +} + +impl Drop for I2sTxDmaTransfer<'_, Dm, BUF> { + fn drop(&mut self) { + self.i2s_tx.i2s.tx_stop(); + + // SAFETY: This is Drop, we know that the parts are no longer used + let view = unsafe { + ManuallyDrop::drop(&mut self.i2s_tx); + ManuallyDrop::take(&mut self.buffer_view) + }; + let _ = BUF::from_view(view); + } +} + +/// A structure representing a DMA transfer. +/// +/// This structure holds references to the driver instance, DMA buffers, and +/// transfer status. +#[instability::unstable] +pub struct I2sRxDmaTransfer<'d, Dm, Buf> +where + Dm: DriverMode, + Buf: DmaRxBuffer, +{ + i2s_rx: ManuallyDrop>, + buffer_view: ManuallyDrop, +} + +impl<'d, Dm, Buf> I2sRxDmaTransfer<'d, Dm, Buf> +where + Dm: DriverMode, + Buf: DmaRxBuffer, +{ + /// Returns true when [Self::wait] will not block. + pub fn is_done(&self) -> bool { + self.i2s_rx.i2s.is_rx_done() + } + + /// Waits for the transfer to finish and returns the peripheral and buffer. + pub fn wait(mut self) -> Result<(I2sRx<'d, Dm>, Buf::Final), DmaError> { + while !self.is_done() {} + + self.i2s_rx.rx_channel.stop_transfer(); + self.i2s_rx.i2s.rx_stop(); + + let (i2s_rx, buf) = self.release(); + + if i2s_rx.rx_channel.has_error() { + Err(DmaError::DescriptorError) + } else { + Ok((i2s_rx, Buf::from_view(buf))) + } + } + + fn release(mut self) -> (I2sRx<'d, Dm>, Buf::View) { + let (i2s_rx, buffer_view) = unsafe { + ( + ManuallyDrop::take(&mut self.i2s_rx), + ManuallyDrop::take(&mut self.buffer_view), + ) + }; + + core::mem::forget(self); + (i2s_rx, buffer_view) + } +} + +impl<'d, Buf> I2sRxDmaTransfer<'d, Async, Buf> +where + Buf: DmaRxBuffer, +{ + /// Waits for the transfer to finish and returns the peripheral and buffer. + pub async fn wait_async(mut self) -> Result<(I2sRx<'d, Async>, Buf::Final), DmaError> { + // we treat DescriptorEmpty as rx transfer is done + DmaRxFuture::new_with_config( + &mut self.i2s_rx.rx_channel, + enum_set!(DmaRxInterrupt::DescriptorEmpty), + enum_set!(DmaRxInterrupt::ErrorEof | DmaRxInterrupt::DescriptorError), + ) + .await?; + + self.i2s_rx.rx_channel.stop_transfer(); + self.i2s_rx.i2s.rx_stop(); + + let (i2s_rx, buf) = self.release(); + + if i2s_rx.rx_channel.has_error() { + Err(DmaError::DescriptorError) + } else { + Ok((i2s_rx, Buf::from_view(buf))) + } + } + + /// Waits for a condition that might indicate more data is available. + pub async fn wait_for_available_async(&mut self) -> Result<(), DmaError> { + DmaRxFuture::new_with_config( + &mut self.i2s_rx.rx_channel, + enum_set!(DmaRxInterrupt::SuccessfulEof | DmaRxInterrupt::Done), + enum_set!( + DmaRxInterrupt::ErrorEof + | DmaRxInterrupt::DescriptorError + | DmaRxInterrupt::DescriptorEmpty + ), + ) + .await + } +} + +impl core::ops::Deref for I2sRxDmaTransfer<'_, Dm, BUF> { + type Target = BUF::View; + + fn deref(&self) -> &Self::Target { + &self.buffer_view + } +} + +impl core::ops::DerefMut for I2sRxDmaTransfer<'_, Dm, BUF> { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.buffer_view + } +} + +impl Drop for I2sRxDmaTransfer<'_, Dm, BUF> { + fn drop(&mut self) { + self.i2s_rx.i2s.rx_stop(); + + // SAFETY: This is Drop, we know that the parts are no longer used + let view = unsafe { + ManuallyDrop::drop(&mut self.i2s_rx); + ManuallyDrop::take(&mut self.buffer_view) + }; + let _ = BUF::from_view(view); + } +} /// I2S Error #[derive(Debug, Clone, Copy, PartialEq)] @@ -979,7 +1201,6 @@ where { i2s: AnyI2s<'d>, tx_channel: ChannelTx>>, - tx_chain: DescriptorChain, _guard: PeripheralGuard, #[cfg(i2s_version = "1")] data_format: DataFormat, @@ -994,79 +1215,33 @@ where } } -impl DmaSupport for I2sTx<'_, Dm> -where - Dm: DriverMode, -{ - type DriverMode = Dm; - - fn peripheral_wait_dma(&mut self, _is_rx: bool, _is_tx: bool) { - self.i2s.wait_for_tx_done(); - } - - fn peripheral_dma_stop(&mut self) { - self.i2s.tx_stop(); - } -} - -impl<'d, Dm> DmaSupportTx for I2sTx<'d, Dm> -where - Dm: DriverMode, -{ - type Channel = PeripheralTxChannel>; - - fn tx(&mut self) -> &mut ChannelTx>> { - &mut self.tx_channel - } - - fn chain(&mut self) -> &mut DescriptorChain { - &mut self.tx_chain - } -} - -impl I2sTx<'_, Dm> +impl<'d, Dm> I2sTx<'d, Dm> where Dm: DriverMode, { - fn write(&mut self, data: &[u8]) -> Result<(), Error> { - self.start_tx_transfer(&data, false)?; - - // wait until I2S_TX_IDLE is 1 - self.i2s.wait_for_tx_done(); - - Ok(()) - } - - fn start_tx_transfer<'t, TXBUF>( - &'t mut self, - words: &'t TXBUF, - circular: bool, - ) -> Result<(), Error> - where - TXBUF: ReadBuffer, - Dm: DriverMode, - { - let (ptr, len) = unsafe { words.read_buffer() }; - - // Reset TX unit and TX FIFO + /// Perform a DMA write. + #[allow(clippy::type_complexity)] + #[instability::unstable] + pub fn write( + mut self, + mut buffer: TX, + ) -> Result, (Error, Self, TX)> { self.i2s.reset_tx(); - - // Enable corresponding interrupts if needed - - // configure DMA outlink - unsafe { - self.tx_chain.fill_for_tx(circular, ptr, len)?; + let res = unsafe { self.tx_channel - .prepare_transfer_without_start(self.i2s.dma_peripheral(), &self.tx_chain) - .and_then(|_| self.tx_channel.start_transfer())?; + .prepare_transfer(self.i2s.dma_peripheral(), &mut buffer) + .and_then(|_| self.tx_channel.start_transfer()) + }; + if let Err(err) = res { + return Err((Error::DmaError(err), self, buffer)); } - // set I2S_TX_STOP_EN if needed - - // start: set I2S_TX_START self.i2s.tx_start(); - Ok(()) + Ok(I2sTxDmaTransfer { + i2s_tx: ManuallyDrop::new(self), + buffer_view: ManuallyDrop::new(buffer.into_view()), + }) } /// Change the I2S Tx unit configuration. @@ -1079,40 +1254,6 @@ where } } } - - /// Writes a slice of data to the I2S peripheral. - pub fn write_words(&mut self, words: &[impl AcceptedWord]) -> Result<(), Error> { - self.write(unsafe { - core::slice::from_raw_parts(words.as_ptr().cast::(), core::mem::size_of_val(words)) - }) - } - - /// Write I2S. - /// Returns [DmaTransferTx] which represents the in-progress DMA - /// transfer - pub fn write_dma<'t>( - &'t mut self, - words: &'t impl ReadBuffer, - ) -> Result, Error> - where - Self: DmaSupportTx, - { - self.start_tx_transfer(words, false)?; - Ok(DmaTransferTx::new(self)) - } - - /// Continuously write to I2S. Returns [DmaTransferTxCircular] which - /// represents the in-progress DMA transfer - pub fn write_dma_circular<'t>( - &'t mut self, - words: &'t impl ReadBuffer, - ) -> Result, Error> - where - Self: DmaSupportTx, - { - self.start_tx_transfer(words, true)?; - Ok(DmaTransferTxCircular::new(self)) - } } /// I2S RX channel @@ -1122,7 +1263,6 @@ where { i2s: AnyI2s<'d>, rx_channel: ChannelRx>>, - rx_chain: DescriptorChain, _guard: PeripheralGuard, #[cfg(i2s_version = "1")] data_format: DataFormat, @@ -1137,80 +1277,39 @@ where } } -impl DmaSupport for I2sRx<'_, Dm> +impl<'d, Dm> I2sRx<'d, Dm> where Dm: DriverMode, { - type DriverMode = Dm; - - fn peripheral_wait_dma(&mut self, _is_rx: bool, _is_tx: bool) { - self.i2s.wait_for_rx_done(); - } - - fn peripheral_dma_stop(&mut self) { - self.i2s.reset_rx(); - } -} - -#[instability::unstable] -impl<'d, Dm> DmaSupportRx for I2sRx<'d, Dm> -where - Dm: DriverMode, -{ - type Channel = PeripheralRxChannel>; - - fn rx(&mut self) -> &mut ChannelRx>> { - &mut self.rx_channel - } - - fn chain(&mut self) -> &mut DescriptorChain { - &mut self.rx_chain - } -} - -impl I2sRx<'_, Dm> -where - Dm: DriverMode, -{ - fn read(&mut self, mut data: &mut [u8]) -> Result<(), Error> { - self.start_rx_transfer(&mut data, false)?; - - // wait until I2S_RX_IDLE is 1 - self.i2s.wait_for_rx_done(); - - Ok(()) - } - - fn start_rx_transfer<'t, RXBUF>( - &'t mut self, - words: &'t mut RXBUF, - circular: bool, - ) -> Result<(), Error> + /// Perform a DMA read. + /// + /// This will return a [I2sRxDmaTransfer] + pub fn read( + mut self, + mut buffer: BUF, + ) -> Result, (Error, Self, BUF)> where - RXBUF: WriteBuffer, + BUF: DmaRxBuffer, { - let (ptr, len) = unsafe { words.write_buffer() }; - - if !len.is_multiple_of(4) { - return Err(Error::IllegalArgument); - } - // Reset RX unit and RX FIFO self.i2s.reset_rx(); - // Enable corresponding interrupts if needed - - // configure DMA inlink - unsafe { - self.rx_chain.fill_for_rx(circular, ptr, len)?; + let res = unsafe { self.rx_channel - .prepare_transfer_without_start(self.i2s.dma_peripheral(), &self.rx_chain) - .and_then(|_| self.rx_channel.start_transfer())?; + .prepare_transfer(self.i2s.dma_peripheral(), &mut buffer) + .and_then(|_| self.rx_channel.start_transfer()) + }; + if let Err(err) = res { + return Err((Error::DmaError(err), self, buffer)); } // start: set I2S_RX_START - self.i2s.rx_start(len); - Ok(()) + self.i2s.rx_start(usize::MAX); // really limited by exhausting DMA rx buffer + + Ok(I2sRxDmaTransfer { + i2s_rx: ManuallyDrop::new(self), + buffer_view: ManuallyDrop::new(buffer.into_view()), + }) } /// Change the I2S Rx unit configuration. @@ -1223,49 +1322,6 @@ where } } } - - /// Reads a slice of data from the I2S peripheral and stores it in the - /// provided buffer. - pub fn read_words(&mut self, words: &mut [impl AcceptedWord]) -> Result<(), Error> { - if core::mem::size_of_val(words) > 4096 || words.is_empty() { - return Err(Error::IllegalArgument); - } - - self.read(unsafe { - core::slice::from_raw_parts_mut( - words.as_mut_ptr().cast::(), - core::mem::size_of_val(words), - ) - }) - } - - /// Read I2S. - /// Returns [DmaTransferRx] which represents the in-progress DMA - /// transfer - pub fn read_dma<'t>( - &'t mut self, - words: &'t mut impl WriteBuffer, - ) -> Result, Error> - where - Self: DmaSupportRx, - { - self.start_rx_transfer(words, false)?; - Ok(DmaTransferRx::new(self)) - } - - /// Continuously read from I2S. - /// Returns [DmaTransferRxCircular] which represents the in-progress DMA - /// transfer - pub fn read_dma_circular<'t>( - &'t mut self, - words: &'t mut impl WriteBuffer, - ) -> Result, Error> - where - Self: DmaSupportRx, - { - self.start_rx_transfer(words, true)?; - Ok(DmaTransferRxCircular::new(self)) - } } /// A peripheral singleton compatible with the I2S master driver. @@ -1284,7 +1340,7 @@ mod private { use crate::pac::i2s0::RegisterBlock; use crate::{ DriverMode, - dma::{ChannelRx, ChannelTx, DescriptorChain, DmaDescriptor, DmaEligible}, + dma::{ChannelRx, ChannelTx, DmaEligible}, gpio::{ InputConfig, InputSignal, @@ -1316,12 +1372,11 @@ mod private { where Dm: DriverMode, { - pub fn build(self, descriptors: &'static mut [DmaDescriptor]) -> I2sTx<'d, Dm> { + pub fn build(self) -> I2sTx<'d, Dm> { let peripheral = self.i2s.peripheral(); I2sTx { i2s: self.i2s, tx_channel: self.tx_channel, - tx_chain: DescriptorChain::new(descriptors), _guard: PeripheralGuard::new(peripheral), #[cfg(i2s_version = "1")] data_format: self.data_format, @@ -1377,12 +1432,11 @@ mod private { where Dm: DriverMode, { - pub fn build(self, descriptors: &'static mut [DmaDescriptor]) -> I2sRx<'d, Dm> { + pub fn build(self) -> I2sRx<'d, Dm> { let peripheral = self.i2s.peripheral(); I2sRx { i2s: self.i2s, rx_channel: self.rx_channel, - rx_chain: DescriptorChain::new(descriptors), _guard: PeripheralGuard::new(peripheral), #[cfg(i2s_version = "1")] data_format: self.data_format, @@ -1677,6 +1731,9 @@ mod private { w.tx_fifo_reset().bit(bit) }); + #[cfg(esp32s2)] + while self.regs().conf().read().tx_reset_st().bit_is_set() {} + self.regs().lc_conf().toggle(|w, bit| w.out_rst().bit(bit)); self.regs().int_clr().write(|w| { @@ -1687,18 +1744,18 @@ mod private { fn tx_start(&self) { self.regs().conf().modify(|_, w| w.tx_start().set_bit()); - - while self.regs().state().read().tx_idle().bit_is_set() { - // wait - } } fn tx_stop(&self) { self.regs().conf().modify(|_, w| w.tx_start().clear_bit()); } + fn is_tx_done(&self) -> bool { + self.regs().state().read().tx_idle().bit_is_set() + } + fn wait_for_tx_done(&self) { - while self.regs().state().read().tx_idle().bit_is_clear() { + while !self.is_tx_done() { // wait } @@ -1711,6 +1768,9 @@ mod private { w.rx_fifo_reset().bit(bit) }); + #[cfg(esp32s2)] + while self.regs().conf().read().rx_reset_st().bit_is_set() {} + self.regs().lc_conf().toggle(|w, bit| w.in_rst().bit(bit)); self.regs().int_clr().write(|w| { @@ -1740,8 +1800,16 @@ mod private { self.regs().conf().modify(|_, w| w.rx_start().set_bit()); } + fn rx_stop(&self) { + self.regs().conf().modify(|_, w| w.rx_start().clear_bit()); + } + + fn is_rx_done(&self) -> bool { + self.regs().int_raw().read().in_dscr_empty().bit_is_set() + } + fn wait_for_rx_done(&self) { - while self.regs().int_raw().read().in_suc_eof().bit_is_clear() { + while !self.is_rx_done() { // wait } @@ -2125,8 +2193,12 @@ mod private { .modify(|_, w| w.tx_start().clear_bit()); } + fn is_tx_done(&self) -> bool { + self.regs().state().read().tx_idle().bit_is_set() + } + fn wait_for_tx_done(&self) { - while self.regs().state().read().tx_idle().bit_is_clear() { + while !self.is_tx_done() { // wait } @@ -2160,8 +2232,18 @@ mod private { self.regs().rx_conf().modify(|_, w| w.rx_start().set_bit()); } + fn rx_stop(&self) { + self.regs() + .rx_conf() + .modify(|_, w| w.rx_start().clear_bit()); + } + + fn is_rx_done(&self) -> bool { + self.regs().int_raw().read().rx_done().bit_is_set() + } + fn wait_for_rx_done(&self) { - while self.regs().int_raw().read().rx_done().bit_is_clear() { + while !self.is_rx_done() { // wait } @@ -2499,222 +2581,3 @@ mod private { } } } - -/// Async functionality -pub mod asynch { - use super::{Error, I2sRx, I2sTx, RegisterAccessPrivate}; - use crate::{ - Async, - dma::{ - DmaEligible, - ReadBuffer, - RxCircularState, - TxCircularState, - WriteBuffer, - asynch::{DmaRxDoneChFuture, DmaRxFuture, DmaTxDoneChFuture, DmaTxFuture}, - }, - }; - - impl<'d> I2sTx<'d, Async> { - /// One-shot write I2S. - pub async fn write_dma_async(&mut self, words: &mut [u8]) -> Result<(), Error> { - let (ptr, len) = (words.as_ptr(), words.len()); - - self.i2s.reset_tx(); - - let future = DmaTxFuture::new(&mut self.tx_channel); - - unsafe { - self.tx_chain.fill_for_tx(false, ptr, len)?; - future - .tx - .prepare_transfer_without_start(self.i2s.dma_peripheral(), &self.tx_chain) - .and_then(|_| future.tx.start_transfer())?; - } - - self.i2s.tx_start(); - future.await?; - - Ok(()) - } - - /// Continuously write to I2S. Returns [I2sWriteDmaTransferAsync] - pub fn write_dma_circular_async( - mut self, - words: TXBUF, - ) -> Result, Error> { - let (ptr, len) = unsafe { words.read_buffer() }; - - // Reset TX unit and TX FIFO - self.i2s.reset_tx(); - - // Enable corresponding interrupts if needed - - // configure DMA outlink - unsafe { - self.tx_chain.fill_for_tx(true, ptr, len)?; - self.tx_channel - .prepare_transfer_without_start(self.i2s.dma_peripheral(), &self.tx_chain) - .and_then(|_| self.tx_channel.start_transfer())?; - } - - // set I2S_TX_STOP_EN if needed - - // start: set I2S_TX_START - self.i2s.tx_start(); - - let state = TxCircularState::new(&mut self.tx_chain); - Ok(I2sWriteDmaTransferAsync { - i2s_tx: self, - state, - _buffer: words, - }) - } - } - - /// An in-progress async circular DMA write transfer. - pub struct I2sWriteDmaTransferAsync<'d, BUFFER> { - i2s_tx: I2sTx<'d, Async>, - state: TxCircularState, - _buffer: BUFFER, - } - - impl I2sWriteDmaTransferAsync<'_, BUFFER> { - /// How many bytes can be pushed into the DMA transaction. - /// Will wait for more than 0 bytes available. - pub async fn available(&mut self) -> Result { - loop { - self.state.update(&self.i2s_tx.tx_channel)?; - let res = self.state.available; - - if res != 0 { - break Ok(res); - } - - DmaTxDoneChFuture::new(&mut self.i2s_tx.tx_channel).await? - } - } - - /// Push bytes into the DMA transaction. - pub async fn push(&mut self, data: &[u8]) -> Result { - let avail = self.available().await?; - let to_send = usize::min(avail, data.len()); - Ok(self.state.push(&data[..to_send])?) - } - - /// Push bytes into the DMA buffer via the given closure. - /// The closure *must* return the actual number of bytes written. - /// The closure *might* get called with a slice which is smaller than - /// the total available buffer. Only useful for circular DMA - /// transfers - pub async fn push_with( - &mut self, - f: impl FnOnce(&mut [u8]) -> usize, - ) -> Result { - let _avail = self.available().await; - Ok(self.state.push_with(f)?) - } - } - - impl<'d> I2sRx<'d, Async> { - /// One-shot read I2S. - pub async fn read_dma_async(&mut self, words: &mut [u8]) -> Result<(), Error> { - let (ptr, len) = (words.as_mut_ptr(), words.len()); - - if !len.is_multiple_of(4) { - return Err(Error::IllegalArgument); - } - - // Reset RX unit and RX FIFO - self.i2s.reset_rx(); - - let future = DmaRxFuture::new(&mut self.rx_channel); - - // configure DMA inlink - unsafe { - self.rx_chain.fill_for_rx(false, ptr, len)?; - future - .rx - .prepare_transfer_without_start(self.i2s.dma_peripheral(), &self.rx_chain) - .and_then(|_| future.rx.start_transfer())?; - } - - // start: set I2S_RX_START - self.i2s.rx_start(len); - - future.await?; - - Ok(()) - } - - /// Continuously read from I2S. Returns [I2sReadDmaTransferAsync] - pub fn read_dma_circular_async( - mut self, - mut words: RXBUF, - ) -> Result, Error> - where - RXBUF: WriteBuffer, - { - let (ptr, len) = unsafe { words.write_buffer() }; - - if !len.is_multiple_of(4) { - return Err(Error::IllegalArgument); - } - - // Reset RX unit and RX FIFO - self.i2s.reset_rx(); - - // Enable corresponding interrupts if needed - - // configure DMA inlink - unsafe { - self.rx_chain.fill_for_rx(true, ptr, len)?; - self.rx_channel - .prepare_transfer_without_start(self.i2s.dma_peripheral(), &self.rx_chain) - .and_then(|_| self.rx_channel.start_transfer())?; - } - - // start: set I2S_RX_START - self.i2s.rx_start(len); - - let state = RxCircularState::new(&mut self.rx_chain); - Ok(I2sReadDmaTransferAsync { - i2s_rx: self, - state, - _buffer: words, - }) - } - } - - /// An in-progress async circular DMA read transfer. - pub struct I2sReadDmaTransferAsync<'d, BUFFER> { - i2s_rx: I2sRx<'d, Async>, - state: RxCircularState, - _buffer: BUFFER, - } - - impl I2sReadDmaTransferAsync<'_, BUFFER> { - /// How many bytes can be popped from the DMA transaction. - /// Will wait for more than 0 bytes available. - pub async fn available(&mut self) -> Result { - loop { - self.state.update()?; - - let res = self.state.available; - - if res != 0 { - break Ok(res); - } - - DmaRxDoneChFuture::new(&mut self.i2s_rx.rx_channel).await?; - } - } - - /// Pop bytes from the DMA transaction. - pub async fn pop(&mut self, data: &mut [u8]) -> Result { - let avail = self.available().await?; - let to_rcv = usize::min(avail, data.len()); - Ok(self.state.pop(&mut data[..to_rcv])?) - } - } -} diff --git a/hil-test/src/bin/embassy_timers_executors.rs b/hil-test/src/bin/embassy_timers_executors.rs index 0db3c7291ad..d9848b98a09 100644 --- a/hil-test/src/bin/embassy_timers_executors.rs +++ b/hil-test/src/bin/embassy_timers_executors.rs @@ -478,15 +478,22 @@ mod interrupt_spi_dma { #[cfg(not(any(esp32, esp32s2, esp32s3)))] #[embassy_executor::task] async fn interrupt_driven_task(i2s_tx: esp_hal::i2s::master::I2s<'static, Blocking>) { - let (_, _, _, tx_descriptors) = dma_buffers!(128); + use esp_hal::dma_tx_stream_buffer; - let mut i2s_tx = i2s_tx.into_async().i2s_tx.build(tx_descriptors); + let mut buffer = dma_tx_stream_buffer!(128, 128); - loop { - let mut buffer: [u8; 8] = [0; 8]; + let mut i2s_tx = i2s_tx.into_async().i2s_tx.build(); + loop { INTERRUPT_TASK_WORKING.store(true, portable_atomic::Ordering::Relaxed); - i2s_tx.write_dma_async(&mut buffer).await.unwrap(); + (i2s_tx, buffer) = i2s_tx + .write(buffer) + .ok() + .unwrap() + .wait_async() + .await + .ok() + .unwrap(); INTERRUPT_TASK_WORKING.store(false, portable_atomic::Ordering::Relaxed); if STOP_INTERRUPT_TASK.load(portable_atomic::Ordering::Relaxed) { diff --git a/hil-test/src/bin/i2s.rs b/hil-test/src/bin/i2s.rs index e9a072300cf..68c2bd9f395 100644 --- a/hil-test/src/bin/i2s.rs +++ b/hil-test/src/bin/i2s.rs @@ -16,7 +16,9 @@ mod tests { use esp_hal::{ Async, delay::Delay, - dma_buffers, + dma::DmaTxStreamBuf, + dma_rx_stream_buffer, + dma_tx_stream_buffer, gpio::{AnyPin, NoPin, Pin}, i2s::master::{Channels, Config, DataFormat, I2s, I2sTx}, peripherals::I2S0, @@ -59,24 +61,28 @@ mod tests { } #[embassy_executor::task] - async fn writer(tx_buffer: &'static mut [u8], i2s_tx: I2sTx<'static, Async>) { + async fn writer(mut tx_buffer: DmaTxStreamBuf, i2s_tx: I2sTx<'static, Async>) { let mut samples = SampleSource::new(); - for b in tx_buffer.iter_mut() { - *b = samples.next().unwrap(); - } + tx_buffer.push_with(|buf| { + for b in buf.iter_mut() { + *b = samples.next().unwrap(); + } + buf.len() + }); - let mut tx_transfer = i2s_tx.write_dma_circular_async(tx_buffer).unwrap(); + let mut tx_transfer = i2s_tx.write(tx_buffer).ok().unwrap(); loop { - tx_transfer - .push_with(|buffer| { + tx_transfer.wait_for_available_async().await.ok().unwrap(); + + if tx_transfer.available_bytes() > 0 { + tx_transfer.push_with(|buffer| { for b in buffer.iter_mut() { *b = samples.next().unwrap(); } buffer.len() - }) - .await - .unwrap(); + }); + } } } @@ -113,8 +119,8 @@ mod tests { async fn test_i2s_loopback_async(ctx: Context) { let spawner = unsafe { embassy_executor::Spawner::for_current_executor().await }; - let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = - esp_hal::dma_circular_buffers!(BUFFER_SIZE, BUFFER_SIZE); + let rx_buffer = dma_rx_stream_buffer!(BUFFER_SIZE, 128); + let tx_buffer = dma_tx_stream_buffer!(BUFFER_SIZE, 128); let i2s = I2s::new( ctx.i2s, @@ -135,23 +141,25 @@ mod tests { .with_bclk(NoPin) .with_ws(NoPin) .with_dout(dout) - .build(tx_descriptors); + .build(); let i2s_rx = i2s .i2s_rx .with_bclk(NoPin) .with_ws(NoPin) .with_din(din) - .build(rx_descriptors); + .build(); - let mut rx_transfer = i2s_rx.read_dma_circular_async(rx_buffer).unwrap(); + let mut rx_transfer = i2s_rx.read(rx_buffer).ok().unwrap(); spawner.spawn(writer(tx_buffer, i2s_tx).unwrap()); let mut rcv = [0u8; BUFFER_SIZE]; let mut sample_idx = 0; let mut samples = SampleSource::new(); for _ in 0..30 { - let len = rx_transfer.pop(&mut rcv).await.unwrap(); + rx_transfer.wait_for_available_async().await.unwrap(); + let avl = rx_transfer.available_bytes(); + let len = rx_transfer.pop(&mut rcv[..avl]); for &b in &rcv[..len] { let expected = samples.next().unwrap(); assert_eq!( @@ -166,7 +174,8 @@ mod tests { #[test] fn test_i2s_loopback(ctx: Context) { - let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(16000, 16000); + let rx_buffer = dma_rx_stream_buffer!(BUFFER_SIZE * 4, 1000); + let mut tx_buffer = dma_tx_stream_buffer!(BUFFER_SIZE * 4, 1000); let i2s = I2s::new( ctx.i2s, @@ -181,70 +190,68 @@ mod tests { let (din, dout) = unsafe { ctx.dout.split() }; - let mut i2s_tx = i2s + let i2s_tx = i2s .i2s_tx .with_bclk(NoPin) .with_ws(NoPin) .with_dout(dout) - .build(tx_descriptors); + .build(); - let mut i2s_rx = i2s + let i2s_rx = i2s .i2s_rx .with_bclk(NoPin) .with_ws(NoPin) .with_din(din) - .build(rx_descriptors); + .build(); let mut samples = SampleSource::new(); - for b in tx_buffer.iter_mut() { - *b = samples.next().unwrap(); - } + tx_buffer.push_with(|buf| { + for b in buf.iter_mut() { + *b = samples.next().unwrap(); + } + buf.len() + }); let mut rcv = [0u8; 11000]; let mut filler = [0x1u8; 12000]; - let mut rx_transfer = i2s_rx.read_dma_circular(rx_buffer).unwrap(); - // trying to pop data before calling `available` should just do nothing - assert_eq!(0, rx_transfer.pop(&mut rcv[..100]).unwrap()); + let mut rx_transfer = i2s_rx.read(rx_buffer).ok().unwrap(); // no data available yet - assert_eq!(0, rx_transfer.available().unwrap()); + assert_eq!(0, rx_transfer.available_bytes()); - let mut tx_transfer = i2s_tx.write_dma_circular(tx_buffer).unwrap(); + let mut tx_transfer = i2s_tx.write(tx_buffer).ok().unwrap(); let mut iteration = 0; let mut sample_idx = 0; let mut check_samples = SampleSource::new(); - loop { - let tx_avail = tx_transfer.available().unwrap(); - // make sure there are more than one descriptor buffers ready to push - if tx_avail > 5000 { - for b in &mut filler[0..tx_avail].iter_mut() { - *b = samples.next().unwrap(); + loop { + loop { + let tx_avail = tx_transfer.available_bytes(); + if tx_avail > 0 { + for b in &mut filler[0..tx_avail].iter_mut() { + *b = samples.next().unwrap(); + } + tx_transfer.push(&filler[0..tx_avail]); + } else { + break; } - tx_transfer.push(&filler[0..tx_avail]).unwrap(); } // test calling available multiple times doesn't break anything - rx_transfer.available().unwrap(); - rx_transfer.available().unwrap(); - rx_transfer.available().unwrap(); - rx_transfer.available().unwrap(); - rx_transfer.available().unwrap(); - rx_transfer.available().unwrap(); - let rx_avail = rx_transfer.available().unwrap(); + rx_transfer.available_bytes(); + rx_transfer.available_bytes(); + rx_transfer.available_bytes(); + rx_transfer.available_bytes(); + rx_transfer.available_bytes(); + rx_transfer.available_bytes(); + let rx_avail = rx_transfer.available_bytes(); // make sure there are more than one descriptor buffers ready to pop if rx_avail > 0 { - // trying to pop less data than available is an error - assert_eq!( - Err(esp_hal::dma::DmaError::BufferTooSmall), - rx_transfer.pop(&mut rcv[..rx_avail / 2]) - ); - rcv.fill(0xff); - let len = rx_transfer.pop(&mut rcv).unwrap(); + let len = rx_transfer.pop(&mut rcv[..rx_avail]); assert!(len > 0); for &b in &rcv[..len] { @@ -262,7 +269,7 @@ mod tests { if iteration == 1 { // delay to make it likely `available` will need to handle more than one // descriptor next time - Delay::new().delay_millis(160); + Delay::new().delay_millis(60); } } @@ -272,66 +279,6 @@ mod tests { } } - #[test] - fn test_i2s_push_too_late(ctx: Context) { - let (_, _, tx_buffer, tx_descriptors) = dma_buffers!(0, 16000); - - let i2s = I2s::new( - ctx.i2s, - ctx.dma_channel, - Config::new_tdm_philips() - .with_sample_rate(Rate::from_hz(16000)) - .with_data_format(DataFormat::Data16Channel16) - .with_channels(Channels::STEREO), - ) - .unwrap(); - - let mut i2s_tx = i2s - .i2s_tx - .with_bclk(NoPin) - .with_ws(NoPin) - .with_dout(ctx.dout) - .build(tx_descriptors); - - let mut tx_transfer = i2s_tx.write_dma_circular(tx_buffer).unwrap(); - - let delay = esp_hal::delay::Delay::new(); - delay.delay_millis(300); - - assert!(matches!(tx_transfer.push(&[0; 128]), Err(_))); - } - - #[test] - #[timeout(1)] - fn test_i2s_read_too_late(ctx: Context) { - let (rx_buffer, rx_descriptors, _, _) = dma_buffers!(16000, 0); - - let i2s = I2s::new( - ctx.i2s, - ctx.dma_channel, - Config::new_tdm_philips() - .with_sample_rate(Rate::from_hz(16000)) - .with_data_format(DataFormat::Data16Channel16) - .with_channels(Channels::STEREO), - ) - .unwrap(); - - let mut i2s_rx = i2s - .i2s_rx - .with_bclk(NoPin) - .with_ws(NoPin) - .with_din(ctx.dout) // not a typo - .build(rx_descriptors); - - let mut buffer = [0u8; 1024]; - let mut rx_transfer = i2s_rx.read_dma_circular(rx_buffer).unwrap(); - - let delay = esp_hal::delay::Delay::new(); - delay.delay_millis(300); - - assert!(matches!(rx_transfer.pop(&mut buffer), Err(_))); - } - #[test] #[cfg(not(esp32s2))] fn test_i2s_rx_half_sample_bits_regression(ctx: Context) { diff --git a/qa-test/src/bin/embassy_i2s_read.rs b/qa-test/src/bin/embassy_i2s_read.rs index 76a3543c4e7..ee18a434bd3 100644 --- a/qa-test/src/bin/embassy_i2s_read.rs +++ b/qa-test/src/bin/embassy_i2s_read.rs @@ -19,7 +19,7 @@ use embassy_executor::Spawner; use esp_backtrace as _; use esp_hal::{ - dma_buffers, + dma_rx_stream_buffer, i2s::master::{Channels, Config, DataFormat, I2s}, interrupt::software::SoftwareInterruptControl, time::Rate, @@ -31,6 +31,7 @@ esp_bootloader_esp_idf::esp_app_desc!(); #[esp_hal::main] async fn main(_spawner: Spawner) { + esp_println::logger::init_logger_from_env(); println!("Init!"); let peripherals = esp_hal::init(esp_hal::Config::default()); let sw_int = SoftwareInterruptControl::new(peripherals.SW_INTERRUPT); @@ -45,7 +46,7 @@ async fn main(_spawner: Spawner) { } } - let (rx_buffer, rx_descriptors, _, _) = dma_buffers!(4092 * 4, 0); + let buffer = dma_rx_stream_buffer!(4092 * 8, 2048); let i2s = I2s::new( peripherals.I2S0, @@ -64,29 +65,32 @@ async fn main(_spawner: Spawner) { .with_bclk(peripherals.GPIO2) .with_ws(peripherals.GPIO4) .with_din(peripherals.GPIO5) - .build(rx_descriptors); + .build(); - let buffer = rx_buffer; println!("Start"); - let mut data = [0u8; 5000]; - let mut transaction = i2s_rx.read_dma_circular_async(buffer).unwrap(); + let mut data = [0u8; 9000]; + let mut transaction = i2s_rx.read(buffer).ok().unwrap(); loop { - let avail = transaction.available().await.unwrap(); + transaction.wait_for_available_async().await.unwrap(); + + let avail = transaction.available_bytes(); println!("available {}", avail); - let count = transaction.pop(&mut data).await.unwrap(); + if avail > 0 { + let count = transaction.pop(&mut data[..avail]); - #[cfg(not(feature = "esp32s2"))] - println!( - "got {} bytes, {:x?}..{:x?}", - count, - &data[..10], - &data[count - 10..count] - ); + #[cfg(not(feature = "esp32s2"))] + println!( + "got {} bytes, {:x?}..{:x?}", + count, + &data[..10], + &data[count - 10..count] + ); - // esp-println is a bit slow on ESP32-S2 - don't run into DMA too late errors - #[cfg(feature = "esp32s2")] - println!("got {} bytes", count,); + // esp-println is a bit slow on ESP32-S2 - don't run into DMA too late errors + #[cfg(feature = "esp32s2")] + println!("got {} bytes", count,); + } } } diff --git a/qa-test/src/bin/embassy_i2s_sound.rs b/qa-test/src/bin/embassy_i2s_sound.rs index a0e724f9c74..c1197d8d3bc 100644 --- a/qa-test/src/bin/embassy_i2s_sound.rs +++ b/qa-test/src/bin/embassy_i2s_sound.rs @@ -33,7 +33,7 @@ use embassy_executor::Spawner; use esp_backtrace as _; use esp_hal::{ - dma_buffers, + dma_tx_stream_buffer, i2s::master::{Channels, Config, DataFormat, I2s}, interrupt::software::SoftwareInterruptControl, time::Rate, @@ -67,7 +67,7 @@ async fn main(_spawner: Spawner) { } } - let (_, _, tx_buffer, tx_descriptors) = dma_buffers!(0, 32000); + let mut buffer = dma_tx_stream_buffer!(4092 * 4, 2048); let i2s = I2s::new( peripherals.I2S0, @@ -85,36 +85,41 @@ async fn main(_spawner: Spawner) { .with_bclk(peripherals.GPIO2) .with_ws(peripherals.GPIO4) .with_dout(peripherals.GPIO5) - .build(tx_descriptors); + .build(); let data = unsafe { core::slice::from_raw_parts(&SINE as *const _ as *const u8, SINE.len() * 2) }; - let buffer = tx_buffer; let mut idx = 0; - for i in 0..usize::min(data.len(), buffer.len()) { - buffer[i] = data[idx]; - - idx += 1; - - if idx >= data.len() { - idx = 0; + buffer.push_with(|buf| { + for b in buf.iter_mut() { + *b = data[idx]; + idx += 1; + + if idx >= data.len() { + idx = 0; + } } - } - - let mut filler = [0u8; 10000]; - let mut idx = 32000 % data.len(); + buf.len() + }); println!("Start"); - let mut transaction = i2s_tx.write_dma_circular_async(buffer).unwrap(); + let mut transaction = i2s_tx.write(buffer).ok().unwrap(); loop { - for i in 0..filler.len() { - filler[i] = data[(idx + i) % data.len()]; - } - println!("Next"); + transaction.wait_for_available_async().await.unwrap(); + + let written = transaction.push_with(|buf| { + for b in buf.iter_mut() { + *b = data[idx]; + idx += 1; + + if idx >= data.len() { + idx = 0; + } + } + buf.len() + }); - let written = transaction.push(&filler).await.unwrap(); - idx = (idx + written) % data.len(); println!("written {}", written); } } diff --git a/qa-test/src/bin/embassy_wifi_i2s.rs b/qa-test/src/bin/embassy_wifi_i2s.rs index dc048367e68..b4e94b85130 100644 --- a/qa-test/src/bin/embassy_wifi_i2s.rs +++ b/qa-test/src/bin/embassy_wifi_i2s.rs @@ -13,7 +13,7 @@ use embassy_time::{Duration, Instant, Timer}; use esp_alloc; use esp_backtrace as _; use esp_hal::{ - dma_buffers, + dma::DmaRxStreamBuf, i2s::master::{Channels, Config as I2sConfig, DataFormat, I2s}, interrupt::software::SoftwareInterruptControl, ram, @@ -81,7 +81,7 @@ async fn connection_manager( #[embassy_executor::task] async fn i2s_dma_drain( i2s_rx: esp_hal::i2s::master::I2sRx<'static, esp_hal::Async>, - buffer: &'static mut [u8], + buffer: DmaRxStreamBuf, connected_signal: &'static Signal, ) { // Temporary buffer for DMA pops @@ -89,74 +89,41 @@ async fn i2s_dma_drain( let i2s_data = I2S_DATA_BUFFER.init([0u8; 8192]); // Create circular DMA transaction - println!( - "🎛️ Creating I2S DMA circular transaction with ring size: {} bytes", - buffer.len() - ); - let mut transaction = match i2s_rx.read_dma_circular_async(buffer) { + println!("🎛️ Creating I2S DMA circular transaction"); + let mut transaction = match i2s_rx.read(buffer) { Ok(t) => t, Err(e) => { - println!("Failed to start I2S DMA: {:?}", e); + println!("Failed to start I2S DMA: {:?}", e.0); return; } }; let mut total_drained: usize = 0; - let mut late_errors: u32 = 0; let start = Instant::now(); // Start continuous draining to prevent DmaError(Late) println!("Starting continuous buffer drain to keep DMA synchronized..."); while !connected_signal.signaled() { // Check for available data and drain it - match transaction.pop(i2s_data).await { - Ok(bytes) => { - total_drained += bytes; - } - Err(e) => { - if matches!( - e, - esp_hal::i2s::master::Error::DmaError(esp_hal::dma::DmaError::Late) - ) { - println!( - "Late error during drain - (waiting connection signal) {:?}", - esp_hal::system::Cpu::current() - ); - late_errors += 1; - } else { - println!( - "Error during continuous drain: {:?} {:?}", - e, - esp_hal::system::Cpu::current() - ); - } - } - } + transaction.wait_for_available_async().await.unwrap(); + + let avl = transaction.available_bytes(); - // Small delay to prevent busy waiting - Timer::after(Duration::from_millis(1)).await; + if avl > 0 { + let bytes = transaction.pop(&mut i2s_data[..avl]); + total_drained += bytes; + } } println!( - "Drained: {} bytes | Late errors: {} | uptime: {} ms", + "Drained: {} bytes | uptime: {} ms", total_drained, - late_errors, start.elapsed().as_millis() ); - match transaction.pop(i2s_data).await { - Ok(bytes) => { - println!("Final drained {} bytes total", bytes); - } - Err(e) => { - if matches!( - e, - esp_hal::i2s::master::Error::DmaError(esp_hal::dma::DmaError::Late) - ) { - println!("Late error during drain - (final drain)"); - } else { - println!("Error during final drain: {:?}", e); - } - } - } + + transaction.wait_for_available_async().await.unwrap(); + let avl = transaction.available_bytes(); + let bytes = transaction.pop(&mut i2s_data[..avl]); + println!("Final drained {} bytes total", bytes); } #[esp_hal::main] @@ -181,7 +148,7 @@ async fn main(spawner: Spawner) { let dma_channel = peripherals.DMA_I2S0; #[cfg(not(any(feature = "esp32", feature = "esp32s2")))] let dma_channel = peripherals.DMA_CH0; - let (rx_buffer, rx_descriptors, _, _) = dma_buffers!(I2S_BUFFER_SIZE, 0); + let buffer = esp_hal::dma_rx_stream_buffer!(I2S_BUFFER_SIZE, 2048); let i2s_cfg = I2sConfig::new_tdm_philips() .with_sample_rate(Rate::from_hz(SAMPLE_RATE)) @@ -197,7 +164,7 @@ async fn main(spawner: Spawner) { .with_bclk(peripherals.GPIO2) // SCK .with_ws(peripherals.GPIO4) // WS .with_din(peripherals.GPIO5) // SD - .build(rx_descriptors); + .build(); // WiFi + network stack let station_config = Config::Station( @@ -233,5 +200,5 @@ async fn main(spawner: Spawner) { // Tasks spawner.spawn(net_task(runner).unwrap()); spawner.spawn(connection_manager(controller, connected_signal).unwrap()); - spawner.spawn(i2s_dma_drain(i2s_rx, rx_buffer, connected_signal).unwrap()); + spawner.spawn(i2s_dma_drain(i2s_rx, buffer, connected_signal).unwrap()); } From 71d3f589fad938fb49ed810afcd614412eb9e43e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Tue, 26 May 2026 12:57:35 +0200 Subject: [PATCH 02/15] Expect unused where needed --- esp-hal/src/dma/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index de0e1b7f57f..3e0f0402afa 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -2123,6 +2123,7 @@ pub(crate) mod asynch { } } + #[cfg_attr(not(any(i2s_driver_supported)), expect(unused))] pub fn new_with_config( rx: &'a mut ChannelRx, success_interrupts: EnumSet, From 8728a861fe26055bac75043ab92da080fbdee30e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Tue, 26 May 2026 13:30:28 +0200 Subject: [PATCH 03/15] Remove truly unused --- esp-hal/src/soc/esp32c5/mod.rs | 10 ---------- esp-hal/src/soc/esp32s2/mod.rs | 10 ---------- esp-hal/src/soc/esp32s3/mod.rs | 10 ---------- 3 files changed, 30 deletions(-) diff --git a/esp-hal/src/soc/esp32c5/mod.rs b/esp-hal/src/soc/esp32c5/mod.rs index f6b85c9bc26..7da13c54979 100644 --- a/esp-hal/src/soc/esp32c5/mod.rs +++ b/esp-hal/src/soc/esp32c5/mod.rs @@ -72,13 +72,3 @@ pub unsafe fn cache_invalidate_addr(addr: u32, size: u32) { Cache_Invalidate_Addr(addr, size); } } - -/// Get the size of a cache line in the DCache. -#[doc(hidden)] -#[unsafe(link_section = ".rwtext")] -pub unsafe fn cache_get_dcache_line_size() -> u32 { - unsafe extern "C" { - fn Cache_Get_Line_Size() -> u32; - } - unsafe { Cache_Get_Line_Size() } -} diff --git a/esp-hal/src/soc/esp32s2/mod.rs b/esp-hal/src/soc/esp32s2/mod.rs index ae911dc361c..267c7f38907 100644 --- a/esp-hal/src/soc/esp32s2/mod.rs +++ b/esp-hal/src/soc/esp32s2/mod.rs @@ -56,16 +56,6 @@ pub unsafe fn cache_invalidate_addr(addr: u32, size: u32) { } } -/// Get the size of a cache line in the DCache. -#[doc(hidden)] -#[unsafe(link_section = ".rwtext")] -pub unsafe fn cache_get_dcache_line_size() -> u32 { - unsafe extern "C" { - fn Cache_Get_DCache_Line_Size() -> u32; - } - unsafe { Cache_Get_DCache_Line_Size() } -} - #[crate::ram] pub(crate) unsafe fn configure_cpu_caches() { // Set up caches. Doesn't work when put in `configure_cpu_caches`. diff --git a/esp-hal/src/soc/esp32s3/mod.rs b/esp-hal/src/soc/esp32s3/mod.rs index 8ef46848f78..c7f39f2fd82 100644 --- a/esp-hal/src/soc/esp32s3/mod.rs +++ b/esp-hal/src/soc/esp32s3/mod.rs @@ -137,14 +137,4 @@ pub unsafe fn cache_invalidate_addr(addr: u32, size: u32) { } } -/// Get the size of a cache line in the DCache. -#[doc(hidden)] -#[unsafe(link_section = ".rwtext")] -pub unsafe fn cache_get_dcache_line_size() -> u32 { - unsafe extern "C" { - fn Cache_Get_DCache_Line_Size() -> u32; - } - unsafe { Cache_Get_DCache_Line_Size() } -} - pub(crate) fn pre_init() {} From 0a1d1c9c81e8845d1dcd5c5f8a5acdaeb70dc15a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 09:31:09 +0200 Subject: [PATCH 04/15] Add (generated) stream buffer tests and fix impl --- esp-hal/src/dma/buffers/mod.rs | 8 +- hil-test/src/bin/misc_non_drivers.rs | 4 + .../src/bin/misc_non_drivers/dma_buffers.rs | 329 ++++++++++++++++++ 3 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 hil-test/src/bin/misc_non_drivers/dma_buffers.rs diff --git a/esp-hal/src/dma/buffers/mod.rs b/esp-hal/src/dma/buffers/mod.rs index d64bddc4f37..f0e54eb0057 100644 --- a/esp-hal/src/dma/buffers/mod.rs +++ b/esp-hal/src/dma/buffers/mod.rs @@ -1137,6 +1137,9 @@ impl DmaRxStreamBufView { let mut remaining = buf; loop { let available = self.peek(); + if available.is_empty() { + break; + } if available.len() >= remaining.len() { remaining.copy_from_slice(&available[0..remaining.len()]); self.consume(remaining.len()); @@ -1497,12 +1500,15 @@ impl DmaTxStreamBufView { let total_len = data.len(); let mut remaining = data; - while self.available_bytes() >= remaining.len() && !remaining.is_empty() { + while !remaining.is_empty() && self.available_bytes() > 0 { let written = self.push_with(|buffer| { let len = usize::min(buffer.len(), remaining.len()); buffer[..len].copy_from_slice(&remaining[..len]); len }); + if written == 0 { + break; + } remaining = &remaining[written..]; } diff --git a/hil-test/src/bin/misc_non_drivers.rs b/hil-test/src/bin/misc_non_drivers.rs index a27708bdd21..6067a2af863 100644 --- a/hil-test/src/bin/misc_non_drivers.rs +++ b/hil-test/src/bin/misc_non_drivers.rs @@ -40,6 +40,10 @@ mod delay_async; #[cfg(dma_driver_supported)] mod dma_macros; +#[path = "misc_non_drivers/dma_buffers.rs"] +#[cfg(dma_driver_supported)] +mod dma_buffers; + #[path = "misc_non_drivers/dma_mem2mem.rs"] #[cfg(all(dma_driver_supported, dma_supports_mem2mem))] mod dma_mem2mem; diff --git a/hil-test/src/bin/misc_non_drivers/dma_buffers.rs b/hil-test/src/bin/misc_non_drivers/dma_buffers.rs new file mode 100644 index 00000000000..3841d16dd77 --- /dev/null +++ b/hil-test/src/bin/misc_non_drivers/dma_buffers.rs @@ -0,0 +1,329 @@ +#[embedded_test::tests(default_timeout = 3)] +mod tests { + const BUFFER_SIZE: usize = 4096; + const CHUNK_SIZE: usize = 1024; + + // 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::*; + + use esp_hal::{ + dma::{ + DmaBufError, DmaDescriptor, DmaRxBuffer, DmaRxStreamBuf, DmaRxStreamBufView, + DmaTxBuffer, DmaTxStreamBuf, DmaTxStreamBufView, Owner, + }, + dma_rx_stream_buffer, dma_tx_stream_buffer, + }; + + fn chunk_size(descriptors: &[DmaDescriptor]) -> usize { + descriptors[0].size() + } + + fn simulate_dma_rx_fill( + descriptors: &mut [DmaDescriptor], + buffer: &mut [u8], + descriptor_index: usize, + data: &[u8], + eof: bool, + ) { + let chunk = chunk_size(descriptors); + let offset = descriptor_index * chunk; + buffer[offset..offset + data.len()].copy_from_slice(data); + descriptors[descriptor_index].set_length(data.len()); + descriptors[descriptor_index].set_suc_eof(eof); + descriptors[descriptor_index].set_owner(Owner::Cpu); + } + + fn with_rx_view(initial_data: &[u8], eof: bool, f: impl FnOnce(DmaRxStreamBufView)) { + let mut buf = dma_rx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + buf.prepare(); + let (mut descriptors, mut buffer) = buf.split(); + if !initial_data.is_empty() { + simulate_dma_rx_fill(&mut descriptors, &mut buffer, 0, initial_data, eof); + } + let buf = DmaRxStreamBuf::new(descriptors, buffer).unwrap(); + f(buf.into_view()); + } + + fn with_rx_view_multi(fills: &[(&[u8], bool)], f: F) + where + F: FnOnce(DmaRxStreamBufView), + { + let mut buf = dma_rx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + buf.prepare(); + let (mut descriptors, mut buffer) = buf.split(); + for (index, (data, eof)) in fills.iter().enumerate() { + simulate_dma_rx_fill(&mut descriptors, &mut buffer, index, data, *eof); + } + let buf = DmaRxStreamBuf::new(descriptors, buffer).unwrap(); + f(buf.into_view()); + } + + fn with_tx_view(all_cpu_owned: bool, f: impl FnOnce(DmaTxStreamBufView)) { + let mut buf = dma_tx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + buf.prepare(); + let (descriptors, buffer) = buf.split(); + if all_cpu_owned { + for desc in descriptors.iter_mut() { + desc.set_owner(Owner::Cpu); + desc.set_length(0); + } + } + let mut buf = DmaTxStreamBuf::new(descriptors, buffer).unwrap(); + buf.push(&[]); + f(buf.into_view()); + } + + #[test] + fn test_dma_rx_stream_buf_new() { + let buf = dma_rx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + let (descriptors, buffer) = buf.split(); + core::assert_eq!(buffer.len(), BUFFER_SIZE); + core::assert_eq!(descriptors.len(), BUFFER_SIZE / CHUNK_SIZE); + core::assert!(descriptors.len() >= 4); + } + + #[test] + fn test_dma_tx_stream_buf_new() { + let buf = dma_tx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + let (descriptors, buffer) = buf.split(); + core::assert_eq!(buffer.len(), BUFFER_SIZE); + core::assert_eq!(descriptors.len(), BUFFER_SIZE / CHUNK_SIZE); + core::assert!(descriptors.len() >= 4); + } + + #[test] + fn test_dma_rx_stream_buf_insufficient_descriptors() { + let (buffer, descriptors) = + esp_hal::dma_buffers_impl!(BUFFER_SIZE, CHUNK_SIZE * 2, is_circular = false); + match DmaRxStreamBuf::new(descriptors, buffer) { + Err(DmaBufError::InsufficientDescriptors) => (), + _ => core::panic!("expected InsufficientDescriptors"), + } + } + + #[test] + fn test_dma_tx_stream_buf_insufficient_descriptors() { + let (buffer, descriptors) = + esp_hal::dma_buffers_impl!(BUFFER_SIZE, CHUNK_SIZE * 2, is_circular = false); + match DmaTxStreamBuf::new(descriptors, buffer) { + Err(DmaBufError::InsufficientDescriptors) => (), + _ => core::panic!("expected InsufficientDescriptors"), + } + } + + #[test] + fn test_dma_rx_stream_buf_prepare_is_idempotent() { + let mut buf = dma_rx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + buf.prepare(); + buf.prepare(); + } + + #[test] + fn test_dma_tx_stream_buf_prepare_is_idempotent() { + let mut buf = dma_tx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + buf.prepare(); + buf.prepare(); + } + + #[test] + fn test_dma_tx_stream_buf_push_before_transfer() { + let mut tx_buf = dma_tx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + let first = [1u8, 2, 3, 4, 5]; + let second = [6u8, 7, 8]; + + core::assert_eq!(tx_buf.push(&first), first.len()); + core::assert_eq!(tx_buf.push(&second), second.len()); + + let (_, buffer) = tx_buf.split(); + core::assert_eq!(&buffer[..first.len()], &first); + core::assert_eq!( + &buffer[first.len()..first.len() + second.len()], + &second + ); + } + + #[test] + fn test_dma_tx_stream_buf_push_with_before_transfer() { + let mut tx_buf = dma_tx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + let pushed = tx_buf.push_with(|buf| { + for (index, byte) in buf.iter_mut().enumerate().take(10) { + *byte = index as u8; + } + 10 + }); + core::assert_eq!(pushed, 10); + + let (_, buffer) = tx_buf.split(); + for (index, byte) in buffer[..10].iter().enumerate() { + core::assert_eq!(*byte, index as u8); + } + } + + #[test] + fn test_dma_tx_stream_buf_view_unprefilled_has_no_available_space() { + let mut buf = dma_tx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + buf.prepare(); + let view = buf.into_view(); + core::assert_eq!(view.available_bytes(), 0); + } + + #[test] + fn test_dma_tx_stream_buf_view_available_bytes() { + with_tx_view(true, |view| { + core::assert_eq!(view.available_bytes(), BUFFER_SIZE); + }); + } + + #[test] + fn test_dma_tx_stream_buf_view_push() { + with_tx_view(true, |mut view| { + let data = b"hello, dma tx stream"; + let pushed = view.push(data); + core::assert_eq!(pushed, data.len()); + core::assert_eq!( + view.available_bytes(), + BUFFER_SIZE - data.len() + ); + + let buf = ::from_view(view); + let (_, buffer) = buf.split(); + core::assert_eq!(&buffer[..data.len()], data); + }); + } + + #[test] + fn test_dma_tx_stream_buf_view_push_with() { + with_tx_view(true, |mut view| { + let pushed = view.push_with(|buf| { + buf[..3].copy_from_slice(&[10, 11, 12]); + 3 + }); + core::assert_eq!(pushed, 3); + core::assert_eq!(view.available_bytes(), BUFFER_SIZE - 3); + + let buf = ::from_view(view); + let (_, buffer) = buf.split(); + core::assert_eq!(&buffer[..3], &[10, 11, 12]); + }); + } + + #[test] + fn test_dma_tx_stream_buf_view_push_respects_available_space() { + with_tx_view(true, |mut view| { + let oversized = [0u8; BUFFER_SIZE + 1]; + let pushed = view.push(&oversized); + core::assert_eq!(pushed, BUFFER_SIZE); + core::assert_eq!(view.available_bytes(), 0); + }); + } + + #[test] + fn test_dma_rx_stream_buf_view_empty() { + with_rx_view(&[], false, |view| { + core::assert_eq!(view.available_bytes(), 0); + core::assert_eq!(view.peek(), &[] as &[u8]); + }); + } + + #[test] + fn test_dma_rx_stream_buf_view_peek_and_available_bytes() { + let data = b"peek me"; + with_rx_view(data, false, |view| { + core::assert_eq!(view.available_bytes(), data.len()); + core::assert_eq!(view.peek(), data); + }); + } + + #[test] + fn test_dma_rx_stream_buf_view_peek_until_eof() { + let data = b"eof test"; + with_rx_view(data, true, |view| { + let (slice, eof) = view.peek_until_eof(); + core::assert_eq!(slice, data); + core::assert!(eof); + }); + } + + #[test] + fn test_dma_rx_stream_buf_view_pop() { + let data = b"pop this data"; + with_rx_view(data, false, |mut view| { + let mut out = [0u8; 32]; + let popped = view.pop(&mut out); + core::assert_eq!(popped, data.len()); + core::assert_eq!(&out[..data.len()], data); + core::assert_eq!(view.available_bytes(), 0); + }); + } + + #[test] + fn test_dma_rx_stream_buf_view_consume_partial() { + let data = b"0123456789"; + with_rx_view(data, false, |mut view| { + core::assert_eq!(view.peek(), data); + core::assert_eq!(view.consume(4), 4); + core::assert_eq!(view.available_bytes(), data.len() - 4); + core::assert_eq!(view.peek(), &data[4..]); + }); + } + + #[test] + fn test_dma_rx_stream_buf_view_consume_full_descriptor() { + let data = [0xABu8; CHUNK_SIZE]; + with_rx_view(&data, false, |mut view| { + core::assert_eq!(view.available_bytes(), CHUNK_SIZE); + core::assert_eq!(view.consume(CHUNK_SIZE), CHUNK_SIZE); + core::assert_eq!(view.available_bytes(), 0); + }); + } + + #[test] + fn test_dma_rx_stream_buf_view_peek_multiple_descriptors() { + const SECOND_LEN: usize = 128; + let first = [1u8; CHUNK_SIZE]; + let second = [2u8; SECOND_LEN]; + with_rx_view_multi(&[(&first, false), (&second, false)], |view| { + core::assert_eq!(view.available_bytes(), CHUNK_SIZE + SECOND_LEN); + let peeked = view.peek(); + core::assert_eq!(peeked.len(), CHUNK_SIZE + SECOND_LEN); + core::assert_eq!(&peeked[..CHUNK_SIZE], &first); + core::assert_eq!(&peeked[CHUNK_SIZE..], &second); + }); + } + + #[test] + fn test_dma_rx_stream_buf_view_pop_multiple_descriptors() { + const SECOND_LEN: usize = 64; + let first = [3u8; CHUNK_SIZE]; + let second = [4u8; SECOND_LEN]; + with_rx_view_multi(&[(&first, false), (&second, false)], |mut view| { + let mut out = [0u8; CHUNK_SIZE + SECOND_LEN]; + let popped = view.pop(&mut out); + core::assert_eq!(popped, CHUNK_SIZE + SECOND_LEN); + core::assert_eq!(&out[..CHUNK_SIZE], &first); + core::assert_eq!(&out[CHUNK_SIZE..], &second); + }); + } + + #[test] + fn test_dma_rx_stream_buf_from_view() { + with_rx_view(b"round trip", false, |view| { + let buf = ::from_view(view); + let (descriptors, buffer) = buf.split(); + core::assert_eq!(buffer.len(), BUFFER_SIZE); + core::assert_eq!(descriptors.len(), BUFFER_SIZE / CHUNK_SIZE); + }); + } + + #[test] + fn test_dma_tx_stream_buf_from_view() { + with_tx_view(true, |view| { + let buf = ::from_view(view); + let (descriptors, buffer) = buf.split(); + core::assert_eq!(buffer.len(), BUFFER_SIZE); + core::assert_eq!(descriptors.len(), BUFFER_SIZE / CHUNK_SIZE); + }); + } +} From cf8da64eef81d9e7703900fb66e8d551444dc71d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 09:54:00 +0200 Subject: [PATCH 05/15] Fix DmaTxStreamBufView::advance circular descriptor iteration Co-authored-by: Cursor --- esp-hal/src/dma/buffers/mod.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/esp-hal/src/dma/buffers/mod.rs b/esp-hal/src/dma/buffers/mod.rs index f0e54eb0057..459df26cc13 100644 --- a/esp-hal/src/dma/buffers/mod.rs +++ b/esp-hal/src/dma/buffers/mod.rs @@ -1468,15 +1468,21 @@ impl DmaTxStreamBufView { /// Advances the first `n` bytes from the available data pub fn advance(&mut self, bytes_pushed: usize) { + if bytes_pushed == 0 { + return; + } + let mut bytes_filled = 0; - for d in (self.descriptor_idx..self.buf.descriptors.len()).chain(core::iter::once(0)) { + let num_descriptors = self.buf.descriptors.len(); + + for i in 0..num_descriptors { + let d = (self.descriptor_idx + i) % num_descriptors; let desc = &mut self.buf.descriptors[d]; let bytes_in_d = desc.size() - self.descriptor_offset; - // There is at least one byte left in `desc`. if bytes_in_d + bytes_filled > bytes_pushed { self.descriptor_idx = d; self.descriptor_offset = self.descriptor_offset + bytes_pushed - bytes_filled; - break; + return; } bytes_filled += bytes_in_d; self.descriptor_offset = 0; @@ -1485,7 +1491,7 @@ impl DmaTxStreamBufView { desc.set_owner(Owner::Dma); desc.set_length(desc.size()); desc.set_suc_eof(true); - let p = d.checked_sub(1).unwrap_or(self.buf.descriptors.len() - 1); + let p = d.checked_sub(1).unwrap_or(num_descriptors - 1); if p != d { let [prev, desc] = self.buf.descriptors.get_disjoint_mut([p, d]).unwrap(); desc.next = null_mut(); From c7153a12bf6d892b0a40a5672df2a04df4fd2dfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 09:56:35 +0200 Subject: [PATCH 06/15] Align DmaRxStreamBuf::new chunk sizing with DmaTxStreamBuf::new Co-authored-by: Cursor --- esp-hal/src/dma/buffers/mod.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/esp-hal/src/dma/buffers/mod.rs b/esp-hal/src/dma/buffers/mod.rs index 459df26cc13..44e3c413f05 100644 --- a/esp-hal/src/dma/buffers/mod.rs +++ b/esp-hal/src/dma/buffers/mod.rs @@ -1024,17 +1024,9 @@ impl DmaRxStreamBuf { } // Evenly distribute the buffer between the descriptors. - let chunk_size = buffer.len() / descriptors.len(); - - if chunk_size > 4095 { - return Err(DmaBufError::InsufficientDescriptors); - } - - // Check that the last descriptor can hold the excess - let excess = buffer.len() % descriptors.len(); - if chunk_size + excess > 4095 { - return Err(DmaBufError::InsufficientDescriptors); - } + let chunk_size = Some(buffer.len() / descriptors.len()) + .filter(|x| *x <= 4095) + .ok_or(DmaBufError::InsufficientDescriptors)?; let mut chunks = buffer.chunks_exact_mut(chunk_size); for (desc, chunk) in descriptors.iter_mut().zip(chunks.by_ref()) { @@ -1043,12 +1035,15 @@ impl DmaRxStreamBuf { } let remainder = chunks.into_remainder(); - debug_assert_eq!(remainder.len(), excess); if !remainder.is_empty() { // Append any excess to the last descriptor. let last_descriptor = descriptors.last_mut().unwrap(); - last_descriptor.set_size(last_descriptor.size() + remainder.len()); + let size = last_descriptor.size() + remainder.len(); + if size > 4095 { + return Err(DmaBufError::InsufficientDescriptors); + } + last_descriptor.set_size(size); } Ok(Self { From 6b2882c4fbb85db8b714ac07167812870af4ec5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 10:07:20 +0200 Subject: [PATCH 07/15] Initialize DmaTxStreamBuf descriptors as CPU-owned in prepare Co-authored-by: Cursor --- esp-hal/src/dma/buffers/mod.rs | 148 ++++++++++++++++++++++++--------- 1 file changed, 109 insertions(+), 39 deletions(-) diff --git a/esp-hal/src/dma/buffers/mod.rs b/esp-hal/src/dma/buffers/mod.rs index 44e3c413f05..da4fc6901eb 100644 --- a/esp-hal/src/dma/buffers/mod.rs +++ b/esp-hal/src/dma/buffers/mod.rs @@ -1304,6 +1304,8 @@ pub struct DmaTxStreamBuf { buffer: &'static mut [u8], burst: BurstConfig, pre_filled: Option, + view_descriptor_idx: usize, + view_descriptor_offset: usize, } impl DmaTxStreamBuf { @@ -1352,6 +1354,8 @@ impl DmaTxStreamBuf { buffer, burst: Default::default(), pre_filled: None, + view_descriptor_idx: 0, + view_descriptor_offset: 0, }) } @@ -1381,6 +1385,96 @@ impl DmaTxStreamBuf { self.pre_filled = Some(start + bytes_pushed); bytes_pushed } + + fn setup_view_state(&mut self) { + self.view_descriptor_idx = 0; + self.view_descriptor_offset = 0; + let pre_filled = self.pre_filled.unwrap_or(self.buffer.len()); + mark_tx_stream_descriptors_ready( + self.descriptors, + &mut self.view_descriptor_idx, + &mut self.view_descriptor_offset, + pre_filled, + ); + } +} + +/// Marks descriptors containing data that should be transmitted when the DMA +/// channel starts. Unlike [advance_tx_stream_descriptors], this does not modify +/// the `next` pointers because the linked list must remain intact until the +/// transfer is running. +fn mark_tx_stream_descriptors_ready( + descriptors: &mut [DmaDescriptor], + descriptor_idx: &mut usize, + descriptor_offset: &mut usize, + bytes_pushed: usize, +) { + if bytes_pushed == 0 { + return; + } + + let mut bytes_filled = 0; + let num_descriptors = descriptors.len(); + + for i in 0..num_descriptors { + let d = (*descriptor_idx + i) % num_descriptors; + let desc = &mut descriptors[d]; + let bytes_in_d = desc.size() - *descriptor_offset; + + if bytes_in_d + bytes_filled > bytes_pushed { + *descriptor_idx = d; + *descriptor_offset = *descriptor_offset + bytes_pushed - bytes_filled; + desc.set_owner(Owner::Dma); + desc.set_length(*descriptor_offset); + desc.set_suc_eof(true); + return; + } + + bytes_filled += bytes_in_d; + *descriptor_offset = 0; + + desc.set_owner(Owner::Dma); + desc.set_length(desc.size()); + desc.set_suc_eof(true); + } +} + +fn advance_tx_stream_descriptors( + descriptors: &mut [DmaDescriptor], + descriptor_idx: &mut usize, + descriptor_offset: &mut usize, + bytes_pushed: usize, +) { + if bytes_pushed == 0 { + return; + } + + let mut bytes_filled = 0; + let num_descriptors = descriptors.len(); + + for i in 0..num_descriptors { + let d = (*descriptor_idx + i) % num_descriptors; + let desc = &mut descriptors[d]; + let bytes_in_d = desc.size() - *descriptor_offset; + if bytes_in_d + bytes_filled > bytes_pushed { + *descriptor_idx = d; + *descriptor_offset = *descriptor_offset + bytes_pushed - bytes_filled; + return; + } + bytes_filled += bytes_in_d; + *descriptor_offset = 0; + + // Put the current descriptor at the end of the list + desc.set_owner(Owner::Dma); + desc.set_length(desc.size()); + desc.set_suc_eof(true); + let p = d.checked_sub(1).unwrap_or(num_descriptors - 1); + if p != d { + let [prev, desc] = descriptors.get_disjoint_mut([p, d]).unwrap(); + desc.next = null_mut(); + prev.next = desc; + } + } } unsafe impl DmaTxBuffer for DmaTxStreamBuf { @@ -1394,9 +1488,13 @@ unsafe impl DmaTxBuffer for DmaTxStreamBuf { desc.next = next; next = desc; - desc.reset_for_tx(true); + desc.set_owner(Owner::Cpu); + desc.set_length(0); + desc.set_suc_eof(false); } + self.setup_view_state(); + Preparation { start: self.descriptors.as_mut_ptr(), direction: TransferDirection::Out, @@ -1414,15 +1512,11 @@ unsafe impl DmaTxBuffer for DmaTxStreamBuf { } fn into_view(self) -> Self::View { - // If the buffer is not pre-filled, treat it as fully filled - let pre_filled = self.pre_filled.unwrap_or(self.buffer.len()); - let mut view = DmaTxStreamBufView { + DmaTxStreamBufView { + descriptor_idx: self.view_descriptor_idx, + descriptor_offset: self.view_descriptor_offset, buf: self, - descriptor_idx: 0, - descriptor_offset: 0, - }; - view.advance(pre_filled); - view + } } fn from_view(view: Self::View) -> Self { @@ -1463,36 +1557,12 @@ impl DmaTxStreamBufView { /// Advances the first `n` bytes from the available data pub fn advance(&mut self, bytes_pushed: usize) { - if bytes_pushed == 0 { - return; - } - - let mut bytes_filled = 0; - let num_descriptors = self.buf.descriptors.len(); - - for i in 0..num_descriptors { - let d = (self.descriptor_idx + i) % num_descriptors; - let desc = &mut self.buf.descriptors[d]; - let bytes_in_d = desc.size() - self.descriptor_offset; - if bytes_in_d + bytes_filled > bytes_pushed { - self.descriptor_idx = d; - self.descriptor_offset = self.descriptor_offset + bytes_pushed - bytes_filled; - return; - } - bytes_filled += bytes_in_d; - self.descriptor_offset = 0; - - // Put the current descriptor at the end of the list - desc.set_owner(Owner::Dma); - desc.set_length(desc.size()); - desc.set_suc_eof(true); - let p = d.checked_sub(1).unwrap_or(num_descriptors - 1); - if p != d { - let [prev, desc] = self.buf.descriptors.get_disjoint_mut([p, d]).unwrap(); - desc.next = null_mut(); - prev.next = desc; - } - } + advance_tx_stream_descriptors( + self.buf.descriptors, + &mut self.descriptor_idx, + &mut self.descriptor_offset, + bytes_pushed, + ); } /// Pushes a buffer into the stream buffer. From bb1a3cf58568e1b8b4fc39ac4440e5d6914d4184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 11:43:08 +0200 Subject: [PATCH 08/15] Fix & fmt --- esp-hal/src/dma/mod.rs | 9 ++++--- .../src/bin/misc_non_drivers/dma_buffers.rs | 25 ++++++++++--------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 3e0f0402afa..df3fa8e8e90 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -2031,7 +2031,10 @@ pub(crate) mod asynch { where CH: DmaTxChannel, { - #[cfg_attr(esp32c2, expect(dead_code))] + #[cfg_attr( + not(any(i2s_driver_supported, uhci_driver_supported)), + expect(dead_code) + )] pub fn new(tx: &'a mut ChannelTx) -> Self { Self { tx, @@ -2040,7 +2043,7 @@ pub(crate) mod asynch { } } - #[cfg_attr(any(esp32c2, esp32c61), expect(dead_code))] + #[cfg_attr(not(i2s_driver_supported), expect(dead_code))] pub fn new_with_config( tx: &'a mut ChannelTx, success_interrupts: EnumSet, @@ -2123,7 +2126,7 @@ pub(crate) mod asynch { } } - #[cfg_attr(not(any(i2s_driver_supported)), expect(unused))] + #[cfg_attr(not(i2s_driver_supported), expect(unused))] pub fn new_with_config( rx: &'a mut ChannelRx, success_interrupts: EnumSet, diff --git a/hil-test/src/bin/misc_non_drivers/dma_buffers.rs b/hil-test/src/bin/misc_non_drivers/dma_buffers.rs index 3841d16dd77..7119534e722 100644 --- a/hil-test/src/bin/misc_non_drivers/dma_buffers.rs +++ b/hil-test/src/bin/misc_non_drivers/dma_buffers.rs @@ -7,13 +7,20 @@ mod tests { // using defmt's non-const assert. Doing so would result in a compile error. #[allow(unused_imports)] use defmt::*; - use esp_hal::{ dma::{ - DmaBufError, DmaDescriptor, DmaRxBuffer, DmaRxStreamBuf, DmaRxStreamBufView, - DmaTxBuffer, DmaTxStreamBuf, DmaTxStreamBufView, Owner, + DmaBufError, + DmaDescriptor, + DmaRxBuffer, + DmaRxStreamBuf, + DmaRxStreamBufView, + DmaTxBuffer, + DmaTxStreamBuf, + DmaTxStreamBufView, + Owner, }, - dma_rx_stream_buffer, dma_tx_stream_buffer, + dma_rx_stream_buffer, + dma_tx_stream_buffer, }; fn chunk_size(descriptors: &[DmaDescriptor]) -> usize { @@ -138,10 +145,7 @@ mod tests { let (_, buffer) = tx_buf.split(); core::assert_eq!(&buffer[..first.len()], &first); - core::assert_eq!( - &buffer[first.len()..first.len() + second.len()], - &second - ); + core::assert_eq!(&buffer[first.len()..first.len() + second.len()], &second); } #[test] @@ -182,10 +186,7 @@ mod tests { let data = b"hello, dma tx stream"; let pushed = view.push(data); core::assert_eq!(pushed, data.len()); - core::assert_eq!( - view.available_bytes(), - BUFFER_SIZE - data.len() - ); + core::assert_eq!(view.available_bytes(), BUFFER_SIZE - data.len()); let buf = ::from_view(view); let (_, buffer) = buf.split(); From c5030d909af377b5b5879557b25e539b54d5bf24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 12:21:37 +0200 Subject: [PATCH 09/15] Address review comments --- esp-hal/src/i2s/master.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs index 72b733498e6..3b8c09e0d44 100644 --- a/esp-hal/src/i2s/master.rs +++ b/esp-hal/src/i2s/master.rs @@ -224,6 +224,7 @@ where /// Waits for the transfer to finish and returns the peripheral and buffer. pub async fn wait_async(mut self) -> Result<(I2sTx<'d, Async>, Buf::Final), DmaError> { DmaTxFuture::new(&mut self.i2s_tx.tx_channel).await?; + while !self.is_done() {} self.i2s_tx.tx_channel.stop_transfer(); self.i2s_tx.i2s.tx_stop(); @@ -264,6 +265,7 @@ impl core::ops::DerefMut for I2sTxDmaTransfer< impl Drop for I2sTxDmaTransfer<'_, Dm, BUF> { fn drop(&mut self) { + self.i2s_tx.tx_channel.stop_transfer(); self.i2s_tx.i2s.tx_stop(); // SAFETY: This is Drop, we know that the parts are no longer used @@ -303,8 +305,8 @@ where pub fn wait(mut self) -> Result<(I2sRx<'d, Dm>, Buf::Final), DmaError> { while !self.is_done() {} - self.i2s_rx.rx_channel.stop_transfer(); self.i2s_rx.i2s.rx_stop(); + self.i2s_rx.rx_channel.stop_transfer(); let (i2s_rx, buf) = self.release(); @@ -342,8 +344,8 @@ where ) .await?; - self.i2s_rx.rx_channel.stop_transfer(); self.i2s_rx.i2s.rx_stop(); + self.i2s_rx.rx_channel.stop_transfer(); let (i2s_rx, buf) = self.release(); @@ -385,6 +387,7 @@ impl core::ops::DerefMut for I2sRxDmaTransfer< impl Drop for I2sRxDmaTransfer<'_, Dm, BUF> { fn drop(&mut self) { + self.i2s_rx.rx_channel.stop_transfer(); self.i2s_rx.i2s.rx_stop(); // SAFETY: This is Drop, we know that the parts are no longer used From 48eace97f99373ef573897ff3906a45a394f12f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 13:43:30 +0200 Subject: [PATCH 10/15] i2s one shot rx test --- hil-test/src/bin/i2s.rs | 116 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 112 insertions(+), 4 deletions(-) diff --git a/hil-test/src/bin/i2s.rs b/hil-test/src/bin/i2s.rs index 68c2bd9f395..24b813a89fd 100644 --- a/hil-test/src/bin/i2s.rs +++ b/hil-test/src/bin/i2s.rs @@ -4,19 +4,19 @@ //! with loopback mode enabled). //% CHIPS: esp32 esp32c3 esp32c5 esp32c6 esp32c61 esp32h2 esp32s2 esp32s3 -//% FEATURES: unstable +//% FEATURES: unstable defmt // FIXME: re-enable on ESP32 when it no longer fails spuriously #![no_std] #![no_main] -#[cfg(not(esp32))] // FIXME #[embedded_test::tests(default_timeout = 3, executor = hil_test::Executor::new())] mod tests { + #[cfg_attr(esp32, expect(unused))] use esp_hal::{ Async, delay::Delay, - dma::DmaTxStreamBuf, + dma::{DmaDescriptor, DmaRxBuf, DmaTxStreamBuf}, dma_rx_stream_buffer, dma_tx_stream_buffer, gpio::{AnyPin, NoPin, Pin}, @@ -33,13 +33,16 @@ mod tests { } } + #[cfg_attr(esp32, expect(unused))] const BUFFER_SIZE: usize = 2000; + #[cfg_attr(esp32, expect(unused))] #[derive(Clone)] struct SampleSource { i: u8, } + #[cfg(not(esp32))] impl SampleSource { // choose values which DON'T restart on every descriptor buffer's start const ADD: u8 = 5; @@ -50,6 +53,7 @@ mod tests { } } + #[cfg(not(esp32))] impl Iterator for SampleSource { type Item = u8; @@ -60,6 +64,7 @@ mod tests { } } + #[cfg(not(esp32))] #[embassy_executor::task] async fn writer(mut tx_buffer: DmaTxStreamBuf, i2s_tx: I2sTx<'static, Async>) { let mut samples = SampleSource::new(); @@ -115,6 +120,11 @@ mod tests { } } + // FIXME - in loopback mode, the ESP32's I2S RX seems to randomly miss the first sample, causing + // this test to fail spuriously. Re-enable when this is fixed. + // While this is usually not a problem for audio applications, it does make testing more + // difficult. + #[cfg(not(esp32))] #[test] async fn test_i2s_loopback_async(ctx: Context) { let spawner = unsafe { embassy_executor::Spawner::for_current_executor().await }; @@ -172,6 +182,11 @@ mod tests { } } + // FIXME - in loopback mode, the ESP32's I2S RX seems to randomly miss the first sample, causing + // this test to fail spuriously. Re-enable when this is fixed. + // While this is usually not a problem for audio applications, it does make testing more + // difficult. + #[cfg(not(esp32))] #[test] fn test_i2s_loopback(ctx: Context) { let rx_buffer = dma_rx_stream_buffer!(BUFFER_SIZE * 4, 1000); @@ -280,7 +295,7 @@ mod tests { } #[test] - #[cfg(not(esp32s2))] + #[cfg(not(any(esp32, esp32s2)))] fn test_i2s_rx_half_sample_bits_regression(ctx: Context) { // Regression test for rx_half_sample_bits configuration bug. // Validates that TX and RX half_sample_bits registers are configured identically. @@ -322,6 +337,99 @@ mod tests { rx_half_sample_bits, expected_value ); } + + #[test] + fn test_i2s_read_one_shot(ctx: Context) { + let buffer = hil_test::mk_static!([u8; 8000], [0u8; 8000]); + let descr = hil_test::mk_static!([DmaDescriptor; 4], [DmaDescriptor::EMPTY; 4]); + let rx_buffer = DmaRxBuf::new(descr, buffer).unwrap(); + + let i2s = I2s::new( + ctx.i2s, + ctx.dma_channel, + Config::new_tdm_philips() + .with_sample_rate(Rate::from_hz(16000)) + .with_data_format(DataFormat::Data16Channel16) + .with_channels(Channels::STEREO), + ) + .unwrap(); + + // reading WS as input will generate a certain bit pattern on the data line which we can + // check was received correctly + let (din, ws) = unsafe { ctx.dout.split() }; + + let i2s_rx = i2s + .i2s_rx + .with_bclk(NoPin) + .with_ws(ws) + .with_din(din) + .build(); + + let rx_transfer = i2s_rx.read(rx_buffer).unwrap(); + let (_, done_rx) = rx_transfer.wait().unwrap(); + + assert!(done_rx.number_of_received_bytes() != 0); + + let mut tmp = [0u8; 8000]; + let read = done_rx.read_received_data(&mut tmp); + + // I2S RX might not fill each buffer for every descriptor - so the read byte count might be + // less than the buffer size, but it should never be zero if the transfer completed + // successfully. + assert!(read != 0); + + let l = read - read % 4; + for chunk in tmp[..l].chunks(4) { + assert_eq!(chunk, [1, 0, 254, 255]); + } + } + + #[test] + async fn test_i2s_read_one_shot_async(ctx: Context) { + let buffer = hil_test::mk_static!([u8; 8000], [0u8; 8000]); + let descr = hil_test::mk_static!([DmaDescriptor; 4], [DmaDescriptor::EMPTY; 4]); + let rx_buffer = DmaRxBuf::new(descr, buffer).unwrap(); + + let i2s = I2s::new( + ctx.i2s, + ctx.dma_channel, + Config::new_tdm_philips() + .with_sample_rate(Rate::from_hz(16000)) + .with_data_format(DataFormat::Data16Channel16) + .with_channels(Channels::STEREO), + ) + .unwrap() + .into_async(); + + // reading WS as input will generate a certain bit pattern on the data line which we can + // check was received correctly + let (din, ws) = unsafe { ctx.dout.split() }; + + let i2s_rx = i2s + .i2s_rx + .with_bclk(NoPin) + .with_ws(ws) + .with_din(din) + .build(); + + let rx_transfer = i2s_rx.read(rx_buffer).unwrap(); + let (_, done_rx) = rx_transfer.wait_async().await.unwrap(); + + assert!(done_rx.number_of_received_bytes() != 0); + + let mut tmp = [0u8; 8000]; + let read = done_rx.read_received_data(&mut tmp); + + // I2S RX might not fill each buffer for every descriptor - so the read byte count might be + // less than the buffer size, but it should never be zero if the transfer completed + // successfully. + assert!(read != 0); + + let l = read - read % 4; + for chunk in tmp[..l].chunks(4) { + assert_eq!(chunk, [1, 0, 254, 255]); + } + } } #[cfg(esp32)] From edb5b2783c253f78d0a69350eff45c3aaf6697fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 15:15:32 +0200 Subject: [PATCH 11/15] Exercise the one shot write in HIL --- esp-hal/src/i2s/master.rs | 3 ++ hil-test/src/bin/i2s.rs | 60 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs index 3b8c09e0d44..c6d8fab81d2 100644 --- a/esp-hal/src/i2s/master.rs +++ b/esp-hal/src/i2s/master.rs @@ -1286,6 +1286,9 @@ where { /// Perform a DMA read. /// + /// The number of read bytes might be less than the capacity of the provided buffer since the + /// peripheral might not completely fill each descriptor's buffer. + /// /// This will return a [I2sRxDmaTransfer] pub fn read( mut self, diff --git a/hil-test/src/bin/i2s.rs b/hil-test/src/bin/i2s.rs index 24b813a89fd..05491a8cfb7 100644 --- a/hil-test/src/bin/i2s.rs +++ b/hil-test/src/bin/i2s.rs @@ -4,7 +4,7 @@ //! with loopback mode enabled). //% CHIPS: esp32 esp32c3 esp32c5 esp32c6 esp32c61 esp32h2 esp32s2 esp32s3 -//% FEATURES: unstable defmt +//% FEATURES: unstable // FIXME: re-enable on ESP32 when it no longer fails spuriously #![no_std] @@ -12,6 +12,7 @@ #[embedded_test::tests(default_timeout = 3, executor = hil_test::Executor::new())] mod tests { + use esp_hal::dma::DmaTxBuf; #[cfg_attr(esp32, expect(unused))] use esp_hal::{ Async, @@ -430,6 +431,63 @@ mod tests { assert_eq!(chunk, [1, 0, 254, 255]); } } + + // We don't actually check the output but just make sure the write completes. + #[test] + fn test_i2s_write_one_shot(ctx: Context) { + let buffer = hil_test::mk_static!([u8; 8000], [0u8; 8000]); + let descr = hil_test::mk_static!([DmaDescriptor; 4], [DmaDescriptor::EMPTY; 4]); + let tx_buffer = DmaTxBuf::new(descr, buffer).unwrap(); + + let i2s = I2s::new( + ctx.i2s, + ctx.dma_channel, + Config::new_tdm_philips() + .with_sample_rate(Rate::from_hz(16000)) + .with_data_format(DataFormat::Data16Channel16) + .with_channels(Channels::STEREO), + ) + .unwrap(); + + let i2s_tx = i2s + .i2s_tx + .with_bclk(NoPin) + .with_ws(NoPin) + .with_dout(ctx.dout) + .build(); + + let tx_transfer = i2s_tx.write(tx_buffer).unwrap(); + let (_, _done_tx) = tx_transfer.wait().unwrap(); + } + + // We don't actually check the output but just make sure the write completes. + #[test] + async fn test_i2s_write_one_shot_async(ctx: Context) { + let buffer = hil_test::mk_static!([u8; 8000], [0u8; 8000]); + let descr = hil_test::mk_static!([DmaDescriptor; 4], [DmaDescriptor::EMPTY; 4]); + let tx_buffer = DmaTxBuf::new(descr, buffer).unwrap(); + + let i2s = I2s::new( + ctx.i2s, + ctx.dma_channel, + Config::new_tdm_philips() + .with_sample_rate(Rate::from_hz(16000)) + .with_data_format(DataFormat::Data16Channel16) + .with_channels(Channels::STEREO), + ) + .unwrap() + .into_async(); + + let i2s_tx = i2s + .i2s_tx + .with_bclk(NoPin) + .with_ws(NoPin) + .with_dout(ctx.dout) + .build(); + + let tx_transfer = i2s_tx.write(tx_buffer).unwrap(); + let (_, _done_tx) = tx_transfer.wait_async().await.unwrap(); + } } #[cfg(esp32)] From 88710aa4f13e6ef6176c112e44c40da41bbb15bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Wed, 27 May 2026 15:54:00 +0200 Subject: [PATCH 12/15] Add wait_for_done_async --- esp-hal/src/i2s/master.rs | 58 +++++++++++++++++++++++++++++++-------- hil-test/src/bin/i2s.rs | 10 +++++-- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs index c6d8fab81d2..907d3990547 100644 --- a/esp-hal/src/i2s/master.rs +++ b/esp-hal/src/i2s/master.rs @@ -176,6 +176,7 @@ where { i2s_tx: ManuallyDrop>, buffer_view: ManuallyDrop, + completed: bool, } impl<'d, Dm, Buf> I2sTxDmaTransfer<'d, Dm, Buf> @@ -185,12 +186,13 @@ where { /// Returns true when [Self::wait] will not block. pub fn is_done(&self) -> bool { - self.i2s_tx.i2s.is_tx_done() + self.completed || self.i2s_tx.i2s.is_tx_done() } /// Waits for the transfer to finish and returns the peripheral and buffer. pub fn wait(mut self) -> Result<(I2sTx<'d, Dm>, Buf::Final), DmaError> { while !self.is_done() {} + self.completed = true; self.i2s_tx.tx_channel.stop_transfer(); self.i2s_tx.i2s.tx_stop(); @@ -223,8 +225,11 @@ where { /// Waits for the transfer to finish and returns the peripheral and buffer. pub async fn wait_async(mut self) -> Result<(I2sTx<'d, Async>, Buf::Final), DmaError> { - DmaTxFuture::new(&mut self.i2s_tx.tx_channel).await?; - while !self.is_done() {} + if !self.completed { + DmaTxFuture::new(&mut self.i2s_tx.tx_channel).await?; + while !self.is_done() {} + self.completed = true; + } self.i2s_tx.tx_channel.stop_transfer(); self.i2s_tx.i2s.tx_stop(); @@ -238,6 +243,16 @@ where } } + /// Waits for the transfer to finish. + pub async fn wait_for_done_async(&mut self) -> Result<(), DmaError> { + if !self.completed { + DmaTxFuture::new(&mut self.i2s_tx.tx_channel).await?; + while !self.is_done() {} + self.completed = true; + } + Ok(()) + } + /// Waits for a condition that might indicate more data is available. pub async fn wait_for_available_async(&mut self) -> Result<(), DmaError> { DmaTxFuture::new_with_config( @@ -289,6 +304,7 @@ where { i2s_rx: ManuallyDrop>, buffer_view: ManuallyDrop, + completed: bool, } impl<'d, Dm, Buf> I2sRxDmaTransfer<'d, Dm, Buf> @@ -298,7 +314,7 @@ where { /// Returns true when [Self::wait] will not block. pub fn is_done(&self) -> bool { - self.i2s_rx.i2s.is_rx_done() + self.completed || self.i2s_rx.i2s.is_rx_done() } /// Waits for the transfer to finish and returns the peripheral and buffer. @@ -336,13 +352,16 @@ where { /// Waits for the transfer to finish and returns the peripheral and buffer. pub async fn wait_async(mut self) -> Result<(I2sRx<'d, Async>, Buf::Final), DmaError> { - // we treat DescriptorEmpty as rx transfer is done - DmaRxFuture::new_with_config( - &mut self.i2s_rx.rx_channel, - enum_set!(DmaRxInterrupt::DescriptorEmpty), - enum_set!(DmaRxInterrupt::ErrorEof | DmaRxInterrupt::DescriptorError), - ) - .await?; + if !self.completed { + // we treat DescriptorEmpty as rx transfer is done + DmaRxFuture::new_with_config( + &mut self.i2s_rx.rx_channel, + enum_set!(DmaRxInterrupt::DescriptorEmpty), + enum_set!(DmaRxInterrupt::ErrorEof | DmaRxInterrupt::DescriptorError), + ) + .await?; + self.completed = true; + } self.i2s_rx.i2s.rx_stop(); self.i2s_rx.rx_channel.stop_transfer(); @@ -356,6 +375,21 @@ where } } + /// Waits for the transfer to finish and returns the peripheral and buffer. + pub async fn wait_for_done_async(&mut self) -> Result<(), DmaError> { + if !self.completed { + // we treat DescriptorEmpty as rx transfer is done + DmaRxFuture::new_with_config( + &mut self.i2s_rx.rx_channel, + enum_set!(DmaRxInterrupt::DescriptorEmpty), + enum_set!(DmaRxInterrupt::ErrorEof | DmaRxInterrupt::DescriptorError), + ) + .await?; + self.completed = true; + } + Ok(()) + } + /// Waits for a condition that might indicate more data is available. pub async fn wait_for_available_async(&mut self) -> Result<(), DmaError> { DmaRxFuture::new_with_config( @@ -1244,6 +1278,7 @@ where Ok(I2sTxDmaTransfer { i2s_tx: ManuallyDrop::new(self), buffer_view: ManuallyDrop::new(buffer.into_view()), + completed: false, }) } @@ -1315,6 +1350,7 @@ where Ok(I2sRxDmaTransfer { i2s_rx: ManuallyDrop::new(self), buffer_view: ManuallyDrop::new(buffer.into_view()), + completed: false, }) } diff --git a/hil-test/src/bin/i2s.rs b/hil-test/src/bin/i2s.rs index 05491a8cfb7..96ee6a0bc78 100644 --- a/hil-test/src/bin/i2s.rs +++ b/hil-test/src/bin/i2s.rs @@ -413,7 +413,9 @@ mod tests { .with_din(din) .build(); - let rx_transfer = i2s_rx.read(rx_buffer).unwrap(); + let mut rx_transfer = i2s_rx.read(rx_buffer).unwrap(); + rx_transfer.wait_for_done_async().await.unwrap(); + assert_eq!(rx_transfer.is_done(), true); let (_, done_rx) = rx_transfer.wait_async().await.unwrap(); assert!(done_rx.number_of_received_bytes() != 0); @@ -433,6 +435,7 @@ mod tests { } // We don't actually check the output but just make sure the write completes. + // On supported chips we could use PCNT to verify at least the expected clock edges or similar. #[test] fn test_i2s_write_one_shot(ctx: Context) { let buffer = hil_test::mk_static!([u8; 8000], [0u8; 8000]); @@ -461,6 +464,7 @@ mod tests { } // We don't actually check the output but just make sure the write completes. + // On supported chips we could use PCNT to verify at least the expected clock edges or similar. #[test] async fn test_i2s_write_one_shot_async(ctx: Context) { let buffer = hil_test::mk_static!([u8; 8000], [0u8; 8000]); @@ -485,7 +489,9 @@ mod tests { .with_dout(ctx.dout) .build(); - let tx_transfer = i2s_tx.write(tx_buffer).unwrap(); + let mut tx_transfer = i2s_tx.write(tx_buffer).unwrap(); + tx_transfer.wait_for_done_async().await.unwrap(); + assert_eq!(tx_transfer.is_done(), true); let (_, _done_tx) = tx_transfer.wait_async().await.unwrap(); } } From a26361f1234cc760543192a6961f9887cf96373f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Thu, 28 May 2026 11:23:00 +0200 Subject: [PATCH 13/15] Address review comments --- esp-hal/src/dma/buffers/mod.rs | 4 -- esp-hal/src/i2s/master.rs | 42 ++++++++++++++----- hil-test/src/bin/i2s.rs | 6 +++ .../src/bin/misc_non_drivers/dma_buffers.rs | 27 ++++++++++++ 4 files changed, 64 insertions(+), 15 deletions(-) diff --git a/esp-hal/src/dma/buffers/mod.rs b/esp-hal/src/dma/buffers/mod.rs index da4fc6901eb..d648851765c 100644 --- a/esp-hal/src/dma/buffers/mod.rs +++ b/esp-hal/src/dma/buffers/mod.rs @@ -1487,10 +1487,6 @@ unsafe impl DmaTxBuffer for DmaTxStreamBuf { for desc in self.descriptors.iter_mut().rev() { desc.next = next; next = desc; - - desc.set_owner(Owner::Cpu); - desc.set_length(0); - desc.set_suc_eof(false); } self.setup_view_state(); diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs index 907d3990547..1baea461211 100644 --- a/esp-hal/src/i2s/master.rs +++ b/esp-hal/src/i2s/master.rs @@ -190,7 +190,9 @@ where } /// Waits for the transfer to finish and returns the peripheral and buffer. - pub fn wait(mut self) -> Result<(I2sTx<'d, Dm>, Buf::Final), DmaError> { + pub fn wait( + mut self, + ) -> Result<(I2sTx<'d, Dm>, Buf::Final), (DmaError, I2sTx<'d, Dm>, Buf::Final)> { while !self.is_done() {} self.completed = true; @@ -200,7 +202,7 @@ where let (i2s_tx, buf) = self.release(); if i2s_tx.tx_channel.has_error() { - Err(DmaError::DescriptorError) + Err((DmaError::DescriptorError, i2s_tx, Buf::from_view(buf))) } else { Ok((i2s_tx, Buf::from_view(buf))) } @@ -224,9 +226,16 @@ where Buf: DmaTxBuffer, { /// Waits for the transfer to finish and returns the peripheral and buffer. - pub async fn wait_async(mut self) -> Result<(I2sTx<'d, Async>, Buf::Final), DmaError> { + pub async fn wait_async( + mut self, + ) -> Result<(I2sTx<'d, Async>, Buf::Final), (DmaError, I2sTx<'d, Async>, Buf::Final)> { if !self.completed { - DmaTxFuture::new(&mut self.i2s_tx.tx_channel).await?; + if let Err(err) = DmaTxFuture::new(&mut self.i2s_tx.tx_channel).await { + self.i2s_tx.tx_channel.stop_transfer(); + self.i2s_tx.i2s.tx_stop(); + let (i2s_tx, buf) = self.release(); + return Err((err, i2s_tx, Buf::from_view(buf))); + } while !self.is_done() {} self.completed = true; } @@ -237,7 +246,7 @@ where let (i2s_tx, buf) = self.release(); if i2s_tx.tx_channel.has_error() { - Err(DmaError::DescriptorError) + Err((DmaError::DescriptorError, i2s_tx, Buf::from_view(buf))) } else { Ok((i2s_tx, Buf::from_view(buf))) } @@ -318,7 +327,9 @@ where } /// Waits for the transfer to finish and returns the peripheral and buffer. - pub fn wait(mut self) -> Result<(I2sRx<'d, Dm>, Buf::Final), DmaError> { + pub fn wait( + mut self, + ) -> Result<(I2sRx<'d, Dm>, Buf::Final), (DmaError, I2sRx<'d, Dm>, Buf::Final)> { while !self.is_done() {} self.i2s_rx.i2s.rx_stop(); @@ -327,7 +338,7 @@ where let (i2s_rx, buf) = self.release(); if i2s_rx.rx_channel.has_error() { - Err(DmaError::DescriptorError) + Err((DmaError::DescriptorError, i2s_rx, Buf::from_view(buf))) } else { Ok((i2s_rx, Buf::from_view(buf))) } @@ -351,15 +362,24 @@ where Buf: DmaRxBuffer, { /// Waits for the transfer to finish and returns the peripheral and buffer. - pub async fn wait_async(mut self) -> Result<(I2sRx<'d, Async>, Buf::Final), DmaError> { + pub async fn wait_async( + mut self, + ) -> Result<(I2sRx<'d, Async>, Buf::Final), (DmaError, I2sRx<'d, Async>, Buf::Final)> { if !self.completed { // we treat DescriptorEmpty as rx transfer is done - DmaRxFuture::new_with_config( + if let Err(err) = DmaRxFuture::new_with_config( &mut self.i2s_rx.rx_channel, enum_set!(DmaRxInterrupt::DescriptorEmpty), enum_set!(DmaRxInterrupt::ErrorEof | DmaRxInterrupt::DescriptorError), ) - .await?; + .await + { + self.completed = true; + self.i2s_rx.i2s.rx_stop(); + self.i2s_rx.rx_channel.stop_transfer(); + let (i2s_rx, buf) = self.release(); + return Err((err, i2s_rx, Buf::from_view(buf))); + } self.completed = true; } @@ -369,7 +389,7 @@ where let (i2s_rx, buf) = self.release(); if i2s_rx.rx_channel.has_error() { - Err(DmaError::DescriptorError) + Err((DmaError::DescriptorError, i2s_rx, Buf::from_view(buf))) } else { Ok((i2s_rx, Buf::from_view(buf))) } diff --git a/hil-test/src/bin/i2s.rs b/hil-test/src/bin/i2s.rs index 96ee6a0bc78..de056d2c5b3 100644 --- a/hil-test/src/bin/i2s.rs +++ b/hil-test/src/bin/i2s.rs @@ -79,7 +79,13 @@ mod tests { let mut tx_transfer = i2s_tx.write(tx_buffer).ok().unwrap(); loop { + let before = tx_transfer.available_bytes(); tx_transfer.wait_for_available_async().await.ok().unwrap(); + let after1 = tx_transfer.available_bytes(); + tx_transfer.wait_for_available_async().await.ok().unwrap(); + let after2 = tx_transfer.available_bytes(); + assert!(after1 >= before); + assert!(after2 >= after1); if tx_transfer.available_bytes() > 0 { tx_transfer.push_with(|buffer| { diff --git a/hil-test/src/bin/misc_non_drivers/dma_buffers.rs b/hil-test/src/bin/misc_non_drivers/dma_buffers.rs index 7119534e722..e9bf4ed60e0 100644 --- a/hil-test/src/bin/misc_non_drivers/dma_buffers.rs +++ b/hil-test/src/bin/misc_non_drivers/dma_buffers.rs @@ -327,4 +327,31 @@ mod tests { core::assert_eq!(descriptors.len(), BUFFER_SIZE / CHUNK_SIZE); }); } + + #[test] + fn test_dma_tx_stream_buf_initial() { + with_tx_view(false, |view| { + // initially all descriptors are owned by the DMA, so no space should be available + core::assert_eq!(view.available_bytes(), 0); + + let buf = ::from_view(view); + let (descriptors, buffer) = buf.split(); + + // DMA processed first descriptor, now it should be owned by CPU + descriptors[0].set_owner(Owner::Cpu); + + let buf = DmaTxStreamBuf::new(descriptors, buffer).unwrap(); + let mut view = buf.into_view(); + + // the first descriptor is now owned by CPU, so its chunk should be available for + // writing + core::assert_eq!(view.available_bytes(), CHUNK_SIZE); + + // we can push more data + core::assert_eq!(view.push(&[0u8; CHUNK_SIZE]), CHUNK_SIZE); + + // and after pushing, the available space should be reduced accordingly + core::assert_eq!(view.available_bytes(), 0); + }); + } } From afefdc9308e6c798b9c64871f560b468a18aeb01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Thu, 28 May 2026 11:48:06 +0200 Subject: [PATCH 14/15] Clippy --- esp-hal/src/i2s/master.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs index 1baea461211..bd6e8bd78a0 100644 --- a/esp-hal/src/i2s/master.rs +++ b/esp-hal/src/i2s/master.rs @@ -190,6 +190,10 @@ where } /// Waits for the transfer to finish and returns the peripheral and buffer. + #[allow( + clippy::type_complexity, + reason = "Need to return both the error and the peripheral/buffer for proper cleanup" + )] pub fn wait( mut self, ) -> Result<(I2sTx<'d, Dm>, Buf::Final), (DmaError, I2sTx<'d, Dm>, Buf::Final)> { @@ -327,6 +331,10 @@ where } /// Waits for the transfer to finish and returns the peripheral and buffer. + #[allow( + clippy::type_complexity, + reason = "Need to return both the error and the peripheral/buffer for proper cleanup" + )] pub fn wait( mut self, ) -> Result<(I2sRx<'d, Dm>, Buf::Final), (DmaError, I2sRx<'d, Dm>, Buf::Final)> { From 90ab486cc07a9f6d0ae2e00544973e354e3e8c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Quentin?= Date: Thu, 28 May 2026 17:42:05 +0200 Subject: [PATCH 15/15] Address review comments --- esp-hal/src/dma/buffers/mod.rs | 3 ++ esp-hal/src/i2s/master.rs | 44 ++++++------------- hil-test/src/bin/embassy_timers_executors.rs | 13 ++---- hil-test/src/bin/i2s.rs | 12 +++-- .../src/bin/misc_non_drivers/dma_buffers.rs | 15 +++++++ 5 files changed, 44 insertions(+), 43 deletions(-) diff --git a/esp-hal/src/dma/buffers/mod.rs b/esp-hal/src/dma/buffers/mod.rs index d648851765c..a540856dea4 100644 --- a/esp-hal/src/dma/buffers/mod.rs +++ b/esp-hal/src/dma/buffers/mod.rs @@ -1486,6 +1486,9 @@ unsafe impl DmaTxBuffer for DmaTxStreamBuf { let mut next = null_mut(); for desc in self.descriptors.iter_mut().rev() { desc.next = next; + // set the owner for *all* descriptors - otherwise descriptors with pre-filled data + // might be owned by CPU + desc.set_owner(Owner::Dma); next = desc; } diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs index bd6e8bd78a0..6d22fc6bc4b 100644 --- a/esp-hal/src/i2s/master.rs +++ b/esp-hal/src/i2s/master.rs @@ -190,13 +190,7 @@ where } /// Waits for the transfer to finish and returns the peripheral and buffer. - #[allow( - clippy::type_complexity, - reason = "Need to return both the error and the peripheral/buffer for proper cleanup" - )] - pub fn wait( - mut self, - ) -> Result<(I2sTx<'d, Dm>, Buf::Final), (DmaError, I2sTx<'d, Dm>, Buf::Final)> { + pub fn wait(mut self) -> (Result<(), DmaError>, I2sTx<'d, Dm>, Buf::Final) { while !self.is_done() {} self.completed = true; @@ -206,9 +200,9 @@ where let (i2s_tx, buf) = self.release(); if i2s_tx.tx_channel.has_error() { - Err((DmaError::DescriptorError, i2s_tx, Buf::from_view(buf))) + (Err(DmaError::DescriptorError), i2s_tx, Buf::from_view(buf)) } else { - Ok((i2s_tx, Buf::from_view(buf))) + (Ok(()), i2s_tx, Buf::from_view(buf)) } } @@ -230,15 +224,13 @@ where Buf: DmaTxBuffer, { /// Waits for the transfer to finish and returns the peripheral and buffer. - pub async fn wait_async( - mut self, - ) -> Result<(I2sTx<'d, Async>, Buf::Final), (DmaError, I2sTx<'d, Async>, Buf::Final)> { + pub async fn wait_async(mut self) -> (Result<(), DmaError>, I2sTx<'d, Async>, Buf::Final) { if !self.completed { if let Err(err) = DmaTxFuture::new(&mut self.i2s_tx.tx_channel).await { self.i2s_tx.tx_channel.stop_transfer(); self.i2s_tx.i2s.tx_stop(); let (i2s_tx, buf) = self.release(); - return Err((err, i2s_tx, Buf::from_view(buf))); + return (Err(err), i2s_tx, Buf::from_view(buf)); } while !self.is_done() {} self.completed = true; @@ -250,9 +242,9 @@ where let (i2s_tx, buf) = self.release(); if i2s_tx.tx_channel.has_error() { - Err((DmaError::DescriptorError, i2s_tx, Buf::from_view(buf))) + (Err(DmaError::DescriptorError), i2s_tx, Buf::from_view(buf)) } else { - Ok((i2s_tx, Buf::from_view(buf))) + (Ok(()), i2s_tx, Buf::from_view(buf)) } } @@ -331,13 +323,7 @@ where } /// Waits for the transfer to finish and returns the peripheral and buffer. - #[allow( - clippy::type_complexity, - reason = "Need to return both the error and the peripheral/buffer for proper cleanup" - )] - pub fn wait( - mut self, - ) -> Result<(I2sRx<'d, Dm>, Buf::Final), (DmaError, I2sRx<'d, Dm>, Buf::Final)> { + pub fn wait(mut self) -> (Result<(), DmaError>, I2sRx<'d, Dm>, Buf::Final) { while !self.is_done() {} self.i2s_rx.i2s.rx_stop(); @@ -346,9 +332,9 @@ where let (i2s_rx, buf) = self.release(); if i2s_rx.rx_channel.has_error() { - Err((DmaError::DescriptorError, i2s_rx, Buf::from_view(buf))) + (Err(DmaError::DescriptorError), i2s_rx, Buf::from_view(buf)) } else { - Ok((i2s_rx, Buf::from_view(buf))) + (Ok(()), i2s_rx, Buf::from_view(buf)) } } @@ -370,9 +356,7 @@ where Buf: DmaRxBuffer, { /// Waits for the transfer to finish and returns the peripheral and buffer. - pub async fn wait_async( - mut self, - ) -> Result<(I2sRx<'d, Async>, Buf::Final), (DmaError, I2sRx<'d, Async>, Buf::Final)> { + pub async fn wait_async(mut self) -> (Result<(), DmaError>, I2sRx<'d, Async>, Buf::Final) { if !self.completed { // we treat DescriptorEmpty as rx transfer is done if let Err(err) = DmaRxFuture::new_with_config( @@ -386,7 +370,7 @@ where self.i2s_rx.i2s.rx_stop(); self.i2s_rx.rx_channel.stop_transfer(); let (i2s_rx, buf) = self.release(); - return Err((err, i2s_rx, Buf::from_view(buf))); + return (Err(err), i2s_rx, Buf::from_view(buf)); } self.completed = true; } @@ -397,9 +381,9 @@ where let (i2s_rx, buf) = self.release(); if i2s_rx.rx_channel.has_error() { - Err((DmaError::DescriptorError, i2s_rx, Buf::from_view(buf))) + (Err(DmaError::DescriptorError), i2s_rx, Buf::from_view(buf)) } else { - Ok((i2s_rx, Buf::from_view(buf))) + (Ok(()), i2s_rx, Buf::from_view(buf)) } } diff --git a/hil-test/src/bin/embassy_timers_executors.rs b/hil-test/src/bin/embassy_timers_executors.rs index d9848b98a09..40f0fbeeae4 100644 --- a/hil-test/src/bin/embassy_timers_executors.rs +++ b/hil-test/src/bin/embassy_timers_executors.rs @@ -480,20 +480,15 @@ mod interrupt_spi_dma { async fn interrupt_driven_task(i2s_tx: esp_hal::i2s::master::I2s<'static, Blocking>) { use esp_hal::dma_tx_stream_buffer; - let mut buffer = dma_tx_stream_buffer!(128, 128); + let mut buffer = dma_tx_stream_buffer!(1024, 256); let mut i2s_tx = i2s_tx.into_async().i2s_tx.build(); loop { INTERRUPT_TASK_WORKING.store(true, portable_atomic::Ordering::Relaxed); - (i2s_tx, buffer) = i2s_tx - .write(buffer) - .ok() - .unwrap() - .wait_async() - .await - .ok() - .unwrap(); + let res; + (res, i2s_tx, buffer) = i2s_tx.write(buffer).ok().unwrap().wait_async().await; + assert!(res.is_ok(), "I2S write transfer failed: {:?}", res.err()); INTERRUPT_TASK_WORKING.store(false, portable_atomic::Ordering::Relaxed); if STOP_INTERRUPT_TASK.load(portable_atomic::Ordering::Relaxed) { diff --git a/hil-test/src/bin/i2s.rs b/hil-test/src/bin/i2s.rs index de056d2c5b3..6229a12c3b3 100644 --- a/hil-test/src/bin/i2s.rs +++ b/hil-test/src/bin/i2s.rs @@ -373,7 +373,8 @@ mod tests { .build(); let rx_transfer = i2s_rx.read(rx_buffer).unwrap(); - let (_, done_rx) = rx_transfer.wait().unwrap(); + let (res, _, done_rx) = rx_transfer.wait(); + assert!(res.is_ok(), "I2S read transfer failed: {:?}", res.err()); assert!(done_rx.number_of_received_bytes() != 0); @@ -422,7 +423,8 @@ mod tests { let mut rx_transfer = i2s_rx.read(rx_buffer).unwrap(); rx_transfer.wait_for_done_async().await.unwrap(); assert_eq!(rx_transfer.is_done(), true); - let (_, done_rx) = rx_transfer.wait_async().await.unwrap(); + let (res, _, done_rx) = rx_transfer.wait_async().await; + assert!(res.is_ok(), "I2S read transfer failed: {:?}", res.err()); assert!(done_rx.number_of_received_bytes() != 0); @@ -466,7 +468,8 @@ mod tests { .build(); let tx_transfer = i2s_tx.write(tx_buffer).unwrap(); - let (_, _done_tx) = tx_transfer.wait().unwrap(); + let (res, _, _done_tx) = tx_transfer.wait(); + assert!(res.is_ok(), "I2S read transfer failed: {:?}", res.err()); } // We don't actually check the output but just make sure the write completes. @@ -498,7 +501,8 @@ mod tests { let mut tx_transfer = i2s_tx.write(tx_buffer).unwrap(); tx_transfer.wait_for_done_async().await.unwrap(); assert_eq!(tx_transfer.is_done(), true); - let (_, _done_tx) = tx_transfer.wait_async().await.unwrap(); + let (res, _, _done_tx) = tx_transfer.wait_async().await; + assert!(res.is_ok(), "I2S read transfer failed: {:?}", res.err()); } } diff --git a/hil-test/src/bin/misc_non_drivers/dma_buffers.rs b/hil-test/src/bin/misc_non_drivers/dma_buffers.rs index e9bf4ed60e0..0e7259c0965 100644 --- a/hil-test/src/bin/misc_non_drivers/dma_buffers.rs +++ b/hil-test/src/bin/misc_non_drivers/dma_buffers.rs @@ -354,4 +354,19 @@ mod tests { core::assert_eq!(view.available_bytes(), 0); }); } + + #[test] + fn test_dma_tx_stream_buf_prefill() { + let mut buf = dma_tx_stream_buffer!(BUFFER_SIZE, CHUNK_SIZE); + buf.push(&[0xffu8; CHUNK_SIZE * 2]); + + buf.prepare(); + + // make sure initially all descriptors are owned by the DMA after prepare, even if we pushed + // data before + let (descriptors, _buffer) = buf.split(); + for desc in descriptors.iter() { + core::assert!(matches!(desc.owner(), Owner::Dma)); + } + } }