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 <[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)'.
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/[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.
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/[email protected]/
Best regards,
Damon
