chipyard icon indicating copy to clipboard operation
chipyard copied to clipboard

DividerOnlyClockGenerator improve naming

Open timsnyder-siv opened this issue 4 years ago • 4 comments

https://github.com/ucb-bar/chipyard/blob/255452f20af2eac4bbcf4888d6252b2260888b85/generators/chipyard/src/main/scala/clocking/DividerOnlyClockGenerator.scala#L89

@davidbiancolin, the ${name} in the code above has spaces in it. Should it be sanitized to swap them out for underscores?

Also, we receive output that looks like:

FireSim RationalClockBridge Frequency Summary
  Input Reference Frequency: 16000.0 MHz
  Output clock implicit_clock, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)
  Output clock subsystem_mbus_address_adjuster_0, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)
  Output clock subsystem_mbus_address_adjuster_1, requested: 1000.0 MHz, actual: 1000.0 MHz (division of 16)
  Output clock subsystem_sbus_0, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)
  Output clock subsystem_sbus_core_0, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)
  Output clock subsystem_sbus_2, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)
  Output clock subsystem_sbus_3, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)
  Output clock subsystem_sbus_4, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)
  Output clock subsystem_sbus_5, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)
  Output clock subsystem_sbus_6, requested: 3200.0 MHz, actual: 3200.0 MHz (division of 5)

how do I translate subsystem_sbus_X into the actual thing that is being driven? For example, I how do I know which of the indices is driving the mbus vs the pbus? Can this naming be improved somehow to be more descriptive of what the clock is eventually going to? Will the naming automagially improve if the naming of the clockSinkNodes improve because that's how the names are generated in: https://github.com/ucb-bar/chipyard/blob/255452f20af2eac4bbcf4888d6252b2260888b85/generators/chipyard/src/main/scala/clocking/ClockGroupNamePrefixer.scala#L30-L39

I know you don't use it but it would be useful to me if the graphml output also included the negotiated frequency information in it's meta-data or perhaps on the clock edges (similar to how the data edges show their width).

timsnyder-siv avatar Feb 06 '21 02:02 timsnyder-siv

Right, naming... So in our Subsystem we've made it so all the buses get useful names by doing this:

https://github.com/ucb-bar/chipyard/blob/master/generators/chipyard/src/main/scala/Subsystem.scala#L60-L75

The comment there should better explain what's happening. Basically if you want useful names you first need to provide one in the clock sink, and then also ensure it's not lost in the graph.

Go ahead and sanitize the name. Perhaps a more sane thing to do is to use the name of the instance for which this summary is being generated, though that's not strictly necessary with the CB because there can only be one. That would already sanitized.

davidbiancolin avatar Feb 06 '21 02:02 davidbiancolin

Thanks for the pointer to getting useful names. I had a feeling it was something missing in how I've integrated things.

timsnyder-siv avatar Feb 06 '21 02:02 timsnyder-siv

What negotiated freq information are you referring to? Do diplomatic parameters get rendered in the graph?

davidbiancolin avatar Feb 06 '21 02:02 davidbiancolin

The widths of datapaths are rendered in the graph. I see edges for clocks that don't currently have a label. Since they are always width 1, perhaps labeling them with the frequency would be interesting & useful?

timsnyder-siv avatar Feb 06 '21 02:02 timsnyder-siv