Merge branch 'L2SDP-88' into 'master'

Resolve L2SDP-88

Closes L2SDP-88

See merge request desp/hdl!21

libraries/dsp/st/hdllib.cfg

@@ -21,6 +21,7 @@ test_bench_files =
     tb/vhdl/tb_mmf_st_sst.vhd
     tb/vhdl/tb_st_histogram.vhd
     tb/vhdl/tb_mms_st_histogram.vhd
+    tb/vhdl/tb_tb_st_histogram.vhd
 
 regression_test_vhdl = 
     tb/vhdl/tb_st_acc.vhd 

libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd

@@ -29,23 +29,31 @@
 --   to generate data that can make related problems with that vissible.
 --
 --   To know if there can constantly new data be witten to the RAM blocks 
---   a simple counter is sufficient.
+--   a counter would be sufficient.
 --   
 --   Because there is a delay between requesting and writing back of data of 
---   2 cycles and it is illegal to read and write on the same adres at the 
---   same time, a special situation can happen where the addresses can toggle
---   (e.g. 0; 1; 0; 1) which causes incorrect counting. To simulate this the 
---   g_snk_in_data_sim_type can be set to 'toggle'
+--   3 cycles and it is illegal to read and write on the same adres at the 
+--   same time, there are 2 special situations that can happen. One where the 
+--   addresses can toggle (e.g. 0; 1; 0; 1) and one where a simultanious read 
+--   and write are triggered (e.g. 0; 1; 1; 0). Both would cause incorrect 
+--   counting as the address count can't be updated (written) before it's 
+--   address is requested again. Due to this the counter in st_histogram can 
+--   not be a simple counter that only counts and compares on repeating 
+--   addresses. It also has to compare on 2 and 3 cycles back - which makes 
+--   it complicated enough that it requires additional test stimuli. 
+--   To simulate with the required stimuli the g_snk_in_data_sim_type can be 
+--   set to 'counter', 'toggle', 'same rw' or a 'mix' of it.
 --
 --   Only incoming data while snk_in.valid = '1' may be counted. To keep the
 --   simulation simple there is the option to let there be some gap's in the
 --   valid data (or not) where snk_in.valid = '0' by setting the g_valid_gap 
---   to TRUE or FALSE.
+--   to 'true', 'false' or 'custom'.
 --
 -------------------------------------------------------------------------------
 
 LIBRARY IEEE, common_lib, mm_lib, dp_lib;
 USE IEEE.std_logic_1164.ALL;
+USE IEEE.numeric_std.ALL;           -- needed by TO_UNSIGNED
 USE common_lib.common_pkg.ALL;
 USE common_lib.common_mem_pkg.ALL;
 USE common_lib.tb_common_pkg.ALL;
@@ -59,8 +67,8 @@ ENTITY tb_st_histogram IS
     g_nof_bins             : NATURAL := 8; --8 ; 2
     g_nof_data             : NATURAL := 200;
     --g_str                  : STRING  := "freq.density";
-    g_valid_gap            : BOOLEAN := TRUE;
-    g_snk_in_data_sim_type : STRING  := "counter"  -- "counter" or "toggle" or "same rw" or "mix"
+    g_valid_gap            : STRING  := "custom"; -- "false" or "true" or "custom" --BOOLEAN := TRUE
+    g_snk_in_data_sim_type : STRING  := "same rw"  -- "counter" or "toggle" or "same rw" or "mix"
     );
 END tb_st_histogram;
 
@@ -79,6 +87,7 @@ ARCHITECTURE tb OF tb_st_histogram IS
   SIGNAL prev_unvalid           : STD_LOGIC := '0';
   SIGNAL init_phase             : STD_LOGIC := '1';
   SIGNAL toggle_start           : STD_LOGIC := '0';
+  SIGNAL pre_sync               : STD_LOGIC := '0';
   
   
   ----------------------------------------------------------------------------
@@ -87,10 +96,26 @@ ARCHITECTURE tb OF tb_st_histogram IS
   TYPE t_srw_arr IS ARRAY (NATURAL RANGE <>) OF INTEGER;
   CONSTANT c_srw_arr            : t_srw_arr := (0,0,1,1,0,0,1,2,3, 1, 2, 3, 0, 3, 3, 0, 3);
                                             --  1.2.3.4.5.6.7.8.9.10.11.12.13.14.15.16.17
+                                            --0:1.2.    3.4.               05.      06.
+                                            --1:    1.2.    3.    04.
+                                            --2:              1.     02.
+                                            --3:                1.      02.   03.04.   05.
+                                            --srw:      x.  x.     x. x. x.       x. x. u.
   
   SIGNAL srw_index_cnt          : NATURAL   := 0;
   
   
+  ----------------------------------------------------------------------------
+  -- Valid stimuli
+  ----------------------------------------------------------------------------
+  TYPE t_val_arr IS ARRAY (NATURAL RANGE <>) OF INTEGER;
+  CONSTANT c_val_arr            : t_val_arr := (1,1,1,1,0,1,1,1,1, 1, 1, 1, 1, 0, 1, 1, 1);
+                                            --  1.2.3.4.5.6.7.8.9.10.11.12.13.14.15.16.17
+  
+  SIGNAL val_index_cnt          : NATURAL   := 0;
+  SIGNAL dbg_valid              : NATURAL;
+  
+  
   ----------------------------------------------------------------------------
   -- Clocks and resets
   ----------------------------------------------------------------------------   
@@ -109,6 +134,34 @@ ARCHITECTURE tb OF tb_st_histogram IS
   SIGNAL st_histogram_snk_in : t_dp_sosi;
   
   
+  ----------------------------------------------------------------------------
+  -- Streaming Output
+  ----------------------------------------------------------------------------
+  
+  SIGNAL st_histogram_ram_miso     : t_mem_miso;
+  SIGNAL st_histogram_dbg_ram_miso : t_mem_miso;
+  
+  
+  ----------------------------------------------------------------------------
+  -- Self check array
+  ----------------------------------------------------------------------------
+  TYPE t_data_check_arr IS ARRAY (0 TO g_nof_bins) OF INTEGER;
+  SIGNAL data_check_arr         : t_data_check_arr := (OTHERS=> 0);
+                                            
+  SIGNAL check_adr               : NATURAL := 0;
+  SIGNAL prev_check_adr          : NATURAL;
+  SIGNAL nxt_check_arr_cnt       : NATURAL;
+  
+  SIGNAL st_histogram_snk_in_ppp : t_dp_sosi;
+  SIGNAL st_histogram_snk_in_pppp: t_dp_sosi;
+--  SIGNAL dbg_check_adr           :STD_LOGIC_VECTOR(g_data_w-1 DOWNTO c_adr_low); --  : NATURAL;
+  
+  SIGNAL dbg_error_location      : STD_LOGIC;
+  SIGNAL error_cnt               : NATURAL;
+  SIGNAL dbg_int_data_miso       : NATURAL;
+  SIGNAL dbg_int_data_arr        : NATURAL;
+  
+  
 BEGIN 
   
   ----------------------------------------------------------------------------
@@ -122,7 +175,7 @@ BEGIN
   
   ----------------------------------------------------------------------------
   -- Source: stimuli
-  --  st_histogram_snk_in.data    counter or toggle stimuli
+  --  st_histogram_snk_in.data    counter or toggle or same_rw stimuli
   --                     .valid   with or without gap's in valid stimuli
   --                     .sync    sync stimuli
   ---------------------------------------------------------------------------- 
@@ -162,7 +215,7 @@ BEGIN
     ELSIF g_snk_in_data_sim_type = "same rw" THEN
       IF dp_rst='1' THEN
         st_histogram_snk_in.data(g_data_w-1 DOWNTO 0) <= (OTHERS=>'0');
