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

fpga: Add VCU128 flow and other IPs #54

Closed
wants to merge 2 commits into from
Closed

fpga: Add VCU128 flow and other IPs #54

wants to merge 2 commits into from

Conversation

CyrilKoe
Copy link
Contributor

@CyrilKoe CyrilKoe commented Jul 14, 2023 •
edited
Loading

This PR goes with the MR 7

Changes :

  • Added flow for VCU128
  • Added flow for ZCU102 (not tested with Linux due to lack impossibility to access SPI/SD)
  • Added STARTUPE3 spi macro for VCU128
  • Added dram_wrapper for DDR3 / DDR4
  • Removed xilinx/Makefile and added xilinx/xilinx.mk
  • Added ILAs support in run.tcl
  • Added device trees per boards
  • Added simulation flow for Xilinx IPs (simulate.mk)
  • Added clk_wiz , vios and ddr4 IPs
  • Added CI for VCU128
  • Added device tree per board (thus Linux image per board)

Status :

  • Genesys2 boots
  • VCU128 boots

@CyrilKoe CyrilKoe requested review from paulsc96 and alex96295 July 14, 2023 15:57
@paulsc96 paulsc96 marked this pull request as draft July 14, 2023 20:22
@paulsc96 paulsc96 changed the title [Draft] fpga: Added VCU128 flow and other IPs fpga: Add VCU128 flow and other IPs Jul 14, 2023
@CyrilKoe CyrilKoe force-pushed the ck/pr1 branch 9 times, most recently from 0e9abf9 to 429e3fe Compare July 18, 2023 08:41
@CyrilKoe CyrilKoe force-pushed the ck/pr1 branch 15 times, most recently from bdea74c to 04b39fe Compare July 28, 2023 10:55
@CyrilKoe CyrilKoe force-pushed the ck/pr1 branch 4 times, most recently from 47db445 to e9ee56b Compare September 4, 2023 00:44
@CyrilKoe CyrilKoe force-pushed the ck/pr1 branch 6 times, most recently from f8a486a to be27e65 Compare September 8, 2023 08:09
@CyrilKoe
Copy link
Contributor Author

CyrilKoe commented Sep 8, 2023 •
edited
Loading

@alex96295 @paulsc96 All good,

  • ID serialize has been added in the dram wrapper.
  • A little artifacts script to fetch pre-compiled IPs with the same environment variables & compile scripts.
  • Timing is met@50MHz appart from the sd_cmd (same as in master), now the CVA6 paths are met too (the fact to not have a clk_divider but directly the output of a MMCM probably helps) even if large routing overhead on Genesys2 will probably soon lead to a negative slack.

Ready for review, remember to check the CI changes as well.

Both genesys2 and VCU128 boots (note I uploaded a new image on the SD card that supports booting both from SD card and SPI nor)

Pipeline running here

paulsc96
paulsc96 requested changes Sep 19, 2023
View reviewed changes
Copy link
Collaborator

@paulsc96 paulsc96 left a comment

Choose a reason for hiding this comment

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

As previously discussed, the most important outstanding changes are to:

  • Rebase onto main and resolve conflicts
  • Extend the "Xilinx FPGAs" Documentation chapter appropriately (no need to extend the README)
  • Some overall cleanup

Bender.yml Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
cheshire.mk Show resolved Hide resolved
cheshire.mk Outdated Show resolved Hide resolved
sw/include/params.h Show resolved Hide resolved
target/xilinx/sim/simulate.mk Outdated Show resolved Hide resolved
target/xilinx/src/cheshire_top_xilinx.sv Show resolved Hide resolved
target/xilinx/src/dram_wrapper.sv Outdated Show resolved Hide resolved
target/xilinx/constraints/genesys2.xdc Outdated Show resolved Hide resolved
target/xilinx/constraints/genesys2.xdc Outdated Show resolved Hide resolved
@CyrilKoe CyrilKoe force-pushed the ck/pr1 branch 3 times, most recently from 5bbbe25 to 886a1f6 Compare October 3, 2023 12:03
@CyrilKoe
Copy link
Contributor Author

CyrilKoe commented Oct 3, 2023

@paulsc96 requested changes applied

