Re: [PATCH v2 3/7] drm/msm/hdmi_bridge: Correct poweroff/audio cleanup order in post_disable

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Moving `hdmi->power_on = false` to after `msm_hdmi_audio_update()` breaks the audio hardware disablement logic, leaving the audio engine active before its power is cut.

Pre-existing issues:
- [High] Unclocked MMIO accesses in ALSA audio callbacks can cause a kernel panic if the DRM bridge is powered off.
--

commit b86cb24a12ce06704d3597339883527b57932c00
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

drm/msm/hdmi_bridge: Correct poweroff/audio cleanup order in post_disable

Turning off the power in msm_hdmi_bridge_atomic_post_disable() should
follow reverse order of powering up, not only because it is logical, but
also because msm_hdmi_audio_update() accesses registers thus block
should still be powered on.

Depending on specific timing, existing code could have accesses to MMIO
of powered off block.

Cc: <stable@vger.kernel.org>

> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index db85400e574e4..04c893062dbd4 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -339,10 +339,10 @@ static void msm_hdmi_bridge_atomic_post_disable(struct drm_bridge *bridge,
>  	msm_hdmi_phy_powerdown(phy);
>  
>  	if (hdmi->power_on) {
> -		power_off(bridge);
> -		hdmi->power_on = false;
>  		if (hdmi->connector->display_info.is_hdmi)
>  			msm_hdmi_audio_update(hdmi);

[Severity: High]
This isn't a bug introduced by this patch, but can unclocked MMIO accesses in
ALSA audio callbacks cause a kernel panic if the DRM bridge is powered off?

If a user-space ALSA application starts or stops audio playback while the
display is powered off (e.g., via DPMS blanking), the ALSA callbacks
msm_hdmi_bridge_audio_prepare() and msm_hdmi_bridge_audio_shutdown() in
drivers/gpu/drm/msm/hdmi/hdmi_audio.c call msm_hdmi_audio_update().

This function reads from MMIO registers unconditionally, without verifying if
the HDMI block is powered on or holding a PM runtime reference. Could this lead
to a synchronous external abort (kernel panic) due to unclocked MMIO access?

> +		power_off(bridge);
> +		hdmi->power_on = false;

[Severity: Medium]
Does moving hdmi->power_on = false to after msm_hdmi_audio_update() break the
audio hardware disablement logic?

The audio shutdown logic in msm_hdmi_audio_update() relies on hdmi->power_on
being false during teardown to correctly disable audio:

drivers/gpu/drm/msm/hdmi/hdmi_audio.c:msm_hdmi_audio_update() {
	...
	if (enabled && !(hdmi->power_on && hdmi->pixclock)) {
		DBG("disabling audio: no video");
		enabled = false;
	}
	...
}

By setting hdmi->power_on = false after the call, msm_hdmi_audio_update() still
evaluates hdmi->power_on as true. Will this leave the audio engine active and
programmed to remain enabled right before its power and clocks are cut,
potentially leading to transient audio glitches or incomplete hardware state
reset?

>  		msm_hdmi_phy_resource_disable(phy);
>  	}
>  	mutex_unlock(&hdmi->state_mutex);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-drm-msm-hdmi-fixes-before-eliza-v2-0-b097cf1eac77@oss.qualcomm.com?part=3