Re: [PATCH 50/50] drm/i915/dp: Use intel_dp_dsc_get_slice_config()

[Imre Deak <imre.deak@intel.com>]

On Thu, Dec 11, 2025 at 12:23:42PM +0200, Imre Deak wrote:
> On Thu, Dec 11, 2025 at 08:59:25AM +0200, Jouni Hogander wrote:
> > On Thu, 2025-11-27 at 19:50 +0200, Imre Deak wrote:
> > > Simplify things by computing the detailed slice configuration using
> > > intel_dp_dsc_get_slice_config(), instead of open-coding the same.
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp.c | 35 +++--------------------
> > > --
> > >  1 file changed, 3 insertions(+), 32 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 003f4b18c1175..d41c75c6f7831 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -2387,7 +2387,6 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> > >  		&pipe_config->hw.adjusted_mode;
> > >  	int num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
> > >  	bool is_mst = intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST);
> > > -	int slices_per_line;
> > >  	int ret;
> > >  
> > >  	/*
> > > @@ -2413,39 +2412,11 @@ int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> > >  		}
> > >  	}
> > >  
> > > -	/* Calculate Slice count */
> > > -	slices_per_line = intel_dp_dsc_get_slice_count(connector,
> > > -						       adjusted_mode->crtc_clock,
> > > -						       adjusted_mode->crtc_hdisplay,
> > > -						       num_joined_pipes);
> > > -	if (!slices_per_line)
> > > +	if (!intel_dp_dsc_get_slice_config(connector, adjusted_mode->crtc_clock,
> > > +					   adjusted_mode->crtc_hdisplay, num_joined_pipes,
> > > +					   &pipe_config->dsc.slice_config))
> > >  		return -EINVAL;
> > >  
> > > -	/*
> > > -	 * VDSC engine operates at 1 Pixel per clock, so if peak pixel rate
> > > -	 * is greater than the maximum Cdclock and if slice count is even
> > > -	 * then we need to use 2 VDSC instances.
> > > -	 * In case of Ultrajoiner along with 12 slices we need to use 3
> > > -	 * VDSC instances.
> > > -	 */
> > 
> > I'll guess you have considered this comment being useless?
> 
> A stricter condition between pixel clock (mode clock) vs. CD clock is
> described already in intel_dp_dsc_min_slice_count(). I can further
> clarify the comment in that function, mentioning also the above VDSC
> engine 1 ppc limit as a reason for the condition there.

After talking with Ville, the 1 pixel per clock vs. CDCLK limitation is
actually explained and accounted for (increasing the CDCLK if necessary)
in intel_vdsc_min_cdclk(). Another thing to do would be to increase the
number slices and with that the number of VDSC engine streams, so that a
lower CDCLK can be used. This optimization isn't attempted atm, but I'll
add a TODO comment to intel_dp_dsc_get_slice_config() to consider doing
that in the future.

> The 12 slices-per-line / 3 VDSC streams-per-pipe logic is already
> described in intel_dsc_get_slice_config().
> 
> > Anyways, patch looks ok:
> > 
> > Reviewed-by: Jouni Högander <jouni.hogander@intel.com>
> > 
> > > -	pipe_config->dsc.slice_config.pipes_per_line = num_joined_pipes;
> > > -
> > > -	if (pipe_config->joiner_pipes && num_joined_pipes == 4 &&
> > > -	    slices_per_line == 12)
> > > -		pipe_config->dsc.slice_config.streams_per_pipe = 3;
> > > -	else if (pipe_config->joiner_pipes || slices_per_line > 1)
> > > -		pipe_config->dsc.slice_config.streams_per_pipe = 2;
> > > -	else
> > > -		pipe_config->dsc.slice_config.streams_per_pipe = 1;
> > > -
> > > -	pipe_config->dsc.slice_config.slices_per_stream =
> > > -		slices_per_line /
> > > -		pipe_config->dsc.slice_config.pipes_per_line /
> > > -		pipe_config->dsc.slice_config.streams_per_pipe;
> > > -
> > > -	drm_WARN_ON(display->drm,
> > > -		    intel_dsc_line_slice_count(&pipe_config->dsc.slice_config) != slices_per_line);
> > > -
> > >  	ret = intel_dp_dsc_compute_params(connector, pipe_config);
> > >  	if (ret < 0) {
> > >  		drm_dbg_kms(display->drm,
> >