RMT: Tentative timing fix/workaround

esp-hal/src/rmt.rs

@@ -2146,9 +2146,7 @@ impl DynChannelAccess<Tx> {
         #[cfg(rmt_has_tx_loop_count)]
         self.set_loopmode(loopmode);
         self.set_tx_wrap_mode(loopmode.is_none());
-        self.update();
         self.start_tx();
-        self.update();
     }
 
     #[inline(always)]
@@ -2174,11 +2172,9 @@ impl DynChannelAccess<Rx> {
         {
             self.set_rx_threshold((_memsize.codes() / 2) as u16);
             self.set_rx_wrap_mode(_wrap);
-            self.update();
         }
 
         self.start_rx();
-        self.update();
     }
 
     #[inline(always)]
@@ -2541,9 +2537,15 @@ mod chip_specific {
 
             rmt.ch_tx_conf0(ch_idx).modify(|_, w| {
                 w.mem_rd_rst().set_bit();
-                w.apb_mem_rst().set_bit();
-                w.tx_start().set_bit()
+                w.apb_mem_rst().set_bit()
             });
+            self.update();
+            // Wait for configuration/data to be effective - there seem to be undocumented timing
+            // constraints when sclk_div is large. 4us corresponds to ~1 SCLK cycle when using the
+            // APB SCLK.
+            crate::rom::ets_delay_us(4);
+            rmt.ch_tx_conf0(self.channel().into())
+                .modify(|_, w| w.tx_start().set_bit());
         }
 
         // Return the first flag that is set of, in order of decreasing priority,
@@ -2686,9 +2688,13 @@ mod chip_specific {
             rmt.ch_rx_conf1(ch_idx.into()).modify(|_, w| {
                 w.mem_owner().set_bit();
                 w.mem_wr_rst().set_bit();
-                w.apb_mem_rst().set_bit();
-                w.rx_en().set_bit()
+                w.apb_mem_rst().set_bit()
             });
+            self.update();
+            crate::rom::ets_delay_us(4);
+            rmt.ch_rx_conf1(ch_idx.into())
+                .modify(|_, w| w.rx_en().set_bit());
+            self.update();
         }
 
         // Return the first flag that is set of, in order of decreasing priority,

esp-hal/src/rmt/reader.rs

@@ -54,6 +54,8 @@ impl RmtReader {
         let memsize = raw.memsize().codes();
         let offset = self.offset as usize;
 
+        // cf. comments on delay in ../rmt.rs
+        crate::rom::ets_delay_us(4);
         let max_count = if final_ {
             let hw_offset = raw.hw_offset();
 

hil-test/src/bin/rmt.rs

@@ -83,6 +83,8 @@ struct LoopbackConfig {
     length1_base: u16,
     length1_step: u16,
     length2: u16,
+    sclk_div: u16, // 1 - 256
+    ch_div: u8,
 }
 
 impl Default for LoopbackConfig {
@@ -100,22 +102,28 @@ impl Default for LoopbackConfig {
             length1_base: 100,
             length1_step: 10,
             length2: 50,
+            sclk_div: 1,
+            ch_div: DIV,
         }
     }
 }
 
 impl LoopbackConfig {
+    fn sclk_freq(&self) -> Rate {
+        FREQ / self.sclk_div as u32
+    }
+
     fn tx_config(&self) -> TxChannelConfig {
         TxChannelConfig::default()
-            .with_clk_divider(DIV)
+            .with_clk_divider(self.ch_div)
             .with_idle_output(self.idle_output)
             .with_idle_output_level(Level::Low)
             .with_memsize(self.tx_memsize)
     }
 
     fn rx_config(&self) -> RxChannelConfig {
         RxChannelConfig::default()
-            .with_clk_divider(DIV)
+            .with_clk_divider(self.ch_div)
             .with_idle_threshold(self.idle_threshold)
             .with_memsize(self.rx_memsize)
     }
@@ -128,37 +136,47 @@ impl LoopbackConfig {
 // buffer in loopback tests with buffer wrapping. If we did, that might hide bugs.
 // FIXME: Make a method on LoopbackConfig?
 fn generate_tx_data(conf: &LoopbackConfig) -> Vec<PulseCode> {
+    const WRAP_COUNT: u16 = 23;
+
     let mut tx_data: Vec<_> = (0..)
         .take(conf.tx_len)
         .map(|i| {
-            PulseCode::new(
+            PulseCode::try_new(
                 Level::High,
-                conf.length1_base + conf.length1_step * (i % 23),
+                conf.length1_base + conf.length1_step * (i % WRAP_COUNT),
                 Level::Low,
                 conf.length2,
             )
+            .expect("pulse code length out of range")
         })
         .collect();
+    let mut i_max = WRAP_COUNT.min(conf.tx_len as u16);
 
     let mut pos = conf.tx_len - 1;
     match conf.end_marker {
         EndMarkerConfig::None => (),
         EndMarkerConfig::Field1 => {
             tx_data[pos] = PulseCode::end_marker();
             pos -= 1;
+            i_max -= 1;
         }
         EndMarkerConfig::Field2 => {
             tx_data[pos] = tx_data[pos].with_length2(0).unwrap();
             pos -= 1;
+            i_max -= 1;
         }
     }
     if conf.write_stop_code {
-        tx_data[pos] = PulseCode::new(
+        i_max -= 1;
+        assert!(conf.length1_base + i_max * conf.length1_step + conf.length2 < conf.idle_threshold);
+
+        tx_data[pos] = PulseCode::try_new(
             Level::High,
             2 * conf.idle_threshold,
             Level::Low,
             conf.length2,
-        );
+        )
+        .expect("idle_threshold out of range");
     }
 
     tx_data
@@ -284,10 +302,13 @@ fn check_data_eq(
 
 struct BitbangTransmitter {
     pin: Flex<'static>,
+    cycles_per_us: u32,
 }
 
 impl BitbangTransmitter {
-    fn new(pin: impl Pin + 'static) -> Self {
+    const SCALE: u32 = 1024;
+
+    fn new(pin: impl Pin + 'static, conf: &LoopbackConfig) -> Self {
         let mut pin = Flex::new(pin);
         pin.set_input_enable(true);
         pin.set_output_enable(true);
@@ -296,30 +317,31 @@ impl BitbangTransmitter {
 
         pin.set_low();
 
-        Self { pin }
+        Self {
+            pin,
+            cycles_per_us: conf.ch_div as u32 * 1000 * Self::SCALE / conf.sclk_freq().as_khz(),
+        }
     }
 
     fn rx_pin(&mut self) -> impl PeripheralInput<'static> {
         self.pin.peripheral_input()
     }
 
     fn transmit(&mut self, tx_data: &[PulseCode]) {
-        let cycles_per_us = DIV as u32 / FREQ.as_mhz();
-
         let delay = Delay::new();
 
         for code in tx_data {
             if code.length1() == 0 {
                 break;
             }
             self.pin.set_level(code.level1());
-            delay.delay_micros(code.length1() as u32 * cycles_per_us);
+            delay.delay_micros(code.length1() as u32 * self.cycles_per_us / Self::SCALE);
 
             if code.length2() == 0 {
                 break;
             }
             self.pin.set_level(code.level2());
-            delay.delay_micros(code.length2() as u32 * cycles_per_us);
+            delay.delay_micros(code.length2() as u32 * self.cycles_per_us / Self::SCALE);
         }
     }
 }
@@ -482,7 +504,7 @@ impl Context {
         &mut self,
         conf: &LoopbackConfig,
     ) -> (Channel<'_, Blocking, Tx>, Channel<'_, Blocking, Rx>) {
-        let rmt = Rmt::new(self.rmt.reborrow(), FREQ).unwrap();
+        let rmt = Rmt::new(self.rmt.reborrow(), conf.sclk_freq()).unwrap();
         let (rx, tx) = pins!(self);
         Self::setup_impl(rmt, rx, tx, conf)
     }
@@ -491,7 +513,9 @@ impl Context {
         &mut self,
         conf: &LoopbackConfig,
     ) -> (Channel<'_, Async, Tx>, Channel<'_, Async, Rx>) {
-        let rmt = Rmt::new(self.rmt.reborrow(), FREQ).unwrap().into_async();
+        let rmt = Rmt::new(self.rmt.reborrow(), conf.sclk_freq())
+            .unwrap()
+            .into_async();
         let (rx, tx) = pins!(self);
         Self::setup_impl(rmt, rx, tx, conf)
     }
@@ -696,7 +720,7 @@ mod tests {
             let conf = LoopbackConfig::default();
 
             let (rx_pin, tx_pin) = pins!($ctx);
-            let mut rmt = Rmt::new($ctx.rmt.reborrow(), FREQ).unwrap();
+            let mut rmt = Rmt::new($ctx.rmt.reborrow(), conf.sclk_freq()).unwrap();
 
             let tx_channel = rmt
                 .$tx_channel
@@ -788,9 +812,9 @@ mod tests {
             ..Default::default()
         };
 
-        let rmt = Rmt::new(ctx.rmt, FREQ).unwrap();
+        let rmt = Rmt::new(ctx.rmt, conf.sclk_freq()).unwrap();
 
-        let mut bitbang_tx = BitbangTransmitter::new(ctx.pin);
+        let mut bitbang_tx = BitbangTransmitter::new(ctx.pin, &conf);
         let (_, rx_channel) = Context::setup_impl(rmt, bitbang_tx.rx_pin(), NoPin, &conf);
 
         let tx_data = generate_tx_data(&conf);
@@ -859,7 +883,7 @@ mod tests {
 
         // Test that dropping & recreating Rmt works
         for _ in 0..3 {
-            let mut rmt = Rmt::new(ctx.rmt.reborrow(), FREQ).unwrap();
+            let mut rmt = Rmt::new(ctx.rmt.reborrow(), conf.sclk_freq()).unwrap();
 
             // Test that dropping & recreating ChannelCreator works
             for _ in 0..3 {
@@ -923,7 +947,9 @@ mod tests {
 
         // Test that dropping & recreating Rmt works
         for _ in 0..3 {
-            let mut rmt = Rmt::new(ctx.rmt.reborrow(), FREQ).unwrap().into_async();
+            let mut rmt = Rmt::new(ctx.rmt.reborrow(), conf.sclk_freq())
+                .unwrap()
+                .into_async();
 
             // Test that dropping & recreating ChannelCreator works
             for _ in 0..3 {
@@ -1082,4 +1108,37 @@ mod tests {
             "tx with loopcount 0 did not complete immediately"
         );
     }
+
+    #[test]
+    fn rmt_dividers(mut ctx: Context) {
+        for sclk_div in [1, 16, 256] {
+            for ch_div in [1, 16, 255] {
+                let div = sclk_div as u32 * ch_div as u32;
+                // the stop code is sent with length 2 * idle_threshold = 40 * step, cf.
+                // generate_tx_data
+                let step = (32767 / (41 * div)).max(5) as u16;
+                let conf = LoopbackConfig {
+                    idle_output: true,
+                    // write_stop_code: false,
+                    sclk_div,
+                    ch_div,
+                    tx_len: 5,
+                    length1_base: 10 * step,
+                    length1_step: step,
+                    length2: 2 * step,
+                    idle_threshold: 20 * step,
+                    ..Default::default()
+                };
+                #[cfg(feature = "defmt")]
+                {
+                    defmt::info!("sclk_div: {}, ch_div: {}, step: {}", sclk_div, ch_div, step);
+                    defmt::flush();
+                }
+
+                let (tx_channel, rx_channel) = ctx.setup_loopback(&conf);
+
+                do_rmt_loopback_inner(&conf, tx_channel, rx_channel);
+            }
+        }
+    }
 }