error propegating clock domain

[chrisjohgorman]

Hello,

First off apologies for not having a minimum working example.

I'm very new to amaranth and would like an opinion on how to properly propagate the domain of a clock module. I'm using Amaranth 0.5.2 on an arch Linux system with Python 3.12.

I am using a tutorial I found to try to learn Amaranth and eventually build a risc-v processor on https://github.com/bl0x/learn-fpga-amaranth . I call a python script to initiate the build and j-tag steps for the FPGA. To run it I call python boards/digilent_cmod_a7.py 2. This runs the second step in the build process for the FPGA, which is a blinking lite demo.

The following is the code for boards/digilent_cmod_a7.py

from amaranth.build import *
from amaranth_boards.cmod_a7 import *

from top import Top

if __name__ == "__main__":
    platform = CmodA7_35Platform(toolchain="Vivado")
    gpio = ("gpio", 0)
    platform.add_resources([
        Resource("uart", 1,
             Subsignal("tx", Pins("1", conn=gpio, dir='o')),
             Subsignal("rx", Pins("2", conn=gpio, dir='i')),
             Attrs(IOSTANDARD="LVCMOS33")
        )
    ])

    # The platform allows access to the various resources defined
    # by the board definition from amaranth-boards.
    led0 = platform.request('led', 0)
    led1 = platform.request('led', 1)
    rgb = platform.request('rgb_led')
    leds = [led0, led1, rgb.r, rgb.g, rgb.b]
    uart = platform.request('uart', 1)

    platform.build(Top(leds, uart), do_program=True)

and the next file is the code for top.py.

from amaranth import *

import sys

class Top(Elaboratable):
    def __init__(self, leds, uart):
        if len(sys.argv) == 1:
            print("Usage: {} step_number".format(sys.argv[0]))
            exit(1)
        step = int(sys.argv[1])
        print("step = {}".format(step))
        self.leds = leds
        self.uart = uart

        # TODO: this is messy and should be done with iterating over dirs
        if step == 1:
            path = "01_blink"
        elif step == 2:
            path = "02_slower_blinky"
        elif step == 3:
            path = "03_blink_from_rom"
        elif step == 4:
            path = "04_instruction_decoder"
        elif step == 5:
            path = "05_register_bank"
        elif step == 6:
            path = "06_alu"
        elif step == 7:
            path = "07_assembler"
        elif step == 8:
            path = "08_jumps"
        elif step == 9:
            path = "09_branches"
        elif step == 10:
            path = "10_lui_auipc"
        elif step == 11:
            path = "11_modules"
        elif step == 12:
            path = "12_size_optimisation"
        elif step == 13:
            path = "13_subroutines"
        elif step == 14:
            path = "14_subroutines_v2"
        elif step == 15:
            path = "15_load"
        elif step == 16:
            path = "16_store"
        elif step == 17:
            path = "17_memory_map"
        elif step == 18:
            path = "18_mandelbrot"
        else:
            print("Invalid step_number {}.".format(step))
            exit(1)

        # add project path to give priority to this project
        # and avoid global including "soc" packages
        sys.path = [path] + sys.path
        from soc import SOC
        self.soc = SOC()

    def elaborate(self, platform):
        m = Module()
        soc = self.soc
        leds = self.leds
        uart = self.uart
        m.submodules.soc = soc

        # We connect the SOC leds signal to the various LEDs on the board.
        m.d.comb += [
            leds[0].o.eq(soc.leds[0]),
            leds[1].o.eq(soc.leds[1]),
            leds[2].o.eq(soc.leds[2]),
            leds[3].o.eq(soc.leds[3]),
            leds[4].o.eq(soc.leds[4]),
        ]

        # The TX port is added only later to the SOC
        if hasattr(soc, "tx"):
            m.d.comb += [
                uart.tx.eq(soc.tx)
            ]

        return m

These two files select the 02_slower_blinky/soc.py file to build. The soc.py file loads the clockworks.py. These files are the two I'm most concerned with as they don't seem to run properly when trying to propagate the domain of the clock domain defined in clockworks.py.

The following is the code for 02_slower_blinky/soc.py

from amaranth import Signal, Module
from amaranth.lib import wiring
from amaranth.lib.wiring import In, Out

# Import the new Clockworks class that handles creating new clock signals
from clockworks import Clockworks

class SOC(wiring.Component):

    leds: Out(5)

    def __init__(self):
        super().__init__()

    def elaborate(self, platform):

        m = Module()

        count = Signal(5)

        # Instantiate the clockwork with a divider of 2^21
        cw = Clockworks(m, slow=21)

        # Add the clockwork to the top module. If this is not done,
        # the logic will not be instantiated.
        m.submodules.cw = cw

        # The clockwork provides a new clock domain called 'slow'.
        # We replace the default sync domain with the new one to have the
        # counter run slower.
        m.d.slow += count.eq(count + 1)

        m.d.comb += self.leds.eq(count)

        return m

and finally the following code is for clockworks.py

from amaranth import Signal, Module, ClockDomain, ClockSignal
from amaranth.lib import wiring
from amaranth.lib.wiring import In, Out

# This module handles clock division and provides a new 'slow' clock domain

clockworks_domain_name = "slow"

class Clockworks(wiring.Component):

    o_slow: Out(1)

    def __init__(self, module, slow=0, sim_slow=None):

        # Since amaranth 0.6 clock domains do not propagate upwards (RFC59)
        module.domains += ClockDomain(clockworks_domain_name)

        # Since the module provides a new clock domain, which is accessible
        # via the top level module, we don't need to explicitly provide the
        # slow clock signal as an output.

        self.slow = slow
        if sim_slow is None:
            self.sim_slow = slow
        else:
            self.sim_slow = sim_slow

        super().__init__()

    def elaborate(self, platform):

        o_clk = Signal()
        m = Module()

        if self.slow != 0:
            # When the design is simulated, platform is None
            if platform is None:
                # Have the simulation run at a different speed than the
                # actual hardware (usually faster).
                slow_bit = self.sim_slow
            else:
                slow_bit = self.slow

            slow_clk = Signal(slow_bit + 1)
            m.d.sync += slow_clk.eq(slow_clk + 1)
            m.d.comb += o_clk.eq(slow_clk[slow_bit])

        else:
            # When no division is requested, just use the clock signal of
            # the default 'sync' domain.
            m.d.comb += o_clk.eq(ClockSignal("sync"))

        # Create the new clock domain
        m.domains += ClockDomain("slow")

        # Assign the slow clock to the clock signal of the new domain
        m.d.comb += ClockSignal("slow").eq(o_clk)

        return m

Running boards/digilent_cmod_a7.py 2 gives me the following error.

