Establish the +1 offset mechanism: tx FIFO underrun, not a stale byte

Reading the frame body at 0x104 (first byte a known 0xDE) instead of the
descriptor at 0x100 (first byte 0x00, indistinguishable from a stale byte)
shows what actually happens:

    good: DE AD BE EF 01 02
    bad:  DE DE AD BE EF 01      <-- the FIRST BYTE IS DUPLICATED

So it is a tx FIFO UNDERRUN, not an inserted byte.  When the FIFO runs dry for
one byte-time the bit engine reloads the stale FIFO head and sends the previous
byte again, shifting the rest of the read.

The register file must sustain one byte per 296 ns at 27 MHz.  The round trip
is request -> spram_req SyncFIFO -> arbiter (ETH has absolute priority and
preempts it) -> SPRAM -> response -> push -> tx AsyncFIFO CDC (2-3 EXI cycles).
That is marginal even idle and over budget during a frame write, and only ONE
request may be in flight so the latency is never hidden.

THREE FIXES TRIED AND FALSIFIED, recorded in CLAUDE.md so they are not
repeated:

  - Fair round-robin arbitration instead of strict ETH priority: still 2/6.
    Bounds the wait to ~2 cycles but does not close the gap, and breaks
    spram_arbiter T3.  Reverted.
  - Deepen tx_fifo 2 -> 8 for cushion: 0/6 with leading zeros.  The engine
    drains leftovers with ONE POP PER HEADER BYTE, so a deeper FIFO leaks
    stale bytes into the data phase.  tx_depth=2 is deliberate.  Reverted.
  - Allow 2 outstanding requests to pipeline: 0/6 at EVERY rate, including
    HALF=8 which previously passed.  With no response FIFO, responses must be
    consumed the cycle they are valid; two in flight loses bytes.  The
    existing comment was right.  Reverted.

Tree is back to the T9-reproduction baseline: 6/6 at HALF=8, 1-2/6 at HALF=1.

PROMISING DIRECTION, not attempted: SPRAM is 16 BITS WIDE and the arbiter
reads a word then returns one byte selected by address bit 0.  Returning both
bytes per request would push two bytes per round trip, doubling the budget to
592 ns and making it comfortable.  That is the only approach identified that
addresses the constraint rather than working around it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-10-09 20:17:19 +02:00
parent 2eba68a013
commit bd4c69c5f2
2 changed files with 46 additions and 0 deletions
+36
View File
@@ -336,6 +336,42 @@ deliberate: the failure is real and the test is the reproduction. Do not
"fix" it by relaxing T9. `EXI_HALF=8` and `EXI_HALF=2` still pass, so use "fix" it by relaxing T9. `EXI_HALF=8` and `EXI_HALF=2` still pass, so use
those while working on unrelated changes. those while working on unrelated changes.
**MECHANISM, established by reading the frame body instead of the descriptor.**
Reading from 0x104 (first byte a known `0xDE`) rather than 0x100 (first byte
`0x00`, indistinguishable from a stale byte) shows what actually happens:
```
good: DE AD BE EF 01 02
bad: DE DE AD BE EF 01 <-- the FIRST BYTE IS DUPLICATED
```
Not an inserted stale byte — **a tx FIFO UNDERRUN.** When the FIFO runs dry for
one byte-time the bit engine reloads the stale FIFO head and sends the previous
byte again, shifting everything after it.
The register file must sustain one byte per **296 ns** at 27 MHz, and the round
trip is request → `spram_req` SyncFIFO → arbiter (where ETH writes have
absolute priority and preempt it) → SPRAM (1 cycle) → response → push → tx
`AsyncFIFO` CDC (2–3 EXI cycles, 74–111 ns). That is marginal even idle and
over budget under a frame write. Only ONE request may be in flight, so the
latency is never hidden.
⚠️ **THREE FIXES TRIED AND FALSIFIED — do not repeat them:**
| attempt | result |
|---|---|
| **Fair (round-robin) arbitration** instead of strict ETH priority | still 2/6. Bounds the wait to ~2 cycles but does not close the gap, and it breaks `spram_arbiter` T3. |
| **Deepen `tx_fifo` 2 → 8** for cushion | **0/6**, with leading zeros. The engine drains leftovers with ONE POP PER HEADER BYTE, so a deeper FIFO leaks stale bytes into the data phase. `tx_depth=2` is deliberate — keep it shallow. |
| **Allow 2 outstanding requests** to pipeline the latency | **0/6 at every rate**, including `HALF=8` which previously passed. With no response FIFO, responses must be consumed the cycle they are valid; two in flight loses bytes. The existing comment was right. |
**The promising direction, not yet attempted:** SPRAM is **16 bits wide**, and
the arbiter currently reads a word and returns a single byte selected by
address bit 0. Returning **both** bytes per request would let the register file
push two bytes per round trip, doubling the budget to 592 ns per request and
making it comfortable. That needs changes to `SPRAMArbiter`, the register
file's response handling, and their tests — the only approach found so far
that addresses the actual constraint rather than working around it.
### ⚠️ Simulation rate — always re-run at the hardware rate ### ⚠️ Simulation rate — always re-run at the hardware rate
`bba_top`'s testbench clocks EXI at `27 MHz / EXI_HALF`. The default `bba_top`'s testbench clocks EXI at `27 MHz / EXI_HALF`. The default
+10
View File
@@ -667,12 +667,22 @@ if __name__ == "__main__":
ctx.set(dut.w5100_int_n, 0) # another frame arriving ctx.set(dut.w5100_int_n, 0) # another frame arriving
mismatches = 0 mismatches = 0
first_bad = None first_bad = None
# Also read the FRAME BODY at 0x104, whose first byte is 0xDE. The
# descriptor at 0x100 starts with 0x00, which is indistinguishable from
# a stale/empty byte -- so reading a region with a known NON-ZERO first
# byte is what identifies whether slot 0 holds a leftover and, from its
# value, where that leftover came from.
body_reads = []
for attempt in range(6): for attempt in range(6):
again = await exi_read(ctx, 0x0100, total_len) again = await exi_read(ctx, 0x0100, total_len)
if again != want: if again != want:
mismatches += 1 mismatches += 1
if first_bad is None: if first_bad is None:
first_bad = again first_bad = again
body_reads.append(await exi_read(ctx, 0x0104, 6))
print(f"T9 body reads from 0x104 (want DE AD BE EF ..):")
for i, b in enumerate(body_reads):
print(f" [{i}] {' '.join(f'{x:02X}' for x in b)}")
ctx.set(dut.w5100_int_n, 1) ctx.set(dut.w5100_int_n, 1)
await ctx.delay(HP * (HALF * 4)) await ctx.delay(HP * (HALF * 4))