[Intel-gfx] [PATCH 02/10] drm/i915/gtt: Use a common type for page directories

[Chris Wilson]

Quoting Mika Kuoppala (2019-06-14 17:43:42)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 63fa357c69de..812717ccc69b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -248,28 +248,14 @@ struct i915_page_dma {
>  
>  struct i915_page_table {
>         struct i915_page_dma base;
> -       atomic_t used_ptes;
> +       atomic_t used;
>  };
>  
>  struct i915_page_directory {
>         struct i915_page_dma base;
> -
> -       struct i915_page_table *page_table[I915_PDES]; /* PDEs */
> -       atomic_t used_pdes;
> -       spinlock_t lock;
> -};
> -
> -struct i915_page_directory_pointer {
> -       struct i915_page_dma base;
> -       struct i915_page_directory **page_directory;
> -       atomic_t used_pdpes;
> -       spinlock_t lock;
> -};
> -
> -struct i915_pml4 {
> -       struct i915_page_dma base;
> -       struct i915_page_directory_pointer *pdps[GEN8_PML4ES_PER_PML4];
> +       atomic_t used;
>         spinlock_t lock;
> +       void *entry[512];

I915_GTT_PAGE_SIZE / sizeof(u64) ?
or SZ_4K / sizeof(u64)
or just plain old 512 as the reason is obvious.

>  };
>  
>  struct i915_vma_ops {
> @@ -321,7 +307,7 @@ struct i915_address_space {
>         struct i915_page_dma scratch_page;
>         struct i915_page_table *scratch_pt;
>         struct i915_page_directory *scratch_pd;
> -       struct i915_page_directory_pointer *scratch_pdp; /* GEN8+ & 48b PPGTT */
> +       struct i915_page_directory *scratch_pdp; /* GEN8+ & 48b PPGTT */
>  
>         /**
>          * List of vma currently bound.
> @@ -428,11 +414,7 @@ struct i915_ppgtt {
>         struct i915_address_space vm;
>  
>         intel_engine_mask_t pd_dirty_engines;
> -       union {
> -               struct i915_pml4 pml4;          /* GEN8+ & 48b PPGTT */
> -               struct i915_page_directory_pointer pdp; /* GEN8+ */
> -               struct i915_page_directory pd;          /* GEN6-7 */
> -       };
> +       struct i915_page_directory *pd;

Ok, I give in :) If only because it made 32/48b much more sensible.

>  struct gen6_ppgtt {
> @@ -466,7 +448,7 @@ static inline struct gen6_ppgtt *to_gen6_ppgtt(struct i915_ppgtt *base)
>  #define gen6_for_each_pde(pt, pd, start, length, iter)                 \
>         for (iter = gen6_pde_index(start);                              \
>              length > 0 && iter < I915_PDES &&                          \
> -               (pt = (pd)->page_table[iter], true);                    \
> +                    (pt = i915_pt_entry(pd, iter), true);              \
>              ({ u32 temp = ALIGN(start+1, 1 << GEN6_PDE_SHIFT);         \
>                     temp = min(temp - start, length);                   \
>                     start += temp, length -= temp; }), ++iter)
> @@ -474,7 +456,7 @@ static inline struct gen6_ppgtt *to_gen6_ppgtt(struct i915_ppgtt *base)
>  #define gen6_for_all_pdes(pt, pd, iter)                                        \
>         for (iter = 0;                                                  \
>              iter < I915_PDES &&                                        \
> -               (pt = (pd)->page_table[iter], true);                    \
> +                    (pt = i915_pt_entry(pd, iter), true);              \
>              ++iter)
>  
>  static inline u32 i915_pte_index(u64 address, unsigned int pde_shift)
> @@ -533,6 +515,27 @@ i915_pdpes_per_pdp(const struct i915_address_space *vm)
>         return GEN8_3LVL_PDPES;
>  }
>  
> +static inline struct i915_page_table *
> +i915_pt_entry(const struct i915_page_directory * const pd,
> +             const unsigned short n)
> +{
> +       return pd->entry[n];
> +}
> +
> +static inline struct i915_page_directory *
> +i915_pd_entry(const struct i915_page_directory * const pdp,
> +             const unsigned short n)
> +{
> +       return pdp->entry[n];
> +}
> +
> +static inline struct i915_page_directory *
> +i915_pdp_entry(const struct i915_page_directory * const pml4,
> +              const unsigned short n)
> +{
> +       return pml4->entry[n];
> +}

I expect to see the differentiation phased out... :)

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris