Re: [PATCH 13/29] drm/i915: clear domains for all objects on reset

Ben Widawsky
2013-08-03T22:24:52Z

On Sat, Aug 03, 2013 at 11:59:42AM +0100, Chris Wilson wrote:
> On Wed, Jul 31, 2013 at 05:00:06PM -0700, Ben Widawsky wrote:
> > Simply iterating over 1 inactive list is insufficient for the way we now
> > track inactive (1 list per address space). We could alternatively do
> > this with bound + unbound lists, and an inactive check. To me, this way
> > is a bit easier to understand.
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index b4c35f0..8ce3545 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2282,7 +2282,7 @@ void i915_gem_restore_fences(struct drm_device *dev)
> > void i915_gem_reset(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - struct i915_address_space *vm = &dev_priv->gtt.base;
> > + struct i915_address_space *vm;
> > struct drm_i915_gem_object *obj;
> > struct intel_ring_buffer *ring;
> > int i;
> > @@ -2293,8 +2293,9 @@ void i915_gem_reset(struct drm_device *dev)
> > /* Move everything out of the GPU domains to ensure we do any
> > * necessary invalidation upon reuse.
> > */
> > - list_for_each_entry(obj, &vm->inactive_list, mm_list)
> > - obj->base.read_domains &= ~I915_GEM_GPU_DOMAINS;
> > + list_for_each_entry(vm, &dev_priv->vm_list, global_link)
> > + list_for_each_entry(obj, &vm->inactive_list, mm_list)
> > + obj->base.read_domains &= ~I915_GEM_GPU_DOMAINS;
>
> This code is dead. Just remove it rather than port it to vma.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre

Got it, and moved to the front of the series.

commit 8472f08863da69159aa0a7555836ca0511754877
Author: Ben Widawsky <ben@bwidawsk.net>
Date: Sat Aug 3 15:22:17 2013 -0700

drm/i915: eliminate dead domain clearing on reset

The code itself is no longer accurate without updating once we have
multiple address space since clearing the domains of every object
requires scanning the inactive list for all VMs.

"This code is dead. Just remove it rather than port it to vma." - Chris
Wilson

Recommended-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3a5d4ba..c7e3cee 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2277,12 +2277,6 @@ void i915_gem_reset(struct drm_device *dev)
for_each_ring(ring, dev_priv, i)
i915_gem_reset_ring_lists(dev_priv, ring);

- /* Move everything out of the GPU domains to ensure we do any
- * necessary invalidation upon reuse.
- */
- list_for_each_entry(obj, &vm->inactive_list, mm_list)
- obj->base.read_domains &= ~I915_GEM_GPU_DOMAINS;
-
i915_gem_restore_fences(dev);
}

--
Ben Widawsky, Intel Open Source Technology Center