> -----Original Message-----
> From: Kandpal, Suraj <[email protected]>
> Sent: 25 August 2026 13:21
> To: Golani, Mitulkumar Ajitkumar <[email protected]>;
> [email protected]
> Cc: [email protected]
> Subject: RE: [PATCH v3] drm/i915/dp: Gate UHBR SST SDP splitting on sink
> capability
> 
> > Subject: [PATCH v3] 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.
> >
> 
> [2]
> 
> > Fixes: 8853750dbad8 ("drm/i915: Enable SDP split for DP2.0")
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Mitul Golani <[email protected]>
> > Reviewed-by: Suraj Kandpal <[email protected]>
> >
> > --v2:
> > - Correct the name in the intel_dp struct. (Suraj)
> > - Add the Fixes tag. (Suraj)
> > - Explain the reasoning for guarding SST separately from MST. (Suraj)
> > - Add a separate helper to read capabilities. (Suraj)
> >
> > --v3:
> > - Use drm_dp_dpcd_read instead of drm_dp_dpcd_readb. (Suraj)
> > - Move Assisted-by above Signed-off-by. (Suraj)
> > - Add the versioning diff. (Suraj)
> 
> This either needs to be moved below the at [1] which you will have to do
> after you make a patch Then resend it or it needs to move above at [2] No
> need to resend can be fixed while merging.
> 
> Regards,
> Suraj Kandpal

No worries, in any case new version needed for below change, 

"> > --v3:
> > - Use drm_dp_dpcd_read instead of drm_dp_dpcd_readb. (Suraj)"

Rest we can fix before merge.

Thanks


> 
> > ---
> >  .../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 6b60e0a1c8ee..0cd5e6b5034c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -3409,12 +3409,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
> > @@ -4456,16 +4466,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)
> > -           return false;
> > +   drm_dp_dpcd_read_data(&intel_dp->aux,
> > DP_DPRX_FEATURE_ENUMERATION_LIST,
> > +                         &dprx, sizeof(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])
> >  {
> > @@ -4765,6 +4784,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);
> @@
> > -4776,8 +4796,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.
> > @@ -4868,13 +4893,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

Reply via email to