[PATCH 04/16] drm/debugfs: Add kerneldoc

From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: [PATCH 04/16] drm/debugfs: Add kerneldoc
Date: Wed, 22 Mar 2017 09:36:05 +0100	[thread overview]
Message-ID: <20170322083617.13361-5-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <20170322083617.13361-1-daniel.vetter@ffwll.ch>

I've decided to not document drm_debugfs_remove_files, it's on the way
out.

The biggest part is a huge todor.rst entry with what all should be
improved.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/gpu/drm-uapi.rst |  9 ++++++++
 Documentation/gpu/todo.rst     | 26 +++++++++++++++++++++
 drivers/gpu/drm/drm_debugfs.c  | 51 ++++++------------------------------------
 drivers/gpu/drm/drm_internal.h |  2 +-
 include/drm/drm_debugfs.h      | 38 +++++++++++++++++++++++++------
 5 files changed, 74 insertions(+), 52 deletions(-)

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 369e8ca12b8e..76356c86e358 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -210,6 +210,15 @@ Display CRC Support
 .. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c
    :export:
 
+Debugfs Support
+---------------
+
+.. kernel-doc:: include/drm/drm_debugfs.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_debugfs.c
+   :export:
+
 VBlank event handling
 =====================
 
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 64e9d16170ce..9ecd2ebb8dd8 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -272,6 +272,32 @@ This is a really varied tasks with lots of little bits and pieces:
 
 Contact: Daniel Vetter
 
+Clean up the debugfs support
+----------------------------
+
+There's a bunch of issues with it:
+
+- The drm_info_list -> show function doesn't even bother to cast to the drm
+  structure for you. This is lazy.
+
+- We probably want to have some support for debugfs files on crtc/connectors and
+  maybe other kms objects directly in core. There's even drm_print support in
+  the funcs for these objects to dump kms state, so it's all there. And then the
+  ->show functions should obviously give you a pointer to the right object.
+
+- The drm_info_list stuff is centered on drm_minor instead of drm_device. For
+  anything we want to print drm_device (or maybe drm_file) is the right thing.
+
+- The drm_driver->debugfs_init hooks we have is just an artifact of the old
+  midlayered load sequence. DRM debugfs should work more like sysfs, where you
+  can create properties/files for an object anytime you want, and the core
+  takes care of publishing/unpuplishing all the files at register/unregister
+  time. Drivers shouldn't need to worry about these technicalities, and fixing
+  this (together with the drm_minor->drm_device move) would allow us to remove
+  debugfs_init.
+
+Contact: Daniel Vetter
+
 Better Testing
 ==============
 
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 4b02f4230562..c1807d5754b2 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -1,10 +1,3 @@
-/**
- * \file drm_debugfs.c
- * debugfs support for DRM
- *
- * \author Ben Gamari <bgamari@gmail.com>
- */
-
 /*
  * Created: Sun Dec 21 13:08:50 2008 by bgamari@gmail.com
  *
@@ -75,16 +68,15 @@ static const struct file_operations drm_debugfs_fops = {
 
 
 /**
- * Initialize a given set of debugfs files for a device
- *
- * \param files The array of files to create
- * \param count The number of files given
- * \param root DRI debugfs dir entry.
- * \param minor device minor number
- * \return Zero on success, non-zero on failure
+ * drm_debugfs_create_files - Initialize a given set of debugfs files for DRM
+ * 			minor
+ * @files: The array of files to create
+ * @count: The number of files given
+ * @root: DRI debugfs dir entry.
+ * @minor: device minor number
  *
  * Create a given set of debugfs files represented by an array of
- * &drm_info_list in the given root directory. These files will be removed
+ * &struct drm_info_list in the given root directory. These files will be removed
  * automatically on drm_debugfs_cleanup().
  */
 int drm_debugfs_create_files(const struct drm_info_list *files, int count,
@@ -133,17 +125,6 @@ int drm_debugfs_create_files(const struct drm_info_list *files, int count,
 }
 EXPORT_SYMBOL(drm_debugfs_create_files);
 
-/**
- * Initialize the DRI debugfs filesystem for a device
- *
- * \param dev DRM device
- * \param minor device minor number
- * \param root DRI debugfs dir entry.
- *
- * Create the DRI debugfs root entry "/sys/kernel/debug/dri", the device debugfs root entry
- * "/sys/kernel/debug/dri/%minor%/", and each entry in debugfs_list as
- * "/sys/kernel/debug/dri/%minor%/%name%".
- */
 int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		     struct dentry *root)
 {
@@ -189,16 +170,6 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 }
 
 
-/**
- * Remove a list of debugfs files
- *
- * \param files The list of files
- * \param count The number of files
- * \param minor The minor of which we should remove the files
- * \return always zero.
- *
- * Remove all debugfs entries created by debugfs_init().
- */
 int drm_debugfs_remove_files(const struct drm_info_list *files, int count,
 			     struct drm_minor *minor)
 {
@@ -235,14 +206,6 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor)
 	mutex_unlock(&minor->debugfs_lock);
 }
 
-/**
- * Cleanup the debugfs filesystem resources.
- *
- * \param minor device minor number.
- * \return always zero.
- *
- * Remove all debugfs entries created by debugfs_init().
- */
 int drm_debugfs_cleanup(struct drm_minor *minor)
 {
 	if (!minor->debugfs_root)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 92ff4b9393b1..3d8e8f878924 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -100,7 +100,7 @@ int drm_gem_open_ioctl(struct drm_device *dev, void *data,
 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);
 
-/* drm_debugfs.c */
+/* drm_debugfs.c drm_debugfs_crc.c */
 #if defined(CONFIG_DEBUG_FS)
 int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		     struct dentry *root);
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
index 17e47c073fa9..d45bc1879412 100644
--- a/include/drm/drm_debugfs.h
+++ b/include/drm/drm_debugfs.h
@@ -33,23 +33,47 @@
 #define _DRM_DEBUGFS_H_
 
 /**
- * Info file list entry. This structure represents a debugfs or proc file to
- * be created by the drm core
+ * struct drm_info_list - debugfs info list entry
+ *
+ * This structure represents a debugfs file to be created by the drm
+ * core.
  */
 struct drm_info_list {
-	const char *name; /** file name */
-	int (*show)(struct seq_file*, void*); /** show callback */
-	u32 driver_features; /**< Required driver features for this entry */
+	/** @name: file name */
+	const char *name;
+	/**
+	 * @show:
+	 *
+	 * Show callback. &seq_file->private will be set to the &struct
+	 * drm_info_node corresponding to the instance of this info on a given
+	 * &struct drm_minor.
+	 */
+	int (*show)(struct seq_file*, void*);
+	/** @driver_features: Required driver features for this entry */
+	u32 driver_features;
+	/** @data: Driver-private data, should not be device-specific. */
 	void *data;
 };
 
 /**
- * debugfs node structure. This structure represents a debugfs file.
+ * struct drm_info_node - Per-minor debugfs node structure
+ *
+ * This structure represents a debugfs file, as an instantiation of a &struct
+ * drm_info_list on a &struct drm_minor.
+ *
+ * FIXME:
+ *
+ * No it doesn't make a hole lot of sense that we duplicate debugfs entries for
+ * both the render and the primary nodes, but that's how this has organically
+ * grown. It should probably be fixed, with a compatibility link, if needed.
  */
 struct drm_info_node {
-	struct list_head list;
+	/** @minor: &struct drm_minor for this node. */
 	struct drm_minor *minor;
+	/** @info_ent: template for thise node. */
 	const struct drm_info_list *info_ent;
+	/* private: */
+	struct list_head list;
 	struct dentry *dent;
 };
 
-- 
2.11.0

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