From d8504bf846e463b3237d2b181df98952a63ac3ae Mon Sep 17 00:00:00 2001 From: occheung Date: Thu, 10 Oct 2024 12:15:13 +0800 Subject: [PATCH 1/7] stream fifo: encourage burst access with available watermarks --- misoc/interconnect/stream.py | 115 ++++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 3 deletions(-) diff --git a/misoc/interconnect/stream.py b/misoc/interconnect/stream.py index 5ba33597a..dff7f46ca 100644 --- a/misoc/interconnect/stream.py +++ b/misoc/interconnect/stream.py @@ -49,7 +49,7 @@ def __getattr__(self, name): class _FIFOWrapper(Module): - def __init__(self, fifo_class, layout, depth): + def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): self.sink = Endpoint(layout) self.source = Endpoint(layout) @@ -78,13 +78,122 @@ def __init__(self, fifo_class, layout, depth): self.fifo.re.eq(self.source.ack) ] + if hi_wm is not None: + self.fifo.add_almost_full(hi_wm) + + transfer_count = Signal(max=hi_wm, reset=hi_wm-1) + transfer_count_ce = Signal() + transfer_count_rst = Signal() + activated = Signal() + eop_count = Signal(max=depth+1) + eop_count_next = Signal(max=depth+1) + has_pending_eop = Signal() + + # helper signals + last = Signal() + do_write = Signal() + do_read = Signal() + + self.sync += [ + If(transfer_count_rst, + transfer_count.eq(transfer_count.reset), + ).Elif(transfer_count_ce, + transfer_count.eq(transfer_count - 1), + ), + eop_count.eq(eop_count_next), + ] + + self.comb += [ + # Avoid downstream overreading + self.fifo.re.eq(self.source.stb & self.source.ack), + + last.eq((transfer_count == 0) | fifo_out.eop), + do_write.eq(self.fifo.we & self.fifo.writable), + do_read.eq(self.fifo.re & self.fifo.readable), + has_pending_eop.eq(eop_count_next != 0), + + eop_count_next.eq(eop_count), + + If(fifo_in.eop & do_write, + If(~(fifo_out.eop & do_read), + eop_count_next.eq(eop_count + 1), + ), + ).Elif(fifo_out.eop & do_read, + eop_count_next.eq(eop_count - 1), + ), + ] + + # Stream control + self.comb += [ + self.source.stb.eq(self.fifo.readable & (self.fifo.almost_full | activated)), + # self.source.eop.eq(last), + transfer_count_ce.eq(do_read), + transfer_count_rst.eq(do_read & last), + ] + + self.sync += [ + If(~activated, + activated.eq(self.fifo.almost_full | (self.sink.eop & do_write)) + ).Elif(do_read & last, + activated.eq(has_pending_eop), + ), + ] + + if lo_wm is not None: + self.fifo.add_almost_empty(lo_wm) + + recv_burst_len = depth-lo_wm + recv_count = Signal(max=recv_burst_len, reset=recv_burst_len-1) + recv_count_ce = Signal() + recv_count_rst = Signal() + recv_activated = Signal() + + # helper signals + do_write = Signal() + recv_last = Signal() + + self.comb += [ + do_write.eq(self.fifo.we & self.fifo.writable), + recv_last.eq((recv_count == 0) | self.sink.eop), + + # Avoid upstream overwriting + self.fifo.we.eq(self.sink.stb & self.sink.ack), + ] + + self.sync += [ + If(recv_count_rst, + recv_count.eq(recv_count.reset), + ).Elif(recv_count_ce, + recv_count.eq(recv_count - 1), + ), + ] + + # recv stream control + self.comb += [ + self.sink.ack.eq(self.fifo.writable & ( + self.fifo.almost_empty # Can accept long burst + | recv_activated # In the middle of a burst + )), + recv_count_ce.eq(do_write), + recv_count_rst.eq(do_write & recv_last), + ] + + self.sync += \ + If(~recv_activated, + # Avoid entry to burst state if it is a 1 word burst + recv_activated.eq(self.fifo.almost_empty & ~(do_write & self.sink.eop)), + ).Elif(recv_activated & (do_write & recv_last), + # almost_empty needs 1 cycle to update + recv_activated.eq(0), + ) + class SyncFIFO(_FIFOWrapper): - def __init__(self, layout, depth, buffered=False): + def __init__(self, layout, depth, buffered=False, hi_wm=None, lo_wm=None): _FIFOWrapper.__init__( self, fifo.SyncFIFOBuffered if buffered else fifo.SyncFIFO, - layout, depth) + layout, depth, hi_wm, lo_wm) class AsyncFIFO(_FIFOWrapper): From 3f6612b8f88231addaf74927d468ac5cf73dda09 Mon Sep 17 00:00:00 2001 From: occheung Date: Tue, 29 Oct 2024 16:16:35 +0800 Subject: [PATCH 2/7] fifo: pass watermarks to constructors --- misoc/interconnect/stream.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/misoc/interconnect/stream.py b/misoc/interconnect/stream.py index dff7f46ca..adb9e5afc 100644 --- a/misoc/interconnect/stream.py +++ b/misoc/interconnect/stream.py @@ -58,7 +58,13 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): description = self.sink.description fifo_layout = [("payload", description.payload_layout), ("eop", 1)] - self.submodules.fifo = fifo_class(layout_len(fifo_layout), depth) + watermark_args = {} + if hi_wm is not None: + watermark_args["hi_wm"] = hi_wm + if lo_wm is not None: + watermark_args["lo_wm"] = lo_wm + + self.submodules.fifo = fifo_class(layout_len(fifo_layout), depth, **watermark_args) fifo_in = Record(fifo_layout) fifo_out = Record(fifo_layout) self.comb += [ @@ -79,8 +85,6 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): ] if hi_wm is not None: - self.fifo.add_almost_full(hi_wm) - transfer_count = Signal(max=hi_wm, reset=hi_wm-1) transfer_count_ce = Signal() transfer_count_rst = Signal() @@ -140,8 +144,6 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): ] if lo_wm is not None: - self.fifo.add_almost_empty(lo_wm) - recv_burst_len = depth-lo_wm recv_count = Signal(max=recv_burst_len, reset=recv_burst_len-1) recv_count_ce = Signal() From ba194c01cec7dc5a2346cdfb3f4f491e7ea32e2f Mon Sep 17 00:00:00 2001 From: occheung Date: Tue, 29 Oct 2024 16:46:10 +0800 Subject: [PATCH 3/7] remove dead code --- misoc/interconnect/stream.py | 1 - 1 file changed, 1 deletion(-) diff --git a/misoc/interconnect/stream.py b/misoc/interconnect/stream.py index adb9e5afc..e1594e4ed 100644 --- a/misoc/interconnect/stream.py +++ b/misoc/interconnect/stream.py @@ -130,7 +130,6 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): # Stream control self.comb += [ self.source.stb.eq(self.fifo.readable & (self.fifo.almost_full | activated)), - # self.source.eop.eq(last), transfer_count_ce.eq(do_read), transfer_count_rst.eq(do_read & last), ] From 0f96f22522fee554f518759dfec9de80e60ccc19 Mon Sep 17 00:00:00 2001 From: occheung Date: Tue, 29 Oct 2024 16:54:00 +0800 Subject: [PATCH 4/7] stream fifo: comment on watermark handling --- misoc/interconnect/stream.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/misoc/interconnect/stream.py b/misoc/interconnect/stream.py index e1594e4ed..85831f90a 100644 --- a/misoc/interconnect/stream.py +++ b/misoc/interconnect/stream.py @@ -84,6 +84,15 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): self.fifo.re.eq(self.source.ack) ] + # Watermarks: + # FIFO only signals only signals availability when a complete + # burst/buffered packet can be transferred. + # + # A complete burst is a continuous lo_wm/hi_wm of words written to/ + # read from the FIFO. + # + # The EoP bit from the sink endpoint signals the end of packet. + if hi_wm is not None: transfer_count = Signal(max=hi_wm, reset=hi_wm-1) transfer_count_ce = Signal() From 327660cea18619f89e7986b6ce043f112d60597c Mon Sep 17 00:00:00 2001 From: occheung Date: Fri, 10 Jan 2025 16:00:42 +0800 Subject: [PATCH 5/7] stream: add last signal to endpoint --- misoc/interconnect/stream.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/misoc/interconnect/stream.py b/misoc/interconnect/stream.py index 85831f90a..fff1b1514 100644 --- a/misoc/interconnect/stream.py +++ b/misoc/interconnect/stream.py @@ -18,7 +18,7 @@ def __init__(self, payload_layout): self.payload_layout = payload_layout def get_full_layout(self): - reserved = {"stb", "ack", "payload", "eop", "description"} + reserved = {"stb", "ack", "payload", "last", "eop", "description"} attributed = set() for f in self.payload_layout: if f[0] in attributed: @@ -30,6 +30,7 @@ def get_full_layout(self): full_layout = [ ("stb", 1, DIR_M_TO_S), ("ack", 1, DIR_S_TO_M), + ("last", 1, DIR_M_TO_S), ("eop", 1, DIR_M_TO_S), ("payload", _make_m2s(self.payload_layout)) ] From a4359408fa2ee6b90a8830eba615482e52e37144 Mon Sep 17 00:00:00 2001 From: occheung Date: Fri, 10 Jan 2025 16:01:41 +0800 Subject: [PATCH 6/7] SyncFIFO: relax lo_wm implementation Burst validity should be enforced by the master. --- misoc/interconnect/stream.py | 50 +++++++++++++++--------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/misoc/interconnect/stream.py b/misoc/interconnect/stream.py index fff1b1514..1ad51b2d4 100644 --- a/misoc/interconnect/stream.py +++ b/misoc/interconnect/stream.py @@ -85,15 +85,18 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): self.fifo.re.eq(self.source.ack) ] - # Watermarks: - # FIFO only signals only signals availability when a complete - # burst/buffered packet can be transferred. - # + # Burst transfer with the use of watermarks: + # FIFO would expect complete bursts to be written to/read from, when + # given the corresponding watermark arguments, unless the burst is + # prematurely terminated by asserting sink.eop. # A complete burst is a continuous lo_wm/hi_wm of words written to/ # read from the FIFO. - # - # The EoP bit from the sink endpoint signals the end of packet. + # With high watermark, FIFO mandates hi_wm length burst read. + # + # source.stb is held low if no complete burst/buffered packet could + # be transferred. + # source.last is driven high to signal end of burst. if hi_wm is not None: transfer_count = Signal(max=hi_wm, reset=hi_wm-1) transfer_count_ce = Signal() @@ -104,7 +107,6 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): has_pending_eop = Signal() # helper signals - last = Signal() do_write = Signal() do_read = Signal() @@ -121,7 +123,6 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): # Avoid downstream overreading self.fifo.re.eq(self.source.stb & self.source.ack), - last.eq((transfer_count == 0) | fifo_out.eop), do_write.eq(self.fifo.we & self.fifo.writable), do_read.eq(self.fifo.re & self.fifo.readable), has_pending_eop.eq(eop_count_next != 0), @@ -139,61 +140,50 @@ def __init__(self, fifo_class, layout, depth, hi_wm=None, lo_wm=None): # Stream control self.comb += [ + self.source.last.eq((transfer_count == 0) | fifo_out.eop), self.source.stb.eq(self.fifo.readable & (self.fifo.almost_full | activated)), transfer_count_ce.eq(do_read), - transfer_count_rst.eq(do_read & last), + transfer_count_rst.eq(do_read & self.source.last), ] self.sync += [ If(~activated, activated.eq(self.fifo.almost_full | (self.sink.eop & do_write)) - ).Elif(do_read & last, + ).Elif(do_read & self.source.last, activated.eq(has_pending_eop), ), ] + # With low watermark, FIFO accepts lo_wm length burst write. + # + # sink.last must indicate the end of burst + # + # It is the upstream's duty to drive sink.last signal appropriately. if lo_wm is not None: - recv_burst_len = depth-lo_wm - recv_count = Signal(max=recv_burst_len, reset=recv_burst_len-1) - recv_count_ce = Signal() - recv_count_rst = Signal() recv_activated = Signal() # helper signals do_write = Signal() - recv_last = Signal() self.comb += [ do_write.eq(self.fifo.we & self.fifo.writable), - recv_last.eq((recv_count == 0) | self.sink.eop), # Avoid upstream overwriting self.fifo.we.eq(self.sink.stb & self.sink.ack), ] - self.sync += [ - If(recv_count_rst, - recv_count.eq(recv_count.reset), - ).Elif(recv_count_ce, - recv_count.eq(recv_count - 1), - ), - ] - # recv stream control - self.comb += [ + self.comb += \ self.sink.ack.eq(self.fifo.writable & ( self.fifo.almost_empty # Can accept long burst | recv_activated # In the middle of a burst - )), - recv_count_ce.eq(do_write), - recv_count_rst.eq(do_write & recv_last), - ] + )) self.sync += \ If(~recv_activated, # Avoid entry to burst state if it is a 1 word burst recv_activated.eq(self.fifo.almost_empty & ~(do_write & self.sink.eop)), - ).Elif(recv_activated & (do_write & recv_last), + ).Elif(recv_activated & (do_write & (self.sink.last | self.sink.eop)), # almost_empty needs 1 cycle to update recv_activated.eq(0), ) From 8bff8d07cfc785a7a7e07336466750a0c3e958ab Mon Sep 17 00:00:00 2001 From: occheung Date: Fri, 10 Jan 2025 16:03:50 +0800 Subject: [PATCH 7/7] Converter: impl last handling --- misoc/interconnect/stream.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/misoc/interconnect/stream.py b/misoc/interconnect/stream.py index 1ad51b2d4..c5d598244 100644 --- a/misoc/interconnect/stream.py +++ b/misoc/interconnect/stream.py @@ -257,7 +257,9 @@ def __init__(self, nbits_from, nbits_to, ratio, reverse, self.comb += [ sink.ack.eq(~strobe_all | source.ack), source.stb.eq(strobe_all), - load_part.eq(sink.stb & sink.ack) + load_part.eq(sink.stb & sink.ack), + # cannot burst + source.last.eq(1) ] demux_last = ((demux == (ratio - 1)) | sink.eop) @@ -309,6 +311,7 @@ def __init__(self, nbits_from, nbits_to, ratio, reverse, last.eq(mux == (ratio-1)), source.stb.eq(sink.stb), source.eop.eq(sink.eop & last), + source.last.eq(sink.last & last), sink.ack.eq(last & source.ack) ] self.sync += \ @@ -393,6 +396,7 @@ def __init__(self, layout_from, layout_to, *args, **kwargs): # cast sink to converter.sink (user fields --> raw bits) self.comb += [ converter.sink.stb.eq(sink.stb), + converter.sink.last.eq(sink.last), converter.sink.eop.eq(sink.eop), sink.ack.eq(converter.sink.ack) ] @@ -412,6 +416,7 @@ def __init__(self, layout_from, layout_to, *args, **kwargs): # cast converter.source to source (raw bits --> user fields) self.comb += [ source.stb.eq(converter.source.stb), + source.last.eq(converter.source.last), source.eop.eq(converter.source.eop), converter.source.ack.eq(source.ack) ]