[Intel-gfx] [PATCH] drm/i915: Use the new vm [un]bind functions

[Chris Wilson]

On Fri, Sep 20, 2013 at 03:24:43PM +0200, Daniel Vetter wrote:
> On Fri, Sep 20, 2013 at 12:43 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > On Thu, Sep 19, 2013 at 09:06:39PM -0700, Ben Widawsky wrote:
> >> @@ -1117,8 +1114,25 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> >>        * batch" bit. Hence we need to pin secure batches into the global gtt.
> >>        * hsw should have this fixed, but let's be paranoid and do it
> >>        * unconditionally for now. */
> >> -     if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping)
> >> -             i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level);
> >> +     if (flags & I915_DISPATCH_SECURE) {
> >> +             struct i915_address_space *ggtt = obj_to_ggtt(batch_obj);
> >> +             /* Assuming all privileged batches are in the global GTT means
> >> +              * we need to make sure we have a global gtt offset, as well as
> >> +              * the PTEs mapped. As mentioned above, we can forego this on
> >> +              * HSW, but don't.
> >> +              */
> >> +             ret = i915_gem_object_bind_to_vm(batch_obj, ggtt, 0, false,
> >> +                                              false);
> >> +             if (ret)
> >> +                     goto err;
> >
> > bind_to_vm() has unwanted side-effects here - notably always allocating
> > a node and corrupting lists.
> >
> > Just pin, ggtt->bind_vma, unpin. Hmmm, except that we also need a
> > move_to_active (as we are not presuming vm == ggtt).
> >
> > pin, ggtt->bind_vma, move_to_active(ggtt), unpin.
> >
> > And then hope we have the correct flushes in place for that to be
> > retired if nothing else is going on with that ggtt.
> 
> New idea: Can't we make this work in an easier fashion by changing the
> vma we look up for the eb lists using the right gtt appropriate for
> the batch?

Not quite, in the eb it does need to be in the ppgtt for self- or
back-references to the batch bo. It is only the CS that needs the GGTT
entry in addition to the PPGTT entry required for everything else. And
we can quite happily handle having different offsets in each address
space.
 
> Then (presuming all our code is clear of unnecessary (obj, vm) -> vma
> lookups) everything should Just Work, including grabing the gtt
> offset. Or am I just dreaming here? Of course a BUG_ON to check that
> vma->vm of the batch object points at the global gtt vm if we have a
> secure dispatch bb would still be dutiful.

Dream on. :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre