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

Add support for Digilent Genesys 2 board #784

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

yqszxx
Copy link

@yqszxx yqszxx commented Feb 2, 2021

Depends on sifive/fpga-shells#159

Related issue: N/A

Type of change: new feature

Impact: new fpga target

Release Notes

Add support for Digilent Genesys 2 board

@yqszxx yqszxx changed the base branch from master to dev February 2, 2021 12:06
import freechips.rocketchip.util.{HeterogeneousBag, PSDTestMode}
import sifive.blocks.devices.uart.HasPeripheryUARTModuleImp

class WithUARTIOPassthrough extends OverrideIOBinder({
Copy link
Contributor

Choose a reason for hiding this comment

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

This code seems like a copy of the VCU118 code. Maybe we can dedup this by creating a folder like: fpga/src/main/scala/common and putting both sources there (with a new package name chipyard.fpga.common). Then we just reference that.

Copy link
Author

Choose a reason for hiding this comment

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

Good idea, I noticed I was just copying and pasting code when porting too 😂. Should I try to do some dedup work now or after this PR get merged?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would wait until we get back to you on the testing side and fpga-shells gets merged.

@abejgonzalez
Copy link
Contributor

Awesome, thanks so much for the contribution! I added one comment to maybe reduce the amount of duplication between this and the VCU118 work. I think before we consider merging this into Chipyard, you should try to get your PR merged into fpga-shells.

With that being said, the Chipyard team is discussing how best to deal with a PR like this... mainly because we have no mechanisms for testing that this works on the Genesys2 board without having one ourselves. We will get back to you on that once we have a decision.

@yqszxx
Copy link
Author

yqszxx commented Feb 9, 2021

Awesome, thanks so much for the contribution! I added one comment to maybe reduce the amount of duplication between this and the VCU118 work. I think before we consider merging this into Chipyard, you should try to get your PR merged into fpga-shells.

With that being said, the Chipyard team is discussing how best to deal with a PR like this... mainly because we have no mechanisms for testing that this works on the Genesys2 board without having one ourselves. We will get back to you on that once we have a decision.

Testing is really a problem, looking forward to your discussion results~

@jerryz123
Copy link
Contributor

Do we need the actual boards to do testing? Wouldn't it be sufficient to test that Vivado can elaborate the design?

It may be a unreasonable goal for us to maintain regressions for many FPGA boards anyways. It may be better to have the board support checked in anyways, with the caveat that it might require more tinkering to make work compared to other flows.

@abejgonzalez
Copy link
Contributor

Do we need the actual boards to do testing? Wouldn't it be sufficient to test that Vivado can elaborate the design?

It may be a unreasonable goal for us to maintain regressions for many FPGA boards anyways. It may be better to have the board support checked in anyways, with the caveat that it might require more tinkering to make work compared to other flows.

Correct, we want to avoid the need for having a board. However, we want to have some sort of simulation flow setup that can run the tests with FPGA collateral provided by Xilinx (through Vivado etc). An example of this would be https://github.com/ucb-bar/chipyard/tree/arty-sim/fpga or FPGA-level sim in FireSim

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.

None yet

4 participants