Re: [PATCH v2 5/8] drm/gem-fb-helper: Add drm_gem_fb_debugfs_show()

On Wed, Oct 18, 2017 at 09:44:22PM +0200, Noralf Trønnes wrote:

Den 17.10.2017 14.56, skrev Daniel Vetter:

...

On Sun, Oct 15, 2017 at 06:30:39PM +0200, Noralf Trønnes wrote:

...

Add drm_gem_fb_debugfs_show() function to provide a debugfs representation of the framebuffer and GEM object(s).

Signed-off-by: Noralf Trønnes noralf@tronnes.org

drivers/gpu/drm/drm_gem_framebuffer_helper.c | 45 ++++++++++++++++++++++++++++ include/drm/drm_gem_framebuffer_helper.h | 6 ++++ 2 files changed, 51 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index aa8cb9bfa499..18fdcc29427a 100644 --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -265,6 +265,51 @@ int drm_gem_fb_prepare_fb(struct drm_plane *plane, } EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb); +#ifdef CONFIG_DEBUG_FS +static void drm_gem_fb_describe(struct drm_framebuffer *fb, struct seq_file *m) +{

• struct drm_gem_object *obj;
• unsigned int i;
• seq_printf(m, "[FB:%d] %dx%d@%4.4s\n", fb->base.id, fb->width,

•    fb->height, (char *)&fb->format->format);
  

• for (i = 0; i < fb->format->num_planes; i++) {

• obj = fb->obj[i];
  

• seq_printf(m, "\t%u: offset=%d pitch=%d, GEM: name=%d",
  

• 	   i, fb->offsets[i], fb->pitches[i], obj->name);
  

• seq_printf(m, " refcount=%d start=%08lx size=%zu%s\n",
  

• 	   kref_read(&obj->refcount),
  

• 	   drm_vma_node_start(&obj->vma_node), obj->size,
  

• 	   obj->import_attach ? " (imported)" : "");
  

• }

+}

Given that gem is the defacto standard and that we can easily skip the gem specific stuff on non-gem drivers by checking the pointer for NULL I think we should add the framebuffer list by default. Like we already do for e.g. atomic state, see drm_atomic_debugfs_init. Probably better to split it up into a generic fb describe, which calls into a gem_obj describe, with maybe in the future the option to also call into driver callbacks to dump more driver state.

How about this:

lgtm. Whoever wants/needs to dump driver private stuff can then sprinkle the callbacks over this. -Daniel

drivers/gpu/drm/drm_gem.c:

#ifdef CONFIG_DEBUG_FS void drm_gem_print(const 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

drivers/gpu/drm/drm_framebuffer.c:

#ifdef CONFIG_DEBUG_FS static void drm_framebuffer_print(const struct drm_framebuffer *fb,                   struct drm_printer *p) {     unsigned int i;

drm_printf(p, "[FB:%d] %dx%d@%4.4s refcount=%d\n", fb->base.id,            fb->width, fb->height, (char *)&fb->format->format,            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

drivers/gpu/drm/drm_debugfs.c:

int drm_debugfs_init(struct drm_minor *minor, int minor_id,              struct dentry *root) { ...     ret = drm_framebuffer_debugfs_init(minor);     if (ret) {         DRM_ERROR("Failed to create framebuffer debugfs files\n");         return ret;     } ... }

Available names for the debugfs file:

$ grep -r ""fbs"" drivers/gpu/drm

$ grep -r ""fb"" drivers/gpu/drm drivers/gpu/drm/tilcdc/tilcdc_drv.c:            { "fb", drm_fb_cma_debugfs_show, 0 }, drivers/gpu/drm/omapdrm/omap_debugfs.c: {"fb", fb_show, 0}, drivers/gpu/drm/tinydrm/mipi-dbi.c:     { "fb", drm_fb_cma_debugfs_show, 0 }, drivers/gpu/drm/arm/hdlcd_drv.c:        { "fb", drm_fb_cma_debugfs_show, 0 }, drivers/gpu/drm/msm/msm_debugfs.c:              { "fb", show_locked, 0, msm_fb_show }, drivers/gpu/drm/arc/arcpgu_drv.c:       { "fb", drm_fb_cma_debugfs_show, 0 },

$ grep -r ""framebuffer"" drivers/gpu/drm

$ grep -r ""framebuffers"" drivers/gpu/drm drivers/gpu/drm/tegra/drm.c:    { "framebuffers", tegra_debugfs_framebuffers, 0 },

Noralf.

...

There's probably more stuff that we should list by default in debugfs, like blob properties and similar things. -Daniel

...

+/**

• • drm_gem_fb_debugfs_show() - Helper to list GEM backed framebuffer objects

• •                         in debugfs.
    

• • @m: Output file
• • @arg: Private data for the callback
• • This function gives a debugfs representation of the framebuffers with their
• • backing GEM objects. See drm_debugfs_create_files() for more info.
• */

+int drm_gem_fb_debugfs_show(struct seq_file *m, void *arg) +{

• struct drm_info_node *node = m->private;
• struct drm_device *dev = node->minor->dev;
• struct drm_framebuffer *fb;
• mutex_lock(&dev->mode_config.fb_lock);
• drm_for_each_fb(fb, dev)

• drm_gem_fb_describe(fb, m);
  

• mutex_unlock(&dev->mode_config.fb_lock);
• return 0;

+} +EXPORT_SYMBOL_GPL(drm_gem_fb_debugfs_show); +#endif

• /**
  • drm_gem_fbdev_fb_create - Create a GEM backed &drm_framebuffer for fbdev

  •                       emulation
    

diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h index 5ca7cdc3f527..1bb73d961aba 100644 --- a/include/drm/drm_gem_framebuffer_helper.h +++ b/include/drm/drm_gem_framebuffer_helper.h @@ -28,6 +28,12 @@ drm_gem_fb_create(struct drm_device *dev, struct drm_file *file, int drm_gem_fb_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state); +#ifdef CONFIG_DEBUG_FS +struct seq_file;

+int drm_gem_fb_debugfs_show(struct seq_file *m, void *arg); +#endif

• struct drm_framebuffer * drm_gem_fbdev_fb_create(struct drm_device *dev, struct drm_fb_helper_surface_size *sizes,

-- 2.14.2