From 92a888565b5f219c8d87fdbaff5ec9d8a44e43d0 Mon Sep 17 00:00:00 2001 From: whitequark Date: Wed, 5 Jun 2019 08:48:36 +0000 Subject: [PATCH] build.{dsl,res,plat}: apply clock constraints to signals, not resources. This adds the Clock() build DSL element, and adds a resource manager function add_clock_constraint() that takes a Pin or a Signal. Note that not all platforms, in particular not any nextpnr platforms at the moment, can add constraints on arbitrary signals. Fixes #86. --- nmigen/build/__init__.py | 4 +- nmigen/build/dsl.py | 50 +++++++++++++++++---- nmigen/build/plat.py | 5 +-- nmigen/build/res.py | 81 +++++++++++++++++----------------- nmigen/test/test_build_dsl.py | 38 ++++++++++++---- nmigen/test/test_build_res.py | 80 ++++++++++++++++----------------- nmigen/vendor/lattice_ice40.py | 4 +- 7 files changed, 154 insertions(+), 108 deletions(-) diff --git a/nmigen/build/__init__.py b/nmigen/build/__init__.py index 26a34ad..c4bc9f3 100644 --- a/nmigen/build/__init__.py +++ b/nmigen/build/__init__.py @@ -1,3 +1,3 @@ -from .dsl import Pins, DiffPairs, Attrs, Subsignal, Resource, Connector +from .dsl import * from .res import ResourceError -from .plat import Platform, TemplatedPlatform +from .plat import * diff --git a/nmigen/build/dsl.py b/nmigen/build/dsl.py index 14cfce3..6a31158 100644 --- a/nmigen/build/dsl.py +++ b/nmigen/build/dsl.py @@ -1,7 +1,7 @@ from collections import OrderedDict -__all__ = ["Pins", "DiffPairs", "Attrs", "Subsignal", "Resource", "Connector"] +__all__ = ["Pins", "DiffPairs", "Attrs", "Clock", "Subsignal", "Resource", "Connector"] class Pins: @@ -83,11 +83,27 @@ class Attrs(OrderedDict): for k, v in self.items())) +class Clock: + def __init__(self, frequency): + if not isinstance(frequency, (float, int)): + raise TypeError("Clock frequency must be a number") + + self.frequency = float(frequency) + + @property + def period(self): + return 1 / self.frequency + + def __repr__(self): + return "(clock {})".format(self.frequency) + + class Subsignal: def __init__(self, name, *args): self.name = name self.ios = [] self.attrs = Attrs() + self.clock = None if not args: raise ValueError("Missing I/O constraints") @@ -108,15 +124,33 @@ class Subsignal: .format(arg, self.ios[-1])) elif isinstance(arg, Attrs): self.attrs.update(arg) + elif isinstance(arg, Clock): + if self.ios and isinstance(self.ios[-1], (Pins, DiffPairs)): + if self.clock is None: + self.clock = arg + else: + raise ValueError("Clock constraint can be applied only once") + else: + raise TypeError("Clock constraint can only be applied to Pins or DiffPairs, " + "not {!r}" + .format(self.ios[-1])) else: - raise TypeError("I/O constraint must be one of Pins, DiffPairs, Subsignal, " - "or Attrs, not {!r}" + raise TypeError("Constraint must be one of Pins, DiffPairs, Subsignal, Attrs, " + "or Clock, not {!r}" .format(arg)) + def _content_repr(self): + parts = [] + for io in self.ios: + parts.append(repr(io)) + if self.clock is not None: + parts.append(repr(self.clock)) + if self.attrs: + parts.append(repr(self.attrs)) + return " ".join(parts) + def __repr__(self): - return "(subsignal {} {} {})".format(self.name, - " ".join(map(repr, self.ios)), - repr(self.attrs)) + return "(subsignal {} {})".format(self.name, self._content_repr()) class Resource(Subsignal): @@ -126,9 +160,7 @@ class Resource(Subsignal): self.number = number def __repr__(self): - return "(resource {} {} {} {})".format(self.name, self.number, - " ".join(map(repr, self.ios)), - repr(self.attrs)) + return "(resource {} {} {})".format(self.name, self.number, self._content_repr()) class Connector: diff --git a/nmigen/build/plat.py b/nmigen/build/plat.py index 7646040..b33f2b9 100644 --- a/nmigen/build/plat.py +++ b/nmigen/build/plat.py @@ -20,10 +20,9 @@ __all__ = ["Platform", "TemplatedPlatform"] class Platform(ResourceManager, metaclass=ABCMeta): resources = abstractproperty() connectors = abstractproperty() - clocks = abstractproperty() def __init__(self): - super().__init__(self.resources, self.connectors, self.clocks) + super().__init__(self.resources, self.connectors) self.extra_files = OrderedDict() @@ -267,7 +266,7 @@ class TemplatedPlatform(Platform): plan = BuildPlan(script="build_{}".format(name)) for filename_tpl, content_tpl in self.file_templates.items(): plan.add_file(render(filename_tpl, origin=filename_tpl), - render(content_tpl, origin=filename_tpl)) + render(content_tpl, origin=content_tpl)) for filename, content in self.extra_files.items(): plan.add_file(filename, content) return plan diff --git a/nmigen/build/res.py b/nmigen/build/res.py index 2d3e7e6..7067798 100644 --- a/nmigen/build/res.py +++ b/nmigen/build/res.py @@ -1,6 +1,6 @@ from collections import OrderedDict -from .. import * +from ..hdl.ast import * from ..hdl.rec import * from ..lib.io import * @@ -15,24 +15,19 @@ class ResourceError(Exception): class ResourceManager: - def __init__(self, resources, connectors, clocks): + def __init__(self, resources, connectors): self.resources = OrderedDict() self._requested = OrderedDict() self.connectors = OrderedDict() self._conn_pins = OrderedDict() - self.clocks = OrderedDict() - # Constraint lists self._ports = [] + self._clocks = SignalDict() self.add_resources(resources) self.add_connectors(connectors) - for name_number, frequency in clocks: - if not isinstance(name_number, tuple): - name_number = (name_number, 0) - self.add_clock(*name_number, frequency) def add_resources(self, resources): for res in resources: @@ -56,19 +51,6 @@ class ResourceManager: assert conn_pin not in self._conn_pins self._conn_pins[conn_pin] = plat_pin - def add_clock(self, name, number, frequency): - resource = self.lookup(name, number) - if isinstance(resource.ios[0], Subsignal): - raise TypeError("Cannot constrain frequency of resource {}#{} because it has " - "subsignals" - .format(resource.name, resource.number, frequency)) - if (resource.name, resource.number) in self.clocks: - other = self.clocks[resource.name, resource.number] - raise ResourceError("Resource {}#{} is already constrained to a frequency of " - "{:f} MHz" - .format(resource.name, resource.number, other / 1e6)) - self.clocks[resource.name, resource.number] = frequency - def lookup(self, name, number=0): if (name, number) not in self.resources: raise ResourceError("Resource {}#{} does not exist" @@ -138,12 +120,15 @@ class ResourceManager: port = Record([("p", len(phys)), ("n", len(phys))], name=name) if dir == "-": - self._ports.append((resource, None, port, attrs)) - return port + pin = None else: - pin = Pin(len(phys), dir, xdr, name=name) - self._ports.append((resource, pin, port, attrs)) - return pin + pin = Pin(len(phys), dir, xdr, name=name) + self._ports.append((resource, pin, port, attrs)) + + if pin is not None and resource.clock is not None: + self.add_clock_constraint(pin, resource.clock.frequency) + + return pin if pin is not None else port else: assert False # :nocov: @@ -206,19 +191,33 @@ class ResourceManager: for bit, pin_name in enumerate(pin_names): yield "{}[{}]".format(port_name, bit), pin_name, attrs - def iter_clock_constraints(self): - for name, number in self.clocks.keys() & self._requested.keys(): - resource = self.resources[name, number] - period = self.clocks[name, number] - pin = self._requested[name, number] - if pin.dir == "io": - raise ResourceError("Cannot constrain frequency of resource {}#{} because " - "it has been requested as a tristate buffer" - .format(name, number)) - if isinstance(resource.ios[0], Pins): - port_name = "{}__io".format(pin.name) - elif isinstance(resource.ios[0], DiffPairs): - port_name = "{}__p".format(pin.name) + def add_clock_constraint(self, clock, frequency): + if not isinstance(clock, (Signal, Pin)): + raise TypeError("Object {!r} is not a Signal or Pin".format(clock)) + if not isinstance(frequency, (int, float)): + raise TypeError("Frequency must be a number, not {!r}".format(frequency)) + + if isinstance(clock, Pin): + for res, pin, port, attrs in self._ports: + if clock is pin: + if isinstance(res.ios[0], Pins): + clock = port.io + elif isinstance(res.ios[0], DiffPairs): + clock = port.p + else: + assert False + break else: - assert False - yield (port_name, period) + raise ValueError("Cannot add clock constraint on a Pin {!r} that is not " + "a previously requested resource" + .format(clock)) + + if clock in self._clocks: + raise ValueError("Cannot add clock constraint on {!r}, which is already constrained " + "to {} Hz" + .format(clock, self._clocks[clock])) + + self._clocks[clock] = float(frequency) + + def iter_clock_constraints(self): + return iter(self._clocks.items()) diff --git a/nmigen/test/test_build_dsl.py b/nmigen/test/test_build_dsl.py index 1ab3807..37c0398 100644 --- a/nmigen/test/test_build_dsl.py +++ b/nmigen/test/test_build_dsl.py @@ -96,6 +96,14 @@ class AttrsTestCase(FHDLTestCase): a = Attrs(FOO=1) +class ClockTestCase(FHDLTestCase): + def test_basic(self): + c = Clock(1_000_000) + self.assertEqual(c.frequency, 1e6) + self.assertEqual(c.period, 1e-6) + self.assertEqual(repr(c), "(clock 1000000.0)") + + class SubsignalTestCase(FHDLTestCase): def test_basic_pins(self): s = Subsignal("a", Pins("A0"), Attrs(IOSTANDARD="LVCMOS33")) @@ -105,15 +113,15 @@ class SubsignalTestCase(FHDLTestCase): def test_basic_diffpairs(self): s = Subsignal("a", DiffPairs("A0", "B0")) self.assertEqual(repr(s), - "(subsignal a (diffpairs io (p A0) (n B0)) (attrs ))") + "(subsignal a (diffpairs io (p A0) (n B0)))") def test_basic_subsignals(self): s = Subsignal("a", Subsignal("b", Pins("A0")), Subsignal("c", Pins("A1"))) self.assertEqual(repr(s), - "(subsignal a (subsignal b (pins io A0) (attrs )) " - "(subsignal c (pins io A1) (attrs )) (attrs ))") + "(subsignal a (subsignal b (pins io A0)) " + "(subsignal c (pins io A1)))") def test_attrs(self): s = Subsignal("a", @@ -128,13 +136,17 @@ class SubsignalTestCase(FHDLTestCase): s = Subsignal("a", Pins("A0"), Attrs(SLEW="FAST"), Attrs(PULLUP="1")) self.assertEqual(s.attrs, {"SLEW": "FAST", "PULLUP": "1"}) + def test_clock(self): + s = Subsignal("a", Pins("A0"), Clock(1e6)) + self.assertEqual(s.clock.frequency, 1e6) + def test_wrong_empty_io(self): with self.assertRaises(ValueError, msg="Missing I/O constraints"): s = Subsignal("a") def test_wrong_io(self): with self.assertRaises(TypeError, - msg="I/O constraint must be one of Pins, DiffPairs, Subsignal, or Attrs, " + msg="Constraint must be one of Pins, DiffPairs, Subsignal, Attrs, or Clock, " "not 'wrong'"): s = Subsignal("a", "wrong") @@ -153,10 +165,20 @@ class SubsignalTestCase(FHDLTestCase): def test_wrong_subsignals(self): with self.assertRaises(TypeError, msg="Pins and DiffPairs are incompatible with other location or subsignal " - "constraints, but (pins io B0) appears after (subsignal b (pins io A0) " - "(attrs ))"): + "constraints, but (pins io B0) appears after (subsignal b (pins io A0))"): s = Subsignal("a", Subsignal("b", Pins("A0")), Pins("B0")) + def test_wrong_clock(self): + with self.assertRaises(TypeError, + msg="Clock constraint can only be applied to Pins or DiffPairs, not " + "(subsignal b (pins io A0))"): + s = Subsignal("a", Subsignal("b", Pins("A0")), Clock(1e6)) + + def test_wrong_clock_many(self): + with self.assertRaises(ValueError, + msg="Clock constraint can be applied only once"): + s = Subsignal("a", Pins("A0"), Clock(1e6), Clock(1e7)) + class ResourceTestCase(FHDLTestCase): def test_basic(self): @@ -165,8 +187,8 @@ class ResourceTestCase(FHDLTestCase): Subsignal("rx", Pins("A1", dir="i")), Attrs(IOSTANDARD="LVCMOS33")) self.assertEqual(repr(r), "(resource serial 0" - " (subsignal tx (pins o A0) (attrs ))" - " (subsignal rx (pins i A1) (attrs ))" + " (subsignal tx (pins o A0))" + " (subsignal rx (pins i A1))" " (attrs IOSTANDARD=LVCMOS33))") diff --git a/nmigen/test/test_build_res.py b/nmigen/test/test_build_res.py index 6f050ad..d4efef3 100644 --- a/nmigen/test/test_build_res.py +++ b/nmigen/test/test_build_res.py @@ -9,8 +9,8 @@ from .tools import * class ResourceManagerTestCase(FHDLTestCase): def setUp(self): self.resources = [ - Resource("clk100", 0, DiffPairs("H1", "H2", dir="i")), - Resource("clk50", 0, Pins("K1")), + Resource("clk100", 0, DiffPairs("H1", "H2", dir="i"), Clock(100e6)), + Resource("clk50", 0, Pins("K1"), Clock(50e6)), Resource("user_led", 0, Pins("A0", dir="o")), Resource("i2c", 0, Subsignal("scl", Pins("N10", dir="o")), @@ -20,14 +20,10 @@ class ResourceManagerTestCase(FHDLTestCase): self.connectors = [ Connector("pmod", 0, "B0 B1 B2 B3 - -"), ] - self.cm = ResourceManager(self.resources, self.connectors, []) + self.cm = ResourceManager(self.resources, self.connectors) def test_basic(self): - self.clocks = [ - ("clk100", 100), - (("clk50", 0), 50), - ] - self.cm = ResourceManager(self.resources, self.connectors, self.clocks) + self.cm = ResourceManager(self.resources, self.connectors) self.assertEqual(self.cm.resources, { ("clk100", 0): self.resources[0], ("clk50", 0): self.resources[1], @@ -37,10 +33,6 @@ class ResourceManagerTestCase(FHDLTestCase): self.assertEqual(self.cm.connectors, { ("pmod", 0): self.connectors[0], }) - self.assertEqual(self.cm.clocks, { - ("clk100", 0): 100, - ("clk50", 0): 50, - }) def test_add_resources(self): new_resources = [ @@ -152,23 +144,28 @@ class ResourceManagerTestCase(FHDLTestCase): ) ]) spi0 = self.cm.request("spi", 0) - self.assertEqual(list(sorted(self.cm.iter_port_constraints())), [ + self.assertEqual(list(self.cm.iter_port_constraints()), [ + ("spi_0__ss__io", ["B0"], {}), ("spi_0__clk__io", ["B1"], {}), ("spi_0__miso__io", ["B2"], {}), ("spi_0__mosi__io", ["B3"], {}), - ("spi_0__ss__io", ["B0"], {}), ]) - def test_add_clock(self): - self.cm.add_clock("clk100", 0, 10e6) - self.assertEqual(self.cm.clocks["clk100", 0], 10e6) - self.cm.add_clock("clk50", 0, 5e6) - + def test_request_clock(self): clk100 = self.cm.request("clk100", 0) clk50 = self.cm.request("clk50", 0, dir="i") - self.assertEqual(list(sorted(self.cm.iter_clock_constraints())), [ - ("clk100_0__p", 10e6), - ("clk50_0__io", 5e6) + clk100_port_p, clk100_port_n, clk50_port = self.cm.iter_ports() + self.assertEqual(list(self.cm.iter_clock_constraints()), [ + (clk100_port_p, 100e6), + (clk50_port, 50e6) + ]) + + def test_add_clock(self): + i2c = self.cm.request("i2c") + self.cm.add_clock_constraint(i2c.scl, 100e3) + scl_port, sda_port = self.cm.iter_ports() + self.assertEqual(list(self.cm.iter_clock_constraints()), [ + (scl_port, 100e3) ]) def test_wrong_resources(self): @@ -177,8 +174,8 @@ class ResourceManagerTestCase(FHDLTestCase): def test_wrong_resources_duplicate(self): with self.assertRaises(NameError, - msg="Trying to add (resource user_led 0 (pins o A1) (attrs )), but " - "(resource user_led 0 (pins o A0) (attrs )) has the same name and number"): + msg="Trying to add (resource user_led 0 (pins o A1)), but " + "(resource user_led 0 (pins o A0)) has the same name and number"): self.cm.add_resources([Resource("user_led", 0, Pins("A1", dir="o"))]) def test_wrong_connectors(self): @@ -196,25 +193,15 @@ class ResourceManagerTestCase(FHDLTestCase): msg="Resource user_led#1 does not exist"): r = self.cm.lookup("user_led", 1) - def test_wrong_frequency_subsignals(self): + def test_wrong_clock_signal(self): with self.assertRaises(TypeError, - msg="Cannot constrain frequency of resource i2c#0 because " - "it has subsignals"): - self.cm.add_clock("i2c", 0, 10e6) - - def test_wrong_frequency_tristate(self): - with self.assertRaises(ResourceError, - msg="Cannot constrain frequency of resource clk50#0 because " - "it has been requested as a tristate buffer"): - self.cm.add_clock("clk50", 0, 20e6) - clk50 = self.cm.request("clk50", 0) - list(self.cm.iter_clock_constraints()) + msg="Object None is not a Signal or Pin"): + self.cm.add_clock_constraint(None, 10e6) - def test_wrong_frequency_duplicate(self): - with self.assertRaises(ResourceError, - msg="Resource clk100#0 is already constrained to a frequency of 10.000000 MHz"): - self.cm.add_clock("clk100", 0, 10e6) - self.cm.add_clock("clk100", 0, 5e6) + def test_wrong_clock_frequency(self): + with self.assertRaises(TypeError, + msg="Frequency must be a number, not None"): + self.cm.add_clock_constraint(Signal(), None) def test_wrong_request_duplicate(self): with self.assertRaises(ResourceError, @@ -238,7 +225,7 @@ class ResourceManagerTestCase(FHDLTestCase): def test_wrong_request_with_dir_dict(self): with self.assertRaises(TypeError, msg="Directions must be a dict, not 'i', because (resource i2c 0 (subsignal scl " - "(pins o N10) (attrs )) (subsignal sda (pins io N11) (attrs )) (attrs )) " + "(pins o N10)) (subsignal sda (pins io N11))) " "has subsignals"): i2c = self.cm.request("i2c", 0, dir="i") @@ -250,6 +237,13 @@ class ResourceManagerTestCase(FHDLTestCase): def test_wrong_request_with_xdr_dict(self): with self.assertRaises(TypeError, msg="Data rate must be a dict, not 2, because (resource i2c 0 (subsignal scl " - "(pins o N10) (attrs )) (subsignal sda (pins io N11) (attrs )) (attrs )) " + "(pins o N10)) (subsignal sda (pins io N11))) " "has subsignals"): i2c = self.cm.request("i2c", 0, xdr=2) + + def test_wrong_clock_constraint_twice(self): + clk100 = self.cm.request("clk100") + with self.assertRaises(ValueError, + msg="Cannot add clock constraint on (sig clk100_0__p), which is already " + "constrained to 100000000.0 Hz"): + self.cm.add_clock_constraint(clk100, 1e6) diff --git a/nmigen/vendor/lattice_ice40.py b/nmigen/vendor/lattice_ice40.py index bef394c..aa9f99e 100644 --- a/nmigen/vendor/lattice_ice40.py +++ b/nmigen/vendor/lattice_ice40.py @@ -76,9 +76,9 @@ class LatticeICE40Platform(TemplatedPlatform): """, "{{name}}_pre_pack.py": r""" # {{autogenerated}} - {% for port_name, frequency in platform.iter_clock_constraints() -%} + {% for signal, frequency in platform.iter_clock_constraints() -%} {# Clock in MHz #} - ctx.addClock("{{port_name}}", {{frequency/1000000}}) + ctx.addClock("{{signal.name}}", {{frequency/1000000}}) {% endfor%} """, } -- 2.30.2