From e74585da24d4c98d97c7881db881a99351e7edcd Mon Sep 17 00:00:00 2001 From: Dietrich Beck Date: Tue, 4 Jul 2023 12:00:26 +0200 Subject: [PATCH] option to not restart if not in time --- embassy-rp/src/adc.rs | 50 ++++- embassy-rp/src/dma.rs | 312 +++++++++++++------------- examples/rp/src/bin/adc_dma.rs | 2 +- examples/rp/src/bin/dma_continuous.rs | 2 +- 4 files changed, 202 insertions(+), 164 deletions(-) diff --git a/embassy-rp/src/adc.rs b/embassy-rp/src/adc.rs index 3bf164d4..8163525d 100644 --- a/embassy-rp/src/adc.rs +++ b/embassy-rp/src/adc.rs @@ -105,7 +105,7 @@ where { pin.pad_ctrl().modify(|w| { w.set_ie(true); - let (pu, pd) = (false, true); // TODO there is another pull request related to this change, also check datasheet chapter 4.9 + let (pu, pd) = (false, true); // datasheet chapter 4.9 w.set_pue(pu); w.set_pde(pd); }); @@ -468,6 +468,11 @@ pub struct ContinuousAdc<'a, 'b, 'c, 'd, 'r, W: Word, C1: DmaChannel, C2: DmaCha corrupted: bool, } +pub enum NextOrStop<'a, 'b, 'c, 'd, 'r, W: Word, C1: DmaChannel, C2: DmaChannel, In: Input> { + Next(ContinuousAdc<'a, 'b, 'c, 'd, 'r, W, C1, C2, In>), + Stop(Adc<'d>), +} + impl<'a, 'b, 'c, 'd, 'r, W: Word, C1: DmaChannel, C2: DmaChannel, In: Input> ContinuousAdc<'a, 'b, 'c, 'd, 'r, W, C1, C2, In> { @@ -477,7 +482,7 @@ impl<'a, 'b, 'c, 'd, 'r, W: Word, C1: DmaChannel, C2: DmaChannel, In: Input> ch2: PeripheralRef<'a, C2>, mut input: In, speed: SamplingSpeed, - control_input: &'c mut [[u32; 4]; 2], + control_input: &'c mut [u32; 4], buffer: &'b mut [W], ) -> ContinuousAdc<'a, 'b, 'c, 'd, 'r, W, C1, C2, In> { assert!(W::size() as u8 <= 1); // u16 or u8 (will right-shift) allowed TODO static_assert possible? @@ -553,20 +558,49 @@ impl<'a, 'b, 'c, 'd, 'r, W: Word, C1: DmaChannel, C2: DmaChannel, In: Input> ) } + pub async fn next_or_stop<'new_buf>( + self, + buffer: &'new_buf mut [W], + ) -> NextOrStop<'a, 'new_buf, 'c, 'd, 'r, W, C1, C2, In> { + let ContinuousAdc { + adc, + transfer, + input, + corrupted, + } = self; + if let Some(transfer) = transfer.next_or_stop(buffer).await { + NextOrStop::Next(ContinuousAdc { + adc, + transfer, + input, + corrupted, + }) + } else { + Self::stop_private(true).await; + NextOrStop::Stop(adc) + } + } + pub async fn stop(self) -> Adc<'d> { self.transfer.stop().await; + Self::stop_private(self.corrupted).await; + + // you only get your adc back if you stop the ContinuousAdc like intended + // (i.e. don't drop it while it is still running) + self.adc + } + + async fn stop_private(hard_reset: bool) { // stop adc let r = Adc::regs(); r.cs().modify(|w| { w.set_start_many(false); }); - if self.input.measure_temperature() { - r.cs().modify(|w| w.set_ts_en(false)); - } + r.cs().modify(|w| w.set_ts_en(false)); Adc::fifo_drain().await; - if self.corrupted { + if hard_reset { // TODO this is a fix to a problem where round robin order is shifted and the first few samples of any following start_many measurements seem to have random order // TODO I was not able to find the real cause, but it would only appear with a certain chance if the next buffer was not provided in time // completely reset adc @@ -579,10 +613,6 @@ impl<'a, 'b, 'c, 'd, 'r, W: Word, C1: DmaChannel, C2: DmaChannel, In: Input> // Wait for ADC ready while !r.cs().read().ready() {} } - - // you only get your adc back if you stop the ContinuousAdc like intended - // (i.e. don't drop it while it is still running) - self.adc } } diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index 6a0a4624..e8bf7d92 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -194,7 +194,7 @@ impl<'a, C: Channel> Future for Transfer<'a, C> { pub enum Read<'a, W: Word> { Constant(&'a W), Increase(&'a [W]), - // TODO ring also possible, but more complicated due to generic size and alignment requirements + // ring also possible, but more complicated due to generic size and alignment requirements } impl<'a, W: Word> Read<'a, W> { @@ -219,73 +219,42 @@ impl<'a, W: Word> Read<'a, W> { } } -struct InnerChannels<'a, C1: Channel, C2: Channel> { +struct InnerContinuous<'a, 'c, 'r, W: Word, C1: Channel, C2: Channel> { data: PeripheralRef<'a, C1>, control: PeripheralRef<'a, C2>, -} - -impl<'a, C1: Channel, C2: Channel> Drop for InnerChannels<'a, C1, C2> { - fn drop(&mut self) { - pac::DMA - .chan_abort() - .modify(|m| m.set_chan_abort((1 << self.data.number()) | (1 << self.control.number()))); - - // wait until both channels are ready again, this should go quite fast so no async used here - while self.data.regs().ctrl_trig().read().busy() || self.control.regs().ctrl_trig().read().busy() {} - } -} - -pub struct ContinuousTransfer<'a, 'b, 'c, 'r, W: Word, C1: Channel, C2: Channel> { - channels: InnerChannels<'a, C1, C2>, - #[allow(dead_code)] // reference is kept to signal that dma channels are writing to it - buffer: &'b mut [W], - control_input: &'c mut [[u32; 4]; 2], + control_input: &'c mut [u32; 4], dreq: TreqSel, read: Read<'r, W>, } -impl<'a, 'b, 'c, 'r, W: Word, C1: Channel, C2: Channel> ContinuousTransfer<'a, 'b, 'c, 'r, W, C1, C2> { - pub fn start_new( - ch1: PeripheralRef<'a, C1>, - ch2: PeripheralRef<'a, C2>, - control_input: &'c mut [[u32; 4]; 2], - buffer: &'b mut [W], - dreq: TreqSel, - mut read: Read<'r, W>, - ) -> ContinuousTransfer<'a, 'b, 'c, 'r, W, C1, C2> { - let channels = InnerChannels { - data: ch1, - control: ch2, - }; +impl<'a, 'c, 'r, W: Word, C1: Channel, C2: Channel> InnerContinuous<'a, 'c, 'r, W, C1, C2> { + // SAFETY: the compiler does not know buffer is still modified after the function returns + unsafe fn start(&mut self, buffer: &mut [W]) { + let pc = self.control.regs(); + let pd = self.data.regs(); + let control_ptr = self.control_input.as_ptr() as u32; // configure what control channel writes - // using registers: READ_ADDR, WRITE_ADDR, TRANS_COUNT, CTRL_TRIG let mut w = CtrlTrig(0); - w.set_treq_sel(dreq); + w.set_treq_sel(self.dreq); w.set_data_size(W::size()); - w.set_incr_read(read.is_increase()); + w.set_incr_read(self.read.is_increase()); w.set_incr_write(true); - w.set_chain_to(channels.data.number()); // chain disabled by default + w.set_chain_to(self.data.number()); // chain disabled by default w.set_en(true); w.set_irq_quiet(false); + // writing to registers: READ_ADDR, WRITE_ADDR, TRANS_COUNT, CTRL_TRIG + *self.control_input = [self.read.address(), buffer.as_ptr() as u32, buffer.len() as u32, w.0]; - *control_input = [ - [read.address(), buffer.as_ptr() as u32, buffer.len() as u32, w.0], // first control write - [0; 4], // Null trigger to stop - ]; - - // Configure data channel - // will be set by control channel - let pd = channels.data.regs(); + // configure data channel to some values, correct ones will be set by control channel pd.read_addr().write_value(0); pd.write_addr().write_value(0); pd.trans_count().write_value(0); pd.al1_ctrl().write_value(0); - // Configure control channel - let pc = channels.control.regs(); + // configure control channel pc.write_addr().write_value(pd.read_addr().as_ptr() as u32); - pc.read_addr().write_value(control_input.as_ptr() as u32); + pc.read_addr().write_value(control_ptr); pc.trans_count().write_value(4); // each control input is 4 u32s long // trigger control channel @@ -297,27 +266,154 @@ impl<'a, 'b, 'c, 'r, W: Word, C1: Channel, C2: Channel> ContinuousTransfer<'a, ' w.set_incr_write(true); // yes, but ring is required w.set_ring_sel(true); // wrap write addresses w.set_ring_size(4); // 1 << 4 = 16 = 4 * sizeof(u32) bytes - w.set_chain_to(channels.control.number()); // disable chain, data channel is triggered by write + w.set_chain_to(self.control.number()); // disable chain, data channel is triggered by write w.set_irq_quiet(false); w.set_en(true); }); compiler_fence(Ordering::SeqCst); - // wait until control ran + self.after_start(buffer.len()); + } + + // SAFETY: the compiler does not know buffer is still modified after the function returns + async unsafe fn next(&mut self, buffer: &mut [W], auto_restart: bool) -> bool { + let pc = self.control.regs(); + let pd = self.data.regs(); + let control_ptr = self.control_input.as_ptr() as u32; + + // configure control input to use new buffer + let mut w = CtrlTrig(0); + w.set_treq_sel(self.dreq); + w.set_data_size(W::size()); + w.set_incr_read(self.read.is_increase()); + w.set_incr_write(true); + w.set_chain_to(self.data.number()); // chain disabled by default + w.set_en(true); + w.set_irq_quiet(false); + *self.control_input = [self.read.address(), buffer.as_ptr() as u32, buffer.len() as u32, w.0]; + + // enable chain of running data channel, now we can't change control safely anymore + // using al1_ctrl to not trigger the channel in case it stopped + compiler_fence(Ordering::SeqCst); + pd.al1_ctrl().write_value({ + let mut ctrl = pd.ctrl_trig().read(); + ctrl.set_chain_to(self.control.number()); + ctrl.0 + }); + compiler_fence(Ordering::SeqCst); + + // order is really important in this if statement, otherwise it can happen that the chain still activated + if pd.ctrl_trig().read().busy() || pc.read_addr().read() > control_ptr { + poll_fn(|cx: &mut Context<'_>| { + CHANNEL_WAKERS[self.data.number() as usize].register(cx.waker()); + if pc.read_addr().read() > control_ptr { + Poll::Ready(()) + } else { + Poll::Pending + } + }) + .await; + + self.after_start(buffer.len()); + true + } else { + if auto_restart { + // trigger control to restart loop + pc.ctrl_trig().write_value(pc.ctrl_trig().read()); + compiler_fence(Ordering::SeqCst); + + self.after_start(buffer.len()); + } + false + } + } + + fn after_start(&mut self, buffer_len: usize) { + let control_ptr = self.control_input.as_ptr() as u32; + let pc = self.control.regs(); + while pc.ctrl_trig().read().busy() {} - // reset control - control_input[0] = [0; 4]; - pc.read_addr().write_value(control_input.as_ptr() as u32); + // don't fail silently, control must not read anything but control_input + assert!(pc.read_addr().read() == control_ptr + 16); - read.forward(buffer.len()); + // reset read adress + pc.read_addr().write_value(control_ptr); - ContinuousTransfer { - channels, - buffer, + // reset control input, not strictly necessary, but helpful if something goes wrong + *self.control_input = [0; 4]; + + self.read.forward(buffer_len); + } + + async fn stop(&mut self) { + // when no longer enabling the chain, the data channel simply stops + poll_fn(|cx| { + CHANNEL_WAKERS[self.data.number() as usize].register(cx.waker()); + if self.data.regs().ctrl_trig().read().busy() { + Poll::Pending + } else { + Poll::Ready(()) + } + }) + .await; + } +} + +impl<'a, 'c, 'r, W: Word, C1: Channel, C2: Channel> Drop for InnerContinuous<'a, 'c, 'r, W, C1, C2> { + fn drop(&mut self) { + pac::DMA + .chan_abort() + .modify(|m| m.set_chan_abort((1 << self.data.number()) | (1 << self.control.number()))); + + // wait until both channels are ready again, this should go quite fast so no async used here + while self.data.regs().ctrl_trig().read().busy() || self.control.regs().ctrl_trig().read().busy() {} + } +} + +// contract: if the user has a ContinuousTransfer, it is always running +// Using InnerContinuous is unsafe, because the rust compiler has no knowledge of the dma +// channels modifying the buffer. This is why we keep a &mut to the buffer here +pub struct ContinuousTransfer<'a, 'b, 'c, 'r, W: Word, C1: Channel, C2: Channel> { + inner: InnerContinuous<'a, 'c, 'r, W, C1, C2>, + #[allow(dead_code)] + buffer: &'b mut [W], +} + +impl<'a, 'b, 'c, 'r, W: Word, C1: Channel, C2: Channel> ContinuousTransfer<'a, 'b, 'c, 'r, W, C1, C2> { + pub fn start_new( + ch1: PeripheralRef<'a, C1>, + ch2: PeripheralRef<'a, C2>, + control_input: &'c mut [u32; 4], + buffer: &'b mut [W], + dreq: TreqSel, + read: Read<'r, W>, + ) -> ContinuousTransfer<'a, 'b, 'c, 'r, W, C1, C2> { + let mut inner = InnerContinuous { + data: ch1, + control: ch2, control_input, dreq, read, + }; + // SAFETY: we keep a &mut to buffer around to signal it is being written to + unsafe { inner.start(buffer) }; + ContinuousTransfer { inner, buffer } + } + + pub async fn next_or_stop<'new_buf>( + self, + buffer: &'new_buf mut [W], + ) -> Option> { + let ContinuousTransfer { + mut inner, + buffer: _old, + } = self; + // SAFETY: we keep a &mut to buffer around to signal it is being written to + let in_time = unsafe { inner.next(buffer, false).await }; + match in_time { + true => Some(ContinuousTransfer { inner, buffer }), + false => None, } } @@ -326,104 +422,16 @@ impl<'a, 'b, 'c, 'r, W: Word, C1: Channel, C2: Channel> ContinuousTransfer<'a, ' buffer: &'new_buf mut [W], ) -> (ContinuousTransfer<'a, 'new_buf, 'c, 'r, W, C1, C2>, bool) { let ContinuousTransfer { - channels, - buffer: _old, // is free now, and the compiler knows it - control_input, - dreq, - mut read, + mut inner, + buffer: _old, } = self; - - let pc = channels.control.regs(); - let pd = channels.data.regs(); - - let mut w = CtrlTrig(0); - w.set_treq_sel(dreq); - w.set_data_size(W::size()); - w.set_incr_read(read.is_increase()); - w.set_incr_write(true); - w.set_chain_to(channels.data.number()); // chain disabled by default - w.set_en(true); - w.set_irq_quiet(false); - - // configure control - control_input[0] = [read.address(), buffer.as_ptr() as u32, buffer.len() as u32, w.0]; - - // enable chain, now we can't change control safely anymore - compiler_fence(Ordering::SeqCst); - pd.al1_ctrl().write_value({ - let mut ctrl = pd.ctrl_trig().read(); - ctrl.set_chain_to(channels.control.number()); - ctrl.0 - }); - - if pc.read_addr().read() == control_input.as_ptr() as u32 && pd.ctrl_trig().read().busy() { - poll_fn(|cx: &mut Context<'_>| { - CHANNEL_WAKERS[channels.data.number() as usize].register(cx.waker()); - if pc.read_addr().read() == control_input.as_ptr() as u32 + 16 { - Poll::Ready(()) - } else { - Poll::Pending - } - }) - .await; - - // reset control - assert!(!pc.ctrl_trig().read().busy()); - control_input[0] = [0; 4]; - pc.read_addr().write_value(control_input.as_ptr() as u32); - - read.forward(buffer.len()); - - ( - ContinuousTransfer { - channels, - buffer, - control_input, - dreq, - read, - }, - true, - ) - } else { - if pc.read_addr().read() == control_input.as_ptr() as u32 { - // trigger control to restart loop - pc.ctrl_trig().write_value(pc.ctrl_trig().read()); - compiler_fence(Ordering::SeqCst); - } - // if control read already moved, data has already been activated - - // wait for control to complete - while pc.ctrl_trig().read().busy() {} - // reset control - control_input[0] = [0; 4]; - pc.read_addr().write_value(control_input.as_ptr() as u32); - - read.forward(buffer.len()); - - ( - ContinuousTransfer { - channels, - control_input, - buffer, - dreq, - read, - }, - false, - ) - } + // SAFETY: we keep a &mut to buffer around to signal it is being written to + let in_time = unsafe { inner.next(buffer, true).await }; + (ContinuousTransfer { inner, buffer }, in_time) } - pub async fn stop(self) { - // when no longer enabling the chain, data simply stops - poll_fn(|cx| { - CHANNEL_WAKERS[self.channels.data.number() as usize].register(cx.waker()); - if self.channels.data.regs().ctrl_trig().read().busy() { - Poll::Pending - } else { - Poll::Ready(()) - } - }) - .await; + pub async fn stop(mut self) { + self.inner.stop().await; } pub fn abort(self) {} // drop channels diff --git a/examples/rp/src/bin/adc_dma.rs b/examples/rp/src/bin/adc_dma.rs index f81cb3f7..7ae3e804 100644 --- a/examples/rp/src/bin/adc_dma.rs +++ b/examples/rp/src/bin/adc_dma.rs @@ -98,7 +98,7 @@ async fn main(_spawner: Spawner) { // this particular input leads to adc measuring temp, p26, p27, p28, temp, p26, p27, p28, ... let input = adc::input_temperature(true).add(&mut p28).add(&mut p27).add(&mut p26); - let mut control_input = [[0u32; 4]; 2]; + let mut control_input = [0u32; 4]; let speed = SamplingSpeed::Fastest; let mut sums = [0u32; 4]; // p26, p27, p28, temp diff --git a/examples/rp/src/bin/dma_continuous.rs b/examples/rp/src/bin/dma_continuous.rs index e0e16262..0af2ac13 100644 --- a/examples/rp/src/bin/dma_continuous.rs +++ b/examples/rp/src/bin/dma_continuous.rs @@ -24,7 +24,7 @@ async fn main(_spawner: Spawner) { *x = (2 - i as u32 / 100) * 100 + i as u32 % 100 + 1; } - let mut control_input = [[0u32; 4]; 2]; + let mut control_input = [0u32; 4]; let mut to_buf0 = [0u32; 100]; let mut to_buf1 = [0u32; 100];