Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Flush pending GTT writes before unbinding
@ 2017-12-05 12:17 Chris Wilson
  2017-12-05 12:17 ` [PATCH 2/2] drm/i915: Track GGTT on the vma Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Chris Wilson @ 2017-12-05 12:17 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson, Joonas Lahtinen, stable

>From the shrinker paths, we want to relinquish the GPU and GGTT access to
the object, releasing the backing storage back to the system for
swapout. As a part of that process we would unpin the pages, marking
them for access by the CPU (for the swapout/swapin). However, if that
process was interrupted after unbind the vma, we missed a flush of the
inflight GGTT writes before we made that GTT space available again for
reuse, with the prospect that we would redirect them to another page.

The bug dates back to the introduction of multiple GGTT vma, but the
code itself dates to commit 02bef8f98d26 ("drm/i915: Unbind closed vma
for i915_gem_object_unbind()").

Fixes: 02bef8f98d26 ("drm/i915: Unbind closed vma for i915_gem_object_unbind()")
Fixes: c5ad54cf7dd8 ("drm/i915: Use partial view in mmap fault handler")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: stable@vger.kernel.org
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e083f242b8dc..80b78fb5daac 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -330,17 +330,10 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 	 * must wait for all rendering to complete to the object (as unbinding
 	 * must anyway), and retire the requests.
 	 */
-	ret = i915_gem_object_wait(obj,
-				   I915_WAIT_INTERRUPTIBLE |
-				   I915_WAIT_LOCKED |
-				   I915_WAIT_ALL,
-				   MAX_SCHEDULE_TIMEOUT,
-				   NULL);
+	ret = i915_gem_object_set_to_cpu_domain(obj, false);
 	if (ret)
 		return ret;
 
-	i915_gem_retire_requests(to_i915(obj->base.dev));
-
 	while ((vma = list_first_entry_or_null(&obj->vma_list,
 					       struct i915_vma,
 					       obj_link))) {
-- 
2.15.1

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

* [PATCH 2/2] drm/i915: Track GGTT on the vma
  2017-12-05 12:17 [PATCH 1/2] drm/i915: Flush pending GTT writes before unbinding Chris Wilson
@ 2017-12-05 12:17 ` Chris Wilson
  2017-12-05 13:12   ` [PATCH v2] drm/i915: Track GGTT writes " Chris Wilson
  2017-12-05 12:55 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-12-05 12:17 UTC (permalink / raw)
  To: intel-gfx

As writes through the GTT and GGTT PTE updates do not share the same
path, they are not strictly ordered and so we must explicitly flush the
indirect writes prior to modifying the PTE. We do track outstanding GGTT
writes on the object itself, but since the object may have multiple GGTT
vma, that is overly coarse as we can track and flush individual vma as
required.

Whilst here, update the GGTT flushing behaviour for Cannonlake.

References: https://bugs.freedesktop.org/show_bug.cgi?id=104002
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_gem.c | 55 +++++++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_vma.c | 22 +++++++++++++++++
 drivers/gpu/drm/i915/i915_vma.h | 19 ++++++++++++++
 4 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 594fd14e66c5..5cf58e049dbd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3879,6 +3879,8 @@ int __must_check i915_gem_evict_for_node(struct i915_address_space *vm,
 					 unsigned int flags);
 int i915_gem_evict_vm(struct i915_address_space *vm);
 
+void i915_gem_flush_ggtt_writes(struct drm_i915_private *dev_priv);
+
 /* belongs in i915_gem_gtt.h */
 static inline void i915_gem_chipset_flush(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 80b78fb5daac..cd8f3d847350 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -666,17 +666,13 @@ fb_write_origin(struct drm_i915_gem_object *obj, unsigned int domain)
 		obj->frontbuffer_ggtt_origin : ORIGIN_CPU);
 }
 
-static void
-flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
+void i915_gem_flush_ggtt_writes(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
-
-	if (!(obj->base.write_domain & flush_domains))
-		return;
-
-	/* No actual flushing is required for the GTT write domain.  Writes
-	 * to it "immediately" go to main memory as far as we know, so there's
-	 * no chipset flush.  It also doesn't land in render cache.
+	/*
+	 * No actual flushing is required for the GTT write domain for reads
+	 * from the GTT domain. Writes to it "immediately" go to main memory
+	 * as far as we know, so there's no chipset flush. It also doesn't
+	 * land in the GPU render cache.
 	 *
 	 * However, we do have to enforce the order so that all writes through
 	 * the GTT land before any writes to the device, such as updates to
@@ -687,22 +683,43 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 	 * timing. This issue has only been observed when switching quickly
 	 * between GTT writes and CPU reads from inside the kernel on recent hw,
 	 * and it appears to only affect discrete GTT blocks (i.e. on LLC
-	 * system agents we cannot reproduce this behaviour).
+	 * system agents we cannot reproduce this behaviour, until Cannonlake
+	 * that was!).
 	 */
+
 	wmb();
 
+	intel_runtime_pm_get(dev_priv);
+	spin_lock_irq(&dev_priv->uncore.lock);
+
+	POSTING_READ_FW(RING_HEAD(dev_priv->engine[RCS]->mmio_base));
+
+	spin_unlock_irq(&dev_priv->uncore.lock);
+	intel_runtime_pm_put(dev_priv);
+}
+
+static void
+flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
+{
+	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+	struct i915_vma *vma;
+
+	if (!(obj->base.write_domain & flush_domains))
+		return;
+
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
-		if (!HAS_LLC(dev_priv)) {
-			intel_runtime_pm_get(dev_priv);
-			spin_lock_irq(&dev_priv->uncore.lock);
-			POSTING_READ_FW(RING_HEAD(dev_priv->engine[RCS]->mmio_base));
-			spin_unlock_irq(&dev_priv->uncore.lock);
-			intel_runtime_pm_put(dev_priv);
-		}
+		i915_gem_flush_ggtt_writes(dev_priv);
 
 		intel_fb_obj_flush(obj,
 				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+
+		list_for_each_entry(vma, &obj->vma_list, obj_link) {
+			if (!i915_vma_is_ggtt(vma))
+				break;
+
+			i915_vma_unset_ggtt_write(vma);
+		}
 		break;
 
 	case I915_GEM_DOMAIN_CPU:
@@ -1965,6 +1982,8 @@ int i915_gem_fault(struct vm_fault *vmf)
 		list_add(&obj->userfault_link, &dev_priv->mm.userfault_list);
 	GEM_BUG_ON(!obj->userfault_count);
 
+	i915_vma_set_ggtt_write(vma);
+
 err_fence:
 	i915_vma_unpin_fence(vma);
 err_unpin:
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index bf6d8d1eaabe..b77715823d34 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -322,6 +322,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 	if (err)
 		goto err_unpin;
 
+	i915_vma_set_ggtt_write(vma);
 	return ptr;
 
 err_unpin:
@@ -330,12 +331,24 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 	return IO_ERR_PTR(err);
 }
 
+void i915_vma_flush_writes(struct i915_vma *vma)
+{
+	if (!i915_vma_has_ggtt_writes(vma))
+		return;
+
+	i915_gem_flush_ggtt_writes(vma->vm->i915);
+
+	i915_vma_unset_ggtt_write(vma);
+}
+
 void i915_vma_unpin_iomap(struct i915_vma *vma)
 {
 	lockdep_assert_held(&vma->obj->base.dev->struct_mutex);
 
 	GEM_BUG_ON(vma->iomap == NULL);
 
+	i915_vma_flush_writes(vma);
+
 	i915_vma_unpin_fence(vma);
 	i915_vma_unpin(vma);
 }
@@ -790,6 +803,15 @@ int i915_vma_unbind(struct i915_vma *vma)
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
 
 	if (i915_vma_is_map_and_fenceable(vma)) {
+		/*
+		 * Check that we have flushed all writes through the GGTT
+		 * before the unbind, other due to non-strict nature of those
+		 * indirect writes they may end up referencing the GGTT PTE
+		 * after the unbind.
+		 */
+		i915_vma_flush_writes(vma);
+		GEM_BUG_ON(i915_vma_has_ggtt_writes(vma));
+
 		/* release the fence reg _after_ flushing */
 		ret = i915_vma_put_fence(vma);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 1e2bc9b3c3ac..de1c51bcc534 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -90,6 +90,7 @@ struct i915_vma {
 #define I915_VMA_CLOSED		BIT(10)
 #define I915_VMA_USERFAULT_BIT	11
 #define I915_VMA_USERFAULT	BIT(I915_VMA_USERFAULT_BIT)
+#define I915_VMA_GGTT_WRITE	BIT(12)
 
 	unsigned int active;
 	struct i915_gem_active last_read[I915_NUM_ENGINES];
@@ -138,6 +139,24 @@ static inline bool i915_vma_is_ggtt(const struct i915_vma *vma)
 	return vma->flags & I915_VMA_GGTT;
 }
 
+static inline bool i915_vma_has_ggtt_writes(const struct i915_vma *vma)
+{
+	return vma->flags & I915_VMA_GGTT_WRITE;
+}
+
+static inline void i915_vma_set_ggtt_write(struct i915_vma *vma)
+{
+	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
+	vma->flags |= I915_VMA_GGTT_WRITE;
+}
+
+static inline void i915_vma_unset_ggtt_write(struct i915_vma *vma)
+{
+	vma->flags &= ~I915_VMA_GGTT_WRITE;
+}
+
+void i915_vma_flush_writes(struct i915_vma *vma);
+
 static inline bool i915_vma_is_map_and_fenceable(const struct i915_vma *vma)
 {
 	return vma->flags & I915_VMA_CAN_FENCE;
-- 
2.15.1

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding
  2017-12-05 12:17 [PATCH 1/2] drm/i915: Flush pending GTT writes before unbinding Chris Wilson
  2017-12-05 12:17 ` [PATCH 2/2] drm/i915: Track GGTT on the vma Chris Wilson
