From 414dbd2fa7773f62d6b240f3c34e1bc35744b70a Mon Sep 17 00:00:00 2001
From: Eric Kooistra <kooistra@astron.nl>
Date: Wed, 9 Aug 2023 12:02:40 +0200
Subject: [PATCH] Add merged payload error bit support in design and in tb.

---
 .../tb_lofar2_unb2c_sdp_station_bf.vhd        |  38 +++++-
 .../tb_lofar2_unb2c_sdp_station_bf_ring.vhd   |   5 +-
 .../sdp/src/vhdl/sdp_beamformer_output.vhd    | 123 ++++++++++++------
 3 files changed, 120 insertions(+), 46 deletions(-)

diff --git a/applications/lofar2/designs/lofar2_unb2c_sdp_station/revisions/lofar2_unb2c_sdp_station_bf/tb_lofar2_unb2c_sdp_station_bf.vhd b/applications/lofar2/designs/lofar2_unb2c_sdp_station/revisions/lofar2_unb2c_sdp_station_bf/tb_lofar2_unb2c_sdp_station_bf.vhd
index 6deb4c7bde..a925ba49ae 100644
--- a/applications/lofar2/designs/lofar2_unb2c_sdp_station/revisions/lofar2_unb2c_sdp_station_bf/tb_lofar2_unb2c_sdp_station_bf.vhd
+++ b/applications/lofar2/designs/lofar2_unb2c_sdp_station/revisions/lofar2_unb2c_sdp_station_bf/tb_lofar2_unb2c_sdp_station_bf.vhd
@@ -111,6 +111,9 @@
 --   are default 0, but in sim the BF weights in node_sdp_beamformer.vhd
 --   are default unit weights. Therefore also write the BF weight for default
 --   beamlet 102 to define it value, in case g_beamlet /= 102.
+-- * A simulation only section in sdp_beamformer_output.vhd disturbs the BSN,
+--   to cause a merged payload error, so that sdp_source_info_payload_error
+--   can be verified here.
 --
 -- Usage:
 --   > as 7    # default
@@ -436,6 +439,7 @@ architecture tb of tb_lofar2_unb2c_sdp_station_bf is
   signal rx_sdp_cep_header   : t_sdp_cep_header;
   signal exp_sdp_cep_header  : t_sdp_cep_header;
   signal exp_dp_bsn          : natural;
+  signal exp_payload_error   : std_logic := '0';
 
   -- Beamlets packets data
   signal rx_beamlet_data     : std_logic_vector(c_longword_w - 1 downto 0);  -- 64 bit
@@ -1245,10 +1249,10 @@ begin
 
   -- Prepare exp_sdp_cep_header before rx_beamlet_sosi.eop, so that
   -- p_verify_cep_header can verify it at rx_beamlet_sosi.eop.
-
   exp_sdp_cep_header <= func_sdp_compose_cep_header(c_exp_ip_header_checksum,
                                                     c_exp_sdp_info,
                                                     c_gn_index,
+                                                    exp_payload_error,
                                                     c_exp_beamlet_scale,
                                                     c_exp_beamlet_index,
                                                     exp_dp_bsn);
@@ -1256,15 +1260,39 @@ begin
   rx_sdp_cep_header <= func_sdp_map_cep_header(rx_hdr_fields_raw);
 
   p_verify_cep_header : process
-    variable v_bool : boolean;
+    variable v_pkt_cnt : natural;
+    variable v_new_pkt : boolean;
+    variable v_error   : std_logic := '0';
+    variable v_bsn     : natural := 0;
+    variable v_bool    : boolean;
   begin
     wait until rising_edge(ext_clk);
+    -- Count packets per beamset
+    v_pkt_cnt := rx_beamlet_sop_cnt / c_sdp_N_beamsets;
+    v_new_pkt := rx_beamlet_sop_cnt mod c_sdp_N_beamsets = 0;
 
     -- Prepare exp_sdp_cep_header at sop, so that it can be verified at eop
     if rx_beamlet_sosi.sop = '1' then
-      -- Expected BSN increments by c_sdp_cep_nof_blocks_per_packet = 4 blocks per packet
-      if rx_beamlet_sop_cnt mod c_sdp_N_beamsets = 0 then
-        exp_dp_bsn <= c_init_bsn + (rx_beamlet_sop_cnt / c_sdp_N_beamsets) * c_sdp_cep_nof_blocks_per_packet;
+      -- Expected BSN increments by c_sdp_cep_nof_blocks_per_packet = 4 blocks per packet,
+      -- both beamsets are outputting packets.
+      if v_new_pkt then
+        -- Default expected
+        v_error := '0';
+        v_bsn := c_init_bsn + v_pkt_cnt * c_sdp_cep_nof_blocks_per_packet;
+
+        -- Expected due to bsn and payload_error stimuli in sdp_beamformer_output.vhd.
+        if v_pkt_cnt = 1 then
+          v_error := '1';
+        elsif v_pkt_cnt = 2 or v_pkt_cnt = 3 then
+          v_bsn := v_bsn + 1;
+        elsif v_pkt_cnt = 4 then
+          v_bsn := v_bsn + 1;
+          v_error := '1';
+        end if;
+
+        -- Apply expected values
+        exp_payload_error <= v_error;
+        exp_dp_bsn <= v_bsn;
       end if;
     end if;
 
diff --git a/applications/lofar2/designs/lofar2_unb2c_sdp_station/revisions/lofar2_unb2c_sdp_station_bf_ring/tb_lofar2_unb2c_sdp_station_bf_ring.vhd b/applications/lofar2/designs/lofar2_unb2c_sdp_station/revisions/lofar2_unb2c_sdp_station_bf_ring/tb_lofar2_unb2c_sdp_station_bf_ring.vhd
index bf565dbc2e..8e15f495d0 100644
--- a/applications/lofar2/designs/lofar2_unb2c_sdp_station/revisions/lofar2_unb2c_sdp_station_bf_ring/tb_lofar2_unb2c_sdp_station_bf_ring.vhd
+++ b/applications/lofar2/designs/lofar2_unb2c_sdp_station/revisions/lofar2_unb2c_sdp_station_bf_ring/tb_lofar2_unb2c_sdp_station_bf_ring.vhd
@@ -439,6 +439,7 @@ architecture tb of tb_lofar2_unb2c_sdp_station_bf_ring is
   signal rx_sdp_cep_header   : t_sdp_cep_header;
   signal exp_sdp_cep_header  : t_sdp_cep_header;
   signal exp_dp_bsn          : natural;
+  signal exp_payload_error   : std_logic := '0';
 
   -- Beamlets packets data
   signal rx_beamlet_data     : std_logic_vector(c_longword_w - 1 downto 0);  -- 64 bit
@@ -577,7 +578,6 @@ begin
   i_QSFP_0_RX(0)         <= i_QSFP_0_TX(c_last_rn);
   i_QSFP_0_RX(c_last_rn) <= i_QSFP_0_TX(0);
 
-
   ------------------------------------------------------------------------------
   -- CEP model
   ------------------------------------------------------------------------------
@@ -721,7 +721,6 @@ begin
     --     END RECORD;
     -- . Write
 
-
     for RN in 0 to c_last_rn loop
       v_gn := g_first_gn + RN;
       mmf_mm_bus_wr(mmf_unb_file_prefix(v_gn / c_quad, v_gn mod c_quad) & "REG_SDP_INFO",  8, TO_UINT(c_exp_sdp_info.antenna_field_index), tb_clk);
@@ -1361,11 +1360,11 @@ begin
 
   -- Prepare exp_sdp_cep_header before rx_beamlet_sosi.eop, so that
   -- p_verify_cep_header can verify it at rx_beamlet_sosi.eop.
-
   exp_sdp_cep_header <= func_sdp_compose_cep_header(c_cep_ip_src_addr,
                                                     c_exp_ip_header_checksum,
                                                     c_exp_sdp_info,
                                                     c_last_gn,
+                                                    exp_payload_error,
                                                     c_exp_beamlet_scale,
                                                     c_exp_beamlet_index,
                                                     exp_dp_bsn);
diff --git a/applications/lofar2/libraries/sdp/src/vhdl/sdp_beamformer_output.vhd b/applications/lofar2/libraries/sdp/src/vhdl/sdp_beamformer_output.vhd
index 890947c591..1fa173a5e2 100644
--- a/applications/lofar2/libraries/sdp/src/vhdl/sdp_beamformer_output.vhd
+++ b/applications/lofar2/libraries/sdp/src/vhdl/sdp_beamformer_output.vhd
@@ -20,10 +20,12 @@
 
 -------------------------------------------------------------------------------
 --
--- Author: R. van der Walle
+-- Author: R. van der Walle, E. Kooistra (payload error support)
 -- Purpose:
 -- The beamformer output (BDO) packetizes the beamlet data into UDP/IP packets.
 -- Description:
+-- * https://support.astron.nl/confluence/display/L2M/L5+SDPFW+Design+Document%3A+Beamformer
+-- * https://support.astron.nl/confluence/display/L2M/L4+SDPFW+Decision%3A+Multiple+beamlet+output+destinations
 -- Remark:
 --
 -------------------------------------------------------------------------------
@@ -73,22 +75,27 @@ entity sdp_beamformer_output is
   );
 end sdp_beamformer_output;
 
-
 architecture str of sdp_beamformer_output is
 
-  constant c_data_w                 : natural := c_nof_complex * c_sdp_W_beamlet;  -- 16b
-  constant c_beamlet_index          : natural := g_beamset_id * c_sdp_S_sub_bf;  -- call beamset 'id' and beamlet 'index'
+  constant c_data_w         : natural := c_nof_complex * c_sdp_W_beamlet;  -- 16b
+  constant c_beamlet_index  : natural := g_beamset_id * c_sdp_S_sub_bf;  -- call beamset 'id' and beamlet 'index'
 
-  -- c_fifo_fill must be the exact size of a payload such that no payload gets stuck in the FIFO or the FIFO gets read out too soon.
-  -- For packets of variable length, dp_fifo_fill_eop must be used. In this case we can use the standard fill fifo.
-  constant c_fifo_fill              : natural := c_sdp_cep_payload_nof_longwords;
-  constant c_fifo_size              : natural := c_fifo_fill * 2;  -- Make fifo size large enough for adding header and muxing beamsets.
+  -- Use c_fifo_fill = c_fifo_size - margin so that FIFO does not get read out too soon.
+  -- The dp_fifo_fill_eop takes care that the FIFO gets read out whenever there is an
+  -- eop in the FIFO, so that no payload gets stuck in the FIFO. Thanks to the use of eop
+  -- it possible to pass on blocks of variable length.
+  -- Make fifo size large enough for adding header, muxing c_sdp_N_beamsets beamsets and
+  -- delaying output to be able to realign snk_in.err field from snk_in.eop to src_out.sop.
+  constant c_fifo_fill      : natural := c_sdp_cep_payload_nof_longwords;  -- 976
+  constant c_fifo_size      : natural := true_log_pow2(c_sdp_cep_payload_nof_longwords) * c_sdp_N_beamsets;  -- 2048
 
   signal snk_in_concat             : t_dp_sosi;
-  signal dp_packet_merge_src_out   : t_dp_sosi;
   signal dp_repack_data_src_out    : t_dp_sosi;
-  signal dp_fifo_sc_src_out        : t_dp_sosi;
-  signal dp_fifo_sc_src_in         : t_dp_siso;
+  signal dp_packet_merge_src_out   : t_dp_sosi;
+  signal dp_fifo_merge_src_out     : t_dp_sosi;
+  signal dp_fifo_merge_src_in      : t_dp_siso;
+  signal dp_pipeline_src_out       : t_dp_sosi;
+  signal dp_pipeline_src_in        : t_dp_siso;
   signal dp_offload_tx_src_out     : t_dp_sosi;
   signal dp_offload_tx_src_in      : t_dp_siso;
   signal ip_checksum_src_out       : t_dp_sosi;
@@ -96,12 +103,12 @@ architecture str of sdp_beamformer_output is
   signal dp_pipeline_ready_src_out : t_dp_sosi;
   signal dp_pipeline_ready_src_in  : t_dp_siso;
 
-  signal common_fifo_rd_req : std_logic;
   signal payload_err        : std_logic_vector(0 downto 0);
   signal station_info       : std_logic_vector(15 downto 0) := (others => '0');
 
   -- Default set all data path driven header fields to 0
   signal dp_offload_tx_hdr_fields : std_logic_vector(1023 downto 0) := (others => '0');
+  signal dp_offload_tx_header     : t_sdp_cep_header;  -- to view dp_offload_tx_hdr_fields in Wave window
 
 begin
 
