Re: [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer

From: Chris Wilson <chris@chris-wilson.co.uk>
To: akash goel <akash.goels@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, "Goel, Akash" <akash.goel@intel.com>
Subject: Re: [PATCH 3/3] drm/i915: Add soft-pinning API for execbuffer
Date: Wed, 2 Dec 2015 13:28:18 +0000	[thread overview]
Message-ID: <20151202132818.GB13583@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <CAK_0AV00FjtGHor38md-MyJnxDd3cY5nd-pgNCGo9+zpWpW7yg@mail.gmail.com>

On Tue, Oct 27, 2015 at 05:21:37PM +0530, akash goel wrote:
> On Tue, Oct 6, 2015 at 4:23 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 19dd6b05ee1d..c35c9dc526e7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -603,6 +603,8 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> >                         flags |= BATCH_OFFSET_BIAS | PIN_OFFSET_BIAS;
> >                 if ((flags & PIN_MAPPABLE) == 0)
> >                         flags |= PIN_HIGH;
> > +               if (entry->flags & EXEC_OBJECT_PINNED)
> > +                       flags |= entry->offset | PIN_OFFSET_FIXED;
> >         }
> >
> >         ret = i915_gem_object_pin(obj, vma->vm,
> > @@ -679,6 +681,10 @@ eb_vma_misplaced(struct i915_vma *vma)
> >         if (vma->node.size < entry->pad_to_size)
> >                 return true;
> >
> > +       if (entry->flags & EXEC_OBJECT_PINNED &&
> > +           vma->node.start != entry->offset)
> > +               return true;
> > +
> >         if (entry->flags & __EXEC_OBJECT_NEEDS_BIAS &&
> >             vma->node.start < BATCH_OFFSET_BIAS)
> >                 return true;
> 
> 
> I think would be better to add the following change here.
> 
> - if ((entry->flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) == 0 &&
> + if (!(entry->flags &
> +    (EXEC_OBJECT_SUPPORTS_48B_ADDRESS | EXEC_OBJECT_PINNED)) &&
>      (vma->node.start + vma->node.size - 1) >> 32)
>   return true;
> 
> This way, User will not have to necessarily pass the 48B_ADDRESS flag
> also along with the OBJECT_PINNED flag, if the offset is > 4 GB.
> The OBJECT_PINNED flag will take precedence over 48B_ADDRESS flag.

No, let's not start having flags mean multiple things if we can avoid
it as that makes the ABI harder to extend in future.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx