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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 13 Jul 2023, Suraj Kandpal <suraj.kandpal@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@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.

>  	} 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?

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