State Machine in VHDL - Design

[Daniel K. (daniel_k80)]

Hallo zusammen,

ich habe ein mehr oder weniger großes Verständigungsproblem bei State 
Machines in VHDL. Im Studium haben wir dieses Design gelernt.

library ieee;
use ieee.std_logic_1164.all;

entity moore_4s is

  port(
    clk     : in  std_logic;
    data_in   : in  std_logic;
    reset   : in  std_logic;
    data_out   : out  std_logic_vector(1 downto 0)
  );
  
end entity;

architecture rtl of moore_4s is

  type state_type is (s0, s1, s2, s3);
  
  signal state   : state_type;

begin
  process (clk, reset)
             variable next : state_type;
  begin
    if reset = '1' then
      next := s0;
    elsif (rising_edge(clk)) then
      case state is
        when s0=>
          if data_in = '1' then
            next := s1;
          else
            next := s0;
          end if;
        when s1=>
          if data_in = '1' then
            next := s2;
          else
            next := s1;
          end if;
        when s2=>
          if data_in = '1' then
            next := s3;
          else
            next := s2;
          end if;
        when s3 =>
          if data_in = '1' then
            next := s0;
          else
            next := s3;
          end if;
      end case;
    end if;
                state <= next;
  end process;
  
  -- Output depends solely on the current state
  process (state)
  begin
  
    case state is
      when s0 =>
        data_out <= "00";
      when s1 =>
        data_out <= "01";
      when s2 =>
        data_out <= "10";
      when s3 =>
        data_out <= "11";
    end case;
  end process;
  
end rtl;

Dieses Design habe ich nun mal übernommen und mein Design funktioniert 
jetzt in der Simulation wunderbar, aber wenn ich dann eine 
Post-Implementation Timing Simulation mache, stimmen die übertragenen 
Daten nicht mehr. Bei der Synthese bekomme ich zudem die Meldung

[Synth 8-327] inferring latch for variable 'TLAST_reg'

Wobei das Signal im nicht getakteten Teil der State Machine steht (also 
im Ausgangsnetzwerk). Die Ursache der Warnung verstehe ich schon, nur 
frage ich mich gerade ob dieser Designentwurf für State Machines 
überhaupt sinnvoll oder eher veraltet ist, da diese Warnung bei jedem 
Ausgangssignal auftritt, welches im Ausgangsnetzwerk geschaltet wird.

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Daniel K. schrieb:
> Im Studium haben wir dieses Design gelernt.
Übel, sowas...

Probiers mal so:

  process (clk, reset)
             variable next : state_type;
  begin

    next := state; --<<< Defaultzuweisung an die Variable muss sein!!!

    if reset = '1' then
      next := s0;
    elsif (rising_edge(clk)) then

Und was lernen wir daraus: lass die ersten paar Monate Variablen aus dem 
Spiel. Denn durch diese Variable wird das hier zum wilden Hack, weil im 
selben Prozess völlig unerwartet ausserhalb des getakteten Bereichs noch 
Zuweisung erfolgen. Der Synthesizer müsste einem sowas als Fehler 
ankreiden.


> nur frage ich mich gerade ob dieser Designentwurf für State Machines
> überhaupt sinnvoll oder eher veraltet ist
Es ist irgendein zusammengeflickter akademischer Ansatz. Im echten Leben 
sieht bei mir eine FSM z.B. so aus wie im 
Beitrag "Re: Hilfe für Code - VHDL" die 
Kaffeeautomat_FSM0b.vhd

[Daniel K. (daniel_k80)]

Es ist halt gerade ziemlich frustrierend, weil anscheinend auch noch 
falsche Zustände in der Timing-Analyse dazu kommen (siehe Bild) :)

Ich schau mir dein Beispiel mal an, werf meinen Code in die Tonne und 
mach es neu ^^

[Sigi (Gast)]

