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

Simple Divider-Only PLL for Multiclock RTL Simulation #676

Merged
merged 20 commits into from
Oct 1, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
895bace
WIP - Simple divider-only PLL generation flow
davidbiancolin Aug 24, 2020
8e4dedc
Remove require guard on divided configs
davidbiancolin Sep 16, 2020
b8d3e4a
Update Idealized PLL config
davidbiancolin Sep 16, 2020
cfa7e30
[clocks] Fix comment in ClockDividerN
davidbiancolin Sep 17, 2020
6a26a35
[clocks] Update dealiaser based on feedback
davidbiancolin Sep 17, 2020
0f33ea3
[clocks] Stringly specified clock frequencies; DRY out schemes
davidbiancolin Sep 17, 2020
ad147ec
[clocks] Remove dealiaser and node injector until they are needed
davidbiancolin Sep 17, 2020
f36183d
[clocks] Update AssignerKey name and comment
davidbiancolin Sep 17, 2020
84195d2
[clocks] Don't override existing take frequency if present.
davidbiancolin Sep 23, 2020
96bf702
[clocks] Factor out the PLL calculations into their own class
davidbiancolin Sep 25, 2020
f6989a1
[clocks] Use the periphery frequency as the default
davidbiancolin Sep 25, 2020
cc949aa
[clocking] Address some of Colin's PR comments
davidbiancolin Sep 25, 2020
7b8a954
[firechip] Rework FireSim clocking to be more similar to default CY t…
davidbiancolin Sep 25, 2020
1b3514f
[clocks] Specify a default frequency for TraceGen
davidbiancolin Sep 25, 2020
67145c6
[clocking] Fix FireSim clock look up
davidbiancolin Sep 25, 2020
b76972d
Merge remote-tracking branch 'origin/dev' into diplomatic-clocks-pll-…
davidbiancolin Sep 25, 2020
a6ce850
[clocks] ClockDividerN: make first output edge occur on first input edge
davidbiancolin Sep 29, 2020
5b414f5
[clocks] Emit frequency summary for divider-only PLL model
davidbiancolin Sep 29, 2020
ebfe310
[clocks] IdealizedPll -> DividerOnlyClockGenerator
davidbiancolin Sep 30, 2020
7d7f7ae
Bump FireSim
davidbiancolin Sep 30, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions generators/chipyard/src/main/resources/vsrc/ClockDividerN.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// See LICENSE for license details.

/**
* An unsynthesizable divide-by-N clock divider.
* Duty cycle is 100 * (ceil(DIV / 2)) / DIV.
*/

module ClockDividerN #(parameter DIV)(output logic clk_out = 1'b0, input clk_in);

localparam DIV_COUNTER_WIDTH = $clog2(DIV);
localparam LOW_CYCLES = DIV / 2;

generate
if (DIV == 1) begin
// This needs to be procedural because of the assignment on declaration
always @(clk_in) begin
clk_out = clk_in;
end
end else begin
reg [DIV_COUNTER_WIDTH - 1: 0] count = '0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this '0 any thing special or just a way to avoid specifying the width?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latter. I'm lazy.

// The blocking assignment to clock out is used to conform what was done
// in RC's clock dividers.
// It should have the effect of preventing registers in the divided clock
// domain latching register updates launched by the fast clock-domain edge
// that occurs at the same simulated time (as the divided clock edge).
always @(posedge clk_in) begin
if (count == (DIV - 1)) begin
clk_out = 1'b0;
count <= '0;
end
else begin
if (count == (LOW_CYCLES - 1)) begin
clk_out = 1'b1;
end
count <= count + 1'b1;
end
end
end
endgenerate
endmodule // ClockDividerN
2 changes: 1 addition & 1 deletion generators/chipyard/src/main/scala/ChipTop.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ChipTop(implicit p: Parameters) extends LazyModule with HasTestHarnessFunc
val lazySystem = LazyModule(p(BuildSystem)(p)).suggestName("system")

// The implicitClockSinkNode provides the implicit clock and reset for the System
val implicitClockSinkNode = ClockSinkNode(Seq(ClockSinkParameters()))
val implicitClockSinkNode = ClockSinkNode(Seq(ClockSinkParameters(name = Some("implicit_clock"))))

// Generate Clocks and Reset
p(ClockingSchemeKey)(this)
Expand Down
109 changes: 36 additions & 73 deletions generators/chipyard/src/main/scala/Clocks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import scala.collection.mutable.{ArrayBuffer}

import freechips.rocketchip.prci._
import freechips.rocketchip.subsystem.{BaseSubsystem, SubsystemDriveAsyncClockGroupsKey}
import freechips.rocketchip.config.{Parameters, Field}
import freechips.rocketchip.config.{Parameters, Field, Config}
import freechips.rocketchip.diplomacy.{OutwardNodeHandle, InModuleBody, LazyModule}
import freechips.rocketchip.util.{ResetCatchAndSync, Pow2ClockDivider}

import barstools.iocell.chisel._

import chipyard.clocking.{IdealizedPLL, ClockGroupNamePrefixer, ClockGroupFrequencySpecifier}

/**
* Chipyard provides three baseline, top-level reset schemes, set using the
* [[GlobalResetSchemeKey]] in a Parameters instance. These are:
Expand Down Expand Up @@ -78,101 +80,62 @@ object GenerateReset {
}


case object ClockingSchemeKey extends Field[ChipTop => Unit](ClockingSchemeGenerators.harnessClock)
case object ClockingSchemeKey extends Field[ChipTop => Unit](ClockingSchemeGenerators.idealizedPLL)
/**
* 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does DFU mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:andrewemoji:

*/
case object ClockFrequencyAssignersKey extends Field[Seq[(String) => Option[Double]]](Seq.empty)
Copy link
Contributor

@jerryz123 jerryz123 Sep 28, 2020

Choose a reason for hiding this comment

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

Why not just make this Map[String, Option[Double]]
Is it to support the partial string matching?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

case object DefaultClockFrequencyKey extends Field[Double](100.0)

class ClockNameMatchesAssignment(name: String, fMHz: Double) extends Config((site, here, up) => {
case ClockFrequencyAssignersKey => up(ClockFrequencyAssignersKey, site) ++
Seq((cName: String) => if (cName == name) Some(fMHz) else None)
})

class ClockNameContainsAssignment(name: String, fMHz: Double) extends Config((site, here, up) => {
case ClockFrequencyAssignersKey => up(ClockFrequencyAssignersKey, site) ++
Seq((cName: String) => if (cName.contains(name)) Some(fMHz) else None)
})

