Re: [PATCH v6 14/29] drm/msm/dp: replace power_on with active_stream_cnt

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Unconditional decrement of `active_stream_cnt` and unclocked register accesses on failure paths cause integer underflow and system crashes.
--

commit d23a7dcc5ef73b43d129d8cbcecd19ea680c25fc
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>
Subject: drm/msm/dp: replace power_on with active_stream_cnt

This commit replaces the boolean power_on state with an active_stream_cnt
tracker to better support DP MST configurations. Link initialization is
performed when the first stream is enabled, and link teardown is deferred
until the last stream is disabled.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 6f79328c8b790..9c5563dcb856e 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c

[ ... ]

> @@ -723,13 +719,12 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp,
>  {
>  	struct msm_dp *msm_dp_display = &dp->msm_dp_display;
>  
> -	if (!msm_dp_display->power_on)
> -		return 0;
> -
>  	msm_dp_panel_disable_vsc_sdp(msm_dp_panel);
>  
>  	msm_dp_ctrl_off_pixel_clk(dp->ctrl, msm_dp_panel->stream_id);
>  
> +	msm_dp_display->active_stream_cnt--;

[Severity: Critical]
If msm_dp_display_prepare_link() fails early (such as when sink_count == 0),
the DP link clocks remain off and the stream is not initialized. Because DRM
core atomic enable callbacks return void, the teardown path will still execute.

Will removing this check cause a regression by unconditionally executing
hardware accesses and decrementing the stream count?

If msm_dp_display_disable() is called when the stream failed to enable, it
results in:

msm_dp_display_disable()
  -> msm_dp_panel_disable_vsc_sdp()
    -> msm_dp_read_link(panel, MMSS_DP_SDP_CFG)

Reading this register while link clocks are disabled triggers a synchronous
external abort (kernel panic).

Furthermore, does this allow active_stream_cnt to underflow? If it wraps to
UINT_MAX, the subsequent call to msm_dp_display_unprepare() would evaluate
active_stream_cnt as true and bypass the PHY teardown and
pm_runtime_put_sync() steps.

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

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-msm-dp-mst-v6-0-c91d35d6fb9e@oss.qualcomm.com?part=14