On Tue, Jul 07, 2026 at 06:05:53PM +0000, [email protected] 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 <[email protected]> > > 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/[email protected]?part=1 -- With best wishes Dmitry