step = 2
Traceback (most recent call last):
  File "/home/chris/src/amaranth/learn-fpga-amaranth/boards/digilent_cmod_a7.py", line 25, in <module>
    platform.build(Top(leds, uart), do_program=True)
  File "/usr/lib/python3.12/site-packages/amaranth/build/plat.py", line 99, in build
    plan = self.prepare(elaboratable, name, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/amaranth/build/plat.py", line 140, in prepare
    fragment._propagate_domains(self.create_missing_domain, platform=self)
  File "/usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py", line 240, in _propagate_domains
    self._propagate_domains_up()
  File "/usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py", line 141, in _propagate_domains_up
    subfrag._propagate_domains_up(hierarchy + (hier_name,))
  File "/usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py", line 184, in _propagate_domains_up
    self.add_domains(domain)
  File "/usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py", line 91, in add_domains
    assert domain.name not in self.domains
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

The code that causes the error is the line with m.submodules.cw = cw from 02_slower_blinky/soc.py. If I edit amaranth/hdl/_ir.py, with the following patch, I can get this to build, but I believe it to be luck rather than knowing what's going on.

--- /home/chris/install/python-amaranth/src/amaranth/amaranth/hdl/_ir.py       2024-10-04 16:12:59.534701199 -0400
+++ /usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py       2024-10-05 11:00:50.106393002 -0400
@@ -86,9 +86,12 @@
         self.domain_renames = {}
 
     def add_domains(self, *domains):
+        count=0
         for domain in flatten(domains):
             assert isinstance(domain, _cd.ClockDomain)
-            assert domain.name not in self.domains
+            if (count != 0):
+                assert domain.name not in self.domains
+            count=count+1
             self.domains[domain.name] = domain
 
     def iter_domains(self):

I would appreciate any ideas on how to fix this in my code without my hacking at amaranth as I have.

Thanks

Chris

[zyp]

RFC 59 suggests the following way:

When propagating a domain upwards is actually desired (eg. for clock generator modules), a workaround is now necessary to route the domain to the toplevel. The simplest way is to set an eg. cd_sync attribute on the module in the constructor, then do m.domains.sync = clkgen.cd_sync in the top-level.

[chrisjohgorman]

Thank you @zyp for the pointer to RFC 59. I hacked at the code a bit, and found a fix that is mostly a guess, but it does load the code with this change.

The process used by @bl0x when he wrote his code appears to be step 3 in the Raitonale and Alternatives section.

3. Add a mechanism to explicitly import a domain from a submodule. A draft is proposed here, for discussion purposes:

m.submodules.clkgen = ClockGenerator(...)
m.domains.sync = ImportedDomain("clkgen", "sync") # Imports a domain defined in the submodule "clkgen" named "sync".

To get amaranth to run the code, I have to replace the += with an = from the ClockDomain assignment in the Clockworks class. I'm not too sure how correct this is, but it works for the blinking led example in this Q & A. The clockworks.py file changes to

rom amaranth import Signal, Module, ClockDomain, ClockSignal
from amaranth.lib import wiring
from amaranth.lib.wiring import In, Out

# This module handles clock division and provides a new 'slow' clock domain

clockworks_domain_name = "slow"

class Clockworks(wiring.Component):

    o_slow: Out(1)

    def __init__(self, module, slow=0, sim_slow=None):

        # Since amaranth 0.6 clock domains do not propagate upwards (RFC59)
        module.domains = ClockDomain(clockworks_domain_name)

        # Since the module provides a new clock domain, which is accessible
        # via the top level module, we don't need to explicitly provide the
        # slow clock signal as an output.

        self.slow = slow
        if sim_slow is None:
            self.sim_slow = slow
        else:
            self.sim_slow = sim_slow

        super().__init__()

    def elaborate(self, platform):

        o_clk = Signal()
        m = Module()

        if self.slow != 0:
            # When the design is simulated, platform is None
            if platform is None:
                # Have the simulation run at a different speed than the
                # actual hardware (usually faster).
                slow_bit = self.sim_slow
            else:
                slow_bit = self.slow

            slow_clk = Signal(slow_bit + 1)
            m.d.sync += slow_clk.eq(slow_clk + 1)
            m.d.comb += o_clk.eq(slow_clk[slow_bit])

        else:
            # When no division is requested, just use the clock signal of
            # the default 'sync' domain.
            m.d.comb += o_clk.eq(ClockSignal("sync"))

        # Create the new clock domain
        m.domains += ClockDomain("slow")

        # Assign the slow clock to the clock signal of the new domain
        m.d.comb += ClockSignal("slow").eq(o_clk)

        return m

I will pass this on to @bl0x in case it is a typo.

[whitequark]

def init(self, module, slow=0, sim_slow=None):

    # Since amaranth 0.6 clock domains do not propagate upwards (RFC59)
    module.domains = ClockDomain(clockworks_domain_name)

I don't recommend doing this. You should not be passing the Module object around. Instead, simply create the domain in the outer module.

[chrisjohgorman]

A question, how is this different module.domains = ClockDomain(clockworks_domain_name) and module.domains += ClockDomain(clockworks_domain_name), because the former works and the latter is what was originally written and does not work (for me anyway). Passing the module around was a solution to not being able to upwards propagate the clock domain.

If you have a moment, I could really use some pseudo code to understand what you mean by creating the domain in the outer module.

[whitequark]

Something like this:

class ClockResetGenerator(Elaboratable):
    def elaborate(self, platform):
        m = Module()
        m.d.comb += ClockSignal("sync").eq(...)
        return m

class Toplevel(Elaboratable):
    def elaborate(self, platform):
        m = Module()
        m.domains += ClockDomain("sync")
        m.submodules += ClockResetGenerator()
        return m

Your example of m.domains = ... seems completely broken, i.e. it should be a syntax error. It reassigns an internal variable in a way that shouldn't be done.

[chrisjohgorman]

It probably is completely broken. I have been hacking without understanding. It will take me a while to get up to speed. Thanks for the code pointer. I will try it out. All the best, Chris

…

On Mon, Oct 7, 2024 at 1:01 PM Catherine ***@***.***> wrote: Something like this: class ClockResetGenerator(Elaboratable): def elaborate(self, platform): m = Module() m.d.comb += ClockSignal("sync").eq(...) return m class Toplevel(Elaboratable): def elaborate(self, platform): m = Module() m.domains += ClockDomain("sync") m.submodules += ClockResetGenerator() return m Your example of m.domains = ... seems completely broken, i.e. it should be a syntax error. It reassigns an internal variable in a way that shouldn't be done. — Reply to this email directly, view it on GitHub <#1527 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG76X44XCZTHPGW5S6Q3RODZ2K46ZAVCNFSM6AAAAABPQJ23SKVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTAOBXGA4DINQ> . You are receiving this because you modified the open/close state.Message ID: ***@***.*** .com>