[2/3] drm/i915: Show human readable GGTT view name in debugfs
diff mbox

Message ID 1432720354-20230-2-git-send-email-tvrtko.ursulin@linux.intel.com
State New
Headers show

Commit Message

Tvrtko Ursulin May 27, 2015, 9:52 a.m. UTC
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(-)

Comments

Joonas Lahtinen May 28, 2015, 11:11 a.m. UTC | #1
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
Tvrtko Ursulin May 28, 2015, 12:05 p.m. UTC | #2
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

Patch
diff mbox

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