Re: [Intel-gfx] [PATCH 2/3] drm/i915: Show human readable GGTT view name in debugfs

[Tvrtko Ursulin]

On 05/28/2015 12:11 PM, Joonas Lahtinen wrote:
On ke, 2015-05-27 at 10:52 +0100, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Makes it a little bit more debug friendly not having to
remember which number is what.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
---
  drivers/gpu/drm/i915/i915_debugfs.c | 11 ++++++-----
  drivers/gpu/drm/i915/i915_gem_gtt.c | 11 +++++++++++
  drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +++
  3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 9d36be8..e26799c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -156,13 +156,14 @@ describe_obj(struct seq_file *m, struct 
drm_i915_gem_object *obj)
        if (obj->fence_reg != I915_FENCE_REG_NONE)
                seq_printf(m, " (fence: %d)", obj->fence_reg);
        list_for_each_entry(vma, &obj->vma_list, vma_link) {
-               seq_printf(m, " (%sgtt offset: %08llx, size: %08llx",
-                          i915_is_ggtt(vma->vm) ? "g" : "pp",
-                          vma->node.start, vma->node.size);
                if (i915_is_ggtt(vma->vm))
-                       seq_printf(m, ", type: %u)", vma->ggtt_view.type);
+                       seq_printf(m, " (ggtt %s",
+                                  i915_ggtt_view_name(vma->ggtt_view.type));
                else
-                       seq_puts(m, ")");
+                       seq_puts(m, " (ppgtt");
+
+               seq_printf(m, " offset: %08llx, size: %08llx)",
+                          vma->node.start, vma->node.size);
        }
        if (obj->stolen)
                seq_printf(m, " (stolen: %08llx)", obj->stolen->start);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 17b7df0..e330e6c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2904,3 +2904,14 @@ i915_ggtt_view_size(struct drm_i915_gem_object *obj,
                return obj->base.size;
        }
  }
+
+const char *i915_ggtt_view_name(enum i915_ggtt_view_type type)
+{
+       static const char *names[3] = { "normal",
+                                       "rotated",
+                                       "partial" };
+
+       BUILD_BUG_ON(ARRAY_SIZE(names) != I915_GGTT_VIEW_LAST);
+
+       return names[type];
+}

I think this is bit of an overkill thing to do, especially as this if
for debugging. Looking at the DRM code elsewhere, an array of
i915_ggtt_view_type_names should suffice?

I just thought it is a handy way of keeping the BUILD_BUG_ON close to 
the array. One thing less scattered around since enum and and string 
array is unavoidable.

Regards,

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