Ndh/fmc vibe check - #2686
Draft
nathanaelhuffman wants to merge 4 commits into
Draft
Ndh/fmc vibe check#2686nathanaelhuffman wants to merge 4 commits into
nathanaelhuffman wants to merge 4 commits into
Conversation
When the FPGA aperture moved from 0x6000_0000 to 0xC000_0000, peek32 and poke32 were taught to translate legacy addresses but the 16- and 64-bit variants were not, so they accessed unmapped address space instead of the FPGA.
Batched peek/poke ops carry ~9 bytes of network payload per 32-bit word, which swamps the sub-microsecond FMC access and makes bus line rate invisible to throughput measurement. Ops 17/18/19 run the access loop server-side with a constant-size payload: checksum block reads (advancing and fixed-address) and a fixed-address fill. quartz's tools/fmc_sweep/fmc_sweep.py delta-times a large-count op against a small-count one so the round trip cancels, leaving pure bus time.
cosmo: CLKDIV 3 -> 1 (200 MHz kernel clock / 2 = 100 MHz FMC_CLK), paired with the cosmo_seq bitstream carrying the fmc_clk MMCM and 10 ns constraints; the MMCM's VCO configuration locks at either 50 or 100 MHz input, so bitstream and divider can move independently. grapefruit: CLKDIV 3 -> 2 (/3 = 66.67 MHz), the rate its FPGA constraints have always been written for. The FPGA side is NWAIT-paced, so these are timing-closure changes, not functional ones; land each board's flip only with a bitstream validated at the new rate on the bench.
Three independent problems left the timing setters nonfunctional: - The task's memory map had the FPGA apertures but not the FMC controller's own registers, so every setter call memory-faulted the task; hiffy reports the resulting Err(<server died>) with exit 0, which made the failures invisible. Add the fmc region to both app configs. - The setters used btr1.write() instead of modify(), resetting the sibling BTR1 fields (CLKDIV in particular, to /16) as a side effect, and set_bus_turnaround_cycles clamped with max(15) where it meant min(15). - The continuous-clock divider does not take a live CLKDIV change; set_clock_divider now drops FMCEN and CCLKEN across the write so the clock generator re-samples it. Also adds get_bcr1/get_btr1 readback ops so a caller can verify a timing change actually landed; quartz's fmc_sweep.py checks the CLKDIV field after every change.
jamesmunns
requested changes
Sep 14, 2026
jamesmunns
left a comment
jamesmunns
left a comment
Contributor
There was a problem hiding this comment.
Added some notes to hopefully get this merged before oxidecomputer/quartz#527 lands.
| // Note from the clock config earlier in this function that AHB3 is running | ||
| // at 200 MHz. | ||
| const CLKDIV: u8 = 3; // /4, for 50 MHz -- field is divisor minus 1 | ||
| const CLKDIV: u8 = 1; // /2, for 100 MHz -- field is divisor minus 1 |
Contributor
There was a problem hiding this comment.
Can we revert this to land the change sooner?
| // Note from the clock config earlier in this function that AHB3 is running | ||
| // at 200 MHz. | ||
| const CLKDIV: u8 = 3; // /4, for 50 MHz -- field is divisor minus 1 | ||
| const CLKDIV: u8 = 2; // /3, for 66.67 MHz -- field is divisor minus 1 |
| // advances through memory, op 18 re-reads one address. | ||
| let count = u16::from_le_bytes(read_chunk(&mut packet)?); | ||
| let mut sum: u32 = 0; | ||
| if byte == 17 { |
Contributor
There was a problem hiding this comment.
If you wanted to get clever to dedupe here a bit:
let offset = if byte == 17 { 4 } else { 0 };
let count = u16::from_le_bytes(read_chunk(&mut packet)?);
let mut sum: u32 = 0;
for _ in 0..count {
let b = unsafe {
core::ptr::read_volatile(address as *const u32)
};
sum = sum.wrapping_add(b);
address += offset;
}
write_chunk(sum.to_le_bytes(), &mut response)?;
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.