Re: [PATCH v2] drm/framebuffer: Add framebuffer debugfs file

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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