Simple Divider-Only PLL for Multiclock RTL Simulation #676
Conversation
| * [Word from on high is that Strings are in...] | ||
| * Overrides the take field of all clocks in a group, by attempting to apply a | ||
| * series of assignment functions: | ||
| * (name: String) => freq-in-MHz: Option[Double] |
There was a problem hiding this comment.
The assigners shouldn't override any take frequencies already specified by the node, right?
chipsalliance/rocket-chip#2641 is giving us some control over take frequencies
There was a problem hiding this comment.
I was meaning to ask about that. It seemed to me that in the short run it made sense to blow away all taken frequencies because it would be less surprising, but it's trivial to not do that.
There was a problem hiding this comment.
If the primary means of controlling frequencies is through various WithTileClockFreq(freq) fragments, we could just change the underlying implementation of those fragments to use the rocketchip API when its ready.
There was a problem hiding this comment.
Yeah, that sounds good.
I'm still a little concerned about specifying absolute frequencies; i do think it would be preferable to be able to specify constraints on derived frequencies but we can punt on that.
There was a problem hiding this comment.
Re: absolute frequencies. I think the more important part is the relative frequencies but it is easy to calculate the ratio with 100 being the default.
davidbiancolin
force-pushed
the
diplomatic-clocks-pll-redux
branch
from
a636a5f
to
f36183d
Compare
| clk_out = clk_in; | ||
| end | ||
| end else begin | ||
| reg [DIV_COUNTER_WIDTH - 1: 0] count = '0; |
There was a problem hiding this comment.
Is this '0 any thing special or just a way to avoid specifying the width?
There was a problem hiding this comment.
The latter. I'm lazy.
| * This is a Seq of assignment functions, that accept a clock name and return an optional frequency. | ||
| * Functions that appear later in this seq have higher precedence that earlier ones. | ||
| * If no function returns a non-empty value, the value specified in | ||
| * [[DefaultClockFrequencyKey]] will be used -- DFU. |
There was a problem hiding this comment.
:andrewemoji:
|
|
||
| // Imports for multiclock sketch |
There was a problem hiding this comment.
Is this sketch forth coming? Or is this just dead code?
There was a problem hiding this comment.
Yeah this can deleted.
| class WithDMIDTM extends Config((site, here, up) => { | ||
| case ExportDebug => up(ExportDebug, site).copy(protocols = Set(DMI)) | ||
| }) | ||
|
|
||
| class WithNoDebug extends Config((site, here, up) => { | ||
| case DebugModuleKey => None | ||
|
|
| * [Word from on high is that Strings are in...] | ||
| * Overrides the take field of all clocks in a group, by attempting to apply a | ||
| * series of assignment functions: | ||
| * (name: String) => freq-in-MHz: Option[Double] |
There was a problem hiding this comment.
Re: absolute frequencies. I think the more important part is the relative frequencies but it is easy to calculate the ratio with 100 being the default.
| ) | ||
|
|
||
| /** | ||
| * Generates a digttal-divider-only PLL model that verilator can simulate. |
| val (outClocks, ClockGroupEdgeParameters(_, outSinkParams, _, _)) = node.out.head | ||
|
|
||
| val referenceFreq = refSinkParam.take.get.freqMHz | ||
| println(s"Idealized PLL Frequency Summary") |
There was a problem hiding this comment.
I think we should also dump this in an artefact, along with the requested freqs and what is being generated for them.
There was a problem hiding this comment.
Any thoughts about the format?
davidbiancolin
force-pushed
the
diplomatic-clocks-pll-redux
branch
from
b6a2b65
to
b76972d
Compare
| @@ -194,7 +194,7 @@ class FireSimArianeConfig extends Config( | |||
| //* Multiclock Configurations | |||
| //*********************************************************************************/ | |||
| class FireSimMulticlockRocketConfig extends Config( | |||
| new WithFireSimRationalTileDomain(2, 1) ++ | |||
| new chipyard.config.WithTileFrequency(6400.0) ++ //lol | |||
There was a problem hiding this comment.
Wouldn't it make more sense to set pbus freq to 1600, and set this to 3200? Is this to avoid changing UART stuff?
There was a problem hiding this comment.
UART and NIC stuff, yes.
| * If no function returns a non-empty value, the value specified in | ||
| * [[DefaultClockFrequencyKey]] will be used -- DFU. | ||
| */ | ||
| case object ClockFrequencyAssignersKey extends Field[Seq[(String) => Option[Double]]](Seq.empty) |
There was a problem hiding this comment.
Why not just make this Map[String, Option[Double]]
Is it to support the partial string matching?
There was a problem hiding this comment.
That was the original motivation. I thought about making it a regex, but then i just settled on doing a more general thing. If you wanted to use the core idx as an argument you could do that with this very simply.
| } | ||
| // Assign resets. The synchronization scheme is still WIP. | ||
| for ((name, clockBundle) <- clockGroupBundle.member.elements) { | ||
| if (name.contains("core")) { |
There was a problem hiding this comment.
Why does this only need to be synchronized for the cores?
There was a problem hiding this comment.
This just matches what was done before. I've been punting on it. The general problem is: in what order should these resets be deasserted, and how should the user specify that (if they don't want to provide their own clocking scheme)? Simply synchronizing them will result in the fastest domains coming out of reset first, which may or may not not be desired. In any case, we have the same problem the idealizedPLL.
As an aside, I have tried synchronizing everything and it seems to just work for FireSim.
Un elated, maybe we should start building our library of PRCI diplomatic widgets and write a synchronizer node. We probably also need a lengthener, as alluded to in some of henry's PRs.
This subsumes #659.
Provides the mechanism to specify clock frequencies by name per interim advise from SiFive, reusing the PLL from #659. (I've applied all the comments from the PR). At a high level, users provide "assigner" functions that accept the name of a clock and return an Option[Double]. Users can register new functions by appending to `ClockFrequencyAssignment. The clock schemes from before have been collapsed one. You request a multiclock design by (1) providing an assigner, and (2) ensuring the rest of the target has the right CDCs.
TODOs: