From 51a94f0baf4ee3c2ef73d67093b4988e32838753 Mon Sep 17 00:00:00 2001 From: Jason <83615043+JJassonn69@users.noreply.github.com> Date: Fri, 1 May 2026 04:06:58 +0545 Subject: [PATCH] chirp-v2 PR-F follow-up: doppler OOB read + dead cfar wires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues caught re-reviewing 7862f4d: 1. doppler_processor.v: at sub_frame = NUM_SUBFRAMES-1 (=2 in production), the read-ahead pointer was advanced one cycle past the last useful chirp, producing an out-of-range mem_read_addr (chirps 48/49 in a 48-chirp frame) on the BRAM read port. The result was never consumed — counter > CPS-1 blocks the multiply — so the OOB read had no functional effect, but it still drives mem_mem[OOB_idx] every frame and would trigger Vivado synth range warnings. Gate the read_doppler_index advance on fft_sample_counter <= CHIRPS_PER_SUBFRAME - 3 so the last NBA at counter = CPS-3 schedules the data needed at counter = CPS-1 and no more. For sub_frame < NUM_SUBFRAMES-1 this just replaces previously-wasted forward reads with redundant reads of the same address; outputs are bit-exact. 2. radar_system_top.v: cfar_detect_class, cfar_detect_threshold_soft, and cfar_detect_count_cand were declared and connected to cfar_inst but went nowhere downstream. They will be wired to USB / telemetry in PR-G; until then they show up as dangling wires that Vivado optimises away with noisy warnings. Drop the wire decls and leave the cfar_ca output ports unconnected. The soft-tier comparison is still synthesized because the 1-bit detect_flag (which IS wired) depends on noise_product_soft via the `else if (cur > thr_soft)` branch, so the candidate logic is preserved in the netlist — only the class / soft-thr / cand-count rails are gone. Tests (parity with the PR-F numbers in 7862f4d): - tb_chirp_controller: 43/43 PASS - tb_chirp_contract: 10/10 PASS - tb_cfar_ca: 24/0 PASS - tb_mti_canceller: 43/43 PASS - tb_doppler_realdata: 2056/2056 PASS - tb_doppler_frame_start_gate: 21/21 PASS - tb_system_e2e: 33/49 PASS (PR-F baseline parity) --- 9_Firmware/9_2_FPGA/doppler_processor.v | 21 ++++++++++++++++----- 9_Firmware/9_2_FPGA/radar_system_top.v | 16 ++++++++++------ 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/9_Firmware/9_2_FPGA/doppler_processor.v b/9_Firmware/9_2_FPGA/doppler_processor.v index 4aa6bff..73f7d81 100644 --- a/9_Firmware/9_2_FPGA/doppler_processor.v +++ b/9_Firmware/9_2_FPGA/doppler_processor.v @@ -488,11 +488,22 @@ always @(posedge clk) begin window_val_reg <= $signed(window_coeff[win_idx]); end // Advance BRAM read: chirp_base + (counter + 2). - // For NUM_SUBFRAMES * CHIRPS_PER_SUBFRAME = CHIRPS_PER_FRAME - // and counter <= CHIRPS_PER_SUBFRAME-1, chirp_base+offset - // is bounded by CHIRPS_PER_FRAME so no clamp is needed. - read_doppler_index <= current_sub_frame * CHIRPS_PER_SUBFRAME - + {2'd0, fft_sample_counter[3:0]} + 6'd2; + // The last useful read is data[chirp_base + CPS-1], needed + // by mult_i_raw at counter=CPS-1. Working back through the + // 2-cycle BRAM-then-multiply pipeline, the last NBA that + // matters is at counter = CPS-3 (= 13 for CPS=16) which + // schedules read of base+CPS-1. After that, advancing + // would address chirp base+CPS or base+CPS+1 — past the + // end of the highest sub-frame's data window (e.g. chirps + // 48 / 49 with sub_frame=2 in a 48-chirp frame), which is + // outside MEM_DEPTH = RANGE_BINS * CHIRPS_PER_FRAME. The + // would-be values are never consumed, but the reads + // would still drive an out-of-range mem_read_addr. Stop + // the read pointer at the last useful chirp instead. + if (fft_sample_counter <= CHIRPS_PER_SUBFRAME - 3) begin + read_doppler_index <= current_sub_frame * CHIRPS_PER_SUBFRAME + + {2'd0, fft_sample_counter[3:0]} + 6'd2; + end end if (fft_sample_counter == CHIRPS_PER_SUBFRAME + 1) begin diff --git a/9_Firmware/9_2_FPGA/radar_system_top.v b/9_Firmware/9_2_FPGA/radar_system_top.v index f4302ea..a9cbffc 100644 --- a/9_Firmware/9_2_FPGA/radar_system_top.v +++ b/9_Firmware/9_2_FPGA/radar_system_top.v @@ -681,17 +681,21 @@ wire [`RP_RANGE_BIN_WIDTH_MAX-1:0] notched_range_bin = rx_range_bin; // See cfar_ca.v for architecture details. wire cfar_detect_flag; -wire [`RP_DETECT_CLASS_WIDTH-1:0] cfar_detect_class; wire cfar_detect_valid; wire [`RP_RANGE_BIN_WIDTH_MAX-1:0] cfar_detect_range; wire [`RP_DOPPLER_BIN_WIDTH-1:0] cfar_detect_doppler; wire [16:0] cfar_detect_magnitude; wire [16:0] cfar_detect_threshold; -wire [16:0] cfar_detect_threshold_soft; wire [15:0] cfar_detect_count; -wire [15:0] cfar_detect_count_cand; wire cfar_busy_w; wire [7:0] cfar_status_w; +// PR-F note: cfar_ca also drives detect_class[1:0], detect_threshold_soft, +// detect_count_cand. The soft-tier comparison still feeds detect_flag (which +// is wired to USB), so the candidate logic is preserved in synth — but the +// class / soft-thr / cand-count outputs themselves don't go anywhere until +// PR-G adds the USB opcodes (0x28 alpha_soft + bulk-frame v2). We deliberately +// leave the cfar_ca output ports unconnected here so they do NOT show up as +// dangling wires in radar_system_top; PR-G will reattach them. cfar_ca cfar_inst ( .clk(clk_100m_buf), @@ -715,17 +719,17 @@ cfar_ca cfar_inst ( // Detection outputs .detect_flag(cfar_detect_flag), - .detect_class(cfar_detect_class), + .detect_class(), // PR-F: routed in PR-G (USB 0x2A read) .detect_valid(cfar_detect_valid), .detect_range(cfar_detect_range), .detect_doppler(cfar_detect_doppler), .detect_magnitude(cfar_detect_magnitude), .detect_threshold(cfar_detect_threshold), - .detect_threshold_soft(cfar_detect_threshold_soft), + .detect_threshold_soft(), // PR-F: routed in PR-G // Status .detect_count(cfar_detect_count), - .detect_count_cand(cfar_detect_count_cand), + .detect_count_cand(), // PR-F: routed in PR-G (telemetry) .cfar_busy(cfar_busy_w), .cfar_status(cfar_status_w) );