-      ELSIF rising_edge(dp_clk) AND pre_valid='1' THEN -- AND init_phase='0'   didn't work
+      ELSIF rising_edge(dp_clk) AND pre_sync='1' THEN -- AND init_phase='0'   didn't work
         st_histogram_snk_in.data(g_data_w-1 DOWNTO c_adr_low) <= TO_UVEC(c_srw_arr(srw_index_cnt), c_adr_w); --placeholder !
         IF srw_index_cnt = c_srw_arr'LENGTH -1 THEN
           srw_index_cnt <= 0;
@@ -209,7 +262,7 @@ BEGIN
   
   p_stimuli : PROCESS
   BEGIN
-    IF g_valid_gap = FALSE THEN
+    IF g_valid_gap = "false" THEN
     
       -- initializing
       st_histogram_snk_in.sync <= '0';
@@ -233,13 +286,15 @@ BEGIN
       tb_end <= '1';
       WAIT;
       
-    ELSIF g_valid_gap = TRUE THEN
+    ELSIF g_valid_gap = "true" THEN
     
       -- initializing
       st_histogram_snk_in.sync <= '0';
       st_histogram_snk_in.valid <= '0';
       WAIT UNTIL rising_edge(dp_clk);
-      FOR I IN 0 TO 9 LOOP WAIT UNTIL rising_edge(dp_clk); END LOOP;
+      FOR I IN 0 TO 8 LOOP WAIT UNTIL rising_edge(dp_clk); END LOOP;
+      pre_sync <= '1';
+      WAIT UNTIL rising_edge(dp_clk);
       pre_valid <= '1';
       st_histogram_snk_in.valid <= '1';
       -- generating g_nof_sync-1 sync pulses with gaps in 'valid'
@@ -264,7 +319,7 @@ BEGIN
         WAIT UNTIL rising_edge(dp_clk);
         st_histogram_snk_in.valid <= '0';
         WAIT UNTIL rising_edge(dp_clk);
-        --st_histogram_snk_in.valid <= '0'; -- gap while sync
+        --st_histogram_snk_in.valid <= '0'; -- gap while sync --should not happen, impossible
         st_histogram_snk_in.sync <= '1';
         pre_valid <= '1';
         WAIT UNTIL rising_edge(dp_clk);
@@ -276,21 +331,66 @@ BEGIN
       FOR I IN 0 TO 9 LOOP WAIT UNTIL rising_edge(dp_clk); END LOOP;
       tb_end <= '1';
       WAIT;
