On Mon, 2017-09-11 at 12:26 +0800, Zhi Wang wrote:
> Only PPAT entries 0/2/3/4 are using. Remove extra PPAT entry allocation
> during initialization.
> 
> v8:
> 
> - Move ppat_index() into i915_gem_gtt.c. (Chris)
> - Change the name of ppat_bits_to_index to ppat_index.
> 
> Cc: Ben Widawsky <benjamin.widaw...@intel.com>
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> Suggested-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Signed-off-by: Zhi Wang <zhi.a.w...@intel.com>

<SNIP>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 53 
> +++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8b388aa..56fdfc6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2979,6 +2979,13 @@ static unsigned int chv_private_pat_match(u8 src, u8 
> dst)
>               INTEL_PPAT_PERFECT_MATCH : 0;
>  }
>  
> +/* PPAT index = 4 * PAT + 2 * PCD + PWT */
> +static inline unsigned int ppat_index(unsigned int bits)
> +{
> +     return (4 * !!(bits & _PAGE_PAT) + 2 * !!(bits & _PAGE_PCD)
> +             + !!(bits & _PAGE_PWT));

Would this be more readable as

        return 4 * !!(bits & _PAGE_PAT) +
               2 * !!(bits & _PAGE_PCD) +
               1 * !!(bits & _PAGE_PWT);

>  /* The GGTT and PPGTT need a private PPAT setup in order to handle 
> cacheability
> @@ -3026,18 +3030,18 @@ static void bdw_setup_private_ppat(struct intel_ppat 
> *ppat)
>                * So we can still hold onto all our assumptions wrt cpu
>                * clflushing on LLC machines.
>                */
> -             __alloc_ppat_entry(ppat, 0, GEN8_PPAT_UC);
> +             __alloc_ppat_entry(ppat, ppat_index(PPAT_CACHED_PDE), 
> GEN8_PPAT_UC);
>               return;
>       }
>  
> -     __alloc_ppat_entry(ppat, 0, GEN8_PPAT_WB | GEN8_PPAT_LLC);      /* for 
> normal objects, no eLLC */
> -     __alloc_ppat_entry(ppat, 1, GEN8_PPAT_WC | GEN8_PPAT_LLCELLC);  /* for 
> something pointing to ptes? */
> -     __alloc_ppat_entry(ppat, 2, GEN8_PPAT_WT | GEN8_PPAT_LLCELLC);  /* for 
> scanout with eLLC */
> -     __alloc_ppat_entry(ppat, 3, GEN8_PPAT_UC);                      /* 
> Uncached objects, mostly for scanout */
> -     __alloc_ppat_entry(ppat, 4, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | 
> GEN8_PPAT_AGE(0));
> -     __alloc_ppat_entry(ppat, 5, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | 
> GEN8_PPAT_AGE(1));
> -     __alloc_ppat_entry(ppat, 6, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | 
> GEN8_PPAT_AGE(2));
> -     __alloc_ppat_entry(ppat, 7, GEN8_PPAT_WB | GEN8_PPAT_LLCELLC | 
> GEN8_PPAT_AGE(3));
> +     /* See gen8_pte_encode() for the mapping from cache-level to PPAT */

Maybe lift these three comments below to the gen8_pte_encode? The
usages are driver wide, really.

Anyway, this is;

Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to