Re: [RFC] libdrm_intel: Add support for userptr objects

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, May 02, 2014 at 10:15:30AM -0700, Ben Widawsky wrote:
> On Fri, May 02, 2014 at 11:27:45AM +0100, Tvrtko Ursulin wrote:
> > 
> > On 05/01/2014 07:47 PM, Ben Widawsky wrote:
> > >On Wed, Feb 26, 2014 at 04:41:41PM +0000, Tvrtko Ursulin wrote:
> > >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >>
> > >>Allow userptr objects to be created and used via libdrm_intel.
> > >>
> > >>At the moment tiling and mapping to GTT aperture is not supported
> > >>due hardware limitations across different generations and uncertainty
> > >>about its usefulness.
> > >>
> > >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > >>---
> > >>  include/drm/i915_drm.h    |  16 +++++
> > >>  intel/intel_bufmgr.c      |  13 ++++
> > >>  intel/intel_bufmgr.h      |   5 ++
> > >>  intel/intel_bufmgr_gem.c  | 154 +++++++++++++++++++++++++++++++++++++++++++++-
> > >>  intel/intel_bufmgr_priv.h |  12 +++-
> > >>  5 files changed, 198 insertions(+), 2 deletions(-)
> > >>
> > >>diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> > >>index 2f4eb8c..d32ef99 100644
> > >>--- a/include/drm/i915_drm.h
> > >>+++ b/include/drm/i915_drm.h
> > >>@@ -223,6 +223,7 @@ typedef struct _drm_i915_sarea {
> > >>  #define DRM_I915_GEM_GET_CACHING	0x30
> > >>  #define DRM_I915_REG_READ		0x31
> > >>  #define DRM_I915_GET_RESET_STATS	0x32
> > >>+#define DRM_I915_GEM_USERPTR		0x34
> > >>
> > >>  #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
> > >>  #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
> > >>@@ -273,6 +274,7 @@ typedef struct _drm_i915_sarea {
> > >>  #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY	DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy)
> > >>  #define DRM_IOCTL_I915_REG_READ			DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read)
> > >>  #define DRM_IOCTL_I915_GET_RESET_STATS		DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GET_RESET_STATS, struct drm_i915_reset_stats)
> > >>+#define DRM_IOCTL_I915_GEM_USERPTR	DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR,  struct drm_i915_gem_userptr)
> > >>
> > >>  /* Allow drivers to submit batchbuffers directly to hardware, relying
> > >>   * on the security mechanisms provided by hardware.
> > >>@@ -498,6 +500,20 @@ struct drm_i915_gem_mmap_gtt {
> > >>  	__u64 offset;
> > >>  };
> > >>
> > >>+struct drm_i915_gem_userptr {
> > >>+	__u64 user_ptr;
> > >>+	__u64 user_size;
> > >
> > >Adding alignment might be a safe bet.
> > 
> > Hmmmm, at first I thought you are raising a good point. But then I don't
> > understand why I don't see any aligned types in
> > linux/include/uapi/drm/i915_drm.h ?
> 
> I meant an alignment field. I was thinking if some buffers have weird
> alignment requirements for the GPU (but not the CPU) making that info
> available to the kernel would be important.

We have a flags param and we can always extend the ioctl at the end, so
imo no need to preemptively add stuff we don't need yet.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch