Roflin
abab26232b
SPRAMArbiter latched the SPRAM word into rsp_buf and raised exi_rsp_valid a cycle later. BBARegisterFile already took the response the cycle it arrived, so the buffer bought nothing and cost a 24 MHz cycle (~42 ns) inside a deadline that was short by about that much. Both are now combinational in the cycle SPRAM's output is valid. The data sampling is unchanged -- the old code read spram_dout in that same cycle, it just registered it before handing it over -- so only the handoff moved. bba_top T9 goes from 1/6 to 6/6 byte-exact at EXI_HALF=1, and stays 6/6 at 2 and 8. The whole suite passes at the hardware rate. Nothing else changed: BBARegisterFile is untouched. Found by measuring instead of guessing, after four falsified attempts. A counter in ExiCapture recording tx_load & ~tx_fifo.r_rdy, with a shift-register history of which byte boundary underran, showed good and bad reads BOTH underrun at pop 3 (harmless, mid-header) while a bad read adds pop 4 -- the pop that loads data slot 0. So the late byte is the FIRST data byte, which rules out every "prefetch further ahead" idea, because byte 0's request cannot be issued before RHDR_LO where the low address byte arrives. That makes the deadline arithmetic with no contention in it: rx CDC ~125ns + arbiter 42 + SPRAM 42 + rsp_buf 42 + tx CDC ~110 = ~361ns against a 296ns byte time. Over budget idle, which is why fair arbitration changed nothing -- ETH contention was only what pushed a marginal path over on some frames. Two further changes were implemented, measured and then removed as unnecessary: issuing byte 1's request during the pad byte, and two requests in flight with a 3-deep response hold. With rsp_buf gone, T9 is 6/6 with both removed, with either alone, and with the register file reverted completely. They were real pipelining but not the constraint, and they cost logic on sync, the binding timing domain. CLAUDE.md records them as do-not-re-add, along with the trap that sank the earlier 2-outstanding attempt: incrementing `outstanding` in the FSM and decrementing it in the response handler are two m.d.sync assignments that Amaranth resolves by PRIORITY, not arithmetic, so the increment is silently discarded when both fire in one cycle. Arbiter TB updated (the response is a cycle earlier) and mutation-tested: re-registering the response fails T1/T2/T4, swapping the byte lanes fails T1/T2. New T4 pins exi_rsp_valid to exactly one cycle. Not yet confirmed on hardware; netboot's reader still probes both offsets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
302 lines
12 KiB
Python
302 lines
12 KiB
Python
"""SPRAM arbiter — sync domain (24 MHz).
|
||
|
||
Owns the iCE40UP5K 128 KB SPRAM (SB_SPRAM256KA, 16-bit wide) and arbitrates
|
||
between two clients:
|
||
|
||
Client A (EXI read) : prefetch pipeline; low priority.
|
||
Client B (ETH write): RXFrameAssembler; high priority.
|
||
|
||
ETH writes win when both clients are active. This is safe because the GC only
|
||
reads pages that the ETH engine has already finished writing (ring-buffer
|
||
invariant).
|
||
|
||
SPRAM addressing
|
||
-----------------
|
||
SB_SPRAM256KA is 64 K × 16-bit. Byte addressing:
|
||
ADDRESS = byte_addr >> 1
|
||
MASKWREN[3:0]:
|
||
0b0011 → write lower byte (byte_addr even)
|
||
0b1100 → write upper byte (byte_addr odd)
|
||
Read: both bytes returned; pick the right one from DATAOUT based on addr bit 0.
|
||
|
||
Read latency: 1 synchronous cycle — result of cycle N is valid at N+1.
|
||
|
||
In simulation (platform is None) a behavioural Array model is used instead of
|
||
the SB_SPRAM256KA Instance so tests run without IceStorm.
|
||
"""
|
||
|
||
from amaranth import *
|
||
from amaranth.lib.memory import Memory
|
||
|
||
__all__ = ["SPRAMArbiter"]
|
||
|
||
_SPRAM_WORDS = 65536 # 64 K 16-bit words = 128 KB
|
||
|
||
|
||
class SPRAMArbiter(Elaboratable):
|
||
"""Arbitrated SPRAM controller in the sync domain.
|
||
|
||
EXI read interface (from BBARegisterFile spram_req / spram_rsp FIFOs)
|
||
----------------------------------------------------------------------
|
||
exi_req_addr : 16-bit byte address to read
|
||
exi_req_valid : FIFO r_rdy — a request is waiting
|
||
exi_req_ready : FIFO r_en — pop the request (asserted when serviced)
|
||
exi_rsp_data : 8-bit result byte — COMBINATIONAL, valid only while
|
||
exi_rsp_valid is high; the consumer must take it that cycle
|
||
exi_rsp_valid : result is valid this cycle
|
||
|
||
ETH write interface (from RXFrameAssembler)
|
||
-------------------------------------------
|
||
eth_wr_addr : 16-bit byte address to write
|
||
eth_wr_data : 8-bit byte value
|
||
eth_wr_valid : write request present
|
||
eth_wr_ready : write accepted this cycle
|
||
"""
|
||
|
||
def __init__(self):
|
||
# EXI read interface
|
||
self.exi_req_addr = Signal(16)
|
||
self.exi_req_valid = Signal()
|
||
self.exi_req_ready = Signal()
|
||
self.exi_rsp_data = Signal(8)
|
||
self.exi_rsp_valid = Signal()
|
||
|
||
# ETH write interface
|
||
self.eth_wr_addr = Signal(16)
|
||
self.eth_wr_data = Signal(8)
|
||
self.eth_wr_valid = Signal()
|
||
self.eth_wr_ready = Signal()
|
||
|
||
def elaborate(self, platform):
|
||
m = Module()
|
||
|
||
# ── SPRAM instantiation (hardware vs simulation) ──────────────────
|
||
spram_addr = Signal(14) # word address (byte_addr >> 1)
|
||
spram_din = Signal(16)
|
||
spram_dout = Signal(16)
|
||
spram_wren = Signal()
|
||
spram_mask = Signal(4) # MASKWREN
|
||
|
||
if platform is None:
|
||
# Behavioural model: synchronous read with 1-cycle latency.
|
||
# Memory is a Component; read/write ports are obtained from it
|
||
# and wired via its submodule ports (not added as separate submodules).
|
||
mem = Memory(shape=16, depth=_SPRAM_WORDS, init=[])
|
||
m.submodules.mem = mem
|
||
mem_rd = mem.read_port(domain="sync", transparent_for=[])
|
||
mem_wr = mem.write_port(domain="sync", granularity=8)
|
||
|
||
# en[0] = lower byte enable, en[1] = upper byte enable
|
||
byte0_en = Signal()
|
||
byte1_en = Signal()
|
||
m.d.comb += [
|
||
byte0_en .eq(spram_wren & (spram_mask[0] | spram_mask[1])),
|
||
byte1_en .eq(spram_wren & (spram_mask[2] | spram_mask[3])),
|
||
mem_rd.addr .eq(spram_addr),
|
||
mem_rd.en .eq(1),
|
||
spram_dout .eq(mem_rd.data),
|
||
mem_wr.addr .eq(spram_addr),
|
||
mem_wr.data .eq(spram_din),
|
||
mem_wr.en .eq(Cat(byte0_en, byte1_en)),
|
||
]
|
||
else:
|
||
# Hardware: instantiate two SB_SPRAM256KA (64K×16 each; use one)
|
||
m.submodules.spram = Instance(
|
||
"SB_SPRAM256KA",
|
||
i_ADDRESS = spram_addr,
|
||
i_DATAIN = spram_din,
|
||
i_MASKWREN = spram_mask,
|
||
i_WREN = spram_wren,
|
||
i_CHIPSELECT = Const(1, 1),
|
||
i_CLOCK = ClockSignal("sync"),
|
||
i_STANDBY = Const(0, 1),
|
||
i_SLEEP = Const(0, 1),
|
||
i_POWEROFF = Const(1, 1),
|
||
o_DATAOUT = spram_dout,
|
||
)
|
||
|
||
# ── Arbiter pipeline ─────────────────────────────────────────────
|
||
# Stage 1: issue SPRAM address and control signals (combinatorial)
|
||
# Stage 2: capture SPRAM output into rsp_buf (synchronous, 1-cycle)
|
||
|
||
read_pending = Signal() # a read address was issued last cycle
|
||
read_was_odd = Signal() # byte address bit 0 of the pending read
|
||
|
||
# The response is COMBINATIONAL in the cycle SPRAM's output is valid,
|
||
# not registered for one more. That register cost a 24 MHz cycle
|
||
# (~42 ns) inside the EXI read's deadline: the request for data byte 0
|
||
# cannot be issued before the header's low address byte has crossed
|
||
# the rx CDC, leaving ~296 ns for rx CDC (~125 ns) + this round trip +
|
||
# the tx CDC (~110 ns). With the extra register that totals ~361 ns
|
||
# and the engine's pop for data slot 0 found tx_fifo empty, resending
|
||
# the stale head (the duplicated-first-byte underrun). The consumer
|
||
# takes the response the cycle it is valid, so the buffer bought
|
||
# nothing.
|
||
|
||
# Combinatorial defaults
|
||
m.d.comb += [
|
||
spram_wren .eq(0),
|
||
spram_mask .eq(0),
|
||
spram_din .eq(0),
|
||
spram_addr .eq(0),
|
||
self.exi_req_ready.eq(0),
|
||
self.eth_wr_ready .eq(0),
|
||
self.exi_rsp_valid.eq(read_pending),
|
||
self.exi_rsp_data .eq(Mux(read_was_odd,
|
||
spram_dout[8:16], spram_dout[0:8])),
|
||
]
|
||
# Registered defaults
|
||
m.d.sync += read_pending.eq(0)
|
||
|
||
# ETH write has priority
|
||
with m.If(self.eth_wr_valid):
|
||
m.d.comb += [
|
||
spram_addr .eq(self.eth_wr_addr[1:]),
|
||
spram_wren .eq(1),
|
||
self.eth_wr_ready.eq(1),
|
||
]
|
||
with m.If(self.eth_wr_addr[0]):
|
||
m.d.comb += [
|
||
spram_din [8:16].eq(self.eth_wr_data),
|
||
spram_mask .eq(0b1100),
|
||
]
|
||
with m.Else():
|
||
m.d.comb += [
|
||
spram_din [0:8].eq(self.eth_wr_data),
|
||
spram_mask .eq(0b0011),
|
||
]
|
||
|
||
# EXI read (lower priority)
|
||
with m.Elif(self.exi_req_valid):
|
||
m.d.comb += [
|
||
spram_addr .eq(self.exi_req_addr[1:]),
|
||
self.exi_req_ready.eq(1),
|
||
]
|
||
m.d.sync += [
|
||
read_pending.eq(1),
|
||
read_was_odd.eq(self.exi_req_addr[0]),
|
||
]
|
||
|
||
return m
|
||
|
||
|
||
# ── Testbench ─────────────────────────────────────────────────────────────
|
||
|
||
if __name__ == "__main__":
|
||
import sys
|
||
from amaranth.sim import Simulator, Period
|
||
|
||
dut = SPRAMArbiter()
|
||
errors = []
|
||
|
||
async def testbench(ctx):
|
||
await ctx.tick("sync").repeat(2)
|
||
|
||
# T1: ETH write to even byte address 0x0100, then EXI read it back
|
||
ctx.set(dut.eth_wr_addr, 0x0100)
|
||
ctx.set(dut.eth_wr_data, 0xAB)
|
||
ctx.set(dut.eth_wr_valid, 1)
|
||
await ctx.tick("sync").repeat(1)
|
||
accepted = ctx.get(dut.eth_wr_ready)
|
||
if not accepted:
|
||
errors.append("T1 eth write not accepted")
|
||
ctx.set(dut.eth_wr_valid, 0)
|
||
await ctx.tick("sync").repeat(1)
|
||
|
||
# Issue EXI read of the same address
|
||
ctx.set(dut.exi_req_addr, 0x0100)
|
||
ctx.set(dut.exi_req_valid, 1)
|
||
await ctx.tick("sync").repeat(1) # clock A: read issued, read_pending=1
|
||
ctx.set(dut.exi_req_valid, 0)
|
||
# Check HERE. The response is COMBINATIONAL: during the cycle after
|
||
# the issue, read_pending is high and SPRAM's output is valid, so
|
||
# exi_rsp_valid and exi_rsp_data are live RIGHT NOW. Waiting another
|
||
# tick reads zeros -- which is the regression this asserts against,
|
||
# since a registered response would need that extra tick and would
|
||
# cost the EXI read 42 ns it does not have.
|
||
|
||
rdata = ctx.get(dut.exi_rsp_data)
|
||
rvalid = ctx.get(dut.exi_rsp_valid)
|
||
if rdata != 0xAB:
|
||
errors.append(f"T1 read back: expected 0xAB, got 0x{rdata:02X}")
|
||
if not rvalid:
|
||
errors.append("T1 exi_rsp_valid not set")
|
||
print(f"T1 even addr read-back: data=0x{rdata:02X} valid={rvalid}")
|
||
|
||
await ctx.tick("sync").repeat(2)
|
||
|
||
# T2: ETH write to ODD byte address 0x0101, read back
|
||
ctx.set(dut.eth_wr_addr, 0x0101)
|
||
ctx.set(dut.eth_wr_data, 0xCD)
|
||
ctx.set(dut.eth_wr_valid, 1)
|
||
await ctx.tick("sync").repeat(1)
|
||
ctx.set(dut.eth_wr_valid, 0)
|
||
await ctx.tick("sync").repeat(1)
|
||
|
||
ctx.set(dut.exi_req_addr, 0x0101)
|
||
ctx.set(dut.exi_req_valid, 1)
|
||
await ctx.tick("sync").repeat(1)
|
||
ctx.set(dut.exi_req_valid, 0)
|
||
|
||
rdata = ctx.get(dut.exi_rsp_data)
|
||
if not ctx.get(dut.exi_rsp_valid):
|
||
errors.append("T2 exi_rsp_valid not set")
|
||
if rdata != 0xCD:
|
||
errors.append(f"T2 odd addr read-back: expected 0xCD, got 0x{rdata:02X}")
|
||
print(f"T2 odd addr read-back: data=0x{rdata:02X}")
|
||
|
||
await ctx.tick("sync").repeat(2)
|
||
|
||
# T3: ETH write wins when both clients active simultaneously
|
||
# Write 0xEE to 0x0200
|
||
ctx.set(dut.eth_wr_addr, 0x0200)
|
||
ctx.set(dut.eth_wr_data, 0xEE)
|
||
ctx.set(dut.eth_wr_valid, 1)
|
||
ctx.set(dut.exi_req_addr, 0x0100) # also wants to read
|
||
ctx.set(dut.exi_req_valid, 1)
|
||
await ctx.tick("sync").repeat(1)
|
||
|
||
eth_won = ctx.get(dut.eth_wr_ready)
|
||
exi_blocked = not ctx.get(dut.exi_req_ready)
|
||
ctx.set(dut.eth_wr_valid, 0)
|
||
ctx.set(dut.exi_req_valid, 0)
|
||
|
||
if not eth_won:
|
||
errors.append("T3 ETH priority: ETH write not accepted")
|
||
if not exi_blocked:
|
||
errors.append("T3 ETH priority: EXI read was not blocked")
|
||
print(f"T3 ETH priority: eth_won={eth_won} exi_blocked={exi_blocked}")
|
||
|
||
await ctx.tick("sync").repeat(2)
|
||
|
||
# T4: exi_rsp_valid is high for EXACTLY the one cycle after the issue.
|
||
# The consumer (BBARegisterFile) relies on that to count responses
|
||
# against requests in flight; a valid that lingered would be counted
|
||
# twice and insert a byte into the GC's read.
|
||
ctx.set(dut.exi_req_addr, 0x0100)
|
||
ctx.set(dut.exi_req_valid, 1)
|
||
await ctx.tick("sync").repeat(1)
|
||
ctx.set(dut.exi_req_valid, 0)
|
||
v_now = ctx.get(dut.exi_rsp_valid)
|
||
await ctx.tick("sync").repeat(1)
|
||
v_after = ctx.get(dut.exi_rsp_valid)
|
||
if not v_now or v_after:
|
||
errors.append(f"T4 rsp_valid window: now={v_now} after={v_after} "
|
||
f"(want 1 then 0)")
|
||
print(f"T4 rsp_valid one cycle only: now={v_now} after={v_after}")
|
||
|
||
sim = Simulator(dut)
|
||
sim.add_clock(Period(MHz=24), domain="sync")
|
||
sim.add_testbench(testbench)
|
||
|
||
with sim.write_vcd("SPRAMArbiter.vcd"):
|
||
sim.run()
|
||
|
||
if errors:
|
||
print("\nFAILURES:")
|
||
for e in errors:
|
||
print(" ", e)
|
||
sys.exit(1)
|
||
else:
|
||
print("\nAll tests passed.")
|