Re: [PATCH v7 04/19] drm/i915/gen8: Generalize PTE writing for GEN8 PPGTT

["Goel, Akash" <akash.goel@intel.com>]

Reviewed the patch & it looks fine.
Reviewed-by: "Akash Goel <akash.goel@intel.com>"


On 7/30/2015 3:32 PM, Michel Thierry wrote:
> The insert_entries function was the function used to write PTEs. For the
> PPGTT it was "hardcoded" to only understand two level page tables, which
> was the case for GEN7. We can reuse this for 4 level page tables, and
> remove the concept of insert_entries, which was never viable past 2
> level page tables anyway, but it requires a bit of rework to make the
> function a bit more generic.
>
> v2: Rebase after Mika's ppgtt cleanup / scratch merge patch series.
> v3: Rebase after final merged version of Mika's ppgtt/scratch patches.
> v4: Check and warn for NULL value of pdp pointer (Akash).
>
> Cc: Akash Goel <akash.goel@intel.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com> (v2)
> ---
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 53 ++++++++++++++++++++++++++++---------
>   1 file changed, 41 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index bd56979..740ad5b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -600,23 +600,23 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt,
>   	return 0;
>   }
>
> -static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
> -				   uint64_t start,
> -				   uint64_t length,
> -				   bool use_scratch)
> +static void gen8_ppgtt_clear_pte_range(struct i915_address_space *vm,
> +				       struct i915_page_directory_pointer *pdp,
> +				       uint64_t start,
> +				       uint64_t length,
> +				       gen8_pte_t scratch_pte)
>   {
>   	struct i915_hw_ppgtt *ppgtt =
>   		container_of(vm, struct i915_hw_ppgtt, base);
> -	struct i915_page_directory_pointer *pdp = &ppgtt->pdp; /* FIXME: 48b */
> -	gen8_pte_t *pt_vaddr, scratch_pte;
> +	gen8_pte_t *pt_vaddr;
>   	unsigned pdpe = start >> GEN8_PDPE_SHIFT & GEN8_PDPE_MASK;
>   	unsigned pde = start >> GEN8_PDE_SHIFT & GEN8_PDE_MASK;
>   	unsigned pte = start >> GEN8_PTE_SHIFT & GEN8_PTE_MASK;
>   	unsigned num_entries = length >> PAGE_SHIFT;
>   	unsigned last_pte, i;
>
> -	scratch_pte = gen8_pte_encode(px_dma(ppgtt->base.scratch_page),
> -				      I915_CACHE_LLC, use_scratch);
> +	if (WARN_ON(!pdp))
> +		return;
>
>   	while (num_entries) {
>   		struct i915_page_directory *pd;
> @@ -656,14 +656,30 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
>   	}
>   }
>
> -static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
> -				      struct sg_table *pages,
> -				      uint64_t start,
> -				      enum i915_cache_level cache_level, u32 unused)
> +static void gen8_ppgtt_clear_range(struct i915_address_space *vm,
> +				   uint64_t start,
> +				   uint64_t length,
> +				   bool use_scratch)
>   {
>   	struct i915_hw_ppgtt *ppgtt =
>   		container_of(vm, struct i915_hw_ppgtt, base);
>   	struct i915_page_directory_pointer *pdp = &ppgtt->pdp; /* FIXME: 48b */
> +
> +	gen8_pte_t scratch_pte = gen8_pte_encode(px_dma(vm->scratch_page),
> +						 I915_CACHE_LLC, use_scratch);
> +
> +	gen8_ppgtt_clear_pte_range(vm, pdp, start, length, scratch_pte);
> +}
> +
> +static void
> +gen8_ppgtt_insert_pte_entries(struct i915_address_space *vm,
> +			      struct i915_page_directory_pointer *pdp,
> +			      struct sg_table *pages,
> +			      uint64_t start,
> +			      enum i915_cache_level cache_level)
> +{
> +	struct i915_hw_ppgtt *ppgtt =
> +		container_of(vm, struct i915_hw_ppgtt, base);
>   	gen8_pte_t *pt_vaddr;
>   	unsigned pdpe = start >> GEN8_PDPE_SHIFT & GEN8_PDPE_MASK;
>   	unsigned pde = start >> GEN8_PDE_SHIFT & GEN8_PDE_MASK;
> @@ -700,6 +716,19 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
>   		kunmap_px(ppgtt, pt_vaddr);
>   }
>
> +static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
> +				      struct sg_table *pages,
> +				      uint64_t start,
> +				      enum i915_cache_level cache_level,
> +				      u32 unused)
> +{
> +	struct i915_hw_ppgtt *ppgtt =
> +		container_of(vm, struct i915_hw_ppgtt, base);
> +	struct i915_page_directory_pointer *pdp = &ppgtt->pdp; /* FIXME: 48b */
> +
> +	gen8_ppgtt_insert_pte_entries(vm, pdp, pages, start, cache_level);
> +}
> +
>   static void gen8_free_page_tables(struct drm_device *dev,
>   				  struct i915_page_directory *pd)
>   {
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx