From aaaca5bff9009b2dd6c62d2162b050087ae79597 Mon Sep 17 00:00:00 2001 From: Niels Garibaldi Date: Thu, 10 Sep 2026 15:58:22 +0200 Subject: [PATCH] fpga: dsp: add FSM for HB FIR temporal decimation Added a state machine to fix a bug in the temporal decimation code of the decimating halfband filter module. Additionally added a testcase to the testbench for the issue this change is fixing. --- .../lib/dsp/hb_filter/axis_hb_decim_fir.sv | 129 ++++++++++++------ .../axis_hb_decim_fir_tb.sv | 75 ++++++++-- 2 files changed, 153 insertions(+), 51 deletions(-) diff --git a/fpga/usrp3/lib/dsp/hb_filter/axis_hb_decim_fir.sv b/fpga/usrp3/lib/dsp/hb_filter/axis_hb_decim_fir.sv index 9ae2d0cb9..bf47cd351 100644 --- a/fpga/usrp3/lib/dsp/hb_filter/axis_hb_decim_fir.sv +++ b/fpga/usrp3/lib/dsp/hb_filter/axis_hb_decim_fir.sv @@ -293,57 +293,102 @@ module axis_hb_decim_fir #( // Decimation logic to reduce samples by 2 if ((SPC_IN == 1) && (SPC_OUT == 1)) begin : decim_over_time - // For single sample per clock, decimation is achieved by taking every 2nd sample in time - logic toggle_sample; - logic pair_phase; - logic delayed_tlast; - logic tlast_fifo_i_data, tlast_fifo_i_valid; - logic tlast_fifo_o_data, tlast_fifo_o_valid, tlast_fifo_o_ready; + // For a scalar stream, decimate continuously across packet boundaries. + // Hold each retained sample until the following dropped sample determines + // whether the retained sample is the last output of its packet. + // The state describes the action applied to the next accepted input sample: + // KEEP_ST retains it, while DROP_ST discards it. + typedef enum logic { + KEEP_ST, + DROP_ST + } decim_state_t; + + decim_state_t decim_state; + // Indicates that output_tdata contains a retained sample whose TLAST is not + // yet known. It becomes AXI-valid when the following dropped sample arrives. + logic pending_tvalid; + logic [SPC_OUT*SAMP_W-1:0] output_tdata = 'X; + logic output_tvalid; + logic output_tlast = 'X; + logic output_tready; + + // Internal output-register load enable: The register can be loaded when + // empty or when its current value transfers downstream on this cycle. + // (similar logic to axi_fifo_flop) + assign output_tready = !output_tvalid | out_tready; + + // A DROP_ST input does not consume output storage. If it releases a pending + // sample, the output register is guaranteed to be invalid, so the input may + // be accepted independently of downstream backpressure. + assign post_filter_tready = output_tready | decim_state == DROP_ST; always_ff @(posedge clk) begin - if (rst | clear | in_tlast_i) begin - pair_phase <= 1'b0; - delayed_tlast <= 1'b0; - end else if (in_tvalid_i && enable && in_tready_i) begin - pair_phase <= ~pair_phase; - delayed_tlast <= in_tlast_i; + // Clear the output register when the downstream can accept it. A later + // assignment below may refill the register on the same edge. + if (out_tready) begin + output_tvalid <= 1'b0; end - end - // Store one tlast bit for each pair of input samples. - assign tlast_fifo_i_valid = in_tvalid_i && enable && in_tready_i && (pair_phase | in_tlast_i); - assign tlast_fifo_i_data = delayed_tlast | in_tlast_i; - assign tlast_fifo_o_ready = out_tvalid && out_tready; + // update state machine on transfer from halfband filter + if (post_filter_tvalid && post_filter_tready) begin + // toggle state for each new sample + decim_state <= decim_state == KEEP_ST ? DROP_ST : KEEP_ST; + pending_tvalid <= 1'b0; - axi_fifo #( - .WIDTH(1), - .SIZE (1) - ) tlast_fifo ( - .clk (clk), - .reset (rst), - .clear (clear), - .i_tdata (tlast_fifo_i_data), - .i_tvalid (tlast_fifo_i_valid), - .i_tready (), - .o_tdata (tlast_fifo_o_data), - .o_tvalid (tlast_fifo_o_valid), - .o_tready (tlast_fifo_o_ready), - .space (), - .occupied () - ); + case (decim_state) + KEEP_ST: begin + // always store the current sample in the output register + output_tdata <= post_filter_tdata; + + // if this is the last sample of the packet, it should be output immediately + if (post_filter_tlast) begin + output_tdata <= post_filter_tdata; + output_tvalid <= 1'b1; + output_tlast <= 1'b1; + // otherwise, the sample will be held until the next one arrives + end else begin + pending_tvalid <= 1'b1; + end + end + DROP_ST: begin + // when dropping a sample, nothing is stored in the output register + // Depending on a previously retained sample, it may be output now + if (pending_tvalid) begin + output_tvalid <= 1'b1; + output_tlast <= post_filter_tlast; + end + end + endcase + end - always_ff @(posedge clk) begin if (rst | clear) begin - toggle_sample <= ~DECIMATION_PHASE; - end else if (post_filter_tvalid && post_filter_tready) begin - toggle_sample <= ~toggle_sample; + decim_state <= DECIMATION_PHASE ? DROP_ST : KEEP_ST; + pending_tvalid <= 1'b0; + output_tvalid <= 1'b0; end end - assign out_tdata = post_filter_tdata; // output the current sample directly - assign out_tvalid = post_filter_tvalid && toggle_sample && tlast_fifo_o_valid; - assign out_tlast = tlast_fifo_o_valid && tlast_fifo_o_data; - assign post_filter_tready = toggle_sample - ? (out_tready && tlast_fifo_o_valid) : 1'b1; + + assign out_tdata = output_tdata; + assign out_tvalid = output_tvalid; + assign out_tlast = output_tlast; + + // synthesis translate_off + property p_packet_has_retained_sample; + @(posedge clk) disable iff (rst || clear) + post_filter_tvalid && post_filter_tready + && decim_state == DROP_ST && post_filter_tlast + |-> pending_tvalid; + endproperty + assert property (p_packet_has_retained_sample) + else $error("Temporal decimation cannot represent an empty output packet"); + + property p_pending_sample_matches_state; + @(posedge clk) disable iff (rst || clear) + decim_state == KEEP_ST |-> !pending_tvalid; + endproperty + assert property (p_pending_sample_matches_state) + else $error("Temporal decimation retained-sample state is inconsistent"); + // synthesis translate_on end else begin : decim_by_2 // For multiple samples per clock, decimation is achieved by taking every 2nd sample in the vector diff --git a/fpga/usrp3/lib/dsp/sim/axis_hb_cascade_decim_tb/axis_hb_decim_fir_tb/axis_hb_decim_fir_tb.sv b/fpga/usrp3/lib/dsp/sim/axis_hb_cascade_decim_tb/axis_hb_decim_fir_tb/axis_hb_decim_fir_tb.sv index f37358a92..c34f208c4 100644 --- a/fpga/usrp3/lib/dsp/sim/axis_hb_cascade_decim_tb/axis_hb_decim_fir_tb/axis_hb_decim_fir_tb.sv +++ b/fpga/usrp3/lib/dsp/sim/axis_hb_cascade_decim_tb/axis_hb_decim_fir_tb/axis_hb_decim_fir_tb.sv @@ -262,6 +262,61 @@ module axis_hb_decim_fir_tb #( endtask : test_filter_response + // Verify packet boundaries when temporal decimation phase continues across + // consecutive odd-length packets. Three 3-word packets retain 2, 1, and 2 + // samples respectively. + task automatic test_temporal_packet_boundaries(); + localparam int NUM_PACKETS = 3; + localparam int PACKET_LEN_WORDS = 3; + localparam int EXPECTED_LENGTHS [NUM_PACKETS] = '{2, 1, 2}; + axis_input_pkt_t input_pkt; + axis_output_pkt_t received_pkt; + axis_input_pkt_mbox_t input_pkts = new(); + axis_pkt_single_sample_mbox_t input_samples = new(); + axis_pkt_single_sample_mbox_t expected_samples = new(); + axis_pkt_single_sample_t expected_sample; + int input_stall_prob; + int output_stall_prob; + AxiFirHBDecimModel #( + .SAMP_W(SAMP_W), + .NUM_COEFFS(NUM_COEFFS) + ) hb_model = new(); + + input_stall_prob = axi_bfm_in.get_master_stall_prob(); + output_stall_prob = axi_bfm_out.get_slave_stall_prob(); + axi_bfm_in.set_master_stall_prob(0); + axi_bfm_out.set_slave_stall_prob(75); + + repeat (NUM_PACKETS) begin + input_pkt = filter_utils_t::generate_zero_packet(PACKET_LEN_WORDS); + input_pkts.put(input_pkt.copy()); + axi_bfm_in.put(input_pkt); + end + + filter_utils_t::collect_and_serialize_packets(input_pkts, input_samples); + hb_model.process_samples(input_samples, expected_samples); + + for (int packet = 0; packet < NUM_PACKETS; packet++) begin + axi_bfm_out.get(received_pkt); + `ASSERT_ERROR(received_pkt.data.size() == EXPECTED_LENGTHS[packet], + $sformatf("Output packet %0d: expected %0d words, received %0d", + packet, EXPECTED_LENGTHS[packet], received_pkt.data.size())); + foreach (received_pkt.data[word]) begin + `ASSERT_ERROR(expected_samples.try_get(expected_sample), + $sformatf("Output packet %0d word %0d has no expected sample", packet, word)); + `ASSERT_ERROR(received_pkt.data[word] == expected_sample.data[0], + $sformatf("Output packet %0d word %0d: expected 0x%0h, received 0x%0h", + packet, word, expected_sample.data[0], received_pkt.data[word])); + end + end + + `ASSERT_ERROR(expected_samples.num() == 0, + $sformatf("Expected model contains %0d unconsumed samples", expected_samples.num())); + axi_bfm_in.set_master_stall_prob(input_stall_prob); + axi_bfm_out.set_slave_stall_prob(output_stall_prob); + endtask : test_temporal_packet_boundaries + + // Test that after a `clear` pulse (without full reset) the PRELOAD_ZEROES // mechanism restores a clean filter state, producing an impulse response // identical to what follows a hard reset (both start from zero tap history). @@ -291,9 +346,8 @@ module axis_hb_decim_fir_tb #( task automatic test_preload_after_clear(); // Enough words to fill the entire FIR data shift register with non-zero data: // SHIFT_REG_WIDTH = (SPC+1)*NUM_COEFFS entries; each word contributes SPC entries. - // Using 2*NUM_COEFFS+1 words ensures an odd packet length so that the last - // output always falls on a "kept" toggle-sample (toggle=1), preventing a - // pending_tlast deadlock in axi_bfm_out.get(). + // Using 2*NUM_COEFFS+1 words ensures the packet contains enough retained + // samples to drain and contaminate the complete filter history. localparam int CONTAM_LEN_WORDS = 2 * NUM_COEFFS + 1; localparam int IMPULSE_LEN_WORDS = FILTER_PIPELINE_DELAY + NUM_COEFFS + 1; // Number of DSP-pipeline output samples to skip before checking impulse data. @@ -327,12 +381,7 @@ module axis_hb_decim_fir_tb #( // Send the contam packet. Wait for full TX completion before draining output: // this guarantees no residual contam words remain queued in axi_bfm_in before // the clear pulse, so data_shift_reg is not re-dirtied after the preload. - // Use an odd packet length to guarantee the contam tlast falls on a "kept" - // toggle-sample slot (toggle_sample=1 at test start; odd length means the - // last word arrives on an even beat index, i.e., toggle=1 -> tlast fires - // immediately instead of setting pending_tlast, which would deadlock get()). - // Use a fixed odd length (not random) so the property holds regardless of - // test ordering or RNG seed. + // Use a fixed length so the test remains independent of RNG ordering. contam_pkt = filter_utils_t::generate_random_packet( CONTAM_LEN_WORDS, CONTAM_LEN_WORDS); axi_bfm_in.put(contam_pkt); @@ -446,6 +495,14 @@ module axis_hb_decim_fir_tb #( test.end_test(); end + if (SPC == 1) begin + test.start_test("Temporal Packet Boundary Test", 100us); + test_temporal_packet_boundaries(); + clk_gen.reset(); + @(negedge rst); + test.end_test(); + end + clk_gen.kill(); test.end_tb(0); end : main