Re: [PATCH v4] drm/i915/display: Enable VRR/HDR over MST

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Bypassing the DP branch device check for MST streams enables VRR and AS-SDP on MST leaf ports that are PCONs, which is explicitly unsupported.
- [Low] Triply duplicated comment block explaining the reuse of the vrr_capable_property.
--

commit 0a351efb0934b17f15e6de16fd0583d3287db2d9
Author: Xiao Lu <xiaolu.xie@intel.com>
Subject: drm/i915/display: Enable VRR/HDR over MST

This commit enables Variable Refresh Rate (VRR) and HDR metadata transmission
over DisplayPort Multi-Stream Transport (MST). It introduces caching of the MSA
timing ignore capability to avoid hot path sideband reads. It also reuses SST
properties for MST streams and lifts previous pipe_bpp caps.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 113d767e62e9c..4b196d26d27f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3107,9 +3110,12 @@ static bool intel_dp_needs_as_sdp(struct intel_dp *intel_dp,
>  		return false;
>  
>  	/*
> -	 * #TODO: Add AS SDP v1 support for PCONs (DP branch devices).
> +	 * AS-SDP is not yet supported for PCONs (DP-to-HDMI converters).
> +	 * MST hubs are also branch devices but do support AS-SDP forwarding
> +	 * to downstream sinks; allow it for MST streams.
>  	 */
> -	if (drm_dp_is_branch(intel_dp->dpcd))
> +	if (drm_dp_is_branch(intel_dp->dpcd) &&
> +	    !intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
>  		return false;

[Severity: Medium]
Does this explicit exemption of MST streams enable AS-SDP packets even if the
downstream MST leaf port is an unsupported PCON?

Because MST streams are exempted from the branch device block here, AS-SDP
packets will be sent to MST streams even when the leaf port is a PCON that
doesn't fully support it.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 3be1643f8d030..021d86b0ed833 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1745,6 +1799,34 @@ static int mst_topology_add_connector_properties(struct intel_dp *intel_dp,
>  	if (connector->base.max_bpc_property)
>  		drm_connector_attach_max_bpc_property(&connector->base, 6, 12);
>  
> +	/*
> +	 * Reuse the vrr_capable prop from the eDP connector (which is always
> +	 * initialized during driver load before device registration), because
> +	 * we're not allowed to create new props after device registration.
> +	 */
> +	/*
> +	 * Reuse the vrr_capable property from the root intel_dp SST connector
> +	 * which was created during driver load (before device registration).
> +	 * MST connectors are created after device registration, so we cannot
> +	 * create new properties here - we must attach the existing one.
> +	 */
> +	/*
> +	 * Reuse the vrr_capable property from the root intel_dp SST connector
> +	 * which was created during driver load (before device registration).
> +	 * MST connectors are created after device registration, so we cannot
> +	 * create new properties here - we must attach the existing one.
> +	 * Also assign connector->vrr_capable_property so that
> +	 * drm_connector_set_vrr_capable_property() can update the value.
> +	 */

[Severity: Low]
This isn't a bug, but it looks like the comment block explaining the reuse of
the vrr_capable_property was duplicated three times.

> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 51e4f3309b8b1..9737a19c1899f 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -55,8 +55,19 @@ bool intel_vrr_is_capable(struct intel_connector *connector)
>  			return false;
>  		fallthrough;
>  	case DRM_MODE_CONNECTOR_DisplayPort:
> -		if (connector->mst.dp)
> -			return false;
> +		if (connector->mst.dp) {
> +			/*
> +			 * Use cached MSA timing ignore capability from the DFP
> +			 * sink's virtual DPCD, set during connector init in
> +			 * intel_dp_mst_read_decompression_port_dsc_caps().
> +			 * Avoid live sideband reads here as this function is
> +			 * called from the atomic check path.
> +			 */
> +			if (!connector->dp.mst_msa_timing_par_ignore)
> +				return false;
> +
> +			break;
> +		}

[Severity: Medium]
Will this early break for MST streams incorrectly bypass the branch device
check for PCONs?

By breaking out early for MST streams here, the function skips the subsequent
drm_dp_is_branch() check. This means MST leaf ports that are PCONs are
treated as direct VRR-capable sinks if they advertise MSA timing ignore in
their virtual DPCD, which is explicitly unsupported.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902032207.259156-1-xiaolu.xie@intel.com?part=1