Skip to content

Commit

Permalink
found an obscure bug in the sample discriminator and redesigned its f…
Browse files Browse the repository at this point in the history 
…unctionality a bit. also improved code hygiene with some refactors of the testing code, putting reused parameterizable functions in a class and package
  • Loading branch information
@reed-foster
reed-foster committed Nov 30, 2023
1 parent eb045b0 commit 17cc159
Show file tree
Hide file tree
Showing 5 changed files with 268 additions and 188 deletions.
198 changes: 92 additions & 106 deletions dds_test.srcs/sim_1/new/sample_discriminator_test.sv
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import sim_util_pkg::*;

`timescale 1ns / 1ps
module sample_discriminator_test();

Expand All @@ -20,15 +22,14 @@ Axis_Parallel_If #(.DWIDTH(SAMPLE_WIDTH*PARALLEL_SAMPLES), .PARALLEL_CHANNELS(N_
Axis_Parallel_If #(.DWIDTH(SAMPLE_WIDTH*PARALLEL_SAMPLES), .PARALLEL_CHANNELS(N_CHANNELS)) data_out();
Axis_Parallel_If #(.DWIDTH(SAMPLE_INDEX_WIDTH+CLOCK_WIDTH), .PARALLEL_CHANNELS(N_CHANNELS)) timestamps_out();

typedef logic signed [SAMPLE_WIDTH-1:0] signed_sample_t;
logic [N_CHANNELS-1:0][SAMPLE_WIDTH-1:0] threshold_high, threshold_low;
always_comb begin
for (int i = 0; i < N_CHANNELS; i++) begin
config_in.data[2*SAMPLE_WIDTH*i+:2*SAMPLE_WIDTH] = {threshold_high[i], threshold_low[i]};
end
end

logic sample_index_reset;
logic reset_state;

sample_discriminator #(
.SAMPLE_WIDTH(SAMPLE_WIDTH),
Expand All @@ -43,7 +44,7 @@ sample_discriminator #(
.data_out,
.timestamps_out,
.config_in,
.sample_index_reset
.reset_state
);

logic [SAMPLE_WIDTH*PARALLEL_SAMPLES-1:0] data_sent [N_CHANNELS][$];
Expand Down Expand Up @@ -79,43 +80,14 @@ end
assign data_out.ready = '1;
assign timestamps_out.ready = '1;

// helper function to check if any of the parallel samples are above the high threshold
// needed to replicate the behavior of the DUT which starts saving samples as
// soon as a sample arrives which is above the high threshold
function logic any_above_high (
input logic [SAMPLE_WIDTH*PARALLEL_SAMPLES-1:0] samples_in,
input logic [SAMPLE_WIDTH-1:0] threshold_high
);
for (int j = 0; j < PARALLEL_SAMPLES; j++) begin
if (signed_sample_t'(samples_in[j*SAMPLE_WIDTH+:SAMPLE_WIDTH]) > signed_sample_t'(threshold_high)) begin
return 1'b1;
end
end
return 1'b0;
endfunction

// helper function to check if all parallel samples are below the low threshold
// needed to replicate the behavior of the DUT which stops saving samples
// once the DUT receives a row of samples all below the low threshold
function logic all_below_low (
input logic [SAMPLE_WIDTH*PARALLEL_SAMPLES-1:0] samples_in,
input logic [SAMPLE_WIDTH-1:0] threshold_low
);
for (int j = 0; j < PARALLEL_SAMPLES; j++) begin
if (signed_sample_t'(samples_in[j*SAMPLE_WIDTH+:SAMPLE_WIDTH]) > signed_sample_t'(threshold_low)) begin
return 1'b0;
end
end
return 1'b1;
endfunction

task check_results (
input logic [N_CHANNELS-1:0][SAMPLE_WIDTH-1:0] threshold_low,
input logic [N_CHANNELS-1:0][SAMPLE_WIDTH-1:0] threshold_high,
inout logic [N_CHANNELS-1:0][CLOCK_WIDTH-1:0] timer,
inout logic [N_CHANNELS-1:0][SAMPLE_INDEX_WIDTH-1:0] sample_index,
inout logic [N_CHANNELS-1:0] is_high
);
sim_util_pkg::sample_discriminator_util #(.SAMPLE_WIDTH(SAMPLE_WIDTH), .PARALLEL_SAMPLES(PARALLEL_SAMPLES)) util;
for (int i = 0; i < N_CHANNELS; i++) begin
// process each channel, first check that we received an appropriate amount of data
$display("data_sent[%0d].size() = %0d", i, data_sent[i].size());
Expand All @@ -128,7 +100,7 @@ task check_results (

// now process the sent/received data to check the timestamps and check for any mismatched data
while (data_sent[i].size() > 0) begin
if (any_above_high(data_sent[i][$], threshold_high[i])) begin
if (util.any_above_high(data_sent[i][$], threshold_high[i])) begin
if (!is_high[i]) begin
// new high, we should get a timestamp
if (timestamps_received[i].size() > 0) begin
Expand All @@ -143,7 +115,7 @@ task check_results (
end
end
is_high[i] = 1'b1;
end else if (all_below_low(data_sent[i][$], threshold_low[i])) begin
end else if (util.all_below_low(data_sent[i][$], threshold_low[i])) begin
is_high[i] = 1'b0;
end
if (is_high[i]) begin
Expand Down Expand Up @@ -178,92 +150,106 @@ initial begin
threshold_high <= '0;
data_in.valid <= '0;
config_in.valid <= '0;
sample_index_reset <= '0;
reset_state <= '0;
is_high <= '0;
timer <= '0;
sample_index <= '0;
repeat (100) @(posedge clk);
reset <= 1'b0;
sample_index_reset <= 1'b1;
reset_state <= 1'b1;
@(posedge clk);
sample_index_reset <= '0;
reset_state <= '0;
repeat (50) @(posedge clk);
config_in.valid <= 1'b1;
@(posedge clk);
config_in.valid <= 1'b0;
repeat (50) @(posedge clk);

// send a bunch of data with discrimination disabled
data_in.send_samples(clk, 10, 1'b1, 1'b1);
repeat (50) @(posedge clk);
data_in.send_samples(clk, 10, 1'b0, 1'b1);
repeat (50) @(posedge clk);
$display("######################################################");
$display("# testing run with all data above thresholds #");
$display("# first sample will be zero #");
$display("######################################################");
check_results(threshold_low, threshold_high, timer, sample_index, is_high);

// send a bunch of data, some below and some above the threshold on channel 0
// for channel 1, keep the same settings as before
data_range_low[0] <= 16'h0000;
data_range_high[0] <= 16'h04ff;
threshold_low[0] <= 16'h03c0;
threshold_high[0] <= 16'h0400;
config_in.valid <= 1'b1;
@(posedge clk);
config_in.valid <= 1'b0;
repeat (50) @(posedge clk);
data_in.send_samples(clk, 100, 1'b1, 1'b1);
repeat (50) @(posedge clk);
data_in.send_samples(clk, 100, 1'b0, 1'b1);
repeat (50) @(posedge clk);
$display("######################################################");
$display("# testing run with channel 0 straddling thresholds #");
$display("# and channel 1 above thresholds #");
$display("######################################################");
check_results(threshold_low, threshold_high, timer, sample_index, is_high);
for (int i = 0; i < 3; i++) begin
// loop a couple times, resetting the state to make sure we get the
// correct behavior
// send a bunch of data with discrimination disabled
data_in.send_samples(clk, 10, 1'b1, 1'b1);
repeat (50) @(posedge clk);
data_in.send_samples(clk, 10, 1'b0, 1'b1);
repeat (50) @(posedge clk);
$display("######################################################");
$display("# testing run with all data above thresholds #");
$display("# first sample will be zero #");
$display("######################################################");
check_results(threshold_low, threshold_high, timer, sample_index, is_high);

// send a bunch of data, some below and some above the threshold on channel 0
// for channel 1, keep the same settings as before
data_range_low[0] <= 16'h0000;
data_range_high[0] <= 16'h04ff;
threshold_low[0] <= 16'h03c0;
threshold_high[0] <= 16'h0400;
config_in.valid <= 1'b1;
@(posedge clk);
config_in.valid <= 1'b0;
repeat (50) @(posedge clk);
data_in.send_samples(clk, 100, 1'b1, 1'b1);
repeat (50) @(posedge clk);
data_in.send_samples(clk, 100, 1'b0, 1'b1);
repeat (50) @(posedge clk);
$display("######################################################");
$display("# testing run with channel 0 straddling thresholds #");
$display("# and channel 1 above thresholds #");
$display("######################################################");
check_results(threshold_low, threshold_high, timer, sample_index, is_high);

// send a bunch of data below the threshold
for (int i = 0; i < N_CHANNELS; i++) begin
data_range_low[i] <= 16'h0000;
data_range_high[i] <= 16'h00ff;
threshold_low[i] <= 16'h03ff;
threshold_high[i] <= 16'h0400;
end
config_in.valid <= 1'b1;
@(posedge clk);
config_in.valid <= 1'b0;
repeat (50) @(posedge clk);
data_in.send_samples(clk, 400, 1'b1, 1'b1);
repeat (50) @(posedge clk);
data_in.send_samples(clk, 400, 1'b0, 1'b1);
repeat (50) @(posedge clk);
$display("######################################################");
$display("# testing run with all data below thresholds #");
$display("######################################################");
check_results(threshold_low, threshold_high, timer, sample_index, is_high);
// send a bunch of data below the threshold
for (int i = 0; i < N_CHANNELS; i++) begin
data_range_low[i] <= 16'h0000;
data_range_high[i] <= 16'h00ff;
threshold_low[i] <= 16'h03ff;
threshold_high[i] <= 16'h0400;
end
config_in.valid <= 1'b1;
@(posedge clk);
config_in.valid <= 1'b0;
repeat (50) @(posedge clk);
data_in.send_samples(clk, 400, 1'b1, 1'b1);
repeat (50) @(posedge clk);
data_in.send_samples(clk, 400, 1'b0, 1'b1);
repeat (50) @(posedge clk);
$display("######################################################");
$display("# testing run with all data below thresholds #");
$display("######################################################");
check_results(threshold_low, threshold_high, timer, sample_index, is_high);

// send a bunch of data close to the threshold
for (int i = 0; i < N_CHANNELS; i++) begin
data_range_low[i] <= 16'h0000;
data_range_high[i] <= 16'h04ff;
threshold_low[i] <= 16'h03c0;
threshold_high[i] <= 16'h0400;
// send a bunch of data close to the threshold
for (int i = 0; i < N_CHANNELS; i++) begin
data_range_low[i] <= 16'h0000;
data_range_high[i] <= 16'h04ff;
threshold_low[i] <= 16'h03c0;
threshold_high[i] <= 16'h0400;
end
config_in.valid <= 1'b1;
@(posedge clk);
config_in.valid <= 1'b0;
repeat (50) @(posedge clk);
data_in.send_samples(clk, 400, 1'b1, 1'b1);
repeat (50) @(posedge clk);
data_in.send_samples(clk, 400, 1'b0, 1'b1);
repeat (50) @(posedge clk);
$display("######################################################");
$display("# testing run with both channels straddling #");
$display("# thresholds #");
$display("######################################################");
check_results(threshold_low, threshold_high, timer, sample_index, is_high);

// reset state of counter and is_high
repeat (10) @(posedge clk);
reset_state <= 1'b1;
@(posedge clk);
reset_state <= '0;
// need to make sure we keep track of this change
is_high <= '0;
sample_index <= '0;
repeat (10) @(posedge clk);
end
config_in.valid <= 1'b1;
@(posedge clk);
config_in.valid <= 1'b0;
repeat (50) @(posedge clk);
data_in.send_samples(clk, 400, 1'b1, 1'b1);
repeat (50) @(posedge clk);
data_in.send_samples(clk, 400, 1'b0, 1'b1);
repeat (50) @(posedge clk);
$display("######################################################");
$display("# testing run with both channels straddling #");
$display("# thresholds #");
$display("######################################################");
check_results(threshold_low, threshold_high, timer, sample_index, is_high);

$display("#################################################");
if (error_count == 0) begin
Expand Down
43 changes: 43 additions & 0 deletions dds_test.srcs/sim_1/new/sim_util_pkg.sv
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package sim_util_pkg;

class sample_discriminator_util #(
parameter int SAMPLE_WIDTH = 16,
parameter int PARALLEL_SAMPLES = 2
);

typedef logic signed [SAMPLE_WIDTH-1:0] signed_sample_t;

// helper function to check if any of the parallel samples are above the high threshold
// needed to replicate the behavior of the sample discriminator which starts saving
// samples as soon as a sample arrives which is above the high threshold
function logic any_above_high (
input logic [SAMPLE_WIDTH*PARALLEL_SAMPLES-1:0] samples_in,
input logic [SAMPLE_WIDTH-1:0] threshold_high
);
for (int j = 0; j < PARALLEL_SAMPLES; j++) begin
if (signed_sample_t'(samples_in[j*SAMPLE_WIDTH+:SAMPLE_WIDTH]) > signed_sample_t'(threshold_high)) begin
return 1'b1;
end
end
return 1'b0;
endfunction

// helper function to check if all parallel samples are below the low threshold
// needed to replicate the behavior of the sample discriminator which stops saving
// samples once all the samples it receives in a single clock cycle are below
// the low threshold
function logic all_below_low (
input logic [SAMPLE_WIDTH*PARALLEL_SAMPLES-1:0] samples_in,
input logic [SAMPLE_WIDTH-1:0] threshold_low
);
for (int j = 0; j < PARALLEL_SAMPLES; j++) begin
if (signed_sample_t'(samples_in[j*SAMPLE_WIDTH+:SAMPLE_WIDTH]) > signed_sample_t'(threshold_low)) begin
return 1'b0;
end
end
return 1'b1;
endfunction

endclass

endpackage
34 changes: 21 additions & 13 deletions dds_test.srcs/sources_1/new/sample_discriminator.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// The timestamp also contains a count of saved samples up to the event that triggered
// the creation of the timestamp.
// This allows the samples to be associated with specific sample that was saved.
// The sample count is reset every time a new capture is started.
// The sample count and hysteresis is reset every time a new capture is started.
module sample_discriminator #(
parameter int SAMPLE_WIDTH = 16,
parameter int PARALLEL_SAMPLES = 16,
Expand All @@ -18,7 +18,7 @@ module sample_discriminator #(
Axis_Parallel_If.Master_Simple data_out,
Axis_Parallel_If.Master_Simple timestamps_out,
Axis_If.Slave_Simple config_in, // {threshold_high, threshold_low} for each channel
input wire sample_index_reset
input wire reset_state
);

localparam int TIMESTAMP_WIDTH = SAMPLE_INDEX_WIDTH + CLOCK_WIDTH;
Expand Down Expand Up @@ -100,22 +100,30 @@ always_ff @(posedge clk) begin
sample_index <= '0;
end else begin
for (int i = 0; i < N_CHANNELS; i++) begin
// update sample_index
if (sample_index_reset) begin
// update sample_index and is_high
// don't reset timer, since we want
// to be able to track the arrival time of
// samples between multiple captures
if (reset_state) begin
sample_index[i] <= '0;
end else if (data_in_valid[i] && is_high[i]) begin
sample_index[i] <= sample_index[i] + 1'b1;
is_high[i] <= 1'b0;
end else begin
if (data_in_valid[i] && is_high[i]) begin
sample_index[i] <= sample_index[i] + 1'b1;
end
// update is_high only when we get a new sample
if (data_in.ok[i]) begin
if (any_above_high[i]) begin
is_high[i] <= 1'b1;
end else if (all_below_low[i]) begin
is_high[i] <= 1'b0;
end
end
end
// update timer
if (data_in.valid[i]) begin
if (data_in.ok[i]) begin
timer[i] <= timer[i] + 1'b1;
end
// update is_high
if (any_above_high[i]) begin
is_high[i] <= 1'b1;
end else if (all_below_low[i]) begin
is_high[i] <= 1'b0;
end
end
end
end
Expand Down
Loading

0 comments on commit 17cc159

Please sign in to comment.