Message-ID: <f726862a-bd18-43ee-b307-8daef2451e6b@rock-chips.com>
Date: Tue, 5 Aug 2025 14:32:17 +0800
From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Vinod Koul <vkoul@...nel.org>,
 Kishon Vijay Abraham I <kishon@...nel.org>, Heiko Stuebner
 <heiko@...ech.de>, Sandy Huang <hjc@...k-chips.com>,
 Andy Yan <andy.yan@...k-chips.com>,
 Yubing Zhang <yubing.zhang@...k-chips.com>,
 Frank Wang <frank.wang@...k-chips.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Amit Sunil Dhamne <amitsd@...gle.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Dragan Simic <dsimic@...jaro.org>, Johan Jonker <jbx6244@...il.com>,
 Diederik de Haas <didi.debian@...ow.org>,
 Peter Robinson <pbrobinson@...il.com>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v3 0/5] Add Type-C DP support for RK3399 EVB IND board

Hi Dmitry,

On 8/5/2025 12:26 PM, Dmitry Baryshkov wrote:
> On 05/08/2025 09:13, Chaoyi Chen wrote:
>> Hi Dmitry,
>>
>> On 8/2/2025 5:55 PM, Dmitry Baryshkov wrote:
>>
>> [...]
>>
>>
>>>> Currently, the software simply selects the first available port. So 
>>>> if user
>>>> plugs in DP dongles in both USB-C ports, the DP driver will select 
>>>> the first
>>>> port. This process is implemented in cdn_dp_connected_port() .
>>>>
>>> I think Stephen Boyd has been looking on similar issues for 
>>> Chromebooks,
>>> which were sharing DP controller between several USB-C ports. I don't
>>> remember what was his last status. I think there it was easier since 
>>> the
>>> bifurcation point was the EC.
>>
>> I think the latest progress should be here: [0]. It seems that it 
>> hasn't been updated for a while.
>
> Might be :-(
>
>>
>> [0]: https://lore.kernel.org/all/20240901040658.157425-1- 
>> swboyd@...omium.org/
>>
>>
>>>
>>> I think, CDN-DP needs to register up to two encoders and up to two
>>> connectors, having a separate drm_bridge chain for each of the DP
>>> signals paths (in the end, you can not guarantee that both branches 
>>> will
>>> have the same simple CDN-DP -> PHY -> USB-C configuration: there can be
>>> different retimers, etc).
>>>
>>> Both encoders should list the same CRTC in possible_crtcs, etc. Of
>>> course, it should not be possible to enable both of them.
>>>
>>> This way if the user plugs in two DP dongles, it would be possible to
>>> select, which output actually gets a signal.
>>
>> That makes sense, but this might make the DP driver quite complex. I 
>> will see if I can make it happen.
>
> I think it's trading one burden for another, because CDN-DP currently 
> has a complication of calling cdn_dp_get_port_lanes() / 
> cdn_dp_enable_phy() in a loop rather than just enabling one instance.

Yep, I will give it a try.


>
>>
>>
>>>
>>>>
>>>>>> BTW, one of the important things to do is to implement extcon-like
>>>>>> notifications. I found include/drm/bridge/aux-bridge.h , but if the
>>>>>> aux-bridge is used, the bridge chain would look like this:
>>>>>>
>>>>>> PHY0 aux-bridge ---+
>>>>>>                      | ----> CDN-DP bridge
>>>>>> PHY1 aux-bridge ---+
>>>>>>
>>>>>> Oh, CDN-DP bridge has two previous aux-bridge!
>>>>>>
>>>>>> Now, I try to use drm_connector_oob_hotplug_event() to notify HPD
>>>>>> state between PHY and CDN-DP controller.
>>>>> Does it actually work? The OOB HPD event will be repoted for the 
>>>>> usb-c
>>>>> connector's fwnode, but the DP controller isn't connected to that 
>>>>> node
>>>>> anyhow. If I'm not mistaken, the HPD signal will not reach DP 
>>>>> driver in
>>>>> this case.
>>>> Yes.  What you mentioned is the case in
>>>> drivers/usb/typec/altmodes/displayport.c . I have also added a new 
>>>> OOB event
>>>> notify in the PHY driver in Patch 3, where the expected fwnode is 
>>>> used in
>>>> the PHY. So now we have two OOB HPD events, one from altmodes/ 
>>>> displayport.c
>>>> and the other from PHY. Only the HPD from PHY is eventually passed 
>>>> to the DP
>>>> driver.
>>> This way you will loose IRQ_HPD pulse events from the DP. They are used
>>> by DPRX (aka DP Sink) to signal to DPTX (DP Source) that there was a
>>> change on the DPRX side and the DPTX should reread link params and 
>>> maybe
>>> retrain the link.
>>
>> Sorry, I still don't quite understand your point. I think the entire 
>> notification path is as follows:
>>
>> Type-C mux callback -> RK3399 USBDP PHY -> PHY calls 
>> drm_connector_oob_hotplug_event() -> DP driver
>>
>> Are you concerned that the IRQ_HPD event is not being handled 
>> somewhere along the path? Is it that the Type-C mux callback didn't 
>> notify the PHY, or that after the PHY passed the event to the DP 
>> driver via the OOB event, the DP driver didn't handle it?
>
> The IRQ_HPD is an event coming from DPRX, it is delivered as a part of 
> the attention VDM, see DP_STATUS_IRQ_HPD. It's being handled by the 
> altmode displayport.c driver and is then delivered as an OOB hotplug 
> call. However, it's not a mux event, so it is not (and it should not) 
> being broadcasted over the typec_mux devices.
>
> The way we were handling that is by having a chain of drm_aux_bridges 
> for all interim devices, ending up with a drm_dp_hpd_bridge registered 
> by the TCPM. This way when the DPRX triggers the IRQ_HPD event, it is 
> being handled by the displayport.c and then delivered through that 
> bridge to the DP driver.

I think the issue goes back to the beginning. The key is to reuse the 
logic in displayport.c, and the previous approach of directly setting 
the fwnode has already been rejected. Is it a good idea to register the 
aux hpd bridge in displayport.c? In this way, we don't need to register 
it with a bunch of PD drivers (such as fusb302), which seems like a more 
generic solution.

Powered by blists - more mailing lists