[PATCH 1/3] drm/i915/gtt: pde entry encoding is identical

[PATCH 1/3] drm/i915/gtt: pde entry encoding is identical

[Mika Kuoppala]

For all page directory entries, the pde encoding is
identical. Don't compilicate call sites with different
versions of doing the same thing. We check the existence of
physical page before writing the entry into it. This further
generalizes the pd so that manipulation in callsites will be
identical, removing the need to handle pdps differently for gen8.

v2: squash

Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 114 ++++++++++------------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |   3 -
 2 files changed, 40 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8ab820145ea6..5df17db68875 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -215,10 +215,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;
@@ -227,9 +227,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)
@@ -734,24 +731,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 __write_dma_entry(struct i915_page_dma * const pdma,
+			      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(pdma->page);
+	vaddr[pde] = encoded_entry;
+	kunmap_atomic(vaddr);
 }
 
+static inline void
+__set_pd_entry(struct i915_page_directory * const pd,
+	       const unsigned short pde,
+	       struct i915_page_dma * const to,
+	       u64 (*encode)(const dma_addr_t, const enum i915_cache_level))
+{
+	pd->entry[pde] = to;
+	__write_dma_entry(px_base(pd), pde, encode(to->daddr, I915_CACHE_LLC));
+}
+
+#define set_pd_entry(pd, pde, to) \
+	__set_pd_entry((pd), (pde), px_base(to), gen8_pde_encode)
+
 /*
  * PDE TLBs are a pain to invalidate on GEN8+. When we modify
  * the page table structures, we mark them dirty so that
@@ -781,18 +790,6 @@ static bool gen8_ppgtt_clear_pt(const struct i915_address_space *vm,
 	return !atomic_sub_return(num_entries, &pt->used);
 }
 
-static void gen8_ppgtt_set_pde(struct i915_address_space *vm,
-			       struct i915_page_directory *pd,
-			       struct i915_page_table *pt,
-			       unsigned int pde)
-{
-	gen8_pde_t *vaddr;
-
-	vaddr = kmap_atomic_px(pd);
-	vaddr[pde] = gen8_pde_encode(px_dma(pt), I915_CACHE_LLC);
-	kunmap_atomic(vaddr);
-}
-
 static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 				struct i915_page_directory *pd,
 				u64 start, u64 length)
@@ -810,8 +807,7 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 
 		spin_lock(&pd->lock);
 		if (!atomic_read(&pt->used)) {
-			gen8_ppgtt_set_pde(vm, pd, vm->scratch_pt, pde);
-			pd->entry[pde] = vm->scratch_pt;
+			set_pd_entry(pd, pde, vm->scratch_pt);
 
 			GEM_BUG_ON(!atomic_read(&pd->used));
 			atomic_dec(&pd->used);
@@ -825,20 +821,6 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 	return !atomic_read(&pd->used);
 }
 
-static void gen8_ppgtt_set_pdpe(struct i915_page_directory *pdp,
-				struct i915_page_directory *pd,
-				unsigned int pdpe)
-{
-	gen8_ppgtt_pdpe_t *vaddr;
-
-	if (!pd_has_phys_page(pdp))
-		return;
-
-	vaddr = kmap_atomic_px(pdp);
-	vaddr[pdpe] = gen8_pdpe_encode(px_dma(pd), I915_CACHE_LLC);
-	kunmap_atomic(vaddr);
-}
-
 /* Removes entries from a single page dir pointer, releasing it if it's empty.
  * Caller can use the return value to update higher-level entries
  */
