diff --git a/libraries/dsp/st/hdllib.cfg b/libraries/dsp/st/hdllib.cfg
index 21443d39f3629e98383ef0678cc9191f97d8bb4b..3617a5976a7da4b76d245c3cbcd1c1cfdcfb6d71 100644
--- a/libraries/dsp/st/hdllib.cfg
+++ b/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
diff --git a/libraries/dsp/st/src/vhdl/st_histogram_8_april.vhd b/libraries/dsp/st/src/vhdl/st_histogram_8_april.vhd
index 91589c0f535a511a810e88f961aefd8490ee5cdf..f97fe410bead4491983e3b3531b8cf7090e607a8 100644
--- a/libraries/dsp/st/src/vhdl/st_histogram_8_april.vhd
+++ b/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';
@@ -92,11 +93,18 @@ BEGIN
-----------------------------------------------------------------------------
-- Bin reader: Convert snk_in data to bin_reader_mosi with read request
- -- . in : snk_in (latency: 0)
- -- . out : bin_reader_mosi (latency: 0)
- -- . out : bin_reader_mosi_pp (latency: 2)
- -- - out : rd_cnt_allowed_pp (latency: 2)
+ -- . 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
@@ -290,7 +299,10 @@ BEGIN
p_bin_writer_mosi : PROCESS(dp_clk, dp_rst, nxt_bin_writer_mosi, nxt_prev_wrdata, prev_wrdata, prev_prev_wrdata) IS
BEGIN
IF dp_rst = '1' THEN
- bin_writer_mosi <= c_mem_mosi_rst;
+ 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;
@@ -331,6 +347,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;
-----------------------------------------------------------------------------
diff --git a/libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd b/libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd
index 8fef744bfa7754c47fe7a290ed6c17018bcdeb6f..3ea1472ef8d64ec30d22a176f02c905da40c9fb2 100644
--- a/libraries/dsp/st/tb/vhdl/tb_st_histogram.vhd
+++ b/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'.
--
-------------------------------------------------------------------------------
diff --git a/libraries/dsp/st/tb/vhdl/tb_tb_st_histogram.vhd b/libraries/dsp/st/tb/vhdl/tb_tb_st_histogram.vhd
new file mode 100644
index 0000000000000000000000000000000000000000..15c4f2fddae29e9d2f00dd172de76c904ecff142
--- /dev/null
+++ b/libraries/dsp/st/tb/vhdl/tb_tb_st_histogram.vhd
@@ -0,0 +1,63 @@
+-------------------------------------------------------------------------------
+--
+-- 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;