[PATCH v2 0/5] Support for creating/using Stolen memory backed objects

[PATCH v2 0/5] Support for creating/using Stolen memory backed objects

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>

This patch series adds support for creating/using Stolen memory backed
objects.

Despite being a unified memory architecture (UMA) some bits of memory
are more equal than others. In particular we have the thorny issue of
stolen memory, memory stolen from the system by the BIOS and reserved
for igfx use. Stolen memory is required for some functions of the GPU
and display engine, but in general it goes wasted. Whilst we cannot
return it back to the system, we need to find some other method for
utilising it. As we do not support direct access to the physical address
in the stolen region, it behaves like a different class of memory,
closer in kin to local GPU memory. This strongly suggests that we need a
placement model like TTM if we are to fully utilize these discrete
chunks of differing memory.

To add support for creating Stolen memory backed objects, we extend the
drm_i915_gem_create structure, by adding 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.

This patch series adds support for clearing buffer objects via blitter
engines. This is particularly useful for clearing out the memory from
stolen region, but can also be used for other shmem allocated objects.
Also adding support for stealing purgable stolen pages, if we run out
of stolen memory when trying to allocate an object.

This also adds support for read/write from/to objects not backed by
shmem using the pread/pwrite interface.
Series also extends the current get_aperture ioctl to retrieve the
total and available size of the stolen region

This can be verified using IGT tests: igt/gem_create_stolen

Ankitprasad Sharma (4):
  drm/i915: Clearing buffer objects via blitter engine
  drm/i915: Support for creating Stolen memory backed objects
  drm/i915: Support for pread/pwrite from/to non shmem backed objects
  drm/i915: Add support for getting size of the stolen region

Chris Wilson (1):
  drm/i915: Add support for stealing purgable stolen pages

 drivers/gpu/drm/i915/Makefile          |   1 +
 drivers/gpu/drm/i915/i915_dma.c        |   3 +
 drivers/gpu/drm/i915/i915_drv.h        |  17 +++
 drivers/gpu/drm/i915/i915_gem.c        | 171 +++++++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_gem_exec.c   | 197 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_stolen.c | 121 ++++++++++++++++++--
 drivers/gpu/drm/i915/intel_lrc.c       |   2 +-
 drivers/gpu/drm/i915/intel_lrc.h       |   2 +
 include/uapi/drm/i915_drm.h            |  21 ++++
 9 files changed, 509 insertions(+), 26 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_gem_exec.c

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/5] drm/i915: Clearing buffer objects via blitter engine

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>

This patch adds support for clearing buffer objects via blitter
engines. This is particularly useful for clearing out the memory
from stolen region.

v2: Add support for using execlists & PPGTT

v3: Fix issues in legacy ringbuffer submission mode

testcase: igt/gem_create_stolen

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Deepak S <deepak.s@linux.intel.com>
Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
---
 drivers/gpu/drm/i915/Makefile        |   1 +
 drivers/gpu/drm/i915/i915_drv.h      |   4 +
 drivers/gpu/drm/i915/i915_gem_exec.c | 197 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c     |   2 +-
 drivers/gpu/drm/i915/intel_lrc.h     |   2 +
 5 files changed, 205 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/i915_gem_exec.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a69002e..711a87d 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -25,6 +25,7 @@ i915-y += i915_cmd_parser.o \
 	  i915_gem_debug.o \
 	  i915_gem_dmabuf.o \
 	  i915_gem_evict.o \
+	  i915_gem_exec.o \
 	  i915_gem_execbuffer.o \
 	  i915_gem_gtt.o \
 	  i915_gem.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb38cd1..21a2b1f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2927,6 +2927,10 @@ int __must_check i915_gem_evict_something(struct drm_device *dev,
 int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle);
 int i915_gem_evict_everything(struct drm_device *dev);
 
+/* i915_gem_exec.c */
+int i915_gem_exec_clear_object(struct drm_i915_gem_object *obj,
+			       struct drm_i915_file_private *file_priv);
+
 /* belongs in i915_gem_gtt.h */
 static inline void i915_gem_chipset_flush(struct drm_device *dev)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem_exec.c b/drivers/gpu/drm/i915/i915_gem_exec.c
new file mode 100644
index 0000000..224bd5f
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_gem_exec.c
@@ -0,0 +1,197 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Chris Wilson <chris at chris-wilson.co.uk>
+ *
+ */
+
+#include <drm/drmP.h>
+#include <drm/i915_drm.h>
+#include "i915_drv.h"
+
+#define GEN8_COLOR_BLT_CMD (2<<29 | 0x50<<22)
+
+#define BPP_8 0
+#define BPP_16 (1<<24)
+#define BPP_32 (1<<25 | 1<<24)
+
+#define ROP_FILL_COPY (0xf0 << 16)
+
+static int i915_gem_exec_flush_object(struct drm_i915_gem_object *obj,
+				      struct intel_engine_cs *ring,
+				      struct intel_context *ctx)
+{
+	int ret;
+	struct intel_ringbuffer *ringbuf = ctx->engine[ring->id].ringbuf;
+
+	ret = i915_gem_object_sync(obj, ring);
+	if (ret)
+		return ret;
+
+	if (obj->base.write_domain & I915_GEM_DOMAIN_CPU) {
+		if (i915_gem_clflush_object(obj, false))
+			i915_gem_chipset_flush(obj->base.dev);
+		obj->base.write_domain &= ~I915_GEM_DOMAIN_CPU;
+	}
+	if (obj->base.write_domain & I915_GEM_DOMAIN_GTT) {
+		wmb();
+		obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
+	}
+
+
+	return i915.enable_execlists ?
+			logical_ring_invalidate_all_caches(ringbuf, ctx) :
+			intel_ring_invalidate_all_caches(ring);
+}
+
+static void i915_gem_exec_dirty_object(struct drm_i915_gem_object *obj,
+				       struct intel_engine_cs *ring,
+				       struct i915_address_space *vm)
+{
+	struct drm_i915_gem_request *req;
+	req = intel_ring_get_request(ring);
+
+	i915_gem_request_assign(&obj->last_write_req, req);
+	obj->base.read_domains = I915_GEM_DOMAIN_RENDER;
+	obj->base.write_domain = I915_GEM_DOMAIN_RENDER;
+	i915_vma_move_to_active(i915_gem_obj_to_vma(obj, vm), ring);
+	obj->dirty = 1;
+
+	ring->gpu_caches_dirty = true;
+}
+
+int i915_gem_exec_clear_object(struct drm_i915_gem_object *obj,
+			       struct drm_i915_file_private *file_priv)
+{
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_engine_cs *ring;
+	struct intel_context *ctx;
+	struct intel_ringbuffer *ringbuf;
+	struct i915_address_space *vm;
+	int ret = 0;
+
+	lockdep_assert_held(&dev->struct_mutex);
+
+	ring = &dev_priv->ring[HAS_BLT(dev) ? BCS : RCS];
+	ctx = i915_gem_context_get(file_priv, DEFAULT_CONTEXT_HANDLE);
+	if (ctx->ppgtt)
+		vm = &ctx->ppgtt->base;
+	else
+		vm = &dev_priv->gtt.base;
+
+	if (i915.enable_execlists && !ctx->engine[ring->id].state) {
+		ret = intel_lr_context_deferred_create(ctx, ring);
+		if (ret)
+			return ret;
+	}
+
+	ringbuf = ctx->engine[ring->id].ringbuf;
+
+	ret = i915_gem_object_pin(obj, vm, PAGE_SIZE, 0);
+	if (ret)
+		return ret;
+
+	if (obj->tiling_mode && INTEL_INFO(dev)->gen <= 3) {
+		ret = i915_gem_object_put_fence(obj);
+		if (ret)
+			goto unpin;
+	}
+
+	ret = i915_gem_exec_flush_object(obj, ring, ctx);
+	if (ret)
+		goto unpin;
+
+	if (i915.enable_execlists) {
+		if (dev_priv->info.gen >= 8) {
+			ret = intel_logical_ring_begin(ringbuf, ctx, 8);
+			if (ret)
+				goto unpin;
+
+			intel_logical_ring_emit(ringbuf, GEN8_COLOR_BLT_CMD |
+							 BLT_WRITE_RGBA |
+							 (7-2));
+			intel_logical_ring_emit(ringbuf, BPP_32 |
+							 ROP_FILL_COPY |
+							 PAGE_SIZE);
+			intel_logical_ring_emit(ringbuf, 0);
+			intel_logical_ring_emit(ringbuf,
+						obj->base.size >> PAGE_SHIFT
+						<< 16 | PAGE_SIZE / 4);
+			intel_logical_ring_emit(ringbuf,
+						i915_gem_obj_offset(obj, vm));
+			intel_logical_ring_emit(ringbuf, 0);
+			intel_logical_ring_emit(ringbuf, 0);
+			intel_logical_ring_emit(ringbuf, MI_NOOP);
+
+			intel_logical_ring_advance(ringbuf);
+		} else {
+			DRM_ERROR("Execlists not supported for gen %d\n",
+				  dev_priv->info.gen);
+			ret = -EINVAL;
+			goto unpin;
+		}
+	} else {
+		if (IS_GEN8(dev)) {
+			ret = intel_ring_begin(ring, 8);
+			if (ret)
+				goto unpin;
+
+			intel_ring_emit(ring, GEN8_COLOR_BLT_CMD |
+					      BLT_WRITE_RGBA | (7-2));
+			intel_ring_emit(ring, BPP_32 |
+					      ROP_FILL_COPY | PAGE_SIZE);
+			intel_ring_emit(ring, 0);
+			intel_ring_emit(ring,
+					obj->base.size >> PAGE_SHIFT << 16 |
+					PAGE_SIZE / 4);
+			intel_ring_emit(ring, i915_gem_obj_offset(obj, vm));
+			intel_ring_emit(ring, 0);
+			intel_ring_emit(ring, 0);
+			intel_ring_emit(ring, MI_NOOP);
+		} else {
+			ret = intel_ring_begin(ring, 6);
+			if (ret)
+				goto unpin;
+
+			intel_ring_emit(ring, COLOR_BLT_CMD |
+					      BLT_WRITE_RGBA);
+			intel_ring_emit(ring, BPP_32 |
+					      ROP_FILL_COPY | PAGE_SIZE);
+			intel_ring_emit(ring,
+					obj->base.size >> PAGE_SHIFT << 16 |
+					PAGE_SIZE);
+			intel_ring_emit(ring, i915_gem_obj_offset(obj, vm));
+			intel_ring_emit(ring, 0);
+			intel_ring_emit(ring, MI_NOOP);
+		}
+
+		__intel_ring_advance(ring);
+	}
+
+	i915_gem_exec_dirty_object(obj, ring, vm);
+
+unpin:
+	i915_gem_obj_to_vma(obj, vm)->pin_count--;
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fcb074b..5481638 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -559,7 +559,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
 	return 0;
 }
 
-static int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf,
+int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf,
 					      struct intel_context *ctx)
 {
 	struct intel_engine_cs *ring = ringbuf->ring;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index adb731e4..80a873b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -42,6 +42,8 @@ int intel_logical_rings_init(struct drm_device *dev);
 
 int logical_ring_flush_all_caches(struct intel_ringbuffer *ringbuf,
 				  struct intel_context *ctx);
+int logical_ring_invalidate_all_caches(struct intel_ringbuffer *ringbuf,
+				       struct intel_context *ctx);
 /**
  * intel_logical_ring_advance() - advance the ringbuffer tail
  * @ringbuf: Ringbuffer to advance.
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/5] drm/i915: Support for creating Stolen memory backed objects

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@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.

testcase: igt/gem_create_stolen

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |  3 +++
 drivers/gpu/drm/i915/i915_gem.c | 31 +++++++++++++++++++++++++++----
 include/uapi/drm/i915_drm.h     | 15 +++++++++++++++
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index d49ed68..491c91f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -163,6 +163,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 		if (!value)
 			return -ENODEV;
 		break;
+	case I915_PARAM_CREATE_VERSION:
+		value = 1;
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 84e2a23..81c5381 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -391,7 +391,8 @@ static int
 i915_gem_create(struct drm_file *file,
 		struct drm_device *dev,
 		uint64_t size,
-		uint32_t *handle_p)
+		uint32_t *handle_p,
+		uint32_t flags)
 {
 	struct drm_i915_gem_object *obj;
 	int ret;
@@ -401,8 +402,29 @@ i915_gem_create(struct drm_file *file,
 	if (size == 0)
 		return -EINVAL;
 
+	if (flags & ~(I915_CREATE_PLACEMENT_STOLEN))
+		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;
+		}
+
+		ret = i915_gem_exec_clear_object(obj, file->driver_priv);
+		if (ret) {
+			i915_gem_object_free(obj);
+			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;
 
@@ -425,7 +447,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, &args->handle, 0);
 }
 
 /**
@@ -438,7 +460,8 @@ 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->handle,
+			       args->flags);
 }
 
 static inline int
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 8d1be90..ab4f3a9 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -350,6 +350,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_REVISION              32
 #define I915_PARAM_SUBSLICE_TOTAL	 33
 #define I915_PARAM_EU_TOTAL		 34
+#define I915_PARAM_CREATE_VERSION	 35
 
 typedef struct drm_i915_getparam {
 	int param;
@@ -445,6 +446,20 @@ 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 or pread/pwrite */
 };
 
 struct drm_i915_gem_pread {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/5] drm/i915: Add support for stealing purgable stolen pages

[ankitprasad.r.sharma]

From: Chris Wilson <chris@chris-wilson.co.uk>

If we run out of stolen memory when trying to allocate an object, see if
we can reap enough purgeable objects to free up enough contiguous free
space for the allocation. This is in principle very much like evicting
objects to free up enough contiguous space in the vma when binding
a new object - and you will be forgiven for thinking that the code looks
very similar.

At the moment, we do not allow userspace to allocate objects in stolen,
so there is neither the memory pressure to trigger stolen eviction nor
any purgeable objects inside the stolen arena. However, this will change
in the near future, and so better management and defragmentation of
stolen memory will become a real issue.

v2: Remember to remove the drm_mm_node.

testcase: igt/gem_create_stolen

Signed-off-by: Chris Wilson <chri@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_stolen.c | 121 ++++++++++++++++++++++++++++++---
 1 file changed, 110 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index f8da716..0a38d71 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -430,18 +430,29 @@ cleanup:
 	return NULL;
 }
 
-struct drm_i915_gem_object *
-i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
+static bool mark_free(struct drm_i915_gem_object *obj, struct list_head *unwind)
+{
+	if (obj->stolen == NULL)
+		return false;
+
+	if (obj->madv != I915_MADV_DONTNEED)
+		return false;
+
+	if (i915_gem_obj_is_pinned(obj))
+		return false;
+
+	list_add(&obj->obj_exec_link, unwind);
+	return drm_mm_scan_add_block(obj->stolen);
+}
+
+static struct drm_mm_node *
+stolen_alloc(struct drm_i915_private *dev_priv, u32 size)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_i915_gem_object *obj;
 	struct drm_mm_node *stolen;
+	struct drm_i915_gem_object *obj;
+	struct list_head unwind, evict;
 	int ret;
 
-	if (!drm_mm_initialized(&dev_priv->mm.stolen))
-		return NULL;
-
-	DRM_DEBUG_KMS("creating stolen object: size=%x\n", size);
 	if (size == 0)
 		return NULL;
 
@@ -451,11 +462,99 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
 
 	ret = drm_mm_insert_node(&dev_priv->mm.stolen, stolen, size,
 				 4096, DRM_MM_SEARCH_DEFAULT);
-	if (ret) {
-		kfree(stolen);
-		return NULL;
+	if (ret == 0)
+		return stolen;
+
+	/* No more stolen memory available, or too fragmented.
+	 * Try evicting purgeable objects and search again.
+	 */
+
+	drm_mm_init_scan(&dev_priv->mm.stolen, size, 4096, 0);
+	INIT_LIST_HEAD(&unwind);
+
+	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list)
+		if (mark_free(obj, &unwind))
+			goto found;
+
+	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
+		if (mark_free(obj, &unwind))
+			goto found;
+
+found:
+	INIT_LIST_HEAD(&evict);
+	while (!list_empty(&unwind)) {
+		obj = list_first_entry(&unwind,
+				       struct drm_i915_gem_object,
+				       obj_exec_link);
+		list_del_init(&obj->obj_exec_link);
+
+		if (drm_mm_scan_remove_block(obj->stolen)) {
+			list_add(&obj->obj_exec_link, &evict);
+			drm_gem_object_reference(&obj->base);
+		}
 	}
 
+	ret = 0;
+	while (!list_empty(&evict)) {
+		obj = list_first_entry(&evict,
+				       struct drm_i915_gem_object,
+				       obj_exec_link);
+		list_del_init(&obj->obj_exec_link);
+
+		if (ret == 0) {
+			struct i915_vma *vma, *vma_next;
+
+			list_for_each_entry_safe(vma, vma_next,
+						 &obj->vma_list,
+						 vma_link)
+				if (i915_vma_unbind(vma))
+					break;
+
+			/* Stolen pins its pages to prevent the
+			 * normal shrinker from processing stolen
+			 * objects.
+			 */
+			i915_gem_object_unpin_pages(obj);
+
+			ret = i915_gem_object_put_pages(obj);
+			if (ret == 0) {
+				i915_gem_object_release_stolen(obj);
+				obj->madv = __I915_MADV_PURGED;
+			} else
+				i915_gem_object_pin_pages(obj);
+		}
+
+		drm_gem_object_unreference(&obj->base);
+	}
+
+	if (ret == 0)
+		ret = drm_mm_insert_node(&dev_priv->mm.stolen, stolen, size,
+					 4096, DRM_MM_SEARCH_DEFAULT);
+	if (ret == 0)
+		return stolen;
+
+	kfree(stolen);
+	return NULL;
+}
+
+struct drm_i915_gem_object *
+i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj;
+	struct drm_mm_node *stolen;
+
+	lockdep_assert_held(&dev->struct_mutex);
+
+	if (!drm_mm_initialized(&dev_priv->mm.stolen))
+		return NULL;
+
+	DRM_DEBUG_KMS("creating stolen object: size=%x\n", size);
+
+	stolen = stolen_alloc(dev_priv, size);
+	if (stolen == NULL)
+		return NULL;
+
 	obj = _i915_gem_object_create_stolen(dev, stolen);
 	if (obj)
 		return obj;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/5] drm/i915: Support for pread/pwrite from/to non shmem backed objects

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>

This patch adds support for extending the pread/pwrite functionality
for objects not backed by shmem. The access will be made through
gtt interface.
This will cover prime objects as well as stolen memory backed objects
but for userptr objects it is still forbidden.

testcase: igt/gem_create_stolen

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |   7 +++
 drivers/gpu/drm/i915/i915_gem.c | 125 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 123 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 21a2b1f..a568cd1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3270,4 +3270,11 @@ static inline void i915_trace_irq_get(struct intel_engine_cs *ring,
 		i915_gem_request_assign(&ring->trace_irq_req, req);
 }
 
+/* Checking if pread/pwrite is allowed for the object */
+inline static bool i915_gem_obj_is_prw_allowed(struct drm_i915_gem_object *obj)
+{
+	/* pread/pwrite is forbidden for userptrs */
+	return !obj->userptr.mm;
+}
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 81c5381..3491bd3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -631,6 +631,96 @@ shmem_pread_slow(struct page *page, int shmem_page_offset, int page_length,
 	return ret ? - EFAULT : 0;
 }
 
