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 <[email protected]>
---
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/[email protected]/
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/[email protected]
v4: https://lore.kernel.org/r/[email protected]
(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/[email protected]/
