Re: [PATCH v8 23/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 to consider:
- [Medium] drm/msm/dp: unlocked mst_active check in
  msm_dp_bridge_detect()
--

commit 485c09f4556b36ae4b303173f11055d3729894af
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>

drm/msm/dp: mark the SST bridge disconnected when mst is active

This commit returns disconnected from msm_dp_bridge_detect() for the SST
bridge when dp->mst_active is set.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index b4e788d1f611d..f87eea19c4f58 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -976,7 +976,11 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
>  
>  	priv = container_of(dp, struct msm_dp_display_private, msm_dp_display);
>  
> +	if (dp->mst_active)
> +		return status;
> +
>  	guard(mutex)(&priv->plugged_lock);

[Severity: Medium]
Should dp->mst_active be checked after acquiring priv->plugged_lock, as
msm_dp_irq_hpd_handle() does?

drm_bridge_connector_hpd_notify() runs without dev->mode_config.mutex, so
priv->plugged_lock is what serializes HPD plug handling against concurrent
connector probing:

CPU0                                  CPU1
msm_dp_hpd_plug_handle()
  mutex_lock(&dp->plugged_lock)
  msm_dp_display_process_hpd_high()
                                      msm_dp_bridge_detect()
                                        if (dp->mst_active) // false
                                        mutex_lock(&priv->plugged_lock)
    ...
    msm_dp_display_mst_enable()
      msm_dp->mst_active = true
  dp->plugged = true
  mutex_unlock(&dp->plugged_lock)
                                        pm_runtime_resume_and_get(...)
                                        ...
                                        priv->plugged = true
                                        // returns without pm_runtime_put_sync
                                        // <-- leaks runtime PM reference

Once msm_dp_bridge_detect() enters the SST detection path after MST has been
enabled, a successful DPCD read leaves an extra runtime PM reference that
msm_dp_hpd_unplug_handle() never drops, while a failed AUX transfer clears
priv->plugged and causes msm_dp_hpd_unplug_handle() to bail out without
disabling MST.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-msm-dp-mst-v8-0-d040fb33db5f@oss.qualcomm.com?part=23