cfelton / rhea

A collection of MyHDL cores and tools for complex digital circuit design
MIT License
84 stars 33 forks source link

DE1_Soc Pull request #25

Closed Godtec closed 8 years ago

Godtec commented 8 years ago

Added support for the Terasic DE1_Soc board, So far lt24lcd has compiled and able to create bit-stream in Quartus. unable to test so far.

Please review the changes with a fine tooth comb, First pull request.

cfelton commented 8 years ago

@Godtec, thanks for the PR this will be a good addition. There are many files, it will take awhile to go through this PR. For starters, don't include any of the FPGA tool files (Quartus, ISE, etc). This will only clog up the repository.

Second, we need to be consistent with the current naming, see PEP8 for naming guidelines.

One exmaple, don't use SoC simply use soc.

Godtec commented 8 years ago

Ok, Will make the necessary changes. Thanks. MikeK

cfelton commented 8 years ago

@Godtec I made most of the changes for you, you should be able to merge this and push to you master and it will update this PR:

   # add the remote repository
   >> git remote add rhea https://github.com/cfelton/rhea
   # get the changes I made based of your changes, should merge fine
   >> git pull rhea de1_soc
   # push back to your fork
   >> git push

Note, I removed most of the examples in examples/boards/altera/de1_soc, you will want to make a copy of these before doing the above update/merge. If you don't the changes aren't lost but it will be a little bit of work to get them out of git. I did this simply to minimize the changes so we can focus on the board definitions and the basic tests for the board tests.

Couple suggestions for future contributions:

  1. try and keep the PR focused on a small set of changes (easier to review and merge)
  2. run pylint or use and editor like pycharm that can check basic PEP8 stuff.
  3. for board definitions see the comments in the documentation.
    this outlines how to name the ports/pins in the board definitions. Any feedback on the documentation will be appreciated.

Also, you changes had a mix of tabs and spaces make sure you code has no tabs only spaces

cfelton commented 8 years ago

@Godtec I realize this PR indicates it has been merged but it hasn't. I inadvertently pushed to the master but I removed those changes. If you follow the steps in the previous comment it should (hopefully) update this PR, if not I can commit this set of changes and you can merge and create a PR for the example/boards/de1_soc examples (e.g. vga).

Godtec commented 8 years ago

I see that! Thanks! I hope it was not too much trouble. Still figuring out my git commands. Let me try the above before you make any commits, I want to get my hands wet in this. Please give me a day or two. I will only work in de1_soc branch, to leave the master up to you.

So, I have made most of these changes you wanted on my local branch yesterday, I was just waiting until the VGA portion was finished before make another PR. But I guess that was not the best idea.

I will look into pylint and pycharm. I never heard of these tools before.

So I do have a feed on documentation, And will be adding it. But even with me, it's hard to figure it out.

One Big Question I do have about Rhea, is modules! (verilog) I noticed that the examples you have creates the verilog as one big module that you enter into Quartus to compile. Not broken down into sub modules, (called with the . dot operator), Is that included in RHEA that I am unaware of. Thanks.
MikeK

Godtec commented 8 years ago

@cfelton: Sorry for not getting the commit done yet, I have been working on the VGA port for a few days now and have some questions. So I have changed the code so that the VGA "works" on the DE1_SoC board. So, what is happening is that there is actually a problem with your VGA code. So after rewriting the de1_soc version using the de0nano as an example. I get the code to create the H_Sync and V_sync signals for the a vga monitor with the colour bar patter. But a few monitors that I have tested on, I do not get the monitor to sync or show and image. So I have traced it to your code. What I have noticed on different FPGA VGA code is that the H_Sync signal continues to create the horizontal sync signals even during the V_sync vertical blanking period. So, I believe that this is the problem. So I spend the last day trying to understand your vga_sync.py file. Since this file is what generates the vga sync signals and needs to be adjusted. I will be continuing working on this over the next few days until I get it. But Honestly, I am having trouble understanding your code. Basically, what I need is the horizontal sync signal to continue to send sync pulses even through the vertical blank period. But going through your code, your state machine is too robust and not allowing that to happen. IF you have a quick second to look into that that would be great. At this point, I am going to rewrite your state machine code to do exactly this. Thanks. MikeK

cfelton commented 8 years ago

@Godtec no problem, there is no timeline :)

As mentioned before it would be best to break the changes into smaller chunks:

  1. de1soc board definition and basic tests
  2. updated vga tests (specifically the model) and vga_sync changes
  3. the de1soc vga example

The vga_sync state-machine is a counter based on a bunch of termination values. You simply need to change when the hsync starts and stops. I am not aware of this requirement but I am not a vga expert ...

In the state-machine, removing the following middle condition should do it:

if vcnt == full_screen:
    vcnt[:] = 0
    hcnt[:] = 0
# elif vcnt > R:
#     hcnt[:] = A-1
elif hcnt >= A:
    hcnt[:] = 0

I will (hopefully) have more time later this week to test it out myself.