[Intel-gfx] [PATCH v4] drm/i915/icl: Fix clockgating issue when using scalers

[Ville Syrjälä]

On Thu, Mar 28, 2019 at 10:35:19AM -0700, Radhakrishna Sripada wrote:
> Fixes the clock-gating issue when pipe scaling is enabled.
> (Lineage #2006604312)
> 
> V2: Fix typo in headline(Chris)
>     Handle the non double buffered nature of the register(Ville)
> V3: Fix checkpatch warning. BAT failure for V2 on gen3 looks unrelated.
> V4: Split the icl and skl wa's(Ville)
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Aditya Swarup <aditya.swarup at intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8576a7f799f2..c3ca9cfd36fe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -466,6 +466,7 @@ static const struct intel_limit intel_limits_bxt = {
>  	.p2 = { .p2_slow = 1, .p2_fast = 20 },
>  };
>  
> +/* WA Display #0827: Gen9:all */
>  static void
>  skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable)
>  {
> @@ -478,6 +479,17 @@ skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable)
>  			   ~(DUPS1_GATING_DIS | DUPS2_GATING_DIS));
>  }
>  
> +/* Wa_2006604312:icl */
> +static void
> +icl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable)

These functions names (including the skl one) are rather too generic
and don't describe which clock gating we're disabling. But fixing
that is probably material for another patch.

> +{
> +	if (enable)
> +		I915_WRITE(CLKGATE_DIS_PSL(pipe), DPFR_GATING_DIS);
> +	else
> +		I915_WRITE(CLKGATE_DIS_PSL(pipe),
> +			   I915_READ(CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS);

No RMW for enable but RMW for disable? Seems rather inconsistent.

> +}
> +
>  static bool
>  needs_modeset(const struct drm_crtc_state *state)
>  {
> @@ -5481,14 +5493,18 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s
>  	return !old_crtc_state->ips_enabled;
>  }
>  
> -static bool needs_nv12_wa(struct drm_i915_private *dev_priv,
> -			  const struct intel_crtc_state *crtc_state)
> +static bool skl_needs_clk_wa(struct drm_i915_private *dev_priv,
> +			     const struct intel_crtc_state *crtc_state)

Still mixing our workarounds more than I'd like. I suggest keeping
the two entirely separate.

>  {
> -	if (!crtc_state->nv12_planes)
> -		return false;
> -
>  	/* WA Display #0827: Gen9:all */
> -	if (IS_GEN(dev_priv, 9) && !IS_GEMINILAKE(dev_priv))
> +	if (!!crtc_state->nv12_planes && IS_GEN(dev_priv, 9) &&
> +	    !IS_GEMINILAKE(dev_priv))
> +		return true;
> +
> +	/*
> +	 * Wa_2006604312:icl
> +	 */
> +	if (IS_ICELAKE(dev_priv) && crtc_state->pch_pfit.enabled)
>  		return true;
>  
>  	return false;
> @@ -5527,10 +5543,12 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
>  			intel_post_enable_primary(&crtc->base, pipe_config);
>  	}
>  
> -	/* Display WA 827 */
> -	if (needs_nv12_wa(dev_priv, old_crtc_state) &&
> -	    !needs_nv12_wa(dev_priv, pipe_config)) {
> -		skl_wa_clkgate(dev_priv, crtc->pipe, false);
> +	if (skl_needs_clk_wa(dev_priv, old_crtc_state) &&
> +	    !skl_needs_clk_wa(dev_priv, pipe_config)) {
> +		if (IS_ICELAKE(dev_priv))
> +			icl_wa_clkgate(dev_priv, crtc->pipe, false);
> +		else
> +			skl_wa_clkgate(dev_priv, crtc->pipe, false);
>  	}
>  }
>  
> @@ -5566,10 +5584,12 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
>  			intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
>  	}
>  
> -	/* Display WA 827 */
> -	if (!needs_nv12_wa(dev_priv, old_crtc_state) &&
> -	    needs_nv12_wa(dev_priv, pipe_config)) {
> -		skl_wa_clkgate(dev_priv, crtc->pipe, true);
> +	if (!skl_needs_clk_wa(dev_priv, old_crtc_state) &&
> +	    skl_needs_clk_wa(dev_priv, pipe_config)) {
> +		if (IS_ICELAKE(dev_priv))
> +			icl_wa_clkgate(dev_priv, crtc->pipe, true);
> +		else
> +			skl_wa_clkgate(dev_priv, crtc->pipe, true);
>  	}
>  
>  	/*
> -- 
> 2.20.0.rc2.7.g965798d1f299

-- 
Ville Syrjälä
Intel