diff mbox series

[v9,36/61] drm/vc4: hdmi: Propagate -EDEADLK to the top level

Message ID 20260723-dw-hdmi-qp-scramb-v9-36-4fb12ea22ac9@collabora.com (mailing list archive)
State New
Headers
Series Add HDMI 2.0 support to DW HDMI QP TX |

Commit Message

Cristian Ciocaltea July 23, 2026, 1:35 a.m. UTC 
drm_connector_helper_funcs.detect_ctx() is expected to propagate
-EDEADLK so that context owners, e.g. drm_helper_probe_detect_ctx(), can
perform the required backoff and retry.

vc4_hdmi_handle_hotplug() instead runs its own retry loop and calls
drm_modeset_backoff() on an acquire context it does not own.  This drops
all locks in the context, including connection_mutex held by the core.

Drop the local retry logic and propagate -EDEADLK to the caller instead.
This aligns VC4 with the expected detect_ctx() semantics.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

Comments

sashiko-bot@kernel.org July 23, 2026, 1:59 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The return value of `drm_atomic_helper_connector_hdmi_hotplug()` is ignored, swallowing `-EDEADLK` errors and causing lock API violations.
--

--- Patch [36]: [PATCH v9 36/61] drm/vc4: hdmi: Propagate -EDEADLK to the top level ---
commit 386da5b87a0ad478948abb5cc75141b0bedce489
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Subject: drm/vc4: hdmi: Propagate -EDEADLK to the top level

drm_connector_helper_funcs.detect_ctx() is expected to propagate
-EDEADLK so that context owners, e.g. drm_helper_probe_detect_ctx(), can
perform the required backoff and retry.

> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 4f7fc6689869e..75a251530970e 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -344,12 +344,11 @@ static int vc4_hdmi_reset_link(struct drm_connector *connector,
>  	return drm_atomic_helper_reset_crtc(crtc, ctx);
>  }
>  
> -static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> -				    struct drm_modeset_acquire_ctx *ctx,
> -				    enum drm_connector_status status)
> +static int vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
> +				   struct drm_modeset_acquire_ctx *ctx,
> +				   enum drm_connector_status status)
>  {
>  	struct drm_connector *connector = &vc4_hdmi->connector;
> -	int ret;
>  
>  	/*
>  	 * NOTE: This function should really be called with vc4_hdmi->mutex
> @@ -368,17 +367,9 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
>  	drm_atomic_helper_connector_hdmi_hotplug(connector, ctx, status);
>  
>  	if (status != connector_status_connected)
> -		return;
> -
> -	for (;;) {
> -		ret = vc4_hdmi_reset_link(connector, ctx);
> -		if (ret == -EDEADLK) {
> -			drm_modeset_backoff(ctx);
> -			continue;
> -		}
> +		return 0;

[Severity: High]
Should the return value of drm_atomic_helper_connector_hdmi_hotplug() be
captured and propagated to the caller?

If drm_atomic_helper_connector_hdmi_hotplug() fails to acquire locks, for
example inside drm_connector_hdmi_sync_scdc(), it can return -EDEADLK. By
ignoring this return value, the standard DRM locking retry semantics are
bypassed.

When a deadlock occurs, ctx->contended is set. If we proceed to use this
context in vc4_hdmi_reset_link(), or eventually drop the locks via
drm_modeset_drop_locks(), it triggers a WARN_ON(ctx->contended) stack
trace in drm_modeset_lock.c.

Additionally, if the connector was disconnected, returning 0 tricks the
caller into believing the operation succeeded. This skips essential
teardown steps like CEC physical address invalidation, Audio InfoFrame
clearing, and SCDC sync retry.

>  
> -		break;
> -	}
> +	return vc4_hdmi_reset_link(connector, ctx);
>  }
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 4f7fc6689869..75a251530970 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -344,12 +344,11 @@  static int vc4_hdmi_reset_link(struct drm_connector *connector,
 	return drm_atomic_helper_reset_crtc(crtc, ctx);
 }
 
-static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
-				    struct drm_modeset_acquire_ctx *ctx,
-				    enum drm_connector_status status)
+static int vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
+				   struct drm_modeset_acquire_ctx *ctx,
+				   enum drm_connector_status status)
 {
 	struct drm_connector *connector = &vc4_hdmi->connector;
-	int ret;
 
 	/*
 	 * NOTE: This function should really be called with vc4_hdmi->mutex
@@ -368,17 +367,9 @@  static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
 	drm_atomic_helper_connector_hdmi_hotplug(connector, ctx, status);
 
 	if (status != connector_status_connected)
-		return;
+		return 0;
 
-	for (;;) {
-		ret = vc4_hdmi_reset_link(connector, ctx);
-		if (ret == -EDEADLK) {
-			drm_modeset_backoff(ctx);
-			continue;
-		}
-
-		break;
-	}
+	return vc4_hdmi_reset_link(connector, ctx);
 }
 
 static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
@@ -416,10 +407,10 @@  static int vc4_hdmi_connector_detect_ctx(struct drm_connector *connector,
 			status = connector_status_connected;
 	}
 
-	vc4_hdmi_handle_hotplug(vc4_hdmi, ctx, status);
+	ret = vc4_hdmi_handle_hotplug(vc4_hdmi, ctx, status);
 	pm_runtime_put(&vc4_hdmi->pdev->dev);
 
-	return status;
+	return ret == -EDEADLK ? ret : status;
 }
 
 static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)