+static inline int
+slow_user_access(struct io_mapping *mapping,
+		 loff_t page_base, int page_offset,
+		 char __user *user_data,
+		 int length, bool write)
+{
+	void __iomem *vaddr_inatomic;
+	void *vaddr;
+	unsigned long unwritten;
+
+	vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
+	/* We can use the cpu mem copy function because this is X86. */
+	vaddr = (void __force *)vaddr_inatomic + page_offset;
+	if (write)
+		unwritten = __copy_from_user(vaddr, user_data, length);
+	else
+		unwritten = __copy_to_user(user_data, vaddr, length);
+
+	io_mapping_unmap(vaddr_inatomic);
+	return unwritten;
+}
+
+static int
+i915_gem_gtt_pread_pwrite(struct drm_device *dev,
+			  struct drm_i915_gem_object *obj, uint64_t size,
+			  uint64_t data_offset, uint64_t data_ptr, bool write)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	char __user *user_data;
+	ssize_t remain;
+	loff_t offset, page_base;
+	int page_offset, page_length, ret = 0;
+
+	ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
+	if (ret)
+		goto out;
+
+	ret = i915_gem_object_set_to_gtt_domain(obj, write);
+	if (ret)
+		goto out_unpin;
+
+	ret = i915_gem_object_put_fence(obj);
+	if (ret)
+		goto out_unpin;
+
+	user_data = to_user_ptr(data_ptr);
+	remain = size;
+
+	offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
+
+	if (write)
+		intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT);
+
+	while (remain > 0) {
+		/* Operation in this page
+		 *
+		 * page_base = page offset within aperture
+		 * page_offset = offset within page
+		 * page_length = bytes to copy for this page
+		 */
+		page_base = offset & PAGE_MASK;
+		page_offset = offset_in_page(offset);
+		page_length = remain;
+		if ((page_offset + remain) > PAGE_SIZE)
+			page_length = PAGE_SIZE - page_offset;
+
+		/* This is a slow read/write as it tries to read from
+		 * and write to user memory which may result into page
+		 * faults
+		 */
+		ret = slow_user_access(dev_priv->gtt.mappable, page_base,
+				       page_offset, user_data,
+				       page_length, write);
+
+		if (ret) {
+			ret = -EINVAL;
+			break;
+		}
+
+		remain -= page_length;
+		user_data += page_length;
+		offset += page_length;
+	}
+
+out_unpin:
+	i915_gem_object_ggtt_unpin(obj);
+out:
+	return ret;
+}
+
 static int
 i915_gem_shmem_pread(struct drm_device *dev,
 		     struct drm_i915_gem_object *obj,
@@ -754,17 +844,24 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
 		goto out;
 	}
 
-	/* prime objects have no backing filp to GEM pread/pwrite
-	 * pages from.
-	 */
-	if (!obj->base.filp) {
+	if (!i915_gem_obj_is_prw_allowed(obj)) {
 		ret = -EINVAL;
 		goto out;
 	}
 
 	trace_i915_gem_object_pread(obj, args->offset, args->size);
 
-	ret = i915_gem_shmem_pread(dev, obj, args, file);
+	if (!obj->base.filp) {
+		/* pread for non shmem backed objects */
+		if (obj->tiling_mode == I915_TILING_NONE)
+			ret = i915_gem_gtt_pread_pwrite(dev, obj, args->size,
+							args->offset,
+							args->data_ptr,
+							false);
+		else
+			ret = -EINVAL;
+	} else
+		ret = i915_gem_shmem_pread(dev, obj, args, file);
 
 out:
 	drm_gem_object_unreference(&obj->base);
@@ -1107,10 +1204,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 		goto out;
 	}
 
-	/* prime objects have no backing filp to GEM pread/pwrite
-	 * pages from.
-	 */
-	if (!obj->base.filp) {
+	if (!i915_gem_obj_is_prw_allowed(obj)) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1118,6 +1212,19 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	trace_i915_gem_object_pwrite(obj, args->offset, args->size);
 
 	ret = -EFAULT;
+
+	/* pwrite for non shmem backed objects */
+	if (!obj->base.filp) {
+		if (obj->tiling_mode == I915_TILING_NONE)
+			ret = i915_gem_gtt_pread_pwrite(dev, obj, args->size,
+							args->offset,
+							args->data_ptr,
+							true);
+		else
+			ret = -EINVAL;
+
+		goto out;
+	}
 	/* We can only do the GTT pwrite on untiled buffers, as otherwise
 	 * it would end up going through the fenced access, and we'll get
 	 * different detiling behavior between reading and writing.
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/5] drm/i915: Add support for getting size of the stolen region

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>

This patch extends the get_aperture_ioctl to add support
for getting total size of the stolen region and available
size of the stolen region.

testcase: igt/gem_create_stolen

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
 drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++++-
 include/uapi/drm/i915_drm.h     |  6 ++++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a568cd1..a40b44f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3277,4 +3277,10 @@ inline static bool i915_gem_obj_is_prw_allowed(struct drm_i915_gem_object *obj)
 	return !obj->userptr.mm;
 }
 
+inline static bool i915_gem_obj_is_stolen_used(struct drm_i915_gem_object *obj)
+{
+	return obj->stolen && (i915_gem_obj_is_pinned(obj)
+			       || obj->madv == I915_MADV_WILLNEED);
+}
+
 #endif
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3491bd3..ee93508 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -150,17 +150,30 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_get_aperture *args = data;
 	struct drm_i915_gem_object *obj;
-	size_t pinned;
+	size_t pinned, pinned_stolen;
 
 	pinned = 0;
+	pinned_stolen = 0;
 	mutex_lock(&dev->struct_mutex);
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
 		if (i915_gem_obj_is_pinned(obj))
 			pinned += i915_gem_obj_ggtt_size(obj);
+
+	/* Calculating available stolen size */
+	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
+		if (i915_gem_obj_is_stolen_used(obj))
+			pinned_stolen += i915_gem_obj_ggtt_size(obj);
+
+	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list)
+		if (i915_gem_obj_is_stolen_used(obj))
+			pinned_stolen += i915_gem_obj_ggtt_size(obj);
+
 	mutex_unlock(&dev->struct_mutex);
 
 	args->aper_size = dev_priv->gtt.base.total;
 	args->aper_available_size = args->aper_size - pinned;
+	args->stolen_size = dev_priv->gtt.stolen_size;
+	args->stolen_available_size = args->stolen_size - pinned_stolen;
 
 	return 0;
 }
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index ab4f3a9..49766b8 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -932,6 +932,12 @@ struct drm_i915_gem_get_aperture {
 	 * bytes
 	 */
 	__u64 aper_available_size;
+
+	/** Total size of the stolen region, in bytes */
+	__u64 stolen_size;
+
+	/* Available space in the stolen region, in bytes */
+	__u64 stolen_available_size;
 };
 
 struct drm_i915_get_pipe_from_crtc_id {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm/i915: Support for pread/pwrite from/to non shmem backed objects

[Chris Wilson]

On Wed, Apr 29, 2015 at 03:01:58PM +0530, ankitprasad.r.sharma@intel.com wrote:
> From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> 
> This patch adds support for extending the pread/pwrite functionality
> for objects not backed by shmem. The access will be made through
> gtt interface.
> This will cover prime objects as well as stolen memory backed objects
> but for userptr objects it is still forbidden.
> 
> testcase: igt/gem_create_stolen
> 
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |   7 +++
>  drivers/gpu/drm/i915/i915_gem.c | 125 +++++++++++++++++++++++++++++++++++++---
>  2 files changed, 123 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 21a2b1f..a568cd1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3270,4 +3270,11 @@ static inline void i915_trace_irq_get(struct intel_engine_cs *ring,
>  		i915_gem_request_assign(&ring->trace_irq_req, req);
>  }
>  
> +/* Checking if pread/pwrite is allowed for the object */
> +inline static bool i915_gem_obj_is_prw_allowed(struct drm_i915_gem_object *obj)
> +{
> +	/* pread/pwrite is forbidden for userptrs */
> +	return !obj->userptr.mm;
> +}

Not quite. Just wasn't implemented (as there wasn't much point since we
couldn't provide other CPU access) but there is no reason to arbitrary
limit the API - especially as you are in the process of removing that
arbitrary limitation!

> +
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 81c5381..3491bd3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -631,6 +631,96 @@ shmem_pread_slow(struct page *page, int shmem_page_offset, int page_length,
>  	return ret ? - EFAULT : 0;
>  }
>  
> +static inline int
> +slow_user_access(struct io_mapping *mapping,
> +		 loff_t page_base, int page_offset,
> +		 char __user *user_data,
> +		 int length, bool write)
> +{
> +	void __iomem *vaddr_inatomic;
> +	void *vaddr;
> +	unsigned long unwritten;
> +
> +	vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
> +	/* We can use the cpu mem copy function because this is X86. */
> +	vaddr = (void __force *)vaddr_inatomic + page_offset;
> +	if (write)
> +		unwritten = __copy_from_user(vaddr, user_data, length);
> +	else
> +		unwritten = __copy_to_user(user_data, vaddr, length);
> +
> +	io_mapping_unmap(vaddr_inatomic);
> +	return unwritten;
> +}
> +
> +static int
> +i915_gem_gtt_pread_pwrite(struct drm_device *dev,
> +			  struct drm_i915_gem_object *obj, uint64_t size,
> +			  uint64_t data_offset, uint64_t data_ptr, bool write)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	char __user *user_data;
> +	ssize_t remain;
> +	loff_t offset, page_base;
> +	int page_offset, page_length, ret = 0;
> +
> +	ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> +	if (ret)
> +		goto out;
> +
> +	ret = i915_gem_object_set_to_gtt_domain(obj, write);
> +	if (ret)
> +		goto out_unpin;
> +
> +	ret = i915_gem_object_put_fence(obj);
> +	if (ret)
> +		goto out_unpin;
> +
> +	user_data = to_user_ptr(data_ptr);
> +	remain = size;
> +
> +	offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
> +
> +	if (write)
> +		intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT);
> +
> +	while (remain > 0) {
> +		/* Operation in this page
> +		 *
> +		 * page_base = page offset within aperture
> +		 * page_offset = offset within page
> +		 * page_length = bytes to copy for this page
> +		 */
> +		page_base = offset & PAGE_MASK;
> +		page_offset = offset_in_page(offset);
> +		page_length = remain;
> +		if ((page_offset + remain) > PAGE_SIZE)
> +			page_length = PAGE_SIZE - page_offset;
> +
> +		/* This is a slow read/write as it tries to read from
> +		 * and write to user memory which may result into page
> +		 * faults
> +		 */

Hmm, I am in the process of rewriting this function (GTT pwrite) to reduce
aperture pressure (which will also be important here). However for your
use case, we do need to drop the locks around slow_user_access() so that
we can use non-atomic copy functions and *pagefault*. We do need the full
level of complexity like i915_gem_shmem_pread()
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Add support for getting size of the stolen region

[Chris Wilson]

On Wed, Apr 29, 2015 at 03:01:59PM +0530, ankitprasad.r.sharma@intel.com wrote:
> From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> 
> This patch extends the get_aperture_ioctl to add support
> for getting total size of the stolen region and available
> size of the stolen region.
> 
> testcase: igt/gem_create_stolen
> 
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
>  drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++++-
>  include/uapi/drm/i915_drm.h     |  6 ++++++
>  3 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a568cd1..a40b44f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3277,4 +3277,10 @@ inline static bool i915_gem_obj_is_prw_allowed(struct drm_i915_gem_object *obj)
>  	return !obj->userptr.mm;
>  }
>  
> +inline static bool i915_gem_obj_is_stolen_used(struct drm_i915_gem_object *obj)
> +{
> +	return obj->stolen && (i915_gem_obj_is_pinned(obj)
> +			       || obj->madv == I915_MADV_WILLNEED);
> +}
> +
>  #endif
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3491bd3..ee93508 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -150,17 +150,30 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_gem_get_aperture *args = data;
>  	struct drm_i915_gem_object *obj;
> -	size_t pinned;
> +	size_t pinned, pinned_stolen;
>  
>  	pinned = 0;
> +	pinned_stolen = 0;
>  	mutex_lock(&dev->struct_mutex);
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
>  		if (i915_gem_obj_is_pinned(obj))
>  			pinned += i915_gem_obj_ggtt_size(obj);
> +
> +	/* Calculating available stolen size */
> +	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
> +		if (i915_gem_obj_is_stolen_used(obj))
> +			pinned_stolen += i915_gem_obj_ggtt_size(obj);
> +
> +	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list)
> +		if (i915_gem_obj_is_stolen_used(obj))
> +			pinned_stolen += i915_gem_obj_ggtt_size(obj);

Ah, here you will want to iterate over the stolen drm_mm range manager
to get an accurate count.

Could you respin this on top of
id:1422276205-8532-5-git-send-email-rodrigo.vivi@intel.com ?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/5] drm/i915: Support for pread/pwrite from/to non shmem backed objects

[Chris Wilson]

On Wed, Apr 29, 2015 at 11:19:18AM +0100, Chris Wilson wrote:
> On Wed, Apr 29, 2015 at 03:01:58PM +0530, ankitprasad.r.sharma@intel.com wrote:
> > +	while (remain > 0) {
> > +		/* Operation in this page
> > +		 *
> > +		 * page_base = page offset within aperture
> > +		 * page_offset = offset within page
> > +		 * page_length = bytes to copy for this page
> > +		 */
> > +		page_base = offset & PAGE_MASK;
> > +		page_offset = offset_in_page(offset);
> > +		page_length = remain;
> > +		if ((page_offset + remain) > PAGE_SIZE)
> > +			page_length = PAGE_SIZE - page_offset;
> > +
> > +		/* This is a slow read/write as it tries to read from
> > +		 * and write to user memory which may result into page
> > +		 * faults
> > +		 */
> 
> Hmm, I am in the process of rewriting this function (GTT pwrite) to reduce
> aperture pressure (which will also be important here). However for your
> use case, we do need to drop the locks around slow_user_access() so that
> we can use non-atomic copy functions and *pagefault*. We do need the full
> level of complexity like i915_gem_shmem_pread()

It would be a good exercise to try and reproduce the fautling failure condition
with igt/gem_stolen.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Add support for getting size of the stolen region

[Ankitprasad Sharma]

On Wed, 2015-04-29 at 11:27 +0100, Chris Wilson wrote:
> On Wed, Apr 29, 2015 at 03:01:59PM +0530, ankitprasad.r.sharma@intel.com wrote:
> > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > 
> > This patch extends the get_aperture_ioctl to add support
> > for getting total size of the stolen region and available
> > size of the stolen region.
> > 
> > testcase: igt/gem_create_stolen
> > 
> > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
> >  drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++++-
> >  include/uapi/drm/i915_drm.h     |  6 ++++++
> >  3 files changed, 26 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index a568cd1..a40b44f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3277,4 +3277,10 @@ inline static bool i915_gem_obj_is_prw_allowed(struct drm_i915_gem_object *obj)
> >  	return !obj->userptr.mm;
> >  }
> >  
> > +inline static bool i915_gem_obj_is_stolen_used(struct drm_i915_gem_object *obj)
> > +{
> > +	return obj->stolen && (i915_gem_obj_is_pinned(obj)
> > +			       || obj->madv == I915_MADV_WILLNEED);
> > +}
> > +
> >  #endif
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 3491bd3..ee93508 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -150,17 +150,30 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	struct drm_i915_gem_get_aperture *args = data;
> >  	struct drm_i915_gem_object *obj;
> > -	size_t pinned;
> > +	size_t pinned, pinned_stolen;
> >  
> >  	pinned = 0;
> > +	pinned_stolen = 0;
> >  	mutex_lock(&dev->struct_mutex);
> >  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
> >  		if (i915_gem_obj_is_pinned(obj))
> >  			pinned += i915_gem_obj_ggtt_size(obj);
> > +
> > +	/* Calculating available stolen size */
> > +	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
> > +		if (i915_gem_obj_is_stolen_used(obj))
> > +			pinned_stolen += i915_gem_obj_ggtt_size(obj);
> > +
> > +	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list)
> > +		if (i915_gem_obj_is_stolen_used(obj))
> > +			pinned_stolen += i915_gem_obj_ggtt_size(obj);
> 
> Ah, here you will want to iterate over the stolen drm_mm range manager
> to get an accurate count.
> 
> Could you respin this on top of
> id:1422276205-8532-5-git-send-email-rodrigo.vivi@intel.com ?

I have incorporated the comments suggested by you, as well as I went through Rodrigo's patch and as such there is no dependency between the two patches. 

So how do I float this patch? Shall I resend this as a separate patch in the series (1422276205-8532-5-git-send-email-rodrigo.vivi@intel.com) rebasing on top of Rodrigo's changes Or as a part of this series only?

-Ankit

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/5] drm/i915: Add support for getting size of the stolen region

[Chris Wilson]

On Tue, May 05, 2015 at 05:12:25PM +0530, Ankitprasad Sharma wrote:
> On Wed, 2015-04-29 at 11:27 +0100, Chris Wilson wrote:
> > On Wed, Apr 29, 2015 at 03:01:59PM +0530, ankitprasad.r.sharma@intel.com wrote:
> > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > 
> > > This patch extends the get_aperture_ioctl to add support
> > > for getting total size of the stolen region and available
> > > size of the stolen region.
> > > 
> > > testcase: igt/gem_create_stolen
> > > 
> > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
> > >  drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++++-
> > >  include/uapi/drm/i915_drm.h     |  6 ++++++
> > >  3 files changed, 26 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index a568cd1..a40b44f 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -3277,4 +3277,10 @@ inline static bool i915_gem_obj_is_prw_allowed(struct drm_i915_gem_object *obj)
> > >  	return !obj->userptr.mm;
> > >  }
> > >  
> > > +inline static bool i915_gem_obj_is_stolen_used(struct drm_i915_gem_object *obj)
> > > +{
> > > +	return obj->stolen && (i915_gem_obj_is_pinned(obj)
> > > +			       || obj->madv == I915_MADV_WILLNEED);
> > > +}
> > > +
> > >  #endif
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 3491bd3..ee93508 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -150,17 +150,30 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > >  	struct drm_i915_gem_get_aperture *args = data;
> > >  	struct drm_i915_gem_object *obj;
> > > -	size_t pinned;
> > > +	size_t pinned, pinned_stolen;
> > >  
> > >  	pinned = 0;
> > > +	pinned_stolen = 0;
> > >  	mutex_lock(&dev->struct_mutex);
> > >  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
> > >  		if (i915_gem_obj_is_pinned(obj))
> > >  			pinned += i915_gem_obj_ggtt_size(obj);
> > > +
> > > +	/* Calculating available stolen size */
> > > +	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
> > > +		if (i915_gem_obj_is_stolen_used(obj))
> > > +			pinned_stolen += i915_gem_obj_ggtt_size(obj);
> > > +
> > > +	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list)
> > > +		if (i915_gem_obj_is_stolen_used(obj))
> > > +			pinned_stolen += i915_gem_obj_ggtt_size(obj);
> > 
> > Ah, here you will want to iterate over the stolen drm_mm range manager
> > to get an accurate count.
> > 
> > Could you respin this on top of
> > id:1422276205-8532-5-git-send-email-rodrigo.vivi@intel.com ?
> 
> I have incorporated the comments suggested by you, as well as I went through Rodrigo's patch and as such there is no dependency between the two patches. 
> 
> So how do I float this patch? Shall I resend this as a separate patch in the series (1422276205-8532-5-git-send-email-rodrigo.vivi@intel.com) rebasing on top of Rodrigo's changes Or as a part of this series only?

I see this patch as primarily improving the get-aperture ioctl, in which
case I would like to address the known deficiency in it as well. It can
be more than one patche, though the ABI will have an implicit dependency
on them all.

I honestly don't mind how you present it, I just want the mappable info
through an ioctl as well...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/5] drm/i915: Support for creating Stolen memory backed objects

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@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@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@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 2193cc2..8319e07 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 8db905a..e6ef083 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3159,7 +3159,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 94533bf..6568a7f 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);
+		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 a36cb95..99f2bce 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -457,7 +457,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;
@@ -467,7 +467,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 dbd16a2..f71f75c 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)
 };
 
 struct drm_i915_gem_pread {
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx