All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Only unwind the local pgtable layer if empty
@ 2017-02-25 23:25 Chris Wilson
  2017-02-25 23:25 ` [PATCH 2/3] drm/i915: Unwind vma->pages allocation upon failure Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-25 23:25 UTC (permalink / raw)
  To: intel-gfx

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 | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6fdbd5ae4fcb..c3a121ab8914 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -716,10 +716,13 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
 	u32 pde;
 
 	gen8_for_each_pde(pt, pd, start, length, pde) {
+		GEM_BUG_ON(pt == vm->scratch_pt);
+
 		if (!gen8_ppgtt_clear_pt(vm, pt, start, length))
 			continue;
 
 		gen8_ppgtt_set_pde(vm, pd, vm->scratch_pt, pde);
+		GEM_BUG_ON(!pd->used_pdes);
 		pd->used_pdes--;
 
 		free_pt(vm, pt);
@@ -755,10 +758,13 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
 	unsigned int pdpe;
 
 	gen8_for_each_pdpe(pd, pdp, start, length, pdpe) {
+		GEM_BUG_ON(pd == vm->scratch_pd);
+
 		if (!gen8_ppgtt_clear_pd(vm, pd, start, length))
 			continue;
 
 		gen8_ppgtt_set_pdpe(vm, pdp, vm->scratch_pd, pdpe);
+		GEM_BUG_ON(!pdp->used_pdpes);
 		pdp->used_pdpes--;
 
 		free_pd(vm, pd);
@@ -801,6 +807,8 @@ static void gen8_ppgtt_clear_4lvl(struct i915_address_space *vm,
 	GEM_BUG_ON(!USES_FULL_48BIT_PPGTT(vm->i915));
 
 	gen8_for_each_pml4e(pdp, pml4, start, length, pml4e) {
+		GEM_BUG_ON(pdp == vm->scratch_pdp);
+
 		if (!gen8_ppgtt_clear_pdp(vm, pdp, start, length))
 			continue;
 
@@ -1089,6 +1097,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);
@@ -1118,21 +1127,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;
@@ -1166,15 +1179,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;
-- 
2.11.0

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

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

* [PATCH 2/3] drm/i915: Unwind vma->pages allocation upon failure
  2017-02-25 23:25 [PATCH 1/3] drm/i915: Only unwind the local pgtable layer if empty Chris Wilson
@ 2017-02-25 23:25 ` Chris Wilson
  2017-02-27 12:07   ` Matthew Auld
  2017-02-25 23:25 ` [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2017-02-25 23:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: Matthew Auld

If we fail to allocate the ppgtt range after allocating the pages for
the vma, we should unwind the local allocation before reporting back the
failure.

Fixes: ff685975d97f ("drm/i915: Move allocate_va_range to GTT")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c3a121ab8914..875a48b9d05a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2312,7 +2312,7 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
 							     vma->node.start,
 							     vma->node.size);
 			if (ret)
-				return ret;
+				goto err_pages;
 		}
 
 		appgtt->base.insert_entries(&appgtt->base,
@@ -2329,6 +2329,17 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
 	}
 
 	return 0;
+
+err_pages:
+	if (!(vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND))) {
+		if (vma->pages != vma->obj->mm.pages) {
+			GEM_BUG_ON(!vma->pages);
+			sg_free_table(vma->pages);
+			kfree(vma->pages);
+		}
+		vma->pages = NULL;
+	}
+	return ret;
 }
 
 static void aliasing_gtt_unbind_vma(struct i915_vma *vma)
-- 
2.11.0

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

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

* [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails
  2017-02-25 23:25 [PATCH 1/3] drm/i915: Only unwind the local pgtable layer if empty Chris Wilson
  2017-02-25 23:25 ` [PATCH 2/3] drm/i915: Unwind vma->pages allocation upon failure Chris Wilson
@ 2017-02-25 23:25 ` Chris Wilson
  2017-02-27 12:02     ` Joonas Lahtinen
  2017-02-27 11:22 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Only unwind the local pgtable layer if empty Patchwork
  2017-02-27 11:48 ` [PATCH 1/3] " Matthew Auld
  3 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2017-02-25 23:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson, Matthew Auld, Joonas Lahtinen, # v4 . 9+

As we track whether a vma has been inserted into the drm_mm using the
vma->flags, if we fail to bind the vma into the GTT we do not update
those bits and will attempt to reinsert the vma into the drm_mm on
future passes. To prevent that, we want to unwind i915_vma_insert() if
we fail in our attempt to bind.

Fixes: 59bfa1248e22 ("drm/i915: Start passing around i915_vma from execbuffer")
Testcase: igt/drv_selftest/live_gtt
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.9+
---
 drivers/gpu/drm/i915/i915_vma.c | 54 ++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 6e9eade304b8..59dd53b1240a 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -509,6 +509,31 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 	return ret;
 }
 
+static void
+i915_vma_remove(struct i915_vma *vma)
+{
+	struct drm_i915_gem_object *obj = vma->obj;
+
+	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
+	GEM_BUG_ON(vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND));
+
+	drm_mm_remove_node(&vma->node);
+	list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
+
+	/* Since the unbound list is global, only move to that list if
+	 * no more VMAs exist. */
+	if (--obj->bind_count == 0)
+		list_move_tail(&obj->global_link,
+			       &to_i915(obj->base.dev)->mm.unbound_list);
+
+	/* And finally now the object is completely decoupled from this vma,
+	 * we can drop its hold on the backing storage and allow it to be
+	 * reaped by the shrinker.
+	 */
+	i915_gem_object_unpin_pages(obj);
+	GEM_BUG_ON(atomic_read(&obj->mm.pages_pin_count) < obj->bind_count);
+}
+
 int __i915_vma_do_pin(struct i915_vma *vma,
 		      u64 size, u64 alignment, u64 flags)
 {
@@ -521,18 +546,18 @@ int __i915_vma_do_pin(struct i915_vma *vma,
 
 	if (WARN_ON(bound & I915_VMA_PIN_OVERFLOW)) {
 		ret = -EBUSY;
-		goto err;
+		goto err_unpin;
 	}
 
 	if ((bound & I915_VMA_BIND_MASK) == 0) {
 		ret = i915_vma_insert(vma, size, alignment, flags);
 		if (ret)
-			goto err;
+			goto err_unpin;
 	}
 
 	ret = i915_vma_bind(vma, vma->obj->cache_level, flags);
 	if (ret)
-		goto err;
+		goto err_remove;
 
 	if ((bound ^ vma->flags) & I915_VMA_GLOBAL_BIND)
 		__i915_vma_set_map_and_fenceable(vma);
@@ -541,7 +566,12 @@ int __i915_vma_do_pin(struct i915_vma *vma,
 	GEM_BUG_ON(i915_vma_misplaced(vma, size, alignment, flags));
 	return 0;
 
-err:
+err_remove:
+	if ((vma->flags & I915_VMA_BIND_MASK) == 0) {
+		GEM_BUG_ON(vma->pages);
+		i915_vma_remove(vma);
+	}
+err_unpin:
 	__i915_vma_unpin(vma);
 	return ret;
 }
@@ -654,9 +684,6 @@ int i915_vma_unbind(struct i915_vma *vma)
 	}
 	vma->flags &= ~(I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND);
 
-	drm_mm_remove_node(&vma->node);
-	list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
-
 	if (vma->pages != obj->mm.pages) {
 		GEM_BUG_ON(!vma->pages);
 		sg_free_table(vma->pages);
@@ -664,18 +691,7 @@ int i915_vma_unbind(struct i915_vma *vma)
 	}
 	vma->pages = NULL;
 
-	/* Since the unbound list is global, only move to that list if
-	 * no more VMAs exist. */
-	if (--obj->bind_count == 0)
-		list_move_tail(&obj->global_link,
-			       &to_i915(obj->base.dev)->mm.unbound_list);
-
-	/* And finally now the object is completely decoupled from this vma,
-	 * we can drop its hold on the backing storage and allow it to be
-	 * reaped by the shrinker.
-	 */
-	i915_gem_object_unpin_pages(obj);
-	GEM_BUG_ON(atomic_read(&obj->mm.pages_pin_count) < obj->bind_count);
+	i915_vma_remove(vma);
 
 destroy:
 	if (unlikely(i915_vma_is_closed(vma)))
-- 
2.11.0

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

* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Only unwind the local pgtable layer if empty
  2017-02-25 23:25 [PATCH 1/3] drm/i915: Only unwind the local pgtable layer if empty Chris Wilson
  2017-02-25 23:25 ` [PATCH 2/3] drm/i915: Unwind vma->pages allocation upon failure Chris Wilson
  2017-02-25 23:25 ` [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails Chris Wilson
@ 2017-02-27 11:22 ` Patchwork
  2017-02-27 11:48 ` [PATCH 1/3] " Matthew Auld
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-02-27 11:22 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Only unwind the local pgtable layer if empty
URL   : https://patchwork.freedesktop.org/series/20257/
State : success

== Summary ==

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

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:108  pass:95   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 

78e102923da4589d97e202e86168d86f8251b416 drm-tip: 2017y-02m-27d-09h-37m-07s UTC integration manifest
0f9969e drm/i915: Remove the vma from the drm_mm if binding fails
917eef9 drm/i915: Unwind vma->pages allocation upon failure
0fa7142 drm/i915: Only unwind the local pgtable layer if empty

== Logs ==

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

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

* Re: [PATCH 1/3] drm/i915: Only unwind the local pgtable layer if empty
  2017-02-25 23:25 [PATCH 1/3] drm/i915: Only unwind the local pgtable layer if empty Chris Wilson
                   ` (2 preceding siblings ...)
  2017-02-27 11:22 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Only unwind the local pgtable layer if empty Patchwork
@ 2017-02-27 11:48 ` Matthew Auld
  2017-02-27 12:07   ` Chris Wilson
  3 siblings, 1 reply; 11+ messages in thread
From: Matthew Auld @ 2017-02-27 11:48 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Intel Graphics Development

On 25 February 2017 at 23:25, Chris Wilson <chris@chris-wilson.co.uk> 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.
overwriting

>
> 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
https://bugs.freedesktop.org/show_bug.cgi?id=99947 ?

Tested-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails
  2017-02-25 23:25 ` [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails Chris Wilson
@ 2017-02-27 12:02     ` Joonas Lahtinen
  0 siblings, 0 replies; 11+ messages in thread
From: Joonas Lahtinen @ 2017-02-27 12:02 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: # v4 . 9+

On la, 2017-02-25 at 23:25 +0000, Chris Wilson wrote:
> As we track whether a vma has been inserted into the drm_mm using the
> vma->flags, if we fail to bind the vma into the GTT we do not update
> those bits and will attempt to reinsert the vma into the drm_mm on
> future passes. To prevent that, we want to unwind i915_vma_insert() if
> we fail in our attempt to bind.
> 
> Fixes: 59bfa1248e22 ("drm/i915: Start passing around i915_vma from execbuffer")
> Testcase: igt/drv_selftest/live_gtt
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v4.9+

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

One note below.

> @@ -541,7 +566,12 @@ int __i915_vma_do_pin(struct i915_vma *vma,
>  	GEM_BUG_ON(i915_vma_misplaced(vma, size, alignment, flags));
>  	return 0;
>  
> -err:
> +err_remove:
> +	if ((vma->flags & I915_VMA_BIND_MASK) == 0) {

This condition could be more symmetric.

Regards, Joonas

> +		GEM_BUG_ON(vma->pages);
> +		i915_vma_remove(vma);
> +	}
> +err_unpin:
>  	__i915_vma_unpin(vma);
>  	return ret;
>  }

-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails
@ 2017-02-27 12:02     ` Joonas Lahtinen
  0 siblings, 0 replies; 11+ messages in thread
From: Joonas Lahtinen @ 2017-02-27 12:02 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Matthew Auld, # v4 . 9+

On la, 2017-02-25 at 23:25 +0000, Chris Wilson wrote:
> As we track whether a vma has been inserted into the drm_mm using the
> vma->flags, if we fail to bind the vma into the GTT we do not update
> those bits and will attempt to reinsert the vma into the drm_mm on
> future passes. To prevent that, we want to unwind i915_vma_insert() if
> we fail in our attempt to bind.
> 
> Fixes: 59bfa1248e22 ("drm/i915: Start passing around i915_vma from execbuffer")
> Testcase: igt/drv_selftest/live_gtt
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v4.9+

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

One note below.

> @@ -541,7 +566,12 @@ int __i915_vma_do_pin(struct i915_vma *vma,
>  	GEM_BUG_ON(i915_vma_misplaced(vma, size, alignment, flags));
>  	return 0;
>  
> -err:
> +err_remove:
> +	if ((vma->flags & I915_VMA_BIND_MASK) == 0) {

This condition could be more symmetric.

Regards, Joonas

> +		GEM_BUG_ON(vma->pages);
> +		i915_vma_remove(vma);
> +	}
> +err_unpin:
>  	__i915_vma_unpin(vma);
>  	return ret;
>  }

-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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

* Re: [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails
  2017-02-27 12:02     ` Joonas Lahtinen
@ 2017-02-27 12:06       ` Chris Wilson
  -1 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-27 12:06 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: intel-gfx, # v4 . 9+

On Mon, Feb 27, 2017 at 02:02:00PM +0200, Joonas Lahtinen wrote:
> On la, 2017-02-25 at 23:25 +0000, Chris Wilson wrote:
> > As we track whether a vma has been inserted into the drm_mm using the
> > vma->flags, if we fail to bind the vma into the GTT we do not update
> > those bits and will attempt to reinsert the vma into the drm_mm on
> > future passes. To prevent that, we want to unwind i915_vma_insert() if
> > we fail in our attempt to bind.
> > 
> > Fixes: 59bfa1248e22 ("drm/i915: Start passing around i915_vma from execbuffer")
> > Testcase: igt/drv_selftest/live_gtt
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: <stable@vger.kernel.org> # v4.9+
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> One note below.
> 
> > @@ -541,7 +566,12 @@ int __i915_vma_do_pin(struct i915_vma *vma,
> >  	GEM_BUG_ON(i915_vma_misplaced(vma, size, alignment, flags));
> >  	return 0;
> >  
> > -err:
> > +err_remove:
> > +	if ((vma->flags & I915_VMA_BIND_MASK) == 0) {
> 
> This condition could be more symmetric.

Done: if ((bound & I915_VMA_BIND_MASK) == 0) and made bound const to be
clear.
-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

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

* Re: [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails
@ 2017-02-27 12:06       ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-27 12:06 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: intel-gfx, Matthew Auld, # v4 . 9+

On Mon, Feb 27, 2017 at 02:02:00PM +0200, Joonas Lahtinen wrote:
> On la, 2017-02-25 at 23:25 +0000, Chris Wilson wrote:
> > As we track whether a vma has been inserted into the drm_mm using the
> > vma->flags, if we fail to bind the vma into the GTT we do not update
> > those bits and will attempt to reinsert the vma into the drm_mm on
> > future passes. To prevent that, we want to unwind i915_vma_insert() if
> > we fail in our attempt to bind.
> > 
> > Fixes: 59bfa1248e22 ("drm/i915: Start passing around i915_vma from execbuffer")
> > Testcase: igt/drv_selftest/live_gtt
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: <stable@vger.kernel.org> # v4.9+
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> One note below.
> 
> > @@ -541,7 +566,12 @@ int __i915_vma_do_pin(struct i915_vma *vma,
> > �	GEM_BUG_ON(i915_vma_misplaced(vma, size, alignment, flags));
> > �	return 0;
> > �
> > -err:
> > +err_remove:
> > +	if ((vma->flags & I915_VMA_BIND_MASK) == 0) {
> 
> This condition could be more symmetric.

Done: if ((bound & I915_VMA_BIND_MASK) == 0) and made bound const to be
clear.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH 2/3] drm/i915: Unwind vma->pages allocation upon failure
  2017-02-25 23:25 ` [PATCH 2/3] drm/i915: Unwind vma->pages allocation upon failure Chris Wilson
@ 2017-02-27 12:07   ` Matthew Auld
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew Auld @ 2017-02-27 12:07 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Intel Graphics Development, Matthew Auld

On 25 February 2017 at 23:25, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> If we fail to allocate the ppgtt range after allocating the pages for
> the vma, we should unwind the local allocation before reporting back the
> failure.
>
> Fixes: ff685975d97f ("drm/i915: Move allocate_va_range to GTT")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915: Only unwind the local pgtable layer if empty
  2017-02-27 11:48 ` [PATCH 1/3] " Matthew Auld
@ 2017-02-27 12:07   ` Chris Wilson
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2017-02-27 12:07 UTC (permalink / raw)
  To: Matthew Auld; +Cc: Intel Graphics Development

On Mon, Feb 27, 2017 at 11:48:46AM +0000, Matthew Auld wrote:
> On 25 February 2017 at 23:25, Chris Wilson <chris@chris-wilson.co.uk> 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.
> overwriting
> 
> >
> > 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
> https://bugs.freedesktop.org/show_bug.cgi?id=99947 ?

Oops, bad cut'n'paste.
-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

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

end of thread, other threads:[~2017-02-27 13:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-25 23:25 [PATCH 1/3] drm/i915: Only unwind the local pgtable layer if empty Chris Wilson
2017-02-25 23:25 ` [PATCH 2/3] drm/i915: Unwind vma->pages allocation upon failure Chris Wilson
2017-02-27 12:07   ` Matthew Auld
2017-02-25 23:25 ` [PATCH 3/3] drm/i915: Remove the vma from the drm_mm if binding fails Chris Wilson
2017-02-27 12:02   ` Joonas Lahtinen
2017-02-27 12:02     ` Joonas Lahtinen
2017-02-27 12:06     ` Chris Wilson
2017-02-27 12:06       ` Chris Wilson
2017-02-27 11:22 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Only unwind the local pgtable layer if empty Patchwork
2017-02-27 11:48 ` [PATCH 1/3] " Matthew Auld
2017-02-27 12:07   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.