VHDL State Machine for LCD initialization

Question

I'm trying to implement the initialization of a LCD of the Spartan 3AN. I'm very new to this so every advise is very welcome.

My code is as follows:

----------------------------------------------------------------------------------
-- Company: 
-- Engineer: 
-- 
-- Create Date:    17:31:49 11/04/2011 
-- Design Name: 
-- Module Name:    LCD - Behavioral 
-- Project Name: 
-- Target Devices: 
-- Tool versions: 
-- Description: 
--
-- Dependencies: 
--
-- Revision: 
-- Revision 0.01 - File Created
-- Additional Comments: 
--
----------------------------------------------------------------------------------
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
--use IEEE.STD_LOGIC_ARITH.ALL;
--use IEEE.STD_LOGIC_UNSIGNED.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 LCD is
    Port ( clk : in  STD_LOGIC;
           LCD_DB : out  STD_LOGIC_VECTOR (7 downto 0);
           LCD_E : out  STD_LOGIC;
           LCD_RS : out  STD_LOGIC;
           LCD_RW : out  STD_LOGIC);
end LCD;

architecture Behavioral of LCD is

--      CAUTION!!!  When using 4-bit mode, FPGA must drive the LCD_DB<3:0> signals HIGH ( = '1' )   
--      MAQUINA DE ESTADOS INICIALIZACION
    type initialization_state is (A, B, C, D, E, F, G, H, I, done);
    signal iCurrentState: initialization_state := A;
    signal iNextState: initialization_state := A;
    signal cycleCounter: integer := 0;

--      MAQUINA DE ESTADOS

begin

    initializationLCD:
    process (clk) begin
        if (clk'event and clk = '1') then
            case iCurrentState is
                when A =>
                    if (cycleCounter = 750000) then
                        iCurrentState <= B;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when B =>
                    LCD_RS <= '1';
                    LCD_RW <= '0';
                    LCD_DB <= "00000001";
                    if (cycleCounter = 50000) then
                        iCurrentState <= C;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when C =>
                    if (cycleCounter = 50000) then
                        iCurrentState <= D;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when D =>
                    LCD_RS <= '1';
                    LCD_RW <= '0';
                    LCD_DB <= "00000010";
                    if (cycleCounter = 50000) then
                        iCurrentState <= E;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when E =>
                    if (cycleCounter = 50000) then
                        iCurrentState <= F;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when F =>
                    LCD_RS <= '1';
                    LCD_RW <= '0';
                    LCD_DB <= "00000100";
                    if (cycleCounter = 12) then
                        iCurrentState <= G;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when G =>
                    if (cycleCounter = 50000) then
                        iCurrentState <= H;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when H =>
                    LCD_RS <= '1';
                    LCD_RW <= '0';
                    LCD_DB <= "00001000";
                    if (cycleCounter = 12) then
                        iCurrentState <= I;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when I =>
                    if (cycleCounter = 50000) then
                        iCurrentState <= done;
                        cycleCounter <= 0;
                    else
                        cycleCounter <= cycleCounter + 1;
                    end if;
                when others =>
                    iCurrentState <= done;
            end case;
        end if;
    end process;

end Behavioral;

So there are 2 questions:

1. Is this code fine? I know I have much more code to do but I just want to see if I'm doing well, what takes me to question 2

2. I simulated with ISim (BTW I'm on Xilinx 12.3) and the state never changes always is A, is my code missing something? or maybe I'm simulating this the wrong way, can you tell me how to simulate it?

Thanks so much !

Answer 1

A few things I see, some are style, some are not:

1. Do not use the std_logic_arith and std_logic_unsigned libraries - they are not IEEE standard, and can really hurt beginners because it takes away a lot of the strong types the VHDL provides. Use numeric_std instead. An internet search can tell you more.
2. The "modern" macro rising_edge() is preferred for clock checking because it also handles the L->H state (which in an FPGA/ASIC never happens, but anyway...)
3. You have a iNextState which seems to be unused.
4. You have a boundary condition where none of the LCD outputs are set on startup. Either add a reset to the FSM, or you need to re-think the way the outputs are set (cf. Mealy vs Moore style FSMs).
5. If you have an enumeration for initialization_state you might want to give them more meaningful names and/or comment on what they're supposed to be doing at each state.

Otherwise, a quick look seems fine. As others noted, 750K clocks is a long simulation, and without seeing the testbench code that drives the clock, we have no idea if it will work. I'd recommend switching the 750K and all the others into constants that you can change to very small values for testing purposes.