lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAE-0n514QMaQC2yjKP8bZqyfbv6B3AQm=+NJ87vxo6NdYiL03A@mail.gmail.com>
Date: Tue, 22 Oct 2024 18:15:47 -0700
From: Stephen Boyd <swboyd@...omium.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.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

Quoting Dmitry Baryshkov (2024-09-20 02:38:53)
> On Sat, Aug 31, 2024 at 09:06:53PM GMT, Stephen Boyd wrote:
>
> Based on our disccusions at LPC, here are several DT examples that seem
> sensible to implement this case and several related cases from other
> ChromeBooks.

(Apologies for getting back to this late. I've been brewing on this topic
for a month; only appropriate for Halloween.)

This is the Trogdor case, one DP controller with 2 lanes DP coming out
that is steered with an analog mux controlled by the EC:

>
> typec {
>         compatible = "google,cros-ec-typec";
>
>         port {
>                 typec_dp_in: endpoint {
>                         remote-endpoint = <&usb_1_qmp_phy_out_dp>;
>                 };
>         };
>
>         usb_c0: connector@0 {
>                 compatible = "usb-c-connector";
>                 reg = <0>;
>
>                 ports {
>                         port@0 {
>                                 reg = <0>;
>                                 usb_c0_hs_in: endpoint {
>                                         remote-endpoint = <&usb_hub_dfp1_hs>;
>                                 };
>                         };
>
>                         port@1 {
>                                 reg = <1>;
>                                 usb_c0_ss_in: endpoint {
>                                         remote-endpoint = <&usb_hub_dfp1_ss>;
>                                 };
>                         };
>                 };
>         };
>
>         usb_c1: connector@1 {
>                 compatible = "usb-c-connector";
>                 reg = <1>;
>
>                 ports {
>                         port@0 {
>                                 reg = <0>;
>                                 usb_c1_hs_in: endpoint {
>                                         remote-endpoint = <&usb_hub_dfp2_hs>;
>                                 };
>                         };
>
>                         port@1 {
>                                 reg = <1>;
>                                 usb_c1_ss_in: endpoint {
>                                         remote-endpoint = <&usb_hub_dfp2_ss>;
>                                 };
>                         };
>                 };
>         };
> };
>
> &usb_1_qmpphy {
>         ports {
>                 port@0 {
>                         endpoint@0 {
>                                 data-lanes = <0 1>;
>                                 // this might go to USB-3 hub
>                         };
>
>                         usb_1_qmp_phy_out_dp: endpoint@1 {
>                                 remote-endpoint = <&typec_dp_in>;
>                                 data-lanes = <2 3>;
>                         };
>                 }
>         };
> };
>
> -------

This is one Corsola case, one DP controller (IT6505) that's an external
bridge that has 4 lanes DP but only 2 lanes of DP are usable at a time
because 2 physical lanes are wired to one usb-c-connector while the
other 2 physical lanes are wired to the other usb-c-connector. I say
"one Corsola case" because there's also a DP bridge (ANX7625) on some
Corsola variants that only has two DP lanes with a crosspoint switch
that can send those DP lanes out of one of two pairs of lanes. The other
two lanes are for USB3 output data if the part is connected to USB3 on
the input side. I suspect this ANX7625 case can be described in the same
way as below with two output endpoints and data-lanes describing which
lanes are used for either DP endpoint.

>
> typec {
>         connector@0 {
>                 port@1 {
>                         endpoint@0 {
>                                 remtoe = <&usb_hub_0>;
>                         };
>
>                         endpoint@1 {
>                                 remote = <&dp_bridge_out_0>;
>                         };

(TL;DR: I think I have a plan with the last paragraph in this section, so
I'll hack on it for a bit to see what happens.)

I'm not thrilled about having two endpoints in the SuperSpeed port@1 to
hold both signals in the Corsola case but not the Trogdor case. The
problem is that there's only one DP endpoint on Trogdor and we can't
have two remote-endpoint properties in there pointing to either
usb-c-connector. But then on Corsola we have two DP endpoints and they
both can't go to one DP input node in the typec node's graph.

To harmonize this the typec graph can have one DP input endpoint on
Trogdor while the typec graph can have two DP input endpoints on
Corsola. In both cases, the typec graph would have two USB input
endpoints, and then we can connect output endpoints to each
usb-c-connector node. It would be similar to my existing binding in this
series, except now the typec DP port can have multiple input endpoints.

I understand from our discussions at LPC that I should use
drm_connector_oob_hotplug_event() from the displayport altmode driver
(drivers/usb/typec/altmodes/displayport.c) to tell the DP bridge like
ANX7625 or IT6505 which output endpoint should be displaying DP. I think
dp_altmode_probe() looks at the usb-c-connector node's parent, e.g.
typec in the examples above, for the drm_bridge. That means we'll need
to register a drm_bridge in the cros-ec-typec compatible node? Or we
need to use the 'displayport' property in cros-ec-typec to point at the
drm_bridge node?

Either way the problem seems to be that I need to associate one
drm_bridge with two displayport altmode drivers and pass some fwnode
handle to drm_connector_oob_hotplug_event() in a way that we can map
that back to the right output endpoint in the DP bridge graph. That
seems to imply that we need to pass the fwnode for the usb-c-connector
in addition to the fwnode for the drm_bridge, so that the drm_bridge
code can look at its DT graph and find the remote node connected.
Basically something like this:

  void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
                                       struct fwnode_handle
*usb_connector_fwnode,
                                       enum drm_connector_status status)

(We might as well also pass the number of lanes here)

Corsola could work with this design, but we'll need to teach
dp_altmode_probe() to look for the drm_bridge elsewhere besides as the
parent of the usb-c-connector node. That implies using the 'displayport'
property in the cros-ec-typec node or teaching dp_altmode_probe() to
look for the port@...ndpoint@1 remote-endpoint handle in the
usb-c-connector graph.

Assuming the bindings you've presented here are fine and good and I got
over the differences between Trogdor and Corsola, then I can make mostly
everything work with the drm_connector_oob_hotplug_event() signature
change from above and some tweaks to dp_altmode_probe() to look for
port@...ndpoint@1 first because that's the "logical" DP input endpoint
in the usb-c-connector binding's graph. Great! The final roadblock I'm
at is that HPD doesn't work on Trogdor, so I can't signal HPD through
the typec framework.

This series fixes that problem by "capturing" HPD state from the
upstream drm_bridge, e.g. msm_dp, by hooking the struct
drm_bridge_funcs::hpd_notify() path and injecting HPD into the typec
messages received from the EC. That's a workaround to make the typec
framework see HPD state changes that are otherwise invisible to the
kernel. Newer firmwares actually tell us the state of HPD properly, but
even then we have to read a gpio mux controlled by the EC to figure out
which usb-c-connector is actually muxing DP when HPD changes on either
typec_port. Having a drm_bridge in cros-ec-typec helped here because we
could hook this path and signal HPD if we knew the firmware was fixed.
If we don't have the drm_bridge anymore, I'm lost how to do this.

Maybe the right answer here is to introduce a drm_connector_dp_typec
structure that is created by the TCPM (cros-ec-typec) that sets a new
DRM_BRIDGE_OP_DP_TYPEC bridge op flag? And then teach
drm_bridge_connector about this new flag, similar to the HDMI one. The
drm_bridge could implement some function that maps the typec_port
(usb-c-connector) to the upstream drm_bridge (ANX7625) graph port,
possibly all in drm_bridge_connector_oob_hotplug_event() so that nothing
else really changes. It could also let us keep hooking the hpd_notify()
path for the workaround needed on Trogdor. And finally it may let us
harmonize the two DT bindings so that we only have one port@...ndpoint
node in the usb-c-connector.


>                 };
>         };
>
>         connector@1 {
>                 port@1 {
>                         endpoint@0 {
>                                 remtoe = <&usb_hub_1>;
>                         };
>
>                         endpoint@1 {
>                                 remote = <&dp_bridge_out_1>;
>                         };
>                 };
>         };
> };
>
> dp_bridge {
>         ports {
>                 port@1 {
>                         dp_bridge_out_0: endpoint@0 {
>                                 remote = <usb_c0_ss_dp>;
>                                 data-lanes = <0 1>;
>                         };
>
>                         dp_bridge_out_1: endpoint@1 {
>                                 remote = <usb_c1_ss_dp>;
>                                 data-lanes = <2 3>;
>                         };
>                 };
>         };
> };
>
> -------
>
> This one is really tough example, we didn't reach a conclusion here.
> If the EC doesn't handle lane remapping, dp_bridge has to get
> orientation-switch and mode-switch properties (as in the end it is the
> dp_bridge that handles reshuffling of the lanes for the Type-C). Per the
> DisplayPort standard the lanes are fixed (e.g. DPCD 101h explicitly
> names lane 0, lanes 0-1, lanes 0-1-2-3).

Are those logical or physical lanes?

I think we'll punt on this one anyway though. We don't have any plans to
do this orientation control mechanism so far. Previous attempts failed
and we put an extra orientation switch control on the board to do the
orientation flipping.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