Message ID | 1432720354-20230-2-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On ke, 2015-05-27 at 10:52 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Makes it a little bit more debug friendly not having to > remember which number is what. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@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? > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index 0d46dd2..13442e0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -119,6 +119,7 @@ enum i915_ggtt_view_type { > I915_GGTT_VIEW_NORMAL = 0, > I915_GGTT_VIEW_ROTATED, > I915_GGTT_VIEW_PARTIAL, > + I915_GGTT_VIEW_LAST /* Do not use and keep last. */ > }; > > struct intel_rotation_info { > @@ -514,4 +515,6 @@ size_t > i915_ggtt_view_size(struct drm_i915_gem_object *obj, > const struct i915_ggtt_view *view); > > +const char *i915_ggtt_view_name(enum i915_ggtt_view_type); > + > #endif
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.ursulin@intel.com> >> >> Makes it a little bit more debug friendly not having to >> remember which number is what. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@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
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]; +} diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 0d46dd2..13442e0 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -119,6 +119,7 @@ enum i915_ggtt_view_type { I915_GGTT_VIEW_NORMAL = 0, I915_GGTT_VIEW_ROTATED, I915_GGTT_VIEW_PARTIAL, + I915_GGTT_VIEW_LAST /* Do not use and keep last. */ }; struct intel_rotation_info { @@ -514,4 +515,6 @@ size_t i915_ggtt_view_size(struct drm_i915_gem_object *obj, const struct i915_ggtt_view *view); +const char *i915_ggtt_view_name(enum i915_ggtt_view_type); + #endif