InterruptExecutor hangs on esp32 when used on APP_CPU with a DMA peripheral

[liebman]

Bug description

If you run a DMA peripheral with an InterruptExecutor on the second core it looks like the task is not woken when the first transfer finishes. The test code by default runs spi to continuously output in a loop in an Interrupt executor.

It can be configured (by commenting and uncommenting sections of code) to:

• run the interrupt executor on the first core (this works fine)
• run in a normal executor on the second core (this also works fine)

I found this using an i2s parallel driver for the esp32 I'm working on so its not just SPI thats affected.

To Reproduce

1. run the included code, it will send one transfer then hang waiting for DMA to complete

#![no_std]
#![no_main]

use core::sync::atomic::AtomicU32;
use core::sync::atomic::Ordering;

use embassy_executor::task;
use embassy_executor::Spawner;
use embassy_time::Duration;
use embassy_time::Instant;
use embassy_time::Timer;
use esp_backtrace as _;
use esp_hal::dma::Dma;
use esp_hal::dma::DmaPriority;
use esp_hal::dma::DmaRxBuf;
use esp_hal::dma::DmaTxBuf;
use esp_hal::dma::Spi2DmaChannelCreator;
use esp_hal::dma_buffers;
use esp_hal::gpio::AnyPin;
use esp_hal::gpio::Io;
use esp_hal::peripherals::SPI2;
use esp_hal::prelude::*;
use esp_hal::spi::master::Spi;
use esp_hal::spi::SpiMode;
use esp_hal::timer::timg::TimerGroup;
use log::info;

const BUFFER_SIZE: usize = 256;
static LOOP_COUNT: AtomicU32 = AtomicU32::new(0);

// When you are okay with using a nightly compiler it's better to use https://docs.rs/static_cell/2.1.0/static_cell/macro.make_static.html
macro_rules! mk_static {
    ($t:ty,$val:expr) => {{
        static STATIC_CELL: static_cell::StaticCell<$t> = static_cell::StaticCell::new();
        #[deny(unused_attributes)]
        let x = STATIC_CELL.uninit().write(($val));
        x
    }};
}

pub struct SpiPeripherals {
    pub spi: SPI2,
    pub dma_channel: Spi2DmaChannelCreator,
    pub sclk: AnyPin,
    pub mosi: AnyPin,
    pub miso: AnyPin,
    pub cs: AnyPin,
}

#[task]
async fn run_spi(peripherals: SpiPeripherals) {
    info!("Starting SPI task");
    let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(32000);
    let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap();
    let dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap();

    let mut spi = Spi::new(peripherals.spi, 100.kHz(), SpiMode::Mode0)
        .with_pins(peripherals.sclk, peripherals.mosi, peripherals.miso, peripherals.cs)
        .with_dma(peripherals.dma_channel.configure_for_async(false, DmaPriority::Priority0))
        .with_buffers(dma_rx_buf, dma_tx_buf);

    let send_buffer = mk_static!([u8;BUFFER_SIZE], [0u8; BUFFER_SIZE]);
    for i in 0..BUFFER_SIZE {
        send_buffer[i] = (i&0xff) as u8;
    }
    loop {
        let mut buffer = [0; 8];
        embedded_hal_async::spi::SpiBus::transfer(&mut spi, &mut buffer, send_buffer)
            .await
            .unwrap();
        LOOP_COUNT.fetch_add(1, Ordering::Relaxed);
    }
}