@ 2017-12-05 12:55 ` Patchwork
  2017-12-05 14:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding (rev2) Patchwork
  2017-12-05 16:41 ` ✗ Fi.CI.IGT: warning " Patchwork
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-12-05 12:55 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding
URL   : https://patchwork.freedesktop.org/series/34900/
State : failure

== Summary ==

Series 34900v1 series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding
https://patchwork.freedesktop.org/api/1.0/series/34900/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-FAIL (fi-ilk-650)
                pass       -> INCOMPLETE (fi-snb-2600)
                pass       -> INCOMPLETE (fi-ivb-3520m)
                pass       -> INCOMPLETE (fi-ivb-3770)
                pass       -> INCOMPLETE (fi-byt-j1900)
                pass       -> INCOMPLETE (fi-byt-n2820)
                pass       -> INCOMPLETE (fi-hsw-4770)
                pass       -> INCOMPLETE (fi-hsw-4770r)
                pass       -> DMESG-FAIL (fi-bdw-5557u)
                pass       -> DMESG-FAIL (fi-bdw-gvtdvm)
                pass       -> DMESG-FAIL (fi-bsw-n3050)
                pass       -> DMESG-FAIL (fi-skl-6260u)
                pass       -> DMESG-FAIL (fi-skl-6600u)
                pass       -> DMESG-FAIL (fi-skl-6700hq)
                pass       -> DMESG-FAIL (fi-skl-6700k)
                pass       -> DMESG-FAIL (fi-skl-6770hq)
                pass       -> DMESG-FAIL (fi-skl-gvtdvm)
                pass       -> DMESG-FAIL (fi-bxt-dsi)
                pass       -> DMESG-FAIL (fi-bxt-j4205)
                pass       -> DMESG-FAIL (fi-kbl-7500u)
                pass       -> DMESG-FAIL (fi-kbl-7560u)
                pass       -> DMESG-FAIL (fi-kbl-7567u)
                pass       -> DMESG-FAIL (fi-kbl-r)
                pass       -> DMESG-FAIL (fi-glk-1)
        Subgroup basic-no-display:
                pass       -> FAIL       (fi-ilk-650)
                pass       -> FAIL       (fi-bdw-5557u)
                pass       -> FAIL       (fi-bdw-gvtdvm)
                pass       -> DMESG-FAIL (fi-bsw-n3050)
                pass       -> FAIL       (fi-skl-6260u)
                pass       -> FAIL       (fi-skl-6600u)
                pass       -> FAIL       (fi-skl-6700hq)
                pass       -> FAIL       (fi-skl-6700k)
                pass       -> FAIL       (fi-skl-6770hq)
                pass       -> FAIL       (fi-skl-gvtdvm)
                pass       -> FAIL       (fi-bxt-dsi)
                pass       -> FAIL       (fi-bxt-j4205)
                pass       -> FAIL       (fi-kbl-7500u)
                pass       -> FAIL       (fi-kbl-7560u)
                pass       -> FAIL       (fi-kbl-7567u)
                pass       -> FAIL       (fi-kbl-r)
                pass       -> FAIL       (fi-glk-1)
        Subgroup basic-reload-inject:
                pass       -> FAIL       (fi-ilk-650)
                pass       -> FAIL       (fi-bdw-5557u)
                pass       -> FAIL       (fi-bdw-gvtdvm)
                pass       -> FAIL       (fi-bsw-n3050) fdo#103706 +9
                pass       -> FAIL       (fi-skl-6260u)
                pass       -> FAIL       (fi-skl-6600u)
                pass       -> FAIL       (fi-skl-6700hq)
                pass       -> FAIL       (fi-skl-6700k)
                pass       -> FAIL       (fi-skl-6770hq)
                pass       -> FAIL       (fi-skl-gvtdvm)
                pass       -> FAIL       (fi-kbl-7500u)
                pass       -> FAIL       (fi-kbl-7567u)
                pass       -> FAIL       (fi-kbl-r)
