Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

D2D Dummy Loopback Module #49

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

D2D Dummy Loopback Module #49

wants to merge 4 commits into from

Conversation

vighneshiyer
Copy link
Collaborator

Add an empty D2D module with the FDI Bundle that just loops back lp_data to pl_data through a latency pipe. Tie off unused outputs. Add a test to make sure it elaborates cleanly. Next steps (in future PRs) are 1) generate SystemVerilog collateral from reflecting over the FDI Bundle (interface, module wrapper, structs) and 2) creating simple VIP to drive lp_data and fetch pl_data (both in Scala and SystemVerilog, sharing transaction and agent hierarchies).

@vighneshiyer vighneshiyer requested a review from rahulk29 October 19, 2023 05:41
@vighneshiyer
Copy link
Collaborator Author

I can't reproduce the CI scalafmt failure locally, @ethanwu10 can you try to repro locally?

~/8/uciedigital on main ◦ sbt --java-home /usr/lib/jvm/java-17-openjdk/
[info] welcome to sbt 1.9.6 (N/A Java 17.0.8.1)
[info] loading global plugins from /home/vighnesh/.sbt/1.0/plugins
[info] loading settings for project uciedigital-build from plugins.sbt ...
[info] loading project definition from /home/vighnesh/80-temp/uciedigital/project
[info] loading settings for project root from build.sbt ...
[info] set current project to uciedigital (in build file:/home/vighnesh/80-temp/uciedigital/)
[info] sbt server started at local:///home/vighnesh/.sbt/1.0/server/863d3e6e291cf82aee87/sock
[info] started sbt server
sbt:uciedigital> set ThisBuild / scalacOptions += "-Xfatal-warnings"
[info] Defining ThisBuild / scalacOptions
[info] The new value will be used by Compile / scalacOptions
[info] Reapplying settings...
[info] set current project to uciedigital (in build file:/home/vighnesh/80-temp/uciedigital/)
sbt:uciedigital> scalafmtCheckAll
[success] Total time: 2 s, completed Oct 19, 2023, 12:02:51 PM
sbt:uciedigital> scalafmtAll
[success] Total time: 0 s, completed Oct 19, 2023, 12:02:56 PM
sbt:uciedigital> Test / scalafmtCheck
[success] Total time: 0 s, completed Oct 19, 2023, 12:03:22 PM

@ethanwu10
Copy link
Collaborator

CI failure is a deprecation warning for Chisel Scala FIRRTL compiler; do we want to use it anyways, or use the CIRCT stage?

io.plStateStatus := PhyState.active // TODO: this is part of the FDI state machine
}

object D2DDummyLoopbackMain extends App {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To suppress the SFC deprecation warning:

Suggested change
object D2DDummyLoopbackMain extends App {
@annotation.nowarn("cat=deprecation&origin=chisel3.stage.ChiselStage")
object D2DDummyLoopbackMain extends App {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, we will just disable the warning. CirctStage is only a thing in Chisel 5+ and Chipyard will be stuck on Chisel 3.6 for a while. This is also not maintained anymore, so there's no point in using it (https://github.com/sifive/chisel-circt).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

circt.stage.CirctStage does exist (and works) in Chisel 3.6; using SFC is an option though to avoid non-Scala dependencies to elaborate.

io.plStateStatus := PhyState.active // TODO: this is part of the FDI state machine
}

object D2DDummyLoopbackMain extends App {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

circt.stage.CirctStage does exist (and works) in Chisel 3.6; using SFC is an option though to avoid non-Scala dependencies to elaborate.

@@ -81,6 +81,7 @@ class Fdi(params: FdiParams) extends Bundle {
*
* Encompasses `lp_dllp` and `lp_dllp_valid` from the UCIe specification.
*/
// TODO: we don't use dllp's at all, make these IOs optional
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep the IOs in the bundle even if we are never using them, and drive them with constants / add assertions instead; this way our IO is always spec-compliant.

Comment on lines +10 to +11
// LatencyPipe from rocket-chip
// https://github.com/chipsalliance/rocket-chip/blob/master/src/main/scala/util/LatencyPipe.scala
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to vendor utils from rocket-chip in a dedicated package / directory (there are definitely more things we want to steal, e.g. AsyncQueue)

@@ -0,0 +1,95 @@
package edu.berkeley.cs.ucie.digital
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should go in the main build, in the interfaces package - it could conceivably be used by other consumers of this repo to test their own IP. We're putting other verification-related IP for FDI and RDI in the interfaces package as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants