public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-05-13 12:07 [PATCH 0/2] Extending GET_APERTURE ioctl ankitprasad.r.sharma
@ 2015-05-13 12:07 ` ankitprasad.r.sharma
  0 siblings, 0 replies; 16+ messages in thread
From: ankitprasad.r.sharma @ 2015-05-13 12:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: akash.goel, shashidhar.hiremath, Rodrigo Vivi

From: Rodrigo Vivi <rodrigo.vivi@intel.com>

When constructing a batchbuffer, it is sometimes crucial to know the
largest hole into which we can fit a fenceable buffer (for example when
handling very large objects on gen2 and gen3). This depends on the
fragmentation of pinned buffers inside the aperture, a question only the
kernel can easily answer.

This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
include a couple of new fields in its reply to userspace - the total
amount of space available in the mappable region of the aperture and
also the single largest block available.

This is not quite what userspace wants to answer the question of whether
this batch will fit as fences are also required to meet severe alignment
constraints within the batch. For this purpose, a third conservative
estimate of largest fence available is also provided. For when userspace
needs more than one batch, we also provide the culmulative space
available for fences such that it has some additional guidance to how
much space it could allocate to fences. Conservatism still wins.

The patch also adds a debugfs file for convenient testing and reporting.

v2: The first object cannot end at offset 0, so we can use last==0 to
detect the empty list.

v3: Expand all values to 64bit, just in case.
    Report total mappable aperture size for userspace that cannot easily
    determine it by inspecting the PCI device.

v4: (Rodrigo) Fixed rebase conflicts.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  27 +++++++++
 drivers/gpu/drm/i915/i915_gem.c     | 116 ++++++++++++++++++++++++++++++++++--
 include/uapi/drm/i915_drm.h         |  25 ++++++++
 3 files changed, 164 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 007c7d7..70c6df2 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -499,6 +499,32 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 	return 0;
 }
 
+static int i915_gem_aperture_info(struct seq_file *m, void *data)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_i915_gem_get_aperture arg;
+	int ret;
+
+	ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
+	if (ret)
+		return ret;
+
+	seq_printf(m, "Total size of the GTT: %llu bytes\n",
+		   arg.aper_size);
+	seq_printf(m, "Available space in the GTT: %llu bytes\n",
+		   arg.aper_available_size);
+	seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
+		   arg.map_available_size);
+	seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
+		   arg.map_largest_size);
+	seq_printf(m, "Available space for fences: %llu bytes\n",
+		   arg.fence_available_size);
+	seq_printf(m, "Single largest fence available: %llu bytes\n",
+		   arg.fence_largest_size);
+
+	return 0;
+}
+
 static int i915_gem_gtt_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
@@ -4646,6 +4672,7 @@ static int i915_debugfs_create(struct dentry *root,
 static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_capabilities", i915_capabilities, 0},
 	{"i915_gem_objects", i915_gem_object_info, 0},
+	{"i915_gem_aperture", i915_gem_aperture_info, 0},
 	{"i915_gem_gtt", i915_gem_gtt_info, 0},
 	{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
 	{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fe14ddc..fd964b0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -32,6 +32,7 @@
 #include "i915_vgpu.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include <linux/list_sort.h>
 #include <linux/shmem_fs.h>
 #include <linux/slab.h>
 #include <linux/swap.h>
@@ -143,6 +144,55 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 	return 0;
 }
 
+static inline bool
+i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
+{
+	return i915_gem_obj_bound_any(obj) && !obj->active;
+}
+
+static int obj_rank_by_ggtt(void *priv,
+			    struct list_head *A,
+			    struct list_head *B)
+{
+	struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
+	struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
+
+	return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
+}
+
+static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32 end)
+{
+	u32 size = end - start;
+	u32 fence_size;
+
+	if (INTEL_INFO(dev_priv)->gen < 4) {
+		u32 fence_max;
+		u32 fence_next;
+
+		if (IS_GEN3(dev_priv)) {
+			fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
+			fence_next = 1024*1024;
+		} else {
+			fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
+			fence_next = 512*1024;
+		}
+
+		fence_max = min(fence_max, size);
+		fence_size = 0;
+		while (fence_next <= fence_max) {
+			u32 base = ALIGN(start, fence_next);
+			if (base + fence_next > end)
+				break;
+
+			fence_size = fence_next;
+			fence_next <<= 1;
+		}
+	} else
+		fence_size = size;
+
+	return fence_size;
+}
+
 int
 i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 			    struct drm_file *file)
@@ -150,17 +200,75 @@ 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;
+	struct list_head map_list;
+	const u32 map_limit = dev_priv->gtt.mappable_end;
+	size_t pinned, map_space, map_largest, fence_space, fence_largest;
+	u32 last, size;
+
+	INIT_LIST_HEAD(&map_list);
 
 	pinned = 0;
+	map_space = map_largest = 0;
+	fence_space = fence_largest = 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);
+	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
+		struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
+
+		if (vma == NULL || !vma->pin_count)
+			continue;
+
+		pinned += vma->node.size;
+
+		if (vma->node.start < map_limit)
+			list_add(&obj->obj_exec_link, &map_list);
+	}
+
+	last = 0;
+	list_sort(NULL, &map_list, obj_rank_by_ggtt);
+	while (!list_empty(&map_list)) {
+		struct i915_vma *vma;
+
+		obj = list_first_entry(&map_list, typeof(*obj), obj_exec_link);
+		list_del_init(&obj->obj_exec_link);
+
+		vma = i915_gem_obj_to_ggtt(obj);
+		if (last == 0)
+			goto skip_first;
+
+		size = vma->node.start - last;
+		if (size > map_largest)
+			map_largest = size;
+		map_space += size;
+
+		size = __fence_size(dev_priv, last, vma->node.start);
+		if (size > fence_largest)
+			fence_largest = size;
+		fence_space += size;
+
+skip_first:
+		last = vma->node.start + vma->node.size;
+	}
+	if (last < map_limit) {
+		size = map_limit - last;
+		if (size > map_largest)
+			map_largest = size;
+		map_space += size;
+
+		size = __fence_size(dev_priv, last, map_limit);
+		if (size > fence_largest)
+			fence_largest = size;
+		fence_space += size;
+	}
 	mutex_unlock(&dev->struct_mutex);
 
 	args->aper_size = dev_priv->gtt.base.total;
 	args->aper_available_size = args->aper_size - pinned;
+	args->map_available_size = map_space;
+	args->map_largest_size = map_largest;
+	args->map_total_size = dev_priv->gtt.mappable_end;
+	args->fence_available_size = fence_space;
+	args->fence_largest_size = fence_largest;
 
 	return 0;
 }
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index ab4f3a9..ce22415 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -932,6 +932,31 @@ struct drm_i915_gem_get_aperture {
 	 * bytes
 	 */
 	__u64 aper_available_size;
+
+	/**
+	 * Total space in the mappable region of the aperture, in bytes
+	 */
+	__u64 map_total_size;
+
+	/**
+	 * Available space in the mappable region of the aperture, in bytes
+	 */
+	__u64 map_available_size;
+
+	/**
+	 * Single largest available region inside the mappable region, in bytes.
+	 */
+	__u64 map_largest_size;
+
+	/**
+	 * Culmulative space available for fences, in bytes
+	 */
+	__u64 fence_available_size;
+
+	/**
+	 * Single largest fenceable region, in bytes.
+	 */
+	__u64 fence_largest_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

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

* [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-01  9:25 [PATCH v2 0/2] Extending GET_APERTURE ioctl ankitprasad.r.sharma
@ 2015-07-01  9:25 ` ankitprasad.r.sharma
  2015-07-01 13:39   ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: ankitprasad.r.sharma @ 2015-07-01  9:25 UTC (permalink / raw)
  To: intel-gfx; +Cc: akash.goel, Rodrigo Vivi, shashidhar.hiremath

From: Rodrigo Vivi <rodrigo.vivi@intel.com>

When constructing a batchbuffer, it is sometimes crucial to know the
largest hole into which we can fit a fenceable buffer (for example when
handling very large objects on gen2 and gen3). This depends on the
fragmentation of pinned buffers inside the aperture, a question only the
kernel can easily answer.

This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
include a couple of new fields in its reply to userspace - the total
amount of space available in the mappable region of the aperture and
also the single largest block available.

This is not quite what userspace wants to answer the question of whether
this batch will fit as fences are also required to meet severe alignment
constraints within the batch. For this purpose, a third conservative
estimate of largest fence available is also provided. For when userspace
needs more than one batch, we also provide the culmulative space
available for fences such that it has some additional guidance to how
much space it could allocate to fences. Conservatism still wins.

The patch also adds a debugfs file for convenient testing and reporting.

v2: The first object cannot end at offset 0, so we can use last==0 to
detect the empty list.

v3: Expand all values to 64bit, just in case.
    Report total mappable aperture size for userspace that cannot easily
    determine it by inspecting the PCI device.

v4: (Rodrigo) Fixed rebase conflicts.

v5: Rebased to the latest drm-intel-nightly (Ankit)

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  27 +++++++++
 drivers/gpu/drm/i915/i915_gem.c     | 116 ++++++++++++++++++++++++++++++++++--
 2 files changed, 139 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 31d8768..49ec438 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -512,6 +512,32 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 	return 0;
 }
 
+static int i915_gem_aperture_info(struct seq_file *m, void *data)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_i915_gem_get_aperture arg;
+	int ret;
+
+	ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
+	if (ret)
+		return ret;
+
+	seq_printf(m, "Total size of the GTT: %llu bytes\n",
+		   arg.aper_size);
+	seq_printf(m, "Available space in the GTT: %llu bytes\n",
+		   arg.aper_available_size);
+	seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
+		   arg.map_available_size);
+	seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
+		   arg.map_largest_size);
+	seq_printf(m, "Available space for fences: %llu bytes\n",
+		   arg.fence_available_size);
+	seq_printf(m, "Single largest fence available: %llu bytes\n",
+		   arg.fence_largest_size);
+
+	return 0;
+}
+
 static int i915_gem_gtt_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
@@ -5030,6 +5056,7 @@ static int i915_debugfs_create(struct dentry *root,
 static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_capabilities", i915_capabilities, 0},
 	{"i915_gem_objects", i915_gem_object_info, 0},
+	{"i915_gem_aperture", i915_gem_aperture_info, 0},
 	{"i915_gem_gtt", i915_gem_gtt_info, 0},
 	{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
 	{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a2a4a27..ccfc8d3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -32,6 +32,7 @@
 #include "i915_vgpu.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include <linux/list_sort.h>
 #include <linux/shmem_fs.h>
 #include <linux/slab.h>
 #include <linux/swap.h>
@@ -143,6 +144,55 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 	return 0;
 }
 
+static inline bool
+i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
+{
+	return i915_gem_obj_bound_any(obj) && !obj->active;
+}
+
+static int obj_rank_by_ggtt(void *priv,
+			    struct list_head *A,
+			    struct list_head *B)
+{
+	struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
+	struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
+
+	return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
+}
+
+static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32 end)
+{
+	u32 size = end - start;
+	u32 fence_size;
+
+	if (INTEL_INFO(dev_priv)->gen < 4) {
+		u32 fence_max;
+		u32 fence_next;
+
+		if (IS_GEN3(dev_priv)) {
+			fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
+			fence_next = 1024*1024;
+		} else {
+			fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
+			fence_next = 512*1024;
+		}
+
+		fence_max = min(fence_max, size);
+		fence_size = 0;
+		while (fence_next <= fence_max) {
+			u32 base = ALIGN(start, fence_next);
+			if (base + fence_next > end)
+				break;
+
+			fence_size = fence_next;
+			fence_next <<= 1;
+		}
+	} else
+		fence_size = size;
+
+	return fence_size;
+}
+
 int
 i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 			    struct drm_file *file)
@@ -150,17 +200,75 @@ 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;
+	struct list_head map_list;
+	const u32 map_limit = dev_priv->gtt.mappable_end;
+	size_t pinned, map_space, map_largest, fence_space, fence_largest;
+	u32 last, size;
+
+	INIT_LIST_HEAD(&map_list);
 
 	pinned = 0;
+	map_space = map_largest = 0;
+	fence_space = fence_largest = 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);
+	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
+		struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
+
+		if (vma == NULL || !vma->pin_count)
+			continue;
+
+		pinned += vma->node.size;
+
+		if (vma->node.start < map_limit)
+			list_add(&obj->obj_exec_link, &map_list);
+	}
+
+	last = 0;
+	list_sort(NULL, &map_list, obj_rank_by_ggtt);
+	while (!list_empty(&map_list)) {
+		struct i915_vma *vma;
+
+		obj = list_first_entry(&map_list, typeof(*obj), obj_exec_link);
+		list_del_init(&obj->obj_exec_link);
+
+		vma = i915_gem_obj_to_ggtt(obj);
+		if (last == 0)
+			goto skip_first;
+
+		size = vma->node.start - last;
+		if (size > map_largest)
+			map_largest = size;
+		map_space += size;
+
+		size = __fence_size(dev_priv, last, vma->node.start);
+		if (size > fence_largest)
+			fence_largest = size;
+		fence_space += size;
+
+skip_first:
+		last = vma->node.start + vma->node.size;
+	}
+	if (last < map_limit) {
+		size = map_limit - last;
+		if (size > map_largest)
+			map_largest = size;
+		map_space += size;
+
+		size = __fence_size(dev_priv, last, map_limit);
+		if (size > fence_largest)
+			fence_largest = size;
+		fence_space += size;
+	}
 	mutex_unlock(&dev->struct_mutex);
 
 	args->aper_size = dev_priv->gtt.base.total;
 	args->aper_available_size = args->aper_size - pinned;
+	args->map_available_size = map_space;
+	args->map_largest_size = map_largest;
+	args->map_total_size = dev_priv->gtt.mappable_end;
+	args->fence_available_size = fence_space;
+	args->fence_largest_size = fence_largest;
 
 	return 0;
 }
-- 
1.9.1

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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-01  9:25 ` [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space ankitprasad.r.sharma
@ 2015-07-01 13:39   ` Daniel Vetter
  2015-07-01 16:34     ` Ankitprasad Sharma
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-07-01 13:39 UTC (permalink / raw)
  To: ankitprasad.r.sharma
  Cc: intel-gfx, shashidhar.hiremath, akash.goel, Rodrigo Vivi

On Wed, Jul 01, 2015 at 02:55:12PM +0530, ankitprasad.r.sharma@intel.com wrote:
> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> When constructing a batchbuffer, it is sometimes crucial to know the
> largest hole into which we can fit a fenceable buffer (for example when
> handling very large objects on gen2 and gen3). This depends on the
> fragmentation of pinned buffers inside the aperture, a question only the
> kernel can easily answer.
> 
> This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> include a couple of new fields in its reply to userspace - the total
> amount of space available in the mappable region of the aperture and
> also the single largest block available.
> 
> This is not quite what userspace wants to answer the question of whether
> this batch will fit as fences are also required to meet severe alignment
> constraints within the batch. For this purpose, a third conservative
> estimate of largest fence available is also provided. For when userspace
> needs more than one batch, we also provide the culmulative space
> available for fences such that it has some additional guidance to how
> much space it could allocate to fences. Conservatism still wins.
> 
> The patch also adds a debugfs file for convenient testing and reporting.
> 
> v2: The first object cannot end at offset 0, so we can use last==0 to
> detect the empty list.
> 
> v3: Expand all values to 64bit, just in case.
>     Report total mappable aperture size for userspace that cannot easily
>     determine it by inspecting the PCI device.
> 
> v4: (Rodrigo) Fixed rebase conflicts.
> 
> v5: Rebased to the latest drm-intel-nightly (Ankit)
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  27 +++++++++
>  drivers/gpu/drm/i915/i915_gem.c     | 116 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 139 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 31d8768..49ec438 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -512,6 +512,32 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>  	return 0;
>  }
>  
> +static int i915_gem_aperture_info(struct seq_file *m, void *data)
> +{
> +	struct drm_info_node *node = m->private;
> +	struct drm_i915_gem_get_aperture arg;
> +	int ret;
> +
> +	ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
> +	if (ret)
> +		return ret;
> +
> +	seq_printf(m, "Total size of the GTT: %llu bytes\n",
> +		   arg.aper_size);
> +	seq_printf(m, "Available space in the GTT: %llu bytes\n",
> +		   arg.aper_available_size);
> +	seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
> +		   arg.map_available_size);
> +	seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
> +		   arg.map_largest_size);
> +	seq_printf(m, "Available space for fences: %llu bytes\n",
> +		   arg.fence_available_size);
> +	seq_printf(m, "Single largest fence available: %llu bytes\n",
> +		   arg.fence_largest_size);
> +
> +	return 0;
> +}
> +
>  static int i915_gem_gtt_info(struct seq_file *m, void *data)
>  {
>  	struct drm_info_node *node = m->private;
> @@ -5030,6 +5056,7 @@ static int i915_debugfs_create(struct dentry *root,
>  static const struct drm_info_list i915_debugfs_list[] = {
>  	{"i915_capabilities", i915_capabilities, 0},
>  	{"i915_gem_objects", i915_gem_object_info, 0},
> +	{"i915_gem_aperture", i915_gem_aperture_info, 0},
>  	{"i915_gem_gtt", i915_gem_gtt_info, 0},
>  	{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
>  	{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a2a4a27..ccfc8d3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -32,6 +32,7 @@
>  #include "i915_vgpu.h"
>  #include "i915_trace.h"
>  #include "intel_drv.h"
> +#include <linux/list_sort.h>
>  #include <linux/shmem_fs.h>
>  #include <linux/slab.h>
>  #include <linux/swap.h>
> @@ -143,6 +144,55 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
>  	return 0;
>  }
>  
> +static inline bool
> +i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
> +{
> +	return i915_gem_obj_bound_any(obj) && !obj->active;
> +}
> +
> +static int obj_rank_by_ggtt(void *priv,
> +			    struct list_head *A,
> +			    struct list_head *B)
> +{
> +	struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
> +	struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
> +
> +	return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
> +}
> +
> +static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32 end)
> +{
> +	u32 size = end - start;
> +	u32 fence_size;
> +
> +	if (INTEL_INFO(dev_priv)->gen < 4) {
> +		u32 fence_max;
> +		u32 fence_next;
> +
> +		if (IS_GEN3(dev_priv)) {
> +			fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
> +			fence_next = 1024*1024;
> +		} else {
> +			fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
> +			fence_next = 512*1024;
> +		}
> +
> +		fence_max = min(fence_max, size);
> +		fence_size = 0;
> +		while (fence_next <= fence_max) {
> +			u32 base = ALIGN(start, fence_next);
> +			if (base + fence_next > end)
> +				break;
> +
> +			fence_size = fence_next;
> +			fence_next <<= 1;
> +		}
> +	} else
> +		fence_size = size;
> +
> +	return fence_size;
> +}
> +
>  int
>  i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
>  			    struct drm_file *file)
> @@ -150,17 +200,75 @@ 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;
> +	struct list_head map_list;
> +	const u32 map_limit = dev_priv->gtt.mappable_end;
> +	size_t pinned, map_space, map_largest, fence_space, fence_largest;
> +	u32 last, size;
> +
> +	INIT_LIST_HEAD(&map_list);
>  
>  	pinned = 0;
> +	map_space = map_largest = 0;
> +	fence_space = fence_largest = 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);
> +	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> +		struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
> +
> +		if (vma == NULL || !vma->pin_count)
> +			continue;
> +
> +		pinned += vma->node.size;
> +
> +		if (vma->node.start < map_limit)
> +			list_add(&obj->obj_exec_link, &map_list);
> +	}
> +
> +	last = 0;
> +	list_sort(NULL, &map_list, obj_rank_by_ggtt);
> +	while (!list_empty(&map_list)) {
> +		struct i915_vma *vma;
> +
> +		obj = list_first_entry(&map_list, typeof(*obj), obj_exec_link);
> +		list_del_init(&obj->obj_exec_link);
> +
> +		vma = i915_gem_obj_to_ggtt(obj);
> +		if (last == 0)
> +			goto skip_first;
> +
> +		size = vma->node.start - last;
> +		if (size > map_largest)
> +			map_largest = size;
> +		map_space += size;
> +
> +		size = __fence_size(dev_priv, last, vma->node.start);
> +		if (size > fence_largest)
> +			fence_largest = size;
> +		fence_space += size;
> +
> +skip_first:
> +		last = vma->node.start + vma->node.size;
> +	}
> +	if (last < map_limit) {
> +		size = map_limit - last;
> +		if (size > map_largest)
> +			map_largest = size;
> +		map_space += size;
> +
> +		size = __fence_size(dev_priv, last, map_limit);
> +		if (size > fence_largest)
> +			fence_largest = size;
> +		fence_space += size;
> +	}
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	args->aper_size = dev_priv->gtt.base.total;
>  	args->aper_available_size = args->aper_size - pinned;
> +	args->map_available_size = map_space;
> +	args->map_largest_size = map_largest;
> +	args->map_total_size = dev_priv->gtt.mappable_end;
> +	args->fence_available_size = fence_space;
> +	args->fence_largest_size = fence_largest;

Does this really compile? You only add all the new structure stuff in the
2nd patch.
-Daniel

>  
>  	return 0;
>  }
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-01 13:39   ` Daniel Vetter
@ 2015-07-01 16:34     ` Ankitprasad Sharma
  0 siblings, 0 replies; 16+ messages in thread
From: Ankitprasad Sharma @ 2015-07-01 16:34 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, shashidhar.hiremath, akash.goel, Rodrigo Vivi

On Wed, 2015-07-01 at 15:39 +0200, Daniel Vetter wrote:
> On Wed, Jul 01, 2015 at 02:55:12PM +0530, ankitprasad.r.sharma@intel.com wrote:
> > From: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > When constructing a batchbuffer, it is sometimes crucial to know the
> > largest hole into which we can fit a fenceable buffer (for example when
> > handling very large objects on gen2 and gen3). This depends on the
> > fragmentation of pinned buffers inside the aperture, a question only the
> > kernel can easily answer.
> > 
> > This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> > include a couple of new fields in its reply to userspace - the total
> > amount of space available in the mappable region of the aperture and
> > also the single largest block available.
> > 
> > This is not quite what userspace wants to answer the question of whether
> > this batch will fit as fences are also required to meet severe alignment
> > constraints within the batch. For this purpose, a third conservative
> > estimate of largest fence available is also provided. For when userspace
> > needs more than one batch, we also provide the culmulative space
> > available for fences such that it has some additional guidance to how
> > much space it could allocate to fences. Conservatism still wins.
> > 
> > The patch also adds a debugfs file for convenient testing and reporting.
> > 
> > v2: The first object cannot end at offset 0, so we can use last==0 to
> > detect the empty list.
> > 
> > v3: Expand all values to 64bit, just in case.
> >     Report total mappable aperture size for userspace that cannot easily
> >     determine it by inspecting the PCI device.
> > 
> > v4: (Rodrigo) Fixed rebase conflicts.
> > 
> > v5: Rebased to the latest drm-intel-nightly (Ankit)
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |  27 +++++++++
> >  drivers/gpu/drm/i915/i915_gem.c     | 116 ++++++++++++++++++++++++++++++++++--
> >  2 files changed, 139 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 31d8768..49ec438 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -512,6 +512,32 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
> >  	return 0;
> >  }
> >  
> > +static int i915_gem_aperture_info(struct seq_file *m, void *data)
> > +{
> > +	struct drm_info_node *node = m->private;
> > +	struct drm_i915_gem_get_aperture arg;
> > +	int ret;
> > +
> > +	ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	seq_printf(m, "Total size of the GTT: %llu bytes\n",
> > +		   arg.aper_size);
> > +	seq_printf(m, "Available space in the GTT: %llu bytes\n",
> > +		   arg.aper_available_size);
> > +	seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
> > +		   arg.map_available_size);
> > +	seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
> > +		   arg.map_largest_size);
> > +	seq_printf(m, "Available space for fences: %llu bytes\n",
> > +		   arg.fence_available_size);
> > +	seq_printf(m, "Single largest fence available: %llu bytes\n",
> > +		   arg.fence_largest_size);
> > +
> > +	return 0;
> > +}
> > +
> >  static int i915_gem_gtt_info(struct seq_file *m, void *data)
> >  {
> >  	struct drm_info_node *node = m->private;
> > @@ -5030,6 +5056,7 @@ static int i915_debugfs_create(struct dentry *root,
> >  static const struct drm_info_list i915_debugfs_list[] = {
> >  	{"i915_capabilities", i915_capabilities, 0},
> >  	{"i915_gem_objects", i915_gem_object_info, 0},
> > +	{"i915_gem_aperture", i915_gem_aperture_info, 0},
> >  	{"i915_gem_gtt", i915_gem_gtt_info, 0},
> >  	{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
> >  	{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index a2a4a27..ccfc8d3 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -32,6 +32,7 @@
> >  #include "i915_vgpu.h"
> >  #include "i915_trace.h"
> >  #include "intel_drv.h"
> > +#include <linux/list_sort.h>
> >  #include <linux/shmem_fs.h>
> >  #include <linux/slab.h>
> >  #include <linux/swap.h>
> > @@ -143,6 +144,55 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
> >  	return 0;
> >  }
> >  
> > +static inline bool
> > +i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
> > +{
> > +	return i915_gem_obj_bound_any(obj) && !obj->active;
> > +}
> > +
> > +static int obj_rank_by_ggtt(void *priv,
> > +			    struct list_head *A,
> > +			    struct list_head *B)
> > +{
> > +	struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
> > +	struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
> > +
> > +	return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
> > +}
> > +
> > +static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32 end)
> > +{
> > +	u32 size = end - start;
> > +	u32 fence_size;
> > +
> > +	if (INTEL_INFO(dev_priv)->gen < 4) {
> > +		u32 fence_max;
> > +		u32 fence_next;
> > +
> > +		if (IS_GEN3(dev_priv)) {
> > +			fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
> > +			fence_next = 1024*1024;
> > +		} else {
> > +			fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
> > +			fence_next = 512*1024;
> > +		}
> > +
> > +		fence_max = min(fence_max, size);
> > +		fence_size = 0;
> > +		while (fence_next <= fence_max) {
> > +			u32 base = ALIGN(start, fence_next);
> > +			if (base + fence_next > end)
> > +				break;
> > +
> > +			fence_size = fence_next;
> > +			fence_next <<= 1;
> > +		}
> > +	} else
> > +		fence_size = size;
> > +
> > +	return fence_size;
> > +}
> > +
> >  int
> >  i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
> >  			    struct drm_file *file)
> > @@ -150,17 +200,75 @@ 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;
> > +	struct list_head map_list;
> > +	const u32 map_limit = dev_priv->gtt.mappable_end;
> > +	size_t pinned, map_space, map_largest, fence_space, fence_largest;
> > +	u32 last, size;
> > +
> > +	INIT_LIST_HEAD(&map_list);
> >  
> >  	pinned = 0;
> > +	map_space = map_largest = 0;
> > +	fence_space = fence_largest = 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);
> > +	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> > +		struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
> > +
> > +		if (vma == NULL || !vma->pin_count)
> > +			continue;
> > +
> > +		pinned += vma->node.size;
> > +
> > +		if (vma->node.start < map_limit)
> > +			list_add(&obj->obj_exec_link, &map_list);
> > +	}
> > +
> > +	last = 0;
> > +	list_sort(NULL, &map_list, obj_rank_by_ggtt);
> > +	while (!list_empty(&map_list)) {
> > +		struct i915_vma *vma;
> > +
> > +		obj = list_first_entry(&map_list, typeof(*obj), obj_exec_link);
> > +		list_del_init(&obj->obj_exec_link);
> > +
> > +		vma = i915_gem_obj_to_ggtt(obj);
> > +		if (last == 0)
> > +			goto skip_first;
> > +
> > +		size = vma->node.start - last;
> > +		if (size > map_largest)
> > +			map_largest = size;
> > +		map_space += size;
> > +
> > +		size = __fence_size(dev_priv, last, vma->node.start);
> > +		if (size > fence_largest)
> > +			fence_largest = size;
> > +		fence_space += size;
> > +
> > +skip_first:
> > +		last = vma->node.start + vma->node.size;
> > +	}
> > +	if (last < map_limit) {
> > +		size = map_limit - last;
> > +		if (size > map_largest)
> > +			map_largest = size;
> > +		map_space += size;
> > +
> > +		size = __fence_size(dev_priv, last, map_limit);
> > +		if (size > fence_largest)
> > +			fence_largest = size;
> > +		fence_space += size;
> > +	}
> >  	mutex_unlock(&dev->struct_mutex);
> >  
> >  	args->aper_size = dev_priv->gtt.base.total;
> >  	args->aper_available_size = args->aper_size - pinned;
> > +	args->map_available_size = map_space;
> > +	args->map_largest_size = map_largest;
> > +	args->map_total_size = dev_priv->gtt.mappable_end;
> > +	args->fence_available_size = fence_space;
> > +	args->fence_largest_size = fence_largest;
> 
> Does this really compile? You only add all the new structure stuff in the
> 2nd patch.
> -Daniel
No, it does not. That is my bad. Some confusion with the patches. Need
to rebase again, carefully. 
-Thanks, Ankit
> 
> >  
> >  	return 0;
> >  }
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


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

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

* [PATCH v3 0/2] Extending GET_APERTURE ioctl
@ 2015-07-08  6:51 ankitprasad.r.sharma
  2015-07-08  6:51 ` [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space ankitprasad.r.sharma
  2015-07-08  6:51 ` [PATCH 2/2] drm/i915: Extend GET_APERTURE ioctl to report size of the stolen region ankitprasad.r.sharma
  0 siblings, 2 replies; 16+ messages in thread
From: ankitprasad.r.sharma @ 2015-07-08  6:51 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ankitprasad Sharma, akash.goel, shashidhar.hiremath

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

When constructing a batchbuffer, it is sometimes crucial to know the
largest hole into which we can fit a fenceable buffer (for example when
handling very large objects on gen2 and gen3). This depends on the
fragmentation of pinned buffers inside the aperture, a question only the
kernel can easily answer.

This set of patches extends the current DRM_I915_GEM_GET_APERTURE ioctl
to include a couple of new fields in its reply to userspace - the total
amount of space available in the mappable region of the aperture and
also the single largest block available.

This is not quite what userspace wants to answer the question of whether
this batch will fit as fences are also required to meet severe alignment
constraints within the batch. For this purpose, a third conservative
estimate of largest fence available is also provided. For when userspace
needs more than one batch, we also provide the culmulative space
available for fences such that it has some additional guidance to how
much space it could allocate to fences. Conservatism still wins.

The patches also support for getting total size and available size of the
stolen region as well as single largest block available in the stolen region.

It also adds a debugfs file for convenient testing and reporting.

v2: Rebased to the latest drm-intel-nightly

v3: Fixed compilation error

Ankitprasad Sharma (1):
  drm/i915: Extend GET_APERTURE ioctl to report size of the stolen
    region

Rodrigo Vivi (1):
  drm/i915: Extend GET_APERTURE ioctl to report available map space

 drivers/gpu/drm/i915/i915_debugfs.c    |  33 +++++++++
 drivers/gpu/drm/i915/i915_drv.h        |   3 +
 drivers/gpu/drm/i915/i915_gem.c        | 123 +++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_gem_stolen.c |  35 ++++++++++
 include/uapi/drm/i915_drm.h            |  40 +++++++++++
 5 files changed, 230 insertions(+), 4 deletions(-)

-- 
1.9.1

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

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

* [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08  6:51 [PATCH v3 0/2] Extending GET_APERTURE ioctl ankitprasad.r.sharma
@ 2015-07-08  6:51 ` ankitprasad.r.sharma
  2015-07-08 13:13   ` Tvrtko Ursulin
  2015-07-08 13:29   ` Tvrtko Ursulin
  2015-07-08  6:51 ` [PATCH 2/2] drm/i915: Extend GET_APERTURE ioctl to report size of the stolen region ankitprasad.r.sharma
  1 sibling, 2 replies; 16+ messages in thread
From: ankitprasad.r.sharma @ 2015-07-08  6:51 UTC (permalink / raw)
  To: intel-gfx; +Cc: akash.goel, shashidhar.hiremath

From: Rodrigo Vivi <rodrigo.vivi at intel.com>

When constructing a batchbuffer, it is sometimes crucial to know the
largest hole into which we can fit a fenceable buffer (for example when
handling very large objects on gen2 and gen3). This depends on the
fragmentation of pinned buffers inside the aperture, a question only the
kernel can easily answer.

This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
include a couple of new fields in its reply to userspace - the total
amount of space available in the mappable region of the aperture and
also the single largest block available.

This is not quite what userspace wants to answer the question of whether
this batch will fit as fences are also required to meet severe alignment
constraints within the batch. For this purpose, a third conservative
estimate of largest fence available is also provided. For when userspace
needs more than one batch, we also provide the culmulative space
available for fences such that it has some additional guidance to how
much space it could allocate to fences. Conservatism still wins.

The patch also adds a debugfs file for convenient testing and reporting.

v2: The first object cannot end at offset 0, so we can use last==0 to
detect the empty list.

v3: Expand all values to 64bit, just in case.
    Report total mappable aperture size for userspace that cannot easily
    determine it by inspecting the PCI device.

v4: (Rodrigo) Fixed rebase conflicts.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  27 +++++++++
 drivers/gpu/drm/i915/i915_gem.c     | 116 ++++++++++++++++++++++++++++++++++--
 include/uapi/drm/i915_drm.h         |  25 ++++++++
 3 files changed, 164 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 31d8768..49ec438 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -512,6 +512,32 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 	return 0;
 }
 
+static int i915_gem_aperture_info(struct seq_file *m, void *data)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_i915_gem_get_aperture arg;
+	int ret;
+
+	ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
+	if (ret)
+		return ret;
+
+	seq_printf(m, "Total size of the GTT: %llu bytes\n",
+		   arg.aper_size);
+	seq_printf(m, "Available space in the GTT: %llu bytes\n",
+		   arg.aper_available_size);
+	seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
+		   arg.map_available_size);
+	seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
+		   arg.map_largest_size);
+	seq_printf(m, "Available space for fences: %llu bytes\n",
+		   arg.fence_available_size);
+	seq_printf(m, "Single largest fence available: %llu bytes\n",
+		   arg.fence_largest_size);
+
+	return 0;
+}
+
 static int i915_gem_gtt_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
@@ -5030,6 +5056,7 @@ static int i915_debugfs_create(struct dentry *root,
 static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_capabilities", i915_capabilities, 0},
 	{"i915_gem_objects", i915_gem_object_info, 0},
+	{"i915_gem_aperture", i915_gem_aperture_info, 0},
 	{"i915_gem_gtt", i915_gem_gtt_info, 0},
 	{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
 	{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a2a4a27..ccfc8d3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -32,6 +32,7 @@
 #include "i915_vgpu.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include <linux/list_sort.h>
 #include <linux/shmem_fs.h>
 #include <linux/slab.h>
 #include <linux/swap.h>
@@ -143,6 +144,55 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 	return 0;
 }
 
+static inline bool
+i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
+{
+	return i915_gem_obj_bound_any(obj) && !obj->active;
+}
+
+static int obj_rank_by_ggtt(void *priv,
+			    struct list_head *A,
+			    struct list_head *B)
+{
+	struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
+	struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
+
+	return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
+}
+
+static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32 end)
+{
+	u32 size = end - start;
+	u32 fence_size;
+
+	if (INTEL_INFO(dev_priv)->gen < 4) {
+		u32 fence_max;
+		u32 fence_next;
+
+		if (IS_GEN3(dev_priv)) {
+			fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
+			fence_next = 1024*1024;
+		} else {
+			fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
+			fence_next = 512*1024;
+		}
+
+		fence_max = min(fence_max, size);
+		fence_size = 0;
+		while (fence_next <= fence_max) {
+			u32 base = ALIGN(start, fence_next);
+			if (base + fence_next > end)
+				break;
+
+			fence_size = fence_next;
+			fence_next <<= 1;
+		}
+	} else
+		fence_size = size;
+
+	return fence_size;
+}
+
 int
 i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 			    struct drm_file *file)
@@ -150,17 +200,75 @@ 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;
+	struct list_head map_list;
+	const u32 map_limit = dev_priv->gtt.mappable_end;
+	size_t pinned, map_space, map_largest, fence_space, fence_largest;
+	u32 last, size;
+
+	INIT_LIST_HEAD(&map_list);
 
 	pinned = 0;
+	map_space = map_largest = 0;
+	fence_space = fence_largest = 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);
+	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
+		struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
+
+		if (vma == NULL || !vma->pin_count)
+			continue;
+
+		pinned += vma->node.size;
+
+		if (vma->node.start < map_limit)
+			list_add(&obj->obj_exec_link, &map_list);
+	}
+
+	last = 0;
+	list_sort(NULL, &map_list, obj_rank_by_ggtt);
+	while (!list_empty(&map_list)) {
+		struct i915_vma *vma;
+
+		obj = list_first_entry(&map_list, typeof(*obj), obj_exec_link);
+		list_del_init(&obj->obj_exec_link);
+
+		vma = i915_gem_obj_to_ggtt(obj);
+		if (last == 0)
+			goto skip_first;
+
+		size = vma->node.start - last;
+		if (size > map_largest)
+			map_largest = size;
+		map_space += size;
+
+		size = __fence_size(dev_priv, last, vma->node.start);
+		if (size > fence_largest)
+			fence_largest = size;
+		fence_space += size;
+
+skip_first:
+		last = vma->node.start + vma->node.size;
+	}
+	if (last < map_limit) {
+		size = map_limit - last;
+		if (size > map_largest)
+			map_largest = size;
+		map_space += size;
+
+		size = __fence_size(dev_priv, last, map_limit);
+		if (size > fence_largest)
+			fence_largest = size;
+		fence_space += size;
+	}
 	mutex_unlock(&dev->struct_mutex);
 
 	args->aper_size = dev_priv->gtt.base.total;
 	args->aper_available_size = args->aper_size - pinned;
+	args->map_available_size = map_space;
+	args->map_largest_size = map_largest;
+	args->map_total_size = dev_priv->gtt.mappable_end;
+	args->fence_available_size = fence_space;
+	args->fence_largest_size = fence_largest;
 
 	return 0;
 }
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index f88cc1c..3e7f4a3 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -922,6 +922,31 @@ struct drm_i915_gem_get_aperture {
 	 * bytes
 	 */
 	__u64 aper_available_size;
+
+	/**
+	 * Total space in the mappable region of the aperture, in bytes
+	 */
+	__u64 map_total_size;
+
+	/**
+	 * Available space in the mappable region of the aperture, in bytes
+	 */
+	__u64 map_available_size;
+
+	/**
+	 * Single largest available region inside the mappable region, in bytes.
+	 */
+	__u64 map_largest_size;
+
+	/**
+	 * Culmulative space available for fences, in bytes
+	 */
+	__u64 fence_available_size;
+
+	/**
+	 * Single largest fenceable region, in bytes.
+	 */
+	__u64 fence_largest_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

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

* [PATCH 2/2] drm/i915: Extend GET_APERTURE ioctl to report size of the stolen region
  2015-07-08  6:51 [PATCH v3 0/2] Extending GET_APERTURE ioctl ankitprasad.r.sharma
  2015-07-08  6:51 ` [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space ankitprasad.r.sharma
@ 2015-07-08  6:51 ` ankitprasad.r.sharma
  2015-07-08 13:33   ` Tvrtko Ursulin
  1 sibling, 1 reply; 16+ messages in thread
From: ankitprasad.r.sharma @ 2015-07-08  6:51 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ankitprasad Sharma, akash.goel, shashidhar.hiremath

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

This patch extends the GET_APERTURE ioctl to add support
for getting total size and available size of the stolen region
as well as single largest block available in the stolen region.
Also adds debugfs support to retieve the size information of the
stolen area.

v2: respinned over Rodrigo's patch which extends the GET_APERTURE
too. Used drm_mm to get the size information of the stolen region
(Chris)
Added debugfs support for testing (Ankit)

v3: Rebased to the latest drm-intel-nightly (Ankit)

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c    |  6 ++++++
 drivers/gpu/drm/i915/i915_drv.h        |  3 +++
 drivers/gpu/drm/i915/i915_gem.c        |  7 +++++++
 drivers/gpu/drm/i915/i915_gem_stolen.c | 35 ++++++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h            | 15 +++++++++++++++
 5 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 49ec438..d12ef0a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -534,6 +534,12 @@ static int i915_gem_aperture_info(struct seq_file *m, void *data)
 		   arg.fence_available_size);
 	seq_printf(m, "Single largest fence available: %llu bytes\n",
 		   arg.fence_largest_size);
+	seq_printf(m, "Total size of the stolen region: %llu bytes\n",
+		   arg.stolen_total_size);
+	seq_printf(m, "Available size of the stolen region: %llu bytes\n",
+		   arg.stolen_available_size);
+	seq_printf(m, "Single largest area in the stolen region: %llu bytes\n",
+		   arg.stolen_largest_size);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ea9caf2..7cd1b2e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3101,6 +3101,9 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 					       u32 stolen_offset,
 					       u32 gtt_offset,
 					       u32 size);
+void i915_gem_stolen_size_info(struct drm_mm *mm, uint64_t *stolen_total,
+			       uint64_t *stolen_free,
+			       uint64_t *stolen_largest);
 
 /* i915_gem_shrinker.c */
 unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ccfc8d3..ec20c67 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -203,6 +203,7 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 	struct list_head map_list;
 	const u32 map_limit = dev_priv->gtt.mappable_end;
 	size_t pinned, map_space, map_largest, fence_space, fence_largest;
+	uint64_t stolen_total, stolen_available, stolen_largest;
 	u32 last, size;
 
 	INIT_LIST_HEAD(&map_list);
@@ -260,6 +261,9 @@ skip_first:
 			fence_largest = size;
 		fence_space += size;
 	}
+
+	i915_gem_stolen_size_info(&dev_priv->mm.stolen, &stolen_total,
+				  &stolen_available, &stolen_largest);
 	mutex_unlock(&dev->struct_mutex);
 
 	args->aper_size = dev_priv->gtt.base.total;
@@ -269,6 +273,9 @@ skip_first:
 	args->map_total_size = dev_priv->gtt.mappable_end;
 	args->fence_available_size = fence_space;
 	args->fence_largest_size = fence_largest;
+	args->stolen_total_size = stolen_total;
+	args->stolen_available_size = stolen_available;
+	args->stolen_largest_size = stolen_largest;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 348ed5a..08d983f 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -551,3 +551,38 @@ err_out:
 	drm_gem_object_unreference(&obj->base);
 	return NULL;
 }
+
+void i915_gem_stolen_size_info(struct drm_mm *mm, uint64_t *stolen_total,
+			       uint64_t *stolen_free,
+			       uint64_t *stolen_largest)
+{
+	struct drm_mm_node *entry;
+	struct drm_mm_node *head_node = &mm->head_node;
+	uint64_t hole_size, hole_start, hole_end, largest_hole = 0;
+	uint64_t total_used = 0, total_free = 0;
+
+	if (head_node->hole_follows) {
+		hole_start = drm_mm_hole_node_start(head_node);
+		hole_end = drm_mm_hole_node_end(head_node);
+		hole_size = hole_end - hole_start;
+		total_free += hole_size;
+		if (largest_hole < hole_size)
+			largest_hole = hole_size;
+	}
+
+	drm_mm_for_each_node(entry, mm) {
+		total_used += entry->size;
+		if (entry->hole_follows) {
+			hole_start = drm_mm_hole_node_start(entry);
+			hole_end = drm_mm_hole_node_end(entry);
+			hole_size = hole_end - hole_start;
+			total_free += hole_size;
+			if (largest_hole < hole_size)
+				largest_hole = hole_size;
+		}
+	}
+
+	*stolen_total = total_free + total_used;
+	*stolen_free = total_free;
+	*stolen_largest = largest_hole;
+}
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 3e7f4a3..9d59fb0 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -947,6 +947,21 @@ struct drm_i915_gem_get_aperture {
 	 * Single largest fenceable region, in bytes.
 	 */
 	__u64 fence_largest_size;
+
+	/**
+	 * Total space in the stolen region, in bytes
+	 */
+	__u64 stolen_total_size;
+
+	/**
+	 * Available space in the stolen region, in bytes
+	 */
+	__u64 stolen_available_size;
+
+	/**
+	 * Single largest block in stolen region, in bytes
+	 */
+	__u64 stolen_largest_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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08  6:51 ` [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space ankitprasad.r.sharma
@ 2015-07-08 13:13   ` Tvrtko Ursulin
  2015-07-08 13:28     ` Chris Wilson
  2015-07-08 13:29   ` Tvrtko Ursulin
  1 sibling, 1 reply; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-07-08 13:13 UTC (permalink / raw)
  To: ankitprasad.r.sharma, intel-gfx; +Cc: akash.goel, shashidhar.hiremath


Hi,

On 07/08/2015 07:51 AM, ankitprasad.r.sharma@intel.com wrote:
> From: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> When constructing a batchbuffer, it is sometimes crucial to know the
> largest hole into which we can fit a fenceable buffer (for example when
> handling very large objects on gen2 and gen3). This depends on the
> fragmentation of pinned buffers inside the aperture, a question only the
> kernel can easily answer.
>
> This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> include a couple of new fields in its reply to userspace - the total
> amount of space available in the mappable region of the aperture and
> also the single largest block available.

Since whatever this returns is a transient number is this really that 
useful? There are no guarantees that by the time caller tries to act on 
it it will still be valid.

> This is not quite what userspace wants to answer the question of whether
> this batch will fit as fences are also required to meet severe alignment
> constraints within the batch. For this purpose, a third conservative
> estimate of largest fence available is also provided. For when userspace
> needs more than one batch, we also provide the culmulative space
> available for fences such that it has some additional guidance to how
> much space it could allocate to fences. Conservatism still wins.
>
> The patch also adds a debugfs file for convenient testing and reporting.
>
> v2: The first object cannot end at offset 0, so we can use last==0 to
> detect the empty list.
>
> v3: Expand all values to 64bit, just in case.
>      Report total mappable aperture size for userspace that cannot easily
>      determine it by inspecting the PCI device.
>
> v4: (Rodrigo) Fixed rebase conflicts.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c |  27 +++++++++
>   drivers/gpu/drm/i915/i915_gem.c     | 116 ++++++++++++++++++++++++++++++++++--
>   include/uapi/drm/i915_drm.h         |  25 ++++++++
>   3 files changed, 164 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 31d8768..49ec438 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -512,6 +512,32 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>   	return 0;
>   }
>
> +static int i915_gem_aperture_info(struct seq_file *m, void *data)
> +{
> +	struct drm_info_node *node = m->private;
> +	struct drm_i915_gem_get_aperture arg;
> +	int ret;
> +
> +	ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
> +	if (ret)
> +		return ret;
> +
> +	seq_printf(m, "Total size of the GTT: %llu bytes\n",
> +		   arg.aper_size);
> +	seq_printf(m, "Available space in the GTT: %llu bytes\n",
> +		   arg.aper_available_size);
> +	seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
> +		   arg.map_available_size);
> +	seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
> +		   arg.map_largest_size);
> +	seq_printf(m, "Available space for fences: %llu bytes\n",
> +		   arg.fence_available_size);
> +	seq_printf(m, "Single largest fence available: %llu bytes\n",
> +		   arg.fence_largest_size);
> +
> +	return 0;
> +}
> +
>   static int i915_gem_gtt_info(struct seq_file *m, void *data)
>   {
>   	struct drm_info_node *node = m->private;
> @@ -5030,6 +5056,7 @@ static int i915_debugfs_create(struct dentry *root,
>   static const struct drm_info_list i915_debugfs_list[] = {
>   	{"i915_capabilities", i915_capabilities, 0},
>   	{"i915_gem_objects", i915_gem_object_info, 0},
> +	{"i915_gem_aperture", i915_gem_aperture_info, 0},
>   	{"i915_gem_gtt", i915_gem_gtt_info, 0},
>   	{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
>   	{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a2a4a27..ccfc8d3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -32,6 +32,7 @@
>   #include "i915_vgpu.h"
>   #include "i915_trace.h"
>   #include "intel_drv.h"
> +#include <linux/list_sort.h>
>   #include <linux/shmem_fs.h>
>   #include <linux/slab.h>
>   #include <linux/swap.h>
> @@ -143,6 +144,55 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
>   	return 0;
>   }
>
> +static inline bool
> +i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
> +{
> +	return i915_gem_obj_bound_any(obj) && !obj->active;
> +}
> +
> +static int obj_rank_by_ggtt(void *priv,
> +			    struct list_head *A,
> +			    struct list_head *B)
> +{
> +	struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
> +	struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);

Nitpick - space after comma.

> +
> +	return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
> +}
> +
> +static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32 end)
> +{
> +	u32 size = end - start;
> +	u32 fence_size;
> +
> +	if (INTEL_INFO(dev_priv)->gen < 4) {
> +		u32 fence_max;
> +		u32 fence_next;
> +
> +		if (IS_GEN3(dev_priv)) {
> +			fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
> +			fence_next = 1024*1024;
> +		} else {
> +			fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
> +			fence_next = 512*1024;
> +		}
> +
> +		fence_max = min(fence_max, size);
> +		fence_size = 0;
> +		while (fence_next <= fence_max) {
> +			u32 base = ALIGN(start, fence_next);
> +			if (base + fence_next > end)
> +				break;
> +
> +			fence_size = fence_next;
> +			fence_next <<= 1;
> +		}

I don't know a lot about fence alignment and size restrictions but since 
this loop is relatively complex could use a comment on what it is doing.

> +	} else
> +		fence_size = size;

I think coding style is for all if/else branches to have braces if one 
has - feel free to correct me if I am wrong.

> +
> +	return fence_size;
> +}
> +
>   int
>   i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
>   			    struct drm_file *file)
> @@ -150,17 +200,75 @@ 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;
> +	struct list_head map_list;
> +	const u32 map_limit = dev_priv->gtt.mappable_end;

mappable_end is u64 so gcc should complain here.

> +	size_t pinned, map_space, map_largest, fence_space, fence_largest;
> +	u32 last, size;

And since the ongoing push to standardize all GTT size variables to u64 
I think you'l have to change the above as well.

> +
> +	INIT_LIST_HEAD(&map_list);
>
>   	pinned = 0;
> +	map_space = map_largest = 0;
> +	fence_space = fence_largest = 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);

Unfortunately you will have to rebase this again since someone managed 
to change it in the meantime. Ooops it was me - sorry! :)

> +	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> +		struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);

The issue with this is it ignores rotated and partial VMA types so will 
give wrong numbers. Rebasing should take care of it.

> +		if (vma == NULL || !vma->pin_count)
> +			continue;
> +
> +		pinned += vma->node.size;
> +
> +		if (vma->node.start < map_limit)
> +			list_add(&obj->obj_exec_link, &map_list);
> +	}
> +
> +	last = 0;
> +	list_sort(NULL, &map_list, obj_rank_by_ggtt);

I suppose the problems really start with the compare function here. 
Since you store objects in the list and not VMAs, compare can't know 
which are the correct VMAs to use.

Are there any free list_heads in the VMA you could use? vma->exec_list 
looks like it could be used. (Please verify :)

That would make map_list store VMAs and compare function can be change 
to use vma->node.start then (and work with normal/rotated/partial views).

Regards,

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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08 13:13   ` Tvrtko Ursulin
@ 2015-07-08 13:28     ` Chris Wilson
  2015-07-08 13:36       ` Tvrtko Ursulin
  2015-07-08 13:38       ` Chris Wilson
  0 siblings, 2 replies; 16+ messages in thread
From: Chris Wilson @ 2015-07-08 13:28 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: ankitprasad.r.sharma, intel-gfx, akash.goel, shashidhar.hiremath

On Wed, Jul 08, 2015 at 02:13:43PM +0100, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 07/08/2015 07:51 AM, ankitprasad.r.sharma@intel.com wrote:
> >From: Rodrigo Vivi <rodrigo.vivi at intel.com>
> >
> >When constructing a batchbuffer, it is sometimes crucial to know the
> >largest hole into which we can fit a fenceable buffer (for example when
> >handling very large objects on gen2 and gen3). This depends on the
> >fragmentation of pinned buffers inside the aperture, a question only the
> >kernel can easily answer.
> >
> >This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> >include a couple of new fields in its reply to userspace - the total
> >amount of space available in the mappable region of the aperture and
> >also the single largest block available.
> 
> Since whatever this returns is a transient number is this really
> that useful? There are no guarantees that by the time caller tries
> to act on it it will still be valid.

Yes. My use case is actually after a failure to capture debug
information. I don't anticipate frequent calls to get_aperture (usually
just a single call during early init).
-Chrs

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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08  6:51 ` [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space ankitprasad.r.sharma
  2015-07-08 13:13   ` Tvrtko Ursulin
@ 2015-07-08 13:29   ` Tvrtko Ursulin
  1 sibling, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-07-08 13:29 UTC (permalink / raw)
  To: ankitprasad.r.sharma, intel-gfx; +Cc: akash.goel, shashidhar.hiremath


On 07/08/2015 07:51 AM, ankitprasad.r.sharma@intel.com wrote:
> From: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> When constructing a batchbuffer, it is sometimes crucial to know the
> largest hole into which we can fit a fenceable buffer (for example when
> handling very large objects on gen2 and gen3). This depends on the
> fragmentation of pinned buffers inside the aperture, a question only the
> kernel can easily answer.
>
> This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> include a couple of new fields in its reply to userspace - the total
> amount of space available in the mappable region of the aperture and
> also the single largest block available.
>
> This is not quite what userspace wants to answer the question of whether
> this batch will fit as fences are also required to meet severe alignment
> constraints within the batch. For this purpose, a third conservative
> estimate of largest fence available is also provided. For when userspace
> needs more than one batch, we also provide the culmulative space
> available for fences such that it has some additional guidance to how
> much space it could allocate to fences. Conservatism still wins.
>
> The patch also adds a debugfs file for convenient testing and reporting.
>
> v2: The first object cannot end at offset 0, so we can use last==0 to
> detect the empty list.
>
> v3: Expand all values to 64bit, just in case.
>      Report total mappable aperture size for userspace that cannot easily
>      determine it by inspecting the PCI device.
>
> v4: (Rodrigo) Fixed rebase conflicts.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c |  27 +++++++++
>   drivers/gpu/drm/i915/i915_gem.c     | 116 ++++++++++++++++++++++++++++++++++--
>   include/uapi/drm/i915_drm.h         |  25 ++++++++
>   3 files changed, 164 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 31d8768..49ec438 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -512,6 +512,32 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>   	return 0;
>   }
>
> +static int i915_gem_aperture_info(struct seq_file *m, void *data)
> +{
> +	struct drm_info_node *node = m->private;
> +	struct drm_i915_gem_get_aperture arg;
> +	int ret;
> +
> +	ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
> +	if (ret)
> +		return ret;
> +
> +	seq_printf(m, "Total size of the GTT: %llu bytes\n",
> +		   arg.aper_size);
> +	seq_printf(m, "Available space in the GTT: %llu bytes\n",
> +		   arg.aper_available_size);
> +	seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
> +		   arg.map_available_size);
> +	seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
> +		   arg.map_largest_size);
> +	seq_printf(m, "Available space for fences: %llu bytes\n",
> +		   arg.fence_available_size);
> +	seq_printf(m, "Single largest fence available: %llu bytes\n",
> +		   arg.fence_largest_size);
> +
> +	return 0;
> +}
> +
>   static int i915_gem_gtt_info(struct seq_file *m, void *data)
>   {
>   	struct drm_info_node *node = m->private;
> @@ -5030,6 +5056,7 @@ static int i915_debugfs_create(struct dentry *root,
>   static const struct drm_info_list i915_debugfs_list[] = {
>   	{"i915_capabilities", i915_capabilities, 0},
>   	{"i915_gem_objects", i915_gem_object_info, 0},
> +	{"i915_gem_aperture", i915_gem_aperture_info, 0},
>   	{"i915_gem_gtt", i915_gem_gtt_info, 0},
>   	{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
>   	{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a2a4a27..ccfc8d3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -32,6 +32,7 @@
>   #include "i915_vgpu.h"
>   #include "i915_trace.h"
>   #include "intel_drv.h"
> +#include <linux/list_sort.h>
>   #include <linux/shmem_fs.h>
>   #include <linux/slab.h>
>   #include <linux/swap.h>
> @@ -143,6 +144,55 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
>   	return 0;
>   }
>
> +static inline bool
> +i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
> +{
> +	return i915_gem_obj_bound_any(obj) && !obj->active;
> +}

Oh and this function is not used.

Regards,

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

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

* Re: [PATCH 2/2] drm/i915: Extend GET_APERTURE ioctl to report size of the stolen region
  2015-07-08  6:51 ` [PATCH 2/2] drm/i915: Extend GET_APERTURE ioctl to report size of the stolen region ankitprasad.r.sharma
@ 2015-07-08 13:33   ` Tvrtko Ursulin
  0 siblings, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-07-08 13:33 UTC (permalink / raw)
  To: ankitprasad.r.sharma, intel-gfx; +Cc: akash.goel, shashidhar.hiremath


Hi,

On 07/08/2015 07:51 AM, 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 and available size of the stolen region
> as well as single largest block available in the stolen region.
> Also adds debugfs support to retieve the size information of the
> stolen area.
>
> v2: respinned over Rodrigo's patch which extends the GET_APERTURE
> too. Used drm_mm to get the size information of the stolen region
> (Chris)
> Added debugfs support for testing (Ankit)
>
> v3: Rebased to the latest drm-intel-nightly (Ankit)
>
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c    |  6 ++++++
>   drivers/gpu/drm/i915/i915_drv.h        |  3 +++
>   drivers/gpu/drm/i915/i915_gem.c        |  7 +++++++
>   drivers/gpu/drm/i915/i915_gem_stolen.c | 35 ++++++++++++++++++++++++++++++++++
>   include/uapi/drm/i915_drm.h            | 15 +++++++++++++++
>   5 files changed, 66 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 49ec438..d12ef0a 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -534,6 +534,12 @@ static int i915_gem_aperture_info(struct seq_file *m, void *data)
>   		   arg.fence_available_size);
>   	seq_printf(m, "Single largest fence available: %llu bytes\n",
>   		   arg.fence_largest_size);
> +	seq_printf(m, "Total size of the stolen region: %llu bytes\n",
> +		   arg.stolen_total_size);
> +	seq_printf(m, "Available size of the stolen region: %llu bytes\n",
> +		   arg.stolen_available_size);
> +	seq_printf(m, "Single largest area in the stolen region: %llu bytes\n",
> +		   arg.stolen_largest_size);
>
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ea9caf2..7cd1b2e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3101,6 +3101,9 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>   					       u32 stolen_offset,
>   					       u32 gtt_offset,
>   					       u32 size);
> +void i915_gem_stolen_size_info(struct drm_mm *mm, uint64_t *stolen_total,
> +			       uint64_t *stolen_free,
> +			       uint64_t *stolen_largest);
>
>   /* i915_gem_shrinker.c */
>   unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ccfc8d3..ec20c67 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -203,6 +203,7 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
>   	struct list_head map_list;
>   	const u32 map_limit = dev_priv->gtt.mappable_end;
>   	size_t pinned, map_space, map_largest, fence_space, fence_largest;
> +	uint64_t stolen_total, stolen_available, stolen_largest;
>   	u32 last, size;
>
>   	INIT_LIST_HEAD(&map_list);
> @@ -260,6 +261,9 @@ skip_first:
>   			fence_largest = size;
>   		fence_space += size;
>   	}
> +
> +	i915_gem_stolen_size_info(&dev_priv->mm.stolen, &stolen_total,
> +				  &stolen_available, &stolen_largest);
>   	mutex_unlock(&dev->struct_mutex);
>
>   	args->aper_size = dev_priv->gtt.base.total;
> @@ -269,6 +273,9 @@ skip_first:
>   	args->map_total_size = dev_priv->gtt.mappable_end;
>   	args->fence_available_size = fence_space;
>   	args->fence_largest_size = fence_largest;
> +	args->stolen_total_size = stolen_total;
> +	args->stolen_available_size = stolen_available;
> +	args->stolen_largest_size = stolen_largest;
>
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 348ed5a..08d983f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -551,3 +551,38 @@ err_out:
>   	drm_gem_object_unreference(&obj->base);
>   	return NULL;
>   }
> +
> +void i915_gem_stolen_size_info(struct drm_mm *mm, uint64_t *stolen_total,
> +			       uint64_t *stolen_free,
> +			       uint64_t *stolen_largest)
> +{
> +	struct drm_mm_node *entry;
> +	struct drm_mm_node *head_node = &mm->head_node;
> +	uint64_t hole_size, hole_start, hole_end, largest_hole = 0;
> +	uint64_t total_used = 0, total_free = 0;
> +
> +	if (head_node->hole_follows) {
> +		hole_start = drm_mm_hole_node_start(head_node);
> +		hole_end = drm_mm_hole_node_end(head_node);
> +		hole_size = hole_end - hole_start;
> +		total_free += hole_size;
> +		if (largest_hole < hole_size)
> +			largest_hole = hole_size;

Guaranteed it is the largest hole by this point, but not worth the 
effort of respining to change it.

Maybe only consider getting rid of hole_start and hole_end since they 
are not used but once. It would compact this block and the one below. 
Unless it would be longer than 80 chars in which case is not worth it.

> +	}
> +
> +	drm_mm_for_each_node(entry, mm) {
> +		total_used += entry->size;
> +		if (entry->hole_follows) {
> +			hole_start = drm_mm_hole_node_start(entry);
> +			hole_end = drm_mm_hole_node_end(entry);
> +			hole_size = hole_end - hole_start;
> +			total_free += hole_size;
> +			if (largest_hole < hole_size)
> +				largest_hole = hole_size;
> +		}
> +	}
> +
> +	*stolen_total = total_free + total_used;

Pitty for such a round about way to get total amount of stolen memory, 
but it looks we don't store it elsewhere.

> +	*stolen_free = total_free;
> +	*stolen_largest = largest_hole;

Again I wonder the usefulness of this data?

But for the code review itself;

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08 13:28     ` Chris Wilson
@ 2015-07-08 13:36       ` Tvrtko Ursulin
  2015-07-08 13:53         ` Chris Wilson
  2015-07-08 13:38       ` Chris Wilson
  1 sibling, 1 reply; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-07-08 13:36 UTC (permalink / raw)
  To: Chris Wilson, ankitprasad.r.sharma, intel-gfx, akash.goel,
	shashidhar.hiremath


On 07/08/2015 02:28 PM, Chris Wilson wrote:
> On Wed, Jul 08, 2015 at 02:13:43PM +0100, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 07/08/2015 07:51 AM, ankitprasad.r.sharma@intel.com wrote:
>>> From: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>>
>>> When constructing a batchbuffer, it is sometimes crucial to know the
>>> largest hole into which we can fit a fenceable buffer (for example when
>>> handling very large objects on gen2 and gen3). This depends on the
>>> fragmentation of pinned buffers inside the aperture, a question only the
>>> kernel can easily answer.
>>>
>>> This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
>>> include a couple of new fields in its reply to userspace - the total
>>> amount of space available in the mappable region of the aperture and
>>> also the single largest block available.
>>
>> Since whatever this returns is a transient number is this really
>> that useful? There are no guarantees that by the time caller tries
>> to act on it it will still be valid.
>
> Yes. My use case is actually after a failure to capture debug
> information. I don't anticipate frequent calls to get_aperture (usually
> just a single call during early init).

Should it better go to debugfs then?

Regards,

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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08 13:28     ` Chris Wilson
  2015-07-08 13:36       ` Tvrtko Ursulin
@ 2015-07-08 13:38       ` Chris Wilson
  1 sibling, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2015-07-08 13:38 UTC (permalink / raw)
  To: Tvrtko Ursulin, ankitprasad.r.sharma, intel-gfx, akash.goel,
	shashidhar.hiremath

On Wed, Jul 08, 2015 at 02:28:33PM +0100, Chris Wilson wrote:
> On Wed, Jul 08, 2015 at 02:13:43PM +0100, Tvrtko Ursulin wrote:
> > 
> > Hi,
> > 
> > On 07/08/2015 07:51 AM, ankitprasad.r.sharma@intel.com wrote:
> > >From: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > >
> > >When constructing a batchbuffer, it is sometimes crucial to know the
> > >largest hole into which we can fit a fenceable buffer (for example when
> > >handling very large objects on gen2 and gen3). This depends on the
> > >fragmentation of pinned buffers inside the aperture, a question only the
> > >kernel can easily answer.
> > >
> > >This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> > >include a couple of new fields in its reply to userspace - the total
> > >amount of space available in the mappable region of the aperture and
> > >also the single largest block available.
> > 
> > Since whatever this returns is a transient number is this really
> > that useful? There are no guarantees that by the time caller tries
> > to act on it it will still be valid.
> 
> Yes. My use case is actually after a failure to capture debug
> information. I don't anticipate frequent calls to get_aperture (usually
> just a single call during early init).

Though that predates full-ppgtt (and even the recent aliasing-ppgtt
split). Time to start thinking about doing something similar for
contexts.
-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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08 13:36       ` Tvrtko Ursulin
@ 2015-07-08 13:53         ` Chris Wilson
  2015-07-08 14:24           ` Tvrtko Ursulin
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2015-07-08 13:53 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: ankitprasad.r.sharma, intel-gfx, akash.goel, shashidhar.hiremath

On Wed, Jul 08, 2015 at 02:36:08PM +0100, Tvrtko Ursulin wrote:
> 
> On 07/08/2015 02:28 PM, Chris Wilson wrote:
> >On Wed, Jul 08, 2015 at 02:13:43PM +0100, Tvrtko Ursulin wrote:
> >>
> >>Hi,
> >>
> >>On 07/08/2015 07:51 AM, ankitprasad.r.sharma@intel.com wrote:
> >>>From: Rodrigo Vivi <rodrigo.vivi at intel.com>
> >>>
> >>>When constructing a batchbuffer, it is sometimes crucial to know the
> >>>largest hole into which we can fit a fenceable buffer (for example when
> >>>handling very large objects on gen2 and gen3). This depends on the
> >>>fragmentation of pinned buffers inside the aperture, a question only the
> >>>kernel can easily answer.
> >>>
> >>>This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> >>>include a couple of new fields in its reply to userspace - the total
> >>>amount of space available in the mappable region of the aperture and
> >>>also the single largest block available.
> >>
> >>Since whatever this returns is a transient number is this really
> >>that useful? There are no guarantees that by the time caller tries
> >>to act on it it will still be valid.
> >
> >Yes. My use case is actually after a failure to capture debug
> >information. I don't anticipate frequent calls to get_aperture (usually
> >just a single call during early init).
> 
> Should it better go to debugfs then?

Hmm, I could accept that. In such a scenario, I would suggest we ignore
the avail_aperture_space field all together, just report it as max (or
whatever) and simply add fields for max stolen, max mappable, max ggtt
(already present) and max ppgtt. (Rather than continue trying to kill
multiple birds with one stone, where 99.9% of users don't want the
overhead.)

Move the current patch series into debugfs.c and just add the limits to
get_aperture?
-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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08 13:53         ` Chris Wilson
@ 2015-07-08 14:24           ` Tvrtko Ursulin
  2015-07-08 14:32             ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Tvrtko Ursulin @ 2015-07-08 14:24 UTC (permalink / raw)
  To: Chris Wilson, ankitprasad.r.sharma, intel-gfx, akash.goel,
	shashidhar.hiremath


On 07/08/2015 02:53 PM, Chris Wilson wrote:
> On Wed, Jul 08, 2015 at 02:36:08PM +0100, Tvrtko Ursulin wrote:
>>
>> On 07/08/2015 02:28 PM, Chris Wilson wrote:
>>> On Wed, Jul 08, 2015 at 02:13:43PM +0100, Tvrtko Ursulin wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 07/08/2015 07:51 AM, ankitprasad.r.sharma@intel.com wrote:
>>>>> From: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>>>>
>>>>> When constructing a batchbuffer, it is sometimes crucial to know the
>>>>> largest hole into which we can fit a fenceable buffer (for example when
>>>>> handling very large objects on gen2 and gen3). This depends on the
>>>>> fragmentation of pinned buffers inside the aperture, a question only the
>>>>> kernel can easily answer.
>>>>>
>>>>> This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
>>>>> include a couple of new fields in its reply to userspace - the total
>>>>> amount of space available in the mappable region of the aperture and
>>>>> also the single largest block available.
>>>>
>>>> Since whatever this returns is a transient number is this really
>>>> that useful? There are no guarantees that by the time caller tries
>>>> to act on it it will still be valid.
>>>
>>> Yes. My use case is actually after a failure to capture debug
>>> information. I don't anticipate frequent calls to get_aperture (usually
>>> just a single call during early init).
>>
>> Should it better go to debugfs then?
>
> Hmm, I could accept that. In such a scenario, I would suggest we ignore
> the avail_aperture_space field all together, just report it as max (or
> whatever) and simply add fields for max stolen, max mappable, max ggtt
> (already present) and max ppgtt. (Rather than continue trying to kill
> multiple birds with one stone, where 99.9% of users don't want the
> overhead.)
>
> Move the current patch series into debugfs.c and just add the limits to
> get_aperture?

It would make a more sensible ioctl if you think we could get away with 
it. Hopefully no userspace tries to do anything smart with 
aper_available_size today? I see one IGT user, gem_ctx_exec, which 
probably isn't a blocker.

In that case for stolen we would add:

  stolen_size = dev_priv->gtt.stolen_size
  stolen_available = stolen_size - bios_reserved

(Bios_reserved would have to be stored in i915_gtt since it is currently 
local to i915_gem_init_stolen.)

Plus the ones you listed, mappable_size and ppgtt_size.

And the used/largest/fence/... ones go to debugfs.

Regards,

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

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

* Re: [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space
  2015-07-08 14:24           ` Tvrtko Ursulin
@ 2015-07-08 14:32             ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2015-07-08 14:32 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: ankitprasad.r.sharma, intel-gfx, akash.goel, shashidhar.hiremath

On Wed, Jul 08, 2015 at 03:24:08PM +0100, Tvrtko Ursulin wrote:
> 
> On 07/08/2015 02:53 PM, Chris Wilson wrote:
> >On Wed, Jul 08, 2015 at 02:36:08PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 07/08/2015 02:28 PM, Chris Wilson wrote:
> >>>On Wed, Jul 08, 2015 at 02:13:43PM +0100, Tvrtko Ursulin wrote:
> >>>>
> >>>>Hi,
> >>>>
> >>>>On 07/08/2015 07:51 AM, ankitprasad.r.sharma@intel.com wrote:
> >>>>>From: Rodrigo Vivi <rodrigo.vivi at intel.com>
> >>>>>
> >>>>>When constructing a batchbuffer, it is sometimes crucial to know the
> >>>>>largest hole into which we can fit a fenceable buffer (for example when
> >>>>>handling very large objects on gen2 and gen3). This depends on the
> >>>>>fragmentation of pinned buffers inside the aperture, a question only the
> >>>>>kernel can easily answer.
> >>>>>
> >>>>>This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> >>>>>include a couple of new fields in its reply to userspace - the total
> >>>>>amount of space available in the mappable region of the aperture and
> >>>>>also the single largest block available.
> >>>>
> >>>>Since whatever this returns is a transient number is this really
> >>>>that useful? There are no guarantees that by the time caller tries
> >>>>to act on it it will still be valid.
> >>>
> >>>Yes. My use case is actually after a failure to capture debug
> >>>information. I don't anticipate frequent calls to get_aperture (usually
> >>>just a single call during early init).
> >>
> >>Should it better go to debugfs then?
> >
> >Hmm, I could accept that. In such a scenario, I would suggest we ignore
> >the avail_aperture_space field all together, just report it as max (or
> >whatever) and simply add fields for max stolen, max mappable, max ggtt
> >(already present) and max ppgtt. (Rather than continue trying to kill
> >multiple birds with one stone, where 99.9% of users don't want the
> >overhead.)
> >
> >Move the current patch series into debugfs.c and just add the limits to
> >get_aperture?
> 
> It would make a more sensible ioctl if you think we could get away
> with it. Hopefully no userspace tries to do anything smart with
> aper_available_size today? I see one IGT user, gem_ctx_exec, which
> probably isn't a blocker.

Now that you mention it, we lie anyway there. The idea behind
gem_ctx_exec is try and guage how much remaining global gtt space we
need to fill with context objects - but important reservations of global
gtt aren't even subtracted from the avail aperture space.

There's also a use in sna when we run very close to the limit, we double
check if the available space is enough. That's semi-sensible, certainly
for the gen2-3 system that hit it most.

Ok, so we get to keep aper_available_size.
-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

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

end of thread, other threads:[~2015-07-08 14:32 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-08  6:51 [PATCH v3 0/2] Extending GET_APERTURE ioctl ankitprasad.r.sharma
2015-07-08  6:51 ` [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space ankitprasad.r.sharma
2015-07-08 13:13   ` Tvrtko Ursulin
2015-07-08 13:28     ` Chris Wilson
2015-07-08 13:36       ` Tvrtko Ursulin
2015-07-08 13:53         ` Chris Wilson
2015-07-08 14:24           ` Tvrtko Ursulin
2015-07-08 14:32             ` Chris Wilson
2015-07-08 13:38       ` Chris Wilson
2015-07-08 13:29   ` Tvrtko Ursulin
2015-07-08  6:51 ` [PATCH 2/2] drm/i915: Extend GET_APERTURE ioctl to report size of the stolen region ankitprasad.r.sharma
2015-07-08 13:33   ` Tvrtko Ursulin
  -- strict thread matches above, loose matches on Subject: below --
2015-07-01  9:25 [PATCH v2 0/2] Extending GET_APERTURE ioctl ankitprasad.r.sharma
2015-07-01  9:25 ` [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space ankitprasad.r.sharma
2015-07-01 13:39   ` Daniel Vetter
2015-07-01 16:34     ` Ankitprasad Sharma
2015-05-13 12:07 [PATCH 0/2] Extending GET_APERTURE ioctl ankitprasad.r.sharma
2015-05-13 12:07 ` [PATCH 1/2] drm/i915: Extend GET_APERTURE ioctl to report available map space ankitprasad.r.sharma

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