All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/10] drm/i915: Use 0 for the unordered context
@ 2019-08-19 16:44 Chris Wilson
  2019-08-19 16:44 ` [PATCH 02/10] drm/i915: Generalise the clflush dma-worker Chris Wilson
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

Since commit 078dec3326e2 ("dma-buf: add dma_fence_get_stub") the 0
fence context became an impossible match as it is used for an always
signaled fence. We can simplify our timeline tracking by knowing that 0
always means no match.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_clflush.c    | 3 +--
 drivers/gpu/drm/i915/gem/i915_gem_client_blt.c | 7 +------
 drivers/gpu/drm/i915/gem/i915_gem_fence.c      | 3 +--
 drivers/gpu/drm/i915/i915_drv.h                | 2 --
 drivers/gpu/drm/i915/i915_gem.c                | 2 --
 drivers/gpu/drm/i915/i915_request.c            | 4 ++--
 6 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
index 724242ec3972..fb0ef176ba5b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
@@ -134,8 +134,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 		dma_fence_init(&clflush->dma,
 			       &i915_clflush_ops,
 			       &clflush_lock,
-			       to_i915(obj->base.dev)->mm.unordered_timeline,
-			       0);
+			       0, 0);
 		i915_sw_fence_init(&clflush->wait, i915_clflush_notify);
 
 		clflush->obj = i915_gem_object_get(obj);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
index 818ac6915bc5..f99920652751 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_client_blt.c
@@ -267,7 +267,6 @@ int i915_gem_schedule_fill_pages_blt(struct drm_i915_gem_object *obj,
 				     struct i915_page_sizes *page_sizes,
 				     u32 value)
 {
-	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	struct clear_pages_work *work;
 	struct i915_sleeve *sleeve;
 	int err;
@@ -290,11 +289,7 @@ int i915_gem_schedule_fill_pages_blt(struct drm_i915_gem_object *obj,
 
 	init_irq_work(&work->irq_work, clear_pages_signal_irq_worker);
 
-	dma_fence_init(&work->dma,
-		       &clear_pages_work_ops,
-		       &fence_lock,
-		       i915->mm.unordered_timeline,
-		       0);
+	dma_fence_init(&work->dma, &clear_pages_work_ops, &fence_lock, 0, 0);
 	i915_sw_fence_init(&work->wait, clear_pages_work_notify);
 
 	i915_gem_object_lock(obj);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_fence.c b/drivers/gpu/drm/i915/gem/i915_gem_fence.c
index 5496f33a9064..2f6100ec2608 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_fence.c
@@ -69,8 +69,7 @@ i915_gem_object_lock_fence(struct drm_i915_gem_object *obj)
 
 	i915_sw_fence_init(&stub->chain, stub_notify);
 	dma_fence_init(&stub->dma, &stub_fence_ops, &stub->chain.wait.lock,
-		       to_i915(obj->base.dev)->mm.unordered_timeline,
-		       0);
+		       0, 0);
 
 	if (i915_sw_fence_await_reservation(&stub->chain,
 					    obj->base.resv, NULL,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0f81ced0a062..b6032af904bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -748,8 +748,6 @@ struct i915_gem_mm {
 	 */
 	struct workqueue_struct *userptr_wq;
 
-	u64 unordered_timeline;
-
 	/** Bit 6 swizzling required for X tiling */
 	u32 bit_6_swizzle_x;
 	/** Bit 6 swizzling required for Y tiling */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cdf2dbf84aa3..22021da28239 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1411,8 +1411,6 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 		mkwrite_device_info(dev_priv)->page_sizes =
 			I915_GTT_PAGE_SIZE_4K;
 
-	dev_priv->mm.unordered_timeline = dma_fence_context_alloc(1);
-
 	intel_timelines_init(dev_priv);
 
 	ret = i915_gem_init_userptr(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 8fe37ab30d90..86c8dd44216b 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -915,7 +915,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence)
 			continue;
 
 		/* Squash repeated waits to the same timelines */
-		if (fence->context != rq->i915->mm.unordered_timeline &&
+		if (fence->context &&
 		    intel_timeline_sync_is_later(rq->timeline, fence))
 			continue;
 
@@ -929,7 +929,7 @@ i915_request_await_dma_fence(struct i915_request *rq, struct dma_fence *fence)
 			return ret;
 
 		/* Record the latest fence used against each timeline */
-		if (fence->context != rq->i915->mm.unordered_timeline)
+		if (fence->context)
 			intel_timeline_sync_set(rq->timeline, fence);
 	} while (--nchild);
 
-- 
2.23.0.rc1

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

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

* [PATCH 02/10] drm/i915: Generalise the clflush dma-worker
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 18:16   ` Matthew Auld
  2019-08-19 16:44 ` [PATCH 03/10] drm/i915: Assume exclusive access to objects inside resume Chris Wilson
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

Extract the dma-fence worker used by clflush for wider use, as we
anticipate using workers coupled to dma-fences more frequently.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Makefile               |   1 +
 drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 117 +++++++-------------
 drivers/gpu/drm/i915/i915_sw_fence_work.c   |  97 ++++++++++++++++
 drivers/gpu/drm/i915/i915_sw_fence_work.h   |  38 +++++++
 4 files changed, 173 insertions(+), 80 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_sw_fence_work.c
 create mode 100644 drivers/gpu/drm/i915/i915_sw_fence_work.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 45add812048b..658b930d34a8 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -62,6 +62,7 @@ i915-y += \
 	i915_memcpy.o \
 	i915_mm.o \
 	i915_sw_fence.o \
+	i915_sw_fence_work.o \
 	i915_syncmap.o \
 	i915_user_extensions.o
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
index fb0ef176ba5b..91f0a61daf99 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
@@ -8,88 +8,61 @@
 
 #include "i915_drv.h"
 #include "i915_gem_clflush.h"
+#include "i915_sw_fence_work.h"
 #include "i915_trace.h"
 
-static DEFINE_SPINLOCK(clflush_lock);
-
 struct clflush {
-	struct dma_fence dma; /* Must be first for dma_fence_free() */
-	struct i915_sw_fence wait;
-	struct work_struct work;
+	struct dma_fence_work base;
 	struct drm_i915_gem_object *obj;
 };
 
-static const char *i915_clflush_get_driver_name(struct dma_fence *fence)
-{
-	return DRIVER_NAME;
-}
-
-static const char *i915_clflush_get_timeline_name(struct dma_fence *fence)
-{
-	return "clflush";
-}
-
-static void i915_clflush_release(struct dma_fence *fence)
-{
-	struct clflush *clflush = container_of(fence, typeof(*clflush), dma);
-
-	i915_sw_fence_fini(&clflush->wait);
-
-	BUILD_BUG_ON(offsetof(typeof(*clflush), dma));
-	dma_fence_free(&clflush->dma);
-}
-
-static const struct dma_fence_ops i915_clflush_ops = {
-	.get_driver_name = i915_clflush_get_driver_name,
-	.get_timeline_name = i915_clflush_get_timeline_name,
-	.release = i915_clflush_release,
-};
-
-static void __i915_do_clflush(struct drm_i915_gem_object *obj)
+static void __do_clflush(struct drm_i915_gem_object *obj)
 {
 	GEM_BUG_ON(!i915_gem_object_has_pages(obj));
 	drm_clflush_sg(obj->mm.pages);
 	intel_frontbuffer_flush(obj->frontbuffer, ORIGIN_CPU);
 }
 
-static void i915_clflush_work(struct work_struct *work)
+static int clflush_work(struct dma_fence_work *base)
 {
-	struct clflush *clflush = container_of(work, typeof(*clflush), work);
-	struct drm_i915_gem_object *obj = clflush->obj;
+	struct clflush *clflush = container_of(base, typeof(*clflush), base);
+	struct drm_i915_gem_object *obj = fetch_and_zero(&clflush->obj);
+	int err;
 
-	if (i915_gem_object_pin_pages(obj)) {
-		DRM_ERROR("Failed to acquire obj->pages for clflushing\n");
-		goto out;
-	}
-
-	__i915_do_clflush(obj);
+	err = i915_gem_object_pin_pages(obj);
+	if (err)
+		goto put;
 
+	__do_clflush(obj);
 	i915_gem_object_unpin_pages(obj);
 
-out:
+put:
 	i915_gem_object_put(obj);
+	return err;
+}
 
-	dma_fence_signal(&clflush->dma);
-	dma_fence_put(&clflush->dma);
+static void clflush_release(struct dma_fence_work *base)
+{
+	struct clflush *clflush = container_of(base, typeof(*clflush), base);
+
+	if (clflush->obj)
+		i915_gem_object_put(clflush->obj);
 }
 
-static int __i915_sw_fence_call
-i915_clflush_notify(struct i915_sw_fence *fence,
-		    enum i915_sw_fence_notify state)
+static struct clflush *clflush_work_create(struct drm_i915_gem_object *obj)
 {
-	struct clflush *clflush = container_of(fence, typeof(*clflush), wait);
+	struct clflush *clflush;
 
-	switch (state) {
-	case FENCE_COMPLETE:
-		schedule_work(&clflush->work);
-		break;
+	GEM_BUG_ON(!obj->cache_dirty);
 
-	case FENCE_FREE:
-		dma_fence_put(&clflush->dma);
-		break;
-	}
+	clflush = kmalloc(sizeof(*clflush), GFP_KERNEL);
+	if (!clflush)
+		return NULL;
 
-	return NOTIFY_DONE;
+	dma_fence_work_init(&clflush->base, clflush_work, clflush_release);
+	clflush->obj = i915_gem_object_get(obj); /* obj <-> clflush cycle */
+
+	return clflush;
 }
 
 bool i915_gem_clflush_object(struct drm_i915_gem_object *obj,
@@ -127,32 +100,16 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj,
 
 	clflush = NULL;
 	if (!(flags & I915_CLFLUSH_SYNC))
-		clflush = kmalloc(sizeof(*clflush), GFP_KERNEL);
+		clflush = clflush_work_create(obj);
 	if (clflush) {
-		GEM_BUG_ON(!obj->cache_dirty);
-
-		dma_fence_init(&clflush->dma,
-			       &i915_clflush_ops,
-			       &clflush_lock,
-			       0, 0);
-		i915_sw_fence_init(&clflush->wait, i915_clflush_notify);
-
-		clflush->obj = i915_gem_object_get(obj);
-		INIT_WORK(&clflush->work, i915_clflush_work);
-
-		dma_fence_get(&clflush->dma);
-
-		i915_sw_fence_await_reservation(&clflush->wait,
-						obj->base.resv, NULL,
-						true, I915_FENCE_TIMEOUT,
+		i915_sw_fence_await_reservation(&clflush->base.chain,
+						obj->base.resv, NULL, true,
+						I915_FENCE_TIMEOUT,
 						I915_FENCE_GFP);
-
-		dma_resv_add_excl_fence(obj->base.resv,
-						  &clflush->dma);
-
-		i915_sw_fence_commit(&clflush->wait);
+		dma_resv_add_excl_fence(obj->base.resv, &clflush->base.dma);
+		dma_fence_work_commit(&clflush->base);
 	} else if (obj->mm.pages) {
-		__i915_do_clflush(obj);
+		__do_clflush(obj);
 	} else {
 		GEM_BUG_ON(obj->write_domain != I915_GEM_DOMAIN_CPU);
 	}
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
new file mode 100644
index 000000000000..ccd3a6708225
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: MIT
+
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#include "i915_sw_fence_work.h"
+
+static void fence_work(struct work_struct *work)
+{
+	struct dma_fence_work *f = container_of(work, typeof(*f), work);
+	int err;
+
+	err = f->fn(f);
+	if (err)
+		dma_fence_set_error(&f->dma, err);
+	dma_fence_signal(&f->dma);
+	dma_fence_put(&f->dma);
+}
+
+static int __i915_sw_fence_call
+fence_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
+{
+	struct dma_fence_work *f = container_of(fence, typeof(*f), chain);
+
+	switch (state) {
+	case FENCE_COMPLETE:
+		if (fence->error) {
+			dma_fence_set_error(&f->dma, fence->error);
+			f->fn = NULL;
+		}
+
+		if (f->fn) {
+			dma_fence_get(&f->dma);
+			queue_work(system_unbound_wq, &f->work);
+		} else {
+			dma_fence_signal(&f->dma);
+		}
+		break;
+
+	case FENCE_FREE:
+		dma_fence_put(&f->dma);
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static const char *get_driver_name(struct dma_fence *fence)
+{
+	return "dma-fence";
+}
+
+static const char *get_timeline_name(struct dma_fence *fence)
+{
+	return "work";
+}
+
+static void fence_release(struct dma_fence *fence)
+{
+	struct dma_fence_work *f = container_of(fence, typeof(*f), dma);
+
+	if (f->release)
+		f->release(f);
+
+	i915_sw_fence_fini(&f->chain);
+
+	BUILD_BUG_ON(offsetof(typeof(*f), dma));
+	dma_fence_free(&f->dma);
+}
+
+static const struct dma_fence_ops fence_ops = {
+	.get_driver_name = get_driver_name,
+	.get_timeline_name = get_timeline_name,
+	.release = fence_release,
+};
+
+void dma_fence_work_init(struct dma_fence_work *f,
+			 int (*fn)(struct dma_fence_work *f),
+			 void (*release)(struct dma_fence_work *f))
+{
+	spin_lock_init(&f->lock);
+	dma_fence_init(&f->dma, &fence_ops, &f->lock, 0, 0);
+	i915_sw_fence_init(&f->chain, fence_notify);
+	INIT_WORK(&f->work, fence_work);
+
+	f->fn = fn;
+	f->release = release;
+}
+
+int dma_fence_work_chain(struct dma_fence_work *f, struct dma_fence *signal)
+{
+	if (!signal)
+		return 0;
+
+	return __i915_sw_fence_await_dma_fence(&f->chain, signal, &f->cb);
+}
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.h b/drivers/gpu/drm/i915/i915_sw_fence_work.h
new file mode 100644
index 000000000000..22f3eaf091aa
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef I915_SW_FENCE_WORK_H
+#define I915_SW_FENCE_WORK_H
+
+#include <linux/dma-fence.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+#include "i915_sw_fence.h"
+
+struct dma_fence_work {
+	struct dma_fence dma;
+	spinlock_t lock;
+
+	struct i915_sw_fence chain;
+	struct i915_sw_dma_fence_cb cb;
+
+	struct work_struct work;
+	int (*fn)(struct dma_fence_work *f);
+	void (*release)(struct dma_fence_work *f);
+};
+
+void dma_fence_work_init(struct dma_fence_work *f,
+			 int (*fn)(struct dma_fence_work *f),
+			 void (*release)(struct dma_fence_work *f));
+int dma_fence_work_chain(struct dma_fence_work *f, struct dma_fence *signal);
+
+static inline void dma_fence_work_commit(struct dma_fence_work *f)
+{
+	i915_sw_fence_commit(&f->chain);
+}
+
+#endif /* I915_SW_FENCE_WORK_H */
-- 
2.23.0.rc1

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

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

* [PATCH 03/10] drm/i915: Assume exclusive access to objects inside resume
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
  2019-08-19 16:44 ` [PATCH 02/10] drm/i915: Generalise the clflush dma-worker Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 19:57   ` Matthew Auld
  2019-08-19 16:44 ` [PATCH 04/10] drm/i915: Replace PIN_NONFAULT with calls to PIN_NOEVICT Chris Wilson
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

Inside gtt_restore_mappings() we currently take the obj->resv->lock, but
in the future we need to avoid taking this fs-reclaim tainted lock as we
need to extend the coverage of the vm->mutex. Take advantage of the
single-threaded nature of the early resume phase, and do a single
wbinvd() to flush all the GTT objects en masse.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5413c2ff51a2..e48df11a19fb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3274,6 +3274,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt)
 static void ggtt_restore_mappings(struct i915_ggtt *ggtt)
 {
 	struct i915_vma *vma, *vn;
+	bool flush = false;
 
 	intel_gt_check_and_clear_faults(ggtt->vm.gt);
 
@@ -3298,10 +3299,9 @@ static void ggtt_restore_mappings(struct i915_ggtt *ggtt)
 		WARN_ON(i915_vma_bind(vma,
 				      obj ? obj->cache_level : 0,
 				      PIN_UPDATE));
-		if (obj) {
-			i915_gem_object_lock(obj);
-			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
-			i915_gem_object_unlock(obj);
+		if (obj) { /* only used during resume => exclusive access */
+			flush |= fetch_and_zero(&obj->write_domain);
+			obj->read_domains |= I915_GEM_DOMAIN_GTT;
 		}
 
 lock:
@@ -3312,6 +3312,9 @@ static void ggtt_restore_mappings(struct i915_ggtt *ggtt)
 	ggtt->invalidate(ggtt);
 
 	mutex_unlock(&ggtt->vm.mutex);
+
+	if (flush)
+		wbinvd_on_all_cpus();
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *i915)
-- 
2.23.0.rc1

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

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

* [PATCH 04/10] drm/i915: Replace PIN_NONFAULT with calls to PIN_NOEVICT
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
  2019-08-19 16:44 ` [PATCH 02/10] drm/i915: Generalise the clflush dma-worker Chris Wilson
  2019-08-19 16:44 ` [PATCH 03/10] drm/i915: Assume exclusive access to objects inside resume Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 16:44 ` [PATCH 05/10] drm/i915: Track ggtt fence reservations under its own mutex Chris Wilson
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

When under severe stress for GTT mappable space, the LRU eviction model
falls off a cliff. We spend all our time scanning the much large
non-mappable vma searching for something within the mappable zone we can
evict. Turn this on its head by only using the full vma if it is already
pinned in the mappable zone or there is sufficient *free* space to
accommodate it(prioritizing speedy reuse). If there is not, immediately
fall back to using small chunks (tilerow for GTT mmap, single pages for
pwrite/relocation) and using random eviction before doing a full search.

Testcase: igt/gem_concurrent_blt
References: https://bugs.freedesktop.org/show_bug.cgi?id=110848
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 4 ++--
 drivers/gpu/drm/i915/gem/i915_gem_mman.c       | 8 ++++----
 drivers/gpu/drm/i915/i915_gem.c                | 8 ++++----
 drivers/gpu/drm/i915/i915_gem_evict.c          | 8 --------
 drivers/gpu/drm/i915/i915_gem_gtt.c            | 6 +++++-
 drivers/gpu/drm/i915/i915_gem_gtt.h            | 6 +++---
 6 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 53752006d307..3be33271b5f6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1035,8 +1035,8 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 
 		vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
 					       PIN_MAPPABLE |
-					       PIN_NONBLOCK |
-					       PIN_NONFAULT);
+					       PIN_NONBLOCK /* NOWARN */ |
+					       PIN_NOEVICT);
 		if (IS_ERR(vma)) {
 			memset(&cache->node, 0, sizeof(cache->node));
 			err = drm_mm_insert_node_in_range
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 48c2cbe9b278..dba5dd779149 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -264,15 +264,15 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	/* Now pin it into the GTT as needed */
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
 				       PIN_MAPPABLE |
-				       PIN_NONBLOCK |
-				       PIN_NONFAULT);
+				       PIN_NONBLOCK /* NOWARN */ |
+				       PIN_NOSEARCH);
 	if (IS_ERR(vma)) {
 		/* Use a partial view if it is bigger than available space */
 		struct i915_ggtt_view view =
 			compute_partial_view(obj, page_offset, MIN_CHUNK_PAGES);
 		unsigned int flags;
 
-		flags = PIN_MAPPABLE;
+		flags = PIN_MAPPABLE | PIN_NOSEARCH;
 		if (view.type == I915_GGTT_VIEW_NORMAL)
 			flags |= PIN_NONBLOCK; /* avoid warnings for pinned */
 
@@ -282,7 +282,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 		 */
 
 		vma = i915_gem_object_ggtt_pin(obj, &view, 0, 0, flags);
-		if (IS_ERR(vma) && !view.type) {
+		if (IS_ERR(vma)) {
 			flags = PIN_MAPPABLE;
 			view.type = I915_GGTT_VIEW_PARTIAL;
 			vma = i915_gem_object_ggtt_pin(obj, &view, 0, 0, flags);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 22021da28239..68976689d569 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -345,8 +345,8 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
 				       PIN_MAPPABLE |
-				       PIN_NONFAULT |
-				       PIN_NONBLOCK);
+				       PIN_NONBLOCK /* NOWARN */ |
+				       PIN_NOEVICT);
 	if (!IS_ERR(vma)) {
 		node.start = i915_ggtt_offset(vma);
 		node.allocated = false;
@@ -559,8 +559,8 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 
 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
 				       PIN_MAPPABLE |
-				       PIN_NONFAULT |
-				       PIN_NONBLOCK);
+				       PIN_NONBLOCK /* NOWARN */ |
+				       PIN_NOEVICT);
 	if (!IS_ERR(vma)) {
 		node.start = i915_ggtt_offset(vma);
 		node.allocated = false;
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 1796391bc598..52c86c6e0673 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -61,9 +61,6 @@ mark_free(struct drm_mm_scan *scan,
 	if (i915_vma_is_pinned(vma))
 		return false;
 
-	if (flags & PIN_NONFAULT && i915_vma_has_userfault(vma))
-		return false;
-
 	list_add(&vma->evict_link, unwind);
 	return drm_mm_scan_add_block(scan, &vma->node);
 }
@@ -330,11 +327,6 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 			break;
 		}
 
-		if (flags & PIN_NONFAULT && i915_vma_has_userfault(vma)) {
-			ret = -ENOSPC;
-			break;
-		}
-
 		/* Overlap of objects in the same batch? */
 		if (i915_vma_is_pinned(vma)) {
 			ret = -ENOSPC;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index e48df11a19fb..aed61e35969f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3751,7 +3751,8 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
 	if (flags & PIN_NOEVICT)
 		return -ENOSPC;
 
-	/* No free space, pick a slot at random.
+	/*
+	 * No free space, pick a slot at random.
 	 *
 	 * There is a pathological case here using a GTT shared between
 	 * mmap and GPU (i.e. ggtt/aliasing_ppgtt but not full-ppgtt):
@@ -3779,6 +3780,9 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
 	if (err != -ENOSPC)
 		return err;
 
+	if (flags & PIN_NOSEARCH)
+		return -ENOSPC;
+
 	/* Randomly selected placement is pinned, do a search */
 	err = i915_gem_evict_something(vm, size, alignment, color,
 				       start, end, flags);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 51274483502e..b97a47fc7a68 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -600,9 +600,9 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
 			u64 start, u64 end, unsigned int flags);
 
 /* Flags used by pin/bind&friends. */
-#define PIN_NONBLOCK		BIT_ULL(0)
-#define PIN_NONFAULT		BIT_ULL(1)
-#define PIN_NOEVICT		BIT_ULL(2)
+#define PIN_NOEVICT		BIT_ULL(0)
+#define PIN_NOSEARCH		BIT_ULL(1)
+#define PIN_NONBLOCK		BIT_ULL(2)
 #define PIN_MAPPABLE		BIT_ULL(3)
 #define PIN_ZONE_4G		BIT_ULL(4)
 #define PIN_HIGH		BIT_ULL(5)
-- 
2.23.0.rc1

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

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

* [PATCH 05/10] drm/i915: Track ggtt fence reservations under its own mutex
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (2 preceding siblings ...)
  2019-08-19 16:44 ` [PATCH 04/10] drm/i915: Replace PIN_NONFAULT with calls to PIN_NOEVICT Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 16:44 ` [PATCH 06/10] drm/i915: Only track bound elements of the GTT Chris Wilson
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

We can reduce the locking for fence registers from the dev->struct_mutex
to a local mutex. We could introduce a mutex for the sole purpose of
tracking the fence acquisition, except there is a little bit of overlap
with the fault tracking, so use the i915_ggtt.mutex as it covers both.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c |   7 ++
 drivers/gpu/drm/i915/gvt/aperture_gm.c       |  10 +-
 drivers/gpu/drm/i915/i915_debugfs.c          |   5 +-
 drivers/gpu/drm/i915/i915_gem_fence_reg.c    | 108 ++++++++++++-------
 drivers/gpu/drm/i915/i915_gem_fence_reg.h    |   2 +-
 drivers/gpu/drm/i915/i915_vma.h              |   4 +-
 6 files changed, 87 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 298c4d191439..a0098fc35921 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -1157,7 +1157,14 @@ static int evict_fence(void *data)
 		goto out_unlock;
 	}
 
+	err = i915_vma_pin(arg->vma, 0, 0, PIN_GLOBAL | PIN_MAPPABLE);
+	if (err) {
+		pr_err("Unable to pin vma for Y-tiled fence; err:%d\n", err);
+		goto out_unlock;
+	}
+
 	err = i915_vma_pin_fence(arg->vma);
+	i915_vma_unpin(arg->vma);
 	if (err) {
 		pr_err("Unable to pin Y-tiled fence; err:%d\n", err);
 		goto out_unlock;
diff --git a/drivers/gpu/drm/i915/gvt/aperture_gm.c b/drivers/gpu/drm/i915/gvt/aperture_gm.c
index c3d19d88da40..5ff2437b2998 100644
--- a/drivers/gpu/drm/i915/gvt/aperture_gm.c
+++ b/drivers/gpu/drm/i915/gvt/aperture_gm.c
@@ -172,14 +172,14 @@ static void free_vgpu_fence(struct intel_vgpu *vgpu)
 
 	intel_runtime_pm_get(&dev_priv->runtime_pm);
 
-	mutex_lock(&dev_priv->drm.struct_mutex);
+	mutex_lock(&dev_priv->ggtt.vm.mutex);
 	_clear_vgpu_fence(vgpu);
 	for (i = 0; i < vgpu_fence_sz(vgpu); i++) {
 		reg = vgpu->fence.regs[i];
 		i915_unreserve_fence(reg);
 		vgpu->fence.regs[i] = NULL;
 	}
-	mutex_unlock(&dev_priv->drm.struct_mutex);
+	mutex_unlock(&dev_priv->ggtt.vm.mutex);
 
 	intel_runtime_pm_put_unchecked(&dev_priv->runtime_pm);
 }
@@ -195,7 +195,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu)
 	intel_runtime_pm_get(rpm);
 
 	/* Request fences from host */
-	mutex_lock(&dev_priv->drm.struct_mutex);
+	mutex_lock(&dev_priv->ggtt.vm.mutex);
 
 	for (i = 0; i < vgpu_fence_sz(vgpu); i++) {
 		reg = i915_reserve_fence(dev_priv);
@@ -207,7 +207,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu)
 
 	_clear_vgpu_fence(vgpu);
 
-	mutex_unlock(&dev_priv->drm.struct_mutex);
+	mutex_unlock(&dev_priv->ggtt.vm.mutex);
 	intel_runtime_pm_put_unchecked(rpm);
 	return 0;
 out_free_fence:
@@ -220,7 +220,7 @@ static int alloc_vgpu_fence(struct intel_vgpu *vgpu)
 		i915_unreserve_fence(reg);
 		vgpu->fence.regs[i] = NULL;
 	}
-	mutex_unlock(&dev_priv->drm.struct_mutex);
+	mutex_unlock(&dev_priv->ggtt.vm.mutex);
 	intel_runtime_pm_put_unchecked(rpm);
 	return -ENOSPC;
 }
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b39226d7f8d2..f5d6702ec7df 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -652,10 +652,11 @@ static int i915_gem_fence_regs_info(struct seq_file *m, void *data)
 
 	rcu_read_lock();
 	for (i = 0; i < i915->ggtt.num_fences; i++) {
-		struct i915_vma *vma = i915->ggtt.fence_regs[i].vma;
+		struct i915_fence_reg *reg = &i915->ggtt.fence_regs[i];
+		struct i915_vma *vma = reg->vma;
 
 		seq_printf(m, "Fence %d, pin count = %d, object = ",
-			   i, i915->ggtt.fence_regs[i].pin_count);
+			   i, atomic_read(&reg->pin_count));
 		if (!vma)
 			seq_puts(m, "unused");
 		else
diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
index c9654f1a468f..6a33a0bb97a9 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
@@ -299,15 +299,24 @@ static int fence_update(struct i915_fence_reg *fence,
  */
 int i915_vma_put_fence(struct i915_vma *vma)
 {
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
 	struct i915_fence_reg *fence = vma->fence;
+	int err;
 
 	if (!fence)
 		return 0;
 
-	if (fence->pin_count)
+	if (atomic_read(&fence->pin_count))
 		return -EBUSY;
 
-	return fence_update(fence, NULL);
+	err = mutex_lock_interruptible(&ggtt->vm.mutex);
+	if (err)
+		return err;
+
+	err = fence_update(fence, NULL);
+	mutex_unlock(&ggtt->vm.mutex);
+
+	return err;
 }
 
 static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
@@ -317,7 +326,7 @@ static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
 	list_for_each_entry(fence, &i915->ggtt.fence_list, link) {
 		GEM_BUG_ON(fence->vma && fence->vma->fence != fence);
 
-		if (fence->pin_count)
+		if (atomic_read(&fence->pin_count))
 			continue;
 
 		return fence;
@@ -330,6 +339,48 @@ static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
 	return ERR_PTR(-EDEADLK);
 }
 
+static int __i915_vma_pin_fence(struct i915_vma *vma)
+{
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
+	struct i915_fence_reg *fence;
+	struct i915_vma *set = i915_gem_object_is_tiled(vma->obj) ? vma : NULL;
+	int err;
+
+	/* Just update our place in the LRU if our fence is getting reused. */
+	if (vma->fence) {
+		fence = vma->fence;
+		GEM_BUG_ON(fence->vma != vma);
+		atomic_inc(&fence->pin_count);
+		if (!fence->dirty) {
+			list_move_tail(&fence->link, &ggtt->fence_list);
+			return 0;
+		}
+	} else if (set) {
+		fence = fence_find(vma->vm->i915);
+		if (IS_ERR(fence))
+			return PTR_ERR(fence);
+
+		GEM_BUG_ON(atomic_read(&fence->pin_count));
+		atomic_inc(&fence->pin_count);
+	} else {
+		return 0;
+	}
+
+	err = fence_update(fence, set);
+	if (err)
+		goto out_unpin;
+
+	GEM_BUG_ON(fence->vma != set);
+	GEM_BUG_ON(vma->fence != (set ? fence : NULL));
+
+	if (set)
+		return 0;
+
+out_unpin:
+	atomic_dec(&fence->pin_count);
+	return err;
+}
+
 /**
  * i915_vma_pin_fence - set up fencing for a vma
  * @vma: vma to map through a fence reg
@@ -350,8 +401,6 @@ static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
  */
 int i915_vma_pin_fence(struct i915_vma *vma)
 {
-	struct i915_fence_reg *fence;
-	struct i915_vma *set = i915_gem_object_is_tiled(vma->obj) ? vma : NULL;
 	int err;
 
 	/*
@@ -359,39 +408,16 @@ int i915_vma_pin_fence(struct i915_vma *vma)
 	 * must keep the device awake whilst using the fence.
 	 */
 	assert_rpm_wakelock_held(&vma->vm->i915->runtime_pm);
+	GEM_BUG_ON(!i915_vma_is_pinned(vma));
+	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
 
-	/* Just update our place in the LRU if our fence is getting reused. */
-	if (vma->fence) {
-		fence = vma->fence;
-		GEM_BUG_ON(fence->vma != vma);
-		fence->pin_count++;
-		if (!fence->dirty) {
-			list_move_tail(&fence->link,
-				       &fence->i915->ggtt.fence_list);
-			return 0;
-		}
-	} else if (set) {
-		fence = fence_find(vma->vm->i915);
-		if (IS_ERR(fence))
-			return PTR_ERR(fence);
-
-		GEM_BUG_ON(fence->pin_count);
-		fence->pin_count++;
-	} else
-		return 0;
-
-	err = fence_update(fence, set);
+	err = mutex_lock_interruptible(&vma->vm->mutex);
 	if (err)
-		goto out_unpin;
+		return err;
 
-	GEM_BUG_ON(fence->vma != set);
-	GEM_BUG_ON(vma->fence != (set ? fence : NULL));
-
-	if (set)
-		return 0;
+	err = __i915_vma_pin_fence(vma);
+	mutex_unlock(&vma->vm->mutex);
 
-out_unpin:
-	fence->pin_count--;
 	return err;
 }
 
@@ -404,16 +430,17 @@ int i915_vma_pin_fence(struct i915_vma *vma)
  */
 struct i915_fence_reg *i915_reserve_fence(struct drm_i915_private *i915)
 {
+	struct i915_ggtt *ggtt = &i915->ggtt;
 	struct i915_fence_reg *fence;
 	int count;
 	int ret;
 
-	lockdep_assert_held(&i915->drm.struct_mutex);
+	lockdep_assert_held(&ggtt->vm.mutex);
 
 	/* Keep at least one fence available for the display engine. */
 	count = 0;
-	list_for_each_entry(fence, &i915->ggtt.fence_list, link)
-		count += !fence->pin_count;
+	list_for_each_entry(fence, &ggtt->fence_list, link)
+		count += !atomic_read(&fence->pin_count);
 	if (count <= 1)
 		return ERR_PTR(-ENOSPC);
 
@@ -429,6 +456,7 @@ struct i915_fence_reg *i915_reserve_fence(struct drm_i915_private *i915)
 	}
 
 	list_del(&fence->link);
+
 	return fence;
 }
 
@@ -440,9 +468,11 @@ struct i915_fence_reg *i915_reserve_fence(struct drm_i915_private *i915)
  */
 void i915_unreserve_fence(struct i915_fence_reg *fence)
 {
-	lockdep_assert_held(&fence->i915->drm.struct_mutex);
+	struct i915_ggtt *ggtt = &fence->i915->ggtt;
+
+	lockdep_assert_held(&ggtt->vm.mutex);
 
-	list_add(&fence->link, &fence->i915->ggtt.fence_list);
+	list_add(&fence->link, &ggtt->fence_list);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.h b/drivers/gpu/drm/i915/i915_gem_fence_reg.h
index 37e4f104f7c0..99866fb9d94f 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence_reg.h
+++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.h
@@ -41,7 +41,7 @@ struct i915_fence_reg {
 	struct list_head link;
 	struct drm_i915_private *i915;
 	struct i915_vma *vma;
-	int pin_count;
+	atomic_t pin_count;
 	int id;
 	/**
 	 * Whether the tiling parameters for the currently
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index ba1c6a74e64a..eded2646126b 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -418,8 +418,8 @@ int __must_check i915_vma_put_fence(struct i915_vma *vma);
 
 static inline void __i915_vma_unpin_fence(struct i915_vma *vma)
 {
-	GEM_BUG_ON(vma->fence->pin_count <= 0);
-	vma->fence->pin_count--;
+	GEM_BUG_ON(atomic_read(&vma->fence->pin_count) <= 0);
+	atomic_dec(&vma->fence->pin_count);
 }
 
 /**
-- 
2.23.0.rc1

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

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

* [PATCH 06/10] drm/i915: Only track bound elements of the GTT
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (3 preceding siblings ...)
  2019-08-19 16:44 ` [PATCH 05/10] drm/i915: Track ggtt fence reservations under its own mutex Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 16:44 ` [PATCH 07/10] drm/i915: Pull obj->userfault tracking under the ggtt->mutex Chris Wilson
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

The premise here is to simply avoiding having to acquire the vm->mutex
inside vma create/destroy to update the vm->unbound_lists, to avoid some
nasty lock recursions later.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c    |  2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c           | 23 ++++---------------
 drivers/gpu/drm/i915/i915_gem_gtt.h           |  5 ----
 drivers/gpu/drm/i915/i915_vma.c               | 12 ++--------
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  2 +-
 5 files changed, 8 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index aa533b4ab5f5..2e1bfd5e4adf 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -689,7 +689,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
 	__i915_vma_set_map_and_fenceable(vma);
 
 	mutex_lock(&ggtt->vm.mutex);
-	list_move_tail(&vma->vm_link, &ggtt->vm.bound_list);
+	list_add_tail(&vma->vm_link, &ggtt->vm.bound_list);
 	mutex_unlock(&ggtt->vm.mutex);
 
 	GEM_BUG_ON(i915_gem_object_is_shrinkable(obj));
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index aed61e35969f..f599cb3e48f5 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -497,19 +497,12 @@ static void i915_address_space_fini(struct i915_address_space *vm)
 
 static void ppgtt_destroy_vma(struct i915_address_space *vm)
 {
-	struct list_head *phases[] = {
-		&vm->bound_list,
-		&vm->unbound_list,
-		NULL,
-	}, **phase;
+	struct i915_vma *vma, *vn;
 
 	mutex_lock(&vm->i915->drm.struct_mutex);
-	for (phase = phases; *phase; phase++) {
-		struct i915_vma *vma, *vn;
-
-		list_for_each_entry_safe(vma, vn, *phase, vm_link)
-			i915_vma_destroy(vma);
-	}
+	list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link)
+		i915_vma_destroy(vma);
+	GEM_BUG_ON(!list_empty(&vm->bound_list));
 	mutex_unlock(&vm->i915->drm.struct_mutex);
 }
 
@@ -520,9 +513,6 @@ static void __i915_vm_release(struct work_struct *work)
 
 	ppgtt_destroy_vma(vm);
 
-	GEM_BUG_ON(!list_empty(&vm->bound_list));
-	GEM_BUG_ON(!list_empty(&vm->unbound_list));
-
 	vm->cleanup(vm);
 	i915_address_space_fini(vm);
 
@@ -561,7 +551,6 @@ static void i915_address_space_init(struct i915_address_space *vm, int subclass)
 
 	stash_init(&vm->free_pages);
 
-	INIT_LIST_HEAD(&vm->unbound_list);
 	INIT_LIST_HEAD(&vm->bound_list);
 }
 
@@ -1880,10 +1869,6 @@ static struct i915_vma *pd_vma_create(struct gen6_ppgtt *ppgtt, int size)
 	INIT_LIST_HEAD(&vma->obj_link);
 	INIT_LIST_HEAD(&vma->closed_link);
 
-	mutex_lock(&vma->vm->mutex);
-	list_add(&vma->vm_link, &vma->vm->unbound_list);
-	mutex_unlock(&vma->vm->mutex);
-
 	return vma;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index b97a47fc7a68..61bbbfb964b1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -320,11 +320,6 @@ struct i915_address_space {
 	 */
 	struct list_head bound_list;
 
-	/**
-	 * List of vma that are not unbound.
-	 */
-	struct list_head unbound_list;
-
 	struct pagestash free_pages;
 
 	/* Global GTT */
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 73b02234affe..53a075f2cca3 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -219,10 +219,6 @@ vma_create(struct drm_i915_gem_object *obj,
 
 	spin_unlock(&obj->vma.lock);
 
-	mutex_lock(&vm->mutex);
-	list_add(&vma->vm_link, &vm->unbound_list);
-	mutex_unlock(&vm->mutex);
-
 	return vma;
 
 err_vma:
@@ -660,7 +656,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 	GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, cache_level));
 
 	mutex_lock(&vma->vm->mutex);
-	list_move_tail(&vma->vm_link, &vma->vm->bound_list);
+	list_add_tail(&vma->vm_link, &vma->vm->bound_list);
 	mutex_unlock(&vma->vm->mutex);
 
 	if (vma->obj) {
@@ -688,7 +684,7 @@ i915_vma_remove(struct i915_vma *vma)
 
 	mutex_lock(&vma->vm->mutex);
 	drm_mm_remove_node(&vma->node);
-	list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
+	list_del(&vma->vm_link);
 	mutex_unlock(&vma->vm->mutex);
 
 	/*
@@ -801,10 +797,6 @@ static void __i915_vma_destroy(struct i915_vma *vma)
 	GEM_BUG_ON(vma->node.allocated);
 	GEM_BUG_ON(vma->fence);
 
-	mutex_lock(&vma->vm->mutex);
-	list_del(&vma->vm_link);
-	mutex_unlock(&vma->vm->mutex);
-
 	if (vma->obj) {
 		struct drm_i915_gem_object *obj = vma->obj;
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 31a51ca1ddcb..a90c9be95f8c 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1237,7 +1237,7 @@ static void track_vma_bind(struct i915_vma *vma)
 	vma->pages = obj->mm.pages;
 
 	mutex_lock(&vma->vm->mutex);
-	list_move_tail(&vma->vm_link, &vma->vm->bound_list);
+	list_add_tail(&vma->vm_link, &vma->vm->bound_list);
 	mutex_unlock(&vma->vm->mutex);
 }
 
-- 
2.23.0.rc1

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

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

* [PATCH 07/10] drm/i915: Pull obj->userfault tracking under the ggtt->mutex
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (4 preceding siblings ...)
  2019-08-19 16:44 ` [PATCH 06/10] drm/i915: Only track bound elements of the GTT Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 16:44 ` [PATCH 08/10] drm/i915: Replace i915_vma_put_fence() Chris Wilson
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

Since we want to revoke the ggtt vma from only under the ggtt->mutex, we
need to move protection of the userfault tracking from the struct_mutex
to the ggtt->mutex.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 10 +++++++---
 drivers/gpu/drm/i915/i915_debugfs.c      |  2 +-
 drivers/gpu/drm/i915/i915_vma.c          |  4 +++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index dba5dd779149..595539a09e38 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -306,14 +306,17 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
 	if (ret)
 		goto err_fence;
 
-	/* Mark as being mmapped into userspace for later revocation */
 	assert_rpm_wakelock_held(rpm);
+
+	/* Mark as being mmapped into userspace for later revocation */
+	mutex_lock(&i915->ggtt.vm.mutex);
 	if (!i915_vma_set_userfault(vma) && !obj->userfault_count++)
 		list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
+	mutex_unlock(&i915->ggtt.vm.mutex);
+
 	if (CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
 		intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
 				   msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
-	GEM_BUG_ON(!obj->userfault_count);
 
 	i915_vma_set_ggtt_write(vma);
 
@@ -408,8 +411,8 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	 * requirement that operations to the GGTT be made holding the RPM
 	 * wakeref.
 	 */
-	lockdep_assert_held(&i915->drm.struct_mutex);
 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
+	mutex_lock(&i915->ggtt.vm.mutex);
 
 	if (!obj->userfault_count)
 		goto out;
@@ -426,6 +429,7 @@ void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
 	wmb();
 
 out:
+	mutex_unlock(&i915->ggtt.vm.mutex);
 	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f5d6702ec7df..b0f51591f2e4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -94,7 +94,7 @@ static char get_tiling_flag(struct drm_i915_gem_object *obj)
 
 static char get_global_flag(struct drm_i915_gem_object *obj)
 {
-	return obj->userfault_count ? 'g' : ' ';
+	return READ_ONCE(obj->userfault_count) ? 'g' : ' ';
 }
 
 static char get_pin_mapped_flag(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 53a075f2cca3..791cb7990d7d 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -857,7 +857,7 @@ void i915_vma_revoke_mmap(struct i915_vma *vma)
 	struct drm_vma_offset_node *node = &vma->obj->base.vma_node;
 	u64 vma_offset;
 
-	lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
+	lockdep_assert_held(&vma->vm->mutex);
 
 	if (!i915_vma_has_userfault(vma))
 		return;
@@ -980,7 +980,9 @@ int i915_vma_unbind(struct i915_vma *vma)
 			return ret;
 
 		/* Force a pagefault for domain tracking on next user access */
+		mutex_lock(&vma->vm->mutex);
 		i915_vma_revoke_mmap(vma);
+		mutex_unlock(&vma->vm->mutex);
 
 		__i915_vma_iounmap(vma);
 		vma->flags &= ~I915_VMA_CAN_FENCE;
-- 
2.23.0.rc1

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

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

* [PATCH 08/10] drm/i915: Replace i915_vma_put_fence()
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (5 preceding siblings ...)
  2019-08-19 16:44 ` [PATCH 07/10] drm/i915: Pull obj->userfault tracking under the ggtt->mutex Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 16:44 ` [PATCH 09/10] drm/i915: Make shrink/unshrink be atomic Chris Wilson
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

Avoid calling i915_vma_put_fence() by using our alternate paths that
bind a secondary vma avoiding the original fenced vma. For the few
instances where we need to release the fence (i.e. on binding when the
GGTT range becomes invalid), replace the put_fence with a revoke_fence.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/display/intel_overlay.c  |  4 ---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c    | 16 ++++++---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  9 ++---
 drivers/gpu/drm/i915/i915_gem.c               | 36 ++++++++-----------
 drivers/gpu/drm/i915/i915_gem_fence_reg.c     | 16 +++------
 drivers/gpu/drm/i915/i915_vma.c               |  4 ++-
 drivers/gpu/drm/i915/i915_vma.h               |  4 +--
 7 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_overlay.c b/drivers/gpu/drm/i915/display/intel_overlay.c
index eca41c4a5aa6..29edfc343716 100644
--- a/drivers/gpu/drm/i915/display/intel_overlay.c
+++ b/drivers/gpu/drm/i915/display/intel_overlay.c
@@ -770,10 +770,6 @@ static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 	}
 	intel_frontbuffer_flush(new_bo->frontbuffer, ORIGIN_DIRTYFB);
 
-	ret = i915_vma_put_fence(vma);
-	if (ret)
-		goto out_unpin;
-
 	if (!overlay->active) {
 		u32 oconfig;
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index a1afc2690e9e..60134c5aefa7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -221,6 +221,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 	 * state and so involves less work.
 	 */
 	if (atomic_read(&obj->bind_count)) {
+		struct drm_i915_private *i915 = to_i915(obj->base.dev);
+
 		/* Before we change the PTE, the GPU must not be accessing it.
 		 * If we wait upon the object, we know that all the bound
 		 * VMA are no longer active.
@@ -232,8 +234,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 		if (ret)
 			return ret;
 
-		if (!HAS_LLC(to_i915(obj->base.dev)) &&
-		    cache_level != I915_CACHE_NONE) {
+		if (!HAS_LLC(i915) && cache_level != I915_CACHE_NONE) {
 			/* Access to snoopable pages through the GTT is
 			 * incoherent and on some machines causes a hard
 			 * lockup. Relinquish the CPU mmaping to force
@@ -241,6 +242,10 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 			 * then double check if the GTT mapping is still
 			 * valid for that pointer access.
 			 */
+			ret = mutex_lock_interruptible(&i915->ggtt.vm.mutex);
+			if (ret)
+				return ret;
+
 			i915_gem_object_release_mmap(obj);
 
 			/* As we no longer need a fence for GTT access,
@@ -251,10 +256,13 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 			 * supposed to be linear.
 			 */
 			for_each_ggtt_vma(vma, obj) {
-				ret = i915_vma_put_fence(vma);
+				ret = i915_vma_revoke_fence(vma);
 				if (ret)
-					return ret;
+					break;
 			}
+			mutex_unlock(&i915->ggtt.vm.mutex);
+			if (ret)
+				return ret;
 		} else {
 			/* We either have incoherent backing store and
 			 * so no GTT access or the architecture is fully
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 3be33271b5f6..5466a7881d84 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1024,6 +1024,9 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 		struct i915_vma *vma;
 		int err;
 
+		if (i915_gem_object_is_tiled(obj))
+			return ERR_PTR(-EINVAL);
+
 		if (use_cpu_reloc(cache, obj))
 			return NULL;
 
@@ -1047,12 +1050,6 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 			if (err) /* no inactive aperture space, use cpu reloc */
 				return NULL;
 		} else {
-			err = i915_vma_put_fence(vma);
-			if (err) {
-				i915_vma_unpin(vma);
-				return ERR_PTR(err);
-			}
-
 			cache->node.start = vma->node.start;
 			cache->node.mm = (void *)vma;
 		}
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 68976689d569..bef404fa7556 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -343,20 +343,16 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 		return ret;
 
 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
-	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
-				       PIN_MAPPABLE |
-				       PIN_NONBLOCK /* NOWARN */ |
-				       PIN_NOEVICT);
+	vma = ERR_PTR(-ENODEV);
+	if (!i915_gem_object_is_tiled(obj))
+		vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
+					       PIN_MAPPABLE |
+					       PIN_NONBLOCK /* NOWARN */ |
+					       PIN_NOEVICT);
 	if (!IS_ERR(vma)) {
 		node.start = i915_ggtt_offset(vma);
 		node.allocated = false;
-		ret = i915_vma_put_fence(vma);
-		if (ret) {
-			i915_vma_unpin(vma);
-			vma = ERR_PTR(ret);
-		}
-	}
-	if (IS_ERR(vma)) {
+	} else {
 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
 		if (ret)
 			goto out_unlock;
@@ -557,20 +553,16 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 		wakeref = intel_runtime_pm_get(rpm);
 	}
 
-	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
-				       PIN_MAPPABLE |
-				       PIN_NONBLOCK /* NOWARN */ |
-				       PIN_NOEVICT);
+	vma = ERR_PTR(-ENODEV);
+	if (i915_gem_object_is_tiled(obj))
+		vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
+					       PIN_MAPPABLE |
+					       PIN_NONBLOCK /* NOWARN */ |
+					       PIN_NOEVICT);
 	if (!IS_ERR(vma)) {
 		node.start = i915_ggtt_offset(vma);
 		node.allocated = false;
-		ret = i915_vma_put_fence(vma);
-		if (ret) {
-			i915_vma_unpin(vma);
-			vma = ERR_PTR(ret);
-		}
-	}
-	if (IS_ERR(vma)) {
+	} else {
 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
 		if (ret)
 			goto out_rpm;
diff --git a/drivers/gpu/drm/i915/i915_gem_fence_reg.c b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
index 6a33a0bb97a9..615a9f4ef30c 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence_reg.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence_reg.c
@@ -287,7 +287,7 @@ static int fence_update(struct i915_fence_reg *fence,
 }
 
 /**
- * i915_vma_put_fence - force-remove fence for a VMA
+ * i915_vma_revoke_fence - force-remove fence for a VMA
  * @vma: vma to map linearly (not through a fence reg)
  *
  * This function force-removes any fence from the given object, which is useful
@@ -297,26 +297,18 @@ static int fence_update(struct i915_fence_reg *fence,
  *
  * 0 on success, negative error code on failure.
  */
-int i915_vma_put_fence(struct i915_vma *vma)
+int i915_vma_revoke_fence(struct i915_vma *vma)
 {
-	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vma->vm);
 	struct i915_fence_reg *fence = vma->fence;
-	int err;
 
+	lockdep_assert_held(&vma->vm->mutex);
 	if (!fence)
 		return 0;
 
 	if (atomic_read(&fence->pin_count))
 		return -EBUSY;
 
-	err = mutex_lock_interruptible(&ggtt->vm.mutex);
-	if (err)
-		return err;
-
-	err = fence_update(fence, NULL);
-	mutex_unlock(&ggtt->vm.mutex);
-
-	return err;
+	return fence_update(fence, NULL);
 }
 
 static struct i915_fence_reg *fence_find(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 791cb7990d7d..aa1bcd9feabf 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -975,7 +975,9 @@ int i915_vma_unbind(struct i915_vma *vma)
 		GEM_BUG_ON(i915_vma_has_ggtt_write(vma));
 
 		/* release the fence reg _after_ flushing */
-		ret = i915_vma_put_fence(vma);
+		mutex_lock(&vma->vm->mutex);
+		ret = i915_vma_revoke_fence(vma);
+		mutex_unlock(&vma->vm->mutex);
 		if (ret)
 			return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index eded2646126b..910d365d5599 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -413,8 +413,8 @@ static inline struct page *i915_vma_first_page(struct i915_vma *vma)
  *
  * True if the vma has a fence, false otherwise.
  */
-int i915_vma_pin_fence(struct i915_vma *vma);
-int __must_check i915_vma_put_fence(struct i915_vma *vma);
+int __must_check i915_vma_pin_fence(struct i915_vma *vma);
+int __must_check i915_vma_revoke_fence(struct i915_vma *vma);
 
 static inline void __i915_vma_unpin_fence(struct i915_vma *vma)
 {
-- 
2.23.0.rc1

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

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

* [PATCH 09/10] drm/i915: Make shrink/unshrink be atomic
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (6 preceding siblings ...)
  2019-08-19 16:44 ` [PATCH 08/10] drm/i915: Replace i915_vma_put_fence() Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 16:44 ` [PATCH 10/10] drm/i915: Be defensive when starting vma activity Chris Wilson
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

Add an atomic counter and always take the spinlock around the pin/unpin
events, so that we can perform the list manipulation concurrently.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c    |  3 +-
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_pages.c     |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c  | 36 +++++++++++--------
 drivers/gpu/drm/i915/gt/intel_context.c       |  2 +-
 5 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 60134c5aefa7..fbe51f4852ab 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -489,7 +489,8 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
 
 		spin_lock_irqsave(&i915->mm.obj_lock, flags);
 
-		if (obj->mm.madv == I915_MADV_WILLNEED)
+		if (obj->mm.madv == I915_MADV_WILLNEED &&
+		    !atomic_read(&obj->mm.shrink_pin))
 			list_move_tail(&obj->mm.link, &i915->mm.shrink_list);
 
 		spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index ede0eb4218a8..511fce7913ec 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -158,6 +158,7 @@ struct drm_i915_gem_object {
 	struct {
 		struct mutex lock; /* protects the pages and their use */
 		atomic_t pages_pin_count;
+		atomic_t shrink_pin;
 
 		struct sg_table *pages;
 		void *mapping;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 18f0ce0135c1..2e941f093a20 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -71,6 +71,7 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
 			list = &i915->mm.shrink_list;
 		list_add_tail(&obj->mm.link, list);
 
+		atomic_set(&obj->mm.shrink_pin, 0);
 		spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
 	}
 }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index edd21d14e64f..94970f60d7f6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -523,46 +523,52 @@ void i915_gem_shrinker_taints_mutex(struct drm_i915_private *i915,
 
 void i915_gem_object_make_unshrinkable(struct drm_i915_gem_object *obj)
 {
+	struct drm_i915_private *i915 = obj_to_i915(obj);
+	unsigned long flags;
+
 	/*
 	 * We can only be called while the pages are pinned or when
 	 * the pages are released. If pinned, we should only be called
 	 * from a single caller under controlled conditions; and on release
 	 * only one caller may release us. Neither the two may cross.
 	 */
-	if (!list_empty(&obj->mm.link)) { /* pinned by caller */
-		struct drm_i915_private *i915 = obj_to_i915(obj);
-		unsigned long flags;
-
-		spin_lock_irqsave(&i915->mm.obj_lock, flags);
-		GEM_BUG_ON(list_empty(&obj->mm.link));
+	if (atomic_add_unless(&obj->mm.shrink_pin, 1, 0))
+		return;
 
+	spin_lock_irqsave(&i915->mm.obj_lock, flags);
+	if (!atomic_fetch_inc(&obj->mm.shrink_pin) &&
+	    !list_empty(&obj->mm.link)) {
 		list_del_init(&obj->mm.link);
 		i915->mm.shrink_count--;
 		i915->mm.shrink_memory -= obj->base.size;
-
-		spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
 	}
+	spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
 }
 
 static void __i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj,
 					      struct list_head *head)
 {
+	struct drm_i915_private *i915 = obj_to_i915(obj);
+	unsigned long flags;
+
 	GEM_BUG_ON(!i915_gem_object_has_pages(obj));
-	GEM_BUG_ON(!list_empty(&obj->mm.link));
+	if (!i915_gem_object_is_shrinkable(obj))
+		return;
 
-	if (i915_gem_object_is_shrinkable(obj)) {
-		struct drm_i915_private *i915 = obj_to_i915(obj);
-		unsigned long flags;
+	if (atomic_add_unless(&obj->mm.shrink_pin, -1, 1))
+		return;
 
-		spin_lock_irqsave(&i915->mm.obj_lock, flags);
-		GEM_BUG_ON(!kref_read(&obj->base.refcount));
+	spin_lock_irqsave(&i915->mm.obj_lock, flags);
+	GEM_BUG_ON(!kref_read(&obj->base.refcount));
+	if (atomic_dec_and_test(&obj->mm.shrink_pin)) {
+		GEM_BUG_ON(!list_empty(&obj->mm.link));
 
 		list_add_tail(&obj->mm.link, head);
 		i915->mm.shrink_count++;
 		i915->mm.shrink_memory += obj->base.size;
 
-		spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
 	}
+	spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
 }
 
 void i915_gem_object_make_shrinkable(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index f55691d151ae..c0495811f493 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -134,8 +134,8 @@ static int __context_pin_state(struct i915_vma *vma)
 
 static void __context_unpin_state(struct i915_vma *vma)
 {
-	__i915_vma_unpin(vma);
 	i915_vma_make_shrinkable(vma);
+	__i915_vma_unpin(vma);
 }
 
 static void __intel_context_retire(struct i915_active *active)
-- 
2.23.0.rc1

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

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

* [PATCH 10/10] drm/i915: Be defensive when starting vma activity
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (7 preceding siblings ...)
  2019-08-19 16:44 ` [PATCH 09/10] drm/i915: Make shrink/unshrink be atomic Chris Wilson
@ 2019-08-19 16:44 ` Chris Wilson
  2019-08-19 17:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/10] drm/i915: Use 0 for the unordered context Patchwork
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 16:44 UTC (permalink / raw)
  To: intel-gfx

Before we acquire the vma for GPU activity, ensure that the underlying
object is not already in the process of being freed.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_vma.c | 3 +--
 drivers/gpu/drm/i915/i915_vma.h | 8 ++++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index aa1bcd9feabf..8fc2d44c20f5 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -87,8 +87,7 @@ static inline struct i915_vma *active_to_vma(struct i915_active *ref)
 
 static int __i915_vma_active(struct i915_active *ref)
 {
-	i915_vma_get(active_to_vma(ref));
-	return 0;
+	return i915_vma_tryget(active_to_vma(ref)) ? 0 : -ENOENT;
 }
 
 static void __i915_vma_retire(struct i915_active *ref)
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 910d365d5599..889fc7cb910a 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -231,6 +231,14 @@ static inline struct i915_vma *i915_vma_get(struct i915_vma *vma)
 	return vma;
 }
 
+static inline struct i915_vma *i915_vma_tryget(struct i915_vma *vma)
+{
+	if (likely(kref_get_unless_zero(&vma->obj->base.refcount)))
+		return vma;
+
+	return NULL;
+}
+
 static inline void i915_vma_put(struct i915_vma *vma)
 {
 	i915_gem_object_put(vma->obj);
-- 
2.23.0.rc1

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

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

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/10] drm/i915: Use 0 for the unordered context
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (8 preceding siblings ...)
  2019-08-19 16:44 ` [PATCH 10/10] drm/i915: Be defensive when starting vma activity Chris Wilson
@ 2019-08-19 17:00 ` Patchwork
  2019-08-19 17:23 ` ✗ Fi.CI.BAT: failure " Patchwork
  2019-08-19 17:36 ` [PATCH 01/10] " Matthew Auld
  11 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2019-08-19 17:00 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915: Use 0 for the unordered context
URL   : https://patchwork.freedesktop.org/series/65426/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f3745550c94b drm/i915: Use 0 for the unordered context
18f8647117cd drm/i915: Generalise the clflush dma-worker
-:187: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#187: 
new file mode 100644

-:312: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment
#312: FILE: drivers/gpu/drm/i915/i915_sw_fence_work.h:18:
+	spinlock_t lock;

total: 0 errors, 1 warnings, 1 checks, 299 lines checked
f9ed69ac9f3b drm/i915: Assume exclusive access to objects inside resume
dc8c79266acb drm/i915: Replace PIN_NONFAULT with calls to PIN_NOEVICT
e6cace67721a drm/i915: Track ggtt fence reservations under its own mutex
82f065549179 drm/i915: Only track bound elements of the GTT
965f4453c69e drm/i915: Pull obj->userfault tracking under the ggtt->mutex
d0405a9011ab drm/i915: Replace i915_vma_put_fence()
5b7dd0fb5eb9 drm/i915: Make shrink/unshrink be atomic
295f8efb617d drm/i915: Be defensive when starting vma activity

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

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

* ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915: Use 0 for the unordered context
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (9 preceding siblings ...)
  2019-08-19 17:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/10] drm/i915: Use 0 for the unordered context Patchwork
@ 2019-08-19 17:23 ` Patchwork
  2019-08-19 17:29   ` Chris Wilson
  2019-08-19 17:36 ` [PATCH 01/10] " Matthew Auld
  11 siblings, 1 reply; 17+ messages in thread
From: Patchwork @ 2019-08-19 17:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915: Use 0 for the unordered context
URL   : https://patchwork.freedesktop.org/series/65426/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6737 -> Patchwork_14082
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14082 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14082, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14082:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_gem_contexts:
    - fi-byt-j1900:       [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-byt-j1900/igt@i915_selftest@live_gem_contexts.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-byt-j1900/igt@i915_selftest@live_gem_contexts.html
    - fi-ivb-3770:        [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-ivb-3770/igt@i915_selftest@live_gem_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-ivb-3770/igt@i915_selftest@live_gem_contexts.html
    - fi-hsw-4770:        [PASS][5] -> [DMESG-WARN][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-hsw-4770/igt@i915_selftest@live_gem_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-hsw-4770/igt@i915_selftest@live_gem_contexts.html
    - fi-hsw-peppy:       [PASS][7] -> [DMESG-WARN][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
    - fi-hsw-4770r:       [PASS][9] -> [DMESG-WARN][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-hsw-4770r/igt@i915_selftest@live_gem_contexts.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-hsw-4770r/igt@i915_selftest@live_gem_contexts.html
    - fi-byt-n2820:       [PASS][11] -> [DMESG-WARN][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][13] ([fdo#111407]) -> [FAIL][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
Known issues
------------

  Here are the changes found in Patchwork_14082 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@double-flink:
    - fi-icl-u3:          [PASS][15] -> [DMESG-WARN][16] ([fdo#107724])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-icl-u3/igt@gem_flink_basic@double-flink.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-icl-u3/igt@gem_flink_basic@double-flink.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [PASS][17] -> [FAIL][18] ([fdo#110627])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-bxt-j4205:       [PASS][19] -> [FAIL][20] ([fdo#103167])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-bxt-j4205/igt@kms_frontbuffer_tracking@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-bxt-j4205/igt@kms_frontbuffer_tracking@basic.html
    - fi-bsw-kefka:       [PASS][21] -> [FAIL][22] ([fdo#103167])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-bsw-kefka/igt@kms_frontbuffer_tracking@basic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-bsw-kefka/igt@kms_frontbuffer_tracking@basic.html
    - fi-bxt-dsi:         [PASS][23] -> [FAIL][24] ([fdo#103167])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-bxt-dsi/igt@kms_frontbuffer_tracking@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-bxt-dsi/igt@kms_frontbuffer_tracking@basic.html
    - fi-apl-guc:         [PASS][25] -> [FAIL][26] ([fdo#103167])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-apl-guc/igt@kms_frontbuffer_tracking@basic.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-apl-guc/igt@kms_frontbuffer_tracking@basic.html
    - fi-byt-j1900:       [PASS][27] -> [FAIL][28] ([fdo#103167])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-byt-j1900/igt@kms_frontbuffer_tracking@basic.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-byt-j1900/igt@kms_frontbuffer_tracking@basic.html
    - fi-ilk-650:         [PASS][29] -> [FAIL][30] ([fdo#103167])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-ilk-650/igt@kms_frontbuffer_tracking@basic.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-ilk-650/igt@kms_frontbuffer_tracking@basic.html
    - fi-byt-n2820:       [PASS][31] -> [FAIL][32] ([fdo#103167])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-byt-n2820/igt@kms_frontbuffer_tracking@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-byt-n2820/igt@kms_frontbuffer_tracking@basic.html
    - fi-bdw-gvtdvm:      [PASS][33] -> [FAIL][34] ([fdo#103167])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-bdw-gvtdvm/igt@kms_frontbuffer_tracking@basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-bdw-gvtdvm/igt@kms_frontbuffer_tracking@basic.html
    - fi-glk-dsi:         [PASS][35] -> [FAIL][36] ([fdo#103167])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-glk-dsi/igt@kms_frontbuffer_tracking@basic.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-glk-dsi/igt@kms_frontbuffer_tracking@basic.html
    - fi-skl-gvtdvm:      [PASS][37] -> [FAIL][38] ([fdo#103167])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-skl-gvtdvm/igt@kms_frontbuffer_tracking@basic.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-skl-gvtdvm/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          [FAIL][39] ([fdo#103167]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
    - fi-icl-u2:          [FAIL][41] ([fdo#103167]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407


Participating hosts (55 -> 46)
------------------------------

  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6737 -> Patchwork_14082

  CI-20190529: 20190529
  CI_DRM_6737: 7c1b750b342c7ded6d590589393a40590e0bb7ab @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5141: 7102b417fedc2a1ea6f72d768a9f1bd100a34f13 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14082: 295f8efb617d41eea2d749d8d3fcdbb13d415014 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

295f8efb617d drm/i915: Be defensive when starting vma activity
5b7dd0fb5eb9 drm/i915: Make shrink/unshrink be atomic
d0405a9011ab drm/i915: Replace i915_vma_put_fence()
965f4453c69e drm/i915: Pull obj->userfault tracking under the ggtt->mutex
82f065549179 drm/i915: Only track bound elements of the GTT
e6cace67721a drm/i915: Track ggtt fence reservations under its own mutex
dc8c79266acb drm/i915: Replace PIN_NONFAULT with calls to PIN_NOEVICT
f9ed69ac9f3b drm/i915: Assume exclusive access to objects inside resume
18f8647117cd drm/i915: Generalise the clflush dma-worker
f3745550c94b drm/i915: Use 0 for the unordered context

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915: Use 0 for the unordered context
  2019-08-19 17:23 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-08-19 17:29   ` Chris Wilson
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 17:29 UTC (permalink / raw)
  To: Patchwork; +Cc: intel-gfx

Quoting Patchwork (2019-08-19 18:23:51)
> == Series Details ==
> 
> Series: series starting with [01/10] drm/i915: Use 0 for the unordered context
> URL   : https://patchwork.freedesktop.org/series/65426/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_6737 -> Patchwork_14082
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_14082 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_14082, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_14082:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@i915_selftest@live_gem_contexts:
>     - fi-byt-j1900:       [PASS][1] -> [DMESG-WARN][2]
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-byt-j1900/igt@i915_selftest@live_gem_contexts.html
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-byt-j1900/igt@i915_selftest@live_gem_contexts.html
>     - fi-ivb-3770:        [PASS][3] -> [DMESG-WARN][4]
>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-ivb-3770/igt@i915_selftest@live_gem_contexts.html
>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-ivb-3770/igt@i915_selftest@live_gem_contexts.html
>     - fi-hsw-4770:        [PASS][5] -> [DMESG-WARN][6]
>    [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-hsw-4770/igt@i915_selftest@live_gem_contexts.html
>    [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-hsw-4770/igt@i915_selftest@live_gem_contexts.html
>     - fi-hsw-peppy:       [PASS][7] -> [DMESG-WARN][8]
>    [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
>    [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
>     - fi-hsw-4770r:       [PASS][9] -> [DMESG-WARN][10]
>    [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-hsw-4770r/igt@i915_selftest@live_gem_contexts.html
>    [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-hsw-4770r/igt@i915_selftest@live_gem_contexts.html
>     - fi-byt-n2820:       [PASS][11] -> [DMESG-WARN][12]
>    [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6737/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
>    [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14082/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html

Hmm. We haven't removed the struct_mutex around i915_vma_destroy() yet,
so it can't be that. Consider it's only gen7, that suggests it's the
pd_vma. But that's in gen6_ppgtt_cleanup() not the free_objects callers.

Weird. Time for a bisection.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 01/10] drm/i915: Use 0 for the unordered context
  2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
                   ` (10 preceding siblings ...)
  2019-08-19 17:23 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-08-19 17:36 ` Matthew Auld
  11 siblings, 0 replies; 17+ messages in thread
From: Matthew Auld @ 2019-08-19 17:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Intel Graphics Development

On Mon, 19 Aug 2019 at 17:45, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Since commit 078dec3326e2 ("dma-buf: add dma_fence_get_stub") the 0
> fence context became an impossible match as it is used for an always
> signaled fence. We can simplify our timeline tracking by knowing that 0
> always means no match.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
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] 17+ messages in thread

* Re: [PATCH 02/10] drm/i915: Generalise the clflush dma-worker
  2019-08-19 16:44 ` [PATCH 02/10] drm/i915: Generalise the clflush dma-worker Chris Wilson
@ 2019-08-19 18:16   ` Matthew Auld
  2019-08-19 18:32     ` Chris Wilson
  0 siblings, 1 reply; 17+ messages in thread
From: Matthew Auld @ 2019-08-19 18:16 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Intel Graphics Development

On Mon, 19 Aug 2019 at 17:45, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Extract the dma-fence worker used by clflush for wider use, as we
> anticipate using workers coupled to dma-fences more frequently.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<snip>

> +
> +static const char *get_timeline_name(struct dma_fence *fence)
> +{
> +       return "work";
> +}

work->name, or not all that useful for debug?

Anyway,
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] 17+ messages in thread

* Re: [PATCH 02/10] drm/i915: Generalise the clflush dma-worker
  2019-08-19 18:16   ` Matthew Auld
@ 2019-08-19 18:32     ` Chris Wilson
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2019-08-19 18:32 UTC (permalink / raw)
  To: Matthew Auld; +Cc: Intel Graphics Development

Quoting Matthew Auld (2019-08-19 19:16:20)
> On Mon, 19 Aug 2019 at 17:45, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Extract the dma-fence worker used by clflush for wider use, as we
> > anticipate using workers coupled to dma-fences more frequently.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> <snip>
> 
> > +
> > +static const char *get_timeline_name(struct dma_fence *fence)
> > +{
> > +       return "work";
> > +}
> 
> work->name, or not all that useful for debug?

Hmm. work->name feels like overkill. If we could "%pS" work->fn...

That turns out to kallsyms_lookup(), but it takes a namebuf.

With two function pointers, I was thinking we should move to a const
vfunc table, given the various issues with overwriting function points.
We can throw a name in there.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 03/10] drm/i915: Assume exclusive access to objects inside resume
  2019-08-19 16:44 ` [PATCH 03/10] drm/i915: Assume exclusive access to objects inside resume Chris Wilson
@ 2019-08-19 19:57   ` Matthew Auld
  0 siblings, 0 replies; 17+ messages in thread
From: Matthew Auld @ 2019-08-19 19:57 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Intel Graphics Development

On Mon, 19 Aug 2019 at 17:45, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Inside gtt_restore_mappings() we currently take the obj->resv->lock, but
> in the future we need to avoid taking this fs-reclaim tainted lock as we
> need to extend the coverage of the vm->mutex. Take advantage of the
> single-threaded nature of the early resume phase, and do a single
> wbinvd() to flush all the GTT objects en masse.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
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] 17+ messages in thread

end of thread, other threads:[~2019-08-19 19:57 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 16:44 [PATCH 01/10] drm/i915: Use 0 for the unordered context Chris Wilson
2019-08-19 16:44 ` [PATCH 02/10] drm/i915: Generalise the clflush dma-worker Chris Wilson
2019-08-19 18:16   ` Matthew Auld
2019-08-19 18:32     ` Chris Wilson
2019-08-19 16:44 ` [PATCH 03/10] drm/i915: Assume exclusive access to objects inside resume Chris Wilson
2019-08-19 19:57   ` Matthew Auld
2019-08-19 16:44 ` [PATCH 04/10] drm/i915: Replace PIN_NONFAULT with calls to PIN_NOEVICT Chris Wilson
2019-08-19 16:44 ` [PATCH 05/10] drm/i915: Track ggtt fence reservations under its own mutex Chris Wilson
2019-08-19 16:44 ` [PATCH 06/10] drm/i915: Only track bound elements of the GTT Chris Wilson
2019-08-19 16:44 ` [PATCH 07/10] drm/i915: Pull obj->userfault tracking under the ggtt->mutex Chris Wilson
2019-08-19 16:44 ` [PATCH 08/10] drm/i915: Replace i915_vma_put_fence() Chris Wilson
2019-08-19 16:44 ` [PATCH 09/10] drm/i915: Make shrink/unshrink be atomic Chris Wilson
2019-08-19 16:44 ` [PATCH 10/10] drm/i915: Be defensive when starting vma activity Chris Wilson
2019-08-19 17:00 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/10] drm/i915: Use 0 for the unordered context Patchwork
2019-08-19 17:23 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-08-19 17:29   ` Chris Wilson
2019-08-19 17:36 ` [PATCH 01/10] " Matthew Auld

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.