[PATCH v2] drm/framebuffer: Add framebuffer debugfs file
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Oct 25 16:18:28 UTC 2017
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 at 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 at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
More information about the dri-devel
mailing list