public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs
@ 2019-06-14 16:43 Mika Kuoppala
  2019-06-14 16:43 ` [PATCH 02/10] drm/i915/gtt: Use a common type for page directories Mika Kuoppala
                   ` (12 more replies)
  0 siblings, 13 replies; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

We set them to scratch right after allocation so prevent
useless zeroing before.

v2: atomic_t
v3: allow pdp alloc fail

Cc: Chris Wilson <chris@chris-wilson.co.uk>
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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 278de04a96aa..b210cb021931 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -687,7 +687,7 @@ static struct i915_page_directory *alloc_pd(struct i915_address_space *vm)
 {
 	struct i915_page_directory *pd;
 
-	pd = kzalloc(sizeof(*pd), I915_GFP_ALLOW_FAIL);
+	pd = kmalloc(sizeof(*pd), I915_GFP_ALLOW_FAIL);
 	if (unlikely(!pd))
 		return ERR_PTR(-ENOMEM);
 
@@ -747,7 +747,7 @@ alloc_pdp(struct i915_address_space *vm)
 
 	GEM_BUG_ON(!i915_vm_is_4lvl(vm));
 
-	pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
+	pdp = kmalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
 	if (!pdp)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/10] drm/i915/gtt: Use a common type for page directories
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 16:56   ` Chris Wilson
  2019-06-14 16:43 ` [PATCH 03/10] drm/i915/gtt: Introduce init_pd_with_page Mika Kuoppala
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

All page directories are identical in function, only the position in the
hierarchy differ. Use same base type for directory functionality.

v2: cleanup, size always 512, init to null

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c |   2 +-
 drivers/gpu/drm/i915/gt/intel_lrc_reg.h     |   2 +-
 drivers/gpu/drm/i915/gt/intel_ringbuffer.c  |   2 +-
 drivers/gpu/drm/i915/gvt/scheduler.c        |  30 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c         | 351 ++++++++++----------
 drivers/gpu/drm/i915/i915_gem_gtt.h         |  64 ++--
 6 files changed, 236 insertions(+), 215 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index c86ca9f21532..dbab0ab1cef1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1038,7 +1038,7 @@ static int emit_ppgtt_update(struct i915_request *rq, void *data)
 
 	if (i915_vm_is_4lvl(vm)) {
 		struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
-		const dma_addr_t pd_daddr = px_dma(&ppgtt->pml4);
+		const dma_addr_t pd_daddr = px_dma(ppgtt->pd);
 
 		cs = intel_ring_begin(rq, 6);
 		if (IS_ERR(cs))
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
index 5ef932d810a7..6bf34738b4e5 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
+++ b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
@@ -55,7 +55,7 @@
 
 #define ASSIGN_CTX_PML4(ppgtt, reg_state) do { \
 	u32 *reg_state__ = (reg_state); \
-	const u64 addr__ = px_dma(&ppgtt->pml4); \
+	const u64 addr__ = px_dma(ppgtt->pd); \
 	(reg_state__)[CTX_PDP0_UDW + 1] = upper_32_bits(addr__); \
 	(reg_state__)[CTX_PDP0_LDW + 1] = lower_32_bits(addr__); \
 } while (0)
diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
index b3bf47e8162f..ecd4e2008ef3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
@@ -1523,7 +1523,7 @@ static int load_pd_dir(struct i915_request *rq, const struct i915_ppgtt *ppgtt)
 
 	*cs++ = MI_LOAD_REGISTER_IMM(1);
 	*cs++ = i915_mmio_reg_offset(RING_PP_DIR_BASE(engine->mmio_base));
-	*cs++ = ppgtt->pd.base.ggtt_offset << 10;
+	*cs++ = ppgtt->pd->base.ggtt_offset << 10;
 
 	intel_ring_advance(rq, cs);
 
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 9029de4b4651..2144fb46d0e1 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -375,11 +375,13 @@ static int set_context_ppgtt_from_shadow(struct intel_vgpu_workload *workload,
 		return -EINVAL;
 
 	if (mm->ppgtt_mm.root_entry_type == GTT_TYPE_PPGTT_ROOT_L4_ENTRY) {
-		px_dma(&ppgtt->pml4) = mm->ppgtt_mm.shadow_pdps[0];
+		px_dma(ppgtt->pd) = mm->ppgtt_mm.shadow_pdps[0];
 	} else {
 		for (i = 0; i < GVT_RING_CTX_NR_PDPS; i++) {
-			px_dma(ppgtt->pdp.page_directory[i]) =
-				mm->ppgtt_mm.shadow_pdps[i];
+			struct i915_page_directory * const pd =
+				i915_pd_entry(ppgtt->pd, i);
+
+			px_dma(pd) = mm->ppgtt_mm.shadow_pdps[i];
 		}
 	}
 
@@ -1128,11 +1130,14 @@ i915_context_ppgtt_root_restore(struct intel_vgpu_submission *s,
 	int i;
 
 	if (i915_vm_is_4lvl(&ppgtt->vm)) {
-		px_dma(&ppgtt->pml4) = s->i915_context_pml4;
+		px_dma(ppgtt->pd) = s->i915_context_pml4;
 	} else {
-		for (i = 0; i < GEN8_3LVL_PDPES; i++)
-			px_dma(ppgtt->pdp.page_directory[i]) =
-				s->i915_context_pdps[i];
+		for (i = 0; i < GEN8_3LVL_PDPES; i++) {
+			struct i915_page_directory * const pd =
+				i915_pd_entry(ppgtt->pd, i);
+
+			px_dma(pd) = s->i915_context_pdps[i];
+		}
 	}
 }
 
@@ -1186,11 +1191,14 @@ i915_context_ppgtt_root_save(struct intel_vgpu_submission *s,
 	int i;
 
 	if (i915_vm_is_4lvl(&ppgtt->vm)) {
-		s->i915_context_pml4 = px_dma(&ppgtt->pml4);
+		s->i915_context_pml4 = px_dma(ppgtt->pd);
 	} else {
-		for (i = 0; i < GEN8_3LVL_PDPES; i++)
-			s->i915_context_pdps[i] =
-				px_dma(ppgtt->pdp.page_directory[i]);
+		for (i = 0; i < GEN8_3LVL_PDPES; i++) {
+			struct i915_page_directory * const pd =
+				i915_pd_entry(ppgtt->pd, i);
+
+			s->i915_context_pdps[i] = px_dma(pd);
+		}
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b210cb021931..ef3fa6b5ef10 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -661,7 +661,8 @@ static struct i915_page_table *alloc_pt(struct i915_address_space *vm)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	atomic_set(&pt->used_ptes, 0);
+	atomic_set(&pt->used, 0);
+
 	return pt;
 }
 
@@ -683,11 +684,30 @@ static void gen6_initialize_pt(struct i915_address_space *vm,
 	fill32_px(vm, pt, vm->scratch_pte);
 }
 
-static struct i915_page_directory *alloc_pd(struct i915_address_space *vm)
+static struct i915_page_directory *__alloc_pd(void)
 {
 	struct i915_page_directory *pd;
 
 	pd = kmalloc(sizeof(*pd), I915_GFP_ALLOW_FAIL);
+
+	if (unlikely(!pd))
+		return NULL;
+
+	memset(&pd->base, 0, sizeof(pd->base));
+	atomic_set(&pd->used, 0);
+	spin_lock_init(&pd->lock);
+
+	/* for safety */
+	pd->entry[0] = NULL;
+
+	return pd;
+}
+
+static struct i915_page_directory *alloc_pd(struct i915_address_space *vm)
+{
+	struct i915_page_directory *pd;
+
+	pd = __alloc_pd();
 	if (unlikely(!pd))
 		return ERR_PTR(-ENOMEM);
 
@@ -696,8 +716,6 @@ static struct i915_page_directory *alloc_pd(struct i915_address_space *vm)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	atomic_set(&pd->used_pdes, 0);
-	spin_lock_init(&pd->lock);
 	return pd;
 }
 
@@ -713,88 +731,56 @@ static void gen8_initialize_pd(struct i915_address_space *vm,
 {
 	fill_px(vm, pd,
 		gen8_pde_encode(px_dma(vm->scratch_pt), I915_CACHE_LLC));
-	memset_p((void **)pd->page_table, vm->scratch_pt, I915_PDES);
+	memset_p(pd->entry, vm->scratch_pt, I915_PDES);
 }
 
-static int __pdp_init(struct i915_address_space *vm,
-		      struct i915_page_directory_pointer *pdp)
+static struct i915_page_directory *alloc_pdp(struct i915_address_space *vm)
 {
-	const unsigned int pdpes = i915_pdpes_per_pdp(vm);
-
-	pdp->page_directory = kmalloc_array(pdpes, sizeof(*pdp->page_directory),
-					    I915_GFP_ALLOW_FAIL);
-	if (unlikely(!pdp->page_directory))
-		return -ENOMEM;
+	struct i915_page_directory *pdp;
 
-	memset_p((void **)pdp->page_directory, vm->scratch_pd, pdpes);
-
-	atomic_set(&pdp->used_pdpes, 0);
-	spin_lock_init(&pdp->lock);
-	return 0;
-}
-
-static void __pdp_fini(struct i915_page_directory_pointer *pdp)
-{
-	kfree(pdp->page_directory);
-	pdp->page_directory = NULL;
-}
-
-static struct i915_page_directory_pointer *
-alloc_pdp(struct i915_address_space *vm)
-{
-	struct i915_page_directory_pointer *pdp;
-	int ret = -ENOMEM;
-
-	GEM_BUG_ON(!i915_vm_is_4lvl(vm));
-
-	pdp = kmalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
+	pdp = __alloc_pd();
 	if (!pdp)
 		return ERR_PTR(-ENOMEM);
 
-	ret = __pdp_init(vm, pdp);
-	if (ret)
-		goto fail_bitmap;
-
-	ret = setup_px(vm, pdp);
-	if (ret)
-		goto fail_page_m;
+	if (i915_vm_is_4lvl(vm)) {
+		if (unlikely(setup_px(vm, pdp))) {
+			kfree(pdp);
+			return ERR_PTR(-ENOMEM);
+		}
+	}
 
 	return pdp;
-
-fail_page_m:
-	__pdp_fini(pdp);
-fail_bitmap:
-	kfree(pdp);
-
-	return ERR_PTR(ret);
 }
 
 static void free_pdp(struct i915_address_space *vm,
-		     struct i915_page_directory_pointer *pdp)
+		     struct i915_page_directory *pdp)
 {
-	__pdp_fini(pdp);
-
-	if (!i915_vm_is_4lvl(vm))
-		return;
+	if (i915_vm_is_4lvl(vm))
+		cleanup_px(vm, pdp);
 
-	cleanup_px(vm, pdp);
 	kfree(pdp);
 }
 
-static void gen8_initialize_pdp(struct i915_address_space *vm,
-				struct i915_page_directory_pointer *pdp)
+static void gen8_initialize_4lvl_pdp(struct i915_address_space *vm,
+				     struct i915_page_directory *pdp)
 {
 	fill_px(vm, pdp,
 		gen8_pdpe_encode(px_dma(vm->scratch_pd), I915_CACHE_LLC));
+	memset_p(pdp->entry, vm->scratch_pd, 512);
+}
+
+static void gen8_initialize_3lvl_pdp(struct i915_address_space *vm,
+				     struct i915_page_directory *pdp)
+{
+	memset_p(pdp->entry, vm->scratch_pd, GEN8_3LVL_PDPES);
 }
 
 static void gen8_initialize_pml4(struct i915_address_space *vm,
-				 struct i915_pml4 *pml4)
+				 struct i915_page_directory *pml4)
 {
 	fill_px(vm, pml4,
 		gen8_pml4e_encode(px_dma(vm->scratch_pdp), I915_CACHE_LLC));
-	memset_p((void **)pml4->pdps, vm->scratch_pdp, GEN8_PML4ES_PER_PML4);
-	spin_lock_init(&pml4->lock);
+	memset_p(pml4->entry, vm->scratch_pdp, GEN8_PML4ES_PER_PML4);
 }
 
 /*
@@ -822,8 +808,8 @@ static bool gen8_ppgtt_clear_pt(const struct i915_address_space *vm,
 	memset64(vaddr + gen8_pte_index(start), vm->scratch_pte, num_entries);
 	kunmap_atomic(vaddr);
 
-	GEM_BUG_ON(num_entries > atomic_read(&pt->used_ptes));
-	return !atomic_sub_return(num_entries, &pt->used_ptes);
+	GEM_BUG_ON(num_entries > atomic_read(&pt->used));
+	return !atomic_sub_return(num_entries, &pt->used);
 }
 
 static void gen8_ppgtt_set_pde(struct i915_address_space *vm,
@@ -854,12 +840,12 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 			continue;
 
 		spin_lock(&pd->lock);
-		if (!atomic_read(&pt->used_ptes)) {
+		if (!atomic_read(&pt->used)) {
 			gen8_ppgtt_set_pde(vm, pd, vm->scratch_pt, pde);
-			pd->page_table[pde] = vm->scratch_pt;
+			pd->entry[pde] = vm->scratch_pt;
 
-			GEM_BUG_ON(!atomic_read(&pd->used_pdes));
-			atomic_dec(&pd->used_pdes);
+			GEM_BUG_ON(!atomic_read(&pd->used));
+			atomic_dec(&pd->used);
 			free = true;
 		}
 		spin_unlock(&pd->lock);
@@ -867,11 +853,11 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 			free_pt(vm, pt);
 	}
 
-	return !atomic_read(&pd->used_pdes);
+	return !atomic_read(&pd->used);
 }
 
 static void gen8_ppgtt_set_pdpe(struct i915_address_space *vm,
-				struct i915_page_directory_pointer *pdp,
+				struct i915_page_directory *pdp,
 				struct i915_page_directory *pd,
 				unsigned int pdpe)
 {
@@ -889,7 +875,7 @@ static void gen8_ppgtt_set_pdpe(struct i915_address_space *vm,
  * Caller can use the return value to update higher-level entries
  */
 static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
-				 struct i915_page_directory_pointer *pdp,
+				 struct i915_page_directory * const pdp,
 				 u64 start, u64 length)
 {
 	struct i915_page_directory *pd;
@@ -904,12 +890,12 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
 			continue;
 
 		spin_lock(&pdp->lock);
-		if (!atomic_read(&pd->used_pdes)) {
+		if (!atomic_read(&pd->used)) {
 			gen8_ppgtt_set_pdpe(vm, pdp, vm->scratch_pd, pdpe);
-			pdp->page_directory[pdpe] = vm->scratch_pd;
+			pdp->entry[pdpe] = vm->scratch_pd;
 
-			GEM_BUG_ON(!atomic_read(&pdp->used_pdpes));
-			atomic_dec(&pdp->used_pdpes);
+			GEM_BUG_ON(!atomic_read(&pdp->used));
+			atomic_dec(&pdp->used);
 			free = true;
 		}
 		spin_unlock(&pdp->lock);
@@ -917,17 +903,17 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
 			free_pd(vm, pd);
 	}
 
-	return !atomic_read(&pdp->used_pdpes);
+	return !atomic_read(&pdp->used);
 }
 
 static void gen8_ppgtt_clear_3lvl(struct i915_address_space *vm,
 				  u64 start, u64 length)
 {
-	gen8_ppgtt_clear_pdp(vm, &i915_vm_to_ppgtt(vm)->pdp, start, length);
+	gen8_ppgtt_clear_pdp(vm, i915_vm_to_ppgtt(vm)->pd, start, length);
 }
 
-static void gen8_ppgtt_set_pml4e(struct i915_pml4 *pml4,
-				 struct i915_page_directory_pointer *pdp,
+static void gen8_ppgtt_set_pml4e(struct i915_page_directory *pml4,
+				 struct i915_page_directory *pdp,
 				 unsigned int pml4e)
 {
 	gen8_ppgtt_pml4e_t *vaddr;
@@ -945,8 +931,8 @@ static void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
 				  u64 start, u64 length)
 {
 	struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
-	struct i915_pml4 *pml4 = &ppgtt->pml4;
-	struct i915_page_directory_pointer *pdp;
+	struct i915_page_directory * const pml4 = ppgtt->pd;
+	struct i915_page_directory *pdp;
 	unsigned int pml4e;
 
 	GEM_BUG_ON(!i915_vm_is_4lvl(vm));
@@ -959,9 +945,9 @@ static void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
 			continue;
 
 		spin_lock(&pml4->lock);
-		if (!atomic_read(&pdp->used_pdpes)) {
+		if (!atomic_read(&pdp->used)) {
 			gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
-			pml4->pdps[pml4e] = vm->scratch_pdp;
+			pml4->entry[pml4e] = vm->scratch_pdp;
 			free = true;
 		}
 		spin_unlock(&pml4->lock);
@@ -998,7 +984,7 @@ static __always_inline struct gen8_insert_pte gen8_insert_pte(u64 start)
 
 static __always_inline bool
 gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
-			      struct i915_page_directory_pointer *pdp,
+			      struct i915_page_directory *pdp,
 			      struct sgt_dma *iter,
 			      struct gen8_insert_pte *idx,
 			      enum i915_cache_level cache_level,
@@ -1010,8 +996,8 @@ gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
 	bool ret;
 
 	GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(&ppgtt->vm));
-	pd = pdp->page_directory[idx->pdpe];
-	vaddr = kmap_atomic_px(pd->page_table[idx->pde]);
+	pd = i915_pd_entry(pdp, idx->pdpe);
+	vaddr = kmap_atomic_px(i915_pt_entry(pd, idx->pde));
 	do {
 		vaddr[idx->pte] = pte_encode | iter->dma;
 
@@ -1041,11 +1027,11 @@ gen8_ppgtt_insert_pte_entries(struct i915_ppgtt *ppgtt,
 				}
 
 				GEM_BUG_ON(idx->pdpe >= i915_pdpes_per_pdp(&ppgtt->vm));
-				pd = pdp->page_directory[idx->pdpe];
+				pd = pdp->entry[idx->pdpe];
 			}
 
 			kunmap_atomic(vaddr);
-			vaddr = kmap_atomic_px(pd->page_table[idx->pde]);
+			vaddr = kmap_atomic_px(i915_pt_entry(pd, idx->pde));
 		}
 	} while (1);
 	kunmap_atomic(vaddr);
@@ -1062,14 +1048,14 @@ static void gen8_ppgtt_insert_3lvl(struct i915_address_space *vm,
 	struct sgt_dma iter = sgt_dma(vma);
 	struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
 
-	gen8_ppgtt_insert_pte_entries(ppgtt, &ppgtt->pdp, &iter, &idx,
+	gen8_ppgtt_insert_pte_entries(ppgtt, ppgtt->pd, &iter, &idx,
 				      cache_level, flags);
 
 	vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
 }
 
 static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
-					   struct i915_page_directory_pointer **pdps,
+					   struct i915_page_directory *pml4,
 					   struct sgt_dma *iter,
 					   enum i915_cache_level cache_level,
 					   u32 flags)
@@ -1080,8 +1066,9 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
 
 	do {
 		struct gen8_insert_pte idx = gen8_insert_pte(start);
-		struct i915_page_directory_pointer *pdp = pdps[idx.pml4e];
-		struct i915_page_directory *pd = pdp->page_directory[idx.pdpe];
+		struct i915_page_directory *pdp =
+			i915_pdp_entry(pml4, idx.pml4e);
+		struct i915_page_directory *pd = i915_pd_entry(pdp, idx.pdpe);
 		unsigned int page_size;
 		bool maybe_64K = false;
 		gen8_pte_t encode = pte_encode;
@@ -1099,7 +1086,7 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
 
 			vaddr = kmap_atomic_px(pd);
 		} else {
-			struct i915_page_table *pt = pd->page_table[idx.pde];
+			struct i915_page_table *pt = i915_pt_entry(pd, idx.pde);
 
 			index = idx.pte;
 			max = GEN8_PTES;
@@ -1174,7 +1161,8 @@ static void gen8_ppgtt_insert_huge_entries(struct i915_vma *vma,
 				u16 i;
 
 				encode = vma->vm->scratch_pte;
-				vaddr = kmap_atomic_px(pd->page_table[idx.pde]);
+				vaddr = kmap_atomic_px(i915_pt_entry(pd,
+								     idx.pde));
 
 				for (i = 1; i < index; i += 16)
 					memset64(vaddr + i, encode, 15);
@@ -1194,15 +1182,16 @@ static void gen8_ppgtt_insert_4lvl(struct i915_address_space *vm,
 {
 	struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
 	struct sgt_dma iter = sgt_dma(vma);
-	struct i915_page_directory_pointer **pdps = ppgtt->pml4.pdps;
+	struct i915_page_directory * const pml4 = ppgtt->pd;
 
 	if (vma->page_sizes.sg > I915_GTT_PAGE_SIZE) {
-		gen8_ppgtt_insert_huge_entries(vma, pdps, &iter, cache_level,
+		gen8_ppgtt_insert_huge_entries(vma, pml4, &iter, cache_level,
 					       flags);
 	} else {
 		struct gen8_insert_pte idx = gen8_insert_pte(vma->node.start);
 
-		while (gen8_ppgtt_insert_pte_entries(ppgtt, pdps[idx.pml4e++],
+		while (gen8_ppgtt_insert_pte_entries(ppgtt,
+						     i915_pdp_entry(pml4, idx.pml4e++),
 						     &iter, &idx, cache_level,
 						     flags))
 			GEM_BUG_ON(idx.pml4e >= GEN8_PML4ES_PER_PML4);
@@ -1217,8 +1206,8 @@ static void gen8_free_page_tables(struct i915_address_space *vm,
 	int i;
 
 	for (i = 0; i < I915_PDES; i++) {
-		if (pd->page_table[i] != vm->scratch_pt)
-			free_pt(vm, pd->page_table[i]);
+		if (pd->entry[i] != vm->scratch_pt)
+			free_pt(vm, pd->entry[i]);
 	}
 }
 
@@ -1277,7 +1266,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 	gen8_initialize_pt(vm, vm->scratch_pt);
 	gen8_initialize_pd(vm, vm->scratch_pd);
 	if (i915_vm_is_4lvl(vm))
-		gen8_initialize_pdp(vm, vm->scratch_pdp);
+		gen8_initialize_4lvl_pdp(vm, vm->scratch_pdp);
 
 	return 0;
 
@@ -1299,7 +1288,7 @@ static int gen8_ppgtt_notify_vgt(struct i915_ppgtt *ppgtt, bool create)
 	int i;
 
 	if (i915_vm_is_4lvl(vm)) {
-		const u64 daddr = px_dma(&ppgtt->pml4);
+		const u64 daddr = px_dma(ppgtt->pd);
 
 		I915_WRITE(vgtif_reg(pdp[0].lo), lower_32_bits(daddr));
 		I915_WRITE(vgtif_reg(pdp[0].hi), upper_32_bits(daddr));
@@ -1336,17 +1325,17 @@ static void gen8_free_scratch(struct i915_address_space *vm)
 }
 
 static void gen8_ppgtt_cleanup_3lvl(struct i915_address_space *vm,
-				    struct i915_page_directory_pointer *pdp)
+				    struct i915_page_directory *pdp)
 {
 	const unsigned int pdpes = i915_pdpes_per_pdp(vm);
 	int i;
 
 	for (i = 0; i < pdpes; i++) {
-		if (pdp->page_directory[i] == vm->scratch_pd)
+		if (pdp->entry[i] == vm->scratch_pd)
 			continue;
 
-		gen8_free_page_tables(vm, pdp->page_directory[i]);
-		free_pd(vm, pdp->page_directory[i]);
+		gen8_free_page_tables(vm, pdp->entry[i]);
+		free_pd(vm, pdp->entry[i]);
 	}
 
 	free_pdp(vm, pdp);
@@ -1354,16 +1343,19 @@ static void gen8_ppgtt_cleanup_3lvl(struct i915_address_space *vm,
 
 static void gen8_ppgtt_cleanup_4lvl(struct i915_ppgtt *ppgtt)
 {
+	struct i915_page_directory * const pml4 = ppgtt->pd;
 	int i;
 
 	for (i = 0; i < GEN8_PML4ES_PER_PML4; i++) {
-		if (ppgtt->pml4.pdps[i] == ppgtt->vm.scratch_pdp)
+		struct i915_page_directory *pdp = i915_pdp_entry(pml4, i);
+
+		if (pdp == ppgtt->vm.scratch_pdp)
 			continue;
 
-		gen8_ppgtt_cleanup_3lvl(&ppgtt->vm, ppgtt->pml4.pdps[i]);
+		gen8_ppgtt_cleanup_3lvl(&ppgtt->vm, pdp);
 	}
 
-	cleanup_px(&ppgtt->vm, &ppgtt->pml4);
+	free_pd(&ppgtt->vm, pml4);
 }
 
 static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
@@ -1377,7 +1369,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
 	if (i915_vm_is_4lvl(vm))
 		gen8_ppgtt_cleanup_4lvl(ppgtt);
 	else
-		gen8_ppgtt_cleanup_3lvl(&ppgtt->vm, &ppgtt->pdp);
+		gen8_ppgtt_cleanup_3lvl(&ppgtt->vm, ppgtt->pd);
 
 	gen8_free_scratch(vm);
 }
@@ -1406,10 +1398,10 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 			if (count < GEN8_PTES || intel_vgpu_active(vm->i915))
 				gen8_initialize_pt(vm, pt);
 
-			old = cmpxchg(&pd->page_table[pde], vm->scratch_pt, pt);
+			old = cmpxchg(&pd->entry[pde], vm->scratch_pt, pt);
 			if (old == vm->scratch_pt) {
 				gen8_ppgtt_set_pde(vm, pd, pt, pde);
-				atomic_inc(&pd->used_pdes);
+				atomic_inc(&pd->used);
 			} else {
 				free_pt(vm, pt);
 				pt = old;
@@ -1418,7 +1410,7 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 			spin_lock(&pd->lock);
 		}
 
-		atomic_add(count, &pt->used_ptes);
+		atomic_add(count, &pt->used);
 	}
 	spin_unlock(&pd->lock);
 
@@ -1430,7 +1422,7 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 }
 
 static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
-				struct i915_page_directory_pointer *pdp,
+				struct i915_page_directory *pdp,
 				u64 start, u64 length)
 {
 	struct i915_page_directory *pd;
@@ -1451,11 +1443,10 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 
 			gen8_initialize_pd(vm, pd);
 
-			old = cmpxchg(&pdp->page_directory[pdpe],
-				      vm->scratch_pd, pd);
+			old = cmpxchg(&pdp->entry[pdpe], vm->scratch_pd, pd);
 			if (old == vm->scratch_pd) {
 				gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
-				atomic_inc(&pdp->used_pdpes);
+				atomic_inc(&pdp->used);
 			} else {
 				free_pd(vm, pd);
 				pd = old;
@@ -1463,7 +1454,7 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 
 			spin_lock(&pdp->lock);
 		}
-		atomic_inc(&pd->used_pdes);
+		atomic_inc(&pd->used);
 		spin_unlock(&pdp->lock);
 
 		ret = gen8_ppgtt_alloc_pd(vm, pd, start, length);
@@ -1471,7 +1462,7 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 			goto unwind_pd;
 
 		spin_lock(&pdp->lock);
-		atomic_dec(&pd->used_pdes);
+		atomic_dec(&pd->used);
 	}
 	spin_unlock(&pdp->lock);
 
@@ -1479,10 +1470,10 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 
 unwind_pd:
 	spin_lock(&pdp->lock);
-	if (atomic_dec_and_test(&pd->used_pdes)) {
+	if (atomic_dec_and_test(&pd->used)) {
 		gen8_ppgtt_set_pdpe(vm, pdp, vm->scratch_pd, pdpe);
-		GEM_BUG_ON(!atomic_read(&pdp->used_pdpes));
-		atomic_dec(&pdp->used_pdpes);
+		GEM_BUG_ON(!atomic_read(&pdp->used));
+		atomic_dec(&pdp->used);
 		free_pd(vm, pd);
 	}
 	spin_unlock(&pdp->lock);
@@ -1495,23 +1486,24 @@ static int gen8_ppgtt_alloc_3lvl(struct i915_address_space *vm,
 				 u64 start, u64 length)
 {
 	return gen8_ppgtt_alloc_pdp(vm,
-				    &i915_vm_to_ppgtt(vm)->pdp, start, length);
+				    i915_vm_to_ppgtt(vm)->pd, start, length);
 }
 
 static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 				 u64 start, u64 length)
 {
 	struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
-	struct i915_pml4 *pml4 = &ppgtt->pml4;
-	struct i915_page_directory_pointer *pdp;
+	struct i915_page_directory * const pml4 = ppgtt->pd;
+	struct i915_page_directory *pdp;
 	u64 from = start;
 	u32 pml4e;
 	int ret;
 
 	spin_lock(&pml4->lock);
 	gen8_for_each_pml4e(pdp, pml4, start, length, pml4e) {
+
 		if (pdp == vm->scratch_pdp) {
-			struct i915_page_directory_pointer *old;
+			struct i915_page_directory *old;
 
 			spin_unlock(&pml4->lock);
 
@@ -1519,9 +1511,9 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 			if (IS_ERR(pdp))
 				goto unwind;
 
-			gen8_initialize_pdp(vm, pdp);
+			gen8_initialize_4lvl_pdp(vm, pdp);
 
-			old = cmpxchg(&pml4->pdps[pml4e], vm->scratch_pdp, pdp);
+			old = cmpxchg(&pml4->entry[pml4e], vm->scratch_pdp, pdp);
 			if (old == vm->scratch_pdp) {
 				gen8_ppgtt_set_pml4e(pml4, pdp, pml4e);
 			} else {
@@ -1531,7 +1523,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 
 			spin_lock(&pml4->lock);
 		}
-		atomic_inc(&pdp->used_pdpes);
+		atomic_inc(&pdp->used);
 		spin_unlock(&pml4->lock);
 
 		ret = gen8_ppgtt_alloc_pdp(vm, pdp, start, length);
@@ -1539,7 +1531,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 			goto unwind_pdp;
 
 		spin_lock(&pml4->lock);
-		atomic_dec(&pdp->used_pdpes);
+		atomic_dec(&pdp->used);
 	}
 	spin_unlock(&pml4->lock);
 
@@ -1547,7 +1539,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_pdpes)) {
+	if (atomic_dec_and_test(&pdp->used)) {
 		gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
 		free_pdp(vm, pdp);
 	}
@@ -1560,7 +1552,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 {
 	struct i915_address_space *vm = &ppgtt->vm;
-	struct i915_page_directory_pointer *pdp = &ppgtt->pdp;
+	struct i915_page_directory *pdp = ppgtt->pd;
 	struct i915_page_directory *pd;
 	u64 start = 0, length = ppgtt->vm.total;
 	u64 from = start;
@@ -1573,10 +1565,12 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 
 		gen8_initialize_pd(vm, pd);
 		gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
-		atomic_inc(&pdp->used_pdpes);
+
+		atomic_inc(&pdp->used);
 	}
 
-	atomic_inc(&pdp->used_pdpes); /* never remove */
+	atomic_inc(&pdp->used); /* never remove */
+
 	return 0;
 
 unwind:
@@ -1585,7 +1579,7 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 		gen8_ppgtt_set_pdpe(vm, pdp, vm->scratch_pd, pdpe);
 		free_pd(vm, pd);
 	}
-	atomic_set(&pdp->used_pdpes, 0);
+	atomic_set(&pdp->used, 0);
 	return -ENOMEM;
 }
 
@@ -1640,27 +1634,25 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 	if (err)
 		goto err_free;
 
-	if (i915_vm_is_4lvl(&ppgtt->vm)) {
-		err = setup_px(&ppgtt->vm, &ppgtt->pml4);
-		if (err)
-			goto err_scratch;
+	ppgtt->pd = alloc_pdp(&ppgtt->vm);
+	if (IS_ERR(ppgtt->pd)) {
+		err = PTR_ERR(ppgtt->pd);
+		goto err_scratch;
+	}
 
-		gen8_initialize_pml4(&ppgtt->vm, &ppgtt->pml4);
+	if (i915_vm_is_4lvl(&ppgtt->vm)) {
+		gen8_initialize_pml4(&ppgtt->vm, ppgtt->pd);
 
 		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;
 	} else {
-		err = __pdp_init(&ppgtt->vm, &ppgtt->pdp);
-		if (err)
-			goto err_scratch;
+		gen8_initialize_3lvl_pdp(&ppgtt->vm, ppgtt->pd);
 
 		if (intel_vgpu_active(i915)) {
 			err = gen8_preallocate_top_level_pdp(ppgtt);
-			if (err) {
-				__pdp_fini(&ppgtt->pdp);
-				goto err_scratch;
-			}
+			if (err)
+				goto err_pdp;
 		}
 
 		ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc_3lvl;
@@ -1675,6 +1667,8 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 
 	return ppgtt;
 
+err_pdp:
+	free_pdp(&ppgtt->vm, ppgtt->pd);
 err_scratch:
 	gen8_free_scratch(&ppgtt->vm);
 err_free:
@@ -1740,15 +1734,16 @@ static void gen6_ppgtt_enable(struct drm_i915_private *dev_priv)
 static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 				   u64 start, u64 length)
 {
-	struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
-	unsigned int first_entry = start / I915_GTT_PAGE_SIZE;
+	struct gen6_ppgtt * const ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
+	const unsigned int first_entry = start / I915_GTT_PAGE_SIZE;
+	const gen6_pte_t scratch_pte = vm->scratch_pte;
 	unsigned int pde = first_entry / GEN6_PTES;
 	unsigned int pte = first_entry % GEN6_PTES;
 	unsigned int num_entries = length / I915_GTT_PAGE_SIZE;
-	const gen6_pte_t scratch_pte = vm->scratch_pte;
 
 	while (num_entries) {
-		struct i915_page_table *pt = ppgtt->base.pd.page_table[pde++];
+		struct i915_page_table * const pt =
+			i915_pt_entry(ppgtt->base.pd, pde++);
 		const unsigned int count = min(num_entries, GEN6_PTES - pte);
 		gen6_pte_t *vaddr;
 
@@ -1756,8 +1751,8 @@ static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
 
 		num_entries -= count;
 
-		GEM_BUG_ON(count > atomic_read(&pt->used_ptes));
-		if (!atomic_sub_return(count, &pt->used_ptes))
+		GEM_BUG_ON(count > atomic_read(&pt->used));
+		if (!atomic_sub_return(count, &pt->used))
 			ppgtt->scan_for_unused_pt = true;
 
 		/*
@@ -1781,6 +1776,7 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 				      u32 flags)
 {
 	struct i915_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
+	struct i915_page_directory * const pd = ppgtt->pd;
 	unsigned first_entry = vma->node.start / I915_GTT_PAGE_SIZE;
 	unsigned act_pt = first_entry / GEN6_PTES;
 	unsigned act_pte = first_entry % GEN6_PTES;
@@ -1788,9 +1784,9 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 	struct sgt_dma iter = sgt_dma(vma);
 	gen6_pte_t *vaddr;
 
-	GEM_BUG_ON(ppgtt->pd.page_table[act_pt] == vm->scratch_pt);
+	GEM_BUG_ON(i915_pt_entry(pd, act_pt) == vm->scratch_pt);
 
-	vaddr = kmap_atomic_px(ppgtt->pd.page_table[act_pt]);
+	vaddr = kmap_atomic_px(i915_pt_entry(pd, act_pt));
 	do {
 		vaddr[act_pte] = pte_encode | GEN6_PTE_ADDR_ENCODE(iter.dma);
 
@@ -1806,7 +1802,7 @@ static void gen6_ppgtt_insert_entries(struct i915_address_space *vm,
 
 		if (++act_pte == GEN6_PTES) {
 			kunmap_atomic(vaddr);
-			vaddr = kmap_atomic_px(ppgtt->pd.page_table[++act_pt]);
+			vaddr = kmap_atomic_px(i915_pt_entry(pd, ++act_pt));
 			act_pte = 0;
 		}
 	} while (1);
@@ -1819,6 +1815,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 			       u64 start, u64 length)
 {
 	struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
+	struct i915_page_directory * const pd = ppgtt->base.pd;
 	struct i915_page_table *pt;
 	intel_wakeref_t wakeref;
 	u64 from = start;
@@ -1827,14 +1824,14 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 
 	wakeref = intel_runtime_pm_get(&vm->i915->runtime_pm);
 
-	spin_lock(&ppgtt->base.pd.lock);
-	gen6_for_each_pde(pt, &ppgtt->base.pd, start, length, pde) {
+	spin_lock(&pd->lock);
+	gen6_for_each_pde(pt, pd, start, length, pde) {
 		const unsigned int count = gen6_pte_count(start, length);
 
 		if (pt == vm->scratch_pt) {
 			struct i915_page_table *old;
 
-			spin_unlock(&ppgtt->base.pd.lock);
+			spin_unlock(&pd->lock);
 
 			pt = alloc_pt(vm);
 			if (IS_ERR(pt))
@@ -1842,10 +1839,8 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 
 			gen6_initialize_pt(vm, pt);
 
-			old = cmpxchg(&ppgtt->base.pd.page_table[pde],
-				      vm->scratch_pt, pt);
+			old = cmpxchg(&pd->entry[pde], vm->scratch_pt, pt);
 			if (old == vm->scratch_pt) {
-				ppgtt->base.pd.page_table[pde] = pt;
 				if (i915_vma_is_bound(ppgtt->vma,
 						      I915_VMA_GLOBAL_BIND)) {
 					gen6_write_pde(ppgtt, pde, pt);
@@ -1856,12 +1851,12 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 				pt = old;
 			}
 
-			spin_lock(&ppgtt->base.pd.lock);
+			spin_lock(&pd->lock);
 		}
 
-		atomic_add(count, &pt->used_ptes);
+		atomic_add(count, &pt->used);
 	}
-	spin_unlock(&ppgtt->base.pd.lock);
+	spin_unlock(&pd->lock);
 
 	if (flush) {
 		mark_tlbs_dirty(&ppgtt->base);
@@ -1881,6 +1876,7 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
 static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
 {
 	struct i915_address_space * const vm = &ppgtt->base.vm;
+	struct i915_page_directory * const pd = ppgtt->base.pd;
 	struct i915_page_table *unused;
 	u32 pde;
 	int ret;
@@ -1900,9 +1896,9 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
 	}
 
 	gen6_initialize_pt(vm, vm->scratch_pt);
-	gen6_for_all_pdes(unused, &ppgtt->base.pd, pde)
-		ppgtt->base.pd.page_table[pde] = vm->scratch_pt;
-	spin_lock_init(&ppgtt->base.pd.lock);
+
+	gen6_for_all_pdes(unused, pd, pde)
+		pd->entry[pde] = vm->scratch_pt;
 
 	return 0;
 }
@@ -1915,10 +1911,11 @@ static void gen6_ppgtt_free_scratch(struct i915_address_space *vm)
 
 static void gen6_ppgtt_free_pd(struct gen6_ppgtt *ppgtt)
 {
+	struct i915_page_directory * const pd = ppgtt->base.pd;
 	struct i915_page_table *pt;
 	u32 pde;
 
-	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde)
+	gen6_for_all_pdes(pt, pd, pde)
 		if (pt != ppgtt->base.vm.scratch_pt)
 			free_pt(&ppgtt->base.vm, pt);
 }
@@ -1982,6 +1979,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 
 	gen6_ppgtt_free_pd(ppgtt);
 	gen6_ppgtt_free_scratch(vm);
+	kfree(ppgtt->base.pd);
 }
 
 static int pd_vma_set_pages(struct i915_vma *vma)
@@ -2007,10 +2005,10 @@ static int pd_vma_bind(struct i915_vma *vma,
 	struct i915_page_table *pt;
 	unsigned int pde;
 
-	ppgtt->base.pd.base.ggtt_offset = ggtt_offset * sizeof(gen6_pte_t);
+	ppgtt->base.pd->base.ggtt_offset = ggtt_offset * sizeof(gen6_pte_t);
 	ppgtt->pd_addr = (gen6_pte_t __iomem *)ggtt->gsm + ggtt_offset;
 
-	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde)
+	gen6_for_all_pdes(pt, ppgtt->base.pd, pde)
 		gen6_write_pde(ppgtt, pde, pt);
 
 	mark_tlbs_dirty(&ppgtt->base);
@@ -2022,6 +2020,7 @@ static int pd_vma_bind(struct i915_vma *vma,
 static void pd_vma_unbind(struct i915_vma *vma)
 {
 	struct gen6_ppgtt *ppgtt = vma->private;
+	struct i915_page_directory * const pd = ppgtt->base.pd;
 	struct i915_page_table * const scratch_pt = ppgtt->base.vm.scratch_pt;
 	struct i915_page_table *pt;
 	unsigned int pde;
@@ -2030,12 +2029,12 @@ static void pd_vma_unbind(struct i915_vma *vma)
 		return;
 
 	/* Free all no longer used page tables */
-	gen6_for_all_pdes(pt, &ppgtt->base.pd, pde) {
-		if (atomic_read(&pt->used_ptes) || pt == scratch_pt)
+	gen6_for_all_pdes(pt, ppgtt->base.pd, pde) {
+		if (atomic_read(&pt->used) || pt == scratch_pt)
 			continue;
 
 		free_pt(&ppgtt->base.vm, pt);
-		ppgtt->base.pd.page_table[pde] = scratch_pt;
+		pd->entry[pde] = scratch_pt;
 	}
 
 	ppgtt->scan_for_unused_pt = false;
@@ -2164,9 +2163,15 @@ static struct i915_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 		goto err_free;
 	}
 
+	ppgtt->base.pd = __alloc_pd();
+	if (!ppgtt->base.pd) {
+		err = -ENOMEM;
+		goto err_work;
+	}
+
 	err = gen6_ppgtt_init_scratch(ppgtt);
 	if (err)
-		goto err_work;
+		goto err_pd;
 
 	ppgtt->vma = pd_vma_create(ppgtt, GEN6_PD_SIZE);
 	if (IS_ERR(ppgtt->vma)) {
@@ -2178,6 +2183,8 @@ static struct i915_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915)
 
 err_scratch:
 	gen6_ppgtt_free_scratch(&ppgtt->base.vm);
+err_pd:
+	kfree(ppgtt->base.pd);
 err_work:
 	kfree(ppgtt->work);
 err_free:
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];
 };
 
 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;
 };
 
 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];
+}
+
 /* Equivalent to the gen6 version, For each pde iterates over every pde
  * between from start until start + length. On gen8+ it simply iterates
  * over every page directory entry in a page directory.
@@ -540,7 +543,7 @@ i915_pdpes_per_pdp(const struct i915_address_space *vm)
 #define gen8_for_each_pde(pt, pd, start, length, iter)			\
 	for (iter = gen8_pde_index(start);				\
 	     length > 0 && iter < I915_PDES &&				\
-		(pt = (pd)->page_table[iter], true);			\
+		     (pt = i915_pt_entry(pd, iter), true);		\
 	     ({ u64 temp = ALIGN(start+1, 1 << GEN8_PDE_SHIFT);		\
 		    temp = min(temp - start, length);			\
 		    start += temp, length -= temp; }), ++iter)
@@ -548,7 +551,7 @@ i915_pdpes_per_pdp(const struct i915_address_space *vm)
 #define gen8_for_each_pdpe(pd, pdp, start, length, iter)		\
 	for (iter = gen8_pdpe_index(start);				\
 	     length > 0 && iter < i915_pdpes_per_pdp(vm) &&		\
-		(pd = (pdp)->page_directory[iter], true);		\
+		     (pd = i915_pd_entry(pdp, iter), true);		\
 	     ({ u64 temp = ALIGN(start+1, 1 << GEN8_PDPE_SHIFT);	\
 		    temp = min(temp - start, length);			\
 		    start += temp, length -= temp; }), ++iter)
@@ -556,7 +559,7 @@ i915_pdpes_per_pdp(const struct i915_address_space *vm)
 #define gen8_for_each_pml4e(pdp, pml4, start, length, iter)		\
 	for (iter = gen8_pml4e_index(start);				\
 	     length > 0 && iter < GEN8_PML4ES_PER_PML4 &&		\
-		(pdp = (pml4)->pdps[iter], true);			\
+		     (pdp = i915_pdp_entry(pml4, iter), true);		\
 	     ({ u64 temp = ALIGN(start+1, 1ULL << GEN8_PML4E_SHIFT);	\
 		    temp = min(temp - start, length);			\
 		    start += temp, length -= temp; }), ++iter)
@@ -589,7 +592,10 @@ static inline u64 gen8_pte_count(u64 address, u64 length)
 static inline dma_addr_t
 i915_page_dir_dma_addr(const struct i915_ppgtt *ppgtt, const unsigned int n)
 {
-	return px_dma(ppgtt->pdp.page_directory[n]);
+	struct i915_page_directory *pd;
+
+	pd = i915_pdp_entry(ppgtt->pd, n);
+	return px_dma(pd);
 }
 
 static inline struct i915_ggtt *
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/10] drm/i915/gtt: Introduce init_pd_with_page
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
  2019-06-14 16:43 ` [PATCH 02/10] drm/i915/gtt: Use a common type for page directories Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 17:10   ` Chris Wilson
  2019-06-14 16:43 ` [PATCH 04/10] drm/i915/gtt: Introduce init_pd Mika Kuoppala
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

We set the page directory entries to point into a page table.
There is no gen specifics in here so make it simple and
obvious.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ef3fa6b5ef10..a2ab0b7bb234 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -726,12 +726,12 @@ static void free_pd(struct i915_address_space *vm,
 	kfree(pd);
 }
 
-static void gen8_initialize_pd(struct i915_address_space *vm,
-			       struct i915_page_directory *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(vm->scratch_pt), I915_CACHE_LLC));
-	memset_p(pd->entry, vm->scratch_pt, I915_PDES);
+	fill_px(vm, pd, gen8_pde_encode(px_dma(pt), I915_CACHE_LLC));
+	memset_p(pd->entry, pt, 512);
 }
 
 static struct i915_page_directory *alloc_pdp(struct i915_address_space *vm)
@@ -1264,7 +1264,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 	}
 
 	gen8_initialize_pt(vm, vm->scratch_pt);
-	gen8_initialize_pd(vm, vm->scratch_pd);
+	init_pd_with_page(vm, vm->scratch_pd, vm->scratch_pt);
 	if (i915_vm_is_4lvl(vm))
 		gen8_initialize_4lvl_pdp(vm, vm->scratch_pdp);
 
@@ -1441,7 +1441,7 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 			if (IS_ERR(pd))
 				goto unwind;
 
-			gen8_initialize_pd(vm, pd);
+			init_pd_with_page(vm, pd, vm->scratch_pt);
 
 			old = cmpxchg(&pdp->entry[pdpe], vm->scratch_pd, pd);
 			if (old == vm->scratch_pd) {
@@ -1563,7 +1563,7 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 		if (IS_ERR(pd))
 			goto unwind;
 
-		gen8_initialize_pd(vm, pd);
+		init_pd_with_page(vm, pd, vm->scratch_pt);
 		gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
 
 		atomic_inc(&pdp->used);
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/10] drm/i915/gtt: Introduce init_pd
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
  2019-06-14 16:43 ` [PATCH 02/10] drm/i915/gtt: Use a common type for page directories Mika Kuoppala
  2019-06-14 16:43 ` [PATCH 03/10] drm/i915/gtt: Introduce init_pd_with_page Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 17:13   ` Chris Wilson
  2019-06-14 16:43 ` [PATCH 05/10] drm/i915/gtt: Generalize alloc_pd Mika Kuoppala
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

All page directories, excluding last level, are initialized with
pointer to next level page directories. Make common function for it.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a2ab0b7bb234..25805971f771 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -761,26 +761,12 @@ static void free_pdp(struct i915_address_space *vm,
 	kfree(pdp);
 }
 
-static void gen8_initialize_4lvl_pdp(struct i915_address_space *vm,
-				     struct i915_page_directory *pdp)
+static void init_pd(struct i915_address_space *vm,
+		    struct i915_page_directory * const pd,
+		    struct i915_page_directory * const to)
 {
-	fill_px(vm, pdp,
-		gen8_pdpe_encode(px_dma(vm->scratch_pd), I915_CACHE_LLC));
-	memset_p(pdp->entry, vm->scratch_pd, 512);
-}
-
-static void gen8_initialize_3lvl_pdp(struct i915_address_space *vm,
-				     struct i915_page_directory *pdp)
-{
-	memset_p(pdp->entry, vm->scratch_pd, GEN8_3LVL_PDPES);
-}
-
-static void gen8_initialize_pml4(struct i915_address_space *vm,
-				 struct i915_page_directory *pml4)
-{
-	fill_px(vm, pml4,
-		gen8_pml4e_encode(px_dma(vm->scratch_pdp), I915_CACHE_LLC));
-	memset_p(pml4->entry, vm->scratch_pdp, GEN8_PML4ES_PER_PML4);
+	fill_px(vm, pd, gen8_pdpe_encode(px_dma(to), I915_CACHE_LLC));
+	memset_p(pd->entry, to, 512);
 }
 
 /*
@@ -1266,7 +1252,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);
 	if (i915_vm_is_4lvl(vm))
-		gen8_initialize_4lvl_pdp(vm, vm->scratch_pdp);
+		init_pd(vm, vm->scratch_pdp, vm->scratch_pd);
 
 	return 0;
 
@@ -1511,7 +1497,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 			if (IS_ERR(pdp))
 				goto unwind;
 
-			gen8_initialize_4lvl_pdp(vm, pdp);
+			init_pd(vm, pdp, vm->scratch_pd);
 
 			old = cmpxchg(&pml4->entry[pml4e], vm->scratch_pdp, pdp);
 			if (old == vm->scratch_pdp) {
@@ -1641,13 +1627,18 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 	}
 
 	if (i915_vm_is_4lvl(&ppgtt->vm)) {
-		gen8_initialize_pml4(&ppgtt->vm, ppgtt->pd);
+		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;
 	} else {
-		gen8_initialize_3lvl_pdp(&ppgtt->vm, ppgtt->pd);
+		/*
+		 * 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);
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/10] drm/i915/gtt: Generalize alloc_pd
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (2 preceding siblings ...)
  2019-06-14 16:43 ` [PATCH 04/10] drm/i915/gtt: Introduce init_pd Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 17:17   ` Chris Wilson
  2019-06-14 16:43 ` [PATCH 06/10] drm/i915/gtt: pde entry encoding is identical Mika Kuoppala
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

Allocate all page directory variants with alloc_pd. As
the lvl3 and lvl4 variants differ in manipulation, we
need to check for existence of backing phys page before accessing
it.

v2: use err in returns

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 25805971f771..de264b3a0105 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -719,10 +719,17 @@ 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)
 {
-	cleanup_px(vm, pd);
+	if (likely(pd_has_phys_page(pd)))
+		cleanup_px(vm, pd);
+
 	kfree(pd);
 }
 
@@ -734,37 +741,12 @@ static void init_pd_with_page(struct i915_address_space *vm,
 	memset_p(pd->entry, pt, 512);
 }
 
-static struct i915_page_directory *alloc_pdp(struct i915_address_space *vm)
-{
-	struct i915_page_directory *pdp;
-
-	pdp = __alloc_pd();
-	if (!pdp)
-		return ERR_PTR(-ENOMEM);
-
-	if (i915_vm_is_4lvl(vm)) {
-		if (unlikely(setup_px(vm, pdp))) {
-			kfree(pdp);
-			return ERR_PTR(-ENOMEM);
-		}
-	}
-
-	return pdp;
-}
-
-static void free_pdp(struct i915_address_space *vm,
-		     struct i915_page_directory *pdp)
-{
-	if (i915_vm_is_4lvl(vm))
-		cleanup_px(vm, pdp);
-
-	kfree(pdp);
-}
-
 static void init_pd(struct i915_address_space *vm,
 		    struct i915_page_directory * const pd,
 		    struct i915_page_directory * const to)
 {
+	GEM_DEBUG_BUG_ON(!pd_has_phys_page(pd));
+
 	fill_px(vm, pd, gen8_pdpe_encode(px_dma(to), I915_CACHE_LLC));
 	memset_p(pd->entry, to, 512);
 }
@@ -842,14 +824,13 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 	return !atomic_read(&pd->used);
 }
 
-static void gen8_ppgtt_set_pdpe(struct i915_address_space *vm,
-				struct i915_page_directory *pdp,
+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 (!i915_vm_is_4lvl(vm))
+	if (!pd_has_phys_page(pdp))
 		return;
 
 	vaddr = kmap_atomic_px(pdp);
@@ -877,7 +858,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(vm, pdp, vm->scratch_pd, pdpe);
+			gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
 			pdp->entry[pdpe] = vm->scratch_pd;
 
 			GEM_BUG_ON(!atomic_read(&pdp->used));
@@ -938,7 +919,7 @@ static void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
 		}
 		spin_unlock(&pml4->lock);
 		if (free)
-			free_pdp(vm, pdp);
+			free_pd(vm, pdp);
 	}
 }
 
@@ -1242,7 +1223,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 	}
 
 	if (i915_vm_is_4lvl(vm)) {
-		vm->scratch_pdp = alloc_pdp(vm);
+		vm->scratch_pdp = alloc_pd(vm);
 		if (IS_ERR(vm->scratch_pdp)) {
 			ret = PTR_ERR(vm->scratch_pdp);
 			goto free_pd;
@@ -1304,7 +1285,7 @@ static void gen8_free_scratch(struct i915_address_space *vm)
 		return;
 
 	if (i915_vm_is_4lvl(vm))
-		free_pdp(vm, vm->scratch_pdp);
+		free_pd(vm, vm->scratch_pdp);
 	free_pd(vm, vm->scratch_pd);
 	free_pt(vm, vm->scratch_pt);
 	cleanup_scratch_page(vm);
@@ -1324,7 +1305,7 @@ static void gen8_ppgtt_cleanup_3lvl(struct i915_address_space *vm,
 		free_pd(vm, pdp->entry[i]);
 	}
 
-	free_pdp(vm, pdp);
+	free_pd(vm, pdp);
 }
 
 static void gen8_ppgtt_cleanup_4lvl(struct i915_ppgtt *ppgtt)
@@ -1431,7 +1412,7 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 
 			old = cmpxchg(&pdp->entry[pdpe], vm->scratch_pd, pd);
 			if (old == vm->scratch_pd) {
-				gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
+				gen8_ppgtt_set_pdpe(pdp, pd, pdpe);
 				atomic_inc(&pdp->used);
 			} else {
 				free_pd(vm, pd);
@@ -1457,7 +1438,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(vm, pdp, vm->scratch_pd, pdpe);
+		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
 		GEM_BUG_ON(!atomic_read(&pdp->used));
 		atomic_dec(&pdp->used);
 		free_pd(vm, pd);
@@ -1487,13 +1468,12 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 
 	spin_lock(&pml4->lock);
 	gen8_for_each_pml4e(pdp, pml4, start, length, pml4e) {
-
 		if (pdp == vm->scratch_pdp) {
 			struct i915_page_directory *old;
 
 			spin_unlock(&pml4->lock);
 
-			pdp = alloc_pdp(vm);
+			pdp = alloc_pd(vm);
 			if (IS_ERR(pdp))
 				goto unwind;
 
@@ -1503,7 +1483,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 			if (old == vm->scratch_pdp) {
 				gen8_ppgtt_set_pml4e(pml4, pdp, pml4e);
 			} else {
-				free_pdp(vm, pdp);
+				free_pd(vm, pdp);
 				pdp = old;
 			}
 
@@ -1527,7 +1507,7 @@ 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);
-		free_pdp(vm, pdp);
+		free_pd(vm, pdp);
 	}
 	spin_unlock(&pml4->lock);
 unwind:
@@ -1550,7 +1530,7 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 			goto unwind;
 
 		init_pd_with_page(vm, pd, vm->scratch_pt);
-		gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
+		gen8_ppgtt_set_pdpe(pdp, pd, pdpe);
 
 		atomic_inc(&pdp->used);
 	}
@@ -1562,7 +1542,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(vm, pdp, vm->scratch_pd, pdpe);
+		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
 		free_pd(vm, pd);
 	}
 	atomic_set(&pdp->used, 0);
@@ -1620,13 +1600,17 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 	if (err)
 		goto err_free;
 
-	ppgtt->pd = alloc_pdp(&ppgtt->vm);
-	if (IS_ERR(ppgtt->pd)) {
-		err = PTR_ERR(ppgtt->pd);
-		goto err_scratch;
+	ppgtt->pd = __alloc_pd();
+	if (!ppgtt->pd) {
+		err = -ENOMEM;
+		goto err_free_scratch;
 	}
 
 	if (i915_vm_is_4lvl(&ppgtt->vm)) {
+		err = setup_px(&ppgtt->vm, ppgtt->pd);
+		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;
@@ -1643,7 +1627,7 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 		if (intel_vgpu_active(i915)) {
 			err = gen8_preallocate_top_level_pdp(ppgtt);
 			if (err)
-				goto err_pdp;
+				goto err_free_pdp;
 		}
 
 		ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc_3lvl;
@@ -1658,9 +1642,9 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
 
 	return ppgtt;
 
-err_pdp:
-	free_pdp(&ppgtt->vm, ppgtt->pd);
-err_scratch:
+err_free_pdp:
+	free_pd(&ppgtt->vm, ppgtt->pd);
+err_free_scratch:
 	gen8_free_scratch(&ppgtt->vm);
 err_free:
 	kfree(ppgtt);
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/10] drm/i915/gtt: pde entry encoding is identical
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (3 preceding siblings ...)
  2019-06-14 16:43 ` [PATCH 05/10] drm/i915/gtt: Generalize alloc_pd Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 17:21   ` Chris Wilson
  2019-06-14 16:43 ` [PATCH 07/10] drm/i915/gtt: Check for physical page for pd entry always Mika Kuoppala
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

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@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));\
+})
+
 /*
  * PDE TLBs are a pain to invalidate on GEN8+. When we modify
  * the page table structures, we mark them dirty so that
@@ -780,18 +789,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)
@@ -809,8 +806,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);
@@ -824,20 +820,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
  */
@@ -858,8 +840,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_pdp_entry(pdp, pdpe, vm->scratch_pd);
 
 			GEM_BUG_ON(!atomic_read(&pdp->used));
 			atomic_dec(&pdp->used);
@@ -879,17 +860,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.
@@ -913,8 +883,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);
@@ -1231,7 +1200,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);
 
@@ -1367,7 +1336,7 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 
 			old = cmpxchg(&pd->entry[pde], vm->scratch_pt, pt);
 			if (old == vm->scratch_pt) {
-				gen8_ppgtt_set_pde(vm, pd, pt, pde);
+				set_pd_entry(pd, pde, pt);
 				atomic_inc(&pd->used);
 			} else {
 				free_pt(vm, pt);
@@ -1408,11 +1377,11 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 			if (IS_ERR(pd))
 				goto unwind;
 
-			init_pd_with_page(vm, pd, vm->scratch_pt);
+			init_pd(vm, pd, vm->scratch_pt);
 
 			old = cmpxchg(&pdp->entry[pdpe], vm->scratch_pd, pd);
 			if (old == vm->scratch_pd) {
-				gen8_ppgtt_set_pdpe(pdp, pd, pdpe);
+				set_pdp_entry(pdp, pdpe, pd);
 				atomic_inc(&pdp->used);
 			} else {
 				free_pd(vm, pd);
@@ -1438,7 +1407,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_pdp_entry(pdp, pdpe, vm->scratch_pd);
 		GEM_BUG_ON(!atomic_read(&pdp->used));
 		atomic_dec(&pdp->used);
 		free_pd(vm, pd);
@@ -1481,7 +1450,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 
 			old = cmpxchg(&pml4->entry[pml4e], vm->scratch_pdp, pdp);
 			if (old == vm->scratch_pdp) {
-				gen8_ppgtt_set_pml4e(pml4, pdp, pml4e);
+				set_pd_entry(pml4, pml4e, pdp);
 			} else {
 				free_pd(vm, pdp);
 				pdp = old;
@@ -1506,7 +1475,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);
@@ -1529,8 +1498,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_pdp_entry(pdp, pdpe, pd);
 
 		atomic_inc(&pdp->used);
 	}
@@ -1542,7 +1511,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_pdp_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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/10] drm/i915/gtt: Check for physical page for pd entry always
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (4 preceding siblings ...)
  2019-06-14 16:43 ` [PATCH 06/10] drm/i915/gtt: pde entry encoding is identical Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 17:22   ` Chris Wilson
  2019-06-14 16:43 ` [PATCH 08/10] drm/i915/gtt: Make swapping the pd entry generic Mika Kuoppala
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

Check the physical page before writing the entry into
the physical page. This further generalizes the pd so that
manipulation in callsites will be identical, removing the need to
handle pdps differently for gen8.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a61fa24b6294..8baceb9f64c5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -749,17 +749,11 @@ static void __set_pd_entry(struct i915_page_directory * const pd,
 
 #define set_pd_entry(pd, pde, to) ({					\
 	(pd)->entry[(pde)] = (to);					\
-	__set_pd_entry((pd), (pde),					\
+	if (likely(pd_has_phys_page(pd)))				\
+		__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));\
-})
-
 /*
  * PDE TLBs are a pain to invalidate on GEN8+. When we modify
  * the page table structures, we mark them dirty so that
@@ -840,7 +834,7 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
 
 		spin_lock(&pdp->lock);
 		if (!atomic_read(&pd->used)) {
-			set_pdp_entry(pdp, pdpe, vm->scratch_pd);
+			set_pd_entry(pdp, pdpe, vm->scratch_pd);
 
 			GEM_BUG_ON(!atomic_read(&pdp->used));
 			atomic_dec(&pdp->used);
@@ -1381,7 +1375,7 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 
 			old = cmpxchg(&pdp->entry[pdpe], vm->scratch_pd, pd);
 			if (old == vm->scratch_pd) {
-				set_pdp_entry(pdp, pdpe, pd);
+				set_pd_entry(pdp, pdpe, pd);
 				atomic_inc(&pdp->used);
 			} else {
 				free_pd(vm, pd);
@@ -1407,7 +1401,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)) {
-		set_pdp_entry(pdp, pdpe, vm->scratch_pd);
+		set_pd_entry(pdp, pdpe, vm->scratch_pd);
 		GEM_BUG_ON(!atomic_read(&pdp->used));
 		atomic_dec(&pdp->used);
 		free_pd(vm, pd);
@@ -1499,7 +1493,7 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 			goto unwind;
 
 		init_pd(vm, pd, vm->scratch_pt);
-		set_pdp_entry(pdp, pdpe, pd);
+		set_pd_entry(pdp, pdpe, pd);
 
 		atomic_inc(&pdp->used);
 	}
@@ -1511,7 +1505,7 @@ static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
 unwind:
 	start -= from;
 	gen8_for_each_pdpe(pd, pdp, from, start, pdpe) {
-		set_pdp_entry(pdp, pdpe, vm->scratch_pd);
+		set_pd_entry(pdp, pdpe, vm->scratch_pd);
 		free_pd(vm, pd);
 	}
 	atomic_set(&pdp->used, 0);
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 08/10] drm/i915/gtt: Make swapping the pd entry generic
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (5 preceding siblings ...)
  2019-06-14 16:43 ` [PATCH 07/10] drm/i915/gtt: Check for physical page for pd entry always Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 17:26   ` Chris Wilson
  2019-06-14 16:43 ` [PATCH 09/10] drm/i915/gtt: Tear down setup and cleanup macros for page dma Mika Kuoppala
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

