Improved comments. Use r.enable instead of v.enable to ease timing closure....

Improved comments. Use r.enable instead of v.enable to ease timing closure. This required using expected_out_enable2 in tb for g_block_size = 2.

libraries/base/dp/src/vhdl/dp_bsn_sync_scheduler.vhd

@@ -38,7 +38,7 @@
 -- * ctrl_enable:
 --   The output is enabled at the ctrl_start_bsn when ctrl_enable = '1' and the
 --   output is disable after an in_sosi.eop when ctrl_enable = '0'. If the
---   output is diabled, then the sosai control fields are forced to '0', the
+--   output is diabled, then the sosi control fields are forced to '0', the
 --   other sosi fields of the in_sosi are passed on to the out_sosi.
 -- * mon_current_input_bsn:
 --   The user can read mon_current_input_bsn to determine a suitable
@@ -195,13 +195,19 @@ BEGIN
     output_start <= '0';
     output_sync <= '0';
 
+    ---------------------------------------------------------------------------
     -- Detect ctrl_enable rising event
+    ---------------------------------------------------------------------------
     IF ctrl_enable = '1' AND ctrl_enable_evt = '1' THEN
       v.enable_init := '1';
     END IF;
 
-    -- Initialization: calculate number of blocks per output sync interval
-    -- . use r.enable_init instead of v.enable_init to easy timing closure and
+    ---------------------------------------------------------------------------
+    -- Initialization:
+    --   calculate number of blocks per output sync interval, start with
+    --   nof_blk_max in first sync interval
+    ---------------------------------------------------------------------------
+    -- . use r.enable_init instead of v.enable_init to ease timing closure and
     --   because functionally it makes no difference.
     IF r.enable_init = '1' THEN
       -- Assume ctrl_start_bsn is scheduled more than nof_blk block clk cycles
@@ -239,7 +245,9 @@ BEGIN
       v.blk_cnt := 0;
     END IF;
 
-    -- Enable / disable control
+    ---------------------------------------------------------------------------
+    -- Enable / re-enable / disable control
+    ---------------------------------------------------------------------------
     IF ctrl_enable = '0' THEN
       -- Disable output when ctrl_enable requests disable.
       v.enable := '0';
@@ -260,6 +268,12 @@ BEGIN
       END IF;
     END IF;
 
+    ---------------------------------------------------------------------------
+    -- Output enable / disable
+    -- . enable at start_bsn and issue output_start pulse for first output
+    --   sync interval
+    -- . disable after input block has finished.
+    ---------------------------------------------------------------------------
     -- Hold input eop to detect when input has finished a block and to detect
     -- gaps between in_sosi.eop and in_sosi.sop
     IF in_sosi.sop = '1' THEN
@@ -269,7 +283,9 @@ BEGIN
       v.hold_eop := '1';
     END IF;
 
-    IF v.enable = '1' THEN
+    -- . use r.enable instead of v.enable to ease timing closure and because
+    --   functionally it makes no difference.
+    IF r.enable = '1' THEN
       -- Output enable at in_sosi.sop start_bsn
       IF in_sosi.sop = '1' THEN
         IF UNSIGNED(in_sosi.bsn) = UNSIGNED(r.start_bsn) THEN
@@ -285,6 +301,9 @@ BEGIN
       END IF;
     END IF;
 
+    ---------------------------------------------------------------------------
+    -- Issue output_sync at output_sync_bsn and request next output_sync_bsn
+    ---------------------------------------------------------------------------
     -- Generate output sync interval based on input BSN and ctrl_interval_size
     IF v.output_enable = '1' THEN
       IF in_sosi.sop = '1' THEN
@@ -300,7 +319,9 @@ BEGIN
       END IF;
     END IF;
 
-    -- Determine BSN for next output sync
+    ---------------------------------------------------------------------------
+    -- Determine BSN for next output_sync (= output_sync_bsn)
+    ---------------------------------------------------------------------------
     IF r.update_bsn = '1' THEN
       -- Similar code as in proc_dp_verify_sync(), the difference is that:
       -- . Here r.extra is number of extra samples in nof_blk_max compared to
@@ -321,8 +342,9 @@ BEGIN
       -- Assume output_sync_bsn is in future
       v.update_bsn := '0';
 
-      -- else: last r.input_bsn will be used to keep update_bsn active for
-      -- more clk cycles to catch up for lost input blocks.
+      -- If output_sync_bsn is still in past, due to lost input blocks, then
+      -- last r.input_bsn will be used to keep update_bsn active for more clk
+      -- cycles to catch up for lost input blocks.
     END IF;
 
     -- Hold input bsn
@@ -346,7 +368,10 @@ BEGIN
     nxt_r <= v;
   END PROCESS;
 
+  -----------------------------------------------------------------------------
   -- Output in_sosi with programmed sync interval or disable the output
+  -----------------------------------------------------------------------------
+  -- . note this is part of p_comb, but using a separate process is fine too.
   p_output_sosi : PROCESS(in_sosi, nxt_r, output_sync)
   BEGIN
     output_sosi <= in_sosi;
@@ -360,7 +385,9 @@ BEGIN
     END IF;
   END PROCESS;
 
+  -----------------------------------------------------------------------------
   -- Pipeline output to avoid timing closure problems due to use of nxt_r.output_enable
+  -----------------------------------------------------------------------------
   u_out_sosi : ENTITY work.dp_pipeline
   GENERIC MAP (
     g_pipeline  => g_pipeline

libraries/base/dp/tb/vhdl/tb_dp_bsn_sync_scheduler.vhd

@@ -78,8 +78,8 @@ ENTITY tb_dp_bsn_sync_scheduler IS
     -- Input sync period and sosi ctrl
     g_nof_input_sync               : NATURAL := 10;
     g_nof_block_per_input_sync     : NATURAL := 17;
-    g_block_size                   : NATURAL := 10;
-    g_input_gap_size               : NATURAL := 3;
+    g_block_size                   : NATURAL := 2;
+    g_input_gap_size               : NATURAL := 0;
 
     -- Output sync period
     g_nof_samples_per_output_sync  : NATURAL := 45  -- 45 / g_block_size = 4.5
@@ -161,6 +161,8 @@ ARCHITECTURE tb OF tb_dp_bsn_sync_scheduler IS
   SIGNAL prev_out_enable       : STD_LOGIC := '0';
   SIGNAL pending_out_disable   : STD_LOGIC := '0';
   SIGNAL expected_out_enable   : STD_LOGIC := '0';
+  SIGNAL expected_out_enable1  : STD_LOGIC := '0';
+  SIGNAL expected_out_enable2  : STD_LOGIC := '0';
   SIGNAL expecting_out_start   : STD_LOGIC := '0';
   SIGNAL hold_out_eop          : STD_LOGIC := '0';
   SIGNAL hold_out_sop          : STD_LOGIC := '0';
@@ -388,6 +390,9 @@ BEGIN
     END IF;
   END PROCESS;
 
+  expected_out_enable1 <= expected_out_enable  WHEN rising_edge(clk);
+  expected_out_enable2 <= expected_out_enable1 WHEN rising_edge(clk);
+
   p_verify_out_enable : PROCESS(clk)
   BEGIN
     -- Use registered values to compare, to avoid combinatorial differences
@@ -397,7 +402,16 @@ BEGIN
     IF rising_edge(clk) THEN
       IF out_enable /= expected_out_enable THEN
         IF out_enable = '1' THEN
-          REPORT "Unexpected enabled out_enable" SEVERITY ERROR;
+          IF g_block_size > 2 THEN
+            REPORT "Unexpected enabled out_enable" SEVERITY ERROR;
+          ELSIF out_enable /= expected_out_enable2 THEN
+            -- For g_block_size = 2 the use of r.enable (instead of v.enable)
+            -- in dp_bsn_sync_scheduler.vhd causes that the output can stay
+            -- enabled 2 cycles longer, which is ok. Using v.enable does
+            -- avoid this need to use expected_out_enable2, but for timing
+            -- closure it is preferred to use r.enable.
+            REPORT "Unexpected enabled out_enable2" SEVERITY ERROR;
+          END IF;
         ELSE
           REPORT "Unexpected disabled out_enable" SEVERITY ERROR;
         END IF;