[PATCH v4 04/11] drm/framebuffer: Add framebuffer debugfs file
Noralf Trønnes
noralf at tronnes.org
Thu Nov 2 14:12:14 UTC 2017
Hi Laurent,
Thank you for the review.
Den 02.11.2017 05.43, skrev Laurent Pinchart:
> Hi Noralf,
>
> Thank you for the patch.
>
> On Monday, 30 October 2017 18:29:38 EET 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).
>>
>> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
>> ---
>> drivers/gpu/drm/drm_debugfs.c | 6 ++++
>> drivers/gpu/drm/drm_framebuffer.c | 59 ++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/drm_gem.c | 17 +++++++++++
>> drivers/gpu/drm/drm_internal.h | 7 +++++
>> include/drm/drm_drv.h | 11 ++++++++
>> 5 files changed, 100 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..4f7873a1b922 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,60 @@ int drm_framebuffer_plane_height(int height,
>> return fb_plane_height(height, fb->format, plane);
>> }
>> EXPORT_SYMBOL(drm_framebuffer_plane_height);
>> +
>> +void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
>> + const struct drm_framebuffer *fb)
>> +{
>> + struct drm_format_name_buf format_name;
>> + int i;
> i is never negative, you can make it an unsigned int.
Yeah, I did that in a previous version, but Ville wanted it to be int.
Personally I don't really care as long as I remember to use int when
I have a loop that counts down to zero, which has bitten me before.
So based on my experience maybe I should always use plain int in loop
indices, too avoid that rare count down case where I continue to use
unsigned int for the loop index, me being a limited human being :-)
Noralf.
>> + drm_printf_indent(p, indent, "refcount=%d\n",
>> + drm_framebuffer_read_refcount(fb));
> drm_framebuffer_read_refcount() returns an unsigned int, the printk format
> should be %u.
>
>> + drm_printf_indent(p, indent, "format=%s\n",
>> + drm_get_format_name(fb->format->format, &format_name));
>> + drm_printf_indent(p, indent, "modifier=0x%llx\n", fb->modifier);
>> + drm_printf_indent(p, indent, "size=%dx%d\n", fb->width, fb->height);
> Ditto here, %ux%u.
>
>> + drm_printf_indent(p, indent, "layers:\n");
>> +
>> + for (i = 0; i < fb->format->num_planes; i++) {
>> + drm_printf_indent(p, indent + 1, "size[%d]=%dx%d\n", i,
>> + drm_framebuffer_plane_width(fb->width, fb, i),
>> + drm_framebuffer_plane_height(fb->height, fb, i));
>> + drm_printf_indent(p, indent + 1, "pitch[%d]=%u\n", i, fb->pitches[i]);
>> + drm_printf_indent(p, indent + 1, "offset[%d]=%u\n", i, fb->offsets[i]);
>> + drm_printf_indent(p, indent + 1, "obj[%d]:%s\n", i,
>> + fb->obj[i] ? "" : "(null)");
> And in various places here too.
>
>> + if (fb->obj[i])
>> + drm_gem_print_info(p, indent + 2, fb->obj[i]);
>> + }
>> +}
>> +
>> +#ifdef CONFIG_DEBUG_FS
>> +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_printf(&p, "framebuffer[%d]:\n", fb->base.id);
> fb->base.id is an unsigned int, the printk format should be %u.
>
>> + drm_framebuffer_print_info(&p, 1, fb);
>> + }
>> + 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 4c84b23d37cc..152bd6210dde 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,19 @@ int drm_gem_mmap(struct file *filp, struct
>> vm_area_struct *vma) return ret;
>> }
>> EXPORT_SYMBOL(drm_gem_mmap);
>> +
>> +void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
>> + const struct drm_gem_object *obj)
>> +{
>> + drm_printf_indent(p, indent, "name=%d\n", obj->name);
>> + drm_printf_indent(p, indent, "refcount=%d\n",
>> + kref_read(&obj->refcount));
> kref_read() returns an unsigned int, the printk format should be %u.
>
>> + drm_printf_indent(p, indent, "start=%08lx\n",
>> + drm_vma_node_start(&obj->vma_node));
>> + drm_printf_indent(p, indent, "size=%zu\n", obj->size);
>> + drm_printf_indent(p, indent, "imported=%s\n",
>> + obj->import_attach ? "yes" : "no");
>> +
>> + if (obj->dev->driver->gem_print_info)
>> + obj->dev->driver->gem_print_info(p, indent, obj);
>> +}
>> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
>> index fbc3f308fa19..430ce3fe4f3b 100644
>> --- a/drivers/gpu/drm/drm_internal.h
>> +++ b/drivers/gpu/drm/drm_internal.h
>> @@ -106,6 +106,8 @@ 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_info(struct drm_printer *p, unsigned int indent,
>> + const struct drm_gem_object *obj);
>>
>> /* drm_debugfs.c drm_debugfs_crc.c */
>> #if defined(CONFIG_DEBUG_FS)
>> @@ -173,3 +175,8 @@ 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 */
>> +void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
>> + const struct drm_framebuffer *fb);
>> +int drm_framebuffer_debugfs_init(struct drm_minor *minor);
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index 0e90ef24214b..434de999c3ba 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -39,6 +39,7 @@ struct drm_minor;
>> struct dma_buf_attachment;
>> struct drm_display_mode;
>> struct drm_mode_create_dumb;
>> +struct drm_printer;
>>
>> /* driver capabilities and requirements mask */
>> #define DRIVER_USE_AGP 0x1
>> @@ -428,6 +429,16 @@ struct drm_driver {
>> */
>> void (*gem_close_object) (struct drm_gem_object *, struct drm_file *);
>>
>> + /**
>> + * @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().
> Should you also document the meaning of the indent argument ?
>
>> + */
>> + void (*gem_print_info)(struct drm_printer *p, unsigned int indent,
>> + const struct drm_gem_object *obj);
>> +
>> /**
>> * @gem_create_object: constructor for gem objects
>> *
> With all this fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
More information about the dri-devel
mailing list