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

Simplify Portability and Maintenance and unify common VHDL code #41

Open
sy2002 opened this issue Aug 8, 2020 · 13 comments
Open

Simplify Portability and Maintenance and unify common VHDL code #41

sy2002 opened this issue Aug 8, 2020 · 13 comments
Assignees
Labels

Comments

@sy2002
Copy link
Owner

sy2002 commented Aug 8, 2020

One of the basic ideas of QNICE-FPGA is, that it is meant to be highly portable. For sure we are not there, yet ;-)
Currently, everything is quite Xilinx specific.

  • Right now, we have two hardware targets: Nexys 4 DDR and MEGA65
  • I assume that there will be more hardware targets in future, because for example the Terasic DE10-Nano dev board is becoming highly popular (and I recently bought one ;-)).
  • And then, the Nexys 4 DDR is deprecated, the successor is called Nexys4 A7: I never tested on it, because I do not have one, yet. We might want to suport the Nexys A7 one day without giving up the Nexys4 DDR support.
  • So the vhdl/hw as well as the hw/xilinx , hw/intel folders might grow in future, making maintenance hard: Particularly due to duplicated and mainly identical files with slight platform specific changes

Is there a way to work in VHDL a bit similar than with the C preproccessor? And then add some build system like CMAKE or the GNU build system?

E.g. in the source files as high level as #ifdef xilinx and/or as low level as #ifdef enable_hdmi and then something like

make nexys4
make MEGA65
make DE10

And the build environment is smart enough to grasp: OK, Nexys and MEGA65 are Xilinx, DE10 is intel...

Can we unite the zoo of vhdl files that right now have been doubled just because I did not know better how to do it? Like uniting MEGA65 globals and env1 globals. Uniting MEGA65_ISE.vhd with MEGA65_Vivado.vhd with env1 ISE and Vivado and so on.

Having some vhdl/hw/common folder where all the common stuff resides and then the TRULY hardware specific stuff, such as HDMI and HyperRAM for MEGA65 then in a vhdl/MEGA65 folder.

@sy2002 sy2002 added the V1.7 label Aug 8, 2020
sy2002 added a commit that referenced this issue Aug 20, 2020
…"Simplify Portability and Maintenance and unify common VHDL code" (issue #41)
@sy2002
Copy link
Owner Author

sy2002 commented Aug 20, 2020

@MJoergen I am currently knee-deep in making MEGA65 work again with all the changes we made in V1.6 :-) (so I am working on issue #69 right now). While doing so, I made the mmio_mux more powerful, so that we do not need multiple versions of it while we support multiple hardware targets. I used boolean switches in a generic section:

generic (
   GD_TIL            : boolean := true;   -- support TIL leds (e.g. as available on the Nexys 4 DDR)
   GD_SWITCHES       : boolean := true;   -- support SWITCHES (e.g. as available on the Nexys 4 DDR)
   GD_HRAM           : boolean := false   -- support HyperRAM (e.g. as available on the MEGA65)
);

Could you please have a look at the changes in develop and do The Smoke Test on Nexys 4 DDR because my recent commit to develop might have broken things (even though I do not think it did) while I continue on MEGA65?

@MJoergen
Copy link
Collaborator

I'll have time tomorrow to check the generics in mmio_mux. By the way, I really like this, and we should probably use this even more.

Meanwhile, since this issue is about portability and refactoring, I have another issue:
I notice that whenever I've run compile_pore.sh the files pore/boot_message.asm and pore/boot_message_mega65.asm get updated. This means that I no longer can do a simple git checkout <branch> because of "local changes not committed". I then manually have to revert these two files before switching to a new branch.

I propose that we remove the two mentioned files from the repository, since they get automatically generated every time compile_pore.sh is run.

@sy2002
Copy link
Owner Author

sy2002 commented Aug 20, 2020

By the way, I really like this, and we should probably use this even more.

Yes, for example to avoid having to make specific versions of mmio_mux for simulations. But I guess such a (bigger) change might have time until V1.7, because we then also need to update the simulation code.

I propose that we remove the two mentioned files from the repository, since they get automatically generated every time compile_pore.sh is run.

Very good point. I just did that. My next commit to develop will update this.

@bernd-ulmann
Copy link
Collaborator

Removing the aforementioned files from the git is a good idea, and I can relate to the problem of Michael with the files in dist_kit which force me once in a while to checkout these files before I can even perform a git pull. Shouldn't we remove those files from the git as well? :-)

@sy2002
Copy link
Owner Author

sy2002 commented Aug 20, 2020

Removing the aforementioned files from the git is a good idea, and I can relate to the problem of Michael with the files in dist_kit which force me once in a while to checkout these files before I can even perform a git pull. Shouldn't we remove those files from the git as well? :-)

I would oppose removing the files from dist_kit because the whole point of dist_kit is to be our distribution kit, which is completely redundant (also when it comes to the bitstreams there), yet very valuable for doing our QuickStart Guide on the main README.md

@bernd-ulmann
Copy link
Collaborator

...but the dist_kit is created easily and automatically by running compile_and_distribute etc. Couldn't we, when we have a stable release such as the forthcoming 1.6 create a dist_kit and include it as ZIP-file in the git? This way users who can't create their own dist_kit could unpack the ZIP-file while all others would just follow the build instructions, what do you think?

@sy2002
Copy link
Owner Author

sy2002 commented Aug 20, 2020

You are right: Everybody needs to "go through" the make-toolchain process, because otherwise he would not even have an assembler. So yes, I'll take care of this.

@sy2002
Copy link
Owner Author

sy2002 commented Sep 2, 2020

Is there a way to work in VHDL a bit similar than with the C preproccessor?

The answer is: Usage of generic variable in unified VHDL files

And then add some build system like CMAKE or the GNU build system?

This is something we might want to move to the future; right now the IDE based approach is fully OK, so this is out of scope for the issue at hand.

sy2002 added a commit that referenced this issue Sep 9, 2020
* This is a preparation of the Daisy chain refactoring (issue #107)
* I also refactored the mmio_mux, so that it can now also be used in simulations (issue #41)
* Additionally, mmio_mux is now able to output a combined (pre and post pore) reset signal
MJoergen added a commit that referenced this issue Sep 17, 2020
Move MMCM instantiation to a separate file `clk.vhd`. This improves
portability, because the MMCM is Xilinx specific. This new `clk.vhd`
file has just a single responsibility: To generate the CPU and VGA
clocks.

Also refactored the timing constraints, to make them apply more
specifically: Basically, any registers wrapped within the lines
gen_cdc : if true generate
end generate gen_cdc;
will be treated as a Clock Domain Crossing.

This was done to resolve a Xilinx warning regarding the timing
constraints and the new timing generation.

See Issue #41.
@MJoergen
Copy link
Collaborator

MJoergen commented Oct 3, 2020

I've ordered a Terasic DE10-Nano for myself (will get it in a few weeks). Then we can work on porting to that platform too!

@sy2002
Copy link
Owner Author

sy2002 commented Oct 3, 2020

Super cool! 🚀 😃

@MJoergen
Copy link
Collaborator

MJoergen commented Oct 9, 2020

One thing we might want to consider as part of this issue is to rename the CPU bus signals as follows:

Old name New name
WAIT_FOR_DATA waitrequest
ADDR address
DATA_IN readdata
DATA_OUT writedata
DATA_DIR read
DATA_VALID write

The reason for this renaming is that the new signal names (and their semantics, i.e. the bus protocol) then conforms to the "Avalon Memory-Mapped" interface specification. Since Avalon-MM is an industry standard this might make the interface easier to understand/recognize/reuse as well as make it easier to integrate new components later on.

The Avalon-MM interface is described in detail in Chapter 3 of this link https://www.intel.com/content/dam/www/programmable/us/en/pdfs/literature/manual/mnl_avalon_spec.pdf.

Note, when it comes to the DATA_DIR and DATA_VALID signals it might be necessary to modify some of the logic in the various I/O devices and/or the CPU, since the semantics of these signals is not completely identical to the read/write signals in the Avalon-MM standard. Figure 7 in the above mentioned document shows the details. However, I consider this a minor change.

So the question is, whether this is something we want to bother with? I think it is more of a nice-to-have, but on the other hand I don't really see any down sides to it.

Repository owner deleted a comment from mhbrandmaker Oct 10, 2020
@sy2002
Copy link
Owner Author

sy2002 commented Oct 10, 2020

@MJoergen Absolutely relevant and valid topic. Are we already sure, that Avalon-MM is the way to go? I am no specialist in these industry standards, but I observed that in the Open Source world (e.g. on opencores.org), also WishBone seems to be a common standard? So yes, we should do something here. But let us please move it to V1.8 as V1.7 is having a lot of loose ends currently (also on the MEGA65 side) and given my restricted time budget, I would appreciate V1.8 for that 😃 I will open a new issue and assign it to both of us.

@sy2002
Copy link
Owner Author

sy2002 commented Oct 24, 2020

What is left here to do

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