-Cleaned code, added comments;

-Fixed self-checking TB mechanism;
-Added TB instances with different parameter sets to TB_TB;
-TB_TB passes OK without errors.

libraries/dsp/st/src/vhdl/st_histogram.vhd

--------------------------------------------------------------------------------
+------------------------------------------------------------------------------
 --
 -- Copyright 2020
 -- ASTRON (Netherlands Institute for Radio Astronomy) <http://www.astron.nl/>
@@ -97,10 +97,10 @@ ARCHITECTURE rtl OF st_histogram IS
   -------------------------------------------------------------------------------
   -- bin_reader
   -------------------------------------------------------------------------------
-  SIGNAL bin_reader_mosi         : t_mem_mosi;
-  SIGNAL bin_reader_miso         : t_mem_miso;
+  SIGNAL bin_reader_mosi     : t_mem_mosi;
+  SIGNAL bin_reader_miso     : t_mem_miso;
 
-  SIGNAL prv_rd_address          : STD_LOGIC_VECTOR(c_mem_address_w-1 DOWNTO 0);
+  SIGNAL prv_bin_reader_mosi : t_mem_mosi;
 
   -------------------------------------------------------------------------------
   -- bin_writer
@@ -117,40 +117,41 @@ ARCHITECTURE rtl OF st_histogram IS
   SIGNAL bin_arbiter_rd_ram_pointer     : STD_LOGIC;
   SIGNAL prv_bin_arbiter_rd_ram_pointer : STD_LOGIC;
 
-  SIGNAL write_allowed           : BOOLEAN;
+  SIGNAL read_allowed                   : BOOLEAN;
+  SIGNAL prv_read_allowed               : BOOLEAN;
 
-  SIGNAL nxt_bin_arbiter_wr_mosi : t_mem_mosi;
-  SIGNAL bin_arbiter_wr_mosi     : t_mem_mosi;
-  SIGNAL bin_arbiter_rd_mosi     : t_mem_mosi;
-  SIGNAL bin_arbiter_rd_miso     : t_mem_miso;
+  SIGNAL nxt_bin_arbiter_wr_mosi        : t_mem_mosi;
+  SIGNAL bin_arbiter_wr_mosi            : t_mem_mosi;
+  SIGNAL bin_arbiter_rd_mosi            : t_mem_mosi;
+  SIGNAL bin_arbiter_rd_miso            : t_mem_miso;
 
   -------------------------------------------------------------------------------
   -- 2x RAM (common_ram_r_w) instances
   -------------------------------------------------------------------------------
-  CONSTANT c_nof_common_ram_r_w : NATURAL := 2;
+  CONSTANT c_nof_common_ram_r_w     : NATURAL := 2;
 
-  CONSTANT c_ram                : t_c_mem := (latency  => 1,
-                                              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');
+  CONSTANT c_ram                    : t_c_mem := (latency  => 1,
+                                                  adr_w    => c_ram_adr_w, 
+                                                  dat_w    => c_ram_dat_w,
+                                                  nof_dat  => g_nof_bins,
+                                                  init_sl  => '0');
 
   SIGNAL common_ram_r_w_wr_mosi_arr : t_mem_mosi_arr(1 DOWNTO 0);
   SIGNAL common_ram_r_w_rd_mosi_arr : t_mem_mosi_arr(1 DOWNTO 0);
   SIGNAL common_ram_r_w_rd_miso_arr : t_mem_miso_arr(1 DOWNTO 0); 
 
-  SIGNAL histogram_wr_mosi      : t_mem_mosi;
-  SIGNAL histogram_rd_mosi      : t_mem_mosi;
-  SIGNAL histogram_rd_miso      : t_mem_miso;
+  SIGNAL histogram_wr_mosi          : t_mem_mosi;
+  SIGNAL histogram_rd_mosi          : t_mem_mosi;
+  SIGNAL histogram_rd_miso          : t_mem_miso;
 
   -------------------------------------------------------------------------------
   -- ram_clear 
   -------------------------------------------------------------------------------
-  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_clear_address     : STD_LOGIC_VECTOR(c_ram_adr_w-1 DOWNTO 0);
+  SIGNAL nxt_ram_clear_address : STD_LOGIC_VECTOR(c_ram_adr_w-1 DOWNTO 0);
 
-  SIGNAL ram_clearing     : STD_LOGIC;
-  SIGNAL nxt_ram_clearing : STD_LOGIC;
+  SIGNAL ram_clearing          : STD_LOGIC;
+  SIGNAL nxt_ram_clearing      : STD_LOGIC;
 
 BEGIN 
 
@@ -180,7 +181,7 @@ BEGIN
 
 
   -------------------------------------------------------------------------------
-  -- bin_reader : reads bin from RAM, sends bin + count to bin_writer.
+  -- bin_reader : reads bin from RAM, sends bin to bin_writer.
   -- . Input  : snk_in          (input data stream)
   --            bin_reader_miso (reply to RAM read request: rddata = bin count)
   --            ram_pointer (to put in MOSI buses as MS address bit)
@@ -194,19 +195,19 @@ BEGIN
   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
+  p_prv_bin_reader_mosi : PROCESS(dp_clk, dp_rst) IS
   BEGIN
     IF dp_rst = '1' THEN
-      prv_rd_address <= (OTHERS=>'0');
+      prv_bin_reader_mosi <= c_mem_mosi_rst;
     ELSIF RISING_EDGE(dp_clk) THEN
-      prv_rd_address  <= bin_reader_mosi.address;
+      prv_bin_reader_mosi  <= bin_reader_mosi;
     END IF;
   END PROCESS;
 
   -- 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(c_ram_dat_w-1 DOWNTO 0), c_mem_data_w);
-  bin_reader_to_writer_mosi.address <= prv_rd_address;
+  bin_reader_to_writer_mosi.address <= prv_bin_reader_mosi.address;
 
 
   -------------------------------------------------------------------------------
@@ -217,15 +218,15 @@ BEGIN
   nxt_bin_writer_mosi.rd      <= '0';
   nxt_bin_writer_mosi.wr      <= bin_reader_to_writer_mosi.wr;
   nxt_bin_writer_mosi.address <= bin_reader_to_writer_mosi.address;
-  nxt_bin_writer_mosi.wrdata  <= INCR_UVEC(bin_reader_to_writer_mosi.wrdata, 1);
+  nxt_bin_writer_mosi.wrdata  <= INCR_UVEC(bin_reader_to_writer_mosi.wrdata, 1) WHEN bin_reader_to_writer_mosi.wr='1' ELSE bin_writer_mosi.wrdata; 
  
   -- Register the outputs to bin_arbiter (above we have a combinational adder = propagation delay)
   p_bin_writer_mosi : PROCESS(dp_clk, dp_rst) IS
   BEGIN
     IF dp_rst = '1' THEN
-      bin_writer_mosi <= c_mem_mosi_rst;
+      bin_writer_mosi     <= c_mem_mosi_rst;
     ELSIF RISING_EDGE(dp_clk) THEN
-      bin_writer_mosi <= nxt_bin_writer_mosi;
+      bin_writer_mosi     <= nxt_bin_writer_mosi;
     END IF;
   END PROCESS;
 
@@ -239,28 +240,41 @@ BEGIN
   --           bin_arbiter_rd_mosi (rd requests to RAM)
   --           bin_reader_miso (carries the bins requested by bin_reader)
   -------------------------------------------------------------------------------
-  -- Really simple arbitration: always allow reads, only allow writes when possible (rd_addr != wr_addr).
-  write_allowed <= TRUE WHEN bin_writer_mosi.address/=bin_reader_mosi.address ELSE FALSE;
-
-  bin_arbiter_rd_mosi.wr          <= '0';
-  bin_arbiter_rd_mosi.rd          <= bin_reader_mosi.rd; -- Reading is always allowed
-  bin_arbiter_rd_mosi.address     <= bin_reader_mosi.address;
-
-  nxt_bin_arbiter_wr_mosi.rd      <= '0';
-  nxt_bin_arbiter_wr_mosi.wr      <= bin_writer_mosi.wr      WHEN write_allowed  ELSE '0';
-  nxt_bin_arbiter_wr_mosi.wrdata  <= bin_writer_mosi.wrdata  WHEN write_allowed  ELSE (OTHERS=>'0');
-  nxt_bin_arbiter_wr_mosi.address <= bin_writer_mosi.address WHEN write_allowed  ELSE bin_reader_mosi.address;
-
-  -- Forward the rd_miso to the bin_reader
-  bin_reader_miso <= bin_arbiter_rd_miso;
-
-  -- Register the outputs to RAM
-  p_bin_arbiter_mosi : PROCESS(dp_clk, dp_rst) IS
+  -- Really simple arbitration: always allow writes, only allow reads when possible (rd_addr != wr_addr).
+  read_allowed <= FALSE WHEN bin_writer_mosi.wr='1' AND bin_writer_mosi.address=bin_reader_mosi.address ELSE TRUE;
+  -- save previous read_allowed
+  p_prv_read_allowed: PROCESS(dp_rst, dp_clk) IS
   BEGIN
-    IF dp_rst = '1' THEN
-      bin_arbiter_wr_mosi <= c_mem_mosi_rst;
+    IF dp_rst='1' THEN
+      prv_read_allowed <= FALSE;
     ELSIF RISING_EDGE(dp_clk) THEN
-      bin_arbiter_wr_mosi <= nxt_bin_arbiter_wr_mosi;
+      prv_read_allowed <= read_allowed;
+    END IF;
+  END PROCESS;
+
+  -- Forward MOSI buses
+  -- . RD MOSI
+  bin_arbiter_rd_mosi.wr      <= '0';
+  bin_arbiter_rd_mosi.rd      <= bin_reader_mosi.rd WHEN read_allowed ELSE '0';
+  bin_arbiter_rd_mosi.address <= bin_reader_mosi.address;
+  -- . WR MOSI
+  bin_arbiter_wr_mosi.rd      <= '0';
+  bin_arbiter_wr_mosi.wr      <= bin_writer_mosi.wr;
+  bin_arbiter_wr_mosi.wrdata  <= bin_writer_mosi.wrdata;
+  bin_arbiter_wr_mosi.address <= bin_writer_mosi.address;
+
+  -- Loop back the WR data to the RD side when read was not allowed or on second read of same address
+  p_bin_reader_miso : PROCESS(prv_read_allowed, bin_reader_mosi, bin_reader_miso, bin_writer_mosi, read_allowed, bin_arbiter_rd_miso) IS
+  BEGIN
+    bin_reader_miso <= bin_arbiter_rd_miso;
+    IF prv_bin_reader_mosi.rd = '1' AND prv_read_allowed = FALSE THEN -- Fake succesful readback when read was not allowed
+      bin_reader_miso.rdval  <= '1';
+      bin_reader_miso.rddata <= bin_writer_mosi.wrdata;
+    ELSIF read_allowed = TRUE THEN
+      bin_reader_miso <= bin_arbiter_rd_miso;
+    ELSIF (prv_bin_reader_mosi.rd = '1' AND bin_reader_mosi.rd='1') AND (prv_bin_reader_mosi.address=bin_reader_mosi.address) THEN -- 2 reads on same address in row: 2nd read is outdated so return wrdata here
+      bin_reader_miso.rdval  <= '1';
+      bin_reader_miso.rddata <= bin_writer_mosi.wrdata;
     END IF;
   END PROCESS;
 
@@ -334,13 +348,13 @@ 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)=g_nof_bins-1 ELSE ram_clearing;
+  nxt_ram_clearing <= '1' WHEN ram_clear='1' ELSE '0' WHEN TO_UINT(ram_clear_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');
+  nxt_ram_clear_address <= INCR_UVEC(ram_clear_address, 1) WHEN ram_clearing='1' ELSE (OTHERS=>'0');
 
   histogram_wr_mosi.wr                              <= ram_clearing;
-  histogram_wr_mosi.address(c_ram_adr_w-1 DOWNTO 0) <= address;
+  histogram_wr_mosi.address(c_ram_adr_w-1 DOWNTO 0) <= ram_clear_address;
   histogram_wr_mosi.wrdata                          <= (OTHERS=>'0');
   histogram_wr_mosi.rd                              <= '0';
 
@@ -348,11 +362,11 @@ BEGIN
   p_ram_clearing : PROCESS(dp_clk, dp_rst) IS
   BEGIN
     IF dp_rst = '1' THEN
-      address <= (OTHERS=>'0');
-      ram_clearing <= '0';     
+      ram_clear_address <= (OTHERS=>'0');
+      ram_clearing      <= '0';     
     ELSIF RISING_EDGE(dp_clk) THEN
-      address <= nxt_address;
-      ram_clearing <= nxt_ram_clearing;
+      ram_clear_address <= nxt_ram_clear_address;
+      ram_clearing      <= nxt_ram_clearing;
     END IF;
   END PROCESS;
 

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

@@ -69,7 +69,7 @@ ARCHITECTURE tb OF tb_st_histogram IS
   ---------------------------------------------------------------------------
   -- Constants derived from generics
   ---------------------------------------------------------------------------
-  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_nof_data_before_ram_clear : NATURAL := largest(1, 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_ram_dat_w : NATURAL := ceil_log2(g_nof_data_per_sync)+1;
@@ -104,6 +104,7 @@ ARCHITECTURE tb OF tb_st_histogram IS
    -- Automatic verification of RAM readout
    ----------------------------------------------------------------------------
   SIGNAL ram_rd_word           : STD_LOGIC_VECTOR(c_ram_dat_w-1 DOWNTO 0);
+  SIGNAL ram_rd_word_int       : NATURAL;
   SIGNAL ram_rd_word_valid     : STD_LOGIC;
   SIGNAL nxt_ram_rd_word_valid : STD_LOGIC;
 
@@ -200,11 +201,12 @@ BEGIN
   BEGIN
     st_histogram_ram_mosi.wr <= '0';
     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, stimuli_src_out.sync);  -- Wait for sync
+      proc_common_wait_some_cycles(dp_clk, 10); -- give it a couple of more cycles
       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(c_ram_dat_w-1 DOWNTO 0);
+        ram_rd_word_int <= TO_UINT(ram_rd_word);
       END LOOP;
     END LOOP;
   END PROCESS;
@@ -225,13 +227,14 @@ BEGIN
   BEGIN
     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);        
+      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;
+        ASSERT ram_rd_word_int=0                      REPORT "RAM contains wrong bin count (expected 0, actual " & INTEGER'IMAGE(ram_rd_word_int) & ")" SEVERITY ERROR;
       ELSE -- Sync period 1 onwards
-        ASSERT TO_UINT(ram_rd_word)=c_expected_ram_content REPORT "RAM contains wrong bin count" SEVERITY ERROR;
+        ASSERT ram_rd_word_int=c_expected_ram_content REPORT "RAM contains wrong bin count (expected " & INTEGER'IMAGE(c_expected_ram_content) & ", actual " & INTEGER'IMAGE(ram_rd_word_int) & ")" SEVERITY ERROR;
       END IF;
     END LOOP;
+    WAIT FOR 5 ns;
   END PROCESS;
   
 END tb;

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

@@ -42,9 +42,11 @@ BEGIN
 --  g_nof_bins             : NATURAL := 256;
 --  g_nof_data             : NATURAL := 1024;
 
-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);
+u_tb_st_histogram_0 : ENTITY work.tb_st_histogram GENERIC MAP ( 7,  8,  256, 1024); -- Incoming data wraps (repeats) 1024/ 256= 4 times: Bin count =  4
+u_tb_st_histogram_1 : ENTITY work.tb_st_histogram GENERIC MAP ( 6, 10,  256, 4096); -- Incoming data wraps (repeats) 4096/ 256=16 times: Bin count = 16
+u_tb_st_histogram_2 : ENTITY work.tb_st_histogram GENERIC MAP ( 5, 12,  512, 4096); -- Incoming data wraps (repeats) 4096/ 512= 8 times: Bin count =  8
+u_tb_st_histogram_3 : ENTITY work.tb_st_histogram GENERIC MAP ( 4, 13, 1024, 8192); -- Incoming data wraps (repeats) 8192/1024= 8 times: Bin count =  8
+u_tb_st_histogram_4 : ENTITY work.tb_st_histogram GENERIC MAP (40,  6,   64,  128); -- Incoming data wraps (repeats)  128/  64= 2 times: Bin count =  2
 
 
 END tb;