csr.bus: instantiate a Multiplexer from a MemoryMap.

Before this commit, CSR elements were added with `Multiplexer.add()`.

Now, a memory map of elements is populated beforehand, then given as
argument to `Multiplexer.__init__()`.

This decouples the definition of a group of neighboring registers from
their implementation by a CSR Multiplexer.

Author: jfng

amaranth_soc/csr/bus.py

@@ -155,6 +155,18 @@ def width(self):
     def access(self):
         return self.signature.access
 
+    def add_to(self, memory_map, *, name, addr=None, alignment=None):
+        """Add register to memory map.
+
+        See :meth:`MemoryMap.add_resource` for details.
+        """
+        if not isinstance(memory_map, MemoryMap):
+            raise TypeError("Memory map must be an instance of MemoryMap, not {!r}"
+                            .format(memory_map))
+        size = (self.width + memory_map.data_width - 1) // memory_map.data_width
+        return memory_map.add_resource(self, size=size, addr=addr, alignment=alignment,
+                                       name=name)
+
     def __repr__(self):
         return f"csr.Element({self.signature!r})"
 
@@ -519,31 +531,25 @@ def chunks(self):
     are possible for connecting the CSR bus to the CPU:
         * The CPU could access the CSR bus directly (with no intervening logic other than simple
           translation of control signals). In this case, the register alignment should be set
-          to 1 (i.e. `alignment` should be set to 0), and each *w*-bit register would occupy
-          *ceil(w/n)* addresses from the CPU perspective, requiring the same amount of memory
-          instructions to access.
+          to 1 (i.e. `memory_map.alignment` should be set to 0), and each *w*-bit register would
+          occupy *ceil(w/n)* addresses from the CPU perspective, requiring the same amount of
+          memory instructions to access.
         * The CPU could also access the CSR bus through a width down-converter, which would issue
           *k/n* CSR accesses for each CPU access. In this case, the register alignment should be
           set to *k/n*, and each *w*-bit register would occupy *ceil(w/k)* addresses from the CPU
           perspective, requiring the same amount of memory instructions to access.
 
-    If the register alignment (i.e. `2 ** alignment`) is greater than 1, it affects which CSR bus
-    write is considered a write to the last register chunk. For example, if a 24-bit register is
-    used with a 8-bit CSR bus and a CPU with a 32-bit datapath, a write to this register requires
-    4 CSR bus writes to complete and the 4th write is the one that actually writes the value to
-    the register. This allows determining write latency solely from the amount of addresses the
-    register occupies in the CPU address space, and the width of the CSR bus.
+    If the register alignment (i.e. `2 ** memory_map.alignment`) is greater than 1, it affects
+    which CSR bus write is considered a write to the last register chunk. For example, if a 24-bit
+    register is used with a 8-bit CSR bus and a CPU with a 32-bit datapath, a write to this
+    register requires 4 CSR bus writes to complete and the 4th write is the one that actually
+    writes the value to the register. This allows determining write latency solely from the amount
+    of addresses the register occupies in the CPU address space, and the width of the CSR bus.
 
     Parameters
     ----------
-    addr_width : int
-        Address width. See :class:`Interface`.
-    data_width : int
-        Data width. See :class:`Interface`.
-    alignment : int, power-of-2 exponent
-        Register alignment. See :class:`..memory.MemoryMap`.
-    name : :class:`str`
-        Window name. Optional. See :class:`..memory.MemoryMap`.
+    memory_map : :class:`..memory.MemoryMap`
+        Memory map of the CSR bus.
     shadow_overlaps : int
         Maximum number of CSR registers that can share a chunk of a shadow register.
         Optional. If ``None``, any number of CSR registers can share a shadow chunk.
@@ -554,31 +560,24 @@ def chunks(self):
     bus : :class:`Interface`
         CSR bus providing access to registers.
     """
-    def __init__(self, *, addr_width, data_width, alignment=0, name=None, shadow_overlaps=None):
-        super().__init__({"bus": In(Signature(addr_width=addr_width, data_width=data_width))})
-        self.bus.memory_map = MemoryMap(addr_width=addr_width, data_width=data_width,
-                                        alignment=alignment, name=name)
-        self._r_shadow = Multiplexer._Shadow(data_width, shadow_overlaps, name="r_shadow")
-        self._w_shadow = Multiplexer._Shadow(data_width, shadow_overlaps, name="w_shadow")
-
-    def align_to(self, alignment):
-        """Align the implicit address of the next register.
-
-        See :meth:`MemoryMap.align_to` for details.
-        """
-        return self.bus.memory_map.align_to(alignment)
-
-    def add(self, elem, *, name, addr=None, alignment=None):
-        """Add a register.
-
-        See :meth:`MemoryMap.add_resource` for details.
-        """
-        if not isinstance(elem, Element):
-            raise TypeError(f"Element must be an instance of csr.Element, not {elem!r}")
-
-        size = (elem.width + self.bus.memory_map.data_width - 1) // self.bus.memory_map.data_width
-        return self.bus.memory_map.add_resource(elem, size=size, addr=addr, alignment=alignment,
-                                                name=name)
+    def __init__(self, memory_map, *, shadow_overlaps=None):
+        if not isinstance(memory_map, MemoryMap):
+            raise TypeError("Memory map must be an instance of MemoryMap, not {!r}"
+                            .format(memory_map))
+        for elem, elem_name, (elem_start, elem_end) in memory_map.resources():
+            if not isinstance(elem, Element):
+                raise TypeError("Memory map resource must be an instance of csr.Element, not {!r}"
+                                .format(elem))
+        if list(memory_map.windows()):
+            raise ValueError("Memory map cannot have windows")
+
+        super().__init__({
+            "bus": In(Signature(addr_width=memory_map.addr_width,
+                                data_width=memory_map.data_width))
+        })
+        self.bus.memory_map = memory_map
+        self._r_shadow = self._Shadow(memory_map.data_width, shadow_overlaps, name="r_shadow")
+        self._w_shadow = self._Shadow(memory_map.data_width, shadow_overlaps, name="w_shadow")
 
     def elaborate(self, platform):
         m = Module()

amaranth_soc/csr/event.py

@@ -59,10 +59,11 @@ def __init__(self, event_map, *, trigger="level", data_width, alignment=0, name=
 
         elem_size  = ceil(event_map.size / data_width)
         addr_width = 1 + max(ceil_log2(elem_size), alignment)
-        self._mux  = Multiplexer(addr_width=addr_width, data_width=data_width,
-                                 alignment=alignment)
-        self._mux.add(self._enable,  name="enable")
-        self._mux.add(self._pending, name="pending")
+        memory_map = MemoryMap(addr_width=addr_width, data_width=data_width, alignment=alignment)
+
+        self._enable .add_to(memory_map, name="enable")
+        self._pending.add_to(memory_map, name="pending")
+        self._mux = Multiplexer(memory_map)
 
         super().__init__({
             "src": Out(self._monitor.src.signature),

tests/test_csr_bus.py

@@ -176,62 +176,53 @@ def test_set_wrong_map_data_width(self):
 
 
 class MultiplexerTestCase(unittest.TestCase):
-    def setUp(self):
-        self.dut = csr.Multiplexer(addr_width=16, data_width=8)
-
-    def test_add_4b(self):
-        elem_4b = csr.Element(4, "rw", path=("elem_4b",))
-        self.assertEqual(self.dut.add(elem_4b, name="elem_4b"), (0, 1))
+    def test_memory_map(self):
+        elem_4_rw  = csr.Element(4, "rw", path=("elem_4_rw",))
+        elem_8_rw  = csr.Element(8, "rw", path=("elem_8_rw",))
 
-    def test_add_8b(self):
-        elem_8b = csr.Element(8, "rw", path=("elem_8b",))
-        self.assertEqual(self.dut.add(elem_8b, name="elem_8b"), (0, 1))
+        memory_map = MemoryMap(addr_width=2, data_width=4)
+        memory_map.add_resource(elem_4_rw, name="elem_4_rw", size=1)
+        memory_map.add_resource(elem_8_rw, name="elem_8_rw", size=2)
+        memory_map.freeze()
 
-    def test_add_12b(self):
-        elem_12b = csr.Element(12, "rw", path=("elem_12b",))
-        self.assertEqual(self.dut.add(elem_12b, name="elem_8b"), (0, 2))
+        dut = csr.Multiplexer(memory_map)
 
-    def test_add_16b(self):
-        elem_16b = csr.Element(16, "rw", path=("elem_16b",))
-        self.assertEqual(self.dut.add(elem_16b, name="elem_16b"), (0, 2))
+        self.assertIs(dut.bus.memory_map, memory_map)
+        self.assertEqual(dut.bus.addr_width, 2)
+        self.assertEqual(dut.bus.data_width, 4)
 
-    def test_add_two(self):
-        elem_8b  = csr.Element( 8, "rw", path=("elem_8b",))
-        elem_16b = csr.Element(16, "rw", path=("elem_16b",))
-        self.assertEqual(self.dut.add(elem_16b, name="elem_16b"), (0, 2))
-        self.assertEqual(self.dut.add(elem_8b,  name="elem_8b"), (2, 3))
-
-    def test_add_wrong(self):
+    def test_wrong_memory_map(self):
         with self.assertRaisesRegex(TypeError,
-                r"Element must be an instance of csr\.Element, not 'foo'"):
-            self.dut.add(elem="foo", name="elem_4b")
+                r"Memory map must be an instance of MemoryMap, not 'foo'"):
+            csr.Multiplexer("foo")
 
-    def test_align_to(self):
-        elem_0 = csr.Element(8, "rw", path=("elem_0",))
-        elem_1 = csr.Element(8, "rw", path=("elem_1",))
-        self.assertEqual(self.dut.add(elem_0, name="elem_0"), (0, 1))
-        self.assertEqual(self.dut.align_to(2), 4)
-        self.assertEqual(self.dut.add(elem_1, name="elem_1"), (4, 5))
+    def test_wrong_memory_map_resource(self):
+        memory_map = MemoryMap(addr_width=1, data_width=8)
+        memory_map.add_resource("foo", name="foo", size=1)
+        with self.assertRaisesRegex(TypeError,
+                r"Memory map resource must be an instance of csr\.Element, not 'foo'"):
+            csr.Multiplexer(memory_map)
 
-    def test_add_wrong_out_of_bounds(self):
-        elem = csr.Element(8, "rw", path=("elem",))
-        with self.assertRaisesRegex(ValueError,
-                r"Address range 0x10000\.\.0x10001 out of bounds for memory map spanning "
-                r"range 0x0\.\.0x10000 \(16 address bits\)"):
-            self.dut.add(elem, name="elem", addr=0x10000)
+    def test_wrong_memory_map_windows(self):
+        memory_map_0 = MemoryMap(addr_width=1, data_width=8)
+        memory_map_1 = MemoryMap(addr_width=1, data_width=8)
+        memory_map_0.add_window(memory_map_1)
+        with self.assertRaisesRegex(ValueError, r"Memory map cannot have windows"):
+            csr.Multiplexer(memory_map_0)
 
     def test_sim(self):
         for shadow_overlaps in [None, 0, 1]:
             with self.subTest(shadow_overlaps=shadow_overlaps):
-                dut = csr.Multiplexer(addr_width=16, data_width=8, shadow_overlaps=shadow_overlaps)
-
-                elem_4_r = csr.Element(4, "r", path=("elem_4_r",))
-                dut.add(elem_4_r, name="elem_4_r")
-                elem_8_w = csr.Element(8, "w", path=("elem_8_w",))
-                dut.add(elem_8_w, name="elem_8_w")
+                elem_4_r   = csr.Element( 4, "r",  path=("elem_4_r",))
+                elem_8_w   = csr.Element( 8, "w",  path=("elem_8_w",))
                 elem_16_rw = csr.Element(16, "rw", path=("elem_16_rw",))
-                dut.add(elem_16_rw, name="elem_16_rw")
 
+                memory_map = MemoryMap(addr_width=16, data_width=8)
+                memory_map.add_resource(elem_4_r,   name="elem_4_r",   size=1)
+                memory_map.add_resource(elem_8_w,   name="elem_8_w",   size=1)
+                memory_map.add_resource(elem_16_rw, name="elem_16_rw", size=2)
+
+                dut = csr.Multiplexer(memory_map, shadow_overlaps=shadow_overlaps)
                 bus = dut.bus
 
                 def sim_test():
@@ -309,38 +300,14 @@ def sim_test():
 
 
 class MultiplexerAlignedTestCase(unittest.TestCase):
-    def setUp(self):
-        self.dut = csr.Multiplexer(addr_width=16, data_width=8, alignment=2)
-
-    def test_add_two(self):
-        elem_0 = csr.Element( 8, "rw", path=("elem_0",))
-        elem_1 = csr.Element(16, "rw", path=("elem_1",))
-        self.assertEqual(self.dut.add(elem_0, name="elem_0"), (0, 4))
-        self.assertEqual(self.dut.add(elem_1, name="elem_1"), (4, 8))
-
-    def test_over_align_to(self):
-        elem_0 = csr.Element(8, "rw", path=("elem_0",))
-        elem_1 = csr.Element(8, "rw", path=("elem_1",))
-        self.assertEqual(self.dut.add(elem_0, name="elem_0"), (0, 4))
-        self.assertEqual(self.dut.align_to(3), 8)
-        self.assertEqual(self.dut.add(elem_1, name="elem_1"), (8, 12))
-
-    def test_under_align_to(self):
-        elem_0 = csr.Element(8, "rw", path=("elem_0",))
-        elem_1 = csr.Element(8, "rw", path=("elem_1",))
-        self.assertEqual(self.dut.add(elem_0, name="elem_0"), (0, 4))
-        self.assertEqual(self.dut.align_to(alignment=1), 4)
-        self.assertEqual(self.dut.add(elem_1, name="elem_1"), (4, 8))
-
     def test_sim(self):
         for shadow_overlaps in [None, 0, 1]:
             with self.subTest(shadow_overlaps=shadow_overlaps):
-                dut = csr.Multiplexer(addr_width=16, data_width=8, alignment=2,
-                                      shadow_overlaps=shadow_overlaps)
-
                 elem_20_rw = csr.Element(20, "rw", path=("elem_20_rw",))
-                dut.add(elem_20_rw, name="elem_20_rw")
+                memory_map = MemoryMap(addr_width=16, data_width=8, alignment=2)
+                memory_map.add_resource(elem_20_rw, name="elem_20_rw", size=3)
 
+                dut = csr.Multiplexer(memory_map, shadow_overlaps=shadow_overlaps)
                 bus = dut.bus
 
                 def sim_test():
@@ -392,7 +359,7 @@ def test_add_wrong_sub_bus(self):
             self.dut.add(sub_bus=1)
 
     def test_add_wrong_data_width(self):
-        mux = csr.Multiplexer(addr_width=10, data_width=16)
+        mux = csr.Multiplexer(MemoryMap(addr_width=10, data_width=16))
         Fragment.get(mux, platform=None) # silence UnusedElaboratable
 
         with self.assertRaisesRegex(ValueError,
@@ -409,14 +376,19 @@ def test_add_wrong_out_of_bounds(self):
             self.dut.add(iface)
 
     def test_sim(self):
-        mux_1  = csr.Multiplexer(addr_width=10, data_width=8)
         elem_1 = csr.Element(8, "rw", path=("elem_1",))
-        mux_1.add(elem_1, name="elem_1")
-        self.dut.add(mux_1.bus)
-
-        mux_2  = csr.Multiplexer(addr_width=10, data_width=8)
         elem_2 = csr.Element(8, "rw", path=("elem_2",))
-        mux_2.add(elem_2, name="elem_2", addr=2)
+
+        memory_map_1 = MemoryMap(addr_width=10, data_width=8)
+        memory_map_1.add_resource(elem_1, name="elem_1", size=1)
+
+        memory_map_2 = MemoryMap(addr_width=10, data_width=8)
+        memory_map_2.add_resource(elem_2, name="elem_2", size=1, addr=2)
+
+        mux_1 = csr.Multiplexer(memory_map_1)
+        mux_2 = csr.Multiplexer(memory_map_2)
+
+        self.dut.add(mux_1.bus)
         self.dut.add(mux_2.bus)
 
         elem_1_info = self.dut.bus.memory_map.find_resource(elem_1)

tests/test_csr_wishbone.py

@@ -6,6 +6,7 @@
 
 from amaranth_soc import csr
 from amaranth_soc.csr.wishbone import *
+from amaranth_soc.memory import MemoryMap
 
 
 class MockRegister(Elaboratable):
@@ -42,12 +43,15 @@ def test_wrong_csr_bus_data_width(self):
             WishboneCSRBridge(csr_bus)
 
     def test_narrow(self):
-        mux   = csr.Multiplexer(addr_width=10, data_width=8)
-        reg_1 = MockRegister(8, name="reg_1")
-        mux.add(reg_1.element, name="reg_1")
+        reg_1 = MockRegister( 8, name="reg_1")
         reg_2 = MockRegister(16, name="reg_2")
-        mux.add(reg_2.element, name="reg_2")
-        dut   = WishboneCSRBridge(mux.bus)
+
+        memory_map = MemoryMap(addr_width=10, data_width=8)
+        reg_1.element.add_to(memory_map, name="reg_1")
+        reg_2.element.add_to(memory_map, name="reg_2")
+
+        mux = csr.Multiplexer(memory_map)
+        dut = WishboneCSRBridge(mux.bus)
 
         def sim_test():
             yield dut.wb_bus.cyc.eq(1)
@@ -143,9 +147,12 @@ def sim_test():
             sim.run()
 
     def test_wide(self):
-        mux = csr.Multiplexer(addr_width=10, data_width=8)
         reg = MockRegister(32, name="reg")
-        mux.add(reg.element, name="reg")
+
+        memory_map = MemoryMap(addr_width=10, data_width=8)
+        reg.element.add_to(memory_map, name="reg")
+
+        mux = csr.Multiplexer(memory_map)
         dut = WishboneCSRBridge(mux.bus, data_width=32)
 
         def sim_test():