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

AXI Memory Over TL Serial Link #121

Merged
merged 14 commits into from
Mar 23, 2021
Merged

AXI Memory Over TL Serial Link #121

merged 14 commits into from
Mar 23, 2021

Conversation

abejgonzalez
Copy link
Contributor

This setup matches closely with the current Berkeley bringup setup where the AXI memory interface resides over the serial link and is in a different clock domain.

  • Adds new connectHarnessMultiClockAXIRAM and MultiClockSerialAXIRAM functions/modules that have AXI memory interface over TL serial link
  • Defaults to using async crossing
  • Expands SerialTLParams to keep track of AXI clock freq. (used in IO/HarnessBinder to specify clock source and correctly give SimDRAM frequency)
  • Add config. fragments to change AXI clock freq. and modify BlockDevice attach locations

@jerryz123
Copy link
Contributor

Niceeeee.
Since this should be strictly better than the Harness TLRAM, can we replace the chipyard examples (if any) that use the TLRAM with this implementation?

@abejgonzalez
Copy link
Contributor Author

While this is strictly better than those examples... there is one caveat... passing an extra clock/reset wire from the SoC over to this harness module. This is needed for FireSim to work (since all clocks need to come from the SoC... unless you want the clock bridge to be wired weirdly to both the harness and the SoC - though both reside in the "target" domain)

Copy link
Contributor

@colinschmidt colinschmidt left a comment

Choose a reason for hiding this comment

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

Seems pretty reasonable to me.

src/main/scala/Configs.scala Outdated Show resolved Hide resolved
src/main/scala/SerialAdapter.scala Outdated Show resolved Hide resolved
src/main/scala/SerialAdapter.scala Outdated Show resolved Hide resolved
src/main/scala/SerialAdapter.scala Show resolved Hide resolved
Copy link
Contributor

@colinschmidt colinschmidt left a comment

Choose a reason for hiding this comment

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

Everything looks good except for the reset change at the end.

axiClockParams match {
case Some(clkParams) => clkParams.clockFreqMHz * (1000 * 1000)
case None => {
// get freq. from what the master of the serial link specifies
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this getting the frequency from what the serial link masters rather than what masters the serial link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yea this comment is off. The serial link masters the FBUS and is slave to the MBUS. The code is still fine though.

src/main/scala/TileResetCtrl.scala Show resolved Hide resolved
Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

LGTM

@abejgonzalez abejgonzalez merged commit 9255690 into master Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants