diff mbox series

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

Message ID 20260723-dw-hdmi-qp-scramb-v9-16-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 
In preparation for supporting SCDC status synchronization on hotplug
events, extend drm_atomic_helper_connector_hdmi_hotplug() and
drm_atomic_helper_connector_hdmi_update() to take a struct
drm_modeset_acquire_ctx argument.  This will be used when a CRTC reset
is required.

Additionally, change their return type from void to int to allow
propagation of errors such as -EDEADLK caused by lock contention.

Update existing callers accordingly.  Note that in the VC4 case, the
return code is ignored, as it cannot fail: being in the .detect_ctx()
call path, the connection mutex is already held through ctx, and it
bails out early because the driver relies on a local SCDC
state-management implementation, and therefore does not yet expose the
scrambler capability through the common helpers.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/gpu/drm/display/drm_bridge_connector.c  |  8 ++++--
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 34 ++++++++++++++++++++-----
 drivers/gpu/drm/vc4/vc4_hdmi.c                  |  2 +-
 include/drm/display/drm_hdmi_state_helper.h     |  6 +++--
 4 files changed, 39 insertions(+), 11 deletions(-)

Comments

sashiko-bot@kernel.org July 23, 2026, 1:51 a.m. UTC | #1 
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 <cristian.ciocaltea@collabora.com>

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);
>  }
diff mbox series

Patch

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index ed3b5b73a88a..1dcce4ab8035 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -297,12 +297,16 @@  static int drm_bridge_connector_detect_ctx(struct drm_connector *connector,
 	struct drm_bridge *detect = bridge_connector->bridge_detect;
 	struct drm_bridge *hdmi = bridge_connector->bridge_hdmi;
 	enum drm_connector_status status;
+	int ret;
 
 	if (detect) {
 		status = detect->funcs->detect(detect, connector);
 
-		if (hdmi)
-			drm_atomic_helper_connector_hdmi_hotplug(connector, status);
+		if (hdmi) {
+			ret = drm_atomic_helper_connector_hdmi_hotplug(connector, ctx, status);
+			if (ret == -EDEADLK)
+				return ret;
+		}
 
 		drm_bridge_connector_hpd_notify(connector, status);
 	} else {
diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index 92be6278ea44..04d5196a2010 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -1199,8 +1199,9 @@  drm_atomic_helper_connector_hdmi_clear_audio_infoframe(struct drm_connector *con
 }
 EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_clear_audio_infoframe);
 
-static void
+static int
 drm_atomic_helper_connector_hdmi_update(struct drm_connector *connector,
+					struct drm_modeset_acquire_ctx *ctx,
 					enum drm_connector_status status)
 {
 	const struct drm_edid *drm_edid;
@@ -1210,7 +1211,7 @@  drm_atomic_helper_connector_hdmi_update(struct drm_connector *connector,
 		drm_connector_hdmi_audio_plugged_notify(connector, false);
 		drm_edid_connector_update(connector, NULL);
 		drm_connector_cec_phys_addr_invalidate(connector);
-		return;
+		return 0;
 	}
 
 	if (connector->hdmi.funcs->read_edid)
@@ -1227,20 +1228,28 @@  drm_atomic_helper_connector_hdmi_update(struct drm_connector *connector,
 		drm_connector_hdmi_audio_plugged_notify(connector, true);
 		drm_connector_cec_phys_addr_set(connector);
 	}
+
+	return 0;
 }
 
 /**
  * drm_atomic_helper_connector_hdmi_hotplug - Handle the hotplug event for the HDMI connector
  * @connector: A pointer to the HDMI connector
+ * @ctx: Lock acquisition context to be used for resetting CRTC
  * @status: Connection status
  *
  * This function should be called as a part of the .detect() / .detect_ctx()
  * callbacks for all status changes.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ * If @ctx is set, it might also return -EDEADLK.
  */
-void drm_atomic_helper_connector_hdmi_hotplug(struct drm_connector *connector,
-					      enum drm_connector_status status)
+int drm_atomic_helper_connector_hdmi_hotplug(struct drm_connector *connector,
+					     struct drm_modeset_acquire_ctx *ctx,
+					     enum drm_connector_status status)
 {
-	drm_atomic_helper_connector_hdmi_update(connector, status);
+	return drm_atomic_helper_connector_hdmi_update(connector, ctx, status);
 }
 EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_hotplug);
 
@@ -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);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
 }
 EXPORT_SYMBOL(drm_atomic_helper_connector_hdmi_force);
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 72ff95ca97de..7f613f912368 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -375,7 +375,7 @@  static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
 	 * the lock for now.
 	 */
 
-	drm_atomic_helper_connector_hdmi_hotplug(connector, status);
+	drm_atomic_helper_connector_hdmi_hotplug(connector, ctx, status);
 
 	if (status != connector_status_connected)
 		return;
diff --git a/include/drm/display/drm_hdmi_state_helper.h b/include/drm/display/drm_hdmi_state_helper.h
index 13375bd0f4ae..c26c6a87795e 100644
--- a/include/drm/display/drm_hdmi_state_helper.h
+++ b/include/drm/display/drm_hdmi_state_helper.h
@@ -7,6 +7,7 @@  struct drm_atomic_commit;
 struct drm_connector;
 struct drm_connector_state;
 struct drm_display_mode;
+struct drm_modeset_acquire_ctx;
 struct hdmi_audio_infoframe;
 
 enum drm_connector_status;
@@ -22,8 +23,9 @@  int drm_atomic_helper_connector_hdmi_update_audio_infoframe(struct drm_connector
 int drm_atomic_helper_connector_hdmi_clear_audio_infoframe(struct drm_connector *connector);
 int drm_atomic_helper_connector_hdmi_update_infoframes(struct drm_connector *connector,
 						       struct drm_atomic_commit *state);
-void drm_atomic_helper_connector_hdmi_hotplug(struct drm_connector *connector,
-					      enum drm_connector_status status);
+int drm_atomic_helper_connector_hdmi_hotplug(struct drm_connector *connector,
+					     struct drm_modeset_acquire_ctx *ctx,
+					     enum drm_connector_status status);
 void drm_atomic_helper_connector_hdmi_force(struct drm_connector *connector);
 
 enum drm_mode_status