On Tue, 2025-12-09 at 11:53 +0200, Imre Deak wrote:
> On Tue, Dec 09, 2025 at 10:51:10AM +0200, Luca Coelho wrote:
> > On Thu, 2025-11-27 at 19:49 +0200, Imre Deak wrote:
> > > A DSC sink supporting DSC slice count N, not necessarily supports slice
> > > counts less than N. Hence the driver should check the sink's support for
> > > a particular slice count before using that slice count, fix
> > > intel_dp_dsc_get_slice_count() accordingly.
> > > 
> > > Cc: [email protected]
> > > Signed-off-by: Imre Deak <[email protected]>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp.c | 18 +++++++++++++-----
> > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 0ec82fcbcf48e..6d232c15a0b5a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -1013,6 +1013,8 @@ u8 intel_dp_dsc_get_slice_count(const struct 
> > > intel_connector *connector,
> > >                           int num_joined_pipes)
> > >  {
> > >   struct intel_display *display = to_intel_display(connector);
> > > + u32 sink_slice_count_mask =
> > > +         drm_dp_dsc_sink_slice_count_mask(connector->dp.dsc_dpcd, false);
> > >   u8 min_slice_count, i;
> > >   int max_slice_width;
> > >   int tp_rgb_yuv444;
> > > @@ -1084,9 +1086,9 @@ u8 intel_dp_dsc_get_slice_count(const struct 
> > > intel_connector *connector,
> > >               (!HAS_DSC_3ENGINES(display) || num_joined_pipes != 4))
> > >                   continue;
> > >  
> > > -         if (test_slice_count >
> > > -             drm_dp_dsc_sink_max_slice_count(connector->dp.dsc_dpcd, 
> > > false))
> > > -                 break;
> > > +         if (!(drm_dp_dsc_slice_count_to_mask(test_slice_count) &
> > > +               sink_slice_count_mask))
> > > +                 continue;
> > >  
> > >            /*
> > >             * Bigjoiner needs small joiner to be enabled.
> > > @@ -1103,8 +1105,14 @@ u8 intel_dp_dsc_get_slice_count(const struct 
> > > intel_connector *connector,
> > >                   return test_slice_count;
> > >   }
> > >  
> > > - drm_dbg_kms(display->drm, "Unsupported Slice Count %d\n",
> > > -             min_slice_count);
> > > + /* Print slice count 1,2,4,..24 if bit#0,1,3,..23 is set in the mask. */
> > > + sink_slice_count_mask <<= 1;
> > > + drm_dbg_kms(display->drm,
> > > +             "[CONNECTOR:%d:%s] Unsupported slice count (min: %d, sink 
> > > supported: %*pbl)\n",
> > > +             connector->base.base.id, connector->base.name,
> > > +             min_slice_count,
> > > +             (int)BITS_PER_TYPE(sink_slice_count_mask), 
> > > &sink_slice_count_mask);
> > > +
> > >   return 0;
> > >  }
> > > 
> > 
> > I think this patch could be squashed into the previous one.  IMHO it
> > makes it a bit easier to see how those functions defined in the
> > previous patch would be used.
> 
> The practice I follow is to keep the DRM core and driver changes in
> separate patches. At least one reason for that is that the DRM core
> patches may need to be applied to the DRM core trees separately.

Oh, of course, makes 100% sense.  I overlooked this point.

--
Cheers,
Luca.

Reply via email to