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

Heterogeneous doc patches #796

Merged
merged 3 commits into from
Feb 12, 2021
Merged

Conversation

tymcauley
Copy link
Contributor

@tymcauley tymcauley commented Feb 11, 2021

Related issue: #786

Type of change: documentation

Impact: documentation

Release Notes
Fix documentation for Heterogeneous SoCs, add documentation on hart ID ordering.

There were two problems here:
    1. The docs indicated that this should be a dual-BOOM and
       single-Rocket config, but the config actually had two Rocket
       cores.
    2. Since the doc include string was 'DualBoomAndRocket', it was
       accidentally matching against the 'DualBoomAndRocketOneHwacha'
       section, which comes first in the file.

So, I created a new 'DualLargeBoomAndSingleRocketConfig' config which
only has one Rocket core, and changed the doc include string to
'DualBoomAndSingleRocket'.
@jerryz123
Copy link
Contributor

The ordering is sequential from bottom up (or right to left), following the behavior of other config fragments.
In other words

WithNLargeBooms(2) ++
WithNBigCores(1)

Generates
hart0: Rocket
hart1/2: LargeBoom

@tymcauley
Copy link
Contributor Author

Great, thanks @jerryz123 ! In that case, looking at this config:

class LargeBoomAndHwachaRocketConfig extends Config(
  new chipyard.config.WithMultiRoCC ++                                  // support heterogeneous rocc
  new chipyard.config.WithMultiRoCCHwacha(1) ++                         // put hwacha on hart-1 (rocket)
  new hwacha.DefaultHwachaConfig ++                                     // set default hwacha config keys
  new boom.common.WithNLargeBooms(1) ++                                 // add 1 boom core
  new freechips.rocketchip.subsystem.WithNBigCores(1) ++                // add 1 rocket core
  new chipyard.config.AbstractConfig)

the rocket core should be hart 0, and the boom core hart 1. That means we should change WithMultiRoCCHwacha(1) to WithMultiRoCCHwacha(0) if we want the Hwacha connected to the Rocket core, correct?

@jerryz123
Copy link
Contributor

Oh yeah, that comment is wrong. That should be WithMultiRoCCHwacha(0) to put it on Rocket. Thanks for the catch.

The docs indicate that this should be a dual-BOOM and single-Rocket
config, with the Hwacha attached to the Rocket. However, the
'LargeBoomAndHwachaRocketConfig' config only has a single Rocket core.
Added the 'DualLargeBoomAndHwachaRocketConfig' config to accurately
reflect what's stated in the docs.

Additionally, this fixes hart numbering to place the Hwacha accelerator
on the Rocket core rather than on the BOOM core.
@tymcauley tymcauley force-pushed the heterogeneous-doc-patches branch from 1d57c44 to 01948f6 Compare February 12, 2021 00:05
@tymcauley tymcauley changed the title WIP: Heterogeneous doc patches Heterogeneous doc patches Feb 12, 2021
@tymcauley
Copy link
Contributor Author

Alright, I fixed that issue, and I also added some text at the bottom of that page explaining hart ID ordering in a bit more detail.

@tymcauley
Copy link
Contributor Author

Alright, I'm happy with this PR, let me know if you'd like me to change anything.

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.

Thanks for the fixes.

@jerryz123 jerryz123 merged commit 54039f6 into ucb-bar:dev Feb 12, 2021
Copy link
Contributor

@alonamid alonamid left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for these corrections!

@tymcauley tymcauley deleted the heterogeneous-doc-patches branch February 15, 2021 19:26
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.

3 participants