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

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

[Yuho Choi]

msm_dp_ctrl_on_stream() enables the stream pixel clock before waiting for
video readiness. If that wait times out, it returns without disabling the
clock or clearing stream_clks_on.

Route the timeout path through cleanup and disable the clock only if this
call enabled it.

Fixes: 9bd0946d5ca1 ("drm/msm/dp: simplify stream clocks handling")
Signed-off-by: Yuho Choi <dbgh9129@gmail.com>
---
 drivers/gpu/drm/msm/dp/dp_ctrl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 86ef8c89ad44..139e9fa25d6a 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -2469,6 +2469,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
 {
 	int ret = 0;
 	bool mainlink_ready = false;
+	bool stream_clk_enabled = false;
 	struct msm_dp_ctrl_private *ctrl;
 	unsigned long pixel_rate;
 	unsigned long pixel_rate_orig;
@@ -2514,6 +2515,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train
 			goto end;
 		}
 		ctrl->stream_clks_on = true;
+		stream_clk_enabled = true;
 	}
 
 	if (force_link_train || !msm_dp_ctrl_channel_eq_ok(ctrl))
@@ -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;
+	}
 	return ret;
 }
 
-- 
2.43.0

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

[sashiko-bot]

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