[PATCH] drm/bridge: analogix_dp: Fix connector status detection for bridges

[PATCH] drm/bridge: analogix_dp: Fix connector status detection for bridges

[Heiko Stuebner]

Right now if there is a next bridge attached to the analogix-dp controller
the driver always assumes this bridge is connected to something, but this
is of course not always true, as that bridge could also be a hotpluggable
dp port for example.

On the other hand, as stated in commit cb640b2ca546 ("drm/bridge: display-
connector: don't set OP_DETECT for DisplayPorts"), "Detecting the monitor
for DisplayPort targets is more complicated than just reading the HPD pin
level" and we should be "letting the actual DP driver perform detection."

So use drm_bridge_detect() to detect the next bridge's state but ignore
that bridge if the analogix-dp is handling the hpd-gpio.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
As this patch stands, it would go on top of v6 of Damon's bridge-connector
work, but could very well be also integrated into one of the changes there.

I don't know yet if my ordering and/or reasoning is the correct one or if
a better handling could be done, but with that change I do get a nice
hotplug behaviour on my rk3588-tiger-dp-carrier board, where the
Analogix-DP ends in a full size DP port.

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index c04b5829712b..cdc56e83b576 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -983,8 +983,12 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
 	struct analogix_dp_device *dp = to_dp(bridge);
 	enum drm_connector_status status = connector_status_disconnected;
 
-	if (dp->plat_data->next_bridge)
-		return connector_status_connected;
+	/*
+	 * An optional next bridge should be in charge of detection the
+	 * connection status, except if we manage a actual hpd gpio.
+	 */
+	if (dp->plat_data->next_bridge && !dp->hpd_gpiod)
+		return drm_bridge_detect(dp->plat_data->next_bridge, connector);
 
 	if (!analogix_dp_detect_hpd(dp))
 		status = connector_status_connected;
-- 
2.47.2

Re: [PATCH] drm/bridge: analogix_dp: Fix connector status detection for bridges

[Dmitry Baryshkov]

On Thu, Oct 09, 2025 at 09:30:28PM +0200, Heiko Stuebner wrote:
> Right now if there is a next bridge attached to the analogix-dp controller
> the driver always assumes this bridge is connected to something, but this
> is of course not always true, as that bridge could also be a hotpluggable
> dp port for example.
> 
> On the other hand, as stated in commit cb640b2ca546 ("drm/bridge: display-
> connector: don't set OP_DETECT for DisplayPorts"), "Detecting the monitor
> for DisplayPort targets is more complicated than just reading the HPD pin
> level" and we should be "letting the actual DP driver perform detection."
> 
> So use drm_bridge_detect() to detect the next bridge's state but ignore
> that bridge if the analogix-dp is handling the hpd-gpio.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> As this patch stands, it would go on top of v6 of Damon's bridge-connector
> work, but could very well be also integrated into one of the changes there.
> 
> I don't know yet if my ordering and/or reasoning is the correct one or if
> a better handling could be done, but with that change I do get a nice
> hotplug behaviour on my rk3588-tiger-dp-carrier board, where the
> Analogix-DP ends in a full size DP port.
> 
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index c04b5829712b..cdc56e83b576 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -983,8 +983,12 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
>  	struct analogix_dp_device *dp = to_dp(bridge);
>  	enum drm_connector_status status = connector_status_disconnected;
>  
> -	if (dp->plat_data->next_bridge)
> -		return connector_status_connected;
> +	/*
> +	 * An optional next bridge should be in charge of detection the
> +	 * connection status, except if we manage a actual hpd gpio.
> +	 */
> +	if (dp->plat_data->next_bridge && !dp->hpd_gpiod)
> +		return drm_bridge_detect(dp->plat_data->next_bridge, connector);

And it's also not correct because the next bridge might be a retimer
with the bridge next to it being a one with the actual detection
capabilities. drm_bridge_connector solves that in a much better way. See
the series at [1]

[1] https://lore.kernel.org/dri-devel/41c2a141-a72e-4780-ab32-f22f3a2e0179@samsung.com/

>  
>  	if (!analogix_dp_detect_hpd(dp))
>  		status = connector_status_connected;
> -- 
> 2.47.2
> 

-- 
With best wishes
Dmitry

Re: [PATCH] drm/bridge: analogix_dp: Fix connector status detection for bridges

[Heiko Stübner]

Hi Dmitry,

Am Freitag, 10. Oktober 2025, 00:30:11 Mitteleuropäische Sommerzeit schrieb Dmitry Baryshkov:
> On Thu, Oct 09, 2025 at 09:30:28PM +0200, Heiko Stuebner wrote:
> > Right now if there is a next bridge attached to the analogix-dp controller
> > the driver always assumes this bridge is connected to something, but this
> > is of course not always true, as that bridge could also be a hotpluggable
> > dp port for example.
> > 
> > On the other hand, as stated in commit cb640b2ca546 ("drm/bridge: display-
> > connector: don't set OP_DETECT for DisplayPorts"), "Detecting the monitor
> > for DisplayPort targets is more complicated than just reading the HPD pin
> > level" and we should be "letting the actual DP driver perform detection."
> > 
> > So use drm_bridge_detect() to detect the next bridge's state but ignore
> > that bridge if the analogix-dp is handling the hpd-gpio.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > As this patch stands, it would go on top of v6 of Damon's bridge-connector
> > work, but could very well be also integrated into one of the changes there.
> > 
> > I don't know yet if my ordering and/or reasoning is the correct one or if
> > a better handling could be done, but with that change I do get a nice
> > hotplug behaviour on my rk3588-tiger-dp-carrier board, where the
> > Analogix-DP ends in a full size DP port.
> > 
> >  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > index c04b5829712b..cdc56e83b576 100644
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -983,8 +983,12 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
> >  	struct analogix_dp_device *dp = to_dp(bridge);
> >  	enum drm_connector_status status = connector_status_disconnected;
> >  
> > -	if (dp->plat_data->next_bridge)
> > -		return connector_status_connected;
> > +	/*
> > +	 * An optional next bridge should be in charge of detection the
> > +	 * connection status, except if we manage a actual hpd gpio.
> > +	 */
> > +	if (dp->plat_data->next_bridge && !dp->hpd_gpiod)
> > +		return drm_bridge_detect(dp->plat_data->next_bridge, connector);
> 
> And it's also not correct because the next bridge might be a retimer
> with the bridge next to it being a one with the actual detection
> capabilities. drm_bridge_connector solves that in a much better way. See
> the series at [1]
> 
> [1] https://lore.kernel.org/dri-devel/41c2a141-a72e-4780-ab32-f22f3a2e0179@samsung.com/

Hence my comment above about that possibly not being the right variant.
Sort of asking for direction :-) .

I am working on top of Damon's drm-bridge-connector series as noted above,
but it looks like the detect function still is called at does then stuff.

My board is the rk3588-tiger-displayport-carrier [0], with a dp-connector
which is the next bridge, so _without_ any changes, the analogix-dp
always assumes "something" is connected and I end up with

[    9.869198] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
[    9.980422] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
[   10.091522] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
[   10.202419] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
[   10.313651] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110

when no display is connected.

With this change I do get the expected hotplug behaviour, so something is
missing still even with the bridge-connector series.


Heiko


[0] v3: https://lore.kernel.org/r/20250812083217.1064185-3-heiko@sntech.de
    v4: https://lore.kernel.org/r/20251009225050.88192-3-heiko@sntech.de
    (moved hpd-gpios from dp-connector back to analogix-dp per dp-connector
    being not able to detect dp-monitors)
> 
> >  
> >  	if (!analogix_dp_detect_hpd(dp))
> >  		status = connector_status_connected;
> 
> 

Re: [PATCH] drm/bridge: analogix_dp: Fix connector status detection for bridges

[Damon Ding]

Hi,

On 10/10/2025 7:42 AM, Heiko Stübner wrote:
> Hi Dmitry,
> 
> Am Freitag, 10. Oktober 2025, 00:30:11 Mitteleuropäische Sommerzeit schrieb Dmitry Baryshkov:
>> On Thu, Oct 09, 2025 at 09:30:28PM +0200, Heiko Stuebner wrote:
>>> Right now if there is a next bridge attached to the analogix-dp controller
>>> the driver always assumes this bridge is connected to something, but this
>>> is of course not always true, as that bridge could also be a hotpluggable
>>> dp port for example.
>>>
>>> On the other hand, as stated in commit cb640b2ca546 ("drm/bridge: display-
>>> connector: don't set OP_DETECT for DisplayPorts"), "Detecting the monitor
>>> for DisplayPort targets is more complicated than just reading the HPD pin
>>> level" and we should be "letting the actual DP driver perform detection."
>>>
>>> So use drm_bridge_detect() to detect the next bridge's state but ignore
>>> that bridge if the analogix-dp is handling the hpd-gpio.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>> ---
>>> As this patch stands, it would go on top of v6 of Damon's bridge-connector
>>> work, but could very well be also integrated into one of the changes there.
>>>
>>> I don't know yet if my ordering and/or reasoning is the correct one or if
>>> a better handling could be done, but with that change I do get a nice
>>> hotplug behaviour on my rk3588-tiger-dp-carrier board, where the
>>> Analogix-DP ends in a full size DP port.
>>>
>>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index c04b5829712b..cdc56e83b576 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> @@ -983,8 +983,12 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
>>>   	struct analogix_dp_device *dp = to_dp(bridge);
>>>   	enum drm_connector_status status = connector_status_disconnected;
>>>   
>>> -	if (dp->plat_data->next_bridge)
>>> -		return connector_status_connected;
>>> +	/*
>>> +	 * An optional next bridge should be in charge of detection the
>>> +	 * connection status, except if we manage a actual hpd gpio.
>>> +	 */
>>> +	if (dp->plat_data->next_bridge && !dp->hpd_gpiod)
>>> +		return drm_bridge_detect(dp->plat_data->next_bridge, connector);

I have tried to use the drm_bridge_detect() API to do this as 
simple-bridge driver, but it does not work well for bridges that do not 
declare OP_DETECT.

Take nxp-ptn3460 as an example, the connected status will be treated as 
connector_status_unknown via the following call stack:

drm_helper_probe_single_connector_modes()
   -> drm_helper_probe_detect()
      -> drm_bridge_connector_detect()
         -> analogix_dp_bridge_detect()
            -> drm_bridge_detect()
               -> return connector_status_unknown due to !OP_DETECT

However, the connected status will be connector_status_connected as 
expected if Analogix DP does not declare OP_DETECT[0]:

drm_helper_probe_single_connector_modes()
   -> drm_helper_probe_detect()
      -> drm_bridge_connector_detect()
         -> return connector_status_connected due to CONNECTOR_LVDS

Base on Andy's commit 5d156a9c3d5e ("drm/bridge: Pass down connector to 
drm bridge detect hook"), the drm_connector becomes available in 
drm_bridge_detect().

It may be better to unify the logic of drm_bridge_detect() and 
drm_bridge_connector_detect(), which sets the connected status according 
to the connector_type. Then the codes will be more reasonable and become 
similar to the simple-bridge demo via 
'drm_bridge_detect(dp->plat_data->next_bridge, connector)'.

But we still need a specific check for DP-connector to resolve this 
issue. The '!dp->hpd_gpiod' may not be well-considered. Perhaps we could 
introduce a new API, similar to drm_bridge_is_panel(), called 
drm_bridge_is_display_connector()?

>>
>> And it's also not correct because the next bridge might be a retimer
>> with the bridge next to it being a one with the actual detection
>> capabilities. drm_bridge_connector solves that in a much better way. See
>> the series at [1]
>>
>> [1] https://lore.kernel.org/dri-devel/41c2a141-a72e-4780-ab32-f22f3a2e0179@samsung.com/
> 
> Hence my comment above about that possibly not being the right variant.
> Sort of asking for direction :-) .
> 
> I am working on top of Damon's drm-bridge-connector series as noted above,
> but it looks like the detect function still is called at does then stuff.
> 
> My board is the rk3588-tiger-displayport-carrier [0], with a dp-connector
> which is the next bridge, so _without_ any changes, the analogix-dp
> always assumes "something" is connected and I end up with
> 
> [    9.869198] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> [    9.980422] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> [   10.091522] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> [   10.202419] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> [   10.313651] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> 
> when no display is connected.
> 
> With this change I do get the expected hotplug behaviour, so something is
> missing still even with the bridge-connector series.
> 
> 
> Heiko
> 
> 
> [0] v3: https://lore.kernel.org/r/20250812083217.1064185-3-heiko@sntech.de
>      v4: https://lore.kernel.org/r/20251009225050.88192-3-heiko@sntech.de
>      (moved hpd-gpios from dp-connector back to analogix-dp per dp-connector
>      being not able to detect dp-monitors)
>>
>>>   
>>>   	if (!analogix_dp_detect_hpd(dp))
>>>   		status = connector_status_connected;
>>
>>
> 
> 

I see... There is also a way to leave the connected status check in 
Analogix DP bridge:

1.If the later bridge does not support HPD function, the 'force-hpd' 
property must be set under the DP DT node. The DT modifications may be
large by this way.
2.If the later bridge do support HPD function like the DP-connector, the
connected status detection method is GPIO HPD or functional pin HPD.

With the DT modifications for above 1, the analogix_dp_bridge_detect() 
can be simplified to:

static enum drm_connector_status
analogix_dp_bridge_detect(struct drm_bridge *bridge, struct 
drm_connector *connector)
{
	struct analogix_dp_device *dp = to_dp(bridge);
	enum drm_connector_status status = connector_status_disconnected;

	if (!analogix_dp_detect_hpd(dp))
		status = connector_status_connected;

	return status;
}

Best regards,
Damon

[0]https://lore.kernel.org/all/22a5119c-01da-4a7a-bafb-f657b1552a56@rock-chips.com/

Re: [PATCH] drm/bridge: analogix_dp: Fix connector status detection for bridges

[Dmitry Baryshkov]

On Fri, Oct 10, 2025 at 12:02:43PM +0800, Damon Ding wrote:
> Hi,
> 
> On 10/10/2025 7:42 AM, Heiko Stübner wrote:
> > Hi Dmitry,
> > 
> > Am Freitag, 10. Oktober 2025, 00:30:11 Mitteleuropäische Sommerzeit schrieb Dmitry Baryshkov:
> > > On Thu, Oct 09, 2025 at 09:30:28PM +0200, Heiko Stuebner wrote:
> > > > Right now if there is a next bridge attached to the analogix-dp controller
> > > > the driver always assumes this bridge is connected to something, but this
> > > > is of course not always true, as that bridge could also be a hotpluggable
> > > > dp port for example.
> > > > 
> > > > On the other hand, as stated in commit cb640b2ca546 ("drm/bridge: display-
> > > > connector: don't set OP_DETECT for DisplayPorts"), "Detecting the monitor
> > > > for DisplayPort targets is more complicated than just reading the HPD pin
> > > > level" and we should be "letting the actual DP driver perform detection."
> > > > 
> > > > So use drm_bridge_detect() to detect the next bridge's state but ignore
> > > > that bridge if the analogix-dp is handling the hpd-gpio.
> > > > 
> > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > > ---
> > > > As this patch stands, it would go on top of v6 of Damon's bridge-connector
> > > > work, but could very well be also integrated into one of the changes there.
> > > > 
> > > > I don't know yet if my ordering and/or reasoning is the correct one or if
> > > > a better handling could be done, but with that change I do get a nice
> > > > hotplug behaviour on my rk3588-tiger-dp-carrier board, where the
> > > > Analogix-DP ends in a full size DP port.
> > > > 
> > > >   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 8 ++++++--
> > > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > index c04b5829712b..cdc56e83b576 100644
> > > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > > > @@ -983,8 +983,12 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
> > > >   	struct analogix_dp_device *dp = to_dp(bridge);
> > > >   	enum drm_connector_status status = connector_status_disconnected;
> > > > -	if (dp->plat_data->next_bridge)
> > > > -		return connector_status_connected;
> > > > +	/*
> > > > +	 * An optional next bridge should be in charge of detection the
> > > > +	 * connection status, except if we manage a actual hpd gpio.
> > > > +	 */
> > > > +	if (dp->plat_data->next_bridge && !dp->hpd_gpiod)
> > > > +		return drm_bridge_detect(dp->plat_data->next_bridge, connector);
> 
> I have tried to use the drm_bridge_detect() API to do this as simple-bridge
> driver, but it does not work well for bridges that do not declare OP_DETECT.
> 
> Take nxp-ptn3460 as an example, the connected status will be treated as
> connector_status_unknown via the following call stack:
> 
> drm_helper_probe_single_connector_modes()
>   -> drm_helper_probe_detect()
>      -> drm_bridge_connector_detect()
>         -> analogix_dp_bridge_detect()
>            -> drm_bridge_detect()
>               -> return connector_status_unknown due to !OP_DETECT
> 
> However, the connected status will be connector_status_connected as expected
> if Analogix DP does not declare OP_DETECT[0]:
> 
> drm_helper_probe_single_connector_modes()
>   -> drm_helper_probe_detect()
>      -> drm_bridge_connector_detect()
>         -> return connector_status_connected due to CONNECTOR_LVDS
> 
> Base on Andy's commit 5d156a9c3d5e ("drm/bridge: Pass down connector to drm
> bridge detect hook"), the drm_connector becomes available in
> drm_bridge_detect().
> 
> It may be better to unify the logic of drm_bridge_detect() and
> drm_bridge_connector_detect(), which sets the connected status according to
> the connector_type. Then the codes will be more reasonable and become
> similar to the simple-bridge demo via
> 'drm_bridge_detect(dp->plat_data->next_bridge, connector)'.

I'm not sure, what you are trying to achieve here. The
drm_bridge_connector should handle the OP_DETECT and use the last bridge
in the chain that supports detection.

Note: OP_HPD and OP_DETECT are not that tightly connected, especially
for DP bridges. It is fine to have a later bridge which generates HPD
events, while Analogix DP bridge responds to .hpd_notify() events and
performs its duties.

For example, it's perfectly fine to have dp-connector with the HPD GPIO
pin routed to the connector (in which case it will declare OP_HPD). But
the Analogix bridge should perform actual detection. Normally this is
handled by reading DPCD and ensuring that there is an actual connected
device (i.e. either a non-branch device or a branch with at least 1
sink).

> 
> But we still need a specific check for DP-connector to resolve this issue.
> The '!dp->hpd_gpiod' may not be well-considered. Perhaps we could introduce
> a new API, similar to drm_bridge_is_panel(), called
> drm_bridge_is_display_connector()?
> 
> > > 
> > > And it's also not correct because the next bridge might be a retimer
> > > with the bridge next to it being a one with the actual detection
> > > capabilities. drm_bridge_connector solves that in a much better way. See
> > > the series at [1]
> > > 
> > > [1] https://lore.kernel.org/dri-devel/41c2a141-a72e-4780-ab32-f22f3a2e0179@samsung.com/
> > 
> > Hence my comment above about that possibly not being the right variant.
> > Sort of asking for direction :-) .
> > 
> > I am working on top of Damon's drm-bridge-connector series as noted above,
> > but it looks like the detect function still is called at does then stuff.
> > 
> > My board is the rk3588-tiger-displayport-carrier [0], with a dp-connector
> > which is the next bridge, so _without_ any changes, the analogix-dp
> > always assumes "something" is connected and I end up with
> > 
> > [    9.869198] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> > [    9.980422] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> > [   10.091522] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> > [   10.202419] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> > [   10.313651] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
> > 
> > when no display is connected.
> > 
> > With this change I do get the expected hotplug behaviour, so something is
> > missing still even with the bridge-connector series.
> > 
> > 
> > Heiko
> > 
> > 
> > [0] v3: https://lore.kernel.org/r/20250812083217.1064185-3-heiko@sntech.de
> >      v4: https://lore.kernel.org/r/20251009225050.88192-3-heiko@sntech.de
> >      (moved hpd-gpios from dp-connector back to analogix-dp per dp-connector
> >      being not able to detect dp-monitors)
> > > 
> > > >   	if (!analogix_dp_detect_hpd(dp))
> > > >   		status = connector_status_connected;
> > > 
> > > 
> > 
> > 
> 
> I see... There is also a way to leave the connected status check in Analogix
> DP bridge:
> 
> 1.If the later bridge does not support HPD function, the 'force-hpd'
> property must be set under the DP DT node. The DT modifications may be
> large by this way.

Well... The drivers should continue to work with old DTs, if they did so
before.

> 2.If the later bridge do support HPD function like the DP-connector, the
> connected status detection method is GPIO HPD or functional pin HPD.

dp-connector should set OP_HPD
analogix-dp should set OP_DETECT

> 
> With the DT modifications for above 1, the analogix_dp_bridge_detect() can
> be simplified to:
> 
> static enum drm_connector_status
> analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector
> *connector)
> {
> 	struct analogix_dp_device *dp = to_dp(bridge);
> 	enum drm_connector_status status = connector_status_disconnected;
> 
> 	if (!analogix_dp_detect_hpd(dp))
> 		status = connector_status_connected;
> 
> 	return status;
> }
> 
> Best regards,
> Damon
> 
> [0]https://lore.kernel.org/all/22a5119c-01da-4a7a-bafb-f657b1552a56@rock-chips.com/
> 

-- 
With best wishes
Dmitry

Re: [PATCH] drm/bridge: analogix_dp: Fix connector status detection for bridges

[Damon Ding]

Hi Dmitry,

On 10/10/2025 8:43 PM, Dmitry Baryshkov wrote:
> On Fri, Oct 10, 2025 at 12:02:43PM +0800, Damon Ding wrote:
>> Hi,
>>
>> On 10/10/2025 7:42 AM, Heiko Stübner wrote:
>>> Hi Dmitry,
>>>
>>> Am Freitag, 10. Oktober 2025, 00:30:11 Mitteleuropäische Sommerzeit schrieb Dmitry Baryshkov:
>>>> On Thu, Oct 09, 2025 at 09:30:28PM +0200, Heiko Stuebner wrote:
>>>>> Right now if there is a next bridge attached to the analogix-dp controller
>>>>> the driver always assumes this bridge is connected to something, but this
>>>>> is of course not always true, as that bridge could also be a hotpluggable
>>>>> dp port for example.
>>>>>
>>>>> On the other hand, as stated in commit cb640b2ca546 ("drm/bridge: display-
>>>>> connector: don't set OP_DETECT for DisplayPorts"), "Detecting the monitor
>>>>> for DisplayPort targets is more complicated than just reading the HPD pin
>>>>> level" and we should be "letting the actual DP driver perform detection."
>>>>>
>>>>> So use drm_bridge_detect() to detect the next bridge's state but ignore
>>>>> that bridge if the analogix-dp is handling the hpd-gpio.
>>>>>
>>>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>>>> ---
>>>>> As this patch stands, it would go on top of v6 of Damon's bridge-connector
>>>>> work, but could very well be also integrated into one of the changes there.
>>>>>
>>>>> I don't know yet if my ordering and/or reasoning is the correct one or if
>>>>> a better handling could be done, but with that change I do get a nice
>>>>> hotplug behaviour on my rk3588-tiger-dp-carrier board, where the
>>>>> Analogix-DP ends in a full size DP port.
>>>>>
>>>>>    drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 8 ++++++--
>>>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>>> index c04b5829712b..cdc56e83b576 100644
>>>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>>> @@ -983,8 +983,12 @@ analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector *conne
>>>>>    	struct analogix_dp_device *dp = to_dp(bridge);
>>>>>    	enum drm_connector_status status = connector_status_disconnected;
>>>>> -	if (dp->plat_data->next_bridge)
>>>>> -		return connector_status_connected;
>>>>> +	/*
>>>>> +	 * An optional next bridge should be in charge of detection the
>>>>> +	 * connection status, except if we manage a actual hpd gpio.
>>>>> +	 */
>>>>> +	if (dp->plat_data->next_bridge && !dp->hpd_gpiod)
>>>>> +		return drm_bridge_detect(dp->plat_data->next_bridge, connector);
>>
>> I have tried to use the drm_bridge_detect() API to do this as simple-bridge
>> driver, but it does not work well for bridges that do not declare OP_DETECT.
>>
>> Take nxp-ptn3460 as an example, the connected status will be treated as
>> connector_status_unknown via the following call stack:
>>
>> drm_helper_probe_single_connector_modes()
>>    -> drm_helper_probe_detect()
>>       -> drm_bridge_connector_detect()
>>          -> analogix_dp_bridge_detect()
>>             -> drm_bridge_detect()
>>                -> return connector_status_unknown due to !OP_DETECT
>>
>> However, the connected status will be connector_status_connected as expected
>> if Analogix DP does not declare OP_DETECT[0]:
>>
>> drm_helper_probe_single_connector_modes()
>>    -> drm_helper_probe_detect()
>>       -> drm_bridge_connector_detect()
>>          -> return connector_status_connected due to CONNECTOR_LVDS
>>
>> Base on Andy's commit 5d156a9c3d5e ("drm/bridge: Pass down connector to drm
>> bridge detect hook"), the drm_connector becomes available in
>> drm_bridge_detect().
>>
>> It may be better to unify the logic of drm_bridge_detect() and
>> drm_bridge_connector_detect(), which sets the connected status according to
>> the connector_type. Then the codes will be more reasonable and become
>> similar to the simple-bridge demo via
>> 'drm_bridge_detect(dp->plat_data->next_bridge, connector)'.
> 
> I'm not sure, what you are trying to achieve here. The
> drm_bridge_connector should handle the OP_DETECT and use the last bridge
> in the chain that supports detection.
> 
> Note: OP_HPD and OP_DETECT are not that tightly connected, especially
> for DP bridges. It is fine to have a later bridge which generates HPD
> events, while Analogix DP bridge responds to .hpd_notify() events and
> performs its duties.
> 
> For example, it's perfectly fine to have dp-connector with the HPD GPIO
> pin routed to the connector (in which case it will declare OP_HPD). But
> the Analogix bridge should perform actual detection. Normally this is
> handled by reading DPCD and ensuring that there is an actual connected
> device (i.e. either a non-branch device or a branch with at least 1
> sink).
> 

I see. Your kind explanation helped me understand OP_HPD and OP_DETECT 
better.

Back to Heiko's issue, the v3, in which dp-connector declares OP_HPD, 
should be better (refers to arm64/qcom/qcs6490-rb3gen2 and 
arm64/qcom/sa8295p-adp). And the .hpd_notify() of Analogix DP bridge 
will be supported in the future if something needs to be handle with the 
HPD status changes (refers to driver drivers/gpu/drm/msm/dp/dp_display.c).

Additionally, I will keep analogix_dp_bridge_detect() the same as before.

>>
>> But we still need a specific check for DP-connector to resolve this issue.
>> The '!dp->hpd_gpiod' may not be well-considered. Perhaps we could introduce
>> a new API, similar to drm_bridge_is_panel(), called
>> drm_bridge_is_display_connector()?
>>
>>>>
>>>> And it's also not correct because the next bridge might be a retimer
>>>> with the bridge next to it being a one with the actual detection
>>>> capabilities. drm_bridge_connector solves that in a much better way. See
>>>> the series at [1]
>>>>
>>>> [1] https://lore.kernel.org/dri-devel/41c2a141-a72e-4780-ab32-f22f3a2e0179@samsung.com/
>>>
>>> Hence my comment above about that possibly not being the right variant.
>>> Sort of asking for direction :-) .
>>>
>>> I am working on top of Damon's drm-bridge-connector series as noted above,
>>> but it looks like the detect function still is called at does then stuff.
>>>
>>> My board is the rk3588-tiger-displayport-carrier [0], with a dp-connector
>>> which is the next bridge, so _without_ any changes, the analogix-dp
>>> always assumes "something" is connected and I end up with
>>>
>>> [    9.869198] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
>>> [    9.980422] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
>>> [   10.091522] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
>>> [   10.202419] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
>>> [   10.313651] [drm:analogix_dp_bridge_atomic_enable] *ERROR* failed to get hpd single ret = -110
>>>
>>> when no display is connected.
>>>
>>> With this change I do get the expected hotplug behaviour, so something is
>>> missing still even with the bridge-connector series.
>>>
>>>
>>> Heiko
>>>
>>>
>>> [0] v3: https://lore.kernel.org/r/20250812083217.1064185-3-heiko@sntech.de
>>>       v4: https://lore.kernel.org/r/20251009225050.88192-3-heiko@sntech.de
>>>       (moved hpd-gpios from dp-connector back to analogix-dp per dp-connector
>>>       being not able to detect dp-monitors)
>>>>
>>>>>    	if (!analogix_dp_detect_hpd(dp))
>>>>>    		status = connector_status_connected;
>>>>
>>>>
>>>
>>>
>>
>> I see... There is also a way to leave the connected status check in Analogix
>> DP bridge:
>>
>> 1.If the later bridge does not support HPD function, the 'force-hpd'
>> property must be set under the DP DT node. The DT modifications may be
>> large by this way.
> 
> Well... The drivers should continue to work with old DTs, if they did so
> before.
> 
>> 2.If the later bridge do support HPD function like the DP-connector, the
>> connected status detection method is GPIO HPD or functional pin HPD.
> 
> dp-connector should set OP_HPD
> analogix-dp should set OP_DETECT
> 

Got it.

>>
>> With the DT modifications for above 1, the analogix_dp_bridge_detect() can
>> be simplified to:
>>
>> static enum drm_connector_status
>> analogix_dp_bridge_detect(struct drm_bridge *bridge, struct drm_connector
>> *connector)
>> {
>> 	struct analogix_dp_device *dp = to_dp(bridge);
>> 	enum drm_connector_status status = connector_status_disconnected;
>>
>> 	if (!analogix_dp_detect_hpd(dp))
>> 		status = connector_status_connected;
>>
>> 	return status;
>> }
>>
>> Best regards,
>> Damon
>>
>> [0]https://lore.kernel.org/all/22a5119c-01da-4a7a-bafb-f657b1552a56@rock-chips.com/
>>
> 

Best regards,
Damon