object ClockingSchemeGenerators {
// A simple clock provider, for testing
val harnessClock: ChipTop => Unit = { chiptop =>
val idealizedPLL: ChipTop => Unit = { chiptop =>
implicit val p = chiptop.p

val implicitClockSourceNode = ClockSourceNode(Seq(ClockSourceParameters()))
chiptop.implicitClockSinkNode := implicitClockSourceNode

// Drive the diplomaticclock graph of the DigitalTop (if present)
val simpleClockGroupSourceNode = chiptop.lazySystem match {
case l: BaseSubsystem if (p(SubsystemDriveAsyncClockGroupsKey).isEmpty) => {
val n = ClockGroupSourceNode(Seq(ClockGroupSourceParameters()))
l.asyncClockGroupsNode := n
Some(n)
}
case _ => None
// Requires existence of undriven asyncClockGroups in subsystem
val systemAsyncClockGroup = chiptop.lazySystem match {
case l: BaseSubsystem if (p(SubsystemDriveAsyncClockGroupsKey).isEmpty) =>
l.asyncClockGroupsNode
}

InModuleBody {
//this needs directionality so generateIOFromSignal works
val clock_wire = Wire(Input(Clock()))
val reset_wire = GenerateReset(chiptop, clock_wire)
val (clock_io, clockIOCell) = IOCell.generateIOFromSignal(clock_wire, Some("iocell_clock"))
chiptop.iocells ++= clockIOCell
clock_io.suggestName("clock")
val aggregator = LazyModule(new ClockGroupAggregator("allClocks")).node
chiptop.implicitClockSinkNode := ClockGroup() := aggregator
systemAsyncClockGroup := ClockGroupNamePrefixer() := aggregator

implicitClockSourceNode.out.unzip._1.map { o =>
o.clock := clock_wire
o.reset := reset_wire
}
val referenceClockSource = ClockSourceNode(Seq(ClockSourceParameters()))
(aggregator
:= ClockGroupFrequencySpecifier(p(ClockFrequencyAssignersKey), p(DefaultClockFrequencyKey))
:= IdealizedPLL()
:= referenceClockSource)

simpleClockGroupSourceNode.map { n => n.out.unzip._1.map { out: ClockGroupBundle =>
out.member.data.foreach { o =>
o.clock := clock_wire
o.reset := reset_wire
}
}}

chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => {
clock_io := th.harnessClock
Nil
})
}

}


val harnessDividedClock: ChipTop => Unit = { chiptop =>
implicit val p = chiptop.p

require(false, "Divided clock is broken until we fix passing onchip clocks to TestHarness objects")

val implicitClockSourceNode = ClockSourceNode(Seq(ClockSourceParameters()))
chiptop.implicitClockSinkNode := implicitClockSourceNode

val simpleClockGroupSourceNode = chiptop.lazySystem match {
case l: BaseSubsystem if (p(SubsystemDriveAsyncClockGroupsKey).isEmpty) => {
val n = ClockGroupSourceNode(Seq(ClockGroupSourceParameters()))
l.asyncClockGroupsNode := n
Some(n)
}
case _ => throw new Exception("Harness multiclock assumes BaseSubsystem")
}

InModuleBody {
// this needs directionality so generateIOFromSignal works
val clock_wire = Wire(Input(Clock()))
val reset_wire = GenerateReset(chiptop, clock_wire)
val (clock_io, clockIOCell) = IOCell.generateIOFromSignal(clock_wire, Some("iocell_clock"))
chiptop.iocells ++= clockIOCell
clock_io.suggestName("clock")
val div_clock = Pow2ClockDivider(clock_wire, 2)

implicitClockSourceNode.out.unzip._1.map { o =>
o.clock := div_clock
referenceClockSource.out.unzip._1.map { o =>
o.clock := clock_wire
o.reset := reset_wire
}

simpleClockGroupSourceNode.map { n => n.out.unzip._1.map { out: ClockGroupBundle =>
out.member.elements.map { case (name, data) =>
// This is mega hacks, how are you actually supposed to do this?
data.clock := (if (name.contains("core")) clock_wire else div_clock)
data.reset := reset_wire
}
}}

chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => {
clock_io := th.harnessClock
Nil
})
Nil })
}

}
}
12 changes: 6 additions & 6 deletions generators/chipyard/src/main/scala/ConfigFragments.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ import sifive.blocks.devices.gpio._
import sifive.blocks.devices.uart._
import sifive.blocks.devices.spi._

import chipyard.{BuildTop, BuildSystem, ClockingSchemeGenerators, ClockingSchemeKey, TestSuitesKey, TestSuiteHelper}

import chipyard.{BuildTop, BuildSystem, ClockingSchemeGenerators, ClockingSchemeKey, TestSuitesKey, TestSuiteHelper, ClockNameContainsAssignment}

// Imports for multiclock sketch
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this sketch forth coming? Or is this just dead code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this can deleted.

import boom.common.{BoomTile, BoomTileParams}
import ariane.{ArianeTile, ArianeTileParams}
// -----------------------
// Common Config Fragments
// -----------------------
Expand Down Expand Up @@ -159,14 +161,12 @@ class WithNoSubsystemDrivenClocks extends Config((site, here, up) => {
case SubsystemDriveAsyncClockGroupsKey => None
})

class WithTileDividedClock extends Config((site, here, up) => {
case ClockingSchemeKey => ClockingSchemeGenerators.harnessDividedClock
})

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove empty newline

})
class WithTileFrequency(fMHz: Double) extends ClockNameContainsAssignment("core", fMHz)
23 changes: 23 additions & 0 deletions generators/chipyard/src/main/scala/clocking/ClockDividerN.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// See LICENSE.SiFive for license details.

package chipyard.clocking

import chisel3._
import chisel3.util._

class ClockDividerN(div: Int) extends BlackBox(Map("DIV" -> div)) with HasBlackBoxResource {
require(div > 0);
val io = IO(new Bundle {
val clk_out = Output(Clock())
val clk_in = Input(Clock())
})
addResource("/vsrc/ClockDividerN.sv")
}

object ClockDivideByN {
def apply(clockIn: Clock, div: Int): Clock = {
val clockDivider = Module(new ClockDividerN(div))
clockDivider.io.clk_in := clockIn
clockDivider.io.clk_out
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package chipyard.clocking

import chisel3._

import freechips.rocketchip.config.{Parameters}
import freechips.rocketchip.diplomacy._
import freechips.rocketchip.prci._

/**
* This sort of node can be used when it is a connectivity passthrough, but modifies
* the flow of parameters (which may result in changing the names of the underlying signals).
*/
class ClockGroupParameterModifier(
sourceFn: ClockGroupSourceParameters => ClockGroupSourceParameters = { m => m },
sinkFn: ClockGroupSinkParameters => ClockGroupSinkParameters = { s => s })(
implicit p: Parameters, v: ValName) extends LazyModule {
val node = ClockGroupAdapterNode(sourceFn, sinkFn)
lazy val module = new LazyRawModuleImp(this) {
(node.out zip node.in).map { case ((o, _), (i, _)) =>
(o.member.data zip i.member.data).foreach { case (oD, iD) => oD := iD }
}
}
}

/**
* Pushes the ClockGroup's name into each member's name field as a prefix. This is
* intended to be used before a ClockGroupAggregator so that sources from
* different aggregated ClockGroups can be disambiguated by their names.
*/
object ClockGroupNamePrefixer {
def apply()(implicit p: Parameters, valName: ValName): ClockGroupAdapterNode =
LazyModule(new ClockGroupParameterModifier(sinkFn = { s => s.copy(members = s.members.zipWithIndex.map { case (m, idx) =>
m.copy(name = m.name match {
// This matches what the chisel would do if the names were not modified
case Some(clockName) => Some(s"${s.name}_${clockName}")
case None => Some(s"${s.name}_${idx}")
})
})})).node
}

/**
* [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]
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

* to each sink. Later functions that return non-empty values take priority.
* The default if all functions return None.
*/
object ClockGroupFrequencySpecifier {
def apply(
assigners: Seq[(String) => Option[Double]],
defaultFreq: Double)(
implicit p: Parameters, valName: ValName): ClockGroupAdapterNode = {

def lookupFrequencyForName(clock: ClockSinkParameters): ClockSinkParameters = {
require(clock.name.nonEmpty, "All clocks in clock group must have an assigned name")
val clockFreq = assigners.foldLeft(defaultFreq)(
(currentFreq, candidateFunc) => candidateFunc(clock.name.get).getOrElse(currentFreq))

clock.copy(take = clock.take.map(_.copy(freqMHz = clockFreq)).orElse(Some(ClockParameters(clockFreq))))
}

LazyModule(new ClockGroupParameterModifier(sinkFn = { s => s.copy(members = s.members.map(lookupFrequencyForName)) })).node
}
}
Loading