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

Make Carfield domains configurable on-demand #245

Merged
merged 49 commits into from
Mar 2, 2024
Merged

Make Carfield domains configurable on-demand #245

merged 49 commits into from
Mar 2, 2024

Conversation

yvantor
Copy link
Contributor

@yvantor yvantor commented Dec 30, 2023

Make Carfield domains configurable on-demand: domains can be enabled through a dedicated configuration package under hw/configs

@yvantor yvantor self-assigned this Jan 2, 2024
@yvantor yvantor added the enhancement New feature or request label Jan 2, 2024
@yvantor yvantor changed the title Draft: create parametric top. Create parametric top. Jan 21, 2024
@yvantor
Copy link
Contributor Author

yvantor commented Jan 21, 2024

This is not complete yet, but is ready to start reviewing it.
The principle is that of allowing multiple configuration packages for enabling/disabling certain islands and defining their memory map. It is based on how it is done in CVA6 to allow different core configurations.

What is missing:

  • the top-level parametrization is done only for resizing the AXI xbar. This should be done for the RegBus as well.
  • allow for usage of multiple clusters of the same spieces (e.g. 2 PULP clusters).

Issues encountered:

  • The code for the Spatz Cluster is still completely Python-generated... This means that if we disable one of the available islands, Questasim will complain at compile time because the AXI ID widths of the Cheshire configuration will no longer match with the ones defined in the Spatz Cluster configuration script. In my honest opinion, we could directly use parametric wrappers instead of Python scripts for System Verilog code generation...
  • The PULP cluster has a dedicated AXI ports due to the need for AXI ID serializers. This is blocking for the top-level parametrization, so I am considering unifying the ID width in the Cheshire configuration parameter to match the PULP cluster one and remove the need for dedicated master/slave ports.
  • It is not clear to me why the Mailbox also has dedicated AXI ports. I hope it is not for the same reasons as the point above.

@alex96295 If you have some opinions regarding the code organizations and the perplexities I left above (or if you have more), please let me know.
Of course, opinions from others (@anga93, @micprog, @bluewww, for instance, also @CyrilKoe) are more than welcome!

@alex96295
Copy link
Collaborator

@yvantor thanks for the work! I will have a look in the next days and come back at you :)

@micprog micprog self-requested a review January 23, 2024 15:12
@bluewww bluewww self-requested a review February 1, 2024 22:02
@yvantor yvantor force-pushed the yt/cfg_top branch 4 times, most recently from 0d1e044 to e332604 Compare February 6, 2024 00:05
tb/carfield_fix.sv Outdated Show resolved Hide resolved
tb/carfield_fix.sv Outdated Show resolved Hide resolved
tb/carfield_fix.sv Outdated Show resolved Hide resolved
hw/carfield_cfg_pkg.sv Outdated Show resolved Hide resolved
Bender.yml Outdated Show resolved Hide resolved
@yvantor
Copy link
Contributor Author

yvantor commented Feb 7, 2024

@alex96295 thank you for the review, I will address the comments!

@yvantor
Copy link
Contributor Author

yvantor commented Feb 7, 2024

@alex96295 thank you for the review, I will address the comments!

@alex96295 I should have addressed all the points. There is also a PR open in the nonfree for CI updates.

@alex96295 alex96295 changed the title Create parametric top. Make Carfield domains configurable Feb 12, 2024
@alex96295 alex96295 changed the title Make Carfield domains configurable Make Carfield domains configurable on-demand Feb 12, 2024
carfield.mk Outdated Show resolved Hide resolved
hw/carfield_cfg_pkg.sv Outdated Show resolved Hide resolved
hw/carfield_cfg_pkg.sv Outdated Show resolved Hide resolved
target/sim/sim.mk Outdated Show resolved Hide resolved
target/sim/sim.mk Outdated Show resolved Hide resolved
hw/cheshire_wrap.sv Outdated Show resolved Hide resolved
Yvan Tortorella and others added 18 commits March 1, 2024 15:16
…r and slave devices

and use slave IDs structure to compute the memory map.
Using /tmp to store temporary files may cause reusing scripts generated by previous runs
xilinx: Change makefile for new carfield config parameter

xilinx: Use islands tcl constraints script

xilinx: Mutualize constraints between bd and vanilla

xilinx: Remove now unecessary overrides script

Appropriate constraints on tc_clk_mux makes overriding them unecessary

ci: Remove temporary bender fix, and auto select fpga boards in CI

xilinx: Grab rtc_clk in constraints via its net
@yvantor
Copy link
Contributor Author

yvantor commented Mar 1, 2024

@alex96295 as soon as the CI completes we can squash and merge.

@yvantor
Copy link
Contributor Author

yvantor commented Mar 1, 2024

Forgot to update the non-free rebasing it...

@yvantor
Copy link
Contributor Author

yvantor commented Mar 2, 2024

Forgot to update the non-free rebasing it...

Solved, it is good to go.

@alex96295
Copy link
Collaborator

LGTM, merging

@alex96295 alex96295 merged commit 6adf641 into main Mar 2, 2024
7 checks passed
@alex96295 alex96295 deleted the yt/cfg_top branch March 2, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants