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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 24, 2017 at 9:39 AM, Noralf Trønnes <noralf@xxxxxxxxxxx> wrote:
>
> Den 23.10.2017 23.32, skrev Kristian Høgsberg:
>>
>> On Mon, Oct 23, 2017 at 9:47 AM, Noralf Trønnes <noralf@xxxxxxxxxxx>
>> 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@xxxxxxxxxxx>
>>> ---
>>>   drivers/gpu/drm/drm_debugfs.c     |  6 +++++
>>>   drivers/gpu/drm/drm_framebuffer.c | 51
>>> +++++++++++++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/drm_gem.c         | 11 +++++++++
>>>   drivers/gpu/drm/drm_internal.h    |  4 +++
>>>   4 files changed, 72 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..ebdfe2b5689f 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,52 @@ 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,
>>> +                                 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));
>>
>> Could you print the format modifier as well?
>>
>>> +
>>> +       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 },
>>
>> This is quite useful, thanks. Ultimately, it would be very nice to
>> have something like i915_display_info, but in a generic way. There's
>> not much there that's actually i915 specific:
>
>
> How about mimicking drm_state_info(), debugfs file 'state', and create
> an 'info' file?
>
> struct drm_crtc_funcs {
>     /**
>      * @print_info:
>      *
>      * If driver subclasses struct &drm_crtc, it can implement this
>      * optional hook for printing additional driver specific info.
>      * See drm_crtc_print_info().
>      */
>     void (*print_info)(struct drm_printer *p, struct drm_crtc *crtc);
> }
>
> void drm_crtc_print_info(struct drm_printer *p, struct drm_crtc *crtc)
> {
>     drm_modeset_lock(&crtc->mutex, NULL);
>
>     drm_printf(p, "[CRTC:%d:%s] ...");
>
>     if (crtc->funcs->print_info)
>         crtc->funcs->print_info(p, crtc);
>
>     drm_modeset_unlock(&crtc->mutex);
> }
>
> static int drm_debugfs_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_plane *plane;
>     struct drm_crtc *crtc;
>     struct drm_encoder *encoder;
>     struct drm_connector *connector;
>     struct drm_connector_list_iter conn_iter;
>     struct drm_framebuffer *fb;
>
>     mutex_lock(&dev->struct_mutex);
>
>     drm_for_each_plane(plane, dev)
>         drm_plane_print_info(&p, plane);
>
>     drm_for_each_crtc(crtc, dev)
>         drm_crtc_print_info(&p, crtc);
>
>     drm_for_each_encoder(encoder, dev)
>         drm_encoder_print_info(&p, encoder);
>
>     drm_connector_list_iter_begin(dev, &conn_iter);
>     drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>     drm_for_each_connector_iter(connector, &conn_iter)
>         drm_connector_print_info(&p, connector);
>     drm_modeset_unlock(&dev->mode_config.connection_mutex);
>     drm_connector_list_iter_end(&conn_iter);
>
>     mutex_lock(&dev->mode_config.fb_lock);
>     drm_for_each_fb(fb, dev)
>         drm_framebuffer_print_info(&p, fb);
>     mutex_unlock(&dev->mode_config.fb_lock);
>
>     mutex_unlock(&dev->struct_mutex);
> }
>
>  static const struct drm_info_list drm_debugfs_list[] = {
>      {"name", drm_name_info, 0},
>      {"clients", drm_clients_info, 0},
>      {"gem_names", drm_gem_name_info, DRIVER_GEM},
> +     {"info", drm_debugfs_info, 0},
>  };
>
>
> Or is it too much info in one single file so it should be split into one
> file per object: plane, framebuffer, etc. ?
> Daniel mentioned blob properties, and there are probably more stuff that
> can be useful for debugging.

Oh oops, my bad, I wasn't aware of 'state'. That goes a long way
towards what I had in mind with info.

Kristian

> Noralf.
>
>
>> localhost ~ # cat /sys/kernel/debug/dri/0/i915_display_info
>> CRTC info
>> ---------
>> CRTC 36: pipe: A, active=yes, (size=2400x1600), dither=no, bpp=24
>>          fb: 88, pos: 0x0, size: 2400x1600
>>          encoder 57: type: TMDS-57, connectors:
>>                  connector 58: type: eDP-1, status: connected, mode:
>>                  id 0:"2400x1600" freq 60 clock 252750 hdisp 2400 hss
>> 2448 hse 2480 htot 2560 vdisp 1600 vss 1603 vse 1613 vtot 1646 type
>> 0x48 flags 0xa
>>          cursor visible? no, position (0, 0), size 0x0, addr
>> 0x00000000, active? no
>>          num_scalers=2, scaler_users=0 scaler_id=-1, scalers[0]:
>> use=no, mode=0, scalers[1]: use=no, mode=0
>>          --Plane id 27: type=PRI, crtc_pos=   0x   0,
>> crtc_size=2400x1600, src_pos=0.0000x0.0000,
>> src_size=2400.0000x1600.0000, format=XR24 little-endian (0x34325258),
>> rotation=0 (0x00000001)
>>          --Plane id 30: type=OVL, crtc_pos=   0x   0, crtc_size=   0x
>> 0, src_pos=0.0000x0.0000, src_size=0.0000x0.0000, format=N/A,
>> rotation=0 (0x00000001)
>>          --Plane id 33: type=CUR, crtc_pos=   0x   0, crtc_size=   0x
>> 0, src_pos=0.0000x0.0000, src_size=0.0000x0.0000, format=N/A,
>> rotation=0 (0x00000001)
>>          underrun reporting: cpu=yes pch=yes
>> CRTC 46: pipe: B, active=no, (size=0x0), dither=no, bpp=0
>>          underrun reporting: cpu=yes pch=yes
>> CRTC 56: pipe: C, active=no, (size=0x0), dither=no, bpp=0
>>          underrun reporting: cpu=yes pch=yes
>>
>> Connector info
>> --------------
>> connector 58: type eDP-1, status: connected
>>          name:
>>          physical dimensions: 260x170mm
>>          subpixel order: Unknown
>>          CEA rev: 0
>>          DPCD rev: 12
>>          audio support: no
>>          fixed mode:
>>                  id 61:"2400x1600" freq 60 clock 252750 hdisp 2400 hss
>> 2448 hse 2480 htot 2560 vdisp 1600 vss 1603 vse 1613 vtot 1646 type
>> 0x48 flags 0xa
>>          DP branch device present: no
>>          modes:
>>                  id 59:"2400x1600" freq 60 clock 252750 hdisp 2400 hss
>> 2448 hse 2480 htot 2560 vdisp 1600 vss 1603 vse 1613 vtot 1646 type
>> 0x48 flags 0xa
>> connector 66: type DP-1, status: disconnected
>>          DPCD rev: 0
>>          audio support: no
>>          DP branch device present: no
>>          modes:
>> connector 70: type HDMI-A-1, status: disconnected
>>          audio support: no
>>          modes:
>> connector 73: type DP-2, status: disconnected
>>          DPCD rev: 0
>>          audio support: no
>>          DP branch device present: no
>>          modes:
>> connector 77: type HDMI-A-2, status: disconnected
>>          audio support: no
>>          modes:
>>
>> Kristian
>>
>>> +};
>>> +
>>> +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..f42977b28701 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(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
>>> diff --git a/drivers/gpu/drm/drm_internal.h
>>> b/drivers/gpu/drm/drm_internal.h
>>> index fbc3f308fa19..7f4564419bf4 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(const 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@xxxxxxxxxxxxxxxxxxxxx
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux