Re: [PATCH] drm/i915: Move VMAs to inactive as request are retired

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 24/11/15 17:47, Daniel Vetter wrote:
On Mon, Nov 23, 2015 at 03:12:35PM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Current code moves _any_ VMA to the inactive list only when
_all_ rendering on an object (so from any context or VM) has
been completed.

This creates an un-natural situation where the context (and
VM) destructors can run with VMAs still on the respective
active list.

Change here is to move VMAs to the inactive list as the
requests are getting retired.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92638
Testcase: igt/gem_request_retire/retire-vma-not-inactive
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++++++++-
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cd7e102720f4..47a743246d2c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2413,17 +2413,32 @@ static void
  i915_gem_object_retire__read(struct drm_i915_gem_object *obj, int ring)
  {
  	struct i915_vma *vma;
+	struct i915_address_space *vm;

  	RQ_BUG_ON(obj->last_read_req[ring] == NULL);
  	RQ_BUG_ON(!(obj->active & (1 << ring)));

  	list_del_init(&obj->ring_list[ring]);
-	i915_gem_request_assign(&obj->last_read_req[ring], NULL);

  	if (obj->last_write_req && obj->last_write_req->ring->id == ring)
  		i915_gem_object_retire__write(obj);

  	obj->active &= ~(1 << ring);
+
+	if (obj->last_read_req[ring]->ctx->ppgtt)
+		vm = &obj->last_read_req[ring]->ctx->ppgtt->base;
+	else
+		vm = &obj->last_read_req[ring]->i915->gtt.base;
+
+	list_for_each_entry(vma, &obj->vma_list, vma_link) {
+		if (vma->vm == vm &&
+		    vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL &&
+		    !list_empty(&vma->mm_list))
+			list_move_tail(&vma->mm_list, &vma->vm->inactive_list);
+	}

This is only a partial solution since with schedulers and semaphores and a
few depencies on a given object, but in different vm you can still end up
with an object that is idle in a vm, but slipped through here.

Could you describe the exact scenario you had in mind? I won't pretend it this is simple code and I have it all figured out.

Also, checking for the view type is some strange layering. Why that?

!ppgtt to skip potential other views I thought, no?

Regards,

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux