Re: [PATCH] drm/i915/glk, cnl: Implement WaDisableScalarClockGating

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Sep 28, 2017 at 07:54:36PM +0000, Imre Deak wrote:
> On GLK and CNL enabling a pipe with its pipe scaler enabled will result
> in a FIFO underrun. This happens only once after driver loading or
> system/runtime resume, more specifically after power well 1 gets
> enabled; subsequent modesets seem to be free of underruns. The BSpec
> workaround for this is to disable the pipe scaler clock gating for the
> duration of modeset. Based on my tests disabling clock gating must be
> done before enabling pipe scaling and we can re-enable it after the pipe
> is enabled and one vblank has passed.

Oh! Great!
I had this Wa in my list here, but I was without access to HDSES
and was postponing it...

But now that you mention the bits it was easy to find on BSpec as well.

> 
> For consistency I also checked if plane scaling would cause the same
> problem, but that doesn't seem to trigger this problem.
> 
> The patch is based on an earlier version from Ander.
> 
> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100302
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  8 ++++++++
>  drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 82f36dd0cd94..40a3c045d9d0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3811,6 +3811,14 @@ enum {
>  #define   PWM2_GATING_DIS		(1 << 14)
>  #define   PWM1_GATING_DIS		(1 << 13)
>  
> +#define _CLKGATE_DIS_PSL_A		0x46520
> +#define _CLKGATE_DIS_PSL_B		0x46524
> +#define _CLKGATE_DIS_PSL_C		0x46528
> +#define   DPF_GATING_DIS		(1 << 10)

On BSpec they also tells us to disable bits 8 and 9:

"To disable Scaler clock gating, set bits 8, 9, and 10 of 0x46520 (Pipe A), 0x46524 (Pipe B), or 0x46528 (Pipe C)"

> +
> +#define CLKGATE_DIS_PSL(pipe) \
> +	_MMIO_PIPE(pipe, _CLKGATE_DIS_PSL_A, _CLKGATE_DIS_PSL_B)
> +
>  /*
>   * GEN10 clock gating regs
>   */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 026fa5460fe5..9d0b5a5596a5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5459,6 +5459,19 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
>  	return HAS_IPS(to_i915(crtc->base.dev)) && crtc->pipe == PIPE_A;
>  }
>  
> +static void glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
> +					    enum pipe pipe, bool apply)
> +{
> +	u32 tmp = I915_READ(CLKGATE_DIS_PSL(pipe));
> +
> +	if (apply)
> +		tmp |= DPF_GATING_DIS;
> +	else
> +		tmp &= ~DPF_GATING_DIS;
> +
> +	I915_WRITE(CLKGATE_DIS_PSL(pipe), tmp);
> +}
> +
>  static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  				struct drm_atomic_state *old_state)
>  {
> @@ -5469,6 +5482,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  	struct intel_atomic_state *old_intel_state =
>  		to_intel_atomic_state(old_state);
> +	bool psl_clkgate_wa;
>  
>  	if (WARN_ON(intel_crtc->active))
>  		return;
> @@ -5522,6 +5536,12 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  	if (!transcoder_is_dsi(cpu_transcoder))
>  		intel_ddi_enable_pipe_clock(pipe_config);
>  
> +	/* WaDisableScalarClockGating: glk, cnl */

Please also add the BSpec reference Display WA #1180.


> +	psl_clkgate_wa = (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +			 intel_crtc->config->pch_pfit.enabled;
> +	if (psl_clkgate_wa)
> +		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> +

Is this place enough? Wouldn't be better to start that on atomic commit before we set cdclock
and mainly before we do a pre plane update?

Also on spec they say:
"
Chance of underflows when the Pipe Scaler is enabled during a mode-set while the Planes are disabled.
Workaround: Disable the Scaler clock gating when entering a mode-set routine.
The Scaler clock gating should be re-enabled at the end of the mode-set sequence.
"

So I wonder if here is not already too late to disable the clock gatings.

>  	if (INTEL_GEN(dev_priv) >= 9)
>  		skylake_pfit_enable(intel_crtc);
>  	else
> @@ -5555,6 +5575,11 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  
>  	intel_encoders_enable(crtc, pipe_config, old_state);
>  
> +	if (psl_clkgate_wa) {
> +		intel_wait_for_vblank(dev_priv, pipe);
> +		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, false);
> +	}
> +
>  	if (intel_crtc->config->has_pch_encoder) {
>  		intel_wait_for_vblank(dev_priv, pipe);
>  		intel_wait_for_vblank(dev_priv, pipe);
> -- 
> 2.13.2
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx