Re: [PATCH 3/3] drm/i915/glk: Turn on workarounds that apply to Geminilake too

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

On Fri, 2017-01-20 at 08:04 -0800, Rodrigo Vivi wrote:
> On Thu, Jan 12, 2017 at 3:47 AM, Ander Conselvan de Oliveira
> <ander.conselvan.de.oliveira@intel.com> wrote:
> > 
> > Apply workarounds to Geminilake, and annoatate those that are applied
> > uncondionally when they apply to GLK based on the workaround database.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@inte
> > l.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c     |  4 ++--
> >  drivers/gpu/drm/i915/intel_lrc.c        |  6 +++---
> >  drivers/gpu/drm/i915/intel_mocs.c       |  2 +-
> >  drivers/gpu/drm/i915/intel_pm.c         | 23 ++++++++++++++++----
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 37 +++++++++++++++++++++++++-----
> > ---
> >  5 files changed, 53 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 0ed99adf..7024144 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -2167,14 +2167,14 @@ static void gtt_write_workarounds(struct
> > drm_i915_private *dev_priv)
> >          * called on driver load and after a GPU reset, so you can place
> >          * workarounds here even if they get overwritten by GPU reset.
> >          */
> > -       /* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt */
> > +       /* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt,glk */
> >         if (IS_BROADWELL(dev_priv))
> >                 I915_WRITE(GEN8_L3_LRA_1_GPGPU,
> > GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW);
> >         else if (IS_CHERRYVIEW(dev_priv))
> >                 I915_WRITE(GEN8_L3_LRA_1_GPGPU,
> > GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV);
> >         else if (IS_SKYLAKE(dev_priv))
> >                 I915_WRITE(GEN8_L3_LRA_1_GPGPU,
> > GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
> > -       else if (IS_BROXTON(dev_priv))
> > +       else if (IS_GEN9_LP(dev_priv))
> >                 I915_WRITE(GEN8_L3_LRA_1_GPGPU,
> > GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT);
> >  }
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index db714dc..7bb3d0a 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -1126,13 +1126,13 @@ static int gen9_init_indirectctx_bb(struct
> > intel_engine_cs *engine,
> >         if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1))
> >                 wa_ctx_emit(batch, index, MI_ARB_ON_OFF | MI_ARB_DISABLE);
> > 
> > -       /* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */
> > +       /* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt,glk */
> >         ret = gen8_emit_flush_coherentl3_wa(engine, batch, index);
> >         if (ret < 0)
> >                 return ret;
> >         index = ret;
> > 
> > -       /* WaDisableGatherAtSetShaderCommonSlice:skl,bxt,kbl */
> > +       /* WaDisableGatherAtSetShaderCommonSlice:skl,bxt,kbl,glk */
> >         wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
> >         wa_ctx_emit_reg(batch, index, COMMON_SLICE_CHICKEN2);
> >         wa_ctx_emit(batch, index, _MASKED_BIT_DISABLE(
> > @@ -1156,7 +1156,7 @@ static int gen9_init_indirectctx_bb(struct
> > intel_engine_cs *engine,
> >                 wa_ctx_emit(batch, index, 0);
> >         }
> > 
> > -       /* WaMediaPoolStateCmdInWABB:bxt */
> > +       /* WaMediaPoolStateCmdInWABB:bxt,glk */
> >         if (HAS_POOLED_EU(engine->i915)) {
> >                 /*
> >                  * EU pool configuration is setup along with golden context
> > diff --git a/drivers/gpu/drm/i915/intel_mocs.c
> > b/drivers/gpu/drm/i915/intel_mocs.c
> > index c787fc4..9c67534 100644
> > --- a/drivers/gpu/drm/i915/intel_mocs.c
> > +++ b/drivers/gpu/drm/i915/intel_mocs.c
> > @@ -191,7 +191,7 @@ static bool get_mocs_settings(struct drm_i915_private
> > *dev_priv,
> >                           "Platform that should have a MOCS table does
> > not.\n");
> >         }
> > 
> > -       /* WaDisableSkipCaching:skl,bxt,kbl */
> > +       /* WaDisableSkipCaching:skl,bxt,kbl,glk */
> >         if (IS_GEN9(dev_priv)) {
> >                 int i;
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index b257343..c8ebf1d 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -65,12 +65,12 @@ static void gen9_init_clock_gating(struct
> > drm_i915_private *dev_priv)
> >         I915_WRITE(GEN8_CONFIG0,
> >                    I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES);
> > 
> > -       /* WaEnableChickenDCPR:skl,bxt,kbl */
> > +       /* WaEnableChickenDCPR:skl,bxt,kbl,glk */
> >         I915_WRITE(GEN8_CHICKEN_DCPR_1,
> >                    I915_READ(GEN8_CHICKEN_DCPR_1) | MASK_WAKEMEM);
> > 
> >         /* WaFbcTurnOffFbcWatermark:skl,bxt,kbl */
> > -       /* WaFbcWakeMemOn:skl,bxt,kbl */
> > +       /* WaFbcWakeMemOn:skl,bxt,kbl,glk */
> >         I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
> >                    DISP_FBC_WM_DIS |
> >                    DISP_FBC_MEMORY_WAKE);
> > @@ -107,6 +107,19 @@ static void bxt_init_clock_gating(struct
> > drm_i915_private *dev_priv)
> >                            PWM1_GATING_DIS | PWM2_GATING_DIS);
> >  }
> > 
> > +static void glk_init_clock_gating(struct drm_i915_private *dev_priv)
> > +{
> > +       gen9_init_clock_gating(dev_priv);
> > +
> > +       /*
> > +        * WaDisablePWMClockGating:glk
> > +        * Backlight PWM may stop in the asserted state, causing backlight
> > +        * to stay fully on.
> > +        */
> > +       I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
> > +                  PWM1_GATING_DIS | PWM2_GATING_DIS);
> > +}
> > +
> >  static void i915_pineview_get_mem_freq(struct drm_i915_private *dev_priv)
> >  {
> >         u32 tmp;
> > @@ -2176,7 +2189,7 @@ static void intel_read_wm_latency(struct
> > drm_i915_private *dev_priv,
> >                 }
> > 
> >                 /*
> > -                * WaWmMemoryReadLatency:skl
> > +                * WaWmMemoryReadLatency:skl,glk
> >                  *
> >                  * punit doesn't take into account the read latency so we
> > need
> >                  * to add 2us to the various latency levels we retrieve from
> > the
> > @@ -7659,8 +7672,10 @@ void intel_init_clock_gating_hooks(struct
> > drm_i915_private *dev_priv)
> >                 dev_priv->display.init_clock_gating =
> > skylake_init_clock_gating;
> >         else if (IS_KABYLAKE(dev_priv))
> >                 dev_priv->display.init_clock_gating =
> > kabylake_init_clock_gating;
> > -       else if (IS_GEN9_LP(dev_priv))
> > +       else if (IS_BROXTON(dev_priv))
> >                 dev_priv->display.init_clock_gating = bxt_init_clock_gating;
> > +       else if (IS_GEMINILAKE(dev_priv))
> > +               dev_priv->display.init_clock_gating = glk_init_clock_gating;
> >         else if (IS_BROADWELL(dev_priv))
> >                 dev_priv->display.init_clock_gating =
> > broadwell_init_clock_gating;
> >         else if (IS_CHERRYVIEW(dev_priv))
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index ab83fc2..e17f339 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -812,10 +812,10 @@ static int gen9_init_workarounds(struct
> > intel_engine_cs *engine)
> >         struct drm_i915_private *dev_priv = engine->i915;
> >         int ret;
> > 
> > -       /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl */
> > +       /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */
> >         I915_WRITE(GEN9_CSFE_CHICKEN1_RCS,
> > _MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));
> > 
> > -       /* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl */
> > +       /* WaEnableLbsSlaRetryTimerDecrement:skl,bxt,kbl,glk */
> >         I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) |
> >                    GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE);
> > 
> > @@ -823,8 +823,8 @@ static int gen9_init_workarounds(struct intel_engine_cs
> > *engine)
> >         I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
> >                    ECOCHK_DIS_TLB);
> > 
> > -       /* WaClearFlowControlGpgpuContextSave:skl,bxt,kbl */
> > -       /* WaDisablePartialInstShootdown:skl,bxt,kbl */
> > +       /* WaClearFlowControlGpgpuContextSave:skl,bxt,kbl,glk */
> > +       /* WaDisablePartialInstShootdown:skl,bxt,kbl,glk */
> >         WA_SET_BIT_MASKED(GEN8_ROW_CHICKEN,
> >                           FLOW_CONTROL_ENABLE |
> >                           PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE);
> > @@ -853,12 +853,12 @@ static int gen9_init_workarounds(struct
> > intel_engine_cs *engine)
> >         WA_SET_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN7,
> >                           GEN9_ENABLE_GPGPU_PREEMPTION);
> > 
> > -       /* Wa4x4STCOptimizationDisable:skl,bxt,kbl */
> > +       /* Wa4x4STCOptimizationDisable:skl,bxt,kbl,glk */
> >         /* WaDisablePartialResolveInVc:skl,bxt,kbl */
> >         WA_SET_BIT_MASKED(CACHE_MODE_1, (GEN8_4x4_STC_OPTIMIZATION_DISABLE |
> >                                          GEN9_PARTIAL_RESOLVE_IN_VC_DISABLE)
> > );
> > 
> > -       /* WaCcsTlbPrefetchDisable:skl,bxt,kbl */
> > +       /* WaCcsTlbPrefetchDisable:skl,bxt,kbl,glk */
> >         WA_CLR_BIT_MASKED(GEN9_HALF_SLICE_CHICKEN5,
> >                           GEN9_CCS_TLB_PREFETCH_ENABLE);
> > 
> > @@ -900,14 +900,14 @@ static int gen9_init_workarounds(struct
> > intel_engine_cs *engine)
> >                 WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN3,
> >                                   GEN8_SAMPLER_POWER_BYPASS_DIS);
> > 
> > -       /* WaDisableSTUnitPowerOptimization:skl,bxt,kbl */
> > +       /* WaDisableSTUnitPowerOptimization:skl,bxt,kbl,glk */
> >         WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE);
> > 
> >         /* WaOCLCoherentLineFlush:skl,bxt,kbl */
> >         I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) |
> >                                     GEN8_LQSC_FLUSH_COHERENT_LINES));
> > 
> > -       /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt */
> > +       /* WaVFEStateAfterPipeControlwithMediaStateClear:skl,bxt,glk */
> >         ret = wa_ring_whitelist_reg(engine, GEN9_CTX_PREEMPT_REG);
> >         if (ret)
> >                 return ret;
> > @@ -917,7 +917,7 @@ static int gen9_init_workarounds(struct intel_engine_cs
> > *engine)
> >         if (ret)
> >                 return ret;
> > 
> > -       /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl */
> > +       /* WaAllowUMDToModifyHDCChicken1:skl,bxt,kbl,glk */
> >         ret = wa_ring_whitelist_reg(engine, GEN8_HDC_CHICKEN1);
> >         if (ret)
> >                 return ret;
> > @@ -1128,6 +1128,22 @@ static int kbl_init_workarounds(struct
> > intel_engine_cs *engine)
> >         return 0;
> >  }
> > 
> > +static int glk_init_workarounds(struct intel_engine_cs *engine)
> > +{
> > +       struct drm_i915_private *dev_priv = engine->i915;
> > +       int ret;
> > +
> > +       ret = gen9_init_workarounds(engine);
> > +       if (ret)
> > +               return ret;
> > +
> > +       /* WaToEnableHwFixForPushConstHWBug:glk */
> My first thought on this was wondering if we should have that for KBL and
> BXT...
> but then I noticed it is only up to C0 there while here is forever....
> 
> > 
> > +       WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
> > +                         GEN8_SBE_DISABLE_REPLAY_BUF_OPTIMIZATION);
> But also how did you come up with these bit?
> Resolution I see there tells to reset some counter with invalidate
> or something like inserting 2 zero length push PS at some update...
> not sure, but noting that pointed me to this bit here..

If I understood correctly, WaInsertDummyPushConstPs is the dummy push
workaround. That is fixed in GKL, BXT C0 and KBL C0, but requires this bit to
enable the fix. Enabling the fix is called WaToEnableHwFixForPushConstHWBug.

Is this correct Matthew?

Thanks,
Ander

> > +
> > +       return 0;
> > +}
> > +
> >  int init_workarounds_ring(struct intel_engine_cs *engine)
> >  {
> >         struct drm_i915_private *dev_priv = engine->i915;
> > @@ -1152,6 +1168,9 @@ int init_workarounds_ring(struct intel_engine_cs
> > *engine)
> >         if (IS_KABYLAKE(dev_priv))
> >                 return kbl_init_workarounds(engine);
> > 
> > +       if (IS_GEMINILAKE(dev_priv))
> > +               return glk_init_workarounds(engine);
> > +
> >         return 0;
> >  }
> > 
> > --
> > 2.5.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx