Speed up resume by focused clflushing

Speed up resume by focused clflushing

[Chris Wilson]

If we track the domain changes from hiberation, we can forgo the forced
flushing of objects inside i915_gem_restore_gtt_mappings() and instead
rely on lazy flushes of objects via the normal domain handling. The
caveat are objects currently in use by the hardware, which need to be
coherent upon resume for the ongoing access.
-Chris

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

[PATCH 1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Chris Wilson]

Currently for handling the extra hibernation phases we just call the
equivalent suspend/resume phases. In the next couple of patches, I wish
to specialise the hibernation phases to reduce the amount of work
required for handling GEM objects.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5ae79601335c..4c17e3e8d0ab 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1115,6 +1115,32 @@ static int i915_pm_resume(struct device *dev)
 	return i915_drm_resume(drm_dev);
 }
 
+/* thaw and thaw_early are called after creating the hibernation image,
+ * but before turning off.
+ */
+static int i915_pm_thaw_early(struct device *dev)
+{
+	return i915_pm_resume_early(dev);
+}
+
+static int i915_pm_thaw(struct device *dev)
+{
+	return i915_pm_resume(dev);
+}
+
+/* restore and restore_early are called when booting from the hibernation
+ * image.
+ */
+static int i915_pm_restore_early(struct device *dev)
+{
+	return i915_pm_resume_early(dev);
+}
+
+static int i915_pm_restore(struct device *dev)
+{
+	return i915_pm_resume(dev);
+}
+
 /*
  * Save all Gunit registers that may be lost after a D3 and a subsequent
  * S0i[R123] transition. The list of registers needing a save/restore is
@@ -1671,12 +1697,12 @@ static const struct dev_pm_ops i915_pm_ops = {
 	 */
 	.freeze = i915_pm_suspend,
 	.freeze_late = i915_pm_suspend_late,
-	.thaw_early = i915_pm_resume_early,
-	.thaw = i915_pm_resume,
+	.thaw_early = i915_pm_thaw_early,
+	.thaw = i915_pm_thaw,
 	.poweroff = i915_pm_suspend,
 	.poweroff_late = i915_pm_poweroff_late,
-	.restore_early = i915_pm_resume_early,
-	.restore = i915_pm_resume,
+	.restore_early = i915_pm_restore_early,
+	.restore = i915_pm_restore,
 
 	/* S0ix (via runtime suspend) event handlers */
 	.runtime_suspend = intel_runtime_suspend,
-- 
2.8.1

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

Re: [PATCH 1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Imre Deak]

On Thu, 2016-05-12 at 12:41 +0100, Chris Wilson wrote:
> Currently for handling the extra hibernation phases we just call the
> equivalent suspend/resume phases. In the next couple of patches, I wish
> to specialise the hibernation phases to reduce the amount of work
> required for handling GEM objects.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 5ae79601335c..4c17e3e8d0ab 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1115,6 +1115,32 @@ static int i915_pm_resume(struct device *dev)
>  	return i915_drm_resume(drm_dev);
>  }
>  
> +/* thaw and thaw_early are called after creating the hibernation image,
> + * but before turning off.
> + */

We have these explained for all the phases already around i915_pm_ops.

Also would be good to know if you chose restore over freeze to set the
object domains. I don't see a practical problem with it, but freeze
would be a more proper place logically.

> +static int i915_pm_thaw_early(struct device *dev)
> +{
> +	return i915_pm_resume_early(dev);
> +}
> +
> +static int i915_pm_thaw(struct device *dev)
> +{
> +	return i915_pm_resume(dev);
> +}
> +
> +/* restore and restore_early are called when booting from the hibernation
> + * image.
> + */
> +static int i915_pm_restore_early(struct device *dev)
> +{
> +	return i915_pm_resume_early(dev);
> +}
> +
> +static int i915_pm_restore(struct device *dev)
> +{
> +	return i915_pm_resume(dev);
> +}
> +
>  /*
>   * Save all Gunit registers that may be lost after a D3 and a subsequent
>   * S0i[R123] transition. The list of registers needing a save/restore is
> @@ -1671,12 +1697,12 @@ static const struct dev_pm_ops i915_pm_ops = {
>  	 */
>  	.freeze = i915_pm_suspend,
>  	.freeze_late = i915_pm_suspend_late,
> -	.thaw_early = i915_pm_resume_early,
> -	.thaw = i915_pm_resume,
> +	.thaw_early = i915_pm_thaw_early,
> +	.thaw = i915_pm_thaw,
>  	.poweroff = i915_pm_suspend,
>  	.poweroff_late = i915_pm_poweroff_late,
> -	.restore_early = i915_pm_resume_early,
> -	.restore = i915_pm_resume,
> +	.restore_early = i915_pm_restore_early,
> +	.restore = i915_pm_restore,
>  
>  	/* S0ix (via runtime suspend) event handlers */
>  	.runtime_suspend = intel_runtime_suspend,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Chris Wilson]

On Thu, May 12, 2016 at 02:52:55PM +0300, Imre Deak wrote:
> On Thu, 2016-05-12 at 12:41 +0100, Chris Wilson wrote:
> > Currently for handling the extra hibernation phases we just call the
> > equivalent suspend/resume phases. In the next couple of patches, I wish
> > to specialise the hibernation phases to reduce the amount of work
> > required for handling GEM objects.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: David Weinehall <david.weinehall@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 34 ++++++++++++++++++++++++++++++----
> >  1 file changed, 30 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 5ae79601335c..4c17e3e8d0ab 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1115,6 +1115,32 @@ static int i915_pm_resume(struct device *dev)
> >  	return i915_drm_resume(drm_dev);
> >  }
> >  
> > +/* thaw and thaw_early are called after creating the hibernation image,
> > + * but before turning off.
> > + */
> 
> We have these explained for all the phases already around i915_pm_ops.

Yes. I was adding little reminders so that when reading the code for
each phase that information was close to hand.
 
> Also would be good to know if you chose restore over freeze to set the
> object domains. I don't see a practical problem with it, but freeze
> would be a more proper place logically.

Ok, lets have a look at freeze.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm/i915: Update domain tracking for GEM objects on hibernation

[Chris Wilson]

