Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DMA 3/8] Remove ChannelCreator types, temporarily disable burst mode #2403

Merged
merged 8 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions esp-hal/MIGRATING-0.22.md
Original file line number Diff line number Diff line change
@@ -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);
```
140 changes: 101 additions & 39 deletions esp-hal/src/dma/buffers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
bugadani marked this conversation as resolved.
Show resolved Hide resolved
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,
MabezDev marked this conversation as resolved.
Show resolved Hide resolved

/// 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)]
bugadani marked this conversation as resolved.
Show resolved Hide resolved
pub external_memory_block_size: Option<DmaBufBlkSize>,
bugadani marked this conversation as resolved.
Show resolved Hide resolved

/// 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.
///
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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),
}
Expand Down
84 changes: 46 additions & 38 deletions esp-hal/src/dma/gdma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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>) {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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> {
Expand All @@ -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(),
}
}
}
}
Loading