[PATCH 1/5] drm/i915/gtt: Prefer i915_vm_is_48bit() over macro

[PATCH 1/5] drm/i915/gtt: Prefer i915_vm_is_48bit() over macro

[Mika Kuoppala]

If we setup the vm size early, we can use the newly introduced
i915_vm_is_48bit() in majority of callsites wanting to know the vm size.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a5162cb..1fdab96 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -554,7 +554,7 @@ alloc_pdp(struct i915_address_space *vm)
 	struct i915_page_directory_pointer *pdp;
 	int ret = -ENOMEM;
 
-	WARN_ON(!USES_FULL_48BIT_PPGTT(vm->i915));
+	WARN_ON(!i915_vm_is_48bit(vm));
 
 	pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
 	if (!pdp)
@@ -582,10 +582,12 @@ static void free_pdp(struct i915_address_space *vm,
 		     struct i915_page_directory_pointer *pdp)
 {
 	__pdp_fini(pdp);
-	if (USES_FULL_48BIT_PPGTT(vm->i915)) {
-		cleanup_px(vm, pdp);
-		kfree(pdp);
-	}
+
+	if (!i915_vm_is_48bit(vm))
+		return;
+
+	cleanup_px(vm, pdp);
+	kfree(pdp);
 }
 
 static void gen8_initialize_pdp(struct i915_address_space *vm,
@@ -736,7 +738,7 @@ static void gen8_ppgtt_set_pdpe(struct i915_address_space *vm,
 	gen8_ppgtt_pdpe_t *vaddr;
 
 	pdp->page_directory[pdpe] = pd;
-	if (!USES_FULL_48BIT_PPGTT(vm->i915))
+	if (!i915_vm_is_48bit(vm))
 		return;
 
 	vaddr = kmap_atomic_px(pdp);
@@ -798,7 +800,7 @@ static void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
 	struct i915_page_directory_pointer *pdp;
 	unsigned int pml4e;
 
-	GEM_BUG_ON(!USES_FULL_48BIT_PPGTT(vm->i915));
+	GEM_BUG_ON(!i915_vm_is_48bit(vm));
 
 	gen8_for_each_pml4e(pdp, pml4, start, length, pml4e) {
 		if (!gen8_ppgtt_clear_pdp(vm, pdp, start, length))
@@ -941,7 +943,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 		goto free_pt;
 	}
 
-	if (USES_FULL_48BIT_PPGTT(dev)) {
+	if (i915_vm_is_48bit(vm)) {
 		vm->scratch_pdp = alloc_pdp(vm);
 		if (IS_ERR(vm->scratch_pdp)) {
 			ret = PTR_ERR(vm->scratch_pdp);
@@ -951,7 +953,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 (USES_FULL_48BIT_PPGTT(dev_priv))
+	if (i915_vm_is_48bit(vm))
 		gen8_initialize_pdp(vm, vm->scratch_pdp);
 
 	return 0;
@@ -968,12 +970,13 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 
 static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool create)
 {
+	struct i915_address_space *vm = &ppgtt->base;
+	struct drm_i915_private *dev_priv = vm->i915;
 	enum vgt_g2v_type msg;
-	struct drm_i915_private *dev_priv = ppgtt->base.i915;
 	int i;
 
-	if (USES_FULL_48BIT_PPGTT(dev_priv)) {
-		u64 daddr = px_dma(&ppgtt->pml4);
+	if (i915_vm_is_48bit(vm)) {
+		const u64 daddr = px_dma(&ppgtt->pml4);
 
 		I915_WRITE(vgtif_reg(pdp[0].lo), lower_32_bits(daddr));
 		I915_WRITE(vgtif_reg(pdp[0].hi), upper_32_bits(daddr));
@@ -982,7 +985,7 @@ static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool create)
 				VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY);
 	} else {
 		for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
-			u64 daddr = i915_page_dir_dma_addr(ppgtt, i);
+			const u64 daddr = i915_page_dir_dma_addr(ppgtt, i);
 
 			I915_WRITE(vgtif_reg(pdp[i].lo), lower_32_bits(daddr));
 			I915_WRITE(vgtif_reg(pdp[i].hi), upper_32_bits(daddr));
@@ -999,7 +1002,7 @@ static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool create)
 
 static void gen8_free_scratch(struct i915_address_space *vm)
 {
-	if (USES_FULL_48BIT_PPGTT(vm->i915))
+	if (i915_vm_is_48bit(vm))
 		free_pdp(vm, vm->scratch_pdp);
 	free_pd(vm, vm->scratch_pd);
 	free_pt(vm, vm->scratch_pt);
@@ -1044,10 +1047,10 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
 	if (intel_vgpu_active(dev_priv))
 		gen8_ppgtt_notify_vgt(ppgtt, false);
 
-	if (!USES_FULL_48BIT_PPGTT(vm->i915))
-		gen8_ppgtt_cleanup_3lvl(&ppgtt->base, &ppgtt->pdp);
-	else
+	if (i915_vm_is_48bit(vm))
 		gen8_ppgtt_cleanup_4lvl(ppgtt);
+	else
+		gen8_ppgtt_cleanup_3lvl(&ppgtt->base, &ppgtt->pdp);
 
 	gen8_free_scratch(vm);
 }
@@ -1222,9 +1225,7 @@ static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 		gen8_pte_encode(vm->scratch_page.daddr, I915_CACHE_LLC);
 	u64 start = 0, length = ppgtt->base.total;
 
-	if (!USES_FULL_48BIT_PPGTT(vm->i915)) {
-		gen8_dump_pdp(ppgtt, &ppgtt->pdp, start, length, scratch_pte, m);
-	} else {
+	if (i915_vm_is_48bit(vm)) {
 		u64 pml4e;
 		struct i915_pml4 *pml4 = &ppgtt->pml4;
 		struct i915_page_directory_pointer *pdp;
@@ -1236,6 +1237,8 @@ static void gen8_dump_ppgtt(struct i915_hw_ppgtt *ppgtt, struct seq_file *m)
 			seq_printf(m, "    PML4E #%llu\n", pml4e);
 			gen8_dump_pdp(ppgtt, pdp, start, length, scratch_pte, m);
 		}
+	} else {
+		gen8_dump_pdp(ppgtt, &ppgtt->pdp, start, length, scratch_pte, m);
 	}
 }
 
@@ -1280,12 +1283,19 @@ static int gen8_preallocate_top_level_pdp(struct i915_hw_ppgtt *ppgtt)
  */
 static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 {
-	struct drm_i915_private *dev_priv = ppgtt->base.i915;
+	struct i915_address_space *vm = &ppgtt->base;
+	struct drm_i915_private *dev_priv = vm->i915;
 	int ret;
 
+	ppgtt->base.total = USES_FULL_48BIT_PPGTT(dev_priv) ?
+		1ULL << 48 :
+		1ULL << 32;
+
 	ret = gen8_init_scratch(&ppgtt->base);
-	if (ret)
+	if (ret) {
+		ppgtt->base.total = 0;
 		return ret;
+	}
 
 	ppgtt->base.cleanup = gen8_ppgtt_cleanup;
 	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
@@ -1298,14 +1308,13 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 	if (IS_CHERRYVIEW(dev_priv) || IS_BROXTON(dev_priv))
 		ppgtt->base.pt_kmap_wc = true;
 
-	if (USES_FULL_48BIT_PPGTT(dev_priv)) {
+	if (i915_vm_is_48bit(vm)) {
 		ret = setup_px(&ppgtt->base, &ppgtt->pml4);
 		if (ret)
 			goto free_scratch;
 
 		gen8_initialize_pml4(&ppgtt->base, &ppgtt->pml4);
 
-		ppgtt->base.total = 1ULL << 48;
 		ppgtt->switch_mm = gen8_48b_mm_switch;
 
 		ppgtt->base.allocate_va_range = gen8_ppgtt_alloc_4lvl;
@@ -1316,7 +1325,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		if (ret)
 			goto free_scratch;
 
-		ppgtt->base.total = 1ULL << 32;
 		ppgtt->switch_mm = gen8_legacy_mm_switch;
 
 		if (intel_vgpu_active(dev_priv)) {
-- 
2.7.4

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

[PATCH 2/5] drm/i915: Don't mark pdps clear if pdps are not submitted

[Mika Kuoppala]

Don't mark pdps clear if never do the necessary actions
with the hardware to make them clear.

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

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 39329d4..5dc2bbb 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1321,7 +1321,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
 			      u64 offset, u32 len,
 			      unsigned int dispatch_flags)
 {
-	bool ppgtt = !(dispatch_flags & I915_DISPATCH_SECURE);
+	const bool use_ppgtt = !(dispatch_flags & I915_DISPATCH_SECURE);
 	u32 *cs;
 	int ret;
 
@@ -1332,13 +1332,12 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
 	 * not idle). PML4 is allocated during ppgtt init so this is
 	 * not needed in 48-bit.*/
 	if (req->ctx->ppgtt &&
-	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings)) {
-		if (!i915_vm_is_48bit(&req->ctx->ppgtt->base) &&
-		    !intel_vgpu_active(req->i915)) {
-			ret = intel_logical_ring_emit_pdps(req);
-			if (ret)
-				return ret;
-		}
+	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings) &&
+	    !i915_vm_is_48bit(&req->ctx->ppgtt->base) &&
+	    !intel_vgpu_active(req->i915)) {
+		ret = intel_logical_ring_emit_pdps(req);
+		if (ret)
+			return ret;
 
 		req->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(req->engine);
 	}
@@ -1348,7 +1347,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
 		return PTR_ERR(cs);
 
 	/* FIXME(BDW): Address space and security selectors. */
-	*cs++ = MI_BATCH_BUFFER_START_GEN8 | (ppgtt << 8) | (dispatch_flags &
+	*cs++ = MI_BATCH_BUFFER_START_GEN8 | (use_ppgtt << 8) | (dispatch_flags &
 		I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);
 	*cs++ = lower_32_bits(offset);
 	*cs++ = upper_32_bits(offset);
-- 
2.7.4

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

[PATCH 3/5] drm/i915/gtt: Use 32bit instead of legacy

[Mika Kuoppala]

Instead of using the subjective term legacy, use more descriptive and
less loaded name '32bit' where appropriate.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 20 ++++++++++----------
 drivers/gpu/drm/i915/i915_gem_gtt.h |  8 ++++----
 drivers/gpu/drm/i915/intel_lrc.c    |  4 ++--
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1fdab96..269a8f1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -636,12 +636,12 @@ static int gen8_write_pdp(struct drm_i915_gem_request *req,
 	return 0;
 }
 
-static int gen8_legacy_mm_switch(struct i915_hw_ppgtt *ppgtt,
-				 struct drm_i915_gem_request *req)
+static int gen8_mm_switch_32bit(struct i915_hw_ppgtt *ppgtt,
+				struct drm_i915_gem_request *req)
 {
 	int i, ret;
 
-	for (i = GEN8_LEGACY_PDPES - 1; i >= 0; i--) {
+	for (i = GEN8_32BIT_PDPES - 1; i >= 0; i--) {
 		const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
 
 		ret = gen8_write_pdp(req, i, pd_daddr);
@@ -652,8 +652,8 @@ static int gen8_legacy_mm_switch(struct i915_hw_ppgtt *ppgtt,
 	return 0;
 }
 
-static int gen8_48b_mm_switch(struct i915_hw_ppgtt *ppgtt,
-			      struct drm_i915_gem_request *req)
+static int gen8_mm_switch_48bit(struct i915_hw_ppgtt *ppgtt,
+				struct drm_i915_gem_request *req)
 {
 	return gen8_write_pdp(req, 0, px_dma(&ppgtt->pml4));
 }
@@ -857,7 +857,7 @@ gen8_ppgtt_insert_pte_entries(struct i915_hw_ppgtt *ppgtt,
 				}
 
 				GEM_BUG_ON(!i915_vm_is_48bit(&ppgtt->base) &&
-					   pdpe >= GEN8_LEGACY_PDPES);
+					   pdpe >= GEN8_32BIT_PDPES);
 				pd = pdp->page_directory[pdpe];
 				pde = 0;
 			}
@@ -984,7 +984,7 @@ static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool create)
 		msg = (create ? VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE :
 				VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY);
 	} else {
-		for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
+		for (i = 0; i < GEN8_32BIT_PDPES; i++) {
 			const u64 daddr = i915_page_dir_dma_addr(ppgtt, i);
 
 			I915_WRITE(vgtif_reg(pdp[i].lo), lower_32_bits(daddr));
@@ -1275,7 +1275,7 @@ static int gen8_preallocate_top_level_pdp(struct i915_hw_ppgtt *ppgtt)
 }
 
 /*
- * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers
+ * GEN8 32bit 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
  * PDP represents 1GB of memory 4 * 512 * 512 * 4096 = 4GB legacy 32b address
  * space.
@@ -1315,7 +1315,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 
 		gen8_initialize_pml4(&ppgtt->base, &ppgtt->pml4);
 
-		ppgtt->switch_mm = gen8_48b_mm_switch;
+		ppgtt->switch_mm = gen8_mm_switch_48bit;
 
 		ppgtt->base.allocate_va_range = gen8_ppgtt_alloc_4lvl;
 		ppgtt->base.insert_entries = gen8_ppgtt_insert_4lvl;
@@ -1325,7 +1325,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		if (ret)
 			goto free_scratch;
 
-		ppgtt->switch_mm = gen8_legacy_mm_switch;
+		ppgtt->switch_mm = gen8_mm_switch_32bit;
 
 		if (intel_vgpu_active(dev_priv)) {
 			ret = gen8_preallocate_top_level_pdp(ppgtt);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f7d4e19..83bb718 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -101,13 +101,13 @@ typedef u64 gen8_ppgtt_pml4e_t;
 #define HSW_GTT_ADDR_ENCODE(addr)	((addr) | (((addr) >> 28) & 0x7f0))
 #define HSW_PTE_ADDR_ENCODE(addr)	HSW_GTT_ADDR_ENCODE(addr)
 
-/* GEN8 legacy style address is defined as a 3 level page table:
+/* GEN8 32b style address is defined as a 3 level page table:
  * 31:30 | 29:21 | 20:12 |  11:0
  * PDPE  |  PDE  |  PTE  | offset
  * The difference as compared to normal x86 3 level page table is the PDPEs are
  * programmed via register.
  *
- * GEN8 48b legacy style address is defined as a 4 level page table:
+ * GEN8 48b style address is defined as a 4 level page table:
  * 47:39 | 38:30 | 29:21 | 20:12 |  11:0
  * PML4E | PDPE  |  PDE  |  PTE  | offset
  */
@@ -122,11 +122,11 @@ typedef u64 gen8_ppgtt_pml4e_t;
 #define GEN8_PDE_MASK			0x1ff
 #define GEN8_PTE_SHIFT			12
 #define GEN8_PTE_MASK			0x1ff
-#define GEN8_LEGACY_PDPES		4
+#define GEN8_32BIT_PDPES		4
 #define GEN8_PTES			I915_PTES(sizeof(gen8_pte_t))
 
 #define I915_PDPES_PER_PDP(dev_priv)	(USES_FULL_48BIT_PPGTT(dev_priv) ?\
-					GEN8_PML4ES_PER_PML4 : GEN8_LEGACY_PDPES)
+					GEN8_PML4ES_PER_PML4 : GEN8_32BIT_PDPES)
 
 #define PPAT_UNCACHED_INDEX		(_PAGE_PWT | _PAGE_PCD)
 #define PPAT_CACHED_PDE_INDEX		0 /* WB LLC */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 5dc2bbb..6762c82 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1293,7 +1293,7 @@ static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
 {
 	struct i915_hw_ppgtt *ppgtt = req->ctx->ppgtt;
 	struct intel_engine_cs *engine = req->engine;
-	const int num_lri_cmds = GEN8_LEGACY_PDPES * 2;
+	const int num_lri_cmds = GEN8_32BIT_PDPES * 2;
 	u32 *cs;
 	int i;
 
@@ -1302,7 +1302,7 @@ static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
 		return PTR_ERR(cs);
 
 	*cs++ = MI_LOAD_REGISTER_IMM(num_lri_cmds);
-	for (i = GEN8_LEGACY_PDPES - 1; i >= 0; i--) {
+	for (i = GEN8_32BIT_PDPES - 1; i >= 0; i--) {
 		const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
 
 		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(engine, i));
-- 
2.7.4

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

[PATCH 4/5] drm/i915/gtt: No need to do tlb flush with 48bit vm

[Mika Kuoppala]

The pdp rewrite to force tlb flush is only a 32bit ppgtt
trick. That should not be needed with 48bit ppgtt.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 269a8f1..791cb81 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1103,7 +1103,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 			gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
 			pdp->used_pdpes++;
 
-			mark_tlbs_dirty(i915_vm_to_ppgtt(vm));
+			if (!i915_vm_is_48bit(vm))
+				mark_tlbs_dirty(i915_vm_to_ppgtt(vm));
 		}
 
 		ret = gen8_ppgtt_alloc_pd(vm, pd, start, length);
-- 
2.7.4

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

[PATCH 5/5] drm/i915/gtt: Setup vm callbacks late

[Mika Kuoppala]

If we manage to tangle errorpaths and get call to callbacks,
it is better to defensively keep them as null until object init is
finished so that we get clean null deref on callsite,
instead of more cryptic wreckage with partly initialized vm objects.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 791cb81..77276c9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1298,11 +1298,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		return ret;
 	}
 
-	ppgtt->base.cleanup = gen8_ppgtt_cleanup;
-	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-	ppgtt->base.bind_vma = ppgtt_bind_vma;
-	ppgtt->debug_dump = gen8_dump_ppgtt;
-
 	/* There are only few exceptions for gen >=6. chv and bxt.
 	 * And we are not sure about the latter so play safe for now.
 	 */
@@ -1317,7 +1312,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		gen8_initialize_pml4(&ppgtt->base, &ppgtt->pml4);
 
 		ppgtt->switch_mm = gen8_mm_switch_48bit;
-
 		ppgtt->base.allocate_va_range = gen8_ppgtt_alloc_4lvl;
 		ppgtt->base.insert_entries = gen8_ppgtt_insert_4lvl;
 		ppgtt->base.clear_range = gen8_ppgtt_clear_4lvl;
@@ -1326,8 +1320,6 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		if (ret)
 			goto free_scratch;
 
-		ppgtt->switch_mm = gen8_mm_switch_32bit;
-
 		if (intel_vgpu_active(dev_priv)) {
 			ret = gen8_preallocate_top_level_pdp(ppgtt);
 			if (ret) {
@@ -1336,6 +1328,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 			}
 		}
 
+		ppgtt->switch_mm = gen8_mm_switch_32bit;
 		ppgtt->base.allocate_va_range = gen8_ppgtt_alloc_3lvl;
 		ppgtt->base.insert_entries = gen8_ppgtt_insert_3lvl;
 		ppgtt->base.clear_range = gen8_ppgtt_clear_3lvl;
@@ -1344,6 +1337,11 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 	if (intel_vgpu_active(dev_priv))
 		gen8_ppgtt_notify_vgt(ppgtt, true);
 
+	ppgtt->base.cleanup = gen8_ppgtt_cleanup;
+	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
+	ppgtt->base.bind_vma = ppgtt_bind_vma;
+	ppgtt->debug_dump = gen8_dump_ppgtt;
+
 	return 0;
 
 free_scratch:
@@ -1787,13 +1785,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 	if (ret)
 		return ret;
 
-	ppgtt->base.clear_range = gen6_ppgtt_clear_range;
-	ppgtt->base.insert_entries = gen6_ppgtt_insert_entries;
-	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-	ppgtt->base.bind_vma = ppgtt_bind_vma;
-	ppgtt->base.cleanup = gen6_ppgtt_cleanup;
 	ppgtt->base.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
-	ppgtt->debug_dump = gen6_dump_ppgtt;
 
 	gen6_scratch_va_range(ppgtt, 0, ppgtt->base.total);
 	gen6_write_page_range(ppgtt, 0, ppgtt->base.total);
@@ -1804,6 +1796,13 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		return ret;
 	}
 
+	ppgtt->base.clear_range = gen6_ppgtt_clear_range;
+	ppgtt->base.insert_entries = gen6_ppgtt_insert_entries;
+	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
+	ppgtt->base.bind_vma = ppgtt_bind_vma;
+	ppgtt->base.cleanup = gen6_ppgtt_cleanup;
+	ppgtt->debug_dump = gen6_dump_ppgtt;
+
 	DRM_DEBUG_DRIVER("Allocated pde space (%lldM) at GTT entry: %llx\n",
 			 ppgtt->node.size >> 20,
 			 ppgtt->node.start / PAGE_SIZE);
-- 
2.7.4

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

Re: [PATCH 2/5] drm/i915: Don't mark pdps clear if pdps are not submitted

[Chris Wilson]

On Wed, Feb 22, 2017 at 06:54:22PM +0200, Mika Kuoppala wrote:
> Don't mark pdps clear if never do the necessary actions
> with the hardware to make them clear.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 39329d4..5dc2bbb 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1321,7 +1321,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  			      u64 offset, u32 len,
>  			      unsigned int dispatch_flags)
>  {
> -	bool ppgtt = !(dispatch_flags & I915_DISPATCH_SECURE);
> +	const bool use_ppgtt = !(dispatch_flags & I915_DISPATCH_SECURE);
>  	u32 *cs;
>  	int ret;
>  
> @@ -1332,13 +1332,12 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  	 * not idle). PML4 is allocated during ppgtt init so this is
>  	 * not needed in 48-bit.*/
>  	if (req->ctx->ppgtt &&
> -	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings)) {
> -		if (!i915_vm_is_48bit(&req->ctx->ppgtt->base) &&
> -		    !intel_vgpu_active(req->i915)) {
> -			ret = intel_logical_ring_emit_pdps(req);
> -			if (ret)
> -				return ret;
> -		}
> +	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings) &&
> +	    !i915_vm_is_48bit(&req->ctx->ppgtt->base) &&
> +	    !intel_vgpu_active(req->i915)) {
> +		ret = intel_logical_ring_emit_pdps(req);
> +		if (ret)
> +			return ret;
>  
>  		req->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(req->engine);
>  	}
> @@ -1348,7 +1347,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  		return PTR_ERR(cs);
>  
>  	/* FIXME(BDW): Address space and security selectors. */
> -	*cs++ = MI_BATCH_BUFFER_START_GEN8 | (ppgtt << 8) | (dispatch_flags &
> +	*cs++ = MI_BATCH_BUFFER_START_GEN8 | (use_ppgtt << 8) | (dispatch_flags &
>  		I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);

Missed an opportunity here.

	*cs++ = MI_BATCH_BUFFER_START_GEN8 |
		(dispatch_flags & I915_DISPATCH_SECURE) ? 0 : BIT(8) |
		(dispatch_flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/5] drm/i915/gtt: Use 32bit instead of legacy

[Chris Wilson]

On Wed, Feb 22, 2017 at 06:54:23PM +0200, Mika Kuoppala wrote:
> Instead of using the subjective term legacy, use more descriptive and
> less loaded name '32bit' where appropriate.

Hmm. We have a mix of 32/48bit and 3/4 level. I think here we are better
using e.g.

	gen8_mm_switch_3lvl
	gen8_mm_switch_4lvl

Thoughts? Want to convert these or the others to 32/48?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915/gtt: Setup vm callbacks late

[Chris Wilson]

On Wed, Feb 22, 2017 at 06:54:25PM +0200, Mika Kuoppala wrote:
> If we manage to tangle errorpaths and get call to callbacks,
> it is better to defensively keep them as null until object init is
> finished so that we get clean null deref on callsite,
> instead of more cryptic wreckage with partly initialized vm objects.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

Series looks ok, but you opened a can of bikeshed! :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm/i915/gtt: No need to do tlb flush with 48bit vm

[Chris Wilson]

On Wed, Feb 22, 2017 at 06:54:24PM +0200, Mika Kuoppala wrote:
> The pdp rewrite to force tlb flush is only a 32bit ppgtt
> trick. That should not be needed with 48bit ppgtt.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 269a8f1..791cb81 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1103,7 +1103,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
>  			gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
>  			pdp->used_pdpes++;
>  
> -			mark_tlbs_dirty(i915_vm_to_ppgtt(vm));
> +			if (!i915_vm_is_48bit(vm))
> +				mark_tlbs_dirty(i915_vm_to_ppgtt(vm));

Just thinking the relative merits of unconditionally setting bits vs a
branch miss. It's immaterial as this should not be a hotpath!

I'd like the idea of perhaps calling this mark_pdp_dirty), and have
mark_pml4_dirty, mark_pte_dirty() etc. But they're not used, so pretty
pointless.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/gtt: Prefer i915_vm_is_48bit() over macro

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/i915/gtt: Prefer i915_vm_is_48bit() over macro
URL   : https://patchwork.freedesktop.org/series/20086/
State : success

== Summary ==

Series 20086v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/20086/revisions/1/mbox/

fi-bdw-5557u     total:253  pass:242  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:253  pass:214  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:253  pass:234  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:253  pass:226  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:253  pass:222  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:253  pass:237  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:253  pass:237  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:253  pass:203  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:253  pass:235  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:253  pass:235  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:253  pass:235  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:253  pass:243  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:253  pass:236  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:253  pass:231  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:253  pass:243  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:253  pass:225  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:253  pass:224  dwarn:0   dfail:0   fail:0   skip:29 

bf89ec45d0822835b03910371ac0baf46c4efa2d drm-tip: 2017y-02m-22d-14h-30m-04s UTC integration manifest
1eb3847 drm/i915/gtt: Setup vm callbacks late
f0c09dc drm/i915/gtt: No need to do tlb flush with 48bit vm
d2e664a drm/i915/gtt: Use 32bit instead of legacy
4114551 drm/i915: Don't mark pdps clear if pdps are not submitted
9777694 drm/i915/gtt: Prefer i915_vm_is_48bit() over macro

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3936/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/5] drm/i915: Don't mark pdps clear if pdps are not submitted

[Chris Wilson]

On Tue, Feb 28, 2017 at 05:28:08PM +0200, Mika Kuoppala wrote:
> Don't mark pdps clear if never do the necessary actions
> with the hardware to make them clear.
> 
> v2: totally get rid of confusing ppgtt bool (Chris)
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/5] drm/i915: Don't mark pdps clear if pdps are not submitted

[Mika Kuoppala]

Don't mark pdps clear if never do the necessary actions
with the hardware to make them clear.

v2: totally get rid of confusing ppgtt bool (Chris)

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f9a8545..4ae9f1f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1319,9 +1319,8 @@ static int intel_logical_ring_emit_pdps(struct drm_i915_gem_request *req)
 
 static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
 			      u64 offset, u32 len,
-			      unsigned int dispatch_flags)
+			      const unsigned int flags)
 {
-	bool ppgtt = !(dispatch_flags & I915_DISPATCH_SECURE);
 	u32 *cs;
 	int ret;
 
@@ -1332,13 +1331,12 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
 	 * not idle). PML4 is allocated during ppgtt init so this is
 	 * not needed in 48-bit.*/
 	if (req->ctx->ppgtt &&
-	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings)) {
-		if (!i915_vm_is_48bit(&req->ctx->ppgtt->base) &&
-		    !intel_vgpu_active(req->i915)) {
-			ret = intel_logical_ring_emit_pdps(req);
-			if (ret)
-				return ret;
-		}
+	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings) &&
+	    !i915_vm_is_48bit(&req->ctx->ppgtt->base) &&
+	    !intel_vgpu_active(req->i915)) {
+		ret = intel_logical_ring_emit_pdps(req);
+		if (ret)
+			return ret;
 
 		req->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(req->engine);
 	}
@@ -1348,8 +1346,9 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
 		return PTR_ERR(cs);
 
 	/* FIXME(BDW): Address space and security selectors. */
-	*cs++ = MI_BATCH_BUFFER_START_GEN8 | (ppgtt << 8) | (dispatch_flags &
-		I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);
+	*cs++ = MI_BATCH_BUFFER_START_GEN8 |
+		(flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) |
+		(flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);
 	*cs++ = lower_32_bits(offset);
 	*cs++ = upper_32_bits(offset);
 	*cs++ = MI_NOOP;
-- 
2.7.4

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