Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm/dp: dropping plugged PM ref in detect() breaks PHY init on replug --
commit c051f74425731ad4ec4c7067fd0c9ebc38bc16ab Author: Joonhoe Kim <[email protected]> drm/msm/dp: hold one runtime PM reference per plugged state This commit modifies the DP runtime PM handling to take exactly one reference for the plugged state, avoiding leaks where multiple HPD events or detect() calls accumulate references and prevent the controller from suspending. > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index 6abe30bdff120..4e47b659c71df 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c [ ... ] > @@ -960,6 +969,12 @@ enum drm_connector_status msm_dp_bridge_detect(struct > drm_bridge *bridge, > } > > pm_runtime_put_sync(&dp->pdev->dev); > + /* the sink is gone: drop the plugged state's reference too */ > + if (was_plugged) > + pm_runtime_put_sync(&dp->pdev->dev); [Severity: High] Does dropping the plugged state PM reference here allow the controller to suspend while the PHY software state still thinks it is initialized? If msm_dp_bridge_detect() fails during a physical unplug (for example, failing to read DPCD caps), it sets priv->plugged to false and jumps here to drop both its own PM reference and the plugged state's PM reference. The hardware unplug interrupt would then trigger msm_dp_hpd_unplug_handle(), which has an early return: drivers/gpu/drm/msm/dp/dp_display.c:msm_dp_hpd_unplug_handle() { ... guard(mutex)(&dp->plugged_lock); if (!dp->plugged) return 0; } Because detect() already cleared dp->plugged, this early return skips tearing down the PHY state with msm_dp_display_host_phy_exit(). Then, on the next plug, msm_dp_display_host_phy_init() will silently skip PHY initialization because dp->phy_initialized was never reset to false: drivers/gpu/drm/msm/dp/dp_display.c:msm_dp_display_host_phy_init() { ... if (!dp->phy_initialized) { msm_dp_ctrl_phy_init(dp->ctrl); dp->phy_initialized = true; return true; } return false; } Can this sequence leave the hardware PHY uninitialized on replug, causing link training to fail and the display to remain broken until reboot? > + } else if (was_plugged) { > + /* the plugged state already holds its reference */ > + pm_runtime_put_sync(&dp->pdev->dev); > } > > return status; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
