| Message ID | 20220810204750.3672362-2-bjorn.andersson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | usb: typec: mux: GPIO-based SBU mux | expand |
On 10/08/2022 23:47, Bjorn Andersson wrote: > Introduce a binding for GPIO-based mux hardware used for connecting, > disconnecting and switching orientation of the SBU lines in USB Type-C > applications. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 77 +++++++++++++++++++ > 1 file changed, 77 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > > diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > new file mode 100644 > index 000000000000..7d8aca40c7ca > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > @@ -0,0 +1,77 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/usb/gpio-sbu-mux.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: GPIO-based SBU mux > + > +maintainers: > + - Bjorn Andersson <bjorn.andersson@linaro.org> > + > +description: > + In USB Type-C applications the SBU lines needs to be connected, disconnected > + and swapped depending on the altmode and orientation. This binding describes > + a family of hardware which perform this based on GPIO controls. +Cc few folks. This looks familiar to: https://lore.kernel.org/linux-devicetree/eaf2fda8-0cd6-b518-10cb-4e21b5f8c909@linaro.org/T/#m39254b7f8970b3e1264f9d1a979557bb46ab162c Rob and Stephen had several concerns about that approach. Best regards, Krzysztof
On Thu, Aug 11, 2022 at 12:14:48PM +0300, Krzysztof Kozlowski wrote: > On 10/08/2022 23:47, Bjorn Andersson wrote: > > Introduce a binding for GPIO-based mux hardware used for connecting, > > disconnecting and switching orientation of the SBU lines in USB Type-C > > applications. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 77 +++++++++++++++++++ > > 1 file changed, 77 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > > > > diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > > new file mode 100644 > > index 000000000000..7d8aca40c7ca > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > > @@ -0,0 +1,77 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: "http://devicetree.org/schemas/usb/gpio-sbu-mux.yaml#" > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > + > > +title: GPIO-based SBU mux > > + > > +maintainers: > > + - Bjorn Andersson <bjorn.andersson@linaro.org> > > + > > +description: > > + In USB Type-C applications the SBU lines needs to be connected, disconnected > > + and swapped depending on the altmode and orientation. This binding describes > > + a family of hardware which perform this based on GPIO controls. > > +Cc few folks. > > This looks familiar to: > > https://lore.kernel.org/linux-devicetree/eaf2fda8-0cd6-b518-10cb-4e21b5f8c909@linaro.org/T/#m39254b7f8970b3e1264f9d1a979557bb46ab162c > > Rob and Stephen had several concerns about that approach. My overall concern is a bunch of one-off bindings with no one thinking about a variety of USB-C h/w. I need h/w diagrams and corresponding bindings. The key part being more than 1. I'm not all that familiar with the former to help on the bindings. Rob
On Sun 14 Aug 16:01 CDT 2022, Rob Herring wrote: > On Thu, Aug 11, 2022 at 12:14:48PM +0300, Krzysztof Kozlowski wrote: > > On 10/08/2022 23:47, Bjorn Andersson wrote: > > > Introduce a binding for GPIO-based mux hardware used for connecting, > > > disconnecting and switching orientation of the SBU lines in USB Type-C > > > applications. > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > --- > > > .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 77 +++++++++++++++++++ > > > 1 file changed, 77 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > > > new file mode 100644 > > > index 000000000000..7d8aca40c7ca > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml > > > @@ -0,0 +1,77 @@ > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > > +%YAML 1.2 > > > +--- > > > +$id: "http://devicetree.org/schemas/usb/gpio-sbu-mux.yaml#" > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > > + > > > +title: GPIO-based SBU mux > > > + > > > +maintainers: > > > + - Bjorn Andersson <bjorn.andersson@linaro.org> > > > + > > > +description: > > > + In USB Type-C applications the SBU lines needs to be connected, disconnected > > > + and swapped depending on the altmode and orientation. This binding describes > > > + a family of hardware which perform this based on GPIO controls. > > > > +Cc few folks. > > > > This looks familiar to: > > > > https://lore.kernel.org/linux-devicetree/eaf2fda8-0cd6-b518-10cb-4e21b5f8c909@linaro.org/T/#m39254b7f8970b3e1264f9d1a979557bb46ab162c > > > > Rob and Stephen had several concerns about that approach. > > My overall concern is a bunch of one-off bindings with no one thinking > about a variety of USB-C h/w. I need h/w diagrams and corresponding > bindings. The key part being more than 1. I'm not all that familiar with > the former to help on the bindings. > This is the setup that we're dealing with: +------------- - - USB connector | SoC +-+ | | | | +-----+ |*|<------- HS -----|-->| HS | |*|<------- HS -----|-->| phy |<-+ +--------+ | | | +-----+ \->| | | | | | dwc3 | | | | +-----+ /->| | |*|<------- SS -----|-->| |<-+ +--------+ |*|<------- SS -----|-->| QMP | |*|<------- SS -----|-->| phy | |*|<------- SS -----|-->| |<-\ +------------+ | | | +-----+ \->| | | | | | DP | | | +-----+ | | controller | |*|<-->| SBU |<-----|--------------->| | |*|<-->| mux |<-----|--------------->| | | | +----+ | +------------+ +-+ | +------------- - - The dwc3 controller is connected to the HS phy for HighSpeed signals and QMP phy to be muxed out on 0, 2 or 4 of the SuperSpeed pins (for DP-only, USB/DP combo or USB-only mode). The DisplayPort controller is connected to the same QMP phy, for and is muxed onto the same 0, 2 or 4 of the SuperSpeed pins (for USB-only, USB/DP combo or DP-only mode). The SuperSpeed pins can be switched around within the QMP phy, to handle the case where the USB Type-C cable is flipped around. The AUX pins of the DP controller are connected to the SBU pins in the connector. These signals needs to be disconnected while DP mode is not negotiated with the remote. The DP controller does not support swapping the two lines. The disconnecting and swapping thereby needs to be performed by an external entity. For which we have a few examples already, such as fcs,fsa4480. Lastly, in USB Power Delivery, the hot-plug signal found in a physical DisplayPort or HDMI cable is communicated as a message. So the USB Type-C controller must be able to pass this onto the DP controller. I model the usb-c-connector as a child of the USB Type-C controller, with the following representation of the connections: connector { compatible = "usb-c-connector"; ports { port@0 { reg = <0>; endpoint { remote-endpoint = <&dwc3>; }; }; port@1 { reg = <1>; endpoint@0 { reg = <0>; remote-endpoint = <&qmp_phy>; }; endpoint@1 { reg = <1>; remote-endpoint = <&dp_controller>; }; port@2 { reg = <2>; endpoint { remote-endpoint = <&sbu_mux>; }; }; }; }; This allows the USB Type-C controller to: 1) Perform USB role switching in the dwc3 on port@0 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on port@1:0, implement a drm_bridge for signalling HPD back to the DP controller on port@1:1 3) Orientation and muxing (connecting/disconnecting) the SBU/AUX lines in the SBU mux on port@2. The SBU mux in several of these designs is a component that takes a power supply and two GPIOs, for enabling/disabling the connection and flip the switch (which is used to swap the lines). I hope this helps with the bigger picture. Regards, Bjorn
Quoting Bjorn Andersson (2022-08-17 16:00:05) > > This is the setup that we're dealing with: > > +------------- - - > USB connector | SoC > +-+ | > | | | +-----+ > |*|<------- HS -----|-->| HS | > |*|<------- HS -----|-->| phy |<-+ +--------+ > | | | +-----+ \->| | > | | | | dwc3 | > | | | +-----+ /->| | > |*|<------- SS -----|-->| |<-+ +--------+ > |*|<------- SS -----|-->| QMP | > |*|<------- SS -----|-->| phy | > |*|<------- SS -----|-->| |<-\ +------------+ > | | | +-----+ \->| | > | | | | DP | > | | +-----+ | | controller | > |*|<-->| SBU |<-----|--------------->| | > |*|<-->| mux |<-----|--------------->| | > | | +----+ | +------------+ > +-+ | > +------------- - - > > The dwc3 controller is connected to the HS phy for HighSpeed signals and > QMP phy to be muxed out on 0, 2 or 4 of the SuperSpeed pins (for > DP-only, USB/DP combo or USB-only mode). > > The DisplayPort controller is connected to the same QMP phy, for and is > muxed onto the same 0, 2 or 4 of the SuperSpeed pins (for USB-only, > USB/DP combo or DP-only mode). > > The SuperSpeed pins can be switched around within the QMP phy, to handle > the case where the USB Type-C cable is flipped around. > > > The AUX pins of the DP controller are connected to the SBU pins in the > connector. These signals needs to be disconnected while DP mode is not > negotiated with the remote. The DP controller does not support swapping > the two lines. > The disconnecting and swapping thereby needs to be performed by an > external entity. For which we have a few examples already, such as > fcs,fsa4480. By swapping you mean making SBU1 in the usb-c-connector be SBU2 and SBU2 in the usb-c-connector be SBU1 to the DP controller, right? For the case when the cable is flipped. > > Lastly, in USB Power Delivery, the hot-plug signal found in a physical > DisplayPort or HDMI cable is communicated as a message. So the USB > Type-C controller must be able to pass this onto the DP controller. > > > I model the usb-c-connector as a child of the USB Type-C controller, > with the following representation of the connections: > > connector { > compatible = "usb-c-connector"; > > ports { > port@0 { > reg = <0>; > endpoint { > remote-endpoint = <&dwc3>; > }; > }; > > port@1 { > reg = <1>; > endpoint@0 { > reg = <0>; > remote-endpoint = <&qmp_phy>; > }; > endpoint@1 { > reg = <1>; > remote-endpoint = <&dp_controller>; > }; I'd expect the QMP phy to physically be the only thing connected on the board. That matches the block diagram above. Inside the SoC the SS lines will be muxed through the QMP phy to the DP controller or the USB controller. Therefore, in the binding I'd expect there to be a single port@1 for the connector: port@1 { reg = <1>; endpoint@0 { reg = <0>; remote-endpoint = <&qmp_phy>; }; }; Somewhere above we would want 'data-lanes' to indicate how many SS lanes are connected between the connector and the phy and if any of those lanes need to be remapped in the phy. port@1 { reg = <1>; endpoint@0 { reg = <0>; remote-endpoint = <&qmp_phy>; data-lanes = <0 1 2 3>; }; }; This would be the block diagram configuration, but it doesn't seem right. Other designs only connect two lanes to the qmp phy and the other two connect to a USB hub. That's where it gets interesting because we don't know how to represent that. Do we make two endpoints in the usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we make two endpoints then do we need to have two endpoints all the time even though in this 4 SS line design it is redundant? port@1 { reg = <1>; endpoint@0 { // Represents RX1/TX1 reg = <0>; remote-endpoint = <&qmp_phy_lanes01>; }; endpoint@1 { // Represents RX2/TX2 reg = <1>; remote-endpoint = <&qmp_phy_lanes23>; }; }; I think we may need to always have two endpoints in the usb-c-connector because data-lanes alone can't always handle it for us. Especially when you consider the hub and DP phy designs. port@1 { reg = <1>; endpoint@0 { // Represents RX1/TX1 reg = <0>; remote-endpoint = <&usb3_hub_portN>; }; endpoint@1 { // Represents RX2/TX2 reg = <1>; remote-endpoint = <&qmp_phy_lanes23>; }; }; The remote-endpoint phandle is different, so data-lanes can't save us. I suspect putting data-lanes in the usb-c-connector is really just nonsense too, because the usb-c-connector is a dumb devicenode and it can't actively change lane routing. The endpoint that's connected should have the data-lanes property if for example, RX2 is connected to the phy's TX2 pins and TX2 is connected to the phy's RX2 pins. Then the driver for that endpoint can change the lane mapping at runtime to present the proper data on the right pins in the connector. > > port@2 { > reg = <2>; > endpoint { > remote-endpoint = <&sbu_mux>; > }; > }; > }; > }; > > This allows the USB Type-C controller to: > 1) Perform USB role switching in the dwc3 on port@0 > 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on > port@1:0, implement a drm_bridge for signalling HPD back to the DP > controller on port@1:1 We may need to have a port connection from the DP controller to the QMP phy. But given that the DP controller already has a 'phys' phandle to the QMP phy I think the DP controller driver could try to link to a drm bridge created in the phy driver that mainly handles the HPD signaling and any lane muxing/routing that needs to happen when DP pin configuration is present. > 3) Orientation and muxing (connecting/disconnecting) the SBU/AUX lines > in the SBU mux on port@2. > > The SBU mux in several of these designs is a component that takes a > power supply and two GPIOs, for enabling/disabling the connection and > flip the switch (which is used to swap the lines). The SBU mux sounds OK to me. I don't think the SBU lines can be split up, they're a pair that has to go together, just like CC lines and SS pairs are in the typec spec. Of course, in DP spec we can have a single DP lane which corresponds to a single RX or TX differential pair, but with typec that isn't possible, we always have RX and TX together. This actually brings up one final point. On the QMP phy we can reorder the lanes willy nilly from what I recall, so that RX1 could be RX2 and TX1 could be TX2. In such a design, we would have two ports in the qmp phy to connect to the two TX/RX pairs in the connector, but then we would need to tell the QMP phy that the lanes are all shifted. qmp_phy { ports { port@0 { reg = <0>; endpoint@0 { reg = <0>; remote-endpoint = <&usb_c_txrx1>; data-lanes = <1 2> }; endpoint@1 { reg = <1>; remote-endpoint = <&usb_c_txrx2>; data-lanes = <3 0>; }; }; }; }; This would do that for us, but when all four lanes are connected from the qmp phy directly to the connector we could just as easily have done it with one endpoint. qmp_phy { ports { port@0 { reg = <0>; endpoint@0 { reg = <0>; remote-endpoint = <&usb_c_ss>; data-lanes = <1 2 3 0> }; }; }; }; So should we explicitly have two endpoints in the usb-c-connector for the two pairs all the time, or should we represent that via data-lanes and only split up the connector's endpoint if we need to connect the usb-c-connector to two different endpoints?
> This would do that for us, but when all four lanes are connected from > the qmp phy directly to the connector we could just as easily have done > it with one endpoint. > > qmp_phy { > ports { > port@0 { > reg = <0>; > endpoint@0 { > reg = <0>; > remote-endpoint = <&usb_c_ss>; > data-lanes = <1 2 3 0> > }; > }; > }; > }; > > So should we explicitly have two endpoints in the usb-c-connector for > the two pairs all the time, or should we represent that via data-lanes > and only split up the connector's endpoint if we need to connect the > usb-c-connector to two different endpoints? I like 2 endpoints to represent the usb-c-connector, but that doesn't seem to be compatible (without introducing `data-lanes`, at least) with all the various combinations on the remote side, if that remote side is a DRM bridge with DP output capability (like it6505 or anx7625). That type of DRM bridge supports 1, 2 or 4 lane DP connections. So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1? That should support every conceivable configuration and bridge/PHY hardware. and also allows a way to specify any lane remapping (similar to what "data lanes" does) if that is required. Then we are consistent with what an endpoint represents, regardless of whether the DRM bridge has a DP panel (1,2 or 4 lane) or Type-C connector (2 or 4 lane) on its output side.
Quoting Prashant Malani (2022-08-19 13:14:25) > > This would do that for us, but when all four lanes are connected from > > the qmp phy directly to the connector we could just as easily have done > > it with one endpoint. > > > > qmp_phy { > > ports { > > port@0 { > > reg = <0>; > > endpoint@0 { > > reg = <0>; > > remote-endpoint = <&usb_c_ss>; > > data-lanes = <1 2 3 0> > > }; > > }; > > }; > > }; > > > > So should we explicitly have two endpoints in the usb-c-connector for > > the two pairs all the time, or should we represent that via data-lanes > > and only split up the connector's endpoint if we need to connect the > > usb-c-connector to two different endpoints? > > I like 2 endpoints to represent the usb-c-connector, but that doesn't seem > to be compatible (without introducing `data-lanes`, at least) with all > the various > combinations on the remote side, if that remote side is a DRM bridge with DP > output capability (like it6505 or anx7625). > That type of DRM bridge supports 1, 2 or 4 lane DP connections. Why can't the remote side that's a pure DP bridge (it6505) bundle however many lanes it wants into one endpoint? If it's a pure DP bridge we should design the bridge binding to have up to 4 endpoints, but sometimes 2 or 1 and then overlay data-lanes onto that binding so that we can tell the driver how to remap the lanes if it can. If the hardware can't support remapping lanes then data-lanes shouldn't be in the binding. > > So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1? > That should support every conceivable configuration and bridge/PHY hardware. > and also allows a way to specify any lane remapping (similar to what > "data lanes" does) > if that is required. > Then we are consistent with what an endpoint represents, regardless of whether > the DRM bridge has a DP panel (1,2 or 4 lane) or Type-C connector (2 > or 4 lane) on its output side. I'd like to think in terms of the usb-c-connector, where the DP altmode doesn't support one lane of DP and USB is only carried across two SS lanes. Essentially, two SS lanes are always together, hence the idea that we should have two endpoints in the SS port@1. In the simple case above it seems we can get away with only one endpoint in the SS port@1 which is probably fine? I just don't know how that is represented in the schema, but I suspect making another endpoint optional in the SS port@1 is the way to go. Will there ever be a time when all 4 usb-c-connector remote-endpoint phandles point to endpoints that are child nodes of different ports (i.e. different qmp_phy nodes) with a 4 endpoint schema? I don't think that is possible, so 4 endpoints is flexible but also verbose. It also means we would have to walk the endpoints to figure out lane remapping, wheres we can simply find the endpoint in the DP bridge ports and look at data-lanes directly.
On Thu 18 Aug 20:09 CDT 2022, Stephen Boyd wrote: > Quoting Bjorn Andersson (2022-08-17 16:00:05) > > > > This is the setup that we're dealing with: > > > > +------------- - - > > USB connector | SoC > > +-+ | > > | | | +-----+ > > |*|<------- HS -----|-->| HS | > > |*|<------- HS -----|-->| phy |<-+ +--------+ > > | | | +-----+ \->| | > > | | | | dwc3 | > > | | | +-----+ /->| | > > |*|<------- SS -----|-->| |<-+ +--------+ > > |*|<------- SS -----|-->| QMP | > > |*|<------- SS -----|-->| phy | > > |*|<------- SS -----|-->| |<-\ +------------+ > > | | | +-----+ \->| | > > | | | | DP | > > | | +-----+ | | controller | > > |*|<-->| SBU |<-----|--------------->| | > > |*|<-->| mux |<-----|--------------->| | > > | | +----+ | +------------+ > > +-+ | > > +------------- - - > > > > The dwc3 controller is connected to the HS phy for HighSpeed signals and > > QMP phy to be muxed out on 0, 2 or 4 of the SuperSpeed pins (for > > DP-only, USB/DP combo or USB-only mode). > > > > The DisplayPort controller is connected to the same QMP phy, for and is > > muxed onto the same 0, 2 or 4 of the SuperSpeed pins (for USB-only, > > USB/DP combo or DP-only mode). > > > > The SuperSpeed pins can be switched around within the QMP phy, to handle > > the case where the USB Type-C cable is flipped around. > > > > > > The AUX pins of the DP controller are connected to the SBU pins in the > > connector. These signals needs to be disconnected while DP mode is not > > negotiated with the remote. The DP controller does not support swapping > > the two lines. > > The disconnecting and swapping thereby needs to be performed by an > > external entity. For which we have a few examples already, such as > > fcs,fsa4480. > > By swapping you mean making SBU1 in the usb-c-connector be SBU2 and SBU2 > in the usb-c-connector be SBU1 to the DP controller, right? For the case > when the cable is flipped. > Correct. > > > > Lastly, in USB Power Delivery, the hot-plug signal found in a physical > > DisplayPort or HDMI cable is communicated as a message. So the USB > > Type-C controller must be able to pass this onto the DP controller. > > > > > > I model the usb-c-connector as a child of the USB Type-C controller, > > with the following representation of the connections: > > > > > > connector { > > compatible = "usb-c-connector"; > > > > ports { > > port@0 { > > reg = <0>; > > endpoint { > > remote-endpoint = <&dwc3>; > > }; > > }; > > > > port@1 { > > reg = <1>; > > endpoint@0 { > > reg = <0>; > > remote-endpoint = <&qmp_phy>; > > }; > > endpoint@1 { > > reg = <1>; > > remote-endpoint = <&dp_controller>; > > }; > > I'd expect the QMP phy to physically be the only thing connected on the > board. That matches the block diagram above. Inside the SoC the SS lines > will be muxed through the QMP phy to the DP controller or the USB > controller. Therefore, in the binding I'd expect there to be a single > port@1 for the connector: > > port@1 { > reg = <1>; > endpoint@0 { > reg = <0>; > remote-endpoint = <&qmp_phy>; > }; > }; > That is correct, the 4 SS pairs in the USB connector are connected to the QMP PHY pads. The second endpoint in port@1 comes from my RFC where I suggested adding a 4th port to the usb-c-connector for connecting the usb-c-connector to the DP controller for passing the virtual HPD signal. Rob suggested that this indication relates to the SS pins and wanted this to be part of port@1. But it's not actually a definition of any electrical connection. > Somewhere above we would want 'data-lanes' to indicate how many SS lanes > are connected between the connector and the phy and if any of those > lanes need to be remapped in the phy. > > port@1 { > reg = <1>; > endpoint@0 { > reg = <0>; > remote-endpoint = <&qmp_phy>; > data-lanes = <0 1 2 3>; > }; > }; > > This would be the block diagram configuration, but it doesn't seem > right. > The QMP phy will always be in control of the 4 lanes. The question though is what those 4 lanes are allocated for, because depending on the USB PD negotiation is might be 2 lanes DP + 2 lanes USB, or 4 lanes of either function. There are designs where we can only do 2 lanes DP, which today is described in the DP controller... > Other designs only connect two lanes to the qmp phy and the other two > connect to a USB hub. That's where it gets interesting because we don't > know how to represent that. Do we make two endpoints in the > usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS > RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we > make two endpoints then do we need to have two endpoints all the time > even though in this 4 SS line design it is redundant? > > port@1 { > reg = <1>; > endpoint@0 { // Represents RX1/TX1 > reg = <0>; > remote-endpoint = <&qmp_phy_lanes01>; > }; > endpoint@1 { // Represents RX2/TX2 > reg = <1>; > remote-endpoint = <&qmp_phy_lanes23>; > }; > }; > So on the other side of that PHY we would have a multi-port USB controller, or two USB controllers? Either way, this seems like a proper representation of the two different ports, but not something we can do with the QMP. > I think we may need to always have two endpoints in the usb-c-connector > because data-lanes alone can't always handle it for us. Especially when > you consider the hub and DP phy designs. > > port@1 { > reg = <1>; > endpoint@0 { // Represents RX1/TX1 > reg = <0>; > remote-endpoint = <&usb3_hub_portN>; > }; > endpoint@1 { // Represents RX2/TX2 > reg = <1>; > remote-endpoint = <&qmp_phy_lanes23>; > }; > }; > > The remote-endpoint phandle is different, so data-lanes can't save us. I > suspect putting data-lanes in the usb-c-connector is really just > nonsense too, because the usb-c-connector is a dumb devicenode and it > can't actively change lane routing. The endpoint that's connected should > have the data-lanes property if for example, RX2 is connected to the > phy's TX2 pins and TX2 is connected to the phy's RX2 pins. Then the > driver for that endpoint can change the lane mapping at runtime to > present the proper data on the right pins in the connector. > The QMP phy has certain ability to swap the signals around, so it's conceivable that a data-lanes property in the outgoing port definition could be used to reorder the SS lanes... But it would be unrelated to the USB vs DP selection in my view. All we want here is a connection between the usb-c-connector and the QMP phy, such that the usb-c-connector's Type-C controller can inform the QMP what has been negotiated. > > > > port@2 { > > reg = <2>; > > endpoint { > > remote-endpoint = <&sbu_mux>; > > }; > > }; > > }; > > }; > > > > This allows the USB Type-C controller to: > > 1) Perform USB role switching in the dwc3 on port@0 > > 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on > > port@1:0, implement a drm_bridge for signalling HPD back to the DP > > controller on port@1:1 > > We may need to have a port connection from the DP controller to the QMP > phy. But given that the DP controller already has a 'phys' phandle to > the QMP phy I think the DP controller driver could try to link to a drm > bridge created in the phy driver that mainly handles the HPD signaling > and any lane muxing/routing that needs to happen when DP pin > configuration is present. > The QMP has no knowledge of HPD signalling in Type-C, it's strictly a virtual thing living in the Type-C controller. The Type-C controller will request mux changes from the QMP and HPD signal changes as two completely independent events. Implementing a drm_bridge in the implementation backing the usb-c-connector mimics e.g. dp-connector (implemented in drivers/gpu/drm/bridge/display-connector.c) nicely. Implementing the drm_bridge in the QMP phy means that we just add state tracking for something that it doesn't know, hence we need another mechanism to the Type-C controller to inform the phy that the HPD signal has changed. This is analog to the case you have today, where the QMP has no knowledge of the GPIO pin that carries the HPD state in your design. > > 3) Orientation and muxing (connecting/disconnecting) the SBU/AUX lines > > in the SBU mux on port@2. > > > > The SBU mux in several of these designs is a component that takes a > > power supply and two GPIOs, for enabling/disabling the connection and > > flip the switch (which is used to swap the lines). > > The SBU mux sounds OK to me. I don't think the SBU lines can be split > up, they're a pair that has to go together, just like CC lines and SS > pairs are in the typec spec. Of course, in DP spec we can have a single > DP lane which corresponds to a single RX or TX differential pair, but > with typec that isn't possible, we always have RX and TX together. > > This actually brings up one final point. On the QMP phy we can reorder > the lanes willy nilly from what I recall, so that RX1 could be RX2 and > TX1 could be TX2. In such a design, we would have two ports in the qmp > phy to connect to the two TX/RX pairs in the connector, but then we > would need to tell the QMP phy that the lanes are all shifted. > > qmp_phy { > ports { > port@0 { > reg = <0>; > endpoint@0 { > reg = <0>; > remote-endpoint = <&usb_c_txrx1>; > data-lanes = <1 2> > }; > endpoint@1 { > reg = <1>; > remote-endpoint = <&usb_c_txrx2>; > data-lanes = <3 0>; > }; > }; > }; > }; > > This would do that for us, but when all four lanes are connected from > the qmp phy directly to the connector we could just as easily have done > it with one endpoint. > > qmp_phy { > ports { > port@0 { > reg = <0>; > endpoint@0 { > reg = <0>; > remote-endpoint = <&usb_c_ss>; > data-lanes = <1 2 3 0> > }; > }; > }; > }; > > So should we explicitly have two endpoints in the usb-c-connector for > the two pairs all the time, or should we represent that via data-lanes > and only split up the connector's endpoint if we need to connect the > usb-c-connector to two different endpoints? I think the endpoint of port@1 should represent the set of signals connected to the other side, in our case 1:1 with the QMP. I like the idea of adding data-lanes to the QMP side in order to describe any swapping of the pads, but I see that as a separate thing. If you have a design where your usb-c-connector is wired to two different PHYs and you have a Type-C controller that only negotiates the 2+2 mode, then I think it makes sense to represent that as two endpoint of port@1 - but the QMP side would only reference one of these endpoints. Regards, Bjorn
On Fri 19 Aug 15:49 CDT 2022, Stephen Boyd wrote: > Quoting Prashant Malani (2022-08-19 13:14:25) > > > This would do that for us, but when all four lanes are connected from > > > the qmp phy directly to the connector we could just as easily have done > > > it with one endpoint. > > > > > > qmp_phy { > > > ports { > > > port@0 { > > > reg = <0>; > > > endpoint@0 { > > > reg = <0>; > > > remote-endpoint = <&usb_c_ss>; > > > data-lanes = <1 2 3 0> > > > }; > > > }; > > > }; > > > }; > > > > > > So should we explicitly have two endpoints in the usb-c-connector for > > > the two pairs all the time, or should we represent that via data-lanes > > > and only split up the connector's endpoint if we need to connect the > > > usb-c-connector to two different endpoints? > > > > I like 2 endpoints to represent the usb-c-connector, but that doesn't seem > > to be compatible (without introducing `data-lanes`, at least) with all > > the various > > combinations on the remote side, if that remote side is a DRM bridge with DP > > output capability (like it6505 or anx7625). > > That type of DRM bridge supports 1, 2 or 4 lane DP connections. > > Why can't the remote side that's a pure DP bridge (it6505) bundle > however many lanes it wants into one endpoint? If it's a pure DP bridge > we should design the bridge binding to have up to 4 endpoints, but > sometimes 2 or 1 and then overlay data-lanes onto that binding so that > we can tell the driver how to remap the lanes if it can. If the hardware > can't support remapping lanes then data-lanes shouldn't be in the > binding. > I don't see why we would want to further complicate the of_graph by representing each signal pair between two fixed endpoint as individual endpoints. Let's describe the connections between the components, not the signals. > > > > So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1? > > That should support every conceivable configuration and bridge/PHY hardware. > > and also allows a way to specify any lane remapping (similar to what > > "data lanes" does) > > if that is required. > > Then we are consistent with what an endpoint represents, regardless of whether > > the DRM bridge has a DP panel (1,2 or 4 lane) or Type-C connector (2 > > or 4 lane) on its output side. > > I'd like to think in terms of the usb-c-connector, where the DP altmode > doesn't support one lane of DP and USB is only carried across two SS > lanes. Essentially, two SS lanes are always together, hence the idea > that we should have two endpoints in the SS port@1. In the simple case > above it seems we can get away with only one endpoint in the SS port@1 > which is probably fine? I just don't know how that is represented in the > schema, but I suspect making another endpoint optional in the SS port@1 > is the way to go. > > Will there ever be a time when all 4 usb-c-connector remote-endpoint > phandles point to endpoints that are child nodes of different ports > (i.e. different qmp_phy nodes) with a 4 endpoint schema? I don't think > that is possible, so 4 endpoints is flexible but also verbose. It also > means we would have to walk the endpoints to figure out lane remapping, > wheres we can simply find the endpoint in the DP bridge ports and look > at data-lanes directly. The existing implementation provides the interfaces usb_role_switch, usb_typec_mux and usb_typec_switch. These works based on the concept that the USB Type-C controller will request the endpoints connected to the usb-c-connector about changes such as "switch to host mode", "switch to 2+2 USB/DP combo" and "switch orientation to reverse". We use this same operations to inform any endpoint at any port about these events and they all react accordingly. Perhaps I'm misunderstanding your suggestion, but if you start representing each individual lane in the SuperSpeed interface I believe you would have to just abandon this interface and replace it with something like "give me USB on port@1/endpoint@0 and port@1/endpoint@1 and give me DP on port@1/endpoint@2 and port@1/endpoint@3". I presume that such an interface would work, but I'm afraid I don't see the merits of it and we would have to replace the Linux implementation. Regards, Bjorn
On Fri 19 Aug 15:14 CDT 2022, Prashant Malani wrote: > > This would do that for us, but when all four lanes are connected from > > the qmp phy directly to the connector we could just as easily have done > > it with one endpoint. > > > > qmp_phy { > > ports { > > port@0 { > > reg = <0>; > > endpoint@0 { > > reg = <0>; > > remote-endpoint = <&usb_c_ss>; > > data-lanes = <1 2 3 0> > > }; > > }; > > }; > > }; > > > > So should we explicitly have two endpoints in the usb-c-connector for > > the two pairs all the time, or should we represent that via data-lanes > > and only split up the connector's endpoint if we need to connect the > > usb-c-connector to two different endpoints? > > I like 2 endpoints to represent the usb-c-connector, but that doesn't seem > to be compatible (without introducing `data-lanes`, at least) with all > the various > combinations on the remote side, if that remote side is a DRM bridge with DP > output capability (like it6505 or anx7625). > That type of DRM bridge supports 1, 2 or 4 lane DP connections. > You can't physically connect 1, 2 or 4 lanes of DP from a DP chip to your usb-c-connector at the same time as you physically connect 0, 2 or 4 lanes of USB from a USB PHY. You must either have another component inbetween, or you will connect some predefined subset of those signals to each output. In the case where you have a mux of some sort inbetween, that would be the thing that the usb-c-connector's port@1/endpoint references. In the case that you hardwire 2 SS lanes to USB and 2 to the DP hardware, you could specify port@1 with two endpoints and the Type-C controller would be able to signal both when to turn on/off their signals. But you wouldn't be able to do orientation switching. > So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1? > That should support every conceivable configuration and bridge/PHY hardware. > and also allows a way to specify any lane remapping (similar to what > "data lanes" does) > if that is required. Wouldn't that prevent you from handling orientation switching, given that the graph is static? > Then we are consistent with what an endpoint represents, regardless of whether > the DRM bridge has a DP panel (1,2 or 4 lane) or Type-C connector (2 > or 4 lane) on its output side. We can represent that perfectly fine with the proposed bindings. In the USB Type-C case I have: dp-controller { phys = <&qmp>; ports { dp_hpd: port@1 { endpoint = <&port_1_endpoint_1>; }; }; }; qmp: qmp { port { qmp_out: endpoint { remote-endpoint = <&port_1_endpoint_0>; }; }; }; connector { compatible = "usb-c-connector"; ports { port@1 { port_1_endpoint_0: endpoint@0 { remote-endpoint = <&qmp_out>; }; port_1_endpoint_1: endpoint@1 { remote-endpoint = <&dp_hpd>; }; }; }; }; The dp-controller binding is defined to have the output on port@1 and by implementing a drm_bridge in the controller backing the connector it will find that. The controller can use the links to inform the QMP about muxing and orientation switching. In the case of DP we have: dp-controller { phys = <&dp_phy>; ports { dp_hpd: port@1 { endpoint = <&dp_connector>; }; }; }; dp_phy: dp-phy { compatible = "qcom,dp-phy"; }; connector { compatible = "dp-connector"; port { dp_connector: endpoint@0 { remote-endpoint = <&dp_hpd>; }; }; }; The link between the dp_phy and the dp connector could be expressed further, but this is a binding that already exists... Regards, Bjorn
On Fri, Aug 19, 2022 at 2:39 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Fri 19 Aug 15:49 CDT 2022, Stephen Boyd wrote: > > > > I like 2 endpoints to represent the usb-c-connector, but that doesn't seem > > > to be compatible (without introducing `data-lanes`, at least) with all > > > the various > > > combinations on the remote side, if that remote side is a DRM bridge with DP > > > output capability (like it6505 or anx7625). > > > That type of DRM bridge supports 1, 2 or 4 lane DP connections. > > > > Why can't the remote side that's a pure DP bridge (it6505) bundle > > however many lanes it wants into one endpoint? If it's a pure DP bridge > > we should design the bridge binding to have up to 4 endpoints, but > > sometimes 2 or 1 and then overlay data-lanes onto that binding so that > > we can tell the driver how to remap the lanes if it can. If the hardware > > can't support remapping lanes then data-lanes shouldn't be in the > > binding. 2 endpoints sounds fine to me. The overloading of the bridge-side endpoint to mean different things depending on what it's connected to seemed odd to me, but if that is acceptable for the bridge binding, then great. > The existing implementation provides the interfaces usb_role_switch, > usb_typec_mux and usb_typec_switch. These works based on the concept > that the USB Type-C controller will request the endpoints connected to > the usb-c-connector about changes such as "switch to host mode", "switch > to 2+2 USB/DP combo" and "switch orientation to reverse". We use this > same operations to inform any endpoint at any port about these events > and they all react accordingly. Right, but that implementation/assumption doesn't work so well when you have 2 Type-C ports which might route to the same bridge (2 lane from each). The other 2 lanes from the other endpoints can go to (say) a USB HUB. > > Perhaps I'm misunderstanding your suggestion, but if you start > representing each individual lane in the SuperSpeed interface I believe > you would have to just abandon this interface and replace it with > something like "give me USB on port@1/endpoint@0 and port@1/endpoint@1 > and give me DP on port@1/endpoint@2 and port@1/endpoint@3". I don't think that is necessary. The switch driver can register the switches ( and it can find out which end-points map to the same usb-c-connector).
On Fri, Aug 19, 2022 at 3:01 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > > You can't physically connect 1, 2 or 4 lanes of DP from a DP chip to > your usb-c-connector at the same time as you physically connect 0, 2 or > 4 lanes of USB from a USB PHY. I apologize in case I'm misunderstanding, but why is that not possible? anx7625 allows that configuration (2 lane DP + 2 lane USB going to a single USB-C-connector) Since the discussion is to support various conceivable hardware configurations That same anx7625 can support 1 1-lane DP (or 2 1-lane DPs), and 1 2-lane Type-C output. The cross-point switch allows for that level of configuration. > > So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1? > > That should support every conceivable configuration and bridge/PHY hardware. > > and also allows a way to specify any lane remapping (similar to what > > "data lanes" does) > > if that is required. > > Wouldn't that prevent you from handling orientation switching, given > that the graph is static? It depends. If the end-points from the usb-c-connector go to the same switch, then it should allow orientation switching (anx7625 allows this). The port driver would just tell the orientation switch(es) attached to it that we are in NORMAL or REVERSE orientation. The graph is static, since the hardware line routing between components doesn't change (e.g SSTX1 from the Type-C port is always routed to Pin X1,X2 on the switch hardware), but that is what the switch is for. Note that in this case, the expectation is that the switch driver only registers 1 switch (it can figure out that all 4 endpoints go to the same Type-C port). The limitation to orientation switching would depend on how the hardware is routed.
I realize I've hijacked this thread to discuss the QMP binding :-/ Quoting Bjorn Andersson (2022-08-19 14:26:32) > On Thu 18 Aug 20:09 CDT 2022, Stephen Boyd wrote: > > > Quoting Bjorn Andersson (2022-08-17 16:00:05) > > > > > > This is the setup that we're dealing with: > > > > > > +------------- - - > > > USB connector | SoC > > > +-+ | > > > | | | +-----+ > > > |*|<------- HS -----|-->| HS | > > > |*|<------- HS -----|-->| phy |<-+ +--------+ > > > | | | +-----+ \->| | > > > | | | | dwc3 | > > > | | | +-----+ /->| | > > > |*|<------- SS -----|-->| |<-+ +--------+ > > > |*|<------- SS -----|-->| QMP | > > > |*|<------- SS -----|-->| phy | > > > |*|<------- SS -----|-->| |<-\ +------------+ > > > | | | +-----+ \->| | > > > | | | | DP | > > > | | +-----+ | | controller | > > > |*|<-->| SBU |<-----|--------------->| | > > > |*|<-->| mux |<-----|--------------->| | > > > | | +----+ | +------------+ > > > +-+ | > > > +------------- - - > > > [...] > > I'd expect the QMP phy to physically be the only thing connected on the > > board. That matches the block diagram above. Inside the SoC the SS lines > > will be muxed through the QMP phy to the DP controller or the USB > > controller. Therefore, in the binding I'd expect there to be a single > > port@1 for the connector: > > > > port@1 { > > reg = <1>; > > endpoint@0 { > > reg = <0>; > > remote-endpoint = <&qmp_phy>; > > }; > > }; > > > > That is correct, the 4 SS pairs in the USB connector are connected to > the QMP PHY pads. > > > The second endpoint in port@1 comes from my RFC where I suggested adding > a 4th port to the usb-c-connector for connecting the usb-c-connector to > the DP controller for passing the virtual HPD signal. Rob suggested that > this indication relates to the SS pins and wanted this to be part of > port@1. But it's not actually a definition of any electrical connection. I suspect this is the root of the debate. Should the binding be describing logical connections between components or actual physical connections? And should the connector binding have endpoints for different altmodes, e.g. DP? What do we do if thunderbolt support is yet another PHY that sits behind the QMP phy? Do we need to make yet another endpoint in the usb-c-connector to represent this connection? I hope not. The QMP phy is doing type-c "muxing" of different PHYs (USB and DP) onto the SS lanes. Other altmodes that want to use the SS lanes would similarly need to be routed through the QMP phy and muxed onto the lanes when the altmode is used, e.g. thunderbolt. While it is certainly convenient to have a "DP" endpoint in the usb-c-connector, I feel that it is wrong, primarily because the DP phy has the QMP phy in between it and the usb-c-connector, but also because DP is an altmode/virtual construct built on top of the 4 lanes in the typec pinout. We should look at the binding from the perspective of the connector and figure out how the pinout can be mapped to the binding. That would allow board designers to ignore the internal SoC details, and stay focused on what is in the schematic, which is the qmp phy and the usb-c-connector in this case. My understanding is that two SS lanes always have to go together in the type-c spec, hence the two endpoints in the graph, but if all the SS lanes are physically wired to the same PHY then we can omit the second endpoint and use data-lanes if for example DP is handled by a different phy. > > > Other designs only connect two lanes to the qmp phy and the other two > > connect to a USB hub. That's where it gets interesting because we don't > > know how to represent that. Do we make two endpoints in the > > usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS > > RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we > > make two endpoints then do we need to have two endpoints all the time > > even though in this 4 SS line design it is redundant? > > > > port@1 { > > reg = <1>; > > endpoint@0 { // Represents RX1/TX1 > > reg = <0>; > > remote-endpoint = <&qmp_phy_lanes01>; > > }; > > endpoint@1 { // Represents RX2/TX2 > > reg = <1>; > > remote-endpoint = <&qmp_phy_lanes23>; > > }; > > }; > > > > So on the other side of that PHY we would have a multi-port USB > controller, or two USB controllers? I'm thinking of a single USB+DP PHY. > Either way, this seems like a proper > representation of the two different ports, but not something we can do > with the QMP. This example I gave is for the usb-c-connector, hence the remote-endpoint pointing to the USB+DP PHY "bundled lanes" endpoints for 0+1 and 2+3. Sorry if that wasn't clear. > > The QMP phy has certain ability to swap the signals around, so it's > conceivable that a data-lanes property in the outgoing port definition > could be used to reorder the SS lanes... > > But it would be unrelated to the USB vs DP selection in my view. > > All we want here is a connection between the usb-c-connector and the QMP > phy, such that the usb-c-connector's Type-C controller can inform the > QMP what has been negotiated. Ok. By Type-C controller you mean the typec manager? Is that all Linux for you? > > > > > > > port@2 { > > > reg = <2>; > > > endpoint { > > > remote-endpoint = <&sbu_mux>; > > > }; > > > }; > > > }; > > > }; > > > > > > This allows the USB Type-C controller to: > > > 1) Perform USB role switching in the dwc3 on port@0 > > > 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on > > > port@1:0, implement a drm_bridge for signalling HPD back to the DP > > > controller on port@1:1 > > > > We may need to have a port connection from the DP controller to the QMP > > phy. But given that the DP controller already has a 'phys' phandle to > > the QMP phy I think the DP controller driver could try to link to a drm > > bridge created in the phy driver that mainly handles the HPD signaling > > and any lane muxing/routing that needs to happen when DP pin > > configuration is present. > > > > The QMP has no knowledge of HPD signalling in Type-C, it's strictly a > virtual thing living in the Type-C controller. The Type-C controller > will request mux changes from the QMP and HPD signal changes as two > completely independent events. > > Implementing a drm_bridge in the implementation backing the > usb-c-connector mimics e.g. dp-connector (implemented in > drivers/gpu/drm/bridge/display-connector.c) nicely. > > Implementing the drm_bridge in the QMP phy means that we just add state > tracking for something that it doesn't know, hence we need another > mechanism to the Type-C controller to inform the phy that the HPD signal > has changed. Ok, so the idea is to make a drm bridge in the device registering the usb-c-connector? Doesn't the qmp_phy register the usb-c-connector for you? I'm not really following along on this part. On your design I believe the QMP phy is a mode-switch and an orientation-switch. The orientation-switch is implemented as some bit in the QMP registers to flip the SS lanes and the mode-switch is implemented somehow that I don't really understand. Probably the QMP can shut off USB for 4 lanes DP over the SS lanes? I recall some bit for the different modes is in the QMP registers. Or is the idea to make the USB (dwc3) and DP (msm_dp) controllers call phy framework APIs to change the qmp mode (USB, DP, or USB+DP)? > > > This is analog to the case you have today, where the QMP has no > knowledge of the GPIO pin that carries the HPD state in your design. Indeed, in my design the QMP configuration is "fixed" and two lanes are dedicated for DP while another two lanes are dedicated to USB. The USB lanes go to a USB hub and the hub ports are connected to two different usb-c-connectors. The DP lanes go to another mux (similar to the SBU mux logically) and the two lanes are muxed to one of the two usb-c-connectors depending on what the typec manager decides. The HPD signal bypasses QMP and goes directly to the DP controller (msm_dp) via a GPIO. The HPD signal could just as easily be virtual like in your design, but we use a GPIO for now. For the QMP driver to figure this out it will need to be able to parse the graph properties or we'll need more properties to describe the configuration. I was hoping we could describe this solely through the graph binding. We can probably do it by having reg numbered ports/endpoints in the QMP's ports binding to represent the USB or DP functionality (e.g. reg 0 is USB and reg 1 is DP) and then use 'data-lanes' to represent the number of lanes being used for that functionality (and also if they're remapped). Someone needs to write out all possible combinations and make sure the QMP binding can handle them all. If the ports binding isn't present then the driver should default to existing behavior (2 lanes DP, 2 lanes USB, normal orientation, no lane remapping). When they do this they should also consider static configurations that differ from the default, where the QMP doesn't flip the lines and/or change modes. That would allow hardware engineers to reroute lanes if that makes signal integrity better. I expect that having the device registering the usb-c-connector make the drm bridge would work on ChromeOS. We would have the cros-ec-typec driver register the drm bridge(s) and notify HPD to the DP controller(s) through the drm bridge instead of using the GPIO path. We'd have to figure out how to express the connection to the dp controller in DT so when it searches for the next bridge it can find the one made in cros-ec-typec. > [...] > > > > So should we explicitly have two endpoints in the usb-c-connector for > > the two pairs all the time, or should we represent that via data-lanes > > and only split up the connector's endpoint if we need to connect the > > usb-c-connector to two different endpoints? > > I think the endpoint of port@1 should represent the set of signals > connected to the other side, in our case 1:1 with the QMP. I like the > idea of adding data-lanes to the QMP side in order to describe any > swapping of the pads, but I see that as a separate thing. > > If you have a design where your usb-c-connector is wired to two > different PHYs and you have a Type-C controller that only negotiates the > 2+2 mode, then I think it makes sense to represent that as two endpoint > of port@1 - but the QMP side would only reference one of these > endpoints. > Agreed. I think that means at most two endpoints are possible in port@1 in the usb-c-connector binding. We would only use the second endpoint if we had two different PHYs that required it, otherwise only a single endpoint.
On Fri 19 Aug 17:18 CDT 2022, Prashant Malani wrote: > On Fri, Aug 19, 2022 at 2:39 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Fri 19 Aug 15:49 CDT 2022, Stephen Boyd wrote: > > > > > > I like 2 endpoints to represent the usb-c-connector, but that doesn't seem > > > > to be compatible (without introducing `data-lanes`, at least) with all > > > > the various > > > > combinations on the remote side, if that remote side is a DRM bridge with DP > > > > output capability (like it6505 or anx7625). > > > > That type of DRM bridge supports 1, 2 or 4 lane DP connections. > > > > > > Why can't the remote side that's a pure DP bridge (it6505) bundle > > > however many lanes it wants into one endpoint? If it's a pure DP bridge > > > we should design the bridge binding to have up to 4 endpoints, but > > > sometimes 2 or 1 and then overlay data-lanes onto that binding so that > > > we can tell the driver how to remap the lanes if it can. If the hardware > > > can't support remapping lanes then data-lanes shouldn't be in the > > > binding. > > 2 endpoints sounds fine to me. The overloading of the bridge-side endpoint > to mean different things depending on what it's connected to seemed odd to > me, but if that is acceptable for the bridge binding, then great. > > > The existing implementation provides the interfaces usb_role_switch, > > usb_typec_mux and usb_typec_switch. These works based on the concept > > that the USB Type-C controller will request the endpoints connected to > > the usb-c-connector about changes such as "switch to host mode", "switch > > to 2+2 USB/DP combo" and "switch orientation to reverse". We use this > > same operations to inform any endpoint at any port about these events > > and they all react accordingly. > > Right, but that implementation/assumption doesn't work so well when you > have 2 Type-C ports which might route to the same bridge (2 lane from each). > The other 2 lanes from the other endpoints can go to (say) a USB HUB. > Are you saying that two of your SS-lanes in connector A are connected to directly to the QMP PHY at the same time as two SS-lanes from connector B are connected to the same two pads on the QMP PHY - without any mux etc inbetween? I.e. that there are a set of pins in connector A which is directly connected to a set of pins in connector B? I was under the impression that in your hardware there's some component muxing the single DP output to one of the connectors. If so there should be no graph-link directly connecting the two usb-c-connectors and the one QMP PHY. Is this not the case? > > > > Perhaps I'm misunderstanding your suggestion, but if you start > > representing each individual lane in the SuperSpeed interface I believe > > you would have to just abandon this interface and replace it with > > something like "give me USB on port@1/endpoint@0 and port@1/endpoint@1 > > and give me DP on port@1/endpoint@2 and port@1/endpoint@3". > > I don't think that is necessary. The switch driver can register the switches ( > and it can find out which end-points map to the same usb-c-connector). > > From the port driver, the port driver just needs to tell each switch > registered for it's port that "I want > DP Pin assignment C/ DP Pin assignment D / Plain USB3.x" and the > switch driver(s) can figure out what to output on its pins (since > the Type-C binding will specify ep0 = A2-A3 (TX1), ep1 = B10-B11 , etc) > > orientation-switch can tell the switch if the signals need to be swapped around. > In a typical Qualcomm design the QMP PHY is directly connected to the usb-c-connector and as such it is the component that implements usb_typec_mux and usb_typec_switch. Regards, Bjorn > The above notwithstanding, it sounds like the 2-ep approach has more support > than 4 ep-approach, so this specific example is moot.
On Fri 19 Aug 17:55 CDT 2022, Prashant Malani wrote: > On Fri, Aug 19, 2022 at 3:01 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > > > You can't physically connect 1, 2 or 4 lanes of DP from a DP chip to > > your usb-c-connector at the same time as you physically connect 0, 2 or > > 4 lanes of USB from a USB PHY. > > I apologize in case I'm misunderstanding, but why is that not possible? > anx7625 allows that configuration (2 lane DP + 2 lane USB going to > a single USB-C-connector) > Right, but you can not connect 4 lanes DP from one chip at the same time that you connect 4 lanes USB from another chip. > Since the discussion is to support various conceivable hardware configurations > That same anx7625 can support 1 1-lane DP (or 2 1-lane DPs), and 1 > 2-lane Type-C output. > The cross-point switch allows for that level of configuration. > We're talking about the static configuration here, where you describe which component are connected together. We can not dynamically switch the Devicetree representation around to match what the Type-C controller negotiates. > > > So, how about 4 endpoints (1 for each SS lane) in the usb-c-connector port@1? > > > That should support every conceivable configuration and bridge/PHY hardware. > > > and also allows a way to specify any lane remapping (similar to what > > > "data lanes" does) > > > if that is required. > > > > Wouldn't that prevent you from handling orientation switching, given > > that the graph is static? > > It depends. If the end-points from the usb-c-connector > go to the same switch, then it should allow orientation switching > (anx7625 allows > this). The port driver would just tell the orientation switch(es) attached to > it that we are in NORMAL or REVERSE orientation. > But why do you need to express the relationship between these 2 components with > 1 link in the graph? > The graph is static, since the hardware line routing between components > doesn't change (e.g SSTX1 from the Type-C port is always routed to Pin > X1,X2 on the switch hardware), but that is what the switch is for. > Note that in this case, the expectation is that > the switch driver only registers 1 switch (it can figure out that all > 4 endpoints > go to the same Type-C port). > Why do we need to express this with 4 endpoints and then implement code to discover that the 4 endpoints points to the same remote? Why not just describe the logical relationship between the two components in one endpoint reference? > The limitation to orientation switching would depend on how the > hardware is routed. Regards, Bjorn
On Fri 19 Aug 22:51 CDT 2022, Stephen Boyd wrote: > I realize I've hijacked this thread to discuss the QMP binding :-/ > > Quoting Bjorn Andersson (2022-08-19 14:26:32) > > On Thu 18 Aug 20:09 CDT 2022, Stephen Boyd wrote: > > > > > Quoting Bjorn Andersson (2022-08-17 16:00:05) > > > > > > > > This is the setup that we're dealing with: > > > > > > > > +------------- - - > > > > USB connector | SoC > > > > +-+ | > > > > | | | +-----+ > > > > |*|<------- HS -----|-->| HS | > > > > |*|<------- HS -----|-->| phy |<-+ +--------+ > > > > | | | +-----+ \->| | > > > > | | | | dwc3 | > > > > | | | +-----+ /->| | > > > > |*|<------- SS -----|-->| |<-+ +--------+ > > > > |*|<------- SS -----|-->| QMP | > > > > |*|<------- SS -----|-->| phy | > > > > |*|<------- SS -----|-->| |<-\ +------------+ > > > > | | | +-----+ \->| | > > > > | | | | DP | > > > > | | +-----+ | | controller | > > > > |*|<-->| SBU |<-----|--------------->| | > > > > |*|<-->| mux |<-----|--------------->| | > > > > | | +----+ | +------------+ > > > > +-+ | > > > > +------------- - - > > > > > [...] > > > I'd expect the QMP phy to physically be the only thing connected on the > > > board. That matches the block diagram above. Inside the SoC the SS lines > > > will be muxed through the QMP phy to the DP controller or the USB > > > controller. Therefore, in the binding I'd expect there to be a single > > > port@1 for the connector: > > > > > > port@1 { > > > reg = <1>; > > > endpoint@0 { > > > reg = <0>; > > > remote-endpoint = <&qmp_phy>; > > > }; > > > }; > > > > > > > That is correct, the 4 SS pairs in the USB connector are connected to > > the QMP PHY pads. > > > > > > The second endpoint in port@1 comes from my RFC where I suggested adding > > a 4th port to the usb-c-connector for connecting the usb-c-connector to > > the DP controller for passing the virtual HPD signal. Rob suggested that > > this indication relates to the SS pins and wanted this to be part of > > port@1. But it's not actually a definition of any electrical connection. > > I suspect this is the root of the debate. Should the binding be > describing logical connections between components or actual physical > connections? And should the connector binding have endpoints for > different altmodes, e.g. DP? > > What do we do if thunderbolt support is yet another PHY that sits behind > the QMP phy? Do we need to make yet another endpoint in the > usb-c-connector to represent this connection? I hope not. > It sounds like you would be comfortable with expressing the relationship between the usb-c-connector and the QMP PHY in this design and then express the relationship between the QMP and the thunderbolt separately, forcing the QMP implementation to bridge any operations needed. I've not looked enough at such design to argue for or against that, but I can definitely see the merit of having the usb-c-connector graph be linked to the component in the SoC that owns the pads - and not everything behind that. > The QMP phy is doing type-c "muxing" of different PHYs (USB and DP) onto > the SS lanes. Other altmodes that want to use the SS lanes would > similarly need to be routed through the QMP phy and muxed onto the lanes > when the altmode is used, e.g. thunderbolt. While it is certainly > convenient to have a "DP" endpoint in the usb-c-connector, I feel that > it is wrong, primarily because the DP phy has the QMP phy in between it > and the usb-c-connector, but also because DP is an altmode/virtual > construct built on top of the 4 lanes in the typec pinout. > > We should look at the binding from the perspective of the connector and > figure out how the pinout can be mapped to the binding. That would allow > board designers to ignore the internal SoC details, and stay focused on > what is in the schematic, which is the qmp phy and the usb-c-connector > in this case. My understanding is that two SS lanes always have to go > together in the type-c spec, hence the two endpoints in the graph, but > if all the SS lanes are physically wired to the same PHY then we can > omit the second endpoint and use data-lanes if for example DP is handled > by a different phy. > There is nothing in the schematics representing how the HPD signal comes from the Type-C controller to the DP controller - but it is a M:N relationship, so we must represent it in some way. I suggested a new port for describing this virtual connection, Rob asked for it to be a separate endpoint in port@1. I'm fine with either path. But as Benson described to us, we do muxing of the signals in one operation and we do HPD signalling in a completely separate operation - from the Type-C controller's PoV. As such the QMP has nothing to do with the HPD signal. > > > > > Other designs only connect two lanes to the qmp phy and the other two > > > connect to a USB hub. That's where it gets interesting because we don't > > > know how to represent that. Do we make two endpoints in the > > > usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS > > > RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we > > > make two endpoints then do we need to have two endpoints all the time > > > even though in this 4 SS line design it is redundant? > > > > > > port@1 { > > > reg = <1>; > > > endpoint@0 { // Represents RX1/TX1 > > > reg = <0>; > > > remote-endpoint = <&qmp_phy_lanes01>; > > > }; > > > endpoint@1 { // Represents RX2/TX2 > > > reg = <1>; > > > remote-endpoint = <&qmp_phy_lanes23>; > > > }; > > > }; > > > > > > > So on the other side of that PHY we would have a multi-port USB > > controller, or two USB controllers? > > I'm thinking of a single USB+DP PHY. > > > Either way, this seems like a proper > > representation of the two different ports, but not something we can do > > with the QMP. > > This example I gave is for the usb-c-connector, hence the > remote-endpoint pointing to the USB+DP PHY "bundled lanes" endpoints for > 0+1 and 2+3. Sorry if that wasn't clear. > > > > > The QMP phy has certain ability to swap the signals around, so it's > > conceivable that a data-lanes property in the outgoing port definition > > could be used to reorder the SS lanes... > > > > But it would be unrelated to the USB vs DP selection in my view. > > > > All we want here is a connection between the usb-c-connector and the QMP > > phy, such that the usb-c-connector's Type-C controller can inform the > > QMP what has been negotiated. > > Ok. By Type-C controller you mean the typec manager? Is that all Linux > for you? > I mean the entity that tells the remote-endpoints of the usb-c-connector about the outcome of USB PD negotiations. This might be implemented fully in Linux or partially in firmware. But this something will be the thing that ultimately calls typec_switch_set() et al. Can you please elaborate on the operations you see that the typec manager would perform on the remote-endpoint of endpoint@0 and endpoint@1? > > > > > > > > > > port@2 { > > > > reg = <2>; > > > > endpoint { > > > > remote-endpoint = <&sbu_mux>; > > > > }; > > > > }; > > > > }; > > > > }; > > > > > > > > This allows the USB Type-C controller to: > > > > 1) Perform USB role switching in the dwc3 on port@0 > > > > 2) Orientation and muxing of the SuperSpeed lines in the QMP phy on > > > > port@1:0, implement a drm_bridge for signalling HPD back to the DP > > > > controller on port@1:1 > > > > > > We may need to have a port connection from the DP controller to the QMP > > > phy. But given that the DP controller already has a 'phys' phandle to > > > the QMP phy I think the DP controller driver could try to link to a drm > > > bridge created in the phy driver that mainly handles the HPD signaling > > > and any lane muxing/routing that needs to happen when DP pin > > > configuration is present. > > > > > > > The QMP has no knowledge of HPD signalling in Type-C, it's strictly a > > virtual thing living in the Type-C controller. The Type-C controller > > will request mux changes from the QMP and HPD signal changes as two > > completely independent events. > > > > Implementing a drm_bridge in the implementation backing the > > usb-c-connector mimics e.g. dp-connector (implemented in > > drivers/gpu/drm/bridge/display-connector.c) nicely. > > > > Implementing the drm_bridge in the QMP phy means that we just add state > > tracking for something that it doesn't know, hence we need another > > mechanism to the Type-C controller to inform the phy that the HPD signal > > has changed. > > Ok, so the idea is to make a drm bridge in the device registering the > usb-c-connector? Doesn't the qmp_phy register the usb-c-connector for > you? I'm not really following along on this part. > No, it's not a part of the QMP. We want to use the graph from the usb-c-connector to signal the provider of HS, SS and SBU-signals about changes related to the connector. As such we associate the usb-c-connector with the Type-C manager/controller. Like described here, for a single usb-c-connector: https://lore.kernel.org/all/20220818031512.319310-2-bjorn.andersson@linaro.org/ In this case, the pmic_glink firmware will send Linux messages which can be directly translated to a set of typec_mux_set(), typec_switch_set() and drm_bridge_hpd_notify() calls - with the graph defining which remote components should receive these events. > On your design I believe the QMP phy is a mode-switch and an > orientation-switch. The orientation-switch is implemented as some bit in > the QMP registers to flip the SS lanes and the mode-switch is > implemented somehow that I don't really understand. Probably the QMP can > shut off USB for 4 lanes DP over the SS lanes? I recall some bit for the > different modes is in the QMP registers. > Correct, typec_mux_set() passes the negotiated pin assignment, which the QMP PHY can react to and reconfigure the muxing. Similarly the QMP can implement typec_switch_set() to flip the bit to do the orientation switching. > Or is the idea to make the USB (dwc3) and DP (msm_dp) controllers call > phy framework APIs to change the qmp mode (USB, DP, or USB+DP)? > No. > > > > > > This is analog to the case you have today, where the QMP has no > > knowledge of the GPIO pin that carries the HPD state in your design. > > Indeed, in my design the QMP configuration is "fixed" and two lanes are > dedicated for DP while another two lanes are dedicated to USB. The USB > lanes go to a USB hub and the hub ports are connected to two different > usb-c-connectors. The DP lanes go to another mux (similar to the SBU mux > logically) and the two lanes are muxed to one of the two > usb-c-connectors depending on what the typec manager decides. The HPD > signal bypasses QMP and goes directly to the DP controller (msm_dp) via > a GPIO. The HPD signal could just as easily be virtual like in your > design, but we use a GPIO for now. > > For the QMP driver to figure this out it will need to be able to parse > the graph properties or we'll need more properties to describe the > configuration. I was hoping we could describe this solely through the > graph binding. We can probably do it by having reg numbered > ports/endpoints in the QMP's ports binding to represent the USB or DP > functionality (e.g. reg 0 is USB and reg 1 is DP) and then use > 'data-lanes' to represent the number of lanes being used for that > functionality (and also if they're remapped). Someone needs to write out > all possible combinations and make sure the QMP binding can handle them > all. If the ports binding isn't present then the driver should default > to existing behavior (2 lanes DP, 2 lanes USB, normal orientation, no > lane remapping). When they do this they should also consider static > configurations that differ from the default, where the QMP doesn't flip > the lines and/or change modes. That would allow hardware engineers to > reroute lanes if that makes signal integrity better. > > I expect that having the device registering the usb-c-connector make the > drm bridge would work on ChromeOS. We would have the cros-ec-typec > driver register the drm bridge(s) and notify HPD to the DP controller(s) > through the drm bridge instead of using the GPIO path. We'd have to > figure out how to express the connection to the dp controller in DT so > when it searches for the next bridge it can find the one made in > cros-ec-typec. > I don't think it makes sense in your design to register a drm_bridge per usb-c-connector, because then you need to connect the one DP controller to both the drm_bridges and you need to spill the mux-logic from the EC into the DP controller as well. If you put the muxing logic in entity that does the muxing and implement a signle drm_bridge there you will mimic the current design nicely, where there is a single connection (GPIO) between the EC and the DP controller for propagating the HPD signal. You could choose to model the two usb-c-connectors there somehow as well, perhaps just as static entities directly in /, but that would then be a question of how to describing the link between the EC and the two connectors. Similarly, describing the relationship between the QMP PHY and the mux makes sense to me (or just not describe it at all, if you're not going to invoke any of the muxing/switching operations on the PHY) > > > [...] > > > > > > So should we explicitly have two endpoints in the usb-c-connector for > > > the two pairs all the time, or should we represent that via data-lanes > > > and only split up the connector's endpoint if we need to connect the > > > usb-c-connector to two different endpoints? > > > > I think the endpoint of port@1 should represent the set of signals > > connected to the other side, in our case 1:1 with the QMP. I like the > > idea of adding data-lanes to the QMP side in order to describe any > > swapping of the pads, but I see that as a separate thing. > > > > If you have a design where your usb-c-connector is wired to two > > different PHYs and you have a Type-C controller that only negotiates the > > 2+2 mode, then I think it makes sense to represent that as two endpoint > > of port@1 - but the QMP side would only reference one of these > > endpoints. > > > > Agreed. I think that means at most two endpoints are possible in port@1 > in the usb-c-connector binding. We would only use the second endpoint if > we had two different PHYs that required it, otherwise only a single > endpoint. Sure. But I do need to have a link between a DP controller and something representing each USB-C port. By registering a drm_bridge associated with the usb-c-connector the DP controller implementation and binding will look identical between the dp-connector case and the usb-c-connector case. But the two options I see is to either add it in port@1 as a separate logical endpoint or to add a new logical port. The alternative to this would be to have a separate of graph outside the multiple connectors, where each port@N implements the drm_bridge for connector@N - but I feel we're just making things overly complicated, just to avoid adding a logical endpoint/port in the usb-c-connector. Regards, Bjorn.
On Fri, Aug 19, 2022 at 9:04 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Fri 19 Aug 17:18 CDT 2022, Prashant Malani wrote: > > > On Fri, Aug 19, 2022 at 2:39 PM Bjorn Andersson > > <bjorn.andersson@linaro.org> wrote: > > Are you saying that two of your SS-lanes in connector A are connected to > directly to the QMP PHY at the same time as two SS-lanes from connector > B are connected to the same two pads on the QMP PHY - without any > mux etc inbetween? > > I.e. that there are a set of pins in connector A which is directly > connected to a set of pins in connector B? > > > I was under the impression that in your hardware there's some component > muxing the single DP output to one of the connectors. If so there should > be no graph-link directly connecting the two usb-c-connectors and the > one QMP PHY. > > Is this not the case? I can't speak to the QMP PHY specifically (since I'm not using that hardware), but your impression is right. There is a component (anx7625) muxing the single DP output to the 2 usb-c-connectors (specifically, 2 lanes each from the 2 usb-c-connectors). The other 2 lanes (from the 2 USB-C-connectors) go to a USB3 hub; hence the need for 2 endpoints for each usb-c-connector). So, the anx7625 should register the mode switches and it needs the graph connections from 2 usb-c-connectors BR, -Prashant
On Fri, Aug 19, 2022 at 9:09 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > > We're talking about the static configuration here, where you describe > which component are connected together. We can not dynamically switch > the Devicetree representation around to match what the Type-C controller > negotiates. Yes, but we don't need to switch the device tree representation at all. The pin routing/connections from the connector (not the cable or the partner), to the muxing hardware (QMP phy or anx7625) remains fixed always The port driver tells what orientation the peripheral is connected with, and the muxing/orientation hardware routes the signals according to that. > > But why do you need to express the relationship between these 2 > components with > 1 link in the graph? > > > The graph is static, since the hardware line routing between components > > doesn't change (e.g SSTX1 from the Type-C port is always routed to Pin > > X1,X2 on the switch hardware), but that is what the switch is for. > > Note that in this case, the expectation is that > > the switch driver only registers 1 switch (it can figure out that all > > 4 endpoints > > go to the same Type-C port). > > > > Why do we need to express this with 4 endpoints and then implement code > to discover that the 4 endpoints points to the same remote? Why not just > describe the logical relationship between the two components in one > endpoint reference? The issue I see is with the "supplier" side of that graph relationship (i.e the DRM bridge side). Since the bridge can be directly connected to a DP panel, the endpoints can (technically) represent a single DP lane. So, using 4 end-points for the usb-c-connector port@1 gives us something which is compatible with the bridge side endpoints too (regardless of what the bridge is connected to on the "output" side). Reading the discussion, I agree 4 lanes is over-specifying, and 2 endpoints is probably enough (especially if we can use data-lanes on the bridge side to define the number of lanes if needed for DP panel connections). BR, -Prashant
Quoting Bjorn Andersson (2022-08-19 21:56:17) > On Fri 19 Aug 22:51 CDT 2022, Stephen Boyd wrote: > > > I realize I've hijacked this thread to discuss the QMP binding :-/ > > > > The QMP phy is doing type-c "muxing" of different PHYs (USB and DP) onto > > the SS lanes. Other altmodes that want to use the SS lanes would > > similarly need to be routed through the QMP phy and muxed onto the lanes > > when the altmode is used, e.g. thunderbolt. While it is certainly > > convenient to have a "DP" endpoint in the usb-c-connector, I feel that > > it is wrong, primarily because the DP phy has the QMP phy in between it > > and the usb-c-connector, but also because DP is an altmode/virtual > > construct built on top of the 4 lanes in the typec pinout. > > > > We should look at the binding from the perspective of the connector and > > figure out how the pinout can be mapped to the binding. That would allow > > board designers to ignore the internal SoC details, and stay focused on > > what is in the schematic, which is the qmp phy and the usb-c-connector > > in this case. My understanding is that two SS lanes always have to go > > together in the type-c spec, hence the two endpoints in the graph, but > > if all the SS lanes are physically wired to the same PHY then we can > > omit the second endpoint and use data-lanes if for example DP is handled > > by a different phy. > > > > There is nothing in the schematics representing how the HPD signal comes > from the Type-C controller to the DP controller - but it is a M:N > relationship, so we must represent it in some way. > > I suggested a new port for describing this virtual connection, Rob asked > for it to be a separate endpoint in port@1. I'm fine with either path. I don't think we should be making a virtual connection. I'm not sure Rob wants it to be a virtual connection either. > > But as Benson described to us, we do muxing of the signals in one > operation and we do HPD signalling in a completely separate operation - > from the Type-C controller's PoV. As such the QMP has nothing to do with > the HPD signal. > > > > > > > > Other designs only connect two lanes to the qmp phy and the other two > > > > connect to a USB hub. That's where it gets interesting because we don't > > > > know how to represent that. Do we make two endpoints in the > > > > usb-c-connector port@1 and split the SS lines into SS RX1/TX1 and SS > > > > RX2/TX2 pairs? Or do we use data-lanes to represent the SS lines? If we > > > > make two endpoints then do we need to have two endpoints all the time > > > > even though in this 4 SS line design it is redundant? > > > > > > > > port@1 { > > > > reg = <1>; > > > > endpoint@0 { // Represents RX1/TX1 > > > > reg = <0>; > > > > remote-endpoint = <&qmp_phy_lanes01>; > > > > }; > > > > endpoint@1 { // Represents RX2/TX2 > > > > reg = <1>; > > > > remote-endpoint = <&qmp_phy_lanes23>; > > > > }; > > > > }; > > > > > > > > > > So on the other side of that PHY we would have a multi-port USB > > > controller, or two USB controllers? > > > > I'm thinking of a single USB+DP PHY. > > > > > Either way, this seems like a proper > > > representation of the two different ports, but not something we can do > > > with the QMP. > > > > This example I gave is for the usb-c-connector, hence the > > remote-endpoint pointing to the USB+DP PHY "bundled lanes" endpoints for > > 0+1 and 2+3. Sorry if that wasn't clear. > > > > > > > > The QMP phy has certain ability to swap the signals around, so it's > > > conceivable that a data-lanes property in the outgoing port definition > > > could be used to reorder the SS lanes... > > > > > > But it would be unrelated to the USB vs DP selection in my view. > > > > > > All we want here is a connection between the usb-c-connector and the QMP > > > phy, such that the usb-c-connector's Type-C controller can inform the > > > QMP what has been negotiated. > > > > Ok. By Type-C controller you mean the typec manager? Is that all Linux > > for you? > > > > I mean the entity that tells the remote-endpoints of the usb-c-connector > about the outcome of USB PD negotiations. This might be implemented > fully in Linux or partially in firmware. > > But this something will be the thing that ultimately calls > typec_switch_set() et al. > > > Can you please elaborate on the operations you see that the typec > manager would perform on the remote-endpoint of endpoint@0 and > endpoint@1? Sorry, which endpoints are we talking about here? > > > > Ok, so the idea is to make a drm bridge in the device registering the > > usb-c-connector? Doesn't the qmp_phy register the usb-c-connector for > > you? I'm not really following along on this part. > > > > No, it's not a part of the QMP. > > We want to use the graph from the usb-c-connector to signal the provider > of HS, SS and SBU-signals about changes related to the connector. As > such we associate the usb-c-connector with the Type-C > manager/controller. > > Like described here, for a single usb-c-connector: > https://lore.kernel.org/all/20220818031512.319310-2-bjorn.andersson@linaro.org/ > > In this case, the pmic_glink firmware will send Linux messages which can > be directly translated to a set of typec_mux_set(), typec_switch_set() > and drm_bridge_hpd_notify() calls - with the graph defining which remote > components should receive these events. Ok, got it. The usb-c-connector is registered by the qcom,pmic-glink driver. That is similar to the google,cros-ec-typec driver that we have on chromeos. > > > > > > > > > > This is analog to the case you have today, where the QMP has no > > > knowledge of the GPIO pin that carries the HPD state in your design. Yes. > > I don't think it makes sense in your design to register a drm_bridge per > usb-c-connector, because then you need to connect the one DP controller > to both the drm_bridges and you need to spill the mux-logic from the EC > into the DP controller as well. > > If you put the muxing logic in entity that does the muxing and implement > a signle drm_bridge there you will mimic the current design nicely, > where there is a single connection (GPIO) between the EC and the DP > controller for propagating the HPD signal. Yes, agreed. I believe implementing the drm_bridge in an EC driver is the approach we will take. We will have to add a port binding to the EC binding that accepts displayport as an input. Essentially the driver will act as a DP connector that accepts the 2 DP lanes coming from QMP and then muxes them onto one or the other usb-c-connector. The same driver will need to support multiple DP input ports and USB inputs. > > You could choose to model the two usb-c-connectors there somehow as > well, perhaps just as static entities directly in /, but that would then > be a question of how to describing the link between the EC and the two > connectors. > > Similarly, describing the relationship between the QMP PHY and the mux > makes sense to me (or just not describe it at all, if you're not going > to invoke any of the muxing/switching operations on the PHY) > > > > > > [...] > > > > > > > > So should we explicitly have two endpoints in the usb-c-connector for > > > > the two pairs all the time, or should we represent that via data-lanes > > > > and only split up the connector's endpoint if we need to connect the > > > > usb-c-connector to two different endpoints? > > > > > > I think the endpoint of port@1 should represent the set of signals > > > connected to the other side, in our case 1:1 with the QMP. I like the > > > idea of adding data-lanes to the QMP side in order to describe any > > > swapping of the pads, but I see that as a separate thing. > > > > > > If you have a design where your usb-c-connector is wired to two > > > different PHYs and you have a Type-C controller that only negotiates the > > > 2+2 mode, then I think it makes sense to represent that as two endpoint > > > of port@1 - but the QMP side would only reference one of these > > > endpoints. > > > > > > > Agreed. I think that means at most two endpoints are possible in port@1 > > in the usb-c-connector binding. We would only use the second endpoint if > > we had two different PHYs that required it, otherwise only a single > > endpoint. > > Sure. > > But I do need to have a link between a DP controller and something > representing each USB-C port. Do you? I think you need a link between the DP driver and the drm_bridge driver that sends virtual hpd signals based on typec messages. That may be the usb-c port driver, or it may be the driver implementing the mode-switch. It doesn't need to be the usb-c-connector. > > By registering a drm_bridge associated with the usb-c-connector the DP > controller implementation and binding will look identical between the > dp-connector case and the usb-c-connector case. > > But the two options I see is to either add it in port@1 as a separate > logical endpoint or to add a new logical port. > > The alternative to this would be to have a separate of graph outside the > multiple connectors, where each port@N implements the drm_bridge for > connector@N - but I feel we're just making things overly complicated, > just to avoid adding a logical endpoint/port in the usb-c-connector. > In your case you have a 1:1 relationship between the QMP and the usb-c-connector, so anything extra in the graph relationship between QMP and the usb-c-connector looks like extra overly complicated binding. In the chromeos trogdor case, we have a physical switch on the DP lanes to steer DP from QMP to one of the two usb-c-connectors that are controlled by the EC. Making a direct connection between DP and the usb-c-connectors is impossible because DP has 1 graph output that can't be connected to two usb-c-connectors at the same time. The creation of a drm_bridge for a usb-c-connector doesn't work. Connecting the QMP to the EC is a solution, or connecting the DP controller to the EC is another solution, but the QMP path is preferred. Here's why: (brace yourself for the wall of text!) 1. QMP needs to know lane routing We need to know which lanes coming out of QMP are for DP. Are they the first two lanes (TX1/RX1) or the second two lanes (TX2/RX2), or all 4 lanes? The orientation bit inside QMP needs to be configured properly too. In the 1:1 case you have this is not important until the DP pinconf is assigned; connecting the DP controller directly to the usb-c-connector in DT works because typec framework controls the orientation switch inside QMP that's connected to the other port in usb-c-connector. In the 1:N case the mapping is static, and we need a way to express which lanes from QMP are for DP and which lanes from QMP are for USB. 2. DP lane remapping is controlled in the DP phy before orientation is controlled in the QMP phy DP lanes coming out of the DP phy can be remapped however desired via a register in the DP phy. The orientation control works on the TX1/RX1 and TX2/RX2 pairs via QMP registers, so that two lanes DP and two lanes USB can't put the two USB lanes on TX1/TX2 and the two DP lanes on RX1/RX2 when the orientation is flipped. If QMP has all four lanes wired to a DP connector, i.e. USB is disabled, then we would use the data-lanes property inside the DP phy's graph endpoint to figure out how to remap the four DP lanes going to the QMP and out of the SoC to the connector. The QMP orientation bit would need to be set to normal, so that engineers can ignore QMP and how it flips the lanes it took from the DP phy. 3. QMP should have incoming and outgoing graph ports The data-lanes property should only be inside graph endpoints, hence the requirement to use a graph binding from the DP phy to the QMP node. Sometimes the lanes from QMP are directly connected to a usb-c-connector, hence the requirement to have a graph binding in QMP that can connect to a usb-c-connector. Other times the QMP node would be directly connected to a dp-connector. In that case we would also use a graph binding to link the connector to QMP. 4. Graph endpoints without a remote-endpoint property are bad style In your design this wouldn't be the case, but in chromeos' case we would connect the DP controller to the EC or one of the two usb-c-connectors and then if we want to configure QMP lanes we would have to implement the graph in QMP but omit the remote-endpoint property. That's because we have 2 usb-c-connectors for 1 QMP. Having two endpoints in usb-c-connector doesn't help us, because we can only connect QMP to one of the two usb-c-connectors. We're unable to express that both usb-c-connectors support DP from this one DP controller. TL;DR: We have a verbose binding indeed, but it is required because trogdor takes 2 DP lanes and muxes them to different usb-c-connectors. To do that, we have to send the DP lanes to the thing that controls the muxing (the EC). It's also complicated by the fact that the DP and USB phy are split from the controllers, and put behind QMP (basically a typec phy). I propose we make the graph binding for QMP have logically numbered endpoints: 0 for USB+DP (most common), 1 for USB, and 2 for DP and use data-lanes to physically map the pins while connecting the DP and USB phys to QMP within the graph and using data-lanes again. Here are some examples: - 4 lanes DP only, normal orientation of QMP, remapped DP lanes qmp { ports { qmp_dp_in: port@1 { reg = <1>; remote-endpoint = <&dp_phy_out>; }; port@2 { reg = <2>; qmp_dp_out: endpoint@2 { reg = <2>; // data-lanes indicates how many lanes are used for DP // and if the lanes are flipped // // data-lanes = <0 1 2 3> == normal orientation (default) // data-lanes = <2 3 0 1> == flipped orientation // data-lanes = <0 1> == 2 lane DP, normal orientation // data-lanes = <2 3> == 2 lane DP, flipped orientation // data-lanes = <0/1/2/3> == 1 lane DP remote-endpoint = <&dp_connector>; }; }; }; dp-phy { ports { dp_phy_out: port { remote-endpoint = <&qmp_dp_in>; data-lanes = <3 1 2 0>; // remap lanes }; }; }; }; dp-connector { compatible = "dp-connector"; ports { dp_connector: port@0 { remote-endpoint = <&qmp_dp_out>; }; }; }; - 2 lanes DP, 2 lanes USB fixed, flipped orientation of QMP qmp { ports { qmp_usb_in: port@0 { reg = <0>; remote-endpoint = <&usb_phy_out>; }; qmp_dp_in: port@1 { reg = <1>; remote-endpoint = <&dp_phy_out>; }; port@2 { reg = <2>; qmp_usb_out: endpoint@1 { reg = <1>; data-lanes = <2 3>; // SSTRX2 (flipped) remote-endpoint = <&usb_hub_in>; }; qmp_dp_out: endpoint@2 { reg = <2>; data-lanes = <0 1>; // SSTRX1 (flipped) remote-endpoint = <&cros_ec_dp_in>; }; }; }; dp-phy { ports { dp_phy_out: port { remote-endpoint = <&qmp_dp_in>; data-lanes = <0 1>; // DP phy fixed at two lanes, remap possible }; }; }; usb-phy { ports { usb_phy_out: port { remote-endpoint = <&qmp_usb_in>; data-lanes = <0 1>; // remap lanes possibly? otherwise implicit }; }; }; }; usb-hub { compatible = "usb-hub"; ports { usb_hub_in: port@0 { remote-endpoint = <&qmp_usb_out>; }; }; }; ec { cros_ec_typec { ports { port@0 { // inputs reg = <1>; cros_ec_dp_in: endpoint@0 { reg = <0>; remote-endpoint = <&qmp_dp_out>; }; }; }; usb-c0 { compatible = "usb-c-connector"; // Do we care to connect this in the graph? }; usb-c1 { compatible = "usb-c-connector"; }; }; - 2/4 lanes DP, 2 lanes USB (i.e. USB+DP what you have) qmp { mode-switch; orientation-switch; ports { qmp_usb_in: port@0 { reg = <0>; remote-endpoint = <&usb_phy_out>; }; qmp_dp_in: port@1 { reg = <1>; remote-endpoint = <&dp_phy_out>; }; port@2 { reg = <2>; qmp_usb_dp_out: endpoint@0 { reg = <0>; // data-lanes indicates orientation if this // doesn't have an orientation-switch property // // data-lanes = <0 1 2 3> == normal (default) // data-lanes = <2 3 0 1> == flipped remote-endpoint = <&usb_c_connector_ss0>; }; }; }; dp-phy { ports { dp_phy_out: port { remote-endpoint = <&qmp_dp_in>; // data-lanes can only be <0 1> or <1 0> and // orientation-switch can't be present in qmp // when data-lanes is here. }; }; }; usb-phy { ports { usb_phy_out: port { remote-endpoint = <&qmp_usb_in>; }; }; }; }; glink { usb-c-connector { compatible = "usb-c-connector"; ports { port@1 { reg = <1>; endpoint@0 { reg = <0>; remote-endpoint = <&qmp_usb_dp_out>; }; }; }; }; }; In your case (this last example) you don't need to have an extra graph outside the connector in glink, because you have a direct connection between QMP and the usb-c-connector. As I understand it, you want to add another endpoint to usb-c-connector above, endpoint@1 in port@1, that connects directly to the DP controller. It doesn't scale if we add another altmode though, we'll have to add another virtual endpoint. We can also see that DP is connected if we walk the graph to the dp phy. ------ Assuming everything is good above, the primary concern I'm left with is how to find the drm_bridge from the DP controller driver. It would be convenient to make a graph connection from the DP controller to the mode-switch device. Then we could cut out the DP phy and the QMP part and avoid walking the graph from DP phy to qmp to the next endpoint that may or may not be a drm_bridge. In your case, the mode-switch is the qmp, because it is used to control 2 or 4 lanes of DP. In chromeos' case it's the EC. Either way, we're talking about a virtual link in the binding, to make things simpler for the drm_bridge linkage code devm_drm_of_get_bridge(). It would leave us with a parallel graph connection from the DP controller node and the QMP node. I'm not excited about this approach. I wonder if drm can learn to walk from the 'phys' property to the graph of the phy node and then search from there for a drm_bridge. Or if it's simpler we can make a drm_bridge in the dp phy, qmp, and in cros_ec/glink, where the dp phy and qmp would do nothing besides have a drm_bridge_funcs::attach() function that knew which port in their graph to search for the next bridge on. Then we could connect the port in the DP controller's graph representing the DP output to the DP phy graph as an input. Then the drm_bridge isn't entirely useless, just a small bit of code to do the walk on attach. The glink driver would make a drm_bridge for each usb-c-connector and associate the connector of_node with the bridge. Similarly, with a cros EC where the relationship is 1:1 we would make a drm_bridge for each usb-c-connector and omit the graph binding outside the connector in cros_ec_typec, directly connecting the graph from QMP to the connector. This is because of_drm_find_bridge() looks for the parent of the graph to find the bridge, and we would have two bridges in this case that need different nodes.
On Tue, Aug 23, 2022 at 11:54:53AM -0700, Prashant Malani wrote: > On Fri, Aug 19, 2022 at 9:09 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > > > We're talking about the static configuration here, where you describe > > which component are connected together. We can not dynamically switch > > the Devicetree representation around to match what the Type-C controller > > negotiates. > > Yes, but we don't need to switch the device tree representation at all. > The pin routing/connections from the connector (not the cable or the partner), > to the muxing hardware (QMP phy or anx7625) remains fixed always > The port driver tells what orientation the peripheral is connected with, > and the muxing/orientation hardware routes the signals according to that. > > > > > But why do you need to express the relationship between these 2 > > components with > 1 link in the graph? > > > > > The graph is static, since the hardware line routing between components > > > doesn't change (e.g SSTX1 from the Type-C port is always routed to Pin > > > X1,X2 on the switch hardware), but that is what the switch is for. > > > Note that in this case, the expectation is that > > > the switch driver only registers 1 switch (it can figure out that all > > > 4 endpoints > > > go to the same Type-C port). > > > > > > > Why do we need to express this with 4 endpoints and then implement code > > to discover that the 4 endpoints points to the same remote? Why not just > > describe the logical relationship between the two components in one > > endpoint reference? > > The issue I see is with the "supplier" side of that graph relationship > (i.e the DRM bridge side). > Since the bridge can be directly connected to a DP panel, the > endpoints can (technically) > represent a single DP lane. So, using 4 end-points for the > usb-c-connector port@1 gives > us something which is compatible with the bridge side endpoints too > (regardless of what > the bridge is connected to on the "output" side). > Reading the discussion, I agree 4 lanes is over-specifying, and 2 > endpoints is probably > enough (especially if we can use data-lanes on the bridge side > to define the number of lanes if needed for DP panel connections). > I'm sorry, but the part I don't understand is what you gain from representing each physical line in your connection with a remote-endpoint pair? What I propose is that you tie the two pieces together with a single reference. If you need to express the number of data-lanes we have several places where this is described separately, using the "data-lanes" property. With this model, if you have a 1:1 connection you have a single remote-endpoint pair, if you have a 1:N connection, then you would have N remote-endpoint pairs. Regards, Bjorn
diff --git a/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml new file mode 100644 index 000000000000..7d8aca40c7ca --- /dev/null +++ b/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/usb/gpio-sbu-mux.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: GPIO-based SBU mux + +maintainers: + - Bjorn Andersson <bjorn.andersson@linaro.org> + +description: + In USB Type-C applications the SBU lines needs to be connected, disconnected + and swapped depending on the altmode and orientation. This binding describes + a family of hardware which perform this based on GPIO controls. + +properties: + compatible: + items: + - enum: + - onnn,fsusb43l10x + - pericom,pi3usb102 + - const: gpio-sbu-mux + + enable-gpios: + description: Switch enable GPIO + + select-gpios: + description: Orientation select + + vcc-supply: + description: power supply + + mode-switch: + description: Flag the port as possible handle of altmode switching + type: boolean + + orientation-switch: + description: Flag the port as possible handler of orientation switching + type: boolean + + port: + $ref: /schemas/graph.yaml#/properties/port + description: + A port node to link the SBU mux to a TypeC controller for the purpose of + handling altmode muxing and orientation switching. + +required: + - compatible + - enable-gpios + - select-gpios + - mode-switch + - orientation-switch + - port + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + sbu-mux { + compatible = "pericom,pi3usb102", "gpio-sbu-mux"; + + enable-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>; + select-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>; + + mode-switch; + orientation-switch; + + port { + endpoint { + remote-endpoint = <&pmic_glink_dp0_sbu>; + }; + }; + }; +...
Introduce a binding for GPIO-based mux hardware used for connecting, disconnecting and switching orientation of the SBU lines in USB Type-C applications. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- .../devicetree/bindings/usb/gpio-sbu-mux.yaml | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml