siphaser: autocalibrate skew using RX synchronizer

* removes the hardcoded, (poorly) manually determined skew value
* does not need si5324_clkout_fabric anymore (broken on Sayma RTM due to wrong IO voltage)

artiq/firmware/libboard_artiq/si5324.rs

@@ -291,66 +291,45 @@ pub mod siphaser {
         clock::spin_us(500);
     }
 
-    fn get_phaser_sample() -> bool {
+    fn has_error() -> bool {
-        let mut sample = true;
+        unsafe {
-        for _ in 0..32 {
+            csr::siphaser::error_write(1);
-            if unsafe { csr::siphaser::sample_result_read() } == 0 {
+        }
-                sample = false;
+        clock::spin_us(5000);
-            }
+        unsafe {
+            csr::siphaser::error_read() != 0
         }
-        sample
     }
 
-    const PS_MARGIN: u32 = 28;
+    fn find_edge(target: bool) -> Result<u16> {
+        let mut nshifts = 0;
 
-    fn get_stable_phaser_sample() -> (bool, u32) {
+        let mut previous = has_error();
-        let mut nshifts: u32 = 0;
         loop {
-            let s1 = get_phaser_sample();
-            for _ in 0..PS_MARGIN {
-                phase_shift(1);
-            }
-            let s2 = get_phaser_sample();
-            for _ in 0..PS_MARGIN {
-                phase_shift(1);
-            }
-            let s3 = get_phaser_sample();
-            nshifts += 2*PS_MARGIN;
-            if s1 == s2 && s2 == s3 {
-                for _ in 0..PS_MARGIN {
-                    phase_shift(0);
-                }
-                nshifts -= PS_MARGIN;
-                return (s2, nshifts);
-            }
-        }
-    }
-
-    pub fn calibrate_skew(skew: u16) -> Result<()> {
-        // Get into a 0 region
-        let (s1, mut nshifts) = get_stable_phaser_sample();
-        if s1 {
-            while get_phaser_sample() {
-                phase_shift(1);
-                nshifts += 1;
-            }
-            for _ in 0..PS_MARGIN {
-                phase_shift(1);
-            }
-            nshifts += PS_MARGIN;
-        }
-
-        // Get to the 0->1 transition
-        while !get_phaser_sample() {
             phase_shift(1);
             nshifts += 1;
+            let current = has_error();
+            if previous != target && current == target {
+                return Ok(nshifts);
+            }
+            if nshifts > 5000 {
+                return Err("failed to find timing error edge");
+            }
+            previous = current;
         }
-        info!("nshifts to 0->1 siphaser transition: {} ({}deg)", nshifts, nshifts*360/(56*8));
+    }
 
-        // Apply specified skew referenced to that transition
+    pub fn calibrate_skew() -> Result<()> {
-        for _ in 0..skew {
+        let lead = find_edge(false)?;
-            phase_shift(1);
+        let width = find_edge(true)?;
+        info!("calibration successful, lead: {}, width: {} ({}deg)", lead, width, width*360/(56*8));
+
+        // Apply reverse phase shift for half the width to get into the
+        // middle of the working region.
+        for _ in 0..width/2 {
+            phase_shift(0);
         }
+
         Ok(())
     }
 }

artiq/firmware/satman/main.rs

@@ -394,8 +394,6 @@ const SI5324_SETTINGS: si5324::FrequencySettings
     crystal_ref: true
 };
 
-const SIPHASER_PHASE: u16 = 32;
-
 #[no_mangle]
 pub extern fn main() -> i32 {
     clock::init();
@@ -443,7 +441,7 @@ pub extern fn main() -> i32 {
 
         info!("uplink is up, switching to recovered clock");
         si5324::siphaser::select_recovered_clock(true).expect("failed to switch clocks");
-        si5324::siphaser::calibrate_skew(SIPHASER_PHASE).expect("failed to calibrate skew");
+        si5324::siphaser::calibrate_skew().expect("failed to calibrate skew");
 
         #[cfg(has_ad9154)]
         {

artiq/gateware/drtio/rx_synchronizer.py

@@ -29,8 +29,8 @@ class XilinxRXSynchronizer(Module):
     """Deterministic RX synchronizer using a relatively placed macro
     to put the clock-domain-crossing FFs right next to each other.
 
-    To meet setup/hold constraints receiving FFs, adjust the phase shift 
+    To meet setup/hold constraints at the receiving FFs, adjust the phase shift 
-    of the Si5324. 
+    of the jitter cleaner. 
 
     We assume that FPGA routing variations are small enough to be negligible.
     """

artiq/gateware/drtio/siphaser.py

@@ -1,5 +1,5 @@
 from migen import *
-from migen.genlib.cdc import MultiReg
+from migen.genlib.cdc import MultiReg, PulseSynchronizer
 
 from misoc.interconnect.csr import *
 
@@ -8,12 +8,12 @@ from misoc.interconnect.csr import *
 # frequency.
 
 class SiPhaser7Series(Module, AutoCSR):
-    def __init__(self, si5324_clkin, si5324_clkout_fabric,
+    def __init__(self, si5324_clkin, rx_synchronizer,
                  ref_clk=None, ref_div2=False, rtio_clk_freq=150e6):
         self.switch_clocks = CSRStorage()
         self.phase_shift = CSR()
         self.phase_shift_done = CSRStatus(reset=1)
-        self.sample_result = CSRStatus()
+        self.error = CSR()
 
         assert rtio_clk_freq in (125e6, 150e6)
 
@@ -85,16 +85,24 @@ class SiPhaser7Series(Module, AutoCSR):
             )
         ]
 
-        si5324_clkout_se = Signal()
+        # The RX synchronizer is tested for setup/hold violations by feeding it a
-        self.specials += \
+        # toggling pattern and checking that the same toggling pattern comes out.
-            Instance("IBUFDS",
+        toggle_in = Signal()
-                p_DIFF_TERM="TRUE", p_IBUF_LOW_PWR="TRUE",
+        self.sync.rtio_rx0 += toggle_in.eq(~toggle_in)
-                i_I=si5324_clkout_fabric.p, i_IB=si5324_clkout_fabric.n,
+        toggle_out = rx_synchronizer.resync(toggle_in)
-                o_O=si5324_clkout_se),
 
-        clkout_sample1 = Signal()  # IOB register
+        toggle_out_expected = Signal()
-        self.sync.rtio_rx0 += clkout_sample1.eq(si5324_clkout_se)
+        self.sync.rtio += toggle_out_expected.eq(~toggle_out)
-        self.specials += MultiReg(clkout_sample1, self.sample_result.status)
+
+        error = Signal()
+        error_clear = PulseSynchronizer("sys", "rtio")
+        self.submodules += error_clear
+        self.sync.rtio += [
+            If(toggle_out != toggle_out_expected, error.eq(1)),
+            If(error_clear.o, error.eq(0))
+        ]
+        self.specials += MultiReg(error, self.error.w)
+        self.comb += error_clear.i.eq(self.error.re)
 
         # expose MMCM outputs - used for clock constraints
         self.mmcm_freerun_output = mmcm_freerun_output

artiq/gateware/targets/kasli.py

@@ -1046,7 +1046,7 @@ class _SatelliteBase(BaseSoC):
         self.config["RTIO_FREQUENCY"] = str(rtio_clk_freq/1e6)
         self.submodules.siphaser = SiPhaser7Series(
             si5324_clkin=platform.request("si5324_clkin"),
-            si5324_clkout_fabric=platform.request("si5324_clkout_fabric"),
+            rx_synchronizer=self.rx_synchronizer,
             ref_clk=self.crg.clk125_div2, ref_div2=True,
             rtio_clk_freq=rtio_clk_freq)
         platform.add_false_path_constraints(

artiq/gateware/targets/sayma_amc.py

@@ -641,7 +641,7 @@ class Satellite(BaseSoC, RTMCommon):
         self.config["RTIO_FREQUENCY"] = str(rtio_clk_freq/1e6)
         self.submodules.siphaser = SiPhaser7Series(
             si5324_clkin=platform.request("si5324_clkin"),
-            si5324_clkout_fabric=platform.request("si5324_clkout_fabric"))
+            rx_synchronizer=self.rx_synchronizer)
         platform.add_platform_command("set_property CLOCK_DEDICATED_ROUTE FALSE [get_nets {mmcm_ps}]",
             mmcm_ps=self.siphaser.mmcm_ps_output)
         platform.add_false_path_constraints(