Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Colorlight i5 r7.0 with extension board #184

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

k-haze-nil
Copy link

There is already pull request for Colorlight i5 board #148
it has only sodimm and everything else is needed to be build on top of that.
But Colorlight i5 often comes with extension board with support of hdmi and handy organisation of pins for PMOD support.

Idk if adding support for board and board with extension board simultaneously is the right way. Probably not.

Connector("pmod", 0, "M17 R17 T18 K18 - - P17 R18 C18 U16 - -"), # P2A
Connector("pmod", 1, "G20 K20 L20 N18 - - J20 L18 M18 N17 - -"), # P2B
Connector("pmod", 2, "A18 A19 B19 D20 - - C17 B18 B20 F20 - -"), # P3A
Connector("pmod", 3, "E2 D2 B1 A12 - - D1 C1 C2 E3 - -"), # P3B
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pmod_3 pin 4 should be "A3" not "A12"

@BrettRD
Copy link

BrettRD commented Mar 5, 2022

Thanks for the platform file! this saved me a lot of time.
I think the convention is moving towards inheriting from the sodimm and declaring the mapping from sodimm to pmod.
I don't know how you inherit from the i5, i9, and ice_sugar though.

@DaveBerkeley
Copy link

I have a colorlight_i9 platform file. My PMOD connectors are wired with the rows reversed to this one. Have I got them the wrong way round?

Should I change my file to inherit or share this file? So there is a colorlight_ix.py definition, which can be passed the i5 / i9 etc?

https://github.com/DaveBerkeley/amaranth-boards/blob/colorlight_i9/amaranth_boards/colorlight_i9_r7_2.py

@BrettRD
Copy link

BrettRD commented Mar 5, 2022

@DaveBerkeley I think you're right, pin1 is the one further from the edge.
https://digilent.com/reference/_media/reference/pmod/pmod-interface-specification-1_3_1.pdf

There are other expansion boards waiting for a sane design pattern before the maintainer is willing to merge them
I'd love to see this expansion board use inheritance because I have three different cards to load into it.

@DaveBerkeley
Copy link

There are other expansion boards waiting for a sane design pattern before the maintainer is willing to merge them I'd love to see this expansion board use inheritance because I have three different cards to load into it.

Should there be a separate class for the board (with its SODIMM connector) and a wrapper class that connects the FPGA board to the expansion board? I'm now sure how best to do this. Is there a way of using connect() to do this?

@BrettRD
Copy link

BrettRD commented Mar 6, 2022

I was planning on inheriting from here #148 and defining everything on the sodimm

    connectors += [
        Connector("pmod", 0, {
            "1":"63",
            "2":"59",
            "3":"51",
            "4":"81",
            "7":"65",
            "8":"61",
            "9":"57",
            "10":"49",
        }, conn=("ddr2-sodimm-200p",0)),

        Connector("pmod", 1, {
            "1":"89",
            "2":"83",
            "3":"79",
            "4":"75",
            "7":"95",
            "8":"85",
            "9":"81",
            "10":"77",
        }, conn=("ddr2-sodimm-200p",0)),

        ...

    ])

The platform inherits from the LatticeECP5Platform class, that gets changed to ColorLightI9Platform or ColorLightI5Platform but I don't know how to tell the class to inherit from only one.

@DaveBerkeley
Copy link

That looks like a good approach. It reflects the physical wiring.

For litex-boards I added a board="i9" parameter to the platform and target files. Then took a deep copy of the config, modifying it (slightly) for the i9. So there is just one Colorlight_ix class, whose structure is defined by args.

So a base class for the module, with eg ctor(board="i9", revision="7.2"), which defines everything on the module, can then be placed in the expansion board (a derived class), which maps the sodimm to the pmods etc. So you could simply pass the board/revision args to the underlying base class in the expansion board class ctor. The expansion board class may not even need its own constructor. It is a just a set of mappings.

The Litex boards don't do this, but perhaps they should :

https://github.com/litex-hub/litex-boards/blob/master/litex_boards/platforms/colorlight_i5.py
https://github.com/litex-hub/litex-boards/blob/master/litex_boards/targets/colorlight_i5.py

@BrettRD
Copy link

BrettRD commented Mar 9, 2022

The expansion board really is just a collection of connectors and resources,
In the simplest case we could define the list of connectors and resources as const, then create very short sub-classes that define a new platform with the added connectors

# Describe the features of the expansion board and how they're wired
expansion_board_connectors = [...] # but in a sane namespace, or possibly a class
expansion_board_resources = [...]

# List the known compatible boards
class Colorlighti5R70ExtensionBoardPlatform(ColorLightI5Platform):
    connectors = super().connectors + expansion_board_connectors
    resources = super().resources + expansion_board_resources

class ColorLightI9ExtensionBoardPlatform(ColorLightI9Platform):
    connectors = super().connectors + expansion_board_connectors
    resources = super().resources + expansion_board_resources

class IceSugarExtensionBoardPlatform(IceSugarPlatform):
    connectors = super().connectors + expansion_board_connectors
    resources = super().resources + expansion_board_resources

That would allow Luna builds to use the platform as they always have
os.environ["LUNA_PLATFORM"] = "amaranth_boards/colorlight_i5_r7_0_ext_board:Colorlighti5R70ExtensionBoardPlatform"

]

connectors = [
Connector("pmod", 0, "M17 R17 T18 K18 - - P17 R18 C18 U16 - -"), # P2A
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should be swapped, like "P17 R18 C18 U16 - - M17 R17 T18 K18 - -". The PMOD standard mirrors pin numbering between the device and host side, so PMOD pin 1 on the host side is the inboard pin.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... the reason for the confusion is that the Muse Labs PMODs aren't built to spec. At least the 8 LED module I tested isn't. So, as written right now it'll work with Muse Labs PMODs, but to work with PMODs with properly mirrored connector layouts, the pin ordering I suggested earlier is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants