Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Hans de Goede <hdegoede@redhat.com>]

Hi Kyle,

On 20-09-2019 05:24, Kyle Tso wrote:
> *** BLURB HERE ***
> 
> Kyle Tso (2):
>    usb: typec: tcpm: AMS and Collision Avoidance
>    usb: typec: tcpm: AMS for PD2.0

I've finally gotten a chance to test this on one of my own devices
which uses the tcpm framework for its Type-c port.

I am afraid that this series breaks DP altmode support,
specifically, the dp_altmode_configure() function no longer
seems to get called, leading to no pin-assignment being
selected by default and DP thus not working.

So sorry, but I have to NACK this series since it causes
regressions.

It might be easiest if you can get yourself some hardware
which supports DP altmode and uses the fusb302 Type-C
controller (which unlike your controller is actually
supported by the mainline kernel).

2 devices which have this are the original (version 1)
of the GPD win and the GPD pocket. Since the version
is not always clearly marked, make sure you get one which
has a X7-Z8750 CPU, those are the version 1 models, you
can still get these e.g. here:

https://www.geekbuying.com/item/GPD-Pocket-7-Inch-Tablet-PC-Intel-Atom-X7-Z8750-8GB-128GB-375711.html
https://www.geekbuying.com/item/GPD-Win-Intel-Z8750-Windows-10-4GB-64GB-Gamepad-Tablet-PC---Black-378018.html

These 2 devices still need 2 minor patches to hookup the DP
support, I have just finished these 2 patches up and I'm
submitting them upstream today, I will Cc you on these.

If you combine these with one of the many DP-charging pass-through
+ USB-3 out + HDMI out dongles, e.g.:
https://www.aliexpress.com/item/32953320909.html

And then after plugging in do:

cat /sys/class/typec/port0-partner/port0-partner.0/displayport/pin_assignment

This should print:

C [D]

But when I add your patches into the mix it prints just:

C D

And these debug pr_err calls never happen:

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 7845df030b72..d14f94078dd9 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -106,6 +106,7 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
  		break;
  	}

+	pr_err("dp_altmode_configure pin_assign %08x conf %08x\n", pin_assign, DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
  	/* Determining the initial pin assignment. */
  	if (!DP_CONF_GET_PIN_ASSIGN(dp->data.conf)) {
  		/* Is USB together with DP preferred */
@@ -115,6 +116,8 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
  		else if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
  			pin_assign &= DP_PIN_ASSIGN_DP_ONLY_MASK;

+		pr_err("dp_altmode_configure masked pin_assign %08x\n", pin_assign);
+
  		if (!pin_assign)
  			return -EINVAL;


Regards,

Hans