[Intel-gfx] [PATCH 06/10] drm/i915/gtt: pde entry encoding is identical

[Chris Wilson]

Quoting Mika Kuoppala (2019-06-14 17:43:46)
> For all page directory entries, the pde encoding is
> identical. Don't compilicate call sites with different
> versions of doing the same thing.
> 
> Only wart that remains is a 4 entry gen8/bsw pdp, for which
> we need to check the backing phys page.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 111 ++++++++++------------------
>  drivers/gpu/drm/i915/i915_gem_gtt.h |   3 -
>  2 files changed, 40 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index de264b3a0105..a61fa24b6294 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -214,10 +214,10 @@ static u64 gen8_pte_encode(dma_addr_t addr,
>         return pte;
>  }
>  
> -static gen8_pde_t gen8_pde_encode(const dma_addr_t addr,
> -                                 const enum i915_cache_level level)
> +static u64 gen8_pde_encode(const dma_addr_t addr,
> +                          const enum i915_cache_level level)
>  {
> -       gen8_pde_t pde = _PAGE_PRESENT | _PAGE_RW;
> +       u64 pde = _PAGE_PRESENT | _PAGE_RW;
>         pde |= addr;
>         if (level != I915_CACHE_NONE)
>                 pde |= PPAT_CACHED_PDE;
> @@ -226,9 +226,6 @@ static gen8_pde_t gen8_pde_encode(const dma_addr_t addr,
>         return pde;
>  }
>  
> -#define gen8_pdpe_encode gen8_pde_encode
> -#define gen8_pml4e_encode gen8_pde_encode
> -
>  static u64 snb_pte_encode(dma_addr_t addr,
>                           enum i915_cache_level level,
>                           u32 flags)
> @@ -733,24 +730,36 @@ static void free_pd(struct i915_address_space *vm,
>         kfree(pd);
>  }
>  
> -static void init_pd_with_page(struct i915_address_space *vm,
> -                             struct i915_page_directory * const pd,
> -                             struct i915_page_table *pt)
> -{
> -       fill_px(vm, pd, gen8_pde_encode(px_dma(pt), I915_CACHE_LLC));
> -       memset_p(pd->entry, pt, 512);
> +#define init_pd(vm, pd, to) {                                  \
> +       GEM_DEBUG_BUG_ON(!pd_has_phys_page(pd));                \
> +       fill_px((vm), (pd), gen8_pde_encode(px_dma(to), I915_CACHE_LLC)); \
> +       memset_p((pd)->entry, (to), 512);                               \
>  }
>  
> -static void init_pd(struct i915_address_space *vm,
> -                   struct i915_page_directory * const pd,
> -                   struct i915_page_directory * const to)
> +static void __set_pd_entry(struct i915_page_directory * const pd,
> +                          const unsigned short pde,
> +                          const u64 encoded_entry)
>  {
> -       GEM_DEBUG_BUG_ON(!pd_has_phys_page(pd));
> +       u64 *vaddr;
>  
> -       fill_px(vm, pd, gen8_pdpe_encode(px_dma(to), I915_CACHE_LLC));
> -       memset_p(pd->entry, to, 512);
> +       vaddr = kmap_atomic(pd->base.page);
> +       vaddr[pde] = encoded_entry;
> +       kunmap_atomic(vaddr);
>  }
>  
> +#define set_pd_entry(pd, pde, to) ({                                   \
> +       (pd)->entry[(pde)] = (to);                                      \
> +       __set_pd_entry((pd), (pde),                                     \
> +                      gen8_pde_encode(px_dma(to), I915_CACHE_LLC));    \
> +})
> +
> +#define set_pdp_entry(pdp, pdpe, to) ({                \
> +       (pdp)->entry[(pdpe)] = (to);            \
> +       if (pd_has_phys_page(pdp))                                      \
> +               __set_pd_entry((pdp), (pdpe),                           \
> +                              gen8_pde_encode(px_dma(to), I915_CACHE_LLC));\
> +})

Nah. Just keep the levels identical, one function that checks if there
is a page to set.

igt/benchmarks/gem_exec_fault iirc.
-Chris