All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation
@ 2016-08-25  9:15 Chris Wilson
  2016-08-25  9:15 ` [PATCH v2 2/2] drm/i915: Shrink objects prior to hibernation Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Chris Wilson @ 2016-08-25  9:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala, David Weinehall

Recently I have been applying an optimisation to avoid stalling and
clflushing GGTT objects based on their current binding. That is we only
set-to-gtt-domain upon first bind. However, on hibernation the objects
remain bound, but they are in the CPU domain. Currently (since commit
975f7ff42edf ("drm/i915: Lazily migrate the objects after hibernation"))
we only flush scanout objects as all other objects are expected to be
flushed prior to use. That breaks down in the face of the runtime
optimisation above - and we need to flush all GGTT pinned objects
(essentially ringbuffers).

To reduce the burden of extra clflushes, we only flush those objects we
cannot discard from the GGTT. Everything pinned to the scanout, or
current contexts or ringbuffers will be flushed and rebound. Other
objects, such as inactive contexts, will be left unbound and in the CPU
domain until first use after resuming.

Fixes: 7abc98fadfdd ("drm/i915: Only change the context object's domain...")
Fixes: 57e885318119 ("drm/i915: Use VMA for ringbuffer tracking")
References: https://bugs.freedesktop.org/show_bug.cgi?id=94722
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 570e7311a419..72d03127dec4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3234,7 +3234,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
-	struct drm_i915_gem_object *obj;
+	struct drm_i915_gem_object *obj, *on;
 	struct i915_vma *vma;
 
 	i915_check_and_clear_faults(dev_priv);
@@ -3243,20 +3243,31 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 	ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total,
 			       true);
 
+	ggtt->base.closed = true; /* skip rewriting PTE on VMA unbind */
+
 	/* Cache flush objects bound into GGTT and rebind them. */
-	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
+	list_for_each_entry_safe(obj, on,
+				 &dev_priv->mm.bound_list, global_list) {
+		bool ggtt_bound = false;
+
 		list_for_each_entry(vma, &obj->vma_list, obj_link) {
 			if (vma->vm != &ggtt->base)
 				continue;
 
+			if (!i915_vma_unbind(vma))
+				continue;
+
 			WARN_ON(i915_vma_bind(vma, obj->cache_level,
 					      PIN_UPDATE));
+			ggtt_bound = true;
 		}
 
-		if (obj->pin_display)
+		if (ggtt_bound)
 			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
 	}
 
+	ggtt->base.closed = false;
+
 	if (INTEL_INFO(dev)->gen >= 8) {
 		if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev))
 			chv_setup_private_ppat(dev_priv);
-- 
2.9.3

_______________________________________________
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

* [PATCH v2 2/2] drm/i915: Shrink objects prior to hibernation
  2016-08-25  9:15 [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation Chris Wilson
@ 2016-08-25  9:15 ` Chris Wilson
  2016-08-25 11:24   ` Joonas Lahtinen
  2016-08-25 10:50 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-08-25  9:15 UTC (permalink / raw)
  To: intel-gfx

In an attempt to keep the hibernation image as same as possible, let's
try and discard any unwanted pages and our own page arrays.

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

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 04607d4115d6..a04c91e83aad 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4587,6 +4587,11 @@ void i915_gem_load_cleanup(struct drm_device *dev)
 int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
 {
 	struct drm_i915_gem_object *obj;
+	struct list_head *phases[] = {
+		&dev_priv->mm.unbound_list,
+		&dev_priv->mm.bound_list,
+		NULL
+	}, **p;
 
 	/* Called just before we write the hibernation image.
 	 *
@@ -4597,16 +4602,18 @@ int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
 	 *
 	 * To make sure the hibernation image contains the latest state,
 	 * we update that state just before writing out the image.
+	 *
+	 * To try and reduce the hibernation image, we manually shrink
+	 * the objects as well.
 	 */
 
-	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list) {
-		obj->base.read_domains = I915_GEM_DOMAIN_CPU;
-		obj->base.write_domain = I915_GEM_DOMAIN_CPU;
-	}
+	i915_gem_shrink_all(dev_priv);
 
-	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-		obj->base.read_domains = I915_GEM_DOMAIN_CPU;
-		obj->base.write_domain = I915_GEM_DOMAIN_CPU;
+	for (p = phases; *p; p++) {
+		list_for_each_entry(obj, *p, global_list) {
+			obj->base.read_domains = I915_GEM_DOMAIN_CPU;
+			obj->base.write_domain = I915_GEM_DOMAIN_CPU;
+		}
 	}
 
 	return 0;
-- 
2.9.3

_______________________________________________
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 [v2,1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation
  2016-08-25  9:15 [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation Chris Wilson
  2016-08-25  9:15 ` [PATCH v2 2/2] drm/i915: Shrink objects prior to hibernation Chris Wilson
@ 2016-08-25 10:50 ` Patchwork
  2016-09-09 19:06 ` [PATCH v2 1/2] " Chris Wilson
  2016-09-09 19:41 ` Matthew Auld
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-08-25 10:50 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation
URL   : https://patchwork.freedesktop.org/series/11557/
State : failure

== Summary ==

Series 11557v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/11557/revisions/1/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test gem_basic:
        Subgroup bad-close:
                dmesg-warn -> PASS       (fi-snb-2520m)
Test gem_ringfill:
        Subgroup basic-default-forked:
                dmesg-warn -> PASS       (fi-snb-2520m)
        Subgroup basic-default-hang:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> INCOMPLETE (fi-hsw-4770k)

fi-bdw-5557u     total:252  pass:235  dwarn:0   dfail:0   fail:2   skip:15 
fi-bsw-n3050     total:252  pass:205  dwarn:0   dfail:0   fail:1   skip:46 
fi-byt-n2820     total:252  pass:207  dwarn:0   dfail:0   fail:3   skip:42 
fi-hsw-4770k     total:204  pass:183  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:252  pass:224  dwarn:0   dfail:0   fail:2   skip:26 
fi-ivb-3520m     total:252  pass:220  dwarn:0   dfail:0   fail:1   skip:31 
fi-skl-6260u     total:252  pass:236  dwarn:0   dfail:0   fail:2   skip:14 
fi-skl-6700k     total:252  pass:222  dwarn:0   dfail:0   fail:2   skip:28 
fi-snb-2520m     total:252  pass:199  dwarn:8   dfail:0   fail:2   skip:43 
fi-snb-2600      total:252  pass:199  dwarn:8   dfail:0   fail:2   skip:43 

Results at /archive/results/CI_IGT_test/Patchwork_2428/

b3add877e4b0020293e2355930d1104bac1f498f drm-intel-nightly: 2016y-08m-25d-09h-25m-36s UTC integration manifest
9322ffb drm/i915: Shrink objects prior to hibernation
9812aa2 drm/i915: Flush to GTT domain all GGTT bound objects after hibernation

_______________________________________________
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

* Re: [PATCH v2 2/2] drm/i915: Shrink objects prior to hibernation
  2016-08-25  9:15 ` [PATCH v2 2/2] drm/i915: Shrink objects prior to hibernation Chris Wilson
@ 2016-08-25 11:24   ` Joonas Lahtinen
  0 siblings, 0 replies; 6+ messages in thread
From: Joonas Lahtinen @ 2016-08-25 11:24 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On to, 2016-08-25 at 10:15 +0100, Chris Wilson wrote:
> In an attempt to keep the hibernation image as same as possible, let's
> try and discard any unwanted pages and our own page arrays.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

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

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

* Re: [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation
  2016-08-25  9:15 [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation Chris Wilson
  2016-08-25  9:15 ` [PATCH v2 2/2] drm/i915: Shrink objects prior to hibernation Chris Wilson
  2016-08-25 10:50 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation Patchwork
@ 2016-09-09 19:06 ` Chris Wilson
  2016-09-09 19:41 ` Matthew Auld
  3 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2016-09-09 19:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: David Weinehall, Mika Kuoppala

On Thu, Aug 25, 2016 at 10:15:40AM +0100, Chris Wilson wrote:
> Recently I have been applying an optimisation to avoid stalling and
> clflushing GGTT objects based on their current binding. That is we only
> set-to-gtt-domain upon first bind. However, on hibernation the objects
> remain bound, but they are in the CPU domain. Currently (since commit
> 975f7ff42edf ("drm/i915: Lazily migrate the objects after hibernation"))
> we only flush scanout objects as all other objects are expected to be
> flushed prior to use. That breaks down in the face of the runtime
> optimisation above - and we need to flush all GGTT pinned objects
> (essentially ringbuffers).
> 
> To reduce the burden of extra clflushes, we only flush those objects we
> cannot discard from the GGTT. Everything pinned to the scanout, or
> current contexts or ringbuffers will be flushed and rebound. Other
> objects, such as inactive contexts, will be left unbound and in the CPU
> domain until first use after resuming.
> 
> Fixes: 7abc98fadfdd ("drm/i915: Only change the context object's domain...")
> Fixes: 57e885318119 ("drm/i915: Use VMA for ringbuffer tracking")
> References: https://bugs.freedesktop.org/show_bug.cgi?id=94722
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>

Any takers? There's a small risk of failure after hibernate on Baytrail.
(The effect is most likely limited to contexts on !llc, but even then is
going to be rare I think.)

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 570e7311a419..72d03127dec4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3234,7 +3234,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> -	struct drm_i915_gem_object *obj;
> +	struct drm_i915_gem_object *obj, *on;
>  	struct i915_vma *vma;
>  
>  	i915_check_and_clear_faults(dev_priv);
> @@ -3243,20 +3243,31 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  	ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total,
>  			       true);
>  
> +	ggtt->base.closed = true; /* skip rewriting PTE on VMA unbind */
> +
>  	/* Cache flush objects bound into GGTT and rebind them. */
> -	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> +	list_for_each_entry_safe(obj, on,
> +				 &dev_priv->mm.bound_list, global_list) {
> +		bool ggtt_bound = false;
> +
>  		list_for_each_entry(vma, &obj->vma_list, obj_link) {
>  			if (vma->vm != &ggtt->base)
>  				continue;
>  
> +			if (!i915_vma_unbind(vma))
> +				continue;
> +
>  			WARN_ON(i915_vma_bind(vma, obj->cache_level,
>  					      PIN_UPDATE));
> +			ggtt_bound = true;
>  		}
>  
> -		if (obj->pin_display)
> +		if (ggtt_bound)
>  			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
>  	}
>  
> +	ggtt->base.closed = false;
> +
>  	if (INTEL_INFO(dev)->gen >= 8) {
>  		if (IS_CHERRYVIEW(dev) || IS_BROXTON(dev))
>  			chv_setup_private_ppat(dev_priv);
> -- 
> 2.9.3
> 

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
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

* Re: [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation
  2016-08-25  9:15 [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation Chris Wilson
                   ` (2 preceding siblings ...)
  2016-09-09 19:06 ` [PATCH v2 1/2] " Chris Wilson
@ 2016-09-09 19:41 ` Matthew Auld
  3 siblings, 0 replies; 6+ messages in thread
From: Matthew Auld @ 2016-09-09 19:41 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Intel Graphics Development, David Weinehall, Mika Kuoppala

On 25 August 2016 at 10:15, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Recently I have been applying an optimisation to avoid stalling and
> clflushing GGTT objects based on their current binding. That is we only
> set-to-gtt-domain upon first bind. However, on hibernation the objects
> remain bound, but they are in the CPU domain. Currently (since commit
> 975f7ff42edf ("drm/i915: Lazily migrate the objects after hibernation"))
> we only flush scanout objects as all other objects are expected to be
> flushed prior to use. That breaks down in the face of the runtime
> optimisation above - and we need to flush all GGTT pinned objects
> (essentially ringbuffers).
>
> To reduce the burden of extra clflushes, we only flush those objects we
> cannot discard from the GGTT. Everything pinned to the scanout, or
> current contexts or ringbuffers will be flushed and rebound. Other
> objects, such as inactive contexts, will be left unbound and in the CPU
> domain until first use after resuming.
>
> Fixes: 7abc98fadfdd ("drm/i915: Only change the context object's domain...")
> Fixes: 57e885318119 ("drm/i915: Use VMA for ringbuffer tracking")
> References: https://bugs.freedesktop.org/show_bug.cgi?id=94722
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 570e7311a419..72d03127dec4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3234,7 +3234,7 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = to_i915(dev);
>         struct i915_ggtt *ggtt = &dev_priv->ggtt;
> -       struct drm_i915_gem_object *obj;
> +       struct drm_i915_gem_object *obj, *on;
>         struct i915_vma *vma;
An opportune time to make the vma local.

Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2016-09-09 19:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-25  9:15 [PATCH v2 1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation Chris Wilson
2016-08-25  9:15 ` [PATCH v2 2/2] drm/i915: Shrink objects prior to hibernation Chris Wilson
2016-08-25 11:24   ` Joonas Lahtinen
2016-08-25 10:50 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915: Flush to GTT domain all GGTT bound objects after hibernation Patchwork
2016-09-09 19:06 ` [PATCH v2 1/2] " Chris Wilson
2016-09-09 19:41 ` Matthew Auld

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.