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

From: Chris Wilson <chris@chris-wilson.co.uk>
To: "Kristian Høgsberg" <krh@bitplanet.net>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	Michal Winiarsky <michal.winiarsky@intel.com>,
	Akash Goel <akash.goel@intel.com>,
	Vinay Belgaumkar <vinay.belgaumkar@intel.com>
Subject: Re: [PATCH v4] drm/i915: Add soft-pinning API for execbuffer
Date: Sat, 4 Jul 2015 13:23:39 +0100	[thread overview]
Message-ID: <20150704122339.GB5312@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <CAOeoa-fEEacFgiUQhC8B-=vz7HU0UvSdn=rLg8-qKgfC2gkAhg@mail.gmail.com>

On Fri, Jul 03, 2015 at 10:29:44PM -0700, Kristian Høgsberg wrote:
> On Tue, Jun 30, 2015 at 7:13 AM, Thomas Daniel <thomas.daniel@intel.com> wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > Userspace can pass in an offset that it presumes the object is located
> > at. The kernel will then do its utmost to fit the object into that
> > location. The assumption is that userspace is handling its own object
> > locations (for example along with full-ppgtt) and that the kernel will
> > rarely have to make space for the user's requests.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > v2: Fixed incorrect eviction found by Michal Winiarski - fix suggested by Chris
> > Wilson.  Fixed incorrect error paths causing crash found by Michal Winiarski.
> > (Not published externally)
> >
> > v3: Rebased because of trivial conflict in object_bind_to_vm.  Fixed eviction
> > to allow eviction of soft-pinned objects when another soft-pinned object used
> > by a subsequent execbuffer overlaps reported by Michal Winiarski.
> > (Not published externally)
> >
> > v4: Moved soft-pinned objects to the front of ordered_vmas so that they are
> > pinned first after an address conflict happens to avoid repeated conflicts in
> > rare cases (Suggested by Chris Wilson).  Expanded comment on
> > drm_i915_gem_exec_object2.offset to cover this new API.

Note this patch is outdated compared to the one I have in my tree. There
are also requirements to improve drm_mm_reserve_node().

> Can we add an I915_PARAM_HAS_EXEC_PINNED param for this feature?

Yeah, it's not that difficult to test,

static bool test_has_softpin(int fd)
{
   struct drm_i915_gem_create create;
   struct drm_i915_gem_exec_object2 object;
   struct drm_i915_gem_pwrite pwrite;
   struct drm_i915_gem_execbuffer2 execbuf;
   uint32_t batch[2] = { 0xa << 23 };
   bool ret = false;

   if (DBG_NO_SOFTPIN)
      return DBG_NO_SOFTPIN < 0;

   if (gem_param(fd, I915_PARAM_HAS_ALIASING_PPGTT) < 2)
      return false;

   memset(&create, 0, sizeof(create));
   create.size = 4096;
   drmIoctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);

   memset(&pwrite, 0, sizeof(pwrite));
   pwrite.handle = create.handle;
   pwrite.offset = 0;
   pwrite.size = sizeof(batch);
   pwrite.data_ptr = (uintptr_t)batch;
   if (drmIoctl(fd, DRM_IOCTL_I915_GEM_PWRITE, &pwrite))
      goto fail;

   object.handle = create.handle;

   memset(&execbuf, 0, sizeof(execbuf));
   execbuf.buffers_ptr = (uintptr_t)&object;
   execbuf.buffer_count = 1;
   if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
      goto fail;

   object.flags = EXEC_OBJECT_PINNED;
   ret  = drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf) == 0;
fail:
   drmIoctl(fd, DRM_IOCTL_GEM_CLOSE, &create);

   return ret;
}

but compares to

static bool test_has_softpin(int fd)
{
   if (DBG_NO_SOFTPIN)
      return DBG_NO_SOFTPIN < 0;

   if (gem_param(fd, I915_PARAM_HAS_ALIASING_PPGTT) < 2)
      return false;

   return gem_param(fd, I915_PARAM_HAS_EXEC_SOFTPIN) > 0;
}

with a parameter. At some point, we probably want to add a GETPARAMV!
-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