Test gvt_basic:
        Subgroup invalid-placeholder-test:
                skip       -> INCOMPLETE (fi-ilk-650)
                skip       -> INCOMPLETE (fi-bdw-5557u)
                skip       -> INCOMPLETE (fi-skl-6260u)
                skip       -> INCOMPLETE (fi-skl-6600u)
                skip       -> INCOMPLETE (fi-skl-6700hq)
                skip       -> INCOMPLETE (fi-skl-6700k)
                skip       -> INCOMPLETE (fi-skl-6770hq)
                skip       -> INCOMPLETE (fi-kbl-7500u)
                skip       -> INCOMPLETE (fi-kbl-7567u)
                skip       -> INCOMPLETE (fi-kbl-r)

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103706 https://bugs.freedesktop.org/show_bug.cgi?id=103706

fi-bdw-5557u     total:288  pass:264  dwarn:0   dfail:1   fail:2   skip:20 
fi-bdw-gvtdvm    total:288  pass:261  dwarn:0   dfail:1   fail:2   skip:24  time:439s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:384s
fi-bsw-n3050     total:288  pass:239  dwarn:0   dfail:2   fail:1   skip:45 
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:255  dwarn:0   dfail:1   fail:2   skip:29 
fi-bxt-j4205     total:288  pass:256  dwarn:0   dfail:1   fail:2   skip:28 
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:34 
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:38 
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:268s
fi-glk-1         total:288  pass:257  dwarn:0   dfail:1   fail:2   skip:27 
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:26 
fi-hsw-4770r     total:285  pass:221  dwarn:0   dfail:0   fail:0   skip:63 
fi-ilk-650       total:288  pass:225  dwarn:0   dfail:1   fail:2   skip:59 
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:28 
fi-ivb-3770      total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:28 
fi-kbl-7500u     total:288  pass:260  dwarn:1   dfail:1   fail:2   skip:23 
fi-kbl-7560u     total:288  pass:266  dwarn:0   dfail:1   fail:2   skip:18 
fi-kbl-7567u     total:288  pass:265  dwarn:0   dfail:1   fail:2   skip:19 
WARNING: Long output truncated

e53251a19e664fae2666d1b0fba71772335feb2d drm-tip: 2017y-12m-05d-12h-06m-31s UTC integration manifest
f55f877e41f2 drm/i915: Track GGTT on the vma
11f6a95433b6 drm/i915: Flush pending GTT writes before unbinding

== Logs ==

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

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

* [PATCH v2] drm/i915: Track GGTT writes on the vma
  2017-12-05 12:17 ` [PATCH 2/2] drm/i915: Track GGTT on the vma Chris Wilson
@ 2017-12-05 13:12   ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2017-12-05 13:12 UTC (permalink / raw)
  To: intel-gfx

As writes through the GTT and GGTT PTE updates do not share the same
path, they are not strictly ordered and so we must explicitly flush the
indirect writes prior to modifying the PTE. We do track outstanding GGTT
writes on the object itself, but since the object may have multiple GGTT
vma, that is overly coarse as we can track and flush individual vma as
required.

Whilst here, update the GGTT flushing behaviour for Cannonlake.

v2: Hard-code ring offset to allow use during unload (after RCS may have
been freed, or never existed!)

References: https://bugs.freedesktop.org/show_bug.cgi?id=104002
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_gem.c | 55 +++++++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_vma.c | 22 +++++++++++++++++
 drivers/gpu/drm/i915/i915_vma.h | 19 ++++++++++++++
 4 files changed, 80 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 594fd14e66c5..5cf58e049dbd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3879,6 +3879,8 @@ int __must_check i915_gem_evict_for_node(struct i915_address_space *vm,
 					 unsigned int flags);
 int i915_gem_evict_vm(struct i915_address_space *vm);
 
+void i915_gem_flush_ggtt_writes(struct drm_i915_private *dev_priv);
+
 /* belongs in i915_gem_gtt.h */
 static inline void i915_gem_chipset_flush(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 80b78fb5daac..37600000bb05 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -666,17 +666,13 @@ fb_write_origin(struct drm_i915_gem_object *obj, unsigned int domain)
 		obj->frontbuffer_ggtt_origin : ORIGIN_CPU);
 }
 
-static void
-flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
+void i915_gem_flush_ggtt_writes(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
-
-	if (!(obj->base.write_domain & flush_domains))
-		return;
-
-	/* No actual flushing is required for the GTT write domain.  Writes
-	 * to it "immediately" go to main memory as far as we know, so there's
-	 * no chipset flush.  It also doesn't land in render cache.
+	/*
+	 * No actual flushing is required for the GTT write domain for reads
+	 * from the GTT domain. Writes to it "immediately" go to main memory
+	 * as far as we know, so there's no chipset flush. It also doesn't
+	 * land in the GPU render cache.
 	 *
 	 * However, we do have to enforce the order so that all writes through
 	 * the GTT land before any writes to the device, such as updates to
@@ -687,22 +683,43 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 	 * timing. This issue has only been observed when switching quickly
 	 * between GTT writes and CPU reads from inside the kernel on recent hw,
 	 * and it appears to only affect discrete GTT blocks (i.e. on LLC
-	 * system agents we cannot reproduce this behaviour).
+	 * system agents we cannot reproduce this behaviour, until Cannonlake
+	 * that was!).
 	 */
+
 	wmb();
 
+	intel_runtime_pm_get(dev_priv);
+	spin_lock_irq(&dev_priv->uncore.lock);
+
+	POSTING_READ_FW(RING_HEAD(RENDER_RING_BASE));
+
+	spin_unlock_irq(&dev_priv->uncore.lock);
+	intel_runtime_pm_put(dev_priv);
+}
+
+static void
+flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
+{
+	struct drm_i915_private *dev_priv = to_i915(obj->base.dev);
+	struct i915_vma *vma;
+
+	if (!(obj->base.write_domain & flush_domains))
+		return;
+
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
-		if (!HAS_LLC(dev_priv)) {
-			intel_runtime_pm_get(dev_priv);
-			spin_lock_irq(&dev_priv->uncore.lock);
-			POSTING_READ_FW(RING_HEAD(dev_priv->engine[RCS]->mmio_base));
-			spin_unlock_irq(&dev_priv->uncore.lock);
-			intel_runtime_pm_put(dev_priv);
-		}
+		i915_gem_flush_ggtt_writes(dev_priv);
 
 		intel_fb_obj_flush(obj,
 				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+
+		list_for_each_entry(vma, &obj->vma_list, obj_link) {
+			if (!i915_vma_is_ggtt(vma))
+				break;
+
+			i915_vma_unset_ggtt_write(vma);
+		}
 		break;
 
 	case I915_GEM_DOMAIN_CPU:
@@ -1965,6 +1982,8 @@ int i915_gem_fault(struct vm_fault *vmf)
 		list_add(&obj->userfault_link, &dev_priv->mm.userfault_list);
 	GEM_BUG_ON(!obj->userfault_count);
 
+	i915_vma_set_ggtt_write(vma);
+
 err_fence:
 	i915_vma_unpin_fence(vma);
 err_unpin:
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index bf6d8d1eaabe..c1204a01cb08 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -322,6 +322,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 	if (err)
 		goto err_unpin;
 
+	i915_vma_set_ggtt_write(vma);
 	return ptr;
 
 err_unpin:
@@ -330,12 +331,24 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
 	return IO_ERR_PTR(err);
 }
 
+void i915_vma_flush_writes(struct i915_vma *vma)
+{
+	if (!i915_vma_has_ggtt_write(vma))
+		return;
+
+	i915_gem_flush_ggtt_writes(vma->vm->i915);
+
+	i915_vma_unset_ggtt_write(vma);
+}
+
 void i915_vma_unpin_iomap(struct i915_vma *vma)
 {
 	lockdep_assert_held(&vma->obj->base.dev->struct_mutex);
 
 	GEM_BUG_ON(vma->iomap == NULL);
 
+	i915_vma_flush_writes(vma);
+
 	i915_vma_unpin_fence(vma);
 	i915_vma_unpin(vma);
 }
@@ -790,6 +803,15 @@ int i915_vma_unbind(struct i915_vma *vma)
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
 
 	if (i915_vma_is_map_and_fenceable(vma)) {
+		/*
+		 * Check that we have flushed all writes through the GGTT
+		 * before the unbind, other due to non-strict nature of those
+		 * indirect writes they may end up referencing the GGTT PTE
+		 * after the unbind.
+		 */
+		i915_vma_flush_writes(vma);
+		GEM_BUG_ON(i915_vma_has_ggtt_write(vma));
+
 		/* release the fence reg _after_ flushing */
 		ret = i915_vma_put_fence(vma);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 1e2bc9b3c3ac..f636243eb8f7 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -90,6 +90,7 @@ struct i915_vma {
 #define I915_VMA_CLOSED		BIT(10)
 #define I915_VMA_USERFAULT_BIT	11
 #define I915_VMA_USERFAULT	BIT(I915_VMA_USERFAULT_BIT)
+#define I915_VMA_GGTT_WRITE	BIT(12)
 
 	unsigned int active;
 	struct i915_gem_active last_read[I915_NUM_ENGINES];
@@ -138,6 +139,24 @@ static inline bool i915_vma_is_ggtt(const struct i915_vma *vma)
 	return vma->flags & I915_VMA_GGTT;
 }
 
+static inline bool i915_vma_has_ggtt_write(const struct i915_vma *vma)
+{
+	return vma->flags & I915_VMA_GGTT_WRITE;
+}
+
+static inline void i915_vma_set_ggtt_write(struct i915_vma *vma)
+{
+	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
+	vma->flags |= I915_VMA_GGTT_WRITE;
+}
+
+static inline void i915_vma_unset_ggtt_write(struct i915_vma *vma)
+{
+	vma->flags &= ~I915_VMA_GGTT_WRITE;
+}
+
+void i915_vma_flush_writes(struct i915_vma *vma);
+
 static inline bool i915_vma_is_map_and_fenceable(const struct i915_vma *vma)
 {
 	return vma->flags & I915_VMA_CAN_FENCE;
-- 
2.15.1

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

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

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding (rev2)
  2017-12-05 12:17 [PATCH 1/2] drm/i915: Flush pending GTT writes before unbinding Chris Wilson
  2017-12-05 12:17 ` [PATCH 2/2] drm/i915: Track GGTT on the vma Chris Wilson
  2017-12-05 12:55 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding Patchwork
@ 2017-12-05 14:20 ` Patchwork
  2017-12-05 16:41 ` ✗ Fi.CI.IGT: warning " Patchwork
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-12-05 14:20 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding (rev2)
URL   : https://patchwork.freedesktop.org/series/34900/
State : success

== Summary ==

Series 34900v2 series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding
https://patchwork.freedesktop.org/api/1.0/series/34900/revisions/2/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> DMESG-FAIL (fi-elk-e7500) fdo#103989 +1
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:438s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:446s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:381s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:527s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:515s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:489s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:475s
fi-elk-e7500     total:224  pass:163  dwarn:14  dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:1   dfail:0   fail:0   skip:108 time:267s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:538s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:374s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:258s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:391s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:483s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:446s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:489s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:533s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:540s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:586s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:460s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:544s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:572s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:520s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:499s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:457s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:412s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:616s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:614s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:495s

80da3dbebd28bbae73d15acf06eeaea420369b13 drm-tip: 2017y-12m-05d-13h-00m-30s UTC integration manifest
09fc8c2dfb3f drm/i915: Track GGTT writes on the vma
4dc5fea40899 drm/i915: Flush pending GTT writes before unbinding

== Logs ==

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

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

* ✗ Fi.CI.IGT: warning for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding (rev2)
  2017-12-05 12:17 [PATCH 1/2] drm/i915: Flush pending GTT writes before unbinding Chris Wilson
                   ` (2 preceding siblings ...)
  2017-12-05 14:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding (rev2) Patchwork
@ 2017-12-05 16:41 ` Patchwork
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-12-05 16:41 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding (rev2)
URL   : https://patchwork.freedesktop.org/series/34900/
State : warning

