The code below doesn't sound right, can you double-check the intent? I don't think timeout will go to 0, unless you make it a blocking assignment.

        if(~old_download && ioctl_download) begin
                cnt <= 8'b0;
                timeout <= 32'b0;
        end         
        
        timeout <= timeout + 1'b1;

That's why in Verilog RTL we generally try to keep always blocks as small as possible, so they focus on assigning the same small set of signals in all branches.

You are right, I missed that. Thanks for pointing it out!

It's hard to change your mindset from a programming language to a hardware description language and not make mistakes like these, especially when just starting to learn Verilog. :-)

One way to restructure your code is to move all the default assignments to the top of the always block, and then let if conditions change the ones that don't apply for that cycle.

This code would work just fine (many people would argue it's not very readable -- but it saves you a lot of else branches):

        // Default
        timeout <= timeout + 1'b1;

        // Special cases
        if(~old_download && ioctl_download) begin
                cnt <= 8'b0;
                timeout <= 32'b0;
        end

Thank You for the advice! I'm still very much a beginner and since I find the "blink a led" kind of projects too boring, this is basically the first thing I ever wrote in Verilog.

I hope my next project will have cleaner code, be more readable and have less issues like these. HDL languages seem to have a rather steep learning curve and it takes some effort to distance yourself from the usual programming mindset.

In general you should only use a <= to write a reg once within a clock (remember that the RHS of a <= is evaluated when the code is evaluated, the assignment happens later)

And you are mixing <= and = in the same code (look in execute() ) this should never happen.

In general use <= in places that are gated by an "always @(posedge clk)" and = in things gated by "always @(*)"

(with the exception that it's OK to assign a temp variable in a clocked always with '=' provided its lifetime doesn't extend past the block's execution (your use of SKIP_FLAG is an example of this being done correctly)

Thank you, I'll have to do some refactoring to avoid mixing the two assignment principles.

But it is ok to do something like:

cnt <= cnt + 1;

if (cnt > 100) cnt <= 0;

If not, how else should something like this be done?

Thanks for your help, I'm finding HDL to be anything but easy. :-)

The usual way is simply with an else statement, so the reader can follow the logic:

  if (cnt > 100)
    cnt <= 0;
  else
    cnt <= cnt + 1;

probably not a good idea, because you are sampling cnt after think you have incremented it and you will get the wrong value

Essentially what happens is:

    tmp_cnt1 = cnt + 1;
    if (cnt > 100) tmp_cnt2 = 0;

some time later:

    cnt = tmp_cnt1;

and after that (if cnt >100)

    cnt = tmp_cnt2;

Better to say:

    if (cnt >= 99) {
        cnt <= 0;
    } else {
        cnt <= cnt+1;
    }

BTW: as an onetime verilog implementer those temporary storage locations that are made behind your back by the compiler when you use <= are potentially quite expensive, the compiler can optimise the normal case of:

   always @(posedge clk)
       r <= v;

and in some more complex cases where r is only set once in one always statement (or once in any path through an always statement) - but something like:

   always @(*) 
      r <= c;

is a nightmare that essentially means that r can have many changes scheduled in the same instant of time, more importantly it's a number of changes that can't be determined at compile time (could be 1000s of transitions) - resulting in code that's mallocing space to store all those changes - simulation can slow down if you use <= in a non-clocked place because the behaviour can't be determined statically

Also using <= a smart compiler can merge multiple always statements:

    always @(posedge clk)
        r1 <= c1;
    always @(posedge clk)
        r2 <= c2;
    ......

into a single

    always @(posedge clk) {
        r1 <= c1;
        r2 <= c2;
        ......
    }

behind your back, effectively converting 2*N simulation events into 2 (a very good thing)

Thanks for taking the time to explain it, I wish I knew all of this when I started tinkering with Verilog - it would be much easier. Btw, consider writing a FPGA related blog, you have a lot to teach and you explain well! :)

I'm afraid I've mostly worked in standard-cells rather than FPGAs, and that was a while back, I used to be the software guy in the hardware group (and the hardware guy in the software group).

These days I build embedded stuff, living on the hardware/software edge I still get to make the software/hardware tradeoffs that others often can't do

Thanks for pointing it out, I'll try to stick to a simple if-else construct in the future!

There is certainly no consensus that it's the preferred style. Some designers prefer this explicit (and more verbose) style but others prefer the less verbose one (with default first, business logic next, and reset values at the end).

Here is another suggestion that is part stylistic, but may save you debugging effort.

When performing boolean condition testing, eg "if (~a & b)", always use "!" for negation instead of "~", "&&" for logical and, and "||" for logical or for exactly the same reasons you should do it for C code.

If the operands are all 1 bit, either option works fine. But if you ever accidentally use a vector where you intended a bit, one generates a warning and the other doesn't.

I made a beginners guide to Verilog always blocks and assignment statements that talks about this very thing:

https://www.youtube.com/watch?v=0pyQ0ER9Rqc

I think that will simulate and synthesize correctly, but it does look a bit like code smell. If timeout is in a sensitivity list somewhere though, I'm not sure what will happen.

If interested in PDP-1 history and development, check the scanned documents from 1959-1962 at https://gordonbell.azurewebsites.net/digital/timeline/pdp-1s... .

This is an impressive undertaking, and hands-down one of the coolest things I have ever seen. :-)

Thank you very much! I always wanted to check out the PDP-1 and play Spacewar. Because I'm on the wrong continent, the only way to do it was to build my own :-)

There is a follow up - it can also play music, just like the original could. Check it out: https://www.youtube.com/watch?v=ROPyR-tV7A4

Kudos for such an impressive achievement (hrvach is the author of the featured PDP-1 FPGA Implementation.)

A quick question: Were the original PDP-1 manuals enough to implement the system from scratch? Were there details or hidden issues you had to "guess"?

The manuals found on Bitsavers were a great help, they are very detailed and explain the system well.

The original instruction test tapes helped me a lot with debugging the various corner cases. For example, the multiply step opcode (MUS) is re-used in later versions which had full hardware multiply (MUL). The backwards compatibility was retained by flipping a switch inside the computer to selecting between MUS and MUL. I had some programs failing mysteriously because of this.

There were also various pixel brightness available and after implementing it, I discovered Spacewar actually uses that to make some stars brighter than others.

Music playing capability was very advanced for its time (having 4 voices) and after everything was a "silent movie" for a while, there was a rewarding moment when I finally fixed a bug and Mozart started playing from the speakers.

A cool machine, very advanced for its time...

Cheers!

This is great! And it makes me feel bad for not finishing my FPGA PDP-6 and -10 projects.

It's never too late! :) Those are 36-bit machines if I remember correctly and it would be very cool to play around with them.

You can check out my github repos: https://github.com/aap/pdp6, https://github.com/aap/pdp10, but best check out the group if you actually want to run something: https://github.com//pdp-10

This is very impressive and clearly took a lot of work to make. I must admit I know nothing about PDP-10, but it seems interesting and will definitely take a look! Thanks for sharing.

If you want to talk about 36 bits PDPs, join us on #pdp-10 on freenode :) Other PDPs and lispms are also welcome topics.

That is absolutely amazing. What a nice project. The snowflake demo is also very pretty. I got totally lost in reading all the old Digital documents, great picture at the bottom too.

Here's an analysis/walk-through of the Snowflake code:

https://www.masswerk.at/nowgobang/2019/snowflake-archeology

This is a fantastic job with code analysis and Norbert, the author, helped me out with advice a lot. Highly recommended!