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

Jesse Barnes
2015-05-21T20:19:07Z

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>