[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

[Dmitry Baryshkov]

On Tue, Jul 07, 2026 at 06:05:53PM +0000, sashiko-bot@kernel.org wrote:
> 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.
> --

Could you please comment on this Sashiko comment?

> 
> --- 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

-- 
With best wishes
Dmitry

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

[최유호]

On Fri, 31 Jul 2026 at 10:05, Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Tue, Jul 07, 2026 at 06:05:53PM +0000, sashiko-bot@kernel.org wrote:
> > 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.
> > --
>
> Could you please comment on this Sashiko comment?

Thanks, agreed. The posted patch disables the pixel clock before
reverting the controller from SEND_VIDEO, and the bridge disable path
may later issue PUSH_IDLE with the clock disabled. I will send the
revised patch addressing the comment.
I appreciate your time for the review.

Thanks,
Yuho