#[main]
async fn main(_spawner: Spawner) {
    esp_println::logger::init_logger(log::LevelFilter::Info);
    info!("Starting!");
    let peripherals = esp_hal::init(esp_hal::Config::default());
    let dma = Dma::new(peripherals.DMA);
    let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);

    let timg0 = TimerGroup::new(peripherals.TIMG0);

    info!("init embassy");
    esp_hal_embassy::init(timg0.timer0);

    let spi_peripherals = SpiPeripherals {
        spi: peripherals.SPI2,
        dma_channel: dma.spi2channel,
        sclk: io.pins.gpio25.degrade(),
        mosi: io.pins.gpio16.degrade(),
        miso: io.pins.gpio4.degrade(),
        cs: io.pins.gpio17.degrade(),
    };

    // /////////////////////////////////////////////////////////////////////
    // // This runs SPI on core 1 as a high priority in an interrupt executor.
    // // This works!
    // use esp_hal_embassy::InterruptExecutor;
    // use esp_hal::interrupt::Priority;
    // let sw_ints = esp_hal::interrupt::software::SoftwareInterruptControl::new(peripherals.SW_INTERRUPT);
    // let software_interrupt = sw_ints.software_interrupt2;
    // let hp_executor = mk_static!(
    //     InterruptExecutor<2>,
    //     InterruptExecutor::new(software_interrupt)
    // );
    // let high_pri_spawner = hp_executor.start(Priority::Priority2);

    // // hub75 runs as high priority task
    // high_pri_spawner
    //     .spawn(run_spi(spi_peripherals))
    //     .ok();

    let cpu1_fnctn = {
        move || {
            /////////////////////////////////////////////////////////////////////
            // This runs SPI on core 2 as a high priority in an interrupt executor.
            // This hangs at the end of the first transfer (like its not being woken)!
            //
            use esp_hal_embassy::InterruptExecutor;
            use esp_hal::interrupt::Priority;
            let sw_ints = esp_hal::interrupt::software::SoftwareInterruptControl::new(peripherals.SW_INTERRUPT);
            let software_interrupt = sw_ints.software_interrupt2;
            let hp_executor = mk_static!(
                InterruptExecutor<2>,
                InterruptExecutor::new(software_interrupt)
            );
            let high_pri_spawner = hp_executor.start(Priority::Priority2);

            // hub75 runs as high priority task
            high_pri_spawner
                .spawn(run_spi(spi_peripherals))
                .ok();
    
            // //////////////////////////////////////////////////////////////////////
            // // This runs SPI on core 2 in a normal executor. (this works)
            // //
            // use esp_hal_embassy::Executor;
            // let lp_executor = mk_static!(Executor, Executor::new());
            // // display task runs as low priority task
            // lp_executor.run(|spawner| {
            //     spawner.spawn(run_spi(spi_peripherals)).ok();
            // });
        }
    };

    use esp_hal::cpu_control::CpuControl;
    use esp_hal::cpu_control::Stack;
    const DISPLAY_STACK_SIZE: usize = 8192;
    let app_core_stack = mk_static!(Stack<DISPLAY_STACK_SIZE>, Stack::new());
    let cpu_control = CpuControl::new(peripherals.CPU_CTRL);
    let mut _cpu_control = cpu_control;

    #[allow(static_mut_refs)]
    let _guard = _cpu_control
        .start_app_core(app_core_stack, cpu1_fnctn)
        .unwrap();


    let start = Instant::now();
    let mut last = 0u32;
    loop {
        Timer::after(Duration::from_millis(100)).await;

        let next = LOOP_COUNT.load(Ordering::Relaxed);
        if next == last {
            info!("stuck after {} after {} seconds", next, start.elapsed().as_secs());
            while LOOP_COUNT.load(Ordering::Relaxed) == next {
                Timer::after(Duration::from_millis(100)).await;
            }
        }
        last = next;
    }
}

Tested from the main branch

Expected behavior

This should continuously output using SPI on the configured pins.

Environment

• Target device:

  • Chip type: esp32 (revision v0.0)
  • Crystal frequency: 40 MHz
  • Flash size: 4MB
  • Features: WiFi, BT, Dual Core, Coding Scheme None
  • MAC address: 24:0a:c4:03:ea:5c

• Crate name and version: esp-hal latest GIT (d32a733)

[bugadani]

I'm sorry but the solution here might be to disallow moving initialized peripherals to other cores. If you create the SPI driver on the app core, it should work.

[liebman]

I'm sorry but the solution here might be to disallow moving initialized peripherals to other cores. If you create the SPI driver on the app core, it should work.

Thats exactly what the test code does

[bugadani]

Oh hmm apparently I can't read. I'll look into this early next week.

[bugadani]

I can confirm that our interrupt-executor tests have been inadequate and even cases as simple as signalling between cores using embassy_sync::signal::Signal doesn't work.

[bugadani]

Okay I can reproduce this and I know what's going on here.

When you exit the core1 function, we stall core 1. The interrupt executor does not loop after starting it, so essentially you start it, and then the core1 program ends and stops. Add a loop {} at the end of cpu1_fnctn and it should fix your issue.

[liebman]

Why is this not needed for the other chips (at least esp32s3 and esp32c6)?

[bugadani]

C6 is single-core, S3 needs this or if it doesn't, it's a bug.

[liebman]

C6 right! but yea - s3 works fine with the example

[liebman]

ok - wait - maybe not - I never tested that exact example - but I have a similar use case but it has a second normal executor on the second core, maybe that fulfills the loop need.