0
\$\begingroup\$

I have been working on using the ethernet phy on my Nexys4 DDR for the last few weeks. Over the last few days I have been particularly frustrated with one issue I was having with my rx module. I have pasted the code bellow. My module goes through three different states. Waiting for preamble, signaling the start of frame, and receiving the body of the packet. These three states are stored using the state register. The design works fine in the simulator and when I first tested it on the card.

To test it I have been using chipscope and I had reg state marked as debug. Then I removed the chip scope probes and everything started to fail. It would signal the start of frame and then nothing else. Through experimentation I found that when I have a probe on the state register the whole design works, but if I remove that probe the design falls apart. At first this seemed like a timing issue since the eth_* signals come from the ethernet phy directly (I drive the phy with a clock 45 degrees out of phase from clk_mac.) I added a pipeline stage between those signals and my combination logic and it was still failing. Now I don't think it is a timing issue since it is unlikely that I would get a valid preamble and first byte if those signals were wrong.

After some more experimentation I noticed the following in the synthesis log....

    --------------------------------------------------------------------------------------------------
                       State |                     New Encoding |                Previous Encoding 
    ---------------------------------------------------------------------------------------------------
              STATE_PREAMBLE |                                0 |                              000
                   STATE_SOF |                                1 |                              001
    ---------------------------------------------------------------------------------------------------

INFO: [Synth 8-3354] encoded FSM with state register 'state_reg' using encoding 'sequential' in module 'eth_rx'

