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

ISE is not able to place and route MEGA65 any more #92

Closed
sy2002 opened this issue Aug 22, 2020 · 11 comments
Closed

ISE is not able to place and route MEGA65 any more #92

sy2002 opened this issue Aug 22, 2020 · 11 comments
Assignees
Labels

Comments

@sy2002
Copy link
Owner

sy2002 commented Aug 22, 2020

ISE is not able to place and route MEGA65 any more. I do not know which one of our refactorings broke it and/or if I made a super-stupid mistake or typo, when converting the HDMI and HDMI transcoder signals from hw/xilinx/MEGA65/Vivado/mega65.xdc to hw/xilinx/MEGA65/ISE/MEGA65.ucf.

This is strange, because it worked in past (but there I did not have the HDMI signals, yet; no idea if that is the reason).

@MJoergen Can you help me with this one?

Here is the ISE_Report.txt, but you can easily try to synthesize MEGA65 in ISE by yourself using hw/xilinx/MEGA65/ISE/QNICE-MEGA65.xise.

As said: Before our refactoring sessions and me adding the signals for HDMI, MEGA65 worked on ISE. Albeit there was always the problem that on ISE I needed to generate the SLOW_CLOCK manually, otherwise I always got an out-of-resources (or something like that) error. But when I used the manual SLOW_CLOCK, it worked. BTW: This was the main reason, why there are two TOP files for MEGA65: One for ISE and one for Vivado.

If you can help, then please fix on branch develop.

@MJoergen
Copy link
Collaborator

Looking at the ISE report I notice the following:

WARNING:Par:289 - The signal hr_clk_p_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr2_clk_p_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr2_reset_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr_cs0_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr_cs1_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr_reset_OBUF has no driver.  PAR will not attempt to route this signal.

Later on in the report it says:

Unroutable      signal: hdmi_clk_OBUF   pin:  hdmi_clk/O

And finally it says:

WARNING:Par:100 - Design is not completely routed. There are 1 signals that are not
   completely routed in this design. See the "MEGA65.unroutes" file for a list of
   all unrouted signals. Check for other warnings in your PAR report that might
   indicate why these nets are unroutable. These nets can also be evaluated
   in FPGA Editor by selecting "Unrouted Nets" in the List Window.

WARNING:Par:284 - There are 6 sourceless signals in this design. This design will not pass the DRC check run by Bitgen.

This at least gives a hint where to look.

To dive deeper, I would probably compare this report, with a successful report. @sy2002 : Do you have a time (or a commit ID) where the ISE build works ?

@MJoergen
Copy link
Collaborator

Regarding the two top level files for MEGA65 (ISE and Vivado) the preferred solution is to move the clock generation to a separate file. This clock generation file will be fairly small (it will only do clock generation), and this small file can then be separated between the two builds, i.e. a clock_ISE.vhd and a clock_Vivado.vhd. The file names will be different, but the entity name inside the files will be the same. That way, the top level entity env1.vhd can just instantiate the clock entity, and it will automatically get the right one. This approach avoids duplicate code.

@sy2002
Copy link
Owner Author

sy2002 commented Aug 22, 2020

Thank you for your support! Highly appreciated! Some comments about your findings:

WARNING:Par:289 - The signal hr_clk_p_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr2_clk_p_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr2_reset_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr_cs0_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr_cs1_OBUF has no driver.  PAR will not attempt to route this signal.
WARNING:Par:289 - The signal hr_reset_OBUF has no driver.  PAR will not attempt to route this signal.

You are right! This might be an attempt to fixing: I needed to deactivate HyperRAM for ISE because the way Paul programmed his driver, Vivado works but ISE is refusing to synthesize (IMHO a bug in the driver: it drives one signal with two clocks). This is why I wrote in doc/constraints:

HyperRAM does not synthesize on ISE due to the following not yet fixed
warning. This is why HyperRAM is deactivated on ISE:
"[Synth 8-5787] Register current_cache_line_update_flags_reg in module
hyperram is clocked by two different clocks in the same process. This may
cause simulation mismatches and synthesis errors. Consider using different
process statements  ["QNICE-FPGA/vhdl/hw/MEGA65/drivers/hyperram.vhdl":491]

A also did write to Paul this here about this phenomenon:

I truly believe that if we are not fixing this, we will in future run into memory corruption (not always - maybe not even often - but from time to time). And into situations that are not (or near to) impossible to reproduce due to the gross simulation vs. implementation mismatch. Plus we will encounter completely unexpected behavior while upgrading to newer versions of Vivado, because these are the edge cases, where Xilinx is free to do whatever they want to do.

Anyway, this is why I deactivated HyperRAM on ISE. But in the MEGA65.ucf the HyperRAM signals are still sitting around. Might the fix be as easy as removing them from the UCF? You might want to try to remove them as they are not needed on ISE at this time.

Unroutable      signal: hdmi_clk_OBUF   pin:  hdmi_clk/O

This is the HDMI clock for the HDMI transciever chip - maybe I did some obscure typo, when migrating the pin data from mega65.xdc (Vivado, where I did the HDMI first) to ISE in MEGA65.ucf? Or in the top file? Strange... Very strange...

Regarding the two top level files for MEGA65 (ISE and Vivado) the preferred solution is to move the clock generation to a separate file.

This is a great idea. There is one more reason for the two TOP files. MEGA65 has a VDAC for VGA. For some reason, Vivado @ MEGA65 needs an inverted VGA signal (otherwise VGA was blurry on the screen) while ISE @ Vivado does not. Some strange timing issue, that I never solved.

ISE Code:

-- make the VDAC output the image                                               
   vdac_sync_n <= '0';
   vdac_blank_n <= '1';
   vdac_clk <= clk25MHz;

Vivado Code:

-- make the VDAC output the image    
   vdac_sync_n <= '0';
   vdac_blank_n <= '1';
   
   -- Fix of the Vivado induced "blurry VGA screen":
   -- As of the  time writing this (June 2020): it is absolutely unclear for me, why I need to
   -- invert the phase of the vdac_clk when use Vivado 2019.2. When using ISE 14.7, it works
   -- fine without the phase shift.   
   vdac_clk <= not clk25MHz;

Do you have a time (or a commit ID) where the ISE build works ?

I am quite sure that commit 7d92c3f worked, because this was the commit, where I fixed the above-mentioned blurry Vivado and therefore I assume that ISE worked at that time, because my reference for a "non blurry picture" was ISE at that time. The date is June, 6 2020.

@MJoergen
Copy link
Collaborator

Looking at the commit from June 6 I see that the HDMI was not added then. So the question is whether HDMI has ever worked on ISE ?

Anyway, I could not determine the reason for why ISE is not able to place and route, given that the same design works with Vivado. I also looked into the SLOW_CLOCK issue on ISE, and I have no idea there either. I unfortunately lack experience with ISE.

I feel the problems with ISE are piling up, and that we should re-consider dropping support for ISE. The only real downside with this choice is that older boards based on Spartan-3 and/or Spartan-6 are not supported by Vivado. So for instance, the old BASYS2 board from Digilent only works with ISE and not Vivado. But on the other hand, since we seem to be piling more and more features into the design, perhaps these older (and smaller) FPGAs are not interesting to work with?

MJoergen added a commit that referenced this issue Aug 27, 2020
@bernd-ulmann
Copy link
Collaborator

I, personally, have no objections to dropping ISE. :-)

@sy2002
Copy link
Owner Author

sy2002 commented Aug 27, 2020

If possible somehow, I'd like to keep ISE, at least for 1.6 and then deprecate in 1.7. But obviously only, when we manage to let it place and route.

@sy2002
Copy link
Owner Author

