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@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Cc: David Weinehall <david.weinehall@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
