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@redhat.com> 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
>>