amaranth-lang / amaranth

A modern hardware definition language and toolchain based on Python
https://amaranth-lang.org/docs/amaranth/
BSD 2-Clause "Simplified" License
1.53k stars 168 forks source link

[RFC] Add InstanceResource to use Instance I/O as Platform I/O #525

Closed Ravenslofty closed 7 months ago

Ravenslofty commented 3 years ago

This is an RFC for #308.

Some chips like the Intel Cyclone V SoC and Xilinx Zynq require Instances to access certain I/O functions not directly available through top-level module ports. Others, such as the Lattice iCE40, use Instances to source clocks.

Presently, the latter chip manually instantiates SB_HFOSC, but I think there should be a better way.

nMigen should provide an InstanceResource (or perhaps an InstanceConnector?) to treat I/O from an Instance as global I/O provided by platform.request.

whitequark commented 3 years ago

Okay, this isn't really an RFC because this doesn't explain the exact mechanism through which this will happen. There isn't anything to comment on.

anuejn commented 3 years ago

I propose, that we just allow to pass and Elaboratable object instead of a Pins object to Resource. This way one could add arbitrary logic to the platform. This, however, has one major downside: Since Instances dont expose their ports as attributes but rather as constructor arguments, every Instance first needs to be wrapped.

Usage Example for the proposal:

class ExamplePlatform(LatticeMachXO2Platform):
    device = "LCMXO2-2000HC"
    package = "TG100"
    speed = "6"
    default_clk = "clk_internal"

    class Osc(Elaboratable):
        def __init__(self, freq=2.08e6):
            self.freq = freq
            self.i = Signal()

        def elaborate(self, platform):
            m = Module()

            inst = m.submodules.inst = Instance("OSCH", o_OSC=self.i)
            inst.attrs["NOM_FREQ"] = str(self.freq / 1e6)

            return m

    resources = [
        Resource("clk_internal", 0, Osc())
    ]
whitequark commented 3 years ago

I propose, that we just allow to pass and Elaboratable object instead of a Pins object to Resource.

I like this direction. I think wrapping Instances is reasonable: you will usually want to do error checking, change pin polarity, make sure everything is hooked to the right clocks and resets...

There are multiple unresolved questions:

whitequark commented 3 years ago

Here is my counter-RFC.

Mechanism

We currently have a bunch of methods like:

def get_input_output(self, pin, port, attrs, invert):

Let's add two more, which connect IO of a requested resource to a particular port of a particular internal component. Both the name and the port in this case are purely symbolic--they are not interpreted other than by the get_internal function. You can still specify attrs (for auxiliary data, the need for which will no doubt arise) and invert (many common buses/instances do have active-low signals).

def get_internal_signal(self, signal, name, port, attrs, invert):
def get_internal_pin(self, pin, name, port, attrs, invert):

There would be two behaviors added to the DSL.

First, there would be a new Internal resource part with this signature:

class Internal:
    def __init__(self, name, port, *, dir, width=1, invert=False):

When requested, this Internal resource part produces a bare Signal--i.e. not a Pin. This resource part is convenient for internal buses, such as in PS7 or hps_system. The only possible XDR (as specified through request) is 0 and the only possible directions are i, o, and io.

Second, the existing Pins resource part would be altered to recognize Pins("@NAME.port[W]") similarly to Internal("NAME", "port", width=W). It would be possible to pass only one @-prefixed name passed to Pins, and its format must be strictly followed, except that the [W] part may be omitted for 1-bit pins.

When requested, this @-prefixed Pins resource part produces a Pin object that has the usual width, xdr, etc. This resource part is convenient for external buses driven through instances, such as in USRMCLK. (USRMCLK would have dir="oe", for example.) This feature is critically important because without it, it would not be possible to use USRMCLK with e.g. SPIFlashResources because this resource factory accepts as arguments pin names, not resource parts, and because I/O cores expect a Pin object and not a Signal.

Like other similar methods, get_internal_{signal,pin} would return an elaboratable (or None), which would then be injected into the toplevel fragment in the same way as normal I/O buffers.

Examples

