feat(gtx7): resolve odd comma landings in fabric behind RX_ODD_ALIGN_MODE_G - #1475
Open
ruck314 wants to merge 3 commits into
Open
feat(gtx7): resolve odd comma landings in fabric behind RX_ODD_ALIGN_MODE_G#1475ruck314 wants to merge 3 commits into
ruck314 wants to merge 3 commits into
Conversation
Two independent Gtx7Core additions, both backward compatible. cPllRefClkLostOut exposes CPLLREFCLKLOST from the GTXE2_CHANNEL, which the core already consumed internally but never brought out to a caller. WAIT_TIME_CDRLOCK_G decouples the CDR-lock wait from the primitive's simulation speedup. SIM_GTRESET_SPEEDUP_G previously fed both the WAIT_TIME_CDRLOCK_C fabric constant and the GTXE2_CHANNEL SIM_RESET_SPEEDUP attribute, so a simulation could not select the CDR-lock wait independently of the primitive's own reset speedup. The new integer generic defaults to -1, which reproduces the previous derivation exactly and changes no elaborated behavior for any instance that leaves it at the default. Any value of 0 or greater is used directly as the stable-clock cycle count for WAIT_TIME_CDRLOCK_C. SIM_RESET_SPEEDUP continues to consume SIM_GTRESET_SPEEDUP_G unmodified.
…MODE_G
Gtx7RxFixedLatPhaseAligner unilaterally forced Gtx7RxRst back to INIT on
every odd comma landing, unbounded and unobservable. Add
RX_ODD_ALIGN_MODE_G ("RESET" or "BITSLIP") to gate a fabric escape from
that relock loop.
Under BITSLIP, an odd landed offset i issues i-2 rxSlide pulses, which
SLIDE_WAIT_S turns into i-1 actual pulses (always even, so the recovered
clock phase is never disturbed), reducing the residue to 1. The aligned
word is then presented as a 1-bit-shifted slice of the aligner's own
r.last history through two new output ports, rxDataAligned and
rxDataAlignedSel, muxed onto Gtx7Core's rxDataOut only when
RX_ODD_ALIGN_MODE_G = "BITSLIP".
Both terminal states are sourced one register stage deep so that the
resulting latency does not depend on where the comma landed: ALIGNED_S
from the previous GT word, ALIGNED_SLIP_S from that word shifted up one
bit with its missing MSB taken live. One stage is the floor, since at
offset 1 the aligned word's last bit only arrives with the next GT word.
The select therefore asserts in both terminal states, and BITSLIP always
costs exactly one rxUsrClk of fiber-to-rxDataOut latency relative to
RESET, whose RX_DATA_OUT_RESET_GEN drives rxDataOut combinationally off
RXDATA and adds no fabric stage. That is 5.385 ns on an LCLS-II link at
3.714 Gbps and 8.403 ns on an LCLS-I link at 2.380 Gbps. It is constant
across bring-ups, so it costs a caller one re-calibration rather than
run-to-run jitter, but it is a real change to the absolute number.
rxDataAligned and rxDataAlignedSel are driven exclusively inside
RX_FIX_LAT_ALIGN_GEN, so BITSLIP asserts that generate's full condition,
RX_BUF_EN_G = false and RX_ALIGN_MODE_G = "FIXED_LAT". RX_BUF_EN_G
defaults to true, which makes the unguarded combination the likeliest
caller mistake: it would elaborate RX_NO_ALIGN_GEN instead of the
aligner, tie rxPhaseAlignmentDone high, leave rxDataAlignedSel at its
declared '0', and fall through to the raw rxDataInt path, giving no
comma alignment at all while reporting alignment done. No caller in surf
selects BITSLIP, and the one external caller that does (lcls-timing-core
TimingGtCoreWrapper) already sets RX_BUF_EN_G => false, so nothing that
elaborates today starts failing.
BITSLIP never asserts the aligner rxReset, which removes one of the two
paths that previously restarted alignment. The remaining path is
Gtx7RxRst MONITOR_DATA_VALID/FSM_DONE, fed by Gtx7Core rxDataValidIn,
which defaults to '1'. Left at that default the supervision loop is
permanently satisfied and a lost alignment is never re-attempted;
DO_PHASE_ALIGNMENT time_out_wait_bypass only covers an alignment that
never completes, not one that completes and is later lost. Several
in-tree callers hardwire rxDataValidIn => '1' while already being
RX_BUF_EN_G => false with FIXED_LAT, so this is a live constraint. Both
properties are documented at the generic and at the mux.
A dont_touch'd signal inside a BITSLIP_MODE_C-gated generate exposes the
first resolved odd offset and an odd-landing count for hardware
observability. Only the first odd offset is latched, since every odd
landing above 1 re-scans at 1 and would otherwise overwrite the offset
the CDR actually landed on. Scoping the state to the generate keeps it
out of the netlist entirely under RESET; the earlier null-range record
form did not, and Vivado tied four degenerate bits low on lanes that use
neither field.
RX_ODD_ALIGN_MODE_G defaults to "RESET", reproducing today's behavior
exactly. Every change is behind BITSLIP_MODE_C or inside the gated
generate, and RX_DATA_OUT_RESET_GEN is untouched. Verified by
co-simulating the pre-change and post-change aligners from identical
stimulus: cycle-identical on rxReset, rxSlide and rxPhaseAlignmentDone
over 12000 cycles across 5 aligner-driven RX resets.
Not addressed here: even landings settle at GT offset 0 and odd landings
at offset 1. If an odd final offset costs a sub-UI recovered-clock phase
step, a <=1 UI difference between the two classes remains that no fabric
logic can remove, since crossing parity classes requires a fresh CDR
lock. That is a property of the silicon and is being measured on
hardware, binned on landing parity.
Add the aligner to the non-Vivado GHDL source list and a pytest/cocotb suite sweeping all 20 comma landings in both RX_ODD_ALIGN_MODE_G modes. Per-landing invariants. Every landing settles with an even slide count, so the recovered clock phase is never disturbed; BITSLIP never asserts rxReset; and every landing presents the same frame on the same cycle as every other landing in its mode. Absolute latency, not just landing-invariance. EXPECTED_TRAIL carries one entry per mode, in whole rxUsrClk of fiber-to-rxDataOut latency, so what switching a link from RESET to BITSLIP costs is stated rather than recoverable only by reading Gtx7Core's two output-mux legs against the aligner's pipeline depth. RESET adds no fabric stage; BITSLIP always adds the aligner's one stage. The delta is one rxUsrClk. Mutation-tested per property. Restoring the odd-branch off-by-one fails the 9 odd BITSLIP landings; restoring a combinational even path fails the 10 even ones. Moving the RESET trail entry alone fails exactly the ten even RESET landings and nothing else; moving the BITSLIP entry alone fails all twenty BITSLIP landings and nothing else. Deepening both aligner terminal states by one stage fails all twenty BITSLIP landings, confirming the trail entry tracks the RTL rather than a constant. Scope is recorded in the file. The aligner's end of the contract is elaborated, but Gtx7Core's output mux is reproduced by Harness.data_out() rather than built, because Gtx7Core needs GTXE2_CHANNEL and does not compile under GHDL. An edit to the mux expressions themselves is out of reach of this suite.
ruck314
force-pushed
the
gtx7-rx-odd-align-mode
branch
from
August 23, 2026 23:32
9101d63 to
134e40c
Compare
ruck314
marked this pull request as ready for review
August 23, 2026 23:46
This was referenced Aug 23, 2026
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
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.
Description
Gtx7RxFixedLatPhaseAligneranswered an odd comma landing by forcingGtx7RxRstback toINIT, unbounded and unobservable, so a link whose CDR kept landing odd never finished its RX reset.New
RX_ODD_ALIGN_MODE_GonGtx7CoreandGtx7RxFixedLatPhaseAligner:"RESET"(default) - today's behavior, bit for bit. Callers that leave the generic unset are unaffected."BITSLIP"- resolves an odd landing in fabric: slide to a residue of 1 using only evenrxSlidecounts, then present a constant one-bit slice of the aligner's word history. The RX is never reset, so bring-up is bounded."RESET""BITSLIP"rxUsrClk: 5.385 ns @ 3.714 Gbps, 8.403 ns @ 2.380 GbpsrxReset, re-locks the CDR"BITSLIP"is fixed-latency for every landing, odd and even. Three quantities are in play and they are easy to conflate:rxDataOutrxUsrClk"RESET"rxUsrClk+1 rxUsrClk.The first row is what a user application sees on
rxData, and it holds becauserxDataAlignedSelasserts in both of the aligner's terminal states, not only the slipped one:An even landing therefore takes the registered path too. Letting it fall through to
rxDataIntis the natural-looking design and is the trap: even landings would be combinational offRXDATAwhile odd landings sat one stage behind, so the two parities would differ by a full parallel-clock period, 5.385 ns @ 3.714 Gbps. Both terminal states instead source the word onerxUsrClkafter the GT presented it,ALIGNED_Sfrom the previous GT word unshifted andALIGNED_SLIP_Sfrom that word shifted up one bit with its missing MSB taken live. One stage is the floor rather than a convenience: at a final offset of 1 the aligned word's last bit only arrives with the next GT word, so it cannot be presented combinationally.Getting there also depends on
slideCount := i-2in the odd branch, which issuesi-1pulses, an even count, so every odd landing settles at a residue of exactly 1 and the fabric slice is a constant bit range rather than an offset-dependent one.That constant is
EXPECTED_TRAILin the regression, asserted per mode rather than argued, and each of the twenty landings is checked to present the same frame on the same cycle as every other one in its mode. Choosing"BITSLIP"costs one re-calibration of the absolute number, not run-to-run jitter.Opting into
"BITSLIP"Needs
RX_ALIGN_MODE_G = "FIXED_LAT"andRX_BUF_EN_G = false, both enforced by an elaboration assert, plusrxDataValidIndriven from a decoder rather than left at its'1'default."BITSLIP"never asserts the aligner'srxReset, so recovery from a lost alignment rests entirely onGtx7RxRst's DATA_VALID supervision. Several in-tree callers hardwirerxDataValidIn => '1'and would need that port wired first.The one open question: sub-UI, silicon and not RTL
Even landings settle at GT offset 0, odd landings at offset 1. Whether a final offset of 1 costs a sub-UI recovered-clock phase step relative to offset 0 is a property of the GTX PMA, not of this RTL, and no fabric simulation can settle it. It is bounded by 1 UI, one twentieth of the word clock: 269.2 ps @ 3.714 Gbps, 420.2 ps @ 2.380 Gbps. This is separate from the fabric latency contract above, which is constant across landings either way.
Removing the term at all would mean refusing odd landings, which is exactly what
"RESET"does and what"BITSLIP"declines to do. Settling it is a hardware measurement (power-cycle N times, bin a latency measurement on landing parity) that has not been made, so a link with a sub-UI phase budget should stay on"RESET"until it is.For that measurement,
landedValidis the bin discriminator, notlandedOffset: the observer latches only the first odd offset seen since reset, solandedValid = '1'marks the odd class with its actual offset inlandedOffset, andlandedValid = '0'marks the even class. There is no positive even-landing marker.Also on this branch, independent of the above
cPllRefClkLostOutadded toGtx7Core, exposingCPLLREFCLKLOST. This is an entity interface change: a newoutport with no default, so a caller using positional association must be updated. Named association, the SLAC convention, needs no change.WAIT_TIME_CDRLOCK_Gadded, decoupling the CDR-lock wait count fromSIM_RESET_SPEEDUP. Its-1default selects the previous derivation, so it is behavior-preserving.Verification
tests/xilinx/gtx7/test_Gtx7RxFixedLatPhaseAligner.py): both modes across all twenty comma landings, driven from a bit-accurate serial stream. Each landing must align, present a correctly comma-aligned word, and present the same frame on the same cycle as every other landing in its mode; per-mode absolute latency is pinned againstEXPECTED_TRAILand mutation-tested. Scope limit: the aligner is elaborated, butGtx7Core's output mux is reproduced by the harness, sinceGtx7CoreneedsGTXE2_CHANNELand does not build under GHDL."RESET"equivalence: pre-change and post-change aligners co-simulated from identical stimulus are cycle-identical onrxReset,rxSlideandrxPhaseAlignmentDoneover 12000 cycles spanning five aligner-driven RX resets."BITSLIP"the affected lane reaches RX reset done from an odd landing and holds it for 2.07 ms with no further cycling. With both mode generics at their defaults, the same bench reproduces the pre-change failure identically.XC7K160TFBG484-2: timing closes with no new critical-warning message classes, and the untouched lane's elaboratedGTXE2_CHANNELparameters match the pre-change baseline apart from an enumerated set of corrected constants.landedOffsetandoddLandingCountare declared inside the"BITSLIP"generate, so the legacy path gains no logic, but they are reachable only by hierarchical netlist name underdont_touch. Promoting them to real output ports is left to a follow-up.