cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 
macellan85
Explorer
Explorer
1,233 Views
Registered: ‎01-05-2017

problem with writing into and reading from a FIFO

Jump to solution

Hello,

Depending on the testbenches and simulation results eveything is fine with available fifo designs I got from the web. But when I try to write a synthesizble code (below) to write data into the fifo and read from there I couldn' t make it work. Because "full" flag never set so far and read attempt didn' t give anything. The main difference I observed here is available testbenches use wait blocks.

Could anyone please have a look at my "fifo controller" below and let me know what is the problem and why I can' t observe full flag is set?

p.s Please let me know if you have suggestion for a better design as well.

Best wishes

 

 process(clk, state, press)
    begin        
        if ( rising_edge(clk) ) then            
            case state is                
                when ST_IDLE    =>
                    ST <= "00";
                    if ( press = '1' ) then
                        state <= ST_WRITE;
                    else
                        state <= ST_IDLE;
                    end if;                    
                when ST_WRITE   =>
                    ST <= "01";            
                    if ( s_index < 20 and s_id = 1) then    -- there are 16 availlabe place in fifo and I expect "full" flag to set after 15
                        s_wr_en     <= '1' ;
                        s_data_in   <= std_logic_vector(to_unsigned(s_index*2, data_width));
                        s_index     <= s_index + 1 ;                        
                    elsif ( s_index >= 20 ) then
                        s_index <= 0;
                        s_wr_en <= '0';
                        state   <= ST_IDLE;                        
                    else
                        s_wr_en <= '0';                     
                    end if; 
                    if ( s_flag_cntr < 10 ) then     -- kind of a delay to keep wr_en set for 2 periods
                        s_flag_cntr <= s_flag_cntr + 1 ;
                        s_id        <= 0;                        
                    else
                        s_flag_cntr <= 0;
                        s_id        <= 1;                        
                    end if;  
            end case;            
        end if;

 

 

Tags (1)
0 Kudos
1 Solution

Accepted Solutions
macellan85
Explorer
Explorer
661 Views
Registered: ‎01-05-2017

Dear @richardhead  and @dgisselq thank you for your time and support. This is solved !

First of all I did some more simulations by instantiating the real module in the testbench and got the same result as in previous.

Yes using oscilloscope for such purpose is not easy but useful since I don' t have a external logic analyser yet. In case of internal logic analyser, I need some more time to understand how it works. First test I did was to observe the "data part" values (the transmitted 8 Bit data parts) if they are loading correct. However even I select the auto trigger I couldn' t observe them. For 115200 baudrate I expect to see 13 bytes, not achieved. Probably, I didn' t get the working principle of it and I' ll look for that again later, I know it is a useful tool.

All in all, what I did finally and made the design work is as follows: 

ST_WRITE:

I set if clause as below

if ( s_full = '0' and  s_index < d_DEPTH ) then

I didn' t clearly understand why s_full is not enough here by itself. When I leave it alone, loop misses last data part. I' ll maybe look at that later. 

ST_READ: 

1st modification, I set elsif clause set as below

elsif ( s_empty = '0' and s_index_2 < d_DEPTH + 1 ) then

in here s_empty is enough to perform the task correct!

2nd modification, I removed s_rd_en from below part

 if    ( s_part_index = 0 ) then
	s_rd_en      <= '1';
	Data_Part    <= s_data_out(23 downto 16);
	Sent_Flag    <= '1';
	s_part_index <= s_part_index + 1;
elsif ( s_part_index = 1 ) then
	s_rd_en      <= '0';
	Data_Part    <= s_data_out(15 downto  8);
	Sent_Flag    <= '1';
	s_part_index <= s_part_index + 1;
elsif ( s_part_index = 2 ) then
	s_rd_en      <= '0';
	Data_Part    <= s_data_out( 7 downto  0);
	Sent_Flag    <= '1';
	s_part_index <=  0 ;
	s_index_2    <= s_index_2  + 1;
else
	Sent_Flag    <= '0';
	s_part_index <=  0 ;
	s_index_2    <=  0 ;
end if;

 

Best wishes

 

 

View solution in original post

0 Kudos
14 Replies
macellan85
Explorer
Explorer
1,171 Views
Registered: ‎01-05-2017

 

Really no one has any idea??

 

0 Kudos
richardhead
Scholar
Scholar
1,130 Views
Registered: ‎08-01-2012

The code you posted doesn't really relate to the problem. What testbench have you run?

Usually you wouldn't have a state machine for a fifo.  Usually you just have read and write pointers and take the difference for the fill level.  This means you can write and read on every clock.

0 Kudos
macellan85
Explorer
Explorer
1,055 Views
Registered: ‎01-05-2017

Dear @richardhead 

Below I attached the final version of the "fifo controller".

 I expect it to write some measurement data into the FIFO when a period of time is over. This happens at state ST_WRITE and then reads each data at another state called ST_READ and perform transmission on UART link. After read and during transmission I need to divide 24 bit data to 8 bit parts. Read process occurs depending on the line status. Thus there are two "index"es one for FIFO location (0-5) and the other for 8 bit parts (0-2) of each fifo location.

What occurs now, FIFO read enable doesn' t set and reset depending on the s_index_2 and s_part_index values but only for once. And also even for this single set there is no data output.

So, looking forward to hearing from you for any idea and help...

 

0 Kudos
richardhead
Scholar
Scholar
1,046 Views
Registered: ‎08-01-2012

Have you written a testbench? have you tested this case in simulation?

macellan85
Explorer
Explorer
1,040 Views
Registered: ‎01-05-2017

@richardhead 

Second file is the test bench version of the driver design. I did some other modifications and reuploaded by changing the name as well.

0 Kudos
macellan85
Explorer
Explorer
893 Views
Registered: ‎01-05-2017

I had some progress with this topic but the problem continue...

First of  all, fifo write process works very well. However read process doesn' t work properly in contrast with simulation. I checked the simulation logic and I can' t see any conflict between two codes. 

So the continuing problem is, read and transmission process works correct only for the first time. Later, it usually transmits the last data part but sometime the correct ones. So what can be the reason of this behaviour?

 

p.s. Each data has 24 bits and it is separated to three 8 bit parts. Thus, I have 2 indexes, one for counting the main data and the other for 8 bit parts. 

 

Looking forward to hearing from you...

0 Kudos
richardhead
Scholar
Scholar
883 Views
Registered: ‎08-01-2012

There are bugs in the testbench. The period_over_flag signal is not aligned with the clock and actually changes just before a clock edge (delta cycle problems). Instead of  waiting for specified periods of time, you should wait for N clock edges.

for i in in 1 to N loop
  wait until rising_edge(clk);  -- wait for N clocks
end loop;

I also cannot see how the testbench is anything other than a copy/paste of the FIFO Driver. Why is the testbench not driving the FIFO driver entity? I assume this is what is in the design, so why isnt the FIFO Driver code being tested?

Have you also investigated using chipscope? is a write ever attempted when the FIFO is full? does it hold off and wait until its not full? the same question for when its empty and attempting to read?

The FIFO code looks like a standard FIFO, so I dont think this is at fault - the fault will lie with the FIFO_Driver or surrounding components. If the FIFO is re-reading old data, you have probably lost some data into the FIFO by attempting to write data to a full FIFO.

dgisselq
Scholar
Scholar
873 Views
Registered: ‎05-21-2015

@macellan85,

First things first, there is a bug in your FIFO.  It may not be anything you've seen yet, but let's at least get that out of the way: You should never write data into the FIFO unless you are also changing your pointers.  As it is written now, if you fill the FIFO, and then try writing while the FIFO is full, the last item written to the FIFO will be corrupted.

