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

[Dmitry Baryshkov]

On Tue, Nov 19, 2024 at 08:09:31PM -0500, Stephen Boyd wrote:
> 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

just to clarify: I'm only talking about a dynamic lane management here.
If the DP bridge hardware supports remapping lanes in a weird way and
board designers decided to use that "feature", then having a property
linke data-lanes = <2 1 3 0>; makes perfect sense to me.

> 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.

/me keeps fingers crossed that hw designers won't do such a thing

But I see your point.

> 
> 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 was assuming that orientation switch is such kind of a hook.

> 
> 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.

If you call it from drm_bridge_connector's oob_hotplug_event handler,
this should fly. Does it cover your 3-DP or 4-DP usecases?

-- 
With best wishes
Dmitry