[Intel-gfx] [PATCH 2/6] drm/i915: Support for creating Stolen memory backed objects

[Chris Wilson]

On Wed, Nov 11, 2015 at 04:06:09PM +0530, ankitprasad.r.sharma at intel.com wrote:
> From: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
> 
> Extend the drm_i915_gem_create structure to add support for
> creating Stolen memory backed objects. Added a new flag through
> which user can specify the preference to allocate the object from
> stolen memory, which if set, an attempt will be made to allocate
> the object from stolen memory subject to the availability of
> free space in the stolen region.
> 
> v2: Rebased to the latest drm-intel-nightly (Ankit)
> 
> v3: Changed versioning of GEM_CREATE param, added new comments (Tvrtko)
> 
> v4: Changed size from 32b to 64b to prevent userspace overflow (Tvrtko)
> Corrected function arguments ordering (Chris)
> 
> Testcase: igt/gem_stolen
> 
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c        |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h        |  2 +-
>  drivers/gpu/drm/i915/i915_gem.c        | 30 +++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_gem_stolen.c |  4 ++--
>  include/uapi/drm/i915_drm.h            | 16 ++++++++++++++++
>  5 files changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index ffcb9c6..6927c7e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -170,6 +170,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_HAS_RESOURCE_STREAMER:
>  		value = HAS_RESOURCE_STREAMER(dev);
>  		break;
> +	case I915_PARAM_CREATE_VERSION:
> +		value = 2;
> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 39b29d3..9c731f6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3213,7 +3213,7 @@ void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
>  int i915_gem_init_stolen(struct drm_device *dev);
>  void i915_gem_cleanup_stolen(struct drm_device *dev);
>  struct drm_i915_gem_object *
> -i915_gem_object_create_stolen(struct drm_device *dev, u32 size);
> +i915_gem_object_create_stolen(struct drm_device *dev, u64 size);
>  struct drm_i915_gem_object *
>  i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>  					       u32 stolen_offset,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 645b8a0..898904e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -375,6 +375,7 @@ static int
>  i915_gem_create(struct drm_file *file,
>  		struct drm_device *dev,
>  		uint64_t size,
> +		uint32_t flags,
>  		uint32_t *handle_p)
>  {
>  	struct drm_i915_gem_object *obj;
> @@ -385,8 +386,31 @@ i915_gem_create(struct drm_file *file,
>  	if (size == 0)
>  		return -EINVAL;
>  
> +	if (flags & __I915_CREATE_UNKNOWN_FLAGS)
> +		return -EINVAL;
> +
>  	/* Allocate the new object */
> -	obj = i915_gem_alloc_object(dev, size);
> +	if (flags & I915_CREATE_PLACEMENT_STOLEN) {
> +		mutex_lock(&dev->struct_mutex);
> +		obj = i915_gem_object_create_stolen(dev, size);
> +		if (!obj) {
> +			mutex_unlock(&dev->struct_mutex);
> +			return -ENOMEM;
> +		}
> +
> +		/* Always clear fresh buffers before handing to userspace */
> +		ret = i915_gem_clear_object(obj);

Missed it in the last patch, this should be i915_gem_object_clear().
(the idiom is object_verb).
> +		if (ret) {
> +			drm_gem_object_unreference(&obj->base);
> +			mutex_unlock(&dev->struct_mutex);
> +			return ret;
> +		}
> +
> +		mutex_unlock(&dev->struct_mutex);
> +	} else {
> +		obj = i915_gem_alloc_object(dev, size);
> +	}
> +
>  	if (obj == NULL)
>  		return -ENOMEM;
>  
> @@ -409,7 +433,7 @@ i915_gem_dumb_create(struct drm_file *file,
>  	args->pitch = ALIGN(args->width * DIV_ROUND_UP(args->bpp, 8), 64);
>  	args->size = args->pitch * args->height;
>  	return i915_gem_create(file, dev,
> -			       args->size, &args->handle);
> +			       args->size, 0, &args->handle);
>  }
>  
>  /**
> @@ -422,7 +446,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
>  	struct drm_i915_gem_create *args = data;
>  
>  	return i915_gem_create(file, dev,
> -			       args->size, &args->handle);
> +			       args->size, args->flags, &args->handle);
>  }
>  
>  static inline int
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 598ed2f..b98a3bf 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -583,7 +583,7 @@ cleanup:
>  }
>  
>  struct drm_i915_gem_object *
> -i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
> +i915_gem_object_create_stolen(struct drm_device *dev, u64 size)

I look forward to the machines with >4GiB of stolen! More seriously
though, you need to propage the 64bit conversion through to
stolen_alloc() so that we don't truncate it before we figure out there
isn't enough memory.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre