Re: [PATCH] usb: typec: altmodes/displayport: Fix probe pin assign check

From: Prashant Malani <pmalani@chromium.org>
To: Martin Kepplinger <martin.kepplinger@puri.sm>
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	bleung@chromium.org, stable@vger.kernel.org,
	Diana Zigterman <dzigterman@chromium.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Guillaume Ranquet <granquet@baylibre.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Macpaul Lin <macpaul.lin@mediatek.com>,
	Pablo Sun <pablo.sun@mediatek.com>
Subject: Re: [PATCH] usb: typec: altmodes/displayport: Fix probe pin assign check
Date: Fri, 17 Feb 2023 09:31:36 -0800	[thread overview]
Message-ID: <CACeCKacT2eMC_JzObCji9ZToq4oPqVcdc2CeDiS=DJdj3QdR5w@mail.gmail.com> (raw)
In-Reply-To: <44487e67c4101db4b57090a1ece66974aeab28b9.camel@puri.sm>

On Fri, Feb 17, 2023 at 3:39 AM Martin Kepplinger
<martin.kepplinger@puri.sm> wrote:
>
> Am Mittwoch, dem 08.02.2023 um 20:53 +0000 schrieb Prashant Malani:
> > While checking Pin Assignments of the port and partner during probe,
> > we
> > don't take into account whether the peripheral is a plug or
> > receptacle.
> >
> > This manifests itself in a mode entry failure on certain docks and
> > dongles with captive cables. For instance, the Startech.com Type-C to
> > DP
> > dongle (Model #CDP2DP) advertises its DP VDO as 0x405. This would
> > fail
> > the Pin Assignment compatibility check, despite it supporting
> > Pin Assignment C as a UFP.
> >
> > Update the check to use the correct DP Pin Assign macros that
> > take the peripheral's receptacle bit into account.
> >
> > Fixes: c1e5c2f0cb8a ("usb: typec: altmodes/displayport: correct pin
> > assignment for UFP receptacles")
> > Cc: stable@vger.kernel.org
> > Reported-by: Diana Zigterman <dzigterman@chromium.org>
> > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > ---
> >
> > I realize this is a bit late in the release cycle, but figured since
> > it
> > is a fix it might still be considered. Please let me know if it's too
> > late and I can re-send this after the 6.3-rc1 is released. Thanks!
>
>
> on the imx8mq-librem5r4.dts board, when using a typec-hub with HDMI,
> this patch breaks image output in one case for me: For a monitor where
> negotiation of resolution fails, a lower resolution works though, I now
> get an oops and hence an unusable system, see the
> dmesg_typec_hub_hdmi_new.txt logs I append. this should definitely not
> happen.

I'll let others comment here too, but more information is required here:
- What's the DP VDO being exported by the Hub?
- What DPConfigure VDM is being sent now (and what was being sent earlier) ?
- Which version of the kernel are you using?
- Can you point to where in the upstream kernel this board file is present?

>
> with your patch reverted, I get no oops and a perfectly usable system
> like before, which is the file dmesg_typec_hub_hdmi_old_ok.txt
>
> could this patch be wrong or at least no universally good for everyone?
> it looks like a regression to me.

I don't think this patch is the cause of the regression you are seeing.

I don't know about the 2nd part, but for the first part, it was
definitely not right
earlier; Pin compatibility checks need to take into account whether the
UFP is a receptacle or not. The stack trace you have shared does not
seem related to PD/Type-C or DRM.

Perhaps this change is uncovering a previously hidden bug on this board.
I'm not sure how this patch causes the failure you see; the patch alters
the conditions for a probe to succeed: either the HUB you are using will
pass this check (in which case you will get DP working) or it won't (in which
case DP should not work at all). Whatever happens after that depends
on the display driver.

BR,