pavel-demin / red-pitaya-notes

Notes on the Red Pitaya Open Source Instrument
http://pavel-demin.github.io/red-pitaya-notes/
MIT License
338 stars 210 forks source link

TREADY issue in axis_selector.v #1122

Closed svhum closed 9 months ago

svhum commented 10 months ago

I am not sure if the ready signal logic is correct in cores/axis_selector.v. The only project that seems to use this core it is mcpha, which (like many other projects) does not use the TREADY signal, so the ready signal logic is never exercised. This core did not seem to work properly for me in a simple test where I connected the slaves of axis_selector to the outputs of an axis_broadcaster IP with 2 master ports and appropriate sources/sinks connected to each. When I enable the TREADY signals in the broadcaster, there is no output from axis_selector regardless of the state of cfg_data. However, if I disable the TREADY signal in the broadcaster, axis_selector works properly.

I also tried hard-wiring the ready logic in axis_selector.v to be asserted all the time, and confirmed (obviously) that the core will work with TREADY signals from the broadcaster, since it effectively ignores them in this case:

assign s00_axis_tready = 1'b1;
assign s01_axis_tready = 1'b1;

I tried studying the logic in the core but could not see anything immediately wrong, but thought I should report this here.

pavel-demin commented 10 months ago

Thank you for testing the axis_selector module. I think that the behavior that you observe is expected.

As far as I know, the axis_broadcaster module requires tready/tvalid handshakes on all its m*_axis interfaces. A similar issue has already been discussed on the Xilinx forum:

https://support.xilinx.com/s/question/0D52E00006iHvq9SAC/axis-broadcaster

Maybe a solution that would work properly with the axis_broadcaster module would be to connect the int_ready_wire signal directly to the two tready outputs:

  assign s00_axis_tready = int_ready_wire;
  assign s01_axis_tready = int_ready_wire;
svhum commented 10 months ago

Thank you! I was wondering if it had something to do with the broadcaster, and confirm this fix seems to work.