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

Support PSRAM in DmaTxBuf #2161

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Renamed `DummyPin` to `NoPin` and removed all internal logic from it. (#2133)
- The `NO_PIN` constant has been removed. (#2133)
- MSRV bump to 1.79 (#2156)
- Change `DmaTxBuf` to support PSRAM on `esp32s3` (#2161)

### Fixed

Expand Down
181 changes: 168 additions & 13 deletions esp-hal/src/dma/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ where
bitfield::bitfield! {
#[doc(hidden)]
#[derive(Clone, Copy)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub struct DmaDescriptorFlags(u32);

u16;
Expand All @@ -226,6 +225,20 @@ impl Debug for DmaDescriptorFlags {
}
}

#[cfg(feature = "defmt")]
impl defmt::Format for DmaDescriptorFlags {
fn format(&self, fmt: defmt::Formatter<'_>) {
defmt::write!(
fmt,
"DmaDescriptorFlags {{ size: {}, length: {}, suc_eof: {}, owner: {} }}",
self.size(),
self.length(),
self.suc_eof(),
if self.owner() { "DMA" } else { "CPU" }
);
}
}

/// A DMA transfer descriptor.
#[derive(Clone, Copy, Debug)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
Expand Down Expand Up @@ -286,6 +299,8 @@ use enumset::{EnumSet, EnumSetType};
pub use self::gdma::*;
#[cfg(pdma)]
pub use self::pdma::*;
#[cfg(esp32s3)]
use crate::soc::is_slice_in_psram;
use crate::{interrupt::InterruptHandler, soc::is_slice_in_dram, Mode};

#[cfg(gdma)]
Expand Down Expand Up @@ -511,7 +526,7 @@ macro_rules! dma_circular_buffers_chunk_size {
macro_rules! dma_descriptors_chunk_size {
liebman marked this conversation as resolved.
Show resolved Hide resolved
($rx_size:expr, $tx_size:expr, $chunk_size:expr) => {{
// these will check for size at compile time
const _: () = ::core::assert!($chunk_size <= 4092, "chunk size must be <= 4092");
const _: () = ::core::assert!($chunk_size <= 4095, "chunk size must be <= 4095");
const _: () = ::core::assert!($chunk_size > 0, "chunk size must be > 0");

static mut RX_DESCRIPTORS: [$crate::dma::DmaDescriptor;
Expand Down Expand Up @@ -546,7 +561,7 @@ macro_rules! dma_descriptors_chunk_size {
macro_rules! dma_circular_descriptors_chunk_size {
($rx_size:expr, $tx_size:expr, $chunk_size:expr) => {{
// these will check for size at compile time
const _: () = ::core::assert!($chunk_size <= 4092, "chunk size must be <= 4092");
const _: () = ::core::assert!($chunk_size <= 4095, "chunk size must be <= 4095");
const _: () = ::core::assert!($chunk_size > 0, "chunk size must be > 0");

const rx_descriptor_len: usize = if $rx_size > $chunk_size * 2 {
Expand All @@ -573,6 +588,32 @@ macro_rules! dma_circular_descriptors_chunk_size {
};
}

/// Convenience macro to create a DmaTxBuf from buffer size. The buffer and
/// descriptors are statically allocated and used to create the `DmaTxBuf`.
///
/// ## Usage
/// ```rust,no_run
#[doc = crate::before_snippet!()]
/// use esp_hal::dma_tx_buffer;
/// use esp_hal::dma::DmaBufBlkSize;
///
/// let tx_buf =
/// dma_tx_buffer!(32000);
/// # }
/// ```
#[macro_export]
macro_rules! dma_tx_buffer {
($tx_size:expr) => {{
const TX_DESCRIPTOR_LEN: usize =
$crate::dma::DmaTxBuf::compute_descriptor_count($tx_size, None);
static mut TX_BUFFER: [u8; $tx_size] = [0u8; $tx_size];
static mut TX_DESCRIPTORS: [$crate::dma::DmaDescriptor; TX_DESCRIPTOR_LEN] =
[$crate::dma::DmaDescriptor::EMPTY; TX_DESCRIPTOR_LEN];
let (tx_buffer, tx_descriptors) = unsafe { (&mut TX_BUFFER, &mut TX_DESCRIPTORS) };
$crate::dma::DmaTxBuf::new(tx_descriptors, tx_buffer)
}};
}

/// DMA Errors
#[derive(Debug, Clone, Copy, PartialEq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
Expand Down Expand Up @@ -954,6 +995,16 @@ pub enum DmaExtMemBKSize {
Size64 = 2,
}

impl From<DmaBufBlkSize> for DmaExtMemBKSize {
liebman marked this conversation as resolved.
Show resolved Hide resolved
fn from(size: DmaBufBlkSize) -> Self {
match size {
DmaBufBlkSize::Size16 => DmaExtMemBKSize::Size16,
DmaBufBlkSize::Size32 => DmaExtMemBKSize::Size32,
DmaBufBlkSize::Size64 => DmaExtMemBKSize::Size64,
}
}
}

pub(crate) struct TxCircularState {
write_offset: usize,
write_descr_ptr: *mut DmaDescriptor,
Expand Down Expand Up @@ -1387,7 +1438,6 @@ where
if des.buffer as usize % alignment != 0 && des.size() % alignment != 0 {
return Err(DmaError::InvalidAlignment);
}
// TODO: make this optional?
crate::soc::cache_invalidate_addr(des.buffer as u32, des.size() as u32);
}
}
Expand Down Expand Up @@ -1704,6 +1754,7 @@ 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
// alignment and writeback the cache for that buffer
#[cfg(esp32s3)]
Expand All @@ -1729,7 +1780,19 @@ 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 insure that block_size is some only for PSRAM addresses
if preparation.block_size.is_some() {
return Err(DmaError::UnsupportedMemoryRegion);
}
}
);
// TODO: Get burst mode from DmaBuf.
self.tx_impl
.prepare_transfer_without_start(preparation.start, peri)
}
Expand Down Expand Up @@ -1967,6 +2030,10 @@ where
/// Holds all the information needed to configure a DMA channel for a transfer.
pub struct Preparation {
start: *mut DmaDescriptor,
/// block size for PSRAM transfers (TODO: enable burst mode for non external
/// memory?)
#[cfg_attr(not(esp32s3), allow(dead_code))]
block_size: Option<DmaBufBlkSize>,
// burst_mode, alignment, check_owner, etc.
}

Expand Down Expand Up @@ -2014,17 +2081,35 @@ pub enum DmaBufError {
InsufficientDescriptors,
/// Descriptors or buffers are not located in a supported memory region
UnsupportedMemoryRegion,
/// Buffer is not aligned to the required size
InvalidAlignment,
/// Invalid chunk size: must be > 0 and <= 4095
InvalidChunkSize,
}

/// DMA buffer allignments
#[derive(Debug, Clone, Copy, PartialEq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum DmaBufBlkSize {
/// 16 bytes
Size16 = 16,
/// 32 bytes
Size32 = 32,
/// 64 bytes
Size64 = 64,
}

/// DMA transmit buffer
///
/// This is a contiguous buffer linked together by DMA descriptors of length
/// 4092. It can only be used for transmitting data to a peripheral's FIFO.
/// See [DmaRxBuf] for receiving data.
/// 4095 at most. It can only be used for transmitting data to a peripheral's
/// FIFO. See [DmaRxBuf] for receiving data.
#[derive(Debug)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub struct DmaTxBuf {
descriptors: &'static mut [DmaDescriptor],
buffer: &'static mut [u8],
block_size: Option<DmaBufBlkSize>,
}

impl DmaTxBuf {
Expand All @@ -2034,23 +2119,75 @@ impl DmaTxBuf {
/// Each descriptor can handle 4092 bytes worth of buffer.
///
/// Both the descriptors and buffer must be in DMA-capable memory.
/// Only DRAM is supported.
/// Only DRAM is supported for descriptors.
pub fn new(
descriptors: &'static mut [DmaDescriptor],
buffer: &'static mut [u8],
) -> Result<Self, DmaBufError> {
let min_descriptors = buffer.len().div_ceil(CHUNK_SIZE);
Self::new_with_block_size(descriptors, buffer, None)
}

/// Compute max chunk size based on block size
pub const fn compute_chunk_size(block_size: Option<DmaBufBlkSize>) -> usize {
match block_size {
Some(size) => 4096 - size as usize,
#[cfg(esp32)]
None => 4092, // esp32 requires 4 byte alignment
#[cfg(not(esp32))]
None => 4095,
}
}

/// Compute the number of descriptors required for a given block size and
/// buffer size
pub const fn compute_descriptor_count(
buffer_size: usize,
block_size: Option<DmaBufBlkSize>,
) -> usize {
buffer_size.div_ceil(Self::compute_chunk_size(block_size))
}

/// Creates a new [DmaTxBuf] from some descriptors and a buffer.
///
/// There must be enough descriptors for the provided buffer.
/// Each descriptor can handle at most 4095 bytes worth of buffer.
/// Optionally, a block size can be provided for PSRAM & Burst transfers.
///
/// Both the descriptors and buffer must be in DMA-capable memory.
/// Only DRAM is supported for descriptors.
pub fn new_with_block_size(
descriptors: &'static mut [DmaDescriptor],
buffer: &'static mut [u8],
block_size: Option<DmaBufBlkSize>,
) -> Result<Self, DmaBufError> {
let chunk_size = Self::compute_chunk_size(block_size);
let min_descriptors = Self::compute_descriptor_count(buffer.len(), block_size);
if descriptors.len() < min_descriptors {
return Err(DmaBufError::InsufficientDescriptors);
}

if !is_slice_in_dram(descriptors) || !is_slice_in_dram(buffer) {
// descriptors are required to be in DRAM
if !is_slice_in_dram(descriptors) {
return Err(DmaBufError::UnsupportedMemoryRegion);
}

cfg_if::cfg_if! {
if #[cfg(esp32s3)] {
// buffer can be either DRAM or PSRAM (if supported)
if !is_slice_in_dram(buffer) && !is_slice_in_psram(buffer) {
return Err(DmaBufError::UnsupportedMemoryRegion);
}
} else {
// buffer can only be DRAM
if !is_slice_in_dram(buffer) {
return Err(DmaBufError::UnsupportedMemoryRegion);
}
}
}

// Setup size and buffer pointer as these will not change for the remainder of
// this object's lifetime
let chunk_iter = descriptors.iter_mut().zip(buffer.chunks_mut(CHUNK_SIZE));
let chunk_iter = descriptors.iter_mut().zip(buffer.chunks_mut(chunk_size));
for (desc, chunk) in chunk_iter {
desc.set_size(chunk.len());
desc.buffer = chunk.as_mut_ptr();
Expand All @@ -2059,9 +2196,13 @@ impl DmaTxBuf {
let mut buf = Self {
descriptors,
buffer,
block_size,
};
buf.set_length(buf.capacity());

// no need for block size if the buffer is in DRAM
if is_slice_in_dram(buf.buffer) {
buf.block_size = None;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a check here to make sure block_size is provided when the buffer is in PSRAM

Copy link
Contributor Author

@liebman liebman Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested dma from psram with all of the block sizes and they all worked on octal psram. I think even none worked - that would have used the default register value. But yea - maybe we should force users to be specific.

Ok(buf)
}

Expand Down Expand Up @@ -2097,7 +2238,7 @@ impl DmaTxBuf {
assert!(len <= self.buffer.len());

// Get the minimum number of descriptors needed for this length of data.
let descriptor_count = len.div_ceil(CHUNK_SIZE).max(1);
let descriptor_count = len.div_ceil(self.descriptors[0].size()).max(1);
let required_descriptors = &mut self.descriptors[0..descriptor_count];

// Link up the relevant descriptors.
Expand Down Expand Up @@ -2158,8 +2299,19 @@ impl DmaTxBuffer for DmaTxBuf {
}
}

#[cfg(esp32s3)]
if crate::soc::is_valid_psram_address(self.buffer.as_ptr() as u32) {
unsafe {
crate::soc::cache_writeback_addr(
self.buffer.as_ptr() as u32,
self.buffer.len() as u32,
)
};
}

Preparation {
start: self.descriptors.as_mut_ptr(),
block_size: self.block_size,
}
}

Expand Down Expand Up @@ -2396,6 +2548,7 @@ impl DmaRxBuffer for DmaRxBuf {

Preparation {
start: self.descriptors.as_mut_ptr(),
block_size: None,
}
}

Expand Down Expand Up @@ -2588,6 +2741,7 @@ impl DmaTxBuffer for DmaRxTxBuf {

Preparation {
start: self.tx_descriptors.as_mut_ptr(),
block_size: None, // TODO: support block size!
}
}

Expand Down Expand Up @@ -2617,6 +2771,7 @@ impl DmaRxBuffer for DmaRxTxBuf {

Preparation {
start: self.rx_descriptors.as_mut_ptr(),
block_size: None, // TODO: support block size!
}
}

Expand Down
7 changes: 7 additions & 0 deletions esp-hal/src/soc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ pub(crate) fn is_valid_psram_address(address: u32) -> bool {
false
}

#[allow(unused)]
pub(crate) fn is_slice_in_psram<T>(slice: &[T]) -> bool {
let start = slice.as_ptr() as u32;
let end = start + slice.len() as u32;
is_valid_psram_address(start) && is_valid_psram_address(end)
}

#[allow(unused)]
pub(crate) fn is_valid_memory_address(address: u32) -> bool {
is_valid_ram_address(address) || is_valid_psram_address(address)
Expand Down
Loading
Loading