Re: [PATCH v10 43/69] drm/vc4: hdmi: Propagate -EDEADLK to the top level

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

On 8/20/26 11:55 AM, Maxime Ripard wrote:
> On Fri, Jul 31, 2026 at 07:19:50PM +0300, Cristian Ciocaltea wrote:
>> 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(-)
>>
>> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
>> index ba16e7944cf3..72042fad04c5 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;
> 
> Don't we mask any error that isn't EDEADLK here?

Yes, per .detect_ctx() doc, implementation should return either a
drm_connector_status or -EDEADLK.

This matches what the probe helpers expect, and prevents ending up with
connector_status_unknown, which would be wrong, since the detection was
successful:

int drm_helper_probe_single_connector_modes()
{
	[...]
	ret = drm_helper_probe_detect(connector, &ctx, true);

	if (ret == -EDEADLK) {
		drm_modeset_backoff(&ctx);
		goto retry;
	} else if (WARN(ret < 0, "Invalid return value %i for connector detection\n", ret))
		ret = connector_status_unknown;

	connector->status = ret;
	[...]
}

drm_helper_probe_detect_ctx(struct drm_connector *connector, bool force)
{
	[...]
	ret = drm_modeset_lock(&connector->dev->mode_config.connection_mutex, &ctx);
	if (!ret)
		ret = detect_connector_status(connector, &ctx, force);

	if (ret == -EDEADLK) {
		drm_modeset_backoff(&ctx);
		goto retry;
	}

	if (WARN_ON(ret < 0))
		ret = connector_status_unknown;
	[...]
}