Skip to content

xilinx: IFFDELMUXE3 changes IDDR capture on silicon (direction NOT established -- do not merge) - #119

Closed
gHashTag wants to merge 1 commit into
openXC7:stable-backportsfrom
gHashTag:fix/iddr-iffdelmuxe3
Closed

xilinx: IFFDELMUXE3 changes IDDR capture on silicon (direction NOT established -- do not merge)#119
gHashTag wants to merge 1 commit into
openXC7:stable-backportsfrom
gHashTag:fix/iddr-iffdelmuxe3

Conversation

@gHashTag

Copy link
Copy Markdown

Fixes #114.

Measurement

ALINX AX7203 (xc7a200tfbg484-2). The IDDR's D is held at a constant 1 by an internal pull-up and both outputs are sticky-ORed to LEDs — so no external signal, cable or link partner is needed to see the failure. Reflashing between variants, A/B/A:

bitstream Q1 Q2
without IFFDELMUXE3.P0 0 0
with IFFDELMUXE3.P0 1 1
without, again 0 0

Reproducible in both directions. The ILOGICE3_IFF path never selected the IFF data path, so the flops captured nothing.

Relation to the reported symptom

#114 reports Q1 dead and Q2 alive on live RGMII traffic. On a static input both are dead. Same omission — with a running clock and real data, something recovers half of it; with nothing to recover, both stay at their initial value.

Verification chain

The FASM emitted by this patch is byte-identical to the hand-edited FASM used for the silicon measurement, so what was measured is what this emits. Adding the line by hand changed 7 bytes of the bitstream.

Note for review — a polarity question I could not settle

The ISERDESE2 path writes IFFDELMUXE3.P0 on the opposite condition, (iobdelay == "IFD"). This patch writes it when no IDELAYE2 drives D, matching the configuration measured.

One of the two is likely inverted. prjxray-db documents only .P0 for this feature and no .P1, so the polarity cannot be read off the database — and I have not measured the ISERDESE2 path. Worth a second measurement before trusting that branch.

A retraction

Earlier on #114 I argued from that same database asymmetry that IFFDELMUXE3 was probably not the cause: only .P0 exists, so "unset" is a meaningful state, and for a plain IDDR with no IDELAY unset looked like what you want.

The reasoning about the encoding was sound; the conclusion drawn from it was not. That "unset is meaningful" says nothing about whether set is a no-op, and I presented it as though it did. The measurement took one pull-up and three flashes and settles what the argument could not.

IFF.ZINV_OCLK remains deliberately unwritten — tested separately, captures bit-identical with and without.

Environment: nextpnr-xilinx 0.9.2-44-ga70ae4a8, prjxray and prjxray-db artix7 built from source, chipdb generated for xc7a200tfbg484-2, fully open flow, macOS arm64.

🤖 Generated with Claude Code

@gHashTag

Copy link
Copy Markdown
Author

Reviewing my own patch against the ISERDESE2 branch, I found something that a reviewer should see before this merges. The measurement stands; the semantics I implied may not.

The two bits are mutually exclusive on the ISERDESE2 path

std::string iobdelay = str_or_default(ci->params, ctx->id("IOBDELAY"), "NONE");
write_bit("IFFDELMUXE3.P0", (iobdelay == "IFD"));
write_bit("ZINV_D", !bool_or_default(..., "IS_D_INVERTED", false) && (iobdelay != "IFD"));

IFFDELMUXE3.P0 set ⟺ ZINV_D not set. That reads as a coherent story: ZINV_D configures the inversion of the direct D input, so it is meaningless when the flop takes its data from the IDELAY instead. Which would make IFFDELMUXE3.P0 mean "IFF data comes from the IDELAY path".

Under that reading my patch sets a contradictory pair

The ILOGICE3_IFF branch writes ZINV_D unconditionally, so with my change an IDDR now emits both IFFDELMUXE3.P0 and ZINV_D — the combination the ISERDESE2 branch treats as impossible. And it does so on a design with no IDELAYE2 anywhere.

So one of these is true, and I cannot tell which from the database:

  1. P0 means "bypass the IDELAY", the ISERDESE2 condition is inverted, and my patch is right for the wrong stated reason — in which case that branch has the same class of bug.
  2. P0 means "use the IDELAY", and my IDDR is capturing through an unconfigured IDELAY that happens to pass data through. It works on the bench, but it would be working by accident, and ZINV_D should probably be suppressed alongside.

prjxray-db documents only .P0 and no .P1, so the polarity is not readable from the encoding. I have not measured the ISERDESE2 path.

What would settle it

Two more bench variants on the same rig — IFFDELMUXE3.P0 with ZINV_D suppressed, and an IDDR fed through an explicit IDELAYE2 — would distinguish (1) from (2). I can run them; each is a FASM edit and a 15-second flash, with the same pull-up-and-sticky-OR readout that needs no external signal.

I would rather flag this than have it found in review. The A/B/A measurement is solid — without the bit neither edge captures, with it both do, reproducibly in both directions. What is not solid is my account of why, and the fix may need to be P0 plus suppressing ZINV_D rather than P0 alone.

Happy to hold this PR until those two variants are measured, if you would prefer the explanation to arrive with the fix.

@janrinze

