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.

Sign up for GitHub

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

Jump to bottom

Add hamsternz display port #60

Merged
merged 2 commits into from
Oct 14, 2020
Merged

Add hamsternz display port #60

merged 2 commits into from
Oct 14, 2020

Conversation

wtatarski
Copy link
Contributor

@wtatarski wtatarski commented Apr 14, 2020 •
edited
Loading

Fixes #49

@wtatarski wtatarski changed the title Add hamsternz display port Add hamsternz display port Fixes #49 Apr 14, 2020
@wtatarski wtatarski changed the title Add hamsternz display port Fixes #49 Add hamsternz display port Apr 14, 2020
@mithro
Copy link
Collaborator

mithro commented Apr 14, 2020 •
edited by unfurl-links bot
Loading

This looks like a pretty great test case. Can you also update the doc at https://docs.google.com/document/d/16L50pyS3RjYStvRKRoWyVac7NoZKyu45fpPQXJqeKYo/edit ?

Google Docs
SymbiFlow FPGA Tool Performance (for Xilinx Devices) https://github.com/SymbiFlow/fpga-tool-perf Modes Fixed fmax -- Looking at run time and memory usage Find fastest fmax -- Try and find the fastest possible Toolchains Synthesis Place and Route Vivado Yosys Vivado Yosys ...

@mithro mithro requested review from kgugala and acomodi April 14, 2020 14:31
@mithro
Copy link
Collaborator

mithro commented Apr 14, 2020

All third party code should be under the third_party directory. Can we just use a git submodule here?

@wtatarski wtatarski force-pushed the add-hamsternz-display-port branch 2 times, most recently from baf8436 to 1ca7669 Compare April 15, 2020 10:15
@wtatarski
Copy link
Contributor Author

Added hamsternz-displayport as submodule.

acomodi
acomodi approved these changes Apr 16, 2020
View reviewed changes
Copy link
Contributor

@acomodi acomodi left a comment

Choose a reason for hiding this comment

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

LGTM

@mithro
Copy link
Collaborator

mithro commented Apr 16, 2020

@acomodi - Just wanted to make sure that you are checking these imports comply with the standards set out in f4pga/ideas#47 !
(PS We probably should have an automated system which checks this....)

@acomodi
Copy link
Contributor

acomodi commented Apr 16, 2020

@mithro I am actually unsure how to treat the submodules regarding licensing. The hamsternz display port is unmodified and there are no modifications on the source files. For submodules we have no control over their license and the README.fpga-tool-perf should not be needed.

@wtatarski wtatarski force-pushed the add-hamsternz-display-port branch 5 times, most recently from ff8b16c to 294ca32 Compare April 30, 2020 11:24
@kamilrakoczy kamilrakoczy force-pushed the add-hamsternz-display-port branch from 294ca32 to 44e96d9 Compare October 12, 2020 14:32
@kamilrakoczy kamilrakoczy requested a review from acomodi October 13, 2020 06:45
acomodi
acomodi suggested changes Oct 13, 2020
View reviewed changes
Copy link
Contributor

@acomodi acomodi left a comment

Choose a reason for hiding this comment

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

Comment in

src/hamsternz-displayport/constr/vivado-yosys/xc7_a200t_sbg484-1_nexys.xdc Outdated
@@ -0,0 +1,73 @@
##Clock Signal
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be required anymore

Copy link
Collaborator

Choose a reason for hiding this comment

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

Removed.

@kamilrakoczy kamilrakoczy force-pushed the add-hamsternz-display-port branch from 44e96d9 to 779ff7a Compare October 13, 2020 13:58
@kamilrakoczy kamilrakoczy requested a review from acomodi October 14, 2020 06:05
acomodi
acomodi approved these changes Oct 14, 2020
View reviewed changes
Copy link
Contributor

@acomodi acomodi left a comment

Choose a reason for hiding this comment

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

LGTM

@acomodi acomodi merged commit f8e946c into chipsalliance:master Oct 14, 2020
@umarcor umarcor deleted the add-hamsternz-display-port branch April 21, 2022 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kamilrakoczy kamilrakoczy kamilrakoczy left review comments

@acomodi acomodi acomodi approved these changes

@kgugala kgugala Awaiting requested review from kgugala

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add hamsternz's DisplayPort core example
5 participants
@wtatarski @mithro @acomodi @kamilrakoczy @kokoro-team