From b8dd8a13f1f19df16c0c7f5345b4d1050e521d00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20Nguyen?= Date: Wed, 17 Jul 2024 14:52:04 +0200 Subject: [PATCH] lib.fifo: fix reset handling of asynchronous FIFOs. --- amaranth/lib/fifo.py | 49 ++++++++------- tests/test_lib_fifo.py | 136 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 22 deletions(-) diff --git a/amaranth/lib/fifo.py b/amaranth/lib/fifo.py index 5b8be9cae..6c8f02310 100644 --- a/amaranth/lib/fifo.py +++ b/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,7 +406,7 @@ 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 @@ -414,24 +414,25 @@ def elaborate(self, platform): 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,11 +478,11 @@ 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() @@ -489,12 +490,14 @@ def elaborate(self, platform): 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 diff --git a/tests/test_lib_fifo.py b/tests/test_lib_fifo.py index d7afd0e43..8db971b83 100644 --- a/tests/test_lib_fifo.py +++ b/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)