== Summary ==

Test kms_draw_crc:
        Subgroup draw-method-xrgb2101010-mmap-wc-untiled:
                pass       -> SKIP       (shard-hsw)
Test kms_cursor_legacy:
        Subgroup long-nonblocking-modeset-vs-cursor-atomic:
                skip       -> PASS       (shard-snb)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-pri-indfb-draw-mmap-wc:
                skip       -> PASS       (shard-snb)
Test drv_selftest:
        Subgroup mock_sanitycheck:
                dmesg-warn -> PASS       (shard-hsw) fdo#102707
Test kms_atomic:
        Subgroup crtc_invalid_params_fence:
                pass       -> SKIP       (shard-snb)
Test kms_universal_plane:
        Subgroup universal-plane-pipe-b-functional:
                pass       -> SKIP       (shard-snb)
Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (shard-hsw) fdo#103706
Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252 +1
Test kms_flip:
        Subgroup flip-vs-modeset-interruptible:
                pass       -> DMESG-WARN (shard-hsw) fdo#102614
        Subgroup plain-flip-ts-check-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-a-planes:
                pass       -> INCOMPLETE (shard-hsw) fdo#103540

fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#103706 https://bugs.freedesktop.org/show_bug.cgi?id=103706
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540

shard-hsw        total:2645 pass:1516 dwarn:3   dfail:0   fail:12  skip:1113 time:9265s
shard-snb        total:2612 pass:1272 dwarn:1   dfail:0   fail:10  skip:1328 time:7815s
Blacklisted hosts:
shard-apl        total:2590 pass:1613 dwarn:3   dfail:0   fail:21  skip:951 time:13083s

== Logs ==

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

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

end of thread, other threads:[~2017-12-05 16:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-05 12:17 [PATCH 1/2] drm/i915: Flush pending GTT writes before unbinding Chris Wilson
2017-12-05 12:17 ` [PATCH 2/2] drm/i915: Track GGTT on the vma Chris Wilson
2017-12-05 13:12   ` [PATCH v2] drm/i915: Track GGTT writes " Chris Wilson
2017-12-05 12:55 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding Patchwork
2017-12-05 14:20 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Flush pending GTT writes before unbinding (rev2) Patchwork
2017-12-05 16:41 ` ✗ Fi.CI.IGT: warning " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox