From beb0444083407adc7db60ce1007ae28edf5ba2d0 Mon Sep 17 00:00:00 2001
From: Eric Kooistra <kooistra@astron.nl>
Date: Tue, 21 Sep 2021 17:49:50 +0200
Subject: [PATCH] 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.
---
.../dp/src/vhdl/dp_bsn_sync_scheduler.vhd | 43 +++++++++++++++----
.../dp/tb/vhdl/tb_dp_bsn_sync_scheduler.vhd | 20 +++++++--
2 files changed, 52 insertions(+), 11 deletions(-)
diff --git a/libraries/base/dp/src/vhdl/dp_bsn_sync_scheduler.vhd b/libraries/base/dp/src/vhdl/dp_bsn_sync_scheduler.vhd
index 9c88855862..0d5b21ea61 100644
--- a/libraries/base/dp/src/vhdl/dp_bsn_sync_scheduler.vhd
+++ b/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
diff --git a/libraries/base/dp/tb/vhdl/tb_dp_bsn_sync_scheduler.vhd b/libraries/base/dp/tb/vhdl/tb_dp_bsn_sync_scheduler.vhd
index a94570c92e..67939303a1 100644
--- a/libraries/base/dp/tb/vhdl/tb_dp_bsn_sync_scheduler.vhd
+++ b/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;
--
GitLab