added tb_tb_st_histogram.vhd and changed part of the descriptions

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/src/vhdl/st_histogram_8_april.vhd

@@ -57,6 +57,7 @@ ARCHITECTURE rtl OF st_histogram_8_april IS
   SIGNAL nxt_bin_arbiter_rd_mosi  : t_mem_mosi;
   SIGNAL bin_arbiter_rd_mosi      : t_mem_mosi;
 
+  SIGNAL bin_arbiter_rd_miso      : t_mem_miso := c_mem_miso_rst;
   SIGNAL common_ram_r_w_0_miso    : t_mem_miso := c_mem_miso_rst;
   
   SIGNAL init_phase           : STD_LOGIC := '1';
@@ -93,10 +94,17 @@ BEGIN
   -----------------------------------------------------------------------------
   -- Bin reader: Convert snk_in data to bin_reader_mosi with read request
   -- . in  : snk_in               (latency: 0)
+  -- . out : init_phase           (latency: 0 ?
   -- . out : bin_reader_mosi      (latency: 0)
+  -- . out : prev_bin_reader_mosi (latency: 1)
   -- . out : bin_reader_mosi_pp   (latency: 2)
+  -- . out : bin_reader_mosi_ppp  (latency: 3)
   -- - out : rd_cnt_allowed_pp    (latency: 2)
   -- . out : dp_pipeline_src_out_pp (latency: 2)                                  -- unnecesary
+  -- . out : same_r_w_address_pp  (latency: 2)
+  -- . out : toggle_detect_pp     (latency: 2)
+  -- . out : sync_detect          (latency: 0)
+  -- . out : sync_detect_pp       (latency: 2)
   -----------------------------------------------------------------------------
   bin_reader_mosi.rd                          <= snk_in.valid; -- when 1, count allowed
   bin_reader_mosi.address(c_adr_w-1 DOWNTO 0) <= snk_in.data(g_in_data_w-1 DOWNTO c_adr_low); 
@@ -113,7 +121,7 @@ BEGIN
     src_out      => dp_pipeline_src_out_p
   );
   
-  init_phase <= '0' WHEN dp_pipeline_src_out_p.sync = '1';
+  init_phase <= '0' WHEN dp_pipeline_src_out_p.sync = '1'; -- ELSE will be impossible since the init_phase may only be triggered once on the first sync
   
   -- Enable sync comparisons
   u_dp_pipeline_snk_in_2_cycle : ENTITY dp_lib.dp_pipeline
@@ -129,8 +137,8 @@ BEGIN
   
   dbg_snk_data <= dp_pipeline_src_out_pp.data(g_in_data_w-1 DOWNTO 0);
   
-  toggle_detect_false <= '0' WHEN dp_pipeline_src_out_pp.sync = '1';
-  sync_detect <= snk_in.valid WHEN (snk_in.sync='1' OR dp_pipeline_src_out_p.sync='1' OR dp_pipeline_src_out_pp.sync='1') ELSE '0';
+  toggle_detect_false <= '0' WHEN dp_pipeline_src_out_pp.sync = '1'; -- ELSE will be impossible since the toggle_detect_false may only be triggered once on the first sync
+  sync_detect <= snk_in.valid WHEN (snk_in.sync='1' OR dp_pipeline_src_out_p.sync='1' OR dp_pipeline_src_out_pp.sync='1') ELSE '0'; -- @sync, first 3 cycles would try to read from the wrong (old) RAM block, detect this problem
   
   -- Line up to p_nxt_bin_writer_mosi process  
   u_common_pipeline_sl_sync_detect_2_cycle : ENTITY common_lib.common_pipeline_sl
@@ -241,20 +249,21 @@ BEGIN
   -- . in  : toggle_detect_pp      (latency: 2)
   -- . in  : same_r_w_address_pp   (latency: 2)
   -- . in  : bin_reader_mosi_pp    (latency: 2)
-  -- . in  : common_ram_r_w_0_miso (latency: 2)
+  -- . in  : bin_arbiter_rd_miso   (latency: 2) common_ram_r_w_0_miso
   -- . in  : rd_cnt_allowed_pp     (latency: 2)
+  -- . in  : sync_detect_pp
   -- . out : bin_writer_mosi  (latency: 3)
   -----------------------------------------------------------------------------
-  p_nxt_bin_writer_mosi : PROCESS(common_ram_r_w_0_miso, common_ram_r_w_0_miso.rdval, common_ram_r_w_0_miso.rddata, 
-                                  bin_reader_mosi_pp.address, toggle_detect_pp, rd_cnt_allowed_pp, init_phase, prev_wrdata, prev_prev_wrdata, prev_prev_prev_wrdata, sync_detect_pp, same_r_w_address_pp, dp_pipeline_src_out_pp.valid) IS -- dp_pipeline_src_out_pp necesary??
+  p_nxt_bin_writer_mosi : PROCESS(bin_arbiter_rd_miso, 
+                                  bin_reader_mosi_pp.address, toggle_detect_pp, rd_cnt_allowed_pp, init_phase, prev_wrdata, prev_prev_wrdata, prev_prev_prev_wrdata, sync_detect_pp, same_r_w_address_pp, dp_pipeline_src_out_pp.valid) IS -- dp_pipeline_src_out_pp necesary?? init_phase unnecesary? ; removed: common_ram_r_w_0_miso.rdval, common_ram_r_w_0_miso.rddata,
   BEGIN
     nxt_bin_writer_mosi <= c_mem_mosi_rst;
     dbg_state_string <= "unv";
-    IF common_ram_r_w_0_miso.rdval='1' THEN
+    IF bin_arbiter_rd_miso.rdval='1' THEN  --  common_ram_r_w_0_miso
       nxt_bin_writer_mosi.wr      <= '1';
-      nxt_bin_writer_mosi.wrdata  <= INCR_UVEC(common_ram_r_w_0_miso.rddata, 1);
+      nxt_bin_writer_mosi.wrdata  <= INCR_UVEC(bin_arbiter_rd_miso.rddata, 1); -- common_ram_r_w_0_miso
       nxt_bin_writer_mosi.address <= bin_reader_mosi_pp.address;
-      nxt_prev_wrdata             <= TO_UINT(common_ram_r_w_0_miso.rddata) + 1;
+      nxt_prev_wrdata             <= TO_UINT(bin_arbiter_rd_miso.rddata) + 1;  -- common_ram_r_w_0_miso
       dbg_state_string <= "val";
 
     ELSIF toggle_detect_pp = '1' THEN
@@ -291,6 +300,9 @@ BEGIN
   BEGIN
     IF dp_rst = '1' THEN
        bin_writer_mosi       <= c_mem_mosi_rst;
+       prev_wrdata           <= 0;
+       prev_prev_wrdata      <= 0;
+       prev_prev_prev_wrdata <= 0;
     ELSIF RISING_EDGE(dp_clk) THEN
        bin_writer_mosi <= nxt_bin_writer_mosi;
        IF nxt_bin_writer_mosi.wr = '1' THEN
@@ -307,8 +319,12 @@ BEGIN
   -- . in  : bin_reader_mosi      (latency: 0)
   --       : init_phase           (latency: 0)
   --       : prev_bin_reader_mosi (latency: 1)
+  --       : bin_reader_mosi_pp   (latency: 2)
+  --       : bin_reader_mosi_ppp  (latency: 3)
   --       : bin_writer_mosi      (latency: 3)
+  --       : sync_detect          (latency:    0? or 3?
   -- . out : bin_arbiter_rd_mosi (latency: 1)
+  -- .     : bin_arbiter_rd_miso (latency: 2)
   -- .     : bin_arbiter_wr_mosi (latency: 4)
   -----------------------------------------------------------------------------
   nxt_bin_arbiter_wr_mosi <= bin_writer_mosi;
@@ -332,6 +348,9 @@ BEGIN
   -- Temporary debug data
   ram_miso.rddata <= bin_arbiter_wr_mosi.wrdata;
   
+  -- Make RAM data available for the bin_reader (or bin_writer)
+  bin_arbiter_rd_miso <= common_ram_r_w_0_miso;
+
 
   -----------------------------------------------------------------------------
   -- RAM that contains the bins

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

@@ -29,18 +29,25 @@
 --   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'.
 --
 -------------------------------------------------------------------------------
 

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;