From f4ade6af8bb2571ce2de0531d9c9715a7b8b941c Mon Sep 17 00:00:00 2001 From: pennae Date: Tue, 25 Apr 2023 20:40:18 +0200 Subject: [PATCH] rp/pio: write instr memory only from common instruction memory is a shared resource. writing it only from PioCommon clarifies this, and perhaps makes it more obvious that multiple state machines can share the same instructions. this also allows *freeing* of instruction memory to reprogram the system, although this interface is not entirely safe yet. it's safe in the sense rusts understands things, but state machines may misbehave if their instruction memory is freed and rewritten while they are running. fixing this is out of scope for now since it requires some larger changes to how state machines are handled. the interface provided currently is already unsafe in that it lets people execute instruction memory that has never been written, so this isn't much of a drawback for now. --- embassy-rp/src/pio.rs | 106 ++++++++++++------------------ examples/rp/src/bin/pio_async.rs | 28 +++++--- examples/rp/src/bin/pio_dma.rs | 6 +- examples/rp/src/bin/ws2812-pio.rs | 4 +- 4 files changed, 64 insertions(+), 80 deletions(-) diff --git a/embassy-rp/src/pio.rs b/embassy-rp/src/pio.rs index 8492ca31..7faec10b 100644 --- a/embassy-rp/src/pio.rs +++ b/embassy-rp/src/pio.rs @@ -772,19 +772,6 @@ pub trait PioStateMachine: sealed::PioStateMachine + Sized + Unpin { } } - fn write_instr(&mut self, start: usize, instrs: I) - where - I: Iterator, - { - write_instr( - Self::Pio::PIO, - Self::Pio::PIO_NO, - start, - instrs, - MEM_USED_BY_STATEMACHINE | Self::Sm::SM_NO as u32, - ); - } - fn wait_push<'a>(&'a mut self, value: u32) -> FifoOutFuture<'a, Self::Pio, Self> { FifoOutFuture::new(self, value) } @@ -880,71 +867,59 @@ pub trait PioStateMachine: sealed::PioStateMachine + Sized + Unpin { } } -/* -This is a bit array containing 4 bits for every word in the PIO instruction memory. -*/ -// Bit 3-2 -//const MEM_USE_MASK: u32 = 0b1100; -const MEM_NOT_USED: u32 = 0b0000; -const MEM_USED_BY_STATEMACHINE: u32 = 0b0100; -const MEM_USED_BY_COMMON: u32 = 0b1000; - -// Bit 1-0 is the number of the state machine -//const MEM_STATE_MASK: u32 = 0b0011; - -// Should use mutex if running on multiple cores -static mut INSTR_MEM_STATUS: &'static mut [[u32; 4]; 2] = &mut [[0; 4]; 2]; - -fn instr_mem_get_status(pio_no: u8, addr: u8) -> u32 { - ((unsafe { INSTR_MEM_STATUS[pio_no as usize][(addr >> 3) as usize] }) >> ((addr & 0x07) * 4)) & 0xf -} - -fn instr_mem_set_status(pio_no: u8, addr: u8, status: u32) { - let w = unsafe { &mut INSTR_MEM_STATUS[pio_no as usize][(addr >> 3) as usize] }; - let shift = (addr & 0x07) * 4; - *w = (*w & !(0xf << shift)) | (status << shift); -} - -fn instr_mem_is_free(pio_no: u8, addr: u8) -> bool { - instr_mem_get_status(pio_no, addr) == MEM_NOT_USED -} - pub struct PioCommonInstance { + instructions_used: u32, + pio: PhantomData, +} + +pub struct PioInstanceMemory { + used_mask: u32, pio: PhantomData, } impl sealed::PioCommon for PioCommonInstance { type Pio = PIO; } -impl PioCommon for PioCommonInstance {} - -fn write_instr(pio: &pac::pio::Pio, pio_no: u8, start: usize, instrs: I, mem_user: u32) -where - I: Iterator, -{ - for (i, instr) in instrs.enumerate() { - let addr = (i + start) as u8; - assert!( - instr_mem_is_free(pio_no, addr), - "Trying to write already used PIO instruction memory at {}", - addr - ); - unsafe { - pio.instr_mem(addr as usize).write(|w| { - w.set_instr_mem(instr); - }); - instr_mem_set_status(pio_no, addr, mem_user); +impl PioCommon for PioCommonInstance { + fn write_instr(&mut self, start: usize, instrs: I) -> PioInstanceMemory + where + I: Iterator, + { + let mut used_mask = 0; + for (i, instr) in instrs.enumerate() { + let addr = (i + start) as u8; + let mask = 1 << (addr as usize); + assert!( + self.instructions_used & mask == 0, + "Trying to write already used PIO instruction memory at {}", + addr + ); + unsafe { + PIO::PIO.instr_mem(addr as usize).write(|w| { + w.set_instr_mem(instr); + }); + } + used_mask |= mask; } + self.instructions_used |= used_mask; + PioInstanceMemory { + used_mask, + pio: PhantomData, + } + } + + fn free_instr(&mut self, instrs: PioInstanceMemory) { + self.instructions_used &= !instrs.used_mask; } } pub trait PioCommon: sealed::PioCommon + Sized { - fn write_instr(&mut self, start: usize, instrs: I) + fn write_instr(&mut self, start: usize, instrs: I) -> PioInstanceMemory where - I: Iterator, - { - write_instr(Self::Pio::PIO, Self::Pio::PIO_NO, start, instrs, MEM_USED_BY_COMMON); - } + I: Iterator; + + // TODO make instruction memory that is currently in use unfreeable + fn free_instr(&mut self, instrs: PioInstanceMemory); fn is_irq_set(&self, irq_no: u8) -> bool { assert!(irq_no < 8); @@ -1027,6 +1002,7 @@ pub trait PioPeripheral: sealed::PioPeripheral + Sized { ) { ( PioCommonInstance { + instructions_used: 0, pio: PhantomData::default(), }, PioStateMachineInstance { diff --git a/examples/rp/src/bin/pio_async.rs b/examples/rp/src/bin/pio_async.rs index 3cfeec71..1b075b8f 100644 --- a/examples/rp/src/bin/pio_async.rs +++ b/examples/rp/src/bin/pio_async.rs @@ -28,7 +28,7 @@ fn setup_pio_task_sm0(pio: &mut PioCommonInstance, sm: &mut PioStateMachin let out_pin = pio.make_pio_pin(pin); let pio_pins = [&out_pin]; sm.set_out_pins(&pio_pins); - sm.write_instr(relocated.origin() as usize, relocated.code()); + pio.write_instr(relocated.origin() as usize, relocated.code()); pio_instr_util::exec_jmp(sm, relocated.origin()); sm.set_clkdiv((125e6 / 20.0 / 2e2 * 256.0) as u32); sm.set_set_range(0, 1); @@ -51,16 +51,15 @@ async fn pio_task_sm0(mut sm: PioStateMachineInstance) { } } -#[embassy_executor::task] -async fn pio_task_sm1(mut sm: PioStateMachineInstance) { +fn setup_pio_task_sm1(pio: &mut PioCommonInstance, sm: &mut PioStateMachineInstance) { // Setupm sm1 // Read 0b10101 repeatedly until ISR is full let prg = pio_proc::pio_asm!(".origin 8", "set x, 0x15", ".wrap_target", "in x, 5 [31]", ".wrap",); let relocated = RelocatedProgram::new(&prg.program); - sm.write_instr(relocated.origin() as usize, relocated.code()); - pio_instr_util::exec_jmp(&mut sm, relocated.origin()); + pio.write_instr(relocated.origin() as usize, relocated.code()); + pio_instr_util::exec_jmp(sm, relocated.origin()); sm.set_clkdiv((125e6 / 2e3 * 256.0) as u32); sm.set_set_range(0, 0); let pio::Wrap { source, target } = relocated.wrap(); @@ -68,6 +67,10 @@ async fn pio_task_sm1(mut sm: PioStateMachineInstance) { sm.set_autopush(true); sm.set_in_shift_dir(ShiftDirection::Right); +} + +#[embassy_executor::task] +async fn pio_task_sm1(mut sm: PioStateMachineInstance) { sm.set_enable(true); loop { let rx = sm.wait_pull().await; @@ -75,8 +78,7 @@ async fn pio_task_sm1(mut sm: PioStateMachineInstance) { } } -#[embassy_executor::task] -async fn pio_task_sm2(mut sm: PioStateMachineInstance) { +fn setup_pio_task_sm2(pio: &mut PioCommonInstance, sm: &mut PioStateMachineInstance) { // Setup sm2 // Repeatedly trigger IRQ 3 @@ -90,13 +92,17 @@ async fn pio_task_sm2(mut sm: PioStateMachineInstance) { ".wrap", ); let relocated = RelocatedProgram::new(&prg.program); - sm.write_instr(relocated.origin() as usize, relocated.code()); + pio.write_instr(relocated.origin() as usize, relocated.code()); let pio::Wrap { source, target } = relocated.wrap(); sm.set_wrap(source, target); - pio_instr_util::exec_jmp(&mut sm, relocated.origin()); + pio_instr_util::exec_jmp(sm, relocated.origin()); sm.set_clkdiv((125e6 / 2e3 * 256.0) as u32); +} + +#[embassy_executor::task] +async fn pio_task_sm2(mut sm: PioStateMachineInstance) { sm.set_enable(true); loop { sm.wait_irq(3).await; @@ -109,9 +115,11 @@ async fn main(spawner: Spawner) { let p = embassy_rp::init(Default::default()); let pio = p.PIO0; - let (mut pio0, mut sm0, sm1, sm2, ..) = pio.split(); + let (mut pio0, mut sm0, mut sm1, mut sm2, ..) = pio.split(); setup_pio_task_sm0(&mut pio0, &mut sm0, p.PIN_0.degrade()); + setup_pio_task_sm1(&mut pio0, &mut sm1); + setup_pio_task_sm2(&mut pio0, &mut sm2); spawner.spawn(pio_task_sm0(sm0)).unwrap(); spawner.spawn(pio_task_sm1(sm1)).unwrap(); spawner.spawn(pio_task_sm2(sm2)).unwrap(); diff --git a/examples/rp/src/bin/pio_dma.rs b/examples/rp/src/bin/pio_dma.rs index 145e4a65..7d4919f7 100644 --- a/examples/rp/src/bin/pio_dma.rs +++ b/examples/rp/src/bin/pio_dma.rs @@ -4,7 +4,7 @@ use defmt::info; use embassy_executor::Spawner; use embassy_futures::join::join; -use embassy_rp::pio::{PioPeripheral, PioStateMachine, ShiftDirection}; +use embassy_rp::pio::{PioCommon, PioPeripheral, PioStateMachine, ShiftDirection}; use embassy_rp::relocate::RelocatedProgram; use embassy_rp::{pio_instr_util, Peripheral}; use {defmt_rtt as _, panic_probe as _}; @@ -19,7 +19,7 @@ fn swap_nibbles(v: u32) -> u32 { async fn main(_spawner: Spawner) { let p = embassy_rp::init(Default::default()); let pio = p.PIO0; - let (_, mut sm, ..) = pio.split(); + let (mut pio0, mut sm, ..) = pio.split(); let prg = pio_proc::pio_asm!( ".origin 0", @@ -34,7 +34,7 @@ async fn main(_spawner: Spawner) { ); let relocated = RelocatedProgram::new(&prg.program); - sm.write_instr(relocated.origin() as usize, relocated.code()); + pio0.write_instr(relocated.origin() as usize, relocated.code()); pio_instr_util::exec_jmp(&mut sm, relocated.origin()); sm.set_clkdiv((125e6 / 10e3 * 256.0) as u32); let pio::Wrap { source, target } = relocated.wrap(); diff --git a/examples/rp/src/bin/ws2812-pio.rs b/examples/rp/src/bin/ws2812-pio.rs index 211c60c4..041e8ae1 100644 --- a/examples/rp/src/bin/ws2812-pio.rs +++ b/examples/rp/src/bin/ws2812-pio.rs @@ -19,7 +19,7 @@ pub struct Ws2812 { } impl Ws2812 { - pub fn new(pio: PioCommonInstance

, mut sm: PioStateMachineInstance, pin: gpio::AnyPin) -> Self { + pub fn new(mut pio: PioCommonInstance

, mut sm: PioStateMachineInstance, pin: gpio::AnyPin) -> Self { // Setup sm0 // prepare the PIO program @@ -50,7 +50,7 @@ impl Ws2812 { let prg = a.assemble_with_wrap(wrap_source, wrap_target); let relocated = RelocatedProgram::new(&prg); - sm.write_instr(relocated.origin() as usize, relocated.code()); + pio.write_instr(relocated.origin() as usize, relocated.code()); pio_instr_util::exec_jmp(&mut sm, relocated.origin()); // Pin config