| Message ID | 1435656482-12672-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 30, 2015 at 10:28:02AM +0100, Tvrtko Ursulin wrote: > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index a2a4a27..7d69294 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -156,7 +156,7 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, > 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); > + pinned += i915_gem_obj_total_ggtt_size(obj); Please just rewrite this as a single vma walk, or review the patches I sent to do so. -Chris
On 06/30/2015 10:36 AM, Chris Wilson wrote: > On Tue, Jun 30, 2015 at 10:28:02AM +0100, Tvrtko Ursulin wrote: >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index a2a4a27..7d69294 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -156,7 +156,7 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, >> 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); >> + pinned += i915_gem_obj_total_ggtt_size(obj); > > Please just rewrite this as a single vma walk, or review the patches I > sent to do so. Sorry there is a lot of patches floating around and I don't know which ones you refer too. I just noticed debugfs displays wrong values when GGTT views other than normal are present. So just open code the loop in get_aperture you say? It is slightly ugly when there are helpers, but it is also true different callers want to know slightly different things. (Difference between GGTT usage reported in get_aperture vs debugfs - pinned or just allocated.) Regards, Tvrtko
On Tue, Jun 30, 2015 at 10:50:15AM +0100, Tvrtko Ursulin wrote: > > On 06/30/2015 10:36 AM, Chris Wilson wrote: > >On Tue, Jun 30, 2015 at 10:28:02AM +0100, Tvrtko Ursulin wrote: > >>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >>index a2a4a27..7d69294 100644 > >>--- a/drivers/gpu/drm/i915/i915_gem.c > >>+++ b/drivers/gpu/drm/i915/i915_gem.c > >>@@ -156,7 +156,7 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, > >> 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); > >>+ pinned += i915_gem_obj_total_ggtt_size(obj); > > > >Please just rewrite this as a single vma walk, or review the patches I > >sent to do so. > > Sorry there is a lot of patches floating around and I don't know > which ones you refer too. > > I just noticed debugfs displays wrong values when GGTT views other > than normal are present. > > So just open code the loop in get_aperture you say? Yes. > It is slightly ugly when there are helpers, but it is also true > different callers want to know slightly different things. > (Difference between GGTT usage reported in get_aperture vs debugfs - > pinned or just allocated.) I've stopped taking notice of the GGTT usage in the debugfs output, it's archaic and doesn't help me anymore. The only case where it really matters is debugfs/i915_gem_gtt. -Chris
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6676
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 302/302 302/302
SNB 312/316 312/316
IVB 345/345 345/345
BYT -3 287/287 284/287
HSW 382/382 382/382
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*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 '*'
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 31d8768..3bb6c99 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -269,7 +269,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 +299,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 +405,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 +535,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_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ea9caf2..690b541 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3010,6 +3010,9 @@ i915_gem_obj_ggtt_size(struct drm_i915_gem_object *obj) return i915_gem_obj_size(obj, i915_obj_to_ggtt(obj)); } +unsigned long +i915_gem_obj_total_ggtt_size(struct drm_i915_gem_object *obj); + static inline int __must_check i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *obj, uint32_t alignment, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index a2a4a27..7d69294 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -156,7 +156,7 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, 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); + pinned += i915_gem_obj_total_ggtt_size(obj); mutex_unlock(&dev->struct_mutex); args->aper_size = dev_priv->gtt.base.total; @@ -5469,3 +5469,17 @@ bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj) return false; } +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; +}