[PATCH 1/2] drm/i915: Remove vma from object on destroy, not close

[PATCH 1/2] drm/i915: Remove vma from object on destroy, not close

[Chris Wilson]

Originally we translated from the object to the vma by walking
obj->vma_list to find the matching vm (for user lookups). Now we process
user lookups using the rbtree, and we only use obj->vma_list itself for
maintaining state (e.g. ensuring that all vma are flushed or rebound).
As such maintenance needs to go on beyond the user's awareness of the
vma, defer removal of the vma from the obj->vma_list from i915_vma_close()
to i915_vma_destroy()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 ++-
 drivers/gpu/drm/i915/i915_vma.c | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 80b78fb5daac..5504be753092 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3746,7 +3746,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 			return -EBUSY;
 		}
 
-		if (i915_gem_valid_gtt_space(vma, cache_level))
+		if (!i915_vma_is_closed(vma) &&
+		    i915_gem_valid_gtt_space(vma, cache_level))
 			continue;
 
 		ret = i915_vma_unbind(vma);
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index bf6d8d1eaabe..1013403fcfea 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -466,6 +466,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 	u64 start, end;
 	int ret;
 
+	GEM_BUG_ON(i915_vma_is_closed(vma));
 	GEM_BUG_ON(vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND));
 	GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
 
@@ -678,7 +679,9 @@ static void i915_vma_destroy(struct i915_vma *vma)
 		GEM_BUG_ON(i915_gem_active_isset(&vma->last_read[i]));
 	GEM_BUG_ON(i915_gem_active_isset(&vma->last_fence));
 
+	list_del(&vma->obj_link);
 	list_del(&vma->vm_link);
+
 	if (!i915_vma_is_ggtt(vma))
 		i915_ppgtt_put(i915_vm_to_ppgtt(vma->vm));
 
@@ -690,7 +693,6 @@ void i915_vma_close(struct i915_vma *vma)
 	GEM_BUG_ON(i915_vma_is_closed(vma));
 	vma->flags |= I915_VMA_CLOSED;
 
-	list_del(&vma->obj_link);
 	rb_erase(&vma->obj_node, &vma->obj->vma_tree);
 
 	if (!i915_vma_is_active(vma) && !i915_vma_is_pinned(vma))
-- 
2.15.1

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

[PATCH 2/2] drm/i915: Track GGTT writes on the vma

[Chris Wilson]

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 | 58 ++++++++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_vma.c | 22 ++++++++++++++++
 drivers/gpu/drm/i915/i915_vma.h | 19 ++++++++++++++
 4 files changed, 83 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 5504be753092..67dc11effc8e 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,46 @@ 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;
+
+			if (vma->iomap)
+				continue;
+
+			i915_vma_unset_ggtt_write(vma);
+		}
 		break;
 
 	case I915_GEM_DOMAIN_CPU:
@@ -1965,6 +1985,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 1013403fcfea..0ebd75693505 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);
 }
@@ -792,6 +805,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

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Remove vma from object on destroy, not close

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Remove vma from object on destroy, not close
URL   : https://patchwork.freedesktop.org/series/34965/
State : success

== Summary ==

Series 34965v1 series starting with [1/2] drm/i915: Remove vma from object on destroy, not close
https://patchwork.freedesktop.org/api/1.0/series/34965/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> PASS       (fi-elk-e7500) fdo#103989
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:435s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:388s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:514s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:280s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:508s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:499s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:486s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:468s
fi-elk-e7500     total:224  pass:164  dwarn:14  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:269s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:372s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:262s
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:448s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:526s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:473s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:536s
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:449s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:546s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:558s
fi-skl-6700k     total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:516s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:495s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:553s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:411s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:612s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:488s

1a0d67efb4cc5611887c79adc5c3315790f78df5 drm-tip: 2017y-12m-06d-00h-51m-07s UTC integration manifest
c041b3fac9f0 drm/i915: Track GGTT writes on the vma
f8edb975157d drm/i915: Remove vma from object on destroy, not close

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Remove vma from object on destroy, not close

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Remove vma from object on destroy, not close
URL   : https://patchwork.freedesktop.org/series/34965/
State : failure

== Summary ==

Test kms_cursor_legacy:
        Subgroup pipe-c-forked-move:
                pass       -> INCOMPLETE (shard-hsw)
Test kms_plane:
        Subgroup plane-position-covered-pipe-c-planes:
                skip       -> PASS       (shard-hsw)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                fail       -> PASS       (shard-snb) fdo#101623 +2
        Subgroup fbc-1p-primscrn-pri-indfb-draw-pwrite:
                pass       -> SKIP       (shard-snb)
Test kms_flip:
        Subgroup vblank-vs-suspend-interruptible:
                incomplete -> PASS       (shard-hsw) fdo#100368
Test perf:
        Subgroup oa-exponents:
                fail       -> PASS       (shard-hsw) fdo#102254

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254

shard-hsw        total:2666 pass:1529 dwarn:1   dfail:0   fail:10  skip:1125 time:9333s
shard-snb        total:2679 pass:1307 dwarn:1   dfail:0   fail:12  skip:1359 time:8112s
Blacklisted hosts:
shard-apl        total:2604 pass:1632 dwarn:1   dfail:0   fail:24  skip:947 time:13317s
shard-kbl        total:2614 pass:1751 dwarn:1   dfail:0   fail:23  skip:837 time:10139s

== Logs ==

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

Re: [PATCH 1/2] drm/i915: Remove vma from object on destroy, not close

[Joonas Lahtinen]

On Wed, 2017-12-06 at 12:49 +0000, Chris Wilson wrote:
> Originally we translated from the object to the vma by walking
> obj->vma_list to find the matching vm (for user lookups). Now we process
> user lookups using the rbtree, and we only use obj->vma_list itself for
> maintaining state (e.g. ensuring that all vma are flushed or rebound).
> As such maintenance needs to go on beyond the user's awareness of the
> vma, defer removal of the vma from the obj->vma_list from i915_vma_close()
> to i915_vma_destroy()
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Track GGTT writes on the vma

[Joonas Lahtinen]

On Wed, 2017-12-06 at 12:49 +0000, Chris Wilson wrote:
> 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>

One comment below, not strictly related to this patch.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas

> +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))

This pattern could use for_each_ggtt_vma() macro or such.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Track GGTT writes on the vma

[Chris Wilson]

Quoting Joonas Lahtinen (2017-12-07 13:42:40)
> On Wed, 2017-12-06 at 12:49 +0000, Chris Wilson wrote:
> > 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>
> 
> One comment below, not strictly related to this patch.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Regards, Joonas
> 
> > +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))
> 
> This pattern could use for_each_ggtt_vma() macro or such.

Ok.

Thanks for the review, my Braswell thanks you, but it should be
reproducible on Broxton+ as well (and presumably Cannonlake+ if QA
reports are to believed). Fortunately, it's such a rare event, requiring
some writes into memory to overtake overs that I doubt anyone but igt
stress tests would notice.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx