From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CE480C61DFD for ; Wed, 2 Sep 2026 08:37:38 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 33D6910E49B; Wed, 2 Sep 2026 08:37:38 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="K5utWWzX"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id D71CD10E49B for ; Wed, 2 Sep 2026 08:37:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1788338257; x=1819874257; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=+nBrU+TKfEULvYdv5uf7T2kfB7LXVZTZnrhh4ljjkc8=; b=K5utWWzXyJPOPh6WCkAHXXdXsKcXM1p1AEnVSYEWnuB0y2oEjJQLFXew QnnA+JmoKy0q5hArRBb13nSouhOyAEO5eeS+HNLgn3ALIKVqwbP4YUxss jkcbxEr5bRoZa6m8tnUzVN+L8AikvUCdYN6awBhNWr7wJcAV+QAhAhHxu tNfRfd+tzH85RisBUuvelWJ1T+mKN+VE/2aKT37ALhsmPkD5HIDOpuQ+/ 5g/2Y3rchaZxZJ2d98bh6d+BeFU/6D8QPLB3k1zIFfcJBXspYZnAWmups ZGpCTLNz5xax/tHeHhhxDdnGVi5EYYak9lQJyEcfeVHS5oMCCQaemMN1w g==; X-CSE-ConnectionGUID: iD+ILpn9ScKg8KXH9ddVcw== X-CSE-MsgGUID: 4iwFYWSuTym+nNZvzHUxCg== X-IronPort-AV: E=McAfee;i="6800,10657,11893"; a="88713836" X-IronPort-AV: E=Sophos;i="6.25,257,1779174000"; d="scan'208";a="88713836" Received: from fmviesa008.fm.intel.com ([10.60.135.148]) by orvoesa111.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Sep 2026 01:37:36 -0700 X-CSE-ConnectionGUID: /Uk94Ge+QlmANE1fuh1hcA== X-CSE-MsgGUID: RoEhtvSnTYWmL0hCSPINMg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.25,257,1779174000"; d="scan'208";a="266748597" Received: from hrotuna-mobl2.ger.corp.intel.com (HELO localhost) ([10.245.245.84]) by fmviesa008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Sep 2026 01:37:34 -0700 From: Jani Nikula To: Xiao Lu , intel-gfx@lists.freedesktop.org Cc: ville.syrjala@linux.intel.com, imre.deak@intel.com, Xiao Lu Subject: Re: [PATCH v6] drm/i915/display: Enable VRR/HDR over MST In-Reply-To: <20260902051206.264215-1-xiaolu.xie@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs Bertel Jungin Aukio 5, 02600 Espoo, Finland References: <20260901132601.239131-1-xiaolu.xie@intel.com> <20260902051206.264215-1-xiaolu.xie@intel.com> Date: Wed, 02 Sep 2026 11:37:32 +0300 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Wed, 02 Sep 2026, Xiao Lu 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 > --- > 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