rewrite the axi fabric to support pipelines. - #513
Open
nathanaelhuffman wants to merge 6 commits into
Open
Conversation
The read-address valid, read-data ready, and read-data resp/valid assignments drove the wrong side of the interface view, so the responder never saw ARVALID and the fabric never saw RVALID/RRESP. Only spi_nor_th used this entity and spi_nor_tb never reads, which is why no read had ever actually traversed it.
The fabric was a purely combinational crossbar behind a one cycle decode, and the decode itself built two 32 bit magnitude compares per responder plus a variable width address mask shared by every responder path. On cosmo_hp that mask and its carry chains are the reported critical path. Spans are powers of two and bases are aligned to their own span, so range membership is just an equality compare on the address bits above the span. The compare stays 32 bits wide against a resized address so a narrow initiator cannot match a base it has no way to reach; the extra bits are constant zeros and fold away. Each responder now masks the address to its own span with a compile time constant, which also lets the fabric side of each responder's address bus collapse to the bits it actually decodes. The responder select becomes a registered one-hot instead of an integer index, so the return path is a flat AND-OR tree rather than an integer to one-hot decode buried inside combinational logic. Three behavioural fixes fall out of the rework: - The responder read address was driven from the initiator *write* address. This only worked because both initiators drive AWADDR and ARADDR from the same register. - Teardown now takes priority over arming, and a write is only decoded once AW and W are both present, matching the condition every responder already applies before asserting AWREADY. Previously an initiator that left AWVALID asserted pinned the fabric to the completed transaction's responder, so the next read decoded against a stale address. - Re-arming is blocked per channel until the initiator drops the request it just completed. Initiators here deassert VALID a cycle after the handshake, so without this a stale request re-armed the fabric and a duplicate transaction went out behind the initiator's back. The error responder also only answers the channel that was actually decoded, so an unmapped read can no longer hand an AWREADY to a write that has not presented its data yet. Add span_mask/bases_aligned/ranges_disjoint/bases_reachable to axil_common_pkg along with elaboration asserts for the invariants the decode now relies on.
The interconnect had no testbench, and because axil8_resizer drove four assignments in the wrong direction and spi_nor_tb never reads, no read had ever traversed this fabric in simulation. The harness instantiates the flat port axil_interconnect_2k8 with a mixed responder map that includes an unmapped hole, and can be driven either by vunit_lib.axi_lite_master or by hand so the testbench can reproduce initiator handshake patterns the bus functional model never generates. Two responder models, deliberately with different handshake shapes: - axil_sram_responder wraps the production axil_target_txn, so it reproduces the contract every register block in the tree presents: a registered AWREADY pulse gated on AWVALID and WVALID, combinational ARREADY, and a single cycle BVALID pulse when BREADY is already asserted. - axil_slow_responder accepts AW and W independently after LFSR driven stalls and holds its responses until READY, which is the shape nothing in the tree currently exercises. The harness also counts handshakes on both sides of the fabric so a duplicated transaction is caught even when the data happens to land correctly, and checks that a stalled channel's payload stays put. Nine of the ten cases fail against the fabric as it was before the previous commit.
Adds the knob that the next commit implements. Every site is set to 0, so this is functionally and structurally a no-op. VHDL records have no field defaults and aggregates must be complete, so adding a field breaks every config aggregate in the tree. Since they all had to be touched anyway, they now go through a resp_cfg() constructor with defaulted arguments, so the next field addition will not break them.
axil_pipe inserts config_array(i).pipe_stages register stages in each direction between the fabric and one responder, so a responder that sits a long way from the fabric no longer has to be reached and answered inside a single clock period. Because the fabric admits one transaction at a time, this does not need five independent AXI channel register slices. The whole transaction serializes into one request bundle out and one response bundle back, which is roughly a third of the flops a pair of full register slices costs and needs one token chain instead of five sets of valid/ready control. Only the bits a responder actually decodes are carried, so an 8 bit responder pipes 45 bits rather than 32 bit addresses. Each payload stage advances only behind its own token, so every stage holds what it was given until the next transaction pushes through it. That keeps the far end stable across a multi-cycle handshake and removes the need for a separate capture register at either end. Gating the chain on the OR of all tokens instead would clobber the last stage on the cycle the token reached it. Deliberately no timing exceptions are required, because cosmo_hp goes through yosys and nextpnr where there is no way to express one. The sink side asserts AWREADY and WREADY together as a registered one-shot, never combinationally off AWVALID, and will not re-arm until the request that just completed is off the bus. The source side samples all the responder handshakes in one state, because axil_target_txn presents BVALID as a single cycle pulse when BREADY is already asserted and ARREADY combinationally. stages = 0 generates a plain pass-through, so it is free and the netlist is unchanged where the knob is left alone. The testbench now runs a mixed 0/1/3/2 stage map and adds a latency case that proves the stages are really in the path: reads answer in 2, 6 and 8 clocks at 0, 1 and 2 stages respectively. Note: multitool format could not be run here, vsg is not installed in this environment.
eSPI is the largest register file and the most distant block in the design, and it owned the worst clk_125m path. With one stage in each direction WNS goes from -0.003ns (failing) to +0.120ns and the worst path moves out of eSPI entirely into the DIMM SPD proxy. The other responders stay unpiped. cosmo_hp and grapefruit need no stages at all after the decode rework.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As #480 mentions, our somewhat naive axi fabric has some improvement opportunities.
This PR rewrites the bock to support user-specified pipeline stages between each of the axi responders, and adds more test coverage since the block is not as simple. Some additional logic improvements are integrated in the rewrite including a one-hot decode methodolgy to reduce logic and increase fmax.
This also fixes a couple of bugs in the axi8 responder that we weren't experiencing in hardware given the previous design but would experience now given the implementation removed some of the simplifications.
This is running successfully on cosmo hw.
Fixes #480