Re: [PATCH] drm/msm/dp: hold one runtime PM reference per plugged state

[sashiko-bot@kernel.org]

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 <26rote@gmail.com>

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/20260929130627.959-1-26rote@gmail.com?part=1