fix(fpga): frame-boundary atomic commit for USB timing regs

C-4: GUI Waveform Timing opcodes (0x10..0x14) write one parameter per
USB packet with arbitrary inter-opcode latency. Previously each opcode
landed directly on a live register consumed by radar_mode_controller,
so a mid-scan reconfiguration exposed the RX FSM to torn sets (e.g.
new long_chirp with old long_listen), and a value below the running
timer truncated the current chirp and corrupted one range-Doppler
frame.

Fix: split each of the five timing regs into pending (USB write target)
and live (FSM consumer). radar_mode_controller exports a new
cfg_commit_strobe that is HIGH while scanning is idle (S_IDLE) and
pulses for 1 cycle at every elevation/azimuth boundary in S_ADVANCE.
radar_system_top snapshots pending -> live on the strobe, guaranteeing
the FSM always sees a self-consistent set. No CDC added: strobe is in
clk_100m same as the USB-cmd path.

Regression: tb_radar_mode_controller adds 3 checks for strobe
semantics (idle-HIGH, scanning-LOW, one pulse per elevation boundary).
32/32 full regression PASS; Radar Mode Controller TB grows from 78 to
81 checks.
This commit is contained in:
Jason
2026-04-23 04:43:02 +05:45
parent 52a3497ea2
commit 5617d552df
4 changed files with 128 additions and 23 deletions

View File

@@ -97,7 +97,15 @@ module radar_mode_controller #(
// Status
output wire scanning, // 1 = scan in progress
output wire scan_complete // pulse when full scan done
output wire scan_complete, // pulse when full scan done
// Timing-config commit strobe (see "Timing-Config Commit" note below).
// Held HIGH while scanning is idle (S_IDLE) and pulsed for 1 clock at
// every elevation/azimuth boundary so radar_system_top can atomically
// copy its USB-written pending timing regs into the live regs the FSM
// reads. Guarantees no torn (cfg_long_chirp, cfg_long_listen, ...)
// reconfiguration mid-frame.
output wire cfg_commit_strobe
`ifdef FORMAL
,
@@ -128,6 +136,29 @@ assign fv_scan_state = scan_state;
assign fv_timer = timer;
`endif
// ============================================================================
// Timing-Config Commit Strobe
//
// radar_system_top uses this to snapshot pending USB-written timing regs
// (opcodes 0x10..0x14) into the live regs this FSM consumes. The strobe
// is ASSERTED whenever it is safe to commit new timing:
// (a) FSM is in S_IDLE (no chirp underway) -> held HIGH continuously
// so an opcode written before the first trigger takes effect on
// the first chirp.
// (b) FSM is in S_ADVANCE at the elevation/azimuth boundary (i.e.,
// chirp_count has reached the last chirp of the current elevation)
// -> pulsed for the single S_ADVANCE cycle. The next frame of
// CHIRPS_PER_ELEVATION chirps runs with the newly-committed set.
//
// Combinational (no added FF). scan_state and chirp_count are registers
// in the same clock domain, so this wire is glitch-free relative to the
// destination register's clock edge. Downstream is a synchronous-enable
// FF in radar_system_top, which captures cleanly on the rising edge.
// ============================================================================
assign cfg_commit_strobe =
(scan_state == S_IDLE) ||
((scan_state == S_ADVANCE) && (chirp_count >= cfg_chirps_per_elev - 1));
// Edge detection for STM32 pass-through
reg stm32_new_chirp_prev;
reg stm32_new_elevation_prev;

View File

@@ -105,7 +105,13 @@ module radar_receiver_final (
// silent sample drop between the 400 MHz CIC output and the 100 MHz
// FIR input; stays high until the next reset. OR'd into the GPIO
// diagnostic bit at the top level.
output wire ddc_cic_fir_overrun
output wire ddc_cic_fir_overrun,
// C-4: timing-config commit strobe from rmc. radar_system_top uses
// this to atomically snapshot pending USB-written timing regs
// (opcodes 0x10..0x14) into the live regs this RX chain consumes,
// eliminating the mid-frame torn-reconfig hazard.
output wire cfg_commit_strobe
);
// ========== INTERNAL SIGNALS ==========
@@ -216,7 +222,8 @@ radar_mode_controller rmc (
.elevation_count(rmc_elevation_count),
.azimuth_count(rmc_azimuth_count),
.scanning(rmc_scanning),
.scan_complete(rmc_scan_complete)
.scan_complete(rmc_scan_complete),
.cfg_commit_strobe(cfg_commit_strobe)
);
wire clk_400m;

View File

@@ -217,6 +217,10 @@ wire rx_range_decim_watchdog;
// CICFIR CDC overrun sticky (audit F-1.2). High = at least one baseband
// sample has been silently dropped between the 400 MHz CIC and 100 MHz FIR.
wire rx_ddc_cic_fir_overrun;
// C-4: timing-config commit strobe from rmc (clk_100m domain). HIGH in
// S_IDLE and pulsed at elevation/azimuth boundaries. Used to latch the
// pending USB-written timing regs into the live regs the RX FSM reads.
wire cfg_commit_strobe;
// Data packing for USB
wire [31:0] usb_range_profile;
@@ -254,12 +258,27 @@ reg [3:0] host_gain_shift;
// These override the compile-time defaults in radar_mode_controller when
// written via USB command. Defaults match the parameter values in
// radar_mode_controller.v so behavior is unchanged until the host writes them.
reg [15:0] host_long_chirp_cycles; // Opcode 0x10 (default 3000)
reg [15:0] host_long_listen_cycles; // Opcode 0x11 (default 13700)
reg [15:0] host_guard_cycles; // Opcode 0x12 (default 17540)
reg [15:0] host_short_chirp_cycles; // Opcode 0x13 (default 50)
reg [15:0] host_short_listen_cycles; // Opcode 0x14 (default 17450)
reg [5:0] host_chirps_per_elev; // Opcode 0x15 (default 32)
// C-4: Split each timing reg into pending (USB write target) and live
// (FSM consumer). The RX radar_mode_controller pulses cfg_commit_strobe
// at every elevation/azimuth boundary (and holds it HIGH in S_IDLE),
// and the live regs snapshot the pending set on that strobe. This
// guarantees that a multi-opcode GUI reconfiguration (0x10 0x11
// 0x12, separated by USB round-trips) can never be observed
// mid-reconfiguration by the FSM: between frames the pending set is
// either entirely old or entirely new. Without this, a mid-chirp update
// to cfg_long_chirp_cycles below the current timer would immediately
// truncate the chirp and corrupt that frame's range-Doppler map.
reg [15:0] host_long_chirp_cycles_pending; // Opcode 0x10 (default 3000)
reg [15:0] host_long_listen_cycles_pending; // Opcode 0x11 (default 13700)
reg [15:0] host_guard_cycles_pending; // Opcode 0x12 (default 17540)
reg [15:0] host_short_chirp_cycles_pending; // Opcode 0x13 (default 50)
reg [15:0] host_short_listen_cycles_pending; // Opcode 0x14 (default 17450)
reg [15:0] host_long_chirp_cycles; // Live: committed at frame boundary
reg [15:0] host_long_listen_cycles; // Live: committed at frame boundary
reg [15:0] host_guard_cycles; // Live: committed at frame boundary
reg [15:0] host_short_chirp_cycles; // Live: committed at frame boundary
reg [15:0] host_short_listen_cycles; // Live: committed at frame boundary
reg [5:0] host_chirps_per_elev; // Opcode 0x15 (default 32) has dedicated clamp logic
reg host_status_request; // Opcode 0xFF (self-clearing pulse)
// Fix 4: Doppler/chirps mismatch protection
@@ -596,7 +615,9 @@ radar_receiver_final rx_inst (
.mti_saturation_count_out(rx_mti_saturation_count),
// Range-bin decimator watchdog (audit F-6.4)
.range_decim_watchdog(rx_range_decim_watchdog),
.ddc_cic_fir_overrun(rx_ddc_cic_fir_overrun)
.ddc_cic_fir_overrun(rx_ddc_cic_fir_overrun),
// C-4: timing-reg commit strobe from rmc (live regs update only when high)
.cfg_commit_strobe(cfg_commit_strobe)
);
// ============================================================================
@@ -968,12 +989,18 @@ always @(posedge clk_100m_buf or negedge sys_reset_n) begin
host_detect_threshold <= 16'd10000; // Default threshold
host_stream_control <= `RP_STREAM_CTRL_DEFAULT; // Default: all streams, mag-only mode
host_gain_shift <= 4'd0; // Default: pass-through (no gain change)
// Gap 2: chirp timing defaults (match radar_mode_controller parameters)
host_long_chirp_cycles <= 16'd3000;
host_long_listen_cycles <= 16'd13700;
host_guard_cycles <= 16'd17540;
host_short_chirp_cycles <= 16'd50;
host_short_listen_cycles <= 16'd17450;
// Gap 2: chirp timing defaults initialize pending AND live to the
// same default so pre-first-trigger behavior matches the old code.
host_long_chirp_cycles_pending <= 16'd3000;
host_long_listen_cycles_pending <= 16'd13700;
host_guard_cycles_pending <= 16'd17540;
host_short_chirp_cycles_pending <= 16'd50;
host_short_listen_cycles_pending <= 16'd17450;
host_long_chirp_cycles <= 16'd3000;
host_long_listen_cycles <= 16'd13700;
host_guard_cycles <= 16'd17540;
host_short_chirp_cycles <= 16'd50;
host_short_listen_cycles <= 16'd17450;
host_chirps_per_elev <= 6'd32;
host_status_request <= 1'b0;
chirps_mismatch_error <= 1'b0;
@@ -1005,12 +1032,15 @@ always @(posedge clk_100m_buf or negedge sys_reset_n) begin
8'h02: host_trigger_pulse <= 1'b1;
8'h03: host_detect_threshold <= usb_cmd_value;
8'h04: host_stream_control <= usb_cmd_value[5:0];
// Gap 2: chirp timing configuration
8'h10: host_long_chirp_cycles <= usb_cmd_value;
8'h11: host_long_listen_cycles <= usb_cmd_value;
8'h12: host_guard_cycles <= usb_cmd_value;
8'h13: host_short_chirp_cycles <= usb_cmd_value;
8'h14: host_short_listen_cycles <= usb_cmd_value;
// Gap 2: chirp timing configuration writes land in the
// PENDING set. The live set atomically snapshots pending
// on cfg_commit_strobe (S_IDLE or elevation boundary).
// See C-4 note at the timing-reg declaration.
8'h10: host_long_chirp_cycles_pending <= usb_cmd_value;
8'h11: host_long_listen_cycles_pending <= usb_cmd_value;
8'h12: host_guard_cycles_pending <= usb_cmd_value;
8'h13: host_short_chirp_cycles_pending <= usb_cmd_value;
8'h14: host_short_listen_cycles_pending <= usb_cmd_value;
8'h15: begin
// Fix 4: Clamp chirps_per_elev to the fixed Doppler frame size.
// If host requests a different value, clamp and set error flag.
@@ -1056,6 +1086,20 @@ always @(posedge clk_100m_buf or negedge sys_reset_n) begin
default: ;
endcase
end
// C-4: atomic commit of pending live timing regs. The strobe is
// HIGH while the RX FSM is in S_IDLE and pulses for 1 clk_100m
// cycle at every elevation/azimuth boundary. Live regs are only
// updated on the strobe, so the FSM always sees a self-consistent
// set even when the GUI sends opcodes 0x10..0x14 as separate USB
// packets with arbitrary inter-opcode latency.
if (cfg_commit_strobe) begin
host_long_chirp_cycles <= host_long_chirp_cycles_pending;
host_long_listen_cycles <= host_long_listen_cycles_pending;
host_guard_cycles <= host_guard_cycles_pending;
host_short_chirp_cycles <= host_short_chirp_cycles_pending;
host_short_listen_cycles <= host_short_listen_cycles_pending;
end
end
end

View File

@@ -44,6 +44,7 @@ module tb_radar_mode_controller;
wire [5:0] azimuth_count;
wire scanning;
wire scan_complete;
wire cfg_commit_strobe;
// Test bookkeeping
integer pass_count;
@@ -60,6 +61,7 @@ module tb_radar_mode_controller;
integer elevation_toggles;
integer azimuth_toggles;
integer scan_completes;
integer commit_pulses;
// Saved values for toggle checks
reg saved_mc_new_chirp;
@@ -104,7 +106,8 @@ module tb_radar_mode_controller;
.elevation_count (elevation_count),
.azimuth_count (azimuth_count),
.scanning (scanning),
.scan_complete (scan_complete)
.scan_complete (scan_complete),
.cfg_commit_strobe (cfg_commit_strobe)
);
// Check task
@@ -173,6 +176,12 @@ module tb_radar_mode_controller;
check(azimuth_count === 6'd0, "azimuth_count=0 after reset");
check(scanning === 1'b0, "scanning=0 after reset");
check(scan_complete === 1'b0, "scan_complete=0 after reset");
// C-4: cfg_commit_strobe must be HIGH while idle so a timing-reg
// opcode written before the first trigger is latched into the
// live set immediately (no stale default on first chirp).
check(cfg_commit_strobe === 1'b1,
"cfg_commit_strobe=1 while idle (pre-scan commit window)");
reset_n = 1;
@(posedge clk); #1;
@@ -249,11 +258,18 @@ module tb_radar_mode_controller;
elevation_toggles = 0;
azimuth_toggles = 0;
scan_completes = 0;
commit_pulses = 0;
// Check: scanning starts immediately
@(posedge clk); #1;
check(scanning === 1'b1, "Scanning starts immediately in auto mode");
// C-4: once scanning is active, cfg_commit_strobe must drop low
// (no longer in S_IDLE). Sample immediately after the IDLEchirp
// transition. It will pulse again only at elevation boundaries.
check(cfg_commit_strobe === 1'b0,
"cfg_commit_strobe=0 once scan has left IDLE");
// Run for enough cycles to complete one full scan
for (i = 0; i < 15000; i = i + 1) begin
@(posedge clk); #1;
@@ -266,6 +282,8 @@ module tb_radar_mode_controller;
azimuth_toggles = azimuth_toggles + 1;
if (scan_complete)
scan_completes = scan_completes + 1;
if (cfg_commit_strobe)
commit_pulses = commit_pulses + 1;
mc_new_chirp_prev = mc_new_chirp;
mc_new_elevation_prev = mc_new_elevation;
@@ -288,6 +306,11 @@ module tb_radar_mode_controller;
check(chirp_toggles >= SIM_CHIRPS * SIM_ELEVATIONS * SIM_AZIMUTHS,
"At least 24 chirp toggles in full scan");
// C-4: one commit pulse per elevation boundary (commit window).
// With SIM_ELEVATIONS*SIM_AZIMUTHS elevations visited per scan,
// the counter should see at least that many pulses.
check(commit_pulses >= SIM_ELEVATIONS * SIM_AZIMUTHS,
"cfg_commit_strobe pulses at every elevation boundary");
check(scan_completes >= 1,
"At least 1 scan completion detected");
check(elevation_toggles >= SIM_AZIMUTHS,