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/[email protected] Link: https://sashiko.dev/#/patchset/20260513-dp-connector-hpd-v2-0-42f757bfcbf9%40oss.qualcomm.com Signed-off-by: Dmitry Baryshkov <[email protected]> --- 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
