| Message ID | 20130803222447.GB28218@bwidawsk.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Aug 03, 2013 at 03:24:47PM -0700, Ben Widawsky wrote: > 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> That's not a properly formatted patch, so I've stopped merging for now. /me loves cheap excuses But overall I _really_ like what the series looks now, I can dwell in the cozy feeling that I actually understand what's going on. So if the name of the game is to keep your maintainer happy I think the goal is unlocked ;-) Cheers, Daniel > > 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 > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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); }