What's wrong with my HDL FIFO?

2»

Comments

  • Black Friday sale - use free FPGA tools

    https://www.tindie.com/products/tinyfpga/tinyfpga-bx/

    I just bought one.
  • A yes, YoSys, I have installed and running already. Sadly I did not get the sale price:)

    The edit/run turn around time of Quartus has been driving me nuts while try to get that state machine to work.

    It's time to get that tinyfpga board or two.

  • And later on this will be a nice addition:

    https://www.crowdsupply.com/tinyfpga/tinyfpga-ex

    As far as your state machine, can you build a simpler example for quicker iteration? How long does an iteration take currently?
  • Heater.Heater. Posts: 21,205
    edited November 24 Vote Up0Vote Down
    Interesting. Isn't the SymbiFlow tool chain for ECP5 a bit far off?

    Currently my state machine is in my UART Rx, connected to a picorcv32. I'm not sure how I would test it on the FPGA without the processor. I'd have to build another state machine to exercise it...

    From hitting the "go" button in Quartus to blinky lights on FPGA is almost 1 minute and 20 seconds. That's only using 12% of the FPGA.
  • Wow, it seems Clifford and co. are racing ahead to supporting all kind of FPGA:





  • Did you figure out what was happening to your state machine?
  • Heater.Heater. Posts: 21,205
    edited November 30 Vote Up0Vote Down
    KeithE,

    Not really. The state machine I have inside my async receiver has been working solidly for days now. My serial test has been running as fast as possible all the while, with no error.

    However, if I make any seemingly innocuous change, like using "one hot" encoding for the state then Quartus makes random junk out of it that does not work. Even if my Verilator simulation is still fine. I dare not touch anything! I have no idea what breaks it.

    Meanwhile, I can make no sense out of the Quartus generated state machine as shown in the netlist viewer. Which looks like this:

    xoro_top_AsyncReceiver_uartRx_next_1.png

    When I inspect the table of transitions that comes with that diagram I see crazy things that basically say "If you are not in this state, this is how you transition to the next state".

    I.e. there is no such possible transition as shown in the diagram! Why is it shown with an arc in the diagram then?


  • Hey check this out. Page 12-60 and 12-61.

    https://courses.cs.washington.edu/courses/cse467/08au/labs/Resources/Quartus II Netlist Viewers.pdf

    "You can analyze this state machine instance using the state machine
    diagram, transition table, and encoding table. Clearly something is
    wrong with the state machine because every state has a transition to
    every other state (Figure 12–34). After inspecting the state machine
    behavior, you determine that in this scenario, the designer forgot to
    create default assignments for the next state (that is, next_state =
    current_state if the conditions are not met)."

    I didn't look back at your generated Verilog, but here's hoping it is your issue.
  • Heater.Heater. Posts: 21,205
    edited December 1 Vote Up0Vote Down
    KeithE,

    Interesting. Thanks.

    So I spent an hour adding default assignments, in my case "next = state" to suitable places in my code. Some caused it to fail. One worked. But always that state machine diagram shows all possible transitions. Something is not right.

    Now I just notice something likely very important. Quartus has generated two state machines in my async receiver. One for "state" and one for "next". The diagram I posted earlier was for "next". Here is the one for "state". It's even more weird, it has no transitions out of sate zero!

    I'm guessing it's not correct to have two state machines.
    xoro_top_AsyncReceiver_uartRx_state.png
  • That wasn’t the response I was hoping for. Can you post you latest state machine code? I can’t look for a bit, but I will look later on Sat Cali time.
  • Heater.Heater. Posts: 21,205
    edited December 1 Vote Up0Vote Down
    KeithE,

    Oh my God, I fixed it! What a struggle.

    I was looking at the Mealy State Machine template that Intel has here: https://www.intel.com/content/www/us/en/programmable/support/support-resources/design-examples/design-software/verilog/ver-state-machine.html how simple could it be?! I noticed a couple of points:

    1) They don't use a "next_state", they only have the "state" variable.

    2) Their case statement is directly driven by "posedge clk or posedge reset"

    3) All possible paths through a case assign to state what the next state should be.

    4) Their state machine has two case statements. One does nothing but update state and the other sets output according to state.

    With that in mind I rearranged my SpinalHDL code a bit:

    1) Got rid of the "next". I never did like that next state thing, it's redundant. Why do so many example Verilog state machines around the net advise to do that?

    2) My "switch" statement (verilog case) was inside a test for the baud rate clock edge. My hunch is that this confuses Quartus so I turned things inside out. I lifted the switch to the outer level and put the test for baud clock edge inside every case. This is logically equivalent, and works, so I have no idea why Quartus might get confused.

    3) I put a default state assignment into every case.

    4) Nothing, unlike the Intel example I left mine as a single switc/case block. Why do they split things up into two case blocks like that?


    With those changes BINGO! Quartus now generates a single state machine instead of two and the state machine viewer shows only the correct state transitions. Oh, and it actually works!

    A big thank you Keith for prompting me to look into that some more.

    xoro_top_AsyncReceiver_uartRx_state.png
  • Heater.Heater. Posts: 21,205
    edited December 2 Vote Up0Vote Down
    My latest, working, SpinalHDL code is here: https://github.com/ZiCog/xoroshiro/blob/master/src/main/scala/xoroshiro/AsyncReceiver.scala
    and the Verilog it generates here: https://github.com/ZiCog/xoroshiro/blob/master/AsyncReceiver.v
    Also below for convenience.

    The crucial difference, I believe, is that the "switch" is now at the top level with the tests for "baudClockX64Sync2.rise" inside each case. Rather than the other way around, clock test at the top level and the switch inside that.

    Logically this is the same. And it did work. But this way round Quartus draws me a correct transition diagram. And only one of them!

    Looking at it now I think I can simplify it further. I could remove the test for "baudClockX64Sync2.rise" in each case and just test "when((bitTimer === 0).rise)".

    Edit: I made the above state machine simplifications, removed state S4.

    xoro_top_AsyncReceiver_uartRx_state.jpg
    module AsyncReceiver (
          input   io_enable,
          input   io_mem_valid,
          output  io_mem_ready,
          input  [3:0] io_mem_addr,
          output [31:0] io_mem_rdata,
          input   io_baudClockX64,
          input   io_rx,
          input   clk,
          input   reset);
      reg  _zz_5;
      reg  _zz_6;
      wire [7:0] _zz_7;
      wire  _zz_8;
      wire  _zz_9;
      wire  _zz_10;
      wire  _zz_11;
      wire [31:0] _zz_12;
      wire [0:0] _zz_13;
      reg [1:0] state;
      reg [5:0] bitTimer;
      reg [2:0] bitCount;
      reg [7:0] shifter;
      reg  baudClockX64Sync1;
      reg  baudClockX64Sync2;
      reg  rxSync1;
      reg  _zz_1;
      reg  _zz_2;
      reg  _zz_3;
      reg [7:0] rdata;
      reg  ready;
      wire  busCycle;
      reg  _zz_4;
      assign _zz_10 = (busCycle && (! _zz_4));
      assign _zz_11 = (bitTimer == (6'b000000));
      assign _zz_12 = {24'd0, rdata};
      assign _zz_13 = (! _zz_9);
      Fifo fifo_1 ( 
        .io_dataIn(shifter),
        .io_dataOut(_zz_7),
        .io_read(_zz_5),
        .io_write(_zz_6),
        .io_full(_zz_8),
        .io_empty(_zz_9),
        .clk(clk),
        .reset(reset) 
      );
      always @ (*) begin
        _zz_5 = 1'b0;
        if(_zz_10)begin
          case(io_mem_addr)
            4'b0000 : begin
              _zz_5 = 1'b1;
            end
            4'b0100 : begin
            end
            default : begin
            end
          endcase
        end
      end
    
      always @ (*) begin
        _zz_6 = 1'b0;
        case(state)
          2'b00 : begin
          end
          2'b01 : begin
          end
          2'b10 : begin
          end
          default : begin
            if(_zz_11)begin
              if((rxSync1 == 1'b1))begin
                if((! _zz_8))begin
                  _zz_6 = 1'b1;
                end
              end
            end
          end
        endcase
      end
    
      assign busCycle = (io_mem_valid && io_enable);
      assign io_mem_rdata = (busCycle ? _zz_12 : (32'b00000000000000000000000000000000));
      assign io_mem_ready = (busCycle ? ready : 1'b0);
      always @ (posedge clk or posedge reset) begin
        if (reset) begin
          state <= (2'b00);
          bitTimer <= (6'b000000);
          bitCount <= (3'b000);
          shifter <= (8'b00000000);
          baudClockX64Sync1 <= 1'b0;
          baudClockX64Sync2 <= 1'b0;
          rxSync1 <= 1'b0;
          rdata <= (8'b00000000);
          ready <= 1'b0;
        end else begin
          baudClockX64Sync1 <= io_baudClockX64;
          baudClockX64Sync2 <= baudClockX64Sync1;
          rxSync1 <= io_rx;
          if((baudClockX64Sync2 && (! _zz_1)))begin
            bitTimer <= (bitTimer - (6'b000001));
          end
    
          case(state)
            2'b00 : begin
              state <= (2'b00);
              if(((! rxSync1) && _zz_2))begin
                state <= (2'b01);
                bitTimer <= (6'b011111);
              end
            end
            2'b01 : begin
              state <= (2'b01);
              if((bitTimer == (6'b000000)))begin
                if((rxSync1 == 1'b0))begin
                  bitTimer <= (6'b111111);
                  state <= (2'b10);
                end else begin
                  state <= (2'b00);
                end
              end
            end
            2'b10 : begin
              state <= (2'b10);
              if((baudClockX64Sync2 && (! _zz_3)))begin
                if((bitTimer == (6'b000000)))begin
                  shifter[bitCount] <= rxSync1;
                  bitCount <= (bitCount + (3'b001));
                  if((bitCount == (3'b111)))begin
                    state <= (2'b11);
                  end
                end
              end
            end
            default : begin
              state <= (2'b11);
              if(_zz_11)begin
                state <= (2'b00);
              end
            end
          endcase
    
    
          ready <= busCycle;
          if(_zz_10)begin
            case(io_mem_addr)
              4'b0000 : begin
                rdata <= _zz_7;
              end
              4'b0100 : begin
                rdata <= {7'd0, _zz_13};
              end
              default : begin
              end
            endcase
          end
        end
      end
    
      always @ (posedge clk) begin
        _zz_1 <= baudClockX64Sync2;
        _zz_4 <= busCycle;
      end
    
      always @ (posedge clk) begin
        _zz_2 <= rxSync1;
      end
    
      always @ (posedge clk) begin
        _zz_3 <= baudClockX64Sync2;
      end
    
    endmodule
    


  • KeithEKeithE Posts: 953
    edited December 2 Vote Up0Vote Down
    Great! Thought you might be interested ... I just ran across this fairly complicated SpinalHDL project:

    https://tomverbeure.github.io/rtl/2018/11/26/Racing-the-Beam-Ray-Tracer.html

    He has a link to yet another SpinalHDL RISC V as well.

    And then there’s this in Chisel:

    https://www2.eecs.berkeley.edu/Pubs/TechRpts/2018/EECS-2018-151.html
  • Wow, that is one audacious project and an excellent write up too. He has lot's ideas there to borrow.

    Never heard of that panologic device. Sounds like a sweet FPGA box to get hold of.

    Thanks for all your hints and tips re: my FIFO and state machine. Especially prompting me to investigate this rabbit hole to the end rather than giving up in confusion and despair.

    I might be back with FIFO woes if I ever decide to make it asynchronous!
Sign In or Register to comment.