memory: remove support for address space extension.

The extend= parameter of MemoryMap.add_resource()/.add_window() was
added in 2d18ac7.

However, it prevents interfaces whose shape depend on a memory map from
being instantiated until the latter is frozen. Also, bus interfaces may
have a strict requirement on their address width, and it should not be
possible to implicitly increase it by adding a subordinate resource or
window.

As a side-effect of this change, csr.EventMonitor requires a populated
EventMap in order to be instantiated.

Author: jfng

amaranth_soc/csr/bus.py

@@ -577,7 +577,7 @@ def align_to(self, alignment):
         """
         return self._map.align_to(alignment)
 
-    def add(self, element, *, name, addr=None, alignment=None, extend=False):
+    def add(self, element, *, name, addr=None, alignment=None):
         """Add a register.
 
         See :meth:`MemoryMap.add_resource` for details.
@@ -587,7 +587,7 @@ def add(self, element, *, name, addr=None, alignment=None, extend=False):
 
         size = (element.width + self._map.data_width - 1) // self._map.data_width
         return self._map.add_resource(element, size=size, addr=addr, alignment=alignment,
-                                      extend=extend, name=name)
+                                      name=name)
 
     def elaborate(self, platform):
         m = Module()
@@ -721,7 +721,7 @@ def align_to(self, alignment):
         """
         return self._map.align_to(alignment)
 
-    def add(self, sub_bus, *, addr=None, extend=False):
+    def add(self, sub_bus, *, addr=None):
         """Add a window to a subordinate bus.
 
         See :meth:`MemoryMap.add_resource` for details.
@@ -732,7 +732,7 @@ def add(self, sub_bus, *, addr=None, extend=False):
             raise ValueError(f"Subordinate bus has data width {sub_bus.data_width}, which is not "
                              f"the same as decoder data width {self._map.data_width}")
         self._subs[sub_bus.memory_map] = sub_bus
-        return self._map.add_window(sub_bus.memory_map, addr=addr, extend=extend)
+        return self._map.add_window(sub_bus.memory_map, addr=addr)
 
     def elaborate(self, platform):
         m = Module()

amaranth_soc/csr/event.py

@@ -1,6 +1,8 @@
 # amaranth: UnusedElaboratable=no
+from math import ceil
 
 from amaranth import *
+from amaranth.utils import log2_int
 
 from . import Element, Multiplexer
 from .. import event
@@ -16,50 +18,40 @@ class EventMonitor(Elaboratable):
 
     CSR registers
     -------------
-    enable : ``self.src.event_map.size``, read/write
+    enable : ``event_map.size``, read/write
         Enabled events. See :meth:`..event.EventMap.sources` for layout.
-    pending : ``self.src.event_map.size``, read/clear
+    pending : ``event_map.size``, read/clear
         Pending events. See :meth:`..event.EventMap.sources` for layout.
 
     Parameters
     ----------
+    event_map : :class:`..event.EventMap`
+        A collection of event sources.
+    trigger : :class:`..event.Source.Trigger`
+        Trigger mode. See :class:`..event.Source`.
     data_width : int
         CSR bus data width. See :class:`..csr.Interface`.
     alignment : int, power-of-2 exponent
         CSR address alignment. See :class:`..memory.MemoryMap`.
-    trigger : :class:`..event.Source.Trigger`
-        Trigger mode. See :class:`..event.Source`.
     name : str
         Window name. Optional. See :class:`..memory.MemoryMap`.
     """
-    def __init__(self, *, data_width, alignment=0, trigger="level", name=None):
-        choices = ("level", "rise", "fall")
-        if not isinstance(trigger, event.Source.Trigger) and trigger not in choices:
-            raise ValueError("Invalid trigger mode {!r}; must be one of {}"
-                             .format(trigger, ", ".join(choices)))
-
-        self._trigger = event.Source.Trigger(trigger)
-        self._map     = event.EventMap()
-        self._monitor = None
-        self._enable  = None
-        self._pending = None
-        self._mux     = Multiplexer(addr_width=1, data_width=data_width, alignment=alignment,
-                                    name=name)
-        self._frozen  = False
-
-    def freeze(self):
-        """Freeze the event monitor.
-
-        Once the event monitor is frozen, subordinate sources cannot be added anymore.
-        """
-        if self._frozen:
-            return
-        self._monitor = event.Monitor(self._map, trigger=self._trigger)
-        self._enable  = Element(self._map.size, "rw", path=("enable",))
-        self._pending = Element(self._map.size, "rw", path=("pending",))
-        self._mux.add(self._enable,  name="enable",  extend=True)
-        self._mux.add(self._pending, name="pending", extend=True)
-        self._frozen  = True
+    def __init__(self, event_map, *, trigger="level", data_width, alignment=0, name=None):
+        if not isinstance(data_width, int) or data_width <= 0:
+            raise ValueError(f"Data width must be a positive integer, not {data_width!r}")
+        if not isinstance(alignment, int) or alignment < 0:
+            raise ValueError(f"Alignment must be a non-negative integer, not {alignment!r}")
+
+        self._monitor = event.Monitor(event_map, trigger=trigger)
+        self._enable  = Element(event_map.size, "rw", path=("enable",))
+        self._pending = Element(event_map.size, "rw", path=("pending",))
+
+        elem_size  = ceil(event_map.size / data_width)
+        addr_width = 1 + max(log2_int(elem_size, need_pow2=False), alignment)
+        self._mux  = Multiplexer(addr_width=addr_width, data_width=data_width,
+                                 alignment=alignment, name=name)
+        self._mux.add(self._enable,  name="enable")
+        self._mux.add(self._pending, name="pending")
 
     @property
     def src(self):
@@ -70,7 +62,6 @@ def src(self):
         An :class:`..event.Source`. Its input line is asserted by the monitor when a subordinate
         event is enabled and pending.
         """
-        self.freeze()
         return self._monitor.src
 
     @property
@@ -81,19 +72,9 @@ def bus(self):
         ------------
         A :class:`..csr.Interface` providing access to registers.
         """
-        self.freeze()
         return self._mux.bus
 
-    def add(self, src):
-        """Add a subordinate event source.
-
-        See :meth:`..event.EventMap.add` for details.
-        """
-        self._map.add(src)
-
     def elaborate(self, platform):
-        self.freeze()
-
         m = Module()
         m.submodules.monitor = self._monitor
         m.submodules.mux     = self._mux

amaranth_soc/memory.py

@@ -172,20 +172,6 @@ def __init__(self, *, addr_width, data_width, alignment=0, name=None):
     def addr_width(self):
         return self._addr_width
 
-    @addr_width.setter
-    def addr_width(self, addr_width):
-        if self._frozen:
-            raise ValueError("Memory map has been frozen. Address width cannot be extended "
-                             "further")
-        if not isinstance(addr_width, int) or addr_width <= 0:
-            raise ValueError("Address width must be a positive integer, not {!r}"
-                             .format(addr_width))
-        if addr_width < self._addr_width:
-            raise ValueError("Address width {!r} must not be less than its previous value {!r}, "
-                             "because resources that were previously added may not fit anymore"
-                             .format(addr_width, self._addr_width))
-        self._addr_width = addr_width
-
     @property
     def data_width(self):
         return self._data_width
@@ -202,7 +188,7 @@ def freeze(self):
         """Freeze the memory map.
 
         Once the memory map is frozen, its visible state becomes immutable. Resources and windows
-        cannot be added anymore, and its address width cannot be extended further.
+        cannot be added anymore.
         """
         self._frozen = True
 
@@ -231,7 +217,7 @@ def align_to(self, alignment):
         self._next_addr = self._align_up(self._next_addr, max(alignment, self.alignment))
         return self._next_addr
 
-    def _compute_addr_range(self, addr, size, step=1, *, alignment, extend):
+    def _compute_addr_range(self, addr, size, step=1, *, alignment):
         if addr is not None:
             if not isinstance(addr, int) or addr < 0:
                 raise ValueError("Address must be a non-negative integer, not {!r}"
@@ -249,12 +235,9 @@ def _compute_addr_range(self, addr, size, step=1, *, alignment, extend):
         size = self._align_up(max(size, 1), alignment)
 
         if addr > (1 << self.addr_width) or addr + size > (1 << self.addr_width):
-            if extend:
-                self.addr_width = bits_for(addr + size)
-            else:
-                raise ValueError("Address range {:#x}..{:#x} out of bounds for memory map spanning "
-                                 "range {:#x}..{:#x} ({} address bits)"
-                                 .format(addr, addr + size, 0, 1 << self.addr_width, self.addr_width))
+            raise ValueError("Address range {:#x}..{:#x} out of bounds for memory map spanning "
+                             "range {:#x}..{:#x} ({} address bits)"
+                             .format(addr, addr + size, 0, 1 << self.addr_width, self.addr_width))
 
         addr_range = range(addr, addr + size, step)
         overlaps = self._ranges.overlaps(addr_range)
@@ -274,7 +257,7 @@ def _compute_addr_range(self, addr, size, step=1, *, alignment, extend):
 
         return addr_range
 
-    def add_resource(self, resource, *, name, size, addr=None, alignment=None, extend=False):
+    def add_resource(self, resource, *, name, size, addr=None, alignment=None):
         """Add a resource.
 
         A resource is any device on the bus that is a destination for bus transactions, e.g.
@@ -296,9 +279,6 @@ def add_resource(self, resource, *, name, size, addr=None, alignment=None, exten
             ``2 ** max(alignment, self.alignment)``.
         alignment : int, power-of-2 exponent
             Alignment of the resource. Optional. If ``None``, the memory map alignment is used.
-        extend: bool
-            Allow memory map extension. If ``True``, the upper bound of the address space is
-            raised as needed, in order to fit a resource that would otherwise be out of bounds.
 
         Return value
         ------------
@@ -336,7 +316,7 @@ def add_resource(self, resource, *, name, size, addr=None, alignment=None, exten
         else:
             alignment = self.alignment
 
-        addr_range = self._compute_addr_range(addr, size, alignment=alignment, extend=extend)
+        addr_range = self._compute_addr_range(addr, size, alignment=alignment)
         self._ranges.insert(addr_range, resource)
         self._resources[id(resource)] = resource, name, addr_range
         self._namespace[name] = resource
@@ -356,7 +336,7 @@ def resources(self):
         for resource, resource_name, resource_range in self._resources.values():
             yield resource, resource_name, (resource_range.start, resource_range.stop)
 
-    def add_window(self, window, *, addr=None, sparse=None, extend=False):
+    def add_window(self, window, *, addr=None, sparse=None):
         """Add a window.
 
         A window is a device on a bus that provides access to a different bus, i.e. a bus bridge.
@@ -386,9 +366,6 @@ def add_window(self, window, *, addr=None, sparse=None, extend=False):
         sparse : bool
             Address translation type. Optional. Ignored if the datapath widths of both memory maps
             are equal; must be specified otherwise.
-        extend : bool
-            Allow memory map extension. If ``True``, the upper bound of the address space is
-            raised as needed, in order to fit a window that would otherwise be out of bounds.
 
         Return value
         ------------
@@ -464,8 +441,7 @@ def add_window(self, window, *, addr=None, sparse=None, extend=False):
         # a window can still be aligned using align_to().
         alignment = max(self.alignment, window.addr_width // ratio)
 
-        addr_range = self._compute_addr_range(addr, size, ratio, alignment=alignment,
-                                              extend=extend)
+        addr_range = self._compute_addr_range(addr, size, ratio, alignment=alignment)
         window.freeze()
         self._ranges.insert(addr_range, window)
         self._windows[id(window)] = window, addr_range

amaranth_soc/wishbone/bus.py

@@ -342,7 +342,7 @@ def align_to(self, alignment):
         """
         return self._map.align_to(alignment)
 
-    def add(self, sub_bus, *, addr=None, sparse=False, extend=False):
+    def add(self, sub_bus, *, addr=None, sparse=False):
         """Add a window to a subordinate bus.
 
         The decoder can perform either sparse or dense address translation. If dense address
@@ -376,7 +376,7 @@ def add(self, sub_bus, *, addr=None, sparse=False, extend=False):
                                  f"decoder does not have a corresponding input")
 
         self._subs[sub_bus.memory_map] = sub_bus
-        return self._map.add_window(sub_bus.memory_map, addr=addr, sparse=sparse, extend=extend)
+        return self._map.add_window(sub_bus.memory_map, addr=addr, sparse=sparse)
 
     def elaborate(self, platform):
         m = Module()

tests/test_csr_bus.py

@@ -152,11 +152,10 @@ def test_get_map_frozen(self):
         iface = csr.Interface(addr_width=16, data_width=8)
         iface.memory_map = MemoryMap(addr_width=16, data_width=8)
         with self.assertRaisesRegex(ValueError,
-                r"Memory map has been frozen. Address width cannot be extended "
-                r"further"):
-            iface.memory_map.addr_width = 24
+                r"Memory map has been frozen\. Cannot add resource 'foo'"):
+            iface.memory_map.add_resource("foo", name="foo", size=1)
 
-    def test_set_map_wrong(self):
+    def test_set_wrong_map(self):
         iface = csr.Interface(addr_width=16, data_width=8)
         with self.assertRaisesRegex(TypeError,
                 r"Memory map must be an instance of MemoryMap, not 'foo'"):
@@ -203,12 +202,6 @@ def test_add_two(self):
         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_extend(self):
-        elem_8b = csr.Element(8, "rw", path=("elem_8b",))
-        self.assertEqual(self.dut.add(elem_8b, name="elem_8b", addr=0x10000, extend=True),
-                         (0x10000, 0x10001))
-        self.assertEqual(self.dut.bus.addr_width, 17)
-
     def test_add_wrong(self):
         with self.assertRaisesRegex(TypeError,
                 r"Element must be an instance of csr\.Element, not 'foo'"):
@@ -409,12 +402,6 @@ def test_align_to(self):
         sub_2.memory_map = MemoryMap(addr_width=10, data_width=8)
         self.assertEqual(self.dut.add(sub_2), (0x1000, 0x1400, 1))
 
-    def test_add_extend(self):
-        iface = csr.Interface(addr_width=17, data_width=8, path=("iface",))
-        iface.memory_map = MemoryMap(addr_width=17, data_width=8)
-        self.assertEqual(self.dut.add(iface, extend=True), (0, 0x20000, 1))
-        self.assertEqual(self.dut.bus.addr_width, 18)
-
     def test_add_wrong_sub_bus(self):
         with self.assertRaisesRegex(TypeError,
                 r"Subordinate bus must be an instance of csr\.Interface, not 1"):