Swapping a pd entry is same across the page directories, if
we succeed we need to increment the count and write the phys page
entry. Make a common function for it.

v2: inline (Chris)

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8baceb9f64c5..0fffa0608ea5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -754,6 +754,28 @@ static void __set_pd_entry(struct i915_page_directory * const pd,
 		       gen8_pde_encode(px_dma(to), I915_CACHE_LLC));	\
 })
 
+static inline void *
+__swap_pd_entry(struct i915_page_directory * const pd,
+		const unsigned short pde,
+		void * const old_val,
+		void * const new_val,
+		const dma_addr_t daddr,
+		u64 (*encode)(const dma_addr_t, const enum i915_cache_level))
+{
+	void * const old = cmpxchg(&pd->entry[pde], old_val, new_val);
+
+	if (likely(old == old_val)) {
+		atomic_inc(&pd->used);
+		if (likely(pd_has_phys_page(pd)))
+			__set_pd_entry(pd, pde, encode(daddr, I915_CACHE_LLC));
+	}
+
+	return old;
+}
+
+#define swap_pd_entry(pd, pde, old, to) \
+	__swap_pd_entry((pd), (pde), (old), (to), px_dma(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
@@ -1328,11 +1350,8 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 			if (count < GEN8_PTES || intel_vgpu_active(vm->i915))
 				gen8_initialize_pt(vm, pt);
 
-			old = cmpxchg(&pd->entry[pde], vm->scratch_pt, pt);
-			if (old == vm->scratch_pt) {
-				set_pd_entry(pd, pde, pt);
-				atomic_inc(&pd->used);
-			} else {
+			old = swap_pd_entry(pd, pde, vm->scratch_pt, pt);
+			if (unlikely(old != vm->scratch_pt)) {
 				free_pt(vm, pt);
 				pt = old;
 			}
@@ -1373,11 +1392,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 
 			init_pd(vm, pd, vm->scratch_pt);
 
-			old = cmpxchg(&pdp->entry[pdpe], vm->scratch_pd, pd);
-			if (old == vm->scratch_pd) {
-				set_pd_entry(pdp, pdpe, pd);
-				atomic_inc(&pdp->used);
-			} else {
+			old = swap_pd_entry(pdp, pdpe, vm->scratch_pd, pd);
+			if (unlikely(old != vm->scratch_pd)) {
 				free_pd(vm, pd);
 				pd = old;
 			}
@@ -1442,10 +1458,8 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 
 			init_pd(vm, pdp, vm->scratch_pd);
 
-			old = cmpxchg(&pml4->entry[pml4e], vm->scratch_pdp, pdp);
-			if (old == vm->scratch_pdp) {
-				set_pd_entry(pml4, pml4e, pdp);
-			} else {
+			old = swap_pd_entry(pml4, pml4e, vm->scratch_pdp, pdp);
+			if (unlikely(old != vm->scratch_pdp)) {
 				free_pd(vm, pdp);
 				pdp = old;
 			}
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 09/10] drm/i915/gtt: Tear down setup and cleanup macros for page dma
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (6 preceding siblings ...)
  2019-06-14 16:43 ` [PATCH 08/10] drm/i915/gtt: Make swapping the pd entry generic Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 17:30   ` Chris Wilson
  2019-06-14 16:43 ` [PATCH 10/10] drm/i915/gtt: Setup phys pages for 3lvl pdps Mika Kuoppala
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

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>
---
 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 0fffa0608ea5..ba2802c25d13 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -550,8 +550,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))
 
@@ -653,7 +651,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);
 	}
@@ -665,7 +663,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);
 }
 
@@ -708,7 +706,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);
 	}
@@ -725,7 +723,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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 10/10] drm/i915/gtt: Setup phys pages for 3lvl pdps
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (7 preceding siblings ...)
  2019-06-14 16:43 ` [PATCH 09/10] drm/i915/gtt: Tear down setup and cleanup macros for page dma Mika Kuoppala
@ 2019-06-14 16:43 ` Mika Kuoppala
  2019-06-14 17:36   ` Chris Wilson
  2019-06-14 17:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs Patchwork
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 23+ messages in thread
From: Mika Kuoppala @ 2019-06-14 16:43 UTC (permalink / raw)
  To: intel-gfx

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 | 112 +++++++++++++++++-----------
 1 file changed, 68 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ba2802c25d13..c76c92072d54 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -714,22 +714,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);				\
 }
@@ -747,8 +739,7 @@ static void __set_pd_entry(struct i915_page_directory * const pd,
 
 #define set_pd_entry(pd, pde, to) ({					\
 	(pd)->entry[(pde)] = (to);					\
-	if (likely(pd_has_phys_page(pd)))				\
-		__set_pd_entry((pd), (pde),				\
+	__set_pd_entry((pd), (pde),					\
 		       gen8_pde_encode(px_dma(to), I915_CACHE_LLC));	\
 })
 
@@ -764,8 +755,7 @@ __swap_pd_entry(struct i915_page_directory * const pd,
 
 	if (likely(old == old_val)) {
 		atomic_inc(&pd->used);
-		if (likely(pd_has_phys_page(pd)))
-			__set_pd_entry(pd, pde, encode(daddr, I915_CACHE_LLC));
+		__set_pd_entry(pd, pde, encode(daddr, I915_CACHE_LLC));
 	}
 
 	return old;
@@ -1539,6 +1529,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 +1582,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 +1592,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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 02/10] drm/i915/gtt: Use a common type for page directories
  2019-06-14 16:43 ` [PATCH 02/10] drm/i915/gtt: Use a common type for page directories Mika Kuoppala
@ 2019-06-14 16:56   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 16:56 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

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@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (8 preceding siblings ...)
  2019-06-14 16:43 ` [PATCH 10/10] drm/i915/gtt: Setup phys pages for 3lvl pdps Mika Kuoppala
@ 2019-06-14 17:00 ` Patchwork
  2019-06-14 17:04 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-06-14 17:00 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs
URL   : https://patchwork.freedesktop.org/series/62122/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a0c0d406ac14 drm/i915/gtt: No need to zero the table for page dirs
0a2593a39056 drm/i915/gtt: Use a common type for page directories
-:680: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#680: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:1504:
 	gen8_for_each_pml4e(pdp, pml4, start, length, pml4e) {
+

total: 0 errors, 0 warnings, 1 checks, 1051 lines checked
a580ad51d6ce drm/i915/gtt: Introduce init_pd_with_page
464bd2d5e1a9 drm/i915/gtt: Introduce init_pd
b8cb2f9e813d drm/i915/gtt: Generalize alloc_pd
ff23d85ebb8f drm/i915/gtt: pde entry encoding is identical
-:53: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pd' - possible side-effects?
#53: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:733:
+#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);				\
 }

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

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

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

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

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

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

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

total: 0 errors, 0 warnings, 8 checks, 232 lines checked
6bd860d004cd drm/i915/gtt: Check for physical page for pd entry always
cf0c87dc8f77 drm/i915/gtt: Make swapping the pd entry generic
-:41: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'to' - possible side-effects?
#41: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:776:
+#define swap_pd_entry(pd, pde, old, to) \
+	__swap_pd_entry((pd), (pde), (old), (to), px_dma(to), gen8_pde_encode)

total: 0 errors, 0 warnings, 1 checks, 66 lines checked
a5962f1ec818 drm/i915/gtt: Tear down setup and cleanup macros for page dma
f33418fc780b 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* ✗ Fi.CI.SPARSE: warning for series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (9 preceding siblings ...)
  2019-06-14 17:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs Patchwork
@ 2019-06-14 17:04 ` Patchwork
  2019-06-15  4:59 ` ✓ Fi.CI.BAT: success " Patchwork
  2019-06-17 10:32 ` ✓ Fi.CI.IGT: " Patchwork
  12 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-06-14 17:04 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs
URL   : https://patchwork.freedesktop.org/series/62122/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/gtt: No need to zero the table for page dirs
Okay!

Commit: drm/i915/gtt: Use a common type for page directories
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1512:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1512:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1503:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1503:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1752:44: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1752:44: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1831:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1831:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1747:44: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1747:44: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1828:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1828:9: warning: expression using sizeof(void)
-./include/linux/slab.h:666:13: error: undefined identifier '__builtin_mul_overflow'
-./include/linux/slab.h:666:13: warning: call with no type!

Commit: drm/i915/gtt: Introduce init_pd_with_page
Okay!

Commit: drm/i915/gtt: Introduce init_pd
Okay!

Commit: drm/i915/gtt: Generalize alloc_pd
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1489:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1489:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1470:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1470:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1564:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1564:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1544:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1544:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: pde entry encoding is identical
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1544:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1544:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1513:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1513:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Check for physical page for pd entry always
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1513:9: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/i915_gem_gtt.c:1513:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1507:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:1507:9: warning: expression using sizeof(void)

Commit: drm/i915/gtt: Make swapping the pd entry generic
Okay!

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

Commit: drm/i915/gtt: Setup phys pages for 3lvl pdps
-drivers/gpu/drm/i915/i915_gem_gtt.c:3493:25: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/i915_gem_gtt.c:3493:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:3493:25: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:3493:25: warning: expression using sizeof(void)

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/10] drm/i915/gtt: Introduce init_pd_with_page
  2019-06-14 16:43 ` [PATCH 03/10] drm/i915/gtt: Introduce init_pd_with_page Mika Kuoppala
@ 2019-06-14 17:10   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 17:10 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-06-14 17:43:43)
> We set the page directory entries to point into a page table.
> There is no gen specifics in here so make it simple and
> obvious.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index ef3fa6b5ef10..a2ab0b7bb234 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -726,12 +726,12 @@ static void free_pd(struct i915_address_space *vm,
>         kfree(pd);
>  }
>  
> -static void gen8_initialize_pd(struct i915_address_space *vm,
> -                              struct i915_page_directory *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(vm->scratch_pt), I915_CACHE_LLC));
> -       memset_p(pd->entry, vm->scratch_pt, I915_PDES);
> +       fill_px(vm, pd, gen8_pde_encode(px_dma(pt), I915_CACHE_LLC));
> +       memset_p(pd->entry, pt, 512);

Looks good,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 04/10] drm/i915/gtt: Introduce init_pd
  2019-06-14 16:43 ` [PATCH 04/10] drm/i915/gtt: Introduce init_pd Mika Kuoppala
@ 2019-06-14 17:13   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 17:13 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-06-14 17:43:44)
> All page directories, excluding last level, are initialized with
> pointer to next level page directories. Make common function for it.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 37 +++++++++++------------------
>  1 file changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a2ab0b7bb234..25805971f771 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -761,26 +761,12 @@ static void free_pdp(struct i915_address_space *vm,
>         kfree(pdp);
>  }
>  
> -static void gen8_initialize_4lvl_pdp(struct i915_address_space *vm,
> -                                    struct i915_page_directory *pdp)
> +static void init_pd(struct i915_address_space *vm,
> +                   struct i915_page_directory * const pd,
> +                   struct i915_page_directory * const to)
>  {
> -       fill_px(vm, pdp,
> -               gen8_pdpe_encode(px_dma(vm->scratch_pd), I915_CACHE_LLC));
> -       memset_p(pdp->entry, vm->scratch_pd, 512);
> -}
> -
> -static void gen8_initialize_3lvl_pdp(struct i915_address_space *vm,
> -                                    struct i915_page_directory *pdp)
> -{
> -       memset_p(pdp->entry, vm->scratch_pd, GEN8_3LVL_PDPES);
> -}
> -
> -static void gen8_initialize_pml4(struct i915_address_space *vm,
> -                                struct i915_page_directory *pml4)
> -{
> -       fill_px(vm, pml4,
> -               gen8_pml4e_encode(px_dma(vm->scratch_pdp), I915_CACHE_LLC));
> -       memset_p(pml4->entry, vm->scratch_pdp, GEN8_PML4ES_PER_PML4);
> +       fill_px(vm, pd, gen8_pdpe_encode(px_dma(to), I915_CACHE_LLC));
> +       memset_p(pd->entry, to, 512);
>  }
>  
>  /*
> @@ -1266,7 +1252,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);
>         if (i915_vm_is_4lvl(vm))
> -               gen8_initialize_4lvl_pdp(vm, vm->scratch_pdp);
> +               init_pd(vm, vm->scratch_pdp, vm->scratch_pd);
>  
>         return 0;
>  
> @@ -1511,7 +1497,7 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
>                         if (IS_ERR(pdp))
>                                 goto unwind;
>  
> -                       gen8_initialize_4lvl_pdp(vm, pdp);
> +                       init_pd(vm, pdp, vm->scratch_pd);
>  
>                         old = cmpxchg(&pml4->entry[pml4e], vm->scratch_pdp, pdp);
>                         if (old == vm->scratch_pdp) {
> @@ -1641,13 +1627,18 @@ static struct i915_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
>         }
>  
>         if (i915_vm_is_4lvl(&ppgtt->vm)) {
> -               gen8_initialize_pml4(&ppgtt->vm, ppgtt->pd);
> +               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;
>         } else {
> -               gen8_initialize_3lvl_pdp(&ppgtt->vm, ppgtt->pd);
> +               /*
> +                * 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);

Hmm, I was tempted to say just do it.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/10] drm/i915/gtt: Generalize alloc_pd
  2019-06-14 16:43 ` [PATCH 05/10] drm/i915/gtt: Generalize alloc_pd Mika Kuoppala
@ 2019-06-14 17:17   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 17:17 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-06-14 17:43:45)
> Allocate all page directory variants with alloc_pd. As
> the lvl3 and lvl4 variants differ in manipulation, we
> need to check for existence of backing phys page before accessing
> it.
> 
> v2: use err in returns
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 88 ++++++++++++-----------------
>  1 file changed, 36 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 25805971f771..de264b3a0105 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -719,10 +719,17 @@ 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;

Ok. I have no better magic.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 06/10] drm/i915/gtt: pde entry encoding is identical
  2019-06-14 16:43 ` [PATCH 06/10] drm/i915/gtt: pde entry encoding is identical Mika Kuoppala
@ 2019-06-14 17:21   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 17:21 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 07/10] drm/i915/gtt: Check for physical page for pd entry always
  2019-06-14 16:43 ` [PATCH 07/10] drm/i915/gtt: Check for physical page for pd entry always Mika Kuoppala
@ 2019-06-14 17:22   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 17:22 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-06-14 17:43:47)
> Check the physical page before writing the entry into
> the physical page. This further generalizes the pd so that
> manipulation in callsites will be identical, removing the need to
> handle pdps differently for gen8.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Squash or not, both are
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
then :-p
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 08/10] drm/i915/gtt: Make swapping the pd entry generic
  2019-06-14 16:43 ` [PATCH 08/10] drm/i915/gtt: Make swapping the pd entry generic Mika Kuoppala
@ 2019-06-14 17:26   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 17:26 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-06-14 17:43:48)
> Swapping a pd entry is same across the page directories, if
> we succeed we need to increment the count and write the phys page
> entry. Make a common function for it.
> 
> v2: inline (Chris)
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 42 +++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8baceb9f64c5..0fffa0608ea5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -754,6 +754,28 @@ static void __set_pd_entry(struct i915_page_directory * const pd,
>                        gen8_pde_encode(px_dma(to), I915_CACHE_LLC));    \
>  })
>  
> +static inline void *
> +__swap_pd_entry(struct i915_page_directory * const pd,
> +               const unsigned short pde,

These can be the struct px_dma base type (or whatever that was called)

> +               void * const old_val,
> +               void * const new_val,
> +               const dma_addr_t daddr,
> +               u64 (*encode)(const dma_addr_t, const enum i915_cache_level))
> +{
> +       void * const old = cmpxchg(&pd->entry[pde], old_val, new_val);
> +
> +       if (likely(old == old_val)) {
> +               atomic_inc(&pd->used);
> +               if (likely(pd_has_phys_page(pd)))
> +                       __set_pd_entry(pd, pde, encode(daddr, I915_CACHE_LLC));

and then s/daddr/px_dma(to))/
?

> +       }
> +
> +       return old;
> +}
> +
> +#define swap_pd_entry(pd, pde, old, to) \
> +       __swap_pd_entry((pd), (pde), (old), (to), px_dma(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
> @@ -1328,11 +1350,8 @@ static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
>                         if (count < GEN8_PTES || intel_vgpu_active(vm->i915))
>                                 gen8_initialize_pt(vm, pt);
>  
> -                       old = cmpxchg(&pd->entry[pde], vm->scratch_pt, pt);
> -                       if (old == vm->scratch_pt) {
> -                               set_pd_entry(pd, pde, pt);
> -                               atomic_inc(&pd->used);
> -                       } else {
> +                       old = swap_pd_entry(pd, pde, vm->scratch_pt, pt);
> +                       if (unlikely(old != vm->scratch_pt)) {
>                                 free_pt(vm, pt);
>                                 pt = old;
>                         }
> @@ -1373,11 +1392,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
>  
>                         init_pd(vm, pd, vm->scratch_pt);
>  
> -                       old = cmpxchg(&pdp->entry[pdpe], vm->scratch_pd, pd);
> -                       if (old == vm->scratch_pd) {
> -                               set_pd_entry(pdp, pdpe, pd);
> -                               atomic_inc(&pdp->used);
> -                       } else {
> +                       old = swap_pd_entry(pdp, pdpe, vm->scratch_pd, pd);
> +                       if (unlikely(old != vm->scratch_pd)) {
>                                 free_pd(vm, pd);
>                                 pd = old;
>                         }
> @@ -1442,10 +1458,8 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
>  
>                         init_pd(vm, pdp, vm->scratch_pd);
>  
> -                       old = cmpxchg(&pml4->entry[pml4e], vm->scratch_pdp, pdp);
> -                       if (old == vm->scratch_pdp) {
> -                               set_pd_entry(pml4, pml4e, pdp);
> -                       } else {
> +                       old = swap_pd_entry(pml4, pml4e, vm->scratch_pdp, pdp);
> +                       if (unlikely(old != vm->scratch_pdp)) {
>                                 free_pd(vm, pdp);
>                                 pdp = old;

Otherwise, the only difference between these stanzas is now
vm->scratch[lvl] :)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 09/10] drm/i915/gtt: Tear down setup and cleanup macros for page dma
  2019-06-14 16:43 ` [PATCH 09/10] drm/i915/gtt: Tear down setup and cleanup macros for page dma Mika Kuoppala
@ 2019-06-14 17:30   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 17:30 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-06-14 17:43:49)
> 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>

And I had just about got used to px!

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 10/10] drm/i915/gtt: Setup phys pages for 3lvl pdps
  2019-06-14 16:43 ` [PATCH 10/10] drm/i915/gtt: Setup phys pages for 3lvl pdps Mika Kuoppala
@ 2019-06-14 17:36   ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2019-06-14 17:36 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2019-06-14 17:43:50)
> 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 | 112 +++++++++++++++++-----------
>  1 file changed, 68 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index ba2802c25d13..c76c92072d54 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -714,22 +714,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);                               \
>  }
> @@ -747,8 +739,7 @@ static void __set_pd_entry(struct i915_page_directory * const pd,
>  
>  #define set_pd_entry(pd, pde, to) ({                                   \
>         (pd)->entry[(pde)] = (to);                                      \
> -       if (likely(pd_has_phys_page(pd)))                               \
> -               __set_pd_entry((pd), (pde),                             \
> +       __set_pd_entry((pd), (pde),                                     \
>                        gen8_pde_encode(px_dma(to), I915_CACHE_LLC));    \
>  })

The macro seems like it is redundant now as well, since __set_pd_entry
should be able to set pd->entry[pde] = to. Or at least one step too
large.

Ok, I'm willing to make the sacrifice for simplicity. I think the extra
allocation will be in the noise, you need a lot of contexts with small
workloads for it to be a significant factor i.e. microbenchmarks. And
even GL microbenchmarks tend to do some GL so aren't that sensitive to
context allocation rates. OglDrvCtx springs to mind. That would be a
reasonable sanity check that this is indeed in the noise.

The only question is does this not become simpler if we do this earlier?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (10 preceding siblings ...)
  2019-06-14 17:04 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-06-15  4:59 ` Patchwork
  2019-06-17 10:32 ` ✓ Fi.CI.IGT: " Patchwork
  12 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-06-15  4:59 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs
URL   : https://patchwork.freedesktop.org/series/62122/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6277 -> Patchwork_13289
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][1] -> [DMESG-WARN][2] ([fdo#102614])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@gem_ctx_exec@basic:
    - fi-icl-guc:         [INCOMPLETE][3] ([fdo#107713]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/fi-icl-guc/igt@gem_ctx_exec@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/fi-icl-guc/igt@gem_ctx_exec@basic.html
    - fi-icl-y:           [INCOMPLETE][5] ([fdo#107713]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/fi-icl-y/igt@gem_ctx_exec@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/fi-icl-y/igt@gem_ctx_exec@basic.html

  * igt@i915_selftest@live_gtt:
    - fi-glk-dsi:         [INCOMPLETE][7] ([fdo#103359] / [k.org#198133]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/fi-glk-dsi/igt@i915_selftest@live_gtt.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/fi-glk-dsi/igt@i915_selftest@live_gtt.html

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (50 -> 44)
------------------------------

  Additional (3): fi-kbl-7567u fi-skl-iommu fi-icl-u3 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-gdg-551 fi-pnv-d510 fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6277 -> Patchwork_13289

  CI_DRM_6277: a0195925ba8732ffae5fed3c0b966ca331bae66c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5057: 3b91c82b90d45c1a30569410c1142b541956740a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13289: f33418fc780b3aa9f3beaf73dc2586d42a045264 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f33418fc780b drm/i915/gtt: Setup phys pages for 3lvl pdps
a5962f1ec818 drm/i915/gtt: Tear down setup and cleanup macros for page dma
cf0c87dc8f77 drm/i915/gtt: Make swapping the pd entry generic
6bd860d004cd drm/i915/gtt: Check for physical page for pd entry always
ff23d85ebb8f drm/i915/gtt: pde entry encoding is identical
b8cb2f9e813d drm/i915/gtt: Generalize alloc_pd
464bd2d5e1a9 drm/i915/gtt: Introduce init_pd
a580ad51d6ce drm/i915/gtt: Introduce init_pd_with_page
0a2593a39056 drm/i915/gtt: Use a common type for page directories
a0c0d406ac14 drm/i915/gtt: No need to zero the table for page dirs

== Logs ==

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs
  2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
                   ` (11 preceding siblings ...)
  2019-06-15  4:59 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-06-17 10:32 ` Patchwork
  12 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-06-17 10:32 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs
URL   : https://patchwork.freedesktop.org/series/62122/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6277_full -> Patchwork_13289_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-internal-10ms:
    - shard-skl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#110913 ]) +2 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-skl10/igt@gem_eio@in-flight-internal-10ms.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-skl10/igt@gem_eio@in-flight-internal-10ms.html

  * igt@gem_eio@in-flight-internal-1us:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#110913 ]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-apl1/igt@gem_eio@in-flight-internal-1us.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-apl4/igt@gem_eio@in-flight-internal-1us.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#110854])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb4/igt@gem_exec_balancer@smoke.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb7/igt@gem_exec_balancer@smoke.html

  * igt@gem_persistent_relocs@forked-thrashing:
    - shard-kbl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#110913 ]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-kbl3/igt@gem_persistent_relocs@forked-thrashing.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-kbl6/igt@gem_persistent_relocs@forked-thrashing.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         [PASS][9] -> [FAIL][10] ([fdo#108686])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb7/igt@gem_tiled_swapping@non-threaded.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb4/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-glk:          [PASS][11] -> [DMESG-WARN][12] ([fdo#110913 ])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-glk3/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-glk7/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
    - shard-iclb:         [PASS][13] -> [DMESG-WARN][14] ([fdo#110913 ])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb6/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb7/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
    - shard-snb:          [PASS][15] -> [DMESG-WARN][16] ([fdo#110913 ])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-snb5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-snb7/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-hsw:          [PASS][17] -> [DMESG-WARN][18] ([fdo#110913 ])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-hsw7/igt@gem_userptr_blits@sync-unmap-cycles.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-hsw7/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@i915_pm_rpm@modeset-lpsp-stress-no-wait:
    - shard-iclb:         [PASS][19] -> [INCOMPLETE][20] ([fdo#107713] / [fdo#108840])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb2/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb2/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][21] -> [DMESG-WARN][22] ([fdo#108566]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html
    - shard-skl:          [PASS][23] -> [INCOMPLETE][24] ([fdo#104108])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-skl6/igt@i915_suspend@fence-restore-tiled2untiled.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-skl8/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][25] -> [FAIL][26] ([fdo#105363])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-glk9/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][27] -> [FAIL][28] ([fdo#105363])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-render:
    - shard-iclb:         [PASS][29] -> [FAIL][30] ([fdo#103167]) +4 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-render.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-render.html

  * igt@kms_psr@psr2_primary_blt:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109441]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb2/igt@kms_psr@psr2_primary_blt.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb5/igt@kms_psr@psr2_primary_blt.html

  * igt@perf@blocking:
    - shard-skl:          [PASS][33] -> [FAIL][34] ([fdo#110728])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-skl7/igt@perf@blocking.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-skl4/igt@perf@blocking.html

  * igt@prime_busy@after-render:
    - shard-hsw:          [PASS][35] -> [INCOMPLETE][36] ([fdo#103540])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-hsw7/igt@prime_busy@after-render.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-hsw7/igt@prime_busy@after-render.html

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-iclb:         [FAIL][37] ([fdo#110667]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb8/igt@gem_eio@in-flight-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb8/igt@gem_eio@in-flight-suspend.html

  * igt@gem_persistent_relocs@forked-faulting-reloc-thrashing:
    - shard-glk:          [DMESG-WARN][39] ([fdo#110913 ]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-glk3/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-glk5/igt@gem_persistent_relocs@forked-faulting-reloc-thrashing.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-hsw:          [DMESG-WARN][41] ([fdo#110913 ]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-hsw5/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-hsw7/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
    - shard-skl:          [DMESG-WARN][43] ([fdo#110913 ]) -> [PASS][44] +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-skl2/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-skl3/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
    - shard-apl:          [DMESG-WARN][45] ([fdo#110913 ]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-apl4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-apl4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-snb:          [DMESG-WARN][47] ([fdo#110913 ]) -> [PASS][48] +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-snb1/igt@gem_userptr_blits@sync-unmap-cycles.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-snb2/igt@gem_userptr_blits@sync-unmap-cycles.html
    - shard-kbl:          [DMESG-WARN][49] ([fdo#110913 ]) -> [PASS][50] +2 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-kbl6/igt@gem_userptr_blits@sync-unmap-cycles.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-kbl1/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@i915_pm_backlight@fade_with_suspend:
    - shard-skl:          [INCOMPLETE][51] ([fdo#104108]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-skl3/igt@i915_pm_backlight@fade_with_suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-skl2/igt@i915_pm_backlight@fade_with_suspend.html

  * igt@i915_pm_rpm@universal-planes-dpms:
    - shard-iclb:         [INCOMPLETE][53] ([fdo#107713] / [fdo#108840]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb2/igt@i915_pm_rpm@universal-planes-dpms.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb2/igt@i915_pm_rpm@universal-planes-dpms.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][55] ([fdo#108566]) -> [PASS][56] +4 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-apl:          [FAIL][57] ([fdo#102887] / [fdo#105363]) -> [PASS][58]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-apl2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-apl8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][59] ([fdo#103167]) -> [PASS][60] +4 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][61] ([fdo#108566]) -> [PASS][62] +3 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-apl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.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_6277/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-skl5/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

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

  * igt@kms_setmode@basic:
    - shard-skl:          [FAIL][67] ([fdo#99912]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6277/shard-skl3/igt@kms_setmode@basic.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13289/shard-skl2/igt@kms_setmode@basic.html

  
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110667]: https://bugs.freedesktop.org/show_bug.cgi?id=110667
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#110913 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110913 
  [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_6277 -> Patchwork_13289

  CI_DRM_6277: a0195925ba8732ffae5fed3c0b966ca331bae66c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5057: 3b91c82b90d45c1a30569410c1142b541956740a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13289: f33418fc780b3aa9f3beaf73dc2586d42a045264 @ 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_13289/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2019-06-17 10:32 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-14 16:43 [PATCH 01/10] drm/i915/gtt: No need to zero the table for page dirs Mika Kuoppala
2019-06-14 16:43 ` [PATCH 02/10] drm/i915/gtt: Use a common type for page directories Mika Kuoppala
2019-06-14 16:56   ` Chris Wilson
2019-06-14 16:43 ` [PATCH 03/10] drm/i915/gtt: Introduce init_pd_with_page Mika Kuoppala
2019-06-14 17:10   ` Chris Wilson
2019-06-14 16:43 ` [PATCH 04/10] drm/i915/gtt: Introduce init_pd Mika Kuoppala
2019-06-14 17:13   ` Chris Wilson
2019-06-14 16:43 ` [PATCH 05/10] drm/i915/gtt: Generalize alloc_pd Mika Kuoppala
2019-06-14 17:17   ` Chris Wilson
2019-06-14 16:43 ` [PATCH 06/10] drm/i915/gtt: pde entry encoding is identical Mika Kuoppala
2019-06-14 17:21   ` Chris Wilson
2019-06-14 16:43 ` [PATCH 07/10] drm/i915/gtt: Check for physical page for pd entry always Mika Kuoppala
2019-06-14 17:22   ` Chris Wilson
2019-06-14 16:43 ` [PATCH 08/10] drm/i915/gtt: Make swapping the pd entry generic Mika Kuoppala
2019-06-14 17:26   ` Chris Wilson
2019-06-14 16:43 ` [PATCH 09/10] drm/i915/gtt: Tear down setup and cleanup macros for page dma Mika Kuoppala
2019-06-14 17:30   ` Chris Wilson
2019-06-14 16:43 ` [PATCH 10/10] drm/i915/gtt: Setup phys pages for 3lvl pdps Mika Kuoppala
2019-06-14 17:36   ` Chris Wilson
2019-06-14 17:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/10] drm/i915/gtt: No need to zero the table for page dirs Patchwork
2019-06-14 17:04 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-06-15  4:59 ` ✓ Fi.CI.BAT: success " Patchwork
2019-06-17 10:32 ` ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox