amaranth-lang / amaranth-boards

Board definitions for Amaranth HDL
BSD 2-Clause "Simplified" License
108 stars 110 forks source link

How to handle dual-row PMODs? #219

Closed josuah closed 1 year ago

josuah commented 1 year ago

PMODs are coming in two flavors:

BIG Pmod (2 rows), like how Amaranth offers to declare them:

┌───┬───┬───┬───┬───┬───┐
│VCC│GND│ 4 │ 3 │ 2 │ 1 │
├───┼───┼───┼───┼───┼───┤
│VCC│GND│10 │ 9 │ 8 │ 7 │
└───┴───┴───┴───┴───┴───┘

little Pmod (1 row):

┌───┬───┬───┬───┬───┬───┐
│VCC│GND│ 4 │ 3 │ 2 │ 1 │
└───┴───┴───┴───┴───┴───┘

Also, larger pmods can be split in two little PMODs, and then the pinout declared for the larger PMOD does not cover the lower half anymore:

┌───┬───┬───┬───┬───┬───┐
│VCC│GND│ 4 │ 3 │ 2 │ 1 │
├───┼───┼───┼───┼───┼───┤
│VCC│GND│ 4 │ 3 │ 2 │ 1 │ <- how to use this in i.e. PmodSPIType2Resource?
└───┴───┴───┴───┴───┴───┘

My current approach for it is declaring the board like this:

    connectors  = [
        Connector("pmod", 0, "4  2  47 45 - - 3  48 46 44 - -"),
        Connector("pmod", 1,                 "3  48 46 44 - -"),
        Connector("pmod", 2, "43 38 34 31 - - 42 36 32 28 - -"),
        Connector("pmod", 3,                 "42 36 32 28 - -"),
        Connector("pmod", 4, "27 25 21 19 - - 26 23 20 18 - -"),
        Connector("pmod", 5,                 "26 23 20 18 - -"),
    ]

Then I can use the even numbers as 1x or 2x Pmods, and the odd numbers as 1x Pmods.

Is this the way to go? That would likely mean a big patch changing all the numbering of all PMODs, Then I would provide a commit that split them all that way?

Or Maybe an extra wrapper function that isolates the "top half" or "bottom half" for a resource? Eventually a decorator to add in extensions/pmod.py?

I am fine with implementing it ad-hoc on my projects though, that might be how everyone else does it in practice.

P.S.: not sure if better asked here or on amaranth repo's discussions

mithro commented 1 year ago

FWIW - I have a spreadsheet which goes into quite a bit of detail about the various options around PMOD connectors -> https://docs.google.com/spreadsheets/d/1D-GboyrP57VVpejQzEm0P1WEORo1LAIt92hk1bZGEoo/edit#gid=1854802810 -- it is getting a bit old these days but should still be pretty relavent.

See the "Low Speed Config" for the "Type classification" and "PMOD High Speed" for how high speed lanes are generally laid out.

mithro commented 1 year ago

A long time ago I started trying to make something for Migen @ https://github.com/mithro/HDMI2USB-litex-firmware-old/compare/nextgen...pmod-extra

josuah commented 1 year ago

@mithro I saw the data table, thank you for this! it helps In the end, how they are used in the wild will tell what need to be done with them.

About what was done for Migen, this would be working on the Connector instead of Resource level. Interesting as well as this gave some inheritance.

Then out of that the Resources would get declared by the user rather than by the board I assume.

Maybe @whitequark declared Resources instead of Connectors for configuring Pmods types separately from their pinout... How to declare a physical pinout and then derive it into a particular Pmod type with classes? Maybe by declaring one class per connector, so that you can do class Pmod1Top(ConnectorPmod1Connector):...

josuah commented 1 year ago

Now that I think of it, there could be some work done on the Connector level:

Instead of this:

    connectors  = [
        Connector("pmod", 0, "4  2  47 45 - - 3  48 46 44 - -"),
        Connector("pmod", 1,                 "3  48 46 44 - -"),
        Connector("pmod", 2, "43 38 34 31 - - 42 36 32 28 - -"),
        Connector("pmod", 3,                 "42 36 32 28 - -"),
    ]

Go with something new like this:

    connectors  = [
        *DualPmodConnector(0, "4  2  47 45 - - 3  48 46 44 - -"),
        *DualPmodConnector(1, "43 38 34 31 - - 42 36 32 28 - -"),
    ]

Which could expand into something like this:

    connectors  = [
        Connector("pmod", 0, "4  2  47 45 - - 3  48 46 44 - -"),
        Connector("pmod_top", 0, "4  2  47 45 - -"),
        Connector("pmod_bot", 0, "3  48 46 44 - -"),
        Connector("pmod", 1, "43 38 34 31 - - 42 36 32 28 - -"),
        Connector("pmod_top", 1, "43 38 34 31 - -"),
        Connector("pmod_bot", 1, "42 36 32 28 - -"),
    ]

(+) This would be a drop-in replacement (avoid clashing with existing code). (+) This would allow some batch conversion process (s/Connector("pmod"/DualPmodConnector(/) (+) The top or bottom half can now be requested explicitly. (-) Not sure this would work: PmodSPIType2Resource("my_spi_port", 0, pmod_top=1) (-) Not sure that is the direction @whitequark wants to go.

Let's try this locally and see how it goes...

josuah commented 1 year ago

There might be some more work on the PMOD class for something like this...

$ python synthesize.py
Traceback (most recent call last):
  File "/home/josuah/tinyvision-ai-inc/amaranth/synthesize.py", line 22, in <module>
    PmodGPIOType1Resource("gpio", 0, pmod_top=0)
TypeError: PmodGPIOType1Resource() got an unexpected keyword argument 'pmod_top'

It would be nice if something simple could be done, but not sure if worth making Amaranth more complex for this.

josuah commented 1 year ago

What about an extra top=True kwarg:

 def PmodSPIType2Resource(name, number, *args, pmod, top=True):
+    if top:
+        p1, p2, p3, p4 = "1","2","3","4"
+    else:
+        p1, p2, p3, p4 = "7","8","9","10"
     return Resource(name, number,
         Subsignal("cs",   PinsN(p1, dir="o", conn=("pmod", pmod))),
         Subsignal("clk",   Pins(p2, dir="o", conn=("pmod", pmod))),
         Subsignal("copi",  Pins(p3, dir="o", conn=("pmod", pmod))),
         Subsignal("cipo",  Pins(p4, dir="i", conn=("pmod", pmod))),
         *args
     )

(+) Less intrusive (no change to anything else than pmod.py) (+) Easier to debug (no extra indirection, directly the right line on error messages) (-) A bit less generic maybe

josuah commented 1 year ago

I can define my local pmod.py with the above, that solves my problem. I will test it a bit then propose it upstream.