alexforencich / verilog-axis

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

Issue with tuser in axis_adapter #31

Open immbelgique opened 8 months ago

immbelgique commented 8 months ago

tuser is supposed to line up with tvalid and tready but it is not hence the reason why in the axis_async_fifo_adapter no tuser is not propagated through the asynchronous FIFO. This case might not have been tested because tuser in network interfaces is not used but it is the start of frame for axis video while tlast is the end of line . axis_adapter_tuser_issue

Will look at the code and come up with a solution.

Thank you,

Wim

immbelgique commented 8 months ago

screen shot from second frame where tready is already high but tuser is one clock late. axis_adapter_second_frame_issue

alexforencich commented 8 months ago

This is the intended behavior. Two cycles are being merged, and the tuser value of the last cycle is kept.

alexforencich commented 8 months ago

This is always going to be a potential problem when doing width conversion - when upsizing, something has to be combined or discarded; when downsizing, additional data has to be created somehow. Not much that can be done about upsizing, but downsizing I suppose an option could be added to select first cycle vs. last cycle.

immbelgique commented 8 months ago

I seem to remember that for upsizing the Xilinx adapter creates the upsize factor number of tusers. My main issue is that I noticed that tuser stayed 0 using the axis_async_fifo_adapter and when looking at it I noticed when tuser was high tvalid was low so not propagated. I will come up with a solution. Too many times over the past decade using Xilinx IP finding bugs which of course cannot be fixed and for most things video/image processing I use my own IP but then I found your library for axi blocks and liked it...

alexforencich commented 8 months ago

I'm definitely open to implementing a more general solution. It's just not immediately clear what the appropriate behavior should be, since presumably you want to get a single-cycle tuser when both upsizing and downsizing. Maybe there needs to be a "video mode" or "tuser pulse" or similar option.

The width converter right now was designed around two main assumptions: 1. most of the sideband signals are constant (tid, tdest) and 2. tuser is used to indicate invalid frames, and is asserted coincident with tlast.

But, these assumptions may not always apply, so it might be necessary to add some additional config options.