[Intel-gfx] [PATCH 02/11] drm/i915/gtt: Teach restore-gtt to walk the ggtt vma list not the object list

[Chris Wilson]

Quoting Joonas Lahtinen (2018-06-05 08:51:39)
> Quoting Chris Wilson (2018-06-05 10:19:40)
> > In preparation, for having non-vma objects stored inside the ggtt, to
> > handle restoration of the GGTT following resume, we need to walk over
> > the ggtt address space rebinding vma, as opposed to walking over bound
> > objects looking for ggtt entries.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Cc: Matthew Auld <matthew.william.auld at gmail.com>
> 
> Comment below.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> 
> > @@ -3578,21 +3578,15 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
> >         ggtt->vm.closed = true; /* skip rewriting PTE on VMA unbind */
> >  
> >         /* clflush objects bound into the GGTT and rebind them. */
> > -       list_for_each_entry_safe(obj, on, &dev_priv->mm.bound_list, mm.link) {
> > -               bool ggtt_bound = false;
> > -               struct i915_vma *vma;
> > -
> > -               for_each_ggtt_vma(vma, obj) {
> > -                       if (!i915_vma_unbind(vma))
> > -                               continue;
> > +       GEM_BUG_ON(!list_empty(&ggtt->vm.active_list));
> > +       list_for_each_entry_safe(vma, vn, &ggtt->vm.inactive_list, vm_link) {
> > +               struct drm_i915_gem_object *obj = vma->obj;
> >  
> > -                       WARN_ON(i915_vma_bind(vma, obj->cache_level,
> > -                                             PIN_UPDATE));
> > -                       ggtt_bound = true;
> > -               }
> > +               if (!i915_vma_unbind(vma))
> > +                       continue;
> >  
> > -               if (ggtt_bound)
> > -                       WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
> > +               WARN_ON(i915_vma_bind(vma, obj->cache_level, PIN_UPDATE));
> > +               WARN_ON(i915_gem_object_set_to_gtt_domain(obj, false));
> 
> This gets called multiple times per an object for partial and rotated
> views, but that should not be our performance bottleneck.

Correct. I thought I had mentioned the drawback, but it appears I just
thought about it. It should be the case that for the majority of those,
we will be able to unbind them anyway. It's only the few objects pinned
for use that we have to flush, and they are the scanout and context
objects.

Furthermore, repeated calls to gtt_domain should come in the wash as
being no-ops, just many checks to determine we have nothing to do.
Improving the whole domain management is on the permanent wishlist
(killing struct_mutex is of immediate priority).
-Chris