target/xilinx: Add block design flow

[CyrilKoe]

The ethernet IP is quite complex and cannot be implemented by hand on FPGA, i.e. by instantiating Xilinx IP within the source files in System Verilog.

To solve this issue, we add two bitstream flavors:

• Vanilla: IPs integrated by hand
• BD (block design): IPs integrated by block design

---

Summary of changes

• The flow has gone through major changes to allow adding new flavor or boards easily.
• Safety island was bumped to change default burst mode (for compatibility with xilinx ips)
• A PCIe IP has been added but not tested yet.
• CI have been adapted to the new fpga flow

[CyrilKoe]

The readme went though major changes due to automatic scripts of mkdocs, the removed lines are not lost but are now in the docs

[alex96295]

@CyrilKoe, thanks for the PR, I will have a look at it ASAP (note: you have draft in the title, but the PR is opened in 'ready for review' state: could you convert it as draft PR in case it is not ready please?)

Second: an open PR #237 is already prepared to add documentation (@anga93 will handle it soon). Could you please move your documentation there, since it is orthogonal to the technical changes of this PR, which will be merged first?

[CyrilKoe]

@alex96295 Docs were removed, I will remove the draft after CI finishes

[CyrilKoe]

Ready to be reviewed @alex96295

[alex96295]

@CyrilKoe thanks for the great work

I went through it and only have minor comments that you can find in this PR. Before we merge, I also have an observation. I think the current directory structure inside the target/ folder could be improved:

• The two flavors vanilla and bd are not currently reflected in the directories names; I suggested as an alternative to rename xilinx/ in xilinx_vanilla/ and then add both xilinx_vanilla/ and xilinx_bd/ under a xilinx/ directory within target/ (see suggested tree below)
• The readme inside the current xilinx/ directory could be removed, or moved to the top-level xilinx/ folder if you apply the suggestions in the step above, pointing to the documentation
• For both flavors, we rely on ooc implementation of IPs: in the vanilla design, the various xilinx wizards; in the block design, carfield itself. So I would keep symmetry with an additional ips/ folder where the ooc designs are kept, for both favors

Here follows a tree with what I had in mind for the directory structure (plus comments marked with #)

target
├── synth
│   └── carfield_synth_wrap.sv
└── xilinx # wrap xilinx_bd/ and xilinx_vanilla in xilinx top directory
    ├── xilinx_bd
    │   ├── constraints # keep ooc carfield constraints at top since they are typically reused
    │   │   ├── carfield_ip.xdc
    │   │   ├── ooc_carfield_ip.xdc
    │   │   ├── vcu128.xdc
    │   │   └── zcu102.xdc
    │   ├── ips # add carfield in a ips/ folder as ooc IP
    │   │   └── xlnx_carfield
    │   │       ├── scripts
    │   │       │   └── run.tcl # script is renamed run.tcl since the tree structure makes you understand where you are already (i.e., implementing carfield ooc)
    │   │       └── src
    │   │           ├── carfield_xilinx_ip.v
    │   │           └── carfield_xilinx.sv
    │   ├── scripts
    │   │   ├── carfield_bd_vcu128.tcl
    │   │   └── run.tcl
    │   └── xilinx_bd.mk
    └── xilinx_vanilla
        ├── constraints
        │   ├── carfield.xdc
        │   ├── vcu128.xdc
        │   └── zcu102.xdc
        ├── ips # renamed 'xilinx/' to 'ips/' as discussed above 
        │   ├── common.mk
        │   ├── xlnx_clk_wiz
        │   │   ├── Makefile
        │   │   └── tcl
        │   │       └── run.tcl
        │   ├── xlnx_mig_ddr4
        │   │   ├── Makefile
        │   │   └── tcl
        │   │       └── run.tcl
        │   └── xlnx_vio
        │       ├── Makefile
        │       └── tcl
        │           └── run.tcl
        ├── scripts
        │   ├── flash_spi.tcl
        │   ├── overrides.sh
        │   ├── program.tcl
        │   ├── prologue.tcl
        │   ├── run.tcl
        │   └── write_cfgmem.tcl
        ├── src
        │   ├── carfield_top_xilinx.sv
        │   ├── cdc_dst_axi_err.sv
        │   ├── dram_wrapper.sv
        │   ├── fan_ctrl.sv
        │   ├── overrides
        │   │   └── tc_clk_xilinx.sv
        │   └── phy_definitions.svh
        └── xilinx_vanilla.mk

Could you please have a look at it and provide your opinion? :)

