Skip to content

Commit

Permalink
Fix dma nvic issues on dual core lines
Browse files Browse the repository at this point in the history
This commit addresses embassy-rs#3256 by disabling dma NVIC interrupt configuration at startup.
Instead, per-channel NVIC interrupt configuration is now done with the rest of the dma channel configuration.
This ensures that each core will only handle the interrupts of the DMA channels that it uses.
  • Loading branch information
liarokapisv committed Aug 17, 2024
1 parent 6d9ed4c commit 6dd66d8
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 76 deletions.
52 changes: 32 additions & 20 deletions embassy-stm32/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,34 @@ fn main() {
.flat_map(|p| &p.registers)
.any(|p| p.kind == "dmamux");

let mut dma_irqs: BTreeMap<&str, Vec<String>> = BTreeMap::new();

for p in METADATA.peripherals {
if let Some(r) = &p.registers {
if r.kind == "dma" || r.kind == "bdma" || r.kind == "gpdma" {
for irq in p.interrupts {
let ch_name = format!("{}_{}", p.name, irq.signal);
let ch = METADATA.dma_channels.iter().find(|c| c.name == ch_name).unwrap();

// Some H7 chips have BDMA1 hardcoded for DFSDM, ie no DMAMUX. It's unsupported, skip it.
if has_dmamux && ch.dmamux.is_none() {
continue;
}

dma_irqs.entry(irq.interrupt).or_default().push(ch_name);
}
}
}
}

let mut dma_ch_to_irq: BTreeMap<&str, Vec<String>> = BTreeMap::new();

for (irq, channels) in &dma_irqs {
for channel in channels {
dma_ch_to_irq.entry(channel).or_default().push(irq.to_string());
}
}

for (ch_idx, ch) in METADATA.dma_channels.iter().enumerate() {
// Some H7 chips have BDMA1 hardcoded for DFSDM, ie no DMAMUX. It's unsupported, skip it.
if has_dmamux && ch.dmamux.is_none() {
Expand All @@ -1502,6 +1530,9 @@ fn main() {

let name = format_ident!("{}", ch.name);
let idx = ch_idx as u8;
let irq_name = format_ident!("{}", &dma_ch_to_irq[ch.name].get(0).unwrap());
let irq = quote!(crate::pac::Interrupt::#irq_name);

g.extend(quote!(dma_channel_impl!(#name, #idx);));

let dma = format_ident!("{}", ch.dma);
Expand Down Expand Up @@ -1536,6 +1567,7 @@ fn main() {
crate::dma::ChannelInfo {
dma: #dma_info,
num: #ch_num,
irq: #irq,
#dmamux
},
});
Expand All @@ -1544,26 +1576,6 @@ fn main() {
// ========
// Generate DMA IRQs.

let mut dma_irqs: BTreeMap<&str, Vec<String>> = BTreeMap::new();

for p in METADATA.peripherals {
if let Some(r) = &p.registers {
if r.kind == "dma" || r.kind == "bdma" || r.kind == "gpdma" {
for irq in p.interrupts {
let ch_name = format!("{}_{}", p.name, irq.signal);
let ch = METADATA.dma_channels.iter().find(|c| c.name == ch_name).unwrap();

// Some H7 chips have BDMA1 hardcoded for DFSDM, ie no DMAMUX. It's unsupported, skip it.
if has_dmamux && ch.dmamux.is_none() {
continue;
}

dma_irqs.entry(irq.interrupt).or_default().push(ch_name);
}
}
}
}

let dma_irqs: TokenStream = dma_irqs
.iter()
.map(|(irq, channels)| {
Expand Down
22 changes: 5 additions & 17 deletions embassy-stm32/src/dma/dma_bdma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@ use core::pin::Pin;
use core::sync::atomic::{fence, AtomicUsize, Ordering};
use core::task::{Context, Poll, Waker};

use embassy_hal_internal::interrupt::InterruptExt as _;
use embassy_hal_internal::{into_ref, Peripheral, PeripheralRef};
use embassy_sync::waitqueue::AtomicWaker;

use super::ringbuffer::{DmaCtrl, OverrunError, ReadableDmaRingBuffer, WritableDmaRingBuffer};
use super::word::{Word, WordSize};
use super::{AnyChannel, Channel, Dir, Request, STATE};
use crate::interrupt::typelevel::Interrupt;
use crate::{interrupt, pac};
use crate::pac;

pub(crate) struct ChannelInfo {
pub(crate) dma: DmaInfo,
pub(crate) num: usize,
pub(crate) irq: pac::Interrupt,
#[cfg(dmamux)]
pub(crate) dmamux: super::DmamuxInfo,
}
Expand Down Expand Up @@ -251,21 +252,7 @@ impl ChannelState {
}

/// safety: must be called only once
pub(crate) unsafe fn init(
cs: critical_section::CriticalSection,
#[cfg(dma)] dma_priority: interrupt::Priority,
#[cfg(bdma)] bdma_priority: interrupt::Priority,
) {
foreach_interrupt! {
($peri:ident, dma, $block:ident, $signal_name:ident, $irq:ident) => {
crate::interrupt::typelevel::$irq::set_priority_with_cs(cs, dma_priority);
crate::interrupt::typelevel::$irq::enable();
};
($peri:ident, bdma, $block:ident, $signal_name:ident, $irq:ident) => {
crate::interrupt::typelevel::$irq::set_priority_with_cs(cs, bdma_priority);
crate::interrupt::typelevel::$irq::enable();
};
}
pub(crate) unsafe fn init() {
crate::_generated::init_dma();
crate::_generated::init_bdma();
}
Expand Down Expand Up @@ -341,6 +328,7 @@ impl AnyChannel {
options: TransferOptions,
) {
let info = self.info();
info.irq.enable();

#[cfg(dmamux)]
super::dmamux::configure_dmamux(&info.dmamux, _request);
Expand Down
2 changes: 1 addition & 1 deletion embassy-stm32/src/dma/dmamux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ pub(crate) fn configure_dmamux(info: &DmamuxInfo, request: u8) {
}

/// safety: must be called only once
pub(crate) unsafe fn init(_cs: critical_section::CriticalSection) {
pub(crate) unsafe fn init() {
crate::_generated::init_dmamux();
}
13 changes: 4 additions & 9 deletions embassy-stm32/src/dma/gpdma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ use core::pin::Pin;
use core::sync::atomic::{fence, Ordering};
use core::task::{Context, Poll};

use embassy_hal_internal::interrupt::InterruptExt as _;
use embassy_hal_internal::{into_ref, Peripheral, PeripheralRef};
use embassy_sync::waitqueue::AtomicWaker;

use super::word::{Word, WordSize};
use super::{AnyChannel, Channel, Dir, Request, STATE};
use crate::interrupt::typelevel::Interrupt;
use crate::interrupt::Priority;
use crate::pac;
use crate::pac::gpdma::vals;

pub(crate) struct ChannelInfo {
pub(crate) dma: pac::gpdma::Gpdma,
pub(crate) num: usize,
pub(crate) irq: pac::Interrupt,
}

/// GPDMA transfer options.
Expand Down Expand Up @@ -53,13 +53,7 @@ impl ChannelState {
}

/// safety: must be called only once
pub(crate) unsafe fn init(cs: critical_section::CriticalSection, irq_priority: Priority) {
foreach_interrupt! {
($peri:ident, gpdma, $block:ident, $signal_name:ident, $irq:ident) => {
crate::interrupt::typelevel::$irq::set_priority_with_cs(cs, irq_priority);
crate::interrupt::typelevel::$irq::enable();
};
}
pub(crate) unsafe fn init() {
crate::_generated::init_gpdma();
}

Expand Down Expand Up @@ -210,6 +204,7 @@ impl<'a> Transfer<'a> {
assert!(mem_len > 0 && mem_len <= 0xFFFF);

let info = channel.info();
info.irq.enable();
let ch = info.dma.ch(info.num);

// "Preceding reads and writes cannot be moved past subsequent writes."
Expand Down
22 changes: 5 additions & 17 deletions embassy-stm32/src/dma/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ pub mod word;

use embassy_hal_internal::{impl_peripheral, Peripheral};

use crate::interrupt;

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
enum Dir {
Expand Down Expand Up @@ -70,6 +68,7 @@ macro_rules! dma_channel_impl {
$index
}
}

impl crate::dma::ChannelInterrupt for crate::peripherals::$channel_peri {
unsafe fn on_irq() {
crate::dma::AnyChannel { id: $index }.on_irq();
Expand Down Expand Up @@ -120,22 +119,11 @@ pub struct NoDma;
impl_peripheral!(NoDma);

// safety: must be called only once at startup
pub(crate) unsafe fn init(
cs: critical_section::CriticalSection,
#[cfg(bdma)] bdma_priority: interrupt::Priority,
#[cfg(dma)] dma_priority: interrupt::Priority,
#[cfg(gpdma)] gpdma_priority: interrupt::Priority,
) {
pub(crate) unsafe fn init() {
#[cfg(any(dma, bdma))]
dma_bdma::init(
cs,
#[cfg(dma)]
dma_priority,
#[cfg(bdma)]
bdma_priority,
);
dma_bdma::init();
#[cfg(gpdma)]
gpdma::init(cs, gpdma_priority);
gpdma::init();
#[cfg(dmamux)]
dmamux::init(cs);
dmamux::init();
}
16 changes: 4 additions & 12 deletions embassy-stm32/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,9 @@ mod dual_core {
fn init_secondary_hw(shared_data: &'static SharedData) -> Peripherals {
rcc::set_freqs_ptr(shared_data.clocks.get());

// We use different timers on the different cores, so we have to still initialize one here
#[cfg(feature = "_time-driver")]
critical_section::with(|cs| {
critical_section::with(|#[allow(unused)] cs| {
// We use different timers on the different cores, so we have to still initialize one here
#[cfg(feature = "_time-driver")]
// must be after rcc init
time_driver::init(cs);
});
Expand Down Expand Up @@ -479,15 +479,7 @@ fn init_hw(config: Config) -> Peripherals {
});

gpio::init(cs);
dma::init(
cs,
#[cfg(bdma)]
config.bdma_interrupt_priority,
#[cfg(dma)]
config.dma_interrupt_priority,
#[cfg(gpdma)]
config.gpdma_interrupt_priority,
);
dma::init();
#[cfg(feature = "exti")]
exti::init(cs);

Expand Down

0 comments on commit 6dd66d8

Please sign in to comment.