Re: [PATCH v6] drm/bridge: dw-hdmi-qp: serialize HDMI audio callbacks with PHY enable/disable

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/bridge: dw-hdmi-qp: sleeping in atomic context via mutex in atomic callbacks
--

commit 1e5aaa063a714d3e9e667b95f0e3087ce7b8924d
Author: Frank Zhang <rmxpzlb@gmail.com>

drm/bridge: dw-hdmi-qp: serialize HDMI audio callbacks with PHY enable/disable

This commit introduces a mutex lock and PHY enabled state to serialize PHY
state transitions and HDMI audio callbacks, aiming to fix a kernel panic 
observed during system reboot when ALSA races with the DRM atomic framework.

Link: https://lore.kernel.org/all/20260908070220.41574-1-rmxpzlb@gmail.com/

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> index c9568aea98dc8..1ffe159e5a099 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c

[ ... ]

> @@ -547,6 +571,7 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
>  	struct drm_connector_state *conn_state;
>  	struct drm_connector *connector;
>  	unsigned int op_mode;
> +	int ret;
>  
>  	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
>  	if (WARN_ON(!connector))
> @@ -556,6 +581,7 @@ static void dw_hdmi_qp_bridge_atomic_enable(struct drm_bridge *bridge,
>  	if (WARN_ON(!conn_state))
>  		return;
>  
> +	mutex_lock(&hdmi->phy.lock);

[Severity: High]
Does this code acquire a sleeping lock in an atomic context?

According to the DRM subsystem rules, bridge atomic enable/disable callbacks
execute in atomic context during non-blocking commits where sleeping is
strictly forbidden. Acquiring a mutex here will cause a might_sleep()
warning, scheduler errors, and possible system deadlocks when a userspace
application or compositor issues a non-blocking atomic modeset.

>  	if (connector->display_info.is_hdmi) {
>  		dev_dbg(hdmi->dev, "%s mode=HDMI %s rate=%llu bpc=%u\n", __func__,
>  			drm_hdmi_connector_get_output_format_name(conn_state->hdmi.output_format),

[ ... ]

> @@ -580,9 +615,10 @@ static void dw_hdmi_qp_bridge_atomic_disable(struct drm_bridge *bridge,
>  {
>  	struct dw_hdmi_qp *hdmi = bridge->driver_private;
>  
> +	guard(mutex)(&hdmi->phy.lock);

[Severity: High]
Can this sequence cause a panic during atomic commits?

Similar to the enable path, dw_hdmi_qp_bridge_atomic_disable() executes in
atomic context. Using guard(mutex) acquires a sleeping lock while atomic,
which violates the DRM atomic context requirements.

>  	hdmi->tmds_char_rate = 0;
> -
>  	hdmi->phy.ops->disable(hdmi, hdmi->phy.data);
> +	hdmi->phy.enabled = false;
>  }

[ ... ]

> @@ -780,7 +825,11 @@ static int dw_hdmi_qp_bridge_write_audio_infoframe(struct drm_bridge *bridge,
>  {
>  	struct dw_hdmi_qp *hdmi = bridge->driver_private;
>  
> -	dw_hdmi_qp_bridge_clear_audio_infoframe(bridge);
> +	guard(mutex)(&hdmi->phy.lock);

[Severity: High]
Is this lock safe to acquire here?

dw_hdmi_qp_bridge_write_audio_infoframe() is called downstream from the
atomic enable path via drm_atomic_helper_connector_hdmi_update_infoframes().
Since it is reachable from the non-blocking atomic commit chain, acquiring a
sleeping mutex here can also lead to system deadlocks.

> +	if (!hdmi->phy.enabled)
> +		return -EOPNOTSUPP;
> +
> +	dw_hdmi_qp_clear_audio_infoframe_regs(hdmi);
>  
>  	/*
>  	 * AUDI_CONTENTS0: { RSV, HB2, HB1, RSV }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915103433.13163-1-rmxpzlb@gmail.com?part=1