Spartan3A Support #138
Spartan3A Support #138
Conversation
Codecov Report
@@ Coverage Diff @@
## master #138 +/- ##
=======================================
Coverage 80.74% 80.74%
=======================================
Files 32 32
Lines 5111 5111
Branches 1106 1106
=======================================
Hits 4127 4127
Misses 848 848
Partials 136 136Continue to review full report at Codecov.
|
nmigen/vendor/xilinx_spartan_3_6.py
Outdated
| {{name}}_par.ncd | ||
| {{name}}.bit | ||
| """ | ||
| ] | ||
|
|
||
| @property | ||
| def family(self): | ||
| if self.device.startswith("xc3s"): |
There was a problem hiding this comment.
This should normalize the case with something like device = self.device.upcase().
| @@ -76,6 +81,10 @@ class XilinxSpartan6Platform(TemplatedPlatform): | |||
| -ifn {{name}}.prj | |||
| -ofn {{name}}.ngc | |||
| -top {{name}} | |||
| {% if platform.family in ["3", "3E", "3A"] %} | |||
| -use_new_parser yes | |||
There was a problem hiding this comment.
What does this switch does for Spartan 6? Should we not always use it? Is that a part of ISE or just its Spartan support?
There was a problem hiding this comment.
I was under the impression that you don't add any options you don't have to the templates (e.g. that are default for the family). E.g. while -use_new_parser yes is the default for S6 already (and thus harmless to put on the command line), it is not the default for S3.
nmigen/vendor/xilinx_spartan_3_6.py
Outdated
| @@ -126,12 +135,28 @@ class XilinxSpartan6Platform(TemplatedPlatform): | |||
| """, | |||
| r""" | |||
| {{get_tool("bitgen")}} | |||
| {{get_override("bitgen_opts")|default(["-w"])|options}} | |||
| {{get_override("bitgen_opts")|default(["-g Binary:Yes -w"])|options}} | |||
There was a problem hiding this comment.
Should this be a defaulted option? This means that anyone who provides NMIGEN_bitgen_opts for a board that requires bitstream.bin will break the programmer for that board unless they specifically know the default. That seems bad. What are the drawbacks of -g Binary:Yes?
There was a problem hiding this comment.
What are the drawbacks of -g Binary:Yes
It generates bitstream.bin in addition to bitstream.bit. I don't see any drawbacks besides the one you mentioned.
In my case, Mercury needs -g Binary:Yes unconditionally- the programmer for that board cannot handle bitstream.bit. I suppose I could do an override of command_templates at the platform level to add in the -g Binary:Yes option and change the defaults back to the existing behavior.
There was a problem hiding this comment.
Why not just generate bitstream.bin on every board? Unless it's extremely slow or something.
There was a problem hiding this comment.
Why not just generate bitstream.bin on every board? Unless it's extremely slow or something.
That is exactly what -g Binary:Yes already does. The issue, AIUI, is an override would disable this option unless the user already knew that the option -g Binary:Yes is required to get .bin files from bitgen, correct?
There was a problem hiding this comment.
Yes. If boards rely on it it shouldn't be an override. It should either be a per-board switch based on a property, or always enabled. If it's not too slow, always enabled is preferable.
As per discussion on IRC, Spartan3(A) support for trailing-edge-but-still-made boards is a minimally invasive addition to the already-existing Spartan 6 support, so I added it for my own purposes :).
In addition to changing the module name to
xilinx_spartan_3_6, I added two other features I feel usefulfor ISE that were in
omigenbut didn't make their way tonmigen:bitgen -g Binary:Yesby default to get a raw binary bitstream usable beyond Xilinx tools.xst -register_balancing yes. This is off by default in both Spartan6 and Spartan 3A, per the XST User Guides.-ifmt MIXEDis also passed toxstinomigen, but this option is ignored in ISE 14.7 and is assumed default.