From 6a1e306a7128ecfd9fe32b27bd2acd3f94370eb4 Mon Sep 17 00:00:00 2001
From: Eric Kooistra <kooistra@astron.nl>
Date: Mon, 8 Aug 2022 15:44:58 +0200
Subject: [PATCH] Use new_interval to avoid offloading void XST data in first
 sync interval.

---
 .../sdp/src/vhdl/node_sdp_correlator.vhd      | 14 +++++++---
 .../src/vhdl/sdp_crosslets_subband_select.vhd | 26 ++++++++++++-------
 .../sdp/src/vhdl/sdp_statistics_offload.vhd   | 14 ++++++++--
 .../sdp/tb/vhdl/tb_sdp_statistics_offload.vhd | 12 +++++++--
 4 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/applications/lofar2/libraries/sdp/src/vhdl/node_sdp_correlator.vhd b/applications/lofar2/libraries/sdp/src/vhdl/node_sdp_correlator.vhd
index 17f3ac609a..d7467674f8 100644
--- a/applications/lofar2/libraries/sdp/src/vhdl/node_sdp_correlator.vhd
+++ b/applications/lofar2/libraries/sdp/src/vhdl/node_sdp_correlator.vhd
@@ -22,11 +22,13 @@
 --
 -- Author: R. van der Walle
 -- Purpose: 
--- . Implements the functionality of the Subband Correlator in the 
+-- . Implements the functionality of the Subband Correlator (F_sub) in the
 --   LOFAR2 SDPFW design.
 -- Description:
 -- Remark:
--- .
+-- . Use new_interval to avoid offloading undefined XST data for the first
+--   sync interval after an F_sub restart, because then there is no previous
+--   sync interval with valid XST data yet.
 -------------------------------------------------------------------------------
 
 LIBRARY IEEE, common_lib, dp_lib, reorder_lib, st_lib, mm_lib, ring_lib;
@@ -118,6 +120,8 @@ ARCHITECTURE str OF node_sdp_correlator IS
   SIGNAL xsel_data_sosi                : t_dp_sosi := c_dp_sosi_rst;
   SIGNAL local_sosi                    : t_dp_sosi := c_dp_sosi_rst;
 
+  SIGNAL new_interval                  : STD_LOGIC;
+
   SIGNAL ring_mux_sosi                 : t_dp_sosi := c_dp_sosi_rst;
   SIGNAL ring_mux_siso                 : t_dp_siso := c_dp_siso_rdy;
   SIGNAL dp_fifo_fill_sosi             : t_dp_sosi := c_dp_sosi_rst;
@@ -175,6 +179,8 @@ BEGIN
     in_sosi_arr    => quant_sosi_arr,
     out_sosi       => xsel_sosi,
 
+    new_interval   => new_interval,
+
     mm_rst         => mm_rst,
     mm_clk         => mm_clk,
 
@@ -464,7 +470,9 @@ BEGIN
     reg_bsn_monitor_v2_offload_copi => reg_bsn_monitor_v2_xst_offload_copi,
     reg_bsn_monitor_v2_offload_cipo => reg_bsn_monitor_v2_xst_offload_cipo,
 
-    in_sosi   => crosslets_sosi,
+    in_sosi      => crosslets_sosi,
+    new_interval => new_interval,
+
     out_sosi  => mon_xst_udp_sosi_arr(0),
     out_siso  => xst_udp_siso,
 
diff --git a/applications/lofar2/libraries/sdp/src/vhdl/sdp_crosslets_subband_select.vhd b/applications/lofar2/libraries/sdp/src/vhdl/sdp_crosslets_subband_select.vhd
index d76f1d6833..2e9acfb004 100644
--- a/applications/lofar2/libraries/sdp/src/vhdl/sdp_crosslets_subband_select.vhd
+++ b/applications/lofar2/libraries/sdp/src/vhdl/sdp_crosslets_subband_select.vhd
@@ -24,15 +24,18 @@
 -- Purpose: 
 -- Select subbands from incoming blocks
 -- Description:
---   The Crosslet subband select selects N_crosslets from each incoming block.
+-- * The Crosslet subband select selects N_crosslets from each incoming block.
 --   Per crosslet there are S_pn = 12 subbands, one from each signal input of
 --   the PN.
---   The cur_crosslets_info is valid at the out_sosi.sync and for the entire
+-- * The cur_crosslets_info is valid at the out_sosi.sync and for the entire
 --   sync interval. The cur_crosslets_info identifies the crosslets that are
 --   being calculated during this out_sosi.sync interval.
 --   The prev_crosslets_info identifies the crosslets that were calculated
 --   during the previous out_sosi.sync interval, so the XST for those crosslets
 --   are then pending to be offloaded.
