[Intel-gfx] [PATCH v13 4/5] drm/i915: Do not allocate unused PPAT entries

[Joonas Lahtinen]

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.widawsky at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Suggested-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang at 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.lahtinen at linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation