From c175e0c7c6a8b32abc7bb37d17952f3ff5de578e Mon Sep 17 00:00:00 2001 From: Eric Kooistra <kooistra@astron.nl> Date: Fri, 8 Oct 2021 14:09:53 +0200 Subject: [PATCH] Added g_pipeline_output = 1 for out_sosi_arr. --- .../base/dp/src/vhdl/dp_bsn_align_v2.vhd | 50 ++++++++----- .../base/dp/tb/vhdl/tb_dp_bsn_align_v2.vhd | 71 +++++++++++-------- .../base/dp/tb/vhdl/tb_tb_dp_bsn_align_v2.vhd | 32 +++++---- 3 files changed, 93 insertions(+), 60 deletions(-) diff --git a/libraries/base/dp/src/vhdl/dp_bsn_align_v2.vhd b/libraries/base/dp/src/vhdl/dp_bsn_align_v2.vhd index ca7a757abe..e94cd4efd0 100644 --- a/libraries/base/dp/src/vhdl/dp_bsn_align_v2.vhd +++ b/libraries/base/dp/src/vhdl/dp_bsn_align_v2.vhd @@ -65,7 +65,8 @@ ENTITY dp_bsn_align_v2 IS g_data_w : NATURAL; -- number of bits in sosi data g_data_replacement_value : INTEGER := 0; -- output sosi data value for missing input blocks g_use_mm_output : BOOLEAN := FALSE; -- output via MM or via streaming DP - g_pipeline_input : NATURAL := 1; -- >= 0, choose 0 for wires, choose 1 to ease timing closure + g_pipeline_input : NATURAL := 1; -- >= 0, choose 0 for wires, choose 1 to ease timing closure of in_sosi_arr + g_pipeline_output : NATURAL := 1; -- >= 0, choose 0 for wires, choose 1 to ease timing closure of out_sosi_arr g_rd_latency : NATURAL := 2 -- 1 or 2, choose 2 to ease timing closure ); PORT ( @@ -170,24 +171,25 @@ ARCHITECTURE rtl OF dp_bsn_align_v2 IS (OTHERS=>(OTHERS=>'0'))); -- State registers for p_comb - SIGNAL r : t_reg; - SIGNAL nxt_r : t_reg; + SIGNAL r : t_reg; + SIGNAL nxt_r : t_reg; -- Memoryless signals in p_comb (wires used as local variables) - SIGNAL dbg_wires : t_comb; + SIGNAL dbg_wires : t_comb; -- Structural signals (wires used to connect components and IO) - SIGNAL dp_done : STD_LOGIC; - SIGNAL dp_done_arr : STD_LOGIC_VECTOR(g_nof_streams-1 DOWNTO 0); - SIGNAL dp_copi : t_mem_copi; - SIGNAL dp_copi_arr : t_mem_copi_arr(g_nof_streams-1 DOWNTO 0); + SIGNAL dp_done : STD_LOGIC; + SIGNAL dp_done_arr : STD_LOGIC_VECTOR(g_nof_streams-1 DOWNTO 0); + SIGNAL dp_copi : t_mem_copi; + SIGNAL dp_copi_arr : t_mem_copi_arr(g_nof_streams-1 DOWNTO 0); - SIGNAL rd_sosi_arr : t_dp_sosi_arr(g_nof_streams-1 DOWNTO 0); - SIGNAL rd_cipo_arr : t_mem_cipo_arr(g_nof_streams-1 DOWNTO 0) := (OTHERS=>c_mem_cipo_rst); + SIGNAL rd_sosi_arr : t_dp_sosi_arr(g_nof_streams-1 DOWNTO 0); + SIGNAL rd_cipo_arr : t_mem_cipo_arr(g_nof_streams-1 DOWNTO 0) := (OTHERS=>c_mem_cipo_rst); -- Pipeline registers - SIGNAL in_sosi_arr_p : t_dp_sosi_arr(g_nof_streams-1 DOWNTO 0); - SIGNAL rd_copi : t_mem_copi; + SIGNAL in_sosi_arr_p : t_dp_sosi_arr(g_nof_streams-1 DOWNTO 0); + SIGNAL rd_copi : t_mem_copi; + SIGNAL comb_out_sosi_arr : t_dp_sosi_arr(g_nof_streams-1 DOWNTO 0); -- Debug signals SIGNAL dbg_nof_streams : NATURAL := g_nof_streams; @@ -205,7 +207,8 @@ ARCHITECTURE rtl OF dp_bsn_align_v2 IS BEGIN - mm_sosi <= r.mm_sosi WHEN g_use_mm_output = TRUE ELSE c_dp_sosi_rst; + -- Output mm_sosi, also when g_use_mm_output = FALSE. + mm_sosi <= r.mm_sosi; p_reg : PROCESS(dp_clk, dp_rst) BEGIN @@ -328,7 +331,7 @@ BEGIN mm_cipo_arr <= v.fill_cipo_arr; -- . no output via DP streaming interface - out_sosi_arr <= (OTHERS => c_dp_sosi_rst); + comb_out_sosi_arr <= (OTHERS => c_dp_sosi_rst); ELSE -------------------------------------------------------------------------- -- Do the output via the DP streaming interface @@ -370,7 +373,7 @@ BEGIN END IF; -- . output via DP streaming interface - out_sosi_arr <= w.out_sosi_arr; + comb_out_sosi_arr <= w.out_sosi_arr; -- . no output via MM interface mm_cipo_arr <= (OTHERS => c_mem_cipo_rst); @@ -445,7 +448,7 @@ BEGIN -- Pipelining ------------------------------------------------------------------------------ - -- . input + -- . input streams u_in_sosi_arr_p : ENTITY work.dp_pipeline_arr GENERIC MAP ( g_nof_streams => g_nof_streams, @@ -463,4 +466,19 @@ BEGIN -- . read RAM rd_copi <= nxt_r.rd_copi WHEN g_rd_latency = 1 ELSE r.rd_copi; + -- . output streams + u_out_sosi_arr_p : ENTITY work.dp_pipeline_arr + GENERIC MAP ( + g_nof_streams => g_nof_streams, + g_pipeline => g_pipeline_output + ) + PORT MAP ( + rst => dp_rst, + clk => dp_clk, + -- ST sink + snk_in_arr => comb_out_sosi_arr, + -- ST source + src_out_arr => out_sosi_arr + ); + END rtl; diff --git a/libraries/base/dp/tb/vhdl/tb_dp_bsn_align_v2.vhd b/libraries/base/dp/tb/vhdl/tb_dp_bsn_align_v2.vhd index cc02e72100..1c464c468c 100644 --- a/libraries/base/dp/tb/vhdl/tb_dp_bsn_align_v2.vhd +++ b/libraries/base/dp/tb/vhdl/tb_dp_bsn_align_v2.vhd @@ -38,25 +38,27 @@ -- Remark: -- For this BSN aligner component it was essential to have an almost -- complete, reviewed, detailed design document, because it is a complex --- component. The clear design made it much easier to achieve a draft --- implementation that was almost correct. For the DUT it implementation --- it was also essential to use the p_reg, p_comb coding template. +-- component. Main difference after review was addding build in support +-- for streaming output via g_use_mm_output. The clear design made it +-- much easier to achieve a draft implementation that was almost correct. +-- For the DUT it implementation it was also essential to use the p_reg, +-- p_comb coding template. -- The initial DUT implementation did not change much anymore after the -- first tb tests, expect for some small (but important) corrections. -- Each feature of the tb took several hours or up to a day to add. Much --- time was also spent to clean up and simply the code. Hence implementing --- the tb took more time than the DUT, but the design document and the --- especially the design decisions took the most time. The last tb --- feature to verify a chain of DUTs is realy nice to see, e.g. by --- expanding dut_out_sosi_2_arr in the Wave window. The ascii drawing at --- gen_bsn_align_chain was used to more easily connect and clarify the --- wiring for the chain of DUTs. +-- time was also spent to regularly clean up and simply the code. The +-- last tb feature that verifies a chain of DUTs is realy nice to see, +-- e.g. by expanding dut_out_sosi_2_arr in the Wave window. The ascii +-- drawing at gen_bsn_align_chain was used to more easily connect and +-- clarify the wiring for the chain of DUTs. +-- Implementing the tb took more time than the DUT, but the design +-- document and the especially the design decisions took the most time. -- The design decision to use a circular buffer instead of FIFOs and the -- design decision to rely on a local reference and fixed latencies were --- important. Initial prestudy was done based on experience with aligning --- streams in LOFAR1 and with bsn_aligner.vhd in APERTIF. Then about --- year later the detailed design and implementation were done. --- . prestudy ~ 1 week end 2020 +-- important. Initial prestudy end 2020 was done based on experience with +-- aligning streams in LOFAR1 and with the bsn_aligner.vhd in APERTIF. +-- About a in autumn 2021 the detailed design and implementation were done. +-- . prestudy ~ 1 week -- . design decisions doc ~ 1 week (based on prestudy) -- . detailed design doc ~ 1 week -- . review and process review ~ 2 days @@ -64,6 +66,7 @@ -- . implement tb (and adding to tb_tb) ~ 1 week -- . implement tb_mmp ~ 1 day -- Total: ~ 6 weeks +-- -- Usage: -- > as 10 -- > run -all @@ -94,7 +97,8 @@ ENTITY tb_dp_bsn_align_v2 IS g_lost_stream_id : NATURAL := 0; -- default 0 to have all streams, > 0 selects stream that will be lost g_lost_bsn_id : NATURAL := 0; -- for stream 1 the block with bsn = g_lost_bsn_id will be lost g_use_mm_output : BOOLEAN := FALSE; -- output via MM or via streaming DP - g_pipeline_input : NATURAL := 0; -- >= 0, choose 0 for wires, choose 1 to ease timing closure + g_pipeline_input : NATURAL := 0; -- >= 0, choose 0 for wires, choose 1 to ease timing closure of in_sosi_arr + g_pipeline_output : NATURAL := 0; -- >= 0, choose 0 for wires, choose 1 to ease timing closure of out_sop_arr g_rd_latency : NATURAL := 1; -- 1 or 2, choose 2 to ease timing closure -- TB @@ -124,16 +128,16 @@ ARCHITECTURE tb OF tb_dp_bsn_align_v2 IS CONSTANT c_diff_delay_max : NATURAL := g_bsn_latency_max * g_block_period - sel_a_b(g_rd_latency > 1, 0, 1); CONSTANT c_diff_delay : NATURAL := sel_a_b(g_tb_diff_delay < 0, c_diff_delay_max, g_tb_diff_delay); - CONSTANT c_gap_size : NATURAL := g_block_period - g_block_size; - - CONSTANT c_lost_bsn_stream_id : NATURAL := 1; -- fixed use stream 1 to verify g_lost_bsn_id - -- Return input delay as function of inputs stream index I FUNCTION func_input_delay(I : NATURAL) RETURN NATURAL IS BEGIN RETURN c_diff_delay * I / (g_nof_streams - 1); END; + CONSTANT c_gap_size : NATURAL := g_block_period - g_block_size; + + CONSTANT c_lost_bsn_stream_id : NATURAL := 1; -- fixed use stream 1 to verify g_lost_bsn_id + -- In the tb only support MM interface verification for c_nof_aligners_max = 1 CONSTANT c_nof_aligners_max : POSITIVE := sel_a_b(g_use_mm_output, 1, g_nof_aligners_max); @@ -141,7 +145,7 @@ ARCHITECTURE tb OF tb_dp_bsn_align_v2 IS -- independent c_nof_aligners_max. This is because the c_dut_latency of the -- other DUTs is covered by the buffer latency. CONSTANT c_mm_to_dp_latency : NATURAL := 1; - CONSTANT c_dut_latency : NATURAL := g_pipeline_input + g_rd_latency + c_mm_to_dp_latency; + CONSTANT c_dut_latency : NATURAL := g_pipeline_input + g_rd_latency + c_mm_to_dp_latency + g_pipeline_output; -- DUT buffer latency for chain of DUTs CONSTANT c_align_latency_nof_blocks : NATURAL := g_bsn_latency_max * c_nof_aligners_max; -- in number blocks @@ -237,9 +241,16 @@ ARCHITECTURE tb OF tb_dp_bsn_align_v2 IS SIGNAL expected_out_data_arr : t_data_arr; SIGNAL expected_out_channel_arr : t_channel_arr; + -- Debug signals to view in Wave window SIGNAL dbg_func_delay_max : NATURAL := func_input_delay(g_nof_streams - 1); - -- Debug signals to view in Wave window that verification conditions actually occur + SIGNAL dbg_c_align_latency_nof_blocks : NATURAL := c_align_latency_nof_blocks; + SIGNAL dbg_c_align_latency_nof_valid : NATURAL := c_align_latency_nof_valid; + SIGNAL dbg_c_align_latency_nof_clk : NATURAL := c_align_latency_nof_clk; + SIGNAL dbg_c_total_latency : NATURAL := c_total_latency; + SIGNAL dbg_c_verify_nof_blocks : NATURAL := c_verify_nof_blocks; + + -- Debug signals to view that verification conditions actually occur SIGNAL dbg_verify_sosi_control_arr : STD_LOGIC_VECTOR(g_nof_streams-1 DOWNTO 0) := (OTHERS => '0'); -- '1' when sosi control is verified SIGNAL dbg_verify_passed_on_data_arr : STD_LOGIC_VECTOR(g_nof_streams-1 DOWNTO 0) := (OTHERS => '0'); -- '1' when passed on data is verified SIGNAL dbg_verify_replaced_data_arr : STD_LOGIC_VECTOR(g_nof_streams-1 DOWNTO 0) := (OTHERS => '0'); -- '1' when replaced data is verified @@ -257,7 +268,7 @@ BEGIN ------------------------------------------------------------------------------ -- Generate data path input data - gen_input : FOR I IN g_nof_streams-1 DOWNTO 0 GENERATE + gen_stimuli : FOR I IN g_nof_streams-1 DOWNTO 0 GENERATE p_stimuli : PROCESS VARIABLE v_sync : STD_LOGIC := '0'; VARIABLE v_bsn : NATURAL; @@ -425,7 +436,7 @@ BEGIN exp_bsn_lost_arr(c_lost_bsn_stream_id) <= '1' WHEN TO_UINT(out_sosi_arr_exp(c_lost_bsn_stream_id).bsn) = g_lost_bsn_id ELSE '0'; gen_verify_streams : FOR I IN g_nof_streams-1 DOWNTO 0 GENERATE - p_verify_sosi : PROCESS(clk) + p_verify_stream : PROCESS(clk) BEGIN IF rising_edge(clk) THEN dbg_verify_sosi_control_arr(I) <= '0'; @@ -495,9 +506,10 @@ BEGIN g_bsn_w => g_bsn_w, g_data_w => g_data_w, g_data_replacement_value => g_data_replacement_value, - g_use_mm_output => g_use_mm_output, -- output via MM or via streaming DP - g_pipeline_input => g_pipeline_input, -- >= 0, choose 0 for wires, choose 1 to ease timing closure - g_rd_latency => g_rd_latency -- 1 or 2, choose 2 to ease timing closure + g_use_mm_output => g_use_mm_output, + g_pipeline_input => g_pipeline_input, + g_pipeline_output => g_pipeline_output, + g_rd_latency => g_rd_latency ) PORT MAP ( dp_rst => rst, @@ -546,9 +558,10 @@ BEGIN g_bsn_w => g_bsn_w, g_data_w => g_data_w, g_data_replacement_value => g_data_replacement_value, - g_use_mm_output => g_use_mm_output, -- output via MM or via streaming DP - g_pipeline_input => g_pipeline_input, -- >= 0, choose 0 for wires, choose 1 to ease timing closure - g_rd_latency => g_rd_latency -- 1 or 2, choose 2 to ease timing closure + g_use_mm_output => g_use_mm_output, + g_pipeline_input => g_pipeline_input, + g_pipeline_output => g_pipeline_output, + g_rd_latency => g_rd_latency ) PORT MAP ( dp_rst => rst, diff --git a/libraries/base/dp/tb/vhdl/tb_tb_dp_bsn_align_v2.vhd b/libraries/base/dp/tb/vhdl/tb_tb_dp_bsn_align_v2.vhd index f4d4198a40..dfafc97e2f 100644 --- a/libraries/base/dp/tb/vhdl/tb_tb_dp_bsn_align_v2.vhd +++ b/libraries/base/dp/tb/vhdl/tb_tb_dp_bsn_align_v2.vhd @@ -55,7 +55,8 @@ BEGIN -- g_lost_stream_id : NATURAL := 0; -- default 0 to have all streams, > 0 selects stream that will be lost -- g_lost_bsn_id : NATURAL := 10; -- for stream 1 the block with bsn = g_lost_bsn_id will be lost -- g_use_mm_output : BOOLEAN := FALSE; -- output via MM or via streaming DP - -- g_pipeline_input : NATURAL := 1; -- >= 0, choose 0 for wires, choose 1 to ease timing closure + -- g_pipeline_input : NATURAL := 0; -- >= 0, choose 0 for wires, choose 1 to ease timing closure of in_sosi_arr + -- g_pipeline_output : NATURAL := 0; -- >= 0, choose 0 for wires, choose 1 to ease timing closure of out_sop_arr -- g_rd_latency : NATURAL := 2; -- 1 or 2, choose 2 to ease timing closure -- -- -- TB @@ -64,19 +65,20 @@ BEGIN -- g_tb_nof_restart : NATURAL := 1; -- number of times to restart the input stimuli -- g_tb_nof_blocks : NATURAL := 10 -- number of input blocks per restart - u_mm_output : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, TRUE, 0, 1, 0, 2, c_nof_blk); - u_dp_output : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 1, 0, 2, c_nof_blk); - u_bsn_lat_max_2 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 2, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 1, 0, 2, c_nof_blk); - u_bsn_lat_max_3 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 3, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 1, 0, 2, c_nof_blk); - u_p1_rd2 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 1, 2, 0, 2, c_nof_blk); - u_zero_gap : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_block, 32, 16, 17, 0, 0, 0, FALSE, 0, 1, 0, 2, c_nof_blk); - u_zero_gap_p1_rd2 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_block, 32, 16, 17, 0, 0, 0, FALSE, 1, 2, 0, 2, c_nof_blk); - u_stream_disable : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (3, 1, 1, c_block, c_period, 32, 16, 17, 2, 0, 0, FALSE, 0, 1, 0, 2, c_nof_blk); - u_stream_lost : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (3, 1, 1, c_block, c_period, 32, 16, 17, 0, 2, 0, FALSE, 0, 1, 0, 2, c_nof_blk); - u_stream_disable_lost : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (4, 1, 1, c_block, c_period, 32, 16, 17, 1, 2, 0, FALSE, 0, 1, 0, 2, c_nof_blk); - u_bsn_lost : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (3, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 10, FALSE, 0, 1, 0, 2, c_nof_blk); - u_diff_delay : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (3, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 1, -1, 2, c_nof_blk); - u_nof_aligners : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 8, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 1, 0, 2, c_nof_blk); - u_nof_aligners_diff_delay : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (4, 1, 3, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 1, -1, 2, c_nof_blk); + u_mm_output : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, TRUE, 0, 0, 1, 0, 2, c_nof_blk); + u_dp_output : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_dp_output_p1 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 1, 1, 1, 0, 2, c_nof_blk); + u_bsn_lat_max_2 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 2, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_bsn_lat_max_3 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 3, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_p1_rd2 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 1, 0, 2, 0, 2, c_nof_blk); + u_zero_gap : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_block, 32, 16, 17, 0, 0, 0, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_zero_gap_p1_rd2 : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 1, c_block, c_block, 32, 16, 17, 0, 0, 0, FALSE, 1, 1, 2, 0, 2, c_nof_blk); + u_stream_disable : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (3, 1, 1, c_block, c_period, 32, 16, 17, 2, 0, 0, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_stream_lost : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (3, 1, 1, c_block, c_period, 32, 16, 17, 0, 2, 0, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_stream_disable_lost : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (4, 1, 1, c_block, c_period, 32, 16, 17, 1, 2, 0, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_bsn_lost : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (3, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 10, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_diff_delay : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (3, 1, 1, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 0, 1, -1, 2, c_nof_blk); + u_nof_aligners : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (2, 1, 8, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 0, 1, 0, 2, c_nof_blk); + u_nof_aligners_diff_delay : ENTITY work.tb_dp_bsn_align_v2 GENERIC MAP (4, 1, 3, c_block, c_period, 32, 16, 17, 0, 0, 0, FALSE, 0, 0, 1, -1, 2, c_nof_blk); END tb; -- GitLab