[CyrilKoe]

@alex96295 thanks for the review and the feedback, about the directory structure here are the reasons motivating it:

• Makefiles: In order to have a normalized flow and reduce phonies (to only car-xil-all), I needed to have a common xilinx.mk that includes both flavoured makefiles before describing the common rules at the end (to allow using flavor-dependant variables as target dependencies for these rules). It could be that xilinx_vanilla.mk includes the other flavors but it would make it long and hard to read :
  "Common variables, Vanilla variables, Vanilla targets, Incude other flavors, Flavor dependant Variables, Common targets"
• Future flavors: If a contributor decides to add a new flavor (let's imagine a template-generated RTL top level - as Occamy - that would be very different from the vanilla flavor while still not relying on a block design). It would also require xilinx_ips and this is why I consider them as a "common" more than a "vanilla" feature. The IP config file would then contain if FLAVOR==vanilla_2 if reconfiguration is needed. But appart from run.tcl everything is common and should not be associated with vanilla only to avoid duplication.
• Extension to other projects: one can copy this emulation flow in a new project without the vanilla flavor if they only wish to implement a block design (which is easier in most of the cases, and also the Vivado legit way to do). Vanilla directory should not be required.

This is why I think the top level Xilinx directory, is necessary and should contain common scripts (flash; program; overrides; write_cgmem); common srcs (like the axi_cdc_err_slave it should maybe even be added helow /hw at the root of the repository) and the common xilinx.mk. But:

• flavor_vanilla and flavor_bd could be renamed in xilinx_flavor_vanilla and xilinx_flavor_bd. For more clarity if one do not care about the fpga flow.
• A user_ips folder can be created below the flavor_bd to split constraints and scripts between block design related and carfield ip related. I would recommend keeping it as user_ips as the goal is quite different from xilinx_ip. (Note: why having carfield_ip.xdc one directory above in your tree?)
  Improvement: users_ip could even be actually at the root as it would be a common between different flavor of block designs, but this may be exagerated in the current state...
• Improvement: carfield.xdc could be a common by taking a variable to override each path with a given prefix.
• The readme could be removed as there is the doc. As any flavor will share anyways the same carfield_soc.sv)

Precision, in my view new flavors should be added like this:

├── xilinx
│    ├── scripts
│    ├── constraints
│    ├── src
│    ├── xilinx_flavor_vanilla
│    ├── xilinx_flavor_vanilla_generated
│    ├── xilinx_flavor_bd_ethernet
│    ├── xilinx_flavor_bd_pcie_device
│    ├── xilinx.mk

Of course these may not be relevant for Carfield but may be for future SoCs.

[alex96295]

@alex96295 thanks for the review and the feedback, about the directory structure here are the reasons motivating it:

* **Makefiles:** In order to have a normalized flow and reduce phonies (to only `car-xil-all`), I needed to have a common `xilinx.mk` that includes both flavoured makefiles before describing the common rules **at the end** (to allow using flavor-dependant variables as target dependencies for these rules). It could be that `xilinx_vanilla.mk` includes the other flavors but it would make it long and hard to read :
  "Common variables, Vanilla variables, Vanilla targets, Incude other flavors, Flavor dependant Variables, Common targets"

