Hi Hans, It seems that there is a bug in the patch. Thank you for catching this! I will revise the patch and upload it again. On Thu, Oct 3, 2019 at 6:38 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi, > > On 03-10-2019 12:04, Kyle Tso wrote: > > Hi Hans > > > > Could you append the TCPM log? > > I've attached both good and bad logs, both start with plugging in > one of these PD charging pass-through + USB-3 + HDMI out dongles. > > at a quick glance the problem > seems to be that with the 2 AMS patches added we stop transmitting > after: > > [ 137.751964] Rx VDM cmd 0xff018144 type 1 cmd 4 len 1 > > Where as the good logs still transmits (and receives) a couple of > packets extra after this: > > [ 4475.965108] Rx VDM cmd 0xff018144 type 1 cmd 4 len 1 > [ 4475.965224] PD TX, header: 0x2f6f > [ 4475.968979] PD TX complete, status: 0 > [ 4475.980811] PD RX, header: 0x2a4f [1] > [ 4475.980816] Rx VDM cmd 0xff018150 type 1 cmd 16 len 2 > [ 4475.980929] PD TX, header: 0x216f > [ 4475.984093] PD TX complete, status: 0 > [ 4475.996798] PD RX, header: 0x1c4f [1] > [ 4475.996803] Rx VDM cmd 0xff018151 type 1 cmd 17 len 1 > > Regards, > > Hans > > > > > > On Thu, Oct 3, 2019 at 5:47 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > >> > >> 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 > >>
