lib.fifo: fix reset handling of asynchronous FIFOs.

amaranth/lib/fifo.py

@@ -392,7 +392,7 @@ def __init__(self, *, width, depth, r_domain="read", w_domain="write", exact_dep
             depth_bits = 0
         super().__init__(width=width, depth=depth)
 
-        self.r_rst = Signal()
+        self.r_rst = Signal(init=1)
         self._r_domain = r_domain
         self._w_domain = w_domain
         self._ctr_bits = depth_bits + 1
@@ -406,32 +406,33 @@ def elaborate(self, platform):
             ]
             return m
 
-        # The design of this queue is the "style #2" from Clifford E. Cummings' paper "Simulation
+        # The design of this queue is the "style #1" from Clifford E. Cummings' paper "Simulation
         # and Synthesis Techniques for Asynchronous FIFO Design":
         # http://www.sunburst-design.com/papers/CummingsSNUG2002SJ_FIFO1.pdf
 
         do_write = self.w_rdy & self.w_en
         do_read  = self.r_rdy & self.r_en
 
         # TODO: extract this pattern into lib.cdc.GrayCounter
-        produce_w_bin = Signal(self._ctr_bits)
+
+        # Note: Both write-domain counters must be reset_less (see comments below)
+        produce_w_bin = Signal(self._ctr_bits, reset_less=True)
         produce_w_nxt = Signal(self._ctr_bits)
         m.d.comb += produce_w_nxt.eq(produce_w_bin + do_write)
         m.d[self._w_domain] += produce_w_bin.eq(produce_w_nxt)
 
-        # Note: Both read-domain counters must be reset_less (see comments below)
-        consume_r_bin = Signal(self._ctr_bits, reset_less=True)
+        consume_r_bin = Signal(self._ctr_bits)
         consume_r_nxt = Signal(self._ctr_bits)
         m.d.comb += consume_r_nxt.eq(consume_r_bin + do_read)
         m.d[self._r_domain] += consume_r_bin.eq(consume_r_nxt)
 
-        produce_w_gry = Signal(self._ctr_bits)
+        produce_w_gry = Signal(self._ctr_bits, reset_less=True)
         produce_r_gry = Signal(self._ctr_bits)
         produce_cdc = m.submodules.produce_cdc = \
             FFSynchronizer(produce_w_gry, produce_r_gry, o_domain=self._r_domain)
         m.d[self._w_domain] += produce_w_gry.eq(_gray_encode(produce_w_nxt))
 
-        consume_r_gry = Signal(self._ctr_bits, reset_less=True)
+        consume_r_gry = Signal(self._ctr_bits)
         consume_w_gry = Signal(self._ctr_bits)
         consume_cdc = m.submodules.consume_cdc = \
             FFSynchronizer(consume_r_gry, consume_w_gry, o_domain=self._w_domain)
@@ -477,24 +478,26 @@ def elaborate(self, platform):
         # are reset to 0 violate their Gray code invariant. One way to handle this is to ensure
         # that both sides of the FIFO are asynchronously reset by the same signal. We adopt a
         # slight variation on this approach - reset control rests entirely with the write domain.
-        # The write domain's reset signal is used to asynchronously reset the read domain's
-        # counters and force the FIFO to be empty when the write domain's reset is asserted.
-        # This requires the two read domain counters to be marked as "reset_less", as they are
-        # reset through another mechanism. See https://github.com/amaranth-lang/amaranth/issues/181
-        # for the full discussion.
+        # The write domain reset signal is used to asynchronously force the FIFO to be empty. The
+        # write domain counters are overwritten with the value of the read domain counters. This
+        # requires the two write domain counters to be marked as "reset_less", as they are reset
+        # through another mechanism.
+        # See https://github.com/amaranth-lang/amaranth/issues/181 for the full discussion.
         w_rst = ResetSignal(domain=self._w_domain, allow_reset_less=True)
         r_rst = Signal()
 
         # Async-set-sync-release synchronizer avoids CDC hazards
         rst_cdc = m.submodules.rst_cdc = \
             AsyncFFSynchronizer(w_rst, r_rst, o_domain=self._r_domain)
 
-        # Decode Gray code counter synchronized from write domain to overwrite binary
-        # counter in read domain.
+        # Decode Gray code counter synchronized from read domain to overwrite binary counter in
+        # write domain.
+        with m.If(w_rst):
+            m.d[self._w_domain] += produce_w_gry.eq(consume_w_gry)
+            m.d[self._w_domain] += produce_w_bin.eq(_gray_decode(consume_w_gry))
+
         with m.If(r_rst):
             m.d.comb += r_empty.eq(1)
-            m.d[self._r_domain] += consume_r_gry.eq(produce_r_gry)
-            m.d[self._r_domain] += consume_r_bin.eq(_gray_decode(produce_r_gry))
             m.d[self._r_domain] += self.r_rst.eq(1)
         with m.Else():
             m.d[self._r_domain] += self.r_rst.eq(0)
@@ -550,7 +553,7 @@ def __init__(self, *, width, depth, r_domain="read", w_domain="write", exact_dep
             depth = (1 << depth_bits) + 1
         super().__init__(width=width, depth=depth)
 
-        self.r_rst = Signal()
+        self.r_rst = Signal(init=1)
         self._r_domain = r_domain
         self._w_domain = w_domain
 
@@ -580,14 +583,16 @@ def elaborate(self, platform):
         m.submodules.consume_buffered_cdc = FFSynchronizer(r_consume_buffered, w_consume_buffered, o_domain=self._w_domain, stages=4)
         m.d.comb += self.w_level.eq(fifo.w_level + w_consume_buffered)
 
-        with m.If(self.r_en | ~self.r_rdy):
+        with m.If(fifo.r_rst):
+            m.d.comb += r_consume_buffered.eq(0)
+            m.d[self._r_domain] += self.r_rdy.eq(0)
+        with m.Elif(self.r_en | ~self.r_rdy):
             m.d[self._r_domain] += [
                 self.r_data.eq(fifo.r_data),
                 self.r_rdy.eq(fifo.r_rdy),
-                self.r_rst.eq(fifo.r_rst),
-            ]
-            m.d.comb += [
-                fifo.r_en.eq(1)
             ]
+            m.d.comb += fifo.r_en.eq(1)
+
+        m.d[self._r_domain] += self.r_rst.eq(fifo.r_rst)
 
         return m

tests/test_lib_fifo.py

@@ -364,6 +364,7 @@ async def testbench_write(ctx):
                 await ctx.tick("write")
             ctx.set(fifo.w_en, 0)
             await ctx.tick ("write")
+
             self.assertEqual(ctx.get(fifo.w_level), expected_level)
             ctx.set(write_done, 1)
 
@@ -405,3 +406,138 @@ def test_async_buffered_fifo_level_full(self):
     def test_async_buffered_fifo_level_empty(self):
         fifo = AsyncFIFOBuffered(width=32, depth=9, r_domain="read", w_domain="write")
         self.check_async_fifo_level(fifo, fill_in=0, expected_level=0, read=True)
+
+    def check_async_fifo_reset(self, fifo, r_period, w_period, r_phase=None, w_phase=None):
+        write_rst         = Signal()
+        read_non_empty_1  = Signal()
+        read_non_empty_2  = Signal()
+        reset_write_reset = Signal()
+
+        m = Module()
+        m.submodules.fifo = fifo
+        m.d.comb += ResetSignal("write").eq(write_rst)
+
+        async def testbench_write(ctx):
+            # First refill ========================================================================
+
+            # - wait until the FIFO read interface comes out of reset:
+            await ctx.tick("write").until(~fifo.r_rst)
+
+            # - fill the FIFO:
+            ctx.set(fifo.w_en, 1)
+            for i in range(fifo.depth):
+                ctx.set(fifo.w_data, 0x5a5a5a00 | i)
+                await ctx.tick("write").until(fifo.w_rdy)
+            ctx.set(fifo.w_en, 0)
+
+            # - wait until the FIFO is readable:
+            await ctx.tick("write").until(read_non_empty_1)
+
+            # Back-to-back reset + refill =========================================================
+
+            # - reset the write domain:
+            ctx.set(write_rst, 1)
+            await ctx.tick("write")
+            ctx.set(write_rst, 0)
+            self.assertEqual(ctx.get(fifo.w_rdy), 1)
+
+            # - fill the FIFO:
+            ctx.set(fifo.w_en, 1)
+            for i in range(fifo.depth):
+                ctx.set(fifo.w_data, 0xa5a5a500 | i)
+                await ctx.tick("write").until(fifo.w_rdy)
+            ctx.set(fifo.w_en, 0)
+
+            # - wait until the FIFO is readable:
+            await ctx.tick("write").until(read_non_empty_2)
+
+            # Back-to-back reset + write + reset ==================================================
+
+            # - reset the write domain:
+            ctx.set(write_rst, 1)
+            await ctx.tick("write")
+            ctx.set(write_rst, 0)
+            self.assertEqual(ctx.get(fifo.w_rdy), 1)
+
+            # - write to the FIFO:
+            ctx.set(fifo.w_en, 1)
+            ctx.set(fifo.w_data, 0xc3c3c3c3)
+            await ctx.tick("write")
+            ctx.set(fifo.w_en, 0)
+
+            # - reset the write domain:
+            ctx.set(write_rst, 1)
+            await ctx.tick("write")
+            ctx.set(write_rst, 0)
+            await ctx.tick("write")
+            ctx.set(reset_write_reset, 1)
+
+        async def testbench_read(ctx):
+            # First refill ========================================================================
+
+            # - wait until the FIFO read interface comes out of reset:
+            self.assertEqual(ctx.get(fifo.r_rst), 1)
+            await ctx.tick("read").until(~fifo.r_rst)
+
+            # - wait until the FIFO is readable:
+            self.assertEqual(ctx.get(fifo.r_rdy), 0)
+            fifo_r_data, = await ctx.tick("read").sample(fifo.r_data).until(fifo.r_rdy)
+            ctx.set(read_non_empty_1, 1)
+            self.assertEqual(fifo_r_data, 0x5a5a5a00)
+
+            # Back-to-back reset + refill =========================================================
+
+            # - wait until the FIFO read interface comes out of reset:
+            await ctx.posedge(fifo.r_rst)
+            await ctx.tick("read").until(~fifo.r_rst)
+
+            # - wait until the FIFO is readable:
+            fifo_r_data, = await ctx.tick("read").sample(fifo.r_data).until(fifo.r_rdy)
+            ctx.set(read_non_empty_2, 1)
+            self.assertEqual(fifo_r_data, 0xa5a5a500)
+
+            # Back-to-back reset + write + reset ==================================================
+
+            # - wait until the FIFO read interface comes out of reset:
+            await ctx.tick("read").until(reset_write_reset & ~fifo.r_rst)
+            self.assertEqual(ctx.get(fifo.r_rdy), 0)
+
+        simulator = Simulator(m)
+        simulator.add_clock(w_period, phase=w_phase, domain="write")
+        simulator.add_clock(r_period, phase=r_phase, domain="read")
+        simulator.add_testbench(testbench_write)
+        simulator.add_testbench(testbench_read)
+        with simulator.write_vcd("test.vcd"):
+            simulator.run()
+
+    def test_async_fifo_reset_same_clk(self):
+        fifo = AsyncFIFO(width=32, depth=2, r_domain="read", w_domain="write")
+        self.check_async_fifo_reset(fifo, r_period=10e-9, w_period=10e-9)
+
+    def test_async_fifo_reset_phase_180deg(self):
+        fifo = AsyncFIFO(width=32, depth=2, r_domain="read", w_domain="write")
+        self.check_async_fifo_reset(fifo, r_period=10e-9, w_period=10e-9, r_phase=0.0, w_phase=5e-9)
+
+    def test_async_fifo_reset_faster_write_clk(self):
+        fifo = AsyncFIFO(width=32, depth=2, r_domain="read", w_domain="write")
+        self.check_async_fifo_reset(fifo, r_period=50e-9, w_period=10e-9)
+
+    def test_async_fifo_reset_faster_read_clk(self):
+        fifo = AsyncFIFO(width=32, depth=2, r_domain="read", w_domain="write")
+        self.check_async_fifo_reset(fifo, r_period=10e-9, w_period=50e-9)
+
+    def test_async_buffered_fifo_reset_same_clk(self):
+        fifo = AsyncFIFOBuffered(width=32, depth=3, r_domain="read", w_domain="write")
+        self.check_async_fifo_reset(fifo, r_period=10e-9, w_period=10e-9)
+
+    def test_async_buffered_fifo_reset_phase_180deg(self):
+        fifo = AsyncFIFOBuffered(width=32, depth=3, r_domain="read", w_domain="write")
+        self.check_async_fifo_reset(fifo, r_period=10e-9, w_period=10e-9, r_phase=0.0, w_phase=5e-9)
+
+    def test_async_buffered_fifo_reset_faster_write_clk(self):
+        fifo = AsyncFIFOBuffered(width=32, depth=3, r_domain="read", w_domain="write")
+        self.check_async_fifo_reset(fifo, r_period=50e-9, w_period=10e-9)
+
+    def test_async_buffered_fifo_reset_faster_read_clk(self):
+        fifo = AsyncFIFOBuffered(width=32, depth=3, r_domain="read", w_domain="write")
+        self.check_async_fifo_reset(fifo, r_period=10e-9, w_period=50e-9)