Re: [PATCH v6 26/29] drm/msm/dp: mark the SST bridge disconnected when mst is active

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] PM runtime reference leak in msm_dp_bridge_detect when called repeatedly while the display is already connected.
--

commit 17d7f7fedae42b8cc437cd2008aeec3429a04725
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>
Subject: drm/msm/dp: mark the SST bridge disconnected when mst is active

The bridge detect function is only applicable for SST. In MST mode,
connector detection is handled by MST bridges. Skip detection for the
SST bridge when MST is active, matching the behavior of other DP MST
implementations.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 22b9252774620..f350c1d03b06d 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -959,6 +959,10 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
>  	priv = container_of(dp, struct msm_dp_display_private, msm_dp_display);
>  
>  	guard(mutex)(&priv->plugged_lock);
> +
> +	if (dp->mst_active)
> +		return status;
> +
>  	ret = pm_runtime_resume_and_get(&dp->pdev->dev);

[Severity: High]
This is a pre-existing issue, but is it possible that this unconditionally
acquired PM reference leaks when the display is already connected?

If userspace repeatedly polls the connector while it is already connected,
msm_dp_bridge_detect() takes a PM reference here without checking if the
driver already holds one.

>  	if (ret) {
>  		DRM_ERROR("failed to pm_runtime_resume\n");

[ ... ]

>  	status = connector_status_connected;
>  	priv->plugged = true;
>  
>  	if (drm_dp_read_sink_count_cap(connector, dpcd, &desc)) {
>  		int sink_count = drm_dp_read_sink_count(priv->aux);
>  
>  		drm_dbg_dp(dp->drm_dev, "sink_count = %d\n", sink_count);
>  
>  		if (sink_count <= 0)
>  			status = connector_status_disconnected;
>  	}
>  
>  end:
>  	/*
>  	 * If we detected the DPRX, leave the controller on so that it doesn't
>  	 * lose the state.
>  	 */
>  	if (!priv->plugged) {
>  		if (phy_deinit) {
>  			msm_dp_aux_enable_xfers(priv->aux, false);
>  			msm_dp_display_host_phy_exit(priv);
>  		}
>  
>  		pm_runtime_put_sync(&dp->pdev->dev);
>  	}

Because pm_runtime_put_sync() is only called when priv->plugged is false,
does this mean that if priv->plugged was already true, the PM runtime usage
counter will increment on every poll and never be released?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-msm-dp-mst-v6-0-c91d35d6fb9e@oss.qualcomm.com?part=26