diff mbox

drm/i915: Only unwind the local pgtable layer if empty

Message ID 20170225211959.15873-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Feb. 25, 2017, 9:19 p.m. UTC 
Only if we allocated the layer and the lower level failed should we
remove this layer when unwinding. Otherwise we ignore the overlapping
entries by overwritting the old layer with scratch.

Fixes: c5d092a4293f ("drm/i915: Remove bitmap tracking for used-pml4")
Fixes: e2b763caa6eb ("drm/i915: Remove bitmap tracking for used-pdpes")
Reported-by: Matthew Auld <matthew.william.auld@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99948
Testcase: igt/drv_selftest/live_gtt
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

Comments

Chris Wilson Feb. 25, 2017, 9:41 p.m. UTC | #1 
On Sat, Feb 25, 2017 at 09:19:59PM +0000, Chris Wilson wrote:
> Only if we allocated the layer and the lower level failed should we
> remove this layer when unwinding. Otherwise we ignore the overlapping
> entries by overwritting the old layer with scratch.
> 
> Fixes: c5d092a4293f ("drm/i915: Remove bitmap tracking for used-pml4")
> Fixes: e2b763caa6eb ("drm/i915: Remove bitmap tracking for used-pdpes")
> Reported-by: Matthew Auld <matthew.william.auld@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99948

Seems like it got further, but still hits the same bug_on.
-Chris
Chris Wilson Feb. 25, 2017, 10:13 p.m. UTC | #2 
On Sat, Feb 25, 2017 at 09:41:56PM +0000, Chris Wilson wrote:
> On Sat, Feb 25, 2017 at 09:19:59PM +0000, Chris Wilson wrote:
> > Only if we allocated the layer and the lower level failed should we
> > remove this layer when unwinding. Otherwise we ignore the overlapping
> > entries by overwritting the old layer with scratch.
> > 
> > Fixes: c5d092a4293f ("drm/i915: Remove bitmap tracking for used-pml4")
> > Fixes: e2b763caa6eb ("drm/i915: Remove bitmap tracking for used-pdpes")
> > Reported-by: Matthew Auld <matthew.william.auld@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99948
> 
> Seems like it got further, but still hits the same bug_on.

Ah, starting to look like there are two (maybe more bugs) with identical
symptoms!
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 05b9a6cd3fff..512fe4dd7f2e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -725,6 +725,7 @@  static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 			continue;
 
 		gen8_ppgtt_set_pde(vm, pd, vm->scratch_pt, pde);
+		GEM_BUG_ON(!pd->used_pdes);
 		pd->used_pdes--;
 
 		free_pt(vm, pt);
@@ -764,6 +765,7 @@  static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
 			continue;
 
 		gen8_ppgtt_set_pdpe(vm, pdp, vm->scratch_pd, pdpe);
+		GEM_BUG_ON(!pdp->used_pdpes);
 		pdp->used_pdpes--;
 
 		free_pd(vm, pd);
@@ -1094,6 +1096,7 @@  static int gen8_ppgtt_alloc_pd(struct i915_address_space *vm,
 
 			gen8_ppgtt_set_pde(vm, pd, pt, pde);
 			pd->used_pdes++;
+			GEM_BUG_ON(pd->used_pdes > I915_PDES);
 		}
 
 		pt->used_ptes += gen8_pte_count(start, length);
@@ -1123,21 +1126,25 @@  static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 			gen8_initialize_pd(vm, pd);
 			gen8_ppgtt_set_pdpe(vm, pdp, pd, pdpe);
 			pdp->used_pdpes++;
+			GEM_BUG_ON(pdp->used_pdpes > I915_PDPES_PER_PDP(vm));
 
 			mark_tlbs_dirty(i915_vm_to_ppgtt(vm));
 		}
 
 		ret = gen8_ppgtt_alloc_pd(vm, pd, start, length);
-		if (unlikely(ret)) {
-			gen8_ppgtt_set_pdpe(vm, pdp, vm->scratch_pd, pdpe);
-			pdp->used_pdpes--;
-			free_pd(vm, pd);
-			goto unwind;
-		}
+		if (unlikely(ret))
+			goto unwind_pd;
 	}
 
 	return 0;
 
+unwind_pd:
+	if (!pd->used_pdes) {
+		gen8_ppgtt_set_pdpe(vm, pdp, vm->scratch_pd, pdpe);
+		GEM_BUG_ON(!pdp->used_pdpes);
+		pdp->used_pdpes--;
+		free_pd(vm, pd);
+	}
 unwind:
 	gen8_ppgtt_clear_pdp(vm, pdp, from, start - from);
 	return -ENOMEM;
@@ -1171,15 +1178,17 @@  static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 		}
 
 		ret = gen8_ppgtt_alloc_pdp(vm, pdp, start, length);
-		if (unlikely(ret)) {
-			gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
-			free_pdp(vm, pdp);
-			goto unwind;
-		}
+		if (unlikely(ret))
+			goto unwind_pdp;
 	}
 
 	return 0;
 
+unwind_pdp:
+	if (!pdp->used_pdpes) {
+		gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
+		free_pdp(vm, pdp);
+	}
 unwind:
 	gen8_ppgtt_clear_4lvl(vm, from, start - from);
 	return -ENOMEM;