All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Keep the device awake whilst in the GTT write domain
@ 2017-08-31  9:15 Chris Wilson
  2017-08-31 10:39 ` [PATCH v2] " Chris Wilson
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Chris Wilson @ 2017-08-31  9:15 UTC (permalink / raw)
  To: intel-gfx

Since runtime suspend is very harsh on GTT mmappings (they all get
zapped on suspend) keep the device awake while we the buffer remains in
the GTT write domain (as we expect subsequent writes). We special case
writes here, as the write domain is more bounded than the read domains;
a buffer may remain in multiple read domains until it is written to, but
a write from the GTT must be flushed prior to using it elsewhere (e.g.
on the GPU). However, userspace can control the write-domain and
although there is a soft contract that writes must be flushed (for e.g.
flushing scanouts and fbc), in the worst case an idle buffer may keep the
device alive until the buffer is destroyed.
---
 drivers/gpu/drm/i915/i915_gem.c            | 4 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e4cc08bc518c..a87ef5c77a92 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -695,15 +695,14 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
 		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
-			intel_runtime_pm_get(dev_priv);
 			spin_lock_irq(&dev_priv->uncore.lock);
 			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
 			spin_unlock_irq(&dev_priv->uncore.lock);
-			intel_runtime_pm_put(dev_priv);
 		}
 
 		intel_fb_obj_flush(obj,
 				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+		intel_runtime_pm_put(dev_priv);
 		break;
 
 	case I915_GEM_DOMAIN_CPU:
@@ -3555,6 +3554,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 	GEM_BUG_ON((obj->base.write_domain & ~I915_GEM_DOMAIN_GTT) != 0);
 	obj->base.read_domains |= I915_GEM_DOMAIN_GTT;
 	if (write) {
+		intel_runtime_pm_get_noresume(to_i915(obj->base.dev));
 		obj->base.read_domains = I915_GEM_DOMAIN_GTT;
 		obj->base.write_domain = I915_GEM_DOMAIN_GTT;
 		obj->mm.dirty = true;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8a9d37ac16d4..62c215eb38b7 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1865,6 +1865,8 @@ void i915_vma_move_to_active(struct i915_vma *vma,
 	i915_gem_active_set(&vma->last_read[idx], req);
 	list_move_tail(&vma->vm_link, &vma->vm->active_list);
 
+	if (obj->base.write_domain & I915_GEM_DOMAIN_GTT)
+		intel_runtime_pm_put(to_i915(obj->base.dev));
 	obj->base.write_domain = 0;
 	if (flags & EXEC_OBJECT_WRITE) {
 		obj->base.write_domain = I915_GEM_DOMAIN_RENDER;
-- 
2.14.1

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

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

* [PATCH v2] drm/i915: Keep the device awake whilst in the GTT write domain
  2017-08-31  9:15 [PATCH] drm/i915: Keep the device awake whilst in the GTT write domain Chris Wilson
@ 2017-08-31 10:39 ` Chris Wilson
  2017-08-31 12:12 ` ✗ Fi.CI.BAT: failure for drm/i915: Keep the device awake whilst in the GTT write domain (rev2) Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2017-08-31 10:39 UTC (permalink / raw)
  To: intel-gfx

Since runtime suspend is very harsh on GTT mmappings (they all get
zapped on suspend) keep the device awake while we the buffer remains in
the GTT write domain (as we expect subsequent writes). We special case
writes here, as the write domain is more bounded than the read domains;
a buffer may remain in multiple read domains until it is written to, but
a write from the GTT must be flushed prior to using it elsewhere (e.g.
on the GPU). However, userspace can control the write-domain and
although there is a soft contract that writes must be flushed (for e.g.
flushing scanouts and fbc), in the worst case an idle buffer may keep the
device alive until the buffer is destroyed.
---
 drivers/gpu/drm/i915/i915_gem.c            | 12 +++++++++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 ++
 drivers/gpu/drm/i915/i915_gem_object.h     |  3 +++
 drivers/gpu/drm/i915/i915_gem_shrinker.c   |  4 +++-
 drivers/gpu/drm/i915/intel_lrc.c           |  1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c    |  3 +++
 6 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e4cc08bc518c..553cc09e9ab3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -235,6 +235,7 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 
 static void __start_cpu_write(struct drm_i915_gem_object *obj)
 {
+	GEM_BUG_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
 	obj->base.read_domains = I915_GEM_DOMAIN_CPU;
 	obj->base.write_domain = I915_GEM_DOMAIN_CPU;
 	if (cpu_write_needs_clflush(obj))
@@ -667,11 +668,13 @@ fb_write_origin(struct drm_i915_gem_object *obj, unsigned int domain)
 		obj->frontbuffer_ggtt_origin : ORIGIN_CPU);
 }
 
-static void
+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);
 
+	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
 	if (!(obj->base.write_domain & flush_domains))
 		return;
 
@@ -695,15 +698,14 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
 		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
-			intel_runtime_pm_get(dev_priv);
 			spin_lock_irq(&dev_priv->uncore.lock);
 			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
 			spin_unlock_irq(&dev_priv->uncore.lock);
-			intel_runtime_pm_put(dev_priv);
 		}
 
 		intel_fb_obj_flush(obj,
 				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+		intel_runtime_pm_put(dev_priv);
 		break;
 
 	case I915_GEM_DOMAIN_CPU:
@@ -3425,6 +3427,7 @@ static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
 	flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
 	if (obj->cache_dirty)
 		i915_gem_clflush_object(obj, I915_CLFLUSH_FORCE);
+	GEM_BUG_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
 	obj->base.write_domain = 0;
 }
 
@@ -3555,6 +3558,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 	GEM_BUG_ON((obj->base.write_domain & ~I915_GEM_DOMAIN_GTT) != 0);
 	obj->base.read_domains |= I915_GEM_DOMAIN_GTT;
 	if (write) {
+		intel_runtime_pm_get_noresume(to_i915(obj->base.dev));
 		obj->base.read_domains = I915_GEM_DOMAIN_GTT;
 		obj->base.write_domain = I915_GEM_DOMAIN_GTT;
 		obj->mm.dirty = true;
@@ -4394,6 +4398,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 
 		trace_i915_gem_object_destroy(obj);
 
+		flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
+
 		GEM_BUG_ON(i915_gem_object_is_active(obj));
 		list_for_each_entry_safe(vma, vn,
 					 &obj->vma_list, obj_link) {
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8a9d37ac16d4..62c215eb38b7 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1865,6 +1865,8 @@ void i915_vma_move_to_active(struct i915_vma *vma,
 	i915_gem_active_set(&vma->last_read[idx], req);
 	list_move_tail(&vma->vm_link, &vma->vm->active_list);
 
+	if (obj->base.write_domain & I915_GEM_DOMAIN_GTT)
+		intel_runtime_pm_put(to_i915(obj->base.dev));
 	obj->base.write_domain = 0;
 	if (flags & EXEC_OBJECT_WRITE) {
 		obj->base.write_domain = I915_GEM_DOMAIN_RENDER;
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index c30d8f808185..f5f52c4090b0 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -421,5 +421,8 @@ void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
 					 unsigned int cache_level);
 void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj);
 
+void
+flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains);
+
 #endif
 
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 77fb39808131..71110b7d3ca0 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -127,8 +127,10 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
 
 static bool unsafe_drop_pages(struct drm_i915_gem_object *obj)
 {
-	if (i915_gem_object_unbind(obj) == 0)
+	if (i915_gem_object_unbind(obj) == 0) {
+		flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
 		__i915_gem_object_put_pages(obj, I915_MM_SHRINKER);
+	}
 	return !READ_ONCE(obj->mm.pages);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d89e1b8e1cc5..357eee6f907c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -866,6 +866,7 @@ execlists_context_pin(struct intel_engine_cs *engine,
 		i915_ggtt_offset(ce->ring->vma);
 
 	ce->state->obj->mm.dirty = true;
+	flush_write_domain(ce->state->obj, ~0);
 
 	i915_gem_context_get(ctx);
 out:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cdf084ef5aae..571a5b1f4f54 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1321,6 +1321,8 @@ int intel_ring_pin(struct intel_ring *ring,
 	if (IS_ERR(addr))
 		goto err;
 
+	flush_write_domain(vma->obj, ~0);
+
 	ring->vaddr = addr;
 	return 0;
 
@@ -1516,6 +1518,7 @@ intel_ring_context_pin(struct intel_engine_cs *engine,
 			goto err;
 
 		ce->state->obj->mm.dirty = true;
+		flush_write_domain(ce->state->obj, ~0);
 	}
 
 	/* The kernel context is only used as a placeholder for flushing the
-- 
2.14.1

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: Keep the device awake whilst in the GTT write domain (rev2)
  2017-08-31  9:15 [PATCH] drm/i915: Keep the device awake whilst in the GTT write domain Chris Wilson
  2017-08-31 10:39 ` [PATCH v2] " Chris Wilson
