Latches... Latches everywhere

[mobics]

I recently started on a new design using VHDL. VHDL is a long time ago for me, so now I really don´t now why this is happening, I tried a lot of thing to solve, but probably I´m just missing something simple.

I have two state machines for reading a sensor and compensating it with a value. For this I made two frame buffers for reading and two frame buffers for compensating. This is most of my source:

architecture Behavioral of top is

-------------------------------------
-- Camera sensor state types:
-- sIdle: Sensor wacht op actie. [ gaat naar: sIdle, sInit, sRead ]
-- sInit: Initialiseren/instellen van sensor [ gaat naar: sInit, sIdle ]
-- sRead: Leest huidige waarde van sensor op rising edge van de pxl clock [ gaat naar: sRead, sGoNext ]
-- sGoNext: Verhoogt de huidige pixel counter en checkt of er nog nieuwe pixels komen binnen het frame [ gaat naar sRead, sFinish ]
type 		tSensorState is (sIdle,sInit,sRead,sGoNext);

-------------------------------------
-- Compensatie state types:
-- sIdle: Compensatie proces wacht op actie [ gaat naar sIdle, sComp ]
-- sComp: Compenseert pixel: Compenseert de huidige pixel [ gaat naar sGoNext ]
-- sGoNext: Verhoogt de huidige pixel counte ren checkt of er nog nieuwe pixels komen binne het frame [ gaat naar sComp, sFinish ]
type		tCompState is (sIdle, sComp, sGoNext);

-- Camera state machine signals
signal 	currentSensorState, nextSensorState: tSensorState := sIdle;
-- Compensate state machine signals
signal 	currentCompState, nextCompState: tCompState := sIdle;

-- frame ready signals
signal	readFrameReady: STD_LOGIC := '0';
signal 	compFrameReady: STD_LOGIC := '0';

-- buffers voor frames. elk type zijn 2 buffers van 2048 10 bit getallen
type 		tFrameBuffer is array(1 downto 0, 2047 downto 0) of integer range 0 to 1023;
-- compensatie frame type 2048 10bit getallen
type 		tCompFrame  is array(2047 downto 0) of integer range 0 to 1023;

-- Lees buffers
shared variable 	readFrameBuffer	: tFrameBuffer := (OTHERS => (OTHERS=>0));
-- Compensatie buffer
shared variable 	compFrameBuffer	: tFrameBuffer := (OTHERS => (OTHERS=>0));
-- Compensatie frame
shared variable 	compFrame			: tCompFrame := (OTHERS=>0);

-- Frame buffer teller
shared variable 	atFrameBuffer		: integer range 0 to 1 := 0;

-- Lees pixel teller
shared variable 	readAtPixel			: integer range 0 to 2047 := 0;
shared variable 	compAtPixel			: integer range 0 to 2047 := 0;

begin

	

------------------------------------
-- State machine schakelaar
	clkProcess: process (CLKin)
	begin
		if (rising_edge(CLKin)) then
			currentSensorState <= nextSensorState;
			currentCompState <= nextCompState;
		end if;
	end process clkProcess;

------------------------------------
-- Sensor state machine
	process(currentSensorState, DLISKpclk, DLISKfvalid)
	begin
		case currentSensorState is
			when sIdle =>
				readFrameReady <= '0';
				readAtPixel := 0;
				if (DLISKfvalid = '1') then
					nextSensorState <= sRead;
				else
					nextSensorState <= sIdle;
				end if;
				
			when sInit =>
				nextSensorState <= sIdle;
				
			when sRead =>
				if (rising_edge(DLISKpclk)) then
					readFrameBuffer(atFrameBuffer,readAtPixel) := to_integer(unsigned(DLISKdata));
					nextSensorState <= sGoNext;
				else
					readFrameBuffer(atFrameBuffer,readAtPixel) := 0;
					nextSensorState <= sRead;
				end if;
			
			when sGoNext =>
				if (readAtPixel = 2047) then
					readAtPixel := 0;
					readFrameReady <= '1';
					nextSensorState <= sIdle;
					if (atFrameBuffer = 0) then
						atFrameBuffer := 1;
					else
						atFrameBuffer := 0;
					end if;
				else
					readAtPixel := readAtPixel + 1;
					nextSensorState <= sRead;
				end if;
			
			
		end case;
	end process;




-----------------------------
-- Compensatie state machine
	compensateProcess: process (currentCompState, readFrameReady)
	begin
		case currentCompState is
		when sIdle =>
			if (readFrameReady = '1') then
				nextCompState <= sComp;
			else
				nextCompState <= sIdle;
			end if;
		
		when sComp =>
			compFrameBuffer(atFrameBuffer,compAtPixel) := readFrameBuffer(atFrameBuffer+1,compAtPixel) * compFrame(compAtPixel);
			nextCompState <= sGoNext;		
			
		when sGoNext =>
			if (compAtPixel = 2047) then
				compAtPixel := 0;
				if (compFrameBuffer(atFrameBuffer,readAtPixel) = 100) then
					DLISKsenb <= '1';
				else
					DLISKsenb <= '0';
				end if;
				nextCompState <= sIdle;
			else
				compAtPixel := compAtPixel + 1;
				nextCompState <= sComp;
			end if;
			
		when others =>
			nextCompState <= sIdle;
			
		end case;
	end process compensateProcess;


end Behavioral;

And this source gives me 90000+ warnings because of latches like:

WARNING:Xst:2999 - Signal 'compFrame', unconnected in block 'top', is tied to its initial value.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameReady>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<10>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<9>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<8>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<7>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<6>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<5>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<4>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<3>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<2>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<1>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readAtPixel<0>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><9>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><8>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><7>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><6>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><5>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><4>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><3>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><2>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><2047><1>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.

.......

WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><0><2>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><0><1>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<1><0><0>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<0><2047><9>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<0><2047><8>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<0><2047><7>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.
WARNING:Xst:737 - Found 1-bit latch for signal <readFrameBuffer<0><2047><6>>. Latches may be generated from incomplete case or if statements. We do not recommend the use of latches in FPGA/CPLD designs, as they may lead to timing problems.

....

[muzaffer]

I think your best bet is to move to a single process state machine ie change your code so that there is only one clocked process which manages all state transitions and memory storage. That should get rid of the latch problem. Then you probably will have block ram inferrence vs massive flop usage but let's cross that bridge when we come to it. Also make sure that you simulate your design and see that it works properly under the simulator before you try to synthesize it.

[mobics]

Hmmm.. Thanks for the answer so far.

I was also just looking at my source and I don't really know why I'm using an extra process for compensating at all. I'll try to make an improved source.

[rcingham]

> if (rising_edge(DLISKpclk)) [in your 2nd process]

NO! NO!! NO!!!

[mobics]

Why not?

I simplified my software now a lot and there are no latches anymore

-- State machine schakelaar
	clkProcess: process (CLKin)
	begin
		if (rising_edge(CLKin)) then
			currentSensorState <= nextSensorState;
		end if;
	end process clkProcess;

------------------------------------
-- Sensor state machine
	process(currentSensorState, DLISKpclk, DLISKfvalid)
	begin
		case currentSensorState is
			when sIdle =>
				readAtPixel := 0;
				if (DLISKfvalid = '1') then
					nextSensorState <= sRead;
				else
					nextSensorState <= sIdle;
				end if;
				
			when sRead =>
				if (rising_edge(DLISKpclk)) then
					FrameBuffer(readAtPixel) := to_integer(unsigned(DLISKdata(9 downto 0))) * compFrame(readAtPixel);
					if (readAtPixel = 2047) then
						nextSensorState <= sIdle;
					else
						readAtPixel := readAtPixel + 1;
						nextSensorState <= sRead;
					end if;

					-- do something with the output
					if (FrameBuffer(readAtPixel) = 100) then
						DLISKsenb <= '1';
					else
						DLISKsenb <= '0';
					end if;
					------
				end if;
			
		end case;
	end process;

 But also use it here. Still have a warning:

Xst:3002 - This design contains one or more registers/latches that are directly
incompatible with the Spartan6 architecture. The two primary causes of this is
either a register or latch described with both an asynchronous set and
asynchronous reset, or a register or latch described with an asynchronous
set or reset which however has an initialization value of the opposite
polarity (i.e. asynchronous reset with an initialization value of 1).

Is this because of that? And how should I fix it? Want it to happen once and only in that state.

[rcingham]

Currently that process in partially combinational and partially synchronous, and adds a big fat brown and smelly Clock Domain Crossing hazard to your logic.

If 'DLISKpclk' is slower than 'CLKin', use a double flip-flop edge detector (after metastability mitigation, of course).

[bassman59]

---

@mobics wrote:

Hmmm.. Thanks for the answer so far.

I was also just looking at my source and I don't really know why I'm using an extra process for compensating at all. I'll try to make an improved source.

---

Definitely re-write the state machine to use one synchronous process. Two-process state machines are a FAIL. Your latches are the result of not assigning to all left-hand-side signals in ALL states in a combinatorial process.

Plus the rising_edge(DLISKpclk) doesn't do what you expect. If you want an edge detector, you need to code a delay flop and compare it with the current signal value.

Finally: Why are you using shared variables?

[mobics]

Oke added the changes and simplified is even more:

- only one process for the state machine

- created a rising edge flipflop for the pxlClk

- shared variables was because of multiple process. Now they are in the proces.

- added a reset (but want to remove this one in the future)

type 		tSensorState is (sIdle,sRead);

-- buffers voor frames. elk type zijn 2 buffers van 2048 10 bit getallen
--type 		tFrameBuffer is array(1 downto 0, 2047 downto 0) of integer range 0 to 1023;
type 		tFrameBuffer is array(2100 downto 0) of integer range 0 to 1023;

-- Camera state machine signals
signal 	currentSensorState, nextSensorState: tSensorState := sIdle;

-- frame ready signals
signal	readFrameReady: STD_LOGIC := '0';
signal	risingEdgePxlCLK: STD_LOGIC := '0';



begin

	

------------------------------------
-- State machine schakelaar
	clkProcess: process (CLKin,reset)
	variable resync : std_logic_vector(0 to 1);
	begin
		if (reset = '1') then
			currentSensorState <= sIdle;
			risingEdgePxlCLK <= '0';
			resync := "00";
		elsif (rising_edge(CLKin)) then
			currentSensorState <= nextSensorState;
			resync(1) := resync (0);
			resync(0) := DLISKpclk;
			risingEdgePxlCLK <= resync(0) and not resync(1);
		end if;
	end process clkProcess;

------------------------------------
-- Sensor state machine
	process(currentSensorState, risingEdgePxlCLK, DLISKfvalid, DLISKdata)
	variable 	FrameBuffer	: tFrameBuffer := (OTHERS=>0);
	variable 	readAtPixel			: integer range 0 to 2100 := 0;
	begin
		case currentSensorState is
			when sIdle =>
				readAtPixel := 0;
				FrameBuffer := (OTHERS=>0);
				
				if (DLISKfvalid = '1') then
					nextSensorState <= sRead;
				else
					nextSensorState <= sIdle;
				end if;
				
			when sRead =>
				if (risingEdgePxlCLK = '1') then
					FrameBuffer(readAtPixel) := to_integer(unsigned(DLISKdata(9 downto 0)));-- * compFrame(readAtPixel);
					
					if (readAtPixel = 2100 or DLISKfvalid = '0') then
						readAtPixel := readAtPixel;
						nextSensorState <= sIdle;
					else
						readAtPixel := readAtPixel + 1;
						nextSensorState <= sRead;
					end if;

				end if;
			
		end case;
	end process;

 This works in the simulator.

Synthesize also works, but that is normal. There is no output so the RTL block is empty.

Any comments on this so far?

[rcingham]

> Any comments on this so far?

ALWAYS name your processes. And remember that concurrent signal assignments and procedure calls are equivalent to processes...

[bassman59]

---

@mobics wrote:

Oke added the changes and simplified is even more:

- only one process for the state machine

---

Nope, still two processes. You're still decoding the states and determining the next state in a combinatorial process and setting the state register in a synchronous process. Put everything in the synchronous process.

---

- shared variables was because of multiple process. Now they are in the proces.

---

Why are you using variables in the process? Use signals. 

---

- added a reset (but want to remove this one in the future)

---

Synchronous resets are your friend.

[mobics]

Oke thanks for all tips guys!

architecture Behavioral of top is

-------------------------------------
-- Camera sensor state types:
-- sIdle: Sensor wacht op actie. [ gaat naar: sIdle, sInit, sRead ]
-- sInit: Initialiseren/instellen van sensor [ gaat naar: sInit, sIdle ]
-- sRead: Leest huidige waarde van sensor op rising edge van de pxl clock [ gaat naar: sRead, sGoNext ]
-- sGoNext: Verhoogt de huidige pixel counter en checkt of er nog nieuwe pixels komen binnen het frame [ gaat naar sRead, sFinish ]
type 		tSensorState is (sIdle,sRead);

-- buffers voor frames. elk type zijn 2 buffers van 2048 10 bit getallen
--type 		tFrameBuffer is array(1 downto 0, 2047 downto 0) of integer range 0 to 1023;
type 		tFrameBuffer is array(2100 downto 0) of integer range 0 to 1023;

signal 	FrameBuffer		: tFrameBuffer := (OTHERS=>0);
signal 	readAtPixel		: integer range 0 to 2100 := 0;

-- Camera state machine signals
signal 	SensorState: tSensorState := sIdle;

-- frame ready signals
signal	readFrameReady: STD_LOGIC := '0';
signal	risingEdgePxlCLK: STD_LOGIC := '0';
signal	risingEdgeCountWheel: STD_LOGIC := '0';

signal clk250MHz: STD_LOGIC;
signal clk50MHz:	STD_LOGIC;

component clockDiv is
port
 (-- Clock in ports
  CLK_IN          : in     std_logic;
  -- Clock out ports
  CLK_250         : out    std_logic;
  CLK_50          : out    std_logic
 );
end component;



begin

	
	clknetwork : clockDiv
	port map
   (-- Clock in ports
		CLK_IN         => CLKin,
		-- Clock out ports
		CLK_250        => clk250MHz,
		CLK_50         => clk50MHz
	 );
	 
	 
	 DLISKmclk <= clk50MHz;
	 	 

------------------------------------
-- State machine switcher
	clkProcess: process (clk250MHz,reset)
	variable pixelMask : std_logic_vector(0 to 1);
	variable countWheelMask : std_logic_vector(0 to 1);
	variable delayCounter : integer range 10 downto 0;
	begin
		if (reset = '0') then
			risingEdgePxlCLK <= '0';
			risingEdgeCountWheel <= '0';
			pixelMask := "00";
			countWheelMask := "00";
			delayCounter := 0;
		elsif (rising_edge(clk250MHz)) then
			pixelMask(1) := pixelMask (0);
			pixelMask(0) := DLISKpclk;
			countWheelMask(1) := countWheelMask (0);
			countWheelMask(0) := Telwiel;
			
			risingEdgePxlCLK <= pixelMask(0) and not pixelMask(1); 
			
			if (countWheelMask(0) = '1' and countWheelMask(1) = '0') then
				delayCounter := 5;
			end if;
			
			if (delayCounter > 0) then
				delayCounter := delayCounter - 1;
				risingEdgeCountWheel <= '1';
			else
				delayCounter := 0;
				risingEdgeCountWheel <= '0';
			end if;
		end if;
	end process clkProcess;

------------------------------------
-- Sensor state machine
	stateMachine: process(clk250MHz,reset,SensorState, risingEdgePxlCLK, DLISKfvalid, DLISKdata)
	begin
	
		if (reset='0') THEN
			SensorState <= sIdle;
			DLISKstart <= '0';
			DLISKsenb <= '0';
		elsif (rising_edge(clk250MHz)) THEN
		
					
			case SensorState is
			
				when sIdle =>
					readAtPixel <= 0;
					FrameBuffer <= (OTHERS=>0);
					
					if (risingEdgeCountWheel = '1') then
						DLISKstart <= '1';
					else
						DLISKstart <= '0';
					end if;
					
					if (DLISKfvalid = '1') then
						SensorState <= sRead;
					else
						SensorState <= SensorState;
					end if;
					
				when sRead =>
					if (risingEdgePxlCLK = '1') then
						FrameBuffer(readAtPixel) <= to_integer(unsigned(DLISKdata(9 downto 0)));-- * compFrame(readAtPixel);
						
					if (FrameBuffer(readAtPixel) = 200) then
						DLISKsenb <= '1';
					else
						DLISKsenb <= '0';
					end if;
			
						if (readAtPixel = 2100 or DLISKfvalid = '0') then
							readAtPixel <= readAtPixel;
							SensorState <= sIdle;
						else
							readAtPixel <= readAtPixel + 1;
							SensorState <= SensorState;
						end if;

					end if;
				
			end case;
			
		end if;
	end process stateMachine;


end Behavioral;

 1 process state machine.

Names for processes

Delays for some signals.

No more latches and simulation is also working as supposed so far!

But, of course: Xst:1336 - (*) More than 100% of Device resources are used

Comments are still welcome!

Now going to find out how to use the Block Ram memory.

[muzaffer]

One final comment from me would be that when you implement a single process your dependency can and should include only the clock and an asynchronous reset, nothing else. Everything is sampled on clock edge so your process doesn't need any other triggers.

[bassman59]

---

@mobics wrote:

No more latches and simulation is also working as supposed so far!

But, of course: Xst:1336 - (*) More than 100% of Device resources are used

---

Here's why:

type tFrameBuffer is array(2100 downto 0) of integer range 0 to 1023;

signal FrameBuffer : tFrameBuffer := (OTHERS=>0);

That's 2.15 megabits. I'm not sure what device you're targeting.  You do need one of the very large FPGAs. And my guess is you're not getting a proper RAM inference, hence the usage blows up. The tools can be kinda dumb, so it might be easiest to separate out the BRAM inference from the state machine.

[mobics]

Bassman:

You mean 2,6kB?! 2101 * 10 bit = 21010 bit / 8 is about 2,6 kB. This is not that much?!

I will also make it of a single sync and async process.

[muzaffer]

Can you post your utilization report? What device are you using?

[bassman59]

---

@mobics wrote:

Bassman:

You mean 2,6kB?! 2101 * 10 bit = 21010 bit / 8 is about 2,6 kB. This is not that much?!

---

Whoops, sorry, you're right.