From cbc8871a0bb40eb5fec82e7c27ed4c0127844c3c Mon Sep 17 00:00:00 2001 From: pennae Date: Fri, 28 Jul 2023 18:45:57 +0200 Subject: [PATCH] rp: relocate programs implicitly during load this removed the RelocatedProgram construction step from pio uses. there's not all that much to be said for the extra step because the origin can be set on the input program itself, and the remaining information exposed by RelocatedProgram can be exposed from LoadedProgram instead (even though it's already available on the pio_asm programs, albeit perhaps less convenient). we do lose access to the relocated instruction iterator, but we also cannot think of anything this iterator would actually be useful for outside of program loading. --- cyw43-pio/src/lib.rs | 8 +- embassy-rp/src/lib.rs | 2 +- embassy-rp/src/pio.rs | 60 ++++++++++++-- embassy-rp/src/relocate.rs | 5 -- examples/rp/src/bin/pio_async.rs | 10 +-- examples/rp/src/bin/pio_dma.rs | 4 +- examples/rp/src/bin/pio_hd44780.rs | 7 +- examples/rp/src/bin/pio_uart.rs | 41 +++------- examples/rp/src/bin/pio_ws2812.rs | 4 +- tests/rp/src/bin/pio_irq.rs | 4 +- tests/rp/src/bin/pio_multi_load.rs | 126 +++++++++++++++++++++++++++++ 11 files changed, 200 insertions(+), 71 deletions(-) create mode 100644 tests/rp/src/bin/pio_multi_load.rs diff --git a/cyw43-pio/src/lib.rs b/cyw43-pio/src/lib.rs index dca30c74..830a5b44 100644 --- a/cyw43-pio/src/lib.rs +++ b/cyw43-pio/src/lib.rs @@ -8,7 +8,6 @@ use cyw43::SpiBusCyw43; use embassy_rp::dma::Channel; use embassy_rp::gpio::{Drive, Level, Output, Pin, Pull, SlewRate}; use embassy_rp::pio::{Common, Config, Direction, Instance, Irq, PioPin, ShiftDirection, StateMachine}; -use embassy_rp::relocate::RelocatedProgram; use embassy_rp::{pio_instr_util, Peripheral, PeripheralRef}; use fixed::FixedU32; use pio_proc::pio_asm; @@ -88,8 +87,6 @@ where ".wrap" ); - let relocated = RelocatedProgram::new(&program.program); - let mut pin_io: embassy_rp::pio::Pin = common.make_pio_pin(dio); pin_io.set_pull(Pull::None); pin_io.set_schmitt(true); @@ -102,7 +99,8 @@ where pin_clk.set_slew_rate(SlewRate::Fast); let mut cfg = Config::default(); - cfg.use_program(&common.load_program(&relocated), &[&pin_clk]); + let loaded_program = common.load_program(&program.program); + cfg.use_program(&loaded_program, &[&pin_clk]); cfg.set_out_pins(&[&pin_io]); cfg.set_in_pins(&[&pin_io]); cfg.set_set_pins(&[&pin_io]); @@ -142,7 +140,7 @@ where sm, irq, dma: dma.into_ref(), - wrap_target: relocated.wrap().target, + wrap_target: loaded_program.wrap.target, } } diff --git a/embassy-rp/src/lib.rs b/embassy-rp/src/lib.rs index ebec9fec..45156458 100644 --- a/embassy-rp/src/lib.rs +++ b/embassy-rp/src/lib.rs @@ -33,7 +33,7 @@ pub mod watchdog; // TODO: move `pio_instr_util` and `relocate` to inside `pio` pub mod pio; pub mod pio_instr_util; -pub mod relocate; +pub(crate) mod relocate; // Reexports pub use embassy_hal_internal::{into_ref, Peripheral, PeripheralRef}; diff --git a/embassy-rp/src/pio.rs b/embassy-rp/src/pio.rs index 464988b2..3de398af 100644 --- a/embassy-rp/src/pio.rs +++ b/embassy-rp/src/pio.rs @@ -11,7 +11,7 @@ use fixed::types::extra::U8; use fixed::FixedU32; use pac::io::vals::Gpio0ctrlFuncsel; use pac::pio::vals::SmExecctrlStatusSel; -use pio::{SideSet, Wrap}; +use pio::{Program, SideSet, Wrap}; use crate::dma::{Channel, Transfer, Word}; use crate::gpio::sealed::Pin as SealedPin; @@ -734,23 +734,67 @@ pub struct InstanceMemory<'d, PIO: Instance> { pub struct LoadedProgram<'d, PIO: Instance> { pub used_memory: InstanceMemory<'d, PIO>, - origin: u8, - wrap: Wrap, - side_set: SideSet, + pub origin: u8, + pub wrap: Wrap, + pub side_set: SideSet, +} + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum LoadError { + /// Insufficient consecutive free instruction space to load program. + InsufficientSpace, + /// Loading the program would overwrite an instruction address already + /// used by another program. + AddressInUse(usize), } impl<'d, PIO: Instance> Common<'d, PIO> { - pub fn load_program(&mut self, prog: &RelocatedProgram) -> LoadedProgram<'d, PIO> { + /// Load a PIO program. This will automatically relocate the program to + /// an available chunk of free instruction memory if the program origin + /// was not explicitly specified, otherwise it will attempt to load the + /// program only at its origin. + pub fn load_program(&mut self, prog: &Program) -> LoadedProgram<'d, PIO> { match self.try_load_program(prog) { Ok(r) => r, - Err(at) => panic!("Trying to write already used PIO instruction memory at {}", at), + Err(e) => panic!("Failed to load PIO program: {:?}", e), } } + /// Load a PIO program. This will automatically relocate the program to + /// an available chunk of free instruction memory if the program origin + /// was not explicitly specified, otherwise it will attempt to load the + /// program only at its origin. pub fn try_load_program( &mut self, - prog: &RelocatedProgram, + prog: &Program, + ) -> Result, LoadError> { + match prog.origin { + Some(origin) => self + .try_load_program_at(prog, origin) + .map_err(|a| LoadError::AddressInUse(a)), + None => { + // naively search for free space, allowing wraparound since + // PIO does support that. with only 32 instruction slots it + // doesn't make much sense to do anything more fancy. + let mut origin = 0; + while origin < 32 { + match self.try_load_program_at(prog, origin as _) { + Ok(r) => return Ok(r), + Err(a) => origin = a + 1, + } + } + Err(LoadError::InsufficientSpace) + } + } + } + + fn try_load_program_at( + &mut self, + prog: &Program, + origin: u8, ) -> Result, usize> { + let prog = RelocatedProgram::new_with_origin(prog, origin); let used_memory = self.try_write_instr(prog.origin() as _, prog.code())?; Ok(LoadedProgram { used_memory, @@ -760,7 +804,7 @@ impl<'d, PIO: Instance> Common<'d, PIO> { }) } - pub fn try_write_instr(&mut self, start: usize, instrs: I) -> Result, usize> + fn try_write_instr(&mut self, start: usize, instrs: I) -> Result, usize> where I: Iterator, { diff --git a/embassy-rp/src/relocate.rs b/embassy-rp/src/relocate.rs index 9cb279cc..b35b4ed7 100644 --- a/embassy-rp/src/relocate.rs +++ b/embassy-rp/src/relocate.rs @@ -41,11 +41,6 @@ pub struct RelocatedProgram<'a, const PROGRAM_SIZE: usize> { } impl<'a, const PROGRAM_SIZE: usize> RelocatedProgram<'a, PROGRAM_SIZE> { - pub fn new(program: &Program) -> RelocatedProgram { - let origin = program.origin.unwrap_or(0); - RelocatedProgram { program, origin } - } - pub fn new_with_origin(program: &Program, origin: u8) -> RelocatedProgram { RelocatedProgram { program, origin } } diff --git a/examples/rp/src/bin/pio_async.rs b/examples/rp/src/bin/pio_async.rs index c001d644..a6d6144b 100644 --- a/examples/rp/src/bin/pio_async.rs +++ b/examples/rp/src/bin/pio_async.rs @@ -8,7 +8,6 @@ use embassy_executor::Spawner; use embassy_rp::bind_interrupts; use embassy_rp::peripherals::PIO0; use embassy_rp::pio::{Common, Config, InterruptHandler, Irq, Pio, PioPin, ShiftDirection, StateMachine}; -use embassy_rp::relocate::RelocatedProgram; use fixed::traits::ToFixed; use fixed_macro::types::U56F8; use {defmt_rtt as _, panic_probe as _}; @@ -29,9 +28,8 @@ fn setup_pio_task_sm0<'a>(pio: &mut Common<'a, PIO0>, sm: &mut StateMachine<'a, ".wrap", ); - let relocated = RelocatedProgram::new(&prg.program); let mut cfg = Config::default(); - cfg.use_program(&pio.load_program(&relocated), &[]); + cfg.use_program(&pio.load_program(&prg.program), &[]); let out_pin = pio.make_pio_pin(pin); cfg.set_out_pins(&[&out_pin]); cfg.set_set_pins(&[&out_pin]); @@ -65,9 +63,8 @@ fn setup_pio_task_sm1<'a>(pio: &mut Common<'a, PIO0>, sm: &mut StateMachine<'a, ".wrap", ); - let relocated = RelocatedProgram::new(&prg.program); let mut cfg = Config::default(); - cfg.use_program(&pio.load_program(&relocated), &[]); + cfg.use_program(&pio.load_program(&prg.program), &[]); cfg.clock_divider = (U56F8!(125_000_000) / 2000).to_fixed(); cfg.shift_in.auto_fill = true; cfg.shift_in.direction = ShiftDirection::Right; @@ -96,9 +93,8 @@ fn setup_pio_task_sm2<'a>(pio: &mut Common<'a, PIO0>, sm: &mut StateMachine<'a, "irq 3 [15]", ".wrap", ); - let relocated = RelocatedProgram::new(&prg.program); let mut cfg = Config::default(); - cfg.use_program(&pio.load_program(&relocated), &[]); + cfg.use_program(&pio.load_program(&prg.program), &[]); cfg.clock_divider = (U56F8!(125_000_000) / 2000).to_fixed(); sm.set_config(&cfg); } diff --git a/examples/rp/src/bin/pio_dma.rs b/examples/rp/src/bin/pio_dma.rs index 9ab72e1f..86e5017a 100644 --- a/examples/rp/src/bin/pio_dma.rs +++ b/examples/rp/src/bin/pio_dma.rs @@ -8,7 +8,6 @@ use embassy_executor::Spawner; use embassy_futures::join::join; use embassy_rp::peripherals::PIO0; use embassy_rp::pio::{Config, InterruptHandler, Pio, ShiftConfig, ShiftDirection}; -use embassy_rp::relocate::RelocatedProgram; use embassy_rp::{bind_interrupts, Peripheral}; use fixed::traits::ToFixed; use fixed_macro::types::U56F8; @@ -46,9 +45,8 @@ async fn main(_spawner: Spawner) { ".wrap", ); - let relocated = RelocatedProgram::new(&prg.program); let mut cfg = Config::default(); - cfg.use_program(&common.load_program(&relocated), &[]); + cfg.use_program(&common.load_program(&prg.program), &[]); cfg.clock_divider = (U56F8!(125_000_000) / U56F8!(10_000)).to_fixed(); cfg.shift_in = ShiftConfig { auto_fill: true, diff --git a/examples/rp/src/bin/pio_hd44780.rs b/examples/rp/src/bin/pio_hd44780.rs index 8aedd24b..d80c5c24 100644 --- a/examples/rp/src/bin/pio_hd44780.rs +++ b/examples/rp/src/bin/pio_hd44780.rs @@ -14,7 +14,6 @@ use embassy_rp::pio::{ Config, Direction, FifoJoin, InterruptHandler, Pio, PioPin, ShiftConfig, ShiftDirection, StateMachine, }; use embassy_rp::pwm::{self, Pwm}; -use embassy_rp::relocate::RelocatedProgram; use embassy_rp::{bind_interrupts, into_ref, Peripheral, PeripheralRef}; use embassy_time::{Duration, Instant, Timer}; use {defmt_rtt as _, panic_probe as _}; @@ -127,9 +126,8 @@ impl<'l> HD44780<'l> { sm0.set_pin_dirs(Direction::Out, &[&rs, &rw, &e, &db4, &db5, &db6, &db7]); - let relocated = RelocatedProgram::new(&prg.program); let mut cfg = Config::default(); - cfg.use_program(&common.load_program(&relocated), &[&e]); + cfg.use_program(&common.load_program(&prg.program), &[&e]); cfg.clock_divider = 125u8.into(); cfg.set_out_pins(&[&db4, &db5, &db6, &db7]); cfg.shift_out = ShiftConfig { @@ -201,9 +199,8 @@ impl<'l> HD44780<'l> { "# ); - let relocated = RelocatedProgram::new(&prg.program); let mut cfg = Config::default(); - cfg.use_program(&common.load_program(&relocated), &[&e]); + cfg.use_program(&common.load_program(&prg.program), &[&e]); cfg.clock_divider = 8u8.into(); // ~64ns/insn cfg.set_jmp_pin(&db7); cfg.set_set_pins(&[&rs, &rw]); diff --git a/examples/rp/src/bin/pio_uart.rs b/examples/rp/src/bin/pio_uart.rs index ca1c7f39..5fddbe29 100644 --- a/examples/rp/src/bin/pio_uart.rs +++ b/examples/rp/src/bin/pio_uart.rs @@ -222,8 +222,8 @@ mod uart { mut common, sm0, sm1, .. } = Pio::new(pio, Irqs); - let (tx, origin) = PioUartTx::new(&mut common, sm0, tx_pin, baud, None); - let (rx, _) = PioUartRx::new(&mut common, sm1, rx_pin, baud, Some(origin)); + let tx = PioUartTx::new(&mut common, sm0, tx_pin, baud); + let rx = PioUartRx::new(&mut common, sm1, rx_pin, baud); PioUart { tx, rx } } @@ -240,7 +240,6 @@ mod uart_tx { use embassy_rp::gpio::Level; use embassy_rp::peripherals::PIO0; use embassy_rp::pio::{Common, Config, Direction, FifoJoin, PioPin, ShiftDirection, StateMachine}; - use embassy_rp::relocate::RelocatedProgram; use embedded_io::asynch::Write; use embedded_io::Io; use fixed::traits::ToFixed; @@ -256,9 +255,8 @@ mod uart_tx { mut sm_tx: StateMachine<'a, PIO0, 0>, tx_pin: impl PioPin, baud: u64, - origin: Option, - ) -> (Self, u8) { - let mut prg = pio_proc::pio_asm!( + ) -> Self { + let prg = pio_proc::pio_asm!( r#" .side_set 1 opt @@ -272,17 +270,14 @@ mod uart_tx { jmp x-- bitloop [6] ; Each loop iteration is 8 cycles. "# ); - prg.program.origin = origin; let tx_pin = common.make_pio_pin(tx_pin); sm_tx.set_pins(Level::High, &[&tx_pin]); sm_tx.set_pin_dirs(Direction::Out, &[&tx_pin]); - let relocated = RelocatedProgram::new(&prg.program); - let mut cfg = Config::default(); cfg.set_out_pins(&[&tx_pin]); - cfg.use_program(&common.load_program(&relocated), &[&tx_pin]); + cfg.use_program(&common.load_program(&prg.program), &[&tx_pin]); cfg.shift_out.auto_fill = false; cfg.shift_out.direction = ShiftDirection::Right; cfg.fifo_join = FifoJoin::TxOnly; @@ -290,18 +285,7 @@ mod uart_tx { sm_tx.set_config(&cfg); sm_tx.set_enable(true); - // The 4 state machines of the PIO each have their own program counter that starts taking - // instructions at an offset (origin) of the 32 instruction "space" the PIO device has. - // It is up to the programmer to sort out where to place these instructions. - // From the pio_asm! macro you get a ProgramWithDefines which has a field .program.origin - // which takes an Option. - // - // When you load more than one RelocatedProgram into the PIO, - // you load your first program at origin = 0. - // The RelocatedProgram has .code().count() which returns a usize, - // for which you can then use as your next program's origin. - let offset = relocated.code().count() as u8 + origin.unwrap_or_default(); - (Self { sm_tx }, offset) + Self { sm_tx } } pub async fn write_u8(&mut self, data: u8) { @@ -329,7 +313,6 @@ mod uart_rx { use embassy_rp::gpio::Level; use embassy_rp::peripherals::PIO0; use embassy_rp::pio::{Common, Config, Direction, FifoJoin, PioPin, ShiftDirection, StateMachine}; - use embassy_rp::relocate::RelocatedProgram; use embedded_io::asynch::Read; use embedded_io::Io; use fixed::traits::ToFixed; @@ -345,9 +328,8 @@ mod uart_rx { mut sm_rx: StateMachine<'a, PIO0, 1>, rx_pin: impl PioPin, baud: u64, - origin: Option, - ) -> (Self, u8) { - let mut prg = pio_proc::pio_asm!( + ) -> Self { + let prg = pio_proc::pio_asm!( r#" ; Slightly more fleshed-out 8n1 UART receiver which handles framing errors and ; break conditions more gracefully. @@ -369,10 +351,8 @@ mod uart_rx { push ; important in case the TX clock is slightly too fast. "# ); - prg.program.origin = origin; - let relocated = RelocatedProgram::new(&prg.program); let mut cfg = Config::default(); - cfg.use_program(&common.load_program(&relocated), &[]); + cfg.use_program(&common.load_program(&prg.program), &[]); let rx_pin = common.make_pio_pin(rx_pin); sm_rx.set_pins(Level::High, &[&rx_pin]); @@ -387,8 +367,7 @@ mod uart_rx { sm_rx.set_config(&cfg); sm_rx.set_enable(true); - let offset = relocated.code().count() as u8 + origin.unwrap_or_default(); - (Self { sm_rx }, offset) + Self { sm_rx } } pub async fn read_u8(&mut self) -> u8 { diff --git a/examples/rp/src/bin/pio_ws2812.rs b/examples/rp/src/bin/pio_ws2812.rs index 3de2bd48..bc87016e 100644 --- a/examples/rp/src/bin/pio_ws2812.rs +++ b/examples/rp/src/bin/pio_ws2812.rs @@ -12,7 +12,6 @@ use embassy_rp::peripherals::PIO0; use embassy_rp::pio::{ Common, Config, FifoJoin, Instance, InterruptHandler, Pio, PioPin, ShiftConfig, ShiftDirection, StateMachine, }; -use embassy_rp::relocate::RelocatedProgram; use embassy_rp::{bind_interrupts, clocks, into_ref, Peripheral, PeripheralRef}; use embassy_time::{Duration, Timer}; use fixed::types::U24F8; @@ -73,8 +72,7 @@ impl<'d, P: Instance, const S: usize, const N: usize> Ws2812<'d, P, S, N> { cfg.set_out_pins(&[&out_pin]); cfg.set_set_pins(&[&out_pin]); - let relocated = RelocatedProgram::new(&prg); - cfg.use_program(&pio.load_program(&relocated), &[&out_pin]); + cfg.use_program(&pio.load_program(&prg), &[&out_pin]); // Clock config, measured in kHz to avoid overflows // TODO CLOCK_FREQ should come from embassy_rp diff --git a/tests/rp/src/bin/pio_irq.rs b/tests/rp/src/bin/pio_irq.rs index 45004424..bdea63ea 100644 --- a/tests/rp/src/bin/pio_irq.rs +++ b/tests/rp/src/bin/pio_irq.rs @@ -9,7 +9,6 @@ use embassy_executor::Spawner; use embassy_rp::bind_interrupts; use embassy_rp::peripherals::PIO0; use embassy_rp::pio::{Config, InterruptHandler, Pio}; -use embassy_rp::relocate::RelocatedProgram; use {defmt_rtt as _, panic_probe as _}; bind_interrupts!(struct Irqs { @@ -35,9 +34,8 @@ async fn main(_spawner: Spawner) { "irq wait 1", ); - let relocated = RelocatedProgram::new(&prg.program); let mut cfg = Config::default(); - cfg.use_program(&common.load_program(&relocated), &[]); + cfg.use_program(&common.load_program(&prg.program), &[]); sm.set_config(&cfg); sm.set_enable(true); diff --git a/tests/rp/src/bin/pio_multi_load.rs b/tests/rp/src/bin/pio_multi_load.rs new file mode 100644 index 00000000..356f1679 --- /dev/null +++ b/tests/rp/src/bin/pio_multi_load.rs @@ -0,0 +1,126 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] +#[path = "../common.rs"] +mod common; + +use defmt::info; +use embassy_executor::Spawner; +use embassy_rp::bind_interrupts; +use embassy_rp::peripherals::PIO0; +use embassy_rp::pio::{Config, InterruptHandler, LoadError, Pio}; +use {defmt_rtt as _, panic_probe as _}; + +bind_interrupts!(struct Irqs { + PIO0_IRQ_0 => InterruptHandler; +}); + +#[embassy_executor::main] +async fn main(_spawner: Spawner) { + let p = embassy_rp::init(Default::default()); + let pio = p.PIO0; + let Pio { + mut common, + mut sm0, + mut sm1, + mut sm2, + irq_flags, + .. + } = Pio::new(pio, Irqs); + + // load with explicit origin works + let prg1 = pio_proc::pio_asm!( + ".origin 4" + "nop", + "nop", + "nop", + "nop", + "nop", + "nop", + "nop", + "irq 0", + "nop", + "nop", + ); + let loaded1 = common.load_program(&prg1.program); + assert_eq!(loaded1.origin, 4); + assert_eq!(loaded1.wrap.source, 13); + assert_eq!(loaded1.wrap.target, 4); + + // load without origin chooses a free space + let prg2 = pio_proc::pio_asm!("nop", "nop", "nop", "nop", "nop", "nop", "nop", "irq 1", "nop", "nop",); + let loaded2 = common.load_program(&prg2.program); + assert_eq!(loaded2.origin, 14); + assert_eq!(loaded2.wrap.source, 23); + assert_eq!(loaded2.wrap.target, 14); + + // wrapping around the end of program space automatically works + let prg3 = + pio_proc::pio_asm!("nop", "nop", "nop", "nop", "nop", "nop", "nop", "nop", "nop", "nop", "nop", "irq 2",); + let loaded3 = common.load_program(&prg3.program); + assert_eq!(loaded3.origin, 24); + assert_eq!(loaded3.wrap.source, 3); + assert_eq!(loaded3.wrap.target, 24); + + // check that the programs actually work + { + let mut cfg = Config::default(); + cfg.use_program(&loaded1, &[]); + sm0.set_config(&cfg); + sm0.set_enable(true); + while !irq_flags.check(0) {} + sm0.set_enable(false); + } + { + let mut cfg = Config::default(); + cfg.use_program(&loaded2, &[]); + sm1.set_config(&cfg); + sm1.set_enable(true); + while !irq_flags.check(1) {} + sm1.set_enable(false); + } + { + let mut cfg = Config::default(); + cfg.use_program(&loaded3, &[]); + sm2.set_config(&cfg); + sm2.set_enable(true); + while !irq_flags.check(2) {} + sm2.set_enable(false); + } + + // instruction memory is full now. all loads should fail. + { + let prg = pio_proc::pio_asm!(".origin 0", "nop"); + match common.try_load_program(&prg.program) { + Err(LoadError::AddressInUse(0)) => (), + _ => panic!("program loaded when it shouldn't"), + }; + + let prg = pio_proc::pio_asm!("nop"); + match common.try_load_program(&prg.program) { + Err(LoadError::InsufficientSpace) => (), + _ => panic!("program loaded when it shouldn't"), + }; + } + + // freeing some memory should allow further loads though. + unsafe { + common.free_instr(loaded3.used_memory); + } + { + let prg = pio_proc::pio_asm!(".origin 0", "nop"); + match common.try_load_program(&prg.program) { + Ok(_) => (), + _ => panic!("program didn't loaded when it shouldn"), + }; + + let prg = pio_proc::pio_asm!("nop"); + match common.try_load_program(&prg.program) { + Ok(_) => (), + _ => panic!("program didn't loaded when it shouldn"), + }; + } + + info!("Test OK"); + cortex_m::asm::bkpt(); +}