Refactor code to remove critical warnings in Vivado

[MJoergen]

When building for the Nexys4DDR target using Vivado, and viewing the Methodology report gives a number of critical warnings, including:

1. Non-system clock used to drive flip flops in process count in cycle_counter.vhd.
2. Asynchronous reset on input to DSP in process write_eae_registers in EAE.vhd.

Quite generally, it is recommended to use synchronous reset rather than asynchronous reset when developing for FPGA, particularly if the asynchronous reset is driven by a LUT, as this could potentially lead to race conditions. However, changing all source files is a major (and non-trivial and potentially bug-introducing) task and requires re-testing everything. Item 2 is mentioned above because Vivado claims to get better optimization possibilities after the change.

So I propose for now that only the two items mentioned above be considered for now. They are rather easy to fix (in the sense that they only require touching a few files), and rather easy to test separately (using eae.asm and ise.asm).

Should this be pushed to the develop branch immediately? I don't think this will cause any merge problems.

It is conceivable (I say hopefully ...) that item 1 above might fix Issue #4 considering that issue is precisely related to the cycle counters.

[MJoergen]

Another report generated by Vivado "Clock Networks" states that there are a number of unconstrained clocks. This is often due to using a non-system clock to drive a process, just like item 1 mentioned in this issue. This is generally bad, because Vivado is not running any Static Timing Analysis on these networks. Examples found in the report include:

1. The clk input to digit_iterator in drive_7digits.vhd.
2. The signal ps2_clk_int in ps2_keyboard.vhd.

There are other unconstrained clocks than the above two, but I need to investigate them further to understand their reason.

I'm reluctant to keep on piling stuff into the V1.6 release, but on the other hand these code changes are minimal.

[sy2002]

Since I am a big fan of improving the code quality of QNICE-FPGA, I am 100% agreeing to you. And this helps me to learn, how a better "FPGA-ish" coding style works, which is for me personally a great side effect :-) So:

So I propose for now that only the two items mentioned above be considered for now. They are rather easy to fix (in the sense that they only require touching a few files), and rather easy to test separately (using eae.asm and ise.asm).

Yes, please do that and yes, please use the branch develop for doing so.

Also about your second comment fixing/improving drive_7digits.vhd and ps2_keyboard.vhd: Sounds excellent, please do.

[MJoergen]

Fixed in commit #96f75f1.

Tested on Nexys4DDR hardware via UART using Vivado. Tested with:

• ise.asm
• regbank.asm
• cpu_test.asm
• eae.asm

Note: This might require some additional changes to run on the MEGA65 hardware, see the change implemented in vhdl/hw/nexys4ddr/env1.vhd => Reassigned to @sy2002

[MJoergen]

The only remaining unconstrained clock networks are ff_ascii_new and ff_spec_new, but that is already fixed in the dev-io-addr branch.

[sy2002]

Sounds good. Thank you. I will take care of the MEGA65 part (looks like just 1 line of code, that I need to change :-) )

As I am currently not having access to my "nerd equipment" (will have on Saturday again) and as I am curious: Any chance that you can check, if this also solved issue #4 ?

[MJoergen]

I just checked with Issue #4 and it still fails in the same way. This was tested using ISE with the current develop branch together with the refactored mmio_mux.vhd. At the same time I verified that regbank.asm and eae.asm both work.

[sy2002]

OK Thank you for the info. It was worth a try.

[sy2002]

@MJoergen Is there anything else you would like to do in the context of this issue? If not, then please close it and check the box here: https://github.com/sy2002/QNICE-FPGA/issues/80#issuecomment-681885645

[MJoergen]

With the new VGA module there are no more critical warnings => Closed.