Re: [PATCH v2] 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: unplug teardown bypassed when detect() clears priv->plugged
--

commit 6677860d1d5222553e73900478155ec25c55e88b
Author: Joonhoe Kim <26rote@gmail.com>

drm/msm/dp: hold one runtime PM reference per plugged state

This commit adjusts PM runtime references in the DisplayPort driver to hold
exactly one reference per plugged state. It takes a reference on plug and
drops it when detect() notices the sink is gone or when the unplug handler
runs.

Link: https://lore.kernel.org/all/20260929130627.959-1-26rote@gmail.com/

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 6abe30bdff120..4059cb9dba64b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -910,6 +913,7 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
>  		return status;
>  	}
>  
> +	was_plugged = priv->plugged;
>  	phy_deinit = msm_dp_display_host_phy_init(priv);
>  
>  	msm_dp_aux_enable_xfers(priv->aux, true);
[ ... ]
> @@ -962,6 +966,9 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
>  		pm_runtime_put_sync(&dp->pdev->dev);
>  	}
>  
> +	if (was_plugged)
> +		pm_runtime_put_sync(&dp->pdev->dev);
> +
>  	return status;
>  }

[Severity: High]
Does this cement a hardware teardown bypass on unplug?

When a physical unplug occurs, msm_dp_bridge_detect() fails to read DPCD
caps and sets priv->plugged = false. By dropping the PM runtime reference
here using the newly added was_plugged flag, it completes the state
transition early.

The DRM core subsequently calls .hpd_notify() which invokes
msm_dp_hpd_unplug_handle(). Because the plugged flag was already cleared in
detect(), the unplug handler immediately early-returns:

msm_dp_hpd_unplug_handle() {
        guard(mutex)(&dp->plugged_lock);
        if (!dp->plugged)
                return 0;
        // Teardown code is skipped
        ...
}

This entirely bypasses drm_edid_connector_update(..., NULL),
drm_dp_set_subconnector_property(), and
msm_dp_display_handle_plugged_change(..., false). As a result, the EDID is
left stale, and the audio subsystem is never notified of the unplug (unless
an active audio stream was running).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007155848.106-1-26rote@gmail.com?part=1