0
\$\begingroup\$

I am not that experienced in VHDL. I am trying to implement a simple state machine that goes through 3 states - from idle, to DUT_run for 10 clock cycles, then remains at the done state. However the screenshot below shows the unexpected behaviour of oscillating between DUT_run and done states. And sometimes it jumps straight from idle to done. Can anyone figure out my mistakes?

ILA screenshot of behaviour:

ILA screenshot

Code:

    
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_UNSIGNED.all;

entity top_level is
Port (clk_i         : in std_logic;  --100MHz
      sw15_reset_i  : in std_logic;  --active high
      start_button  : in std_logic;
      sw0_i : in std_logic;         --for testing fsm
      led0_o        : out std_logic  --fsm 1 done led 
     );
end top_level;

architecture Behavioral of top_level is

signal clk_100MHz    : std_logic;
signal rst           : std_logic;
signal rst_n         : std_logic;

signal DUT_count            : std_logic_vector(31 downto 0);
signal fsm1_count           : std_logic_vector(7 downto 0);
signal fsm1_done            : std_logic;

signal fifo_wr_en        : std_logic;
signal fifo_rd_en        : std_logic;
signal fifo_full         : std_logic;
signal fifo_empty        : std_logic;
signal fifo_data_out     : std_logic_vector(7 downto 0);
signal fifo_stored_words : std_logic_vector(3 downto 0);

signal DUT_reset_command : std_logic;

type State_type is (fsm1_S_idle , fsm1_S_DUT_run , fsm1_S_done);
signal fsm1_Current_State , fsm1_Next_State : State_type;

attribute keep : boolean;
attribute keep of rst_n                : signal is true;
attribute keep of rst                  : signal is true;
attribute keep of DUT_count            : signal is true;
attribute keep of fsm1_count           : signal is true;
attribute keep of fsm1_done            : signal is true;

attribute keep of fifo_wr_en        : signal is true;
attribute keep of fifo_rd_en        : signal is true;
attribute keep of fifo_full         : signal is true;
attribute keep of fifo_empty        : signal is true;
attribute keep of fifo_data_out     : signal is true;
attribute keep of fifo_stored_words : signal is true;
attribute keep of DUT_reset_command : signal is true;

attribute keep of FSM1_Current_State : signal is true;
attribute keep of FSM1_Next_State    : signal is true;

component clk_wiz_0 
Port (clk_in     : in std_ulogic; 
      clk_100MHz : out std_ulogic);
end component;

--component fifo_generator_0
--  PORT (
--    clk        : IN STD_LOGIC;
--    srst       : IN STD_LOGIC;
--    din        : IN STD_LOGIC_VECTOR(7 DOWNTO 0);
--    wr_en      : IN STD_LOGIC;
--    rd_en      : IN STD_LOGIC;
--    dout       : OUT STD_LOGIC_VECTOR(7 DOWNTO 0);
--    full       : OUT STD_LOGIC;
--    empty      : OUT STD_LOGIC;
--    data_count : OUT STD_LOGIC_VECTOR(3 DOWNTO 0)
--  );
--END COMPONENT;
signal test_start_button           : std_logic;
signal test_state           : std_logic_vector(1 downto 0);

attribute keep of test_start_button : signal is true;
attribute keep of test_state : signal is true;

begin

--- Synchronising reset button with clock and inverting ---
rst    <= sw15_reset_i when rising_edge(clk_100MHz);
rst_n  <= not rst;

---------------------------------------------------------------------------------------------        
--- Clock wizard component instantiation ---

Clock_Wizard_inst : clk_wiz_0
    Port map(clk_in      => clk_i,                 
             clk_100MHz  => clk_100MHz);

---------------------------------------------------------------------------------------------        
--- FIFO generator wizard component instantiation ---
 
--FIFO_generator_0_inst : fifo_generator_0
--    PORT MAP (clk        => clk_100MHz,
--              srst       => rst,
--              din        => DUT_count(7 downto 0),
--              wr_en      => fifo_wr_en,
--              rd_en      => fifo_rd_en,
--              dout       => fifo_data_out,
--              full       => fifo_full,
--              empty      => fifo_empty,
--              data_count => fifo_stored_words
--  );
  
------------------------------------------------------------------------------------------------------------------------------------------------
--- Counter to emulate program counter data to test FIFO ---

DUT_counter : Process (clk_100MHz, DUT_reset_command)

begin

if (DUT_reset_command = '0') then
    DUT_count <= x"00000000";
    
elsif (rising_edge(clk_100MHz)) then        
    DUT_count <= DUT_count + 1;
    
end if;

end Process;

------------------------------------------------------------------------------------------------------------------------------------------------
--- fsm 1 for controlling the reset signal of DUT and the write enable of FIFO ---

test_start_button <= sw0_i;

fsm_1 : Process (clk_100MHz, rst_n)

begin

if (rst_n = '0') then
    fsm1_Current_State <= fsm1_S_idle;
    fsm1_count         <= "00000000";
    
elsif (rising_edge(clk_100MHz)) then
    fsm1_Current_State <= fsm1_Next_State;

    case fsm1_Current_State is

    when fsm1_S_idle =>

        if (test_start_button = '0') then
            fsm1_Next_State <= fsm1_S_idle;
        
        else
            fsm1_Next_State <= fsm1_S_DUT_run;
    
        end if;

    when fsm1_S_DUT_run =>

        fsm1_count <= fsm1_count + 1;
   
        if (fsm1_count = 10-1) then
            fsm1_Next_State <= fsm1_S_done;
            fsm1_count <= "00000000";
        
        else
            fsm1_Next_State <= fsm1_S_DUT_run;

        end if;

    when fsm1_S_done =>
        
        fsm1_Next_State <= fsm1_S_done;
 
    end case;
    
    end if;
    
end Process;

fsm1_outputs : Process (fsm1_Current_State)

begin

case fsm1_Current_State is

when fsm1_S_idle =>
    test_state        <= "01";
    fifo_wr_en        <= '0';
    DUT_reset_command <= '0';
    fsm1_done    <= '0';

when fsm1_S_DUT_run =>
    test_state <= "10";
    fifo_wr_en     <= '1';
    DUT_reset_command <= '1';
    fsm1_done    <= '0';

when fsm1_S_done =>
    test_state <= "11";
    fifo_wr_en     <= '0';
    DUT_reset_command <= '0';
    fsm1_done    <= '1';
    
end case;

end Process;

--led0_o <= fsm_1_done;

------------------------------------------------------------------------------------------------------------------------------------------------

end Behavioral;

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

For some reason, you have "pipelined" your state machine by having registers for both fsm1_Current_State and fsm1_Next_State. This creates a one-clock delay before fsm1_Next_State can affect the current state.

Normally, if one is going to code a state machine in this style, only the current state is a register, and the next state is determined by a purely combinatorial process — just like how you're producing the FSM outputs.

Try adding fsm1_Next_State to your waveform display to see the delay that I'm talking about.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks Dave, this solved my issue. I now understand my mistake. Only the current_state reg should be updated synchronously and the next state should be combinatorial logic. \$\endgroup\$
    – David777
    Commented Jul 11, 2023 at 21:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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