Allow clock control to work independently of links (#1114)

Author: martijnbastiaan

firmware-binaries/Cargo.lock

@@ -16,6 +16,7 @@ riscv-rt = "0.11.0"
 bittide-sys = { path = "../../../firmware-support/bittide-sys" }
 bittide-hal = { path = "../../../firmware-support/bittide-hal" }
 ufmt = "0.2.0"
+itertools = { version = "0.14.0", default-features = false }
 
 [build-dependencies]
 memmap-generate = { path = "../../../firmware-support/memmap-generate" }

firmware-binaries/demos/clock-control/Cargo.toml

@@ -6,6 +6,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use core::panic::PanicInfo;
+use itertools::izip;
 
 use bittide_hal::manual_additions::timer::Duration;
 use bittide_hal::manual_additions::timer::Instant;
@@ -37,19 +38,6 @@ fn main() -> ! {
     uwriteln!(uart, "Starting sync out generator..").unwrap();
     sync_out_generator.set_active(true);
 
-    uwriteln!(uart, "Waiting for at least one pulse..").unwrap();
-    while freeze.number_of_sync_pulses_seen() == 0 {
-        // TOOD: Memory map sync pulse counter -- no need to involve freeze here
-        freeze.set_freeze(());
-    }
-
-    // XXX: CPU is booted after all links are available, so we can blindly enable
-    //      all counters.
-    uwriteln!(uart, "Starting domain diff counters..").unwrap();
-    for i in 0..DomainDiffCounters::ENABLE_LEN {
-        domain_diff_counters.set_enable(i, true);
-    }
-
     uwriteln!(uart, "Starting clock control..").unwrap();
     let mut callisto = Callisto::new(cc.config().callisto);
 
@@ -67,16 +55,34 @@ fn main() -> ! {
     let mut prev_all_stable = false;
 
     loop {
-        // Do clock control on "frozen" counters
+        // Store frozen elastic buffer counters
         freeze.set_freeze(());
-        cc.set_change_speed(callisto.update(&cc, freeze.eb_counters_volatile_iter()));
+
+        // Do clock control update
+        cc.set_change_speed(
+            callisto.update(
+                &cc,
+                izip!(
+                    0..DomainDiffCounters::ENABLE_LEN,
+                    freeze.eb_counters_volatile_iter()
+                )
+                .map(|(i, counter)| {
+                    if domain_diff_counters.enable(i).unwrap_or(false) {
+                        Some(counter)
+                    } else {
+                        None
+                    }
+                }),
+            ),
+        );
 
         // Detect stability
         let stability = stability_detector.update(&cc, timer.now());
 
         // Store debug information. Stop capturing samples if we are stable to
         // reduce plot sizes.
-        if !prev_all_stable {
+        let has_sense_of_global_time = freeze.number_of_sync_pulses_seen() != 0;
+        if !prev_all_stable && has_sense_of_global_time {
             sample_store.store(&freeze, stability, callisto.accumulated_speed_requests);
         }
 
@@ -90,6 +96,13 @@ fn main() -> ! {
         }
         prev_all_stable = all_stable;
 
+        // Update active domain difference counters based on which links are
+        // enabled.
+        let link_mask_rev = cc.link_mask_rev();
+        for i in 0..DomainDiffCounters::ENABLE_LEN {
+            domain_diff_counters.set_enable(i, test_bit(link_mask_rev, i));
+        }
+
         // Wait for next update
         let timer_result = timer.wait_until_stall(next_update);
         panic_on_missed_deadline(&mut uart, &timer, next_update, timer_result);
@@ -115,6 +128,11 @@ fn panic_on_missed_deadline(
     };
 }
 
+/// Test whether the `i`-th bit in `bv` is set.
+fn test_bit(bv: u8, i: usize) -> bool {
+    (bv & (1 << i)) != 0
+}
+
 #[panic_handler]
 fn panic_handler(_info: &PanicInfo) -> ! {
     loop {

firmware-binaries/demos/clock-control/src/main.rs

@@ -9,7 +9,7 @@ use core::panic::PanicInfo;
 use ufmt::uwriteln;
 
 use bittide_hal::manual_additions::si539x_spi::{Config, WriteError};
-use bittide_hal::shared_devices::transceivers::Transceivers;
+use bittide_hal::shared_devices::Transceivers;
 use bittide_hal::switch_demo_boot::DeviceInstances;
 use bittide_macros::load_clock_config_csv;
 
@@ -49,21 +49,12 @@ fn main() -> ! {
 
     uwriteln!(uart, "Enabling bittide domain..").unwrap();
     transceivers.set_transceiver_enable(true);
-    uwriteln!(uart, "Done.").unwrap();
 
     uwriteln!(uart, "Enabling all transceiver channels..").unwrap();
     for channel in 0..Transceivers::CHANNEL_ENABLES_LEN {
         transceivers.set_channel_enables(channel, true);
     }
 
-    for channel in 0..Transceivers::HANDSHAKES_DONE_LEN {
-        while !transceivers.handshakes_done(channel).unwrap_or(false) {}
-        uwriteln!(uart, "Channel {} handshake done.", channel).unwrap();
-        uwriteln!(uart, "{:?}", transceivers.statistics(channel)).unwrap();
-    }
-
-    uwriteln!(uart, "Done.").unwrap();
-
     uwriteln!(uart, "Going into infinite loop..").unwrap();
     #[allow(clippy::empty_loop)]
     loop {}

firmware-binaries/demos/switch-demo1-boot/src/main.rs

@@ -19,7 +19,13 @@ fn main() -> ! {
     let mut uart = INSTANCES.uart;
     let transceivers = &INSTANCES.transceivers;
 
-    uwriteln!(uart, "Hello from management unit..").unwrap();
+    // Channels should be enabled by boot program, so we can simply wait here
+    uwriteln!(uart, "Waiting for channel negotiations..").unwrap();
+    for channel in 0..Transceivers::HANDSHAKES_DONE_LEN {
+        while !transceivers.handshakes_done(channel).unwrap_or(false) {}
+        uwriteln!(uart, "Channel {} negotiation done.", channel).unwrap();
+        uwriteln!(uart, "{:?}", transceivers.statistics(channel)).unwrap();
+    }
 
     uwriteln!(uart, "Centering buffer occupancies").unwrap();
     for (i, eb) in [

firmware-binaries/demos/switch-demo1-mu/src/main.rs

@@ -19,7 +19,13 @@ fn main() -> ! {
     let mut uart = INSTANCES.uart;
     let transceivers = &INSTANCES.transceivers;
 
-    uwriteln!(uart, "Hello from management unit..").unwrap();
+    // Channels should be enabled by boot program, so we can simply wait here
+    uwriteln!(uart, "Waiting for channel negotiations..").unwrap();
+    for channel in 0..Transceivers::HANDSHAKES_DONE_LEN {
+        while !transceivers.handshakes_done(channel).unwrap_or(false) {}
+        uwriteln!(uart, "Channel {} negotiation done.", channel).unwrap();
+        uwriteln!(uart, "{:?}", transceivers.statistics(channel)).unwrap();
+    }
 
     uwriteln!(uart, "Centering buffer occupancies").unwrap();
     for (i, eb) in [

firmware-binaries/demos/switch-demo2-mu/src/main.rs

@@ -60,19 +60,14 @@ impl Callisto {
     /// [https://github.com/bittide/Callisto.jl](https://github.com/bittide/Callisto.jl)
     pub fn update<I>(self: &mut Callisto, cc: &ClockControl, eb_counters_iter: I) -> SpeedChange
     where
-        I: Iterator<Item = i32>,
+        I: Iterator<Item = Option<i32>>,
     {
         // `fStep` should match the step size of the clock boards. For all our HITL
         // tests this is set by `HwCcTopologies.commonStepSizeSelect`.
         const FSTEP: f32 = 10e-9; // 10 PPB
 
-        let link_mask_rev = cc.link_mask_rev();
-
         // Sum the data counts for all active links
-        let measured_sum: i32 = eb_counters_iter
-            .enumerate()
-            .map(|(i, v)| if test_bit(link_mask_rev, i) { v } else { 0 })
-            .sum();
+        let measured_sum: i32 = eb_counters_iter.flatten().sum();
 
         let c_des = self.config.gain * (measured_sum as f32) + self.steady_state_target;
         let c_est = FSTEP * self.accumulated_speed_requests as f32;
@@ -128,8 +123,3 @@ fn speed_change_to_sign(speed_change: SpeedChange) -> i32 {
         SpeedChange::SpeedUp => 1,
     }
 }
-
-/// Test whether the `i`-th bit in `bv` is set.
-fn test_bit(bv: u8, i: usize) -> bool {
-    (bv & (1 << i)) != 0
-}

firmware-support/bittide-sys/src/callisto.rs

@@ -9,6 +9,10 @@ use bittide_hal::{
 use itertools::izip;
 use ufmt::derive::uDebug;
 
+fn test_bit(bv: u8, i: usize) -> bool {
+    (bv & (1 << i)) != 0
+}
+
 pub struct StabilityDetector {
     /// Reference data counts -- sampled at some point in time and used to
     /// compare against the current data counts to determine stability.
@@ -61,21 +65,28 @@ impl StabilityDetector {
     pub fn update(&mut self, cc: &ClockControl, now: Instant) -> Stability {
         let mut stables: u32 = 0;
         let mut settleds: u32 = 0;
+        let link_mask_rev = cc.link_mask_rev();
 
-        for (data_count_stored, maybe_start, data_count, min_seen, max_seen, prev_stable) in izip!(
-            self.data_counts.iter_mut(),
-            self.starts.iter_mut(),
-            cc.data_counts_volatile_iter(),
-            cc.min_data_counts_seen_volatile_iter(),
-            cc.max_data_counts_seen_volatile_iter(),
-            self.prev_stabilities.iter_mut()
-        ) {
+        for (i, (data_count_stored, maybe_start, data_count, min_seen, max_seen, prev_stable)) in
+            izip!(
+                self.data_counts.iter_mut(),
+                self.starts.iter_mut(),
+                cc.data_counts_volatile_iter(),
+                cc.min_data_counts_seen_volatile_iter(),
+                cc.max_data_counts_seen_volatile_iter(),
+                self.prev_stabilities.iter_mut()
+            )
+            .enumerate()
+        {
+            let active = test_bit(link_mask_rev, i);
             let diff0 = data_count_stored.abs_diff(min_seen);
             let diff1 = data_count_stored.abs_diff(max_seen);
             let height_violated = diff0 > self.margin || diff1 > self.margin;
 
-            let (stable, settled) = if height_violated {
-                // Reset everything, as we violated the window height
+            let (stable, settled) = if height_violated || !active {
+                // Reset everything, as we violated the window height or if this
+                // link is inactive. The latter prevents inactive links from
+                // being considered stable.
                 *maybe_start = Some(now);
                 *data_count_stored = data_count;
                 cc.set_clear_data_counts_seen(true);