Restore the TX-done clears to the TX handler; shorten the ACK timeout
TIMING MEASUREMENT, which is the useful part. Raising the host timeout
0.1 -> 0.3 barely moved the retry count (110 -> 95) but tripled the cost of
each one: 16.9 s -> 40.1 s. If those were LATE ACKs, three times the deadline
would have all but eliminated them. They are LOST ACKs, so a SHORT timeout is
correct, not a long one -- the GC re-ACKs a duplicate block, so retrying early
costs nothing. Host default is now 0.05.
REGRESSION, mine, from 92f7d71. Inserting the RBFI handler split the TX-done
block, leaving these two lines attached to rx_overflow -- an RX event:
m.d.sync += r_ncra[1:3].eq(0) # clear ST bits
m.d.sync += tx_queued.eq(0) # ready for the next frame
So after the first transmit tx_queued stayed set forever. The NCRA ST
fallback trigger is guarded on ~tx_queued, so software driving TX via ST
rather than TWD would transmit exactly ONCE and then silently stop, and the ST
bits were left asserted in NCRA. Our own path arms via TWD, which is why it
kept working and why this was not noticed.
T9 covers it and is mutation-tested: with the clears misplaced, NCRA reads
0x0A with ST stuck set.
This does NOT explain the 1/3 ACK loss -- the TWD path was unaffected -- so
that remains open. txto from the next run is the discriminator: near zero
means the frames reach the W5100 and are lost after that, which points at the
chip's TX handling rather than the EXI side.
--seeds 8: clk 27.5-29.4 MHz, EXI 54.7-62.3 MHz, 8/8 PASS. Best seed 5
(clk 29.41, EXI 60.22), flashed. The .dol is unchanged; only send-dol.py's
default timeout moved, and that is read from the repo.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -687,8 +687,17 @@ class BBARegisterFile(Elaboratable):
|
||||
m.d.sync += twd_armed.eq(0)
|
||||
m.d.sync += tx_queued.eq(1)
|
||||
|
||||
# TX done. Clearing the ST bits and tx_queued belongs HERE, with the
|
||||
# completion that they describe. Adding the RBFI handler below split
|
||||
# this block and left those two lines attached to rx_overflow -- an RX
|
||||
# event -- so after the first frame tx_queued stayed set forever, which
|
||||
# disables the NCRA ST fallback trigger entirely (it is guarded on
|
||||
# ~tx_queued) and leaves the ST bits asserted in NCRA. Software that
|
||||
# drives TX via ST instead of TWD would transmit exactly once.
|
||||
with m.If(self.tx_irq_i):
|
||||
m.d.sync += r_ir[2].eq(1) # TI bit
|
||||
m.d.sync += r_ncra[1:3].eq(0) # clear ST bits
|
||||
m.d.sync += tx_queued.eq(0) # ready for the next frame
|
||||
|
||||
# RBFI: the ring was full and a frame was discarded. Reported rather
|
||||
# than hidden -- the previous behaviour was to overwrite unread pages,
|
||||
@@ -696,8 +705,6 @@ class BBARegisterFile(Elaboratable):
|
||||
# gone wrong.
|
||||
with m.If(self.rx_overflow):
|
||||
m.d.sync += r_ir[7].eq(1)
|
||||
m.d.sync += r_ncra[1:3].eq(0) # clear ST bits
|
||||
m.d.sync += tx_queued.eq(0) # ready for the next frame
|
||||
|
||||
return m
|
||||
|
||||
@@ -996,7 +1003,34 @@ if __name__ == "__main__":
|
||||
print(f"T8 multi-byte TXDATA write: {len(got)}/{len(payload)} bytes "
|
||||
f"{'ok' if got == payload else 'MISMATCH'}")
|
||||
|
||||
# NO T9 FOR THE LATE-RESPONSE LEAK -- and that is deliberate.
|
||||
# T9: TX completion must clear the ST bits and re-arm for the next
|
||||
# frame. Those two actions used to sit in the rx_overflow handler by
|
||||
# accident, so after ONE transmit tx_queued stayed set forever: the
|
||||
# NCRA ST fallback trigger is guarded on ~tx_queued, so software that
|
||||
# drives TX via ST rather than TWD transmitted exactly once and then
|
||||
# silently stopped.
|
||||
await exi_write(ctx, _NCRA, [0x08 | 0x02]) # SR + ST0
|
||||
await ctx.tick("sync").repeat(2)
|
||||
ctx.set(reg.tx_irq_i, 1) # frame completed
|
||||
await ctx.tick("sync").repeat(1)
|
||||
ctx.set(reg.tx_irq_i, 0)
|
||||
await ctx.tick("sync").repeat(2)
|
||||
got = await exi_read(ctx, _NCRA, 1)
|
||||
if got and (got[0] & 0x06):
|
||||
errors.append(f"T9 ST bits not cleared on TX done: NCRA=0x{got[0]:02X}")
|
||||
print(f"T9 NCRA after TX done: 0x{got[0]:02X} (ST bits must be clear)")
|
||||
|
||||
# ...and a SECOND ST-triggered transmit must still queue, which it
|
||||
# cannot if tx_queued was left set.
|
||||
tx_ctrl_before = ctx.get(reg.tx_ctrl_r_rdy)
|
||||
await exi_write(ctx, _NCRA, [0x08 | 0x02])
|
||||
await ctx.tick("sync").repeat(4)
|
||||
if not ctx.get(reg.tx_ctrl_r_rdy):
|
||||
errors.append("T9 second ST trigger did not queue a frame")
|
||||
print(f"T9 second ST trigger queues: {bool(ctx.get(reg.tx_ctrl_r_rdy))} "
|
||||
f"(was {bool(tx_ctrl_before)})")
|
||||
|
||||
# NO T10 FOR THE LATE-RESPONSE LEAK -- and that is deliberate.
|
||||
#
|
||||
# The guard added to the response handler (outstanding != 0) and the
|
||||
# sp_pending clear in HEADER0 fix a real latent bug: a response that
|
||||
|
||||
+8
-5
@@ -58,11 +58,14 @@ def main():
|
||||
ap = argparse.ArgumentParser()
|
||||
ap.add_argument("iface", help="network interface facing the GameCube")
|
||||
ap.add_argument("dol", help=".dol to send")
|
||||
ap.add_argument("--timeout", type=float, default=0.3,
|
||||
help="per-block ACK timeout in seconds (default 0.3). "
|
||||
"The GC needs ~20 ms per block and the variance is "
|
||||
"what matters: at 0.1 roughly a third of blocks "
|
||||
"overshot and each miss cost a full timeout.")
|
||||
ap.add_argument("--timeout", type=float, default=0.05,
|
||||
help="per-block ACK timeout in seconds (default 0.05). "
|
||||
"Measured: raising it 0.1 -> 0.3 barely changed the "
|
||||
"retry count (110 -> 95) but tripled the cost of "
|
||||
"each one (16.9 s -> 40.1 s), so the misses are "
|
||||
"LOST ACKs, not late ones -- which makes a SHORT "
|
||||
"timeout the right answer. The GC re-ACKs a "
|
||||
"duplicate block, so retrying early is harmless.")
|
||||
ap.add_argument("--retries", type=int, default=20,
|
||||
help="per-block retransmits before giving up (default 20)")
|
||||
ap.add_argument("--dest", default="/gc_bba_test.dol",
|
||||
|
||||
Reference in New Issue
Block a user