* **Future flavors:** If a contributor decides to add a new flavor (let's imagine a template-generated RTL top level - as Occamy - that would be very different from the vanilla flavor while still not relying on a block design). It would also require `xilinx_ips` and this is why I consider them as a "common" more than a "vanilla" feature. The IP config file would then contain `if FLAVOR==vanilla_2` if reconfiguration is needed. But appart from `run.tcl` everything is common and should not be associated with vanilla only to avoid duplication.

* **Extension to other projects:** one can copy this emulation flow in a new project without the vanilla flavor if they only wish to implement a block design (which is easier in most of the cases, and also the Vivado legit way to do). Vanilla directory should not be required.

This is why I think the top level Xilinx directory, is necessary and should contain common scripts (flash; program; overrides; write_cgmem); common srcs (like the axi_cdc_err_slave it should maybe even be added helow /hw at the root of the repository) and the common xilinx.mk. But:

* `flavor_vanilla` and `flavor_bd` could be renamed in `xilinx_flavor_vanilla` and `xilinx_flavor_bd`. For more clarity if one do not care about the fpga flow.

* A `user_ips` folder can be created below the `flavor_bd` to split constraints and scripts between block design related and carfield ip related. I would recommend keeping it as `user_ips` as the goal is quite different from `xilinx_ip`. (_Note:_ why having carfield_ip.xdc one directory above in your tree?)
  _Improvement:_ `users_ip` could even be actually at the root as it would be a common between different flavor of block designs, but this may be exagerated in the current state...

* _Improvement:_ `carfield.xdc` could be a common by taking a variable to override each path with a given prefix.

* The readme could be removed as there is the doc. As any flavor will share anyways the same `carfield_soc.sv`)

Precision, in my view new flavors should be added like this:

├── xilinx
│    ├── scripts
│    ├── constraints
│    ├── src
│    ├── xilinx_flavor_vanilla
│    ├── xilinx_flavor_vanilla_generated
│    ├── xilinx_flavor_bd_ethernet
│    ├── xilinx_flavor_bd_pcie_device
│    ├── xilinx.mk

Of course these may not be relevant for Carfield but may be for future SoCs.

Yes, so first of all my apologies because I based my tree on ck/xilinx_bd instead of ck/xilinx_bd_wip where you addressed some of the points I mentioned already. I agree we should try to maximize common code, I see in your branch you already did it.

I agree with calling ooc IPs from user user_ips, and if it is possible for you to implement it in this PR, that would be great. I left carfield_ip.xdc at the top in my tree because I thought that the constraints need to be re-included in the top level BD anyway from what I could recall, maybe it's not the case for this flow and we can keep it at user_ip level

I don't think that renaming the flavor_* directory is required as you are already inside the xilinx folder, so no need to prefix it again

For the carfield.xdc and override: up to you if you have time to do it now. I was noticing some overlapping between constraints and IMHO we should try to minimize those as much as possible

---

TL;DR: action items

1. differentiate between user_ips and xilinx_ips folders
2. Try to reduce overlap in constraints as much as possible

If you could address those above, then we are ready to merge for me :)

[CyrilKoe]

After reconsidering; carfield_ip could be included in xilinx_ips, but it would be easier then to change the xilinx_ips into a .mk solution rather than an independant Makefile system (what @paulsc96 recommended on Cheshire).
Like this I could access the common bender targets and other useful variables to buile the carfield ip.

I don't have time right now; but i'll do soonish it if you think it works.

[alex96295]

After reconsidering; carfield_ip could be included in xilinx_ips, but it would be easier then to change the xilinx_ips into a .mk solution rather than an independant Makefile system (what @paulsc96 recommended on Cheshire). Like this I could access the common bender targets and other useful variables to buile the carfield ip.

I don't have time right now; but i'll do soonish it if you think it works.

of course, that works: using a makefrag is recommended. For what concerns adding it to xilinx_ips, we said that's a kind of common place for the various flows, while carfield ooc is just for bd design. So maybe a standalone folder with its own makefrag would help. I liked the user_ips idea because they are two logically separated things

This won't prevent changing the xilinx_ips in a makefrag solution as well

[CyrilKoe]

@alex96295 switched to makefrag for xilinx_ips, as the resulting makefile is the most complex, I prefered to put also carfield_ip under it to not reproduce the complexity.
Pipeline passed with fpga implem

[alex96295]

I see, thanks :)
Just one thing, what do you mean by:

to not reproduce the complexity

[CyrilKoe]

Just that xilinx_ips.mk is pretty complex, so having another complex user_ips.mk would be a lot. And it makes sense that carfield_ip is compiled with the same makefile as the outputs are exactly the same (ip.xci)

[alex96295]

Just that xilinx_ips.mk is pretty complex, so having another complex user_ips.mk would be a lot. And it makes sense that carfield_ip is compiled with the same makefile as the outputs are exactly the same (ip.xci)

Yes, I agree

[alex96295]

If fpga mapping passes on CI, I guess that a rebase on main will be the last step, and then we can merge :)

[CyrilKoe]

Rebased, pipeline running.
Thanks :D