[4/6] drm/i915: Apply the full CPU domain markup before freezing

Commit Message

Chris Wilson June 1, 2018, 9:35 a.m. UTC

Let's not take any chances by using a shortcut to mark the objects as in
the CPU domain upon freezing (all pages will be written to disk and so
on restore all objects will start from the CPU domain). Currently, we
simply mark the objects as being in the CPU domain, bypassing the
flushes. Let's call the full domain transfer function so that we have
less special case code (and symmetry with the suspend path) even though
it will be mostly redundant.

Signed-off-by: Chris Wilson <[e-mail]>
Cc: Joonas Lahtinen <[e-mail]>
Cc: Mika Kuoppala <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Joonas Lahtinen June 1, 2018, 1:04 p.m. UTC | #1

On Fri, 2018-06-01 at 10:35 +0100, Chris Wilson wrote:
> Let's not take any chances by using a shortcut to mark the objects as in
> the CPU domain upon freezing (all pages will be written to disk and so
> on restore all objects will start from the CPU domain). Currently, we
> simply mark the objects as being in the CPU domain, bypassing the
> flushes. Let's call the full domain transfer function so that we have
> less special case code (and symmetry with the suspend path) even though
> it will be mostly redundant.
> 
> Signed-off-by: Chris Wilson <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> Cc: Mika Kuoppala <[e-mail]>

Right, so the naming sees to be symmetry to this function... I'm
assuming this has no performance impact, so:

Reviewed-by: Joonas Lahtinen <[e-mail]>

Regards, Joonas

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d91fb25a8a1f..ea69b4b26e32 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5713,16 +5713,17 @@  int i915_gem_freeze(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
-int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
+int i915_gem_freeze_late(struct drm_i915_private *i915)
 {
 	struct drm_i915_gem_object *obj;
 	struct list_head *phases[] = {
-		&dev_priv->mm.unbound_list,
-		&dev_priv->mm.bound_list,
+		&i915->mm.unbound_list,
+		&i915->mm.bound_list,
 		NULL
 	}, **p;
 
-	/* Called just before we write the hibernation image.
+	/*
+	 * 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
@@ -5736,15 +5737,15 @@  int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
 	 * the objects as well, see i915_gem_freeze()
 	 */
 
-	i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_UNBOUND);
-	i915_gem_drain_freed_objects(dev_priv);
+	i915_gem_shrink(i915, -1UL, NULL, I915_SHRINK_UNBOUND);
+	i915_gem_drain_freed_objects(i915);
 
-	spin_lock(&dev_priv->mm.obj_lock);
+	mutex_lock(&i915->drm.struct_mutex);
 	for (p = phases; *p; p++) {
 		list_for_each_entry(obj, *p, mm.link)
-			__start_cpu_write(obj);
+			WARN_ON(i915_gem_object_set_to_cpu_domain(obj, true));
 	}
-	spin_unlock(&dev_priv->mm.obj_lock);
+	mutex_unlock(&i915->drm.struct_mutex);
 
 	return 0;
 }