* [PATCH v2] drm/framebuffer: Add framebuffer debugfs file
@ 2017-10-25 12:05 Noralf Trønnes
2017-10-25 13:55 ` Ville Syrjälä
0 siblings, 1 reply; 5+ messages in thread
From: Noralf Trønnes @ 2017-10-25 12:05 UTC (permalink / raw)
To: dri-devel; +Cc: hoegsberg
Add debugfs file that dumps info about the framebuffers and its planes.
Also dump info about any connected gem object(s).
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
Changes since version 1:
- Remove const in drm_gem_print() argument (kbuild test robot)
- Print framebuffer modifiers (Kristian Høgsberg)
drivers/gpu/drm/drm_debugfs.c | 6 +++++
drivers/gpu/drm/drm_framebuffer.c | 52 +++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_gem.c | 11 +++++++++
drivers/gpu/drm/drm_internal.h | 4 +++
4 files changed, 73 insertions(+)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index c1807d5754b2..550f29de6c1f 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -158,6 +158,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
}
}
+ ret = drm_framebuffer_debugfs_init(minor);
+ if (ret) {
+ DRM_ERROR("Failed to create framebuffer debugfs file\n");
+ return ret;
+ }
+
if (dev->driver->debugfs_init) {
ret = dev->driver->debugfs_init(minor);
if (ret) {
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index af279844d7ce..86a6b5f61411 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -25,7 +25,9 @@
#include <drm/drm_auth.h>
#include <drm/drm_framebuffer.h>
#include <drm/drm_atomic.h>
+#include <drm/drm_print.h>
+#include "drm_internal.h"
#include "drm_crtc_internal.h"
/**
@@ -955,3 +957,53 @@ int drm_framebuffer_plane_height(int height,
return fb_plane_height(height, fb->format, plane);
}
EXPORT_SYMBOL(drm_framebuffer_plane_height);
+
+#ifdef CONFIG_DEBUG_FS
+static void drm_framebuffer_print(struct drm_framebuffer *fb,
+ struct drm_printer *p)
+{
+ unsigned int i;
+
+ drm_printf(p, "[FB:%d] %dx%d@%4.4s modifier=0x%llx refcount=%d\n",
+ fb->base.id, fb->width, fb->height,
+ (char *)&fb->format->format, fb->modifier,
+ drm_framebuffer_read_refcount(fb));
+
+ for (i = 0; i < fb->format->num_planes; i++) {
+ drm_printf(p, "\t%u: offset=%d pitch=%d",
+ i, fb->offsets[i], fb->pitches[i]);
+ if (fb->obj[i]) {
+ drm_printf(p, " GEM: ");
+ drm_gem_print(fb->obj[i], p);
+ } else {
+ drm_printf(p, "\n");
+ }
+ }
+}
+
+static int drm_framebuffer_info(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = m->private;
+ struct drm_device *dev = node->minor->dev;
+ struct drm_printer p = drm_seq_file_printer(m);
+ struct drm_framebuffer *fb;
+
+ mutex_lock(&dev->mode_config.fb_lock);
+ drm_for_each_fb(fb, dev)
+ drm_framebuffer_print(fb, &p);
+ mutex_unlock(&dev->mode_config.fb_lock);
+
+ return 0;
+}
+
+static const struct drm_info_list drm_framebuffer_debugfs_list[] = {
+ { "framebuffer", drm_framebuffer_info, 0 },
+};
+
+int drm_framebuffer_debugfs_init(struct drm_minor *minor)
+{
+ return drm_debugfs_create_files(drm_framebuffer_debugfs_list,
+ ARRAY_SIZE(drm_framebuffer_debugfs_list),
+ minor->debugfs_root, minor);
+}
+#endif
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 55d6182555c7..3d6ff9417e51 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -40,6 +40,7 @@
#include <drm/drmP.h>
#include <drm/drm_vma_manager.h>
#include <drm/drm_gem.h>
+#include <drm/drm_print.h>
#include "drm_internal.h"
/** @file drm_gem.c
@@ -1040,3 +1041,13 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
return ret;
}
EXPORT_SYMBOL(drm_gem_mmap);
+
+#ifdef CONFIG_DEBUG_FS
+void drm_gem_print(struct drm_gem_object *obj, struct drm_printer *p)
+{
+ drm_printf(p, "name=%d refcount=%d start=%08lx size=%zu%s\n",
+ obj->name, kref_read(&obj->refcount),
+ drm_vma_node_start(&obj->vma_node), obj->size,
+ obj->import_attach ? " (imported)" : "");
+}
+#endif
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index fbc3f308fa19..0f28be586cb3 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -106,6 +106,7 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
+void drm_gem_print(struct drm_gem_object *obj, struct drm_printer *p);
/* drm_debugfs.c drm_debugfs_crc.c */
#if defined(CONFIG_DEBUG_FS)
@@ -173,3 +174,6 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
+
+/* drm_framebuffer.c */
+int drm_framebuffer_debugfs_init(struct drm_minor *minor);
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/framebuffer: Add framebuffer debugfs file
2017-10-25 12:05 [PATCH v2] drm/framebuffer: Add framebuffer debugfs file Noralf Trønnes
@ 2017-10-25 13:55 ` Ville Syrjälä
2017-10-25 15:22 ` Noralf Trønnes
0 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2017-10-25 13:55 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: hoegsberg, dri-devel
On Wed, Oct 25, 2017 at 02:05:02PM +0200, Noralf Trønnes wrote:
> Add debugfs file that dumps info about the framebuffers and its planes.
> Also dump info about any connected gem object(s).
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>
> Changes since version 1:
> - Remove const in drm_gem_print() argument (kbuild test robot)
> - Print framebuffer modifiers (Kristian Høgsberg)
>
> drivers/gpu/drm/drm_debugfs.c | 6 +++++
> drivers/gpu/drm/drm_framebuffer.c | 52 +++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_gem.c | 11 +++++++++
> drivers/gpu/drm/drm_internal.h | 4 +++
> 4 files changed, 73 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index c1807d5754b2..550f29de6c1f 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -158,6 +158,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> }
> }
>
> + ret = drm_framebuffer_debugfs_init(minor);
> + if (ret) {
> + DRM_ERROR("Failed to create framebuffer debugfs file\n");
> + return ret;
> + }
> +
> if (dev->driver->debugfs_init) {
> ret = dev->driver->debugfs_init(minor);
> if (ret) {
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index af279844d7ce..86a6b5f61411 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -25,7 +25,9 @@
> #include <drm/drm_auth.h>
> #include <drm/drm_framebuffer.h>
> #include <drm/drm_atomic.h>
> +#include <drm/drm_print.h>
>
> +#include "drm_internal.h"
> #include "drm_crtc_internal.h"
>
> /**
> @@ -955,3 +957,53 @@ int drm_framebuffer_plane_height(int height,
> return fb_plane_height(height, fb->format, plane);
> }
> EXPORT_SYMBOL(drm_framebuffer_plane_height);
> +
> +#ifdef CONFIG_DEBUG_FS
> +static void drm_framebuffer_print(struct drm_framebuffer *fb,
Can be const
> + struct drm_printer *p)
> +{
> + unsigned int i;
int
> +
> + drm_printf(p, "[FB:%d] %dx%d@%4.4s modifier=0x%llx refcount=%d\n",
> + fb->base.id, fb->width, fb->height,
> + (char *)&fb->format->format, fb->modifier,
drm_format_get_name(), or whatver it was called.
For a bit of extra niceness you could print the dimensions of each
plane rather than just the whole fb.
> + drm_framebuffer_read_refcount(fb));
> +
> + for (i = 0; i < fb->format->num_planes; i++) {
> + drm_printf(p, "\t%u: offset=%d pitch=%d",
> + i, fb->offsets[i], fb->pitches[i]);
> + if (fb->obj[i]) {
Maybe print the handle as well. If ->obj[] is not there it might be
to nice to have something to indicate which bo is used. I wonder if I
should actually make i915 use ->obj[] instead of storing the obj pointer
in struct intel_framebuffer...
That highlights a slight weakness in this apporach. Drivers can extend
drm_framebuffer and drm_gem_object and this of course can't print out
any extened information. But I guess we can worry about that later if
needed.
> + drm_printf(p, " GEM: ");
> + drm_gem_print(fb->obj[i], p);
> + } else {
> + drm_printf(p, "\n");
> + }
> + }
> +}
> +
> +static int drm_framebuffer_info(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = m->private;
> + struct drm_device *dev = node->minor->dev;
> + struct drm_printer p = drm_seq_file_printer(m);
> + struct drm_framebuffer *fb;
> +
> + mutex_lock(&dev->mode_config.fb_lock);
> + drm_for_each_fb(fb, dev)
> + drm_framebuffer_print(fb, &p);
> + mutex_unlock(&dev->mode_config.fb_lock);
> +
> + return 0;
> +}
> +
> +static const struct drm_info_list drm_framebuffer_debugfs_list[] = {
> + { "framebuffer", drm_framebuffer_info, 0 },
> +};
> +
> +int drm_framebuffer_debugfs_init(struct drm_minor *minor)
> +{
> + return drm_debugfs_create_files(drm_framebuffer_debugfs_list,
> + ARRAY_SIZE(drm_framebuffer_debugfs_list),
> + minor->debugfs_root, minor);
> +}
> +#endif
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 55d6182555c7..3d6ff9417e51 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -40,6 +40,7 @@
> #include <drm/drmP.h>
> #include <drm/drm_vma_manager.h>
> #include <drm/drm_gem.h>
> +#include <drm/drm_print.h>
> #include "drm_internal.h"
>
> /** @file drm_gem.c
> @@ -1040,3 +1041,13 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> return ret;
> }
> EXPORT_SYMBOL(drm_gem_mmap);
> +
> +#ifdef CONFIG_DEBUG_FS
> +void drm_gem_print(struct drm_gem_object *obj, struct drm_printer *p)
> +{
> + drm_printf(p, "name=%d refcount=%d start=%08lx size=%zu%s\n",
> + obj->name, kref_read(&obj->refcount),
> + drm_vma_node_start(&obj->vma_node), obj->size,
> + obj->import_attach ? " (imported)" : "");
> +}
> +#endif
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index fbc3f308fa19..0f28be586cb3 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -106,6 +106,7 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
> void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
> +void drm_gem_print(struct drm_gem_object *obj, struct drm_printer *p);
>
> /* drm_debugfs.c drm_debugfs_crc.c */
> #if defined(CONFIG_DEBUG_FS)
> @@ -173,3 +174,6 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_private);
> int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_private);
> +
> +/* drm_framebuffer.c */
> +int drm_framebuffer_debugfs_init(struct drm_minor *minor);
> --
> 2.14.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/framebuffer: Add framebuffer debugfs file
2017-10-25 13:55 ` Ville Syrjälä
@ 2017-10-25 15:22 ` Noralf Trønnes
2017-10-25 16:18 ` Ville Syrjälä
0 siblings, 1 reply; 5+ messages in thread
From: Noralf Trønnes @ 2017-10-25 15:22 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: hoegsberg, dri-devel
Den 25.10.2017 15.55, skrev Ville Syrjälä:
> On Wed, Oct 25, 2017 at 02:05:02PM +0200, Noralf Trønnes wrote:
>> Add debugfs file that dumps info about the framebuffers and its planes.
>> Also dump info about any connected gem object(s).
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>
>> Changes since version 1:
>> - Remove const in drm_gem_print() argument (kbuild test robot)
>> - Print framebuffer modifiers (Kristian Høgsberg)
>>
>> drivers/gpu/drm/drm_debugfs.c | 6 +++++
>> drivers/gpu/drm/drm_framebuffer.c | 52 +++++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/drm_gem.c | 11 +++++++++
>> drivers/gpu/drm/drm_internal.h | 4 +++
>> 4 files changed, 73 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
>> index c1807d5754b2..550f29de6c1f 100644
>> --- a/drivers/gpu/drm/drm_debugfs.c
>> +++ b/drivers/gpu/drm/drm_debugfs.c
>> @@ -158,6 +158,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>> }
>> }
>>
>> + ret = drm_framebuffer_debugfs_init(minor);
>> + if (ret) {
>> + DRM_ERROR("Failed to create framebuffer debugfs file\n");
>> + return ret;
>> + }
>> +
>> if (dev->driver->debugfs_init) {
>> ret = dev->driver->debugfs_init(minor);
>> if (ret) {
>> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
>> index af279844d7ce..86a6b5f61411 100644
>> --- a/drivers/gpu/drm/drm_framebuffer.c
>> +++ b/drivers/gpu/drm/drm_framebuffer.c
>> @@ -25,7 +25,9 @@
>> #include <drm/drm_auth.h>
>> #include <drm/drm_framebuffer.h>
>> #include <drm/drm_atomic.h>
>> +#include <drm/drm_print.h>
>>
>> +#include "drm_internal.h"
>> #include "drm_crtc_internal.h"
>>
>> /**
>> @@ -955,3 +957,53 @@ int drm_framebuffer_plane_height(int height,
>> return fb_plane_height(height, fb->format, plane);
>> }
>> EXPORT_SYMBOL(drm_framebuffer_plane_height);
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +static void drm_framebuffer_print(struct drm_framebuffer *fb,
> Can be const
I call drm_framebuffer_read_refcount() and it doesn't like const.
>> + struct drm_printer *p)
>> +{
>> + unsigned int i;
> int
>
>> +
>> + drm_printf(p, "[FB:%d] %dx%d@%4.4s modifier=0x%llx refcount=%d\n",
>> + fb->base.id, fb->width, fb->height,
>> + (char *)&fb->format->format, fb->modifier,
> drm_format_get_name(), or whatver it was called.
>
> For a bit of extra niceness you could print the dimensions of each
> plane rather than just the whole fb.
Where do I find that info?
>
>> + drm_framebuffer_read_refcount(fb));
>> +
>> + for (i = 0; i < fb->format->num_planes; i++) {
>> + drm_printf(p, "\t%u: offset=%d pitch=%d",
>> + i, fb->offsets[i], fb->pitches[i]);
>> + if (fb->obj[i]) {
> Maybe print the handle as well.
Where do I find that info?
> If ->obj[] is not there it might be
> to nice to have something to indicate which bo is used. I wonder if I
> should actually make i915 use ->obj[] instead of storing the obj pointer
> in struct intel_framebuffer...
>
> That highlights a slight weakness in this apporach. Drivers can extend
> drm_framebuffer and drm_gem_object and this of course can't print out
> any extened information. But I guess we can worry about that later if
> needed.
The idea was to add it later on demand as you say, but I can add it now
if you want it:
struct drm_driver {
/**
* @gem_print_info:
*
* If driver subclasses struct &drm_gem_object, it can implement this
* optional hook for printing additional driver specific info.
* See drm_gem_print_info().
*/
void (*gem_print_info)(struct drm_printer *p,
struct drm_gem_object *obj);
};
struct drm_framebuffer_funcs {
/**
* @print_info:
*
* If driver subclasses struct &drm_framebuffer, it can implement this
* optional hook for printing additional driver specific info.
* See drm_framebuffer_print_info().
*/
void (*print_info)(struct drm_printer *p, struct drm_framebuffer *obj);
};
Noralf.
>> + drm_printf(p, " GEM: ");
>> + drm_gem_print(fb->obj[i], p);
>> + } else {
>> + drm_printf(p, "\n");
>> + }
>> + }
>> +}
>> +
>> +static int drm_framebuffer_info(struct seq_file *m, void *data)
>> +{
>> + struct drm_info_node *node = m->private;
>> + struct drm_device *dev = node->minor->dev;
>> + struct drm_printer p = drm_seq_file_printer(m);
>> + struct drm_framebuffer *fb;
>> +
>> + mutex_lock(&dev->mode_config.fb_lock);
>> + drm_for_each_fb(fb, dev)
>> + drm_framebuffer_print(fb, &p);
>> + mutex_unlock(&dev->mode_config.fb_lock);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct drm_info_list drm_framebuffer_debugfs_list[] = {
>> + { "framebuffer", drm_framebuffer_info, 0 },
>> +};
>> +
>> +int drm_framebuffer_debugfs_init(struct drm_minor *minor)
>> +{
>> + return drm_debugfs_create_files(drm_framebuffer_debugfs_list,
>> + ARRAY_SIZE(drm_framebuffer_debugfs_list),
>> + minor->debugfs_root, minor);
>> +}
>> +#endif
>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> index 55d6182555c7..3d6ff9417e51 100644
>> --- a/drivers/gpu/drm/drm_gem.c
>> +++ b/drivers/gpu/drm/drm_gem.c
>> @@ -40,6 +40,7 @@
>> #include <drm/drmP.h>
>> #include <drm/drm_vma_manager.h>
>> #include <drm/drm_gem.h>
>> +#include <drm/drm_print.h>
>> #include "drm_internal.h"
>>
>> /** @file drm_gem.c
>> @@ -1040,3 +1041,13 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>> return ret;
>> }
>> EXPORT_SYMBOL(drm_gem_mmap);
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +void drm_gem_print(struct drm_gem_object *obj, struct drm_printer *p)
>> +{
>> + drm_printf(p, "name=%d refcount=%d start=%08lx size=%zu%s\n",
>> + obj->name, kref_read(&obj->refcount),
>> + drm_vma_node_start(&obj->vma_node), obj->size,
>> + obj->import_attach ? " (imported)" : "");
>> +}
>> +#endif
>> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
>> index fbc3f308fa19..0f28be586cb3 100644
>> --- a/drivers/gpu/drm/drm_internal.h
>> +++ b/drivers/gpu/drm/drm_internal.h
>> @@ -106,6 +106,7 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *file_priv);
>> void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
>> void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
>> +void drm_gem_print(struct drm_gem_object *obj, struct drm_printer *p);
>>
>> /* drm_debugfs.c drm_debugfs_crc.c */
>> #if defined(CONFIG_DEBUG_FS)
>> @@ -173,3 +174,6 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *file_private);
>> int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
>> struct drm_file *file_private);
>> +
>> +/* drm_framebuffer.c */
>> +int drm_framebuffer_debugfs_init(struct drm_minor *minor);
>> --
>> 2.14.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/framebuffer: Add framebuffer debugfs file
2017-10-25 15:22 ` Noralf Trønnes
@ 2017-10-25 16:18 ` Ville Syrjälä
2017-10-30 10:16 ` Daniel Vetter
0 siblings, 1 reply; 5+ messages in thread
From: Ville Syrjälä @ 2017-10-25 16:18 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: hoegsberg, dri-devel
On Wed, Oct 25, 2017 at 05:22:25PM +0200, Noralf Trønnes wrote:
>
> Den 25.10.2017 15.55, skrev Ville Syrjälä:
> > On Wed, Oct 25, 2017 at 02:05:02PM +0200, Noralf Trønnes wrote:
> >> Add debugfs file that dumps info about the framebuffers and its planes.
> >> Also dump info about any connected gem object(s).
> >>
> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >> ---
> >>
> >> Changes since version 1:
> >> - Remove const in drm_gem_print() argument (kbuild test robot)
> >> - Print framebuffer modifiers (Kristian Høgsberg)
> >>
> >> drivers/gpu/drm/drm_debugfs.c | 6 +++++
> >> drivers/gpu/drm/drm_framebuffer.c | 52 +++++++++++++++++++++++++++++++++++++++
> >> drivers/gpu/drm/drm_gem.c | 11 +++++++++
> >> drivers/gpu/drm/drm_internal.h | 4 +++
> >> 4 files changed, 73 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> >> index c1807d5754b2..550f29de6c1f 100644
> >> --- a/drivers/gpu/drm/drm_debugfs.c
> >> +++ b/drivers/gpu/drm/drm_debugfs.c
> >> @@ -158,6 +158,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> >> }
> >> }
> >>
> >> + ret = drm_framebuffer_debugfs_init(minor);
> >> + if (ret) {
> >> + DRM_ERROR("Failed to create framebuffer debugfs file\n");
> >> + return ret;
> >> + }
> >> +
> >> if (dev->driver->debugfs_init) {
> >> ret = dev->driver->debugfs_init(minor);
> >> if (ret) {
> >> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> >> index af279844d7ce..86a6b5f61411 100644
> >> --- a/drivers/gpu/drm/drm_framebuffer.c
> >> +++ b/drivers/gpu/drm/drm_framebuffer.c
> >> @@ -25,7 +25,9 @@
> >> #include <drm/drm_auth.h>
> >> #include <drm/drm_framebuffer.h>
> >> #include <drm/drm_atomic.h>
> >> +#include <drm/drm_print.h>
> >>
> >> +#include "drm_internal.h"
> >> #include "drm_crtc_internal.h"
> >>
> >> /**
> >> @@ -955,3 +957,53 @@ int drm_framebuffer_plane_height(int height,
> >> return fb_plane_height(height, fb->format, plane);
> >> }
> >> EXPORT_SYMBOL(drm_framebuffer_plane_height);
> >> +
> >> +#ifdef CONFIG_DEBUG_FS
> >> +static void drm_framebuffer_print(struct drm_framebuffer *fb,
> > Can be const
>
> I call drm_framebuffer_read_refcount() and it doesn't like const.
Fix it first then?
>
> >> + struct drm_printer *p)
> >> +{
> >> + unsigned int i;
> > int
> >
> >> +
> >> + drm_printf(p, "[FB:%d] %dx%d@%4.4s modifier=0x%llx refcount=%d\n",
> >> + fb->base.id, fb->width, fb->height,
> >> + (char *)&fb->format->format, fb->modifier,
> > drm_format_get_name(), or whatver it was called.
> >
> > For a bit of extra niceness you could print the dimensions of each
> > plane rather than just the whole fb.
>
> Where do I find that info?
drm_framebuffer_plane_width/height() or something like that.
>
> >
> >> + drm_framebuffer_read_refcount(fb));
> >> +
> >> + for (i = 0; i < fb->format->num_planes; i++) {
> >> + drm_printf(p, "\t%u: offset=%d pitch=%d",
> >> + i, fb->offsets[i], fb->pitches[i]);
> >> + if (fb->obj[i]) {
> > Maybe print the handle as well.
>
> Where do I find that info?
Hmm. Oh, we don't bring that over into drm_framebuffer. Ignore what I
wrote then :)
>
> > If ->obj[] is not there it might be
> > to nice to have something to indicate which bo is used. I wonder if I
> > should actually make i915 use ->obj[] instead of storing the obj pointer
> > in struct intel_framebuffer...
> >
> > That highlights a slight weakness in this apporach. Drivers can extend
> > drm_framebuffer and drm_gem_object and this of course can't print out
> > any extened information. But I guess we can worry about that later if
> > needed.
>
> The idea was to add it later on demand as you say, but I can add it now
> if you want it:
>
> struct drm_driver {
> /**
> * @gem_print_info:
> *
> * If driver subclasses struct &drm_gem_object, it can implement this
> * optional hook for printing additional driver specific info.
> * See drm_gem_print_info().
> */
> void (*gem_print_info)(struct drm_printer *p,
> struct drm_gem_object *obj);
> };
>
> struct drm_framebuffer_funcs {
> /**
> * @print_info:
> *
> * If driver subclasses struct &drm_framebuffer, it can implement this
> * optional hook for printing additional driver specific info.
> * See drm_framebuffer_print_info().
> */
> void (*print_info)(struct drm_printer *p, struct drm_framebuffer *obj);
> };
We'd also need an actual user for those new hooks, so that'd mean changing
at least one driver. I guess it's good to get the basic stuff in first,
and we can worry about extending it later.
It also occurs to me that we might want to combine this fb dumper with
the atomic state dumper. That one already prints out stuf about
framebuffers in drm_atomic_plane_print_state(). Not sure if there
would be a nice way to keep the extra indentation with shared code
though. I guess one could always pass on some kind of prefix string to
thee fb dump function.
>
> Noralf.
>
> >> + drm_printf(p, " GEM: ");
> >> + drm_gem_print(fb->obj[i], p);
> >> + } else {
> >> + drm_printf(p, "\n");
> >> + }
> >> + }
> >> +}
> >> +
> >> +static int drm_framebuffer_info(struct seq_file *m, void *data)
> >> +{
> >> + struct drm_info_node *node = m->private;
> >> + struct drm_device *dev = node->minor->dev;
> >> + struct drm_printer p = drm_seq_file_printer(m);
> >> + struct drm_framebuffer *fb;
> >> +
> >> + mutex_lock(&dev->mode_config.fb_lock);
> >> + drm_for_each_fb(fb, dev)
> >> + drm_framebuffer_print(fb, &p);
> >> + mutex_unlock(&dev->mode_config.fb_lock);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct drm_info_list drm_framebuffer_debugfs_list[] = {
> >> + { "framebuffer", drm_framebuffer_info, 0 },
> >> +};
> >> +
> >> +int drm_framebuffer_debugfs_init(struct drm_minor *minor)
> >> +{
> >> + return drm_debugfs_create_files(drm_framebuffer_debugfs_list,
> >> + ARRAY_SIZE(drm_framebuffer_debugfs_list),
> >> + minor->debugfs_root, minor);
> >> +}
> >> +#endif
> >> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> >> index 55d6182555c7..3d6ff9417e51 100644
> >> --- a/drivers/gpu/drm/drm_gem.c
> >> +++ b/drivers/gpu/drm/drm_gem.c
> >> @@ -40,6 +40,7 @@
> >> #include <drm/drmP.h>
> >> #include <drm/drm_vma_manager.h>
> >> #include <drm/drm_gem.h>
> >> +#include <drm/drm_print.h>
> >> #include "drm_internal.h"
> >>
> >> /** @file drm_gem.c
> >> @@ -1040,3 +1041,13 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> >> return ret;
> >> }
> >> EXPORT_SYMBOL(drm_gem_mmap);
> >> +
> >> +#ifdef CONFIG_DEBUG_FS
> >> +void drm_gem_print(struct drm_gem_object *obj, struct drm_printer *p)
> >> +{
> >> + drm_printf(p, "name=%d refcount=%d start=%08lx size=%zu%s\n",
> >> + obj->name, kref_read(&obj->refcount),
> >> + drm_vma_node_start(&obj->vma_node), obj->size,
> >> + obj->import_attach ? " (imported)" : "");
> >> +}
> >> +#endif
> >> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> >> index fbc3f308fa19..0f28be586cb3 100644
> >> --- a/drivers/gpu/drm/drm_internal.h
> >> +++ b/drivers/gpu/drm/drm_internal.h
> >> @@ -106,6 +106,7 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
> >> struct drm_file *file_priv);
> >> void drm_gem_open(struct drm_device *dev, struct drm_file *file_private);
> >> void drm_gem_release(struct drm_device *dev, struct drm_file *file_private);
> >> +void drm_gem_print(struct drm_gem_object *obj, struct drm_printer *p);
> >>
> >> /* drm_debugfs.c drm_debugfs_crc.c */
> >> #if defined(CONFIG_DEBUG_FS)
> >> @@ -173,3 +174,6 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
> >> struct drm_file *file_private);
> >> int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
> >> struct drm_file *file_private);
> >> +
> >> +/* drm_framebuffer.c */
> >> +int drm_framebuffer_debugfs_init(struct drm_minor *minor);
> >> --
> >> 2.14.2
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/framebuffer: Add framebuffer debugfs file
2017-10-25 16:18 ` Ville Syrjälä
@ 2017-10-30 10:16 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2017-10-30 10:16 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: hoegsberg, dri-devel
On Wed, Oct 25, 2017 at 07:18:28PM +0300, Ville Syrjälä wrote:
> It also occurs to me that we might want to combine this fb dumper with
> the atomic state dumper. That one already prints out stuf about
> framebuffers in drm_atomic_plane_print_state(). Not sure if there
> would be a nice way to keep the extra indentation with shared code
> though. I guess one could always pass on some kind of prefix string to
> thee fb dump function.
Yeah dumping the full details with the same function introduced in this
patch here might be useful there too. Since we just pass around seq_file
it should be all cool (well probably should pass around drm_printer if we
don't do that already).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-30 10:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-25 12:05 [PATCH v2] drm/framebuffer: Add framebuffer debugfs file Noralf Trønnes
2017-10-25 13:55 ` Ville Syrjälä
2017-10-25 15:22 ` Noralf Trønnes
2017-10-25 16:18 ` Ville Syrjälä
2017-10-30 10:16 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.