public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [RFC 0/3] Add new ioctl to resize gem object for deferred allocation
@ 2014-03-27 15:28 arun.siluvery
  2014-03-27 15:28 ` [RFC 1/3] drm/i915: Prepare gem object to handle resize arun.siluvery
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: arun.siluvery @ 2014-03-27 15:28 UTC (permalink / raw)
  To: intel-gfx

From: "Siluvery, Arun" <arun.siluvery@intel.com>

This patch series adds a new ioctl to resize a gem object.

This is required in cases where the actual size of the object is not known
at the time of creation and there is chance that we may need more space later.

A typical use case is memory allocation for mipmaps where you cannot know
whether higher level mipmaps are required or not. If the object is resizeable
we can initially allocate only for level0 and when we know that higher levels
are required we can resize and allocate the rest. This is desirable in case of
low memory devices.

The usage scenario is,
1. Driver allocates original single-level texture.
2. GPU blit for level 0 data added to command buffer by driver.
3. OpenGL Driver requests resize of texture with all mip levels.
4. GPU blit for level 1 data added to command buffer by driver.
5. Command buffer submitted.

The size of gem object is constant and this fact is tightly coupled in i915, so
Daniel suggested an alternative approach. In this approach a scratch page is 
used for lazy allocation.

When the object is created, a new parameter is added to specify the size for
which the backing store is required initially along with the total size. 
A scratch page is also created for lazy allocation. The scatter/gather table
is created for the total size but the entries whose backing store is not created
point to the scratch page. A stop marker is introduced to denote the end of
real pages.

When the resize request is received, new pages are created and dummy entries
are updated with real entries.

In the mipmap usecase there ever will be only one resize request so single
stop marker is sufficient to handle this.

There is a basic i-g-t to test this, I will send it as a separate patch.

This is a not a complete implementation, I am sending this mainly to get
feedback. Please review and let me know how this is going in the right 
direction, how it can be improved, missing things and any other suggestions.

regards
Arun


Siluvery, Arun (3):
  drm/i915: Prepare gem object to handle resize
  drm/i915: Handle gem object resize using scratch page for lazy
    allocation
  drm/i915: Create new ioctl to request gem object resize

 drivers/gpu/drm/i915/i915_dma.c |   2 +
 drivers/gpu/drm/i915/i915_drv.h |   9 ++
 drivers/gpu/drm/i915/i915_gem.c | 191 +++++++++++++++++++++++++++++++++++++++-
 include/uapi/drm/i915_drm.h     |  33 +++++++
 4 files changed, 234 insertions(+), 1 deletion(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC 1/3] drm/i915: Prepare gem object to handle resize
  2014-03-27 15:28 [RFC 0/3] Add new ioctl to resize gem object for deferred allocation arun.siluvery
@ 2014-03-27 15:28 ` arun.siluvery
  2014-03-27 15:28 ` [RFC 2/3] drm/i915: Handle gem object resize using scratch page for lazy allocation arun.siluvery
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: arun.siluvery @ 2014-03-27 15:28 UTC (permalink / raw)
  To: intel-gfx

From: "Siluvery, Arun" <arun.siluvery@intel.com>

This patch adds data structure to handle gem object resize.
One such usecase where it is required is mipmaps; you cannot know
whether higher level mipmaps are required at the time of creating them,
so it is best to defer memory allocation for higher levels if possible.
GEM object should be resizeable to achieve this.

Add new parameter to i915_gem_create to specify initial size of backing store.

When the object is created we allocate backing store only for base size,
in the case of mipmaps it would be level0.
scratch page is used for lazy allocation (Daniel)
A stop marker denotes the end of real pages.
if higher levels are required additional space is requested using new ioctl.

Although the usecase considered is mipmaps, it can be used for other purposes.

Change-Id: I1acf539bb7d8d861deb7fbb1d2f32265f22ad28f
Signed-off-by: Siluvery, Arun <arun.siluvery@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  7 +++++++
 drivers/gpu/drm/i915/i915_gem.c | 28 +++++++++++++++++++++++++++-
 include/uapi/drm/i915_drm.h     |  7 +++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4069800..cf65aad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1875,6 +1875,13 @@ struct drm_i915_gem_object {
 		} userptr;
 	};
 
+	/* to handle resizing of gem object */
+	struct i915_gem_resize {
+		uint32_t stop;
+		uint32_t base_size;
+		struct page *scratch_page;
+	} gem_resize;
+
 	/** Object userdata */
 	uint32_t userdata;
 };
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6153e01..71d7526 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -311,10 +311,36 @@ int
 i915_gem_create_ioctl(struct drm_device *dev, void *data,
 		      struct drm_file *file)
 {
+	int ret;
+	int total_page_count;
+	struct drm_i915_gem_object *obj;
 	struct drm_i915_gem_create *args = data;
 
-	return i915_gem_create(file, dev,
+	ret = i915_gem_create(file, dev,
 			       args->size, &args->handle);
+
+	if (ret)
+		return ret;
+
+	/*
+	 * TODO: args->pad is filled with garbage
+	 * Need a better method to identify that its a resizeable object,
+	 * use a magic number temporarily
+	 */
+	if (args->pad != 0x12345678)
+		return ret;
+
+	obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
+	if (&obj->base == NULL) {
+		printk(KERN_ERR "%s(), object not found\n", __func__);
+		return -ENOENT;
+	}
+	total_page_count = obj->base.size / PAGE_SIZE;
+	obj->gem_resize.base_size = 0;
+	obj->gem_resize.stop = total_page_count;
+	obj->gem_resize.scratch_page = NULL;
+
+	return 0;
 }
 
 static inline int
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index aa8469e..e455c9d 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -521,6 +521,13 @@ struct drm_i915_gem_create {
 	 */
 	__u32 handle;
 	__u32 pad;
+	/**
+	 * When an object needs to be resized this denotes the size for
+	 * which backing storage is created initially
+	 * eg in case of mipmaps, size of level0
+	 * object is resized later to allocate for higher levels
+	 */
+	__u64 base_size;
 };
 
 struct drm_i915_gem_pread {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC 2/3] drm/i915: Handle gem object resize using scratch page for lazy allocation
  2014-03-27 15:28 [RFC 0/3] Add new ioctl to resize gem object for deferred allocation arun.siluvery
  2014-03-27 15:28 ` [RFC 1/3] drm/i915: Prepare gem object to handle resize arun.siluvery
