From 1451ae066b55890d78a41e0baf259ecae5b8943a Mon Sep 17 00:00:00 2001 From: Ryan Volz Date: Thu, 21 Sep 2023 14:56:39 -0400 Subject: [PATCH] fpga/usrp3: Fix DC truncation bias by adding rounding to DDC chain. A digital DC bias caused by various truncations in the DDC chain was previously noticable with small signal levels, especially with high decimation rates. This patch eliminates the bias by replacing the truncation with rounding or simply keeping more bits for longer where it makes sense. This is essentially a forward port of a similar prior fix to the usrp2 DDC chain: https://github.com/EttusResearch/fpga/pull/4. Signed-off-by: Ryan Volz Original-commit: a57c162a0d11ab82a83c911a94272ceb6f7ff70a --- lib/dsp/ddc_chain.v | 71 ++++++++++++++++++++++++++++++++---------- lib/dsp/hb_dec.v | 30 ++++++++++++------ lib/dsp/small_hb_dec.v | 16 ++++++++-- 3 files changed, 88 insertions(+), 29 deletions(-) diff --git a/lib/dsp/ddc_chain.v b/lib/dsp/ddc_chain.v index 9c25d8e..3804264 100644 --- a/lib/dsp/ddc_chain.v +++ b/lib/dsp/ddc_chain.v @@ -191,8 +191,8 @@ module ddc_chain .coef_din(coef_din), // input [17 : 0] coef_din .rfd(rfd2), // output rfd .nd(nd2), // input nd - .din_1(i_hb1[23+HB1_SCALE:HB1_SCALE]), // input [23 : 0] din_1 - .din_2(q_hb1[23+HB1_SCALE:HB1_SCALE]), // input [23 : 0] din_2 + .din_1(i_hb1[WIDTH-1+HB1_SCALE:HB1_SCALE]), // input [23 : 0] din_1 + .din_2(q_hb1[WIDTH-1+HB1_SCALE:HB1_SCALE]), // input [23 : 0] din_2 .rdy(rdy2), // output rdy .data_valid(data_valid2), // output data_valid .dout_1(i_hb2), // output [46 : 0] dout_1 @@ -200,8 +200,8 @@ module ddc_chain - reg [18:0] i_unscaled, q_unscaled; - reg strobe_unscaled; + reg [WIDTH-1:0] i_unscaled, q_unscaled; + reg strobe_unscaled; always @(posedge clk) case({enable_hb1,enable_hb2}) @@ -209,42 +209,61 @@ module ddc_chain 2'd0 : begin strobe_unscaled <= strobe_cic; - i_unscaled <= i_cic[23:5]; - q_unscaled <= q_cic[23:5]; + i_unscaled <= i_cic; + q_unscaled <= q_cic; end // ILLEGAL. Only half sample rate half band enabled. 2'd1 : begin strobe_unscaled <= strobe_cic; - i_unscaled <= i_cic[23:5]; - q_unscaled <= q_cic[23:5]; + i_unscaled <= i_cic; + q_unscaled <= q_cic; end // One Halfband enabled, decimate by 2. 2'd2 : begin strobe_unscaled <= strobe_hb1; - i_unscaled <= i_hb1[23+HB1_SCALE:5+HB1_SCALE]; - q_unscaled <= q_hb1[23+HB1_SCALE:5+HB1_SCALE]; + i_unscaled <= i_hb1[WIDTH-1+HB1_SCALE:HB1_SCALE]; + q_unscaled <= q_hb1[WIDTH-1+HB1_SCALE:HB1_SCALE]; end // Both Halfbands enabled, decimate by 4. 2'd3 : begin strobe_unscaled <= strobe_hb2; - i_unscaled <= i_hb2[23+HB2_SCALE:5+HB2_SCALE]; - q_unscaled <= q_hb2[23+HB2_SCALE:5+HB2_SCALE]; + i_unscaled <= i_hb2[WIDTH-1+HB2_SCALE:HB2_SCALE]; + q_unscaled <= q_hb2[WIDTH-1+HB2_SCALE:HB2_SCALE]; end endcase // case (hb_rate) - // Need to clip 1 bit here or we loose small signal performance out the truncated LSB's for worst case CIC gain cases. + // round to 19 bits for clip (->18) followed by multiplication (total gain of 6 bits) + wire [18:0] i_unscaled_rnd, q_unscaled_rnd; + + round #( + .bits_in(WIDTH), + .bits_out(19) + ) unscaled_rnd_i ( + .in(i_unscaled), + .out(i_unscaled_rnd) + ); + round #( + .bits_in(WIDTH), + .bits_out(19) + ) unscaled_rnd_q ( + .in(q_unscaled), + .out(q_unscaled_rnd) + ); + + // Need to clip 1 bit here or we lose small signal performance out the + // truncated LSB's for worst case CIC gain cases. // NOTE: We can only clip here with CORDIC rotating, CIC in it's highest gain configurations and an input signal thats // saturated. wire strobe_unscaled_clip; wire [17:0] i_unscaled_clip, q_unscaled_clip; clip_reg #(.bits_in(19), .bits_out(18), .STROBED(1)) unscaled_clip_i - (.clk(clk), .in(i_unscaled[18:0]), .strobe_in(strobe_unscaled), .out(i_unscaled_clip[17:0]), .strobe_out(strobe_unscaled_clip)); + (.clk(clk), .in(i_unscaled_rnd), .strobe_in(strobe_unscaled), .out(i_unscaled_clip[17:0]), .strobe_out(strobe_unscaled_clip)); clip_reg #(.bits_in(19), .bits_out(18), .STROBED(1)) unscaled_clip_q - (.clk(clk), .in(q_unscaled[18:0]), .strobe_in(strobe_unscaled), .out(q_unscaled_clip[17:0]), .strobe_out()); + (.clk(clk), .in(q_unscaled_rnd), .strobe_in(strobe_unscaled), .out(q_unscaled_clip[17:0]), .strobe_out()); // Apply scaling gain to compensate for CORDIC and CIC gain adjustments so that signal swing over network transport has // optimal dynamic range. @@ -315,14 +334,32 @@ module ddc_chain (.clk(clk),.rst(rst),.bypass(~enable_hb2),.run(run),.cpi(cpi_hb), .stb_in(strobe_hb1),.data_in(q_hb1),.stb_out(),.data_out(q_hb2)); + // round to 19 bits for clip (->18) followed by multiplication (total gain of 6 bits) + wire [18:0] i_hb_out_rnd, q_hb_out_rnd; + + round #( + .bits_in(WIDTH), + .bits_out(19) + ) hb_out_rnd_i ( + .in(i_hb2), + .out(i_hb_out_rnd) + ); + round #( + .bits_in(WIDTH), + .bits_out(19) + ) hb_out_rnd_q ( + .in(q_hb2), + .out(q_hb_out_rnd) + ); + // Need to clip 1 bit here or we loose small signal performance out the truncated LSB's for worst case CIC gain cases. wire strobe_unscaled_clip; wire [17:0] i_unscaled_clip, q_unscaled_clip; clip_reg #(.bits_in(19), .bits_out(18), .STROBED(1)) unscaled_clip_i - (.clk(clk), .in(i_hb2[WIDTH-1:WIDTH-19]), .strobe_in(strobe_hb2), .out(i_unscaled_clip[17:0]), .strobe_out(strobe_unscaled_clip)); + (.clk(clk), .in(i_hb_out_rnd), .strobe_in(strobe_hb2), .out(i_unscaled_clip[17:0]), .strobe_out(strobe_unscaled_clip)); clip_reg #(.bits_in(19), .bits_out(18), .STROBED(1)) unscaled_clip_q - (.clk(clk), .in(q_hb2[WIDTH-1:WIDTH-19]), .strobe_in(strobe_hb2), .out(q_unscaled_clip[17:0]), .strobe_out()); + (.clk(clk), .in(q_hb_out_rnd), .strobe_in(strobe_hb2), .out(q_unscaled_clip[17:0]), .strobe_out()); //scalar operation (gain of 6 bits) wire [35:0] prod_i, prod_q; diff --git a/lib/dsp/hb_dec.v b/lib/dsp/hb_dec.v index 0cae5c4..7610bc4 100644 --- a/lib/dsp/hb_dec.v +++ b/lib/dsp/hb_dec.v @@ -26,7 +26,7 @@ module hb_dec output reg [WIDTH-1:0] data_out); localparam INTWIDTH = 17; - localparam ACCWIDTH = WIDTH + 3; + localparam ACCWIDTH = 36; // Round off inputs to 17 bits because of 18 bit multipliers wire [INTWIDTH-1:0] data_rnd; @@ -174,20 +174,30 @@ module hb_dec always @(posedge clk) sum_of_prod <= prod1 + prod2; // Can't overflow wire [ACCWIDTH-1:0] acc_out; - acc #(.IWIDTH(ACCWIDTH-2),.OWIDTH(ACCWIDTH)) - acc (.clk(clk),.clear(clear),.acc(do_acc),.in(sum_of_prod[35:38-ACCWIDTH]),.out(acc_out)); + acc #(.IWIDTH(36),.OWIDTH(ACCWIDTH)) + acc (.clk(clk),.clear(clear),.acc(do_acc),.in(sum_of_prod),.out(acc_out)); - wire [ACCWIDTH-1:0] data_even_signext; + wire [WIDTH:0] acc_out_rnd; + round #( + .bits_in(ACCWIDTH), + .bits_out(WIDTH+1) + ) round_acc ( + .in(acc_out), + .out(acc_out_rnd) + ); - localparam SHIFT_FACTOR = 6; + wire [WIDTH:0] data_even_signext; - sign_extend #(.bits_in(INTWIDTH),.bits_out(ACCWIDTH-SHIFT_FACTOR)) signext_data_even - (.in(data_even),.out(data_even_signext[ACCWIDTH-1:SHIFT_FACTOR])); - assign data_even_signext[SHIFT_FACTOR-1:0] = 0; + localparam SHIFT_FACTOR = 17 - (ACCWIDTH - (WIDTH+1)); - always @(posedge clk) final_sum <= acc_out + data_even_signext; + sign_extend #(.bits_in(INTWIDTH),.bits_out(WIDTH+1-SHIFT_FACTOR)) signext_data_even + (.in(data_even),.out(data_even_signext[WIDTH:SHIFT_FACTOR])); + assign data_even_signext[SHIFT_FACTOR-1:0] = 0; - clip #(.bits_in(WIDTH+1), .bits_out(WIDTH)) clip (.in(final_sum), .out(final_sum_clip)); + always @(posedge clk) final_sum <= acc_out_rnd + data_even_signext; + + clip #(.bits_in(WIDTH+1),.bits_out(WIDTH)) clip_finalsum + (.in(final_sum), .out(final_sum_clip)); // Output MUX to allow for bypass wire selected_stb = bypass ? stb_in : stb_out_pre[8]; diff --git a/lib/dsp/small_hb_dec.v b/lib/dsp/small_hb_dec.v index 25a635b..0e94696 100644 --- a/lib/dsp/small_hb_dec.v +++ b/lib/dsp/small_hb_dec.v @@ -110,13 +110,25 @@ module small_hb_dec localparam ACCWIDTH = 30; reg [ACCWIDTH-1:0] accum; + wire [ACCWIDTH-1:0] prod_acc_rnd; + round #( + .bits_in (36), + .bits_out (ACCWIDTH), + .round_to_zero (1), + .round_to_nearest(0), + .trunc (0) + ) round_prod ( + .in(prod), + .out(prod_acc_rnd) + ); + always @(posedge clk) if(rst) accum <= 0; else if(go_d2) - accum <= {middle_d1[17],middle_d1[17],middle_d1,{(16+ACCWIDTH-36){1'b0}}} + {prod[35:36-ACCWIDTH]}; + accum <= {middle_d1[17],middle_d1[17],middle_d1,{(16+ACCWIDTH-36){1'b0}}} + prod_acc_rnd; else if(go_d3) - accum <= accum + {prod[35:36-ACCWIDTH]}; + accum <= accum + prod_acc_rnd; wire [WIDTH:0] accum_rnd; wire [WIDTH-1:0] accum_rnd_clip;