Gute Entscheidung,es neuzumachen:
1. signal state : state_type := S0; --<<< Startzustand!
(oder beliebigen anderen Startzustand). Muss nicht
sein, aber heutzutage motzt da kein Tool mehr.
2. Reset innerhalb des RisingEdge-Blocks
(bei einigen Herstellern ist es egal (Altera),
bei anderen ist ein syncr. Reset besser und wird
glaube ich auch von allen Herstellern akzeptiert)
3. Die next-Variable kann komplett weggelassen
und durch das state-Signal ersetzt werden
(aber mit der Zuweisung <= statt :=)
4. die next-Zuweisung am Schluss ist ja wohl das
gefährlichste, inklusive der fehlenden
Default-Zuweisung ein Latch-Generator!
5. Der nachfolgende Block für die IO-Zuweisung
ist asynchron (die asynchr. next-Zuweisung im
ersten Prozess macht's auch nicht schlechter),
daher deine illegalen Ausgabezustände.
Wandle diesen Prozess auch ich einen synchronen um.

Insgesammt: Lerne auf jeden Fall die Formulierung
von Mealy/Moore/Medwedew-FSMs in Ein- als auch
Zweiprozessschreibweise, und zwar in und auswendig.
Sowas muss in der Praxis blind sitzen!

[Daniel K. (daniel_k80)]

Hab auch gerade bei Intel ein paar Templates gefunden:

https://www.intel.com/content/www/us/en/programmable/support/support-resources/design-examples/design-software/vhdl/vhd-state-machine.html

Ggf. noch für den ein oder anderen recht interessant...

[Sigi (Gast)]

.. und was ich noch vergessen habe:
Nach der Substitution von next durch state
sind in deinem Code alle Zuweisungen im
ELSE-Teil innerhalb des CASE-Blocks
überflüssig. state ist ja registriert.

Mein Punkt 5 ist natürlich gefährlich,
denn er führt ja zu einer Latanz von einem
Takt!

Zu den Intel-Templates: Das Mealy-Beispiel
ist ja grausam: einmal steht da

  ..
  if data_in = '1' then
    state <= s1;
  else
    state <= s0;
  end if;
  ..

(fur S0,S1,S2), und für S3

  ..
  if data_in = '1' then
    state <= s3;
  else
    state <= s1;
  end if;
  ..

statt besser und damit einheitlich

  ..
  if data_in /= '1' then
    state <= s1;
  else
    state <= s3;
  end if;
  ..

und damit lassen sich wieder die ELSE-Teile
weglassen. Ist einfach viel lesbarer/weniger
irreführend (find' ich zumindestens).

[Daniel K. (daniel_k80)]

Hallo,

ich habe noch etwas weiter geschaut und sogar ein Beispiel von Xilinx 
für FSM gefunden (ich nutze selber Xilinx).

https://www.xilinx.com/support/documentation/university/Vivado-Teaching/HDL-Design/2015x/VHDL/docs-pdf/lab10.pdf

Xilinx nutzt dort einen asynchronen Prozess für die Ausgabe. Jetzt ist 
auch da wieder die Frage: Ist das sinnvoll? Immerhin empfiehlt es 
Xilinx?

Und dann noch eine Frage am Rande...
Ich bin gerade auch am Überlegen welcher Automat für meine Anwendung 
sinnvoll ist. Ich möchte ein ROM (BRAM) mit einem AXI4-Stream Interface 
ausstatten. Das ganze Ding soll nachher als Master an was anderes dran 
gemacht werden und Daten senden (u. a. zum Debuggen in der Hardware, 
weshalb die Stream Verification IPs leider weg fallen). Für die 
Implementierung des Stream-Interfaces wollte ich dann den 
Zustandsautomaten nehmen. Wenn ich es jetzt richtig verstanden habe, 
muss es sich dabei um einen Mealy-Automaten handeln, da der Zustand des 
Automaten für den Bus auch von dem TREADY-Signal des Slaves abhängt. 
Sehe ich das so richtig?

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Daniel K. schrieb:
> Xilinx nutzt dort einen asynchronen Prozess für die Ausgabe. Jetzt ist
> auch da wieder die Frage: Ist das sinnvoll?
Ist halt umständlich. Und je nach Geschmack des Entwicklers.

> Ist das sinnvoll?
Wenns drauf ankommt, dann implementiere einfach mal beide Varianten und 
vergleiche das Syntheseergebnis.

> Immerhin empfiehlt es Xilinx?
In irgendeinem Lab-Workbook für VHDL-Anfänger...

Man darf zudem nicht alles bedenkenlos glauben, was irgendwer irgendwo 
schreibt:
Beitrag "Re: Variable vs Signal"

> Immerhin empfiehlt es Xilinx?
Fazit: bei FSM muss jeder seinen Stil finden. Die suche kann dabei 
durchaus ein paar Jahre dauern. Ich schäme mich heute fast, wenn ich 
meine ersten Designs so ansehe... ;-)

[daniel__m (Gast)]

Lothar M. schrieb:
> bei FSM muss jeder seinen Stil finden.

Hi,

wobei der Stil auch von den Vorgaben abhängt. "Langsame" FSMs mache ich 
oft in einem synchronen Process (Moore FSM). Jedoch habe ich oft mit 
Handshaking-Signalen zu tun und möchte aus Performance-Gründen 
"back-to-back" übertragen, dann brauche ich eine Mealy FSM, machmal im 
2-Process-Stil, machmal 1-Process + concurrent Statements, jenachdem was 
dokumentativer bzw. verständlicher ist.

[Sigi (Gast)]

Daniel K. schrieb:
> ich habe noch etwas weiter geschaut und sogar ein Beispiel von Xilinx
> für FSM gefunden (ich nutze selber Xilinx).
>
> 
https://www.xilinx.com/support/documentation/university/Vivado-Teaching/HDL-Design/2015x/VHDL/docs-pdf/lab10.pdf
>
> Xilinx nutzt dort einen asynchronen Prozess für die Ausgabe. Jetzt ist
> auch da wieder die Frage: Ist das sinnvoll? Immerhin empfiehlt es
> Xilinx?

Als kleines Katastrophenbeispiel sicherlich..
1. Beispiel: Wie schon Oben von mir beschrieben
können die ELSE-Teile im Case-Block entfallen
(enfach durch next_state <= state als Default-Zuweisung
ersetzen).
2. Beispiel: next_state <= S0: grausam! Du hast hier
eine 3-Prozess-Schreibweise. Kombinatorische Prozesse
dienen eiglich der Bestimmung des nächsten Zustands,
falls sich nichts ändert, dann bleibt der aktuelle
Zustand. Also ist die Default-Zuweisung next_state<=state
die beste Wahl und nicht next_state<=S0.
3. "when others"-Fälle sind bei selbstdefinierten Datentypen
total überflüssig.
4. Startzustand-Zuweisung fehlt in der Deklaration
(akzeptiert jedes Tool, seit wann eigentlich? 15 Jahren?)

Standard-Frame für die 1-Prozess-Schreibweise:

type FSM is (S0,S1);

const STATE_START  : FSM := S0;
const STATE_OUTSIG : std_logic := '0';

signal state  : FSM := STATE_START;
signal outsig : std_logic := STATE_OUTSIG;

process(clk)
  if rising_edge(clk) then
    if reset = '1' then
      state  <= STATE_START;
      outsig <= STATE_OUTSIG;
    else
      case reg_state is
        when S0 => -- State S0
          if input = ..whatever.. then 
            state  <= S1;
            outsig <= '1';
          else
            outsig <= '0';
          endif;
        when S1 => -- State S1
          if input = ..whatever.. then 
            state  <= S0;
            nxt_outsig <= '0';
          else
            outsig <= '1';
          endif;
      end case;
    endif
  endif
end process;

Standard-Frame für die 2-Prozess-Schreibweise:

type FSM is (S0,S1);

const STATE_START  : FSM := S0;
const STATE_OUTSIG : std_logic := '0';

signal reg_state  : FSM := STATE_START;
signal nxt_state  : FSM; 
signal reg_outsig : std_logic := STATE_OUTSIG;
signal nxt_outsig : std_logic;

process(clk)
  if rising_edge(clk) then
    if reset = '1' then
      reg_state  <= STATE_START;
      reg_outsig <= STATE_OUTSIG;
    else
      reg_state  <= nxt_state;
      reg_outsig <= nxt_outsig;
    endif
  endif
end process;

process(reg_state,reg_outsig,input)
begin
  -- default assignments
  nxt_state  <= reg_state;
  nxt_outsig <= reg_outsig;

  case reg_state is
    when S0 => -- State S0
      if input = ..whatever.. then 
        nxt_state  <= S1;
        nxt_outsig <= '1';
        mealy_out  <= "01";
      else
        nxt_outsig <= '0';
        mealy_out  <= "10";
      endif;
    when S1 => -- State S1
      if input = ..whatever.. then 
        nxt_state  <= S0;
        nxt_outsig <= '0';
        mealy_out  <= "11";
      else
        nxt_outsig <= '1';
        mealy_out  <= "00";
      endif;
  end case;
end process;

moore_out <= reg_outsig;

[Sigi (Gast)]

..ups, es fehlt noch die Default-Mealy-Zuweisung
in der 2-Prozess-Schreibweise

  ..
  -- default assignments
  nxt_state  <= reg_state;
  nxt_outsig <= reg_outsig;

  -- default mealy assignments
  mealy_out <= "00"; -- or whatever
  ..

.. und es ist natürlich überflüssig zu erwähenen,
dass in der 1-Prozess-Schreibweise keine
Mealy-Signale beschrieben werden können. Dafür
ist zu dem 1-Prozess ein zusätzlicher kombinatorischer
Prozess bzw. concurrent-Zuweisungen erforderlich.

[Daniel K. (daniel_k80)]

Danke für die Anmerkungen. Dann werde ich das Dokument mal schnell 
wieder schließen :)
Ich habe nun mal den ersten Teil meines verpfuschten Projektes neu 
gemacht und mich dabei an das Kaffeeautomatbeispiel von Lothar 
orientiert.

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;

entity AXI4S_ROM is
    Port ( ACLK     : in STD_LOGIC;
           ARESETN  : in STD_LOGIC;
           
           -- AXI4S interface
           TDATA    : out STD_LOGIC_VECTOR(15 downto 0);
           TID      : out STD_LOGIC_VECTOR(7 downto 0);
           TREADY   : in STD_LOGIC;
           TVALID   : out STD_LOGIC;
           TLAST    : out STD_LOGIC
           );
end AXI4S_ROM;

architecture AXI4S_ROM_Arch of AXI4S_ROM is

    type STATE_t is (Reset, EndOfReset, WaitForReady, SetData, WaitForHandShake);
    signal CurrentState : STATE_t := Reset;

    signal Address      : INTEGER := 0;

    signal ROM_Address  : STD_LOGIC_VECTOR(10 downto 0) := (others => '0');
    signal ROM_Data     : STD_LOGIC_VECTOR(15 downto 0) := (others => '0');
    signal DataBuffer   : STD_LOGIC_VECTOR(15 downto 0) := (others => '0');

    component ROM is
        Port (  Address : in STD_LOGIC_VECTOR(10 downto 0);
                Clock : in STD_LOGIC;
                DataOut : out STD_LOGIC_VECTOR(15 downto 0)
                );
    end component;