It appears that vivado is optimizing out the upper bits of the state register. (Note: before someone points it out I know I should probably only have [1:0] instead of [2:0] but my design started out with more states than I ended up needing and I haven't changed it yet.) Either way vivado is removing STATE_RECV which requires the state[1] bit to be set. By adding the chipscope I was preventing vivado from doing what seems like an illegal optimization. I think this is a bug with vivado. Has anyone else seen this or is there something wrong you see with my verilog? I am using vivado 2018.1.

`timescale 1 ns / 1 ps
module eth_rx
(
    input             clk_mac,
    input             rst_n,

    input             eth_crsdv,
    input             eth_rxerr,
    input  [1:0]      eth_rxd,

    (* mark_debug = "true" *)
    output reg        rx_vld,
    (* mark_debug = "true" *)
    output reg [7:0]  rx_dat,
    (* mark_debug = "true" *)
    output reg        rx_sof,
    (* mark_debug = "true" *)
    output reg        rx_eof,
    (* mark_debug = "true" *)
    output reg [10:0] rx_len,
    (* mark_debug = "true" *)
    output reg        rx_err
);

    `include "util.vh"

    (* mark_debug = "true" *)
    reg       eth_crsdv_b;
    (* mark_debug = "true" *)
    reg [1:0] eth_rxd_b;
    (* mark_debug = "true" *)
    reg       eth_rxerr_b;
    always @(posedge clk_mac) begin
        eth_crsdv_b <= eth_crsdv;
        eth_rxd_b   <= eth_rxd;
        eth_rxerr_b <= eth_rxerr;
    end

    (* mark_debug = "true" *)
    reg [63:0] data_buffer;
    reg [63:0] next_data_buffer;
    always @(posedge clk_mac) begin
        if(rst_n)
            data_buffer <= next_data_buffer;
        else
            data_buffer <= 0;
    end


    reg [10:0] frame_idx, next_frame_idx;
    reg [1:0]  dibit_cnt, next_dibit_cnt;
    always @(posedge clk_mac) begin
        dibit_cnt <= next_dibit_cnt;
        frame_idx <= next_frame_idx;
    end

    reg        next_rx_vld;
    reg        next_rx_sof;
    reg        next_rx_eof;
    reg [10:0] next_rx_len;
    reg [7:0]  next_rx_dat;
    reg        next_rx_err;
    always @(posedge clk_mac) begin
        rx_vld <= next_rx_vld;
        rx_dat <= next_rx_dat;
        rx_sof <= next_rx_sof;
        rx_eof <= next_rx_eof;
        rx_len <= next_rx_len;
        rx_err <= next_rx_err;
    end

    localparam STATE_PREAMBLE = 0;
    localparam STATE_SOF      = 1;
    localparam STATE_RECV     = 2;

    reg [2:0] state;
    reg [2:0] next_state;
    always @(posedge clk_mac) begin
        if(rst_n)
            state <= next_state; 
        else
            state <= STATE_PREAMBLE;
    end

    wire [31:0] crc32_code;
    crc32 crc32_inst
    (
        .clk (clk_mac),
        .rst (next_rx_sof && next_rx_vld),
        .vld (next_rx_vld && !next_rx_eof),
        .data(next_rx_dat),
        .crc (crc32_code)
    );

    always @* begin
        next_state  = state;
        next_rx_vld = 0;
        next_rx_eof = 0;
        next_rx_sof = 0;
        next_rx_err = 0; 
        next_rx_len = rx_len;
        next_rx_dat = rx_dat;

        next_frame_idx = frame_idx;
        next_dibit_cnt = dibit_cnt + 1;

        if(eth_crsdv_b && !eth_rxerr_b) 
            next_data_buffer = {eth_rxd_b, data_buffer[63:2]};
        else
            next_data_buffer = 0;

        case(state)
            STATE_PREAMBLE: begin
                next_dibit_cnt = 0;
                next_frame_idx = 0;
                if(data_buffer == 64'hd555555555555555) begin
                    next_state  = STATE_SOF;
                end
            end STATE_RECV, STATE_SOF: begin
                if(eth_rxerr_b) begin
                    next_rx_vld = 1;
                    next_rx_eof = 1;
                    next_rx_vld = 1;
                    next_state  = STATE_PREAMBLE; 
                end else if(eth_crsdv_b) begin
                    if(&next_dibit_cnt) begin
                        next_frame_idx = frame_idx + 1;
                        if(frame_idx >= 4) begin
                            next_rx_len = frame_idx - 3;
                            next_rx_vld = 1;
                            next_rx_dat = next_data_buffer[31:24];
                            next_rx_sof = state == STATE_SOF;
                            next_state  = STATE_RECV;
                        end
                    end
                end else begin
                    next_state  = STATE_PREAMBLE;
                    next_rx_eof = 1;
                    next_rx_vld = 1;
                    next_rx_err = crc32_code != bswap32(data_buffer[63:32]);
                end
            end
        endcase
    end

endmodule

UPDATE: I got rid of the FSM extraction and now it works fine...

localparam STATE_PREAMBLE = 3'b001;
localparam STATE_SOF      = 3'b010;
localparam STATE_RECV     = 3'b100;

//Override FSM because Vivado ignores STATE_RECV with auto
(* fsm_encoding = "none" *)
reg [2:0] state = STATE_PREAMBLE, next_state;
always @(posedge clk_mac) begin
    if(rst_n)
        state <= next_state; 
    else
        state <= STATE_PREAMBLE;
end
\$\endgroup\$
0
\$\begingroup\$

As you've noticed, the FPGA synthesizer will automatically recognize and optimize state machines. Part of these optimizations is the removal of unreachable states.

In this case, I believe STATE_RECV is unreachable due to the if(&next_dibit_cnt) conditional. next_dibit_cnt is set to 0 in STATE_PREAMBLE, and is incremented during STATE_SOF. Since the register is incremented during that state, it cannot be zero.

\$\endgroup\$
  • \$\begingroup\$ It makes it to that state in the simulator. With the exception of STATE_PREAMBLE it is incremented above regardless of state. Actually I don't even know why I set it to zero in STATE_PREAMBLE since it will be zero the first time the preamble is recognized anyway. \$\endgroup\$ – chasep255 Jul 23 '18 at 9:16
  • \$\begingroup\$ Nevermind about resetting dibit_cnt to zero. I need that because the stream has arbitrary padding before the preamble and I need to get byte aligned. \$\endgroup\$ – chasep255 Jul 23 '18 at 9:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.