-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Updates to the PCIe interface to allow for shared control signals #234
base: main
Are you sure you want to change the base?
Conversation
This updates the pcie bundle definitions and adds a new set of helper functions for creating pcie-based bundles. The user will likely want to use `pcie-std()` instead of `pcie()` to make pcie bundles.
] | ||
|
||
for btype in bundle-types do: | ||
connect_lanes(btype) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, neat way to reduce typing.
@@ -168,29 +168,58 @@ public pcb-module module : | |||
bd-4x-prsnt.control.PRSNT# => sw.GPIO[12] | |||
bd-4x-prsnt.control.CLKREQ# => sw.GPIO[1] | |||
|
|||
; For the PCIe-2X, I'm going to demonstrate | |||
; how to use a shared port WAKE pin across | |||
; the different. This is common situation where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the different. => the different endpoints.
src/protocols/pcie.stanza
Outdated
PCIe-PRSNT# : make-port(`PRSNT#) ; hot plug detection | ||
PCIe-WAKE# : make-port(`PEWAKE#) | ||
PCIe-PERST# : make-port(`PERST#) ; PCI-E Reset signal | ||
PCIe-CLKREQ# : ,make-port(`CLKREQ#) ; clock request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extraneous comma?
|
||
inst dut1 : jsl/examples/protocols/pcie/pcie-src/module | ||
inst dut2 : jsl/examples/protocols/pcie/pcie-conn/module | ||
inst dut3 : jsl/examples/protocols/pcie/pcie-conn/module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dut3 not used? likely intended to be inst dut2 : jsl/examples/protocols/pcie/pcie-conn/module[2]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch 👍
There is no good way to do this. This is a hack. We will have to develop something better in the pin assignment implementation or just resort to out of band connections. Topologies do present issues here.
Unit tests pass when I run them locally - something weird is happening with the develop docker image. |
@Johnsel - sorry for the delay. What I've determined is that we don't really have a good way of supporting shared pins like the wake signal in pin assignment bundles. My previous attempts at doing this with internal supports/require statements were not satisfactory - they had a lot of limitations. For now, the best we can do is have the shared pins be completely out of band of the pin assignment solver. If you look at the To support this, I've refactor'd the PCIe bundles to allow all of the control pins to be optional. I think this gives some ability to create bundles without the shared pins which will likely be necessary in many cases. The key is that we can't included a shared pin across multiple This approach is also likely going to run into a lot of issues if the signals that are trying to be shared are topology nets. I've started a product issue for this internally and we will work on a fix but it may take some time. |
This updates the pcie bundle definitions and adds a new set of helper functions for creating pcie-based bundles. The user will likely want to use
pcie-std()
instead ofpcie()
to make pcie bundles. All control pins are not optional to allow for the user to share control pin functionality across multiple pin-assigned ports. This is likely going to be a common pattern in other connectors I would guess.This also adds a new PCIe example that uses a shared wake pin with fanout to multiple connectors.
@Johnsel - FYI - here is the example I mentioned in discord. Still rats nest issues but this should work and not require additional jumper resistor components.