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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx>
> > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > Cc: Matthew Auld <matthew.william.auld@xxxxxxxxx>
> 
> Comment below.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> 
> > @@ -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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux