From 7a454547d8608c94e35a7f7bb811b0731d86cd26 Mon Sep 17 00:00:00 2001
From: Daniel van der Schuur <schuur@astron.nl>
Date: Thu, 20 May 2021 13:25:10 +0200
Subject: [PATCH] -Cleaned tb_tb_st_histogram.vhd -Fixed/passed on generics
 -Instantiated tb with 2nd parameter set in tb_tb (fails as expected due to
 rd/wr clash in bin_arbiter.

---
 libraries/dsp/st/src/vhdl/st_histogram.vhd    | 57 +++++++++++--------
 libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd  | 54 +++++++-----------
 .../dsp/st/tb/vhdl/tb_tb_st_histogram.vhd     | 43 +++++---------
 3 files changed, 67 insertions(+), 87 deletions(-)

diff --git a/libraries/dsp/st/src/vhdl/st_histogram.vhd b/libraries/dsp/st/src/vhdl/st_histogram.vhd
index 5c9934f8a5..95ea6a6131 100644
--- a/libraries/dsp/st/src/vhdl/st_histogram.vhd
+++ b/libraries/dsp/st/src/vhdl/st_histogram.vhd
@@ -58,6 +58,11 @@ USE dp_lib.dp_stream_pkg.ALL;
 USE technology_lib.technology_select_pkg.ALL;
 
 ENTITY st_histogram IS
+  GENERIC (
+    g_data_w            : NATURAL := 8;
+    g_nof_bins          : NATURAL := 256;
+    g_nof_data_per_sync : NATURAL := 1024
+  );
   PORT (            
     dp_clk    : IN  STD_LOGIC;
     dp_rst    : IN  STD_LOGIC;
@@ -74,9 +79,12 @@ END st_histogram;
 
 ARCHITECTURE rtl OF st_histogram IS
 
-  CONSTANT c_nof_bins  : NATURAL := 256;
-  CONSTANT c_in_data_w : NATURAL := 8;
-  CONSTANT c_adr_low   : NATURAL := c_in_data_w-8;  -- 0
+  -------------------------------------------------------------------------------
+  -- Constants derived from generics
+  -------------------------------------------------------------------------------
+  CONSTANT c_ram_adr_w : NATURAL := ceil_log2(g_nof_bins);
+  CONSTANT c_adr_low   : NATURAL := g_data_w-c_ram_adr_w; 
+  CONSTANT c_ram_dat_w : NATURAL := ceil_log2(g_nof_data_per_sync)+1;
 
   -------------------------------------------------------------------------------
   -- ram_pointer
@@ -92,7 +100,7 @@ ARCHITECTURE rtl OF st_histogram IS
   SIGNAL bin_reader_mosi         : t_mem_mosi;
   SIGNAL bin_reader_miso         : t_mem_miso;
 
-  SIGNAL prv_rd_address          : STD_LOGIC_VECTOR(32-1 DOWNTO 0);
+  SIGNAL prv_rd_address          : STD_LOGIC_VECTOR(c_mem_address_w-1 DOWNTO 0);
 
   -------------------------------------------------------------------------------
   -- bin_writer
@@ -121,11 +129,10 @@ ARCHITECTURE rtl OF st_histogram IS
   -------------------------------------------------------------------------------
   CONSTANT c_nof_common_ram_r_w : NATURAL := 2;
 
-  CONSTANT c_adr_w              : NATURAL := ceil_log2(c_nof_bins);
   CONSTANT c_ram                : t_c_mem := (latency  => 1,
-                                              adr_w    => c_adr_w,          -- 8 bits needed to adress/select 256 adresses
-                                              dat_w    => c_word_w,         -- 32b, def. in common_pkg; >= c_bin_w
-                                              nof_dat  => c_nof_bins,       -- 256 adresses with 32b words
+                                              adr_w    => c_ram_adr_w,      -- 8 bits needed to adress/select 256 adresses
+                                              dat_w    => c_ram_dat_w, --c_word_w,         -- 32b, def. in common_pkg; >= c_bin_w --FIXME used to be c_word_w, check if this works
+                                              nof_dat  => g_nof_bins,       -- 256 adresses with 32b words
                                               init_sl  => '0');
 
   SIGNAL common_ram_r_w_wr_mosi_arr : t_mem_mosi_arr(1 DOWNTO 0);
@@ -139,8 +146,8 @@ ARCHITECTURE rtl OF st_histogram IS
   -------------------------------------------------------------------------------
   -- ram_clear 
   -------------------------------------------------------------------------------
-  SIGNAL address     : STD_LOGIC_VECTOR(c_adr_w-1 DOWNTO 0);
-  SIGNAL nxt_address : STD_LOGIC_VECTOR(c_adr_w-1 DOWNTO 0);
+  SIGNAL address     : STD_LOGIC_VECTOR(c_ram_adr_w-1 DOWNTO 0);
+  SIGNAL nxt_address : STD_LOGIC_VECTOR(c_ram_adr_w-1 DOWNTO 0);
 
   SIGNAL ram_clearing     : STD_LOGIC;
   SIGNAL nxt_ram_clearing : STD_LOGIC;
@@ -167,9 +174,9 @@ BEGIN
   END PROCESS;
 
   -- Don't toggle the RAM pointer on the first sync as we're already reading the RAM at that point.
-  nxt_toggle_ram_pointer  <= '1'             WHEN snk_in.sync='1' ELSE toggle_ram_pointer;
+  nxt_toggle_ram_pointer <= '1' WHEN snk_in.sync='1' ELSE toggle_ram_pointer;
   -- Toggle the RAM pointer starting from 2nd sync onwards
-  ram_pointer         <= NOT prv_ram_pointer WHEN snk_in.sync='1' AND toggle_ram_pointer='1' ELSE prv_ram_pointer;
+  ram_pointer <= NOT prv_ram_pointer WHEN snk_in.sync='1' AND toggle_ram_pointer='1' ELSE prv_ram_pointer;
 
 
   -------------------------------------------------------------------------------
@@ -184,7 +191,7 @@ BEGIN
   bin_reader_mosi.wrdata  <= (OTHERS=>'0');
   bin_reader_mosi.wr      <= '0';
   bin_reader_mosi.rd      <= snk_in.valid;
-  bin_reader_mosi.address <= RESIZE_UVEC(ram_pointer & snk_in.data(c_in_data_w-1 DOWNTO c_adr_low), 32); 
+  bin_reader_mosi.address <= RESIZE_UVEC(ram_pointer & snk_in.data(g_data_w-1 DOWNTO c_adr_low), c_word_w); 
 
   -- Store the rd address as bin_writer needs to know where to write the bin count
   p_prv_rd_address : PROCESS(dp_clk, dp_rst) IS
@@ -198,7 +205,7 @@ BEGIN
 
   -- Forward the read bin + count to bin writer
   bin_reader_to_writer_mosi.wr      <= bin_reader_miso.rdval;
-  bin_reader_to_writer_mosi.wrdata  <= RESIZE_UVEC(bin_reader_miso.rddata(8-1 DOWNTO 0), 72);
+  bin_reader_to_writer_mosi.wrdata  <= RESIZE_UVEC(bin_reader_miso.rddata(c_ram_dat_w-1 DOWNTO 0), c_mem_data_w);
   bin_reader_to_writer_mosi.address <= prv_rd_address;
 
 
@@ -271,8 +278,8 @@ BEGIN
   --         . bin_arbiter_wr_mosi.address
   --         . bin_arbiter_rd_mosi.address 
   -------------------------------------------------------------------------------
-  bin_arbiter_wr_ram_pointer <= bin_arbiter_wr_mosi.address(8); --FIXME (8) is not generic
-  bin_arbiter_rd_ram_pointer <= bin_arbiter_rd_mosi.address(8);  
+  bin_arbiter_wr_ram_pointer <= bin_arbiter_wr_mosi.address(c_ram_adr_w);
+  bin_arbiter_rd_ram_pointer <= bin_arbiter_rd_mosi.address(c_ram_adr_w);  
 
   -- Store the previous RAM pointer of the read bus
   p_prv_ram_pointer : PROCESS(dp_clk, dp_rst) IS
@@ -307,11 +314,11 @@ BEGIN
       clk      => dp_clk,
       clken    => '1',
       wr_en    => common_ram_r_w_wr_mosi_arr(i).wr,
-      wr_adr   => common_ram_r_w_wr_mosi_arr(i).address(8-1 DOWNTO 0),
-      wr_dat   => common_ram_r_w_wr_mosi_arr(i).wrdata(32-1 DOWNTO 0),
+      wr_adr   => common_ram_r_w_wr_mosi_arr(i).address(c_ram_adr_w-1 DOWNTO 0),
+      wr_dat   => common_ram_r_w_wr_mosi_arr(i).wrdata(c_ram_dat_w-1 DOWNTO 0),
       rd_en    => common_ram_r_w_rd_mosi_arr(i).rd,
-      rd_adr   => common_ram_r_w_rd_mosi_arr(i).address(8-1 DOWNTO 0),
-      rd_dat   => common_ram_r_w_rd_miso_arr(i).rddata(32-1 DOWNTO 0),
+      rd_adr   => common_ram_r_w_rd_mosi_arr(i).address(c_ram_adr_w-1 DOWNTO 0),
+      rd_dat   => common_ram_r_w_rd_miso_arr(i).rddata(c_ram_dat_w-1 DOWNTO 0),
       rd_val   => common_ram_r_w_rd_miso_arr(i).rdval
     );
   END GENERATE;
@@ -327,15 +334,15 @@ BEGIN
   --      ram_clear is set)
   -------------------------------------------------------------------------------
   -- Signal to indicate when RAM is being cleared
-  nxt_ram_clearing <= '1' WHEN ram_clear='1' ELSE '0' WHEN TO_UINT(address)=c_nof_bins-1 ELSE ram_clearing;
+  nxt_ram_clearing <= '1' WHEN ram_clear='1' ELSE '0' WHEN TO_UINT(address)=g_nof_bins-1 ELSE ram_clearing;
 
   -- Address counter: 0 to g_nof_bins-1.
   nxt_address <= INCR_UVEC(address, 1) WHEN ram_clearing='1' ELSE (OTHERS=>'0');
 
-  histogram_wr_mosi.wr                          <= ram_clearing;
-  histogram_wr_mosi.address(c_adr_w-1 DOWNTO 0) <= address;
-  histogram_wr_mosi.wrdata                      <= (OTHERS=>'0');
-  histogram_wr_mosi.rd                          <= '0';
+  histogram_wr_mosi.wr                              <= ram_clearing;
+  histogram_wr_mosi.address(c_ram_adr_w-1 DOWNTO 0) <= address;
+  histogram_wr_mosi.wrdata                          <= (OTHERS=>'0');
+  histogram_wr_mosi.rd                              <= '0';
 
   -- Registers
   p_ram_clearing : PROCESS(dp_clk, dp_rst) IS
diff --git a/libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd b/libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd
index f4f6299097..21756ab8da 100644
--- a/libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd
+++ b/libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd
@@ -22,7 +22,6 @@
 -- 
 -- Author: 
 -- . Daniel van der Schuur
--- . Jan Oudman
 -- Purpose:
 -- . Generate st_histogram input data, verify RAM contents. TB is self checking.
 -- ModelSim usage:
@@ -57,14 +56,10 @@ USE dp_lib.tb_dp_pkg.ALL;
 
 ENTITY tb_st_histogram IS
   GENERIC(
-    g_sync_length          : NATURAL := 200;
-    g_nof_sync             : NATURAL := 3;
-    g_data_w               : NATURAL := 8; --4 ; 1
-    g_nof_bins             : NATURAL := 256; --8 ; 2
-    g_nof_data             : NATURAL := 200;
-    --g_str                  : STRING  := "freq.density";
-    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"
+    g_nof_sync             : NATURAL := 4; -- We're simulating at least 4 g_nof_sync so both RAMs are written and cleared twice.
+    g_data_w               : NATURAL := 8; -- Determines maximum number of bins (2^g_data_w)
+    g_nof_bins             : NATURAL := 256; -- Lower than or equal to 2^g_data_w. Higher is allowed but makes no sense.
+    g_nof_data_per_sync    : NATURAL := 1024 -- Determines max required RAM data width. e.g. 11b to store max bin count '1024'.
     );
 END tb_st_histogram;
 
@@ -72,17 +67,12 @@ END tb_st_histogram;
 ARCHITECTURE tb OF tb_st_histogram IS
 
   ---------------------------------------------------------------------------
-  -- Sim parameters
-  -- . We're simulating at least 4 c_nof_sync_periods so both RAMs are written
-  --   and cleared twice.
+  -- Constants derived from generics
   ---------------------------------------------------------------------------
-  CONSTANT c_nof_sync_periods          : NATURAL := 4;
-  CONSTANT c_nof_packets_per_sync      : NATURAL := 3;
-  CONSTANT c_nof_data_per_packet       : NATURAL := 4*g_nof_bins; 
-  CONSTANT c_nof_data_per_sync         : NATURAL := c_nof_packets_per_sync*c_nof_data_per_packet;
-  CONSTANT c_nof_data_before_ram_clear : NATURAL := c_nof_data_per_sync-g_nof_bins; -- Clear RAM just before next Sync interval
+  CONSTANT c_nof_data_before_ram_clear : NATURAL := g_nof_data_per_sync-g_nof_bins; -- Clear RAM just before next Sync interval
+  CONSTANT c_expected_ram_content      : NATURAL := g_nof_data_per_sync/g_nof_bins;
 
-  CONSTANT c_expected_ram_content : NATURAL := c_nof_packets_per_sync * (c_nof_data_per_packet/g_nof_bins); -- 3*(1024/256)=12
+  CONSTANT c_ram_dat_w : NATURAL := ceil_log2(g_nof_data_per_sync)+1;
     
   ---------------------------------------------------------------------------
   -- Clocks and resets
@@ -113,7 +103,7 @@ ARCHITECTURE tb OF tb_st_histogram IS
    ----------------------------------------------------------------------------
    -- Automatic verification of RAM readout
    ----------------------------------------------------------------------------
-  SIGNAL ram_rd_word           : STD_LOGIC_VECTOR(g_data_w-1 DOWNTO 0);
+  SIGNAL ram_rd_word           : STD_LOGIC_VECTOR(c_ram_dat_w-1 DOWNTO 0);
   SIGNAL ram_rd_word_valid     : STD_LOGIC;
   SIGNAL nxt_ram_rd_word_valid : STD_LOGIC;
 
@@ -131,7 +121,7 @@ BEGIN
   ---------------------------------------------------------------------------- 
   stimuli_src_in <= c_dp_siso_rdy;
 
-  -- Generate c_nof_sync_periods*c_nof_packets_per_sync packets of c_nof_data_per_packet words
+  -- Generate g_nof_sync packets of g_nof_data_per_sync words
   p_generate_packets : PROCESS
     VARIABLE v_sosi : t_dp_sosi := c_dp_sosi_rst;
   BEGIN
@@ -139,13 +129,10 @@ BEGIN
     proc_common_wait_until_low(dp_clk, dp_rst);
     proc_common_wait_some_cycles(dp_clk, 5);
 
-    FOR I IN 0 TO c_nof_sync_periods-1 LOOP
+    FOR I IN 0 TO g_nof_sync-1 LOOP
       v_sosi.sync    := '1';
-      FOR I IN 0 TO c_nof_packets_per_sync-1 LOOP
-        v_sosi.data    := RESIZE_DP_DATA(v_sosi.data(g_data_w-1 DOWNTO 0));  -- wrap when >= 2**g_data_w    
-        proc_dp_gen_block_data(g_data_w, TO_UINT(v_sosi.data), c_nof_data_per_packet, TO_UINT(v_sosi.channel), TO_UINT(v_sosi.err), v_sosi.sync, v_sosi.bsn, dp_clk, stimuli_en, stimuli_src_in, stimuli_src_out);
-        v_sosi.sync    := '0';
-      END LOOP;
+      v_sosi.data    := RESIZE_DP_DATA(v_sosi.data(g_data_w-1 DOWNTO 0));  -- wrap when >= 2**g_data_w    
+      proc_dp_gen_block_data(g_data_w, TO_UINT(v_sosi.data), g_nof_data_per_sync, TO_UINT(v_sosi.channel), TO_UINT(v_sosi.err), v_sosi.sync, v_sosi.bsn, dp_clk, stimuli_en, stimuli_src_in, stimuli_src_out);
     END LOOP;     
 
     proc_common_wait_some_cycles(dp_clk, 50);
@@ -156,7 +143,7 @@ BEGIN
   -- Clear the RAM 
   p_ram_clear : PROCESS
   BEGIN
-    FOR I IN 0 TO c_nof_sync_periods-1 LOOP 
+    FOR I IN 0 TO g_nof_sync-1 LOOP 
       -- Sync up with p_generate_packets above by waiting for stimuli_src_out.sync
      proc_common_wait_until_high(dp_clk, stimuli_src_out.sync);      
       -- Wait until c_nof_data_before_ram_clear cycles have passed
