Hi, On Thu, May 28, 2026 at 10:10:48AM +0300, Dmitry Baryshkov wrote: > 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]> > ---
Reviewed-by: Sebastian Reichel <[email protected]> -- Sebastian > 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 >
signature.asc
Description: PGP signature