@ 2017-08-31 12:12 ` Patchwork
  2017-08-31 14:20 ` [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain Chris Wilson
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-08-31 12:12 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Keep the device awake whilst in the GTT write domain (rev2)
URL   : https://patchwork.freedesktop.org/series/29594/
State : failure

== Summary ==

Series 29594v2 drm/i915: Keep the device awake whilst in the GTT write domain
https://patchwork.freedesktop.org/api/1.0/series/29594/revisions/2/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-snb-2600) fdo#100215
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> FAIL       (fi-hsw-4770) fdo#102402 +1
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-bdw-gvtdvm) fdo#102374
                pass       -> DMESG-WARN (fi-bsw-n3050) fdo#101707
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-skl-6770hq)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
                pass       -> DMESG-WARN (fi-skl-x1585l)
                pass       -> DMESG-WARN (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> DMESG-WARN (fi-glk-2a) fdo#102457
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> FAIL       (fi-byt-j1900)
                pass       -> FAIL       (fi-hsw-4770)
                pass       -> FAIL       (fi-hsw-4770r)
                pass       -> FAIL       (fi-bdw-5557u)
                pass       -> FAIL       (fi-bsw-n3050)
                pass       -> FAIL       (fi-skl-6260u)
                pass       -> FAIL       (fi-skl-6700k)
                pass       -> FAIL       (fi-skl-6770hq)
                pass       -> FAIL       (fi-skl-x1585l)
                pass       -> FAIL       (fi-bxt-j4205)
                pass       -> FAIL       (fi-kbl-7500u)
                pass       -> FAIL       (fi-kbl-7560u)
                pass       -> FAIL       (fi-kbl-r)
                pass       -> FAIL       (fi-glk-2a)
        Subgroup basic-rte:
                pass       -> FAIL       (fi-byt-j1900)
                pass       -> FAIL       (fi-hsw-4770)
                pass       -> FAIL       (fi-hsw-4770r)
                pass       -> FAIL       (fi-bdw-5557u)
                pass       -> FAIL       (fi-bsw-n3050)
                pass       -> FAIL       (fi-skl-6260u)
                pass       -> FAIL       (fi-skl-6700k)
                pass       -> FAIL       (fi-skl-6770hq)
                pass       -> FAIL       (fi-skl-x1585l)
                pass       -> FAIL       (fi-bxt-j4205)
                pass       -> FAIL       (fi-kbl-7500u)
                pass       -> FAIL       (fi-kbl-7560u)
                pass       -> FAIL       (fi-kbl-r)
                pass       -> FAIL       (fi-glk-2a)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102402 https://bugs.freedesktop.org/show_bug.cgi?id=102402
fdo#102374 https://bugs.freedesktop.org/show_bug.cgi?id=102374
fdo#101707 https://bugs.freedesktop.org/show_bug.cgi?id=101707
fdo#102457 https://bugs.freedesktop.org/show_bug.cgi?id=102457

fi-bdw-5557u     total:288  pass:266  dwarn:0   dfail:0   fail:2   skip:20  time:477s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:1   dfail:0   fail:0   skip:23  time:439s
fi-blb-e6850     total:288  pass:224  dwarn:1   dfail:0   fail:0   skip:63  time:364s
fi-bsw-n3050     total:288  pass:240  dwarn:1   dfail:0   fail:2   skip:45  time:583s
fi-bwr-2160      total:288  pass:184  dwarn:0   dfail:0   fail:0   skip:104 time:253s
fi-bxt-j4205     total:288  pass:257  dwarn:1   dfail:0   fail:2   skip:28  time:546s
fi-byt-j1900     total:288  pass:251  dwarn:2   dfail:0   fail:2   skip:33  time:544s
fi-elk-e7500     total:288  pass:230  dwarn:0   dfail:0   fail:0   skip:58  time:441s
fi-glk-2a        total:288  pass:257  dwarn:1   dfail:0   fail:2   skip:28  time:631s
fi-hsw-4770      total:288  pass:259  dwarn:0   dfail:0   fail:4   skip:25  time:485s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:2   skip:25  time:440s
fi-ilk-650       total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:425s
fi-ivb-3520m     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:501s
fi-ivb-3770      total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:478s
fi-kbl-7500u     total:288  pass:258  dwarn:1   dfail:0   fail:2   skip:27  time:501s
fi-kbl-7560u     total:288  pass:267  dwarn:0   dfail:0   fail:2   skip:19  time:609s
fi-kbl-r         total:288  pass:259  dwarn:0   dfail:0   fail:2   skip:27  time:613s
fi-pnv-d510      total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:522s
fi-skl-6260u     total:288  pass:266  dwarn:1   dfail:0   fail:2   skip:19  time:490s
fi-skl-6700k     total:288  pass:262  dwarn:1   dfail:0   fail:2   skip:23  time:555s
fi-skl-6770hq    total:288  pass:266  dwarn:1   dfail:0   fail:2   skip:19  time:507s
fi-skl-gvtdvm    total:288  pass:265  dwarn:1   dfail:0   fail:0   skip:22  time:443s
fi-skl-x1585l    total:288  pass:265  dwarn:1   dfail:0   fail:2   skip:20  time:506s
fi-snb-2520m     total:288  pass:250  dwarn:1   dfail:0   fail:0   skip:37  time:548s
fi-snb-2600      total:288  pass:248  dwarn:1   dfail:0   fail:1   skip:38  time:405s

c399d43adc55a49d028d24ce7cdacc1823a4f159 drm-tip: 2017y-08m-31d-07h-25m-28s UTC integration manifest
5ce2c3d5524f drm/i915: Keep the device awake whilst in the GTT write domain

== Logs ==

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

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

* [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain
  2017-08-31  9:15 [PATCH] drm/i915: Keep the device awake whilst in the GTT write domain Chris Wilson
  2017-08-31 10:39 ` [PATCH v2] " Chris Wilson
  2017-08-31 12:12 ` ✗ Fi.CI.BAT: failure for drm/i915: Keep the device awake whilst in the GTT write domain (rev2) Patchwork
@ 2017-08-31 14:20 ` Chris Wilson
  2017-08-31 14:34 ` ✗ Fi.CI.BAT: failure for drm/i915: Keep the device awake whilst in the GTT write domain (rev3) Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2017-08-31 14:20 UTC (permalink / raw)
  To: intel-gfx

Since runtime suspend is very harsh on GTT mmappings (they all get
zapped on suspend) keep the device awake while the buffer remains in
the GTT domain. However, userspace can control the domain and
although there is a soft contract that writes must be flushed (for e.g.
flushing scanouts and fbc), we are intentionally lax with respect to read
domains, allowing them to persist for as long as is feasible. To ensure
that the device can eventually suspend, we install a timer. So in effect
we have just a fancy pm autosuspend that tries to estimate the cost of
restoring actively used GTT mmappings.
---
 drivers/gpu/drm/i915/i915_debugfs.c      |  2 +
 drivers/gpu/drm/i915/i915_drv.h          |  8 +++
 drivers/gpu/drm/i915/i915_gem.c          | 88 ++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/i915_gem_object.h   |  5 ++
 drivers/gpu/drm/i915/i915_gem_shrinker.c |  4 +-
 drivers/gpu/drm/i915/intel_lrc.c         |  1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c  |  3 ++
 7 files changed, 100 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 48572b157222..1432392fb2f8 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2812,6 +2812,8 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
 	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
 	seq_printf(m, "IRQs disabled: %s\n",
 		   yesno(!intel_irqs_enabled(dev_priv)));
+	seq_printf(m, "GTT wakeref count: %d\n",
+		   atomic_read(&dev_priv->mm.wakeref_count));
 #ifdef CONFIG_PM
 	seq_printf(m, "Usage count: %d\n",
 		   atomic_read(&dev_priv->drm.dev->power.usage_count));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0383e879a315..799ab57cf040 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1460,6 +1460,14 @@ struct i915_gem_mm {
 	 */
 	struct list_head userfault_list;
 
+	/** List of all objects in gtt domain, holding a wakeref.
+	 * The list is reaped periodically.
+	 */
+	struct list_head wakeref_list;
+	struct timer_list wakeref_timer;
+	spinlock_t wakeref_lock;
+	atomic_t wakeref_count;
+
 	/**
 	 * List of objects which are pending destruction.
 	 */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e4cc08bc518c..659fda483f7e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -235,6 +235,7 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 
 static void __start_cpu_write(struct drm_i915_gem_object *obj)
 {
+	GEM_BUG_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
 	obj->base.read_domains = I915_GEM_DOMAIN_CPU;
 	obj->base.write_domain = I915_GEM_DOMAIN_CPU;
 	if (cpu_write_needs_clflush(obj))
@@ -667,11 +668,13 @@ fb_write_origin(struct drm_i915_gem_object *obj, unsigned int domain)
 		obj->frontbuffer_ggtt_origin : ORIGIN_CPU);
 }
 
-static void
+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);
 
+	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
 	if (!(obj->base.write_domain & flush_domains))
 		return;
 
@@ -694,16 +697,20 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
-		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
-			intel_runtime_pm_get(dev_priv);
-			spin_lock_irq(&dev_priv->uncore.lock);
-			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
-			spin_unlock_irq(&dev_priv->uncore.lock);
-			intel_runtime_pm_put(dev_priv);
-		}
+		spin_lock_bh(&dev_priv->mm.wakeref_lock);
+		if (!list_empty(&obj->mm.wakeref_link)) {
+			if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
+				spin_lock_irq(&dev_priv->uncore.lock);
+				POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
+				spin_unlock_irq(&dev_priv->uncore.lock);
+			}
+
+			intel_fb_obj_flush(obj,
+					fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
 
-		intel_fb_obj_flush(obj,
-				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+			list_del_init(&obj->mm.wakeref_link);
+		}
+		spin_unlock_bh(&dev_priv->mm.wakeref_lock);
 		break;
 
 	case I915_GEM_DOMAIN_CPU:
@@ -3425,6 +3432,7 @@ static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
 	flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
 	if (obj->cache_dirty)
 		i915_gem_clflush_object(obj, I915_CLFLUSH_FORCE);
+	GEM_BUG_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
 	obj->base.write_domain = 0;
 }
 
@@ -3501,6 +3509,41 @@ i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write)
 	return 0;
 }
 
+static void wakeref_timeout(unsigned long data)
+{
+	struct drm_i915_private *dev_priv = (typeof(dev_priv))data;
+	struct drm_i915_gem_object *obj, *on;
+	unsigned int count;
+
+	spin_lock(&dev_priv->mm.wakeref_lock);
+	count = atomic_xchg(&dev_priv->mm.wakeref_count, 0);
+	if (count) {
+		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
+			spin_lock_irq(&dev_priv->uncore.lock);
+			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
+			spin_unlock_irq(&dev_priv->uncore.lock);
+		}
+
+		list_for_each_entry_safe(obj, on,
+					 &dev_priv->mm.wakeref_list,
+					 mm.wakeref_link) {
+			list_del_init(&obj->mm.wakeref_link);
+			if (obj->base.write_domain & I915_GEM_DOMAIN_GTT) {
+				intel_fb_obj_flush(obj, fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+				obj->base.write_domain = 0;
+			}
+		}
+
+		mod_timer(&dev_priv->mm.wakeref_timer,
+			  round_jiffies_up(jiffies +
+					   msecs_to_jiffies(500 * (ilog2(count)+1))));
+	} else {
+		intel_runtime_pm_put(dev_priv);
+		atomic_set(&dev_priv->mm.wakeref_count, -1);
+	}
+	spin_unlock(&dev_priv->mm.wakeref_lock);
+}
+
 /**
  * Moves a single object to the GTT read, and possibly write domain.
  * @obj: object to act on
@@ -3512,6 +3555,7 @@ i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write)
 int
 i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 {
+	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	int ret;
 
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
@@ -3525,6 +3569,17 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 	if (ret)
 		return ret;
 
+	spin_lock_bh(&i915->mm.wakeref_lock);
+	if (list_empty(&obj->mm.wakeref_link)) {
+		if (atomic_inc_and_test(&i915->mm.wakeref_count)) {
+			intel_runtime_pm_get(i915);
+			mod_timer(&i915->mm.wakeref_timer,
+				  round_jiffies_up(jiffies + msecs_to_jiffies(1000)));
+		}
+		list_add(&obj->mm.wakeref_link, &i915->mm.wakeref_list);
+	}
+	spin_unlock_bh(&i915->mm.wakeref_lock);
+
 	if (obj->base.write_domain == I915_GEM_DOMAIN_GTT)
 		return 0;
 
@@ -4257,6 +4312,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&obj->global_link);
 	INIT_LIST_HEAD(&obj->userfault_link);
+	INIT_LIST_HEAD(&obj->mm.wakeref_link);
 	INIT_LIST_HEAD(&obj->vma_list);
 	INIT_LIST_HEAD(&obj->lut_list);
 	INIT_LIST_HEAD(&obj->batch_pool_link);
@@ -4394,6 +4450,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 
 		trace_i915_gem_object_destroy(obj);
 
+		flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
+
 		GEM_BUG_ON(i915_gem_object_is_active(obj));
 		list_for_each_entry_safe(vma, vn,
 					 &obj->vma_list, obj_link) {
@@ -4548,6 +4606,10 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	int ret;
 
 	intel_runtime_pm_get(dev_priv);
+	if (atomic_read(&dev_priv->mm.wakeref_count) != -1)
+		wakeref_timeout((unsigned long)dev_priv);
+	del_timer_sync(&dev_priv->mm.wakeref_timer);
+
 	intel_suspend_gt_powersave(dev_priv);
 
 	mutex_lock(&dev->struct_mutex);
@@ -4932,6 +4994,12 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
 	if (err)
 		goto err_priorities;
 
+	INIT_LIST_HEAD(&dev_priv->mm.wakeref_list);
+	setup_timer(&dev_priv->mm.wakeref_timer,
+		    wakeref_timeout, (unsigned long)dev_priv);
+	spin_lock_init(&dev_priv->mm.wakeref_lock);
+	atomic_set(&dev_priv->mm.wakeref_count, -1);
+
 	INIT_WORK(&dev_priv->mm.free_work, __i915_gem_free_work);
 	init_llist_head(&dev_priv->mm.free_list);
 	INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index c30d8f808185..45b6bab4e118 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -177,6 +177,8 @@ struct drm_i915_gem_object {
 			struct mutex lock; /* protects this cache */
 		} get_page;
 
+		struct list_head wakeref_link;
+
 		/**
 		 * Advice: are the backing pages purgeable?
 		 */
@@ -421,5 +423,8 @@ void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
 					 unsigned int cache_level);
 void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj);
 
+void
+flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains);
+
 #endif
 
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 77fb39808131..71110b7d3ca0 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -127,8 +127,10 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
 
 static bool unsafe_drop_pages(struct drm_i915_gem_object *obj)
 {
-	if (i915_gem_object_unbind(obj) == 0)
+	if (i915_gem_object_unbind(obj) == 0) {
+		flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
 		__i915_gem_object_put_pages(obj, I915_MM_SHRINKER);
+	}
 	return !READ_ONCE(obj->mm.pages);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d89e1b8e1cc5..357eee6f907c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -866,6 +866,7 @@ execlists_context_pin(struct intel_engine_cs *engine,
 		i915_ggtt_offset(ce->ring->vma);
 
 	ce->state->obj->mm.dirty = true;
+	flush_write_domain(ce->state->obj, ~0);
 
 	i915_gem_context_get(ctx);
 out:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cdf084ef5aae..571a5b1f4f54 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1321,6 +1321,8 @@ int intel_ring_pin(struct intel_ring *ring,
 	if (IS_ERR(addr))
 		goto err;
 
+	flush_write_domain(vma->obj, ~0);
+
 	ring->vaddr = addr;
 	return 0;
 
@@ -1516,6 +1518,7 @@ intel_ring_context_pin(struct intel_engine_cs *engine,
 			goto err;
 
 		ce->state->obj->mm.dirty = true;
+		flush_write_domain(ce->state->obj, ~0);
 	}
 
 	/* The kernel context is only used as a placeholder for flushing the
-- 
2.14.1

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: Keep the device awake whilst in the GTT write domain (rev3)
  2017-08-31  9:15 [PATCH] drm/i915: Keep the device awake whilst in the GTT write domain Chris Wilson
                   ` (2 preceding siblings ...)
  2017-08-31 14:20 ` [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain Chris Wilson
@ 2017-08-31 14:34 ` Patchwork
  2017-09-01 12:16 ` [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain Chris Wilson
  2017-09-01 12:45 ` ✗ Fi.CI.BAT: warning for drm/i915: Keep the device awake whilst in the GTT write domain (rev4) Patchwork
  5 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-08-31 14:34 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Keep the device awake whilst in the GTT write domain (rev3)
URL   : https://patchwork.freedesktop.org/series/29594/
State : failure

== Summary ==

Series 29594v3 drm/i915: Keep the device awake whilst in the GTT write domain
https://patchwork.freedesktop.org/api/1.0/series/29594/revisions/3/mbox/

Test chamelium:
        Subgroup dp-hpd-fast:
                skip       -> PASS       (fi-kbl-7500u)
        Subgroup dp-edid-read:
                skip       -> PASS       (fi-kbl-7500u)
        Subgroup dp-crc-fast:
                skip       -> PASS       (fi-kbl-7500u)
        Subgroup hdmi-crc-fast:
                skip       -> INCOMPLETE (fi-snb-2520m)
        Subgroup vga-hpd-fast:
                skip       -> INCOMPLETE (fi-byt-j1900)
        Subgroup vga-edid-read:
                skip       -> INCOMPLETE (fi-byt-n2820)
        Subgroup common-hpd-after-suspend:
                pass       -> DMESG-WARN (fi-skl-6700k)
                skip       -> DMESG-WARN (fi-kbl-7500u)
Test core_auth:
        Subgroup basic-auth:
                pass       -> INCOMPLETE (fi-ivb-3770)
Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> INCOMPLETE (fi-ivb-3520m)
                pass       -> INCOMPLETE (fi-hsw-4770r)
Test drv_getparams_basic:
        Subgroup basic-subslice-total:
                pass       -> INCOMPLETE (fi-hsw-4770)
Test gem_basic:
        Subgroup bad-close:
                pass       -> INCOMPLETE (fi-snb-2600)
Test gem_busy:
        Subgroup basic-hang-default:
                pass       -> INCOMPLETE (fi-blb-e6850)
                pass       -> INCOMPLETE (fi-pnv-d510)
                pass       -> INCOMPLETE (fi-elk-e7500)
                pass       -> INCOMPLETE (fi-ilk-650)
                pass       -> INCOMPLETE (fi-bdw-5557u)
                pass       -> INCOMPLETE (fi-bdw-gvtdvm)
                pass       -> INCOMPLETE (fi-bsw-n3050)
                pass       -> INCOMPLETE (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> INCOMPLETE (fi-skl-6770hq)
                pass       -> INCOMPLETE (fi-skl-gvtdvm)
                pass       -> INCOMPLETE (fi-skl-x1585l)
                pass       -> INCOMPLETE (fi-bxt-j4205)
                pass       -> DMESG-WARN (fi-kbl-7500u)
                pass       -> INCOMPLETE (fi-kbl-7560u)
                pass       -> INCOMPLETE (fi-kbl-r)
                pass       -> INCOMPLETE (fi-glk-2a)
Test gem_close_race:
        Subgroup basic-process:
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-threads:
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> INCOMPLETE (fi-bwr-2160)
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test gem_ctx_basic:
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test gem_ctx_create:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-files:
                pass       -> INCOMPLETE (fi-skl-6700k)
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test gem_ctx_exec:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test gem_ctx_param:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-default:
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-default-heavy:
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test gem_exec_basic:
        Subgroup basic-blt:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-bsd:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-default:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-render:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup basic-vebox:
WARNING: Long output truncated

c399d43adc55a49d028d24ce7cdacc1823a4f159 drm-tip: 2017y-08m-31d-07h-25m-28s UTC integration manifest
03541656a674 drm/i915: Keep the device awake whilst in the GTT domain

== Logs ==

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

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

* [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain
  2017-08-31  9:15 [PATCH] drm/i915: Keep the device awake whilst in the GTT write domain Chris Wilson
                   ` (3 preceding siblings ...)
  2017-08-31 14:34 ` ✗ Fi.CI.BAT: failure for drm/i915: Keep the device awake whilst in the GTT write domain (rev3) Patchwork
@ 2017-09-01 12:16 ` Chris Wilson
  2017-09-04  8:16   ` Daniel Vetter
  2017-09-01 12:45 ` ✗ Fi.CI.BAT: warning for drm/i915: Keep the device awake whilst in the GTT write domain (rev4) Patchwork
  5 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2017-09-01 12:16 UTC (permalink / raw)
  To: intel-gfx

Since runtime suspend is very harsh on GTT mmappings (they all get
zapped on suspend) keep the device awake while the buffer remains in
the GTT domain. However, userspace can control the domain and
although there is a soft contract that writes must be flushed (for e.g.
flushing scanouts and fbc), we are intentionally lax with respect to read
domains, allowing them to persist for as long as is feasible.

We acquire a wakeref when using the buffer in the GEM domain so that all
subsequent operations on that object are fast, trying to avoid
suspending while the GTT is still in active use by userspace.  To ensure
that the device can eventually suspend, we install a timer and expire the
GTT wakeref. So in effect we have just a fancy pm autosuspend that tries
to estimate the cost of restoring actively used GTT mmapping.
---
 drivers/gpu/drm/i915/i915_debugfs.c      |   2 +
 drivers/gpu/drm/i915/i915_drv.h          |  11 ++++
 drivers/gpu/drm/i915/i915_gem.c          | 103 ++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_gem_object.h   |   5 ++
 drivers/gpu/drm/i915/i915_gem_shrinker.c |   4 +-
 drivers/gpu/drm/i915/intel_lrc.c         |   1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c  |   3 +
 7 files changed, 118 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 48572b157222..dbb07612aa5a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2809,6 +2809,8 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
 	if (!HAS_RUNTIME_PM(dev_priv))
 		seq_puts(m, "Runtime power management not supported\n");
 
+	seq_printf(m, "GTT wakeref count: %d\n",
+		   atomic_read(&dev_priv->mm.gtt_wakeref.count));
 	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
 	seq_printf(m, "IRQs disabled: %s\n",
 		   yesno(!intel_irqs_enabled(dev_priv)));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0383e879a315..14dcf6614f3c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1460,6 +1460,17 @@ struct i915_gem_mm {
 	 */
 	struct list_head userfault_list;
 
+	/* List of all objects in gtt domain, holding a wakeref.
+	 * The list is reaped periodically, and protected by its own mutex.
+	 */
+	struct {
+		struct mutex lock;
+		struct list_head list;
+		atomic_t count;
+
+		struct delayed_work work;
+	} gtt_wakeref;
+
 	/**
 	 * List of objects which are pending destruction.
 	 */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e4cc08bc518c..09baf80889e8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -235,6 +235,7 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 
 static void __start_cpu_write(struct drm_i915_gem_object *obj)
 {
+	GEM_BUG_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
 	obj->base.read_domains = I915_GEM_DOMAIN_CPU;
 	obj->base.write_domain = I915_GEM_DOMAIN_CPU;
 	if (cpu_write_needs_clflush(obj))
@@ -667,11 +668,13 @@ fb_write_origin(struct drm_i915_gem_object *obj, unsigned int domain)
 		obj->frontbuffer_ggtt_origin : ORIGIN_CPU);
 }
 
-static void
+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);
 
+	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
 	if (!(obj->base.write_domain & flush_domains))
 		return;
 
@@ -694,16 +697,19 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
-		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
-			intel_runtime_pm_get(dev_priv);
-			spin_lock_irq(&dev_priv->uncore.lock);
-			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
-			spin_unlock_irq(&dev_priv->uncore.lock);
-			intel_runtime_pm_put(dev_priv);
-		}
+		mutex_lock(&dev_priv->mm.gtt_wakeref.lock);
+		if (!list_empty(&obj->mm.gtt_wakeref_link)) {
+			if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
+				spin_lock_irq(&dev_priv->uncore.lock);
+				POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
+				spin_unlock_irq(&dev_priv->uncore.lock);
+			}
 
-		intel_fb_obj_flush(obj,
-				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+			intel_fb_obj_flush(obj, fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+
+			list_del_init(&obj->mm.gtt_wakeref_link);
+		}
+		mutex_unlock(&dev_priv->mm.gtt_wakeref.lock);
 		break;
 
 	case I915_GEM_DOMAIN_CPU:
@@ -3425,6 +3431,7 @@ static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
 	flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
 	if (obj->cache_dirty)
 		i915_gem_clflush_object(obj, I915_CLFLUSH_FORCE);
+	GEM_BUG_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
 	obj->base.write_domain = 0;
 }
 
@@ -3501,6 +3508,49 @@ i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write)
 	return 0;
 }
 
+static void wakeref_timeout(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, typeof(*dev_priv), mm.gtt_wakeref.work.work);
+	struct drm_i915_gem_object *obj, *on;
+	unsigned int count;
+
+	mutex_lock(&dev_priv->mm.gtt_wakeref.lock);
+	count = atomic_xchg(&dev_priv->mm.gtt_wakeref.count, 0);
+	if (count) {
+		unsigned long timeout;
+
+		GEM_BUG_ON(count == -1);
+
+		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
+			spin_lock_irq(&dev_priv->uncore.lock);
+			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
+			spin_unlock_irq(&dev_priv->uncore.lock);
+		}
+
+		count = 0;
+		list_for_each_entry_safe(obj, on,
+					 &dev_priv->mm.gtt_wakeref.list,
+					 mm.gtt_wakeref_link) {
+			list_del_init(&obj->mm.gtt_wakeref_link);
+			if (obj->base.write_domain & I915_GEM_DOMAIN_GTT) {
+				intel_fb_obj_flush(obj, fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
+				obj->base.write_domain = 0;
+			}
+			count++;
+		}
+
+		timeout = HZ * (ilog2(count) + 1) / 2;
+		mod_delayed_work(system_wq,
+				 &dev_priv->mm.gtt_wakeref.work,
+				 round_jiffies_up_relative(timeout));
+	} else {
+		intel_runtime_pm_put(dev_priv);
+		atomic_set(&dev_priv->mm.gtt_wakeref.count, -1);
+	}
+	mutex_unlock(&dev_priv->mm.gtt_wakeref.lock);
+}
+
 /**
  * Moves a single object to the GTT read, and possibly write domain.
  * @obj: object to act on
@@ -3512,6 +3562,7 @@ i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write)
 int
 i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 {
+	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 	int ret;
 
 	lockdep_assert_held(&obj->base.dev->struct_mutex);
@@ -3525,6 +3576,17 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 	if (ret)
 		return ret;
 
+	mutex_lock(&i915->mm.gtt_wakeref.lock);
+	if (list_empty(&obj->mm.gtt_wakeref_link)) {
+		if (atomic_inc_and_test(&i915->mm.gtt_wakeref.count)) {
+			intel_runtime_pm_get(i915);
+			schedule_delayed_work(&i915->mm.gtt_wakeref.work,
+					      round_jiffies_up_relative(HZ));
+		}
+		list_add(&obj->mm.gtt_wakeref_link, &i915->mm.gtt_wakeref.list);
+	}
+	mutex_unlock(&i915->mm.gtt_wakeref.lock);
+
 	if (obj->base.write_domain == I915_GEM_DOMAIN_GTT)
 		return 0;
 
@@ -4257,6 +4319,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&obj->global_link);
 	INIT_LIST_HEAD(&obj->userfault_link);
+	INIT_LIST_HEAD(&obj->mm.gtt_wakeref_link);
 	INIT_LIST_HEAD(&obj->vma_list);
 	INIT_LIST_HEAD(&obj->lut_list);
 	INIT_LIST_HEAD(&obj->batch_pool_link);
@@ -4394,6 +4457,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 
 		trace_i915_gem_object_destroy(obj);
 
+		flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
+
+		if (!list_empty_careful(&obj->mm.gtt_wakeref_link)) {
+			mutex_lock(&i915->mm.gtt_wakeref.lock);
+			list_del(&obj->mm.gtt_wakeref_link);
+			mutex_unlock(&i915->mm.gtt_wakeref.lock);
+		}
+
 		GEM_BUG_ON(i915_gem_object_is_active(obj));
 		list_for_each_entry_safe(vma, vn,
 					 &obj->vma_list, obj_link) {
@@ -4548,6 +4619,9 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	int ret;
 
 	intel_runtime_pm_get(dev_priv);
+	while (flush_delayed_work(&dev_priv->mm.gtt_wakeref.work))
+		;
+	WARN_ON(atomic_read(&dev_priv->mm.gtt_wakeref.count) != -1);
 	intel_suspend_gt_powersave(dev_priv);
 
 	mutex_lock(&dev->struct_mutex);
@@ -4932,6 +5006,11 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
 	if (err)
 		goto err_priorities;
 
+	INIT_LIST_HEAD(&dev_priv->mm.gtt_wakeref.list);
+	INIT_DELAYED_WORK(&dev_priv->mm.gtt_wakeref.work, wakeref_timeout);
+	mutex_init(&dev_priv->mm.gtt_wakeref.lock);
+	atomic_set(&dev_priv->mm.gtt_wakeref.count, -1);
+
 	INIT_WORK(&dev_priv->mm.free_work, __i915_gem_free_work);
 	init_llist_head(&dev_priv->mm.free_list);
 	INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
@@ -4978,6 +5057,10 @@ void i915_gem_load_cleanup(struct drm_i915_private *dev_priv)
 	WARN_ON(!list_empty(&dev_priv->gt.timelines));
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
+	while (flush_delayed_work(&dev_priv->mm.gtt_wakeref.work))
+		;
+	WARN_ON(atomic_read(&dev_priv->mm.gtt_wakeref.count) != -1);
+
 	kmem_cache_destroy(dev_priv->priorities);
 	kmem_cache_destroy(dev_priv->dependencies);
 	kmem_cache_destroy(dev_priv->requests);
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index c30d8f808185..3ca13edf32b6 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -177,6 +177,8 @@ struct drm_i915_gem_object {
 			struct mutex lock; /* protects this cache */
 		} get_page;
 
+		struct list_head gtt_wakeref_link;
+
 		/**
 		 * Advice: are the backing pages purgeable?
 		 */
@@ -421,5 +423,8 @@ void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
 					 unsigned int cache_level);
 void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj);
 
+void
+flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains);
+
 #endif
 
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 77fb39808131..71110b7d3ca0 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -127,8 +127,10 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
 
 static bool unsafe_drop_pages(struct drm_i915_gem_object *obj)
 {
-	if (i915_gem_object_unbind(obj) == 0)
+	if (i915_gem_object_unbind(obj) == 0) {
+		flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
 		__i915_gem_object_put_pages(obj, I915_MM_SHRINKER);
+	}
 	return !READ_ONCE(obj->mm.pages);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d89e1b8e1cc5..357eee6f907c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -866,6 +866,7 @@ execlists_context_pin(struct intel_engine_cs *engine,
 		i915_ggtt_offset(ce->ring->vma);
 
 	ce->state->obj->mm.dirty = true;
+	flush_write_domain(ce->state->obj, ~0);
 
 	i915_gem_context_get(ctx);
 out:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cdf084ef5aae..571a5b1f4f54 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1321,6 +1321,8 @@ int intel_ring_pin(struct intel_ring *ring,
 	if (IS_ERR(addr))
 		goto err;
 
+	flush_write_domain(vma->obj, ~0);
+
 	ring->vaddr = addr;
 	return 0;
 
@@ -1516,6 +1518,7 @@ intel_ring_context_pin(struct intel_engine_cs *engine,
 			goto err;
 
 		ce->state->obj->mm.dirty = true;
+		flush_write_domain(ce->state->obj, ~0);
 	}
 
 	/* The kernel context is only used as a placeholder for flushing the
-- 
2.14.1

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

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

* ✗ Fi.CI.BAT: warning for drm/i915: Keep the device awake whilst in the GTT write domain (rev4)
  2017-08-31  9:15 [PATCH] drm/i915: Keep the device awake whilst in the GTT write domain Chris Wilson
                   ` (4 preceding siblings ...)
  2017-09-01 12:16 ` [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain Chris Wilson
@ 2017-09-01 12:45 ` Patchwork
  5 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-09-01 12:45 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Keep the device awake whilst in the GTT write domain (rev4)
URL   : https://patchwork.freedesktop.org/series/29594/
State : warning

== Summary ==

Series 29594v4 drm/i915: Keep the device awake whilst in the GTT write domain
https://patchwork.freedesktop.org/api/1.0/series/29594/revisions/4/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-snb-2600) fdo#100215
        Subgroup basic-flip-after-cursor-varying-size:
                fail       -> PASS       (fi-hsw-4770) fdo#102402 +1
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-bwr-2160)
                pass       -> DMESG-WARN (fi-ilk-650)
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-ivb-3520m)
                pass       -> DMESG-WARN (fi-byt-j1900)
                pass       -> DMESG-WARN (fi-byt-n2820)
                pass       -> DMESG-WARN (fi-skl-6700k)
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (fi-pnv-d510)
                pass       -> DMESG-WARN (fi-ivb-3770)
                pass       -> DMESG-WARN (fi-glk-2a)

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102402 https://bugs.freedesktop.org/show_bug.cgi?id=102402

fi-bdw-5557u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:457s
fi-bdw-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:444s
fi-blb-e6850     total:288  pass:224  dwarn:1   dfail:0   fail:0   skip:63  time:361s
fi-bsw-n3050     total:288  pass:243  dwarn:0   dfail:0   fail:0   skip:45  time:552s
fi-bwr-2160      total:288  pass:183  dwarn:1   dfail:0   fail:0   skip:104 time:254s
fi-bxt-j4205     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:521s
fi-byt-j1900     total:288  pass:253  dwarn:2   dfail:0   fail:0   skip:33  time:526s
fi-byt-n2820     total:288  pass:249  dwarn:2   dfail:0   fail:0   skip:37  time:515s
fi-elk-e7500     total:288  pass:230  dwarn:0   dfail:0   fail:0   skip:58  time:437s
fi-glk-2a        total:288  pass:259  dwarn:1   dfail:0   fail:0   skip:28  time:616s
fi-hsw-4770      total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:448s
fi-hsw-4770r     total:288  pass:263  dwarn:0   dfail:0   fail:0   skip:25  time:433s
fi-ilk-650       total:288  pass:228  dwarn:1   dfail:0   fail:0   skip:59  time:427s
fi-ivb-3520m     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:508s
fi-ivb-3770      total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:475s
fi-kbl-7500u     total:288  pass:264  dwarn:1   dfail:0   fail:0   skip:23  time:520s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:596s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:604s
fi-pnv-d510      total:288  pass:222  dwarn:2   dfail:0   fail:0   skip:64  time:522s
fi-skl-6260u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:472s
fi-skl-6700k     total:288  pass:264  dwarn:1   dfail:0   fail:0   skip:23  time:535s
fi-skl-6770hq    total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:499s
fi-skl-gvtdvm    total:288  pass:266  dwarn:0   dfail:0   fail:0   skip:22  time:440s
fi-skl-x1585l    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:492s
fi-snb-2520m     total:288  pass:251  dwarn:0   dfail:0   fail:0   skip:37  time:549s
fi-snb-2600      total:288  pass:250  dwarn:0   dfail:0   fail:0   skip:38  time:405s

ccf4ca2d93383fe1a234aba83df9c21400216433 drm-tip: 2017y-08m-31d-18h-37m-46s UTC integration manifest
108dda0fc085 drm/i915: Keep the device awake whilst in the GTT domain

== Logs ==

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

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

* Re: [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain
  2017-09-01 12:16 ` [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain Chris Wilson
@ 2017-09-04  8:16   ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2017-09-04  8:16 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Fri, Sep 01, 2017 at 01:16:06PM +0100, Chris Wilson wrote:
> Since runtime suspend is very harsh on GTT mmappings (they all get
> zapped on suspend) keep the device awake while the buffer remains in
> the GTT domain. However, userspace can control the domain and
> although there is a soft contract that writes must be flushed (for e.g.
> flushing scanouts and fbc), we are intentionally lax with respect to read
> domains, allowing them to persist for as long as is feasible.
> 
> We acquire a wakeref when using the buffer in the GEM domain so that all
> subsequent operations on that object are fast, trying to avoid
> suspending while the GTT is still in active use by userspace.  To ensure
> that the device can eventually suspend, we install a timer and expire the
> GTT wakeref. So in effect we have just a fancy pm autosuspend that tries
> to estimate the cost of restoring actively used GTT mmapping.

Please tag with for-CI or something like that when throwing patches at
the shards :-) At least that's what I assuming given lack of sob and
revision of changes ...

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c      |   2 +
>  drivers/gpu/drm/i915/i915_drv.h          |  11 ++++
>  drivers/gpu/drm/i915/i915_gem.c          | 103 ++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_gem_object.h   |   5 ++
>  drivers/gpu/drm/i915/i915_gem_shrinker.c |   4 +-
>  drivers/gpu/drm/i915/intel_lrc.c         |   1 +
>  drivers/gpu/drm/i915/intel_ringbuffer.c  |   3 +
>  7 files changed, 118 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 48572b157222..dbb07612aa5a 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2809,6 +2809,8 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
>  	if (!HAS_RUNTIME_PM(dev_priv))
>  		seq_puts(m, "Runtime power management not supported\n");
>  
> +	seq_printf(m, "GTT wakeref count: %d\n",
> +		   atomic_read(&dev_priv->mm.gtt_wakeref.count));
>  	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
>  	seq_printf(m, "IRQs disabled: %s\n",
>  		   yesno(!intel_irqs_enabled(dev_priv)));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0383e879a315..14dcf6614f3c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1460,6 +1460,17 @@ struct i915_gem_mm {
>  	 */
>  	struct list_head userfault_list;
>  
> +	/* List of all objects in gtt domain, holding a wakeref.
> +	 * The list is reaped periodically, and protected by its own mutex.
> +	 */
> +	struct {
> +		struct mutex lock;
> +		struct list_head list;
> +		atomic_t count;
> +
> +		struct delayed_work work;
> +	} gtt_wakeref;
> +
>  	/**
>  	 * List of objects which are pending destruction.
>  	 */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e4cc08bc518c..09baf80889e8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -235,6 +235,7 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
>  
>  static void __start_cpu_write(struct drm_i915_gem_object *obj)
>  {
> +	GEM_BUG_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
>  	obj->base.read_domains = I915_GEM_DOMAIN_CPU;
>  	obj->base.write_domain = I915_GEM_DOMAIN_CPU;
>  	if (cpu_write_needs_clflush(obj))
> @@ -667,11 +668,13 @@ fb_write_origin(struct drm_i915_gem_object *obj, unsigned int domain)
>  		obj->frontbuffer_ggtt_origin : ORIGIN_CPU);
>  }
>  
> -static void
> +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);
>  
> +	lockdep_assert_held(&dev_priv->drm.struct_mutex);
> +
>  	if (!(obj->base.write_domain & flush_domains))
>  		return;
>  
> @@ -694,16 +697,19 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
>  
>  	switch (obj->base.write_domain) {
>  	case I915_GEM_DOMAIN_GTT:
> -		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
> -			intel_runtime_pm_get(dev_priv);
> -			spin_lock_irq(&dev_priv->uncore.lock);
> -			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> -			spin_unlock_irq(&dev_priv->uncore.lock);
> -			intel_runtime_pm_put(dev_priv);
> -		}
> +		mutex_lock(&dev_priv->mm.gtt_wakeref.lock);
> +		if (!list_empty(&obj->mm.gtt_wakeref_link)) {
> +			if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
> +				spin_lock_irq(&dev_priv->uncore.lock);
> +				POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> +				spin_unlock_irq(&dev_priv->uncore.lock);
> +			}
>  
> -		intel_fb_obj_flush(obj,
> -				   fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
> +			intel_fb_obj_flush(obj, fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
> +
> +			list_del_init(&obj->mm.gtt_wakeref_link);
> +		}
> +		mutex_unlock(&dev_priv->mm.gtt_wakeref.lock);
>  		break;
>  
>  	case I915_GEM_DOMAIN_CPU:
> @@ -3425,6 +3431,7 @@ static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
>  	flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
>  	if (obj->cache_dirty)
>  		i915_gem_clflush_object(obj, I915_CLFLUSH_FORCE);
> +	GEM_BUG_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
>  	obj->base.write_domain = 0;
>  }
>  
> @@ -3501,6 +3508,49 @@ i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write)
>  	return 0;
>  }
>  
> +static void wakeref_timeout(struct work_struct *work)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(work, typeof(*dev_priv), mm.gtt_wakeref.work.work);
> +	struct drm_i915_gem_object *obj, *on;
> +	unsigned int count;
> +
> +	mutex_lock(&dev_priv->mm.gtt_wakeref.lock);
> +	count = atomic_xchg(&dev_priv->mm.gtt_wakeref.count, 0);
> +	if (count) {
> +		unsigned long timeout;
> +
> +		GEM_BUG_ON(count == -1);
> +
> +		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
> +			spin_lock_irq(&dev_priv->uncore.lock);
> +			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
> +			spin_unlock_irq(&dev_priv->uncore.lock);
> +		}
> +
> +		count = 0;
> +		list_for_each_entry_safe(obj, on,
> +					 &dev_priv->mm.gtt_wakeref.list,
> +					 mm.gtt_wakeref_link) {
> +			list_del_init(&obj->mm.gtt_wakeref_link);
> +			if (obj->base.write_domain & I915_GEM_DOMAIN_GTT) {
> +				intel_fb_obj_flush(obj, fb_write_origin(obj, I915_GEM_DOMAIN_GTT));
> +				obj->base.write_domain = 0;
> +			}
> +			count++;
> +		}
> +
> +		timeout = HZ * (ilog2(count) + 1) / 2;
> +		mod_delayed_work(system_wq,
> +				 &dev_priv->mm.gtt_wakeref.work,
> +				 round_jiffies_up_relative(timeout));
> +	} else {
> +		intel_runtime_pm_put(dev_priv);
> +		atomic_set(&dev_priv->mm.gtt_wakeref.count, -1);
> +	}
> +	mutex_unlock(&dev_priv->mm.gtt_wakeref.lock);
> +}
> +
>  /**
>   * Moves a single object to the GTT read, and possibly write domain.
>   * @obj: object to act on
> @@ -3512,6 +3562,7 @@ i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write)
>  int
>  i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
>  {
> +	struct drm_i915_private *i915 = to_i915(obj->base.dev);
>  	int ret;
>  
>  	lockdep_assert_held(&obj->base.dev->struct_mutex);
> @@ -3525,6 +3576,17 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
>  	if (ret)
>  		return ret;
>  
> +	mutex_lock(&i915->mm.gtt_wakeref.lock);
> +	if (list_empty(&obj->mm.gtt_wakeref_link)) {
> +		if (atomic_inc_and_test(&i915->mm.gtt_wakeref.count)) {
> +			intel_runtime_pm_get(i915);
> +			schedule_delayed_work(&i915->mm.gtt_wakeref.work,
> +					      round_jiffies_up_relative(HZ));
> +		}
> +		list_add(&obj->mm.gtt_wakeref_link, &i915->mm.gtt_wakeref.list);
> +	}
> +	mutex_unlock(&i915->mm.gtt_wakeref.lock);
> +
>  	if (obj->base.write_domain == I915_GEM_DOMAIN_GTT)
>  		return 0;
>  
> @@ -4257,6 +4319,7 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
>  
>  	INIT_LIST_HEAD(&obj->global_link);
>  	INIT_LIST_HEAD(&obj->userfault_link);
> +	INIT_LIST_HEAD(&obj->mm.gtt_wakeref_link);
>  	INIT_LIST_HEAD(&obj->vma_list);
>  	INIT_LIST_HEAD(&obj->lut_list);
>  	INIT_LIST_HEAD(&obj->batch_pool_link);
> @@ -4394,6 +4457,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
>  
>  		trace_i915_gem_object_destroy(obj);
>  
> +		flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
> +
> +		if (!list_empty_careful(&obj->mm.gtt_wakeref_link)) {
> +			mutex_lock(&i915->mm.gtt_wakeref.lock);
> +			list_del(&obj->mm.gtt_wakeref_link);
> +			mutex_unlock(&i915->mm.gtt_wakeref.lock);
> +		}
> +
>  		GEM_BUG_ON(i915_gem_object_is_active(obj));
>  		list_for_each_entry_safe(vma, vn,
>  					 &obj->vma_list, obj_link) {
> @@ -4548,6 +4619,9 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
>  	int ret;
>  
>  	intel_runtime_pm_get(dev_priv);
> +	while (flush_delayed_work(&dev_priv->mm.gtt_wakeref.work))
> +		;
> +	WARN_ON(atomic_read(&dev_priv->mm.gtt_wakeref.count) != -1);
>  	intel_suspend_gt_powersave(dev_priv);
>  
>  	mutex_lock(&dev->struct_mutex);
> @@ -4932,6 +5006,11 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
>  	if (err)
>  		goto err_priorities;
>  
> +	INIT_LIST_HEAD(&dev_priv->mm.gtt_wakeref.list);
> +	INIT_DELAYED_WORK(&dev_priv->mm.gtt_wakeref.work, wakeref_timeout);
> +	mutex_init(&dev_priv->mm.gtt_wakeref.lock);
> +	atomic_set(&dev_priv->mm.gtt_wakeref.count, -1);
> +
>  	INIT_WORK(&dev_priv->mm.free_work, __i915_gem_free_work);
>  	init_llist_head(&dev_priv->mm.free_list);
>  	INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
> @@ -4978,6 +5057,10 @@ void i915_gem_load_cleanup(struct drm_i915_private *dev_priv)
>  	WARN_ON(!list_empty(&dev_priv->gt.timelines));
>  	mutex_unlock(&dev_priv->drm.struct_mutex);
>  
> +	while (flush_delayed_work(&dev_priv->mm.gtt_wakeref.work))
> +		;
> +	WARN_ON(atomic_read(&dev_priv->mm.gtt_wakeref.count) != -1);
> +
>  	kmem_cache_destroy(dev_priv->priorities);
>  	kmem_cache_destroy(dev_priv->dependencies);
>  	kmem_cache_destroy(dev_priv->requests);
> diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
> index c30d8f808185..3ca13edf32b6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_object.h
> +++ b/drivers/gpu/drm/i915/i915_gem_object.h
> @@ -177,6 +177,8 @@ struct drm_i915_gem_object {
>  			struct mutex lock; /* protects this cache */
>  		} get_page;
>  
> +		struct list_head gtt_wakeref_link;
> +
>  		/**
>  		 * Advice: are the backing pages purgeable?
>  		 */
> @@ -421,5 +423,8 @@ void i915_gem_object_set_cache_coherency(struct drm_i915_gem_object *obj,
>  					 unsigned int cache_level);
>  void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj);
>  
> +void
> +flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains);
> +
>  #endif
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 77fb39808131..71110b7d3ca0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -127,8 +127,10 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
>  
>  static bool unsafe_drop_pages(struct drm_i915_gem_object *obj)
>  {
> -	if (i915_gem_object_unbind(obj) == 0)
> +	if (i915_gem_object_unbind(obj) == 0) {
> +		flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);
>  		__i915_gem_object_put_pages(obj, I915_MM_SHRINKER);
> +	}
>  	return !READ_ONCE(obj->mm.pages);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d89e1b8e1cc5..357eee6f907c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -866,6 +866,7 @@ execlists_context_pin(struct intel_engine_cs *engine,
>  		i915_ggtt_offset(ce->ring->vma);
>  
>  	ce->state->obj->mm.dirty = true;
> +	flush_write_domain(ce->state->obj, ~0);
>  
>  	i915_gem_context_get(ctx);
>  out:
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index cdf084ef5aae..571a5b1f4f54 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1321,6 +1321,8 @@ int intel_ring_pin(struct intel_ring *ring,
>  	if (IS_ERR(addr))
>  		goto err;
>  
> +	flush_write_domain(vma->obj, ~0);
> +
>  	ring->vaddr = addr;
>  	return 0;
>  
> @@ -1516,6 +1518,7 @@ intel_ring_context_pin(struct intel_engine_cs *engine,
>  			goto err;
>  
>  		ce->state->obj->mm.dirty = true;
> +		flush_write_domain(ce->state->obj, ~0);
>  	}
>  
>  	/* The kernel context is only used as a placeholder for flushing the
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-09-04  8:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-31  9:15 [PATCH] drm/i915: Keep the device awake whilst in the GTT write domain Chris Wilson
2017-08-31 10:39 ` [PATCH v2] " Chris Wilson
2017-08-31 12:12 ` ✗ Fi.CI.BAT: failure for drm/i915: Keep the device awake whilst in the GTT write domain (rev2) Patchwork
2017-08-31 14:20 ` [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain Chris Wilson
2017-08-31 14:34 ` ✗ Fi.CI.BAT: failure for drm/i915: Keep the device awake whilst in the GTT write domain (rev3) Patchwork
2017-09-01 12:16 ` [PATCH v3] drm/i915: Keep the device awake whilst in the GTT domain Chris Wilson
2017-09-04  8:16   ` Daniel Vetter
2017-09-01 12:45 ` ✗ Fi.CI.BAT: warning for drm/i915: Keep the device awake whilst in the GTT write domain (rev4) Patchwork

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.