Replacing AHDL with Verilog

Seairth · 2014-09-01 06:08

I'd like to replace the two AHDL files with VHDL Verilog. The reasons include:

Consolidates all files to a single language
Ability to use ModelSim on all of the files (I wanted to simulate tim.tdf, which is what prompted this effort.)

However, I see that the following "baked in" primitives are being used:

dff
dffe
tri

Altera provides VHDL Verilog components for all of these (DFF, DFFE, TRI), so it's not difficult to do the conversion. But I'm concerned that doing so will tie the code too closely to Altera. And with some Xilinx developers out there, I want to make sure this change will work for them as well.

Any thoughts before I start?

Also, where should I start from? I don't really want to start another fork, but I also don't really want to post the update to the forum in a zip file.

overclocked · 2014-09-01 06:13

The files have already been converted to verilog by Magnus Karlsson as part of the Altera=>Xilinx port in other thread. Why would you write these few files in VHDL when all the rest of the project is verilog?

Seairth · 2014-09-01 06:33

overclocked wrote: »

The files have already been converted to verilog by Magnus Karlsson as part of the Altera=>Xilinx port in other thread. Why would you write these few files in VHDL when all the rest of the project is verilog?

Dang it! I meant Verilog! My fingers were on automatic and my brain was obviously content enough to not notice. No more posts before my morning coffee!

As for the conversion, that's great! No need for me to do it. I'll go look at his fork...

Seairth · 2014-09-01 06:44

Actually, I just looked. That wasn't quite what I had in mind. I would still like to keep the "tim" module, as Magnus' version got rid of it entirely (and changed the behavior, in my opinion).

mkarlsson · 2014-09-01 08:51

Seairth wrote: »

Actually, I just looked. That wasn't quite what I had in mind. I would still like to keep the "tim" module, as Magnus' version got rid of it entirely (and changed the behavior, in my opinion).

In your opinion, what behavior has changed?

Seairth · 2014-09-01 10:24

mkarlsson wrote: »

In your opinion, what behavior has changed?

My apologies. Re-reading the code (in top.v), it effectively looks the same. I was looking for explicit DFF, DFFE, and TRI implementations, instead of relying on inference (though I see you are using IOBUF for TRI).

I still would have liked to keep it in a separate tim.v, so as to mirror the original code a bit more closely. And, as I mentioned in the original post, I had wanted to run that code through ModelSim, which would be much easier to do as a module. But no biggy. I'll just break it out in my local copy.

Thanks for doing the changes!

jac_goudsmit · 2014-09-01 11:43

I agree with Seairth, it would be better to have a tim.v to replace the original and to keep the modules organized in the same way as the original files so that new users can easily observe how the AHDL changed to Verilog, and so that contributors are less likely to clash with each other.

At this time, I have a master branch that has the same file tree as the original Parallax release (with the addition of the BeMicroCV files and some documentation), and an Altera branch that has my combined tree of Altera targets. I don't think I want to make many more changes to the master branch, at least not until Parallax is on board or any other branch is better tested.

If I understand correctly, Overclocked and Magnus Karlsson are working on combining some Xilinx code. In the long term, I'm thinking these should be combined with my Altera branch to create a single code base, and we should either declare that to be "the branch to go to" for newbies, or merge it back into the master branch. In order to make a future "merge of all things" easier, I have no problem with starting a new branch right away where the AHDL is converted to Verilog for the Altera targets.

===Jac

Seairth · 2014-09-10 04:18

I've attached Verilog replacements for the AHDL. This is based on mkarlsson's Xilinx verion (thanks!). However, to keep these functionally the same as the original AHDL files, these will only work for Altera.

Ramon · 2014-09-10 06:16

Seairth, Thanks !

Do you think that simulation will be run with the free version (modelsim starter)?

I were able to run dig.v on three other free simulators (Icarus Verilog, Verilator, and Tachyon DA's CVC) using Magnus code. (Comments in ROM files will need to be removed if you want to use $readmemh).

But simulation failed on all of them when I tried to fake the Xilinx specific components (PLL_BASE, BUFG, dig, IOBUF) using Xilinx´s own verilog modules.

It has been 1 month since initial release and there are a lot of good work done. I wish we will be able to do a full P8X32A simulation before the end of this month !

Seairth · 2014-09-10 08:20

Ramon wrote: »

Seairth, Thanks !

Do you think that simulation will be run with the free version (modelsim starter)?

I were able to run dig.v on three other free simulators (Icarus Verilog, Verilator, and Tachyon DA's CVC) using Magnus code. (Comments in ROM files will need to be removed if you want to use $readmemh).

But simulation failed on all of them when I tried to fake the Xilinx specific components (PLL_BASE, BUFG, dig, IOBUF) using Xilinx´s own verilog modules.

It has been 1 month since initial release and there are a lot of good work done. I wish we will be able to do a full P8X32A simulation before the end of this month !

As the code currently stands, you can't use ModelSim-Altera Starter. It may be worth trying with the other simulators, though. You will probably have to provide an alternative to the altpll, though. This makes me think that it would be a good idea to separate all Altera-specific and Xilinx-specific code into separate modules with identical parameters. This way, we could also provide "simulation" version of those modules as well.

jac_goudsmit · 2014-09-13 15:13

Seairth, are you on Github? If not, do you mind if I check in your changes (giving you full credit of course)?

By the way, I think in tim.v, reg[7:0] cfgx should be reg[6:0] cfgx (it gets copied from cfg which is declared with [6:0])

Also, in top.v I think wire [3:0] _dummy_clk should be wire [4:0] _dummy_clk (Quartus generates a warning otherwise)

===Jac

thoth · 2014-09-13 16:10

jac_goudsmit wrote: »

Seairth, are you on Github? If not, do you mind if I check in your changes (giving you full credit of course)?

By the way, I think in tim.v, reg[7:0] cfgx should be reg[6:0] cfgx (it gets copied from cfg which is declared with [6:0])

Also, in top.v I think wire [3:0] _dummy_clk should be wire [4:0] _dummy_clk (Quartus generates a warning otherwise)

===Jac

When I tried using tim.v instead of tim.tdf in the original distro as well as my version, it compiled and loaded OK but it wouldn't run a simple Hello World program that outputs to the serial terminal. I didn't have time to research it since I was focused on the COGS mods at the time so I continued using tim.tdf since it doesn't need any changes for the COGS stuff. Here is the KISS script I used to test:

CON
  _CLKMODE = XTAL1             'Set to ext low-speed crystal - 9600 baud max for serial communication
  _XINFREQ = 5_000_000         'Frequency on XIN pin is 5 MHz

VAR

OBJ
  Term : "Parallax Serial Terminal"


PUB Main
  Term.Start(9600)
  Term.CharIn
  Term.Str(string("Starting"))
  repeat

2014-09-13 17:54 - The test program works if I append "+ PLL<x>X" where <x> can be from 1 to 16. Changing cfgx[7:0] to [6:0] didn't change behavior.

thoth · 2014-09-13 18:40

I think I've found the bug in tim.v

//                                 cfgx[].q == b"11xx111" # res,
//                                 cfgx[].q == b"11xx110" & !res,
//                                 cfgx[].q == b"11xx101" & !res,
//                                (cfgx[].q == b"11xx100" # cfgx[].q == b"xxxx000") & !res,
//                                (cfgx[].q == b"11xx011" # cfgx[].q == b"x1xx010") & !res,   <------
//                                 b"0",
//                                 b"0",
//                                 b"0",
//                                 b"0",
//                                 b"0",
//                                 b"0",
//                                 b"0",
//                                 cfgx[].q == b"xxxx001" & !res

always @ (posedge clk)
begin
    divide <= divide + 
    {
         clksel == 5'b11111 || res,                                                     // PLL16X or reset
         clksel == 5'b11110 && !res,                                                    // PLL8X
         clksel == 5'b11101 && !res,                                                    // PLL4X
        (clksel == 5'b11100 || clksel[2:0] == 3'b000) && !res,                          // PLL2X or RCFAST
//        (clksel == 5'b11011 || (clksel[4] == 1'b1 && clksel[2:0] == 3'b010)) && !res,   // PLL1X or XINPUT <-------------
        (clksel == 5'b11011 || clksel[3:0] == 4'b1010) && !res,   // PLL1X or XINPUT
         1'b0,
         1'b0,
         1'b0,
         1'b0,
         1'b0,
         1'b0,
         1'b0,
         clksel[2:0] == 3'b001 && !res                                                  // RCSLOW
        };
end

This change makes code without + pll<x>x at the end work right.

Seairth · 2014-09-14 05:23

Sorry for not responding. On travel at the moment. (Typing on tiny phone screen...)

jac,

I am on github, but did not do these changes there. I've been waiting for that stuff to get straightened out before I figure out which repository to work from. As for the dummy wire, I don't recall seeing the warning. And I thought the altpll megafunction output was 5-bit. On the other hand, that might have been for only one of the platforms.

Thoth,

Thanks for catching that. tim.v may be wrong indeed. Still haven't had time to test the builds (beyond basic "identify hardware" in prop tool). Maybe this evening...

thoth · 2014-09-14 06:32

clksel[4] should have been clksel[3] in your original line. But there was no need for the extra comparison so I just collapsed it into clksel[[3:0]. Having the bit off by one causes the code to miss the case where there was no pll<x>x adder. No big deal - most code uses the pll. The failure when not using a frequency multiplier is catastrophic, not subtle so it's not an insidious bug. It's nice to have all Verilog file now - thanks to your efforts I can now define COGS in top.v rather than having to define it separately in dig.v and top.tdf. Makes things a lot cleaner.

jac_goudsmit · 2014-09-14 10:31

Seairth wrote: »

I am on github, but did not do these changes there. I've been waiting for that stuff to get straightened out before I figure out which repository to work from.

Ah, sorry. I tried to find you on Github and didn't see you, so I went ahead and checked the Verilog conversion into my Altera branch, crediting you and Magnus. That was before I saw the PLL fix, though. I still have to take a closer look at that. (EDIT: I checked the bugfix in now).

Feel free to set up your own fork (I recommend forking the Altera branch) and submit further changes via pull request.

As for the dummy wire, I don't recall seeing the warning. And I thought the altpll megafunction output was 5-bit. On the other hand, that might have been for only one of the platforms.

I was wondering about that too but I didn't try building the other platforms until after I changed it. I don't think they generated any new warnings after my fix.

thoth wrote: »

It's nice to have all Verilog file now - thanks to your efforts I can now define COGS in top.v rather than having to define it separately in dig.v and top.tdf. Makes things a lot cleaner.

Thoth, you can also define macros from the "Assignments>Settings>Analysis & Synthesis Settings>Verilog HDL Input" menu in Quartus, in case you didn't know.

If you set your macro from the menu, it will make it easier to find (especially if you're going to redistribute your code and want others to find it). Also, you could create different Revisions in Quartus, for example a quick-compile revision would set the number of cogs to 4 or even lower from the menu, whereas the full version sets the number of cogs to 8. You can build both versions (unfortunately not automatically) without changing the code.

I would name it something other than COGS, though. NUMBER_OF_COGS or NUM_COGS would be better names if you want to prevent future name clashes. And if you do a search through your code, it will make your life much easier if the name is a little longer and isn't a word that's likely to show up in other identifiers and comments and stuff.

===Jac

thoth · 2014-09-14 16:05

jac_goudsmit wrote: »

Thoth, you can also define macros from the "Assignments>Settings>Analysis & Synthesis Settings>Verilog HDL Input" menu in Quartus, in case you didn't know.

If you set your macro from the menu, it will make it easier to find (especially if you're going to redistribute your code and want others to find it). Also, you could create different Revisions in Quartus, for example a quick-compile revision would set the number of cogs to 4 or even lower from the menu, whereas the full version sets the number of cogs to 8. You can build both versions (unfortunately not automatically) without changing the code.

I would name it something other than COGS, though. NUMBER_OF_COGS or NUM_COGS would be better names if you want to prevent future name clashes. And if you do a search through your code, it will make your life much easier if the name is a little longer and isn't a word that's likely to show up in other identifiers and comments and stuff.

===Jac

Thanks for the info. Right now I'm doing some testing and starting multiple cogs doesn't work right.

Feel free to change the constant identifiers around in any way you deem appropriate. The code is a mess of tabs and spaces right now and that needs to be cleaned up eventually.