FPGA Central

Hi everybody,

I have written some little code for a FIFO and I want
to know whether the signals "l_fifo_full" / "l_fifo_empty" are
correct generated.

I would appreciate your opinion.
Thanx in advance.

Here is the code


library ieee;

use ieee.std_logic_1164.all;
use ieee.std_logic_arith.all;


entity frame_fifo is
port( Reset : in std_logic;
Clk : in std_logic;
Data_in : in std_logic_vector(7 downto 0);
Data_in_valid : in std_logic;
Take_data_out : in std_logic;
Erase : in std_logic;
Fifo_full : out std_logic;
Fifo_empty : out std_logic;
Data_out : out std_logic_vector(7 downto 0);
Data_out_valid : out std_logic
);
end frame_fifo;


architecture rtl of frame_fifo is

signal l_fifo_full, next_l_fifo_full : std_logic;
signal l_fifo_empty, next_l_fifo_empty : std_logic;

signal l_data_out : std_logic_vector(7 downto 0);
signal l_data_out_valid : std_logic;

constant ROWS : integer := 16;
constant COLS : integer := 8;

type matrix_buffer is array(0 to ROWS-1, 0 to COLS-1) of std_logic;
signal l_write_array : matrix_buffer;

signal l_read_pointer : integer range 0 to ROWS-1;
signal l_rp_reg : integer range 0 to ROWS-1;
signal l_write_pointer : integer range 0 to ROWS-1;

signal l_wp_turn_around : std_logic;

begin

Fifo_full <= l_fifo_full;
Fifo_empty <= l_fifo_empty;
Data_out <= l_data_out;
Data_out_valid <= l_data_out_valid;

----------------------------------------------------------------------
----------------------------------------------------------------------
-- write FIFO
process(Reset, Clk)
begin
if Reset='1' then
l_write_array <= (others => (others => '0'));
l_write_pointer <= 0;

elsif rising_edge(Clk) then
l_write_array <= l_write_array;
l_write_pointer <= l_write_pointer;

if ((Data_in_valid='1') and (next_l_fifo_full='0')) then

l_write_array(l_write_pointer, 7) <= Data_in(7);
l_write_array(l_write_pointer, 6) <= Data_in(6);
l_write_array(l_write_pointer, 5) <= Data_in(5);
l_write_array(l_write_pointer, 4) <= Data_in(4);
l_write_array(l_write_pointer, 3) <= Data_in(3);
l_write_array(l_write_pointer, 2) <= Data_in(2);
l_write_array(l_write_pointer, 1) <= Data_in(1);
l_write_array(l_write_pointer, 0) <= Data_in(0);

end if;

if (l_fifo_full='1')) then
l_write_pointer <= 0;
end if;

end if;
end process;
----------------------------------------------------------------------
----------------------------------------------------------------------
-- read FIFO
process(Reset, Clk)
begin
if Reset='1' then
l_data_out <= (others => '0');
l_data_out_valid <= '0';
l_read_pointer <= 0;
l_rp_reg <= 0;

elsif rising_edge(Clk) then
l_data_out <= l_data_out;
l_data_out_valid <= '0';
l_read_pointer <= l_read_pointer;
l_rp_reg <= l_read_pointer;

if Take_data_out='1' then

l_data_out(7) <= l_write_array(l_read_pointer, 7);
l_data_out(6) <= l_write_array(l_read_pointer, 6);
l_data_out(5) <= l_write_array(l_read_pointer, 5);
l_data_out(4) <= l_write_array(l_read_pointer, 4);
l_data_out(3) <= l_write_array(l_read_pointer, 3);
l_data_out(2) <= l_write_array(l_read_pointer, 2);
l_data_out(1) <= l_write_array(l_read_pointer, 1);
l_data_out(0) <= l_write_array(l_read_pointer, 0);


l_data_out_valid <= '1';

end if;

if (l_fifo_full='1')) then
l_read_pointer <= 0;
end if;

end if;
end process;
----------------------------------------------------------------------
----------------------------------------------------------------------
-- FIFO full/empty
-- clk transitions
process(Reset, Clk)
begin
if Reset='1' then
l_fifo_full <= '0';
l_fifo_empty <= '0';

elsif rising_edge(Clk) then
l_fifo_full <= next_l_fifo_full;
l_fifo_empty <= next_l_fifo_empty;

end if;
end process;
----------------------------------------------------------------------
-- FIFO full/empty
process(l_fifo_full, l_fifo_empty, l_wp_turn_around, l_write_pointer,
l_read_pointer, l_rp_reg)
begin
next_l_fifo_full <= '0';
next_l_fifo_empty <= '0';

if ((l_wp_turn_around='1') and (l_write_pointer=l_read_pointer) and
(l_read_pointer=l_rp_reg)) then
next_l_fifo_full <= '1';
end if;

if ((l_read_pointer=l_write_pointer) and (l_read_pointer>0)) then
next_l_fifo_empty <= '1';
end if;

end process;
----------------------------------------------------------------------
----------------------------------------------------------------------
-- This signal indicates whether the write pointer has arrived at
-- the maximum value (ROWS-1) and turned back to zero
process(Reset, Clk)
begin
if Reset='1' then
l_wp_turn_around <= '0';

elsif rising_edge(Clk) then
l_wp_turn_around <= l_wp_turn_around;

if (l_write_pointer=ROWS-1) then
l_wp_turn_around <= '1';
elsif (Erase='1') then
l_wp_turn_around <= '0';
end if;

end if;
end process;
----------------------------------------------------------------------
----------------------------------------------------------------------

end rtl;

ALuPin wrote:

> I have written some little code for a FIFO and I want
> to know whether the signals "l_fifo_full" / "l_fifo_empty" are
> correct generated.

If I understand your code correctly, then I'd say: no. From what I
understand you have a FIFO with one clock, and a read/write enable. You use
the FIFO as an elastic store, running the read/write addresses from 0 to
ROW-1, and round.

Ok, before I dive into your code: please, please, please, please don't use
asynchronous resets. Ok, now that I got that off my chest I can comment on
your code without hurting myself.

> process(Reset, Clk)
> begin
<snip>
> if (l_fifo_full='1')) then
> l_write_pointer <= 0;
> end if;

Ok, I don't understand why you'd want to put your write pointer to 0 when
your fifo is full. Do you just want a reset situation whenever your fifo is
full? Or do you want to do this when you cause an overflow, which would
mean writing into the FIFO when it is full. In that case it should be moved
inside the write if.

> process(Reset, Clk)
> begin
<snip>
> if (l_fifo_full='1')) then
> l_read_pointer <= 0;
> end if;

Something similar here, though I'd expect you to reset the FIFO if you try
to read from an empty FIFO. In this case you should be checking for empty,
and place it within the read if construction.


> process(Reset, Clk)
> begin
<snip>
> elsif rising_edge(Clk) then
> l_fifo_full <= next_l_fifo_full;
> l_fifo_empty <= next_l_fifo_empty;

Why do you wish to delay these indications for 1 clock cycle? Wouldn't you
want to use this information as soon as it is available? The next signals
are already clocked as far as I could see.

> process(l_fifo_full, l_fifo_empty, l_wp_turn_around, l_write_pointer,
> l_read_pointer, l_rp_reg)
> begin
<snip>
> if ((l_wp_turn_around='1') and (l_write_pointer=l_read_pointer) and
> (l_read_pointer=l_rp_reg)) then
> next_l_fifo_full <= '1';
> end if;

You define the FIFO as full if you have a wrap around (why is this a
prerequisite?), your write pointer equals your read pointer, and you didn't
do a read action this time. What if you didn't have a read action nor a
write action? It could be that the FIFO was and still is empty?

> if ((l_read_pointer=l_write_pointer) and (l_read_pointer>0)) then
> next_l_fifo_empty <= '1';
> end if;

Why would the read pointer have to be > 0?

> process(Reset, Clk)
> begin
<snip>
> elsif (Erase='1') then
> l_wp_turn_around <= '0';
> end if;

This is the first time I see the erase used. Wouldn't you want to reset the
read and write pointers at such a time as well?

Regards,

Pieter Hulshoff

ALuPin wrote:

> I have written some little code for a FIFO and I want
> to know whether the signals "l_fifo_full" / "l_fifo_empty" are
> correct generated.
> I would appreciate your opinion.

It's not a matter of opinion,
it's a matter of simulation.
Consider writing a testbench, and your
question will be answered.

-- Mike Treseler

Hi ALuPin, I saw your code and i guess that its can be writen more compact.
I'm not sure that your logic is totaly corectly. Do you really to specify
RESET in different process? If you'd put this signal in sample process and
make code more readable you will fine the answer of your question. My
opinion is that these signals are incorrectly defined. I can't understand
what actually you do when comein ERASE comand. Would you tell me, what is
going to happen if your FIFO is half fill in and ERASE going to be "1"?
Are you going to erase all cells of your FIFO.?
On the end rearange your cade and do it more readable and simple. But
:-)) I'm agree withMike Treseler, going to simulate and you will see youg
correctnes ir incorrectness
Best Regards:
Ivaylo Krumov

Here is code I've written about a year ago. It looks much simpler.

entity FIFO_CELL is
generic (
WIDTH : Integer := 8
);
port (
CLK : in std_logic;
RST : in std_logic;
EN : in std_logic;
Write: in std_logic; -- Writing puse from provider
Ack: out std_logic; -- signal to provider on Write, '1' set when not Avail
Read: in std_logic; -- Cleaning pulse from consumer
Avail: out std_logic; -- not full flag
DIN: in std_logic_vector(WIDTH-1 downto 0);
DOUT: out std_logic_vector(WIDTH-1 downto 0)
);
end FIFO_CELL;


-- signal passes through the cell if next cell in the chain is empty
architecture ADVANCED of FIFO_CELL is
--signal AvailReg: std_logic;
signal Data: std_logic_vector(WIDTH-1 downto 0);
signal Empty, Cleaning: std_logic;
begin

Cleaning <= Empty or Read; -- empty next cycle

CLOCK : process(Clk, RST)
begin

if RST = '1' then
Empty <= '1';
elsif Rising_Edge(Clk) and EN = '1' then

Empty <= ((not Write) and Cleaning) or (Empty and Read);

if Cleaning = '1' then
Data <= DIN;
end if;

end if; -- CLK

end process;

Ack <= Write and Cleaning;
Avail <= not Empty or Write;
DOUT <= DIN when (Write and Empty) = '1' else Data;


end ADVANCED;






--cascade FIFO_CELLs into a n-DEPTH chain.
-- data consumed from the first cell and written into the first free cell.
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;

entity FIFO is
generic (
WIDTH : Integer := 8;
DEPTH : Integer := 4
);
port (
CLK : in std_logic;
RST : in std_logic;
EN : in std_logic;
Write: in std_logic; -- Writeing puse
Ack: out std_logic; -- '1' set on Write (if not Avail), Readed on accept
Read: in std_logic; -- Cleaning pulse
Avail: out std_logic; -- flag
DIN: in std_logic_vector(WIDTH-1 downto 0);
DOUT: out std_logic_vector(WIDTH-1 downto 0)
);
end FIFO;


architecture RTL of FIFO is
type TARRAY is array (0 to DEPTH) of std_logic_vector (WIDTH-1 downto 0);
signal WR : STD_LOGIC_VECTOR(0 to DEPTH);
signal RD : STD_LOGIC_VECTOR(0 to DEPTH);
signal DATA : TARRAY;
begin

VECTOR: for I in 0 to DEPTH-1 generate

FIFO_CELL : entity work.FIFO_CELL(ADVANCED)
generic map (
WIDTH => WIDTH
)
port map (
Clk => Clk,
RST => RST,
EN => EN,
Write => WR(I),
Ack => RD(I),
Read => RD(I+1),
Avail => WR(I+1),

--todo: replace this
DIN => DATA(I),
DOUT => DATA(I+1)
);

end generate;

WR(0) <= Write;
Ack <= RD(0);
Avail <= WR(DEPTH);
RD(DEPTH) <= Read;
Data(0) <= DIN;
DOUT <= Data(DEPTH);

end RTL;