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

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

[Imre Deak]

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.

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)
+
+#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 */
+	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);
+
 	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

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

[Rodrigo Vivi]

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

✓ Fi.CI.BAT: success for drm/i915/glk, cnl: Implement WaDisableScalarClockGating

[Patchwork]

== Series Details ==

Series: drm/i915/glk, cnl: Implement WaDisableScalarClockGating
URL   : https://patchwork.freedesktop.org/series/31094/
State : success

== Summary ==

Series 31094v1 drm/i915/glk, cnl: Implement WaDisableScalarClockGating
https://patchwork.freedesktop.org/api/1.0/series/31094/revisions/1/mbox/

Test drv_module_reload:
        Subgroup basic-no-display:
                dmesg-warn -> PASS       (fi-glk-1) fdo#102777

fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:439s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:473s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:420s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:517s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:278s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:494s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:504s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:490s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:484s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:556s
fi-cnl-y         total:289  pass:260  dwarn:1   dfail:0   fail:1   skip:27  time:623s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:426s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:566s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:425s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:405s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:429s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:493s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:466s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:461s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:581s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:597s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:542s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:752s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:481s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:580s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:420s

0369ecdbb55495dd4671ad33d375f37d0707b629 drm-tip: 2017y-09m-28d-20h-01m-55s UTC integration manifest
c31be1a11e6f drm/i915/glk, cnl: Implement WaDisableScalarClockGating

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5852/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/glk, cnl: Implement WaDisableScalarClockGating

[Patchwork]

== Series Details ==

Series: drm/i915/glk, cnl: Implement WaDisableScalarClockGating
URL   : https://patchwork.freedesktop.org/series/31094/
State : success

== Summary ==

Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2429 pass:1332 dwarn:5   dfail:0   fail:9   skip:1083 time:9943s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5852/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Imre Deak]

On Thu, Sep 28, 2017 at 01:18:30PM -0700, Rodrigo Vivi wrote:
> 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)"

Right, thanks for catching this. The original WA was only bit 10, and it
does get rid of the problem. I haven't noticed this change, will update
the patch based on that.

> 
> > +
> > +#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.

Ok.

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

I'd prefer keeping it here leaving the WA's range narrow, unless it's
proven that it's not enough. If CD clock would be a factor, then we'd
have to force a CD clock disable/re-enable if the CD clock rate wouldn't
change and that's not required by the WA. pre_plane_update is only
called on the fastset path, and this WA is only needed during a full
modeset (luckily b/c of the vblank wait).

> 
> 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.

Yea, the wording is a bit unclear, it doesn't either say for example
when to re-enable the clock gating. I will ask for clarification, but
I'd say it's a reasonable assumption that the pipe scaler clock gating
doesn't have any effect until the pipe scaler is enabled and this is
also what I see during my tests. So imo we can go with this until proven
otherwise.

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

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

[Imre Deak]

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.

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.

v2 (Rodrigo):
- Set also CLKGATE_DIS_PSL bits 8 and 9.
- Add also the BSpec workaround ID.

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      | 10 ++++++++++
 drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ee0d4f14ac98..39ad9327e2a0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3819,6 +3819,16 @@ 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)
+#define   DPF_RAM_GATING_DIS		(1 << 9)
+#define   DPFR_GATING_DIS		(1 << 8)
+
+#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 c4b224a3a0ee..b7a6ddc6a66d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5459,6 +5459,20 @@ 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 val = I915_READ(CLKGATE_DIS_PSL(pipe));
+	u32 mask = DPF_GATING_DIS | DPF_RAM_GATING_DIS | DPFR_GATING_DIS;
+
+	if (apply)
+		val |= mask;
+	else
+		val &= ~mask;
+
+	I915_WRITE(CLKGATE_DIS_PSL(pipe), val);
+}
+
 static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 				struct drm_atomic_state *old_state)
 {
@@ -5469,6 +5483,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 +5537,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);
 
+	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
+	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);
+
 	if (INTEL_GEN(dev_priv) >= 9)
 		skylake_pfit_enable(intel_crtc);
 	else
@@ -5555,6 +5576,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

✓ Fi.CI.BAT: success for drm/i915/glk, cnl: Implement WaDisableScalarClockGating (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/glk, cnl: Implement WaDisableScalarClockGating (rev2)
URL   : https://patchwork.freedesktop.org/series/31094/
State : success

== Summary ==

Series 31094v2 drm/i915/glk, cnl: Implement WaDisableScalarClockGating
https://patchwork.freedesktop.org/api/1.0/series/31094/revisions/2/mbox/

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:458s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:471s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:392s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:575s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:289s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:535s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:532s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:542s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:534s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:564s
fi-cnl-y         total:289  pass:261  dwarn:1   dfail:0   fail:0   skip:27  time:620s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:435s
fi-glk-1         total:289  pass:257  dwarn:3   dfail:0   fail:0   skip:29  time:593s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:440s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:426s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:467s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:505s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:479s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:507s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:583s
fi-kbl-7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  time:485s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:589s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:657s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:473s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:543s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:591s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:473s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:584s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:433s

b89a3b1a69f77be194ad4786906048a5e02b9975 drm-tip: 2017y-10m-02d-08h-07m-17s UTC integration manifest
27ae0ed67a28 drm/i915/glk, cnl: Implement WaDisableScalarClockGating

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5867/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/glk, cnl: Implement WaDisableScalarClockGating (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/glk, cnl: Implement WaDisableScalarClockGating (rev2)
URL   : https://patchwork.freedesktop.org/series/31094/
State : success

== Summary ==

Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252

fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2429 pass:1329 dwarn:7   dfail:0   fail:10  skip:1083 time:10165s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5867/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Rodrigo Vivi]

On Mon, Oct 02, 2017 at 07:55:57AM +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.
> 
> 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.
> 
> v2 (Rodrigo):
> - Set also CLKGATE_DIS_PSL bits 8 and 9.
> - Add also the BSpec workaround ID.

Thanks for this and for the explanation around the places where
we need to protect.
They make sense.

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

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h      | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ee0d4f14ac98..39ad9327e2a0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3819,6 +3819,16 @@ 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)
> +#define   DPF_RAM_GATING_DIS		(1 << 9)
> +#define   DPFR_GATING_DIS		(1 << 8)
> +
> +#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 c4b224a3a0ee..b7a6ddc6a66d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5459,6 +5459,20 @@ 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 val = I915_READ(CLKGATE_DIS_PSL(pipe));
> +	u32 mask = DPF_GATING_DIS | DPF_RAM_GATING_DIS | DPFR_GATING_DIS;
> +
> +	if (apply)
> +		val |= mask;
> +	else
> +		val &= ~mask;
> +
> +	I915_WRITE(CLKGATE_DIS_PSL(pipe), val);
> +}
> +
>  static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  				struct drm_atomic_state *old_state)
>  {
> @@ -5469,6 +5483,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 +5537,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);
>  
> +	/* Display WA #1180: WaDisableScalarClockGating: glk, cnl */
> +	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);
> +
>  	if (INTEL_GEN(dev_priv) >= 9)
>  		skylake_pfit_enable(intel_crtc);
>  	else
> @@ -5555,6 +5576,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

Re: ✓ Fi.CI.BAT: success for drm/i915/glk, cnl: Implement WaDisableScalarClockGating (rev2)

[Imre Deak]

On Mon, Oct 02, 2017 at 08:55:28AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/glk, cnl: Implement WaDisableScalarClockGating (rev2)
> URL   : https://patchwork.freedesktop.org/series/31094/
> State : success

Thanks for the review, pushed this to -dinq.

> 
> == Summary ==
> 
> Series 31094v2 drm/i915/glk, cnl: Implement WaDisableScalarClockGating
> https://patchwork.freedesktop.org/api/1.0/series/31094/revisions/2/mbox/
> 
> fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:458s
> fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:471s
> fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:392s
> fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:575s
> fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:289s
> fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:535s
> fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:532s
> fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:542s
> fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:534s
> fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:564s
> fi-cnl-y         total:289  pass:261  dwarn:1   dfail:0   fail:0   skip:27  time:620s
> fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:435s
> fi-glk-1         total:289  pass:257  dwarn:3   dfail:0   fail:0   skip:29  time:593s
> fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:440s
> fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:426s
> fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:467s
> fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:505s
> fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:479s
> fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:507s
> fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:583s
> fi-kbl-7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  time:485s
> fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:589s
> fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:657s
> fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:473s
> fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:543s
> fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:591s
> fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:473s
> fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:584s
> fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:433s
> 
> b89a3b1a69f77be194ad4786906048a5e02b9975 drm-tip: 2017y-10m-02d-08h-07m-17s UTC integration manifest
> 27ae0ed67a28 drm/i915/glk, cnl: Implement WaDisableScalarClockGating
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5867/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx