[v4] drm/i915: Report correct GGTT space usage
diff mbox

Message ID 1435674767-6576-1-git-send-email-tvrtko.ursulin@linux.intel.com
State New
Headers show

Commit Message

Tvrtko Ursulin June 30, 2015, 2:32 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Currently only normal views were accounted which under-accounts
the usage as reported in debugfs.

Introduce new helper, i915_gem_obj_total_ggtt_size, and use it
from call sites which want to know how much GGTT space are
objects using.

v2: Single loop in i915_gem_get_aperture_ioctl. (Chris Wilson)

v3: Walk GGTT active/inactive lists in i915_gem_get_aperture_ioctl
    for better efficiency. (Chris Wilson, Daniel Vetter)

v4: Make i915_gem_obj_total_ggtt_size private to debugfs. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 23 +++++++++++++++++++----
 drivers/gpu/drm/i915/i915_gem.c     | 13 ++++++++-----
 2 files changed, 27 insertions(+), 9 deletions(-)

Comments

Chris Wilson June 30, 2015, 3 p.m. UTC | #1
On Tue, Jun 30, 2015 at 03:32:47PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Currently only normal views were accounted which under-accounts
> the usage as reported in debugfs.
> 
> Introduce new helper, i915_gem_obj_total_ggtt_size, and use it
> from call sites which want to know how much GGTT space are
> objects using.
> 
> v2: Single loop in i915_gem_get_aperture_ioctl. (Chris Wilson)
> 
> v3: Walk GGTT active/inactive lists in i915_gem_get_aperture_ioctl
>     for better efficiency. (Chris Wilson, Daniel Vetter)
> 
> v4: Make i915_gem_obj_total_ggtt_size private to debugfs. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

s/unsigned long/u64/
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Tvrtko Ursulin July 1, 2015, 10:53 a.m. UTC | #2
On 06/30/2015 04:00 PM, Chris Wilson wrote:
> On Tue, Jun 30, 2015 at 03:32:47PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Currently only normal views were accounted which under-accounts
>> the usage as reported in debugfs.
>>
>> Introduce new helper, i915_gem_obj_total_ggtt_size, and use it
>> from call sites which want to know how much GGTT space are
>> objects using.
>>
>> v2: Single loop in i915_gem_get_aperture_ioctl. (Chris Wilson)
>>
>> v3: Walk GGTT active/inactive lists in i915_gem_get_aperture_ioctl
>>      for better efficiency. (Chris Wilson, Daniel Vetter)
>>
>> v4: Make i915_gem_obj_total_ggtt_size private to debugfs. (Chris Wilson)
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> s/unsigned long/u64/

It's all pointless until someone makes it all consistent. But done 
anyway. And sent as a new thread so Daniel hopefully notices it more easily.

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

Thanks,

Tvrtko
Chris Wilson July 1, 2015, 10:58 a.m. UTC | #3
On Wed, Jul 01, 2015 at 11:53:43AM +0100, Tvrtko Ursulin wrote:
> 
> On 06/30/2015 04:00 PM, Chris Wilson wrote:
> >On Tue, Jun 30, 2015 at 03:32:47PM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>Currently only normal views were accounted which under-accounts
> >>the usage as reported in debugfs.
> >>
> >>Introduce new helper, i915_gem_obj_total_ggtt_size, and use it
> >>from call sites which want to know how much GGTT space are
> >>objects using.
> >>
> >>v2: Single loop in i915_gem_get_aperture_ioctl. (Chris Wilson)
> >>
> >>v3: Walk GGTT active/inactive lists in i915_gem_get_aperture_ioctl
> >>     for better efficiency. (Chris Wilson, Daniel Vetter)
> >>
> >>v4: Make i915_gem_obj_total_ggtt_size private to debugfs. (Chris Wilson)
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >>Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >>Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> >s/unsigned long/u64/
> 
> It's all pointless until someone makes it all consistent. But done
> anyway. And sent as a new thread so Daniel hopefully notices it more
> easily.

Mika's in the process of being repeatedly poked until the driver is clean. :)
-Chris
Shuang He July 2, 2015, 7:30 a.m. UTC | #4
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6685
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -1              302/302              301/302
SNB                                  312/316              312/316
IVB                                  343/343              343/343
BYT                 -3              287/287              284/287
HSW                                  380/380              380/380
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@gem_persistent_relocs@forked-interruptible-thrashing      PASS(1)      DMESG_WARN(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...bsd_ring_idle@Hangcheck timer elapsed... bsd ring idle
*BYT  igt@gem_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-display      PASS(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 31d87685f3b9..16d92115c3e6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -117,6 +117,21 @@  static inline const char *get_global_flag(struct drm_i915_gem_object *obj)
 	return i915_gem_obj_to_ggtt(obj) ? "g" : " ";
 }
 
+static unsigned long
+i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj)
+{
+	unsigned long size = 0;
+	struct i915_vma *vma;
+
+	list_for_each_entry(vma, &obj->vma_list, vma_link) {
+		if (i915_is_ggtt(vma->vm) &&
+		    drm_mm_node_allocated(&vma->node))
+			size += vma->node.size;
+	}
+
+	return size;
+}
+
 static void
 describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 {
@@ -269,7 +284,7 @@  static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 		list_add(&obj->obj_exec_link, &stolen);
 
 		total_obj_size += obj->base.size;
-		total_gtt_size += i915_gem_obj_ggtt_size(obj);
+		total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
 		count++;
 	}
 	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list) {
@@ -299,7 +314,7 @@  static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 
 #define count_objects(list, member) do { \
 	list_for_each_entry(obj, list, member) { \
-		size += i915_gem_obj_ggtt_size(obj); \
+		size += i915_gem_obj_total_ggtt_size(obj); \
 		++count; \
 		if (obj->map_and_fenceable) { \
 			mappable_size += i915_gem_obj_ggtt_size(obj); \
@@ -405,7 +420,7 @@  static void print_batch_pool_stats(struct seq_file *m,
 
 #define count_vmas(list, member) do { \
 	list_for_each_entry(vma, list, member) { \
-		size += i915_gem_obj_ggtt_size(vma->obj); \
+		size += i915_gem_obj_total_ggtt_size(vma->obj); \
 		++count; \
 		if (vma->obj->map_and_fenceable) { \
 			mappable_size += i915_gem_obj_ggtt_size(vma->obj); \
@@ -535,7 +550,7 @@  static int i915_gem_gtt_info(struct seq_file *m, void *data)
 		describe_obj(m, obj);
 		seq_putc(m, '\n');
 		total_obj_size += obj->base.size;
-		total_gtt_size += i915_gem_obj_ggtt_size(obj);
+		total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
 		count++;
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a2a4a271555c..49016e01da6c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -149,14 +149,18 @@  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;
+	struct i915_gtt *ggtt = &dev_priv->gtt;
+	struct i915_vma *vma;
 	size_t pinned;
 
 	pinned = 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(vma, &ggtt->base.active_list, mm_list)
+		if (vma->pin_count)
+			pinned += vma->node.size;
+	list_for_each_entry(vma, &ggtt->base.inactive_list, mm_list)
+		if (vma->pin_count)
+			pinned += vma->node.size;
 	mutex_unlock(&dev->struct_mutex);
 
 	args->aper_size = dev_priv->gtt.base.total;
@@ -5468,4 +5472,3 @@  bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj)
 
 	return false;
 }
-