Re: [PATCH v7 01/35] drm/msm/dp: move link-level teardown from display_disable to display_unprepare

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/msm/dp: continuous underflow and SoC reset on post_enable failure
--

commit 71fa7431b57f5bf43596e31ff0dea94064bcb0c2
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>

drm/msm/dp: move link-level teardown from display_disable to display_unprepare

This commit moves link-level teardown operations from msm_dp_display_disable()
to msm_dp_display_unprepare() to handle DP MST where links are shared across
multiple streams. It leaves only stream-level shutdown inside the disable
function.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index a420e3d499281..03ccdf7e03666 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -722,20 +722,6 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp,
>  
>  	msm_dp_ctrl_off_pixel_clk(dp->ctrl);
>  
> -	/* dongle is still connected but sinks are disconnected */
> -	if (dp->link->sink_count == 0)
> -		msm_dp_link_psm_config(dp->link, &msm_dp_panel->link_info, true);
> -
> -	msm_dp_ctrl_off_link(dp->ctrl, msm_dp_panel);
> -
> -	if (dp->link->sink_count == 0)
> -		/* re-init the PHY so that we can listen to Dongle disconnect */
> -		msm_dp_ctrl_reinit_phy(dp->ctrl);
> -	else
> -		msm_dp_display_host_phy_exit(dp);
> -
> -	msm_dp_display->power_on = false;
> -

[Severity: High]
If msm_dp_display_post_enable() fails inside msm_dp_display_atomic_enable(),
the error path calls msm_dp_display_disable().

Since this patch removes the mainlink teardown and power_on = false from
msm_dp_display_disable(), the function now only disables the pixel clock
while leaving the mainlink active and power_on set to true.

Can this lead to a continuous underflow state and an SoC reset on failure?

Because power_on remains true, the subsequent call to
msm_dp_display_atomic_disable() will attempt to execute
msm_dp_ctrl_push_idle(). With the pixel clock already off, this appears to
time out and crash the SoC (such as on Snapdragon X2 Elite).

Additionally, looking forward to later in the series where the power_on flag
is replaced with active_stream_cnt, does the DRM framework's eventual call
to atomic_post_disable() call msm_dp_display_disable() a second time? If so,
could this cause active_stream_cnt to underflow and wrap to UINT_MAX,
permanently preventing hardware teardown in msm_dp_display_unprepare()?

>  	drm_dbg_dp(dp->drm_dev, "sink count: %d\n", dp->link->sink_count);
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-msm-dp-mst-v7-0-4b81473185e7@oss.qualcomm.com?part=1