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