All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/framebuffer: Add framebuffer debugfs file
@ 2017-10-23 16:47 Noralf Trønnes
  2017-10-23 21:32 ` Kristian Høgsberg
  2017-10-23 22:09 ` kbuild test robot
  0 siblings, 2 replies; 6+ messages in thread
From: Noralf Trønnes @ 2017-10-23 16:47 UTC (permalink / raw)
  To: dri-devel

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>
---
 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));
+
+	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
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/framebuffer: Add framebuffer debugfs file
  2017-10-23 16:47 [PATCH] drm/framebuffer: Add framebuffer debugfs file Noralf Trønnes
@ 2017-10-23 21:32 ` Kristian Høgsberg
  2017-10-24 16:39   ` Noralf Trønnes
  2017-10-23 22:09 ` kbuild test robot
  1 sibling, 1 reply; 6+ messages in thread
From: Kristian Høgsberg @ 2017-10-23 21:32 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: dri-devel

On Mon, Oct 23, 2017 at 9:47 AM, Noralf Trønnes <noralf@tronnes.org> 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>
> ---
>  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:

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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/framebuffer: Add framebuffer debugfs file
  2017-10-23 16:47 [PATCH] drm/framebuffer: Add framebuffer debugfs file Noralf Trønnes
  2017-10-23 21:32 ` Kristian Høgsberg
@ 2017-10-23 22:09 ` kbuild test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2017-10-23 22:09 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: kbuild-all, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]

Hi Noralf,

[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.14-rc6 next-20171018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Noralf-Tr-nnes/drm-framebuffer-Add-framebuffer-debugfs-file/20171024-054910
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-randconfig-x001-201743 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/drm_gem.c: In function 'drm_gem_print':
>> drivers/gpu/drm/drm_gem.c:1050:25: warning: passing argument 1 of 'drm_vma_node_start' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
         drm_vma_node_start(&obj->vma_node), obj->size,
                            ^
   In file included from drivers/gpu/drm/drm_gem.c:41:0:
   include/drm/drm_vma_manager.h:155:29: note: expected 'struct drm_vma_offset_node *' but argument is of type 'const struct drm_vma_offset_node *'
    static inline unsigned long drm_vma_node_start(struct drm_vma_offset_node *node)
                                ^~~~~~~~~~~~~~~~~~

vim +1050 drivers/gpu/drm/drm_gem.c

  1044	
  1045	#ifdef CONFIG_DEBUG_FS
  1046	void drm_gem_print(const struct drm_gem_object *obj, struct drm_printer *p)
  1047	{
  1048		drm_printf(p, "name=%d refcount=%d start=%08lx size=%zu%s\n",
  1049			   obj->name, kref_read(&obj->refcount),
> 1050			   drm_vma_node_start(&obj->vma_node), obj->size,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26153 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/framebuffer: Add framebuffer debugfs file
  2017-10-23 21:32 ` Kristian Høgsberg
@ 2017-10-24 16:39   ` Noralf Trønnes
  2017-10-24 23:03     ` Kristian Høgsberg
  0 siblings, 1 reply; 6+ messages in thread
From: Noralf Trønnes @ 2017-10-24 16:39 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: dri-devel


Den 23.10.2017 23.32, skrev Kristian Høgsberg:
> On Mon, Oct 23, 2017 at 9:47 AM, Noralf Trønnes <noralf@tronnes.org> 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>
>> ---
>>   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.

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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/framebuffer: Add framebuffer debugfs file
  2017-10-24 16:39   ` Noralf Trønnes
@ 2017-10-24 23:03     ` Kristian Høgsberg
  2017-10-30 10:03       ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Kristian Høgsberg @ 2017-10-24 23:03 UTC (permalink / raw)
  To: Noralf Trønnes; +Cc: dri-devel

On Tue, Oct 24, 2017 at 9:39 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
>
> Den 23.10.2017 23.32, skrev Kristian Høgsberg:
>>
>> On Mon, Oct 23, 2017 at 9:47 AM, Noralf Trønnes <noralf@tronnes.org>
>> 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>
>>> ---
>>>   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@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/framebuffer: Add framebuffer debugfs file
  2017-10-24 23:03     ` Kristian Høgsberg
@ 2017-10-30 10:03       ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-10-30 10:03 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: dri-devel

On Tue, Oct 24, 2017 at 04:03:46PM -0700, Kristian Høgsberg wrote:
> On Tue, Oct 24, 2017 at 9:39 AM, Noralf Trønnes <noralf@tronnes.org> wrote:
> >
> > Den 23.10.2017 23.32, skrev Kristian Høgsberg:
> >>
> >> On Mon, Oct 23, 2017 at 9:47 AM, Noralf Trønnes <noralf@tronnes.org>
> >> 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>
> >>> ---
> >>>   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.

Imo we should just put all the i915 kms printing into the state stuff and
throw our info stuff away. No one got around to that yet unfortunately,
probably needs someone with an interest to make the cross-driver debugging
experience more consistent ... hint, hint :-)

But yeah it's all there, and I suggested to Noralf he integrate with that
to make an even stronger incentive to use the commont state/kms debugfs
stuff.

Cheers, 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-10-30 10:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-23 16:47 [PATCH] drm/framebuffer: Add framebuffer debugfs file Noralf Trønnes
2017-10-23 21:32 ` Kristian Høgsberg
2017-10-24 16:39   ` Noralf Trønnes
2017-10-24 23:03     ` Kristian Høgsberg
2017-10-30 10:03       ` Daniel Vetter
2017-10-23 22:09 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.