[PATCH 15/17] drm/i915: Allow vma binding to occur asynchronously

Chris Wilson chris at chris-wilson.co.uk
Tue May 28 13:41:50 UTC 2019


If we let pages be allocated asynchronously, we also then want to push
the binding process into an asynchronous task. Make it so, utilising the
recent improvements to fence error tracking and struct_mutex reduction.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  14 +-
 .../drm/i915/gem/selftests/i915_gem_context.c |   4 +
 drivers/gpu/drm/i915/i915_gem_gtt.c           |  18 ++-
 drivers/gpu/drm/i915/i915_vma.c               | 139 ++++++++++++++++--
 drivers/gpu/drm/i915/i915_vma.h               |   9 ++
 drivers/gpu/drm/i915/selftests/i915_vma.c     |   4 +-
 6 files changed, 162 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 2c4f3229361d..db34593aaefc 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -580,6 +580,15 @@ static int eb_reserve_vma(const struct i915_execbuffer *eb,
 	u64 pin_flags;
 	int err;
 
+	/*
+	 * If we load the pages asynchronously, then the user *must*
+	 * obey the reservation_object and not bypass waiting on it.
+	 * On the positive side, if the vma is not yet bound (no pages!),
+	 * then it should not have any annoying implicit fences.
+	 */
+	if (exec_flags & EXEC_OBJECT_ASYNC && !vma->pages)
+		*vma->exec_flags &= ~EXEC_OBJECT_ASYNC;
+
 	pin_flags = PIN_USER | PIN_NONBLOCK;
 	if (exec_flags & EXEC_OBJECT_NEEDS_GTT)
 		pin_flags |= PIN_GLOBAL;
@@ -1245,8 +1254,9 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
 		goto skip_request;
 
 	i915_vma_lock(batch);
-	GEM_BUG_ON(!reservation_object_test_signaled_rcu(batch->resv, true));
-	err = i915_vma_move_to_active(batch, rq, 0);
+	err = i915_request_await_object(rq, batch->obj, false);
+	if (err == 0)
+		err = i915_vma_move_to_active(batch, rq, 0);
 	i915_vma_unlock(batch);
 	if (err)
 		goto skip_request;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 612a4bc37e6e..1bb8f193cac3 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -296,6 +296,10 @@ static int gpu_fill(struct drm_i915_gem_object *obj,
 		goto err_batch;
 	}
 
+	err = i915_request_await_object(rq, batch->obj, false);
+	if (err)
+		goto err_request;
+
 	flags = 0;
 	if (INTEL_GEN(vm->i915) <= 5)
 		flags |= I915_DISPATCH_SECURE;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ed8510d1a110..55a76d18f7fd 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -30,6 +30,7 @@
 #include <linux/random.h>
 #include <linux/seq_file.h>
 #include <linux/stop_machine.h>
+#include <linux/workqueue.h>
 
 #include <asm/set_memory.h>
 
@@ -135,14 +136,14 @@ static inline void i915_ggtt_invalidate(struct drm_i915_private *i915)
 
 static int ppgtt_bind_vma(struct i915_vma *vma,
 			  enum i915_cache_level cache_level,
-			  u32 unused)
+			  u32 flags)
 {
+	struct i915_address_space *vm = vma->vm;
 	u32 pte_flags;
 	int err;
 
-	if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
-		err = vma->vm->allocate_va_range(vma->vm,
-						 vma->node.start, vma->size);
+	if (flags & I915_VMA_ALLOC_BIND) {
+		err = vm->allocate_va_range(vm, vma->node.start, vma->size);
 		if (err)
 			return err;
 	}
@@ -152,14 +153,16 @@ static int ppgtt_bind_vma(struct i915_vma *vma,
 	if (i915_gem_object_is_readonly(vma->obj))
 		pte_flags |= PTE_READ_ONLY;
 
-	vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags);
+	vm->insert_entries(vm, vma, cache_level, pte_flags);
 
 	return 0;
 }
 
 static void ppgtt_unbind_vma(struct i915_vma *vma)
 {
-	vma->vm->clear_range(vma->vm, vma->node.start, vma->size);
+	struct i915_address_space *vm = vma->vm;
+
+	vm->clear_range(vm, vma->node.start, vma->size);
 }
 
 static int ppgtt_set_pages(struct i915_vma *vma)
@@ -2642,6 +2645,7 @@ static int ggtt_bind_vma(struct i915_vma *vma,
 	 * upgrade to both bound if we bind either to avoid double-binding.
 	 */
 	vma->flags |= I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
+	dma_fence_signal(vma->async.dma);
 
 	return 0;
 }
@@ -2671,7 +2675,7 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
 	if (flags & I915_VMA_LOCAL_BIND) {
 		struct i915_hw_ppgtt *appgtt = i915->mm.aliasing_ppgtt;
 
-		if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
+		if (flags & I915_VMA_ALLOC_BIND) {
 			ret = appgtt->vm.allocate_va_range(&appgtt->vm,
 							   vma->node.start,
 							   vma->size);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 98efc6b54348..a5567b126898 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -115,6 +115,69 @@ static void __i915_vma_retire(struct i915_active *ref)
 	i915_gem_object_put(obj); /* and drop the active reference */
 }
 
+static void do_async_bind(struct work_struct *work)
+{
+	struct i915_vma *vma = container_of(work, typeof(*vma), async.work);
+	int err;
+
+	if (vma->async.dma->error)
+		goto out;
+
+	err = vma->ops->bind_vma(vma,
+				 vma->async.cache_level,
+				 vma->async.flags);
+	if (err)
+		dma_fence_set_error(vma->async.dma, err);
+
+out:
+	dma_fence_signal(vma->async.dma);
+	i915_vma_put(vma);
+}
+
+static void __queue_async_bind(struct dma_fence *f, struct dma_fence_cb *cb)
+{
+	struct i915_vma *vma = container_of(cb, typeof(*vma), async.cb);
+
+	if (f->error)
+		dma_fence_set_error(vma->async.dma, f->error);
+
+	INIT_WORK(&vma->async.work, do_async_bind);
+	queue_work(system_unbound_wq, &vma->async.work);
+}
+
+static const char *async_bind_driver_name(struct dma_fence *fence)
+{
+	return DRIVER_NAME;
+}
+
+static const char *async_bind_timeline_name(struct dma_fence *fence)
+{
+	return "bind";
+}
+
+static const struct dma_fence_ops async_bind_ops = {
+	.get_driver_name = async_bind_driver_name,
+	.get_timeline_name = async_bind_timeline_name,
+};
+
+static DEFINE_SPINLOCK(async_lock);
+
+static struct dma_fence *async_fence_create(struct drm_i915_private *i915)
+{
+	struct dma_fence *f;
+
+	f = kmalloc(sizeof(*f), GFP_KERNEL);
+	if (f) {
+		dma_fence_init(f,
+			       &async_bind_ops,
+			       &async_lock,
+			       i915->mm.unordered_timeline,
+			       0);
+	}
+
+	return f;
+}
+
 static struct i915_vma *
 vma_create(struct drm_i915_gem_object *obj,
 	   struct i915_address_space *vm,
@@ -130,9 +193,6 @@ vma_create(struct drm_i915_gem_object *obj,
 	if (vma == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	i915_active_init(vm->i915, &vma->active, __i915_vma_retire);
-	INIT_ACTIVE_REQUEST(&vma->last_fence);
-
 	vma->vm = vm;
 	vma->ops = &vm->vma_ops;
 	vma->obj = obj;
@@ -142,6 +202,11 @@ vma_create(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&vma->closed_link);
 
+	i915_active_init(vm->i915, &vma->active, __i915_vma_retire);
+	INIT_ACTIVE_REQUEST(&vma->last_fence);
+
+	vma->async.dma = async_fence_create(vm->i915);
+
 	if (view && view->type != I915_GGTT_VIEW_NORMAL) {
 		vma->ggtt_view = *view;
 		if (view->type == I915_GGTT_VIEW_PARTIAL) {
@@ -304,6 +369,38 @@ i915_vma_instance(struct drm_i915_gem_object *obj,
 	return vma;
 }
 
+static int queue_async_bind(struct i915_vma *vma,
+			    enum i915_cache_level cache_level,
+			    u32 flags)
+{
+	bool ready = true;
+
+	if (!vma->async.dma)
+		return -ENOMEM;
+
+	i915_vma_get(vma);
+
+	vma->async.cache_level = cache_level;
+	vma->async.flags = flags;
+
+	i915_vma_lock(vma);
+	if (rcu_access_pointer(vma->resv->fence_excl)) { /* async pages */
+		struct dma_fence *f = reservation_object_get_excl(vma->resv);
+
+		if (!dma_fence_add_callback(f,
+					    &vma->async.cb,
+					    __queue_async_bind))
+			ready = false;
+	}
+	reservation_object_add_excl_fence(vma->resv, vma->async.dma);
+	i915_vma_unlock(vma);
+
+	if (ready)
+		__queue_async_bind(vma->async.dma, &vma->async.cb);
+
+	return 0;
+}
+
 /**
  * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space.
  * @vma: VMA to map
@@ -321,17 +418,12 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
 	u32 vma_flags;
 	int ret;
 
+	GEM_BUG_ON(!flags);
 	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(vma->size > vma->node.size);
-
-	if (GEM_DEBUG_WARN_ON(range_overflows(vma->node.start,
-					      vma->node.size,
-					      vma->vm->total)))
-		return -ENODEV;
-
-	if (GEM_DEBUG_WARN_ON(!flags))
-		return -EINVAL;
-
+	GEM_BUG_ON(range_overflows(vma->node.start,
+				   vma->node.size,
+				   vma->vm->total));
 	bind_flags = 0;
 	if (flags & PIN_GLOBAL)
 		bind_flags |= I915_VMA_GLOBAL_BIND;
@@ -348,12 +440,18 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
 
 	GEM_BUG_ON(!vma->pages);
 
+	if ((bind_flags & ~vma_flags) & I915_VMA_LOCAL_BIND)
+		bind_flags |= I915_VMA_ALLOC_BIND;
+
 	trace_i915_vma_bind(vma, bind_flags);
-	ret = vma->ops->bind_vma(vma, cache_level, bind_flags);
+	if (bind_flags & I915_VMA_ALLOC_BIND)
+		ret = queue_async_bind(vma, cache_level, bind_flags);
+	else
+		ret = vma->ops->bind_vma(vma, cache_level, bind_flags);
 	if (ret)
 		return ret;
 
-	vma->flags |= bind_flags;
+	vma->flags |= bind_flags & ~I915_VMA_ALLOC_BIND;
 	return 0;
 }
 
@@ -597,7 +695,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 	}
 
 	if (vma->obj) {
-		ret = i915_gem_object_pin_pages(vma->obj);
+		ret = i915_gem_object_pin_pages_async(vma->obj);
 		if (ret)
 			return ret;
 
@@ -839,6 +937,7 @@ static void __i915_vma_destroy(struct i915_vma *vma)
 		spin_unlock(&obj->vma.lock);
 	}
 
+	dma_fence_put(vma->async.dma);
 	i915_active_fini(&vma->active);
 
 	i915_vma_free(vma);
@@ -979,12 +1078,17 @@ int i915_vma_unbind(struct i915_vma *vma)
 	int ret;
 
 	lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
+	might_sleep();
+
+	if (vma->flags & I915_VMA_LOCAL_BIND &&
+	    dma_fence_wait_timeout(vma->async.dma, true,
+				   MAX_SCHEDULE_TIMEOUT) < 0)
+		return -EINTR;
 
 	/*
 	 * First wait upon any activity as retiring the request may
 	 * have side-effects such as unpinning or even unbinding this vma.
 	 */
-	might_sleep();
 	if (i915_vma_is_active(vma)) {
 		/*
 		 * When a closed VMA is retired, it is unbound - eek.
@@ -1052,6 +1156,9 @@ int i915_vma_unbind(struct i915_vma *vma)
 	}
 	vma->flags &= ~(I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND);
 
+	dma_fence_put(vma->async.dma);
+	vma->async.dma = async_fence_create(vma->vm->i915);
+
 	i915_vma_remove(vma);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 07ed81ed898b..47cbb9713d73 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -101,6 +101,7 @@ struct i915_vma {
 #define I915_VMA_GLOBAL_BIND	BIT(9)
 #define I915_VMA_LOCAL_BIND	BIT(10)
 #define I915_VMA_BIND_MASK (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND | I915_VMA_PIN_OVERFLOW)
+#define I915_VMA_ALLOC_BIND	I915_VMA_PIN_OVERFLOW /* not stored */
 
 #define I915_VMA_GGTT		BIT(11)
 #define I915_VMA_CAN_FENCE	BIT(12)
@@ -142,6 +143,14 @@ struct i915_vma {
 	unsigned int *exec_flags;
 	struct hlist_node exec_node;
 	u32 exec_handle;
+
+	struct i915_vma_async_bind {
+		struct dma_fence *dma;
+		struct dma_fence_cb cb;
+		struct work_struct work;
+		unsigned int cache_level;
+		unsigned int flags;
+	} async;
 };
 
 struct i915_vma *
diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
index 813ff74c6029..eaaeb911fc65 100644
--- a/drivers/gpu/drm/i915/selftests/i915_vma.c
+++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
@@ -204,8 +204,10 @@ static int igt_vma_create(void *arg)
 		mock_context_close(ctx);
 	}
 
-	list_for_each_entry_safe(obj, on, &objects, st_link)
+	list_for_each_entry_safe(obj, on, &objects, st_link) {
+		i915_gem_object_wait(obj, I915_WAIT_ALL, MAX_SCHEDULE_TIMEOUT);
 		i915_gem_object_put(obj);
+	}
 	return err;
 }
 
-- 
2.20.1



More information about the Intel-gfx-trybot mailing list