begin

    DataROM : ROM port map (Address => ROM_Address,
                            DataOut => ROM_Data,
                            Clock => ACLK
                            );

    process(ACLK, ARESETN, TREADY, DataBuffer, CurrentState, Address)
    begin
        if(rising_edge(ACLK)) then
            case CurrentState is

                when Reset =>
                    TVALID <= '0';
                    Address <= 0;

                    if      ARESETN = '1' then CurrentState <= EndOfReset;
                    elsif   ARESETN = '0' then CurrentState <= Reset;
                    end if;

                when EndOfReset =>
                    CurrentState <= WaitForReady;

                when WaitForReady =>
                    TVALID <= '0';
                    DataBuffer <= ROM_Data;

                    -- Wait until TREADY from the slave
                    if      TREADY = '1' then CurrentState <= WaitForReady;
                    else    CurrentState <= SetData;
                    end if;
                    
                when SetData =>
                    TVALID <= '1';
                    TDATA <= DataBuffer;
                    TID <= STD_LOGIC_VECTOR(to_unsigned(Address, 8));
                    
                    -- Set TLAST to indicate the end of the package
                    if      Address = 99 then TLAST <= '1';
                    else    TLAST <= '0';
                    end if;

                    -- Reset the address counter if the end is reached
                    if      Address < 99 then Address <= Address + 1;
                    else    Address <= 0;
                    end if;
                    
                    CurrentState <= WaitForHandShake;

                when WaitForHandShake =>
                    
                    -- Wait until the slave has handshaked the data, signaled by pulling TREADY high
                    if      TREADY = '1' then CurrentState <= WaitForReady;
                                              TVALID <= '0';
                    end if;

            end case;
        end if;
    end process;

    ROM_Address <= STD_LOGIC_VECTOR(to_unsigned(Address, ROM_Address'length));

end AXI4S_ROM_Arch;

Das Ziel war es hier ein ROM mit einem AXI-Stream Interface 
auszustatten, damit mir das ROM als Datenquelle für einen AXI-Stream 
Slave dienen kann. Getestet habe ich das Ding mit der Stream 
Verification IP von Xilinx und da scheint es (in der normalen) 
Simulation zu funktionieren. Eine Post-Implementation Timing Simulation 
kann ich mit der Testbench aber leider nicht machen, da dort der 
Verification-Core nicht funktioniert - was in meinen Augen irgendwie 
Blödsinn ist. Ich dachte die Post-Implementation Timing Simulation nimmt 
die Testbench und steckt diese Signale in das erzeugte Design...

[Lothar M. (Firma: Titel) (lkmiller) (Moderator)]

Daniel K. schrieb:
> process(ACLK, ARESETN, TREADY, DataBuffer, CurrentState, Address)
Da muss nur ACLK rein. Denn nur bei einer Änderung von ACLK muss der 
Simulator den Prozess neu berechnen. Der Synthesizer schert sich sowieso 
nicht um die Sensitvliste.

> Eine Post-Implementation Timing Simulation kann ich mit der Testbench
> aber leider nicht machen
Du brauchst bei einem korrekten synchronem Design (asynchrone externe 
signale werden über 2 FF einsynchronisiert) und korrekten Constraints 
keine Timing-Simulation. Sie bringt keinen Erkenntnisgewinn. Ich z.B. 
habe seit Jahren keine mehr gemacht.

Ein Wort zum Thema Latency.
Dein Kommentar hier lässt vermuten, dass das Signal TLAST aktiv sein 
soll, wenn die Adresse=99 ist:

                    -- Set TLAST to indicate the end of the package
                    if      Address = 99 then TLAST <= '1';
                    else    TLAST <= '0';
                    end if;

                    -- Reset the address counter if the end is reached
                    if      Address < 99 then Address <= Address + 1;
                    else    Address <= 0;
                    end if;

Das wird nicht der Fall sein, weil mit dem selben Takt, mit dem TLAST 
gesetzt wird, auch der Adress Zähler auf 0 gesetzt wird. TLAST ist also 
genau dann aktiv, wenn Adress 0 ist.

Du erkennst es recht einfach, wenn du den Code mal umschreibst:

                    -- Set TLAST to indicate the end of the package
                    if      Address = 99 then TLAST <= '1';
                    else    TLAST <= '0';
                    end if;

                    -- Reset the address counter if the end is reached
                    if      Address = 99 then Address <= 0;
                    else     Address <= Address + 1;
                    end if;

Was ja gleich ist wie das hier:

                    -- Set TLAST to indicate the end of the package
                    -- And reset the address counter if the end is reached
                    if      Address = 99 then 
                        TLAST <= '1';  Address <= 0;
                    else
                        TLAST <= '0';  Address <= Address + 1;
                    end if;

[Daniel K. (daniel_k80)]

Lothar M. schrieb:
> Was ja gleich ist wie das hier:

Stimmt...

Lothar M. schrieb:
> Ein Wort zum Thema Latency.
> Dein Kommentar hier lässt vermuten, dass das Signal TLAST aktiv sein
> soll, wenn die Adresse=99 ist:

Das stimmt schon, aber da die Signale ohnehin alle verzögert kommen, 
passt TLAST auch wieder (siehe Screenshot). Die Ausgabedaten sind eine 
Sinuskurve mit Offset 0x8000. Der letzte Wert ist 0x77F6 und der wird 
korrekt mit TLAST markiert :)