Second, I think I saw an input named "press" in your earlier logic.  Are you triggering this design off of a button press?  If so, you should know that using buttons to trigger logic leads to two serious problems.  First, they can bounce.  Not only can they bounce, but they almost reliably do bounce--one button press will turn into 2-3 presses depending upon the hardware on your board.  Second, buttons are not triggered on clock edges, so you need to cross clock domains to get the button data into your design.  In one design, I (purposely) failed to cross clock domains and I was able to get a counter to count backwards.  Yes, these things do happen.  (I discuss this in lesson 7 of my Verilog beginner's tutorial .)

@richardhead's comment is a good one: do you have a trace from within your design showing the problem?  This is how digital designs are commonly debugged.  First, run your simulation.  Get your simulation looking like you expect.  *LOOK AT THE TRACE* from your simulation.  Get used to doing so.  Now repeat in hardware.  Again, *LOOK AT THE TRACE*.  You should be able to get a trace from within a running design by using Xilinx's internal logic analyzer, although it will require some setup to do so.  The trace should be able to tell you what your logic design is doing that it should (or shouldn't) be doing, allowing you to zero in on and then correct any problems.

Hope this helps,

Dan

macellan85
Explorer
Explorer
858 Views
Registered: ‎01-05-2017

@richardhead 

There are bugs in the testbench. The period_over_flag signal is not aligned with the clock and actually changes just before a clock edge (delta cycle problems). 


Attached pic is from the sim. and looks aligned??

I also cannot see how the testbench is anything other than a copy/paste of the FIFO Driver. Why is the testbench not driving the FIFO driver entity? I assume this is what is in the design, so why isnt the FIFO Driver code being tested?


Here I' ve inserted the driver code inside the tesbench file. Do you mean, instantiating the FIFO driver inside the testbench? if so is there any difference??


Have you also investigated using chipscope? is a write ever attempted when the FIFO is full? does it hold off and wait until its not full? the same question for when its empty and attempting to read?

Not chipscope but I used Logic Analyser and VIO. First attempts didn' t give me any useful info but thinking about to get in detail of it.


The FIFO code looks like a standard FIFO, so I dont think this is at fault - the fault will lie with the FIFO_Driver or surrounding components. If the FIFO is re-reading old data, you have probably lost some data into the FIFO by attempting to write data to a full FIFO.

Maybe I should follow the full and empty flags as well ??

So this maybe take question how a fifo driver design should be designed then?? I searched for examples for that but so far I met only test bench files which full of wait blocks. It would be nice to share your ideas for such a design.

 

pic1.png
0 Kudos
macellan85
Explorer
Explorer
852 Views
Registered: ‎01-05-2017

@dgisselq  thank you for the response and for sure helping

 


First things first, there is a bug in your FIFO.  It may not be anything you've seen yet, but let's at least get that out of the way: You should never write data into the FIFO unless you are also changing your pointers.  As it is written now, if you fill the FIFO, and then try writing while the FIFO is full, the last item written to the FIFO will be corrupted.


Can we summarize it as "follow the full flag when writing" ??

Here I used a block of static data. The received data on the computer is only correct for the first set as I said. So I think the problem is with the reading part?


Second, I think I saw an input named "press" in your earlier logic ...


"press"  is actually for "Period_Over_Flag". This time I uploaded whole code and not using any button. But I' ll read your blog for cross clock domain. 

@richardhead's comment is a good one: do you have a trace from within your design showing the problem?  ...

Yes there are some signals (rd_en, wr_en, full, empty, period_over_flag ...) I follow through oscilloscope. However, I couldn' t figure out what happens after the first data transmission... I reset s_index, s_index_2, s_part_index ...

This must not be this much of difficult but ???

 

0 Kudos
dgisselq
Scholar
Scholar
843 Views
Registered: ‎05-21-2015

@macellan85,


@macellan85 wrote:

@dgisselq  thank you for the response and for sure helping

 


First things first, there is a bug in your FIFO.  It may not be anything you've seen yet, but let's at least get that out of the way: You should never write data into the FIFO unless you are also changing your pointers.  As it is written now, if you fill the FIFO, and then try writing while the FIFO is full, the last item written to the FIFO will be corrupted.


Can we summarize it as "follow the full flag when writing" ??


Probably.  I was being generic because some FIFOs will allow writing and reading on the same cycle even if the FIFO is full.  Yours does not, so checking the full flag in addition to the write flag would fix your problem.


Yes there are some signals (rd_en, wr_en, full, empty, period_over_flag ...) I follow through oscilloscope. However, I couldn' t figure out what happens after the first data transmission... I reset s_index, s_index_2, s_part_index ...


If you are using an external oscilloscope and able to make it work for your project, then God bless you.  I would struggle to do so.  1) I rarely have enough fingers for all of the probes required, 2) you'd need to capture things and resynchronize them to the clock, 3) it helps to have a trigger, etc

An internal logic analyzer is often better: 1) no need for physical probes, 2) everything can come already synchronized to the clock, 3) you can make the trigger a part of your design, etc.

If you don't see a signal changing that should change, then make sure you add the signals that would cause it to change to your list of signals you are capturing and recording.  Hence, if A changes based upon B and C and if A isn't changing, then check whether or not B or C are happening.  If you then find B isn't happening when it should, and that B is dependent upon D and E, then add D and E to your list of captured signals.  Repeat then until you find the problem.  At least ... that's my secondary method for debugging my designs.


This must not be this much of difficult but ???

It's not difficult when you know the tools available to you.  I find most of my bugs via formal methods--to include the FIFO bug you had above.  (Yes, I've had that bug within my own designs before--formal methods caught it.)  What doesn't get caught via formal gets sent to a full simulation where I capture every signal into, out of, and within my design.  Only when something works in a full integrated simulation (i.e. the entire design), only then do I move to hardware.  When things don't work in hardware, I use an internal logic analyzer.  While I tend to use my own, Xilinx's should be good enough for most purposes and it's easier to set up than my own analyzer is.  Once you know how to chase a bug from its symptom (I can't read) to its cause (this line of code was in error), then debugging in general does get much easier.

Dan

richardhead
Scholar
Scholar
836 Views
Registered: ‎08-01-2012

@macellan85 

while the simulation waveform looks aligned, from the testbench code, you have a delta issue. In sumulation, because events naturally have to occur one after another, simulators work using a sequence of delta cycles - each one technically being an infinitely small amount of time. Whenever you assign a signal (rather than a variable) the value doesnt actually change immediately, it is scheduled to update at the end of the current delta cycle (or the end of one after a specified period of time). Simulators limit the number of deltas allowed at any given time via an "iteration limit". You may have accidently hit this when you accidently created a logic loop:

x <= y;
y <= x;

Signal assignments take 1 delta to propogate in simulation. So here, in delta 0, y gets updated to some value, which triggers a change in x on the next delta, which then triggers a change in y ad infinitum - until iteration limit reached.

So, when you create a clock:

process
begin
clk <= '1';
wait for 10 ns;
clk <= '0';
wait for 10 ns;
end process;

anything that uses if rising_edge clock will actually be triggered 1 delta after the actual change from 0 -> 1, because it takes the currrent value and sees if the value in the previous delta was 0. If you write this code for other signals:

some_sig <= '1' after clk_period;

some_sig now changes to '1' at the same time as the clock changes to '1', BUT any process that uses

if rising_edge(clk) then
  if some_sig = '1' then

will trigger 1 delta after some_sig already changed, and can appear as if it triggered at the wrong clock cycle. Hence the only thing that waits for a clock_period in the testbench is the clock. So be practice is to have everything else, including stimulus, wait for clock edges to change. Other gotchas can include re-assigning a clock to another signal, as that also incurs a delta cycle and can mess things right up (just dont do it).

richardhead
Scholar
Scholar
835 Views
Registered: ‎08-01-2012

@macellan85 

RE: your test code.

Whenever you have a testbench you ALWAYS instantiate the code you are testing in the testbench. Copy+paste will just give you two copies of the same code that could then diverge. Always instantiate the same code going into the design. This may mean you need a more complicated testbench, but that is the nature of testing.

macellan85
Explorer
Explorer
662 Views
Registered: ‎01-05-2017

Dear @richardhead  and @dgisselq thank you for your time and support. This is solved !

First of all I did some more simulations by instantiating the real module in the testbench and got the same result as in previous.

Yes using oscilloscope for such purpose is not easy but useful since I don' t have a external logic analyser yet. In case of internal logic analyser, I need some more time to understand how it works. First test I did was to observe the "data part" values (the transmitted 8 Bit data parts) if they are loading correct. However even I select the auto trigger I couldn' t observe them. For 115200 baudrate I expect to see 13 bytes, not achieved. Probably, I didn' t get the working principle of it and I' ll look for that again later, I know it is a useful tool.

All in all, what I did finally and made the design work is as follows: 

ST_WRITE:

I set if clause as below

if ( s_full = '0' and  s_index < d_DEPTH ) then

I didn' t clearly understand why s_full is not enough here by itself. When I leave it alone, loop misses last data part. I' ll maybe look at that later. 

ST_READ: 

1st modification, I set elsif clause set as below

elsif ( s_empty = '0' and s_index_2 < d_DEPTH + 1 ) then

in here s_empty is enough to perform the task correct!

2nd modification, I removed s_rd_en from below part

 if    ( s_part_index = 0 ) then
	s_rd_en      <= '1';
	Data_Part    <= s_data_out(23 downto 16);
	Sent_Flag    <= '1';
	s_part_index <= s_part_index + 1;
elsif ( s_part_index = 1 ) then
	s_rd_en      <= '0';
	Data_Part    <= s_data_out(15 downto  8);
	Sent_Flag    <= '1';
	s_part_index <= s_part_index + 1;
elsif ( s_part_index = 2 ) then
	s_rd_en      <= '0';
	Data_Part    <= s_data_out( 7 downto  0);
	Sent_Flag    <= '1';
	s_part_index <=  0 ;
	s_index_2    <= s_index_2  + 1;
else
	Sent_Flag    <= '0';
	s_part_index <=  0 ;
	s_index_2    <=  0 ;
end if;

 

Best wishes

 

 

View solution in original post

0 Kudos