Copy link
Copy Markdown

@gHashTag did you accidentally delete two sub-modules?

…ing a fix

Supersedes the previous version of this branch, which (a) accidentally
converted xilinx/external/prjxray-db and xilinx/external/nextpnr-xilinx-meta
from submodules into symlinks pointing at a local home directory, breaking
all three CI legs, and (b) asserted more than the measurement supports.

The submodule damage is gone -- this touches xilinx/fasm.cc only.

On the claim: toggling IFFDELMUXE3.P0 does change what the two IDDR flops
output on silicon, reproduced three times in both directions. That part
stands. Which of the two states is the capturing one does not: the readout
was four LEDs reported by position and the position-to-led[] mapping was
never fixed, so the observation does not distinguish "now captures" from
"now reads the other path".

The database also cuts against the polarity used here. IDELMUXE3 is an
arity-2 pair and its P0 is written exactly when an IDELAYE2 drives D, so
P0 means "take the delayed path". IFFDELMUXE3 documents only .P0, and the
ISERDESE2 path writes it on (iobdelay == "IFD") -- the same sense. On that
reading an IDDR with no IDELAYE2 wants the bit clear, and this condition is
inverted.

Left in place rather than dropped because the effect is real and worth the
next person's time, but the branch is a draft and should not merge on this
evidence.
@gHashTag
gHashTag force-pushed the fix/iddr-iffdelmuxe3 branch from 8db3ba3 to 8b33d91 Compare August 10, 2026 16:33
@gHashTag
gHashTag marked this pull request as draft August 10, 2026 16:34
@gHashTag

Copy link
Copy Markdown
Author

Two things wrong with this PR, both mine. Force-pushed a fix for the first and downgraded the second; moving it to draft.

1. It was breaking your CI for a reason that had nothing to do with the patch

The branch had converted xilinx/external/prjxray-db and xilinx/external/nextpnr-xilinx-meta from submodules into symlinks pointing at a path in my home directory. That is why all three demo legs went red at ~32 minutes, and it would have poisoned the tree for anyone who checked the branch out. Entirely an artefact of how I had the db wired up locally.

The branch is now xilinx/fasm.cc only; both gitlinks are byte-identical to stable-backports. Sorry for the noise.

2. I am withdrawing the claim in the title

The title says the IFF "captures nothing" without this bit. I can no longer support that, and you should not merge it on the evidence I gave.

What still stands: toggling IFFDELMUXE3.P0 does change what the two IDDR flops output on silicon, A/B/A, reproduced three times in both directions. There is a real effect here.

What does not stand: which of the two states is the capturing one. The readout was four LEDs reported by position, and I never pinned down the position-to-led[] mapping. Two plausible mappings give opposite conclusions from the same observation, so "0,0 versus 1,1" does not establish direction. I built an experiment whose reading depended on an unestablished correspondence and then drew a directional conclusion from it twice.

And the database argues against the polarity I used:

  • IDELMUXE3 is an arity-2 pair — P0 = 29_101, P1 = !29_101 — and a few lines above, P0 is written exactly when an IDELAYE2 drives D. So P0 means take the delayed path.
  • IFFDELMUXE3 documents only .P0 (28_116); there is no .P1, so not writing it is the other state.
  • The ISERDESE2 path writes IFFDELMUXE3.P0 on (iobdelay == "IFD") — the same sense.

On that reading an IDDR with no IDELAYE2 wants the bit clear, and my condition is inverted. I noted the inconsistency in the original PR but resolved it the wrong way, in favour of a measurement that could not actually distinguish the cases.

What would settle it

An indicator whose value is tied to state independently of which LED is which: one LED, distinguishable blink rates — stuck-0 dark, stuck-1 slow, toggling fast. Then the observation is "does it blink and how fast", not "which of four is lit". I have the board and can do that; I have not yet, and I would rather leave this red-flagged than merge it.

Leaving the branch open as a draft because the effect is real and worth the next person's time, with the code comment rewritten to say only what is established.

Related: on #65 I also posted a diagnosis today that I have since found to be wrong, and I have corrected it there.

@gHashTag gHashTag changed the title xilinx: emit IFFDELMUXE3 for IDDR, without which the IFF captures nothing (fixes #114) xilinx: IFFDELMUXE3 changes IDDR capture on silicon (direction NOT established -- do not merge) Aug 10, 2026
@gHashTag

Copy link
Copy Markdown
Author

Closing this myself. The claim it rests on — that IFFDELMUXE3 sits where I said it does — I withdrew after checking the feature's arity: the value I read as missing is encoded with all bits clear, so my "the bit is absent" observation was the other value, not a defect.

Settling it needs a Vivado-built pair (IDDR straight from the pad vs through IDELAYE2) that I cannot produce. The two designs are in this gist and issue #114 tracks the open question, so nothing is lost by closing the PR.

@hansfbaier — clearing this out of the review queue deliberately, given you are focusing on review and merge. It should not have sat here as a draft after I retracted the claim.

@gHashTag gHashTag closed this Aug 18, 2026
@hansfbaier

Copy link
Copy Markdown
Collaborator

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

xc7: IDDR does not capture at all on silicon — both outputs inert in every edge mode (RGMII receive is dead)

3 participants