[PATCH 21/23] drm/i915: Allow vma binding to occur asynchronously

Chris Wilson chris at chris-wilson.co.uk
Sun Jun 2 08:47:40 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               | 143 ++++++++++++++++--
 drivers/gpu/drm/i915/i915_vma.h               |   9 ++
 drivers/gpu/drm/i915/selftests/i915_vma.c     |   4 +-
 6 files changed, 166 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 528eea44dccf..f065a4648c20 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 852de3836393..d4e1d6a3c67e 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -295,6 +295,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 da936db9a3f3..76dec8fd71a4 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>
 
@@ -139,14 +140,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;
 	}
@@ -156,14 +157,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)
@@ -2691,6 +2694,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;
 }
@@ -2720,7 +2724,7 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
 	if (flags & I915_VMA_LOCAL_BIND) {
 		struct i915_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 d399a8d391e1..4cfcf88c3bef 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -116,6 +116,71 @@ 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);
+	struct i915_address_space *vm = vma->vm;
+	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);
+	i915_vm_put(vm);
+}
+
+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,
@@ -131,9 +196,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;
@@ -143,6 +205,9 @@ 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);
+
 	if (view && view->type != I915_GGTT_VIEW_NORMAL) {
 		vma->ggtt_view = *view;
 		if (view->type == I915_GGTT_VIEW_PARTIAL) {
@@ -305,6 +370,39 @@ 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);
+	i915_vm_get(vma->vm);
+
+	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
@@ -322,17 +420,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;
@@ -347,14 +440,24 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
 	if (bind_flags == 0)
 		return 0;
 
+	vma->async.dma = async_fence_create(vma->vm->i915);
+	if (!vma->async.dma)
+		return -ENOMEM;
+
 	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;
 }
 
@@ -598,7 +701,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;
 
@@ -830,6 +933,7 @@ void i915_vma_reopen(struct i915_vma *vma)
 
 static void __i915_vma_destroy(struct i915_vma *vma)
 {
+	GEM_BUG_ON(vma->async.dma);
 	GEM_BUG_ON(vma->node.allocated);
 	GEM_BUG_ON(vma->fence);
 
@@ -988,12 +1092,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.
@@ -1061,6 +1170,8 @@ int i915_vma_unbind(struct i915_vma *vma)
 	}
 	vma->flags &= ~(I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND);
 
+	dma_fence_put(fetch_and_zero(&vma->async.dma));
+
 	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 630669f1c866..a6d97db21ae4 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