On Mon, Sep 22, 2025 at 11:06:02PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 22, 2025 at 10:35:42PM +0300, Ville Syrjälä wrote:
> > On Fri, Sep 19, 2025 at 12:12:22AM +0300, Imre Deak wrote:
> > > Read out the branch devices' maximum overall DSC pixel throughput and
> > > line width and verify the mode's corresponding pixel clock and hactive
> > > period against these.
> > > 
> > > Signed-off-by: Imre Deak <[email protected]>
> > > ---
> > >  .../drm/i915/display/intel_display_types.h    |  8 +++
> > >  drivers/gpu/drm/i915/display/intel_dp.c       | 62 +++++++++++++++++++
> > >  2 files changed, 70 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 358ab922d7a76..73bdafae604f8 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -552,6 +552,14 @@ struct intel_connector {
> > >  
> > >           u8 dsc_hblank_expansion_quirk:1;
> > >           u8 dsc_decompression_enabled:1;
> > > +
> > > +         struct {
> > > +                 struct {
> > > +                         int rgb_yuv444;
> > > +                         int yuv422_420;
> > > +                 } overall_throughput;
> > > +                 int max_line_width;
> > > +         } dsc_branch_caps;
> > >   } dp;
> > >  
> > >   struct {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 8cc123b0fd752..dd082d2fcc968 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -1042,6 +1042,20 @@ u8 intel_dp_dsc_get_slice_count(const struct 
> > > intel_connector *connector,
> > >   int tp_yuv422_420;
> > >   u8 val;
> > >  
> > > + /*
> > > +  * TODO: Use the throughput value specific to the actual RGB/YUV
> > > +  * format of the output.
> > > +  * The RGB/YUV444 throughput value should be always either equal
> > > +  * or smaller than the YUV422/420 value, but let's not depend on
> > > +  * this assumption.
> > > +  */
> > > + if (mode_clock > 
> > > max(connector->dp.dsc_branch_caps.overall_throughput.rgb_yuv444,
> > > +                      
> > > connector->dp.dsc_branch_caps.overall_throughput.yuv422_420))
> > > +         return 0;
> > > +
> > > + if (mode_hdisplay > connector->dp.dsc_branch_caps.max_line_width)
> > > +         return 0;
> > 
> > Looks like you only initialize these when the branch device supports
> > decompression. What about a branch device that can do pass-through but
> > no decompression? Should we even be checking these when doing
> > pass-through?
> 
> Ah, we read them from the decompression_aux, which I presume is always
> the thing that we want to do the decompression.
> 
> So in the passthrough case it will be the sink, and then we take the
> early !is_branch exit from init_dsc_overall_throughput_limits().

Yes, for both the decompress and pass-through case decompression_aux
points to the device decompressing the stream, and the caps will be read
out for that device.

In theory even in the pass-through case the decompressing device could
be an (SST-only) branch device, in which case the caps will be parsed
for that device as well. Granted I haven't seen such a device yet.

If DSC is not supported then the caps will be left zeroed.

If DSC is supported (either via pass-through or just decompression) by a
branch device then the caps will be either the device advertised ones,
or INT_MAX for caps which the device doesn't set a cap for (i.e. leaves
the DPCD reg at 0).

If DSC is supported by a non-branch device, then the caps will not be
read out and they will be set to INT_MAX.

> Same for eDP.

Yes, for eDP if DSC is not supported, the caps are set to 0, if DSC is
supported, the caps will be set to INT_MAX.

> > > +
> > >   val = connector->dp.dsc_dpcd[DP_DSC_PEAK_THROUGHPUT - DP_DSC_SUPPORT];
> > >   tp_rgb_yuv444 = dsc_per_slice_throughput(display, mode_clock,
> > >                                            
> > > REG_FIELD_GET8(DP_DSC_THROUGHPUT_MODE_0_MASK,
> > > @@ -4204,6 +4218,44 @@ static void intel_dp_read_dsc_dpcd(struct 
> > > drm_dp_aux *aux,
> > >               dsc_dpcd);
> > >  }
> > >  
> > > +static int dsc_branch_overall_throughput(u8 bw_code)
> > > +{
> > > + switch (bw_code) {
> > > + case 0:
> > > +         return INT_MAX;
> > > + case 1:
> > > +         return 680000;
> > > + default:
> > > +         return 600000 + 50000 * bw_code;
> > > + }
> > > +}
> > > +
> > > +static void init_dsc_overall_throughput_limits(struct intel_connector 
> > > *connector, bool is_branch)
> > > +{
> > > + u8 branch_caps[3];
> > > +
> > > + connector->dp.dsc_branch_caps.overall_throughput.rgb_yuv444 = INT_MAX;
> > > + connector->dp.dsc_branch_caps.overall_throughput.yuv422_420 = INT_MAX;
> > > + connector->dp.dsc_branch_caps.max_line_width = INT_MAX;
> > > +
> > > + if (!is_branch)
> > > +         return;
> > > +
> > > + if (drm_dp_dpcd_read_data(connector->dp.dsc_decompression_aux,
> > > +                           DP_DSC_BRANCH_OVERALL_THROUGHPUT_0, 
> > > branch_caps,
> > > +                           sizeof(branch_caps)) != 0)
> > > +         return;
> > > +
> > > + connector->dp.dsc_branch_caps.overall_throughput.rgb_yuv444 =
> > > +         dsc_branch_overall_throughput(branch_caps[0]);
> > > +
> > > + connector->dp.dsc_branch_caps.overall_throughput.yuv422_420 =
> > > +         dsc_branch_overall_throughput(branch_caps[1]);
> > > +
> > > + if (branch_caps[2] != 0)
> > > +         connector->dp.dsc_branch_caps.max_line_width = branch_caps[2] * 
> > > 320;
> > 
> > That max line width calculation should probably be a function as well
> > so that it can later be moved into some drm helper file along with the
> > other functions.

Ok, I added a DRM helper instead as in:
https://github.com/ideak/linux/commit/7f7cab2746f4

> > 
> > > +}
> > > +
> > >  void intel_dp_get_dsc_sink_cap(u8 dpcd_rev,
> > >                          const struct drm_dp_desc *desc, bool is_branch,
> > >                          struct intel_connector *connector)
> > > @@ -4219,6 +4271,8 @@ void intel_dp_get_dsc_sink_cap(u8 dpcd_rev,
> > >   /* Clear fec_capable to avoid using stale values */
> > >   connector->dp.fec_capability = 0;
> > >  
> > > + memset(&connector->dp.dsc_branch_caps, 0, 
> > > sizeof(connector->dp.dsc_branch_caps));
> > > +
> > >   if (dpcd_rev < DP_DPCD_REV_14)
> > >           return;
> > >  
> > > @@ -4233,6 +4287,11 @@ void intel_dp_get_dsc_sink_cap(u8 dpcd_rev,
> > >  
> > >   drm_dbg_kms(display->drm, "FEC CAPABILITY: %x\n",
> > >               connector->dp.fec_capability);
> > > +
> > > + if (!(connector->dp.dsc_dpcd[0] & DP_DSC_DECOMPRESSION_IS_SUPPORTED))
> > > +         return;
> > > +
> > > + init_dsc_overall_throughput_limits(connector, is_branch);
> > >  }
> > >  
> > >  static void intel_edp_get_dsc_sink_cap(u8 edp_dpcd_rev, struct 
> > > intel_connector *connector)
> > > @@ -4241,6 +4300,9 @@ static void intel_edp_get_dsc_sink_cap(u8 
> > > edp_dpcd_rev, struct intel_connector *
> > >           return;
> > >  
> > >   intel_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux, 
> > > connector->dp.dsc_dpcd);
> > > +
> > > + if (connector->dp.dsc_dpcd[0] & DP_DSC_DECOMPRESSION_IS_SUPPORTED)
> > > +         init_dsc_overall_throughput_limits(connector, false);
> > >  }
> > >  
> > >  static void
> > > -- 
> > > 2.49.1
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> 
> -- 
> Ville Syrjälä
> Intel

Reply via email to