Skip to content

Commit

Permalink
Configure burst transfer with a non-bool for future chip support
Browse files Browse the repository at this point in the history
  • Loading branch information
@bugadani
bugadani committed Nov 14, 2024
1 parent b0a7aeb commit 5317fe5
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 59 deletions.
115 changes: 83 additions & 32 deletions esp-hal/src/dma/buffers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,57 @@ 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 BurstTransfer {
/// Burst mode is disabled.
Disabled,

/// Burst mode is enabled.
Enabled,
}

impl BurstTransfer {
pub(super) fn is_burst_enabled(self) -> bool {
!matches!(self, Self::Disabled)
}
}

/// Maximum length of a burst 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 the data should be transferred in burst mode.
/// Block size for PSRAM transfers.
///
/// The implementation of the buffer must provide a block size if the data
/// is in PSRAM.
#[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 TX burst transfers,
/// but RX transfers require all descriptors to have buffer pointers and
/// sizes that are a multiple of 4 (word aligned).
// TODO: currently unused, buffers should not hardcode their preference.
pub burst: bool,
pub burst_transfer: BurstTransfer,

/// Configures the "check owner" feature of the DMA channel.
///
Expand Down Expand Up @@ -328,9 +360,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.
burst: true,
direction: TransferDirection::Out,
#[cfg(esp32s3)]
external_memory_block_size: self.block_size,
// TODO: support burst transfers.
burst_transfer: BurstTransfer::Disabled,
check_owner: None,
}
}
Expand Down Expand Up @@ -477,11 +511,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.
burst: 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: BurstTransfer::Disabled,
check_owner: None,
}
}
Expand Down Expand Up @@ -606,10 +645,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.
burst: true,
// TODO: This is TX, the DMA channel is free to do a burst transfer.
burst_transfer: BurstTransfer::Disabled,
check_owner: None,
}
}
Expand Down Expand Up @@ -637,11 +680,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.
burst: false,
burst_transfer: BurstTransfer::Disabled,
check_owner: None,
}
}
Expand Down Expand Up @@ -778,11 +825,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.
burst: 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: BurstTransfer::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 @@ -992,10 +1043,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.
burst: true,
direction: TransferDirection::Out,
#[cfg(esp32s3)]
external_memory_block_size: None,
burst_transfer: BurstTransfer::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 @@ -1023,10 +1074,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.
burst: true,
direction: TransferDirection::In,
#[cfg(esp32s3)]
external_memory_block_size: None,
burst_transfer: BurstTransfer::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
8 changes: 4 additions & 4 deletions esp-hal/src/dma/gdma.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ impl<C: GdmaChannel> RegisterAccess for ChannelTxImpl<C> {
conf0.modify(|_, w| w.out_rst().clear_bit());
}

