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

[Stephen Boyd]

Quoting Dmitry Baryshkov (2024-11-15 09:17:15)
> On Mon, Nov 11, 2024 at 06:16:27PM -0800, Stephen Boyd wrote:
> > 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.
>
> But EXTCON_PROP_USB_TYPEC_POLARITY is just a Type-C, nothing about DP in it.

It's extcon_get_property(it6505->extcon, EXTCON_DISP_DP,
EXTCON_PROP_USB_TYPEC_POLARITY, ...) which has EXTCON_DISP_DP in there,
so there's something about DP there. That's all I'm saying.

> >
> > 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.
>
> If QMP PHY is a plain DP PHY, it usually has no support for lane remapping
> (e.g. phy-qcom-edp doesn't).
>
> Let me reiterate, please: lane management is outside of the DisplayPort
> spec, at least as far as I can understand it. All lane remapping
> (especially a dynamic one) is a pure vendor extension to the standard.
> I'm trying to find a way to support Corsola and Trogdor without adding
> "this is done specially for Google" kind of API. Usually that doesn't
> fly in the long term.

Got it.

>
> I understand that using Type-C API for the DRM bridge sounds strange.
> But even the mentioned bridge uses Type-C API. It asks for the Type-C
> polarity, not the DP polarity.
>

I understand that lane assignment isn't part of the DisplayPort spec,
while it is part of the USB Type-C DisplayPort Altmode spec.

I'm not entirely convinced that lane assignment is _only_ part of the
altmode spec and should be implemented with a typec switch though,
because I imagine some hardware design could be created that has two
DisplayPort connectors, just like these two USB-C connectors, and some
sort of HPD redriver logic similar to the EC that decides which DP port
"wins" and should have DP sent to it. Or perhaps 2 lanes DP to a DP
connector and 2 lanes DP sent to a DP to HDMI bridge (shudder). In
either case, USB type-c isn't involved.

It sounds like we're debating how to handle lane assignment in the
kernel. Either way, the code is going to be implemented in the bridge
driver because it's the one that has to change what physical lane a
logical lane is assigned to. The question is if it should be some sort
of bridge_funcs callback, or should bridge drivers hook into the typec
framework to expose an orientation switch, or something else?

I'm thinking we should introduce some sort of bridge_funcs callback that
can be called from the DP altmode driver, either parallel to the
drm_connector_oob_hotplug_event() function or from it directly. If we
can pass the fwnode for the usb-c-connector to the oob_hotplug_event
callback, maybe that's all we need to figure out which lanes go where.
And then in the 2 DP connector muxing world we can call
drm_connector_oob_hotplug_event() with one or the other DP connector
node, which will likely be children nodes of the "HPD redriver" device.