alexforencich/cocotbext-axi

AxiStreamSource shows unexpected behaviour with TUSER and multiple byte_lanes

Closed this issue · 4 comments

Hello,
when trying to set TUSER[0] of the first transfer of a AXI Stream frame to 1, and for all following transfers to 0, the following line does NOT work:

frame = AxiStreamFrame(tdata, tuser=[1, 0])

All transfers show a TUSER value of 0.

Instead, using the following line I get the result I want:

frame = AxiStreamFrame(tdata, tuser=[1, 1, 0])

I checked the code and found that it seems like the TUSER list is processed per TDATA bytes, and not per TDATA transfer (which could be multiple bytes, depending on TDATA width). Essentially, only every 'byte_lanes'-multiple of the TUSER list is actually used, all other values are being ignored.

So, [1, 1, 0] accidentally works, because my interface has two byte_lanes. For three byte_lanes, I'd need to put [1, 1, 1, 0] and so on for it to work.

I believe this is unintended and highly counter-intuitive for users.

The code in question is here:

tuser_val = frame.tuser[frame_offset]

If you agree that this is unintended behaviour, I might be able to look into the issue and come up with a PR.

It is the intended behavior - everything is in increments of bytes, and when the byte lanes get "merged" the last value of the sideband signals is used. This will always do something sensible no matter the number of byte lanes in the interface. The width converter in verilog-axis also works the same way - last value of the sideband signals is used when widening. But, I am certainly open to improvements, if you have an idea for how to make things work better.

Oh, thanks for the explanation!
I was certainly expecting it to work differently, e.g. in terms of transactions instead of bytes, but now that I know that it is intended behaviour and a principle throughout the project, I have to think about it some more.

I agree that for the width converter, it is a sensible approach.

Yeah, in the first incarnation, it was based on transactions like you describe. But this introduces the problem that tdata is length X, but the sideband signals are length Y, and the relationship between X and Y can't really be determined by anything other than the source and sink modules which are looking at the actual signals. The AxiStreamFrame object doesn't know this, at least not until it gets handed off to the source. Seemed like doing everything in terms of bytes made things simpler, now the AxiStreamFrame object doesn't need to have any knowledge of the bus configuration. And in most cases all of the sideband signals are going to be constant anyway, so having to do a bit of extra work to duplicate values appropriately if needed seemed like an appropriate trade-off.

Makes sense that AxiStreamFrame can't know about the interface's byte_lanes. Thanks for the support, much appreciated! I think this issue can be closed then.