Re: [PATCH] intel: Add support for userptr objects

[Damien Lespiau <damien.lespiau@intel.com>]

On Thu, Jun 19, 2014 at 03:52:03PM +0100, 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.
> 
> v2: Improved error handling in feature detection per review comments.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Pushed a slightly modified version of this patch as libdrm now has
explicit symbol visibility.

-- 
Damien

>  intel/intel_bufmgr.c      |  13 ++++
>  intel/intel_bufmgr.h      |   5 ++
>  intel/intel_bufmgr_gem.c  | 163 +++++++++++++++++++++++++++++++++++++++++++++-
>  intel/intel_bufmgr_priv.h |  12 +++-
>  4 files changed, 191 insertions(+), 2 deletions(-)
> 
> diff --git a/intel/intel_bufmgr.c b/intel/intel_bufmgr.c
> index 905556f..7f3d795 100644
> --- a/intel/intel_bufmgr.c
> +++ b/intel/intel_bufmgr.c
> @@ -60,6 +60,19 @@ drm_intel_bo *drm_intel_bo_alloc_for_render(drm_intel_bufmgr *bufmgr,
>  	return bufmgr->bo_alloc_for_render(bufmgr, name, size, alignment);
>  }
>  
> +drm_intel_bo *drm_intel_bo_alloc_userptr(drm_intel_bufmgr *bufmgr,
> +					  const char *name, void *addr,
> +					  uint32_t tiling_mode,
> +					  uint32_t stride,
> +					  unsigned long size,
> +					  unsigned long flags)
> +{
> +	if (bufmgr->bo_alloc_userptr)
> +		return bufmgr->bo_alloc_userptr(bufmgr, name, addr, tiling_mode,
> +						stride, size, flags);
> +	return NULL;
> +}
> +
>  drm_intel_bo *
>  drm_intel_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name,
>                          int x, int y, int cpp, uint32_t *tiling_mode,
> diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
> index 9383c72..be83a56 100644
> --- a/intel/intel_bufmgr.h
> +++ b/intel/intel_bufmgr.h
> @@ -113,6 +113,11 @@ drm_intel_bo *drm_intel_bo_alloc_for_render(drm_intel_bufmgr *bufmgr,
>  					    const char *name,
>  					    unsigned long size,
>  					    unsigned int alignment);
> +drm_intel_bo *drm_intel_bo_alloc_userptr(drm_intel_bufmgr *bufmgr,
> +					const char *name,
> +					void *addr, uint32_t tiling_mode,
> +					uint32_t stride, unsigned long size,
> +					unsigned long flags);
>  drm_intel_bo *drm_intel_bo_alloc_tiled(drm_intel_bufmgr *bufmgr,
>  				       const char *name,
>  				       int x, int y, int cpp,
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index 007a6d8..6dd3986 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -182,6 +182,11 @@ struct _drm_intel_bo_gem {
>  	void *mem_virtual;
>  	/** GTT virtual address for the buffer, saved across map/unmap cycles */
>  	void *gtt_virtual;
> +	/**
> +	 * Virtual address of the buffer allocated by user, used for userptr
> +	 * objects only.
> +	 */
> +	void *user_virtual;
>  	int map_count;
>  	drmMMListHead vma_list;
>  
> @@ -221,6 +226,11 @@ struct _drm_intel_bo_gem {
>  	bool idle;
>  
>  	/**
> +	 * Boolean of whether this buffer was allocated with userptr
> +	 */
> +	bool is_userptr;
> +
> +	/**
>  	 * Size in bytes of this buffer and its relocation descendents.
>  	 *
>  	 * Used to avoid costly tree walking in
> @@ -847,6 +857,80 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name,
>  					       tiling, stride);
>  }
>  
> +static drm_intel_bo *
> +drm_intel_gem_bo_alloc_userptr(drm_intel_bufmgr *bufmgr,
> +				const char *name,
> +				void *addr,
> +				uint32_t tiling_mode,
> +				uint32_t stride,
> +				unsigned long size,
> +				unsigned long flags)
> +{
> +	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr;
> +	drm_intel_bo_gem *bo_gem;
> +	int ret;
> +	struct drm_i915_gem_userptr userptr;
> +
> +	/* Tiling with userptr surfaces is not supported
> +	 * on all hardware so refuse it for time being.
> +	 */
> +	if (tiling_mode != I915_TILING_NONE)
> +		return NULL;
> +
> +	bo_gem = calloc(1, sizeof(*bo_gem));
> +	if (!bo_gem)
> +		return NULL;
> +
> +	bo_gem->bo.size = size;
> +
> +	VG_CLEAR(userptr);
> +	userptr.user_ptr = (__u64)((unsigned long)addr);
> +	userptr.user_size = size;
> +	userptr.flags = flags;
> +
> +	ret = drmIoctl(bufmgr_gem->fd,
> +			DRM_IOCTL_I915_GEM_USERPTR,
> +			&userptr);
> +	if (ret != 0) {
> +		DBG("bo_create_userptr: "
> +		    "ioctl failed with user ptr %p size 0x%lx, "
> +		    "user flags 0x%lx\n", addr, size, flags);
> +		free(bo_gem);
> +		return NULL;
> +	}
> +
> +	bo_gem->gem_handle = userptr.handle;
> +	bo_gem->bo.handle = bo_gem->gem_handle;
> +	bo_gem->bo.bufmgr    = bufmgr;
> +	bo_gem->is_userptr   = true;
> +	bo_gem->bo.virtual   = addr;
> +	/* Save the address provided by user */
> +	bo_gem->user_virtual = addr;
> +	bo_gem->tiling_mode  = I915_TILING_NONE;
> +	bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE;
> +	bo_gem->stride       = 0;
> +
> +	DRMINITLISTHEAD(&bo_gem->name_list);
> +	DRMINITLISTHEAD(&bo_gem->vma_list);
> +
> +	bo_gem->name = name;
> +	atomic_set(&bo_gem->refcount, 1);
> +	bo_gem->validate_index = -1;
> +	bo_gem->reloc_tree_fences = 0;
> +	bo_gem->used_as_reloc_target = false;
> +	bo_gem->has_error = false;
> +	bo_gem->reusable = false;
> +
> +	drm_intel_bo_gem_set_in_aperture_size(bufmgr_gem, bo_gem);
> +
> +	DBG("bo_create_userptr: "
> +	    "ptr %p buf %d (%s) size %ldb, stride 0x%x, tile mode %d\n",
> +		addr, bo_gem->gem_handle, bo_gem->name,
> +		size, stride, tiling_mode);
> +
> +	return &bo_gem->bo;
> +}
> +
>  /**
>   * Returns a drm_intel_bo wrapping the given buffer object handle.
>   *
> @@ -1173,6 +1257,12 @@ static int drm_intel_gem_bo_map(drm_intel_bo *bo, int write_enable)
>  	struct drm_i915_gem_set_domain set_domain;
>  	int ret;
>  
> +	if (bo_gem->is_userptr) {
> +		/* Return the same user ptr */
> +		bo->virtual = bo_gem->user_virtual;
> +		return 0;
> +	}
> +
>  	pthread_mutex_lock(&bufmgr_gem->lock);
>  
>  	if (bo_gem->map_count++ == 0)
> @@ -1241,6 +1331,9 @@ map_gtt(drm_intel_bo *bo)
>  	drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
>  	int ret;
>  
> +	if (bo_gem->is_userptr)
> +		return -EINVAL;
> +
>  	if (bo_gem->map_count++ == 0)
>  		drm_intel_gem_bo_open_vma(bufmgr_gem, bo_gem);
>  
> @@ -1385,13 +1478,18 @@ int drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo)
>  
>  static int drm_intel_gem_bo_unmap(drm_intel_bo *bo)
>  {
> -	drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
> +	drm_intel_bufmgr_gem *bufmgr_gem;
>  	drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
>  	int ret = 0;
>  
>  	if (bo == NULL)
>  		return 0;
>  
> +	if (bo_gem->is_userptr)
> +		return 0;
> +
> +	bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
> +
>  	pthread_mutex_lock(&bufmgr_gem->lock);
>  
>  	if (bo_gem->map_count <= 0) {
> @@ -1449,6 +1547,9 @@ drm_intel_gem_bo_subdata(drm_intel_bo *bo, unsigned long offset,
>  	struct drm_i915_gem_pwrite pwrite;
>  	int ret;
>  
> +	if (bo_gem->is_userptr)
> +		return -EINVAL;
> +
>  	VG_CLEAR(pwrite);
>  	pwrite.handle = bo_gem->gem_handle;
>  	pwrite.offset = offset;
> @@ -1501,6 +1602,9 @@ drm_intel_gem_bo_get_subdata(drm_intel_bo *bo, unsigned long offset,
>  	struct drm_i915_gem_pread pread;
>  	int ret;
>  
> +	if (bo_gem->is_userptr)
> +		return -EINVAL;
> +
>  	VG_CLEAR(pread);
>  	pread.handle = bo_gem->gem_handle;
>  	pread.offset = offset;
> @@ -2460,6 +2564,12 @@ drm_intel_gem_bo_set_tiling(drm_intel_bo *bo, uint32_t * tiling_mode,
>  	drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
>  	int ret;
>  
> +	/* Tiling with userptr surfaces is not supported
> +	 * on all hardware so refuse it for time being.
> +	 */
> +	if (bo_gem->is_userptr)
> +		return -EINVAL;
> +
>  	/* Linear buffers have no stride. By ensuring that we only ever use
>  	 * stride 0 with linear buffers, we simplify our code.
>  	 */
> @@ -3181,6 +3291,53 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo *bo,
>  	bo_gem->aub_annotation_count = count;
>  }
>  
> +static bool
> +has_userptr(drm_intel_bufmgr_gem *bufmgr_gem)
> +{
> +	int ret;
> +	void *ptr;
> +	long pgsz;
> +	struct drm_i915_gem_userptr userptr;
> +	struct drm_gem_close close_bo;
> +
> +	pgsz = sysconf(_SC_PAGESIZE);
> +	assert(pgsz > 0);
> +
> +	ret = posix_memalign(&ptr, pgsz, pgsz);
> +	if (ret) {
> +		DBG("Failed to get a page (%ld) for userptr detection!\n",
> +			pgsz);
> +		return false;
> +	}
> +
> +	memset(&userptr, 0, sizeof(userptr));
> +	userptr.user_ptr = (__u64)(unsigned long)ptr;
> +	userptr.user_size = pgsz;
> +
> +retry:
> +	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_USERPTR, &userptr);
> +	if (ret) {
> +		if (errno == ENODEV && userptr.flags == 0) {
> +			userptr.flags = I915_USERPTR_UNSYNCHRONIZED;
> +			goto retry;
> +		}
> +		free(ptr);
> +		return false;
> +	}
> +
> +	close_bo.handle = userptr.handle;
> +	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_GEM_CLOSE, &close_bo);
> +	if (ret == 0) {
> +		free(ptr);
> +	} else {
> +		fprintf(stderr, "Failed to release test userptr object! (%d) "
> +				"i915 kernel driver may not be sane!\n", errno);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  /**
>   * Initializes the GEM buffer manager, which uses the kernel to allocate, map,
>   * and manage map buffer objections.
> @@ -3273,6 +3430,10 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
>  	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp);
>  	bufmgr_gem->has_relaxed_fencing = ret == 0;
>  
> +	if (has_userptr(bufmgr_gem))
> +		bufmgr_gem->bufmgr.bo_alloc_userptr =
> +			drm_intel_gem_bo_alloc_userptr;
> +
>  	gp.param = I915_PARAM_HAS_WAIT_TIMEOUT;
>  	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GETPARAM, &gp);
>  	bufmgr_gem->has_wait_timeout = ret == 0;
> diff --git a/intel/intel_bufmgr_priv.h b/intel/intel_bufmgr_priv.h
> index 2592d42..3aa1abb 100644
> --- a/intel/intel_bufmgr_priv.h
> +++ b/intel/intel_bufmgr_priv.h
> @@ -60,7 +60,17 @@ struct _drm_intel_bufmgr {
>  					      const char *name,
>  					      unsigned long size,
>  					      unsigned int alignment);
> -
> +	/**
> +	 * Allocate a buffer object from an existing user accessible
> +	 * address malloc'd with the provided size.
> +	 * Alignment is used when mapping to the gtt.
> +	 * Flags may be I915_VMAP_READ_ONLY or I915_USERPTR_UNSYNCHRONIZED
> +	 */
> +	drm_intel_bo *(*bo_alloc_userptr)(drm_intel_bufmgr *bufmgr,
> +					  const char *name, void *addr,
> +					  uint32_t tiling_mode, uint32_t stride,
> +					  unsigned long size,
> +					  unsigned long flags);
>  	/**
>  	 * Allocate a tiled buffer object.
>  	 *
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx