[PATCH v4 15/18] dt-bindings: usb: Add ports to google,cros-ec-typec for DP altmode

[Stephen Boyd]

Quoting Dmitry Baryshkov (2024-11-08 23:05:18)
> On Thu, Nov 07, 2024 at 04:28:24PM -0800, Stephen Boyd wrote:
> > Quoting Dmitry Baryshkov (2024-10-31 15:54:49)
> > > On Thu, Oct 31, 2024 at 02:45:29PM -0700, Stephen Boyd wrote:
> > > > Quoting Dmitry Baryshkov (2024-10-31 11:42:36)
> > > > > On Tue, Oct 29, 2024 at 01:15:51PM -0700, Stephen Boyd wrote:
> > Long story short, I don't see how we can avoid _any_ lane assignment
> > logic in drm_bridge. The logic shouldn't walk the entire bridge chain,
> > but it should at least act on the bridge that is a DP bridge. I think
> > you're saying pretty much the same thing here, but you want the lane
> > remapping to be done via the typec layer whereas I want it to be done in
> > the drm_bridge layer. To me it looks out of place to add a
> > typec_switch_desc inside each DP drm_bridge because we duplicate the
> > logic about USB type-c DP altmode lane assignment to each DP bridge. A
> > DP bridge should just think about DP and not know or care about USB
> > type-c.
> >
> > This is what's leading me to think we need some sort of lane assignment
> > capability at the DP connector. How that assignment flows from the DP
> > connector created in drm_bridge_connector.c to the hardware is where it
> > is less clear to me. Should that be implemented as a typec_switch_desc,
> > essentially out of band with drm_bridge, or as some drm_bridge_funcs
> > function similar to struct drm_bridge_funcs::hdmi_*()? If you look at
> > IT6505 in it6505_get_extcon_property() it actually wants to pull the
> > orientation of the type-c port with extcon_get_property(EXTCON_DISP_DP,
> > EXTCON_PROP_USB_TYPEC_POLARITY). Maybe pushing the orientation to the DP
> > bridge is backwards and we should be exposing this as some sort of
> > connector API that the drm_bridge can query whenever it wants.
>
> And it6505_get_extcon_property() / EXTCON_PROP_USB_TYPEC_POLARITY is a
> Type-C code, isn't it?
>

Sort of? It's combining DP and USB_TYPEC enums there so it's not very
clear if it's one or the other instead of just both.

> > and then a drm_bridge is created in cros-ec to terminate the bridge
> > chain. The displayport altmode driver will find the drm_bridge and the
> > drm_connector from the cros-ec node. When DP altmode is entered the
> > displayport altmode driver will set the drm_connector orientation based
> > on the presence of the dp-reverse-orientation property. We'll be able to
> > hook the hpd_notify() path in cros-ec by adding code to the drm_bridge
> > made there to do the HPD workaround. I'm not sure we need to use an
> > auxiliary device in this case, because it's a one-off solution for
> > cros-ec. And we don't even need to signal HPD from the cros-ec
> > drm_bridge because the oob_hotplug event will do it for us. If anything,
> > we need that displayport.c code to skip sending the hotplug event when
> > "no-hpd" is present in the cros-ec node. Note, this works for any number
> > of usb-c-connector nodes. And finally, DP bridges like IT6505 don't need
> > to implement a typec_switch_desc, they can simply support flipping the
> > orientation by querying the drm_connector for the bridge chain when they
> > see fit. ANX7625 can support that as well when it doesn't see the
> > 'orientation-switch' property.
> >
> > Did I miss anything? I suspect a drm_connector having an orientation is
> > the most controversial part of this proposal.
>
> Yes... I understand that having orientation-switch handling in the DRM
> driver sounds strange, but this is what we do in the QMP PHY driver. It
> makes the code easier, as it keeps lane remapping local to the place
> where it belongs - to the Type-C handlers.
>

The QMP PHY is a type-c PHY, similar to ANX7625. It sits on the output
of the DP and USB PHYs and handles the type-c orientation and lane
merging for different USB type-c alternate modes. It's not a great
example of a plain DP bridge because it combines USB and USB type-c
features.

Either way, doing this through Type-C handlers is weird because the port
orientation in the Type-C framework is for the connector and there is an
orientation control hardware that handles the orientation already. For
example, with the IT6505 part on Corsola, the orientation is controlled
by a redriver part that the EC controls. It takes the DP and USB signals
and routes them to the correct pins on the usb-c-connector depending on
the cable orientation. The input side pinout is basically 2 or 4 lanes
DP and 2 lanes USB and the output side pinout is the USB type-c pinout
SSTXRX1 and SSTXRX2.

This redriver is equivalent to the QMP PHY type-c part. Maybe to bring
this example closer to QMP we can imagine if the QMP PHY was split into
two pairs of lanes, and the USB functionality wasn't used. The
orientation control for a usb-c-connector would be on a redriver that
takes 2 DP lanes from the QMP PHY as input. Saying that this QMP PHY is
the "orientation-switch" with that property in DT is confusing, because
it isn't controlling the orientation of the type-c port. The orientation
is handled by the redriver. That redriver may even be controlled by the
kernel as an orientation-switch.

I understand that the QMP PHY driver has implemented the lane control
for orientation with a typec_switch_desc, but the QMP PHY is a plain DP
PHY in this scenario. How would the type-c handlers work here? We
couldn't call them through the type-c framework as far as I can tell.

This is why I'm thinking the end of the bridge chain needs to have some
sort of orientation. If we had that then the place where the chain ends
and becomes muxed onto the usb-c-connector, i.e. the redriver, would be
where the DP bridge is told that it needs to flip the lanes. In the
cases I have, the redriver is the EC, and so we've combined them all
together in one node, cros-ec-typec. In the QMP PHY case the redriver is
the QMP PHY type-c part that sits on the DP and USB PHYs and sends their
signals out of the SoC.

Maybe the DT property in the ANX7625 or IT6505 node should be something
like "dp-orientation-switch" and then we have the type-c framework find
this property? Then we would need to add support for that property in
IT6505 using a typec_switch_desc, which is weird. I guess it all feels
like a hack because it's not always the case that the DP PHY is glued to
a USB type-c PHY.