Re: [PATCH v6 28/29] drm/msm/dp: mark the SST bridge disconnected when an MST-capable sink is present

[sashiko-bot@kernel.org]

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

New issues:
- [High] Implicit boolean evaluation of drm_dp_read_mst_cap() enum return breaks SST sinks with sideband messaging capability.
- [High] Returning disconnected for MST sinks at boot permanently prevents the MST topology manager from initializing.

Pre-existing issues:
- [High] Unbalanced PM runtime usage count leak on repeated detect calls when the device is plugged in.
--

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

drm/msm/dp: mark the SST bridge disconnected when an MST-capable sink is present

When a DP cable is already attached at boot, msm_dp_bridge_detect()
runs from the connector notify path before MST has been activated on
the sink. This commit forces a disconnected status for SST bridges if
the sink supports MST.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index dbc0e82d333a5..d2b2c7141db0f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1052,6 +1052,10 @@ enum drm_connector_status msm_dp_bridge_detect(struct drm_bridge *bridge,
>  			status = connector_status_disconnected;
>  	}
>  
> +	/* Mark SST connector as disconnected in DP boot-up case*/
> +	if (msm_dp_ctrl_get_stream_cnt(priv->ctrl) > 1 && drm_dp_read_mst_cap(priv->aux, dpcd))

[Severity: High]
Does evaluating drm_dp_read_mst_cap() as a boolean prevent SST sinks that
support single-stream sideband messaging from being detected?

drm_dp_read_mst_cap() returns an enum drm_dp_mst_mode. If a sink returns
DRM_DP_SST_SIDEBAND_MSG (which is 2), this condition evaluates to true,
taking the MST disconnection path for an SST sink. Could this cause these
displays to fail detection and remain blank?

[Severity: High]
Does returning connector_status_disconnected at boot prevent the MST
topology manager from initializing?

When msm_dp_bridge_detect() returns connector_status_disconnected for MST
sinks, the DRM core invokes the driver's .hpd_notify with disconnected.
The driver's msm_dp_bridge_hpd_notify() ignores disconnected events if it
thinks it is already disconnected, and skips calling
msm_dp_hpd_plug_handle().

Since msm_dp_hpd_plug_handle() is skipped, msm_dp_display_mst_enable() is
never reached and the MST topology manager is not initialized until a
physical replug generates an HPD interrupt. Could this result in MST
displays connected at boot remaining undetected?

> +		status = connector_status_disconnected;
> +
>  end:
>  	/*
>  	 * If we detected the DPRX, leave the controller on so that it doesn't

[Severity: High]
This isn't a bug introduced by this patch, but does this function leak PM
runtime usage counts on repeated detect calls when the device is plugged in?

A PM runtime reference is acquired unconditionally at the start of the
function:

    ret = pm_runtime_resume_and_get(&dp->pdev->dev);

However, if the link is connected, priv->plugged is set to true, which
bypasses the pm_runtime_put_sync() call at the end:

    if (!priv->plugged) {
        ...
        pm_runtime_put_sync(&dp->pdev->dev);
    }

If userspace continuously polls (e.g., via DRM_IOCTL_MODE_GETCONNECTOR)
while the device is plugged in, won't this repeatedly leak references,
eventually preventing suspend? Unplugging would only drop one reference.

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