[DMA 3/8] Remove ChannelCreator types, temporarily disable burst mode (…

…#2403)

* Remove ChannelCreator types and burst mode

* Fix up I2sParallel

* Always enable burst transfering descriptors

* Configure burst transfer with a non-bool for future chip support

* Reuse buffer preparation code

* Update LoopBuf as well

* Update lcd_cam tests

* Rename config, fix changelog

esp-hal/CHANGELOG.md

@@ -10,13 +10,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - ESP32-S3: Added SDMMC signals (#2556)
+- `dma::{Channel, ChannelRx, ChannelTx}::set_priority` for GDMA devices (#2403)
 
 ### Changed
 
 ### Fixed
 
 ### Removed
 
+- The `configure` and `configure_for_async` DMA channel functions has been removed (#2403)
+
 ## [0.22.0] - 2024-11-20
 
 ### Added

esp-hal/MIGRATING-0.22.md

@@ -1 +1,17 @@
 # Migration Guide from 0.22.x to v1.0.0-beta.0
+
+## DMA configuration changes
+
+- `configure_for_async` and `configure` have been removed
+- PDMA devices (ESP32, ESP32-S2) provide no configurability
+- GDMA devices provide `set_priority` to change DMA in/out channel priority
+
+```diff
+ let mut spi = Spi::new_with_config(
+     peripherals.SPI2,
+     Config::default(),
+ )
+ // other setup
+-.with_dma(dma_channel.configure(false, DmaPriority::Priority0));
++.with_dma(dma_channel);
+```

esp-hal/src/dma/buffers.rs

@@ -8,25 +8,65 @@ use crate::soc::is_slice_in_dram;
 #[cfg(esp32s3)]
 use crate::soc::is_slice_in_psram;
 
+/// Burst transfer configuration.
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum BurstConfig {
+    /// Burst mode is disabled.
+    Disabled,
+
+    /// Burst mode is enabled.
+    Enabled,
+}
+
+impl BurstConfig {
+    pub(super) fn is_burst_enabled(self) -> bool {
+        !matches!(self, Self::Disabled)
+    }
+}
+
+/// The direction of the DMA transfer.
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum TransferDirection {
+    /// DMA transfer from peripheral or external memory to memory.
+    In,
+    /// DMA transfer from memory to peripheral or external memory.
+    Out,
+}
+
 /// Holds all the information needed to configure a DMA channel for a transfer.
 pub struct Preparation {
     /// The descriptor the DMA will start from.
     pub start: *mut DmaDescriptor,
 
-    /// Block size for PSRAM transfers
-    #[cfg_attr(not(esp32s3), allow(dead_code))]
-    pub block_size: Option<DmaBufBlkSize>,
+    /// The direction of the DMA transfer.
+    pub direction: TransferDirection,
 
-    /// Specifies whether descriptor linked list specified in `start` conforms
-    /// to the alignment requirements required to enable burst transfers.
+    /// Block size for PSRAM transfers.
     ///
-    /// Note: This only applies to burst transfer of the buffer data, not the
-    /// descriptors themselves.
+    /// If the buffer is in PSRAM, the implementation must ensure the following:
     ///
-    /// There are no additional alignment requirements for TX burst transfers,
-    /// but RX transfers require all descriptors to have buffer pointers and
-    /// sizes that are a multiple of 4 (word aligned).
-    pub is_burstable: bool,
+    /// - The implementation of the buffer must provide a non-`None` block size.
+    /// - For [`TransferDirection::In`] transfers, the implementation of the
+    ///   buffer must invalidate the cache that contains the buffer before the
+    ///   DMA starts.
+    /// - For [`TransferDirection::Out`] transfers, the implementation of the
+    ///   buffer must write back the cache that contains the buffer before the
+    ///   DMA starts.
+    #[cfg(esp32s3)]
+    pub external_memory_block_size: Option<DmaBufBlkSize>,
+
+    /// Configures the DMA to transfer data in bursts.
+    ///
+    /// The implementation of the buffer must ensure that burst mode is only
+    /// enabled when alignment requirements are met.
+    ///
+    /// There are no additional alignment requirements for
+    /// [`TransferDirection::Out`] burst transfers, but
+    /// [`TransferDirection::In`] transfers require all descriptors to have
+    /// buffer pointers and sizes that are a multiple of 4 (word aligned).
+    pub burst_transfer: BurstConfig,
 
     /// Configures the "check owner" feature of the DMA channel.
     ///
@@ -331,9 +371,11 @@ unsafe impl DmaTxBuffer for DmaTxBuf {
 
         Preparation {
             start: self.descriptors.head(),
-            block_size: self.block_size,
-            // This is TX, the DMA channel is free to do a burst transfer.
-            is_burstable: true,
+            direction: TransferDirection::Out,
+            #[cfg(esp32s3)]
+            external_memory_block_size: self.block_size,
+            // TODO: support burst transfers.
+            burst_transfer: BurstConfig::Disabled,
             check_owner: None,
         }
     }
@@ -480,11 +522,16 @@ unsafe impl DmaRxBuffer for DmaRxBuf {
 
         Preparation {
             start: self.descriptors.head(),
-            block_size: None,
-            // DmaRxBuf doesn't currently enforce the alignment requirements required for bursting.
-            // In the future, it could either enforce the alignment or calculate if the alignment
-            // requirements happen to be met.
-            is_burstable: false,
+            direction: TransferDirection::In,
+
+            // TODO: support external memory access.
+            #[cfg(esp32s3)]
+            external_memory_block_size: None,
+
+            // TODO: DmaRxBuf doesn't currently enforce the alignment requirements required for
+            // bursting. In the future, it could either enforce the alignment or
+            // calculate if the alignment requirements happen to be met.
+            burst_transfer: BurstConfig::Disabled,
             check_owner: None,
         }
     }
@@ -609,10 +656,14 @@ unsafe impl DmaTxBuffer for DmaRxTxBuf {
 
         Preparation {
             start: self.tx_descriptors.head(),
-            block_size: None, // TODO: support block size!
+            direction: TransferDirection::Out,
+
+            // TODO: support external memory access.
+            #[cfg(esp32s3)]
+            external_memory_block_size: None,
 
-            // This is TX, the DMA channel is free to do a burst transfer.
-            is_burstable: true,
+            // TODO: This is TX, the DMA channel is free to do a burst transfer.
+            burst_transfer: BurstConfig::Disabled,
             check_owner: None,
         }
     }
@@ -640,11 +691,15 @@ unsafe impl DmaRxBuffer for DmaRxTxBuf {
 
         Preparation {
             start: self.rx_descriptors.head(),
-            block_size: None, // TODO: support block size!
+            direction: TransferDirection::In,
 
-            // DmaRxTxBuf doesn't currently enforce the alignment requirements required for
+            // TODO: support external memory access.
+            #[cfg(esp32s3)]
+            external_memory_block_size: None,
+
+            // TODO: DmaRxTxBuf doesn't currently enforce the alignment requirements required for
             // bursting.
-            is_burstable: false,
+            burst_transfer: BurstConfig::Disabled,
             check_owner: None,
         }
     }
@@ -781,11 +836,15 @@ unsafe impl DmaRxBuffer for DmaRxStreamBuf {
         }
         Preparation {
             start: self.descriptors.as_mut_ptr(),
-            block_size: None,
+            direction: TransferDirection::In,
 
-            // DmaRxStreamBuf doesn't currently enforce the alignment requirements required for
-            // bursting.
-            is_burstable: false,
+            // TODO: support external memory access.
+            #[cfg(esp32s3)]
+            external_memory_block_size: None,
+
+            // TODO: DmaRxStreamBuf doesn't currently enforce the alignment requirements required
+            // for bursting.
+            burst_transfer: BurstConfig::Disabled,
 
             // 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.
@@ -995,10 +1054,10 @@ unsafe impl DmaTxBuffer for EmptyBuf {
         #[allow(unused_unsafe)] // stable requires unsafe, nightly complains about it
         Preparation {
             start: unsafe { core::ptr::addr_of_mut!(EMPTY).cast() },
-            block_size: None,
-
-            // This is TX, the DMA channel is free to do a burst transfer.
-            is_burstable: true,
+            direction: TransferDirection::Out,
+            #[cfg(esp32s3)]
+            external_memory_block_size: None,
+            burst_transfer: BurstConfig::Disabled,
 
             // As we don't give ownership of the descriptor to the DMA, it's important that the DMA
             // channel does *NOT* check for ownership, otherwise the channel will return an error.
@@ -1026,10 +1085,10 @@ unsafe impl DmaRxBuffer for EmptyBuf {
         #[allow(unused_unsafe)] // stable requires unsafe, nightly complains about it
         Preparation {
             start: unsafe { core::ptr::addr_of_mut!(EMPTY).cast() },
-            block_size: None,
-
-            // As much as bursting is meaningless here, the descriptor does meet the requirements.
-            is_burstable: true,
+            direction: TransferDirection::In,
+            #[cfg(esp32s3)]
+            external_memory_block_size: None,
+            burst_transfer: BurstConfig::Disabled,
 
             // As we don't give ownership of the descriptor to the DMA, it's important that the DMA
             // channel does *NOT* check for ownership, otherwise the channel will return an error.
@@ -1104,8 +1163,11 @@ unsafe impl DmaTxBuffer for DmaLoopBuf {
     fn prepare(&mut self) -> Preparation {
         Preparation {
             start: self.descriptor,
-            block_size: None,
-            is_burstable: true,
+            direction: TransferDirection::Out,
+            // TODO: support external memory access.
+            #[cfg(esp32s3)]
+            external_memory_block_size: None,
+            burst_transfer: BurstConfig::Disabled,
             // The DMA must not check the owner bit, as it is never set.
             check_owner: Some(false),
         }

esp-hal/src/dma/gdma.rs

@@ -169,11 +169,16 @@ impl<C: GdmaChannel> RegisterAccess for ChannelTxImpl<C> {
         conf0.modify(|_, w| w.out_rst().clear_bit());
     }
 
-    fn set_burst_mode(&self, burst_mode: bool) {
-        self.ch().out_conf0().modify(|_, w| {
-            w.out_data_burst_en().bit(burst_mode);
-            w.outdscr_burst_en().bit(burst_mode)
-        });
+    fn set_burst_mode(&self, burst_mode: BurstConfig) {
+        self.ch()
+            .out_conf0()
+            .modify(|_, w| w.out_data_burst_en().bit(burst_mode.is_burst_enabled()));
+    }
+
+    fn set_descr_burst_mode(&self, burst_mode: bool) {
+        self.ch()
+            .out_conf0()
+            .modify(|_, w| w.outdscr_burst_en().bit(burst_mode));
     }
 
     fn set_priority(&self, priority: DmaPriority) {
@@ -388,11 +393,16 @@ impl<C: GdmaChannel> RegisterAccess for ChannelRxImpl<C> {
         conf0.modify(|_, w| w.in_rst().clear_bit());
     }
 
-    fn set_burst_mode(&self, burst_mode: bool) {
-        self.ch().in_conf0().modify(|_, w| {
-            w.in_data_burst_en().bit(burst_mode);
-            w.indscr_burst_en().bit(burst_mode)
-        });
+    fn set_burst_mode(&self, burst_mode: BurstConfig) {
+        self.ch()
+            .in_conf0()
+            .modify(|_, w| w.in_data_burst_en().bit(burst_mode.is_burst_enabled()));
+    }
+
+    fn set_descr_burst_mode(&self, burst_mode: bool) {
+        self.ch()
+            .in_conf0()
+            .modify(|_, w| w.indscr_burst_en().bit(burst_mode));
     }
 
     fn set_priority(&self, priority: DmaPriority) {
@@ -559,10 +569,6 @@ impl<C: GdmaChannel> InterruptAccess<DmaRxInterrupt> for ChannelRxImpl<C> {
     }
 }
 
-/// A Channel can be created from this
-#[non_exhaustive]
-pub struct ChannelCreator<const N: u8> {}
-
 impl<CH: DmaChannel, M: Mode> Channel<'_, M, CH> {
     /// Asserts that the channel is compatible with the given peripheral.
     pub fn runtime_ensure_compatible<P: DmaEligible>(&self, _peripheral: &PeripheralRef<'_, P>) {
@@ -621,19 +627,19 @@ macro_rules! impl_channel {
                 }
             }
 
-            impl ChannelCreator<$num> {
-                /// Configure the channel for use with blocking APIs
-                pub fn configure<'a>(
-                    self,
-                    burst_mode: bool,
-                    priority: DmaPriority,
-                ) -> Channel<'a, Blocking, [<DmaChannel $num>]> {
+            impl [<DmaChannel $num>] {
+                /// Unsafely constructs a new DMA channel.
+                ///
+                /// # Safety
+                ///
+                /// The caller must ensure that only a single instance is used.
+                pub unsafe fn steal<'a>() -> Channel<'a, Blocking, Self> {
                     let mut this = Channel {
                         tx: ChannelTx::new(ChannelTxImpl(SpecificGdmaChannel::<$num> {})),
                         rx: ChannelRx::new(ChannelRxImpl(SpecificGdmaChannel::<$num> {})),
                     };
 
-                    this.configure(burst_mode, priority);
+                    this.set_priority(DmaPriority::Priority0);
 
                     this
                 }
@@ -731,19 +737,19 @@ crate::impl_dma_eligible! {
 pub struct Dma<'d> {
     _inner: PeripheralRef<'d, crate::peripherals::DMA>,
     /// Channel 0
-    pub channel0: ChannelCreator<0>,
+    pub channel0: Channel<'d, Blocking, DmaChannel0>,
     /// Channel 1
     #[cfg(not(esp32c2))]
-    pub channel1: ChannelCreator<1>,
+    pub channel1: Channel<'d, Blocking, DmaChannel1>,
     /// Channel 2
     #[cfg(not(esp32c2))]
-    pub channel2: ChannelCreator<2>,
+    pub channel2: Channel<'d, Blocking, DmaChannel2>,
     /// Channel 3
     #[cfg(esp32s3)]
-    pub channel3: ChannelCreator<3>,
+    pub channel3: Channel<'d, Blocking, DmaChannel3>,
     /// Channel 4
     #[cfg(esp32s3)]
-    pub channel4: ChannelCreator<4>,
+    pub channel4: Channel<'d, Blocking, DmaChannel4>,
 }
 
 impl<'d> Dma<'d> {
@@ -761,17 +767,19 @@ impl<'d> Dma<'d> {
             .modify(|_, w| w.ahbm_rst_inter().clear_bit());
         dma.misc_conf().modify(|_, w| w.clk_en().set_bit());
 
-        Dma {
-            _inner: dma,
-            channel0: ChannelCreator {},
-            #[cfg(not(esp32c2))]
-            channel1: ChannelCreator {},
-            #[cfg(not(esp32c2))]
-            channel2: ChannelCreator {},
-            #[cfg(esp32s3)]
-            channel3: ChannelCreator {},
-            #[cfg(esp32s3)]
-            channel4: ChannelCreator {},
+        unsafe {
+            Dma {
+                _inner: dma,
+                channel0: DmaChannel0::steal(),
+                #[cfg(not(esp32c2))]
+                channel1: DmaChannel1::steal(),
+                #[cfg(not(esp32c2))]
+                channel2: DmaChannel2::steal(),
+                #[cfg(esp32s3)]
+                channel3: DmaChannel3::steal(),
+                #[cfg(esp32s3)]
+                channel4: DmaChannel4::steal(),
+            }
         }
     }
 }