Re: [PATCH v9 16/61] drm/display: hdmi-state-helper: Pass acquire ctx to hotplug helpers

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Self-deadlock due to re-acquiring connection_mutex with a new ww_acquire_ctx in drm_atomic_helper_connector_hdmi_force()
--

--- Patch [16]: [PATCH v9 16/61] drm/display: hdmi-state-helper: Pass acquire ctx to hotplug helpers ---
commit 5cf4eb0e25950e07d4630b5f697abf8096ce1b1d
Author: Cristian Ciocaltea <[email protected]>

drm/display: hdmi-state-helper: Pass acquire ctx to hotplug helpers

Extend drm_atomic_helper_connector_hdmi_hotplug() and
drm_atomic_helper_connector_hdmi_update() to take a struct
drm_modeset_acquire_ctx argument to be used when a CRTC reset is required.
Update callers, including drm_atomic_helper_connector_hdmi_force() which
now initializes its own context.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 92be6278ea443..04d5196a2010f 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
[ ... ]
> @@ -1255,6 +1264,19 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_hotplug);
>   */
>  void drm_atomic_helper_connector_hdmi_force(struct drm_connector *connector)
>  {
> -	drm_atomic_helper_connector_hdmi_update(connector, connector->status);
> +	struct drm_modeset_acquire_ctx ctx;
> +	int ret;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +retry:
> +	ret = drm_atomic_helper_connector_hdmi_update(connector, &ctx, connector->status);

[Severity: High]
Will this cause a self-deadlock when forcing a connector's status?

When a user writes to sysfs to force a connector status, it invokes
drm_helper_probe_single_connector_modes(). This function acquires the
connection_mutex using its own local ww_acquire_ctx before calling the
force() callback:

drm_probe_helper.c:drm_helper_probe_single_connector_modes()
  drm_modeset_acquire_init(&ctx, 0);
  drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx);
  connector->funcs->force(connector);

This new implementation of drm_atomic_helper_connector_hdmi_force() creates
a new local ctx, which is passed down the chain:

drm_atomic_helper_connector_hdmi_force()
  drm_modeset_acquire_init(&new_ctx, 0);
  drm_atomic_helper_connector_hdmi_update(..., &new_ctx, ...)
    drm_connector_hdmi_sync_scdc(..., &new_ctx)
      drm_modeset_lock(&connector->dev->mode_config.connection_mutex, &new_ctx)

Because the current thread already holds connection_mutex but is attempting
to acquire it again using this new, conflicting ww_acquire_ctx with
acquired == 0, won't __ww_mutex_lock() block the thread waiting for itself
to release the lock instead of returning -EDEADLK?

> +	if (ret == -EDEADLK) {
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
>  }

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