[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