* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.