@@ -111,15 +118,43 @@ begin
   -- . send beamlet data big endian with X.re part first, then X.im, Y.re, Y.im
   -------------------------------------------------------------------------------
   p_snk_in_arr : process(in_sosi)
+    variable v_ref_time : time := 0 ns;
   begin
     snk_in_concat <= in_sosi;
     snk_in_concat.data(c_data_w - 1 downto 0) <= in_sosi.re(c_sdp_W_beamlet - 1 downto 0) & in_sosi.im(c_sdp_W_beamlet - 1 downto 0);
+
+    -- synthesis translate_off
+    -- Force BSN error in simulation to verify payload error in tb_lofar2_unb2c_sdp_station_bf.vhd,
+    -- this will cause two times payload errors, one when BSN goes wrong and one when BSN goes ok again.
+    if v_ref_time = 0 ns then
+      if in_sosi.sop = '1' then
+        -- Use start of input as reference time, rather than e.g. fixed 50 us, to be
+        -- independent of how long it takes for the tb to deliver the first block.
+        v_ref_time := NOW;
+        -- Offset the v_ref_time to the second block of the c_sdp_cep_nof_blocks_per_packet
+        -- = 4 blocks that will be merged.
+        v_ref_time := v_ref_time + c_sdp_block_period * 1 ns;
+      end if;
+    elsif NOW > v_ref_time + 1 * c_sdp_cep_nof_blocks_per_packet * c_sdp_block_period * 1 ns and
+          NOW < v_ref_time + 4 * c_sdp_cep_nof_blocks_per_packet * c_sdp_block_period * 1 ns then
+      -- Disturb BSN to cause merged payload error. Expected results for the merged blocks:
+      -- . index 0 : First merged block bsn ok and payload_error = '0'.
+      -- . index 1 : bsn still ok, but payload error = '1', due to bsn++ after first block
+      -- . index 2,3 : bsn wrong due to bsn++, but payload error = '0', because all 4
+      --               merged blocks have incrementing bsn
+      -- . index 4 : bsn still wrong due to bsn++, and payload error = '1', because the bsn
+      --             is restored after first block, so the merged blocks do not have
+      --             incrementing bsn
+      -- . index >= 5 : bsn ok and payload_error = '0'.
+      snk_in_concat.bsn <= INCR_UVEC(in_sosi.bsn, 1);
+    end if;
+    -- synthesis translate_on
   end process;
 
   -------------------------------------------------------------------------------
   -- dp_repack_data
   -- . 16b -> 64b
