From 3165c680d63c1f2307964b3dad1e17e4261a49ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 24 May 2021 20:06:31 +0200 Subject: [PATCH 01/15] dma: don't swap buffers * This uses a new closure-based method to the DMA HAL implementation which gives access to the inactive buffer directly. * It removes changing addresses, the third buffer for DBM, the inactive address poisoning, and allows the cancellation of the redundant repeat memory barriers and compiler fences. * This is now around 20 instructions per buffer down from about 100 cycles before. * Also introduces a new `SampleBuffer` type alias. * The required unpacking of the resources structure is a bit annoying but could probably abstraced away. TODO: * Test * Adapt `lockin` --- Cargo.lock | 3 +- Cargo.toml | 4 +- src/bin/dual-iir.rs | 100 ++++++++++++++++++------------ src/hardware/adc.rs | 65 +++++++------------ src/hardware/dac.rs | 43 +++++-------- src/hardware/design_parameters.rs | 2 + 6 files changed, 107 insertions(+), 110 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 274b2bb..49ac680 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -802,8 +802,7 @@ dependencies = [ [[package]] name = "stm32h7xx-hal" version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67034b80041bc33a48df1c1c435b6ae3bb18c35e42aa7e702ce8363b96793398" +source = "git+https://github.com/quartiq/stm32h7xx-hal.git?rev=cca4ecc#cca4ecc3e0cc8cb2f7a9652c4099d50b44977493" dependencies = [ "bare-metal 1.0.0", "cast", diff --git a/Cargo.toml b/Cargo.toml index 790363a..0d84c27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,9 @@ rev = "523d71d" [dependencies.stm32h7xx-hal] features = ["stm32h743v", "rt", "unproven", "ethernet", "quadspi"] -version = "0.9.0" +# version = "0.9.0" +git = "https://github.com/quartiq/stm32h7xx-hal.git" +rev = "cca4ecc" [patch.crates-io.miniconf] git = "https://github.com/quartiq/miniconf.git" diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index 29eb474..dec0697 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -2,6 +2,8 @@ #![no_std] #![no_main] +use core::sync::atomic::{fence, Ordering}; + use stabilizer::{hardware, net}; use miniconf::Miniconf; @@ -120,51 +122,73 @@ const APP: () = { /// Because the ADC and DAC operate at the same rate, these two constraints actually implement /// the same time bounds, meeting one also means the other is also met. #[task(binds=DMA1_STR4, resources=[adcs, digital_inputs, dacs, iir_state, settings, telemetry], priority=2)] - fn process(c: process::Context) { - let adc_samples = [ - c.resources.adcs.0.acquire_buffer(), - c.resources.adcs.1.acquire_buffer(), - ]; + fn process(mut c: process::Context) { + let adc0 = &mut c.resources.adcs.0; + let adc1 = &mut c.resources.adcs.1; + let dac0 = &mut c.resources.dacs.0; + let dac1 = &mut c.resources.dacs.1; + let di = &c.resources.digital_inputs; + let settings = &c.resources.settings; + let iir_state = &mut c.resources.iir_state; + let telemetry = &mut c.resources.telemetry; + adc0.with_buffer(|a0| { + adc1.with_buffer(|a1| { + dac0.with_buffer(|d0| { + dac1.with_buffer(|d1| { + let adc_samples = [a0, a1]; + let dac_samples = [d0, d1]; - let dac_samples = [ - c.resources.dacs.0.acquire_buffer(), - c.resources.dacs.1.acquire_buffer(), - ]; + let digital_inputs = + [di.0.is_high().unwrap(), di.1.is_high().unwrap()]; - let digital_inputs = [ - c.resources.digital_inputs.0.is_high().unwrap(), - c.resources.digital_inputs.1.is_high().unwrap(), - ]; + let hold = settings.force_hold + || (digital_inputs[1] && settings.allow_hold); - let hold = c.resources.settings.force_hold - || (digital_inputs[1] && c.resources.settings.allow_hold); + // Preserve instruction and data ordering w.r.t. DMA flag access. + fence(Ordering::SeqCst); - for channel in 0..adc_samples.len() { - for sample in 0..adc_samples[0].len() { - let mut y = f32::from(adc_samples[channel][sample] as i16); - for i in 0..c.resources.iir_state[channel].len() { - y = c.resources.settings.iir_ch[channel][i].update( - &mut c.resources.iir_state[channel][i], - y, - hold, - ); - } - // Note(unsafe): The filter limits ensure that the value is in range. - // The truncation introduces 1/2 LSB distortion. - let y = unsafe { y.to_int_unchecked::() }; - // Convert to DAC code - dac_samples[channel][sample] = DacCode::from(y).0; - } - } + for channel in 0..adc_samples.len() { + adc_samples[channel] + .iter() + .zip(dac_samples[channel].iter_mut()) + .map(|(ai, di)| { + let x = f32::from(*ai as i16); + let y = settings.iir_ch[channel] + .iter() + .zip(iir_state[channel].iter_mut()) + .fold(x, |yi, (ch, state)| { + ch.update(state, yi, hold) + }); + // Note(unsafe): The filter limits must ensure that + // the value is in range. + // The truncation introduces 1/2 LSB distortion. + let y: i16 = + unsafe { y.to_int_unchecked() }; + // Convert to DAC code + *di = DacCode::from(y).0; + }) + .last(); + } - // Update telemetry measurements. - c.resources.telemetry.adcs = - [AdcCode(adc_samples[0][0]), AdcCode(adc_samples[1][0])]; + // Update telemetry measurements. + telemetry.adcs = [ + AdcCode(adc_samples[0][0]), + AdcCode(adc_samples[1][0]), + ]; - c.resources.telemetry.dacs = - [DacCode(dac_samples[0][0]), DacCode(dac_samples[1][0])]; + telemetry.dacs = [ + DacCode(dac_samples[0][0]), + DacCode(dac_samples[1][0]), + ]; - c.resources.telemetry.digital_inputs = digital_inputs; + telemetry.digital_inputs = digital_inputs; + + // Preserve instruction and data ordering w.r.t. DMA flag access. + fence(Ordering::SeqCst); + }) + }) + }) + }); } #[idle(resources=[network], spawn=[settings_update])] diff --git a/src/hardware/adc.rs b/src/hardware/adc.rs index 9289097..101d762 100644 --- a/src/hardware/adc.rs +++ b/src/hardware/adc.rs @@ -29,15 +29,9 @@ ///! available. When enough samples have been collected, a transfer-complete interrupt is generated ///! and the ADC samples are available for processing. ///! -///! The SPI peripheral internally has an 8- or 16-byte TX and RX FIFO, which corresponds to a 4- or -///! 8-sample buffer for incoming ADC samples. During the handling of the DMA transfer completion, -///! there is a small window where buffers are swapped over where it's possible that a sample could -///! be lost. In order to avoid this, the SPI RX FIFO is effectively used as a "sample overflow" -///! region and can buffer a number of samples until the next DMA transfer is configured. If a DMA -///! transfer is still not set in time, the SPI peripheral will generate an input-overrun interrupt. -///! This interrupt then serves as a means of detecting if samples have been lost, which will occur -///! whenever data processing takes longer than the collection period. -///! +///! After a complete transfer of a batch of samples, the inactive buffer is available to the +///! user for processing. The processing must complete before the DMA transfer of the next batch +///! completes. ///! ///! ## Starting Data Collection ///! @@ -68,13 +62,12 @@ ///! sample DMA requests, which can be completed by setting e.g. ADC0's comparison to a counter ///! value of 0 and ADC1's comparison to a counter value of 1. ///! -///! In this implementation, single buffer mode DMA transfers are used because the SPI RX FIFO can -///! be used as a means to both detect and buffer ADC samples during the buffer swap-over. Because -///! of this, double-buffered mode does not offer any advantages over single-buffered mode (unless -///! double-buffered mode offers less overhead due to the DMA disable/enable procedure). +///! In this implementation, double buffer mode DMA transfers are used because the SPI RX FIFOs +///! have finite depth, FIFO access is slower than AXISRAM access, and because the single +///! buffer mode DMA disable/enable and buffer update sequence is slow. use stm32h7xx_hal as hal; -use super::design_parameters::SAMPLE_BUFFER_SIZE; +use super::design_parameters::{SampleBuffer, SAMPLE_BUFFER_SIZE}; use super::timers; use hal::dma::{ @@ -119,8 +112,7 @@ static mut SPI_EOT_CLEAR: [u32; 1] = [0x00]; // processed). Note that the contents of AXI SRAM is uninitialized, so the buffer contents on // startup are undefined. The dimensions are `ADC_BUF[adc_index][ping_pong_index][sample_index]`. #[link_section = ".axisram.buffers"] -static mut ADC_BUF: [[[u16; SAMPLE_BUFFER_SIZE]; 2]; 2] = - [[[0; SAMPLE_BUFFER_SIZE]; 2]; 2]; +static mut ADC_BUF: [[SampleBuffer; 2]; 2] = [[[0; SAMPLE_BUFFER_SIZE]; 2]; 2]; macro_rules! adc_input { ($name:ident, $index:literal, $trigger_stream:ident, $data_stream:ident, $clear_stream:ident, @@ -192,12 +184,11 @@ macro_rules! adc_input { /// Represents data associated with ADC. pub struct $name { - next_buffer: Option<&'static mut [u16; SAMPLE_BUFFER_SIZE]>, transfer: Transfer< hal::dma::dma::$data_stream, hal::spi::Spi, PeripheralToMemory, - &'static mut [u16; SAMPLE_BUFFER_SIZE], + &'static mut SampleBuffer, hal::dma::DBTransfer, >, trigger_transfer: Transfer< @@ -316,6 +307,7 @@ macro_rules! adc_input { // data stream is used to trigger a transfer completion interrupt. let data_config = DmaConfig::default() .memory_increment(true) + .double_buffer(true) .transfer_complete_interrupt($index == 1) .priority(Priority::VeryHigh); @@ -333,17 +325,14 @@ macro_rules! adc_input { Transfer::init( data_stream, spi, - // Note(unsafe): The ADC_BUF[$index][0] is "owned" by this peripheral. + // Note(unsafe): The ADC_BUF[$index] is "owned" by this peripheral. // It shall not be used anywhere else in the module. unsafe { &mut ADC_BUF[$index][0] }, - None, + unsafe { Some(&mut ADC_BUF[$index][1]) }, data_config, ); Self { - // Note(unsafe): The ADC_BUF[$index][1] is "owned" by this peripheral. It - // shall not be used anywhere else in the module. - next_buffer: unsafe { Some(&mut ADC_BUF[$index][1]) }, transfer: data_transfer, trigger_transfer, clear_transfer, @@ -364,27 +353,17 @@ macro_rules! adc_input { } - /// Obtain a buffer filled with ADC samples. + /// Wait for the transfer of the currently active buffer to complete, + /// then call a function on the now inactive buffer and acknowledge the + /// transfer complete flag. /// - /// # Returns - /// A reference to the underlying buffer that has been filled with ADC samples. - pub fn acquire_buffer(&mut self) -> &[u16; SAMPLE_BUFFER_SIZE] { - // Wait for the transfer to fully complete before continuing. Note: If a device - // hangs up, check that this conditional is passing correctly, as there is no - // time-out checks here in the interest of execution speed. - while !self.transfer.get_transfer_complete_flag() {} - - let next_buffer = self.next_buffer.take().unwrap(); - - // Start the next transfer. - self.transfer.clear_interrupts(); - let (prev_buffer, _, _) = - self.transfer.next_transfer(next_buffer).unwrap(); - - // .unwrap_none() https://github.com/rust-lang/rust/issues/62633 - self.next_buffer.replace(prev_buffer); - - self.next_buffer.as_ref().unwrap() + /// NOTE(unsafe): Memory safety and access ordering is not guaranteed + /// (see the HAL DMA docs). + pub fn with_buffer(&mut self, f: F) -> R + where + F: FnOnce(&mut SampleBuffer) -> R, + { + unsafe { self.transfer.next_dbm_transfer_with(|buf, _current| f(buf)) } } } } diff --git a/src/hardware/dac.rs b/src/hardware/dac.rs index 56f3033..91f8610 100644 --- a/src/hardware/dac.rs +++ b/src/hardware/dac.rs @@ -52,7 +52,7 @@ ///! served promptly after the transfer completes. use stm32h7xx_hal as hal; -use super::design_parameters::SAMPLE_BUFFER_SIZE; +use super::design_parameters::{SampleBuffer, SAMPLE_BUFFER_SIZE}; use super::timers; use hal::dma::{ @@ -66,8 +66,7 @@ use hal::dma::{ // processed). Note that the contents of AXI SRAM is uninitialized, so the buffer contents on // startup are undefined. The dimensions are `ADC_BUF[adc_index][ping_pong_index][sample_index]`. #[link_section = ".axisram.buffers"] -static mut DAC_BUF: [[[u16; SAMPLE_BUFFER_SIZE]; 3]; 2] = - [[[0; SAMPLE_BUFFER_SIZE]; 3]; 2]; +static mut DAC_BUF: [[SampleBuffer; 2]; 2] = [[[0; SAMPLE_BUFFER_SIZE]; 2]; 2]; /// Custom type for referencing DAC output codes. /// The internal integer is the raw code written to the DAC output register. @@ -137,13 +136,12 @@ macro_rules! dac_output { /// Represents data associated with DAC. pub struct $name { - next_buffer: Option<&'static mut [u16; SAMPLE_BUFFER_SIZE]>, // Note: SPI TX functionality may not be used from this structure to ensure safety with DMA. transfer: Transfer< hal::dma::dma::$data_stream, $spi, MemoryToPeripheral, - &'static mut [u16; SAMPLE_BUFFER_SIZE], + &'static mut SampleBuffer, hal::dma::DBTransfer, >, } @@ -198,33 +196,26 @@ macro_rules! dac_output { trigger_config, ); - Self { - transfer, - // Note(unsafe): This buffer is only used once and provided for the next DMA transfer. - next_buffer: unsafe { Some(&mut DAC_BUF[$index][2]) }, - } + Self { transfer } } pub fn start(&mut self) { self.transfer.start(|spi| spi.start_dma()); } - /// Acquire the next output buffer to populate it with DAC codes. - pub fn acquire_buffer(&mut self) -> &mut [u16; SAMPLE_BUFFER_SIZE] { - // Note: If a device hangs up, check that this conditional is passing correctly, as - // there is no time-out checks here in the interest of execution speed. - while !self.transfer.get_transfer_complete_flag() {} - - let next_buffer = self.next_buffer.take().unwrap(); - - // Start the next transfer. - let (prev_buffer, _, _) = - self.transfer.next_transfer(next_buffer).unwrap(); - - // .unwrap_none() https://github.com/rust-lang/rust/issues/62633 - self.next_buffer.replace(prev_buffer); - - self.next_buffer.as_mut().unwrap() + /// Wait for the transfer of the currently active buffer to complete, + /// then call a function on the now inactive buffer and acknowledge the + /// transfer complete flag. + /// + /// NOTE(unsafe): Memory safety and access ordering is not guaranteed + /// (see the HAL DMA docs). + pub fn with_buffer(&mut self, f: F) -> R + where + F: FnOnce(&mut SampleBuffer) -> R, + { + unsafe { + self.transfer.next_dbm_transfer_with(|buf, _current| f(buf)) + } } } }; diff --git a/src/hardware/design_parameters.rs b/src/hardware/design_parameters.rs index 9a4279b..d04cc75 100644 --- a/src/hardware/design_parameters.rs +++ b/src/hardware/design_parameters.rs @@ -50,5 +50,7 @@ pub const ADC_SAMPLE_TICKS: u16 = 1 << ADC_SAMPLE_TICKS_LOG2; pub const SAMPLE_BUFFER_SIZE_LOG2: u8 = 3; pub const SAMPLE_BUFFER_SIZE: usize = 1 << SAMPLE_BUFFER_SIZE_LOG2; +pub type SampleBuffer = [u16; SAMPLE_BUFFER_SIZE]; + // The MQTT broker IPv4 address pub const MQTT_BROKER: [u8; 4] = [10, 34, 16, 10]; From 2a9657f98c617b88eae2d4b8afc60842bbdfd45b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Mon, 24 May 2021 22:41:22 +0200 Subject: [PATCH 02/15] dual-iir: destructure resources --- src/bin/dual-iir.rs | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index dec0697..855e578 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -123,14 +123,24 @@ const APP: () = { /// the same time bounds, meeting one also means the other is also met. #[task(binds=DMA1_STR4, resources=[adcs, digital_inputs, dacs, iir_state, settings, telemetry], priority=2)] fn process(mut c: process::Context) { - let adc0 = &mut c.resources.adcs.0; - let adc1 = &mut c.resources.adcs.1; - let dac0 = &mut c.resources.dacs.0; - let dac1 = &mut c.resources.dacs.1; - let di = &c.resources.digital_inputs; - let settings = &c.resources.settings; - let iir_state = &mut c.resources.iir_state; - let telemetry = &mut c.resources.telemetry; + let process::Resources { + adcs: (ref mut adc0, ref mut adc1), + dacs: (ref mut dac0, ref mut dac1), + ref digital_inputs, + ref settings, + ref mut iir_state, + ref mut telemetry, + } = c.resources; + + let digital_inputs = [ + digital_inputs.0.is_high().unwrap(), + digital_inputs.1.is_high().unwrap(), + ]; + telemetry.digital_inputs = digital_inputs; + + let hold = + settings.force_hold || (digital_inputs[1] && settings.allow_hold); + adc0.with_buffer(|a0| { adc1.with_buffer(|a1| { dac0.with_buffer(|d0| { @@ -138,12 +148,6 @@ const APP: () = { let adc_samples = [a0, a1]; let dac_samples = [d0, d1]; - let digital_inputs = - [di.0.is_high().unwrap(), di.1.is_high().unwrap()]; - - let hold = settings.force_hold - || (digital_inputs[1] && settings.allow_hold); - // Preserve instruction and data ordering w.r.t. DMA flag access. fence(Ordering::SeqCst); @@ -181,8 +185,6 @@ const APP: () = { DacCode(dac_samples[1][0]), ]; - telemetry.digital_inputs = digital_inputs; - // Preserve instruction and data ordering w.r.t. DMA flag access. fence(Ordering::SeqCst); }) From c13859d486479872415d7a57315fd616aeabc099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 25 May 2021 12:05:19 +0200 Subject: [PATCH 03/15] dual-iir: add closure nesting helper macro --- src/bin/dual-iir.rs | 89 ++++++++++++++++++++++----------------------- src/lib.rs | 3 -- 2 files changed, 43 insertions(+), 49 deletions(-) diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index 855e578..3ce9817 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -52,6 +52,15 @@ impl Default for Settings { } } +macro_rules! flatten_closures { + ($fn:ident, $e:ident, $fun:block) => { + $e.$fn(|$e| $fun ) + }; + ($fn:ident, $e:ident, $($es:ident),+, $fun:block) => { + $e.$fn(|$e| flatten_closures!($fn, $($es),*, $fun)) + }; +} + #[rtic::app(device = stm32h7xx_hal::stm32, peripherals = true, monotonic = stabilizer::hardware::SystemTimer)] const APP: () = { struct Resources { @@ -141,55 +150,43 @@ const APP: () = { let hold = settings.force_hold || (digital_inputs[1] && settings.allow_hold); - adc0.with_buffer(|a0| { - adc1.with_buffer(|a1| { - dac0.with_buffer(|d0| { - dac1.with_buffer(|d1| { - let adc_samples = [a0, a1]; - let dac_samples = [d0, d1]; + flatten_closures!(with_buffer, adc0, adc1, dac0, dac1, { + let adc_samples = [adc0, adc1]; + let dac_samples = [dac0, dac1]; - // Preserve instruction and data ordering w.r.t. DMA flag access. - fence(Ordering::SeqCst); + // Preserve instruction and data ordering w.r.t. DMA flag access. + fence(Ordering::SeqCst); - for channel in 0..adc_samples.len() { - adc_samples[channel] - .iter() - .zip(dac_samples[channel].iter_mut()) - .map(|(ai, di)| { - let x = f32::from(*ai as i16); - let y = settings.iir_ch[channel] - .iter() - .zip(iir_state[channel].iter_mut()) - .fold(x, |yi, (ch, state)| { - ch.update(state, yi, hold) - }); - // Note(unsafe): The filter limits must ensure that - // the value is in range. - // The truncation introduces 1/2 LSB distortion. - let y: i16 = - unsafe { y.to_int_unchecked() }; - // Convert to DAC code - *di = DacCode::from(y).0; - }) - .last(); - } - - // Update telemetry measurements. - telemetry.adcs = [ - AdcCode(adc_samples[0][0]), - AdcCode(adc_samples[1][0]), - ]; - - telemetry.dacs = [ - DacCode(dac_samples[0][0]), - DacCode(dac_samples[1][0]), - ]; - - // Preserve instruction and data ordering w.r.t. DMA flag access. - fence(Ordering::SeqCst); + for channel in 0..adc_samples.len() { + adc_samples[channel] + .iter() + .zip(dac_samples[channel].iter_mut()) + .map(|(ai, di)| { + let x = f32::from(*ai as i16); + let y = settings.iir_ch[channel] + .iter() + .zip(iir_state[channel].iter_mut()) + .fold(x, |yi, (ch, state)| { + ch.update(state, yi, hold) + }); + // Note(unsafe): The filter limits must ensure that the value is in range. + // The truncation introduces 1/2 LSB distortion. + let y: i16 = unsafe { y.to_int_unchecked() }; + // Convert to DAC code + *di = DacCode::from(y).0; }) - }) - }) + .last(); + } + + // Update telemetry measurements. + telemetry.adcs = + [AdcCode(adc_samples[0][0]), AdcCode(adc_samples[1][0])]; + + telemetry.dacs = + [DacCode(dac_samples[0][0]), DacCode(dac_samples[1][0])]; + + // Preserve instruction and data ordering w.r.t. DMA flag access. + fence(Ordering::SeqCst); }); } diff --git a/src/lib.rs b/src/lib.rs index 975b08f..85964a7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,8 +1,5 @@ #![no_std] #![cfg_attr(feature = "nightly", feature(core_intrinsics))] -#[macro_use] -extern crate log; - pub mod hardware; pub mod net; From c5a2704c41ec5f7089c705effe5af14d1a064cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 1 Jun 2021 13:11:16 +0200 Subject: [PATCH 04/15] dma: implement overflow checking --- Cargo.lock | 2 +- Cargo.toml | 3 ++- src/bin/dual-iir.rs | 4 ++-- src/hardware/adc.rs | 4 ++-- src/hardware/dac.rs | 4 ++-- src/hardware/pounder/dds_output.rs | 4 +++- src/net/miniconf_client.rs | 1 + 7 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9fbbe91..de1f22f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -810,7 +810,7 @@ dependencies = [ [[package]] name = "stm32h7xx-hal" version = "0.9.0" -source = "git+https://github.com/quartiq/stm32h7xx-hal.git?rev=cca4ecc#cca4ecc3e0cc8cb2f7a9652c4099d50b44977493" +source = "git+https://github.com/quartiq/stm32h7xx-hal.git?rev=b0b8a93#b0b8a930b2c3bc5fcebc2e905b4c5e13360111a5" dependencies = [ "bare-metal 1.0.0", "cast", diff --git a/Cargo.toml b/Cargo.toml index 93eaa9a..99b390e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,11 +52,12 @@ mcp23017 = "1.0" git = "https://github.com/quartiq/rtt-logger.git" rev = "70b0eb5" +# fast double buffered DMA without poisoning and buffer swapping [dependencies.stm32h7xx-hal] features = ["stm32h743v", "rt", "unproven", "ethernet", "quadspi"] # version = "0.9.0" git = "https://github.com/quartiq/stm32h7xx-hal.git" -rev = "cca4ecc" +rev = "b0b8a93" # link.x section start/end [patch.crates-io.cortex-m-rt] diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index a45d603..ab965ee 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -54,10 +54,10 @@ impl Default for Settings { macro_rules! flatten_closures { ($fn:ident, $e:ident, $fun:block) => { - $e.$fn(|$e| $fun ) + $e.$fn(|$e| $fun ).unwrap() }; ($fn:ident, $e:ident, $($es:ident),+, $fun:block) => { - $e.$fn(|$e| flatten_closures!($fn, $($es),*, $fun)) + $e.$fn(|$e| flatten_closures!($fn, $($es),*, $fun)).unwrap() }; } diff --git a/src/hardware/adc.rs b/src/hardware/adc.rs index 101d762..8919479 100644 --- a/src/hardware/adc.rs +++ b/src/hardware/adc.rs @@ -74,7 +74,7 @@ use hal::dma::{ config::Priority, dma::{DMAReq, DmaConfig}, traits::TargetAddress, - MemoryToPeripheral, PeripheralToMemory, Transfer, + DMAError, MemoryToPeripheral, PeripheralToMemory, Transfer, }; /// A type representing an ADC sample. @@ -359,7 +359,7 @@ macro_rules! adc_input { /// /// NOTE(unsafe): Memory safety and access ordering is not guaranteed /// (see the HAL DMA docs). - pub fn with_buffer(&mut self, f: F) -> R + pub fn with_buffer(&mut self, f: F) -> Result where F: FnOnce(&mut SampleBuffer) -> R, { diff --git a/src/hardware/dac.rs b/src/hardware/dac.rs index 91f8610..013c43a 100644 --- a/src/hardware/dac.rs +++ b/src/hardware/dac.rs @@ -58,7 +58,7 @@ use super::timers; use hal::dma::{ dma::{DMAReq, DmaConfig}, traits::TargetAddress, - MemoryToPeripheral, Transfer, + DMAError, MemoryToPeripheral, Transfer, }; // The following global buffers are used for the DAC code DMA transfers. Two buffers are used for @@ -209,7 +209,7 @@ macro_rules! dac_output { /// /// NOTE(unsafe): Memory safety and access ordering is not guaranteed /// (see the HAL DMA docs). - pub fn with_buffer(&mut self, f: F) -> R + pub fn with_buffer(&mut self, f: F) -> Result where F: FnOnce(&mut SampleBuffer) -> R, { diff --git a/src/hardware/pounder/dds_output.rs b/src/hardware/pounder/dds_output.rs index e755482..bf7acc0 100644 --- a/src/hardware/pounder/dds_output.rs +++ b/src/hardware/pounder/dds_output.rs @@ -52,9 +52,11 @@ ///! compile-time-known register update sequence needed for the application, the serialization ///! process can be done once and then register values can be written into a pre-computed serialized ///! buffer to avoid the software overhead of much of the serialization process. +use log::warn; +use stm32h7xx_hal as hal; + use super::{hrtimer::HighResTimerE, QspiInterface}; use ad9959::{Channel, DdsConfig, ProfileSerializer}; -use stm32h7xx_hal as hal; /// The DDS profile update stream. pub struct DdsOutput { diff --git a/src/net/miniconf_client.rs b/src/net/miniconf_client.rs index cc838f6..e5f328a 100644 --- a/src/net/miniconf_client.rs +++ b/src/net/miniconf_client.rs @@ -11,6 +11,7 @@ ///! Respones to settings updates are sent without quality-of-service guarantees, so there's no ///! guarantee that the requestee will be informed that settings have been applied. use heapless::String; +use log::info; use super::{MqttMessage, NetworkReference, SettingsResponse, UpdateState}; use crate::hardware::design_parameters::MQTT_BROKER; From b90f4ad18563f1577d7c383be255e9ebfa3b4277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 1 Jun 2021 13:17:40 +0200 Subject: [PATCH 05/15] lockin: port to fast double buffered DMA --- src/bin/lockin.rs | 111 +++++++++++++++++++++++++--------------------- 1 file changed, 60 insertions(+), 51 deletions(-) diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 4864760..54ce37b 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -78,6 +78,15 @@ impl Default for Settings { } } +macro_rules! flatten_closures { + ($fn:ident, $e:ident, $fun:block) => { + $e.$fn(|$e| $fun ).unwrap() + }; + ($fn:ident, $e:ident, $($es:ident),+, $fun:block) => { + $e.$fn(|$e| flatten_closures!($fn, $($es),*, $fun)).unwrap() + }; +} + #[rtic::app(device = stm32h7xx_hal::stm32, peripherals = true, monotonic = stabilizer::hardware::SystemTimer)] const APP: () = { struct Resources { @@ -159,26 +168,22 @@ const APP: () = { #[task(binds=DMA1_STR4, resources=[adcs, dacs, lockin, timestamper, pll, settings, telemetry], priority=2)] #[inline(never)] #[link_section = ".itcm.process"] - fn process(c: process::Context) { - let adc_samples = [ - c.resources.adcs.0.acquire_buffer(), - c.resources.adcs.1.acquire_buffer(), - ]; - - let mut dac_samples = [ - c.resources.dacs.0.acquire_buffer(), - c.resources.dacs.1.acquire_buffer(), - ]; - - let lockin = c.resources.lockin; - let settings = c.resources.settings; + fn process(mut c: process::Context) { + let process::Resources { + adcs: (ref mut adc0, ref mut adc1), + dacs: (ref mut dac0, ref mut dac1), + ref settings, + ref mut telemetry, + ref mut lockin, + ref mut pll, + ref mut timestamper, + } = c.resources; let (reference_phase, reference_frequency) = match settings.lockin_mode { LockinMode::External => { - let timestamp = - c.resources.timestamper.latest_timestamp().unwrap_or(None); // Ignore data from timer capture overflows. - let (pll_phase, pll_frequency) = c.resources.pll.update( + let timestamp = timestamper.latest_timestamp().unwrap_or(None); // Ignore data from timer capture overflows. + let (pll_phase, pll_frequency) = pll.update( timestamp.map(|t| t as i32), settings.pll_tc[0], settings.pll_tc[1], @@ -205,45 +210,49 @@ const APP: () = { reference_phase.wrapping_mul(settings.lockin_harmonic), ); - let output: Complex = adc_samples[0] - .iter() - // Zip in the LO phase. - .zip(Accu::new(sample_phase, sample_frequency)) - // Convert to signed, MSB align the ADC sample, update the Lockin (demodulate, filter) - .map(|(&sample, phase)| { - let s = (sample as i16 as i32) << 16; - lockin.update(s, phase, settings.lockin_tc) - }) - // Decimate - .last() - .unwrap() - * 2; // Full scale assuming the 2f component is gone. + flatten_closures!(with_buffer, adc0, adc1, dac0, dac1, { + let adc_samples = [adc0, adc1]; + let mut dac_samples = [dac0, dac1]; - // Convert to DAC data. - for (channel, samples) in dac_samples.iter_mut().enumerate() { - for (i, sample) in samples.iter_mut().enumerate() { - let value = match settings.output_conf[channel] { - Conf::Magnitude => output.abs_sqr() as i32 >> 16, - Conf::Phase => output.arg() >> 16, - Conf::LogPower => (output.log2() << 24) as i32 >> 16, - Conf::ReferenceFrequency => { - reference_frequency as i32 >> 16 - } - Conf::InPhase => output.re >> 16, - Conf::Quadrature => output.im >> 16, - Conf::Modulation => DAC_SEQUENCE[i] as i32, - }; + let output: Complex = adc_samples[0] + .iter() + // Zip in the LO phase. + .zip(Accu::new(sample_phase, sample_frequency)) + // Convert to signed, MSB align the ADC sample, update the Lockin (demodulate, filter) + .map(|(&sample, phase)| { + let s = (sample as i16 as i32) << 16; + lockin.update(s, phase, settings.lockin_tc) + }) + // Decimate + .last() + .unwrap() + * 2; // Full scale assuming the 2f component is gone. - *sample = DacCode::from(value as i16).0; + // Convert to DAC data. + for (channel, samples) in dac_samples.iter_mut().enumerate() { + for (i, sample) in samples.iter_mut().enumerate() { + let value = match settings.output_conf[channel] { + Conf::Magnitude => output.abs_sqr() as i32 >> 16, + Conf::Phase => output.arg() >> 16, + Conf::LogPower => (output.log2() << 24) as i32 >> 16, + Conf::ReferenceFrequency => { + reference_frequency as i32 >> 16 + } + Conf::InPhase => output.re >> 16, + Conf::Quadrature => output.im >> 16, + Conf::Modulation => DAC_SEQUENCE[i] as i32, + }; + + *sample = DacCode::from(value as i16).0; + } } - } + // Update telemetry measurements. + telemetry.adcs = + [AdcCode(adc_samples[0][0]), AdcCode(adc_samples[1][0])]; - // Update telemetry measurements. - c.resources.telemetry.adcs = - [AdcCode(adc_samples[0][0]), AdcCode(adc_samples[1][0])]; - - c.resources.telemetry.dacs = - [DacCode(dac_samples[0][0]), DacCode(dac_samples[1][0])]; + telemetry.dacs = + [DacCode(dac_samples[0][0]), DacCode(dac_samples[1][0])]; + }); } #[idle(resources=[network], spawn=[settings_update])] From f8fa297b20f0bb4f1948972ce13e926f1ddae650 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 1 Jun 2021 14:49:51 +0200 Subject: [PATCH 06/15] lockin: dma fence --- src/bin/lockin.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 54ce37b..dd20278 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -2,6 +2,8 @@ #![no_std] #![no_main] +use core::sync::atomic::{fence, Ordering}; + use embedded_hal::digital::v2::InputPin; use serde::Deserialize; @@ -214,6 +216,9 @@ const APP: () = { let adc_samples = [adc0, adc1]; let mut dac_samples = [dac0, dac1]; + // Preserve instruction and data ordering w.r.t. DMA flag access. + fence(Ordering::SeqCst); + let output: Complex = adc_samples[0] .iter() // Zip in the LO phase. @@ -252,6 +257,9 @@ const APP: () = { telemetry.dacs = [DacCode(dac_samples[0][0]), DacCode(dac_samples[1][0])]; + + // Preserve instruction and data ordering w.r.t. DMA flag access. + fence(Ordering::SeqCst); }); } From 26b261364f7200d3aabe4fcc7689d166b966ef69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 1 Jun 2021 14:19:24 +0000 Subject: [PATCH 07/15] pounder timestmper: don't use DMA * One sample per batch is typical and sufficient. * DMA has more overhead than direct read for one sample. --- src/hardware/configuration.rs | 6 --- src/hardware/pounder/timestamp.rs | 74 +++++-------------------------- 2 files changed, 10 insertions(+), 70 deletions(-) diff --git a/src/hardware/configuration.rs b/src/hardware/configuration.rs index fca38e1..7d8c92b 100644 --- a/src/hardware/configuration.rs +++ b/src/hardware/configuration.rs @@ -233,11 +233,6 @@ pub fn setup( let dma_streams = hal::dma::dma::StreamsTuple::new(device.DMA1, ccdr.peripheral.DMA1); - // Early, before the DMA1 peripherals (#272) - #[cfg(feature = "pounder_v1_1")] - let dma2_streams = - hal::dma::dma::StreamsTuple::new(device.DMA2, ccdr.peripheral.DMA2); - // Configure timer 2 to trigger conversions for the ADC let mut sampling_timer = { // The timer frequency is manually adjusted below, so the 1KHz setting here is a @@ -946,7 +941,6 @@ pub fn setup( pounder::timestamp::Timestamper::new( timestamp_timer, - dma2_streams.0, tim8_channels.ch1, &mut sampling_timer, etr_pin, diff --git a/src/hardware/pounder/timestamp.rs b/src/hardware/pounder/timestamp.rs index 0c06192..1e8f862 100644 --- a/src/hardware/pounder/timestamp.rs +++ b/src/hardware/pounder/timestamp.rs @@ -24,27 +24,12 @@ ///! schedule. use stm32h7xx_hal as hal; -use hal::dma::{dma::DmaConfig, PeripheralToMemory, Transfer}; - -use crate::hardware::{design_parameters::SAMPLE_BUFFER_SIZE, timers}; - -// Three buffers are required for double buffered mode - 2 are owned by the DMA stream and 1 is the -// working data provided to the application. These buffers must exist in a DMA-accessible memory -// region. Note that AXISRAM is not initialized on boot, so their initial contents are undefined. -#[link_section = ".axisram.buffers"] -static mut BUF: [[u16; SAMPLE_BUFFER_SIZE]; 3] = [[0; SAMPLE_BUFFER_SIZE]; 3]; +use crate::hardware::timers; /// Software unit to timestamp stabilizer ADC samples using an external pounder reference clock. pub struct Timestamper { - next_buffer: Option<&'static mut [u16; SAMPLE_BUFFER_SIZE]>, timer: timers::PounderTimestampTimer, - transfer: Transfer< - hal::dma::dma::Stream0, - timers::tim8::Channel1InputCapture, - PeripheralToMemory, - &'static mut [u16; SAMPLE_BUFFER_SIZE], - hal::dma::DBTransfer, - >, + capture_channel: timers::tim8::Channel1InputCapture, } impl Timestamper { @@ -65,18 +50,12 @@ impl Timestamper { /// The new pounder timestamper in an operational state. pub fn new( mut timestamp_timer: timers::PounderTimestampTimer, - stream: hal::dma::dma::Stream0, capture_channel: timers::tim8::Channel1, sampling_timer: &mut timers::SamplingTimer, _clock_input: hal::gpio::gpioa::PA0< hal::gpio::Alternate, >, ) -> Self { - let config = DmaConfig::default() - .memory_increment(true) - .circular_buffer(true) - .double_buffer(true); - // The sampling timer should generate a trigger output when CH1 comparison occurs. sampling_timer.generate_trigger(timers::TriggerGenerator::ComparePulse); @@ -87,62 +66,29 @@ impl Timestamper { // The capture channel should capture whenever the trigger input occurs. let input_capture = capture_channel .into_input_capture(timers::tim8::CaptureSource1::TRC); - input_capture.listen_dma(); - - // The data transfer is always a transfer of data from the peripheral to a RAM buffer. - let data_transfer: Transfer<_, _, PeripheralToMemory, _, _> = - Transfer::init( - stream, - input_capture, - // Note(unsafe): BUF[0] and BUF[1] are "owned" by this peripheral. - // They shall not be used anywhere else in the module. - unsafe { &mut BUF[0] }, - unsafe { Some(&mut BUF[1]) }, - config, - ); Self { timer: timestamp_timer, - transfer: data_transfer, - - // Note(unsafe): BUF[2] is "owned" by this peripheral. It shall not be used anywhere - // else in the module. - next_buffer: unsafe { Some(&mut BUF[2]) }, + capture_channel: input_capture, } } - /// Start the DMA transfer for collecting timestamps. - #[allow(dead_code)] + /// Start collecting timestamps. pub fn start(&mut self) { - self.transfer - .start(|capture_channel| capture_channel.enable()); + self.capture_channel.enable(); } /// Update the period of the underlying timestamp timer. - #[allow(dead_code)] pub fn update_period(&mut self, period: u16) { self.timer.set_period_ticks(period); } - /// Obtain a buffer filled with timestamps. + /// Obtain a timestamp. /// /// # Returns - /// A reference to the underlying buffer that has been filled with timestamps. - #[allow(dead_code)] - pub fn acquire_buffer(&mut self) -> &[u16; SAMPLE_BUFFER_SIZE] { - // Wait for the transfer to fully complete before continuing. - // Note: If a device hangs up, check that this conditional is passing correctly, as there is - // no time-out checks here in the interest of execution speed. - while !self.transfer.get_transfer_complete_flag() {} - - let next_buffer = self.next_buffer.take().unwrap(); - - // Start the next transfer. - let (prev_buffer, _, _) = - self.transfer.next_transfer(next_buffer).unwrap(); - - self.next_buffer.replace(prev_buffer); // .unwrap_none() https://github.com/rust-lang/rust/issues/62633 - - self.next_buffer.as_ref().unwrap() + /// A `Result` potentially indicating capture overflow and containing a `Option` of a captured + /// timestamp. + pub fn latest_timestamp(&mut self) -> Result, Option> { + self.capture_channel.latest_capture() } } From d97ee3f0c427b4f7b9c5af458d15fb9be7dc6653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 1 Jun 2021 16:57:51 +0200 Subject: [PATCH 08/15] Revert "pounder timestmper: don't use DMA" This reverts commit 26b261364f7200d3aabe4fcc7689d166b966ef69. First needs to reduce capture rate to batch interval. Otherwise it's jittery due to polling alignment. --- src/hardware/configuration.rs | 6 +++ src/hardware/pounder/timestamp.rs | 74 ++++++++++++++++++++++++++----- 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/hardware/configuration.rs b/src/hardware/configuration.rs index 7d8c92b..fca38e1 100644 --- a/src/hardware/configuration.rs +++ b/src/hardware/configuration.rs @@ -233,6 +233,11 @@ pub fn setup( let dma_streams = hal::dma::dma::StreamsTuple::new(device.DMA1, ccdr.peripheral.DMA1); + // Early, before the DMA1 peripherals (#272) + #[cfg(feature = "pounder_v1_1")] + let dma2_streams = + hal::dma::dma::StreamsTuple::new(device.DMA2, ccdr.peripheral.DMA2); + // Configure timer 2 to trigger conversions for the ADC let mut sampling_timer = { // The timer frequency is manually adjusted below, so the 1KHz setting here is a @@ -941,6 +946,7 @@ pub fn setup( pounder::timestamp::Timestamper::new( timestamp_timer, + dma2_streams.0, tim8_channels.ch1, &mut sampling_timer, etr_pin, diff --git a/src/hardware/pounder/timestamp.rs b/src/hardware/pounder/timestamp.rs index 1e8f862..0c06192 100644 --- a/src/hardware/pounder/timestamp.rs +++ b/src/hardware/pounder/timestamp.rs @@ -24,12 +24,27 @@ ///! schedule. use stm32h7xx_hal as hal; -use crate::hardware::timers; +use hal::dma::{dma::DmaConfig, PeripheralToMemory, Transfer}; + +use crate::hardware::{design_parameters::SAMPLE_BUFFER_SIZE, timers}; + +// Three buffers are required for double buffered mode - 2 are owned by the DMA stream and 1 is the +// working data provided to the application. These buffers must exist in a DMA-accessible memory +// region. Note that AXISRAM is not initialized on boot, so their initial contents are undefined. +#[link_section = ".axisram.buffers"] +static mut BUF: [[u16; SAMPLE_BUFFER_SIZE]; 3] = [[0; SAMPLE_BUFFER_SIZE]; 3]; /// Software unit to timestamp stabilizer ADC samples using an external pounder reference clock. pub struct Timestamper { + next_buffer: Option<&'static mut [u16; SAMPLE_BUFFER_SIZE]>, timer: timers::PounderTimestampTimer, - capture_channel: timers::tim8::Channel1InputCapture, + transfer: Transfer< + hal::dma::dma::Stream0, + timers::tim8::Channel1InputCapture, + PeripheralToMemory, + &'static mut [u16; SAMPLE_BUFFER_SIZE], + hal::dma::DBTransfer, + >, } impl Timestamper { @@ -50,12 +65,18 @@ impl Timestamper { /// The new pounder timestamper in an operational state. pub fn new( mut timestamp_timer: timers::PounderTimestampTimer, + stream: hal::dma::dma::Stream0, capture_channel: timers::tim8::Channel1, sampling_timer: &mut timers::SamplingTimer, _clock_input: hal::gpio::gpioa::PA0< hal::gpio::Alternate, >, ) -> Self { + let config = DmaConfig::default() + .memory_increment(true) + .circular_buffer(true) + .double_buffer(true); + // The sampling timer should generate a trigger output when CH1 comparison occurs. sampling_timer.generate_trigger(timers::TriggerGenerator::ComparePulse); @@ -66,29 +87,62 @@ impl Timestamper { // The capture channel should capture whenever the trigger input occurs. let input_capture = capture_channel .into_input_capture(timers::tim8::CaptureSource1::TRC); + input_capture.listen_dma(); + + // The data transfer is always a transfer of data from the peripheral to a RAM buffer. + let data_transfer: Transfer<_, _, PeripheralToMemory, _, _> = + Transfer::init( + stream, + input_capture, + // Note(unsafe): BUF[0] and BUF[1] are "owned" by this peripheral. + // They shall not be used anywhere else in the module. + unsafe { &mut BUF[0] }, + unsafe { Some(&mut BUF[1]) }, + config, + ); Self { timer: timestamp_timer, - capture_channel: input_capture, + transfer: data_transfer, + + // Note(unsafe): BUF[2] is "owned" by this peripheral. It shall not be used anywhere + // else in the module. + next_buffer: unsafe { Some(&mut BUF[2]) }, } } - /// Start collecting timestamps. + /// Start the DMA transfer for collecting timestamps. + #[allow(dead_code)] pub fn start(&mut self) { - self.capture_channel.enable(); + self.transfer + .start(|capture_channel| capture_channel.enable()); } /// Update the period of the underlying timestamp timer. + #[allow(dead_code)] pub fn update_period(&mut self, period: u16) { self.timer.set_period_ticks(period); } - /// Obtain a timestamp. + /// Obtain a buffer filled with timestamps. /// /// # Returns - /// A `Result` potentially indicating capture overflow and containing a `Option` of a captured - /// timestamp. - pub fn latest_timestamp(&mut self) -> Result, Option> { - self.capture_channel.latest_capture() + /// A reference to the underlying buffer that has been filled with timestamps. + #[allow(dead_code)] + pub fn acquire_buffer(&mut self) -> &[u16; SAMPLE_BUFFER_SIZE] { + // Wait for the transfer to fully complete before continuing. + // Note: If a device hangs up, check that this conditional is passing correctly, as there is + // no time-out checks here in the interest of execution speed. + while !self.transfer.get_transfer_complete_flag() {} + + let next_buffer = self.next_buffer.take().unwrap(); + + // Start the next transfer. + let (prev_buffer, _, _) = + self.transfer.next_transfer(next_buffer).unwrap(); + + self.next_buffer.replace(prev_buffer); // .unwrap_none() https://github.com/rust-lang/rust/issues/62633 + + self.next_buffer.as_ref().unwrap() } } From 18b6e99b10c730a861c1c52a6399fe12efa3655f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 1 Jun 2021 17:32:06 +0200 Subject: [PATCH 09/15] fix a few clippy lints on files that are touched --- src/hardware/adc.rs | 1 + src/hardware/dac.rs | 3 ++- src/net/miniconf_client.rs | 6 +++--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/hardware/adc.rs b/src/hardware/adc.rs index 8919479..26752f7 100644 --- a/src/hardware/adc.rs +++ b/src/hardware/adc.rs @@ -81,6 +81,7 @@ use hal::dma::{ #[derive(Copy, Clone)] pub struct AdcCode(pub u16); +#[allow(clippy::from_over_into)] impl Into for AdcCode { /// Convert raw ADC codes to/from voltage levels. /// diff --git a/src/hardware/dac.rs b/src/hardware/dac.rs index 013c43a..9b42e37 100644 --- a/src/hardware/dac.rs +++ b/src/hardware/dac.rs @@ -73,6 +73,7 @@ static mut DAC_BUF: [[SampleBuffer; 2]; 2] = [[[0; SAMPLE_BUFFER_SIZE]; 2]; 2]; #[derive(Copy, Clone)] pub struct DacCode(pub u16); +#[allow(clippy::from_over_into)] impl Into for DacCode { fn into(self) -> f32 { // The DAC output range in bipolar mode (including the external output op-amp) is +/- 4.096 @@ -104,7 +105,7 @@ macro_rules! dac_output { _channel: timers::tim2::$trigger_channel, spi: hal::spi::Spi, ) -> Self { - Self { _channel, spi } + Self { spi, _channel } } /// Start the SPI and begin operating in a DMA-driven transfer mode. diff --git a/src/net/miniconf_client.rs b/src/net/miniconf_client.rs index e5f328a..f280318 100644 --- a/src/net/miniconf_client.rs +++ b/src/net/miniconf_client.rs @@ -103,7 +103,7 @@ where let path = match topic.strip_prefix(prefix) { // For paths, we do not want to include the leading slash. Some(path) => { - if path.len() > 0 { + if !path.is_empty() { &path[1..] } else { path @@ -117,9 +117,9 @@ where let message: SettingsResponse = settings .string_set(path.split('/').peekable(), message) - .and_then(|_| { + .map(|_| { update = true; - Ok(()) + () }) .into(); From 3b737836350a5464a5cb6367dd30144e4cf92510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 1 Jun 2021 17:45:14 +0200 Subject: [PATCH 10/15] clippy recursion --- src/net/miniconf_client.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/net/miniconf_client.rs b/src/net/miniconf_client.rs index f280318..4eaab07 100644 --- a/src/net/miniconf_client.rs +++ b/src/net/miniconf_client.rs @@ -119,7 +119,6 @@ where .string_set(path.split('/').peekable(), message) .map(|_| { update = true; - () }) .into(); From 93081c25c2978077a6fcb677b107b7adf8974f2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Tue, 1 Jun 2021 17:55:42 +0200 Subject: [PATCH 11/15] refactor flatten_closures --- src/bin/dual-iir.rs | 11 +---------- src/bin/lockin.rs | 13 ++----------- src/lib.rs | 13 +++++++++++++ 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index ab965ee..da5af3d 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -4,7 +4,7 @@ use core::sync::atomic::{fence, Ordering}; -use stabilizer::{hardware, net}; +use stabilizer::{flatten_closures, hardware, net}; use miniconf::Miniconf; use serde::Deserialize; @@ -52,15 +52,6 @@ impl Default for Settings { } } -macro_rules! flatten_closures { - ($fn:ident, $e:ident, $fun:block) => { - $e.$fn(|$e| $fun ).unwrap() - }; - ($fn:ident, $e:ident, $($es:ident),+, $fun:block) => { - $e.$fn(|$e| flatten_closures!($fn, $($es),*, $fun)).unwrap() - }; -} - #[rtic::app(device = stm32h7xx_hal::stm32, peripherals = true, monotonic = stabilizer::hardware::SystemTimer)] const APP: () = { struct Resources { diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index dd20278..019ef01 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -10,9 +10,9 @@ use serde::Deserialize; use dsp::{Accu, Complex, ComplexExt, Lockin, RPLL}; -use stabilizer::net; +use stabilizer::{flatten_closures, hardware, net}; -use stabilizer::hardware::{ +use hardware::{ design_parameters, setup, Adc0Input, Adc1Input, AdcCode, AfeGain, Dac0Output, Dac1Output, DacCode, DigitalInput0, DigitalInput1, InputStamper, SystemTimer, AFE0, AFE1, @@ -80,15 +80,6 @@ impl Default for Settings { } } -macro_rules! flatten_closures { - ($fn:ident, $e:ident, $fun:block) => { - $e.$fn(|$e| $fun ).unwrap() - }; - ($fn:ident, $e:ident, $($es:ident),+, $fun:block) => { - $e.$fn(|$e| flatten_closures!($fn, $($es),*, $fun)).unwrap() - }; -} - #[rtic::app(device = stm32h7xx_hal::stm32, peripherals = true, monotonic = stabilizer::hardware::SystemTimer)] const APP: () = { struct Resources { diff --git a/src/lib.rs b/src/lib.rs index 85964a7..e1fdecf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,3 +3,16 @@ pub mod hardware; pub mod net; + +/// Macro to reduce rightward drift when calling the same closure-based API +/// on multiple structs simultaneously, e.g. when accessing DMA buffers. +/// This could be improved a bit using the tuple-based style from `mutex-trait`. +#[macro_export] +macro_rules! flatten_closures { + ($fn:ident, $e:ident, $fun:block) => { + $e.$fn(|$e| $fun ).unwrap() + }; + ($fn:ident, $e:ident, $($es:ident),+, $fun:block) => { + $e.$fn(|$e| flatten_closures!($fn, $($es),*, $fun)).unwrap() + }; +} From 35536c062347a0741aecaedefe241a844a8b6018 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Thu, 3 Jun 2021 10:31:11 +0200 Subject: [PATCH 12/15] Revert "Revert "pounder timestmper: don't use DMA"" This reverts commit d97ee3f0c427b4f7b9c5af458d15fb9be7dc6653. --- src/hardware/configuration.rs | 6 --- src/hardware/pounder/timestamp.rs | 74 +++++-------------------------- 2 files changed, 10 insertions(+), 70 deletions(-) diff --git a/src/hardware/configuration.rs b/src/hardware/configuration.rs index fca38e1..7d8c92b 100644 --- a/src/hardware/configuration.rs +++ b/src/hardware/configuration.rs @@ -233,11 +233,6 @@ pub fn setup( let dma_streams = hal::dma::dma::StreamsTuple::new(device.DMA1, ccdr.peripheral.DMA1); - // Early, before the DMA1 peripherals (#272) - #[cfg(feature = "pounder_v1_1")] - let dma2_streams = - hal::dma::dma::StreamsTuple::new(device.DMA2, ccdr.peripheral.DMA2); - // Configure timer 2 to trigger conversions for the ADC let mut sampling_timer = { // The timer frequency is manually adjusted below, so the 1KHz setting here is a @@ -946,7 +941,6 @@ pub fn setup( pounder::timestamp::Timestamper::new( timestamp_timer, - dma2_streams.0, tim8_channels.ch1, &mut sampling_timer, etr_pin, diff --git a/src/hardware/pounder/timestamp.rs b/src/hardware/pounder/timestamp.rs index 0c06192..1e8f862 100644 --- a/src/hardware/pounder/timestamp.rs +++ b/src/hardware/pounder/timestamp.rs @@ -24,27 +24,12 @@ ///! schedule. use stm32h7xx_hal as hal; -use hal::dma::{dma::DmaConfig, PeripheralToMemory, Transfer}; - -use crate::hardware::{design_parameters::SAMPLE_BUFFER_SIZE, timers}; - -// Three buffers are required for double buffered mode - 2 are owned by the DMA stream and 1 is the -// working data provided to the application. These buffers must exist in a DMA-accessible memory -// region. Note that AXISRAM is not initialized on boot, so their initial contents are undefined. -#[link_section = ".axisram.buffers"] -static mut BUF: [[u16; SAMPLE_BUFFER_SIZE]; 3] = [[0; SAMPLE_BUFFER_SIZE]; 3]; +use crate::hardware::timers; /// Software unit to timestamp stabilizer ADC samples using an external pounder reference clock. pub struct Timestamper { - next_buffer: Option<&'static mut [u16; SAMPLE_BUFFER_SIZE]>, timer: timers::PounderTimestampTimer, - transfer: Transfer< - hal::dma::dma::Stream0, - timers::tim8::Channel1InputCapture, - PeripheralToMemory, - &'static mut [u16; SAMPLE_BUFFER_SIZE], - hal::dma::DBTransfer, - >, + capture_channel: timers::tim8::Channel1InputCapture, } impl Timestamper { @@ -65,18 +50,12 @@ impl Timestamper { /// The new pounder timestamper in an operational state. pub fn new( mut timestamp_timer: timers::PounderTimestampTimer, - stream: hal::dma::dma::Stream0, capture_channel: timers::tim8::Channel1, sampling_timer: &mut timers::SamplingTimer, _clock_input: hal::gpio::gpioa::PA0< hal::gpio::Alternate, >, ) -> Self { - let config = DmaConfig::default() - .memory_increment(true) - .circular_buffer(true) - .double_buffer(true); - // The sampling timer should generate a trigger output when CH1 comparison occurs. sampling_timer.generate_trigger(timers::TriggerGenerator::ComparePulse); @@ -87,62 +66,29 @@ impl Timestamper { // The capture channel should capture whenever the trigger input occurs. let input_capture = capture_channel .into_input_capture(timers::tim8::CaptureSource1::TRC); - input_capture.listen_dma(); - - // The data transfer is always a transfer of data from the peripheral to a RAM buffer. - let data_transfer: Transfer<_, _, PeripheralToMemory, _, _> = - Transfer::init( - stream, - input_capture, - // Note(unsafe): BUF[0] and BUF[1] are "owned" by this peripheral. - // They shall not be used anywhere else in the module. - unsafe { &mut BUF[0] }, - unsafe { Some(&mut BUF[1]) }, - config, - ); Self { timer: timestamp_timer, - transfer: data_transfer, - - // Note(unsafe): BUF[2] is "owned" by this peripheral. It shall not be used anywhere - // else in the module. - next_buffer: unsafe { Some(&mut BUF[2]) }, + capture_channel: input_capture, } } - /// Start the DMA transfer for collecting timestamps. - #[allow(dead_code)] + /// Start collecting timestamps. pub fn start(&mut self) { - self.transfer - .start(|capture_channel| capture_channel.enable()); + self.capture_channel.enable(); } /// Update the period of the underlying timestamp timer. - #[allow(dead_code)] pub fn update_period(&mut self, period: u16) { self.timer.set_period_ticks(period); } - /// Obtain a buffer filled with timestamps. + /// Obtain a timestamp. /// /// # Returns - /// A reference to the underlying buffer that has been filled with timestamps. - #[allow(dead_code)] - pub fn acquire_buffer(&mut self) -> &[u16; SAMPLE_BUFFER_SIZE] { - // Wait for the transfer to fully complete before continuing. - // Note: If a device hangs up, check that this conditional is passing correctly, as there is - // no time-out checks here in the interest of execution speed. - while !self.transfer.get_transfer_complete_flag() {} - - let next_buffer = self.next_buffer.take().unwrap(); - - // Start the next transfer. - let (prev_buffer, _, _) = - self.transfer.next_transfer(next_buffer).unwrap(); - - self.next_buffer.replace(prev_buffer); // .unwrap_none() https://github.com/rust-lang/rust/issues/62633 - - self.next_buffer.as_ref().unwrap() + /// A `Result` potentially indicating capture overflow and containing a `Option` of a captured + /// timestamp. + pub fn latest_timestamp(&mut self) -> Result, Option> { + self.capture_channel.latest_capture() } } From 2ba9e9c2f71f6ee781eb379865c71140d5f886cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Thu, 3 Jun 2021 07:57:18 +0000 Subject: [PATCH 13/15] pounder_timestamper: use input capture prescaler --- src/hardware/pounder/timestamp.rs | 14 +++++++++++--- src/hardware/timers.rs | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/hardware/pounder/timestamp.rs b/src/hardware/pounder/timestamp.rs index 1e8f862..a527f7f 100644 --- a/src/hardware/pounder/timestamp.rs +++ b/src/hardware/pounder/timestamp.rs @@ -22,10 +22,10 @@ ///! mode. As soon as the DMA transfer completes, the hardware automatically swaps over to a second ///! buffer to continue capturing. This alleviates timing sensitivities of the DMA transfer ///! schedule. +use crate::hardware::{design_parameters, timers}; +use core::convert::TryFrom; use stm32h7xx_hal as hal; -use crate::hardware::timers; - /// Software unit to timestamp stabilizer ADC samples using an external pounder reference clock. pub struct Timestamper { timer: timers::PounderTimestampTimer, @@ -64,9 +64,17 @@ impl Timestamper { timestamp_timer.set_trigger_source(timers::TriggerSource::Trigger1); // The capture channel should capture whenever the trigger input occurs. - let input_capture = capture_channel + let mut input_capture = capture_channel .into_input_capture(timers::tim8::CaptureSource1::TRC); + // Capture at the batch period. + input_capture.configure_prescaler( + timers::Prescaler::try_from( + design_parameters::SAMPLE_BUFFER_SIZE_LOG2, + ) + .unwrap(), + ); + Self { timer: timestamp_timer, capture_channel: input_capture, diff --git a/src/hardware/timers.rs b/src/hardware/timers.rs index 78d27b6..23e15ca 100644 --- a/src/hardware/timers.rs +++ b/src/hardware/timers.rs @@ -1,5 +1,6 @@ ///! The sampling timer is used for managing ADC sampling and external reference timestamping. use super::hal; +use num_enum::TryFromPrimitive; use hal::stm32::{ // TIM1 and TIM8 have identical registers. @@ -34,6 +35,8 @@ pub enum TriggerSource { /// Prescalers for externally-supplied reference clocks. #[allow(dead_code)] +#[derive(TryFromPrimitive)] +#[repr(u8)] pub enum Prescaler { Div1 = 0b00, Div2 = 0b01, @@ -353,6 +356,21 @@ macro_rules! timer_channels { let regs = unsafe { &*<$TY>::ptr() }; regs.[< $ccmrx _input >]().modify(|_, w| w.[< ic $index f >]().bits(filter as u8)); } + + /// Configure the input capture prescaler. + /// + /// # Args + /// * `psc` - Prescaler exponent. + #[allow(dead_code)] + pub fn configure_prescaler(&mut self, prescaler: super::Prescaler) { + // Note(unsafe): This channel owns all access to the specific timer channel. + // Only atomic operations on completed on the timer registers. + let regs = unsafe { &*<$TY>::ptr() }; + // Note(unsafe): Enum values are all valid. + #[allow(unused_unsafe)] + regs.[< $ccmrx _input >]().modify(|_, w| unsafe { + w.[< ic $index psc >]().bits(prescaler as u8)}); + } } // Note(unsafe): This manually implements DMA support for input-capture channels. This From 73491fcb7577d03c873817f4faa9de1b395aa6aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 4 Jun 2021 10:45:22 +0200 Subject: [PATCH 14/15] pounder/timestamp: docs updatew --- src/hardware/pounder/timestamp.rs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/hardware/pounder/timestamp.rs b/src/hardware/pounder/timestamp.rs index a527f7f..7f8241d 100644 --- a/src/hardware/pounder/timestamp.rs +++ b/src/hardware/pounder/timestamp.rs @@ -13,15 +13,9 @@ ///! Once the timer is configured, an input capture is configured to record the timer count ///! register. The input capture is configured to utilize an internal trigger for the input capture. ///! The internal trigger is selected such that when a sample is generated on ADC0, the input -///! capture is simultaneously triggered. This results in the input capture triggering identically -///! to when the ADC samples the input. -///! -///! Once the input capture is properly configured, a DMA transfer is configured to collect all of -///! timestamps. The DMA transfer collects 1 timestamp for each ADC sample collected. In order to -///! avoid potentially losing a timestamp for a sample, the DMA transfer operates in double-buffer -///! mode. As soon as the DMA transfer completes, the hardware automatically swaps over to a second -///! buffer to continue capturing. This alleviates timing sensitivities of the DMA transfer -///! schedule. +///! capture is simultaneously triggered. That trigger is prescaled (its rate is divided) by the +///! batch size. This results in the input capture triggering identically to when the ADC samples +///! the last sample of the batch. That sample is then available for processing by the user. use crate::hardware::{design_parameters, timers}; use core::convert::TryFrom; use stm32h7xx_hal as hal; @@ -35,13 +29,8 @@ pub struct Timestamper { impl Timestamper { /// Construct the pounder sample timestamper. /// - /// # Note - /// The DMA is immediately configured after instantiation. It will not collect any samples - /// until the sample timer begins to cause input capture triggers. - /// /// # Args /// * `timestamp_timer` - The timer peripheral used for capturing timestamps from. - /// * `stream` - The DMA stream to use for collecting timestamps. /// * `capture_channel` - The input capture channel for collecting timestamps. /// * `sampling_timer` - The stabilizer ADC sampling timer. /// * `_clock_input` - The input pin for the external clock from Pounder. From d84c79af2ea369a6be9b2daa706c6b3a9645aa62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20J=C3=B6rdens?= Date: Fri, 4 Jun 2021 10:50:09 +0200 Subject: [PATCH 15/15] apps: spi isrs are spi errors --- src/bin/dual-iir.rs | 8 ++++---- src/bin/lockin.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/bin/dual-iir.rs b/src/bin/dual-iir.rs index da5af3d..c76ab1b 100644 --- a/src/bin/dual-iir.rs +++ b/src/bin/dual-iir.rs @@ -238,22 +238,22 @@ const APP: () = { #[task(binds = SPI2, priority = 3)] fn spi2(_: spi2::Context) { - panic!("ADC0 input overrun"); + panic!("ADC0 SPI error"); } #[task(binds = SPI3, priority = 3)] fn spi3(_: spi3::Context) { - panic!("ADC1 input overrun"); + panic!("ADC1 SPI error"); } #[task(binds = SPI4, priority = 3)] fn spi4(_: spi4::Context) { - panic!("DAC0 output error"); + panic!("DAC0 SPI error"); } #[task(binds = SPI5, priority = 3)] fn spi5(_: spi5::Context) { - panic!("DAC1 output error"); + panic!("DAC1 SPI error"); } extern "C" { diff --git a/src/bin/lockin.rs b/src/bin/lockin.rs index 019ef01..6dd7658 100644 --- a/src/bin/lockin.rs +++ b/src/bin/lockin.rs @@ -313,22 +313,22 @@ const APP: () = { #[task(binds = SPI2, priority = 3)] fn spi2(_: spi2::Context) { - panic!("ADC0 input overrun"); + panic!("ADC0 SPI error"); } #[task(binds = SPI3, priority = 3)] fn spi3(_: spi3::Context) { - panic!("ADC1 input overrun"); + panic!("ADC1 SPI error"); } #[task(binds = SPI4, priority = 3)] fn spi4(_: spi4::Context) { - panic!("DAC0 output error"); + panic!("DAC0 SPI error"); } #[task(binds = SPI5, priority = 3)] fn spi5(_: spi5::Context) { - panic!("DAC1 output error"); + panic!("DAC1 SPI error"); } extern "C" {