+-- * The new_interval is active before the first out_sosi.sync and inactive
+--   before the next out_sosi.sync, so it can be used to know when a new
+--   sequence of out_sosi.sync intervals starts.
 -- Remark:
 -- . See L5 SDPFW Design Document: Subband Correlator
 --   Link: https://support.astron.nl/confluence/pages/viewpage.action?spaceKey=L2M&title=L5+SDPFW+Design+Document%3A+Subband+Correlator
@@ -52,14 +55,16 @@ ENTITY sdp_crosslets_subband_select IS
     g_ctrl_interval_size_min : NATURAL := c_sdp_xst_nof_clk_per_sync_min
   );
   PORT (
-    dp_clk        : IN  STD_LOGIC;
-    dp_rst        : IN  STD_LOGIC;
+    dp_clk         : IN  STD_LOGIC;
+    dp_rst         : IN  STD_LOGIC;
 
-    in_sosi_arr   : IN  t_dp_sosi_arr(c_sdp_P_pfb-1 DOWNTO 0);
-    out_sosi      : OUT t_dp_sosi;
+    in_sosi_arr    : IN  t_dp_sosi_arr(c_sdp_P_pfb-1 DOWNTO 0);
+    out_sosi       : OUT t_dp_sosi;
 
-    mm_rst        : IN  STD_LOGIC;
-    mm_clk        : IN  STD_LOGIC;
+    new_interval   : OUT STD_LOGIC;
+
+    mm_rst         : IN  STD_LOGIC;
+    mm_clk         : IN  STD_LOGIC;
 
     reg_crosslets_info_mosi : IN  t_mem_mosi := c_mem_mosi_rst;
     reg_crosslets_info_miso : OUT t_mem_miso := c_mem_miso_rst;
@@ -98,7 +103,7 @@ ARCHITECTURE str OF sdp_crosslets_subband_select IS
   SIGNAL r     : t_crosslets_control_reg;
   SIGNAL nxt_r : t_crosslets_control_reg;
 
-  SIGNAL start_trigger : STD_LOGIC := '0';
+  SIGNAL start_trigger   : STD_LOGIC := '0';
 
   SIGNAL col_select_mosi : t_mem_mosi := c_mem_mosi_rst;
   SIGNAL col_select_miso : t_mem_miso := c_mem_miso_rst;
@@ -142,7 +147,8 @@ BEGIN
     in_sosi_arr  => in_sosi_arr,
     out_sosi_arr => dp_bsn_sync_scheduler_src_out_arr,
 
-    out_start => start_trigger
+    out_start => start_trigger,
+    out_start_interval => new_interval
   );
 
   ---------------------------------------------------------------
diff --git a/applications/lofar2/libraries/sdp/src/vhdl/sdp_statistics_offload.vhd b/applications/lofar2/libraries/sdp/src/vhdl/sdp_statistics_offload.vhd
index d37221c8c5..b9815e9840 100644
--- a/applications/lofar2/libraries/sdp/src/vhdl/sdp_statistics_offload.vhd
+++ b/applications/lofar2/libraries/sdp/src/vhdl/sdp_statistics_offload.vhd
@@ -142,7 +142,8 @@ ENTITY sdp_statistics_offload IS
 
     -- Input timing regarding the integration interval of the statistics
     in_sosi          : IN t_dp_sosi;
-    
+    new_interval     : IN STD_LOGIC := '0';
+
     -- Streaming output of offloaded statistics packets
     out_sosi         : OUT t_dp_sosi;
     out_siso         : IN t_dp_siso;
@@ -225,6 +226,7 @@ ARCHITECTURE str OF sdp_statistics_offload IS
   SIGNAL data_id_rec              : t_sdp_stat_data_id;
   SIGNAL data_id_slv              : STD_LOGIC_VECTOR(31 DOWNTO 0) := (OTHERS => '0');
 
+  SIGNAL in_trigger               : STD_LOGIC;
   SIGNAL trigger_en               : STD_LOGIC := '0';
   SIGNAL trigger_offload          : STD_LOGIC := '0';
   SIGNAL mm_done                  : STD_LOGIC := '0';
@@ -477,6 +479,14 @@ BEGIN
     nxt_r <= v;
   END PROCESS;
 
+  -- The in_trigger can skip the first in_sosi.sync. This is necessary if the
+  -- in_sosi input can be restarted, because then at every restart there is
+  -- no valid previous in_sosi.sync interval yet. This is used for XST offload,
+  -- because then the in_sync interval is MM programmable. For SST and BST the
+  -- new_interval = '0' is not used, because then the in_sosi typically
+  -- remains on after it was started.
+  in_trigger <= in_sosi.sync AND NOT new_interval;
+
   u_mms_common_variable_delay : ENTITY common_lib.mms_common_variable_delay
   PORT MAP (
     mm_rst => mm_rst,
@@ -489,7 +499,7 @@ BEGIN
     reg_enable_miso => reg_enable_miso,
 
     delay           => nof_cycles_dly,
-    trigger         => in_sosi.sync,
+    trigger         => in_trigger,
     trigger_en      => trigger_en,
     trigger_dly     => trigger_offload
   );
diff --git a/applications/lofar2/libraries/sdp/tb/vhdl/tb_sdp_statistics_offload.vhd b/applications/lofar2/libraries/sdp/tb/vhdl/tb_sdp_statistics_offload.vhd
index d44b39a143..5298580a65 100644
--- a/applications/lofar2/libraries/sdp/tb/vhdl/tb_sdp_statistics_offload.vhd
+++ b/applications/lofar2/libraries/sdp/tb/vhdl/tb_sdp_statistics_offload.vhd
@@ -172,6 +172,7 @@ ARCHITECTURE tb OF tb_sdp_statistics_offload IS
   SIGNAL offload_rx_hdr_dat_mosi : t_mem_mosi := c_mem_mosi_rst;
   SIGNAL offload_rx_hdr_dat_miso : t_mem_miso;
 
+  SIGNAL new_interval            : STD_LOGIC := '0';
   SIGNAL in_sosi                 : t_dp_sosi := c_dp_sosi_rst;
   SIGNAL in_crosslets_info_rec   : t_sdp_crosslets_info;
 
@@ -241,6 +242,9 @@ ARCHITECTURE tb OF tb_sdp_statistics_offload IS
   SIGNAL dbg_c_ram_size                  : NATURAL := c_ram_size;
   SIGNAL dbg_c_crosslets_info_rec        : t_sdp_crosslets_info := c_crosslets_info_rec;
   SIGNAL dbg_c_crosslets_info_slv        : STD_LOGIC_VECTOR(c_sdp_crosslets_info_reg_w-1 DOWNTO 0) := c_crosslets_info_slv;
+  SIGNAL dbg_c_nof_block_per_sync        : NATURAL := c_nof_block_per_sync;
+  SIGNAL dbg_c_nof_clk_per_block         : NATURAL := c_nof_clk_per_block;
+  SIGNAL dbg_c_nof_clk_per_sync          : NATURAL := c_nof_clk_per_sync ;
 
 BEGIN
 
@@ -281,7 +285,9 @@ BEGIN
     in_sosi.bsn <= TO_DP_BSN(c_bsn_init);
     in_sosi.valid <= '1';
     in_crosslets_info_rec <= c_crosslets_info_rec;
+    new_interval <= '1';  -- mark first in_sosi.sync interval
     WHILE TRUE LOOP
+      -- One in_sosi.sync interval
       FOR i IN 0 TO c_nof_block_per_sync-1 LOOP
         FOR j IN 0 TO c_nof_clk_per_block-1 LOOP
           in_sosi.sync  <= '0';
@@ -304,6 +310,7 @@ BEGIN
           proc_common_wait_some_cycles(dp_clk, 1);
         END LOOP;
       END LOOP;
+      new_interval <= '0';
     END LOOP;
     WAIT;
   END PROCESS;
@@ -311,8 +318,7 @@ BEGIN
   -- Enable the statistics offload when input is running
   p_enable_trigger : PROCESS
   BEGIN
-    -- Wait at least one sync interval, so that DUT can have measured the integration_interval
-    proc_common_wait_until_hi_lo(dp_clk, in_sosi.sync);
+    proc_common_wait_until_low(dp_clk, mm_rst);
     proc_common_wait_some_cycles(mm_clk, 10);
     -- Enable common variable delay.
     proc_mem_mm_bus_wr(c_reg_enable_mm_addr_enable, 1, mm_clk, enable_miso, enable_mosi);
@@ -645,6 +651,8 @@ BEGIN
 
     -- ST
     in_sosi          => in_sosi,
+    new_interval     => new_interval,
+
     out_sosi         => sdp_offload_sosi,
     out_siso         => sdp_offload_siso,
 
-- 
GitLab