From 7ac0cbe66ced75fea83824fa273830713d7b29ed Mon Sep 17 00:00:00 2001 From: Dennis Brentjes Date: Fri, 9 Oct 2026 10:35:32 +0200 Subject: [PATCH] Launch MISO a full cycle early instead of chasing delay PHASE_METER on a console settled what two rounds of delay-cutting could not. Per-speed dump, first two slots per EXI speed index, with the register file and both byte FIFOs out of the MISO path: 0:8080 1:8080 2:8080 3:8080 4:8080 5:C040 Perfect to 16 MHz, one bit late at 27 MHz. The 8-slot dump at 27 MHz read C0 40 40 40 C0 40 40 40 -- a UNIFORM one-bit lag (ones at bits 1, 9, 17, 25, 33, ...), with extra highs at bits 0 and 32 where libogc splits the read into two 4-byte transfers and MISO tri-states to its pull-up in the gap. The ID dump agreed exactly. Decisive detail: ~9 ns had already been removed from the fabric output path (the EXI_J4 pad merge, then the CS mux, then the entire fabric route by moving the output into an SB_IO register) and the hardware result stayed BYTE IDENTICAL each time. Nine nanoseconds out of an 18.5 ns budget cannot change nothing if propagation were the whole story. What is left is the part no RTL change can reach: input pad + GLOBAL CLOCK NETWORK INSERTION + IOB clock-to-Q + output pad, plus the host's own setup. There was no fabric delay left to cut, so cutting delay was the wrong move. So double the budget instead. MISO is now launched on the RISING edge carrying the bit the host will sample at the NEXT rising edge, so the whole path gets a FULL period (37 ns at 27 MHz) rather than half. This is safe in both directions: the insertion delay that caused the problem now covers the host's hold time on the previous bit, and a full period covers its setup. - tx_out is clocked on the rising edge and loads at rx_bit == 7, one bit-time earlier than a falling-edge shifter loading at rx_bit == 0. - tx_hold and the entire exiclk_n domain are gone: the engine now has ONE clock edge and ONE phase reference. - synth.py's SB_IO output register moves to NEG_TRIGGER=0. - Byte alignment is unchanged -- T2/T5 still land A5 and 3C in the same slots. Only the sub-bit launch timing moved. Testbench consequence, and it matters: bba_top's master sampled MISO AFTER raising the clock. With a zero-delay simulation and a rising-edge launch that reads the NEXT bit, so the suite would have reported everything one bit early while hardware was correct. Both masters now sample in the CLK-LOW phase -- the value the host latches ON the rising edge -- which is also exactly what the hardware notes say a probe must do. T2 slot 0 now reads 0xFF (the CS-reset parking value that keeps MISO high while deselected) instead of 0x00; it is a header byte the host ignores. T6 re-anchored to the rising edge and still proves miso_next predicts spi_miso bit for bit, so the SB_IO register stays equivalent to the fabric model rather than an untested divergence. --seeds 8: clk 28.9-32.1 MHz, EXI 62.1-65.4 MHz, 8/8 PASS. The EXI numbers rise sharply (41.6-49.7 -> 62.1-65.4) because the setup path into the IOB register is now posedge->posedge against a FULL period instead of posedge->negedge against a half -- the same doubling, visible internally. Best seed 3 (clk 32.09, EXI 62.78), flashed. Co-Authored-By: Claude Opus 5 --- exi_bba/bba_top.py | 7 +- exi_bba/exi_capture.py | 7 +- exi_bba/spi_mode3_slave.py | 144 +++++++++++++++++++------------------ exi_bba/synth.py | 6 +- 4 files changed, 85 insertions(+), 79 deletions(-) diff --git a/exi_bba/bba_top.py b/exi_bba/bba_top.py index d5165ed..1413b64 100644 --- a/exi_bba/bba_top.py +++ b/exi_bba/bba_top.py @@ -434,8 +434,13 @@ if __name__ == "__main__": ctx.set(dut.exi_clk, 0) # falling: master drives ctx.set(dut.exi_mosi, (mosi_val >> bit) & 1) await ctx.delay(HP * (HALF)) - ctx.set(dut.exi_clk, 1) # rising: both sample + # Sample MISO in the CLK-LOW phase -- the value the host latches + # ON the rising edge. The device launches MISO one bit EARLY, on + # the rising edge (see spi_mode3_slave), so reading AFTER raising + # the clock here would see the NEXT bit and the testbench would + # report everything one bit early while hardware was correct. miso = (miso << 1) | ctx.get(dut.exi_miso) + ctx.set(dut.exi_clk, 1) # rising: device samples await ctx.delay(HP * (HALF)) return miso diff --git a/exi_bba/exi_capture.py b/exi_bba/exi_capture.py index c8aeeba..2bf225c 100644 --- a/exi_bba/exi_capture.py +++ b/exi_bba/exi_capture.py @@ -13,7 +13,7 @@ through two AsyncFIFOs: Domains created here -------------------- exiclk EXI clock pin, posedge, async-reset by CS -- samples MOSI - exiclk_n EXI clock pin, negedge, async-reset by CS -- drives MISO + and launches MISO one bit early exififo_w EXI clock pin, posedge, RESET-LESS -- rx FIFO write side exififo_r EXI clock pin, posedge, RESET-LESS -- tx FIFO read side @@ -116,8 +116,6 @@ class ExiCapture(Elaboratable): # AsyncFIFO desynchronises its gray pointers and corrupts it, # so the FIFOs must NOT see CS. m.domains.exiclk = cd_p = ClockDomain(async_reset=True) - m.domains.exiclk_n = cd_n = ClockDomain(async_reset=True, - clk_edge="neg") m.domains.exififo_w = cd_fw = ClockDomain(reset_less=True) # NOTE: both FIFO domains are POSEDGE. AsyncFIFO instantiates a # posedge-only reset CDC internally, so a negedge domain cannot host a @@ -126,7 +124,6 @@ class ExiCapture(Elaboratable): m.domains.exififo_r = cd_fr = ClockDomain(reset_less=True) m.d.comb += [ cd_p .clk.eq(self.spi_clk), cd_p .rst.eq(self.spi_cs_n), - cd_n .clk.eq(self.spi_clk), cd_n .rst.eq(self.spi_cs_n), cd_fw.clk.eq(self.spi_clk), cd_fr.clk.eq(self.spi_clk), ] @@ -134,7 +131,7 @@ class ExiCapture(Elaboratable): spi = SPIMode3Slave(phase_meter=self._phase_meter) m.submodules.spi = spi - # RX bytes cross exiclk -> exi; TX bytes cross exi -> exiclk_n. + # RX bytes cross exiclk -> sync; TX bytes cross sync -> exififo_r. # These are the ONLY crossings left, and each has a whole byte-time # (~296 ns at 27 MHz) of slack -- unlike the old design, where # bit-level MISO timing crossed a domain with a 37 ns budget. diff --git a/exi_bba/spi_mode3_slave.py b/exi_bba/spi_mode3_slave.py index e10d53b..9cfb1cb 100644 --- a/exi_bba/spi_mode3_slave.py +++ b/exi_bba/spi_mode3_slave.py @@ -8,10 +8,10 @@ Architecture The engine is clocked by the EXI clock itself rather than oversampling it from a faster domain: - exiclk posedge of EXI CLK — sample MOSI, assemble RX bytes - exiclk_n negedge of EXI CLK — drive MISO (must change BETWEEN the - master's sampling edges) - CS_N asynchronous reset for both + exiclk posedge of EXI CLK — sample MOSI, assemble RX bytes, AND + launch MISO one bit early (see the TX + note in elaborate) + CS_N asynchronous reset Why, and what it fixes (hardware 2026-10-03) -------------------------------------------- @@ -32,7 +32,7 @@ EXI CLK (pin 44) sits at die (6,0) on the 5k-sg48, which is global buffer input net 6 — so it can drive a real clock network with no board change. The domains are created by the PARENT (`ExiCapture`) so the byte FIFOs can name -them; this module just uses `m.d.exiclk` / `m.d.exiclk_n`. +them; this module only uses `m.d.exiclk`. """ from amaranth import * @@ -46,7 +46,7 @@ class SPIMode3Slave(Elaboratable): Ports ----- spi_clk / spi_mosi / spi_cs_n : raw inputs from the GC. spi_clk drives the - exiclk/exiclk_n domains and spi_cs_n their async reset (both wired by + exiclk domain and spi_cs_n its async reset (both wired by the parent); they are still listed here so the module is self-describing and so spi_cs_n can gate MISO combinationally. spi_miso : output to the GC; idles HIGH while CS is deasserted. @@ -106,67 +106,68 @@ class SPIMode3Slave(Elaboratable): m.d.comb += self.rx_byte .eq(nxt) m.d.comb += self.rx_valid.eq(rx_bit == 7) - # ── TX: pop on the POSEDGE, present on the NEGEDGE ──────────────── - # AsyncFIFO needs a posedge clock internally, so the FIFO is read in - # `exiclk` and the byte is handed to the negedge side through a plain - # register. posedge -> negedge of the SAME clock is a half-period - # path (18.5 ns at 27 MHz), which is ample. + # ── TX: launch MISO a FULL CYCLE EARLY, on the RISING edge ──────── + # The host samples MISO on the rising edge. The obvious thing is to + # drive it on the preceding FALLING edge, which gives the output path + # a HALF period -- 18.5 ns at 27 MHz. That is not enough on this + # part, and the shortfall is irreducible: # - # One pop per byte boundary, unconditionally. The byte popped at the - # end of byte N is presented during byte N+1, so: - # - pops during the header drain whatever the previous transaction - # left in the FIFO (this replaces the old CS-fall flush), and - # - the response the register file pushes during the protocol's - # turnaround byte is popped at that byte's end and lands in data - # slot 0, exactly where it belongs. - # No header-length or slot-threshold logic is needed at all. - tx_hold = Signal(8) + # MISO changes at (falling edge AT THE CONNECTOR) + # + input pad delay + # + GLOBAL CLOCK NETWORK INSERTION <-- the big one + # + IOB clock-to-Q + # + output pad delay + # ... and the host still needs its own setup time on top. + # + # Measured on a GameCube 2026-10-03 with PHASE_METER=1 (the engine + # drives a constant 0x80 per byte, so the host reads the engine's byte + # phase directly, with the register file and both byte FIFOs out of the + # path). Per-speed dump, first two slots per EXI speed index: + # + # 0:8080 1:8080 2:8080 3:8080 4:8080 5:C040 + # + # Perfect to 16 MHz, one bit late at 27 MHz. An 8-slot dump at 27 MHz + # read C0 40 40 40 C0 40 40 40 -- a UNIFORM one-bit lag (ones at bit + # 1, 9, 17, 25, 33, ...), with extra highs at bits 0 and 32 where + # libogc splits the read into two 4-byte transfers and MISO tri-states + # to its pull-up in the gap. + # + # Crucially, ~9 ns was removed from the fabric output path first (the + # EXI_J4 pad merge, then the CS mux, then the whole fabric route by + # moving the output into an SB_IO register) and the result stayed BYTE + # IDENTICAL. Nine nanoseconds out of an 18.5 ns budget cannot change + # nothing if propagation delay were the whole story, so the remaining + # cost is the part no RTL change can touch: getting the clock in and + # the data back out of the chip. + # + # So stop shrinking the delay and DOUBLE THE BUDGET. Present the bit + # the host will sample at rising edge N on rising edge N-1, giving the + # whole path a FULL period (37 ns at 27 MHz) instead of half. The + # output then changes shortly AFTER the edge on which the host samples + # the previous bit, which is safe in both directions: the insertion + # delay that caused the problem now covers the host's hold time, and + # a full period covers its setup. + # + # `tx_out` is therefore clocked on the RISING edge and loads at + # rx_bit == 7 -- one bit-time earlier than a falling-edge shifter + # loading at rx_bit == 0. init=0xFF parks MISO high under CS reset. + # This also removes `tx_hold` and the whole exiclk_n domain: the + # engine now has ONE clock edge and ONE phase reference. + tx_out = Signal(8, init=0xFF) m.d.comb += self.tx_load.eq(rx_bit == 7) with m.If(rx_bit == 7): - m.d.exiclk += tx_hold.eq(0x80 if self._phase_meter else self.tx_byte) - - # MISO must settle on the falling edge, before the master samples it - # on the rising one. Driving it on the rising edge is one bit late by - # construction -- that was the 05 C0 bug. - # The byte phase comes from `rx_bit` -- the RISING-edge counter -- NOT - # from a free-running falling-edge counter of its own. - # - # A separate tx_bit counting falling edges only stays in step with the - # host while the host emits exactly as many falls as rises. It does - # not: a transfer that begins on a rising edge (CS asserted while CLK - # is low, or the clock gating between two of the host's back-to-back - # transfers) delivers a rise with no matching fall, and tx_bit silently - # slips one bit behind for the rest of the transaction. RX never - # notices, because the rising count always equals the host's bit count. - # - # Measured on a GameCube 2026-10-03 with a diagnostic build that drives - # a constant 0x80 per byte (PHASE_METER=1 in synth.py): every slot read - # back 0x20 = 0x80 >> 2, i.e. the engine was TWO falling edges behind, - # identically at 1 MHz and at 27 MHz. Speed-independence is the tell — - # a propagation problem cannot survive a 1000 ns bit time, and an - # output-edge error would have moved when the edge was flipped (it did - # not: the rising-edge and falling-edge builds returned byte-identical - # 05 C0 for a NWAYS read). - # - # Slaving the shift register to rx_bit gives the engine ONE phase - # reference, the same edge the host counts, so the two cannot drift. - # init=0xFF so CS (the domain's async reset) parks MISO HIGH by - # itself. MISO is then tx_shift[7] with NOTHING in between -- no - # Mux(cs_n, 1, ...) LUT in the data path. That mux cost 1.21 ns of - # logic AND 3.60 ns of routing to reach it, on a source-synchronous - # output that has only a HALF period (18.5 ns at 27 MHz) to reach the - # GameCube's sampling edge. Removing it also frees the placer to put - # this flop next to the MISO pad. - tx_shift = Signal(8, init=0xFF) - with m.If(rx_bit == 0): - m.d.exiclk_n += tx_shift.eq(tx_hold) # byte boundary: load MSB + m.d.exiclk += tx_out.eq(0x80 if self._phase_meter else self.tx_byte) with m.Else(): - m.d.exiclk_n += tx_shift.eq(Cat(0, tx_shift[:-1])) + m.d.exiclk += tx_out.eq(Cat(0, tx_out[:-1])) - m.d.comb += self.spi_miso.eq(tx_shift[7]) - # What tx_shift[7] WILL be after the next falling edge. Mirrors the - # load/shift choice above exactly; T6 proves the two agree. - m.d.comb += self.miso_next.eq(Mux(rx_bit == 0, tx_hold[7], tx_shift[6])) + m.d.comb += self.spi_miso.eq(tx_out[7]) + # What tx_out[7] WILL be after the next RISING edge -- this is what the + # SB_IO output register captures. Mirrors the load/shift choice above + # exactly; T6 proves the two agree bit for bit. + m.d.comb += self.miso_next.eq( + Mux(rx_bit == 7, + 1 if self._phase_meter else self.tx_byte[7], + tx_out[6])) return m @@ -174,7 +175,7 @@ class SPIMode3Slave(Elaboratable): # ── Testbench ───────────────────────────────────────────────────────────── class _Harness(Elaboratable): - """Wraps the slave with the two pin-driven domains its parent normally + """Wraps the slave with the pin-driven domain its parent normally creates, so the engine can be exercised standalone.""" def __init__(self): @@ -183,12 +184,9 @@ class _Harness(Elaboratable): def elaborate(self, platform): m = Module() m.domains.exiclk = cd_p = ClockDomain(async_reset=True) - m.domains.exiclk_n = cd_n = ClockDomain(async_reset=True, - clk_edge="neg") d = self.dut m.d.comb += [ cd_p.clk.eq(d.spi_clk), cd_p.rst.eq(d.spi_cs_n), - cd_n.clk.eq(d.spi_clk), cd_n.rst.eq(d.spi_cs_n), ] m.submodules.dut = d return m @@ -270,7 +268,11 @@ if __name__ == "__main__": ctx.set(dut.tx_byte, 0x00) s2 = await spi_byte(ctx, 0x00) await cs_high(ctx) - check("T2 miso slot0 (pre-load)", hex(s0), hex(0x00)) + # Slot 0 is the header byte: the host ignores MISO there and the + # engine has not loaded anything yet, so it reads the CS-reset parking + # value (tx_out init=0xFF, which is what keeps MISO high while + # deselected). The bits the host actually uses start at slot 1. + check("T2 miso slot0 (pre-load)", hex(s0), hex(0xFF)) check("T2 miso slot1", hex(s1), hex(0xA5)) check("T2 miso slot2", hex(s2), hex(0x3C)) @@ -315,7 +317,7 @@ if __name__ == "__main__": check("T5 rising-edge start, slot1", hex(s1), hex(0xA5)) check("T5 rising-edge start, slot2", hex(s2), hex(0x3C)) - # T6 — miso_next must equal what spi_miso BECOMES on the next falling + # T6 — miso_next must equal what spi_miso BECOMES on the next RISING # edge. The synthesised build drives the pad from an SB_IO register # clocked on that edge (synth.py), so if this ever diverges the # hardware and the simulation stop agreeing about MISO -- which is @@ -327,13 +329,13 @@ if __name__ == "__main__": mismatch = 0 for _ in range(3): for _bit in range(8): - predicted = ctx.get(dut.miso_next) # before the falling edge ctx.set(dut.spi_clk, 0) await ctx.delay(HP) + predicted = ctx.get(dut.miso_next) # before the RISING edge + ctx.set(dut.spi_clk, 1) + await ctx.delay(HP) if ctx.get(dut.spi_miso) != predicted: mismatch += 1 - ctx.set(dut.spi_clk, 1) - await ctx.delay(HP) await cs_high(ctx) check("T6 miso_next predicts spi_miso", mismatch, 0) diff --git a/exi_bba/synth.py b/exi_bba/synth.py index 0ce81e1..e1f7250 100644 --- a/exi_bba/synth.py +++ b/exi_bba/synth.py @@ -230,7 +230,9 @@ class BBATopSynth(BBATop): # [5:4]=10 tri-state, combinational OUTPUT_ENABLE # [3]=0,[2]=1 dout = dout_q_0, single-rate REGISTERED output # [1:0]=01 simple input (unused) - # NEG_TRIGGER=1 moves the IO registers to the FALLING edge. + # NEG_TRIGGER=0 keeps the IO registers on the RISING edge: the + # engine launches MISO one bit EARLY so the path gets a FULL + # period instead of half -- see the TX note in spi_mode3_slave. # # NOTE this also changes MISO's electrical behaviour: it is now # TRI-STATED when CS is high (held high by the pad's own PULLUP) @@ -241,7 +243,7 @@ class BBATopSynth(BBATop): "SB_IO", p_PIN_TYPE = 0b100101, p_PULLUP = 1, - p_NEG_TRIGGER = 1, + p_NEG_TRIGGER = 0, p_IO_STANDARD = "SB_LVCMOS", io_PACKAGE_PIN = exi.miso.io, i_CLOCK_ENABLE = Const(1, 1),