fn set_burst_mode(&self, burst_mode: bool) {
fn set_burst_mode(&self, burst_mode: BurstTransfer) {
self.ch()
.out_conf0()
.modify(|_, w| w.out_data_burst_en().bit(burst_mode));
.modify(|_, w| w.out_data_burst_en().bit(burst_mode.is_burst_enabled()));
}

fn set_descr_burst_mode(&self, burst_mode: bool) {
Expand Down Expand Up @@ -393,10 +393,10 @@ impl<C: GdmaChannel> RegisterAccess for ChannelRxImpl<C> {
conf0.modify(|_, w| w.in_rst().clear_bit());
}

fn set_burst_mode(&self, burst_mode: bool) {
fn set_burst_mode(&self, burst_mode: BurstTransfer) {
self.ch()
.in_conf0()
.modify(|_, w| w.in_data_burst_en().bit(burst_mode));
.modify(|_, w| w.in_data_burst_en().bit(burst_mode.is_burst_enabled()));
}

fn set_descr_burst_mode(&self, burst_mode: bool) {
Expand Down
32 changes: 15 additions & 17 deletions esp-hal/src/dma/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,9 @@ where
) -> Result<(), DmaError> {
let preparation = buffer.prepare();

self.rx_impl.set_burst_mode(false);
debug_assert_eq!(preparation.direction, TransferDirection::In);

self.rx_impl.set_burst_mode(preparation.burst_transfer);
self.rx_impl.set_descr_burst_mode(true);
self.rx_impl.set_check_owner(preparation.check_owner);

Expand Down Expand Up @@ -2058,8 +2060,9 @@ where
peri: DmaPeripheral,
chain: &DescriptorChain,
) -> Result<(), DmaError> {
// TODO: based on the ESP32-S3 TRM the alignment check is not needed for TX!
// for esp32s3 we check each descriptor buffer that points to psram for
// Based on the ESP32-S3 TRM the alignment check is not needed for TX

// For esp32s3 we check each descriptor buffer that points to PSRAM for
// alignment and writeback the cache for that buffer
#[cfg(esp32s3)]
for des in chain.descriptors.iter() {
Expand Down Expand Up @@ -2095,20 +2098,15 @@ where
buffer: &mut BUF,
) -> Result<(), DmaError> {
let preparation = buffer.prepare();
cfg_if::cfg_if!(
if #[cfg(esp32s3)] {
if let Some(block_size) = preparation.block_size {
self.set_ext_mem_block_size(block_size.into());
}
} else {
// we ensure that block_size is some only for PSRAM addresses
if preparation.block_size.is_some() {
return Err(DmaError::UnsupportedMemoryRegion);
}
}
);

self.tx_impl.set_burst_mode(false);
debug_assert_eq!(preparation.direction, TransferDirection::Out);

#[cfg(esp32s3)]
if let Some(block_size) = preparation.external_memory_block_size {
self.set_ext_mem_block_size(block_size.into());
}

self.tx_impl.set_burst_mode(preparation.burst_transfer);
self.tx_impl.set_descr_burst_mode(true);
self.tx_impl.set_check_owner(preparation.check_owner);

Expand Down Expand Up @@ -2184,7 +2182,7 @@ pub trait RegisterAccess: crate::private::Sealed {

/// Enable/Disable INCR burst transfer for channel reading
/// accessing data in internal RAM.
fn set_burst_mode(&self, burst_mode: bool);
fn set_burst_mode(&self, burst_mode: BurstTransfer);

/// Enable/Disable burst transfer for channel reading
/// descriptors in internal RAM.
Expand Down
12 changes: 6 additions & 6 deletions esp-hal/src/dma/pdma.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ impl<C: PdmaChannel<RegisterBlock = SpiRegisterBlock>> RegisterAccess for SpiDma
spi.dma_conf().modify(|_, w| w.out_rst().clear_bit());
}

fn set_burst_mode(&self, burst_mode: bool) {
fn set_burst_mode(&self, burst_mode: BurstTransfer) {
let spi = self.0.register_block();
spi.dma_conf()
.modify(|_, w| w.out_data_burst_en().bit(burst_mode));
.modify(|_, w| w.out_data_burst_en().bit(burst_mode.is_burst_enabled()));
}

fn set_descr_burst_mode(&self, burst_mode: bool) {
Expand Down Expand Up @@ -222,7 +222,7 @@ impl<C: PdmaChannel<RegisterBlock = SpiRegisterBlock>> RegisterAccess for SpiDma
spi.dma_conf().modify(|_, w| w.in_rst().clear_bit());
}

fn set_burst_mode(&self, _burst_mode: bool) {}
fn set_burst_mode(&self, _burst_mode: BurstTransfer) {}

fn set_descr_burst_mode(&self, burst_mode: bool) {
let spi = self.0.register_block();
Expand Down Expand Up @@ -480,11 +480,11 @@ impl<C: PdmaChannel<RegisterBlock = I2sRegisterBlock>> RegisterAccess for I2sDma
reg_block.lc_conf().modify(|_, w| w.out_rst().clear_bit());
}

fn set_burst_mode(&self, burst_mode: bool) {
fn set_burst_mode(&self, burst_mode: BurstTransfer) {
let reg_block = self.0.register_block();
reg_block
.lc_conf()
.modify(|_, w| w.out_data_burst_en().bit(burst_mode));
.modify(|_, w| w.out_data_burst_en().bit(burst_mode.is_burst_enabled()));
}

fn set_descr_burst_mode(&self, burst_mode: bool) {
Expand Down Expand Up @@ -659,7 +659,7 @@ impl<C: PdmaChannel<RegisterBlock = I2sRegisterBlock>> RegisterAccess for I2sDma
reg_block.lc_conf().modify(|_, w| w.in_rst().clear_bit());
}

fn set_burst_mode(&self, _burst_mode: bool) {}
fn set_burst_mode(&self, _burst_mode: BurstTransfer) {}

fn set_descr_burst_mode(&self, burst_mode: bool) {
let reg_block = self.0.register_block();
Expand Down

0 comments on commit 5317fe5

Please sign in to comment.