sy2002 commented Sep 3, 2020

@MJoergen After googling a bit, I found this here:

https://forums.xilinx.com/t5/Implementation/Par-100-Design-is-not-completely-routed/td-p/477284

It looks very similar to our problem: Only 1 signal that is not routed and it also has to do with a clock. In our case, it is the signal hdmi_clk_OBUF which is not routed (I learned this from a file called MEGA65.unroutes). This signal is a pretty straightforward one: It is driven by clk25MHz, which is generated by MMCME: It is the VGA pixelclock. So this sounds pretty well like the problem described in the above-mentioned article.

Then I sifted through the ISE log and found this info:

Clock Information:
------------------
-----------------------------------+------------------------+-------+
Clock Signal                       | Clock buffer(FF name)  | Load  |
-----------------------------------+------------------------+-------+
CLK                                | IBUF+BUFG              | 1     |
SLOW_CLOCK                         | BUFG                   | 2223  |
hdmi_clk_OBUF                      | NONE(VGA_GREEN_0)      | 99    |
-----------------------------------+------------------------+-------+
INFO:Xst:2169 - HDL ADVISOR - Some clock signals were not automatically
buffered by XST with BUFG/BUFR resources. Please use the buffer_type
constraint in order to insert these buffers to the clock signals to help prevent
skew problems.

So I asked myself: If we could make hdmi_clk (what is hdmi_clk_OBUF?) a BUFG, then maybe the problem is solved?

After googling a bit more and trying around, I still was not able to make sure that hdmi_clk_OBUF becomes a BUFG. Can you imagine how this can be achieved? I guess this could be the solution for the problem.

(Alternately, also this here sounds reasonable, but the above-mentioned stuff about BUFG sounds even more reasonable and therefore worth a try first. But if this does not work, then maybe this is worth at try: https://www.xilinx.com/support/answers/53008.html)

@MJoergen
Copy link
Collaborator

MJoergen commented Sep 4, 2020

Good catch. I'll have time next to week look into this.

MJoergen added a commit that referenced this issue Sep 7, 2020
This fixes Issue #92.

In general, all clock outputs from a MMCM should have a BUFG, as shown
in Figure 2 of [this
document](https://www.xilinx.com/support/documentation/application_notes/xapp462.pdf).
@MJoergen
Copy link
Collaborator

MJoergen commented Sep 7, 2020

With the fix in commit 8931e57 the ISE tool is now able to generate a bit-file. I have not tested the generated bit-file.

@sy2002
Copy link
Owner Author

sy2002 commented Sep 7, 2020

Hooraaaaay :-D 🥳 🎆 🍾

Thank you so much! For our portability and support for older ISE-only based platforms, this is a major step forward.

I will test it right now.

Sidenote (not directly related for this issue and not preventing from this issue to be closed):

After reading your commit comment, I do have a hypothesis: One of the reasons, why I did two top files for the MEGA65, one for Vivado and one for ISE was and is, that as soon as I generated the SLOW_CLOCK by the MMCM (as I am doing in Vivado), ISE was not able to place and route any more. So the solution for that might be as easy as the solution for the issue at hand: Adding a BUFG for SLOW_CLOCK, just as you did it for the pixelclock. But this is nothing, that we need to do in V1.6 any more - I am glad that we can now start proceeding with the realease of V1.6: I will add this hypothesis to my V1.7 list of simplifications (reduction of different files per platform) in issue #41 and not change it for V1.6 any more.

@sy2002
Copy link
Owner Author

sy2002 commented Sep 7, 2020

I syntheszed with ISE and did The Smoke Test on MEGA65 hardware (timer interrupt "multi-screen setup", loading Q-TRIS via SD Card/FAT32 and played Q-TRIS): WORKS LIKE A CHARM!

So I am closing this issue now. I will proceed with the V1.6 release now.

@sy2002 sy2002 closed this as completed Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants