| Message ID | 1510664703-4580-1-git-send-email-valtteri.rantala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 14, 2017 at 03:05:03PM +0200, Valtteri Rantala wrote: > Testing the texture read performance shows that the same tuning for the > SQ credits is needed on GLK as on BXT/APL. This has been also confirmed > by Altug from the HW team. > > Cc: Altug Koker <altug.koker@intel.com> > Signed-off-by: Valtteri Rantala <valtteri.rantala@intel.com> With comments below, and unless Altug has objections: Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 70bbe8e..11fc0bd 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -1093,6 +1093,15 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) > /* WaDisableSTUnitPowerOptimization:skl,bxt,kbl,glk,cfl */ > WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */ > + if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER) || > + IS_GEMINILAKE(dev_priv)) { Since we've dropped workarounds for pre-production Broxtons, I think this could/should be simplified to: if (IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv)) { or even if (IS_GEN9_LP(dev_priv)) { But that's just nitpicking. The fix is sound from a performance perspective. > + 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); > + } > + > /* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */ > I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) | > GEN8_LQSC_FLUSH_COHERENT_LINES)); > @@ -1258,14 +1267,6 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine) > return ret; > } > > - /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > - 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)) > WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, > -- > 2.7.4
On Wed, Nov 15, 2017 at 07:52:05PM +0200, David Weinehall wrote: > On Tue, Nov 14, 2017 at 03:05:03PM +0200, Valtteri Rantala wrote: > > Testing the texture read performance shows that the same tuning for the > > SQ credits is needed on GLK as on BXT/APL. This has been also confirmed > > by Altug from the HW team. > > > > Cc: Altug Koker <altug.koker@intel.com> > > Signed-off-by: Valtteri Rantala <valtteri.rantala@intel.com> > > With comments below, and unless Altug has objections: > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_engine_cs.c | 17 +++++++++-------- > > 1 file changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > > index 70bbe8e..11fc0bd 100644 > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > @@ -1093,6 +1093,15 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) > > /* WaDisableSTUnitPowerOptimization:skl,bxt,kbl,glk,cfl */ > > WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); > > > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */ > > + if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER) || > > + IS_GEMINILAKE(dev_priv)) { > > Since we've dropped workarounds for pre-production Broxtons, > I think this could/should be simplified to: > > if (IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv)) { > > or even > > if (IS_GEN9_LP(dev_priv)) { > > But that's just nitpicking. The fix is sound from a performance > perspective. Yes, meanwhile commit 70a84f3c6075031dbf004a1610ca2471f4c528aa Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Tue Nov 14 13:43:40 2017 +0000 drm/i915: Unconditionally apply the Broxton register workaround set was merged, so we should use IS_GEN9_LP() above. Valtteri, could you rebase/resend the patch? > > > + 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); > > + } > > + > > /* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */ > > I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) | > > GEN8_LQSC_FLUSH_COHERENT_LINES)); > > @@ -1258,14 +1267,6 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine) > > return ret; > > } > > > > - /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > - 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)) > > WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, > > -- > > 2.7.4 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Deak, Imre > Sent: Wednesday, November 22, 2017 2:24 PM > To: Rantala, Valtteri <valtteri.rantala@intel.com>; intel- > gfx@lists.freedesktop.org; Koker, Altug <altug.koker@intel.com>; David > Weinehall <david.weinehall@linux.intel.com> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: Apply > WaProgramL3SqcReg1DefaultForPerf for GLK too > > On Wed, Nov 15, 2017 at 07:52:05PM +0200, David Weinehall wrote: > > On Tue, Nov 14, 2017 at 03:05:03PM +0200, Valtteri Rantala wrote: > > > Testing the texture read performance shows that the same tuning for > > > the SQ credits is needed on GLK as on BXT/APL. This has been also > > > confirmed by Altug from the HW team. > > > > > > Cc: Altug Koker <altug.koker@intel.com> > > > Signed-off-by: Valtteri Rantala <valtteri.rantala@intel.com> > > > > With comments below, and unless Altug has objections: > > > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_engine_cs.c | 17 +++++++++-------- > > > 1 file changed, 9 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > > > b/drivers/gpu/drm/i915/intel_engine_cs.c > > > index 70bbe8e..11fc0bd 100644 > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > > > @@ -1093,6 +1093,15 @@ static int gen9_init_workarounds(struct > intel_engine_cs *engine) > > > /* WaDisableSTUnitPowerOptimization:skl,bxt,kbl,glk,cfl */ > > > WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); > > > > > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */ > > > + if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER) || > > > + IS_GEMINILAKE(dev_priv)) { > > > > Since we've dropped workarounds for pre-production Broxtons, I think > > this could/should be simplified to: > > > > if (IS_BROXTON(dev_priv) || IS_GEMINILAKE(dev_priv)) { > > > > or even > > > > if (IS_GEN9_LP(dev_priv)) { > > > > But that's just nitpicking. The fix is sound from a performance > > perspective. > > Yes, meanwhile > > commit 70a84f3c6075031dbf004a1610ca2471f4c528aa > Author: Chris Wilson <chris@chris-wilson.co.uk> > Date: Tue Nov 14 13:43:40 2017 +0000 > > drm/i915: Unconditionally apply the Broxton register workaround set > > was merged, so we should use IS_GEN9_LP() above. Valtteri, could you > rebase/resend the patch? Yes, I'll rebase/resend the patch. I'll use IS_GEN9_LP(). > > > > > > + 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); > > > + } > > > + > > > /* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */ > > > I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) | > > > GEN8_LQSC_FLUSH_COHERENT_LINES)); > @@ -1258,14 +1267,6 @@ > > > static int bxt_init_workarounds(struct intel_engine_cs *engine) > > > return ret; > > > } > > > > > > - /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > > - 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)) > > > WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2, > > > -- > > > 2.7.4 > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 70bbe8e..11fc0bd 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -1093,6 +1093,15 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) /* WaDisableSTUnitPowerOptimization:skl,bxt,kbl,glk,cfl */ WA_SET_BIT_MASKED(HALF_SLICE_CHICKEN2, GEN8_ST_PO_DISABLE); + /* WaProgramL3SqcReg1DefaultForPerf:bxt,glk */ + if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER) || + IS_GEMINILAKE(dev_priv)) { + 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); + } + /* WaOCLCoherentLineFlush:skl,bxt,kbl,cfl */ I915_WRITE(GEN8_L3SQCREG4, (I915_READ(GEN8_L3SQCREG4) | GEN8_LQSC_FLUSH_COHERENT_LINES)); @@ -1258,14 +1267,6 @@ static int bxt_init_workarounds(struct intel_engine_cs *engine) return ret; } - /* WaProgramL3SqcReg1DefaultForPerf:bxt */ - 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)) WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
Testing the texture read performance shows that the same tuning for the SQ credits is needed on GLK as on BXT/APL. This has been also confirmed by Altug from the HW team. Cc: Altug Koker <altug.koker@intel.com> Signed-off-by: Valtteri Rantala <valtteri.rantala@intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)