[Intel-gfx] [PATCH v2 2/6] drm/i915/vdsc: Add a check for dsc split cases

[Kandpal, Suraj]

> On Thu, 13 Jul 2023, Suraj Kandpal <suraj.kandpal at intel.com> wrote:
> > In intel_vdsc_get_config we only read the primary dsc engine register
> > and not take into account if the other dsc engine is in use and if
> > both registers have the same value or not this patche fixes that by
> > adding a check.
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index 530f3c08a172..d48b8306bfc3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -939,7 +939,7 @@ void intel_dsc_get_config(struct intel_crtc_state
> *crtc_state)
> >  	enum pipe pipe = crtc->pipe;
> >  	enum intel_display_power_domain power_domain;
> >  	intel_wakeref_t wakeref;
> > -	u32 dss_ctl1, dss_ctl2, pps0 = 0, pps1 = 0;
> > +	u32 dss_ctl1, dss_ctl2, pps0 = 0, pps1 = 0, pps_temp0 = 0, pps_temp1
> > += 1;
> >
> >  	if (!intel_dsc_source_support(crtc_state))
> >  		return;
> > @@ -965,11 +965,24 @@ void intel_dsc_get_config(struct intel_crtc_state
> *crtc_state)
> >  	/* PPS0 & PPS1 */
> >  	if (!is_pipe_dsc(crtc, cpu_transcoder)) {
> >  		pps1 = intel_de_read(dev_priv,
> DSCA_PICTURE_PARAMETER_SET_1);
> > +		if (crtc_state->dsc.dsc_split) {
> > +			pps_temp1 = intel_de_read(dev_priv,
> DSCC_PICTURE_PARAMETER_SET_1);
> > +			drm_WARN_ON(&dev_priv->drm, pps1 !=
> pps_temp1);
> > +		}
> > +
> 
> Superfluous newline.
> 

Thanks for the review will fix that 

> >  	} else {
> >  		pps0 = intel_de_read(dev_priv,
> >
> ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe));
> >  		pps1 = intel_de_read(dev_priv,
> >
> ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe));
> > +		if (crtc_state->dsc.dsc_split) {
> > +			pps_temp0 = intel_de_read(dev_priv,
> > +
> ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe));
> > +			pps_temp1 = intel_de_read(dev_priv,
> > +
> ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe));
> 
> Those are the same two registers as above?
> 

Yes they are should have been _DSC1_instead

Regards,
Suraj Kandpal

> BR,
> Jani.
> 
> > +			drm_WARN_ON(&dev_priv->drm, pps0 !=
> pps_temp0);
> > +			drm_WARN_ON(&dev_priv->drm, pps1 !=
> pps_temp1);
> > +		}
> >  	}
> >
> >  	vdsc_cfg->bits_per_pixel = pps1;
> 
> --
> Jani Nikula, Intel Open Source Graphics Center