Re: [PATCH v1] drm/msm/dp: Disable stream clock after video-ready timeout

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Disabling the pixel clock while the DP controller is actively in the SEND_VIDEO state violates the teardown sequence and can lock up the hardware.
--

--- Patch [1]: [PATCH v1] drm/msm/dp: Disable stream clock after video-ready timeout ---
commit d51f59f75198361a5116081d3f135fd586dbcb66
Author: Yuho Choi <dbgh9129@gmail.com>

drm/msm/dp: Disable stream clock after video-ready timeout

This commit routes the video-ready timeout path in msm_dp_ctrl_on_stream() 
through a cleanup path. It disables the stream pixel clock and clears the
stream_clks_on flag if the clock was enabled during the call.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 86ef8c89ad443..139e9fa25d6a7 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
[ ... ]
> @@ -2543,13 +2545,17 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
>  
>  	ret = msm_dp_ctrl_wait4video_ready(ctrl);
>  	if (ret)
> -		return ret;
> +		goto end;
>  
>  	mainlink_ready = msm_dp_ctrl_mainlink_ready(ctrl);
>  	drm_dbg_dp(ctrl->drm_dev,
>  		"mainlink %s\n", mainlink_ready ? "READY" : "NOT READY");
>  
>  end:
> +	if (stream_clk_enabled && ret) {
> +		clk_disable_unprepare(ctrl->pixel_clk);
> +		ctrl->stream_clks_on = false;
> +	}

[Severity: High]
Does this error path leave the DP controller in an active state without
reverting it before disabling the pixel clock?

Earlier in this function, the controller is programmed to send video:

	msm_dp_write_link(ctrl, REG_DP_STATE_CTRL, DP_STATE_CTRL_SEND_VIDEO);

	ret = msm_dp_ctrl_wait4video_ready(ctrl);

If wait4video_ready times out, the code jumps to the end label and disables
the pixel clock. However, if the caller ignores this failure and DRM later
disables the bridge, the teardown sequence will invoke msm_dp_ctrl_push_idle().

Since the controller was left in DP_STATE_CTRL_SEND_VIDEO but its pixel
clock is now disabled, could writing DP_STATE_CTRL_PUSH_IDLE cause a
timeout or wedge the hardware because it cannot process the state transition
without the clock?

>  	return ret;
>  }

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