From 4f272db780b9a636c93db58f803ab894b366ec96 Mon Sep 17 00:00:00 2001 From: whitequark Date: Mon, 23 Sep 2019 14:17:44 +0000 Subject: [PATCH] =?utf8?q?lib.cdc:=20MultiReg=E2=86=92FFSynchronizer.?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Fixes #229. --- examples/basic/cdc.py | 4 +- nmigen/compat/genlib/cdc.py | 20 +++++--- nmigen/lib/cdc.py | 76 ++++++++++++++++------------- nmigen/lib/fifo.py | 6 +-- nmigen/test/test_lib_cdc.py | 6 +-- nmigen/vendor/xilinx_7series.py | 8 +-- nmigen/vendor/xilinx_spartan_3_6.py | 8 +-- 7 files changed, 71 insertions(+), 57 deletions(-) diff --git a/examples/basic/cdc.py b/examples/basic/cdc.py index ab691ae..658d075 100644 --- a/examples/basic/cdc.py +++ b/examples/basic/cdc.py @@ -1,11 +1,11 @@ from nmigen import * -from nmigen.lib.cdc import MultiReg +from nmigen.lib.cdc import FFSynchronizer from nmigen.cli import main i, o = Signal(name="i"), Signal(name="o") m = Module() -m.submodules += MultiReg(i, o) +m.submodules += FFSynchronizer(i, o) if __name__ == "__main__": main(m, ports=[i, o]) diff --git a/nmigen/compat/genlib/cdc.py b/nmigen/compat/genlib/cdc.py index b65fb4b..5264169 100644 --- a/nmigen/compat/genlib/cdc.py +++ b/nmigen/compat/genlib/cdc.py @@ -1,7 +1,7 @@ import warnings from ...tools import deprecated -from ...lib.cdc import MultiReg as NativeMultiReg +from ...lib.cdc import FFSynchronizer as NativeFFSynchronizer from ...hdl.ast import * from ..fhdl.module import CompatModule from ..fhdl.structure import If @@ -10,14 +10,20 @@ from ..fhdl.structure import If __all__ = ["MultiReg", "GrayCounter", "GrayDecoder"] -class MultiReg(NativeMultiReg): +class MultiReg(NativeFFSynchronizer): def __init__(self, i, o, odomain="sync", n=2, reset=0): + old_opts = [] + new_opts = [] if odomain != "sync": - warnings.warn("instead of `MultiReg(..., odomain={!r})`, " - "use `MultiReg(..., o_domain={!r})`" - .format(odomain, odomain), - DeprecationWarning, stacklevel=2) - super().__init__(i, o, o_domain=odomain, n=n, reset=reset) + old_opts.append(", odomain={!r}".format(odomain)) + new_opts.append(", o_domain={!r}".format(odomain)) + if n != 2: + old_opts.append(", n={!r}".format(n)) + new_opts.append(", stages={!r}".format(n)) + warnings.warn("instead of `MultiReg(...{})`, use `FFSynchronizer(...{})`" + .format("".join(old_opts), "".join(new_opts)), + DeprecationWarning, stacklevel=2) + super().__init__(i, o, o_domain=odomain, stages=n, reset=reset) self.odomain = odomain diff --git a/nmigen/lib/cdc.py b/nmigen/lib/cdc.py index 3ed1077..62d3939 100644 --- a/nmigen/lib/cdc.py +++ b/nmigen/lib/cdc.py @@ -1,10 +1,13 @@ +from ..tools import deprecated from .. import * -__all__ = ["MultiReg", "ResetSynchronizer"] +__all__ = ["FFSynchronizer", "ResetSynchronizer"] +# TODO(nmigen-0.2): remove this +__all__ += ["MultiReg"] -class MultiReg(Elaboratable): +class FFSynchronizer(Elaboratable): """Resynchronise a signal to a different clock domain. Consists of a chain of flip-flops. Eliminates metastabilities at the output, but provides @@ -12,70 +15,75 @@ class MultiReg(Elaboratable): Parameters ---------- - i : Signal(), in - Signal to be resynchronised - o : Signal(), out - Signal connected to synchroniser output + i : Signal, in + Signal to be resynchronised. + o : Signal, out + Signal connected to synchroniser output. o_domain : str - Name of output clock domain - n : int - Number of flops between input and output. + Name of output clock domain. + stages : int + Number of synchronization stages between input and output. The lowest safe number is 2, + with higher numbers reducing MTBF further, at the cost of increased latency. reset : int - Reset value of the flip-flops. On FPGAs, even if ``reset_less`` is True, the MultiReg is - still set to this value during initialization. + Reset value of the flip-flops. On FPGAs, even if ``reset_less`` is True, + the :class:`FFSynchronizer` is still set to this value during initialization. reset_less : bool - If True (the default), this MultiReg is unaffected by ``o_domain`` reset. + If True (the default), this :class:`FFSynchronizer` is unaffected by ``o_domain`` reset. See "Note on Reset" below. Platform override ----------------- - Define the ``get_multi_reg`` platform method to override the implementation of MultiReg, + Define the ``get_ff_sync`` platform method to override the implementation of :class:`FFSynchronizer`, e.g. to instantiate library cells directly. Note on Reset ------------- - MultiReg is non-resettable by default. Usually this is the safest option; on FPGAs - the MultiReg will still be initialized to its ``reset`` value when the FPGA loads its - configuration. + :class:`FFSynchronizer` is non-resettable by default. Usually this is the safest option; + on FPGAs the :class:`FFSynchronizer` will still be initialized to its ``reset`` value when + the FPGA loads its configuration. - However, in designs where the value of the MultiReg must be valid immediately after reset, - consider setting ``reset_less`` to False if any of the following is true: + However, in designs where the value of the :class:`FFSynchronizer` must be valid immediately + after reset, consider setting ``reset_less`` to False if any of the following is true: - You are targeting an ASIC, or an FPGA that does not allow arbitrary initial flip-flop states; - Your design features warm (non-power-on) resets of ``o_domain``, so the one-time initialization at power on is insufficient; - - Your design features a sequenced reset, and the MultiReg must maintain its reset value until - ``o_domain`` reset specifically is deasserted. + - Your design features a sequenced reset, and the :class:`FFSynchronizer` must maintain + its reset value until ``o_domain`` reset specifically is deasserted. - MultiReg is reset by the ``o_domain`` reset only. + :class:`FFSynchronizer` is reset by the ``o_domain`` reset only. """ - def __init__(self, i, o, *, o_domain="sync", n=2, reset=0, reset_less=True): + def __init__(self, i, o, *, o_domain="sync", stages=2, reset=0, reset_less=True): self.i = i self.o = o self._o_domain = o_domain - self._regs = [Signal(self.i.shape(), name="cdc{}".format(i), reset=reset, - reset_less=reset_less) - for i in range(n)] + self._stages = [Signal(self.i.shape(), name="stage{}".format(index), + reset=reset, reset_less=reset_less) + for index in range(stages)] def elaborate(self, platform): - if hasattr(platform, "get_multi_reg"): - return platform.get_multi_reg(self) + if hasattr(platform, "get_ff_sync"): + return platform.get_ff_sync(self) m = Module() - for i, o in zip((self.i, *self._regs), self._regs): + for i, o in zip((self.i, *self._stages), self._stages): m.d[self._o_domain] += o.eq(i) - m.d.comb += self.o.eq(self._regs[-1]) + m.d.comb += self.o.eq(self._stages[-1]) return m +# TODO(nmigen-0.2): remove this +MultiReg = deprecated("instead of `MultiReg`, use `FFSynchronizer`")(FFSynchronizer) + + class ResetSynchronizer(Elaboratable): - def __init__(self, arst, *, domain="sync", n=2): + def __init__(self, arst, *, domain="sync", stages=2): self.arst = arst self._domain = domain - self._regs = [Signal(1, name="arst{}".format(i), reset=1) - for i in range(n)] + self._stages = [Signal(1, name="stage{}".format(i), reset=1) + for i in range(stages)] def elaborate(self, platform): if hasattr(platform, "get_reset_sync"): @@ -83,11 +91,11 @@ class ResetSynchronizer(Elaboratable): m = Module() m.domains += ClockDomain("reset_sync", async_reset=True, local=True) - for i, o in zip((0, *self._regs), self._regs): + for i, o in zip((0, *self._stages), self._stages): m.d.reset_sync += o.eq(i) m.d.comb += [ ClockSignal("reset_sync").eq(ClockSignal(self._domain)), ResetSignal("reset_sync").eq(self.arst), - ResetSignal(self._domain).eq(self._regs[-1]) + ResetSignal(self._domain).eq(self._stages[-1]) ] return m diff --git a/nmigen/lib/fifo.py b/nmigen/lib/fifo.py index 4d50f34..3775bf8 100644 --- a/nmigen/lib/fifo.py +++ b/nmigen/lib/fifo.py @@ -4,7 +4,7 @@ from .. import * from ..asserts import * from ..tools import log2_int, deprecated from .coding import GrayEncoder -from .cdc import MultiReg +from .cdc import FFSynchronizer __all__ = ["FIFOInterface", "SyncFIFO", "SyncFIFOBuffered", "AsyncFIFO", "AsyncFIFOBuffered"] @@ -399,7 +399,7 @@ class AsyncFIFO(Elaboratable, FIFOInterface): produce_enc = m.submodules.produce_enc = \ GrayEncoder(self._ctr_bits) produce_cdc = m.submodules.produce_cdc = \ - MultiReg(produce_w_gry, produce_r_gry, o_domain=self._r_domain) + FFSynchronizer(produce_w_gry, produce_r_gry, o_domain=self._r_domain) m.d.comb += produce_enc.i.eq(produce_w_nxt), m.d[self._w_domain] += produce_w_gry.eq(produce_enc.o) @@ -408,7 +408,7 @@ class AsyncFIFO(Elaboratable, FIFOInterface): consume_enc = m.submodules.consume_enc = \ GrayEncoder(self._ctr_bits) consume_cdc = m.submodules.consume_cdc = \ - MultiReg(consume_r_gry, consume_w_gry, o_domain=self._w_domain) + FFSynchronizer(consume_r_gry, consume_w_gry, o_domain=self._w_domain) m.d.comb += consume_enc.i.eq(consume_r_nxt) m.d[self._r_domain] += consume_r_gry.eq(consume_enc.o) diff --git a/nmigen/test/test_lib_cdc.py b/nmigen/test/test_lib_cdc.py index 9aea5d4..0d4894b 100644 --- a/nmigen/test/test_lib_cdc.py +++ b/nmigen/test/test_lib_cdc.py @@ -4,11 +4,11 @@ from ..back.pysim import * from ..lib.cdc import * -class MultiRegTestCase(FHDLTestCase): +class FFSynchronizerTestCase(FHDLTestCase): def test_basic(self): i = Signal() o = Signal() - frag = MultiReg(i, o) + frag = FFSynchronizer(i, o) with Simulator(frag) as sim: sim.add_clock(1e-6) def process(): @@ -26,7 +26,7 @@ class MultiRegTestCase(FHDLTestCase): def test_reset_value(self): i = Signal(reset=1) o = Signal() - frag = MultiReg(i, o, reset=1) + frag = FFSynchronizer(i, o, reset=1) with Simulator(frag) as sim: sim.add_clock(1e-6) def process(): diff --git a/nmigen/vendor/xilinx_7series.py b/nmigen/vendor/xilinx_7series.py index 6bf7ee0..2e7706c 100644 --- a/nmigen/vendor/xilinx_7series.py +++ b/nmigen/vendor/xilinx_7series.py @@ -361,10 +361,10 @@ class Xilinx7SeriesPlatform(TemplatedPlatform): ) return m - def get_multi_reg(self, multireg): + def get_ff_sync(self, ff_sync): m = Module() - for i, o in zip((multireg.i, *multireg._regs), multireg._regs): + for i, o in zip((ff_sync.i, *ff_sync._stages), ff_sync._stages): o.attrs["ASYNC_REG"] = "TRUE" - m.d[multireg._o_domain] += o.eq(i) - m.d.comb += multireg.o.eq(multireg._regs[-1]) + m.d[ff_sync._o_domain] += o.eq(i) + m.d.comb += ff_sync.o.eq(ff_sync._stages[-1]) return m diff --git a/nmigen/vendor/xilinx_spartan_3_6.py b/nmigen/vendor/xilinx_spartan_3_6.py index 05dff90..0a5df0e 100644 --- a/nmigen/vendor/xilinx_spartan_3_6.py +++ b/nmigen/vendor/xilinx_spartan_3_6.py @@ -411,12 +411,12 @@ class XilinxSpartan3Or6Platform(TemplatedPlatform): ) return m - def get_multi_reg(self, multireg): + def get_ff_sync(self, ff_sync): m = Module() - for i, o in zip((multireg.i, *multireg._regs), multireg._regs): + for i, o in zip((ff_sync.i, *ff_sync._stages), ff_sync._stages): o.attrs["ASYNC_REG"] = "TRUE" - m.d[multireg._o_domain] += o.eq(i) - m.d.comb += multireg.o.eq(multireg._regs[-1]) + m.d[ff_sync._o_domain] += o.eq(i) + m.d.comb += ff_sync.o.eq(multireg._stages[-1]) return m -- 2.30.2