@@ -177,10 +164,9 @@ BEGIN
 
   u_st_histogram : ENTITY work.st_histogram
   GENERIC MAP(
-    g_in_data_w         => g_data_w,
+    g_data_w            => g_data_w,
     g_nof_bins          => g_nof_bins,
-    g_nof_data          => g_nof_data,
-    g_ram_miso_sim_mode => FALSE
+    g_nof_data_per_sync => g_nof_data_per_sync
   )
   PORT MAP (
     dp_clk       => dp_clk,           
@@ -213,12 +199,12 @@ BEGIN
   p_verify_mm_read : PROCESS
   BEGIN
     st_histogram_ram_mosi.wr <= '0';
-    FOR i IN 0 TO c_nof_sync_periods-1 LOOP
+    FOR i IN 0 TO g_nof_sync-1 LOOP
       proc_common_wait_until_high(dp_clk, stimuli_src_out.sync);  
   
       FOR j IN 0 TO g_nof_bins-1 LOOP
         proc_mem_mm_bus_rd(j, dp_clk, st_histogram_ram_mosi); 
-        ram_rd_word <= st_histogram_ram_miso.rddata(g_data_w-1 DOWNTO 0);
+        ram_rd_word <= st_histogram_ram_miso.rddata(c_ram_dat_w-1 DOWNTO 0);
       END LOOP;
     END LOOP;
   END PROCESS;
@@ -237,13 +223,13 @@ BEGIN
   -- Perform verification of ram_rd_word when ram_rd_word_valid
   p_verify_assert : PROCESS
   BEGIN
-    FOR i IN 0 TO c_nof_sync_periods-1 LOOP
+    FOR i IN 0 TO g_nof_sync-1 LOOP
       proc_common_wait_until_high(dp_clk, stimuli_src_out.sync);  
       proc_common_wait_until_high(dp_clk, ram_rd_word_valid);        
       IF i=0 THEN -- Sync period 0: we expect RAM to contain zeros
         ASSERT TO_UINT(ram_rd_word)=0                      REPORT "RAM contains wrong bin count (expected 0)" SEVERITY ERROR;
       ELSE -- Sync period 1 onwards
-        ASSERT TO_UINT(ram_rd_word)=c_expected_ram_content REPORT "RAM contains wrong bin count (expected 12)" SEVERITY ERROR;
+        ASSERT TO_UINT(ram_rd_word)=c_expected_ram_content REPORT "RAM contains wrong bin count" SEVERITY ERROR;
       END IF;
     END LOOP;
   END PROCESS;
diff --git a/libraries/dsp/st/tb/vhdl/tb_tb_st_histogram.vhd b/libraries/dsp/st/tb/vhdl/tb_tb_st_histogram.vhd
index 5463e0c575..0459cb2638 100644
--- a/libraries/dsp/st/tb/vhdl/tb_tb_st_histogram.vhd
+++ b/libraries/dsp/st/tb/vhdl/tb_tb_st_histogram.vhd
@@ -18,12 +18,14 @@
 --
 -------------------------------------------------------------------------------
 
--------------------------------------------------------------------------------
--- 
--- Author: J.W.E. Oudman
+-- Author:
+-- . Daniel van der Schuur
 -- Purpose:
--- Description: 
--- .
+-- . Test tb_st_histogram in with several parameter sets
+-- Usage
+-- . as 8
+-- . run -all 
+-- . Testbenches are self-checking
 
 LIBRARY IEEE;
 USE IEEE.std_logic_1164.ALL;
@@ -34,30 +36,15 @@ 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
+  
+--  g_nof_sync             : NATURAL := 4;
+--  g_data_w               : NATURAL := 8;
+--  g_nof_bins             : NATURAL := 256;
+--  g_nof_data             : NATURAL := 1024;
 
--- 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, 8, 8, 200, "true"  , "counter" );
+u_tb_st_histogram_0 : ENTITY work.tb_st_histogram GENERIC MAP (4,  8, 256, 1024);
+u_tb_st_histogram_1 : ENTITY work.tb_st_histogram GENERIC MAP (4, 12, 256, 4096);
+--u_tb_st_histogram_2 : ENTITY work.tb_st_histogram GENERIC MAP (4, 8, 256, 1024);
 
--- 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;
-- 
GitLab