Using ECP5 SPI Flash would look like:

        *SPIFlashResources(0,
            cs="R2", clk="@USRMCLK.MCLK", cipo="V2", copi="W2", wp="Y2", hold="W1",
            attrs=Attrs(IO_TYPE="LVCMOS33")
        ),

Using Cyclone-V HPS might look like:

        Resource("rst", 0, InternalN("hps_system", "hps_fpga_reset_reset_n", dir="i")),
        Resource("clk50", 0, Internal("hps_system", "clk_50_clk", dir="i"), Clock(50e6)),

        Resource("ddr3", 0,
            Subsignal("rst",     PinsN("@hps_system.memory_mem_reset_n", dir="o")),
            Subsignal("clk",     DiffPairs("@hps_system.memory_mem_reset", 
                                           "@hps_system.memory_mem_reset_n", dir="o")),
            Subsignal("clk_en",  Pins("@hps_system.memory_mem_cke", dir="o")),
            Subsignal("cs",      PinsN("@hps_system.memory_mem_cs_n", dir="o")),
            Subsignal("we",      PinsN("@hps_system.memory_mem_we_n", dir="o")),
            Subsignal("ras",     PinsN("@hps_system.memory_mem_ras_n", dir="o")),
            Subsignal("cas",     PinsN("@hps_system.memory_mem_cas_n", dir="o")),

I think Cyclone-V HPS looks like it might require the use of Internal and @-prefixed Pins resource parts at the same time. (I find the DDR3 interface somewhat baffling.) If it does in fact require instantiating e.g. differential buffers to use clk, then this is a strong argument in favor of the @-prefixed system with indirection.

Benefits

This mechanism has many desirable properties:

In particular, this would satisfy:

It would also let us get rid of the weird custom code that instantiates SB_HFOSC and similar oscillator macros. (Some work will be required to plumb the clock constraints through.)

Drawbacks

While this mechanism is fairly simple to implement, it may be hard to understand because of the extra overloading and indirection. This particularly applies to the @-prefixed Pins resource parts.

This mechanism adds two ways to do almost the same thing: Internal and @-prefixed Pins.

Alternatives

smunaut commented 3 years ago

How would passing arguments / config to the internal/hard block work ? Because those config would/could be "global" and not per-signal/pin.

whitequark commented 3 years ago

How would passing arguments / config to the internal/hard block work ?

Depends. Some hard blocks are singletons, but it is actually (counterintuitively) better to instantiate them as many times as requested. E.g. suppose we are exposing SB_HFOSC. We can implement get_internal_pin in such a way that it returns a new SB_HFOSC instance each time you call it. This will obviously blow up during placement if you request two of them, but that's ok. The flipside is that there needs to be no singleton logic, and the SB_HFOSC instance can be configured entirely from attributes (or maybe from the Clock(...). This only really works for instances that are a "single pin".

For the rest I think it is appropriate to configure it out of band. E.g. your USB case. I imagine most designs will have 1 and only 1 USB gadget in them--so you can just add an attribute to the platform. But if you have more than one, then I can imagime some sort of method like platform.add_usb_gadget("USB0", vid=, pid=) which would be requested by a Resource that has a bunch of Internals referring to USB0.

rroohhh commented 3 years ago

One thing that the RFC doesn't mention (but might be obvious) is, how the @-prefixed pins interact with the conn argument?

With the connector system there is already one way of indirection, I was wondering if there is a way to maybe expand it to use it for internal connections aswell.

whitequark commented 3 years ago

One thing that the RFC doesn't mention (but might be obvious) is, how the @-prefixed pins interact with the conn argument?

Excellent question. I have actually spent a good while thinking about it and haven't decided anything.

Chiefly, the issue is that a normal physical pin name always refers to exactly 1 pin, but the @-prefixed pin name does not (and will often not). This works fine in Pins itself (because that one is multibit), but it cannot work in connectors (because they are single bit).

The closest thing to a solution I see is a compromise where @-prefixed pins can be looked up through connectors but only if they have width 1.

whitequark commented 7 months ago

This proposal, or my counter-proposal, would have to go through our new RFC process to be accepted. In general, the current system of defining pins should probably be scrapped entirely and replaced with something better, so there is little point in defining things on top of it.