paulsc96
paulsc96 reviewed Oct 3, 2023
View reviewed changes
sw/sw.mk Outdated
@@ -140,7 +140,7 @@ $(foreach link,$(patsubst $(CHS_SW_LD_DIR)/%.ld,%,$(wildcard $(CHS_SW_LD_DIR)/*.
CHS_CVA6_SDK_IMGS ?= $(addprefix $(CHS_SW_DIR)/deps/cva6-sdk/install64/,fw_payload.bin uImage)

# Create full Linux disk image
$(CHS_SW_DIR)/boot/linux.gpt.bin: $(CHS_SW_DIR)/boot/zsl.rom.bin $(CHS_SW_DIR)/boot/cheshire.dtb $(CHS_CVA6_SDK_IMGS)
$(CHS_SW_DIR)/boot/linux-${BOARD}.gpt.bin: $(CHS_SW_DIR)/boot/zsl.rom.bin $(CHS_SW_DIR)/boot/cheshire_$(BOARD).dtb $(CHS_CVA6_SDK_IMGS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry I only noticed this now, but please don't do this. Making the name/availability of targets depend on variables is bad design and breaks things built on top of it. I tolerated this in the Xilinx Makefiles, but I don't want this in any other Makefiles. Use an appropriate pattern rule instead, e.g:

$(CHS_SW_DIR)/boot/linux-%.gpt.bin: $(CHS_SW_DIR)/boot/zsl.rom.bin $(CHS_SW_DIR)/boot/cheshire_%.dtb $(CHS_CVA6_SDK_IMGS)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, device trees are not an FPGA-specific thing, and neither is Cheshire. The whole point of Cheshire that it is silicon-ready and not another FPGA-only platform. Device trees should be differentiated by generic targets (including ASICs), not specifically FPGA boards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes absolutely thank you!

For the board, this (BOARD)[https://github.com/pulp-platform/cheshire/blob/ck/pr1/cheshire.mk#L15] variable should define more a development board, which is not only about the FPGA development board but also future PCBs.
So maybe the .dtsi should stay where it is but the .dts can be moved to the target/fpga. Just that it will be more annoying for Makefiles.
Anyways I suppose that a chip instantiating Cheshire will need to re-define this target like I do in Carfield

@CyrilKoe
Copy link
Contributor Author

CyrilKoe commented Oct 11, 2023 •
edited
Loading

PR updated Pipeline running

@CyrilKoe CyrilKoe force-pushed the ck/pr1 branch 2 times, most recently from 56eddcd to 310c66d Compare October 11, 2023 19:17
@CyrilKoe
Copy link
Contributor Author

Changes updated, ready to be reviewed @paulsc96 @alex96295

alex96295
alex96295 reviewed Nov 22, 2023
View reviewed changes
target/xilinx/src/dram_wrapper_xilinx.sv
// Instianciate data width resizer //
/////////////////////////////////////

if (cfg.DataWidth != SoC_DataWidth) begin : gen_dw_converter
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, as in carfield, remove

alex96295
alex96295 reviewed Nov 22, 2023
View reviewed changes
target/xilinx/src/dram_wrapper_xilinx.sv
// ID resizer //
/////////////////

if (cfg.IdWidth != SoC_IdWidth) begin : gen_iw_converter
Copy link
Collaborator

Choose a reason for hiding this comment

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

DITTO

@CyrilKoe
fpga: Reworked fully FPGA flow, added support and CI for VCU128, adde…
1574d2c 
…d Vivado IP simulation flow

fpga: Added ddr4 and vcu128 flow, added draft of Vivado IP simulation flow

fpga: Added VIOs

Connect VIO-generated reset signal to dram wrapper

fpga: Support of zcu102

fpga: zcu102.xdc constraint file added

fpga: zcu102 changed phy and added firsts constraints

fpga: Switching to clk_wiz and xilinx.mk

fpga: Testing ddr4

fpga: Start working on SPI driver
Co-Authored-By: Yann Picod <[email protected]>

fpga: SD card test

fpga: Rolled-back SD

fpga: Add vcu128 ci

fpga: Debug new CI

fpga: Adding artifacts management

fpga: Added ID serializer in dram_wrapper and changed a few xdc constraints

fpga: Correcting artifacts mngmt and applied new constraints to genesys2

fpga: Corrected vivado sim script

fpga: Last review updates

fpga: Last review updates 2

fpga: Updated artifact management to take select only useful envvar

fpga: Update cva6-sdk, add openocd configs, and update docs

fpga: Use https for cva6-sdk
@CyrilKoe
fpga: Removed a few phonies/variable defined targets
ff72904
@CyrilKoe CyrilKoe closed this Jan 25, 2024
@paulsc96 paulsc96 mentioned this pull request Apr 18, 2024
Closed
Aquaticfuller pushed a commit that referenced this pull request Jul 16, 2024
@alex96295
Merge pull request #54 from pulp-platform/fix-elab
ca30aa9 
Various minor fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alex96295 alex96295 alex96295 left review comments

@paulsc96 paulsc96 paulsc96 requested changes

Assignees

@paulsc96 paulsc96

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@CyrilKoe @alex96295 @paulsc96