On Fri, Dec 12, 2025 at 04:59:31PM +0200, Vinod Govindapillai wrote:
> On Thu, 2025-11-27 at 19:50 +0200, Imre Deak wrote:

> > The maximum pipe BPP value (used as the DSC input BPP) has been
> > aligned already to the corresponding source/sink input BPP
> > capabilities in intel_dp_compute_config_limits(). So it isn't needed
> > to perform the same alignment again in
> > intel_dp_dsc_compute_pipe_bpp() called later, this function can
> > simply use the already aligned maximum pipe BPP value, do that.
> > 
> > Also, there is no point in trying pipe BPP values lower than the
> > maximum: this would only make dsc_compute_compressed_bpp() start
> > with a lower _compressed_ BPP value, but this lower compressed BPP
> > value has been tried already when dsc_compute_compressed_bpp() was
> > called with the higher pipe BPP value (i.e. the first
> > dsc_compute_compressed_bpp() call tries already all the possible
> > compressed BPP values which are all below the pipe BPP value passed
> > to it). Simplify the function accordingly trying only the maximum
> > pipe BPP value.
> > 
> > Signed-off-by: Imre Deak <[email protected]>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 29 +++++++----------------
> > --
> >  1 file changed, 8 insertions(+), 21 deletions(-)
> > 
> 
> I guess, typically this is a two patch solution. But considering the
> code changes it make sense to have it as one patch.

Yes, the current code before this patch combines the alignment and
fallback logic, so I found it clearer to remove both in one patch,
instead of tweaking the code in one patch to do only one of these and
then remove the tweak in a separate patch.

> 
> Reviewed-by: Vinod Govindapillai <[email protected]>
> 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index ee33759a2f5d7..902f3a054a971 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -2294,11 +2294,8 @@ static int
> > intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
> >                                      struct drm_connector_state *conn_state,
> >                                      const struct link_config_limits 
> > *limits)
> >  {
> > -   const struct intel_connector *connector =
> > -           to_intel_connector(conn_state->connector);
> > -   u8 dsc_bpc[3] = {};
> >     int forced_bpp, pipe_bpp;
> > -   int num_bpc, i, ret;
> > +   int ret;
> >  
> >     forced_bpp = intel_dp_force_dsc_pipe_bpp(intel_dp, limits);
> >  
> > @@ -2311,25 +2308,15 @@ static int
> > intel_dp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,
> >             }
> >     }
> >  
> > -   /*
> > -    * Get the maximum DSC bpc that will be supported by any valid
> > -    * link configuration and compressed bpp.
> > -    */
> > -   num_bpc = drm_dp_dsc_sink_supported_input_bpcs(connector->dp.dsc_dpcd, 
> > dsc_bpc);
> > -   for (i = 0; i < num_bpc; i++) {
> > -           pipe_bpp = dsc_bpc[i] * 3;
> > -           if (pipe_bpp < limits->pipe.min_bpp || pipe_bpp > 
> > limits->pipe.max_bpp)
> > -                   continue;
> > +   pipe_bpp = limits->pipe.max_bpp;
> > +   ret = dsc_compute_compressed_bpp(intel_dp, pipe_config, conn_state,
> > +                                    limits, pipe_bpp);
> > +   if (ret)
> > +           return -EINVAL;
> >  
> > -           ret = dsc_compute_compressed_bpp(intel_dp, pipe_config, 
> > conn_state,
> > -                                            limits, pipe_bpp);
> > -           if (ret == 0) {
> > -                   pipe_config->pipe_bpp = pipe_bpp;
> > -                   return 0;
> > -           }
> > -   }
> > +   pipe_config->pipe_bpp = pipe_bpp;
> >  
> > -   return -EINVAL;
> > +   return 0;
> >  }
> >  
> >  static int intel_edp_dsc_compute_pipe_bpp(struct intel_dp *intel_dp,

Reply via email to