-  -- . We don't need to flow control the source beacause we're going from 16b->64b
+  -- . We don't need to flow control the source because we're going from 16b->64b
   -------------------------------------------------------------------------------
   u_dp_repack_data : entity dp_lib.dp_repack_data
   generic map (
@@ -144,7 +179,8 @@ begin
   -------------------------------------------------------------------------------
   u_dp_packet_merge : entity dp_lib.dp_packet_merge
   generic map(
-    g_nof_pkt       => c_sdp_cep_nof_blocks_per_packet
+    g_nof_pkt       => c_sdp_cep_nof_blocks_per_packet,
+    g_bsn_increment => 1
   )
   port map(
     rst     => dp_rst,
@@ -159,16 +195,9 @@ begin
 
   -------------------------------------------------------------------------------
   -- FIFO
-  -- . We're inserting headers, so dp_offload_tx needs a flow controllable
-  --   source.
-  -- . Also, we need a fill FIFO here because 16b->64b will introduce gaps in our
-  --   TX stream (not allowed by 10G TX MAC).
-  -- . The fill fifo waits until c_fifo_fill words are received before enabling the
-  --   output. The total number of words in the fifo is determined by the
-  --   backpressure.
   -------------------------------------------------------------------------------
-  u_dp_fifo_fill_sc : entity dp_lib.dp_fifo_fill_sc
-  generic map (
+  u_dp_fifo_fill_eop_sc : entity dp_lib.dp_fifo_fill_eop_sc
+  generic map (  -- pass on dp_packet_merge_src_out.err via u_common_fifo_sc_err
     g_data_w         => c_longword_w,
     g_empty_w        => c_byte_w,
     g_use_empty      => true,
@@ -182,28 +211,44 @@ begin
   port map (
     clk     => dp_clk,
     rst     => dp_rst,
-
     snk_in  => dp_packet_merge_src_out,
-
-    src_out => dp_fifo_sc_src_out,
-    src_in  => dp_fifo_sc_src_in
+    src_out => dp_fifo_merge_src_out,
+    src_in  => dp_fifo_merge_src_in
   );
 
-  -- Simple fifo to store the payload error at eop of FIFO input to be used at sop of FIFO output.
-  -- It can then be used in the packet header.
-  common_fifo_rd_req <= dp_fifo_sc_src_out.sop;
-  u_common_fifo_sc : entity common_lib.common_fifo_sc
+  -- Simple fifo to store the payload error bit at eop of FIFO input to be used at sop of FIFO
+  -- output, so that payload_err can then be used in the packet header.
+  -- Typically the u_dp_fifo_fill_eop will store between 0 and c_sdp_N_beamsets = 2 packets.
+  -- Choose g_nof_words > c_sdp_N_beamsets to have some margin compared to c_fifo_size of the
+  -- data FIFO.
+  u_common_fifo_sc_err : entity common_lib.common_fifo_sc
   generic map (
     g_dat_w => 1,
-    g_nof_words => 2
+    g_nof_words => c_sdp_N_beamsets + 2
   )
   port map (
-    rst => dp_rst,
-    clk => dp_clk,
+    rst    => dp_rst,
+    clk    => dp_clk,
     wr_dat => dp_packet_merge_src_out.err(0 downto 0),
     wr_req => dp_packet_merge_src_out.eop,
     rd_dat => payload_err,
-    rd_req => common_fifo_rd_req
+    rd_req => dp_fifo_merge_src_out.sop
+  );
+
+  -- Pipeline FIFO output to align payload_err at dp_pipeline_src_out.sop
+  u_pipeline : entity dp_lib.dp_pipeline
+  generic map (
+    g_pipeline => 1
+  )
+  port map  (
+    rst        => dp_rst,
+    clk        => dp_clk,
+    -- ST sink
+    snk_out    => dp_fifo_merge_src_in,
+    snk_in     => dp_fifo_merge_src_out,
+    -- ST source
+    src_in     => dp_pipeline_src_in,
+    src_out    => dp_pipeline_src_out
   );
 
   -------------------------------------------------------------------------------
@@ -287,8 +332,10 @@ begin
   dp_offload_tx_hdr_fields(field_hi(c_sdp_cep_hdr_field_arr, "sdp_beamlet_index" ) downto field_lo(c_sdp_cep_hdr_field_arr,  "sdp_beamlet_index" )) <= TO_UVEC(c_beamlet_index, c_halfword_w);
   dp_offload_tx_hdr_fields(field_hi(c_sdp_cep_hdr_field_arr, "sdp_block_period"  ) downto field_lo(c_sdp_cep_hdr_field_arr,  "sdp_block_period"  )) <= sdp_info.block_period;
 
-  dp_offload_tx_hdr_fields(field_hi(c_sdp_cep_hdr_field_arr, "dp_bsn" ) downto field_lo(c_sdp_cep_hdr_field_arr, "dp_bsn" )) <= dp_fifo_sc_src_out.bsn(63 downto 0);
+  dp_offload_tx_hdr_fields(field_hi(c_sdp_cep_hdr_field_arr, "dp_bsn" ) downto field_lo(c_sdp_cep_hdr_field_arr, "dp_bsn" )) <= dp_pipeline_src_out.bsn(63 downto 0);
 
+  -- For viewing the header fields in wave window
+  dp_offload_tx_header <= func_sdp_map_cep_header(dp_offload_tx_hdr_fields);
 
   -------------------------------------------------------------------------------
   -- dp_offload_tx_v3
@@ -312,8 +359,8 @@ begin
     reg_hdr_dat_mosi      => reg_hdr_dat_mosi,
     reg_hdr_dat_miso      => reg_hdr_dat_miso,
 
-    snk_in_arr(0)         => dp_fifo_sc_src_out,
-    snk_out_arr(0)        => dp_fifo_sc_src_in,
+    snk_in_arr(0)         => dp_pipeline_src_out,
+    snk_out_arr(0)        => dp_pipeline_src_in,
 
     src_out_arr(0)        => dp_offload_tx_src_out,
     src_in_arr(0)         => dp_offload_tx_src_in,
-- 
GitLab