Add PortAPI between IO and Harness blocks #1610
Conversation
jerryz123
force-pushed
the
port_api
branch
2 times, most recently
from
5b4372e
to
c030b33
Compare
| @@ -55,6 +55,16 @@ class WithScalaTestFeatures extends Config((site, here, up) => { | |||
| case TracePortKey => up(TracePortKey, site).map(_.copy(print = true)) | |||
| }) | |||
|
|
|||
| // Multi-cycle regfile for rocket+boom | |||
There was a problem hiding this comment.
Out of scope of this PR? If this PR takes a while to merge we can separate it out.
There was a problem hiding this comment.
This fixes the old IOBInder-based approach for adding these annotations, to a simpler match statement at the FireSim top level.
| case class ExtIntPort (val io: UInt) | ||
| extends Port[UInt] | ||
|
|
||
| case class DMIPort (val io: ClockedDMIIO) |
There was a problem hiding this comment.
How do you deal with Parameters in these case classes? Sometimes the IO depends on p: Parameters?
There was a problem hiding this comment.
Answered offline. The IO is constructed outside of this case class, thus you don't need the parameters to generate the IO.
| case class I2CPort (val io: sifive.blocks.devices.i2c.I2CPort) | ||
| extends Port[sifive.blocks.devices.i2c.I2CPort] |
There was a problem hiding this comment.
Nit: Why is this fully qualified?
There was a problem hiding this comment.
Because sifive.I2CPort name conflicts with this I2CPort. Ugly, unfortunately
| // TODO FIX: This currently makes each SimDRAM contain the entire memory space | ||
| val memSize = port.params.master.size | ||
| val memBase = port.params.master.base | ||
| val lineSize = 64 // cache block size |
There was a problem hiding this comment.
LineSize isn't parameterized anymore?
There was a problem hiding this comment.
line_size doesn't actually matter for DRAM. We shouldn't even be passing it back to DRAMSim in the first place (and our DRAMSim driver doesn't use the line_size parameter for any calculations). It also requires line_size=64.
Future TODO is to remove the line_size argument to DRAMSim, so we can get rid of this here as well too.
generators/chipyard/src/main/scala/harness/HarnessBinders.scala
Outdated
Show resolved
Hide resolved
generators/chipyard/src/main/scala/harness/HarnessBinders.scala
Outdated
Show resolved
Hide resolved
This adds
trait iobinders.Port, which represents some IO bundle that should be exposed by ChipTop, and connected to in the Harness. Case classes extending this trait should package enough parameter information about the IO bundle to facilitate harness instantiation and/or IO-cell generation.Changes:
Portobjects, instead of raw IO. ThePortobject contains the IO, as well as parameterization information around the IOPorts emitted by the ChipTop. This is only a cosmetic change to the implementationsPorts to connect.Unfortunately, due to the API change, the surface area of this PR is quite large. The main places to look are in
chipyard/src/main/scala/iobinders/Ports.scala, to see the case classes describing top-level ports, as well aschipyard/src/main/scala/harness/HarnessBinders.scala, to see the updated HarnessBinder APIRelated PRs / Issues:
Type of change:
Impact:
Contributor Checklist:
mainas the base branch?changelog:<topic>label?changelog:label?.conda-lock.ymlfile if you updated the conda requirements file?Please Backport?