[PATCH 1/3] drm/i915: Use ilk_init_lp_watermarks() on BDW

[PATCH 1/3] drm/i915: Use ilk_init_lp_watermarks() on BDW

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're not using ilk_init_lp_watermarks() on BDW for some reason.
Probably due to the BDW patches and the relevant WM patches landing
roughlly at the same time. Fix it up.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ce1d079..206bd41 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6166,9 +6166,7 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum pipe pipe;
 
-	I915_WRITE(WM3_LP_ILK, 0);
-	I915_WRITE(WM2_LP_ILK, 0);
-	I915_WRITE(WM1_LP_ILK, 0);
+	ilk_init_lp_watermarks(dev);
 
 	/* WaSwitchSolVfFArbitrationPriority:bdw */
 	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
-- 
2.0.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Move WaProgramL3SqcReg1Default:bdw to init_clock_gating()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

GEN8_L3SQCREG1 isn't saved in the context (verified by going through
a context dump), and so we shouldn't be using the ring w/a code to
initialize it. Also Bspec explicitly talks about MMIO and writing it
with the CPU.

Additionally there's another w/a WaTempDisableDOPClkGating:bdw which
tells us to disable DOP clock gating around the GEN8_L3SQCREG1 write
to make sure everyone notices the change. So let's do that as well.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c         | 10 ++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c |  3 ---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 206bd41..5ec56b6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6165,6 +6165,7 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum pipe pipe;
+	uint32_t misccpctl;
 
 	ilk_init_lp_watermarks(dev);
 
@@ -6195,6 +6196,15 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
 		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
 
+	/*
+	 * WaProgramL3SqcReg1Default:bdw
+	 * WaTempDisableDOPClkGating:bdw
+	 */
+	misccpctl = I915_READ(GEN7_MISCCPCTL);
+	I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE);
+	I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
+	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
+
 	lpt_init_clock_gating(dev);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9b96ed7..50cdd67 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -853,9 +853,6 @@ static int bdw_init_workarounds(struct intel_engine_cs *ring)
 			    GEN6_WIZ_HASHING_MASK,
 			    GEN6_WIZ_HASHING_16x4);
 
-	/* WaProgramL3SqcReg1Default:bdw */
-	WA_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
-
 	return 0;
 }
 
-- 
2.0.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Enable GTT caching on gen8

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

GTT caching was disabled by default on gen8 due to not working with
big pages. Some information suggests that it got fixed, but still
GTT caching has been left disabled by default. Or could be it just
meant that the default was changed to off, and hence the problem
got solved.

Enable GTT caching in the hopes of some performance increase.
Whether or not the big pages issue has been fixed is irrelevant
at this stage since we don't use big pages.

This gives me a 1-2% improvement in xonotic on my BSW. Haven't tried
BDW, but supposedly it has larger TLBs so might not benefit as much.
On HSW GTT caching is enabled by default.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 84af255..90640d5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1461,6 +1461,8 @@ enum skl_disp_power_wells {
 #define RING_HWS_PGA(base)	((base)+0x80)
 #define RING_HWS_PGA_GEN6(base)	((base)+0x2080)
 
+#define HSW_GTT_CACHE_EN	0x4024
+#define   GTT_CACHE_EN_ALL	0xF0007FFF
 #define GEN7_WR_WATERMARK	0x4028
 #define GEN7_GFX_PRIO_CTRL	0x402C
 #define ARB_MODE		0x4030
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5ec56b6..58517a50 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6205,6 +6205,13 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
 	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
 
+	/*
+	 * WaGttCachingOffByDefault:bdw
+	 * GTT cache may not work with big pages, so if those
+	 * are ever enabled GTT cache may need to be disabled.
+	 */
+	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
+
 	lpt_init_clock_gating(dev);
 }
 
@@ -6480,6 +6487,12 @@ static void cherryview_init_clock_gating(struct drm_device *dev)
 	/* WaDisableSDEUnitClockGating:chv */
 	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
 		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
+
+	/*
+	 * GTT cache may not work with big pages, so if those
+	 * are ever enabled GTT cache may need to be disabled.
+	 */
+	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
 }
 
 static void g4x_init_clock_gating(struct drm_device *dev)
-- 
2.0.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Enable GTT caching on gen8

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6434
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  234/234              234/234
ILK                                  262/262              262/262
SNB                 -1              282/282              281/282
IVB                                  300/300              300/300
BYT                                  254/254              254/254
BDW                                  275/275              275/275
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(11)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Use ilk_init_lp_watermarks() on BDW

[Jesse Barnes]

On 05/19/2015 10:32 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We're not using ilk_init_lp_watermarks() on BDW for some reason.
> Probably due to the BDW patches and the relevant WM patches landing
> roughlly at the same time. Fix it up.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ce1d079..206bd41 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6166,9 +6166,7 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum pipe pipe;
>  
> -	I915_WRITE(WM3_LP_ILK, 0);
> -	I915_WRITE(WM2_LP_ILK, 0);
> -	I915_WRITE(WM1_LP_ILK, 0);
> +	ilk_init_lp_watermarks(dev);
>  
>  	/* WaSwitchSolVfFArbitrationPriority:bdw */
>  	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
> 

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Move WaProgramL3SqcReg1Default:bdw to init_clock_gating()

[Jesse Barnes]

On 05/19/2015 10:32 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> GEN8_L3SQCREG1 isn't saved in the context (verified by going through
> a context dump), and so we shouldn't be using the ring w/a code to
> initialize it. Also Bspec explicitly talks about MMIO and writing it
> with the CPU.
> 
> Additionally there's another w/a WaTempDisableDOPClkGating:bdw which
> tells us to disable DOP clock gating around the GEN8_L3SQCREG1 write
> to make sure everyone notices the change. So let's do that as well.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c         | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  3 ---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 206bd41..5ec56b6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6165,6 +6165,7 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum pipe pipe;
> +	uint32_t misccpctl;
>  
>  	ilk_init_lp_watermarks(dev);
>  
> @@ -6195,6 +6196,15 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
>  	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
>  		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
>  
> +	/*
> +	 * WaProgramL3SqcReg1Default:bdw
> +	 * WaTempDisableDOPClkGating:bdw
> +	 */
> +	misccpctl = I915_READ(GEN7_MISCCPCTL);
> +	I915_WRITE(GEN7_MISCCPCTL, misccpctl & ~GEN7_DOP_CLOCK_GATE_ENABLE);
> +	I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
> +	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
> +
>  	lpt_init_clock_gating(dev);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 9b96ed7..50cdd67 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -853,9 +853,6 @@ static int bdw_init_workarounds(struct intel_engine_cs *ring)
>  			    GEN6_WIZ_HASHING_MASK,
>  			    GEN6_WIZ_HASHING_16x4);
>  
> -	/* WaProgramL3SqcReg1Default:bdw */
> -	WA_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
> -
>  	return 0;
>  }
>  
> 

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Enable GTT caching on gen8

[Jesse Barnes]

On 05/19/2015 10:32 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> GTT caching was disabled by default on gen8 due to not working with
> big pages. Some information suggests that it got fixed, but still
> GTT caching has been left disabled by default. Or could be it just
> meant that the default was changed to off, and hence the problem
> got solved.
> 
> Enable GTT caching in the hopes of some performance increase.
> Whether or not the big pages issue has been fixed is irrelevant
> at this stage since we don't use big pages.
> 
> This gives me a 1-2% improvement in xonotic on my BSW. Haven't tried
> BDW, but supposedly it has larger TLBs so might not benefit as much.
> On HSW GTT caching is enabled by default.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 84af255..90640d5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1461,6 +1461,8 @@ enum skl_disp_power_wells {
>  #define RING_HWS_PGA(base)	((base)+0x80)
>  #define RING_HWS_PGA_GEN6(base)	((base)+0x2080)
>  
> +#define HSW_GTT_CACHE_EN	0x4024
> +#define   GTT_CACHE_EN_ALL	0xF0007FFF
>  #define GEN7_WR_WATERMARK	0x4028
>  #define GEN7_GFX_PRIO_CTRL	0x402C
>  #define ARB_MODE		0x4030
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5ec56b6..58517a50 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6205,6 +6205,13 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
>  	I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
>  	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
>  
> +	/*
> +	 * WaGttCachingOffByDefault:bdw
> +	 * GTT cache may not work with big pages, so if those
> +	 * are ever enabled GTT cache may need to be disabled.
> +	 */
> +	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
> +
>  	lpt_init_clock_gating(dev);
>  }
>  
> @@ -6480,6 +6487,12 @@ static void cherryview_init_clock_gating(struct drm_device *dev)
>  	/* WaDisableSDEUnitClockGating:chv */
>  	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
>  		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> +
> +	/*
> +	 * GTT cache may not work with big pages, so if those
> +	 * are ever enabled GTT cache may need to be disabled.
> +	 */
> +	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
>  }
>  
>  static void g4x_init_clock_gating(struct drm_device *dev)
> 

Looks ok to me; I guess testing will be the real review here.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Enable GTT caching on gen8

[Daniel Vetter]

On Thu, May 21, 2015 at 01:18:44PM -0700, Jesse Barnes wrote:
> On 05/19/2015 10:32 AM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > GTT caching was disabled by default on gen8 due to not working with
> > big pages. Some information suggests that it got fixed, but still
> > GTT caching has been left disabled by default. Or could be it just
> > meant that the default was changed to off, and hence the problem
> > got solved.
> > 
> > Enable GTT caching in the hopes of some performance increase.
> > Whether or not the big pages issue has been fixed is irrelevant
> > at this stage since we don't use big pages.
> > 
> > This gives me a 1-2% improvement in xonotic on my BSW. Haven't tried
> > BDW, but supposedly it has larger TLBs so might not benefit as much.
> > On HSW GTT caching is enabled by default.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h |  2 ++
> >  drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 84af255..90640d5 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1461,6 +1461,8 @@ enum skl_disp_power_wells {
> >  #define RING_HWS_PGA(base)	((base)+0x80)
> >  #define RING_HWS_PGA_GEN6(base)	((base)+0x2080)
> >  
> > +#define HSW_GTT_CACHE_EN	0x4024
> > +#define   GTT_CACHE_EN_ALL	0xF0007FFF
> >  #define GEN7_WR_WATERMARK	0x4028
> >  #define GEN7_GFX_PRIO_CTRL	0x402C
> >  #define ARB_MODE		0x4030
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 5ec56b6..58517a50 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -6205,6 +6205,13 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
> >  	I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
> >  	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
> >  
> > +	/*
> > +	 * WaGttCachingOffByDefault:bdw
> > +	 * GTT cache may not work with big pages, so if those
> > +	 * are ever enabled GTT cache may need to be disabled.
> > +	 */
> > +	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
> > +
> >  	lpt_init_clock_gating(dev);
> >  }
> >  
> > @@ -6480,6 +6487,12 @@ static void cherryview_init_clock_gating(struct drm_device *dev)
> >  	/* WaDisableSDEUnitClockGating:chv */
> >  	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> >  		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> > +
> > +	/*
> > +	 * GTT cache may not work with big pages, so if those
> > +	 * are ever enabled GTT cache may need to be disabled.
> > +	 */
> > +	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
> >  }
> >  
> >  static void g4x_init_clock_gating(struct drm_device *dev)
> > 
> 
> Looks ok to me; I guess testing will be the real review here.

Just aside: If you think the real test for a patch is the real world imo
an important part of the review work is to make sure we do have that
testing coverage. We have a few igts that specifically exercise gtt tlb
issues (from previous generations), so I think we're covered here.

> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Merged all three, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Enable GTT caching on gen8

[Jesse Barnes]

On 05/21/2015 11:10 PM, Daniel Vetter wrote:
> On Thu, May 21, 2015 at 01:18:44PM -0700, Jesse Barnes wrote:
>> On 05/19/2015 10:32 AM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> GTT caching was disabled by default on gen8 due to not working with
>>> big pages. Some information suggests that it got fixed, but still
>>> GTT caching has been left disabled by default. Or could be it just
>>> meant that the default was changed to off, and hence the problem
>>> got solved.
>>>
>>> Enable GTT caching in the hopes of some performance increase.
>>> Whether or not the big pages issue has been fixed is irrelevant
>>> at this stage since we don't use big pages.
>>>
>>> This gives me a 1-2% improvement in xonotic on my BSW. Haven't tried
>>> BDW, but supposedly it has larger TLBs so might not benefit as much.
>>> On HSW GTT caching is enabled by default.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_reg.h |  2 ++
>>>  drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
>>>  2 files changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 84af255..90640d5 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -1461,6 +1461,8 @@ enum skl_disp_power_wells {
>>>  #define RING_HWS_PGA(base)	((base)+0x80)
>>>  #define RING_HWS_PGA_GEN6(base)	((base)+0x2080)
>>>  
>>> +#define HSW_GTT_CACHE_EN	0x4024
>>> +#define   GTT_CACHE_EN_ALL	0xF0007FFF
>>>  #define GEN7_WR_WATERMARK	0x4028
>>>  #define GEN7_GFX_PRIO_CTRL	0x402C
>>>  #define ARB_MODE		0x4030
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 5ec56b6..58517a50 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -6205,6 +6205,13 @@ static void broadwell_init_clock_gating(struct drm_device *dev)
>>>  	I915_WRITE(GEN8_L3SQCREG1, BDW_WA_L3SQCREG1_DEFAULT);
>>>  	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
>>>  
>>> +	/*
>>> +	 * WaGttCachingOffByDefault:bdw
>>> +	 * GTT cache may not work with big pages, so if those
>>> +	 * are ever enabled GTT cache may need to be disabled.
>>> +	 */
>>> +	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
>>> +
>>>  	lpt_init_clock_gating(dev);
>>>  }
>>>  
>>> @@ -6480,6 +6487,12 @@ static void cherryview_init_clock_gating(struct drm_device *dev)
>>>  	/* WaDisableSDEUnitClockGating:chv */
>>>  	I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
>>>  		   GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
>>> +
>>> +	/*
>>> +	 * GTT cache may not work with big pages, so if those
>>> +	 * are ever enabled GTT cache may need to be disabled.
>>> +	 */
>>> +	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
>>>  }
>>>  
>>>  static void g4x_init_clock_gating(struct drm_device *dev)
>>>
>>
>> Looks ok to me; I guess testing will be the real review here.
> 
> Just aside: If you think the real test for a patch is the real world imo
> an important part of the review work is to make sure we do have that
> testing coverage. We have a few igts that specifically exercise gtt tlb
> issues (from previous generations), so I think we're covered here.
> 
>> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Merged all three, thanks for patches&review.

Yeah I don't think it's something that requires a dedicated test, just
lots of coverage with our existing stuff to sniff out problems.

Thanks,
Jesse

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx