VHDL FSM not changing states

Question

Hey I'm a beginner when it comes to VHDL and currently I'm trying to write a protocol decoder for the open pixel control (OPC) protocol: http://openpixelcontrol.org/

I have implemented a FSM in VHDL to decode the OPC messages by reading bytes from a serial/RS232 receiver. The RX module outputs the received byte as a std_logic_vector and creates a data ready pulse that is one gclk cycle long.

In my top level entity I have connected the RX and the decoder modules to the same global clock net. The data ready pulse from the RX module is directly connected to the i_dataready input of the FSM in question.

Although I have checked the code for this FSM multiple times in the last couple of days and also searching a fair bit I couldn't find out why my FSM won't change states.

The thing that puzzles me is that two other modules, a serial TX and another module change states just fine upon receiving the data ready signal from the RX module, it's just this FSM that doesn't work.

This is the code that I have written:

EDIT: okay I've done some more simulation of my complete top level entity: The system is currently running at 2MHz (I know very slow but it's all I can do right now).

According to simulation in Active-HDL the system should work as expected.

Here is my top level entity:

library IEEE;
use IEEE.STD_LOGIC_1164.all;

entity serial_test is 
    port(
        i_gclk: in std_logic;
        i_rx: in std_logic;
        o_tx: out std_logic;
        o_go: out std_logic;
        o_newdata: out std_logic;
        o_ws: out std_logic;
        o_leds: out std_logic_vector(2 downto 0)
    );
end serial_test;

architecture rtl of serial_test is

    signal txact, txdone: std_logic;

    signal rxdata: std_logic_vector(7 downto 0);
    signal rgbdata: std_logic_vector(7 downto 0);
    signal opc_chan: std_logic_vector(7 downto 0);
    signal go: std_logic;
    signal opc_state: std_logic_vector(2 downto 0);
    signal newrgb: std_logic;

begin

    opcdec_inst: entity work.opc_decoder
        port map(i_gclk => i_gclk, i_dataready => go, i_rawdata => rxdata, o_channel => opc_chan, o_rgbdata => rgbdata, o_state => opc_state, o_newdata => newrgb);

    urx_inst: entity work.uart_rx    
        port map(i_clk => i_gclk, i_rx_serial => i_rx, o_rx_dv => go, o_rx_byte => rxdata);

    utx_inst: entity work.uart_tx    
        port map(i_clk => i_gclk, i_tx_dv => go, i_tx_byte => rxdata, o_tx_active => txact, o_tx_serial => o_tx, o_tx_done => txdone);

    wsdrv_inst: entity work.ws2812_driver
        port map(i_gclk => i_gclk, i_rgbdata => rgbdata, i_newdata => newrgb, o_serial => o_ws);

    -- output FSM state to LEDs 
    o_leds <= opc_state;

    -- output go signal for signal analyzer
    o_go <= go;
    o_newdata <= newrgb;

end rtl;

My testbench for the top level entity

LIBRARY ieee;
USE ieee.std_logic_1164.ALL;
USE ieee.numeric_std.ALL;

ENTITY testbench IS
END testbench;

ARCHITECTURE behavior OF testbench IS 

    COMPONENT serial_test
    PORT(
        i_gclk : IN std_logic;
        i_rx : IN std_logic;
        o_tx: OUT std_logic;
        o_go : OUT std_logic;
        o_newdata : OUT std_logic;
        o_ws : OUT std_logic;
        o_leds : OUT std_logic_vector(2 downto 0)
        );
    END COMPONENT;

    SIGNAL i_gclk :  std_logic := '1';
    SIGNAL i_rx :  std_logic := '1';    
    SIGNAL o_tx : std_logic;
    SIGNAL o_go :  std_logic;
    SIGNAL o_newdata :  std_logic;
    SIGNAL o_ws :  std_logic;
    SIGNAL o_leds :  std_logic_vector(2 downto 0);

BEGIN

-- Please check and add your generic clause manually
    uut: serial_test PORT MAP(
        i_gclk => i_gclk,
        i_rx => i_rx,    
        o_tx => o_tx,
        o_go => o_go,
        o_newdata => o_newdata,
        o_ws => o_ws,
        o_leds => o_leds
    );

    p_GCLK: process
    begin
        i_gclk <= not i_gclk;
        wait for 250ns;
    end process p_GCLK;

    p_RX: process
    begin  
        --wait for 375ns;
        wait for 1ms;
        -- first byte: 0x01  (CHANNEL)
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;

        -- second byte: 0x00  (COMMAND)
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;

        -- third byte 0x00(LENGTH MSB)
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;

        -- fourth byte 0x03 (LENGTH LSB)
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;

        -- fifth byte 0xCD
        i_rx <= '0';
        wait for 104us;

        i_rx <= '1';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;

        i_rx <= '1';
        wait for 104us;

        -- sixth byte 0xBA
        i_rx <= '0';
        wait for 104us;

        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;

        i_rx <= '1';
        wait for 104us;

        -- seventh byte 0xFE
        i_rx <= '0';
        wait for 104us;

        i_rx <= '0';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;
        i_rx <= '1';
        wait for 104us;

        i_rx <= '1';
        wait for 104us;

        wait for 10ms;

    end process p_RX;

END;

Here you can see the waveforms that the simulator produces. It clearly shows that at least in simulation the FSM does indeed change states and even o_newdata works as expected. eliaselectronics.com/wp-content/uploads/2014/08/opc_dec_simulation.jpg (sorry I can't yet post images on StackOverflow, uploaded them to my webiste instead)

This screenshot shows the signals from the Lattice CPLD that were actually measured with the logic analyzer. In this case we can see that there is no change in states, o_ws and o_newdata don't change either. eliaselectronics.com/wp-content/uploads/2014/08/opc_dec_measured.png

Now knowing that my FSM code should be okay, I presume this has something to do with the actual synthesis into hardware, maybe things like excessive propagation delay or clock skew. Is there a way to get around this in some way?

This is the FSM code in question

library IEEE;
use IEEE.STD_LOGIC_1164.all;
use IEEE.NUMERIC_STD.all;

entity opc_decoder is 
    port(
        i_gclk: in std_logic;                           -- global system clock
        i_dataready: in std_logic;                      -- data ready signal from previous block (length: 1 gclk cycle)
        i_rawdata: in std_logic_vector(7 downto 0);     -- raw byte received by previous block
        o_channel: out std_logic_vector(7 downto 0);    -- output channel address for output MUX
        o_rgbdata: out std_logic_vector(7 downto 0);    -- decoded OPC data byte (R|G|B)
        o_state: out std_logic_vector(2 downto 0); 
        o_newdata: out std_logic                        -- data synchronization signal for next block
    );
end opc_decoder;

architecture rtl of opc_decoder is

    type t_OPC_SM is (s_IDLE, s_OPC_CMD, s_OPC_LEN_HI, s_OPC_LEN_LO, s_OPC_DATA);

    signal r_OPC_SM: t_OPC_SM := s_IDLE;

    signal r_OPC_CHANNEL: std_logic_vector(7 downto 0); -- decoded channel number/address
    signal r_OPC_COUNT: unsigned(15 downto 0);  -- number of actual RGB data bytes to be received

begin

    p_OPC_SM: process(i_gclk) is
    begin   
        if(rising_edge(i_gclk)) then
            o_newdata <= '0';

            case r_OPC_SM is

                when s_IDLE =>

                    r_OPC_CHANNEL <= (others=>'0');
                    r_OPC_COUNT <= (others=>'0');

                    if(i_dataready = '1') then
                        r_OPC_CHANNEL <= i_rawdata;
                        r_OPC_SM <= s_OPC_CMD;
                    else
                        r_OPC_SM <= s_IDLE;
                    end if;

                when s_OPC_CMD =>

                    if(i_dataready = '1') then
                        r_OPC_SM <= s_OPC_LEN_HI;
                    else
                        r_OPC_SM <= s_OPC_CMD;
                    end if;

                when s_OPC_LEN_HI =>

                    if(i_dataready = '1') then
                        r_OPC_COUNT <= unsigned(i_rawdata) & r_OPC_COUNT(7 downto 0);
                        r_OPC_SM <= s_OPC_LEN_LO;
                    else
                        r_OPC_SM <= s_OPC_LEN_HI;
                    end if;

                when s_OPC_LEN_LO =>
                    if(i_dataready = '1') then
                        r_OPC_COUNT <= r_OPC_COUNT(15 downto 8) & unsigned(i_rawdata);
                        r_OPC_SM <= s_OPC_DATA;
                    else
                        r_OPC_SM <= s_OPC_LEN_LO;
                    end if;

                when s_OPC_DATA =>

                    if(i_dataready = '1') then
                        if (r_OPC_COUNT > 0) then
                            o_rgbdata <= i_rawdata;
                            o_newdata <= '1';
                            r_OPC_COUNT <= r_OPC_COUNT - 1;
                            r_OPC_SM <= s_OPC_DATA;
                        else
                            r_OPC_SM <= s_IDLE;
                        end if;
                    else
                        r_OPC_SM <= s_OPC_DATA;
                    end if;

                when others =>
                    r_OPC_SM <= s_IDLE;

            end case;
        end if;
    end process p_OPC_SM;

    -- output state information 
    with r_OPC_SM select
        o_state <= "001" when s_IDLE,
                   "010" when s_OPC_CMD,
                   "011" when s_OPC_LEN_HI,
                   "100" when s_OPC_LEN_LO,
                   "101" when s_OPC_DATA,
                   "000" when others;

    o_channel <= r_OPC_CHANNEL;

end rtl;

Thanks a lot, Elia

Answer 1

I had a similar problem with Synplify Pro and my ice40 project; go to the following site and follow the instructions to specify the encoding:

https://www.doulos.com/knowhow/fpga/fsm_optimization/

Apparently, the FSM optimizer in Synplify Pro is weird and will prevent your state machines from working unless you explicitly tell it what to do through some directives. This seems like a bug; it should probably get reported to Lattice.

Answer 2

Alright, I'll bite. I wrote a test bench, and while there are errors in the execution of your state machine it changes states.

For a continuous i_dataready:

Which shows o_newdata doesn't get to the test bench in state s_OPC_DATA. Setting that aside for the moment a single i_dataready:

Which shows the opc_decoder changing state from s_IDLE to s_OPC_CMD which I would expect from your process statement.

This tells us you haven't presented enough information on the error indication you are seeing which can be as simple as opc_decoder not being bound because it was analyzed after the block statement it is instantiated in for instance.

Another thing that comes to mind is that you are manipulating the length of i_dataready so the length is less than one clock and it's being missed due to delta delays being different between some of the entities using the clock.

The test bench used to generate these waveforms (noting that the STIMULUS process currently only leaves i_dataready at a '1' for one clock):

library ieee;
use ieee.std_logic_1164.all;

entity opc_decoder_tb is
end entity;

architecture foo of opc_decoder_tb is
    signal i_gclk:      std_logic := '0';  -- global system clock

    signal i_dataready: std_logic;         -- data ready signal from previous   
                                           -- block (length: 1 gclk cycle)

    signal i_rawdata: std_logic_vector(7 downto 0) := (others => '0');     
           -- raw byte received by previous block

    signal o_channel: std_logic_vector(7 downto 0);   -- output channel address 
                                                      -- for output MUX

    signal o_rgbdata: std_logic_vector(7 downto 0);  -- decoded OPC data byte 
                                                     -- (R|G|B)
    signal o_state: std_logic_vector(2 downto 0);   
    signal o_newdata: std_logic ; 
begin

DUT:
    entity work.opc_decoder
        port map (
            i_gclk => i_gclk,
            i_dataready => i_dataready,
            i_rawdata => i_rawdata,
            o_channel => o_channel,
            o_rgbdata => o_rgbdata,
            o_newdata => o_newdata
        );

CLOCK:  -- picked the clock period out of a hat
    process
    begin
        wait for 50 ns;
        i_gclk <= not i_gclk;  -- which is why it was initialized to '0'
        if Now > 500 ns then     -- bound he simulation time, stop simulation
            wait;
        end if;
    end process;

STIMULUS:
    process
    begin
        wait for 20 ns;
        i_dataready <= '0';  -- demo 'U'
        wait for 30 ns;
        i_dataready <= '1';
        wait for 100 ns;  -- clock period
        i_dataready <= '0';
        wait;
    end process;
end architecture;

The problem you've described ("why my FSM won't change states") isn't answerable from the code you've display, and may lie in methodology not yet disclosed.

From the information you've supplied to date the FSM will change states.

You can add to your question as the best way to enable other answers.