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
Fixing the timing propagation through PLL/DCM parts in ISE. #226
Conversation
@enjoy-digital Can you take look at this and tell me what you think? |
377463c
to
a04e4d6
Compare
p_CLKOUT4_PHASE=0., p_CLKOUT4_DIVIDE=p//2, | ||
p_CLKOUT5_PHASE=0., p_CLKOUT5_DIVIDE=p//1, # sys | ||
|
||
unbuffered_sdram_full = Signal() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably better to keep something close to others MiSoC targets. (That's easier to compare with others targets). So OK to add comments but when possible we have to keep avoid changing name when it's not needed.
So OK for the comments but not really for renaming signals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should get the names changed in the MiSoC targets too. The pll[5]
ends up with signals in the generated files as pll_5
which makes things like error messages and timing reports much harder to read.
For example, you currently get;
+-------------------------------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+
|TSclk100 | 10.000ns| 5.340ns| 9.980ns| 0| 0| 0| 4876528|
| TS_hdmi2usbsoc_clk100b | 10.000ns| 3.334ns| 9.980ns| 0| 0| 0| 4847676|
| TS_hdmi2usbsoc_pll_3_ | 5.000ns| 1.730ns| N/A| 0| 0| 0| 0|
| TS_hdmi2usbsoc_pll_1_ | 15.000ns| 14.951ns| N/A| 0| 0| 133078| 0|
| TS_hdmi2usbsoc_pll_5_ | 20.000ns| 18.846ns| N/A| 0| 0| 4711465| 0|
| TS_hdmi2usbsoc_pll_0_ | 2.500ns| N/A| N/A| 0| 0| 0| 0|
| TS_hdmi2usbsoc_pll_4_ | 10.000ns| 9.457ns| N/A| 0| 0| 2755| 0|
| TS_hdmi2usbsoc_pll_2_ | 5.000ns| 4.990ns| N/A| 0| 0| 378| 0|
Afterwards;
+-------------------------------+-------------+-------------+-------------+-------------+-------------+-------------+-------------+
|TSclk100 | 10.000ns| 0.925ns| 9.958ns| 0| 0| 0| 4202487|
| TS_hdmi2usbsoc_clk100b | 10.000ns| 3.334ns| 9.958ns| 0| 0| 0| 4202487|
| TS_hdmi2usbsoc_unbuffered_sdr| 5.000ns| 1.730ns| N/A| 0| 0| 0| 0|
| am_half_b | | | | | | | |
| TS_hdmi2usbsoc_unbuffered_enc| 15.000ns| 14.893ns| N/A| 0| 0| 130795| 0|
| oder | | | | | | | |
| TS_hdmi2usbsoc_unbuffered_sys| 20.000ns| 19.552ns| 7.861ns| 0| 0| 4041421| 27160|
| TS_hdmi2usbsoc_unbuffered_sdr| 2.500ns| N/A| N/A| 0| 0| 0| 0|
| am_full | | | | | | | |
| TS_hdmi2usbsoc_unbuffered_sys| 10.000ns| 9.119ns| N/A| 0| 0| 2754| 0|
| 2x | | | | | | | |
| TS_hdmi2usbsoc_unbuffered_sdr| 5.000ns| 4.979ns| N/A| 0| 0| 357| 0|
| am_half_a | | | | | | | |
It might be better to have a little shorter names though....
General comment: This is becoming to complex and difficult to maintain.
|
I agree. It is only going to get worse as we add more features.
Yes, lets try and do the conversion ASAP.
I did a bit of a refactor of the SoC code to reduce the duplication in #231 I was planning on reducing the duplication between the Opsis and Atlys targets further before I realised that the Opsis target is probably going to get a whole bunch more features with the DisplayPort functionality (and later on, things like CEC and stuff).
I sent a patch upstream about this see m-labs/migen#43 and https://ssl.serverraum.org/lists-archive/devel/2016-March/004189.html ISE definately needs period constraints to be on separate TMN nets
I think we want to do both. On the reproducible side, at the moment I'm pretty sure ISE generates different output for even simple designs. I want to eventually get to the position where with the same input we always get the exactly the same output. For the reason why this is a good idea, take a read of the https://reproducible-builds.org/ On the timing analyze - that is something I've been starting to trying to do (for example, getting migen to generate timing reports). I keep running into things like have no idea what net |
OK I'll look at the different points while converting to new Migen/MiSoC. |
This allows period constraints to propagate through PLL/DCM objects. See http://www.xilinx.com/support/answers/37782.html
* 12ns == 83MHz * 10ns == 100MHz
Fixes #210.
This pull request make a bunch of changes to improve the code around PLL and timing constraints.
These are;
class
for the frequency to make it easier to understand.