Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file

2017-11-19 Thread Daniel Vetter
On Mon, Nov 13, 2017 at 11:31:28PM +0100, Noralf Trønnes wrote:
> 
> Den 13.11.2017 23.17, skrev Chris Wilson:
> > Quoting Noralf Trønnes (2017-11-13 22:12:06)
> > > Den 13.11.2017 22.33, skrev Chris Wilson:
> > > > Quoting Noralf Trønnes (2017-11-13 19:54:43)
> > > > > Den 11.11.2017 19.55, skrev Chris Wilson:
> > > > > > Quoting Noralf Trønnes (2017-11-07 19:13:40)
> > > > > > > Add debugfs file that dumps info about the framebuffers and its 
> > > > > > > planes.
> > > > > > > Also dump info about any connected gem object(s).
> > > > > > This isn't too hot for non-modesetting drm drivers.
> > > > Just to be clear; this is currently on fire in i915's CI. As
> > > > framebuffer-info is installed for vgem and vgem hasn't initialised any
> > > > of the data structs being inspected...
> > > Ok, so I have broken vgem :-(
> > > 
> > > drm_framebuffer_info() takes dev->mode_config.fb_lock which hasn't been
> > > initialized because vgem doesn't call drm_mode_config_init().
> > > drm_mode_config_init() in addition to initializing various thing also
> > > calls drm_mode_create_standard_properties().
> > > 
> > > Can we call drm_mode_config_init() in vgem or is this a no-go because of
> > > the properties? Do you see a solution to this?
> > My quick fix idea was a drm_core_check_feature(dev, DRIVER_MODESET)
> > around the drm_framebuffer_debugfs_init().
> 
> Ah, yes, thank you!
> That sounds like a good idea. I will create a patch.

Yeah we need that check, same way we have it in all the ioctls. Sorry for
not spotting this, and yes we need our CI checking dri-devel patches, but
just not there yet :-/
-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


Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file

2017-11-13 Thread Noralf Trønnes


Den 13.11.2017 23.17, skrev Chris Wilson:

Quoting Noralf Trønnes (2017-11-13 22:12:06)

Den 13.11.2017 22.33, skrev Chris Wilson:

Quoting Noralf Trønnes (2017-11-13 19:54:43)

Den 11.11.2017 19.55, skrev Chris Wilson:

Quoting Noralf Trønnes (2017-11-07 19:13:40)

Add debugfs file that dumps info about the framebuffers and its planes.
Also dump info about any connected gem object(s).

This isn't too hot for non-modesetting drm drivers.

Just to be clear; this is currently on fire in i915's CI. As
framebuffer-info is installed for vgem and vgem hasn't initialised any
of the data structs being inspected...

Ok, so I have broken vgem :-(

drm_framebuffer_info() takes dev->mode_config.fb_lock which hasn't been
initialized because vgem doesn't call drm_mode_config_init().
drm_mode_config_init() in addition to initializing various thing also
calls drm_mode_create_standard_properties().

Can we call drm_mode_config_init() in vgem or is this a no-go because of
the properties? Do you see a solution to this?

My quick fix idea was a drm_core_check_feature(dev, DRIVER_MODESET)
around the drm_framebuffer_debugfs_init().


Ah, yes, thank you!
That sounds like a good idea. I will create a patch.

Noralf.

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


Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file

2017-11-13 Thread Chris Wilson
Quoting Noralf Trønnes (2017-11-13 22:12:06)
> 
> Den 13.11.2017 22.33, skrev Chris Wilson:
> > Quoting Noralf Trønnes (2017-11-13 19:54:43)
> >> Den 11.11.2017 19.55, skrev Chris Wilson:
> >>> Quoting Noralf Trønnes (2017-11-07 19:13:40)
>  Add debugfs file that dumps info about the framebuffers and its planes.
>  Also dump info about any connected gem object(s).
> >>> This isn't too hot for non-modesetting drm drivers.
> > Just to be clear; this is currently on fire in i915's CI. As
> > framebuffer-info is installed for vgem and vgem hasn't initialised any
> > of the data structs being inspected...
> 
> Ok, so I have broken vgem :-(
> 
> drm_framebuffer_info() takes dev->mode_config.fb_lock which hasn't been
> initialized because vgem doesn't call drm_mode_config_init().
> drm_mode_config_init() in addition to initializing various thing also
> calls drm_mode_create_standard_properties().
> 
> Can we call drm_mode_config_init() in vgem or is this a no-go because of
> the properties? Do you see a solution to this?

My quick fix idea was a drm_core_check_feature(dev, DRIVER_MODESET)
around the drm_framebuffer_debugfs_init().
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file

2017-11-13 Thread Noralf Trønnes


Den 13.11.2017 22.33, skrev Chris Wilson:

Quoting Noralf Trønnes (2017-11-13 19:54:43)

Den 11.11.2017 19.55, skrev Chris Wilson:

Quoting Noralf Trønnes (2017-11-07 19:13:40)

Add debugfs file that dumps info about the framebuffers and its planes.
Also dump info about any connected gem object(s).

This isn't too hot for non-modesetting drm drivers.

Just to be clear; this is currently on fire in i915's CI. As
framebuffer-info is installed for vgem and vgem hasn't initialised any
of the data structs being inspected...


Ok, so I have broken vgem :-(

drm_framebuffer_info() takes dev->mode_config.fb_lock which hasn't been
initialized because vgem doesn't call drm_mode_config_init().
drm_mode_config_init() in addition to initializing various thing also
calls drm_mode_create_standard_properties().

Can we call drm_mode_config_init() in vgem or is this a no-go because of
the properties? Do you see a solution to this?

Sorry about the fallout.

Noralf.

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


Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file

2017-11-13 Thread Chris Wilson
Quoting Noralf Trønnes (2017-11-13 19:54:43)
> 
> Den 11.11.2017 19.55, skrev Chris Wilson:
> > Quoting Noralf Trønnes (2017-11-07 19:13:40)
> >> Add debugfs file that dumps info about the framebuffers and its planes.
> >> Also dump info about any connected gem object(s).
> > This isn't too hot for non-modesetting drm drivers.

Just to be clear; this is currently on fire in i915's CI. As
framebuffer-info is installed for vgem and vgem hasn't initialised any
of the data structs being inspected...
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file

2017-11-13 Thread Noralf Trønnes


Den 11.11.2017 19.55, skrev Chris Wilson:

Quoting Noralf Trønnes (2017-11-07 19:13:40)

Add debugfs file that dumps info about the framebuffers and its planes.
Also dump info about any connected gem object(s).

This isn't too hot for non-modesetting drm drivers.

And isn't this growing a midlayer? Calling to the drivers instead of the
drivers controlling and calling helpers for the common stuff.


I really can't say, but Daniel asked me to do it like this:
https://lists.freedesktop.org/archives/dri-devel/2017-October/155245.html

Noralf.

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


Re: [PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file

2017-11-11 Thread Chris Wilson
Quoting Noralf Trønnes (2017-11-07 19:13:40)
> Add debugfs file that dumps info about the framebuffers and its planes.
> Also dump info about any connected gem object(s).

This isn't too hot for non-modesetting drm drivers.

And isn't this growing a midlayer? Calling to the drivers instead of the
drivers controlling and calling helpers for the common stuff.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v5 04/12] drm/framebuffer: Add framebuffer debugfs file

2017-11-07 Thread Noralf Trønnes
Add debugfs file that dumps info about the framebuffers and its planes.
Also dump info about any connected gem object(s).

Reviewed-by: Daniel Vetter 
Signed-off-by: Noralf Trønnes 
Reviewed-by: Laurent Pinchart 
---
 drivers/gpu/drm/drm_debugfs.c |  6 
 drivers/gpu/drm/drm_framebuffer.c | 59 +++
 drivers/gpu/drm/drm_gem.c | 17 +++
 drivers/gpu/drm/drm_internal.h|  7 +
 include/drm/drm_drv.h | 15 ++
 5 files changed, 104 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 40e6565105ac..c421dc01b334 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -25,7 +25,9 @@
 #include 
 #include 
 #include 
+#include 
 
+#include "drm_internal.h"
 #include "drm_crtc_internal.h"
 
 /**
@@ -956,3 +958,60 @@ int drm_framebuffer_plane_height(int height,
return fb_plane_height(height, fb->format, plane);
 }
 EXPORT_SYMBOL(drm_framebuffer_plane_height);
+
+void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
+   const struct drm_framebuffer *fb)
+{
+   struct drm_format_name_buf format_name;
+   unsigned int i;
+
+   drm_printf_indent(p, indent, "refcount=%u\n",
+ drm_framebuffer_read_refcount(fb));
+   drm_printf_indent(p, indent, "format=%s\n",
+ drm_get_format_name(fb->format->format, 
&format_name));
+   drm_printf_indent(p, indent, "modifier=0x%llx\n", fb->modifier);
+   drm_printf_indent(p, indent, "size=%ux%u\n", fb->width, fb->height);
+   drm_printf_indent(p, indent, "layers:\n");
+
+   for (i = 0; i < fb->format->num_planes; i++) {
+   drm_printf_indent(p, indent + 1, "size[%u]=%dx%d\n", i,
+ drm_framebuffer_plane_width(fb->width, fb, i),
+ drm_framebuffer_plane_height(fb->height, fb, 
i));
+   drm_printf_indent(p, indent + 1, "pitch[%u]=%u\n", i, 
fb->pitches[i]);
+   drm_printf_indent(p, indent + 1, "offset[%u]=%u\n", i, 
fb->offsets[i]);
+   drm_printf_indent(p, indent + 1, "obj[%u]:%s\n", i,
+ fb->obj[i] ? "" : "(null)");
+   if (fb->obj[i])
+   drm_gem_print_info(p, indent + 2, fb->obj[i]);
+   }
+}
+
+#ifdef CONFIG_DEBUG_FS
+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_printf(&p, "framebuffer[%u]:\n", fb->base.id);
+   drm_framebuffer_print_info(&p, 1, fb);
+   }
+   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 4c84b23d37cc..01f8d9481211 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "drm_internal.h"
 
 /** @file drm_gem.c
@@ -1040,3 +1041,19 @@ int drm_gem_mmap(struct file *filp, struct 
vm_area_struct *vma)
return ret;
 }
 EXPORT_SYMBOL(drm_gem_mmap);
+
+void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
+   const struct drm_gem_object *obj)
+{
+   drm_printf_indent(p, indent, "name=%d\n", obj->name);
+   drm_printf_indent(p, indent, "refcount=%u\n",
+ kref_read(&obj->refcount));
+   drm_printf_indent(p, indent, "start=%08lx\n",
+ drm_vma_node_start(&obj->vma_node));
+   drm_printf_indent(p, indent, "size=%zu\n", obj->size);
+   drm_printf_indent(p, indent, "imported=%s\n",
+