Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm/dp: continuous underflow and SoC reset on post_enable failure --
commit 71fa7431b57f5bf43596e31ff0dea94064bcb0c2 Author: Abhinav Kumar <[email protected]> drm/msm/dp: move link-level teardown from display_disable to display_unprepare This commit moves link-level teardown operations from msm_dp_display_disable() to msm_dp_display_unprepare() to handle DP MST where links are shared across multiple streams. It leaves only stream-level shutdown inside the disable function. > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index a420e3d499281..03ccdf7e03666 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -722,20 +722,6 @@ static int msm_dp_display_disable(struct > msm_dp_display_private *dp, > > msm_dp_ctrl_off_pixel_clk(dp->ctrl); > > - /* dongle is still connected but sinks are disconnected */ > - if (dp->link->sink_count == 0) > - msm_dp_link_psm_config(dp->link, &msm_dp_panel->link_info, > true); > - > - msm_dp_ctrl_off_link(dp->ctrl, msm_dp_panel); > - > - if (dp->link->sink_count == 0) > - /* re-init the PHY so that we can listen to Dongle disconnect */ > - msm_dp_ctrl_reinit_phy(dp->ctrl); > - else > - msm_dp_display_host_phy_exit(dp); > - > - msm_dp_display->power_on = false; > - [Severity: High] If msm_dp_display_post_enable() fails inside msm_dp_display_atomic_enable(), the error path calls msm_dp_display_disable(). Since this patch removes the mainlink teardown and power_on = false from msm_dp_display_disable(), the function now only disables the pixel clock while leaving the mainlink active and power_on set to true. Can this lead to a continuous underflow state and an SoC reset on failure? Because power_on remains true, the subsequent call to msm_dp_display_atomic_disable() will attempt to execute msm_dp_ctrl_push_idle(). With the pixel clock already off, this appears to time out and crash the SoC (such as on Snapdragon X2 Elite). Additionally, looking forward to later in the series where the power_on flag is replaced with active_stream_cnt, does the DRM framework's eventual call to atomic_post_disable() call msm_dp_display_disable() a second time? If so, could this cause active_stream_cnt to underflow and wrap to UINT_MAX, permanently preventing hardware teardown in msm_dp_display_unprepare()? > drm_dbg_dp(dp->drm_dev, "sink count: %d\n", dp->link->sink_count); > return 0; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