When creating the hibernation image, the CPU will read the pages of all
objects and thus conflict with our domain tracking. We need to update
our domain tracking to accurately reflect the state on restoration.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_gem.c | 23 +++++++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4c17e3e8d0ab..978f9f72a55c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1138,7 +1138,17 @@ static int i915_pm_restore_early(struct device *dev)
 
 static int i915_pm_restore(struct device *dev)
 {
-	return i915_pm_resume(dev);
+	int ret;
+
+	ret = i915_gem_restore(dev_to_i915(dev));
+	if (ret)
+		return ret;
+
+	ret = i915_pm_resume(dev);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7a0b51301337..54a468a76f69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2961,6 +2961,8 @@ int i915_gem_wait_ioctl(struct drm_device *dev, void *data,
 void i915_gem_load_init(struct drm_device *dev);
 void i915_gem_load_cleanup(struct drm_device *dev);
 void i915_gem_load_init_fences(struct drm_i915_private *dev_priv);
+int i915_gem_restore(struct drm_i915_private *dev_priv);
+
 void *i915_gem_object_alloc(struct drm_device *dev);
 void i915_gem_object_free(struct drm_i915_gem_object *obj);
 void i915_gem_object_init(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index be75689455b2..1e3872ab9175 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5053,6 +5053,29 @@ void i915_gem_load_cleanup(struct drm_device *dev)
 	kmem_cache_destroy(dev_priv->objects);
 }
 
+int i915_gem_restore(struct drm_i915_private *dev_priv)
+{
+	struct drm_i915_gem_object *obj;
+
+	/* Called after we have been loaded from the hibernation image.
+	 *
+	 * We need to update the domain tracking to reflect that the CPU
+	 * has just accessed all the pages.
+	 */
+
+	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;
+	}
+
+	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;
+	}
+
+	return 0;
+}
+
 void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
-- 
2.8.1

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

[PATCH 3/4] drm/i915: Lazily migrate the objects after hibernation

[Chris Wilson]

Now that we mark the object domains for having been restored from the
hibernation image, we not need to flush everything during resume and
can instead rely on the normal domain tracking to flush only when
required. The only caveat here are objects that are pinned for use by
the hardware, whose contents must be coherent for when the device
resumes reading from then (shortly afterwards with the driver assuming
the objects are in the correct domain).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5fb14c835543..319f3b459b3e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3245,7 +3245,6 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
-	bool flush;
 
 	i915_check_and_clear_faults(dev_priv);
 
@@ -3255,19 +3254,16 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 
 	/* Cache flush objects bound into GGTT and rebind them. */
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-		flush = false;
 		list_for_each_entry(vma, &obj->vma_list, obj_link) {
 			if (vma->vm != &ggtt->base)
 				continue;
 
 			WARN_ON(i915_vma_bind(vma, obj->cache_level,
 					      PIN_UPDATE));
-
-			flush = true;
 		}
 
-		if (flush)
-			i915_gem_clflush_object(obj, obj->pin_display);
+		if (obj->pin_display)
+			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
 	}
 
 	if (INTEL_INFO(dev)->gen >= 8) {
-- 
2.8.1

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

[PATCH 4/4] drm/i915: Skip clearing the GGTT on full-ppgtt systems

[Chris Wilson]

Under full-ppgtt, access to the global GTT is carefully regulated
through hardware functions (i.e. userspace cannot read and write to
arbitrary locations in the GGTT via the GPU). With this restriction in
place, we can forgo clearing stale entries from the GGTT as they will
not be accessed.

For aliasing-ppgtt, we could almost do the same except that we do allow
userspace access to the global-GTT via execbuf in order to workraound
some quirks of certain instructions. (This execbuf path is filtered out
with EINVAL on full-ppgtt.)

The most dramatic effect this will have will be during resume, as with
full-ppgtt the GGTT is only used sparingly.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 319f3b459b3e..7eab619a3eb2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
 }
 
+static void nop_clear_range(struct i915_address_space *vm,
+			    uint64_t start,
+			    uint64_t length,
+			    bool use_scratch)
+{
+}
+
 static void gen8_ggtt_clear_range(struct i915_address_space *vm,
 				  uint64_t start,
 				  uint64_t length,
@@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
 	ret = ggtt_probe_common(dev, ggtt->size);
 
-	ggtt->base.clear_range = gen8_ggtt_clear_range;
-	if (IS_CHERRYVIEW(dev_priv))
-		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
-	else
-		ggtt->base.insert_entries = gen8_ggtt_insert_entries;
 	ggtt->base.bind_vma = ggtt_bind_vma;
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
 
+	ggtt->base.clear_range = nop_clear_range;
+	if (!USES_FULL_PPGTT(dev_priv))
+		ggtt->base.clear_range = gen8_ggtt_clear_range;
+
+	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
+	if (IS_CHERRYVIEW(dev_priv))
+		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
+
 	return ret;
 }
 
-- 
2.8.1

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

✓ Ro.CI.BAT: success for series starting with [1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Add distinct stubs for PM hibernation phases
URL   : https://patchwork.freedesktop.org/series/7063/
State : success

== Summary ==

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


fi-skl-i7-6700k  total:219  pass:191  dwarn:0   dfail:0   fail:0   skip:28 
fi-bdw-i7-5557u failed to connect after reboot
fi-bsw-n3050 failed to connect after reboot
fi-byt-n2820 failed to connect after reboot
fi-hsw-i7-4770k failed to connect after reboot
fi-hsw-i7-4770r failed to connect after reboot
fi-kbl-y failed to connect after reboot
fi-skl-i5-6260u failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot
ro-bdw-i5-5250u failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot
ro-bdw-i7-5600u failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot
ro-byt-n2820 failed to connect after reboot
ro-hsw-i3-4010u failed to connect after reboot
ro-hsw-i7-4770r failed to connect after reboot
ro-ilk1-i5-650 failed to connect after reboot
ro-ilk-i7-620lm failed to connect after reboot
ro-ivb2-i7-3770 failed to connect after reboot
ro-ivb-i7-3770 failed to connect after reboot
ro-skl-i7-6700hq failed to connect after reboot
ro-snb-i7-2620M failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_860/

dd7de1c drm-intel-nightly: 2016y-05m-12d-11h-29m-40s UTC integration manifest
131d299 drm/i915: Skip clearing the GGTT on full-ppgtt systems
3ccff42 drm/i915: Lazily migrate the objects after hibernation
0cc9231 drm/i915: Update domain tracking for GEM objects on hibernation
54c2bf2 drm/i915: Add distinct stubs for PM hibernation phases

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

Re: ✓ Ro.CI.BAT: success for series starting with [1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Chris Wilson]

On Thu, May 12, 2016 at 12:27:40PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/4] drm/i915: Add distinct stubs for PM hibernation phases
> URL   : https://patchwork.freedesktop.org/series/7063/
> State : success
> 
> == Summary ==
> 
> Series 7063v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/7063/revisions/1/mbox
> 
> 
> fi-skl-i7-6700k  total:219  pass:191  dwarn:0   dfail:0   fail:0   skip:28 

Thanks skl!

> fi-bdw-i7-5557u failed to connect after reboot
> fi-bsw-n3050 failed to connect after reboot
> fi-byt-n2820 failed to connect after reboot
> fi-hsw-i7-4770k failed to connect after reboot
> fi-hsw-i7-4770r failed to connect after reboot
> fi-kbl-y failed to connect after reboot
> fi-skl-i5-6260u failed to connect after reboot
> fi-snb-i7-2600 failed to connect after reboot
> ro-bdw-i5-5250u failed to connect after reboot
> ro-bdw-i7-5557U failed to connect after reboot
> ro-bdw-i7-5600u failed to connect after reboot
> ro-bsw-n3050 failed to connect after reboot
> ro-byt-n2820 failed to connect after reboot
> ro-hsw-i3-4010u failed to connect after reboot
> ro-hsw-i7-4770r failed to connect after reboot
> ro-ilk1-i5-650 failed to connect after reboot
> ro-ilk-i7-620lm failed to connect after reboot
> ro-ivb2-i7-3770 failed to connect after reboot
> ro-ivb-i7-3770 failed to connect after reboot
> ro-skl-i7-6700hq failed to connect after reboot
> ro-snb-i7-2620M failed to connect after reboot

We need a new category for phyrric passes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Chris Wilson]

Currently for handling the extra hibernation phases we just call the
equivalent suspend/resume phases. In the next couple of patches, I wish
to specialise the hibernation phases to reduce the amount of work
required for handling GEM objects.

v2: There are more! Don't forget the freeze phases.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 45 +++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5ae79601335c..6a2e7f84276b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1115,6 +1115,39 @@ static int i915_pm_resume(struct device *dev)
 	return i915_drm_resume(drm_dev);
 }
 
+/* freeze: before creating the hibernation_image */
+static int i915_pm_freeze(struct device *dev)
+{
+	return i915_pm_suspend(dev);
+}
+
+static int i915_pm_freeze_late(struct device *dev)
+{
+	return i915_pm_suspend_late(dev);
+}
+
+/* thaw: called after creating the hibernation image, but before turning off. */
+static int i915_pm_thaw_early(struct device *dev)
+{
+	return i915_pm_resume_early(dev);
+}
+
+static int i915_pm_thaw(struct device *dev)
+{
+	return i915_pm_resume(dev);
+}
+
+/* restore: called after loading the hibernation image. */
+static int i915_pm_restore_early(struct device *dev)
+{
+	return i915_pm_resume_early(dev);
+}
+
+static int i915_pm_restore(struct device *dev)
+{
+	return i915_pm_resume(dev);
+}
+
 /*
  * Save all Gunit registers that may be lost after a D3 and a subsequent
  * S0i[R123] transition. The list of registers needing a save/restore is
@@ -1669,14 +1702,14 @@ static const struct dev_pm_ops i915_pm_ops = {
 	 * @restore, @restore_early : called after rebooting and restoring the
 	 *                            hibernation image [PMSG_RESTORE]
 	 */
-	.freeze = i915_pm_suspend,
-	.freeze_late = i915_pm_suspend_late,
-	.thaw_early = i915_pm_resume_early,
-	.thaw = i915_pm_resume,
+	.freeze = i915_pm_freeze,
+	.freeze_late = i915_pm_freeze_late,
+	.thaw_early = i915_pm_thaw_early,
+	.thaw = i915_pm_thaw,
 	.poweroff = i915_pm_suspend,
 	.poweroff_late = i915_pm_poweroff_late,
-	.restore_early = i915_pm_resume_early,
-	.restore = i915_pm_resume,
+	.restore_early = i915_pm_restore_early,
+	.restore = i915_pm_restore,
 
 	/* S0ix (via runtime suspend) event handlers */
 	.runtime_suspend = intel_runtime_suspend,
-- 
2.8.1

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

[PATCH v2 2/4] drm/i915: Update domain tracking for GEM objects on hibernation

[Chris Wilson]

When creating the hibernation image, the CPU will read the pages of all
objects and thus conflict with our domain tracking. We need to update
our domain tracking to accurately reflect the state on restoration.

v2: Perform the domain tracking inside freeze, before the image is
written, rather than upon restoration.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_gem.c | 28 ++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6a2e7f84276b..dba03c026151 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1123,7 +1123,17 @@ static int i915_pm_freeze(struct device *dev)
 
 static int i915_pm_freeze_late(struct device *dev)
 {
-	return i915_pm_suspend_late(dev);
+	int ret;
+
+	ret = i915_pm_suspend_late(dev);
+	if (ret)
+		return ret;
+
+	ret = i915_gem_freeze_late(dev_to_i915(dev));
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 /* thaw: called after creating the hibernation image, but before turning off. */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7a0b51301337..0a4255fa8895 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2961,6 +2961,8 @@ int i915_gem_wait_ioctl(struct drm_device *dev, void *data,
 void i915_gem_load_init(struct drm_device *dev);
 void i915_gem_load_cleanup(struct drm_device *dev);
 void i915_gem_load_init_fences(struct drm_i915_private *dev_priv);
+int i915_gem_freeze_late(struct drm_i915_private *dev_priv);
+
 void *i915_gem_object_alloc(struct drm_device *dev);
 void i915_gem_object_free(struct drm_i915_gem_object *obj);
 void i915_gem_object_init(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index be75689455b2..7bd670e34ee5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5053,6 +5053,34 @@ void i915_gem_load_cleanup(struct drm_device *dev)
 	kmem_cache_destroy(dev_priv->objects);
 }
 
+int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
+{
+	struct drm_i915_gem_object *obj;
+
+	/* Called just before we write the hibernation image.
+	 *
+	 * We need to update the domain tracking to reflect that the CPU
+	 * will be accessing all the pages to create and restore from the
+	 * hibernation, and so upon restoration those pages will be in the
+	 * CPU domain.
+	 *
+	 * To make sure the hibernation image contains the latest state,
+	 * we update that state just before writing out the image.
+	 */
+
+	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;
+	}
+
+	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;
+	}
+
+	return 0;
+}
+
 void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
-- 
2.8.1

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

Re: [PATCH v2 2/4] drm/i915: Update domain tracking for GEM objects on hibernation

[Joonas Lahtinen]

On to, 2016-05-12 at 15:28 +0100, Chris Wilson wrote:
> When creating the hibernation image, the CPU will read the pages of all
> objects and thus conflict with our domain tracking. We need to update
> our domain tracking to accurately reflect the state on restoration.
> 
> v2: Perform the domain tracking inside freeze, before the image is
> written, rather than upon restoration.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 12 +++++++++++-
>  drivers/gpu/drm/i915/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/i915_gem.c | 28 ++++++++++++++++++++++++++++
>  3 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 6a2e7f84276b..dba03c026151 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1123,7 +1123,17 @@ static int i915_pm_freeze(struct device *dev)
>  
>  static int i915_pm_freeze_late(struct device *dev)
>  {
> -	return i915_pm_suspend_late(dev);
> +	int ret;
> +
> +	ret = i915_pm_suspend_late(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = i915_gem_freeze_late(dev_to_i915(dev));
> +	if (ret)
> +		return ret;
> +
> +	return 0;
>  }
>  
>  /* thaw: called after creating the hibernation image, but before turning off. */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7a0b51301337..0a4255fa8895 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2961,6 +2961,8 @@ int i915_gem_wait_ioctl(struct drm_device *dev, void *data,
>  void i915_gem_load_init(struct drm_device *dev);
>  void i915_gem_load_cleanup(struct drm_device *dev);
>  void i915_gem_load_init_fences(struct drm_i915_private *dev_priv);
> +int i915_gem_freeze_late(struct drm_i915_private *dev_priv);
> +
>  void *i915_gem_object_alloc(struct drm_device *dev);
>  void i915_gem_object_free(struct drm_i915_gem_object *obj);
>  void i915_gem_object_init(struct drm_i915_gem_object *obj,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index be75689455b2..7bd670e34ee5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5053,6 +5053,34 @@ void i915_gem_load_cleanup(struct drm_device *dev)
>  	kmem_cache_destroy(dev_priv->objects);
>  }
>  
> +int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_i915_gem_object *obj;
> +
> +	/* Called just before we write the hibernation image.
> +	 *
> +	 * We need to update the domain tracking to reflect that the CPU
> +	 * will be accessing all the pages to create and restore from the
> +	 * hibernation, and so upon restoration those pages will be in the
> +	 * CPU domain.
> +	 *
> +	 * To make sure the hibernation image contains the latest state,
> +	 * we update that state just before writing out the image.
> +	 */
> +
> +	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;
> +	}
> +
> +	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;
> +	}
> +
> +	return 0;
> +}
> +
>  void i915_gem_release(struct drm_device *dev, struct drm_file *file)
>  {
>  	struct drm_i915_file_private *file_priv = file->driver_priv;
-- 
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

[PATCH v2 3/4] drm/i915: Lazily migrate the objects after hibernation

[Chris Wilson]

Now that we mark the object domains for having been restored from the
hibernation image, we not need to flush everything during resume and
can instead rely on the normal domain tracking to flush only when
required. The only caveat here are objects that are pinned for use by
the hardware, whose contents must be coherent for when the device
resumes reading from then (shortly afterwards with the driver assuming
the objects are in the correct domain).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5fb14c835543..319f3b459b3e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3245,7 +3245,6 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
-	bool flush;
 
 	i915_check_and_clear_faults(dev_priv);
 
@@ -3255,19 +3254,16 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 
 	/* Cache flush objects bound into GGTT and rebind them. */
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-		flush = false;
 		list_for_each_entry(vma, &obj->vma_list, obj_link) {
 			if (vma->vm != &ggtt->base)
 				continue;
 
 			WARN_ON(i915_vma_bind(vma, obj->cache_level,
 					      PIN_UPDATE));
-
-			flush = true;
 		}
 
-		if (flush)
-			i915_gem_clflush_object(obj, obj->pin_display);
+		if (obj->pin_display)
+			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
 	}
 
 	if (INTEL_INFO(dev)->gen >= 8) {
-- 
2.8.1

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

Re: [PATCH v2 3/4] drm/i915: Lazily migrate the objects after hibernation

[Joonas Lahtinen]

On to, 2016-05-12 at 15:28 +0100, Chris Wilson wrote:
> Now that we mark the object domains for having been restored from the
> hibernation image, we not need to flush everything during resume and
> can instead rely on the normal domain tracking to flush only when
> required. The only caveat here are objects that are pinned for use by
> the hardware, whose contents must be coherent for when the device
> resumes reading from then (shortly afterwards with the driver assuming
> the objects are in the correct domain).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 5fb14c835543..319f3b459b3e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3245,7 +3245,6 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>  	struct drm_i915_gem_object *obj;
>  	struct i915_vma *vma;
> -	bool flush;
>  
>  	i915_check_and_clear_faults(dev_priv);
>  
> @@ -3255,19 +3254,16 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  
>  	/* Cache flush objects bound into GGTT and rebind them. */
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> -		flush = false;
>  		list_for_each_entry(vma, &obj->vma_list, obj_link) {
>  			if (vma->vm != &ggtt->base)
>  				continue;
>  
>  			WARN_ON(i915_vma_bind(vma, obj->cache_level,
>  					      PIN_UPDATE));
> -
> -			flush = true;
>  		}
>  
> -		if (flush)
> -			i915_gem_clflush_object(obj, obj->pin_display);
> +		if (obj->pin_display)
> +			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
>  	}
>  
>  	if (INTEL_INFO(dev)->gen >= 8) {
-- 
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 v2 3/4] drm/i915: Lazily migrate the objects after hibernation

[David Weinehall]

On Thu, May 12, 2016 at 03:28:15PM +0100, Chris Wilson wrote:
> Now that we mark the object domains for having been restored from the
> hibernation image, we not need to flush everything during resume and
> can instead rely on the normal domain tracking to flush only when
> required. The only caveat here are objects that are pinned for use by
> the hardware, whose contents must be coherent for when the device
> resumes reading from then (shortly afterwards with the driver assuming
> the objects are in the correct domain).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>

I've tested patch 3 + 4 with SuspendResume
(analyze_suspend.py -config suspend-callgraph.cfg).
The takeaway is that restore_gtt (almost) disappears
in the noise.  Nice improvement.

I've also confirmed that hibernate works properly.


Tested-by: David Weinehall <david.weinehall@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 5fb14c835543..319f3b459b3e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3245,7 +3245,6 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>  	struct drm_i915_gem_object *obj;
>  	struct i915_vma *vma;
> -	bool flush;
>  
>  	i915_check_and_clear_faults(dev_priv);
>  
> @@ -3255,19 +3254,16 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
>  
>  	/* Cache flush objects bound into GGTT and rebind them. */
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> -		flush = false;
>  		list_for_each_entry(vma, &obj->vma_list, obj_link) {
>  			if (vma->vm != &ggtt->base)
>  				continue;
>  
>  			WARN_ON(i915_vma_bind(vma, obj->cache_level,
>  					      PIN_UPDATE));
> -
> -			flush = true;
>  		}
>  
> -		if (flush)
> -			i915_gem_clflush_object(obj, obj->pin_display);
> +		if (obj->pin_display)
> +			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
>  	}
>  
>  	if (INTEL_INFO(dev)->gen >= 8) {
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 4/4] drm/i915: Skip clearing the GGTT on full-ppgtt systems

[Chris Wilson]

Under full-ppgtt, access to the global GTT is carefully regulated
through hardware functions (i.e. userspace cannot read and write to
arbitrary locations in the GGTT via the GPU). With this restriction in
place, we can forgo clearing stale entries from the GGTT as they will
not be accessed.

For aliasing-ppgtt, we could almost do the same except that we do allow
userspace access to the global-GTT via execbuf in order to workraound
some quirks of certain instructions. (This execbuf path is filtered out
with EINVAL on full-ppgtt.)

The most dramatic effect this will have will be during resume, as with
full-ppgtt the GGTT is only used sparingly.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 319f3b459b3e..7eab619a3eb2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
 }
 
+static void nop_clear_range(struct i915_address_space *vm,
+			    uint64_t start,
+			    uint64_t length,
+			    bool use_scratch)
+{
+}
+
 static void gen8_ggtt_clear_range(struct i915_address_space *vm,
 				  uint64_t start,
 				  uint64_t length,
@@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
 	ret = ggtt_probe_common(dev, ggtt->size);
 
-	ggtt->base.clear_range = gen8_ggtt_clear_range;
-	if (IS_CHERRYVIEW(dev_priv))
-		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
-	else
-		ggtt->base.insert_entries = gen8_ggtt_insert_entries;
 	ggtt->base.bind_vma = ggtt_bind_vma;
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
 
+	ggtt->base.clear_range = nop_clear_range;
+	if (!USES_FULL_PPGTT(dev_priv))
+		ggtt->base.clear_range = gen8_ggtt_clear_range;
+
+	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
+	if (IS_CHERRYVIEW(dev_priv))
+		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
+
 	return ret;
 }
 
-- 
2.8.1

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

Re: [PATCH v2 4/4] drm/i915: Skip clearing the GGTT on full-ppgtt systems

[Joonas Lahtinen]

On to, 2016-05-12 at 15:28 +0100, Chris Wilson wrote:
> Under full-ppgtt, access to the global GTT is carefully regulated
> through hardware functions (i.e. userspace cannot read and write to
> arbitrary locations in the GGTT via the GPU). With this restriction in
> place, we can forgo clearing stale entries from the GGTT as they will
> not be accessed.
> 
> For aliasing-ppgtt, we could almost do the same except that we do allow
> userspace access to the global-GTT via execbuf in order to workraound
> some quirks of certain instructions. (This execbuf path is filtered out
> with EINVAL on full-ppgtt.)
> 
> The most dramatic effect this will have will be during resume, as with
> full-ppgtt the GGTT is only used sparingly.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 319f3b459b3e..7eab619a3eb2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
>  	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
>  }
>  
> +static void nop_clear_range(struct i915_address_space *vm,
> +			    uint64_t start,
> +			    uint64_t length,
> +			    bool use_scratch)
> +{
> +}
> +
>  static void gen8_ggtt_clear_range(struct i915_address_space *vm,
>  				  uint64_t start,
>  				  uint64_t length,
> @@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>  
>  	ret = ggtt_probe_common(dev, ggtt->size);
>  
> -	ggtt->base.clear_range = gen8_ggtt_clear_range;
> -	if (IS_CHERRYVIEW(dev_priv))
> -		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
> -	else
> -		ggtt->base.insert_entries = gen8_ggtt_insert_entries;
>  	ggtt->base.bind_vma = ggtt_bind_vma;
>  	ggtt->base.unbind_vma = ggtt_unbind_vma;
>  
> +	ggtt->base.clear_range = nop_clear_range;
> +	if (!USES_FULL_PPGTT(dev_priv))
> +		ggtt->base.clear_range = gen8_ggtt_clear_range;
> +
> +	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
> +	if (IS_CHERRYVIEW(dev_priv))
> +		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
> +
>  	return ret;
>  }
>  
-- 
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 v2 4/4] drm/i915: Skip clearing the GGTT on full-ppgtt systems

[David Weinehall]

On Thu, May 12, 2016 at 03:28:16PM +0100, Chris Wilson wrote:
> Under full-ppgtt, access to the global GTT is carefully regulated
> through hardware functions (i.e. userspace cannot read and write to
> arbitrary locations in the GGTT via the GPU). With this restriction in
> place, we can forgo clearing stale entries from the GGTT as they will
> not be accessed.
> 
> For aliasing-ppgtt, we could almost do the same except that we do allow
> userspace access to the global-GTT via execbuf in order to workraound
> some quirks of certain instructions. (This execbuf path is filtered out
> with EINVAL on full-ppgtt.)
> 
> The most dramatic effect this will have will be during resume, as with
> full-ppgtt the GGTT is only used sparingly.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@intel.com>

I've tested patch 3 + 4 with SuspendResume
(analyze_suspend.py -config suspend-callgraph.cfg).
The takeaway is that restore_gtt (almost) disappears
in the noise.  Nice improvement.

I've also confirmed that hibernate works properly.


Tested-by: David Weinehall <david.weinehall@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 319f3b459b3e..7eab619a3eb2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
>  	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
>  }
>  
> +static void nop_clear_range(struct i915_address_space *vm,
> +			    uint64_t start,
> +			    uint64_t length,
> +			    bool use_scratch)
> +{
> +}
> +
>  static void gen8_ggtt_clear_range(struct i915_address_space *vm,
>  				  uint64_t start,
>  				  uint64_t length,
> @@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>  
>  	ret = ggtt_probe_common(dev, ggtt->size);
>  
> -	ggtt->base.clear_range = gen8_ggtt_clear_range;
> -	if (IS_CHERRYVIEW(dev_priv))
> -		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
> -	else
> -		ggtt->base.insert_entries = gen8_ggtt_insert_entries;
>  	ggtt->base.bind_vma = ggtt_bind_vma;
>  	ggtt->base.unbind_vma = ggtt_unbind_vma;
>  
> +	ggtt->base.clear_range = nop_clear_range;
> +	if (!USES_FULL_PPGTT(dev_priv))
> +		ggtt->base.clear_range = gen8_ggtt_clear_range;
> +
> +	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
> +	if (IS_CHERRYVIEW(dev_priv))
> +		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
> +
>  	return ret;
>  }
>  
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Joonas Lahtinen]

On to, 2016-05-12 at 15:28 +0100, Chris Wilson wrote:
> Currently for handling the extra hibernation phases we just call the
> equivalent suspend/resume phases. In the next couple of patches, I wish
> to specialise the hibernation phases to reduce the amount of work
> required for handling GEM objects.
> 
> v2: There are more! Don't forget the freeze phases.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: David Weinehall <david.weinehall@intel.com>

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

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 45 +++++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 5ae79601335c..6a2e7f84276b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1115,6 +1115,39 @@ static int i915_pm_resume(struct device *dev)
>  	return i915_drm_resume(drm_dev);
>  }
>  
> +/* freeze: before creating the hibernation_image */
> +static int i915_pm_freeze(struct device *dev)
> +{
> +	return i915_pm_suspend(dev);
> +}
> +
> +static int i915_pm_freeze_late(struct device *dev)
> +{
> +	return i915_pm_suspend_late(dev);
> +}
> +
> +/* thaw: called after creating the hibernation image, but before turning off. */
> +static int i915_pm_thaw_early(struct device *dev)
> +{
> +	return i915_pm_resume_early(dev);
> +}
> +
> +static int i915_pm_thaw(struct device *dev)
> +{
> +	return i915_pm_resume(dev);
> +}
> +
> +/* restore: called after loading the hibernation image. */
> +static int i915_pm_restore_early(struct device *dev)
> +{
> +	return i915_pm_resume_early(dev);
> +}
> +
> +static int i915_pm_restore(struct device *dev)
> +{
> +	return i915_pm_resume(dev);
> +}
> +
>  /*
>   * Save all Gunit registers that may be lost after a D3 and a subsequent
>   * S0i[R123] transition. The list of registers needing a save/restore is
> @@ -1669,14 +1702,14 @@ static const struct dev_pm_ops i915_pm_ops = {
>  	 * @restore, @restore_early : called after rebooting and restoring the
>  	 *                            hibernation image [PMSG_RESTORE]
>  	 */
> -	.freeze = i915_pm_suspend,
> -	.freeze_late = i915_pm_suspend_late,
> -	.thaw_early = i915_pm_resume_early,
> -	.thaw = i915_pm_resume,
> +	.freeze = i915_pm_freeze,
> +	.freeze_late = i915_pm_freeze_late,
> +	.thaw_early = i915_pm_thaw_early,
> +	.thaw = i915_pm_thaw,
>  	.poweroff = i915_pm_suspend,
>  	.poweroff_late = i915_pm_poweroff_late,
> -	.restore_early = i915_pm_resume_early,
> -	.restore = i915_pm_resume,
> +	.restore_early = i915_pm_restore_early,
> +	.restore = i915_pm_restore,
>  
>  	/* S0ix (via runtime suspend) event handlers */
>  	.runtime_suspend = intel_runtime_suspend,
-- 
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

[CI 1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Chris Wilson]

Currently for handling the extra hibernation phases we just call the
equivalent suspend/resume phases. In the next couple of patches, I wish
to specialise the hibernation phases to reduce the amount of work
required for handling GEM objects.

v2: There are more! Don't forget the freeze phases.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 45 +++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5ae79601335c..6a2e7f84276b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1115,6 +1115,39 @@ static int i915_pm_resume(struct device *dev)
 	return i915_drm_resume(drm_dev);
 }
 
+/* freeze: before creating the hibernation_image */
+static int i915_pm_freeze(struct device *dev)
+{
+	return i915_pm_suspend(dev);
+}
+
+static int i915_pm_freeze_late(struct device *dev)
+{
+	return i915_pm_suspend_late(dev);
+}
+
+/* thaw: called after creating the hibernation image, but before turning off. */
+static int i915_pm_thaw_early(struct device *dev)
+{
+	return i915_pm_resume_early(dev);
+}
+
+static int i915_pm_thaw(struct device *dev)
+{
+	return i915_pm_resume(dev);
+}
+
+/* restore: called after loading the hibernation image. */
+static int i915_pm_restore_early(struct device *dev)
+{
+	return i915_pm_resume_early(dev);
+}
+
+static int i915_pm_restore(struct device *dev)
+{
+	return i915_pm_resume(dev);
+}
+
 /*
  * Save all Gunit registers that may be lost after a D3 and a subsequent
  * S0i[R123] transition. The list of registers needing a save/restore is
@@ -1669,14 +1702,14 @@ static const struct dev_pm_ops i915_pm_ops = {
 	 * @restore, @restore_early : called after rebooting and restoring the
 	 *                            hibernation image [PMSG_RESTORE]
 	 */
-	.freeze = i915_pm_suspend,
-	.freeze_late = i915_pm_suspend_late,
-	.thaw_early = i915_pm_resume_early,
-	.thaw = i915_pm_resume,
+	.freeze = i915_pm_freeze,
+	.freeze_late = i915_pm_freeze_late,
+	.thaw_early = i915_pm_thaw_early,
+	.thaw = i915_pm_thaw,
 	.poweroff = i915_pm_suspend,
 	.poweroff_late = i915_pm_poweroff_late,
-	.restore_early = i915_pm_resume_early,
-	.restore = i915_pm_resume,
+	.restore_early = i915_pm_restore_early,
+	.restore = i915_pm_restore,
 
 	/* S0ix (via runtime suspend) event handlers */
 	.runtime_suspend = intel_runtime_suspend,
-- 
2.8.1

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

[CI 2/4] drm/i915: Update domain tracking for GEM objects on hibernation

[Chris Wilson]

When creating the hibernation image, the CPU will read the pages of all
objects and thus conflict with our domain tracking. We need to update
our domain tracking to accurately reflect the state on restoration.

v2: Perform the domain tracking inside freeze, before the image is
written, rather than upon restoration.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_gem.c | 28 ++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6a2e7f84276b..dba03c026151 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1123,7 +1123,17 @@ static int i915_pm_freeze(struct device *dev)
 
 static int i915_pm_freeze_late(struct device *dev)
 {
-	return i915_pm_suspend_late(dev);
+	int ret;
+
+	ret = i915_pm_suspend_late(dev);
+	if (ret)
+		return ret;
+
+	ret = i915_gem_freeze_late(dev_to_i915(dev));
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 /* thaw: called after creating the hibernation image, but before turning off. */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d9d07b70f05c..593baa3e8da0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2961,6 +2961,8 @@ int i915_gem_wait_ioctl(struct drm_device *dev, void *data,
 void i915_gem_load_init(struct drm_device *dev);
 void i915_gem_load_cleanup(struct drm_device *dev);
 void i915_gem_load_init_fences(struct drm_i915_private *dev_priv);
+int i915_gem_freeze_late(struct drm_i915_private *dev_priv);
+
 void *i915_gem_object_alloc(struct drm_device *dev);
 void i915_gem_object_free(struct drm_i915_gem_object *obj);
 void i915_gem_object_init(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a3d826bb216b..aff386eea8ce 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5051,6 +5051,34 @@ void i915_gem_load_cleanup(struct drm_device *dev)
 	kmem_cache_destroy(dev_priv->objects);
 }
 
+int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
+{
+	struct drm_i915_gem_object *obj;
+
+	/* Called just before we write the hibernation image.
+	 *
+	 * We need to update the domain tracking to reflect that the CPU
+	 * will be accessing all the pages to create and restore from the
+	 * hibernation, and so upon restoration those pages will be in the
+	 * CPU domain.
+	 *
+	 * To make sure the hibernation image contains the latest state,
+	 * we update that state just before writing out the image.
+	 */
+
+	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;
+	}
+
+	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;
+	}
+
+	return 0;
+}
+
 void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
-- 
2.8.1

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

[CI 3/4] drm/i915: Lazily migrate the objects after hibernation

[Chris Wilson]

Now that we mark the object domains for having been restored from the
hibernation image, we not need to flush everything during resume and
can instead rely on the normal domain tracking to flush only when
required. The only caveat here are objects that are pinned for use by
the hardware, whose contents must be coherent for when the device
resumes reading from then (shortly afterwards with the driver assuming
the objects are in the correct domain).

References: https://bugs.freedesktop.org/show_bug.cgi?id=94722
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Tested-by: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5fb14c835543..319f3b459b3e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3245,7 +3245,6 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
-	bool flush;
 
 	i915_check_and_clear_faults(dev_priv);
 
@@ -3255,19 +3254,16 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 
 	/* Cache flush objects bound into GGTT and rebind them. */
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-		flush = false;
 		list_for_each_entry(vma, &obj->vma_list, obj_link) {
 			if (vma->vm != &ggtt->base)
 				continue;
 
 			WARN_ON(i915_vma_bind(vma, obj->cache_level,
 					      PIN_UPDATE));
-
-			flush = true;
 		}
 
-		if (flush)
-			i915_gem_clflush_object(obj, obj->pin_display);
+		if (obj->pin_display)
+			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
 	}
 
 	if (INTEL_INFO(dev)->gen >= 8) {
-- 
2.8.1

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

[CI 4/4] drm/i915: Skip clearing the GGTT on full-ppgtt systems

[Chris Wilson]

Under full-ppgtt, access to the global GTT is carefully regulated
through hardware functions (i.e. userspace cannot read and write to
arbitrary locations in the GGTT via the GPU). With this restriction in
place, we can forgo clearing stale entries from the GGTT as they will
not be accessed.

For aliasing-ppgtt, we could almost do the same except that we do allow
userspace access to the global-GTT via execbuf in order to workraound
some quirks of certain instructions. (This execbuf path is filtered out
with EINVAL on full-ppgtt.)

The most dramatic effect this will have will be during resume, as with
full-ppgtt the GGTT is only used sparingly.

References: https://bugs.freedesktop.org/show_bug.cgi?id=94722
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Tested-by: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 319f3b459b3e..7eab619a3eb2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
 }
 
+static void nop_clear_range(struct i915_address_space *vm,
+			    uint64_t start,
+			    uint64_t length,
+			    bool use_scratch)
+{
+}
+
 static void gen8_ggtt_clear_range(struct i915_address_space *vm,
 				  uint64_t start,
 				  uint64_t length,
@@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
 	ret = ggtt_probe_common(dev, ggtt->size);
 
-	ggtt->base.clear_range = gen8_ggtt_clear_range;
-	if (IS_CHERRYVIEW(dev_priv))
-		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
-	else
-		ggtt->base.insert_entries = gen8_ggtt_insert_entries;
 	ggtt->base.bind_vma = ggtt_bind_vma;
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
 
+	ggtt->base.clear_range = nop_clear_range;
+	if (!USES_FULL_PPGTT(dev_priv))
+		ggtt->base.clear_range = gen8_ggtt_clear_range;
+
+	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
+	if (IS_CHERRYVIEW(dev_priv))
+		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
+
 	return ret;
 }
 
-- 
2.8.1

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

✗ Ro.CI.BAT: failure for series starting with [1/4] drm/i915: Add distinct stubs for PM hibernation phases (rev4)

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Add distinct stubs for PM hibernation phases (rev4)
URL   : https://patchwork.freedesktop.org/series/7063/
State : failure

== Summary ==

Applying: drm/i915: Add distinct stubs for PM hibernation phases
Applying: drm/i915: Update domain tracking for GEM objects on hibernation
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_drv.c
M	drivers/gpu/drm/i915/i915_drv.h
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_drv.h
Auto-merging drivers/gpu/drm/i915/i915_drv.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_drv.c
Patch failed at 0002 drm/i915: Update domain tracking for GEM objects on hibernation
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

[CI 1/4] drm/i915: Add distinct stubs for PM hibernation phases

[Chris Wilson]

Currently for handling the extra hibernation phases we just call the
equivalent suspend/resume phases. In the next couple of patches, I wish
to specialise the hibernation phases to reduce the amount of work
required for handling GEM objects.

v2: There are more! Don't forget the freeze phases.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 45 +++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5ae79601335c..6a2e7f84276b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1115,6 +1115,39 @@ static int i915_pm_resume(struct device *dev)
 	return i915_drm_resume(drm_dev);
 }
 
+/* freeze: before creating the hibernation_image */
+static int i915_pm_freeze(struct device *dev)
+{
+	return i915_pm_suspend(dev);
+}
+
+static int i915_pm_freeze_late(struct device *dev)
+{
+	return i915_pm_suspend_late(dev);
+}
+
+/* thaw: called after creating the hibernation image, but before turning off. */
+static int i915_pm_thaw_early(struct device *dev)
+{
+	return i915_pm_resume_early(dev);
+}
+
+static int i915_pm_thaw(struct device *dev)
+{
+	return i915_pm_resume(dev);
+}
+
+/* restore: called after loading the hibernation image. */
+static int i915_pm_restore_early(struct device *dev)
+{
+	return i915_pm_resume_early(dev);
+}
+
+static int i915_pm_restore(struct device *dev)
+{
+	return i915_pm_resume(dev);
+}
+
 /*
  * Save all Gunit registers that may be lost after a D3 and a subsequent
  * S0i[R123] transition. The list of registers needing a save/restore is
@@ -1669,14 +1702,14 @@ static const struct dev_pm_ops i915_pm_ops = {
 	 * @restore, @restore_early : called after rebooting and restoring the
 	 *                            hibernation image [PMSG_RESTORE]
 	 */
-	.freeze = i915_pm_suspend,
-	.freeze_late = i915_pm_suspend_late,
-	.thaw_early = i915_pm_resume_early,
-	.thaw = i915_pm_resume,
+	.freeze = i915_pm_freeze,
+	.freeze_late = i915_pm_freeze_late,
+	.thaw_early = i915_pm_thaw_early,
+	.thaw = i915_pm_thaw,
 	.poweroff = i915_pm_suspend,
 	.poweroff_late = i915_pm_poweroff_late,
-	.restore_early = i915_pm_resume_early,
-	.restore = i915_pm_resume,
+	.restore_early = i915_pm_restore_early,
+	.restore = i915_pm_restore,
 
 	/* S0ix (via runtime suspend) event handlers */
 	.runtime_suspend = intel_runtime_suspend,
-- 
2.8.1

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

[CI 2/4] drm/i915: Update domain tracking for GEM objects on hibernation

[Chris Wilson]

When creating the hibernation image, the CPU will read the pages of all
objects and thus conflict with our domain tracking. We need to update
our domain tracking to accurately reflect the state on restoration.

v2: Perform the domain tracking inside freeze, before the image is
written, rather than upon restoration.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 12 +++++++++++-
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_gem.c | 28 ++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6a2e7f84276b..dba03c026151 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1123,7 +1123,17 @@ static int i915_pm_freeze(struct device *dev)
 
 static int i915_pm_freeze_late(struct device *dev)
 {
-	return i915_pm_suspend_late(dev);
+	int ret;
+
+	ret = i915_pm_suspend_late(dev);
+	if (ret)
+		return ret;
+
+	ret = i915_gem_freeze_late(dev_to_i915(dev));
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 /* thaw: called after creating the hibernation image, but before turning off. */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1ba614193cc9..72f0b02a8372 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2976,6 +2976,8 @@ int i915_gem_wait_ioctl(struct drm_device *dev, void *data,
 void i915_gem_load_init(struct drm_device *dev);
 void i915_gem_load_cleanup(struct drm_device *dev);
 void i915_gem_load_init_fences(struct drm_i915_private *dev_priv);
+int i915_gem_freeze_late(struct drm_i915_private *dev_priv);
+
 void *i915_gem_object_alloc(struct drm_device *dev);
 void i915_gem_object_free(struct drm_i915_gem_object *obj);
 void i915_gem_object_init(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a3d826bb216b..aff386eea8ce 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5051,6 +5051,34 @@ void i915_gem_load_cleanup(struct drm_device *dev)
 	kmem_cache_destroy(dev_priv->objects);
 }
 
+int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
+{
+	struct drm_i915_gem_object *obj;
+
+	/* Called just before we write the hibernation image.
+	 *
+	 * We need to update the domain tracking to reflect that the CPU
+	 * will be accessing all the pages to create and restore from the
+	 * hibernation, and so upon restoration those pages will be in the
+	 * CPU domain.
+	 *
+	 * To make sure the hibernation image contains the latest state,
+	 * we update that state just before writing out the image.
+	 */
+
+	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;
+	}
+
+	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;
+	}
+
+	return 0;
+}
+
 void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
-- 
2.8.1

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

[CI 3/4] drm/i915: Lazily migrate the objects after hibernation

[Chris Wilson]

Now that we mark the object domains for having been restored from the
hibernation image, we not need to flush everything during resume and
can instead rely on the normal domain tracking to flush only when
required. The only caveat here are objects that are pinned for use by
the hardware, whose contents must be coherent for when the device
resumes reading from then (shortly afterwards with the driver assuming
the objects are in the correct domain).

References: https://bugs.freedesktop.org/show_bug.cgi?id=94722
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: David Weinehall <david.weinehall@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Tested-by: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5fb14c835543..319f3b459b3e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3245,7 +3245,6 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
-	bool flush;
 
 	i915_check_and_clear_faults(dev_priv);
 
@@ -3255,19 +3254,16 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 
 	/* Cache flush objects bound into GGTT and rebind them. */
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-		flush = false;
 		list_for_each_entry(vma, &obj->vma_list, obj_link) {
 			if (vma->vm != &ggtt->base)
 				continue;
 
 			WARN_ON(i915_vma_bind(vma, obj->cache_level,
 					      PIN_UPDATE));
-
-			flush = true;
 		}
 
-		if (flush)
-			i915_gem_clflush_object(obj, obj->pin_display);
+		if (obj->pin_display)
+			WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
 	}
 
 	if (INTEL_INFO(dev)->gen >= 8) {
-- 
2.8.1

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

[CI 4/4] drm/i915: Skip clearing the GGTT on full-ppgtt systems

[Chris Wilson]

Under full-ppgtt, access to the global GTT is carefully regulated
through hardware functions (i.e. userspace cannot read and write to
arbitrary locations in the GGTT via the GPU). With this restriction in
place, we can forgo clearing stale entries from the GGTT as they will
not be accessed.

For aliasing-ppgtt, we could almost do the same except that we do allow
userspace access to the global-GTT via execbuf in order to workraound
some quirks of certain instructions. (This execbuf path is filtered out
with EINVAL on full-ppgtt.)

The most dramatic effect this will have will be during resume, as with
full-ppgtt the GGTT is only used sparingly.

References: https://bugs.freedesktop.org/show_bug.cgi?id=94722
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Tested-by: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 319f3b459b3e..7eab619a3eb2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
 }
 
+static void nop_clear_range(struct i915_address_space *vm,
+			    uint64_t start,
+			    uint64_t length,
+			    bool use_scratch)
+{
+}
+
 static void gen8_ggtt_clear_range(struct i915_address_space *vm,
 				  uint64_t start,
 				  uint64_t length,
@@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
 	ret = ggtt_probe_common(dev, ggtt->size);
 
-	ggtt->base.clear_range = gen8_ggtt_clear_range;
-	if (IS_CHERRYVIEW(dev_priv))
-		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
-	else
-		ggtt->base.insert_entries = gen8_ggtt_insert_entries;
 	ggtt->base.bind_vma = ggtt_bind_vma;
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
 
+	ggtt->base.clear_range = nop_clear_range;
+	if (!USES_FULL_PPGTT(dev_priv))
+		ggtt->base.clear_range = gen8_ggtt_clear_range;
+
+	ggtt->base.insert_entries = gen8_ggtt_insert_entries;
+	if (IS_CHERRYVIEW(dev_priv))
+		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
+
 	return ret;
 }
 
-- 
2.8.1

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

✗ Ro.CI.BAT: failure for series starting with [1/4] drm/i915: Add distinct stubs for PM hibernation phases (rev7)

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Add distinct stubs for PM hibernation phases (rev7)
URL   : https://patchwork.freedesktop.org/series/7063/
State : failure

== Summary ==

Applying: drm/i915: Add distinct stubs for PM hibernation phases
Applying: drm/i915: Update domain tracking for GEM objects on hibernation
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_drv.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_drv.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_drv.c
Patch failed at 0002 drm/i915: Update domain tracking for GEM objects on hibernation
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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