[PATCH v3 1/3] drm/bridge: split hpd_mutex into two mutexes

[Dmitry Baryshkov]

Currently almost all bridge drivers which implement hpd_enable /
hpd_disable callbacks simply toggle the hardware registers generating
the interrupt. However, as pointed out by Jonas Karlman and Sashiko bot,
using those callbacks for enable_irq() / disable_irq() calls or
scheduling and cancelling the work can cause a AB-BA deadlock (between
hpd_mutex lock and the corresponding lock).

Split the hpd_mutex into two locks: one simply making sure that hpd_cb /
hpd_data are consistent and another one, hpd_state_mutex, making sure
that concurrent drm_bridge_hpd_enable() / drm_bridge_hpd_disable() calls
can't end up with inconsistency between hpd_cb/_data and bridge's
internal state.

Link: https://lore.kernel.org/dri-devel/9aa4bd35-bff6-4009-a959-ce31010c7b35@xxxxxxxxx
Link: https://sashiko.dev/#/patchset/20260513-dp-connector-hpd-v2-0-42f757bfcbf9%40oss.qualcomm.com
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
---
 drivers/gpu/drm/drm_bridge.c | 16 +++++++++++++---
 include/drm/drm_bridge.h     |  4 ++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 687b36eea0c7..9a185032a3bd 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -417,6 +417,7 @@ void drm_bridge_add(struct drm_bridge *bridge)
 	if (!list_empty(&bridge->list))
 		list_del_init(&bridge->list);
 
+	mutex_init(&bridge->hpd_state_mutex);
 	mutex_init(&bridge->hpd_mutex);
 
 	if (bridge->ops & DRM_BRIDGE_OP_HDMI)
@@ -469,6 +470,7 @@ void drm_bridge_remove(struct drm_bridge *bridge)
 	mutex_unlock(&bridge_lock);
 
 	mutex_destroy(&bridge->hpd_mutex);
+	mutex_destroy(&bridge->hpd_state_mutex);
 
 	drm_bridge_put(bridge);
 }
@@ -1451,19 +1453,25 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge,
 	if (!(bridge->ops & DRM_BRIDGE_OP_HPD))
 		return;
 
+	mutex_lock(&bridge->hpd_state_mutex);
+
 	mutex_lock(&bridge->hpd_mutex);
 
-	if (WARN(bridge->hpd_cb, "Hot plug detection already enabled\n"))
+	if (WARN(bridge->hpd_cb, "Hot plug detection already enabled\n")) {
+		mutex_unlock(&bridge->hpd_mutex);
 		goto unlock;
+	}
 
 	bridge->hpd_cb = cb;
 	bridge->hpd_data = data;
 
+	mutex_unlock(&bridge->hpd_mutex);
+
 	if (bridge->funcs->hpd_enable)
 		bridge->funcs->hpd_enable(bridge);
 
 unlock:
-	mutex_unlock(&bridge->hpd_mutex);
+	mutex_unlock(&bridge->hpd_state_mutex);
 }
 EXPORT_SYMBOL_GPL(drm_bridge_hpd_enable);
 
@@ -1484,13 +1492,15 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge)
 	if (!(bridge->ops & DRM_BRIDGE_OP_HPD))
 		return;
 
-	mutex_lock(&bridge->hpd_mutex);
+	mutex_lock(&bridge->hpd_state_mutex);
 	if (bridge->funcs->hpd_disable)
 		bridge->funcs->hpd_disable(bridge);
 
+	mutex_lock(&bridge->hpd_mutex);
 	bridge->hpd_cb = NULL;
 	bridge->hpd_data = NULL;
 	mutex_unlock(&bridge->hpd_mutex);
+	mutex_unlock(&bridge->hpd_state_mutex);
 }
 EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable);
 
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4ba3a5deef9a..00a95f927e34 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1256,6 +1256,10 @@ struct drm_bridge {
 	 * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields.
 	 */
 	struct mutex hpd_mutex;
+	/**
+	 * @hpd_state_mutex: Protects the HPD en/disablement state for the bridge.
+	 */
+	struct mutex hpd_state_mutex;
 	/**
 	 * @hpd_cb: Hot plug detection callback, registered with
 	 * drm_bridge_hpd_enable().

-- 
2.47.3