Why does my VHDL countdown timer on Nexys3 FPGA board switch between 59 and 68?

Question

I created a 60 second countdown timer in VHDL and connected it to the 7-seg displays on a nexys3 FPGA boar but it doesn't work. This is a project for my college class.

I'm not really skilled at VHDL or digital electronics at all, but I've managed to put together some code using college materials and a bit of internet. It doesn't work. It starts at 60, goes down to 59, next clock pulse goes to 68, counts down to 59 and back to 68 in a repeating pattern.

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
-- Uncomment the following library declaration if using
-- arithmetic functions with Signed or Unsigned values
--use IEEE.NUMERIC_STD.ALL;
 
-- Uncomment the following library declaration if instantiating
-- any Xilinx primitives in this code.
--library UNISIM;
--use UNISIM.VComponents.all;
 
entity kv_zad is
  port(
    rst, clk: in std_logic;
    an: out std_logic_vector(3 downto 0) := "1100";
    seg: inout std_logic_vector(6 downto 0) :="0000000"
  );
end kv_zad;
 
architecture Behavioral of kv_zad is
  signal temp_jed: std_logic_vector(3 downto 0);
  signal temp_dek: std_logic_vector(3 downto 0);
  signal clk_o: std_logic;
  signal clk_rf: std_logic;
  signal a: std_logic;
  begin
    s1: entity work.freqGen generic map(100_000_000) port map(clk, clk_o);
    s2: entity work.freqGen generic map(500_000) port map(clk, clk_rf);

    process(clk_rf)
    begin
      if (clk_rf'event and clk_rf='1') then
        a <= not a;
      end if;
    end process;

  process(rst, clk_o)
  begin
    if (rst = '0') then
      temp_jed <= "0000";
      temp_dek <= "0110";
      if (clk_o'event and clk_o='1') then
        if (temp_jed = "0000" and temp_dek = "0000") then
          temp_jed <= "0000";
          temp_dek <= "0000";
        elsif (temp_jed = "0000") then
          temp_dek <= temp_dek - 1;
          temp_jed <= "1001";
        else
          temp_jed <= temp_jed - 1;
        end if;
      end if;
    else
      temp_jed <= "0000";
      temp_dek <= "0110";
    end if;
  end process;
  with a select
    an <= "1110" when '0',
          "1101" when '1';
  process(temp_jed, temp_dek)
  begin
    if (a='0') then
      case temp_jed is
        when "0000" => seg <= "0000001";
        when "0001" => seg <= "1001111";
        when "0010" => seg <= "0010010";
        when "0011" => seg <= "0000110";
        when "0100" => seg <= "1101100";
        when "0101" => seg <= "0100100";
        when "0110" => seg <= "0100000";
        when "0111" => seg <= "0001111";
        when "1000" => seg <= "0000000";
        when "1001" => seg <= "0000100";
        when others => seg <= "0110000";
      end case;

    elsif (a='1') then

      case temp_dek is
        when "0000" => seg <= "0000001";
        when "0001" => seg <= "1001111";
        when "0010" => seg <= "0010010";
        when "0011" => seg <= "0000110";
        when "0100" => seg <= "1101100";
        when "0101" => seg <= "0100100";
        when "0110" => seg <= "0100000";
        when others => seg <= "0110000";
      end case;
    end if;
  end process;
end Behavioral;

the an output controls which of the 4 7-seg displays should be on and seg is the number I want to output to the display. temp_jed are the ones and temp_dek are the tens digits. I don't know why it behaves like it does, I've asked my professor and he said the logic looks fine to him and told me to figure it out. I don't know if I'm making a stupid logic mistake or am I doing something wrong with the syntax and the processes? Any help would be GREATLY appreciated.

Answer 1

There's a missing others choice or expression type conversion missing in the selected signal assignment state targeting an:

IEEE Std 1076-1993
9.5.2 Selected signal assignments

The selected signal assignment represents a process statement in which the signal transform is a case statement.
...
The characteristics of the select expression, the waveforms, and the choices in the selected assignment statement must be such that the case statement in the equivalent process statement is a legal statement.

8.8 Case statement

If the expression is the name of an object whose subtype is locally static, whether a scalar type or an array type, then each value of the subtype must be represented once and only once in the set of choices of the case statement, and no other value is allowed; this rule is likewise applied if the expression is a qualified expression or type conversion whose type mark denotes a locally static subtype, or if the expression is a call to a function whose return type mark denotes a locally static subtype.

That type conversion can be of the form with To_bit(a) select... with the conversion function defined in IEEE package std_logic_1164 in lieu of an others choice.

Your question is also missing a minimal, complete, and verifiable example demonstrating the problem which here would imply a testbench driving rst and clk as well as an entity declaration and architecture body for freqGen which would be required to have been analyzed into the working library prior to the component instantiations with the reserved word entity.

There's something peculiar about this process:

process(rst, clk_o)
  begin
    if (rst = '0') then
      temp_jed <= "0000";
      temp_dek <= "0110";
      if (clk_o'event and clk_o='1') then
        if (temp_jed = "0000" and temp_dek = "0000") then
          temp_jed <= "0000";
          temp_dek <= "0000";
        elsif (temp_jed = "0000") then
          temp_dek <= temp_dek - 1;
          temp_jed <= "1001";
        else
          temp_jed <= temp_jed - 1;
        end if;
      end if;
    else   -- if (rst = 0) then .../else 
      temp_jed <= "0000";
      temp_dek <= "0110";
    end if;
  end process;

At first look this doesn't appear synthesis eligible having an asynchronous assignment outside of the clock edge condition that isn't an asynchronous reset. Also note the clock edge condition is evaluated only when reset = '0' in a nested if statement.

A two decade counter comprised of counters temp_jed and temp_dek would have temp_dek only increment when temp_jed is going from "1001" to "000"

TWO_DECADE_COUNTER:       -- counts down from 59 to 0, then repeats.
  process (rst, clk_o)
  begin
      if rst = '0' then
          temp_jed <= "1001";
          temp_dek <= "0101";
      elsif rising_edge(clk_o) then
          if temp_jed <= "0000" then
              temp_jed <= "1001";   -- 9
              if temp_dek <= "0000" then
                  temp_dek <= "0101";  -- 5
              else
                  temp_dek <= temp_dek - 1;
              end if;
          else
              temp_jed <= temp_jed - 1;
          end if;
      end if; 
  end process;

The design model (freqGen and the testbench) has been modified to use a slower clock while running for the same time in seconds. The idea is to not store gigabyte range waveform dump files during simulation filled almost entirely with clock transitions. Ideally the generic constants to the two freqGen instances could be fed through generics in kv_zad from the testbench ignoring any slight of hand in freqGen. Also the anode selects can be fed from bits up stream in a single counter used for the decade counters decrement. This would be done by suing a modulo counter for the anode switch rate providing an enable to a ganged modulo counter to get to the decade counter update rate divided by two to get the decade counter update clock. Here the digit refresh rate for the multiplexed anode switched display looks way to high. Your Nexsys 3 userguide should address what's actually needed.

It's also not a good idea to use mode inout ports in the top level entity of a design specification undergoing synthesis.

There's no reason to do arithmetic on std_logic_vector values here. The results aren't output on ports.

The entire genned together model for simulation:

library ieee;
use ieee.std_logic_1164.all;

entity freqGen is   -- this is modified for simulation
    generic (
        constant counts: positive := 100_000_000;  -- in 100 MHz clocks
        constant scale:  positive := 1             -- for sim clock
    );
    port (
        clk:    in  std_logic;
        clk_o:  out std_logic
    );
end entity;

architecture scaled_clock of freqGen is
    constant MAX_COUNT: natural := counts/scale/2 - 1;
    signal counter: natural range 0 to MAX_COUNT := 0;   
         -- half clk_o period with scaled clock
    signal clk_out: std_logic := '1'; 
begin
    process (clk)
    begin
        if rising_edge(clk) then
            if counter = MAX_COUNT then
                counter <= 0;
                clk_out <= not clk_out;
            else
                counter <= counter + 1;
            end if;
        end if;
    end process;
    clk_o <= clk_out;
end architecture;

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
 
entity kv_zad is
  port(
    rst, clk: in std_logic;
    an: out std_logic_vector(3 downto 0) := "1100";
    seg: inout std_logic_vector(6 downto 0) :="0000000"
  );
end kv_zad;
 
architecture Behavioral of kv_zad is
  signal temp_jed: std_logic_vector(3 downto 0);
  signal temp_dek: std_logic_vector(3 downto 0);
  signal clk_o: std_logic;
  signal clk_rf: std_logic;
  -- signal a: std_logic;
  signal a:     std_logic := '0'; -- CHANGED for SIMULATION a <= not a;
  begin
    s1: entity work.freqGen generic map(100_000_000, 1000) port map(clk, clk_o);
    s2: entity work.freqGen generic map(500_000, 1000) port map(clk, clk_rf);

    process(clk_rf)
    begin
      -- if (clk_rf'event and clk_rf='1') then -- CHANGED for SIMULATION
      if rising_edge(clk_rf) then
        a <= not a;
      end if;
    end process;

  -- process(rst, clk_o)
  -- begin
  --   if (rst = '0') then
  --     temp_jed <= "0000";
  --     temp_dek <= "0110";
  --     -- if (clk_o'event and clk_o='1') then -- CHANGED for SIMULATION
  --     if rising_edge(clk_o) then
  --       if (temp_jed = "0000" and temp_dek = "0000") then
  --         temp_jed <= "0000";
  --         temp_dek <= "0000";
  --       elsif (temp_jed = "0000") then
  --         temp_dek <= temp_dek - 1;
  --         temp_jed <= "1001";
  --       else
  --         temp_jed <= temp_jed - 1;
  --       end if;
  --     end if;
  --   else
  --     temp_jed <= "0000";
  --     temp_dek <= "0110";
  --   end if;
  -- end process;

TWO_DECADE_COUNTER:       -- counts down from 59 to 0, then repeats.
  process (rst, clk_o)
  begin
      if rst = '0' then
          temp_jed <= "1001";
          temp_dek <= "0101";
      elsif rising_edge(clk_o) then
          if temp_jed <= "0000" then
              temp_jed <= "1001";   -- 9
              if temp_dek <= "0000" then
                  temp_dek <= "0101";  -- 5
              else
                  temp_dek <= temp_dek - 1;
              end if;
          else
              temp_jed <= temp_jed - 1;
          end if;
      end if; 
  end process;
  
  with to_bit(a) select  -- CHANGED, for standards compliance
    an <= "1110" when '0',
          "1101" when '1';
  process(a, temp_jed, temp_dek)
  begin
    if (a = '0') then
      case temp_jed is
        when "0000" => seg <= "0000001";
        when "0001" => seg <= "1001111";
        when "0010" => seg <= "0010010";
        when "0011" => seg <= "0000110";
        when "0100" => seg <= "1101100";
        when "0101" => seg <= "0100100";
        when "0110" => seg <= "0100000";
        when "0111" => seg <= "0001111";
        when "1000" => seg <= "0000000";
        when "1001" => seg <= "0000100";
        when others => seg <= "0110000";
      end case;

    elsif (a ='1') then

      case temp_dek is
        when "0000" => seg <= "0000001";
        when "0001" => seg <= "1001111";
        when "0010" => seg <= "0010010";
        when "0011" => seg <= "0000110";
        when "0100" => seg <= "1101100";
        when "0101" => seg <= "0100100";
        when "0110" => seg <= "0100000";
        when others => seg <= "0110000";
      end case;
    end if;
  end process;
end Behavioral;

library ieee;
use ieee.std_logic_1164.all;

entity kv_zad_tb is
end entity;

architecture tb of kv_zad_tb is
    signal clk:     std_logic := '0'; 
    signal rst:     std_logic := '1';
    signal an:      std_logic_vector(3 downto 0);
    signal seg:     std_logic_vector(6 downto 0); -- careful, no driver here
begin
DUT:
    entity work.kv_zad
        port map (
            clk => clk,
            rst => rst,
            an => an,
            seg => seg
        );
    
CLOCK:         -- from the freqGen generic 100 MHz for 1 sec display update
    process    -- we'll modify that and freqGen for faster simulation
    begin
        wait for 5000 ns;  -- 1000X slower clock, high or low interval
        clk <= not clk;
        if now > 65 sec then  -- end simulation by stopping clock
            wait;
        end if;
    end process;
RESET:
    process
    begin
        wait for 400 ns;
        rst <= '0';
        wait for 800 ns;
        rst <= '1';
        wait;
    end process;
end architecture;

The added generic scale can be removed from the component instantiations. It has a default value of 1.

The model produced a simulation waveform:

This was done using ghdl and gtkwave on a 2020 Macbook Air M1 running MacOS Ventura 13.4, the restrictive SSD size the reason for trying to hold down the waveform dump file sizes. A simulation with a 100 MHz clock was very slow and had a 14 GB waveform dump file size before scaling the clock by 1000. The simulation speed up was also appreciated.

Answer 2

Try to remove the following 2 lines. The problem is on the clk_o rising-edge, temp_dek/tmep_jed are decremented. But on falling-edge, they are assigned to default value 60 that causes the counter failure.

  process(rst, clk_o)
  begin
    if (rst = '0') then
      temp_jed <= "0000";  -- Remove this
      temp_dek <= "0110";  -- Remove this
      if (clk_o'event and clk_o='1') then