Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib.fifo: fix reset handling of asynchronous FIFOs. #591

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

lib.fifo: fix reset handling of asynchronous FIFOs. #591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 27 additions & 22 deletions amaranth/lib/fifo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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))
Copy link
Member Author

Choose a reason for hiding this comment

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

This does not work: if a write domain reset happens one clock cycle after a read, then the next value of consume_w_gry is still inside an FFSynchronizer.


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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
136 changes: 136 additions & 0 deletions tests/test_lib_fifo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)