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),