Tim Gore Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > -----Original Message----- > From: Chris Wilson [mailto:chris@xxxxxxxxxxxxxxxxxx] > Sent: Wednesday, April 20, 2016 7:48 PM > To: Ville Syrjälä > Cc: Gore, Tim; Daniel Vetter; Thierry, Michel; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [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@xxxxxxxxxxxxxxxxxxxxx > > > > > Subject: Re: [PATCH v2] drm/i915:bxt: implement > > > > > WaProgramL3SqcReg1DefaultForPerf > > > > > > > > > > On 4/20/2016 3:23 PM, tim.gore@xxxxxxxxx wrote: > > > > > > From: Tim Gore <tim.gore@xxxxxxxxx> > > > > > > > > > > > > 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@xxxxxxxxx> > > > > > > --- > > > > > > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx