[Intel-gfx] [PATCH] drm/i915/display: Enable second VDSC engine for higher moderates

[Ville Syrjälä]

On Tue, Sep 14, 2021 at 07:31:46AM +0000, Kulkarni, Vandita wrote:
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Sent: Tuesday, September 14, 2021 12:59 PM
> > To: Kulkarni, Vandita <vandita.kulkarni at intel.com>
> > Cc: intel-gfx at lists.freedesktop.org; Nikula, Jani <jani.nikula at intel.com>;
> > Navare, Manasi D <manasi.d.navare at intel.com>
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Enable second VDSC
> > engine for higher moderates
> > 
> > On Mon, Sep 13, 2021 at 08:09:23PM +0530, Vandita Kulkarni wrote:
> > > Each VDSC operates with 1ppc throughput, hence enable the second VDSC
> > > engine when moderate is higher that the current cdclk.
> > >
> > > Signed-off-by: Vandita Kulkarni <vandita.kulkarni at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 161c33b2c869..55878f65f724 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -70,6 +70,7 @@
> > >  #include "intel_tc.h"
> > >  #include "intel_vdsc.h"
> > >  #include "intel_vrr.h"
> > > +#include "intel_cdclk.h"
> > >
> > >  #define DP_DPRX_ESI_LEN 14
> > >
> > > @@ -1291,10 +1292,13 @@ static int intel_dp_dsc_compute_config(struct
> > intel_dp *intel_dp,
> > >  				       struct drm_connector_state *conn_state,
> > >  				       struct link_config_limits *limits)  {
> > > +	struct intel_cdclk_state *cdclk_state;
> > >  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > >  	struct drm_i915_private *dev_priv = to_i915(dig_port-
> > >base.base.dev);
> > >  	const struct drm_display_mode *adjusted_mode =
> > >  		&pipe_config->hw.adjusted_mode;
> > > +	struct intel_atomic_state *state =
> > > +				to_intel_atomic_state(pipe_config-
> > >uapi.state);
> > >  	int pipe_bpp;
> > >  	int ret;
> > >
> > > @@ -1373,12 +1377,16 @@ static int intel_dp_dsc_compute_config(struct
> > intel_dp *intel_dp,
> > >  		}
> > >  	}
> > >
> > > +	cdclk_state = intel_atomic_get_cdclk_state(state);
> > > +	if (IS_ERR(cdclk_state))
> > > +		return PTR_ERR(cdclk_state);
> > > +
> > >  	/*
> > >  	 * 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
> > > +	 * is greater than the current Cdclock and if slice count is even
> > >  	 * then we need to use 2 VDSC instances.
> > >  	 */
> > > -	if (adjusted_mode->crtc_clock > dev_priv->max_cdclk_freq ||
> > > +	if (adjusted_mode->crtc_clock > cdclk_state->actual.cdclk ||
> > 
> > This is wrong. We compute the cdclk based on the requirements of the
> > mode/etc., not the other way around.
> 
> Okay , So you suggest that we set the cd clock to max when we have such requirement, than enabling the second engine?

That seems like the easiest solution. Another option might be to come up
with some lower dotclock limit for the use of the second vdsc. But not
sure we know where the tipping point is wrt. powr consumption.

-- 
Ville Syrjälä
Intel