RE: [PATCH v2] drm/i915/dp: Gate UHBR SST SDP splitting on sink capability

> Subject: [PATCH v2] drm/i915/dp: Gate UHBR SST SDP splitting on sink
> capability
> 
> SDP splitting for 128b/132b (UHBR) SST audio must only be enabled when the
> sink advertises support for it. Previously sdp_split_enable was set for every
> UHBR SST stream carrying audio, regardless of sink capability.
> 
> In MST mode SDP splitting is inherently supported, so the sink capability check
> (DP_SST_SPLIT_SDP_CAP) is applied only to the SST path.

Please add the versioning diff

> 
> Fixes: 8853750dbad8 ("drm/i915: Enable SDP split for DP2.0")
> Signed-off-by: Mitul Golani <[email protected]>
> Assisted-by: Claude:claude-opus-4-8

This need to move above the Signed-off-by

> ---
>  .../drm/i915/display/intel_display_types.h    |  2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 44 ++++++++++++++++---
>  2 files changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 20a07ea06b5e..5f0fe18c0614 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1951,6 +1951,8 @@ struct intel_dp {
> 
>  	bool colorimetry_support;
> 
> +	bool sst_split_sdp_support;
> +
>  	struct {
>  		enum transcoder transcoder;
>  		struct mutex lock;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 437b9e7bb864..81c9248516fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3414,12 +3414,22 @@ intel_dp_audio_compute_config(struct
> intel_encoder *encoder,
>  			      struct intel_crtc_state *pipe_config,
>  			      struct drm_connector_state *conn_state)  {
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +
>  	pipe_config->has_audio =
>  		intel_dp_has_audio(encoder, conn_state) &&
>  		intel_audio_compute_config(encoder, pipe_config, conn_state);
> 
>  	pipe_config->sdp_split_enable = pipe_config->has_audio &&
>  					intel_dp_is_uhbr(pipe_config);
> +
> +	/*
> +	 * SDP splitting for UHBR audio requires explicit sink capability in
> +	 * SST mode, whereas in MST mode it is inherently supported.
> +	 */
> +	if (pipe_config->sdp_split_enable &&
> +	    !intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
> +		pipe_config->sdp_split_enable = intel_dp-
> >sst_split_sdp_support;
>  }
> 
>  void
> @@ -4461,16 +4471,25 @@ void intel_dp_configure_protocol_converter(struct
> intel_dp *intel_dp,
>  			    str_enable_disable(tmp));
>  }
> 
> -static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> +static u8 intel_dp_read_dprx_feature_enum(struct intel_dp *intel_dp)
>  {
>  	u8 dprx = 0;
> 
> -	if (drm_dp_dpcd_readb(&intel_dp->aux,
> DP_DPRX_FEATURE_ENUMERATION_LIST,
> -			      &dprx) != 1)

Drm_dp_dpcd_read/writeb has been deprecated and it is recommended we use 
drm_dp_dpcd_read/write_byte instead check  https://lore.kernel.org/r/20250324-drm-rework-dpcd-access-v4-1-e80ff89593df@oss.qualcomm.com
so please go through its usage and use that here instead.

With above comments addressed
Reviewed-by: Suraj Kandpal <[email protected]>

> -		return false;
> +	drm_dp_dpcd_readb(&intel_dp->aux,
> DP_DPRX_FEATURE_ENUMERATION_LIST,
> +			  &dprx);
> +	return dprx;
> +}
> +
> +static bool intel_dp_get_colorimetry_status(u8 dprx) {
>  	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
>  }
> 
> +static bool intel_dp_get_sst_split_sdp_status(u8 dprx) {
> +	return dprx & DP_SST_SPLIT_SDP_CAP;
> +}
> +
>  static int intel_dp_read_dsc_dpcd(struct drm_dp_aux *aux,
>  				  u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
>  {
> @@ -4770,6 +4789,7 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp, struct
> intel_connector *connector  {
>  	struct intel_display *display = to_intel_display(intel_dp);
>  	int ret;
> +	u8 dprx;
> 
>  	/* this function is meant to be called only once */
>  	drm_WARN_ON(display->drm, intel_dp->dpcd[DP_DPCD_REV] != 0);
> @@ -4781,8 +4801,13 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp, struct
> intel_connector *connector
>  			 drm_dp_is_branch(intel_dp->dpcd));
>  	intel_init_dpcd_quirks(intel_dp, &intel_dp->desc.ident);
> 
> +	dprx = intel_dp_read_dprx_feature_enum(intel_dp);
> +
>  	intel_dp->colorimetry_support =
> -		intel_dp_get_colorimetry_status(intel_dp);
> +		intel_dp_get_colorimetry_status(dprx);
> +
> +	intel_dp->sst_split_sdp_support =
> +		intel_dp_get_sst_split_sdp_status(dprx);
> 
>  	/*
>  	 * Read the eDP display control registers.
> @@ -4873,13 +4898,20 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  	 * the OUI/ID since we know it won't change.
>  	 */
>  	if (!intel_dp_is_edp(intel_dp)) {
> +		u8 dprx;
> +
>  		drm_dp_read_desc(&intel_dp->aux, &intel_dp->desc,
>  				 drm_dp_is_branch(intel_dp->dpcd));
> 
>  		intel_init_dpcd_quirks(intel_dp, &intel_dp->desc.ident);
> 
> +		dprx = intel_dp_read_dprx_feature_enum(intel_dp);
> +
>  		intel_dp->colorimetry_support =
> -			intel_dp_get_colorimetry_status(intel_dp);
> +			intel_dp_get_colorimetry_status(dprx);
> +
> +		intel_dp->sst_split_sdp_support =
> +			intel_dp_get_sst_split_sdp_status(dprx);
> 
>  		intel_dp_update_sink_caps(intel_dp);
>  	}
> --
> 2.48.1