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

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Wed Dec 9 06:06:50 PST 2015


Hi,

On 09/12/15 12:46, 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)
>
> v5: Corrected function name (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 8e554d3..d45274e 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 d57e850..296e63f 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_object_clear(obj);
> +		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)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct drm_i915_gem_object *obj;
> @@ -593,7 +593,7 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
>   	if (!drm_mm_initialized(&dev_priv->mm.stolen))
>   		return NULL;
>
> -	DRM_DEBUG_KMS("creating stolen object: size=%x\n", size);
> +	DRM_DEBUG_KMS("creating stolen object: size=%llx\n", size);
>   	if (size == 0)
>   		return NULL;
>
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 67cebe6..8e7e3a4 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -356,6 +356,7 @@ typedef struct drm_i915_irq_wait {
>   #define I915_PARAM_EU_TOTAL		 34
>   #define I915_PARAM_HAS_GPU_RESET	 35
>   #define I915_PARAM_HAS_RESOURCE_STREAMER 36
> +#define I915_PARAM_CREATE_VERSION	 37
>
>   typedef struct drm_i915_getparam {
>   	__s32 param;
> @@ -455,6 +456,21 @@ struct drm_i915_gem_create {
>   	 */
>   	__u32 handle;
>   	__u32 pad;
> +	/**
> +	 * Requested flags (currently used for placement
> +	 * (which memory domain))
> +	 *
> +	 * You can request that the object be created from special memory
> +	 * rather than regular system pages using this parameter. Such
> +	 * irregular objects may have certain restrictions (such as CPU
> +	 * access to a stolen object is verboten).
> +	 *
> +	 * This can be used in the future for other purposes too
> +	 * e.g. specifying tiling/caching/madvise
> +	 */
> +	__u32 flags;
> +#define I915_CREATE_PLACEMENT_STOLEN 	(1<<0) /* Cannot use CPU mmaps */
> +#define __I915_CREATE_UNKNOWN_FLAGS	-(I915_CREATE_PLACEMENT_STOLEN << 1)

I've asked in another reply, now that userspace can create a stolen 
object, what happens if it tries to use it for a batch buffer?

Can it end up in the relocate_entry_cpu with a batch buffer allocated 
from stolen, which would then call i915_gem_object_get_page and crash?

>   };
>
>   struct drm_i915_gem_pread {
>

Regards,

Tvrtko


More information about the Intel-gfx mailing list