| Message ID | 20180601093554.13083-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2018-06-01 at 10:35 +0100, Chris Wilson wrote: > As we have already suspended the device, this should be a no-op except > for marking that all writes are indeed complete. The downside is that > we then have to walk all the lists of objects for what should be a no-op > (in some cases they will be mmio read to ensure the GGTT writes are > indeed flushed, and clflushes to ensure that cpu writes are in memory). > > It seems prudent and the safer course for us to ensure all writes are > flushed to memory before suspend. > > 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> <SNIP> > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -5066,6 +5066,13 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) > > void i915_gem_suspend_late(struct drm_i915_private *i915) > { > + struct drm_i915_gem_object *obj; > + struct list_head *phases[] = { > + &i915->mm.unbound_list, > + &i915->mm.bound_list, > + NULL > + }, **p; I guess the p variable could be on a separate line, and maybe even "phase". > + > /* > * Neither the BIOS, ourselves or any other kernel > * expects the system to be in execlists mode on startup, > @@ -5086,6 +5093,13 @@ void i915_gem_suspend_late(struct drm_i915_private *i915) > * machine in an unusable condition. > */ > > + mutex_lock(&i915->drm.struct_mutex); > + for (p = phases; *p; p++) { > + list_for_each_entry(obj, *p, mm.link) > + WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false)); > + } > + mutex_unlock(&i915->drm.struct_mutex); Braces not exactly needed. Reviewed-by: joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f5c4ef052001..d91fb25a8a1f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5066,6 +5066,13 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) void i915_gem_suspend_late(struct drm_i915_private *i915) { + struct drm_i915_gem_object *obj; + struct list_head *phases[] = { + &i915->mm.unbound_list, + &i915->mm.bound_list, + NULL + }, **p; + /* * Neither the BIOS, ourselves or any other kernel * expects the system to be in execlists mode on startup, @@ -5086,6 +5093,13 @@ void i915_gem_suspend_late(struct drm_i915_private *i915) * machine in an unusable condition. */ + mutex_lock(&i915->drm.struct_mutex); + for (p = phases; *p; p++) { + list_for_each_entry(obj, *p, mm.link) + WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false)); + } + mutex_unlock(&i915->drm.struct_mutex); + intel_uc_sanitize(i915); i915_gem_sanitize(i915); }
As we have already suspended the device, this should be a no-op except for marking that all writes are indeed complete. The downside is that we then have to walk all the lists of objects for what should be a no-op (in some cases they will be mmio read to ensure the GGTT writes are indeed flushed, and clflushes to ensure that cpu writes are in memory). It seems prudent and the safer course for us to ensure all writes are flushed to memory before suspend. 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> --- drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)