VHDL: How to use CLK and RESET in process

Question

I have written VHDL code for VGA controller for spartan 3E board. The code simulates and works well without the reset and clk process in the code below. But after inserting the process(reset,clk) the h_count and v_count counters stop counting and are driven to XXXXX undefined in simulation. Where am i going wrong. The code works perfectly without the clk,reset process(the one commented in bold), I have tested the code on harware also.

The code for VGA Controller

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

entity vga_controller is
    port(clk          : inout std_logic;
         clk_50       : in    std_logic;
         hsync, vsync : out   std_logic;
         video_on     : out   std_logic;
         x_pos, y_pos : out   std_logic_vector(9 downto 0);
         sw           : in    std_logic_vector(2 downto 0) := "100";
         rgb          : out   std_logic_vector(2 downto 0)
    );

end vga_controller;

architecture Behavioral of vga_controller is
    signal h_count, v_count : unsigned(9 downto 0) := (others => '0');

begin
    -- Frequency divider to get 25 MHz clk from 50 MHz clock of Spartan 3E **
    freq_dividr : entity work.t_ff port map(clk_50, clk);
    -- If i remove this process everyting works fine. Why ????**
    process(clk, reset)
    begin
        if reset = '1' then
            h_count  <= (others => '0');
            v_count  <= (others => '0');
            video_on <= '0';
        elsif clk'event and clk = '1' then
            h_count <= h_count;
            v_count <= v_count;
        end if;
    end process;

    process(clk)                        -- Process for horizontal counter
    begin
        if clk'event and clk = '1' then
            if h_count = 799 then
                h_count <= (others => '0');
            else
                h_count <= h_count + 1;
            end if;
        end if;
    end process;

    process(clk)                        -- Process for vertical counter
    begin
        if clk'event and clk = '1' and h_count = 799 then
            if v_count = 524 and h_count = 799 then
                v_count <= (others => '0');
            else
                v_count <= v_count + 1;
            end if;
        end if;
    end process;

    hsync <= '0' when (h_count >= 656 and h_count <= 751) else '1';
    vsync <= '0' when (v_count >= 490 and v_count <= 491) else '1';
    video_on <= '1' when (h_count <= 649 and v_count <= 479) else '0';
    rgb <= sw when (h_count <= 649 and v_count <= 479) else "000";

    x_pos <= std_logic_vector(h_count);
    y_pos <= std_logic_vector(v_count);

end Behavioral;

Answer 1

You should only drive a signal from one process. Just put your reset functionality into the counter processes, and it should work. For instance:

process(clk) -- Process for horizontal counter
begin
  if(rising_edge(clk)) then
    if(rst = '1') then
      h_count <= 0;
    else
      if h_count = 799 then
        h_count <= (others => '0');
      else
        h_count <= h_count + 1;
      end if;
    end if;
  end if;
end process;

A few other notes:

As you can see, I've used a synchronous reset in the above snippet. Unless you absolutely need asynchronous resets, use synchronous resets instead. It helps the synthesizer, as there are some special constructs that are not available using asynchronous resets, and it helps prevent problems when your design gets large (and flip-flops suddenly start getting reset at different times due to signal skew).

Also, don't check for anything but edges (or reset) in the initial if statement of a clocked process. For your vertical counter you have a check for h_count = 799. Do the following instead:

process(clk)
begin
  if(rising_edge(clk)) then
    if(h_count = 799) then
    (...)

It's much clearer, and not as prone to errors.

As a last thing, I've changed the clk'event and clk=1 to the more modern way of doing it, rising_edge(clk). It shouldn't make much difference (unless under certain cicrumstances in simulation), but rising_edge has a few extra checks built-in to make sure that you actually have an edge.