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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 02 Sep 2026, Xiao Lu <xiaolu.xie@intel.com> wrote:
> VRR over MST:
> - Remove the hard MST block in intel_vrr_is_capable(); check the
>   downstream port's MSA timing par ignore capability via REMOTE_DPCD_READ
>   and cache it in connector->dp.mst_msa_timing_par_ignore to avoid hot
>   path sideband round-trips.
> - Read mst_msa_timing_par_ignore unconditionally from the sink's virtual
>   DPCD before the DSC decompression check, so VRR works on MST sinks
>   without DSC support.
> - Attach vrr_capable property by reusing the root SST connector's existing
>   property object (MST connectors are created post device-registration so
>   new properties cannot be created).
> - Update vrr_capable after EDID read; gate AS-SDP and
>   DP_MSA_TIMING_PAR_IGNORE_EN writes on pipe_config->vrr.enable
>   (actual state) not intel_vrr_is_capable() (capability); allow AS-SDP
>   through MST hubs by excluding only non-MST branch devices (PCONs)
>   from the branch device check.
> - Reuse colorspace property from root SST connector for MST connectors
>   to avoid WARN_ON from creating properties post device-registration.
>
> HDR metadata over MST:
> - Add VSC SDP, HDR metadata infoframe SDP and AS-SDP computation to
>   mst_stream_compute_config() and enable their transmission in
>   mst_stream_enable() and intel_ddi_update_pipe().
> - Lift the hard 8bpc pipe_bpp cap and use the bpp capability from sink
>   EDID for MST streams to allow 10bpc, relying on
>   intel_dp_mst_atomic_check_link()'s BW retry loop to fall back if the
>   link cannot fit it.

What changed between versions?

Please don't send a bunch of versions of a patch in quick succession.

Truth is, nobody's going to spend time looking at it, because they think
it's in a state of flux and it'll be wasted effort.


BR,
Jani.

>
> Signed-off-by: Xiao Lu <xiaolu.xie@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_mst_topology.c |  2 +
>  drivers/gpu/drm/i915/display/intel_ddi.c      |  3 +
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 36 ++++---
>  drivers/gpu/drm/i915/display/intel_dp.h       |  8 ++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   | 95 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vrr.c      | 15 ++-
>  7 files changed, 143 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 7ce9e212770a..174fa62cb0dc 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -2164,6 +2164,7 @@ ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
>  	return drm_dp_send_dpcd_read(port->mgr, port,
>  				     offset, size, buffer);
>  }
> +EXPORT_SYMBOL(drm_dp_mst_dpcd_read);
>  
>  /**
>   * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via sideband
> @@ -2187,6 +2188,7 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
>  	return drm_dp_send_dpcd_write(port->mgr, port,
>  				      offset, size, buffer);
>  }
> +EXPORT_SYMBOL(drm_dp_mst_dpcd_write);
>  
>  static int drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, guid_t *guid)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index b8985e1e0a81..f345d9a2154d 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3651,6 +3651,9 @@ void intel_ddi_update_pipe(struct intel_atomic_state *state,
>  		intel_ddi_update_pipe_dp(state, encoder, crtc_state,
>  					 conn_state);
>  
> +	if (intel_encoder_is_mst(encoder))
> +		intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
> +
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>  		intel_ddi_update_pipe_hdmi(encoder, crtc_state,
>  					   conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index b7cc361fd955..d09cf18f5376 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -564,6 +564,7 @@ struct intel_connector {
>  		u8 dsc_hblank_expansion_quirk:1;
>  		u8 dsc_throughput_quirk:1;
>  		u8 dsc_decompression_enabled:1;
> +		u8 mst_msa_timing_par_ignore:1; /* cached from MST virtual DPCD 0x007 */
>  
>  		struct {
>  			struct {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 113d767e62e9..4b196d26d27f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2682,15 +2682,18 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
>  
>  	limits->pipe.min_bpp = intel_dp_min_bpp(crtc_state->output_format);
>  	if (is_mst) {
> +		struct intel_connector *mst_connector =
> +			to_intel_connector(conn_state->connector);
> +		int sink_bpc = mst_connector->base.display_info.bpc ?: 8;
> +
>  		/*
> -		 * FIXME: If all the streams can't fit into the link with their
> -		 * current pipe_bpp we should reduce pipe_bpp across the board
> -		 * until things start to fit. Until then we limit to <= 8bpc
> -		 * since that's what was hardcoded for all MST streams
> -		 * previously. This hack should be removed once we have the
> -		 * proper retry logic in place.
> +		 * For MST streams, use the sink's EDID-reported color depth
> +		 * as the max bpp, rather than the pipe_bpp from a previous
> +		 * modeset (which may be stale) or a hardcoded 8bpc cap.
> +		 * The BW retry loop in intel_dp_mst_atomic_check_link() will
> +		 * reduce bpp if the aggregate link bandwidth is insufficient.
>  		 */
> -		limits->pipe.max_bpp = min(crtc_state->max_pipe_bpp, 24);
> +		limits->pipe.max_bpp = sink_bpc * 3;
>  	} else {
>  		limits->pipe.max_bpp = intel_dp_max_bpp(intel_dp, crtc_state,
>  							respect_downstream_limits);
> @@ -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;
>  
>  	if (intel_psr_needs_alpm_aux_less(intel_dp, crtc_state) &&
> @@ -3119,8 +3125,8 @@ static bool intel_dp_needs_as_sdp(struct intel_dp *intel_dp,
>  	return intel_vrr_possible(crtc_state);
>  }
>  
> -static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
> -				    struct intel_crtc_state *crtc_state)
> +void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
> +			     struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_dp_as_sdp *as_sdp = &crtc_state->infoframes.as_sdp;
>  	const struct drm_display_mode *adjusted_mode =
> @@ -3169,9 +3175,9 @@ static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
>  		as_sdp->coasting_vtotal = crtc_state->vrr.vmax;
>  }
>  
> -static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
> -				     struct intel_crtc_state *crtc_state,
> -				     const struct drm_connector_state *conn_state)
> +void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
> +			      struct intel_crtc_state *crtc_state,
> +			      const struct drm_connector_state *conn_state)
>  {
>  	struct drm_dp_vsc_sdp *vsc;
>  
> @@ -3228,7 +3234,7 @@ intel_dp_in_hdr_mode(const struct drm_connector_state *conn_state)
>  	return hdr_metadata->hdmi_metadata_type1.eotf == HDMI_EOTF_SMPTE_ST2084;
>  }
>  
> -static void
> +void
>  intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
>  					    struct intel_crtc_state *crtc_state,
>  					    const struct drm_connector_state *conn_state)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 04fae203bb69..7887ba63c7a4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -77,6 +77,8 @@ int intel_dp_compute_config(struct intel_atomic_state *state,
>  			    struct intel_encoder *encoder,
>  			    struct intel_crtc_state *pipe_config,
>  			    struct drm_connector_state *conn_state);
> +void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
> +			     struct intel_crtc_state *crtc_state);
>  bool intel_dp_needs_8b10b_fec(const struct intel_crtc_state *crtc_state,
>  			      bool dsc_enabled_on_crtc);
>  void intel_dp_dsc_reset_config(struct intel_crtc_state *crtc_state);
> @@ -130,6 +132,12 @@ bool intel_dp_joiner_needs_dsc(struct intel_display *display,
>  bool intel_dp_has_joiner(struct intel_dp *intel_dp);
>  bool intel_dp_needs_vsc_colorimetry(const struct intel_crtc_state *crtc_state,
>  				    const struct drm_connector_state *conn_state);
> +void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
> +			      struct intel_crtc_state *crtc_state,
> +			      const struct drm_connector_state *conn_state);
> +void intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
> +						 struct intel_crtc_state *crtc_state,
> +						 const struct drm_connector_state *conn_state);
>  void intel_dp_set_infoframes(struct intel_encoder *encoder, bool enable,
>  			     const struct intel_crtc_state *crtc_state,
>  			     const struct drm_connector_state *conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 3be1643f8d03..cbef63e94e2c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -793,6 +793,9 @@ static int mst_stream_compute_config(struct intel_atomic_state *state,
>  			bxt_dpio_phy_calc_lane_lat_optim_mask(pipe_config->lane_count);
>  
>  	intel_vrr_compute_config(pipe_config, conn_state);
> +	intel_dp_compute_as_sdp(intel_dp, pipe_config);
> +	intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> +	intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state);
>  
>  	intel_dp_audio_compute_config(encoder, pipe_config, conn_state);
>  
> @@ -1157,6 +1160,24 @@ static void mst_stream_post_disable(struct intel_atomic_state *state,
>  	 * Power down mst path before disabling the port, otherwise we end
>  	 * up getting interrupts from the sink upon detecting link loss.
>  	 */
> +	if (old_crtc_state->vrr.enable) {
> +		u8 val;
> +		ssize_t ret;
> +
> +		ret = drm_dp_mst_dpcd_read(&connector->mst.port->aux,
> +					   DP_DOWNSPREAD_CTRL, &val, 1);
> +		if (ret < 0) {
> +			drm_dbg_kms(display->drm,
> +				    "[CONNECTOR:%d:%s] Failed to read DP_DOWNSPREAD_CTRL: %zd\n",
> +				    connector->base.base.id,
> +				    connector->base.name, ret);
> +		} else {
> +			val &= ~DP_MSA_TIMING_PAR_IGNORE_EN;
> +			drm_dp_mst_dpcd_write(&connector->mst.port->aux,
> +					      DP_DOWNSPREAD_CTRL, &val, 1);
> +		}
> +	}
> +
>  	drm_dp_send_power_updown_phy(&intel_dp->mst.mgr, connector->mst.port,
>  				     false);
>  
> @@ -1274,6 +1295,24 @@ static void mst_stream_pre_enable(struct intel_atomic_state *state,
>  
>  	drm_dp_send_power_updown_phy(&intel_dp->mst.mgr, connector->mst.port, true);
>  
> +	if (pipe_config->vrr.enable) {
> +		u8 val;
> +		ssize_t ret;
> +
> +		ret = drm_dp_mst_dpcd_read(&connector->mst.port->aux,
> +					   DP_DOWNSPREAD_CTRL, &val, 1);
> +		if (ret < 0) {
> +			drm_dbg_kms(display->drm,
> +				    "[CONNECTOR:%d:%s] Failed to read DP_DOWNSPREAD_CTRL: %zd\n",
> +				    connector->base.base.id,
> +				    connector->base.name, ret);
> +		} else {
> +			val |= DP_MSA_TIMING_PAR_IGNORE_EN;
> +			drm_dp_mst_dpcd_write(&connector->mst.port->aux,
> +					      DP_DOWNSPREAD_CTRL, &val, 1);
> +		}
> +	}
> +
>  	intel_dp_sink_enable_decompression(state, connector, pipe_config);
>  
>  	if (first_mst_stream) {
> @@ -1397,6 +1436,8 @@ static void mst_stream_enable(struct intel_atomic_state *state,
>  
>  	intel_enable_transcoder(pipe_config);
>  
> +	intel_dp_set_infoframes(encoder, true, pipe_config, conn_state);
> +
>  	for_each_pipe_crtc_modeset_enable(display, pipe_crtc, pipe_config) {
>  		const struct intel_crtc_state *pipe_crtc_state =
>  			intel_atomic_get_new_crtc_state(state, pipe_crtc);
> @@ -1451,6 +1492,15 @@ static int mst_connector_get_ddc_modes(struct drm_connector *_connector)
>  
>  	ret = intel_connector_update_modes(&connector->base, drm_edid);
>  
> +	if (HAS_VRR(display)) {
> +		bool vrr_capable = intel_vrr_is_capable(connector);
> +
> +		drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
> +			    connector->base.base.id, connector->base.name,
> +			    str_yes_no(vrr_capable));
> +		drm_connector_set_vrr_capable_property(&connector->base, vrr_capable);
> +	}
> +
>  	drm_edid_free(drm_edid);
>  
>  	if (intel_dp_tunnel_uhbr_lanes_wa_setup(intel_dp)) {
> @@ -1735,6 +1785,22 @@ static int mst_topology_add_connector_properties(struct intel_dp *intel_dp,
>  
>  	intel_attach_force_audio_property(&connector->base);
>  	intel_attach_broadcast_rgb_property(&connector->base);
> +	/*
> +	 * Reuse the colorspace property from the root SST connector.
> +	 * MST connectors are created post device-registration so new
> +	 * properties cannot be created here.
> +	 */
> +	if (intel_dp->attached_connector &&
> +	    intel_dp->attached_connector->base.colorspace_property) {
> +		connector->base.colorspace_property =
> +			intel_dp->attached_connector->base.colorspace_property;
> +		drm_object_attach_property(&connector->base.base,
> +					   connector->base.colorspace_property,
> +					   DRM_MODE_COLORIMETRY_DEFAULT);
> +	}
> +
> +	if (intel_dp_has_gamut_metadata_dip(&dp_to_dig_port(intel_dp)->base))
> +		drm_connector_attach_hdr_output_metadata_property(&connector->base);
>  
>  	/*
>  	 * Reuse the prop from the SST connector because we're
> @@ -1745,6 +1811,23 @@ 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 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.
> +	 */
> +	if (HAS_VRR(display) && intel_dp->attached_connector &&
> +	    intel_dp->attached_connector->base.vrr_capable_property) {
> +		connector->base.vrr_capable_property =
> +			intel_dp->attached_connector->base.vrr_capable_property;
> +		drm_object_attach_property(&connector->base.base,
> +					   connector->base.vrr_capable_property,
> +					   0);
> +	}
> +
>  	return drm_connector_set_path_property(&connector->base, pathprop);
>  }
>  
> @@ -1755,6 +1838,18 @@ intel_dp_mst_read_decompression_port_dsc_caps(struct intel_dp *intel_dp,
>  	u8 dpcd_caps[DP_RECEIVER_CAP_SIZE];
>  	struct drm_dp_desc desc;
>  
> +	/*
> +	 * Read MSA timing par ignore capability from the sink's virtual DPCD.
> +	 * This is independent of DSC support so read it before the DSC check.
> +	 */
> +	{
> +		u8 mst_dpcd[DP_RECEIVER_CAP_SIZE];
> +
> +		if (drm_dp_read_dpcd_caps(&connector->mst.port->aux, mst_dpcd) == 0)
> +			connector->dp.mst_msa_timing_par_ignore =
> +				drm_dp_sink_can_do_video_without_timing_msa(mst_dpcd);
> +	}
> +
>  	if (!connector->dp.dsc_decompression_aux)
>  		return;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 51e4f3309b8b..9737a19c1899 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;
> +		}
>  		intel_dp = intel_attached_dp(connector);
>  		/*
>  		 * Among non-MST DP branch devices, only an HDMI 2.1 sink connected

-- 
Jani Nikula, Intel