Re: [PATCH v3 0/6] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode

[Neil Armstrong <neil.armstrong@linaro.org>]

On 06/08/2025 17:48, Konrad Dybcio wrote:
> On 6/3/25 3:17 PM, Dmitry Baryshkov wrote:
>> On Tue, Jun 03, 2025 at 01:03:11PM +0200, Konrad Dybcio wrote:
>>> On 6/2/25 10:55 AM, Neil Armstrong wrote:
>>>> On 28/05/2025 18:56, Konrad Dybcio wrote:
>>>>> On 5/28/25 1:22 PM, Dmitry Baryshkov wrote:
>>>>>> On Wed, May 28, 2025 at 01:13:26PM +0200, Konrad Dybcio wrote:
>>>>>>> On 5/28/25 11:00 AM, Dmitry Baryshkov wrote:
>>>>>>>> On Wed, May 28, 2025 at 12:24:02AM +0200, Konrad Dybcio wrote:
>>>>>>>>> On 5/27/25 11:10 PM, Dmitry Baryshkov wrote:
>>>>>>>>>> On Tue, May 27, 2025 at 10:40:02PM +0200, Konrad Dybcio wrote:
>>>>>>>>>>> Register a typec mux in order to change the PHY mode on the Type-C
>>>>>>>>>>> mux events depending on the mode and the svid when in Altmode setup.
>>>>>>>>>>>
>>>>>>>>>>> The DisplayPort phy should be left enabled if is still powered on
>>>>>>>>>>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>>>>>>>>>>> PHY is not powered off.
>>>>>>>>>>
>>>>>>>>>> This series doesn't seem to solve the USB side of a problem. When the
>>>>>>>>>> PHY is being switch to the 4-lane mode, USB controller looses PIPE
>>>>>>>>>> clock, so it needs to be switched to the USB-2 mode.
>>>>>>>>>
>>>>>>>>> I didn't find issues with that on X13s.. Not sure if it's related, but
>>>>>>>>> on the SL7, after plugging in a 4ln DP connection, I need to plug in
>>>>>>>>> the USB thumb drive twice for the first time (only in that sequence)
>>>>>>>>
>>>>>>>> Might be.
>>>>>>>>
>>>>>>>>> But there's nothing interesting in dmesg and the phy seems to be
>>>>>>>>> programmed with the same values on both the initial and the subsequent
>>>>>>>>> working plug-in
>>>>>>>>
>>>>>>>> Please try using a DP dongle with USB 2 passthrough (there are some).
>>>>>>>> Or just emulate this by enabling DP PHY / DP chain for plugged in USB3
>>>>>>>> devices. Would you be able to see the USB device on a bus?
>>>>>>>
>>>>>>> I only have a dongle with both display and usb that does 2ln dp
>>>>>>> (I tested 4ln dp on a type-c display that I don't think has a hub)
>>>>>>>
>>>>>>> USB3 - yes, USB2 - no (but it works after a replug)
>>>>>>>
>>>>>>> Are you talking about essentially doing qcom,select-utmi-as-pipe-clk
>>>>>>> at runtime?
>>>>>>
>>>>>> I think so.
>>>>>
>>>>> So after quite some time playing with that, I noticed that the USB2
>>>>> device was never actually kicked off the bus.. and works totally fine
>>>>> after connecting the display output (2ln DP)
>>>>>
>>>>> I was looking at dmesg, checking for discovery/disconnect messages,
>>>>> but the device was simply never disconnected (which makes sense given
>>>>> repurposing USB3 TX/RX lanes doesn't affect the D+/D- of USB2)
>>>>>
>>>>> I also read some docs and learnt that what we call
>>>>> qcom,select-utmi-as-pipe-clk is suppossed to be a debug feature
>>>>> and is unnecessary to set on USB2.0-only controllers
>>>>>
>>>>> The USB controller programming guide though doesn't talk about DP,
>>>>> but I'd expect that we may need to set that override for 4lnDP+USB2
>>>>> use cases (which I don't have a dongle for)
>>>>
>>>> Yeah basically we need to:
>>>> 1) power-off the USB3 PHY
>>>> 2) switch to UTMI clock
>>>>
>>>> In the following states:
>>>> - USB safe mode (USB2 lanes may still be connected)
>>>> - 4lanes DP mode
>>>> - DP-only mode
>>>>
>>>> But for this, the dwc3 should also get USB-C events with an addition mode-switch property.
>>>> The flatten DWC3 node now allows that !
>>>
>>> Yeah, I got even more confirmation this is intended..
>>>
>>> I hacked up something that boils down to:
>>>
>>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>>> index 7977860932b1..e5a0a8ec624d 100644
>>> --- a/drivers/usb/dwc3/drd.c
>>> +++ b/drivers/usb/dwc3/drd.c
>>> @@ -464,6 +464,11 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
>>>   		break;
>>>   	}
>>>   
>>> +	if (dwc->mode_fn)
>>> +		dwc->mode_fn(dwc, mode);
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>>> index 7334de85ad10..ea56f5087ecb 100644
>>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>> +static void mode_fn(struct dwc3 *dwc, enum usb_role role)
>>> +{
>>> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>>> +
>>> +	if (dwc->usb3_generic_phy[0])
>>> +		dwc3_qcom_select_utmi_clk(qcom, role == USB_ROLE_NONE);
>>
>> This part is a hack for devices with no USB-2 passthrough, isn't it?
>>
>>>   }
>>>
>>>
>>> It was easy to tap into because there's already mode switch handling..
>>> But obviously it's a hack - should I register a typec_mux in there?
>>
>> I think so, we should listen to mode changes, instead of host/device
>> changes.
>>
>>> Should it go to dwc3-common or dwc3-qcom?
>>
>> I'd say, dwc3-qcom. Not all dwc3 controllers are USB 3 capable, not
>> talking about the USB-C.
> 
> I did some coding and we can't switch clock sources at runtime (not a
> huge shocker), but the bigger issue is that, paraphrasing the HPG, the
> DWC3 controller must be programmed as if it was not SS-capable (probably
> skipping starting some subcores), which is not trivial
> 
> I also came up with a sneaky workaround of simply keeping the USB PLL
> always-on, but the hardware disagrees to do so if the PHY is configured
> in the DP_ONLY mode (which I suppose makes sense)
> 
> All in all, I was not able to find people with a device that actually
> does 4ln DP + USB2 and IIUC the only drawback would be that USB2 would
> not work (and not stall the system). Not sure if/how it recovers after
> you'd plug something else into that port later on, but again, I don't
> have anyone that could test it.
> 
> With that in mind, would you be okay with me resubmitting this series
> with just a rebase & taking care of the comment to patch 5 (pertaining
> to the default mode setting if svid != DP)?

Sure please do so, and I'll retest on the SM8550 & SM8650 platforms.

Thanks for looking !
Neil

> 
> Konrad