paulscherrerinstitute / PsiIpPackage

TCL framework to package Vivado IP-Cores
Other
13 stars 6 forks source link

Supported families #33

Open Botnic opened 2 years ago

Botnic commented 2 years ago

In Package2017_2_1.tcl on line 728 (commit 81b232aacc3faff6c41964024de181b81e4493aa) the list of supported families is defined as follows:

set_property supported_families { \
                                    spartan7    Production \
                                    artix7      Production \
                                    artix7l     Production \
                                    kintex7     Production \
                                    kintex7l    Production \
                                    kintexu     Production \
                                    kintexuplus Production \
                                    virtex7     Production \
                                    virtexu     Production \
                                    virtexuplus Production \
                                    zynq        Production \
                                    zynquplus   Production \
                                    aspartan7   Production \
                                    aartix7     Production \
                                    azynq       Production \
                                    qartix7     Production \
                                    qkintex7    Production \
                                    qkintex7l   Production \
                                    qvirtex7    Production \
                                    qzynq       Production \
                                } [ipx::current_core]

When I execute the script with Vivado 2020.2 I get a bunch of warnings:

WARNING: [IP_Flow 19-4623] Unrecognized family spartan7. Please verify spelling and reissue command to set the supported files. WARNING: [IP_Flow 19-4623] Unrecognized family kintex7. Please verify spelling and reissue command to set the supported files. WARNING: [IP_Flow 19-4623] Unrecognized family kintex7l. Please verify spelling and reissue command to set the supported files. WARNING: [IP_Flow 19-4623] Unrecognized family kintexu. Please verify spelling and reissue command to set the supported files. .......

Removing this section, seems to work fine. Without having specified it, I get the list of all supported devices (Vivado 2020.2: artix7, artix7l, aartix7, qartix7, zynquplus ). Would it be an option for you to remove this code section or could we implement a command to overwrite/deactivate this list (to make sure no old code get broken)?

jonasppsi commented 2 years ago

In my case removing this section, works also. But it seems it will only take the installed devices. That means when you have a minimal Vivado version, you will create an IP which is missing a few supported devices. And when the IP is used in later project on a different device, it gets annoying... It would be nice if Xilinx offers a option "any device".

Since those are just warnings, I don't really care in this case. But we could add a switch like "supported_devices_only".

Botnic commented 2 years ago

It would be nice if Xilinx offers a option "any device".

That would be the most elegant way (as it does what intend to do with this list). I propose to leave it as it is, until we have a strong need for a change or find a better solution. I can live with the warnings. ;-)

Botnic commented 2 years ago

Update: There is a "any device" option. It is called "Automatic family support"

set_property auto_family_support_level level_1 [ipx::current_core]

jonasppsi commented 2 years ago

Yes, I already tested this, but in the component.xml vivado still adds automatically the specific devices... I used Vivado 2019.1. Did you tried it ?

Botnic commented 2 years ago

Yes I am working on a solution at the moment. (I work with 2021.1 but can test it also with 2019.1)

patrick-studer commented 2 years ago

Hello @Botnic and @jonasppsi

I'm following your discussion and I am glad that you are bothered by the same problems I have. At the moment I see one big problem in adding too much new features to the existing IpPackage2017_2_1.tcl file: We have problems keeping backwards compatibility back to 2017.2.1 version!

The actual code includes some "not so nice" workarounds for fixing problems that Vivado had in previous versions. Some of them are already solved in the newer Vivado versions and now produce new problems (e.g. FREQ_HZ parameter).

In my opinion, it is time to "freeze" the old state and create a new file (e.g. IpPackage2021_1.tcl). For engineers that still want to work with the old release, the scripts just need one simple change: namespace import -force psi::ip_package::2017_2_1::*

On the new script we could clean up functions, remove deprecated functions, add new features (I have a big list! 😄) and so on.

Please provide your feedback. If you agree on my plan to build a new major release of the IpPackager that starts from Vivado 2021.2, then I will create a new/separate Issue for that and provide all my ideas.

jonasppsi commented 2 years ago

Hey @paedu92

Good point. I agree that we should create a copy of the IpPackage2017_2_1.tcl and continue adding new features. As you pointed out, the infrastructure to load certain versions is already there. To be honest, the latest update I did only tested backwards to 2019.1. We still working mostly on 2019.1 but planned to go with 2020.2 for while. Patrick, you ran already into problems with 2017 with the latest changes?

Botnic commented 2 years ago

Hi @paedu92

I fully agree with our point. On our side we will never use the scripts with Vivado versions older than 2021.1.

There is just one little issue I have: We would like to use the PsiIpPackage scripts. Therefore I created a PR for all features urgently needed (and they got merged a while ago), but we agreed on waiting until a new release including the features has been published. So it would be in my sense to create a release with that we have and then start with the branching for different versions.

jonasppsi commented 2 years ago

Yes, I would prefer tagging a new release of the current master and later merging the current pull requests on a new version. If that is okay.

patrick-studer commented 2 years ago

Patrick, you ran already into problems with 2017 with the latest changes?

Not really, we try to stick to newer versions of Vivado as well but there are still some projects that use 2017.4... Our current "stable" Version is 2020.1(.1).

But you see the point. If we start integrating new features that are only possible with newer versions then we break the backwards compatibility. So it would be necessary to test each feature that the current state supports with at least the oldest version (2017.2 in that case) before we freeze the current state..

Regarding the "NEW IpPackager", I would like to open a discussion issue for first defining what new features are needed/helpful and what old features should be moved to the new version and what can be replaced/removed. (Rather than just copy paste the old file and start from it...).

I try to open the Issue and add what I have collected up to now in the next days...