FPGA (Verilog) - RGB pixels to SDRAM using FIFO

[Kyristor]

Hi everyone, let me preface by saying I'm a software engineer by day, and a hobbyist by night.
For this reason working with FPGA is new for me, and I can say it hasn't been the smoothest of roads for me 


There are a few concepts I'm really struggling with, so I hope somebody could help me out.
The basic concept I want to achieve is a 'video frame buffer', the input is a 15bit RGB signal with a pixel clock of around 2.3mhz
the 5 bits deep data needs to be converted to 8 bits deep and stored on the SDRAM. The SDRAM works on a 100mhz clock,
thats why in between I got an Asynchronous FIFO to handle the differents in clock speeds.


What I'm struggling with is how to place this data in the FIFO on every pixel clock pulse, and incrementing the SDRAM address at the same time/clock pulse.
This is the pseudo code I got right now: https://gist.github.com/LarsWerkman/66e65060e44f857ddcabd32aad67abab

My questions is, how can I be sure that I update the pointer of the 'address' consistently before/after a write to the FIFO? For me as a software developer it seems that it will try to do a write around the same time the 'address' will get incremented, causing concurrency issues.

Hope somebody could help me out on this or clarify

[asmi]

I suggest you to watch this video about how to work with FIFO:


In a nutshell, on a write side you need to check if fifo is full and only write to it if it's not, on a read side, you check if fifo is empty and only read from it if it's not. For FIFO itself you can just use a primitive from whatever FPGA vendor you're using. Also SDRAM (and especially DDR1/2/3 RAM!) heavily favor writing and reading in bursts, so for that FIFO primitives have programmable almost_full/almost_empty flags, which allows you to ensure that there are at least programmed_amount of free space to write/data entries to read respectively. All addressing within FIFO is taken care of by the primitive itself (which might be dedicated hardware modules, or hardware module with some logic depending on a FPGA you use).

[Kyristor]

Thanks, thats a great resource!
I think I do understand the concept of writing to a FIFO, but I get stuck on how I would perform certain actions on the same clock cycle that the FIFO is synced to.

Code: [Select]

wire[54:0] write_fifo_data;

assign write_fifo_data[0:31] = {8'hFF, red, green, blue};

wire [0:22] sdram_address = 0;
assign write_fifo_data[32:54] = sdram_address;

wire rst_n = ~rst;

wire full;
wire empty;
wire write_enable;
wire read_enable;

fifo #(.BUS_WIDTH(55)) wr_fifo(
.rst_n(rst_n),
.wr_clk(pixel_dclk),
.rd_clk(clock_100mhz),
.wr_data(write_fifo_data),
  .rd_data(read_fifo_data),
  .wr(write_enable),
  .rd(read_enable),
  .full(write_enable),
  .rst_n(rst_n),
  .empty(empty)
);

always@(posedge pixel_dclk) begin
  write_enable = 1;
  sdram_address <= sdram_address + 1;
end

The wr_clk of the FIFO is the pixel_dclk, this will make sure everytime the pixel_dclk is pulsed write_fifo_data is written to the FIFO, if write_enabled is high.
But on the same clock edge I need to increment the sdram_address which is part of the write_fifo_data.
This is done in the always@(posedge pixel_dclk), and here is where I'm confused how can I make sure this is done reliably always after or always before, the actual data gets written to the FIFO.

I hope this makes sense

[asmi]

But on the same clock edge I need to increment the sdram_address which is part of the write_fifo_data.

What makes you think that you need to do that? As I understand the SDRAM is on a read side of a FIFO, which is running on a different clock. So in that clock domain, you check fifo_empty flag, and if it's low (meaning there is a data to read inside FIFO) - you perform the read followed by the write to SDRAM and increment it's address.

[NorthGuy]

This is done in the always@(posedge pixel_dclk), and here is where I'm confused how can I make sure this is done reliably always after or always before, the actual data gets written to the FIFO.

You need to read something about how RTL logic works. All the combinatorial logic works at the same time, so there's no such thing as "before" or "after". It's rather "at this clock edge" or "at the next clock edge". At clock edges, the sequential elements (flip-flops, registers), latch their inputs and hold them until the next clock edge. For example here:

Code: [Select]

always@(posedge pixel_dclk) begin
  write_enable = 1;
  sdram_address <= sdram_address + 1;
end
I don't use Verilog, but I think "write_fifo_data" should be a register, not wire.

Your code will write to FIFO whatever the value of "sdram_address" is on this clock edge. At the same time, the combinatorial logic will do its work and by the next clock edge, it will produce the incremented value at the input of "sdram_address", then "sdram_address" will latch this incremented value at the next edge.

Thus, don't think in terms of "before" and "after", think in clock edges.

You never set "write_enable" to 0, so it'll write on every cycle. If FIFO overflows, you're in troubles.

Also, there's no need to drag the address through the FIFO - it's easier to generate it at the reading end of the FIFO.

[Kyristor]

Your code will write to FIFO whatever the value of "sdram_address" is on this clock edge. At the same time, the combinatorial logic will do its work and by the next clock edge, it will produce an incremented value at the input of "sdram_address", therefore "sdram_address" will be incremented at the next edge.

Ahhh is see, this makes so much more sense now.. I have been roading the code way too much as a "Sofware Engineer".
I will also put the address incrementation at the read side of the FIFO now.

could you explain why write_fifo_data should be a register instead of a wire?

Also could write_enable be as simple as this?

Code: [Select]

assign write_enable = ~full;


[NorthGuy]

could you explain why write_fifo_data should be a register instead of a wire?

It has to be a register because you use part of it to increment. But you need to re-address this question to someone who uses Verilog. I use VHDL. I don't really know if Verilog can automatically "upgrade" a wire to a register.

Also could write_enable be as simple as this?

Code: [Select]

assign write_enable = ~full;


Unlikely. If you decide not to write, you also need to hold the data flow. Otherwise, the current data will be gone and the address will still be incremented, but nothing gets written - you'll get the same result as if write_enable was 1. Rather, you let the module which produces data analyze the situation and figure out what to do. If you deal with video data, there will be gaps when there's no data and you probably don't want to write FIFO when there's no data.

When the data is pushed on you (as in video), you cannot hold the data flow. In this case you either design your FIFO in such a way that it never overflows (the reading end is guaranteed to read fast enough), or you need to design some logic which would decide what you can or cannot skip - say, it may be ok to skip the whole frame, but not Ok to skip a pixel (or vise versa).

[Kyristor]

It has to be a register because you use part of it to increment.

Ahh yes makes sense!

As for the FIFO I will read at 100mhz, compared to the 2.3mhz its is writing it should never overflow. Also the actual FIFO i'm using right now is only 1 word deep. 

[NorthGuy]

As for the FIFO I will read at 100mhz, compared to the 2.3mhz its is writing it should never overflow. Also the actual FIFO i'm using right now is only 1 word deep. 

Your FPGA may have built-in hardware FIFOs. It usually makes sense to use them.