Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file
On Mon, Nov 13, 2017 at 11:31:28PM +0100, Noralf Trønnes wrote: > > Den 13.11.2017 23.17, skrev Chris Wilson: > > Quoting Noralf Trønnes (2017-11-13 22:12:06) > > > Den 13.11.2017 22.33, skrev Chris Wilson: > > > > Quoting Noralf Trønnes (2017-11-13 19:54:43) > > > > > Den 11.11.2017 19.55, skrev Chris Wilson: > > > > > > Quoting Noralf Trønnes (2017-11-07 19:13:40) > > > > > > > Add debugfs file that dumps info about the framebuffers and its > > > > > > > planes. > > > > > > > Also dump info about any connected gem object(s). > > > > > > This isn't too hot for non-modesetting drm drivers. > > > > Just to be clear; this is currently on fire in i915's CI. As > > > > framebuffer-info is installed for vgem and vgem hasn't initialised any > > > > of the data structs being inspected... > > > Ok, so I have broken vgem :-( > > > > > > drm_framebuffer_info() takes dev->mode_config.fb_lock which hasn't been > > > initialized because vgem doesn't call drm_mode_config_init(). > > > drm_mode_config_init() in addition to initializing various thing also > > > calls drm_mode_create_standard_properties(). > > > > > > Can we call drm_mode_config_init() in vgem or is this a no-go because of > > > the properties? Do you see a solution to this? > > My quick fix idea was a drm_core_check_feature(dev, DRIVER_MODESET) > > around the drm_framebuffer_debugfs_init(). > > Ah, yes, thank you! > That sounds like a good idea. I will create a patch. Yeah we need that check, same way we have it in all the ioctls. Sorry for not spotting this, and yes we need our CI checking dri-devel patches, but just not there yet :-/ -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
Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file
Den 13.11.2017 23.17, skrev Chris Wilson: Quoting Noralf Trønnes (2017-11-13 22:12:06) Den 13.11.2017 22.33, skrev Chris Wilson: Quoting Noralf Trønnes (2017-11-13 19:54:43) Den 11.11.2017 19.55, skrev Chris Wilson: Quoting Noralf Trønnes (2017-11-07 19:13:40) Add debugfs file that dumps info about the framebuffers and its planes. Also dump info about any connected gem object(s). This isn't too hot for non-modesetting drm drivers. Just to be clear; this is currently on fire in i915's CI. As framebuffer-info is installed for vgem and vgem hasn't initialised any of the data structs being inspected... Ok, so I have broken vgem :-( drm_framebuffer_info() takes dev->mode_config.fb_lock which hasn't been initialized because vgem doesn't call drm_mode_config_init(). drm_mode_config_init() in addition to initializing various thing also calls drm_mode_create_standard_properties(). Can we call drm_mode_config_init() in vgem or is this a no-go because of the properties? Do you see a solution to this? My quick fix idea was a drm_core_check_feature(dev, DRIVER_MODESET) around the drm_framebuffer_debugfs_init(). Ah, yes, thank you! That sounds like a good idea. I will create a patch. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file
Quoting Noralf Trønnes (2017-11-13 22:12:06) > > Den 13.11.2017 22.33, skrev Chris Wilson: > > Quoting Noralf Trønnes (2017-11-13 19:54:43) > >> Den 11.11.2017 19.55, skrev Chris Wilson: > >>> Quoting Noralf Trønnes (2017-11-07 19:13:40) > Add debugfs file that dumps info about the framebuffers and its planes. > Also dump info about any connected gem object(s). > >>> This isn't too hot for non-modesetting drm drivers. > > Just to be clear; this is currently on fire in i915's CI. As > > framebuffer-info is installed for vgem and vgem hasn't initialised any > > of the data structs being inspected... > > Ok, so I have broken vgem :-( > > drm_framebuffer_info() takes dev->mode_config.fb_lock which hasn't been > initialized because vgem doesn't call drm_mode_config_init(). > drm_mode_config_init() in addition to initializing various thing also > calls drm_mode_create_standard_properties(). > > Can we call drm_mode_config_init() in vgem or is this a no-go because of > the properties? Do you see a solution to this? My quick fix idea was a drm_core_check_feature(dev, DRIVER_MODESET) around the drm_framebuffer_debugfs_init(). -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file
Den 13.11.2017 22.33, skrev Chris Wilson: Quoting Noralf Trønnes (2017-11-13 19:54:43) Den 11.11.2017 19.55, skrev Chris Wilson: Quoting Noralf Trønnes (2017-11-07 19:13:40) Add debugfs file that dumps info about the framebuffers and its planes. Also dump info about any connected gem object(s). This isn't too hot for non-modesetting drm drivers. Just to be clear; this is currently on fire in i915's CI. As framebuffer-info is installed for vgem and vgem hasn't initialised any of the data structs being inspected... Ok, so I have broken vgem :-( drm_framebuffer_info() takes dev->mode_config.fb_lock which hasn't been initialized because vgem doesn't call drm_mode_config_init(). drm_mode_config_init() in addition to initializing various thing also calls drm_mode_create_standard_properties(). Can we call drm_mode_config_init() in vgem or is this a no-go because of the properties? Do you see a solution to this? Sorry about the fallout. Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file
Quoting Noralf Trønnes (2017-11-13 19:54:43) > > Den 11.11.2017 19.55, skrev Chris Wilson: > > Quoting Noralf Trønnes (2017-11-07 19:13:40) > >> Add debugfs file that dumps info about the framebuffers and its planes. > >> Also dump info about any connected gem object(s). > > This isn't too hot for non-modesetting drm drivers. Just to be clear; this is currently on fire in i915's CI. As framebuffer-info is installed for vgem and vgem hasn't initialised any of the data structs being inspected... -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file
Den 11.11.2017 19.55, skrev Chris Wilson: Quoting Noralf Trønnes (2017-11-07 19:13:40) Add debugfs file that dumps info about the framebuffers and its planes. Also dump info about any connected gem object(s). This isn't too hot for non-modesetting drm drivers. And isn't this growing a midlayer? Calling to the drivers instead of the drivers controlling and calling helpers for the common stuff. I really can't say, but Daniel asked me to do it like this: https://lists.freedesktop.org/archives/dri-devel/2017-October/155245.html Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file
Quoting Noralf Trønnes (2017-11-07 19:13:40) > Add debugfs file that dumps info about the framebuffers and its planes. > Also dump info about any connected gem object(s). This isn't too hot for non-modesetting drm drivers. And isn't this growing a midlayer? Calling to the drivers instead of the drivers controlling and calling helpers for the common stuff. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file
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 Signed-off-by: Noralf Trønnes Reviewed-by: Laurent Pinchart --- 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 | 15 ++ 5 files changed, 104 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 40e6565105ac..c421dc01b334 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -25,7 +25,9 @@ #include #include #include +#include +#include "drm_internal.h" #include "drm_crtc_internal.h" /** @@ -956,3 +958,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; + unsigned int i; + + drm_printf_indent(p, indent, "refcount=%u\n", + drm_framebuffer_read_refcount(fb)); + 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=%ux%u\n", fb->width, fb->height); + drm_printf_indent(p, indent, "layers:\n"); + + for (i = 0; i < fb->format->num_planes; i++) { + drm_printf_indent(p, indent + 1, "size[%u]=%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[%u]=%u\n", i, fb->pitches[i]); + drm_printf_indent(p, indent + 1, "offset[%u]=%u\n", i, fb->offsets[i]); + drm_printf_indent(p, indent + 1, "obj[%u]:%s\n", i, + fb->obj[i] ? "" : "(null)"); + 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[%u]:\n", fb->base.id); + 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..01f8d9481211 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -40,6 +40,7 @@ #include #include #include +#include #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=%u\n", + kref_read(&obj->refcount)); + 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", +