Give TileLink keys more sensible names #349
Conversation
| @@ -50,7 +50,7 @@ abstract class BaseCoreplex(c: CoreplexConfig)(implicit p: Parameters) extends L | |||
|
|
|||
| abstract class BaseCoreplexBundle(val c: CoreplexConfig)(implicit val p: Parameters) extends Bundle with HasCoreplexParameters { | |||
| val master = new Bundle { | |||
| val mem = Vec(c.nMemChannels, new ClientUncachedTileLinkIO()(outermostParams)) | |||
| val mem = Vec(c.nMemChannels, new ClientUncachedTileLinkIO()(outerMemParams)) | |||
There was a problem hiding this comment.
This is changing behavior. Was this a bug before?
There was a problem hiding this comment.
No, before the width adapter was still inside the coreplex. Now the width adapter is in the periphery. That way there are no edge* parameters in the coreplex. I think that's the most sensible and consistent way.
| val backendBuffering = TileLinkDepths(0,0,0,0,0) | ||
| for ((bank, icPort) <- managerEndpoints zip mem_ic.io.in) { | ||
| val enqueued = TileLinkEnqueuer(bank.outerTL, backendBuffering) | ||
| val unwrapped = TileLinkIOUnwrapper(enqueued) | ||
| TileLinkWidthAdapter(icPort, unwrapped) | ||
| icPort <> TileLinkIOUnwrapper(enqueued) |
There was a problem hiding this comment.
I see what you're doing here now. You've moved the width conversion out to the Periphery. I support this change.
There was a problem hiding this comment.
Yes this seems more straightforward
| @@ -173,7 +175,8 @@ trait PeripheryMasterMemModule extends HasPeripheryParameters { | |||
|
|
|||
| // Abuse the fact that zip takes the shorter of the two lists | |||
| ((io.mem_axi zip coreplexIO.master.mem) zipWithIndex) foreach { case ((axi, mem), idx) => | |||
| val axi_sync = PeripheryUtils.convertTLtoAXI(mem) | |||
| val mem_edge = TileLinkWidthAdapter(mem, edgeMemParams) | |||
There was a problem hiding this comment.
Why don't you factor mem_edge out of the specific axi, ahb, tl connection routines?
|
This commit has 3 different things going on. I suggest you split them out to separate commits:
|
|
Howie, can you also take a look at why the regression failed? |
|
Yeah, the regressions failed because I forgot that the old Outermost TLKey was also increasing the size of the client_xact_id. So now the FancyMemtestConfig is failing because ID bits are getting cut off. I'm fixing that now by just making the client_xact_id for L2toMC large enough to get through the arbiters. |
zhemao
force-pushed
the
tl1-params-fix
branch
from
65030f3
to
7d39dd0
Compare
|
I'll try to split the commits. |
| case TLKey("Outermost") => site(TLKey("L2toMC")).copy( | ||
| maxClientXacts = site(NAcquireTransactors) + 2, | ||
| maxClientsPerPort = site(NBanksPerMemoryChannel), | ||
| dataBeats = site(MIFDataBeats)) |
There was a problem hiding this comment.
Coreplex not knowing about "outermost"/"edgeX" also feels correct.
zhemao
force-pushed
the
tl1-params-fix
branch
from
2a64ba7
to
d6ada62
Compare
* Outermost -> MCtoEdge * MMIO_Outermost -> MMIOtoEdge Then the corresponding parameters objects are * L1toL2 -> innerParams * L2toMC -> outerMemParams * L2toMMIO -> outerMMIOParams * MCtoEdge -> edgeMemParams * MMIOtoEdge -> edgeMMIOParams
zhemao
force-pushed
the
tl1-params-fix
branch
from
d6ada62
to
aaaa0f8
Compare
|
This is great! |
The "Outermost" and "MMIO_Outermost" keys are renamed to "MCtoEdge"
and "MMIOtoEdge". They are only used after width conversion to adapt to
external interfaces in the periphery.
That means there is no longer any width adapting done in the Coreplex,
the width adapter is moved to the Periphery just before conversion
to AXI/AHB or sending off chip.
We also add an additional TLKey "EdgetoSlave" for the bus_axi interface.
This means there is also a width adapter from the AXI -> TL converter
to the coreplex slave port.
This also fixes a bug introduced previously in which the wrong
parameters object is given to the width adapter for external MMIO.