| Message ID | 1507928056-6966-2-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Oscar Mateo (2017-10-13 21:53:55) > Otherwise we are blasting other bits in GEN8_L3SQCREG1 that might be important > (although we probably aren't at the moment because 0 seems to be the default > for all the other bits). > > Fixes: 050fc46 ("drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf") > Fixes: 450174f ("drm/i915/chv: Tune L3 SQC credits based on actual latencies") > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> If this gets lost in the series, please send separately to bring attention to the bugfix. Hopefully r-b's will be prompt... > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_engine_cs.c | 9 ++++++--- > drivers/gpu/drm/i915/intel_pm.c | 9 ++++++--- > 3 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d2d0a83..5858f5f 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7041,6 +7041,7 @@ enum { > */ > #define L3_GENERAL_PRIO_CREDITS(x) (((x) >> 1) << 19) > #define L3_HIGH_PRIO_CREDITS(x) (((x) >> 1) << 14) > +#define L3_PRIO_CREDITS_MASK (0x1f << 19) | (0x1f << 14) > > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47FF8C > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index a59b2a3..40e439c 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -1252,9 +1252,12 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine) > } > > /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > - if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) > - I915_WRITE(GEN8_L3SQCREG1, L3_GENERAL_PRIO_CREDITS(62) | > - L3_HIGH_PRIO_CREDITS(2)); > + if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) { > + u32 val = I915_READ(GEN8_L3SQCREG1); > + val &= ~L3_PRIO_CREDITS_MASK; > + val |= L3_GENERAL_PRIO_CREDITS(62) | L3_HIGH_PRIO_CREDITS(2); > + I915_WRITE(GEN8_L3SQCREG1, val); > + } > > /* WaToEnableHwFixForPushConstHWBug:bxt */ > if (IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER)) > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 2fcff97..6c2c8d8 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -8476,15 +8476,18 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv, > int general_prio_credits, > int high_prio_credits) > { > + u32 val; > u32 misccpctl; > > /* WaTempDisableDOPClkGating:bdw */ > misccpctl = I915_READ(GEN7_MISCCPCTL); > I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE); > > - I915_WRITE(GEN8_L3SQCREG1, > - L3_GENERAL_PRIO_CREDITS(general_prio_credits) | > - L3_HIGH_PRIO_CREDITS(high_prio_credits)); > + val = I915_READ(GEN8_L3SQCREG1); > + val &= ~L3_PRIO_CREDITS_MASK; > + val |= L3_GENERAL_PRIO_CREDITS(general_prio_credits); > + val |= L3_HIGH_PRIO_CREDITS(high_prio_credits); > + I915_WRITE(GEN8_L3SQCREG1, val); > > /* > * Wait at least 100 clocks before re-enabling clock gating. > -- > 1.9.1 >
On 10/13/2017 2:28 PM, Chris Wilson wrote: > Quoting Oscar Mateo (2017-10-13 21:53:55) >> Otherwise we are blasting other bits in GEN8_L3SQCREG1 that might be important >> (although we probably aren't at the moment because 0 seems to be the default >> for all the other bits). >> >> Fixes: 050fc46 ("drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf") >> Fixes: 450174f ("drm/i915/chv: Tune L3 SQC credits based on actual latencies") >> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Imre Deak <imre.deak@intel.com> >> Cc: Michel Thierry <michel.thierry@intel.com> > > If this gets lost in the series, please send separately to bring > attention to the bugfix. Hopefully r-b's will be prompt... > This is what happens when someone copies workarounds from another driver verbatim :S Luckily all the other fields should be 0's. >> --- >> drivers/gpu/drm/i915/i915_reg.h | 1 + >> drivers/gpu/drm/i915/intel_engine_cs.c | 9 ++++++--- >> drivers/gpu/drm/i915/intel_pm.c | 9 ++++++--- >> 3 files changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index d2d0a83..5858f5f 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -7041,6 +7041,7 @@ enum { >> */ >> #define L3_GENERAL_PRIO_CREDITS(x) (((x) >> 1) << 19) >> #define L3_HIGH_PRIO_CREDITS(x) (((x) >> 1) << 14) >> +#define L3_PRIO_CREDITS_MASK (0x1f << 19) | (0x1f << 14) checkpatch.pl is asking to enclose this macro^^^ in parentheses, +#define L3_PRIO_CREDITS_MASK ((0x1f << 19) | (0x1f << 14)) With that, Reviewed-by: Michel Thierry <michel.thierry@intel.com> >> >> #define GEN7_L3CNTLREG1 _MMIO(0xB01C) >> #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47FF8C >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c >> index a59b2a3..40e439c 100644 >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >> @@ -1252,9 +1252,12 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine) >> } >> >> /* WaProgramL3SqcReg1DefaultForPerf:bxt */ >> - if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) >> - I915_WRITE(GEN8_L3SQCREG1, L3_GENERAL_PRIO_CREDITS(62) | >> - L3_HIGH_PRIO_CREDITS(2)); >> + if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) { >> + u32 val = I915_READ(GEN8_L3SQCREG1); >> + val &= ~L3_PRIO_CREDITS_MASK; >> + val |= L3_GENERAL_PRIO_CREDITS(62) | L3_HIGH_PRIO_CREDITS(2); >> + I915_WRITE(GEN8_L3SQCREG1, val); >> + } >> >> /* WaToEnableHwFixForPushConstHWBug:bxt */ >> if (IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER)) >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 2fcff97..6c2c8d8 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -8476,15 +8476,18 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv, >> int general_prio_credits, >> int high_prio_credits) >> { >> + u32 val; >> u32 misccpctl; >> >> /* WaTempDisableDOPClkGating:bdw */ >> misccpctl = I915_READ(GEN7_MISCCPCTL); >> I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE); >> >> - I915_WRITE(GEN8_L3SQCREG1, >> - L3_GENERAL_PRIO_CREDITS(general_prio_credits) | >> - L3_HIGH_PRIO_CREDITS(high_prio_credits)); >> + val = I915_READ(GEN8_L3SQCREG1); >> + val &= ~L3_PRIO_CREDITS_MASK; >> + val |= L3_GENERAL_PRIO_CREDITS(general_prio_credits); >> + val |= L3_HIGH_PRIO_CREDITS(high_prio_credits); >> + I915_WRITE(GEN8_L3SQCREG1, val); >> >> /* >> * Wait at least 100 clocks before re-enabling clock gating. >> -- >> 1.9.1 >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d2d0a83..5858f5f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7041,6 +7041,7 @@ enum { */ #define L3_GENERAL_PRIO_CREDITS(x) (((x) >> 1) << 19) #define L3_HIGH_PRIO_CREDITS(x) (((x) >> 1) << 14) +#define L3_PRIO_CREDITS_MASK (0x1f << 19) | (0x1f << 14) #define GEN7_L3CNTLREG1 _MMIO(0xB01C) #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47FF8C diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a59b2a3..40e439c 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1252,9 +1252,12 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine) } /* WaProgramL3SqcReg1DefaultForPerf:bxt */ - if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) - I915_WRITE(GEN8_L3SQCREG1, L3_GENERAL_PRIO_CREDITS(62) | - L3_HIGH_PRIO_CREDITS(2)); + if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) { + u32 val = I915_READ(GEN8_L3SQCREG1); + val &= ~L3_PRIO_CREDITS_MASK; + val |= L3_GENERAL_PRIO_CREDITS(62) | L3_HIGH_PRIO_CREDITS(2); + I915_WRITE(GEN8_L3SQCREG1, val); + } /* WaToEnableHwFixForPushConstHWBug:bxt */ if (IS_BXT_REVID(dev_priv, BXT_REVID_C0, REVID_FOREVER)) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2fcff97..6c2c8d8 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -8476,15 +8476,18 @@ static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv, int general_prio_credits, int high_prio_credits) { + u32 val; u32 misccpctl; /* WaTempDisableDOPClkGating:bdw */ misccpctl = I915_READ(GEN7_MISCCPCTL); I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE); - I915_WRITE(GEN8_L3SQCREG1, - L3_GENERAL_PRIO_CREDITS(general_prio_credits) | - L3_HIGH_PRIO_CREDITS(high_prio_credits)); + val = I915_READ(GEN8_L3SQCREG1); + val &= ~L3_PRIO_CREDITS_MASK; + val |= L3_GENERAL_PRIO_CREDITS(general_prio_credits); + val |= L3_HIGH_PRIO_CREDITS(high_prio_credits); + I915_WRITE(GEN8_L3SQCREG1, val); /* * Wait at least 100 clocks before re-enabling clock gating.
Otherwise we are blasting other bits in GEN8_L3SQCREG1 that might be important (although we probably aren't at the moment because 0 seems to be the default for all the other bits). Fixes: 050fc46 ("drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf") Fixes: 450174f ("drm/i915/chv: Tune L3 SQC credits based on actual latencies") Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_engine_cs.c | 9 ++++++--- drivers/gpu/drm/i915/intel_pm.c | 9 ++++++--- 3 files changed, 13 insertions(+), 6 deletions(-)