Skip to content

Adding MM2S, S2MM converters (umi_stream)#259

Merged
aolofsson merged 14 commits intomainfrom
streaming
Feb 13, 2026
Merged

Adding MM2S, S2MM converters (umi_stream)#259
aolofsson merged 14 commits intomainfrom
streaming

Conversation

@aolofsson
Copy link
Member

  • Dual mode, can work as device/endpoint or as two independent full duplex channels.
  • In device mode, reads pull from the s2mm fifo, writes "done" on write to mm2s fifo
  • Only posted writes in duplex mode
  • S2MM command entry handled outside module, pulling it in would make it too heavy
  • A bit funky, so definitely needs architectural/critical review. 😄

@aolofsson aolofsson requested a review from RiceShelley February 6, 2026 02:32
Copy link
Contributor

@RiceShelley RiceShelley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice add, architecturally seems solid. Working on a Testbench here


assign s2mm_fifo_write = usi_in_valid & ~s2mm_fifo_full;
assign s2mm_fifo_din[DW-1:0] = usi_in_data[DW-1:0];
assign s2mm_fifo_din[DW] = usi_in_last;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usi_in_last is clocked into ififo_s2mm but never used on the umi_clk side of the fifo. The umi_out_cmd is driven only by s2mm_cmd when device mode is 0. I would expect that the syncronized usi_in_last would replace the EOM or EOF bit given by s2mm_cmd.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added issue, will fix in next PR.

// Posted writes: only needs FIFO not full (works in both modes)
// Write with ack: need FIFO not full AND no stall (device mode only)
// Read: need s2mm not empty AND no stall (device mode only)
assign umi_in_ready = (cmd_write_posted & ~mm2s_fifo_full) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready being combinationaly dependent on UMI CMD feels like an extension of the spec rule ready shall not be combinationaly dependent on valid. I think an easy fix would be have a buffer on UMI input and ready is driven high when the buffer is empty.

umi/README.md

Line 462 in e57520f

5. However, it is legal for the READY assertion to be dependent on the VALID assertion (as long as this dependence is not combinational).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added issue, will fix in next PR

@aolofsson aolofsson merged commit 27e6b06 into main Feb 13, 2026
3 of 4 checks passed
@aolofsson aolofsson deleted the streaming branch February 13, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants