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@xxxxxxxxx> > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100302 > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx