[Intel-gfx] [PATCH 1/2] drm/i915/gtt: enable GTT cache by default

[Chris Wilson]

Quoting Matthew Auld (2019-08-09 20:34:55)
> For some platforms the GTT cache is by default not enabled, and
> currently where we explicitly enable it, we make it conditional on 2M GTT
> page support, since the BSpec states that we must disable it if we
> enable 2M/1G pages. To make this more consistent opt for blanket
> enabling the GTT cache for all relevant gens in a single place, while
> still keeping the same behaviour of checking for 2M support.
> 
> BSpec: 9314
> BSpec: 423
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 21 +++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_pm.c     | 12 ------------
>  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 83a02e773c58..72a227c43e35 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2035,6 +2035,27 @@ static void gtt_write_workarounds(struct intel_gt *gt)
>                                  GEN8_GAMW_ECO_DEV_RW_IA,
>                                  0,
>                                  GAMW_ECO_ENABLE_64K_IPS_FIELD);
> +
> +       if (IS_GEN_RANGE(i915, 8, 11)) {
> +               bool can_use_gtt_cache = true;
> +
> +               /*
> +                * According to the BSpec if we use 2M/1G pages then we also
> +                * need to disable the GTT cache. At least on BDW we can see
> +                * visual corruption when using 2M pages, and not disabling the
> +                * GTT cache.
> +                */
> +               if (HAS_PAGE_SIZES(i915, I915_GTT_PAGE_SIZE_2M))
> +                       can_use_gtt_cache = false;
> +
> +               /* WaGttCachingOffByDefault */
> +               intel_uncore_write(uncore,
> +                                  HSW_GTT_CACHE_EN,
> +                                  can_use_gtt_cache ? GTT_CACHE_EN_ALL : 0);
> +               WARN_ON_ONCE(can_use_gtt_cache &&
> +                            intel_uncore_read(uncore,
> +                                              HSW_GTT_CACHE_EN) == 0);

I would opt for GEM_DEBUG_WARN_ON() and adding it to the error state.

This is a much more sensible place.
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris