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

[Chris Wilson]

On Tue, Dec 22, 2015 at 11:50:47AM +0530, ankitprasad.r.sharma at intel.com wrote:
> @@ -456,6 +457,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
> +	 */
> +	__u64 flags;

We've just been discussing future flags, and it seems like we want to
reserve the first 8 bits as a placement enum.

#define I915_CREATE_PLACEMENT_NORMAL 	0 /* standard swappable bo */
#define I915_CREATE_PLACEMENT_STOLEN 	1 /* Cannot use CPU mmaps */
/*
#define I915_CREATE_PLACEMENT_PRIVATE_ACTIVE 	2 /* From the file private freed pool */
#define I915_CREATE_PLACEMENT_PRIVATE_INACTIVE 	3 /* From the file private freed pool */
*/
#define I915_CREATE_PLACEMENT_MASK 0xff
#define __I915_CREATE_UNKNOWN_FLAGS	~I915_CREATE_PLACEMENT_MASK

So thinking ahead, rearranging this

>  	/* 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);
> +	}

as something like:

	u32 placement = flags & I915_CREATE_PLACEMENT_MASK;

	switch (placement) {
	/*
	case I915_CREATE_PLACEMENT_PRIVATE_ACTIVE:
	case I915_CREATE_PLACEMENT_PRIVATE_INACTIVE:
		use_private_pool = true;
		obj = alloc_from_pool(file, size, placement == ACTIVE);
		if (obj != NULL)
			break;
		/* fallthrough */
	*/
	case I915_CREATE_PLACEMENT_NORMAL:
		obj = i915_gem_alloc_object(dev, size);
		break;
	case I915_CREATE_PLACEMENT_STOLEN:
		obj = alloc_from_stolen(dev, size);
		break;
	}

would ease my future plans and look a bit neater :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre