Re: [PATCH 20/29] drm/i915: Fix up map and fenceable for VMA

Ben Widawsky
2013-08-07T18:37:08Z

On Tue, Aug 06, 2013 at 09:11:54PM +0200, Daniel Vetter wrote:
> On Wed, Jul 31, 2013 at 05:00:13PM -0700, Ben Widawsky wrote:
> > formerly: "drm/i915: Create VMAs (part 3.5) - map and fenceable
> > tracking"
> >
> > The map_and_fenceable tracking is per object. GTT mapping, and fences
> > only apply to global GTT. As such, object operations which are not
> > performed on the global GTT should not effect mappable or fenceable
> > characteristics.
> >
> > Functionally, this commit could very well be squashed in to a previous
> > patch which updated object operations to take a VM argument. This
> > commit is split out because it's a bit tricky (or at least it was for
> > me).
> >
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index d4d6444..ec23a5c 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2626,7 +2626,7 @@ int i915_vma_unbind(struct i915_vma *vma)
> >
> > trace_i915_vma_unbind(vma);
> >
> > - if (obj->has_global_gtt_mapping)
> > + if (obj->has_global_gtt_mapping && i915_is_ggtt(vma->vm))
> > i915_gem_gtt_unbind_object(obj);
> > if (obj->has_aliasing_ppgtt_mapping) {
> > i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj);
>
> Hm, shouldn't we do the is_ggtt check for both? After all only the global
> ggtt can be aliased ever ... This would also be more symmetric with some
> of the other global gtt checks I've spotted. You're take or will that run
> afoul of your Great Plan?
> -Daniel
>

You're right. The check makes sense for both cases. In both the original
series, and n a few patches, this code turns into:
vma->vm->unbind_vma(vma);

This ugliness is a result of bad rebasing on my part.

--
Ben Widawsky, Intel Open Source Technology Center