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

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 <[email protected]>
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);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=36