On Mon, Jan 30, 2023 at 01:42:14PM -0800, Bjorn Andersson wrote: > On Mon, Jan 30, 2023 at 10:48:13AM -0600, Rob Herring wrote: > > On Wed, Jan 25, 2023 at 03:40:13PM -0800, Bjorn Andersson wrote: > > > On Tue, Jan 24, 2023 at 08:31:13PM -0600, Rob Herring wrote: > > > > On Tue, Jan 24, 2023 at 11:04 AM Bjorn Andersson > > > > <quic_bjorande@xxxxxxxxxxx> wrote: > > > > > > > > > > On Tue, Jan 24, 2023 at 10:00:12AM -0600, Rob Herring wrote: > > > > > > On Thu, Jan 19, 2023 at 11:40 AM Bjorn Andersson > > > > > > <quic_bjorande@xxxxxxxxxxx> wrote: > [..] > > > > > Are you saying that the connector should link with the mux and then the > > > > > source of the signal should be daisy chained? Or that we should just > > > > > link both of them as two separate endpoints from the connector? > > > > > > > > The former. The data path of the signal in h/w should match in the DT > > > > graph. The caveat being we don't normally show PHYs in that mix > > > > because they are somewhat transparent. That's maybe becoming less true > > > > with routing or muxing included in PHYs. > > > > > > > > > > We have discussed and prototyped this a few times now in the Qualcomm > > > community, and I am not a fan of having to add forwarding-logic to each > > > implementation of a Type-C mux/switch, which in some configuration might > > > have an entity behind it that needs the notifications. > > > > I don't know if we can really escape that. > > > > Okay, we'll have to figure out how to implement that when such need come... > > > > > > But I don't think there's a concern for this binding (in my > > > implementation), there's currently no mode/orientation switching > > > happening beyond this entity. > > > > > > > > > > > > That said, if we're to represent each signal path to the connector, > > > I would like to bring up this problem again: > > > https://lore.kernel.org/all/20220520164810.141400-1-bjorn.andersson@xxxxxxxxxx/ > > > > > > port@0 represent the HS signals going to the connector, port@1 the SS > > > signals going to the connector, port@2 the SBU signals going to the > > > connector. > > > > > > But I need some representation of the HPD (hot plug detection) "signal" > > > (there is no actual signal path in hardware, this is a virtual > > > notification) going _from_ the connector to the DisplayPort controller. > > > > I would assume whatever entity is deciding to enable the DP signals on > > the connector would be what generates the HPD notification. > > The HPD notification comes from the display/connector, and is > conceptually equivalent to hpd-gpios in e.g. the dp-connector binding. Except that it is software based rather than a h/w signal (ignoring the s/w generating a h/w HPD signal for you). > > > I think you want to describe the DP signal connections and muxing, but > > HPD itself wouldn't be in the DT. > > > > Okay, so you're saying that the display driver needs to traverse the > graph representing the display-signal path, in hope to find someone > generating a HPD notification? After further discussion, I think it is still the immediate neighbor, it is just that the immediate neighbor is some other component, not the type-c connector. > > > We discussed this (perhaps in person, as there's no trace on lkml?) and > > > you suggested that I use a second endpoint under port@1, instead of > > > introducing a fourth port. > > > > Multiple endpoints on a port are typically a mux or fanout depending on > > the data direction. But the muxing is not really in the connector, so > > that should probably be represented somewhere else. I think a new port > > suffers from the same issue. Maybe that's close enough? Depends if there > > are cases of more alt modes or more complicated muxing/switching. > > > > > I'm fine either way, but I don't think it would be convenient nor > > > representable to daisy chain this behind any of the existing endpoints; > > > none of the other endpoints should deal with the HPD signal and the > > > direct of_graph-link between the usb-c-connector and the DP controller > > > mimics that of e.g. dp-connector very nicely, both in description and > > > implementation. > > > > That would be nice, but the reality is there's a lot more between DP and > > the connector with USB-C and the graph should reflect that. > > Not when it comes to delivering the HPD notification, afaict. > > The TCPM will configure muxes & switches to ensure that the USB > connector is wired up according to what has been decided over USB PD. > > The HPD signal is orthogonal to that configuration, and should not be > picked up by any of the other components. Agreed as HPD is not a h/w signal routed between components. > > I guess the > > problem there is being able to walk the graph. Suppose we have: > > > > DP out port -> altmode mux in port -> altmode mux out port -> type-c > > port 1 > > > > The issue walking the graph here would be generic code doesn't know > > altmode mux port numbering as that's not a generic thing (could be > > perhaps?). Maybe you can walk from each end and see if you end up in > > the same device. > > > > Of course, I haven't even considered the split cases where it's not > > just either USB3 OR DP, but combinations. > > > > The implementation that toggles between the DP-only and USB/DP-combo is > not stable, so we currently only support USB/DP-combo upstream. Okay, but I don't care about that from a binding standpoint. All possibilities need to be considered whether your h/w can support it or not. > Again, the TCPM will handle the muxing and orientation switching in the > PHY and sbu-mux, then once a datapath capable of delivering DP-altmode > data is established, it might send HPD notifications - to the display > driver. > > > > > What I'd really like to see for all this USB-C stuff is block diagrams > > of the h/w components and then what the corresponding DT looks like. > > Trying to extend things one piece at a time is hard to review and not > > likely going to end with a flexible design. > > > > This is the design we have in a range of different boards: *This* is what I need for every Type-C binding. > > +------------- - - > USB connector | SoC > +-+ | +--------+ +-------+ > | | | | | | | > |*|<------- HS -----|-->| HS phy |<-->| (EUD) |<--+ > | | | | | | | | +--------+ > | | | +--------+ +-------+ +-->| | > | | | | dwc3 | > | | | +--------+ /---------->| | > | | +----------+ | | |<------/ +--------+ > |*|<--|(redriver)|<-|-->| SS phy | > | | +----------+ | | |<-\ +------------+ > | | | +--------+ \->| | > | | | | DP | > | | +-----+ | | controller | > |*|<--->| SBU |<----|------------------>| | > | | | mux | | | | > | | +----+ | +------------+ > +-+ | > +------------- - - Where's the TCPM? > The EUD and redriver are only found/used in some designs. My proposed > representation of this (without those) is: I'd assume a redriver is mostly transparent to s/w? > > /soc { > usb-controller { > dwc3 { > port { > dwc0-out: endpoint { > remote-endpoint = <&connector0_hs>; > }; > }; > }; > > ss_phy: phy { > port { > ss_phy_out: endpoint { > remote-endpoint = <&connector0_ss>; > }; > }; > }; > > display-subsystem { > displayport-controller { > phys = <&ss_phy>; > ports { > port@1 { > reg = <1>; > dp0_out: endpoint { > remote-endpoint = <&connector0_hpd>; > }; > }; > }; > }; > }; > }; > > usb0-sbu-mux { > compatible = "gpio-sbu-mux"; > > port { > sbu0_out: endpoint { > remote-endpoint = <&connector_sbu>; > }; > }; > }; > > tcpm { > connector@0 { > compatible = "usb-c-connector"; > reg = <0>; > ports { > port@0 { > reg = <0>; > connector0_hs: endpoint { > remote-endpoint = <&dwc0_out>; > }; > }; > > port@1 { > reg = <1>; > connector0_ss: endpoint@0 { > remote-endpoint = <&ss_phy_out>; > }; > connector0_hpd: endpoint@1 { > remote-endpoint = <&dp0_out>; > }; This just looks wrong to me because one connection is the output of the phy's mux and one is the input. The USB SS connection is implicit, but I think it should be explicit from dwc3 to ss_phy. It would need an output port and 2 input ports. I want to say we already have bindings doing this. > }; > > port@2 { > reg = <2>; > connector_sbu: endpoint { > remote-endpoint = <&sbu0_out>; > }; > }; > }; > }; > }; > > The EUD needs to be able to override the role-switch state, so the design that > was accepted was to implement the role-switch forwarding logic in the driver > and daisy chain the of-graph. Given EUD is a Qualcomm only thing, I'm not all that worried about it. > > No redriver has made it to LKML, but the this is where the daisy chain vs > reference all instances from port@1 comes in. > > The Type-C port manager (tcpm) might be handling multiple usb-c-connectors, in > which case the reg of the connector identifies which instance is being > described. > > > So I think that all these pieces fits your model, except the port@1/endpoint@1 > and the fact that displayport-controller has a of_graph. > > > In particular we have a number of these: > > /dp-connector { > compatible = "dp-connector"; > > port { > connector: endpoint { > remote-endpoint = <&dp_out>; > }; > }; > }; > > /soc { > display-subsystem { > displayport-controller { > phys = <&some_dp_phy>; > ports { > port@1 { > reg = <1>; > dp_out: endpoint { > remote-endpoint = <&connector>; > }; > }; > }; > }; > }; > }; > > As you said previously, it doesn't make sense to represent the phy in this > graph. We just define the output of the controller as port@1 and link that to > the connector. What I said (or meant) was we don't represent phys which are just providing the electrical interface. Your 'phy' is also a mux/switch, so it does make sense to represent it in the graph. > > So what is the output of the dp controller in the USB case - if we're not > representing that as the HPD link directly between the connector and the > controller? The answer lies in your block diagram... The question I think is whether we could standardize the mux/switch graph ports. Say 'port@0' is the output to type-C connector port@1, and port@[1-9] are altmode connections to USB/DP/TB. If we did that, then generic code can walk the graph from a controller to the connector. We only need to know that port@0 goes to the connector. However, that assumes there is only 1 entity in the middle and I don't know if that holds true. Rob
