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

Scratchpad Config #383

Merged
merged 1 commit into from
Jan 22, 2020
Merged

Scratchpad Config #383

merged 1 commit into from
Jan 22, 2020

Conversation

abejgonzalez
Copy link
Contributor

@abejgonzalez abejgonzalez commented Jan 17, 2020

Fixes #381.

Adds a configuration that removes off-chip memory interface in favor of a configurable scratchpad hanging off the mbus. This builds Verilog but I haven't tested any code with this.

Based on #347 so this must go in after that. (Ill change the base branch when the other PR is done).

@abejgonzalez abejgonzalez self-assigned this Jan 17, 2020
@abejgonzalez abejgonzalez changed the base branch from dev to better_configs January 17, 2020 22:38
@jerryz123
Copy link
Contributor

Can we make this stuff belong in testchipip instead? 1) it reduces code bloat in Chipyard core. 2) a backing scratchpad feels like a "testchip" feature.

@abejgonzalez
Copy link
Contributor Author

Can we make this stuff belong in testchipip instead? 1) it reduces code bloat in Chipyard core. 2) a backing scratchpad feels like a "testchip" feature.

None of this code is "new" per-se... it is just using Rocket Chip. IMO testchipip is a collection of testing IP. This doesn't really fit under that umbrella.

@jerryz123
Copy link
Contributor

Even if all the components are Rocket Chip, it adds a new Field and a new Trait. The guidelines for Keys/Traits in #347 specified that keys and traits should be defined in submodules, since they deal with custom modifications that not all people will use.

The only exception to this rule is for the example widgets.

Copy link
Contributor

@jwright6323 jwright6323 left a comment

Choose a reason for hiding this comment

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

Looks fine- how did you test it?

@jerryz123 I think this should go here. I think you might be able to make an argument that the stuff in TopCakes could go into testchipip, but it's so small I think it's overkill. The rest obviously has to go in chipyard.

@abejgonzalez
Copy link
Contributor Author

While I agree that TopCakes.scala is a bit small for now (and seems excessive), I think there should be a place for pure RC changes (that require little to no "extra" code). As John said, this can live in testchipip but I think it is better to have it in example.

As for the testing, I just finished running the asm/bmark tests and they passed.

@jerryz123 jerryz123 force-pushed the better_configs branch 7 times, most recently from 9a44f6c to cfccf1c Compare January 22, 2020 00:13
Copy link
Contributor

@jwright6323 jwright6323 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 changed the base branch from better_configs to dev January 22, 2020 04:45
@abejgonzalez abejgonzalez merged commit 5358d29 into dev Jan 22, 2020
@abejgonzalez abejgonzalez deleted the scratchpad-config branch January 22, 2020 17:08
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