@@ -859,8 +841,7 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
 
 		spin_lock(&pdp->lock);
 		if (!atomic_read(&pd->used)) {
-			gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
-			pdp->entry[pdpe] = vm->scratch_pd;
+			set_pd_entry(pdp, pdpe, vm->scratch_pd);
 
 			GEM_BUG_ON(!atomic_read(&pdp->used));
 			atomic_dec(&pdp->used);
@@ -880,17 +861,6 @@ static void gen8_ppgtt_clear_3lvl(struct i915_address_space *vm,
 	gen8_ppgtt_clear_pdp(vm, i915_vm_to_ppgtt(vm)->pd, start, length);
 }
 
-static void gen8_ppgtt_set_pml4e(struct i915_page_directory *pml4,
-				 struct i915_page_directory *pdp,
-				 unsigned int pml4e)
-{
-	gen8_ppgtt_pml4e_t *vaddr;
-
-	vaddr = kmap_atomic_px(pml4);
-	vaddr[pml4e] = gen8_pml4e_encode(px_dma(pdp), I915_CACHE_LLC);
-	kunmap_atomic(vaddr);
-}
-
 /* Removes entries from a single pml4.
  * This is the top-level structure in 4-level page tables used on gen8+.
  * Empty entries are always scratch pml4e.
@@ -914,8 +884,7 @@ static void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
 
 		spin_lock(&pml4->lock);
 		if (!atomic_read(&pdp->used)) {
-			gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
-			pml4->entry[pml4e] = vm->scratch_pdp;
+			set_pd_entry(pml4, pml4e, vm->scratch_pdp);
 			free = true;
 		}
 		spin_unlock(&pml4->lock);
@@ -1232,7 +1201,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 	}
 
 	gen8_initialize_pt(vm, vm->scratch_pt);
-	init_pd_with_page(vm, vm->scratch_pd, vm->scratch_pt);
+	init_pd(vm, vm->scratch_pd, vm->scratch_pt);
 	if (i915_vm_is_4lvl(vm))
 		init_pd(vm, vm->scratch_pdp, vm->scratch_pd);
 
@@ -1371,8 +1340,7 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 
 			spin_lock(&pd->lock);
 			if (pd->entry[pde] == vm->scratch_pt) {
-				gen8_ppgtt_set_pde(vm, pd, pt, pde);
-				pd->entry[pde] = pt;
+				set_pd_entry(pd, pde, pt);
 				atomic_inc(&pd->used);
 			} else {
 				alloc = pt;
@@ -1415,12 +1383,11 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 				goto unwind;
 			}
 
-			init_pd_with_page(vm, pd, vm->scratch_pt);
+			init_pd(vm, pd, vm->scratch_pt);
 
 			spin_lock(&pdp->lock);
 			if (pdp->entry[pdpe] == vm->scratch_pd) {
-				gen8_ppgtt_set_pdpe(pdp, pd, pdpe);
-				pdp->entry[pdpe] = pd;
+				set_pd_entry(pdp, pdpe, pd);
 				atomic_inc(&pdp->used);
 			} else {
 				alloc = pd;
@@ -1443,7 +1410,7 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 unwind_pd:
 	spin_lock(&pdp->lock);
 	if (atomic_dec_and_test(&pd->used)) {
-		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
+		set_pd_entry(pdp, pdpe, vm->scratch_pd);
 		GEM_BUG_ON(!atomic_read(&pdp->used));
 		atomic_dec(&pdp->used);
 		free_pd(vm, pd);
@@ -1491,8 +1458,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 
 			spin_lock(&pml4->lock);
 			if (pml4->entry[pml4e] == vm->scratch_pdp) {
-				gen8_ppgtt_set_pml4e(pml4, pdp, pml4e);
-				pml4->entry[pml4e] = pdp;
+				set_pd_entry(pml4, pml4e, pdp);
 			} else {
 				alloc = pdp;
 				pdp = pml4->entry[pml4e];
@@ -1514,7 +1480,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 unwind_pdp:
 	spin_lock(&pml4->lock);
 	if (atomic_dec_and_test(&pdp->used)) {
-		gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
+		set_pd_entry(pml4, pml4e, vm->scratch_pdp);
 		free_pd(vm, pdp);
 	}
 	spin_unlock(&pml4->lock);
@@ -1540,8 +1506,8 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 		if (IS_ERR(pd))
 			goto unwind;
 
-		init_pd_with_page(vm, pd, vm->scratch_pt);
-		gen8_ppgtt_set_pdpe(pdp, pd, pdpe);
+		init_pd(vm, pd, vm->scratch_pt);
+		set_pd_entry(pdp, pdpe, pd);
 
 		atomic_inc(&pdp->used);
 	}
@@ -1553,7 +1519,7 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 unwind:
 	start -= from;
 	gen8_for_each_pdpe(pd, pdp, from, start, pdpe) {
-		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
+		set_pd_entry(pdp, pdpe, vm->scratch_pd);
 		free_pd(vm, pd);
 	}
 	atomic_set(&pdp->used, 0);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 812717ccc69b..6a014d052952 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -67,9 +67,6 @@ struct i915_vma;
 
 typedef u32 gen6_pte_t;
 typedef u64 gen8_pte_t;
-typedef u64 gen8_pde_t;
-typedef u64 gen8_ppgtt_pdpe_t;
-typedef u64 gen8_ppgtt_pml4e_t;
 
 #define ggtt_total_entries(ggtt) ((ggtt)->vm.total >> PAGE_SHIFT)
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/gtt: Tear down setup and cleanup macros for page dma

[Mika Kuoppala]

We don't use common codepaths to setup and cleanup page
directories vs page tables. So their setup and cleanup macros
are of no use.

Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5df17db68875..b521b1ddd19b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -551,8 +551,6 @@ static void cleanup_page_dma(struct i915_address_space *vm,
 
 #define kmap_atomic_px(px) kmap_atomic(px_base(px)->page)
 
-#define setup_px(vm, px) setup_page_dma((vm), px_base(px))
-#define cleanup_px(vm, px) cleanup_page_dma((vm), px_base(px))
 #define fill_px(vm, px, v) fill_page_dma((vm), px_base(px), (v))
 #define fill32_px(vm, px, v) fill_page_dma_32((vm), px_base(px), (v))
 
@@ -654,7 +652,7 @@ static struct i915_page_table *alloc_pt(struct i915_address_space *vm)
 	if (unlikely(!pt))
 		return ERR_PTR(-ENOMEM);
 
-	if (unlikely(setup_px(vm, pt))) {
+	if (unlikely(setup_page_dma(vm, &pt->base))) {
 		kfree(pt);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -666,7 +664,7 @@ static struct i915_page_table *alloc_pt(struct i915_address_space *vm)
 
 static void free_pt(struct i915_address_space *vm, struct i915_page_table *pt)
 {
-	cleanup_px(vm, pt);
+	cleanup_page_dma(vm, &pt->base);
 	kfree(pt);
 }
 
@@ -709,7 +707,7 @@ static struct i915_page_directory *alloc_pd(struct i915_address_space *vm)
 	if (unlikely(!pd))
 		return ERR_PTR(-ENOMEM);
 
-	if (unlikely(setup_px(vm, pd))) {
+	if (unlikely(setup_page_dma(vm, &pd->base))) {
 		kfree(pd);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -726,7 +724,7 @@ static void free_pd(struct i915_address_space *vm,
 		    struct i915_page_directory *pd)
 {
 	if (likely(pd_has_phys_page(pd)))
-		cleanup_px(vm, pd);
+		cleanup_page_dma(vm, &pd->base);
 
 	kfree(pd);
 }
@@ -1584,7 +1582,7 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 	}
 
 	if (i915_vm_is_4lvl(&ppgtt->vm)) {
-		err = setup_px(&ppgtt->vm, ppgtt->pd);
+		err = setup_page_dma(&ppgtt->vm, &ppgtt->pd->base);
 		if (err)
 			goto err_free_pdp;
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/gtt: Setup phys pages for 3lvl pdps

[Mika Kuoppala]

If we setup backing phys page for 3lvl pdps, even they
are not used, we lose 5 pages per ppgtt.

Trading this memory on bsw, we gain more common code paths for all
gen8+ directory manipulation. And those paths are now void of checks
for page directory type, making the hot paths faster.

Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 106 +++++++++++++++++-----------
 1 file changed, 66 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b521b1ddd19b..ea78302c6348 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -715,22 +715,14 @@ static struct i915_page_directory *alloc_pd(struct i915_address_space *vm)
 	return pd;
 }
 
-static inline bool pd_has_phys_page(const struct i915_page_directory * const pd)
-{
-	return pd->base.page;
-}
-
 static void free_pd(struct i915_address_space *vm,
 		    struct i915_page_directory *pd)
 {
-	if (likely(pd_has_phys_page(pd)))
-		cleanup_page_dma(vm, &pd->base);
-
+	cleanup_page_dma(vm, &pd->base);
 	kfree(pd);
 }
 
 #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);				\
 }
@@ -1539,6 +1531,50 @@ static void ppgtt_init(struct drm_i915_private *i915,
 	ppgtt->vm.vma_ops.clear_pages = clear_pages;
 }
 
+static void init_pd_n(struct i915_address_space *vm,
+		      struct i915_page_directory *pd,
+		      struct i915_page_directory *to,
+		      const unsigned int entries)
+{
+	const u64 daddr = gen8_pde_encode(px_dma(to), I915_CACHE_LLC);
+	u64 * const vaddr = kmap_atomic(pd->base.page);
+
+	memset64(vaddr, daddr, entries);
+	kunmap_atomic(vaddr);
+
+	memset_p(pd->entry, to, entries);
+}
+
+static struct i915_page_directory *
+gen8_alloc_top_pd(struct i915_address_space *vm)
+{
+	struct i915_page_directory *pd;
+
+	if (i915_vm_is_4lvl(vm)) {
+		pd = alloc_pd(vm);
+		if (!IS_ERR(pd))
+			init_pd(vm, pd, vm->scratch_pdp);
+
+		return pd;
+	}
+
+	/* 3lvl */
+	pd = __alloc_pd();
+	if (!pd)
+		return ERR_PTR(-ENOMEM);
+
+	pd->entry[GEN8_3LVL_PDPES] = NULL;
+
+	if (unlikely(setup_page_dma(vm, &pd->base))) {
+		kfree(pd);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	init_pd_n(vm, pd, vm->scratch_pd, GEN8_3LVL_PDPES);
+
+	return pd;
+}
+
 /*
  * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers
  * with a net effect resembling a 2-level page table in normal x86 terms. Each
@@ -1548,6 +1584,7 @@ static void ppgtt_init(struct drm_i915_private *i915,
  */
 static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 {
+	struct i915_address_space *vm;
 	struct i915_ppgtt *ppgtt;
 	int err;
 
@@ -1557,70 +1594,59 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 
 	ppgtt_init(i915, ppgtt);
 
+	vm = &ppgtt->vm;
+
 	/*
 	 * From bdw, there is hw support for read-only pages in the PPGTT.
 	 *
 	 * Gen11 has HSDES#:1807136187 unresolved. Disable ro support
 	 * for now.
 	 */
-	ppgtt->vm.has_read_only = INTEL_GEN(i915) != 11;
+	vm->has_read_only = INTEL_GEN(i915) != 11;
 
 	/* There are only few exceptions for gen >=6. chv and bxt.
 	 * And we are not sure about the latter so play safe for now.
 	 */
 	if (IS_CHERRYVIEW(i915) || IS_BROXTON(i915))
-		ppgtt->vm.pt_kmap_wc = true;
+		vm->pt_kmap_wc = true;
 
-	err = gen8_init_scratch(&ppgtt->vm);
+	err = gen8_init_scratch(vm);
 	if (err)
 		goto err_free;
 
-	ppgtt->pd = __alloc_pd();
-	if (!ppgtt->pd) {
-		err = -ENOMEM;
+	ppgtt->pd = gen8_alloc_top_pd(vm);
+	if (IS_ERR(ppgtt->pd)) {
+		err = PTR_ERR(ppgtt->pd);
 		goto err_free_scratch;
 	}
 
-	if (i915_vm_is_4lvl(&ppgtt->vm)) {
-		err = setup_page_dma(&ppgtt->vm, &ppgtt->pd->base);
-		if (err)
-			goto err_free_pdp;
-
-		init_pd(&ppgtt->vm, ppgtt->pd, ppgtt->vm.scratch_pdp);
-
-		ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc_4lvl;
-		ppgtt->vm.insert_entries = gen8_ppgtt_insert_4lvl;
-		ppgtt->vm.clear_range = gen8_ppgtt_clear_4lvl;
+	if (i915_vm_is_4lvl(vm)) {
+		vm->allocate_va_range = gen8_ppgtt_alloc_4lvl;
+		vm->insert_entries = gen8_ppgtt_insert_4lvl;
+		vm->clear_range = gen8_ppgtt_clear_4lvl;
 	} else {
-		/*
-		 * We don't need to setup dma for top level pdp, only
-		 * for entries. So point entries to scratch.
-		 */
-		memset_p(ppgtt->pd->entry, ppgtt->vm.scratch_pd,
-			 GEN8_3LVL_PDPES);
-
 		if (intel_vgpu_active(i915)) {
 			err = gen8_preallocate_top_level_pdp(ppgtt);
 			if (err)
-				goto err_free_pdp;
+				goto err_free_pd;
 		}
 
-		ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc_3lvl;
-		ppgtt->vm.insert_entries = gen8_ppgtt_insert_3lvl;
-		ppgtt->vm.clear_range = gen8_ppgtt_clear_3lvl;
+		vm->allocate_va_range = gen8_ppgtt_alloc_3lvl;
+		vm->insert_entries = gen8_ppgtt_insert_3lvl;
+		vm->clear_range = gen8_ppgtt_clear_3lvl;
 	}
 
 	if (intel_vgpu_active(i915))
 		gen8_ppgtt_notify_vgt(ppgtt, true);
 
-	ppgtt->vm.cleanup = gen8_ppgtt_cleanup;
+	vm->cleanup = gen8_ppgtt_cleanup;
 
 	return ppgtt;
 
-err_free_pdp:
-	free_pd(&ppgtt->vm, ppgtt->pd);
+err_free_pd:
+	free_pd(vm, ppgtt->pd);
 err_free_scratch:
-	gen8_free_scratch(&ppgtt->vm);
+	gen8_free_scratch(vm);
 err_free:
 	kfree(ppgtt);
 	return ERR_PTR(err);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/gtt: pde entry encoding is identical

[Chris Wilson]

Quoting Mika Kuoppala (2019-06-18 17:17:29)
> For all page directory entries, the pde encoding is
> identical. Don't compilicate call sites with different
> versions of doing the same thing. We check the existence of
> physical page before writing the entry into it. This further
> generalizes the pd so that manipulation in callsites will be
> identical, removing the need to handle pdps differently for gen8.

And we can pull in the atomic_inc as well? At the top level the result
goes unused, but since we should not be as frequently inserting new
pages the higher up we go, that should be insignificant.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915/gtt: pde entry encoding is identical

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/gtt: pde entry encoding is identical
URL   : https://patchwork.freedesktop.org/series/62324/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
224a6de52ba2 drm/i915/gtt: pde entry encoding is identical
-:55: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pd' - possible side-effects?
#55: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:734:
+#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);				\
 }

-:55: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'to' - possible side-effects?
#55: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:734:
+#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);				\
 }

total: 0 errors, 0 warnings, 2 checks, 235 lines checked
4f46c24d9757 drm/i915/gtt: Tear down setup and cleanup macros for page dma
3fc849b75c9d drm/i915/gtt: Setup phys pages for 3lvl pdps

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/gtt: Setup phys pages for 3lvl pdps

[Chris Wilson]

Quoting Mika Kuoppala (2019-06-18 17:17:31)
> If we setup backing phys page for 3lvl pdps, even they
                                               even though they
> are not used, we lose 5 pages per ppgtt.
> 
> Trading this memory on bsw, we gain more common code paths for all
> gen8+ directory manipulation. And those paths are now void of checks
> for page directory type, making the hot paths faster.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 106 +++++++++++++++++-----------
>  1 file changed, 66 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b521b1ddd19b..ea78302c6348 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -715,22 +715,14 @@ static struct i915_page_directory *alloc_pd(struct i915_address_space *vm)
>         return pd;
>  }
>  
> -static inline bool pd_has_phys_page(const struct i915_page_directory * const pd)
> -{
> -       return pd->base.page;
> -}
> -
>  static void free_pd(struct i915_address_space *vm,
>                     struct i915_page_directory *pd)
>  {
> -       if (likely(pd_has_phys_page(pd)))
> -               cleanup_page_dma(vm, &pd->base);
> -
> +       cleanup_page_dma(vm, &pd->base);
>         kfree(pd);
>  }
>  
>  #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);                               \
>  }
> @@ -1539,6 +1531,50 @@ static void ppgtt_init(struct drm_i915_private *i915,
>         ppgtt->vm.vma_ops.clear_pages = clear_pages;
>  }
>  
> +static void init_pd_n(struct i915_address_space *vm,
> +                     struct i915_page_directory *pd,
> +                     struct i915_page_directory *to,
> +                     const unsigned int entries)
> +{
> +       const u64 daddr = gen8_pde_encode(px_dma(to), I915_CACHE_LLC);
> +       u64 * const vaddr = kmap_atomic(pd->base.page);
> +
> +       memset64(vaddr, daddr, entries);
> +       kunmap_atomic(vaddr);
> +
> +       memset_p(pd->entry, to, entries);
> +}
> +
> +static struct i915_page_directory *
> +gen8_alloc_top_pd(struct i915_address_space *vm)
> +{
> +       struct i915_page_directory *pd;
> +
> +       if (i915_vm_is_4lvl(vm)) {
> +               pd = alloc_pd(vm);
> +               if (!IS_ERR(pd))
> +                       init_pd(vm, pd, vm->scratch_pdp);
> +
> +               return pd;
> +       }
> +
> +       /* 3lvl */
> +       pd = __alloc_pd();
> +       if (!pd)
> +               return ERR_PTR(-ENOMEM);
> +
> +       pd->entry[GEN8_3LVL_PDPES] = NULL;
> +
> +       if (unlikely(setup_page_dma(vm, &pd->base))) {
> +               kfree(pd);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       init_pd_n(vm, pd, vm->scratch_pd, GEN8_3LVL_PDPES);
> +
> +       return pd;
> +}
> +
>  /*
>   * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers
>   * with a net effect resembling a 2-level page table in normal x86 terms. Each
> @@ -1548,6 +1584,7 @@ static void ppgtt_init(struct drm_i915_private *i915,
>   */
>  static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
>  {
> +       struct i915_address_space *vm;
>         struct i915_ppgtt *ppgtt;
>         int err;
>  
> @@ -1557,70 +1594,59 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
>  
>         ppgtt_init(i915, ppgtt);
>  
> +       vm = &ppgtt->vm;

Been having this debate with Tursulin, whether or not it is more
confusing to have a local alias here. I think on reading it, it is much
clearer that we are setting up one object if we use ppgtt->vm.foo than
it is to alternate between ppgtt->foo and vm->bar.

I'd suggest leaving it as ppgtt->vm.foo in this patch.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/gtt: pde entry encoding is identical

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/gtt: pde entry encoding is identical
URL   : https://patchwork.freedesktop.org/series/62324/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/gtt: pde entry encoding is identical
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1555:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1555:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1521:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1521:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Tear down setup and cleanup macros for page dma
Okay!

Commit: drm/i915/gtt: Setup phys pages for 3lvl pdps
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/gtt: pde entry encoding is identical

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/gtt: pde entry encoding is identical
URL   : https://patchwork.freedesktop.org/series/62324/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6294 -> Patchwork_13331
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/

Known issues
------------

  Here are the changes found in Patchwork_13331 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@basic-all:
    - fi-icl-guc:         [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/fi-icl-guc/igt@gem_exec_basic@basic-all.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/fi-icl-guc/igt@gem_exec_basic@basic-all.html

  * igt@i915_module_load@reload-no-display:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/fi-icl-u3/igt@i915_module_load@reload-no-display.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/fi-icl-u3/igt@i915_module_load@reload-no-display.html

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       [PASS][5] -> [DMESG-WARN][6] ([fdo#107709])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/fi-bsw-kefka/igt@i915_selftest@live_evict.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/fi-bsw-kefka/igt@i915_selftest@live_evict.html

  * igt@kms_chamelium@dp-edid-read:
    - fi-kbl-7500u:       [PASS][7] -> [FAIL][8] ([fdo#106766])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-dsi:         [PASS][9] -> [FAIL][10] ([fdo#103167])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_pread@basic:
    - fi-icl-u3:          [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/fi-icl-u3/igt@gem_pread@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/fi-icl-u3/igt@gem_pread@basic.html

  * igt@i915_module_load@reload:
    - fi-icl-dsi:         [INCOMPLETE][13] ([fdo#107713]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/fi-icl-dsi/igt@i915_module_load@reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/fi-icl-dsi/igt@i915_module_load@reload.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       [INCOMPLETE][15] ([fdo#107718]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#106766]: https://bugs.freedesktop.org/show_bug.cgi?id=106766
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724


Participating hosts (44 -> 37)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6294 -> Patchwork_13331

  CI_DRM_6294: 6ddceb4b9fe7a9f74bf090bd0d788c5f2fec2915 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5059: 1f67ee0d09d6513f487f2be74aae9700e755258a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13331: 3fc849b75c9d69c762c54dc620423bc9804631a3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3fc849b75c9d drm/i915/gtt: Setup phys pages for 3lvl pdps
4f46c24d9757 drm/i915/gtt: Tear down setup and cleanup macros for page dma
224a6de52ba2 drm/i915/gtt: pde entry encoding is identical

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for series starting with [1/3] drm/i915/gtt: pde entry encoding is identical

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/gtt: pde entry encoding is identical
URL   : https://patchwork.freedesktop.org/series/62324/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6294_full -> Patchwork_13331_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13331_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13331_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13331_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_sync@basic-store-each:
    - shard-glk:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-glk2/igt@gem_sync@basic-store-each.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-glk4/igt@gem_sync@basic-store-each.html

  
Known issues
------------

  Here are the changes found in Patchwork_13331_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#110854])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-iclb2/igt@gem_exec_balancer@smoke.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-iclb8/igt@gem_exec_balancer@smoke.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#104108]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-skl2/igt@gem_softpin@noreloc-s3.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-skl9/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-kbl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#110913 ]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-kbl2/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-kbl6/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-snb:          [PASS][9] -> [DMESG-WARN][10] ([fdo#110913 ]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-snb7/igt@gem_userptr_blits@sync-unmap-cycles.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-snb6/igt@gem_userptr_blits@sync-unmap-cycles.html
    - shard-apl:          [PASS][11] -> [DMESG-WARN][12] ([fdo#110913 ]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-apl6/igt@gem_userptr_blits@sync-unmap-cycles.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-apl4/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@i915_selftest@live_evict:
    - shard-kbl:          [PASS][13] -> [INCOMPLETE][14] ([fdo#103665] / [fdo#110938])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-kbl7/igt@i915_selftest@live_evict.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-kbl3/igt@i915_selftest@live_evict.html

  * igt@i915_selftest@live_hangcheck:
    - shard-snb:          [PASS][15] -> [INCOMPLETE][16] ([fdo#105411]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-snb1/igt@i915_selftest@live_hangcheck.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-snb6/igt@i915_selftest@live_hangcheck.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-kbl:          [PASS][17] -> [INCOMPLETE][18] ([fdo#103665])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_cursor_legacy@cursorb-vs-flipb-atomic:
    - shard-hsw:          [PASS][19] -> [SKIP][20] ([fdo#109271]) +13 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-hsw2/igt@kms_cursor_legacy@cursorb-vs-flipb-atomic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-hsw1/igt@kms_cursor_legacy@cursorb-vs-flipb-atomic.html

  * igt@kms_flip_tiling@flip-yf-tiled:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#108145])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-skl5/igt@kms_flip_tiling@flip-yf-tiled.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-skl5/igt@kms_flip_tiling@flip-yf-tiled.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         [PASS][23] -> [FAIL][24] ([fdo#103167]) +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][25] -> [FAIL][26] ([fdo#103166])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109642])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-iclb8/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109441]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-iclb6/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][31] -> [FAIL][32] ([fdo#99912])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-apl6/igt@kms_setmode@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-apl6/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-c-accuracy-idle:
    - shard-iclb:         [PASS][33] -> [INCOMPLETE][34] ([fdo#107713])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-iclb5/igt@kms_vblank@pipe-c-accuracy-idle.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-iclb7/igt@kms_vblank@pipe-c-accuracy-idle.html

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-10ms:
    - shard-kbl:          [DMESG-WARN][35] ([fdo#110913 ]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-kbl6/igt@gem_eio@in-flight-10ms.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-kbl7/igt@gem_eio@in-flight-10ms.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-snb:          [DMESG-WARN][37] ([fdo#110789] / [fdo#110913 ]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-snb6/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-snb4/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_persistent_relocs@forked-interruptible-thrash-inactive:
    - shard-snb:          [INCOMPLETE][39] ([fdo#105411]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-thrash-inactive.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-snb5/igt@gem_persistent_relocs@forked-interruptible-thrash-inactive.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-apl:          [DMESG-WARN][41] ([fdo#110913 ]) -> [PASS][42] +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-apl4/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-apl7/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-snb:          [DMESG-WARN][43] ([fdo#110913 ]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-snb2/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-snb1/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@i915_pm_rpm@gem-idle:
    - shard-kbl:          [DMESG-WARN][45] ([fdo#103558] / [fdo#105602]) -> [PASS][46] +6 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-kbl4/igt@i915_pm_rpm@gem-idle.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-kbl3/igt@i915_pm_rpm@gem-idle.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][47] ([fdo#108566]) -> [PASS][48] +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-apl1/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-snb:          [DMESG-WARN][49] -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-snb1/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-snb1/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x21-random:
    - shard-apl:          [FAIL][51] ([fdo#103232]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-apl3/igt@kms_cursor_crc@pipe-b-cursor-64x21-random.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-apl7/igt@kms_cursor_crc@pipe-b-cursor-64x21-random.html

  * igt@kms_cursor_crc@pipe-c-cursor-64x21-onscreen:
    - shard-skl:          [FAIL][53] ([fdo#103232]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-skl3/igt@kms_cursor_crc@pipe-c-cursor-64x21-onscreen.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-skl10/igt@kms_cursor_crc@pipe-c-cursor-64x21-onscreen.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [INCOMPLETE][55] ([fdo#109507]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-skl3/igt@kms_flip@flip-vs-suspend.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-skl10/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-snb:          [DMESG-WARN][57] ([fdo#102365]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-snb4/igt@kms_flip@flip-vs-suspend-interruptible.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-snb7/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         [FAIL][59] ([fdo#103167]) -> [PASS][60] +6 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite:
    - shard-hsw:          [SKIP][61] ([fdo#109271]) -> [PASS][62] +15 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-hsw6/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][63] ([fdo#108145]) -> [PASS][64]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][65] ([fdo#109441]) -> [PASS][66] +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-iclb6/igt@kms_psr@psr2_basic.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-iclb2/igt@kms_psr@psr2_basic.html

  * igt@kms_setmode@basic:
    - shard-hsw:          [FAIL][67] ([fdo#99912]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-hsw2/igt@kms_setmode@basic.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-hsw1/igt@kms_setmode@basic.html

  
#### Warnings ####

  * igt@kms_atomic_transition@5x-modeset-transitions-nonblocking:
    - shard-kbl:          [SKIP][69] ([fdo#105602] / [fdo#109271] / [fdo#109278]) -> [SKIP][70] ([fdo#109271] / [fdo#109278])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-kbl4/igt@kms_atomic_transition@5x-modeset-transitions-nonblocking.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-kbl3/igt@kms_atomic_transition@5x-modeset-transitions-nonblocking.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-kbl:          [SKIP][71] ([fdo#105602] / [fdo#109271]) -> [SKIP][72] ([fdo#109271]) +1 similar issue
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-gtt.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-skl:          [FAIL][73] ([fdo#108040]) -> [FAIL][74] ([fdo#103167])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6294/shard-skl1/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/shard-skl5/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt.html

  
  [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110789]: https://bugs.freedesktop.org/show_bug.cgi?id=110789
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#110913 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110913 
  [fdo#110938]: https://bugs.freedesktop.org/show_bug.cgi?id=110938
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_6294 -> Patchwork_13331

  CI_DRM_6294: 6ddceb4b9fe7a9f74bf090bd0d788c5f2fec2915 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5059: 1f67ee0d09d6513f487f2be74aae9700e755258a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13331: 3fc849b75c9d69c762c54dc620423bc9804631a3 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13331/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/gtt: pde entry encoding is identical

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-07-04 16:44:05)
>> +#define set_pd_entry(pd, pde, to)  ({                             \
>> +       __write_pd_entry((pd), (pde), px_base(to), gen8_pde_encode); \
>> +       atomic_inc(&(pd)->used);                                   \
>
> inc before write so that you have a nice onion with clear.
>
>> +})
>> +
>> +#define clear_pd_entry(pd, pde, to) ({                              \
>
> You want to pull the GEM_BUG_ON here so that is tightly coupled with the
> atomic_dec -- it's an underflow check.

I think I tried that but found out that when we free the ppgtt,
we want to be fast and don't care about the counts matching.

Well, that could be made to match tho. I will take a look.

-Mika

>
>> +       __write_pd_entry((pd), (pde), px_base(to), gen8_pde_encode); \
>> +       atomic_dec(&pd->used);                                       \
>> +})
>
> I would have preferred these as inlines (even if means "passing" an
> extra arg), but let's see what the next two patches bring.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/gtt: pde entry encoding is identical

[Chris Wilson]

Quoting Mika Kuoppala (2019-07-04 16:44:05)
> +#define set_pd_entry(pd, pde, to)  ({                             \
> +       __write_pd_entry((pd), (pde), px_base(to), gen8_pde_encode); \
> +       atomic_inc(&(pd)->used);                                   \

inc before write so that you have a nice onion with clear.

> +})
> +
> +#define clear_pd_entry(pd, pde, to) ({                              \

You want to pull the GEM_BUG_ON here so that is tightly coupled with the
atomic_dec -- it's an underflow check.

> +       __write_pd_entry((pd), (pde), px_base(to), gen8_pde_encode); \
> +       atomic_dec(&pd->used);                                       \
> +})

I would have preferred these as inlines (even if means "passing" an
extra arg), but let's see what the next two patches bring.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/3] drm/i915/gtt: pde entry encoding is identical

[Mika Kuoppala]

For all page directory entries, the pde encoding is
identical. Don't compilicate call sites with different
versions of doing the same thing. We check the existence of
physical page before writing the entry into it. This further
generalizes the pd so that manipulation in callsites will be
identical, removing the need to handle pdps differently for gen8.

v2: squash
v3: inc/dec with set/clear (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 137 +++++++++++-----------------
 drivers/gpu/drm/i915/i915_gem_gtt.h |   3 -
 2 files changed, 52 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9756f1b670e9..4709948a6c0e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -211,10 +211,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;
@@ -223,9 +223,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)
@@ -777,24 +774,43 @@ 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 __write_dma_entry(struct i915_page_dma * const pdma,
+			      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(pdma->page);
+	vaddr[pde] = encoded_entry;
+	kunmap_atomic(vaddr);
 }
 
+static inline void
+__write_pd_entry(struct i915_page_directory * const pd,
+		 const unsigned short pde,
+		 struct i915_page_dma * const to,
+		 u64 (*encode)(const dma_addr_t, const enum i915_cache_level))
+{
+	pd->entry[pde] = to;
+	__write_dma_entry(px_base(pd), pde, encode(to->daddr, I915_CACHE_LLC));
+}
+
+#define set_pd_entry(pd, pde, to)  ({				   \
+	__write_pd_entry((pd), (pde), px_base(to), gen8_pde_encode); \
+	atomic_inc(&(pd)->used);				   \
+})
+
+#define clear_pd_entry(pd, pde, to) ({				     \
+	__write_pd_entry((pd), (pde), px_base(to), gen8_pde_encode); \
+	atomic_dec(&pd->used);					     \
+})
+
 /*
  * PDE TLBs are a pain to invalidate on GEN8+. When we modify
  * the page table structures, we mark them dirty so that
@@ -824,18 +840,6 @@ static bool gen8_ppgtt_clear_pt(const struct i915_address_space *vm,
 	return !atomic_sub_return(num_entries, &pt->used);
 }
 
-static void gen8_ppgtt_set_pde(struct i915_address_space *vm,
-			       struct i915_page_directory *pd,
-			       struct i915_page_table *pt,
-			       unsigned int pde)
-{
-	gen8_pde_t *vaddr;
-
-	vaddr = kmap_atomic_px(pd);
-	vaddr[pde] = gen8_pde_encode(px_dma(pt), I915_CACHE_LLC);
-	kunmap_atomic(vaddr);
-}
-
 static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 				struct i915_page_directory *pd,
 				u64 start, u64 length)
@@ -853,11 +857,8 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 
 		spin_lock(&pd->lock);
 		if (!atomic_read(&pt->used)) {
-			gen8_ppgtt_set_pde(vm, pd, vm->scratch_pt, pde);
-			pd->entry[pde] = vm->scratch_pt;
-
 			GEM_BUG_ON(!atomic_read(&pd->used));
-			atomic_dec(&pd->used);
+			clear_pd_entry(pd, pde, vm->scratch_pt);
 			free = true;
 		}
 		spin_unlock(&pd->lock);
@@ -868,20 +869,6 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 	return !atomic_read(&pd->used);
 }
 
-static void gen8_ppgtt_set_pdpe(struct i915_page_directory *pdp,
-				struct i915_page_directory *pd,
-				unsigned int pdpe)
-{
-	gen8_ppgtt_pdpe_t *vaddr;
-
-	if (!pd_has_phys_page(pdp))
-		return;
-
-	vaddr = kmap_atomic_px(pdp);
-	vaddr[pdpe] = gen8_pdpe_encode(px_dma(pd), I915_CACHE_LLC);
-	kunmap_atomic(vaddr);
-}
-
 /* Removes entries from a single page dir pointer, releasing it if it's empty.
  * Caller can use the return value to update higher-level entries
  */
@@ -902,11 +889,8 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
 
 		spin_lock(&pdp->lock);
 		if (!atomic_read(&pd->used)) {
-			gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
-			pdp->entry[pdpe] = vm->scratch_pd;
-
 			GEM_BUG_ON(!atomic_read(&pdp->used));
-			atomic_dec(&pdp->used);
+			clear_pd_entry(pdp, pdpe, vm->scratch_pd);
 			free = true;
 		}
 		spin_unlock(&pdp->lock);
@@ -923,17 +907,6 @@ static void gen8_ppgtt_clear_3lvl(struct i915_address_space *vm,
 	gen8_ppgtt_clear_pdp(vm, i915_vm_to_ppgtt(vm)->pd, start, length);
 }
 
-static void gen8_ppgtt_set_pml4e(struct i915_page_directory *pml4,
-				 struct i915_page_directory *pdp,
-				 unsigned int pml4e)
-{
-	gen8_ppgtt_pml4e_t *vaddr;
-
-	vaddr = kmap_atomic_px(pml4);
-	vaddr[pml4e] = gen8_pml4e_encode(px_dma(pdp), I915_CACHE_LLC);
-	kunmap_atomic(vaddr);
-}
-
 /* Removes entries from a single pml4.
  * This is the top-level structure in 4-level page tables used on gen8+.
  * Empty entries are always scratch pml4e.
@@ -957,8 +930,7 @@ static void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
 
 		spin_lock(&pml4->lock);
 		if (!atomic_read(&pdp->used)) {
-			gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
-			pml4->entry[pml4e] = vm->scratch_pdp;
+			clear_pd_entry(pml4, pml4e, vm->scratch_pdp);
 			free = true;
 		}
 		spin_unlock(&pml4->lock);
@@ -1275,7 +1247,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 	}
 
 	gen8_initialize_pt(vm, vm->scratch_pt);
-	init_pd_with_page(vm, vm->scratch_pd, vm->scratch_pt);
+	init_pd(vm, vm->scratch_pd, vm->scratch_pt);
 	if (i915_vm_is_4lvl(vm))
 		init_pd(vm, vm->scratch_pdp, vm->scratch_pd);
 
@@ -1298,6 +1270,11 @@ static int gen8_ppgtt_notify_vgt(struct i915_ppgtt *ppgtt, bool create)
 	enum vgt_g2v_type msg;
 	int i;
 
+	if (create)
+		atomic_inc(&ppgtt->pd->used); /* never remove */
+	else
+		atomic_dec(&ppgtt->pd->used);
+
 	if (i915_vm_is_4lvl(vm)) {
 		const u64 daddr = px_dma(ppgtt->pd);
 
@@ -1414,9 +1391,7 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 
 			spin_lock(&pd->lock);
 			if (pd->entry[pde] == vm->scratch_pt) {
-				gen8_ppgtt_set_pde(vm, pd, pt, pde);
-				pd->entry[pde] = pt;
-				atomic_inc(&pd->used);
+				set_pd_entry(pd, pde, pt);
 			} else {
 				alloc = pt;
 				pt = pd->entry[pde];
@@ -1458,13 +1433,11 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 				goto unwind;
 			}
 
-			init_pd_with_page(vm, pd, vm->scratch_pt);
+			init_pd(vm, pd, vm->scratch_pt);
 
 			spin_lock(&pdp->lock);
 			if (pdp->entry[pdpe] == vm->scratch_pd) {
-				gen8_ppgtt_set_pdpe(pdp, pd, pdpe);
-				pdp->entry[pdpe] = pd;
-				atomic_inc(&pdp->used);
+				set_pd_entry(pdp, pdpe, pd);
 			} else {
 				alloc = pd;
 				pd = pdp->entry[pdpe];
@@ -1490,11 +1463,10 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 	}
 	spin_lock(&pdp->lock);
 	if (atomic_dec_and_test(&pd->used)) {
-		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
 		GEM_BUG_ON(!atomic_read(&pdp->used));
-		atomic_dec(&pdp->used);
 		GEM_BUG_ON(alloc);
 		alloc = pd; /* defer the free to after the lock */
+		clear_pd_entry(pdp, pdpe, vm->scratch_pd);
 	}
 	spin_unlock(&pdp->lock);
 unwind:
@@ -1539,8 +1511,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 
 			spin_lock(&pml4->lock);
 			if (pml4->entry[pml4e] == vm->scratch_pdp) {
-				gen8_ppgtt_set_pml4e(pml4, pdp, pml4e);
-				pml4->entry[pml4e] = pdp;
+				set_pd_entry(pml4, pml4e, pdp);
 			} else {
 				alloc = pdp;
 				pdp = pml4->entry[pml4e];
@@ -1566,9 +1537,9 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 	}
 	spin_lock(&pml4->lock);
 	if (atomic_dec_and_test(&pdp->used)) {
-		gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
 		GEM_BUG_ON(alloc);
 		alloc = pdp; /* defer the free until after the lock */
+		set_pd_entry(pml4, pml4e, vm->scratch_pdp);
 	}
 	spin_unlock(&pml4->lock);
 unwind:
@@ -1593,20 +1564,16 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 		if (IS_ERR(pd))
 			goto unwind;
 
-		init_pd_with_page(vm, pd, vm->scratch_pt);
-		gen8_ppgtt_set_pdpe(pdp, pd, pdpe);
-
-		atomic_inc(&pdp->used);
+		init_pd(vm, pd, vm->scratch_pt);
+		set_pd_entry(pdp, pdpe, pd);
 	}
 
-	atomic_inc(&pdp->used); /* never remove */
-
 	return 0;
 
 unwind:
 	start -= from;
 	gen8_for_each_pdpe(pd, pdp, from, start, pdpe) {
-		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
+		clear_pd_entry(pdp, pdpe, vm->scratch_pd);
 		free_pd(vm, pd);
 	}
 	atomic_set(&pdp->used, 0);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index d0e0905acbbb..57a68ef4eda7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -72,9 +72,6 @@ struct intel_gt;
 
 typedef u32 gen6_pte_t;
 typedef u64 gen8_pte_t;
-typedef u64 gen8_pde_t;
-typedef u64 gen8_ppgtt_pdpe_t;
-typedef u64 gen8_ppgtt_pml4e_t;
 
 #define ggtt_total_entries(ggtt) ((ggtt)->vm.total >> PAGE_SHIFT)
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx