GetLitFam/esp-hal
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix DMA starving SPI (#2152)

Browse Source 
* Fix DMA starving SPI

* Simplify cfgs

* Trigger an update before starting transaction

* Do not update after enable_dma, use start_operation
This commit is contained in:
Dániel Buga 2024-09-13 10:51:31 +02:00 committed by GitHub
parent 351a64937b
commit 2a74addee0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 65 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
esp-hal/src/spi/master.rs
View File

@ -61,9 +61,9 @@
use core::marker::PhantomData; use core::marker::PhantomData;
pub use dma::*; pub use dma::*;
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
use enumset::EnumSet; use enumset::EnumSet;
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
use enumset::EnumSetType; use enumset::EnumSetType;
use fugit::HertzU32; use fugit::HertzU32;
#[cfg(feature = "place-spi-driver-in-ram")] #[cfg(feature = "place-spi-driver-in-ram")]
@ -91,7 +91,7 @@ use crate::{
}; };
/// Enumeration of possible SPI interrupt events. /// Enumeration of possible SPI interrupt events.
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
#[derive(EnumSetType)] #[derive(EnumSetType)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))] #[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum SpiInterrupt { pub enum SpiInterrupt {
@ -999,25 +999,25 @@ mod dma {
} }
/// Listen for the given interrupts /// Listen for the given interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
pub fn listen(&mut self, interrupts: EnumSet<SpiInterrupt>) { pub fn listen(&mut self, interrupts: EnumSet<SpiInterrupt>) {
self.spi.listen(interrupts); self.spi.listen(interrupts);
} }
/// Unlisten the given interrupts /// Unlisten the given interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
pub fn unlisten(&mut self, interrupts: EnumSet<SpiInterrupt>) { pub fn unlisten(&mut self, interrupts: EnumSet<SpiInterrupt>) {
self.spi.unlisten(interrupts); self.spi.unlisten(interrupts);
} }
/// Gets asserted interrupts /// Gets asserted interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
pub fn interrupts(&mut self) -> EnumSet<SpiInterrupt> { pub fn interrupts(&mut self) -> EnumSet<SpiInterrupt> {
self.spi.interrupts() self.spi.interrupts()
} }
/// Resets asserted interrupts /// Resets asserted interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
pub fn clear_interrupts(&mut self, interrupts: EnumSet<SpiInterrupt>) { pub fn clear_interrupts(&mut self, interrupts: EnumSet<SpiInterrupt>) {
self.spi.clear_interrupts(interrupts); self.spi.clear_interrupts(interrupts);
} }
@ -1516,7 +1516,7 @@ mod dma {
} }
/// Listen for the given interrupts /// Listen for the given interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
pub fn listen(&mut self, interrupts: EnumSet<SpiInterrupt>) { pub fn listen(&mut self, interrupts: EnumSet<SpiInterrupt>) {
let (mut spi_dma, rx_buf, tx_buf) = self.wait_for_idle(); let (mut spi_dma, rx_buf, tx_buf) = self.wait_for_idle();
spi_dma.listen(interrupts); spi_dma.listen(interrupts);
@ -1524,7 +1524,7 @@ mod dma {
} }
/// Unlisten the given interrupts /// Unlisten the given interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
pub fn unlisten(&mut self, interrupts: EnumSet<SpiInterrupt>) { pub fn unlisten(&mut self, interrupts: EnumSet<SpiInterrupt>) {
let (mut spi_dma, rx_buf, tx_buf) = self.wait_for_idle(); let (mut spi_dma, rx_buf, tx_buf) = self.wait_for_idle();
spi_dma.unlisten(interrupts); spi_dma.unlisten(interrupts);
@ -1532,7 +1532,7 @@ mod dma {
} }
/// Gets asserted interrupts /// Gets asserted interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
pub fn interrupts(&mut self) -> EnumSet<SpiInterrupt> { pub fn interrupts(&mut self) -> EnumSet<SpiInterrupt> {
let (mut spi_dma, rx_buf, tx_buf) = self.wait_for_idle(); let (mut spi_dma, rx_buf, tx_buf) = self.wait_for_idle();
let interrupts = spi_dma.interrupts(); let interrupts = spi_dma.interrupts();
@ -1542,7 +1542,7 @@ mod dma {
} }
/// Resets asserted interrupts /// Resets asserted interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
pub fn clear_interrupts(&mut self, interrupts: EnumSet<SpiInterrupt>) { pub fn clear_interrupts(&mut self, interrupts: EnumSet<SpiInterrupt>) {
let (mut spi_dma, rx_buf, tx_buf) = self.wait_for_idle(); let (mut spi_dma, rx_buf, tx_buf) = self.wait_for_idle();
spi_dma.clear_interrupts(interrupts); spi_dma.clear_interrupts(interrupts);
@ -2231,9 +2231,8 @@ pub trait InstanceDma: Instance {
.modify(|_, w| w.usr_miso().bit(true).usr_mosi().bit(true)); .modify(|_, w| w.usr_miso().bit(true).usr_mosi().bit(true));
self.enable_dma(); self.enable_dma();
self.update();
self.clear_dma_interrupts(); self.clear_dma_interrupts();
reset_dma_before_load_dma_dscr(reg_block); reset_dma_before_load_dma_dscr(reg_block);
rx.prepare_transfer(self.dma_peripheral(), rx_buffer) rx.prepare_transfer(self.dma_peripheral(), rx_buffer)
.and_then(|_| rx.start_transfer())?; .and_then(|_| rx.start_transfer())?;
@ -2242,7 +2241,7 @@ pub trait InstanceDma: Instance {
reset_dma_before_usr_cmd(reg_block); reset_dma_before_usr_cmd(reg_block);
reg_block.cmd().modify(|_, w| w.usr().set_bit()); self.start_operation();
Ok(()) Ok(())
} }
@ -2281,23 +2280,16 @@ pub trait InstanceDma: Instance {
} }
self.enable_dma(); self.enable_dma();
self.update(); self.clear_dma_interrupts();
reset_dma_before_load_dma_dscr(reg_block); reset_dma_before_load_dma_dscr(reg_block);
self.clear_dma_interrupts();
tx.prepare_transfer(self.dma_peripheral(), buffer)?; tx.prepare_transfer(self.dma_peripheral(), buffer)?;
tx.start_transfer()?; tx.start_transfer()?;
reset_dma_before_usr_cmd(reg_block); reset_dma_before_usr_cmd(reg_block);
// Wait for at least one clock cycle for the DMA to fill the SPI async FIFO, self.start_operation();
// before starting the SPI
#[cfg(riscv)]
riscv::asm::delay(1);
#[cfg(xtensa)]
xtensa_lx::timer::delay(1);
reg_block.cmd().modify(|_, w| w.usr().set_bit());
Ok(()) Ok(())
} }
@ -2330,17 +2322,16 @@ pub trait InstanceDma: Instance {
} }
self.enable_dma(); self.enable_dma();
self.update(); self.clear_dma_interrupts();
reset_dma_before_load_dma_dscr(reg_block); reset_dma_before_load_dma_dscr(reg_block);
self.clear_dma_interrupts();
reset_dma_before_usr_cmd(reg_block);
rx.prepare_transfer(self.dma_peripheral(), buffer)?; rx.prepare_transfer(self.dma_peripheral(), buffer)?;
rx.start_transfer()?; rx.start_transfer()?;
reg_block.cmd().modify(|_, w| w.usr().set_bit()); reset_dma_before_usr_cmd(reg_block);
self.start_operation();
Ok(()) Ok(())
} }
@ -2354,19 +2345,19 @@ pub trait InstanceDma: Instance {
} }
} }
#[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3))] #[cfg(gdma)]
fn enable_dma(&self) { fn enable_dma(&self) {
let reg_block = self.register_block(); let reg_block = self.register_block();
reg_block.dma_conf().modify(|_, w| w.dma_tx_ena().set_bit()); reg_block.dma_conf().modify(|_, w| w.dma_tx_ena().set_bit());
reg_block.dma_conf().modify(|_, w| w.dma_rx_ena().set_bit()); reg_block.dma_conf().modify(|_, w| w.dma_rx_ena().set_bit());
} }
#[cfg(any(esp32, esp32s2))] #[cfg(pdma)]
fn enable_dma(&self) { fn enable_dma(&self) {
// for non GDMA this is done in `assign_tx_device` / `assign_rx_device` // for non GDMA this is done in `assign_tx_device` / `assign_rx_device`
} }
#[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2, esp32s3))] #[cfg(gdma)]
fn clear_dma_interrupts(&self) { fn clear_dma_interrupts(&self) {
let reg_block = self.register_block(); let reg_block = self.register_block();
reg_block.dma_int_clr().write(|w| { reg_block.dma_int_clr().write(|w| {
@ -2383,7 +2374,7 @@ pub trait InstanceDma: Instance {
}); });
} }
#[cfg(any(esp32, esp32s2))] #[cfg(pdma)]
fn clear_dma_interrupts(&self) { fn clear_dma_interrupts(&self) {
let reg_block = self.register_block(); let reg_block = self.register_block();
reg_block.dma_int_clr().write(|w| { reg_block.dma_int_clr().write(|w| {
@ -2409,9 +2400,9 @@ pub trait InstanceDma: Instance {
} }
} }
#[cfg(not(any(esp32, esp32s2)))] fn reset_dma_before_usr_cmd(_reg_block: &RegisterBlock) {
fn reset_dma_before_usr_cmd(reg_block: &RegisterBlock) { #[cfg(gdma)]
reg_block.dma_conf().modify(|_, w| { _reg_block.dma_conf().modify(|_, w| {
w.rx_afifo_rst() w.rx_afifo_rst()
.set_bit() .set_bit()
.buf_afifo_rst() .buf_afifo_rst()
@ -2421,13 +2412,10 @@ fn reset_dma_before_usr_cmd(reg_block: &RegisterBlock) {
}); });
} }
#[cfg(any(esp32, esp32s2))] #[cfg(gdma)]
fn reset_dma_before_usr_cmd(_reg_block: &RegisterBlock) {}
#[cfg(not(any(esp32, esp32s2)))]
fn reset_dma_before_load_dma_dscr(_reg_block: &RegisterBlock) {} fn reset_dma_before_load_dma_dscr(_reg_block: &RegisterBlock) {}
#[cfg(any(esp32, esp32s2))] #[cfg(pdma)]
fn reset_dma_before_load_dma_dscr(reg_block: &RegisterBlock) { fn reset_dma_before_load_dma_dscr(reg_block: &RegisterBlock) {
reg_block.dma_conf().modify(|_, w| { reg_block.dma_conf().modify(|_, w| {
w.out_rst() w.out_rst()
@ -2514,7 +2502,7 @@ pub trait Instance: private::Sealed {
.clear_bit() .clear_bit()
}); });
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
reg_block.clk_gate().modify(|_, w| { reg_block.clk_gate().modify(|_, w| {
w.clk_en() w.clk_en()
.set_bit() .set_bit()
@ -2533,7 +2521,7 @@ pub trait Instance: private::Sealed {
.modify(|_, w| w.spi2_clkm_sel().bits(1)); .modify(|_, w| w.spi2_clkm_sel().bits(1));
} }
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
reg_block.ctrl().modify(|_, w| { reg_block.ctrl().modify(|_, w| {
w.q_pol() w.q_pol()
.clear_bit() .clear_bit()
@ -2853,7 +2841,7 @@ pub trait Instance: private::Sealed {
fn set_interrupt_handler(&mut self, handler: InterruptHandler); fn set_interrupt_handler(&mut self, handler: InterruptHandler);
/// Listen for the given interrupts /// Listen for the given interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
fn listen(&mut self, interrupts: EnumSet<SpiInterrupt>) { fn listen(&mut self, interrupts: EnumSet<SpiInterrupt>) {
let reg_block = self.register_block(); let reg_block = self.register_block();
@ -2869,7 +2857,7 @@ pub trait Instance: private::Sealed {
} }
/// Unlisten the given interrupts /// Unlisten the given interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
fn unlisten(&mut self, interrupts: EnumSet<SpiInterrupt>) { fn unlisten(&mut self, interrupts: EnumSet<SpiInterrupt>) {
let reg_block = self.register_block(); let reg_block = self.register_block();
@ -2885,7 +2873,7 @@ pub trait Instance: private::Sealed {
} }
/// Gets asserted interrupts /// Gets asserted interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
fn interrupts(&mut self) -> EnumSet<SpiInterrupt> { fn interrupts(&mut self) -> EnumSet<SpiInterrupt> {
let mut res = EnumSet::new(); let mut res = EnumSet::new();
let reg_block = self.register_block(); let reg_block = self.register_block();
@ -2900,7 +2888,7 @@ pub trait Instance: private::Sealed {
} }
/// Resets asserted interrupts /// Resets asserted interrupts
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
fn clear_interrupts(&mut self, interrupts: EnumSet<SpiInterrupt>) { fn clear_interrupts(&mut self, interrupts: EnumSet<SpiInterrupt>) {
let reg_block = self.register_block(); let reg_block = self.register_block();
@ -2967,7 +2955,7 @@ pub trait Instance: private::Sealed {
fn ch_bus_freq(&mut self, frequency: HertzU32) { fn ch_bus_freq(&mut self, frequency: HertzU32) {
// Disable clock source // Disable clock source
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
self.register_block().clk_gate().modify(|_, w| { self.register_block().clk_gate().modify(|_, w| {
w.clk_en() w.clk_en()
.clear_bit() .clear_bit()
@ -2981,7 +2969,7 @@ pub trait Instance: private::Sealed {
self.setup(frequency); self.setup(frequency);
// Enable clock source // Enable clock source
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
self.register_block().clk_gate().modify(|_, w| { self.register_block().clk_gate().modify(|_, w| {
w.clk_en() w.clk_en()
.set_bit() .set_bit()
@ -3048,9 +3036,7 @@ pub trait Instance: private::Sealed {
let reg_block = self.register_block(); let reg_block = self.register_block();
reg_block.w(0).write(|w| w.buf().set(word.into())); reg_block.w(0).write(|w| w.buf().set(word.into()));
self.update(); self.start_operation();
reg_block.cmd().modify(|_, w| w.usr().set_bit());
Ok(()) Ok(())
} }
@ -3103,9 +3089,7 @@ pub trait Instance: private::Sealed {
} }
} }
self.update(); self.start_operation();
self.register_block().cmd().modify(|_, w| w.usr().set_bit());
// Wait for all chunks to complete except the last one. // Wait for all chunks to complete except the last one.
// The function is allowed to return before the bus is idle. // The function is allowed to return before the bus is idle.
@ -3222,7 +3206,7 @@ pub trait Instance: private::Sealed {
.bit(command_state) .bit(command_state)
}); });
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
reg_block.clk_gate().modify(|_, w| { reg_block.clk_gate().modify(|_, w| {
w.clk_en() w.clk_en()
.set_bit() .set_bit()
@ -3370,13 +3354,12 @@ pub trait Instance: private::Sealed {
} }
self.configure_datalen(buffer.len() as u32 * 8); self.configure_datalen(buffer.len() as u32 * 8);
self.update(); self.start_operation();
reg_block.cmd().modify(|_, w| w.usr().set_bit());
self.flush()?; self.flush()?;
self.read_bytes_from_fifo(buffer) self.read_bytes_from_fifo(buffer)
} }
#[cfg(not(any(esp32, esp32s2)))] #[cfg(gdma)]
fn update(&self) { fn update(&self) {
let reg_block = self.register_block(); let reg_block = self.register_block();
@ -3387,7 +3370,7 @@ pub trait Instance: private::Sealed {
} }
} }
#[cfg(any(esp32, esp32s2))] #[cfg(pdma)]
fn update(&self) { fn update(&self) {
// not need/available on ESP32/ESP32S2 // not need/available on ESP32/ESP32S2
} }

36
hil-test/tests/spi_full_duplex_dma.rs
View File

@ -60,32 +60,40 @@ mod tests {
} }
} }
let mosi_loopback = mosi.peripheral_input(); let miso = mosi.peripheral_input();
let spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) let spi = Spi::new(peripherals.SPI2, 10000.kHz(), SpiMode::Mode0)
.with_sck(sclk) .with_sck(sclk)
.with_mosi(mosi) .with_mosi(mosi)
.with_miso(mosi_loopback) .with_miso(miso)
.with_dma(dma_channel.configure(false, DmaPriority::Priority0)); .with_dma(dma_channel.configure(false, DmaPriority::Priority0));
Context { spi } Context { spi }
} }
#[test] #[test]
#[timeout(3)]
fn test_symmetric_dma_transfer(ctx: Context) { fn test_symmetric_dma_transfer(ctx: Context) {
let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(4); // This test case sends a large amount of data, twice, to verify that
let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); // https://github.com/esp-rs/esp-hal/issues/2151 is and remains fixed.
let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(32000);
let mut dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap();
let mut dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap(); let mut dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap();
dma_tx_buf.fill(&[0xde, 0xad, 0xbe, 0xef]); for (i, v) in dma_tx_buf.as_mut_slice().iter_mut().enumerate() {
*v = (i % 255) as u8;
}
let transfer = ctx let mut spi = ctx.spi;
.spi for i in 0..4 {
.dma_transfer(dma_rx_buf, dma_tx_buf) dma_tx_buf.as_mut_slice()[0] = i as u8;
.map_err(|e| e.0) *dma_tx_buf.as_mut_slice().last_mut().unwrap() = i as u8;
.unwrap(); let transfer = spi
let (_, (dma_rx_buf, dma_tx_buf)) = transfer.wait(); .dma_transfer(dma_rx_buf, dma_tx_buf)
assert_eq!(dma_tx_buf.as_slice(), dma_rx_buf.as_slice()); .map_err(|e| e.0)
.unwrap();
(spi, (dma_rx_buf, dma_tx_buf)) = transfer.wait();
assert_eq!(dma_tx_buf.as_slice(), dma_rx_buf.as_slice());
}
} }
#[test] #[test]