+      
+    ELSIF g_valid_gap = "custom" THEN
+      
+      -- initializing
+      st_histogram_snk_in.sync <= '0';
+      st_histogram_snk_in.valid <= '0';
+      WAIT UNTIL rising_edge(dp_clk);
+      FOR I IN 0 TO 8 LOOP WAIT UNTIL rising_edge(dp_clk); END LOOP;
+      pre_sync <= '1';
+      WAIT UNTIL rising_edge(dp_clk);
+      pre_valid <= '1';
+ --     st_histogram_snk_in.valid <= '1';
+      -- generating g_nof_sync-1 sync pulses with gaps in 'valid'
+      FOR I IN 0 TO g_nof_sync-2 LOOP
+        toggle_start <= '1';
+        st_histogram_snk_in.sync <= '1';
+        st_histogram_snk_in.valid <= STD_LOGIC( TO_UNSIGNED(c_val_arr(0),1)(0) ); -- TO_UVEC(c_val_arr(0), c_adr_w); --placeholder !
+        WAIT UNTIL rising_edge(dp_clk);
+        st_histogram_snk_in.sync <= '0';
+        FOR I IN 1 TO c_val_arr'LENGTH -1 LOOP
+          st_histogram_snk_in.valid <= STD_LOGIC( TO_UNSIGNED( c_val_arr(I) ,1)(0) ); -- TO_UVEC(c_val_arr(J), c_adr_w);
+          dbg_valid <= I;
+          WAIT UNTIL rising_edge(dp_clk);
+        END LOOP;
+        proc_common_wait_some_cycles(dp_clk, (g_sync_length - (c_val_arr'LENGTH -2) )); --the -2 has to be ditched as the sync happens 2 cycles to soon
+      END LOOP;
+      -- ending
+      FOR I IN 0 TO 9 LOOP WAIT UNTIL rising_edge(dp_clk); END LOOP;
+      tb_end <= '1';
+      WAIT;
     END IF;
   END PROCESS;
   
   
   
+--  p_mm_stimuli : PROCESS --(st_histogram_snk_in.sync)
+--  BEGIN
+--    st_histogram_ram_mosi <= c_mem_mosi_rst;  --.address(c_adr_w-1 DOWNTO 0) <= (OTHERS=>'0');
+--    WAIT UNTIL st_histogram_snk_in.sync = '1';
+--    -- wait till one RAM block is written
+--    FOR I IN 0 TO (g_sync_length) LOOP WAIT UNTIL rising_edge(dp_clk); END LOOP;
+--    -- wait for some more cycles
+--    FOR I IN 0 TO 2 LOOP WAIT UNTIL rising_edge(dp_clk); END LOOP;
+--    -- read all bins
+--    FOR I IN 0 TO g_nof_bins-1 LOOP
+--      proc_mem_mm_bus_rd(I, dp_clk, st_histogram_ram_mosi);
+--      proc_common_wait_some_cycles(dp_clk, 1);
+--    END LOOP;
+--  END PROCESS;
+  
   ----------------------------------------------------------------------------
   -- DUT: Device Under Test
   ----------------------------------------------------------------------------
   
-  u_st_histogram : ENTITY work.st_histogram_8_april
+  u_st_histogram : ENTITY work.st_histogram --_8_april
   GENERIC MAP(
     g_in_data_w         => g_data_w,
     g_nof_bins          => g_nof_bins,
-    g_nof_data   => g_nof_data
-    --g_str        => g_str
+    g_nof_data          => g_nof_data,
+    g_ram_miso_sim_mode => FALSE         -- TRUE
   )
   PORT MAP (
     dp_rst       => dp_rst,
@@ -300,8 +400,141 @@ BEGIN
     snk_in       => st_histogram_snk_in,
   
     -- Memory Mapped
-    ram_mosi     => c_mem_mosi_rst,-- sla_in_
-    ram_miso     => OPEN -- sla_out_
+    sla_in_ram_mosi     => c_mem_mosi_rst,-- sla_in_
+    sla_out_ram_miso     => st_histogram_ram_miso, --OPEN -- sla_out_
+    dbg_ram_miso        => st_histogram_dbg_ram_miso
+  );
+  
+  
+  
+  ----------------------------------------------------------------------------
+  -- Selfcheck:
+  --  The selfcheck is done by counting the adresses created from 3 cycles 
+  --  delayed snk_in data into an address separated array (when in the array, 
+  --  the data is 4 cycles delayed). This data is used as reference for 
+  --  comparing it with the data written into a RAM block in st_histogram. 
+  --  Because the data in st_histogram is written 4 cycles later than it got 
+  --  in, both data are in sync and can be compared directly. 
+  --  When the data is valid but is not the same as the reference data the 
+  --  debug signal dbg_error_location becomes '1' so the location can be 
+  --  easily spotted in the wave window and a report is made.
+  ----------------------------------------------------------------------------
+  
+  
+  u_dp_pipeline_st_histogram_snk_in_3_cycle : ENTITY dp_lib.dp_pipeline
+  GENERIC MAP (
+    g_pipeline   => 3  -- 0 for wires, > 0 for registers, 
+  )
+  PORT MAP (
+    rst          => dp_rst,
+    clk          => dp_clk,
+    snk_in       => st_histogram_snk_in,
+    src_out      => st_histogram_snk_in_ppp
   );
   
+  u_dp_pipeline_st_histogram_snk_in_4_cycle : ENTITY dp_lib.dp_pipeline
+  GENERIC MAP (
+    g_pipeline   => 4  -- 0 for wires, > 0 for registers, 
+  )
+  PORT MAP (
+    rst          => dp_rst,
+    clk          => dp_clk,
+    snk_in       => st_histogram_snk_in,
+    src_out      => st_histogram_snk_in_pppp
+  );
+  
+  ---------------------------------------
+  -- create address from the source data 
+  check_adr <= TO_UINT( st_histogram_snk_in_ppp.data(g_data_w-1 DOWNTO c_adr_low) );
+--  dbg_check_adr <= st_histogram_snk_in_ppp.data(g_data_w -1 DOWNTO c_adr_low);
+  
+  p_prev_check_adr : PROCESS (dp_rst, dp_clk, check_adr)
+  BEGIN
+    IF dp_rst='1' THEN
+      prev_check_adr <= 0;
+    ELSIF rising_edge(dp_clk) THEN
+      prev_check_adr <= check_adr;
+    END IF;
+  END PROCESS;
+  
+  -----------------------------
+  -- when valid increase array based on address 
+  nxt_check_arr_cnt <= data_check_arr(check_adr) + 1 WHEN st_histogram_snk_in_ppp.valid = '1' ELSE data_check_arr(check_adr);
+  
+  
+  --------------------
+  -- filling the array
+  p_cumulate_testdata : PROCESS (dp_rst, dp_clk, nxt_check_arr_cnt, check_adr, st_histogram_snk_in_ppp.sync) --misses prev_check_adr
+  BEGIN 
+    --PROCESS
+    --c_data_check_arr(check_adr) <= nxt_check_arr_cnt;
+    IF dp_rst='1' THEN
+    data_check_arr(0 TO g_nof_bins) <= (OTHERS => 0);
+    ELSIF rising_edge(dp_clk) THEN
+      --data_check_arr(prev_check_adr) <= nxt_check_arr_cnt;
+      data_check_arr(check_adr) <= nxt_check_arr_cnt; --old timing
+      IF st_histogram_snk_in_ppp.sync='1' THEN
+        data_check_arr(0 TO g_nof_bins) <= (check_adr => 1, OTHERS => 0 );  -- null except check_adr
+        --
+      END IF;
+    END IF; 
+  END PROCESS;
+  
+  ---------------------
+  -- extra dbg signals 
+  dbg_int_data_miso <= TO_UINT(st_histogram_dbg_ram_miso.rddata);
+  dbg_int_data_arr <= data_check_arr(prev_check_adr);
+  
+  ---------------------
+  -- selftest
+--  p_selfcheck : PROCESS (dp_rst, dp_clk, data_check_arr, prev_check_adr, st_histogram_dbg_ram_miso.rddata)
+--  BEGIN 
+--    --PROCESS
+--    -- compare cumulated testdata with ram_mosi
+--    
+--    --dbg_int_data_miso <= TO_UINT(st_histogram_dbg_ram_miso.rddata);
+--    --dbg_int_data_arr <= data_check_arr(check_adr);
+--    IF rising_edge(dp_clk) THEN
+--      --dbg_error_location <= '0';
+--      --dbg_int_data_miso <= TO_UINT(st_histogram_dbg_ram_miso.rddata);
+--      --dbg_int_data_arr <= data_check_arr(check_adr);
+--      IF data_check_arr(prev_check_adr) /= TO_UINT(st_histogram_dbg_ram_miso.rddata) AND st_histogram_snk_in_pppp.valid='1' THEN
+--        dbg_error_location <= '1';
+--        REPORT "The value written to the RAM is not what it should be. See signal 'dbg_int_data_arr'. The failure concerns the bin (and array) address: " &integer'image(prev_check_adr) SEVERITY ERROR;
+--        error_cnt <= error_cnt + 1;
+--      ELSE
+--        dbg_error_location <= '0';
+--      END IF;
+--    END IF;
+--
+----    IF dp_rst='1' THEN
+----    data_check_arr(0 TO g_nof_bins) <= (OTHERS => 0);
+----    ELSIF rising_edge(dp_clk) THEN
+----      data_check_arr(check_adr) <= nxt_check_arr_cnt;
+----    END IF; 
+--  END PROCESS;
+
+  
+  -- show the location of an error after a small delay (to prevent spikes) when the data written is not the same as the reference and only when the data was initially valid. Do not allow to be triggered at the testbench end.
+  dbg_error_location <= '1' AFTER c_dp_clk_period/5 WHEN ( (data_check_arr(prev_check_adr) /= TO_UINT(st_histogram_dbg_ram_miso.rddata) ) AND st_histogram_snk_in_pppp.valid='1' AND tb_end='0' ) ELSE '0';
+  ASSERT dbg_error_location='0' REPORT "The value written to the RAM is not what it should be. Comparison failed on (bin and array) address: " &integer'image(prev_check_adr) SEVERITY ERROR;
+  
+
+  --error count
+  p_count_total_error_cnt : PROCESS (dp_clk, dbg_error_location)
+  BEGIN
+    IF dp_rst='1' THEN
+      error_cnt <= 0;
+    ELSIF dbg_error_location='1' AND tb_end='0' AND rising_edge(dp_clk) THEN
+      error_cnt <= error_cnt + 1;
+    END IF;
+  END PROCESS;
+
+  p_view_total_error_cnt : PROCESS (tb_end, error_cnt)
+  BEGIN
+    IF tb_end='1' AND error_cnt>0 THEN
+      REPORT "When comparing there were " &integer'image(error_cnt) &" cycles where the value in the RAM address was not the value expected" SEVERITY ERROR;
+    END IF;
+  END PROCESS;
+  
 END tb;

libraries/dsp/st/tb/vhdl/tb_tb_st_histogram.vhd

+-------------------------------------------------------------------------------
+--
+-- Copyright 2020
+-- ASTRON (Netherlands Institute for Radio Astronomy) <http://www.astron.nl/>
+-- P.O.Box 2, 7990 AA Dwingeloo, The Netherlands
+-- 
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+-- 
+--     http://www.apache.org/licenses/LICENSE-2.0
+-- 
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+-------------------------------------------------------------------------------
+
+-------------------------------------------------------------------------------
+-- 
+-- Author: J.W.E. Oudman
+-- Purpose:
+-- Description: 
+-- .
+
+LIBRARY IEEE;
+USE IEEE.std_logic_1164.ALL;
+
+ENTITY tb_tb_st_histogram IS 
+END tb_tb_st_histogram;
+
+ARCHITECTURE tb OF tb_tb_st_histogram IS
+  SIGNAL tb_end : STD_LOGIC := '0';  -- declare tb_end to avoid 'No objects found' error on 'when -label tb_end'
+BEGIN
+
+-- Usage
+--   > as 8
+--   > run -all 
+--   > Testbenches are self-checking
+
+--    
+--  g_sync_length          : NATURAL := 200;
+--  g_nof_sync             : NATURAL := 3;
+--  g_data_w               : NATURAL := 4;
+--  g_nof_bins             : NATURAL := 8;
+--  g_nof_data             : NATURAL := 200;
+--  --g_str                  : STRING  := "freq.density";
+--  g_valid_gap            : STRING  := "custom";  -- "false" or "true" or "custom"
+--  g_snk_in_data_sim_type : STRING  := "same rw"  -- "counter" or "toggle" or "same rw" or "mix"
+--
+
+-- do test for different number of bins 
+u_tb_st_histogram_counter_nof_2 : ENTITY work.tb_st_histogram GENERIC MAP (200, 3, 1, 2, 200, "true"  , "counter" );
+u_tb_st_histogram_counter_nof_4 : ENTITY work.tb_st_histogram GENERIC MAP (200, 3, 2, 4, 200, "true"  , "counter" );
+u_tb_st_histogram_counter       : ENTITY work.tb_st_histogram GENERIC MAP (200, 3, 4, 8, 200, "true"  , "counter" );
+
+-- do tests for RAM delay issues
+u_tb_st_histogram_toggle        : ENTITY work.tb_st_histogram GENERIC MAP (200, 3, 4, 8, 200, "true"  , "toggle"  );
+u_tb_st_histogram_same_rw       : ENTITY work.tb_st_histogram GENERIC MAP (200, 3, 4, 8, 200, "custom", "same rw" );
+
+END tb;