alexforencich / verilog-axis

Verilog AXI stream components for FPGA implementation
MIT License
747 stars 229 forks source link

axis_async_fifo write pointer CDC #18

Open Basseuph opened 2 years ago

Basseuph commented 2 years ago

While going after the items reported by the CDC related DRCs of intel quartus 21.3, most of the items seem to be OK from an RTL perspective. But one item catched my eye, which is https://github.com/alexforencich/verilog-axis/blob/ce8dcdafe83af2ca610e353bb4cbb0e1afbed92b/rtl/axis_async_fifo.v#L225

I don't get the reason why it is OK in FRAME_FIFO mode to read the first stage of the synchronisation chain of the write pointer.

Another item is that in CDCs it is usually not a good idea to add logic in between the last flop of the source clock and the first flop of the destination clock, see https://github.com/alexforencich/verilog-axis/blob/ce8dcdafe83af2ca610e353bb4cbb0e1afbed92b/rtl/axis_async_fifo.v#L486-L487 The path (last flop in source clock) -> (first flop in destination clock) -> (second flop in destination clock) should be free of any logic to allow for shortest paths in between the flops to reduce the probability of metastability. This special is actually worse, because depending on the condition the input of the synchronizer chain is either driven by the source clock or the destination clock.

alexforencich commented 2 years ago

The FIFO uses a handshake synchronizer in frame FIFO mode. So in this case metastability doesn't matter as the synchronization path is via the wr_ptr_update(_sync) regs, and the source wr_ptr_sync_gray_reg is not updated until the acknowledgement comes back from the destination clock domain.

In frame FIFO mode, the grey coded sync doesn't work properly because it doesn't increment, it jumps by the whole frame size, which defeats the point of the gray code.

alexforencich commented 2 years ago

But, I suppose it could make sense to un-alias the sync registers used by the different modes by changing the register names. That should at least make it easier to understand, and could also make setting up timing constraints more straightforward.

Basseuph commented 2 years ago

OK, I guess for the FRAME_FIFO mode in combination with the set_max_skew constraints this makes sense.

Apropos constraints, we are currently adding some code to the quartuspro constraints to automatically apply the `constrain*` procedures to all respective entities found in the design. This would make them behave more like the vivado constraints and makes is easier to constraint parametric designs.

What do you think about that idea?

alexforencich commented 2 years ago

Yes, that would be excellent. I did some poking around and wasn't able to figure out how to do that, but if there is a way to automatically apply the constraints, that would definitely make things more streamlined. It would be great if it also worked for Quartus Prime as well as Quartus Prime Pro, but I suspect that may not be possible. It would also be great to find a way to make the constraints work without having to specify clock groups (so you timing failures for stuff paths that didn't get explicitly constrained), but I was running in to issues the last time I tried to do that.

Devipriya1921 commented 2 years ago

Hi sir, I was trying to implement the axis_fifo in cocotb on Gitpod platform. I am getting the following error

ModuleNotFoundError: No module named 'pytest'
     0.00ns ERROR    Failing test at simulator request before test run completion: Simulator shutdown prematurely
ERROR: results.xml was not written by the simulation!
make[1]: *** [/workspace/.pyenv_mirror/user/3.8.13/lib/python3.8/site-packages/cocotb/share/makefiles/simulators/Makefile.icarus:77: results.xml] Error 1
make[1]: Leaving directory '/workspace/challenges-Devipriya1921/level3_design'
make: *** [/workspace/.pyenv_mirror/user/3.8.13/lib/python3.8/site-packages/cocotb/share/makefiles/Makefile.inc:40: sim] Error 2

I tried solving this error, But couldn't solve this!! I installed the pytest package using pip install pytest as well. I hope I can get some help on this from you sir!

Thanks and Regards! A Devipriya