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: [email protected]
Reported-by: Diana Zigterman <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
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!
drivers/usb/typec/altmodes/displayport.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 20db51471c98..662cd043b50e 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -547,10 +547,10 @@ int dp_altmode_probe(struct typec_altmode *alt)
/* FIXME: Port can only be DFP_U. */
/* Make sure we have compatiple pin configurations */
- if (!(DP_CAP_DFP_D_PIN_ASSIGN(port->vdo) &
- DP_CAP_UFP_D_PIN_ASSIGN(alt->vdo)) &&
- !(DP_CAP_UFP_D_PIN_ASSIGN(port->vdo) &
- DP_CAP_DFP_D_PIN_ASSIGN(alt->vdo)))
+ if (!(DP_CAP_PIN_ASSIGN_DFP_D(port->vdo) &
+ DP_CAP_PIN_ASSIGN_UFP_D(alt->vdo)) &&
+ !(DP_CAP_PIN_ASSIGN_UFP_D(port->vdo) &
+ DP_CAP_PIN_ASSIGN_DFP_D(alt->vdo)))
return -ENODEV;
ret = sysfs_create_group(&alt->dev.kobj, &dp_altmode_group);
--
2.39.1.519.gcb327c4b5f-goog
On Wed, Feb 08, 2023 at 08:53:19PM +0000, Prashant Malani wrote:
> 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: [email protected]
> Reported-by: Diana Zigterman <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
>
> 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!
>
> drivers/usb/typec/altmodes/displayport.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index 20db51471c98..662cd043b50e 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -547,10 +547,10 @@ int dp_altmode_probe(struct typec_altmode *alt)
> /* FIXME: Port can only be DFP_U. */
>
> /* Make sure we have compatiple pin configurations */
> - if (!(DP_CAP_DFP_D_PIN_ASSIGN(port->vdo) &
> - DP_CAP_UFP_D_PIN_ASSIGN(alt->vdo)) &&
> - !(DP_CAP_UFP_D_PIN_ASSIGN(port->vdo) &
> - DP_CAP_DFP_D_PIN_ASSIGN(alt->vdo)))
> + if (!(DP_CAP_PIN_ASSIGN_DFP_D(port->vdo) &
> + DP_CAP_PIN_ASSIGN_UFP_D(alt->vdo)) &&
> + !(DP_CAP_PIN_ASSIGN_UFP_D(port->vdo) &
> + DP_CAP_PIN_ASSIGN_DFP_D(alt->vdo)))
> return -ENODEV;
>
> ret = sysfs_create_group(&alt->dev.kobj, &dp_altmode_group);
> --
> 2.39.1.519.gcb327c4b5f-goog
thanks,
--
heikki
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: [email protected]
> Reported-by: Diana Zigterman <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>
> ---
>
> 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.
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.
thanks a lot!
martin
On Fri, Feb 17, 2023 at 3:39 AM Martin Kepplinger
<[email protected]> 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: [email protected]
> > Reported-by: Diana Zigterman <[email protected]>
> > Signed-off-by: Prashant Malani <[email protected]>
> > ---
> >
> > 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,
Am Freitag, dem 17.02.2023 um 09:31 -0800 schrieb Prashant Malani:
> On Fri, Feb 17, 2023 at 3:39 AM Martin Kepplinger
> <[email protected]> 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: [email protected]
> > > Reported-by: Diana Zigterman <[email protected]>
> > > Signed-off-by: Prashant Malani <[email protected]>
> > > ---
> > >
> > > 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,
thanks for the quick reply. I think you are right, my report was wrong
and your patch was not the cause of the bug. I was able to reproduce it
without hdmi plugged in. Also, the issue is fixed again in v6.2.
thank you very much and sorry for the false alarm,
martin