* [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf @ 2016-04-20 14:23 tim.gore 2016-04-20 15:00 ` Michel Thierry 2016-04-21 12:40 ` ✗ Fi.CI.BAT: failure for drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf (rev2) Patchwork 0 siblings, 2 replies; 10+ messages in thread From: tim.gore @ 2016-04-20 14:23 UTC (permalink / raw) To: intel-gfx From: Tim Gore <tim.gore@intel.com> This patch applies a performance enhancement workaround based on analysis of DX and OCL S-Curve workloads. v2: Only apply to B0 onwards Signed-off-by: Tim Gore <tim.gore@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f0a6d85..13e154a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { #define GEN8_L3SQCREG1 _MMIO(0xB100) #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 #define GEN7_L3CNTLREG1 _MMIO(0xB01C) #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47FF8C diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b7c2186..eecdc3a6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct drm_device *dev) if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) | PWM1_GATING_DIS | PWM2_GATING_DIS); + + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) + I915_WRITE(GEN8_L3SQCREG1, BXT_WA_L3SQCREG1_DEFAULT); } static void i915_pineview_get_mem_freq(struct drm_device *dev) -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf 2016-04-20 14:23 [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf tim.gore @ 2016-04-20 15:00 ` Michel Thierry 2016-04-20 15:51 ` Gore, Tim 2016-04-21 12:40 ` ✗ Fi.CI.BAT: failure for drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf (rev2) Patchwork 1 sibling, 1 reply; 10+ messages in thread From: Michel Thierry @ 2016-04-20 15:00 UTC (permalink / raw) To: tim.gore, intel-gfx On 4/20/2016 3:23 PM, tim.gore@intel.com wrote: > From: Tim Gore <tim.gore@intel.com> > > This patch applies a performance enhancement workaround > based on analysis of DX and OCL S-Curve workloads. > > v2: Only apply to B0 onwards > > Signed-off-by: Tim Gore <tim.gore@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index f0a6d85..13e154a 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { > > #define GEN8_L3SQCREG1 _MMIO(0xB100) > #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 > > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > #define GEN7_WA_FOR_GEN7_L3_CONTROL 0x3C47FF8C > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index b7c2186..eecdc3a6 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct drm_device *dev) > if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) > I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) | > PWM1_GATING_DIS | PWM2_GATING_DIS); > + > + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) > + I915_WRITE(GEN8_L3SQCREG1, BXT_WA_L3SQCREG1_DEFAULT); > } > > static void i915_pineview_get_mem_freq(struct drm_device *dev) > Isnt it better to add this to bxt_init_workarounds() instead of bxt_init_clock_gating()? And It doesnt hurt to put what it is doing in the commit msg, i.e.: "Increasing GP credits to 62 (from default 56), and reducing HP credits to 2 (from default 8)" -Michel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf 2016-04-20 15:00 ` Michel Thierry @ 2016-04-20 15:51 ` Gore, Tim 2016-04-20 18:19 ` Chris Wilson 0 siblings, 1 reply; 10+ messages in thread From: Gore, Tim @ 2016-04-20 15:51 UTC (permalink / raw) To: Thierry, Michel, intel-gfx@lists.freedesktop.org Tim Gore Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > -----Original Message----- > From: Thierry, Michel > Sent: Wednesday, April 20, 2016 4:00 PM > To: Gore, Tim; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH v2] drm/i915:bxt: implement > WaProgramL3SqcReg1DefaultForPerf > > On 4/20/2016 3:23 PM, tim.gore@intel.com wrote: > > From: Tim Gore <tim.gore@intel.com> > > > > This patch applies a performance enhancement workaround based on > > analysis of DX and OCL S-Curve workloads. > > > > v2: Only apply to B0 onwards > > > > Signed-off-by: Tim Gore <tim.gore@intel.com> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h index f0a6d85..13e154a 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { > > > > #define GEN8_L3SQCREG1 _MMIO(0xB100) > > #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 > > > > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > > #define GEN7_WA_FOR_GEN7_L3_CONTROL > 0x3C47FF8C > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c index b7c2186..eecdc3a6 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct drm_device > *dev) > > if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) > > I915_WRITE(GEN9_CLKGATE_DIS_0, > I915_READ(GEN9_CLKGATE_DIS_0) | > > PWM1_GATING_DIS | PWM2_GATING_DIS); > > + > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) > > + I915_WRITE(GEN8_L3SQCREG1, > BXT_WA_L3SQCREG1_DEFAULT); > > } > > > > static void i915_pineview_get_mem_freq(struct drm_device *dev) > > > > Isnt it better to add this to bxt_init_workarounds() instead of > bxt_init_clock_gating()? There is an equivalent Wa for bdw in bdw_init_clock_gating, so I just tried to stay consistent with that. I'm not sure if this W/a will persist across suspend/resume. bxt_init_workarounds only gets called after reset, not on resume as far as I know. I'll investigate further > > And It doesnt hurt to put what it is doing in the commit msg, i.e.: > "Increasing GP credits to 62 (from default 56), and reducing HP credits to 2 > (from default 8)" Fair comment > > -Michel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf 2016-04-20 15:51 ` Gore, Tim @ 2016-04-20 18:19 ` Chris Wilson 2016-04-20 18:31 ` Ville Syrjälä 0 siblings, 1 reply; 10+ messages in thread From: Chris Wilson @ 2016-04-20 18:19 UTC (permalink / raw) To: Gore, Tim; +Cc: intel-gfx@lists.freedesktop.org On Wed, Apr 20, 2016 at 03:51:49PM +0000, Gore, Tim wrote: > > Tim Gore > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > > > > -----Original Message----- > > From: Thierry, Michel > > Sent: Wednesday, April 20, 2016 4:00 PM > > To: Gore, Tim; intel-gfx@lists.freedesktop.org > > Subject: Re: [PATCH v2] drm/i915:bxt: implement > > WaProgramL3SqcReg1DefaultForPerf > > > > On 4/20/2016 3:23 PM, tim.gore@intel.com wrote: > > > From: Tim Gore <tim.gore@intel.com> > > > > > > This patch applies a performance enhancement workaround based on > > > analysis of DX and OCL S-Curve workloads. > > > > > > v2: Only apply to B0 onwards > > > > > > Signed-off-by: Tim Gore <tim.gore@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > > > 2 files changed, 5 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h index f0a6d85..13e154a 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { > > > > > > #define GEN8_L3SQCREG1 _MMIO(0xB100) > > > #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > > +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 > > > > > > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > > > #define GEN7_WA_FOR_GEN7_L3_CONTROL > > 0x3C47FF8C > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c index b7c2186..eecdc3a6 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct drm_device > > *dev) > > > if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) > > > I915_WRITE(GEN9_CLKGATE_DIS_0, > > I915_READ(GEN9_CLKGATE_DIS_0) | > > > PWM1_GATING_DIS | PWM2_GATING_DIS); > > > + > > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > > + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) > > > + I915_WRITE(GEN8_L3SQCREG1, > > BXT_WA_L3SQCREG1_DEFAULT); > > > } > > > > > > static void i915_pineview_get_mem_freq(struct drm_device *dev) > > > > > > > Isnt it better to add this to bxt_init_workarounds() instead of > > bxt_init_clock_gating()? > > There is an equivalent Wa for bdw in bdw_init_clock_gating, so I just tried to > stay consistent with that. I'm not sure if this W/a will persist across > suspend/resume. bxt_init_workarounds only gets called after reset, not > on resume as far as I know. I'll investigate further init_clock_gating() is called on init/reset/resume. Use it to set global registers. (Once upon a time it did only setup the clock gatings...) init_workarounds() is the equivalent for per-context registers. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf 2016-04-20 18:19 ` Chris Wilson @ 2016-04-20 18:31 ` Ville Syrjälä 2016-04-20 18:47 ` Chris Wilson 0 siblings, 1 reply; 10+ messages in thread From: Ville Syrjälä @ 2016-04-20 18:31 UTC (permalink / raw) To: Chris Wilson, Gore, Tim, Thierry, Michel, intel-gfx@lists.freedesktop.org On Wed, Apr 20, 2016 at 07:19:32PM +0100, Chris Wilson wrote: > On Wed, Apr 20, 2016 at 03:51:49PM +0000, Gore, Tim wrote: > > > > Tim Gore > > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > > > > > > > -----Original Message----- > > > From: Thierry, Michel > > > Sent: Wednesday, April 20, 2016 4:00 PM > > > To: Gore, Tim; intel-gfx@lists.freedesktop.org > > > Subject: Re: [PATCH v2] drm/i915:bxt: implement > > > WaProgramL3SqcReg1DefaultForPerf > > > > > > On 4/20/2016 3:23 PM, tim.gore@intel.com wrote: > > > > From: Tim Gore <tim.gore@intel.com> > > > > > > > > This patch applies a performance enhancement workaround based on > > > > analysis of DX and OCL S-Curve workloads. > > > > > > > > v2: Only apply to B0 onwards > > > > > > > > Signed-off-by: Tim Gore <tim.gore@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > > > > 2 files changed, 5 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > b/drivers/gpu/drm/i915/i915_reg.h index f0a6d85..13e154a 100644 > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { > > > > > > > > #define GEN8_L3SQCREG1 _MMIO(0xB100) > > > > #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > > > +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 > > > > > > > > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > > > > #define GEN7_WA_FOR_GEN7_L3_CONTROL > > > 0x3C47FF8C > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > b/drivers/gpu/drm/i915/intel_pm.c index b7c2186..eecdc3a6 100644 > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct drm_device > > > *dev) > > > > if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) > > > > I915_WRITE(GEN9_CLKGATE_DIS_0, > > > I915_READ(GEN9_CLKGATE_DIS_0) | > > > > PWM1_GATING_DIS | PWM2_GATING_DIS); > > > > + > > > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > > > + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) > > > > + I915_WRITE(GEN8_L3SQCREG1, > > > BXT_WA_L3SQCREG1_DEFAULT); > > > > } > > > > > > > > static void i915_pineview_get_mem_freq(struct drm_device *dev) > > > > > > > > > > Isnt it better to add this to bxt_init_workarounds() instead of > > > bxt_init_clock_gating()? > > > > There is an equivalent Wa for bdw in bdw_init_clock_gating, so I just tried to > > stay consistent with that. I'm not sure if this W/a will persist across > > suspend/resume. bxt_init_workarounds only gets called after reset, not > > on resume as far as I know. I'll investigate further > > init_clock_gating() is called on init/reset/resume. Use it to set global > registers. (Once upon a time it did only setup the clock gatings...) It's not called on reset. Or at least that was the case last time I looked. Which is a rather big problem. My old idea was that we'd move anything that gets clobbered by a GPU reset out from init_clock_gating into some more suitable place that does get called on reset. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf 2016-04-20 18:31 ` Ville Syrjälä @ 2016-04-20 18:47 ` Chris Wilson 2016-04-21 8:19 ` Gore, Tim 2016-04-21 9:15 ` Daniel Vetter 0 siblings, 2 replies; 10+ messages in thread From: Chris Wilson @ 2016-04-20 18:47 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Daniel Vetter, intel-gfx@lists.freedesktop.org On Wed, Apr 20, 2016 at 09:31:57PM +0300, Ville Syrjälä wrote: > On Wed, Apr 20, 2016 at 07:19:32PM +0100, Chris Wilson wrote: > > On Wed, Apr 20, 2016 at 03:51:49PM +0000, Gore, Tim wrote: > > > > > > Tim Gore > > > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > > > > > > > > > > -----Original Message----- > > > > From: Thierry, Michel > > > > Sent: Wednesday, April 20, 2016 4:00 PM > > > > To: Gore, Tim; intel-gfx@lists.freedesktop.org > > > > Subject: Re: [PATCH v2] drm/i915:bxt: implement > > > > WaProgramL3SqcReg1DefaultForPerf > > > > > > > > On 4/20/2016 3:23 PM, tim.gore@intel.com wrote: > > > > > From: Tim Gore <tim.gore@intel.com> > > > > > > > > > > This patch applies a performance enhancement workaround based on > > > > > analysis of DX and OCL S-Curve workloads. > > > > > > > > > > v2: Only apply to B0 onwards > > > > > > > > > > Signed-off-by: Tim Gore <tim.gore@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > > > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > > > > > 2 files changed, 5 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > b/drivers/gpu/drm/i915/i915_reg.h index f0a6d85..13e154a 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { > > > > > > > > > > #define GEN8_L3SQCREG1 _MMIO(0xB100) > > > > > #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > > > > +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 > > > > > > > > > > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > > > > > #define GEN7_WA_FOR_GEN7_L3_CONTROL > > > > 0x3C47FF8C > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > > b/drivers/gpu/drm/i915/intel_pm.c index b7c2186..eecdc3a6 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > > @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct drm_device > > > > *dev) > > > > > if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) > > > > > I915_WRITE(GEN9_CLKGATE_DIS_0, > > > > I915_READ(GEN9_CLKGATE_DIS_0) | > > > > > PWM1_GATING_DIS | PWM2_GATING_DIS); > > > > > + > > > > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > > > > + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) > > > > > + I915_WRITE(GEN8_L3SQCREG1, > > > > BXT_WA_L3SQCREG1_DEFAULT); > > > > > } > > > > > > > > > > static void i915_pineview_get_mem_freq(struct drm_device *dev) > > > > > > > > > > > > > Isnt it better to add this to bxt_init_workarounds() instead of > > > > bxt_init_clock_gating()? > > > > > > There is an equivalent Wa for bdw in bdw_init_clock_gating, so I just tried to > > > stay consistent with that. I'm not sure if this W/a will persist across > > > suspend/resume. bxt_init_workarounds only gets called after reset, not > > > on resume as far as I know. I'll investigate further > > > > init_clock_gating() is called on init/reset/resume. Use it to set global > > registers. (Once upon a time it did only setup the clock gatings...) > > It's not called on reset. Or at least that was the case last time I > looked. Which is a rather big problem. My old idea was that we'd move > anything that gets clobbered by a GPU reset out from init_clock_gating > into some more suitable place that does get called on reset. Oh, I call upon Daniel! -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf 2016-04-20 18:47 ` Chris Wilson @ 2016-04-21 8:19 ` Gore, Tim 2016-04-21 9:16 ` Daniel Vetter 2016-04-21 9:15 ` Daniel Vetter 1 sibling, 1 reply; 10+ messages in thread From: Gore, Tim @ 2016-04-21 8:19 UTC (permalink / raw) To: Chris Wilson, Ville Syrjälä Cc: Daniel Vetter, intel-gfx@lists.freedesktop.org Tim Gore Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Wednesday, April 20, 2016 7:48 PM > To: Ville Syrjälä > Cc: Gore, Tim; Daniel Vetter; Thierry, Michel; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915:bxt: implement > WaProgramL3SqcReg1DefaultForPerf > > On Wed, Apr 20, 2016 at 09:31:57PM +0300, Ville Syrjälä wrote: > > On Wed, Apr 20, 2016 at 07:19:32PM +0100, Chris Wilson wrote: > > > On Wed, Apr 20, 2016 at 03:51:49PM +0000, Gore, Tim wrote: > > > > > > > > Tim Gore > > > > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, > > > > Swindon SN3 1RJ > > > > > > > > > > > > > -----Original Message----- > > > > > From: Thierry, Michel > > > > > Sent: Wednesday, April 20, 2016 4:00 PM > > > > > To: Gore, Tim; intel-gfx@lists.freedesktop.org > > > > > Subject: Re: [PATCH v2] drm/i915:bxt: implement > > > > > WaProgramL3SqcReg1DefaultForPerf > > > > > > > > > > On 4/20/2016 3:23 PM, tim.gore@intel.com wrote: > > > > > > From: Tim Gore <tim.gore@intel.com> > > > > > > > > > > > > This patch applies a performance enhancement workaround based > > > > > > on analysis of DX and OCL S-Curve workloads. > > > > > > > > > > > > v2: Only apply to B0 onwards > > > > > > > > > > > > Signed-off-by: Tim Gore <tim.gore@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > > > > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > > > > > > 2 files changed, 5 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > > b/drivers/gpu/drm/i915/i915_reg.h index f0a6d85..13e154a > > > > > > 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > > @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { > > > > > > > > > > > > #define GEN8_L3SQCREG1 > _MMIO(0xB100) > > > > > > #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > > > > > +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 > > > > > > > > > > > > #define GEN7_L3CNTLREG1 > _MMIO(0xB01C) > > > > > > #define GEN7_WA_FOR_GEN7_L3_CONTROL > > > > > 0x3C47FF8C > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > > > b/drivers/gpu/drm/i915/intel_pm.c index b7c2186..eecdc3a6 > > > > > > 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > > > @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct > > > > > > drm_device > > > > > *dev) > > > > > > if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, > REVID_FOREVER)) > > > > > > I915_WRITE(GEN9_CLKGATE_DIS_0, > > > > > I915_READ(GEN9_CLKGATE_DIS_0) | > > > > > > PWM1_GATING_DIS | > PWM2_GATING_DIS); > > > > > > + > > > > > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > > > > > + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) > > > > > > + I915_WRITE(GEN8_L3SQCREG1, > > > > > BXT_WA_L3SQCREG1_DEFAULT); > > > > > > } > > > > > > > > > > > > static void i915_pineview_get_mem_freq(struct drm_device > > > > > > *dev) > > > > > > > > > > > > > > > > Isnt it better to add this to bxt_init_workarounds() instead of > > > > > bxt_init_clock_gating()? > > > > > > > > There is an equivalent Wa for bdw in bdw_init_clock_gating, so I > > > > just tried to stay consistent with that. I'm not sure if this W/a > > > > will persist across suspend/resume. bxt_init_workarounds only gets > > > > called after reset, not on resume as far as I know. I'll > > > > investigate further > > > > > > init_clock_gating() is called on init/reset/resume. Use it to set > > > global registers. (Once upon a time it did only setup the clock > > > gatings...) > > > > It's not called on reset. Or at least that was the case last time I > > looked. Which is a rather big problem. My old idea was that we'd move > > anything that gets clobbered by a GPU reset out from init_clock_gating > > into some more suitable place that does get called on reset. > > Oh, I call upon Daniel! > -Chris > Looks to me that init_clock_gating is called on reset thus: I915_handle_error -> i915_reset_and_wakeup -> intel_finish_reset -> intel_modeset_init_hw -> intel_init_clock_gating -> display.init_clock_gating Or perhaps we should put WaProgramL3SqcReg1DefaultForPerf in the new gtt_write_workarounds function in i915_gem_gtt.c. I believe this is called on Init, reset and resume. ? Tim > Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf 2016-04-21 8:19 ` Gore, Tim @ 2016-04-21 9:16 ` Daniel Vetter 0 siblings, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2016-04-21 9:16 UTC (permalink / raw) To: Gore, Tim; +Cc: Daniel Vetter, intel-gfx@lists.freedesktop.org On Thu, Apr 21, 2016 at 08:19:46AM +0000, Gore, Tim wrote: > > > Tim Gore > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > > > > -----Original Message----- > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > Sent: Wednesday, April 20, 2016 7:48 PM > > To: Ville Syrjälä > > Cc: Gore, Tim; Daniel Vetter; Thierry, Michel; intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915:bxt: implement > > WaProgramL3SqcReg1DefaultForPerf > > > > On Wed, Apr 20, 2016 at 09:31:57PM +0300, Ville Syrjälä wrote: > > > On Wed, Apr 20, 2016 at 07:19:32PM +0100, Chris Wilson wrote: > > > > On Wed, Apr 20, 2016 at 03:51:49PM +0000, Gore, Tim wrote: > > > > > > > > > > Tim Gore > > > > > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, > > > > > Swindon SN3 1RJ > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Thierry, Michel > > > > > > Sent: Wednesday, April 20, 2016 4:00 PM > > > > > > To: Gore, Tim; intel-gfx@lists.freedesktop.org > > > > > > Subject: Re: [PATCH v2] drm/i915:bxt: implement > > > > > > WaProgramL3SqcReg1DefaultForPerf > > > > > > > > > > > > On 4/20/2016 3:23 PM, tim.gore@intel.com wrote: > > > > > > > From: Tim Gore <tim.gore@intel.com> > > > > > > > > > > > > > > This patch applies a performance enhancement workaround based > > > > > > > on analysis of DX and OCL S-Curve workloads. > > > > > > > > > > > > > > v2: Only apply to B0 onwards > > > > > > > > > > > > > > Signed-off-by: Tim Gore <tim.gore@intel.com> > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > > > > > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > > > > > > > 2 files changed, 5 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > > > b/drivers/gpu/drm/i915/i915_reg.h index f0a6d85..13e154a > > > > > > > 100644 > > > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > > > @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { > > > > > > > > > > > > > > #define GEN8_L3SQCREG1 > > _MMIO(0xB100) > > > > > > > #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > > > > > > +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 > > > > > > > > > > > > > > #define GEN7_L3CNTLREG1 > > _MMIO(0xB01C) > > > > > > > #define GEN7_WA_FOR_GEN7_L3_CONTROL > > > > > > 0x3C47FF8C > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > > > > b/drivers/gpu/drm/i915/intel_pm.c index b7c2186..eecdc3a6 > > > > > > > 100644 > > > > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > > > > @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct > > > > > > > drm_device > > > > > > *dev) > > > > > > > if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, > > REVID_FOREVER)) > > > > > > > I915_WRITE(GEN9_CLKGATE_DIS_0, > > > > > > I915_READ(GEN9_CLKGATE_DIS_0) | > > > > > > > PWM1_GATING_DIS | > > PWM2_GATING_DIS); > > > > > > > + > > > > > > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > > > > > > + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) > > > > > > > + I915_WRITE(GEN8_L3SQCREG1, > > > > > > BXT_WA_L3SQCREG1_DEFAULT); > > > > > > > } > > > > > > > > > > > > > > static void i915_pineview_get_mem_freq(struct drm_device > > > > > > > *dev) > > > > > > > > > > > > > > > > > > > Isnt it better to add this to bxt_init_workarounds() instead of > > > > > > bxt_init_clock_gating()? > > > > > > > > > > There is an equivalent Wa for bdw in bdw_init_clock_gating, so I > > > > > just tried to stay consistent with that. I'm not sure if this W/a > > > > > will persist across suspend/resume. bxt_init_workarounds only gets > > > > > called after reset, not on resume as far as I know. I'll > > > > > investigate further > > > > > > > > init_clock_gating() is called on init/reset/resume. Use it to set > > > > global registers. (Once upon a time it did only setup the clock > > > > gatings...) > > > > > > It's not called on reset. Or at least that was the case last time I > > > looked. Which is a rather big problem. My old idea was that we'd move > > > anything that gets clobbered by a GPU reset out from init_clock_gating > > > into some more suitable place that does get called on reset. > > > > Oh, I call upon Daniel! > > -Chris > > > Looks to me that init_clock_gating is called on reset thus: > I915_handle_error > -> i915_reset_and_wakeup > -> intel_finish_reset > -> intel_modeset_init_hw > -> intel_init_clock_gating > -> display.init_clock_gating > > Or perhaps we should put WaProgramL3SqcReg1DefaultForPerf in the new > gtt_write_workarounds function in i915_gem_gtt.c. I believe this is called on > Init, reset and resume. ? intel_modeset_init_hw should only be called on old crap where a gpu reset kills the entire chip, including the display block. That's not the case on modern stuff, much less once we have TDR. If it's getting called unconditionally that's a bug. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf 2016-04-20 18:47 ` Chris Wilson 2016-04-21 8:19 ` Gore, Tim @ 2016-04-21 9:15 ` Daniel Vetter 1 sibling, 0 replies; 10+ messages in thread From: Daniel Vetter @ 2016-04-21 9:15 UTC (permalink / raw) To: Chris Wilson, Ville Syrjälä, Gore, Tim, Daniel Vetter, Thierry, Michel, intel-gfx@lists.freedesktop.org On Wed, Apr 20, 2016 at 07:47:56PM +0100, Chris Wilson wrote: > On Wed, Apr 20, 2016 at 09:31:57PM +0300, Ville Syrjälä wrote: > > On Wed, Apr 20, 2016 at 07:19:32PM +0100, Chris Wilson wrote: > > > On Wed, Apr 20, 2016 at 03:51:49PM +0000, Gore, Tim wrote: > > > > > > > > Tim Gore > > > > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > > > > > > > > > > > > > -----Original Message----- > > > > > From: Thierry, Michel > > > > > Sent: Wednesday, April 20, 2016 4:00 PM > > > > > To: Gore, Tim; intel-gfx@lists.freedesktop.org > > > > > Subject: Re: [PATCH v2] drm/i915:bxt: implement > > > > > WaProgramL3SqcReg1DefaultForPerf > > > > > > > > > > On 4/20/2016 3:23 PM, tim.gore@intel.com wrote: > > > > > > From: Tim Gore <tim.gore@intel.com> > > > > > > > > > > > > This patch applies a performance enhancement workaround based on > > > > > > analysis of DX and OCL S-Curve workloads. > > > > > > > > > > > > v2: Only apply to B0 onwards > > > > > > > > > > > > Signed-off-by: Tim Gore <tim.gore@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/i915_reg.h | 1 + > > > > > > drivers/gpu/drm/i915/intel_pm.c | 4 ++++ > > > > > > 2 files changed, 5 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > > b/drivers/gpu/drm/i915/i915_reg.h index f0a6d85..13e154a 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > > @@ -6074,6 +6074,7 @@ enum skl_disp_power_wells { > > > > > > > > > > > > #define GEN8_L3SQCREG1 _MMIO(0xB100) > > > > > > #define BDW_WA_L3SQCREG1_DEFAULT 0x784000 > > > > > > +#define BXT_WA_L3SQCREG1_DEFAULT 0xF84000 > > > > > > > > > > > > #define GEN7_L3CNTLREG1 _MMIO(0xB01C) > > > > > > #define GEN7_WA_FOR_GEN7_L3_CONTROL > > > > > 0x3C47FF8C > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > > > b/drivers/gpu/drm/i915/intel_pm.c index b7c2186..eecdc3a6 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > > > @@ -76,6 +76,10 @@ static void bxt_init_clock_gating(struct drm_device > > > > > *dev) > > > > > > if (IS_BXT_REVID(dev_priv, BXT_REVID_B0, REVID_FOREVER)) > > > > > > I915_WRITE(GEN9_CLKGATE_DIS_0, > > > > > I915_READ(GEN9_CLKGATE_DIS_0) | > > > > > > PWM1_GATING_DIS | PWM2_GATING_DIS); > > > > > > + > > > > > > + /* WaProgramL3SqcReg1DefaultForPerf:bxt */ > > > > > > + if (IS_BXT_REVID(dev, BXT_REVID_B0, REVID_FOREVER)) > > > > > > + I915_WRITE(GEN8_L3SQCREG1, > > > > > BXT_WA_L3SQCREG1_DEFAULT); > > > > > > } > > > > > > > > > > > > static void i915_pineview_get_mem_freq(struct drm_device *dev) > > > > > > > > > > > > > > > > Isnt it better to add this to bxt_init_workarounds() instead of > > > > > bxt_init_clock_gating()? > > > > > > > > There is an equivalent Wa for bdw in bdw_init_clock_gating, so I just tried to > > > > stay consistent with that. I'm not sure if this W/a will persist across > > > > suspend/resume. bxt_init_workarounds only gets called after reset, not > > > > on resume as far as I know. I'll investigate further > > > > > > init_clock_gating() is called on init/reset/resume. Use it to set global > > > registers. (Once upon a time it did only setup the clock gatings...) > > > > It's not called on reset. Or at least that was the case last time I > > looked. Which is a rather big problem. My old idea was that we'd move > > anything that gets clobbered by a GPU reset out from init_clock_gating > > into some more suitable place that does get called on reset. > > Oh, I call upon Daniel! We have that place already since ages. Either - wa batch if it's in the context - per-engine init function (most of those are for rcs) init_clock_gating is for display/uncore wa only. I thought this is known by now? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf (rev2) 2016-04-20 14:23 [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf tim.gore 2016-04-20 15:00 ` Michel Thierry @ 2016-04-21 12:40 ` Patchwork 1 sibling, 0 replies; 10+ messages in thread From: Patchwork @ 2016-04-21 12:40 UTC (permalink / raw) To: tim.gore; +Cc: intel-gfx == Series Details == Series: drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf (rev2) URL : https://patchwork.freedesktop.org/series/5990/ State : failure == Summary == Series 5990v2 drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf http://patchwork.freedesktop.org/api/1.0/series/5990/revisions/2/mbox/ Test gem_busy: Subgroup basic-blt: pass -> SKIP (bsw-nuc-2) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: fail -> PASS (bsw-nuc-2) Test kms_force_connector_basic: Subgroup force-load-detect: pass -> SKIP (ivb-t430s) Test kms_pipe_crc_basic: Subgroup nonblocking-crc-pipe-a: pass -> FAIL (ivb-t430s) bdw-nuci7 total:194 pass:182 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultra total:194 pass:170 dwarn:0 dfail:0 fail:1 skip:23 bsw-nuc-2 total:193 pass:153 dwarn:0 dfail:0 fail:0 skip:40 byt-nuc total:193 pass:155 dwarn:0 dfail:0 fail:0 skip:38 hsw-brixbox total:194 pass:170 dwarn:0 dfail:0 fail:0 skip:24 ilk-hp8440p total:194 pass:137 dwarn:0 dfail:0 fail:0 skip:57 ivb-t430s total:194 pass:164 dwarn:0 dfail:0 fail:1 skip:29 skl-i7k-2 total:194 pass:168 dwarn:0 dfail:0 fail:1 skip:25 skl-nuci5 total:194 pass:183 dwarn:0 dfail:0 fail:0 skip:11 Results at /archive/results/CI_IGT_test/Patchwork_1961/ eb848ab2b19d25a08ca3b2b5e4b2f74c7f7c962c drm-intel-nightly: 2016y-04m-20d-18h-48m-11s UTC integration manifest d726e637 drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-04-21 12:40 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-20 14:23 [PATCH v2] drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf tim.gore 2016-04-20 15:00 ` Michel Thierry 2016-04-20 15:51 ` Gore, Tim 2016-04-20 18:19 ` Chris Wilson 2016-04-20 18:31 ` Ville Syrjälä 2016-04-20 18:47 ` Chris Wilson 2016-04-21 8:19 ` Gore, Tim 2016-04-21 9:16 ` Daniel Vetter 2016-04-21 9:15 ` Daniel Vetter 2016-04-21 12:40 ` ✗ Fi.CI.BAT: failure for drm/i915:bxt: implement WaProgramL3SqcReg1DefaultForPerf (rev2) Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox