Message-ID: <5kisfv22tgqwzjpxqrbx56ywr7l4r7pny3pl2r7crv4rijqbwk@azricdasttg7>
Date: Fri, 15 Nov 2024 19:17:15 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Stephen Boyd <swboyd@...omium.org>
Cc: chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	patches@...ts.linux.dev, devicetree@...r.kernel.org, 
	Douglas Anderson <dianders@...omium.org>, Pin-yen Lin <treapking@...omium.org>, 
	Andrzej Hajda <andrzej.hajda@...el.com>, Benson Leung <bleung@...omium.org>, 
	Conor Dooley <conor+dt@...nel.org>, Daniel Vetter <daniel@...ll.ch>, 
	David Airlie <airlied@...il.com>, dri-devel@...ts.freedesktop.org, 
	Guenter Roeck <groeck@...omium.org>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	Jonas Karlman <jonas@...boo.se>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, 
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Lee Jones <lee@...nel.org>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Prashant Malani <pmalani@...omium.org>, 
	Robert Foss <rfoss@...nel.org>, Rob Herring <robh+dt@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, Tzung-Bi Shih <tzungbi@...nel.org>, 
	Alexandre Belloni <alexandre.belloni@...tlin.com>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
	Daniel Scally <djrscally@...il.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Ivan Orlov <ivan.orlov0322@...il.com>, 
	linux-acpi@...r.kernel.org, linux-usb@...r.kernel.org, 
	Mika Westerberg <mika.westerberg@...ux.intel.com>, "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>, 
	Sakari Ailus <sakari.ailus@...ux.intel.com>, Vinod Koul <vkoul@...nel.org>, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v4 15/18] dt-bindings: usb: Add ports to
 google,cros-ec-typec for DP altmode

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.

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

This is clear.

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

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.

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

I think just "orientation-switch" is enough. In the end it's not a
"typec-orientation-switch".

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists