sy2002 / QNICE-FPGA

QNICE-FPGA is a 16-bit computer system for recreational programming built as a fully-fledged System-on-a-Chip in portable VHDL.
http://qnice-fpga.com
Other
69 stars 16 forks source link

Calculate all the hardcoded bit-widths automatically #60

Open sy2002 opened 4 years ago

sy2002 commented 4 years ago

At a ton of places within QNICE, we have the situation that you can tell a vhdl entity via a generic some size or some dimension such as speed (in UART) or clock divisors elsewhere.

Here is one of these many examples - and if there is a solution to my question, I would help you to dig them out and list them here.

So the question goes like this: Look at this line(s):

https://github.com/sy2002/QNICE-FPGA/blob/8baea3493b00a21a0607e64a5796830d4b9b68f2/vhdl/timer.vhd#L90

The challenge is: Is there an automatic way to allow something like

signal   CNT_WIDTH            : natural := f_log2(freq_div_sys_target); 
signal   freq_div_cn   t         : unsigned(CNT_WIDTH downto 0);

that works on Vivado (as well as on ISE)?

I had the challenge that particularly in Vivado's simulation simulation, things that worked in ISE in this field did then fail to launch the simulation on Vivado, so I went back to hardcoded bit widths at many places.

MJoergen commented 4 years ago

Yes, there is a solution:

The signals CNT_WIDTH and freq_div_sys_target should be declared constant.

I've now updated timer.vhd accordingly (and simultaneously changed all constants to UPPER CASE, which is the coding guideline at my current day job, and I suggest we adopt this style).

sy2002 commented 4 years ago

Great, thank you! This will help to fix about 10 other places all over the VHDL code, where we have hardcoded widths. And we need to check, if the hand-crafted log functions works correctly (vhdl/tools.vhd) and if the width of the whole thing is correct. Maybe we always want to add a "+1" to the log result of all these "10 other places" just to ensure that the width is always big enough and we do not run into difficultly to reproduce problems due to a "witdth is 1 to small" problem. Maybe we should do all of this in 1.7 since it changes so much? Your thoughts?

EDIT: Independent, whether we will change the "10 other places" in V1.6 or V1.7: I retested this on hardware using my 3-way-multitasking timer test: WORKS.