@ 2014-03-27 15:28 ` arun.siluvery
  2014-03-27 15:28 ` [RFC 3/3] drm/i915: Create new ioctl to request gem object resize arun.siluvery
  2014-03-27 22:23 ` [RFC 0/3] Add new ioctl to resize gem object for deferred allocation Chris Wilson
  3 siblings, 0 replies; 8+ messages in thread
From: arun.siluvery @ 2014-03-27 15:28 UTC (permalink / raw)
  To: intel-gfx

From: "Siluvery, Arun" <arun.siluvery@intel.com>

GEM object size is fixed and it is tightly coupled in i915.
To make it resizeable a lazy allocation approach is used.
Out of the total size of object, backing store is allocated partially.
The scatter/gather entries for the remaining pages point to scratch page.

A stop marker denotes the end of real pages. For the mipmaps usecase there ever
will be only one resize request hence one value is enough to track single range.

The dummy entries are updated with real pages when it is resized.

Change-Id: I645a0f9817f43bd127d038d9c17cba8466b7ba6e
Signed-off-by: Siluvery, Arun <arun.siluvery@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 54 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 71d7526..d045eee 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -313,6 +313,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
 {
 	int ret;
 	int total_page_count;
+	int base_page_count, scratch_page_count;
 	struct drm_i915_gem_object *obj;
 	struct drm_i915_gem_create *args = data;
 
@@ -340,7 +341,35 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
 	obj->gem_resize.stop = total_page_count;
 	obj->gem_resize.scratch_page = NULL;
 
-	return 0;
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		return ret;
+
+	base_page_count = args->base_size / PAGE_SIZE;
+	if (base_page_count > total_page_count) {
+		DRM_DEBUG_DRIVER("invalid object size, base_size(%d) > total_size(%d)\n",
+				 base_page_count, total_page_count);
+		goto unlock;
+	}
+	obj->gem_resize.base_size = args->base_size;
+	scratch_page_count = total_page_count - base_page_count;
+	/* allocate backing store only for base size */
+	obj->gem_resize.stop = base_page_count;
+
+	obj->gem_resize.scratch_page = alloc_page(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO);
+	if (obj->gem_resize.scratch_page == NULL) {
+		DRM_DEBUG_DRIVER("No memory to allocate scratch page\n");
+		ret = -ENOMEM;
+		goto unlock;
+	}
+	DRM_DEBUG_DRIVER("scratch page created 0x%p, base(%d) + scratch(%d) = total(%d)\n",
+			 obj->gem_resize.scratch_page, base_page_count,
+			 scratch_page_count, total_page_count);
+
+unlock:
+	mutex_unlock(&dev->struct_mutex);
+
+	return ret;
 }
 
 static inline int
@@ -1911,6 +1940,11 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 	struct page *page;
 	unsigned long last_pfn = 0;	/* suppress gcc warning */
 	gfp_t gfp;
+	int j;
+	bool allow_resize = false;
+	struct page *scratch_page = NULL;
+	int scratch_page_count = 0;
+	uint32_t stop;
 
 	/* Assert that the object is not currently in any GPU domain. As it
 	 * wasn't in the GTT, there shouldn't be any way it could have been in
@@ -1929,6 +1963,15 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 		return -ENOMEM;
 	}
 
+	stop = obj->gem_resize.stop;
+	if (stop && (stop < page_count) && obj->gem_resize.scratch_page) {
+		DRM_DEBUG_DRIVER("allow this object to resize later\n");
+		allow_resize = true;
+		scratch_page_count = page_count - obj->gem_resize.stop;
+		page_count = obj->gem_resize.stop;
+		scratch_page = obj->gem_resize.scratch_page;
+	}
+
 	/* Get the list of pages out of our struct file.  They'll be pinned
 	 * at this point until we release them.
 	 *
@@ -1983,6 +2026,15 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 		/* Check that the i965g/gm workaround works. */
 		WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x00100000UL));
 	}
+
+	if (allow_resize == true) {
+		for (j = 0; j < scratch_page_count; ++j) {
+			st->nents++;
+			sg = sg_next(sg);
+			sg_set_page(sg, scratch_page, PAGE_SIZE, 0);
+		}
+	}
+
 #ifdef CONFIG_SWIOTLB
 	if (!swiotlb_nr_tbl())
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC 3/3] drm/i915: Create new ioctl to request gem object resize
  2014-03-27 15:28 [RFC 0/3] Add new ioctl to resize gem object for deferred allocation arun.siluvery
  2014-03-27 15:28 ` [RFC 1/3] drm/i915: Prepare gem object to handle resize arun.siluvery
  2014-03-27 15:28 ` [RFC 2/3] drm/i915: Handle gem object resize using scratch page for lazy allocation arun.siluvery
@ 2014-03-27 15:28 ` arun.siluvery
  2014-03-27 22:23 ` [RFC 0/3] Add new ioctl to resize gem object for deferred allocation Chris Wilson
  3 siblings, 0 replies; 8+ messages in thread
From: arun.siluvery @ 2014-03-27 15:28 UTC (permalink / raw)
  To: intel-gfx

From: "Siluvery, Arun" <arun.siluvery@intel.com>

This patch adds a new ioctl to resize gem object. If the object is created
as resizeable scatter/gather table contains dummy entries that point to a
scratch page. A marker denotes end of real pages.

This ioctl creates new pages and updates the dummy entries in the sg table.
The object size can only be increased, resize request fails otherwise.

Change-Id: I56a7f798e578753623044bb7f6216c70dbaf7543
Signed-off-by: Siluvery, Arun <arun.siluvery@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |   2 +
 drivers/gpu/drm/i915/i915_drv.h |   2 +
 drivers/gpu/drm/i915/i915_gem.c | 111 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h     |  26 ++++++++++
 4 files changed, 141 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 31c499f..2ff1c46 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2000,6 +2000,8 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, \
 						DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_RESIZE, i915_gem_resize_ioctl, \
+						DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_SET_PLANE_180_ROTATION, \
 		i915_set_plane_180_rotation, DRM_AUTH | DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_ENABLE_PLANE_RESERVED_REG_BIT_2,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf65aad..a426f0f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2217,6 +2217,8 @@ int i915_gem_get_tiling(struct drm_device *dev, void *data,
 int i915_gem_init_userptr(struct drm_device *dev);
 int i915_gem_userptr_ioctl(struct drm_device *dev, void *data,
 				struct drm_file *file);
+int i915_gem_resize_ioctl(struct drm_device *dev, void *data,
+				struct drm_file *file);
 int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
 int i915_gem_wait_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d045eee..8c45d3b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -372,6 +372,117 @@ unlock:
 	return ret;
 }
 
+/**
+ * Resizes an existing gem object.
+ */
+int i915_gem_resize_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
+{
+	int i, ret;
+	int page_range;
+	int total_page_count, resize_page_count;
+	struct drm_i915_private *dev_priv;
+	struct drm_i915_gem_object *obj;
+	struct drm_i915_gem_resize *args = data;
+	struct address_space *mapping;
+	struct sg_page_iter sg_iter;
+	struct page *page;
+	gfp_t gfp;
+
+	obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
+	if (&obj->base == NULL)
+		return -ENOENT;
+
+	/* Not valid to be called on unbound objects. */
+	if (!i915_gem_obj_bound_any(obj))
+		return -EINVAL;
+
+	if (obj->base.size != args->size ||
+		obj->gem_resize.base_size != args->curr_size) {
+		DRM_DEBUG_DRIVER("invalid object size(s)\n");
+		return -EINVAL;
+	}
+
+	total_page_count = obj->base.size / PAGE_SIZE;
+	resize_page_count = args->resize / PAGE_SIZE;
+	if (resize_page_count < obj->gem_resize.stop) {
+		DRM_DEBUG_DRIVER("cannot resize to smaller size\n");
+		return -EINVAL;
+	}
+
+	DRM_DEBUG_DRIVER("resize object from %d to %d pages, total pages %d\n",
+			 obj->gem_resize.stop, resize_page_count, total_page_count);
+
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		return ret;
+
+	if (!obj->pages) {
+		DRM_DEBUG_DRIVER("backing store not yet allocated for this obj\n");
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	dev_priv = obj->base.dev->dev_private;
+	page_range = resize_page_count - obj->gem_resize.stop;
+	mapping = file_inode(obj->base.filp)->i_mapping;
+	gfp = mapping_gfp_mask(mapping);
+	gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
+	gfp &= ~(__GFP_IO | __GFP_WAIT);
+
+	i = 0;
+	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0) {
+		page = sg_page_iter_page(&sg_iter);
+
+		if (i < obj->gem_resize.stop) {
+			++i;
+			continue;
+		}
+		WARN_ON(page != obj->gem_resize.scratch_page);
+
+		page = shmem_read_mapping_page_gfp(mapping, i, gfp);
+		if (IS_ERR(page)) {
+			i915_gem_purge(dev_priv, page_count);
+			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
+		}
+		if (IS_ERR(page)) {
+			/* We've tried hard to allocate the memory by reaping
+			 * our own buffer, now let the real VM do its job and
+			 * go down in flames if truly OOM.
+			 */
+			gfp &= ~(__GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD);
+			gfp |= __GFP_IO | __GFP_WAIT;
+
+			i915_gem_shrink_all(dev_priv);
+			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
+			if (IS_ERR(page)) {
+				DRM_ERROR("out of memory\n");
+				goto err_pages;
+			}
+
+			gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
+			gfp &= ~(__GFP_IO | __GFP_WAIT);
+		}
+		sg_set_page((&sg_iter)->sg, page, PAGE_SIZE, 0);
+
+		++i;
+	}
+	obj->gem_resize.stop = resize_page_count;
+
+	mutex_unlock(&dev->struct_mutex);
+	return 0;
+
+err_pages:
+	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
+		page_cache_release(sg_page_iter_page(&sg_iter));
+	sg_free_table(obj->pages);
+	kfree(obj->pages);
+	ret = PTR_ERR(page);
+
+unlock:
+	mutex_unlock(&dev->struct_mutex);
+	return ret;
+}
+
 static inline int
 __copy_to_user_swizzled(char __user *cpu_vaddr,
 			const char *gpu_vaddr, int gpu_offset,
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e455c9d..a807ff4 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -275,6 +275,7 @@ struct csc_coeff {
 #define DRM_I915_GET_RESET_STATS	0x32
 #define DRM_I915_SET_PLANE_ZORDER	0x33
 #define DRM_I915_GEM_USERPTR		0x34
+#define DRM_I915_GEM_RESIZE		0x35
 #define DRM_I915_SET_PLANE_180_ROTATION 0x36
 #define DRM_I915_ENABLE_PLANE_RESERVED_REG_BIT_2	0x37
 #define DRM_I915_SET_CSC		0x39
@@ -339,6 +340,9 @@ struct csc_coeff {
 #define DRM_IOCTL_I915_GEM_USERPTR \
 		DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, \
 				struct drm_i915_gem_userptr)
+#define DRM_IOCTL_I915_GEM_RESIZE \
+		DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_RESIZE, \
+				struct drm_i915_gem_resize)
 #define DRM_IOCTL_I915_SET_PLANE_ALPHA		\
 			DRM_IOW(DRM_COMMAND_BASE + DRM_I915_SET_PLANE_ALPHA, \
 			struct drm_i915_set_plane_alpha)
@@ -530,6 +534,28 @@ struct drm_i915_gem_create {
 	__u64 base_size;
 };
 
+struct drm_i915_gem_resize {
+	/**
+	 * The size for which the backing store is created initially
+	 */
+	__u64 curr_size;
+	/**
+	 * The new size of the object
+	 */
+	__u64 resize;
+	/**
+	 * Requested size for the object, remains constant.
+	 */
+	__u64 size;
+	__u32 flags;
+	/**
+	 * Returned handle for the object.
+	 *
+	 * Object handles are nonzero.
+	 */
+	__u32 handle;
+};
+
 struct drm_i915_gem_pread {
 	/** Handle for the object being read. */
 	__u32 handle;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC 0/3] Add new ioctl to resize gem object for deferred allocation
  2014-03-27 15:28 [RFC 0/3] Add new ioctl to resize gem object for deferred allocation arun.siluvery
                   ` (2 preceding siblings ...)
  2014-03-27 15:28 ` [RFC 3/3] drm/i915: Create new ioctl to request gem object resize arun.siluvery
@ 2014-03-27 22:23 ` Chris Wilson
  2014-04-04 10:45   ` Siluvery, Arun
  2014-04-07 13:18   ` Siluvery, Arun
  3 siblings, 2 replies; 8+ messages in thread
From: Chris Wilson @ 2014-03-27 22:23 UTC (permalink / raw)
  To: arun.siluvery; +Cc: intel-gfx

On Thu, Mar 27, 2014 at 03:28:26PM +0000, arun.siluvery@linux.intel.com wrote:
> From: "Siluvery, Arun" <arun.siluvery@intel.com>
> 
> This patch series adds a new ioctl to resize a gem object.

I'm tired, but off the top of my head, I think you can do away with the
magic extension to create_ioctl. If we allow any one to fallocate()
ranges of any object, the user can create a large object, populate it
all with a scratch page, then later populate regions as required. This
looks quite a reasonable and useful feature.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC 0/3] Add new ioctl to resize gem object for deferred allocation
  2014-03-27 22:23 ` [RFC 0/3] Add new ioctl to resize gem object for deferred allocation Chris Wilson
@ 2014-04-04 10:45   ` Siluvery, Arun
  2014-04-07 13:18   ` Siluvery, Arun
  1 sibling, 0 replies; 8+ messages in thread
From: Siluvery, Arun @ 2014-04-04 10:45 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On 27/03/2014 22:23, Chris Wilson wrote:
> On Thu, Mar 27, 2014 at 03:28:26PM +0000, arun.siluvery@linux.intel.com wrote:
>> From: "Siluvery, Arun" <arun.siluvery@intel.com>
>>
>> This patch series adds a new ioctl to resize a gem object.
>
> I'm tired, but off the top of my head, I think you can do away with the
> magic extension to create_ioctl. If we allow any one to fallocate()
> ranges of any object, the user can create a large object, populate it
> all with a scratch page, then later populate regions as required. This
> looks quite a reasonable and useful feature.
> -Chris
>
Sorry for the delayed response, I was on holiday. I will modify the 
implementation to use fallocate(), shmem is supporting this and it seems 
to be a better approach.
In this case once the object is created we extend gem_create ioctl using 
fallocate() and allocate the additional space but does the userspace 
still see it as a contiguous block when it writes to the additional space?
Once the object is modified using fallocate(), are these changes 
completely transparent when this object is used in other functions?
please clarify.

regards
Arun

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC 0/3] Add new ioctl to resize gem object for deferred allocation
  2014-03-27 22:23 ` [RFC 0/3] Add new ioctl to resize gem object for deferred allocation Chris Wilson
  2014-04-04 10:45   ` Siluvery, Arun
@ 2014-04-07 13:18   ` Siluvery, Arun
  2014-04-07 14:54     ` Tvrtko Ursulin
  1 sibling, 1 reply; 8+ messages in thread
From: Siluvery, Arun @ 2014-04-07 13:18 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On 27/03/2014 22:23, Chris Wilson wrote:
> On Thu, Mar 27, 2014 at 03:28:26PM +0000, arun.siluvery@linux.intel.com wrote:
>> From: "Siluvery, Arun" <arun.siluvery@intel.com>
>>
>> This patch series adds a new ioctl to resize a gem object.
>
> I'm tired, but off the top of my head, I think you can do away with the
> magic extension to create_ioctl. If we allow any one to fallocate()
> ranges of any object, the user can create a large object, populate it
> all with a scratch page, then later populate regions as required. This
> looks quite a reasonable and useful feature.
> -Chris
>
Could you clarify my understanding on how to use fallocate() to resize 
the object, considering the case where we start with an object of size x 
and want to increase its size  to (x+y) at a later point.
My understanding is, first object is created with gem_create ioctl with 
size x. At a later point if it is to be resized, we allocate y at the 
end of x using fallocate(). It is allocating the pages for us and from 
its implementation it is clear that the file size is updated with new 
value if offset+len is greater than initial size.
Do we need to make any changes for GEM to get the new size or it is 
completely transparent to it?
could you give an overview on how you think it should work?

regards
Arun

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC 0/3] Add new ioctl to resize gem object for deferred allocation
  2014-04-07 13:18   ` Siluvery, Arun
@ 2014-04-07 14:54     ` Tvrtko Ursulin
  0 siblings, 0 replies; 8+ messages in thread
From: Tvrtko Ursulin @ 2014-04-07 14:54 UTC (permalink / raw)
  To: Siluvery, Arun, Chris Wilson, intel-gfx


On 04/07/2014 02:18 PM, Siluvery, Arun wrote:
> On 27/03/2014 22:23, Chris Wilson wrote:
>> On Thu, Mar 27, 2014 at 03:28:26PM +0000,
>> arun.siluvery@linux.intel.com wrote:
>>> From: "Siluvery, Arun" <arun.siluvery@intel.com>
>>>
>>> This patch series adds a new ioctl to resize a gem object.
>>
>> I'm tired, but off the top of my head, I think you can do away with the
>> magic extension to create_ioctl. If we allow any one to fallocate()
>> ranges of any object, the user can create a large object, populate it
>> all with a scratch page, then later populate regions as required. This
>> looks quite a reasonable and useful feature.
>> -Chris
>>
> Could you clarify my understanding on how to use fallocate() to resize
> the object, considering the case where we start with an object of size x
> and want to increase its size  to (x+y) at a later point.
> My understanding is, first object is created with gem_create ioctl with
> size x. At a later point if it is to be resized, we allocate y at the
> end of x using fallocate(). It is allocating the pages for us and from
> its implementation it is clear that the file size is updated with new
> value if offset+len is greater than initial size.
> Do we need to make any changes for GEM to get the new size or it is
> completely transparent to it?
> could you give an overview on how you think it should work?

My understanding of what Chris said is that fallocate(2) could be used 
to toggle ranges of an object - from "normal" backing store, to scratch 
page backing store.

There is a mode argument passed in to fallocate, so if that is zero you 
could populate the passed in range with scratch pages. Or if mode is 
FALLOC_FL_KEEP_SIZE you could allocate proper backing.

Tvrtko

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-04-07 14:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-27 15:28 [RFC 0/3] Add new ioctl to resize gem object for deferred allocation arun.siluvery
2014-03-27 15:28 ` [RFC 1/3] drm/i915: Prepare gem object to handle resize arun.siluvery
2014-03-27 15:28 ` [RFC 2/3] drm/i915: Handle gem object resize using scratch page for lazy allocation arun.siluvery
2014-03-27 15:28 ` [RFC 3/3] drm/i915: Create new ioctl to request gem object resize arun.siluvery
2014-03-27 22:23 ` [RFC 0/3] Add new ioctl to resize gem object for deferred allocation Chris Wilson
2014-04-04 10:45   ` Siluvery, Arun
2014-04-07 13:18   ` Siluvery, Arun
2014-04-07 14:54     ` Tvrtko Ursulin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox