From 0be396fc38ef1d88ef5d60eeb82c1211686823af Mon Sep 17 00:00:00 2001
From: Eric Kooistra <kooistra@astron.nl>
Date: Thu, 18 Aug 2022 11:28:03 +0200
Subject: [PATCH] Correct account for latency due to s_bsn_time_offset state.

---
 .../base/dp/src/vhdl/dp_bsn_source_v2.vhd     | 183 +++++++++---------
 1 file changed, 92 insertions(+), 91 deletions(-)

diff --git a/libraries/base/dp/src/vhdl/dp_bsn_source_v2.vhd b/libraries/base/dp/src/vhdl/dp_bsn_source_v2.vhd
index c1f1e2ec35..a2f1756cc8 100644
--- a/libraries/base/dp/src/vhdl/dp_bsn_source_v2.vhd
+++ b/libraries/base/dp/src/vhdl/dp_bsn_source_v2.vhd
@@ -20,6 +20,9 @@
 -------------------------------------------------------------------------------
 
 -- Author : P.Donker  okt. 2020, added bsn_time_offset
+--          E. Kooistra aug 2022, corected s_bsn_time_offset, clarified
+--            sync_size_cnt, removed redundant current_bsn_time_offset
+--            and s_init, simplified implementation of dp_on_status.
 --
 -- Purpose :
 --   Start a periodic block sync interval and maintain a block sequence
@@ -29,18 +32,18 @@
 --   output sync starts pulsing after bsn_time_offset (in clk cycles) with 
 --   a period of g_block_size number of clk cycles and the output valid, 
 --   sop and eop will be active.
---   Alternatively, one can assert dp_on while dp_on_pps is high to 
---   start the data path on the next PPS.
+--   The dp_on is asynchronous. Alternatively, one can assert dp_on while
+--   dp_on_pps is high to start the data path on the next PPS.
 --   The src_out.sync always happens at the src_out.sop.
 --   If nof_clk_per_sync / g_block_size is an integer than all src_out.sync
 --   intervals will have nof_clk_per_sync clk cycles, else nof_clk_per_sync
 --   is the average number of clock cycles between src_out.sync and then the
 --   number of blocks per sync intervals will vary between c_nof_block_hi
 --   and c_nof_block_lo.
---   The dp_on is asynchronous. The dp_bsn_source_v2 takes care that
---   src_out.valid starts with a src_out.sop and that src_out.valid can
---   only go low after a src_out.eop, to ensure that src_out only produces
---   complete sop-eop blocks that enter the subsequent processing.
+--   The dp_bsn_source_v2 takes care that src_out.valid starts with a
+--   src_out.sync and src_out.sop and that src_out.valid can only go low
+--   after a src_out.eop, to ensure that src_out only produces complete
+--   sop-eop blocks that enter the subsequent processing.
 --   The bs_restart is active at the first src_out.sop after dp_on went high.
 -- Remarks:
 -- . Starting the data path is only possible from the dp_off state, so one
@@ -59,7 +62,7 @@ USE work.dp_stream_pkg.ALL;
 
 ENTITY dp_bsn_source_v2 IS
   GENERIC (
-    g_block_size         : NATURAL := 256;  
+    g_block_size         : NATURAL := 256;  -- >= 3, see state machine
     g_nof_clk_per_sync   : NATURAL := 200 * 10**6; 
     g_bsn_w              : NATURAL := 48;
     g_bsn_time_offset_w  : NATURAL := 10 
@@ -78,7 +81,6 @@ ENTITY dp_bsn_source_v2 IS
     nof_clk_per_sync        : IN  STD_LOGIC_VECTOR(c_word_w-1 DOWNTO 0) := TO_UVEC(g_nof_clk_per_sync, c_word_w);
     bsn_init                : IN  STD_LOGIC_VECTOR(g_bsn_w-1 DOWNTO 0) := (OTHERS=>'0');
     bsn_time_offset         : IN  STD_LOGIC_VECTOR(g_bsn_time_offset_w-1 DOWNTO 0) := (OTHERS=>'0');
-    current_bsn_time_offset : OUT STD_LOGIC_VECTOR(g_bsn_time_offset_w-1 DOWNTO 0);
 
     src_out                 : OUT t_dp_sosi  -- only uses sync, bsn[], valid, sop and eop
   );
@@ -89,24 +91,11 @@ ARCHITECTURE rtl OF dp_bsn_source_v2 IS
 
   CONSTANT c_block_size_cnt_w : NATURAL := ceil_log2(g_block_size);
   CONSTANT c_block_cnt_zero   : STD_LOGIC_VECTOR(g_bsn_w-1 DOWNTO 0) := (OTHERS => '0');
+
   CONSTANT c_nof_block_lo     : NATURAL := g_nof_clk_per_sync / g_block_size;
   CONSTANT c_nof_block_hi     : NATURAL := ceil_div(g_nof_clk_per_sync, g_block_size);
 
-  -- The state machine starts synchronously via s_bsn_time_offset, s_dp_on_sop to s_dp_on, or it starts
-  -- directly to s_dp_on. When in dp_on it loops between s_dp_on and s_dp_on_eop for every block.
-  -- If the BSN source is switched off, then after completing the block s_dp_on_eop goes back to
-  -- s_dp_off.
-  --
-  -- Sketch of the state machine:
-  --
-  -- s_init --> s_dp_off --> s_bsn_time_offset --                      ------------ (loop) <---
-  --            \       \                        \                    /                        \ 
-  --             \       ---------------------------> s_dp_on_sop --> s_dp_on --> s_dp_on_eop --
-  --              \                                                                            /
-  --               ----------------------------------------------------------------- (off) <--- 
-
-
-  TYPE t_state_enum IS (s_init, s_dp_off, s_bsn_time_offset, s_dp_on_sop, s_dp_on, s_dp_on_eop);
+  TYPE t_state_enum IS (s_dp_off, s_bsn_time_offset, s_dp_on_sop, s_dp_on, s_dp_on_eop);
 
   SIGNAL state       : t_state_enum;
   SIGNAL nxt_state   : t_state_enum; 
@@ -118,139 +107,151 @@ ARCHITECTURE rtl OF dp_bsn_source_v2 IS
   SIGNAL i_src_out          : t_dp_sosi := c_dp_sosi_init;
   SIGNAL nxt_src_out        : t_dp_sosi;
  
-  SIGNAL i_dp_on_status     : STD_LOGIC;
-  SIGNAL nxt_dp_on_status   : STD_LOGIC;
   SIGNAL nxt_bs_restart     : STD_LOGIC;
 
   SIGNAL nxt_bsn_time_offset_cnt : STD_LOGIC_VECTOR(g_bsn_time_offset_w-1 DOWNTO 0);
   SIGNAL bsn_time_offset_cnt     : STD_LOGIC_VECTOR(g_bsn_time_offset_w-1 DOWNTO 0);
   
-  SIGNAL i_current_bsn_time_offset   : STD_LOGIC_VECTOR(g_bsn_time_offset_w-1 DOWNTO 0);
-  SIGNAL nxt_current_bsn_time_offset : STD_LOGIC_VECTOR(g_bsn_time_offset_w-1 DOWNTO 0);
-  
-  SIGNAL nxt_clk_cnt        : STD_LOGIC_VECTOR(c_word_w-1 DOWNTO 0);
-  SIGNAL clk_cnt            : STD_LOGIC_VECTOR(c_word_w-1 DOWNTO 0);
-  SIGNAL nxt_sync           : STD_LOGIC;
-  SIGNAL sync               : STD_LOGIC;
+  SIGNAL nxt_sync_size_cnt       : STD_LOGIC_VECTOR(c_word_w-1 DOWNTO 0);
+  SIGNAL sync_size_cnt           : STD_LOGIC_VECTOR(c_word_w-1 DOWNTO 0);
+  SIGNAL nxt_sync                : STD_LOGIC;
+  SIGNAL sync                    : STD_LOGIC;
  
 BEGIN
 
   src_out <= i_src_out;
-  dp_on_status <= i_dp_on_status;
-  current_bsn_time_offset <= i_current_bsn_time_offset;
+  dp_on_status <= i_src_out.valid;
 
-  p_state : PROCESS(state, i_src_out, block_size_cnt, clk_cnt, sync, i_dp_on_status, bsn_time_offset_cnt, bsn_time_offset, nof_clk_per_sync, bsn_init, dp_on, dp_on_pps, pps, prev_state)
+  p_state : PROCESS(sync, sync_size_cnt, nof_clk_per_sync,
+                    state, i_src_out, block_size_cnt, bsn_time_offset_cnt,
+                    bsn_init, dp_on, dp_on_pps, pps, bsn_time_offset, prev_state)
   BEGIN  
+    -- Maintain sync_size_cnt for nof_clk_per_sync
+    -- . nof_clk_per_sync is the number of clk per pps interval and the
+    --   average number of clk per src_out.sync interval, due to that the
+    --   src_out.sync has to occur at the src_out.sop of a block.
+    -- . The sync_size_cnt is started in s_dp_off at the pps
+    -- . The pps interval is nof_clk_per_sync, so when the sync_size_cnt
+    --   wraps, then the sync is used to ensure that src_out.sync will
+    --   pulse at the src_out.sop of the next block.
+    nxt_sync <= sync;
+    nxt_sync_size_cnt <= INCR_UVEC(sync_size_cnt, 1);
+    IF UNSIGNED(sync_size_cnt) = UNSIGNED(nof_clk_per_sync) - 1 THEN
+      nxt_sync <= '1';  -- will set src_out.sync on next src_out.sop
+      nxt_sync_size_cnt <= (OTHERS=>'0');
+    END IF;
+
+    -- State machine for src_out
     nxt_state                   <= state;
-    nxt_src_out                 <= i_src_out;
+    nxt_src_out                 <= i_src_out;  -- hold src_out.bsn
     nxt_src_out.sync            <= '0';
     nxt_src_out.valid           <= '0';
     nxt_src_out.sop             <= '0';
     nxt_src_out.eop             <= '0';
     nxt_block_size_cnt          <= block_size_cnt;
-    nxt_clk_cnt                 <= INCR_UVEC(clk_cnt, 1);
-    nxt_sync                    <= sync;
-    nxt_dp_on_status            <= i_dp_on_status;
-    nxt_bs_restart              <= '0';
     nxt_bsn_time_offset_cnt     <= bsn_time_offset_cnt;
-    nxt_current_bsn_time_offset <= bsn_time_offset;
-
-    IF UNSIGNED(clk_cnt) = UNSIGNED(nof_clk_per_sync) - 1 THEN
-      nxt_clk_cnt <=  (OTHERS=>'0');
-      nxt_sync    <= '1';  -- will set src_out.sync on next src_out.sop
-    END IF;
 
     CASE state IS
-
-      WHEN s_init =>
-        nxt_state <= s_dp_off;
-      
       WHEN s_dp_off =>
-        nxt_dp_on_status <= '0';
         nxt_src_out.bsn <= RESIZE_DP_BSN(bsn_init);
+        nxt_bsn_time_offset_cnt <= (OTHERS=>'0');
         nxt_sync <= '0';
-        nxt_clk_cnt <=  (OTHERS=>'0');
-        IF dp_on = '1' THEN 
+        nxt_sync_size_cnt <= (OTHERS=>'0');
+        nxt_block_size_cnt <= (OTHERS=>'0');
+        IF dp_on = '1' THEN
+          nxt_sync <= '1';  -- ensure issue sync at first sync interval
           IF dp_on_pps = '1' THEN
-            nxt_sync <= '1';   -- ensure issue sync at first sync interval for start at PPS.
+            -- start at pps
             IF pps = '1' THEN
-              nxt_bsn_time_offset_cnt <= (OTHERS=>'0');
-              nxt_state          <= s_bsn_time_offset;
+              nxt_state <= s_bsn_time_offset;
             END IF;
-          ELSE 
-            nxt_state   <= s_dp_on_sop;
+          ELSE
+            -- start immediately
+            nxt_state <= s_dp_on_sop;
           END IF;
         END IF;
 
       WHEN s_bsn_time_offset =>
         IF UNSIGNED(bsn_time_offset_cnt) = UNSIGNED(bsn_time_offset) THEN
+          -- The bsn_time_offset can be 0, so IF UNSIGNED(bsn_time_offset)-1
+          -- can not be used to account for on clk latency of state
+          -- s_bsn_time_offset. Therefore do not count sync_size_cnt during
+          -- latency of state s_bsn_time_offset.
+          nxt_sync_size_cnt <= sync_size_cnt;
           nxt_state <= s_dp_on_sop;
         ELSE
           nxt_bsn_time_offset_cnt <= INCR_UVEC(bsn_time_offset_cnt, 1);
         END IF;
 
+      -- using separate states s_dp_on_sop and s_dp_on_eop instead of only
+      -- s_dp_on state and block_size_cnt, cause that g_block_size must be
+      -- >= 3, but that is fine.
       WHEN s_dp_on_sop =>
-        nxt_dp_on_status   <= '1';
-        nxt_src_out.sop    <= '1';
+        -- Start of block
+        nxt_src_out.sop <= '1';
         nxt_src_out.valid  <= '1';
-        nxt_state          <= s_dp_on;
+        nxt_state <= s_dp_on;
+        -- block_size_cnt = 0 at src_out.sop
         nxt_block_size_cnt <= (OTHERS=>'0');
+        -- after first block, increment bsn per block
         IF prev_state = s_dp_on_eop THEN
           nxt_src_out.bsn <= INCR_DP_BSN(i_src_out.bsn, 1, g_bsn_w);
         END IF;
-        IF sync = '1' THEN 
+        -- check for pending sync
+        IF sync = '1' THEN
           nxt_src_out.sync <= '1';
           nxt_sync <= '0';
         END IF;
-        IF i_dp_on_status = '0' THEN -- transition from 0 to 1 is a (re)start
-          nxt_bs_restart <= '1'; -- bs_restart indicates a restart as a pulse on the sop (and sync if dp_on_pps is used).
-        END IF;
 
       WHEN s_dp_on =>
-        nxt_src_out.valid  <= '1';
+        -- During block
+        nxt_src_out.valid <= '1';
+        -- block_size_cnt increments to determine end of block
         nxt_block_size_cnt <= INCR_UVEC(block_size_cnt, 1);
-        IF UNSIGNED(block_size_cnt) = g_block_size -3 THEN
-          nxt_state        <= s_dp_on_eop;
+        IF UNSIGNED(block_size_cnt) >= g_block_size - 3 THEN
+          nxt_state <= s_dp_on_eop;
         END IF;
 
       WHEN s_dp_on_eop =>
-        nxt_src_out.eop    <= '1';
-        nxt_src_out.valid  <= '1'; 
-        nxt_state          <= s_dp_on_sop;
-        nxt_block_size_cnt <= INCR_UVEC(block_size_cnt, 1);
+        -- End of block
+        nxt_src_out.eop <= '1';
+        nxt_src_out.valid <= '1';
+        nxt_state <= s_dp_on_sop;
+        -- block_size_cnt is dont care at at src_out.eop
+        -- accept dp_off after eop, to avoid fractional blocks
         IF dp_on = '0' THEN
           nxt_state <= s_dp_off;
         END IF;
 
-      WHEN OTHERS => -- s_init
+      WHEN OTHERS =>  -- reover from undefined state
         nxt_state <= s_dp_off;
-      
     END CASE;
   END PROCESS;
 
+  -- src_out.valid transition from 0 to 1 is a bs_restart, use nxt_bs_restart
+  -- to have bs_restart at first src_out.sync and src_out.sop.
+  nxt_bs_restart <= nxt_src_out.valid AND NOT i_src_out.valid;
+
   p_clk : PROCESS(rst, clk)
   BEGIN
     IF rst='1' THEN
-      prev_state     <= s_init;
-      state          <= s_init;
-      i_src_out      <= c_dp_sosi_rst;
-      clk_cnt        <= (OTHERS=>'0');
-      sync           <= '0'; 
-      block_size_cnt <= (OTHERS=>'0');
-      i_dp_on_status <= '0';
-      bs_restart     <= '0';
+      prev_state          <= s_dp_off;
+      state               <= s_dp_off;
+      i_src_out           <= c_dp_sosi_rst;
+      sync_size_cnt       <= (OTHERS=>'0');
+      sync                <= '0';
+      block_size_cnt      <= (OTHERS=>'0');
+      bs_restart          <= '0';
       bsn_time_offset_cnt <= (OTHERS=>'0');
     ELSIF rising_edge(clk) THEN
-      prev_state     <= state;
-      state          <= nxt_state;
-      i_src_out      <= nxt_src_out;
-      clk_cnt        <= nxt_clk_cnt;
-      sync           <= nxt_sync;
-      block_size_cnt <= nxt_block_size_cnt;
-      i_dp_on_status <= nxt_dp_on_status;
-      bs_restart     <= nxt_bs_restart;
+      prev_state          <= state;
+      state               <= nxt_state;
+      i_src_out           <= nxt_src_out;
+      sync_size_cnt       <= nxt_sync_size_cnt;
+      sync                <= nxt_sync;
+      block_size_cnt      <= nxt_block_size_cnt;
+      bs_restart          <= nxt_bs_restart;
       bsn_time_offset_cnt <= nxt_bsn_time_offset_cnt;
-      i_current_bsn_time_offset <= nxt_current_bsn_time_offset;
     END IF;
   END PROCESS;
 
-- 
GitLab