All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/7] Introduce debugfs device-centered functions
@ 2022-12-19 12:06 ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal

This series introduces the initial structure to make DRM debugfs more
device-centered and it is the first step to drop the
drm_driver->debugfs_init hooks in the future [1].

Currently, DRM debugfs files are created using drm_debugfs_create_files()
on request. The first patch of this series makes it possible for DRM devices
for creating debugfs files during drm_dev_register(). For it, it introduces
two new functions that can be used by the drivers: drm_debugfs_add_files()
and drm_debugfs_add_file(). The requests are added to a list and are created
all at once during drm_dev_register(). Moreover, the first patch was based on
this RFC series [2].

The main difference between the RFC series and the current series is the
creation of a new fops structure to accommodate the new structs and, also,
the creation of a new drm_debugfs_open. Moreover, the new series uses
device-managed allocation, returns memory allocation errors, and converts
more drivers to the new structure.

Moreover, since v3, the ability to create debugfs files at late_register hooks was
added. In previous versions, modeset components weren't able to create debugfs
files at late_register hooks as the registration of drm_minor happens before the
registration of the modeset abstractions. So, the third patch fixes this problem
by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
catching this problem!

Apart from the third patch, the series looks similiar from its last version.

[1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
[2] https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/

Best Regards,
- Maíra Canal

---

v1 -> v2: https://lore.kernel.org/dri-devel/20221122190314.185015-1-mcanal@igalia.com/T/#t

- Fix compilation errors in the second patch (kernel test robot).
- Drop debugfs_init hook from vkms (Maíra Canal).
- Remove return values and error handling to debugfs related
functions (Jani Nikula).
- Remove entry from list after the file is created, so that drm_debugfs_init
can be called more than once (Maíra Canal).

v2 -> v3: https://lore.kernel.org/dri-devel/20221123220725.1272155-1-mcanal@igalia.com/

- Rebase on top of drm-misc-next

v3 -> v4: https://lore.kernel.org/dri-devel/20221207132325.140393-1-mcanal@igalia.com/

- Add Maxime's Reviewed-by tags
- Add the ability to create debugfs files at late_register hooks (Melissa Wen).

---

Maíra Canal (7):
  drm/debugfs: create device-centered debugfs functions
  drm: use new debugfs device-centered functions on DRM core files
  drm/debugfs: create debugfs late register functions
  drm/vc4: use new debugfs device-centered functions
  drm/v3d: use new debugfs device-centered functions
  drm/vkms: use new debugfs device-centered functions
  drm/todo: update the debugfs clean up task

 Documentation/gpu/todo.rst            |   9 +--
 drivers/gpu/drm/drm_atomic.c          |  11 ++-
 drivers/gpu/drm/drm_client.c          |  11 ++-
 drivers/gpu/drm/drm_debugfs.c         | 102 +++++++++++++++++++++++---
 drivers/gpu/drm/drm_drv.c             |   3 +
 drivers/gpu/drm/drm_framebuffer.c     |  11 ++-
 drivers/gpu/drm/drm_gem_vram_helper.c |  11 ++-
 drivers/gpu/drm/drm_internal.h        |   5 ++
 drivers/gpu/drm/drm_mode_config.c     |   2 +
 drivers/gpu/drm/v3d/v3d_debugfs.c     |  22 +++---
 drivers/gpu/drm/vc4/vc4_bo.c          |  10 +--
 drivers/gpu/drm/vc4/vc4_crtc.c        |   7 +-
 drivers/gpu/drm/vc4/vc4_debugfs.c     |  36 ++-------
 drivers/gpu/drm/vc4/vc4_dpi.c         |   5 +-
 drivers/gpu/drm/vc4/vc4_drv.c         |   1 -
 drivers/gpu/drm/vc4/vc4_drv.h         |  32 ++------
 drivers/gpu/drm/vc4/vc4_dsi.c         |   6 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c        |  12 +--
 drivers/gpu/drm/vc4/vc4_hvs.c         |  24 ++----
 drivers/gpu/drm/vc4/vc4_v3d.c         |  14 +---
 drivers/gpu/drm/vc4/vc4_vec.c         |   6 +-
 drivers/gpu/drm/vkms/vkms_drv.c       |  17 ++---
 include/drm/drm_debugfs.h             |  41 +++++++++++
 include/drm/drm_device.h              |  15 ++++
 24 files changed, 233 insertions(+), 180 deletions(-)

-- 
2.38.1


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

* [PATCH v4 0/7] Introduce debugfs device-centered functions
@ 2022-12-19 12:06 ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen

This series introduces the initial structure to make DRM debugfs more
device-centered and it is the first step to drop the
drm_driver->debugfs_init hooks in the future [1].

Currently, DRM debugfs files are created using drm_debugfs_create_files()
on request. The first patch of this series makes it possible for DRM devices
for creating debugfs files during drm_dev_register(). For it, it introduces
two new functions that can be used by the drivers: drm_debugfs_add_files()
and drm_debugfs_add_file(). The requests are added to a list and are created
all at once during drm_dev_register(). Moreover, the first patch was based on
this RFC series [2].

The main difference between the RFC series and the current series is the
creation of a new fops structure to accommodate the new structs and, also,
the creation of a new drm_debugfs_open. Moreover, the new series uses
device-managed allocation, returns memory allocation errors, and converts
more drivers to the new structure.

Moreover, since v3, the ability to create debugfs files at late_register hooks was
added. In previous versions, modeset components weren't able to create debugfs
files at late_register hooks as the registration of drm_minor happens before the
registration of the modeset abstractions. So, the third patch fixes this problem
by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
catching this problem!

Apart from the third patch, the series looks similiar from its last version.

[1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
[2] https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/

Best Regards,
- Maíra Canal

---

v1 -> v2: https://lore.kernel.org/dri-devel/20221122190314.185015-1-mcanal@igalia.com/T/#t

- Fix compilation errors in the second patch (kernel test robot).
- Drop debugfs_init hook from vkms (Maíra Canal).
- Remove return values and error handling to debugfs related
functions (Jani Nikula).
- Remove entry from list after the file is created, so that drm_debugfs_init
can be called more than once (Maíra Canal).

v2 -> v3: https://lore.kernel.org/dri-devel/20221123220725.1272155-1-mcanal@igalia.com/

- Rebase on top of drm-misc-next

v3 -> v4: https://lore.kernel.org/dri-devel/20221207132325.140393-1-mcanal@igalia.com/

- Add Maxime's Reviewed-by tags
- Add the ability to create debugfs files at late_register hooks (Melissa Wen).

---

Maíra Canal (7):
  drm/debugfs: create device-centered debugfs functions
  drm: use new debugfs device-centered functions on DRM core files
  drm/debugfs: create debugfs late register functions
  drm/vc4: use new debugfs device-centered functions
  drm/v3d: use new debugfs device-centered functions
  drm/vkms: use new debugfs device-centered functions
  drm/todo: update the debugfs clean up task

 Documentation/gpu/todo.rst            |   9 +--
 drivers/gpu/drm/drm_atomic.c          |  11 ++-
 drivers/gpu/drm/drm_client.c          |  11 ++-
 drivers/gpu/drm/drm_debugfs.c         | 102 +++++++++++++++++++++++---
 drivers/gpu/drm/drm_drv.c             |   3 +
 drivers/gpu/drm/drm_framebuffer.c     |  11 ++-
 drivers/gpu/drm/drm_gem_vram_helper.c |  11 ++-
 drivers/gpu/drm/drm_internal.h        |   5 ++
 drivers/gpu/drm/drm_mode_config.c     |   2 +
 drivers/gpu/drm/v3d/v3d_debugfs.c     |  22 +++---
 drivers/gpu/drm/vc4/vc4_bo.c          |  10 +--
 drivers/gpu/drm/vc4/vc4_crtc.c        |   7 +-
 drivers/gpu/drm/vc4/vc4_debugfs.c     |  36 ++-------
 drivers/gpu/drm/vc4/vc4_dpi.c         |   5 +-
 drivers/gpu/drm/vc4/vc4_drv.c         |   1 -
 drivers/gpu/drm/vc4/vc4_drv.h         |  32 ++------
 drivers/gpu/drm/vc4/vc4_dsi.c         |   6 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c        |  12 +--
 drivers/gpu/drm/vc4/vc4_hvs.c         |  24 ++----
 drivers/gpu/drm/vc4/vc4_v3d.c         |  14 +---
 drivers/gpu/drm/vc4/vc4_vec.c         |   6 +-
 drivers/gpu/drm/vkms/vkms_drv.c       |  17 ++---
 include/drm/drm_debugfs.h             |  41 +++++++++++
 include/drm/drm_device.h              |  15 ++++
 24 files changed, 233 insertions(+), 180 deletions(-)

-- 
2.38.1


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

* [PATCH v4 1/7] drm/debugfs: create device-centered debugfs functions
  2022-12-19 12:06 ` Maíra Canal
@ 2022-12-19 12:06   ` Maíra Canal
  -1 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal,
	Wambui Karuga, Maxime Ripard

Introduce the ability to track requests for the addition of DRM debugfs
files at any time and have them added all at once during
drm_dev_register().

Drivers can add DRM debugfs files to a device-managed list and, during
drm_dev_register(), all added files will be created at once.

Now, the drivers can use the functions drm_debugfs_add_file() and
drm_debugfs_add_files() to create DRM debugfs files instead of using the
drm_debugfs_create_files() function.

Co-developed-by: Wambui Karuga <wambui.karugax@gmail.com>
Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_debugfs.c | 70 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_drv.c     |  3 ++
 include/drm/drm_debugfs.h     | 41 ++++++++++++++++++++
 include/drm/drm_device.h      | 15 ++++++++
 4 files changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index ee445f4605ba..988fc07b94b4 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -38,6 +38,7 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_file.h>
 #include <drm/drm_gem.h>
+#include <drm/drm_managed.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -151,6 +152,21 @@ static int drm_debugfs_open(struct inode *inode, struct file *file)
 	return single_open(file, node->info_ent->show, node);
 }
 
+static int drm_debugfs_entry_open(struct inode *inode, struct file *file)
+{
+	struct drm_debugfs_entry *entry = inode->i_private;
+	struct drm_debugfs_info *node = &entry->file;
+
+	return single_open(file, node->show, entry);
+}
+
+static const struct file_operations drm_debugfs_entry_fops = {
+	.owner = THIS_MODULE,
+	.open = drm_debugfs_entry_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
 
 static const struct file_operations drm_debugfs_fops = {
 	.owner = THIS_MODULE,
@@ -207,6 +223,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		     struct dentry *root)
 {
 	struct drm_device *dev = minor->dev;
+	struct drm_debugfs_entry *entry, *tmp;
 	char name[64];
 
 	INIT_LIST_HEAD(&minor->debugfs_list);
@@ -230,6 +247,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	if (dev->driver->debugfs_init)
 		dev->driver->debugfs_init(minor);
 
+	list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) {
+		debugfs_create_file(entry->file.name, S_IFREG | S_IRUGO,
+				    minor->debugfs_root, entry, &drm_debugfs_entry_fops);
+		list_del(&entry->list);
+	}
+
 	return 0;
 }
 
@@ -281,6 +304,53 @@ void drm_debugfs_cleanup(struct drm_minor *minor)
 	minor->debugfs_root = NULL;
 }
 
+/**
+ * drm_debugfs_add_file - Add a given file to the DRM device debugfs file list
+ * @dev: drm device for the ioctl
+ * @name: debugfs file name
+ * @show: show callback
+ * @data: driver-private data, should not be device-specific
+ *
+ * Add a given file entry to the DRM device debugfs file list to be created on
+ * drm_debugfs_init.
+ */
+void drm_debugfs_add_file(struct drm_device *dev, const char *name,
+			  int (*show)(struct seq_file*, void*), void *data)
+{
+	struct drm_debugfs_entry *entry = drmm_kzalloc(dev, sizeof(*entry), GFP_KERNEL);
+
+	if (!entry)
+		return;
+
+	entry->file.name = name;
+	entry->file.show = show;
+	entry->file.data = data;
+	entry->dev = dev;
+
+	mutex_lock(&dev->debugfs_mutex);
+	list_add(&entry->list, &dev->debugfs_list);
+	mutex_unlock(&dev->debugfs_mutex);
+}
+EXPORT_SYMBOL(drm_debugfs_add_file);
+
+/**
+ * drm_debugfs_add_files - Add an array of files to the DRM device debugfs file list
+ * @dev: drm device for the ioctl
+ * @files: The array of files to create
+ * @count: The number of files given
+ *
+ * Add a given set of debugfs files represented by an array of
+ * &struct drm_debugfs_info in the DRM device debugfs file list.
+ */
+void drm_debugfs_add_files(struct drm_device *dev, const struct drm_debugfs_info *files, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++)
+		drm_debugfs_add_file(dev, files[i].name, files[i].show, files[i].data);
+}
+EXPORT_SYMBOL(drm_debugfs_add_files);
+
 static int connector_show(struct seq_file *m, void *data)
 {
 	struct drm_connector *connector = m->private;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 203bf8d6c34c..c287488c776f 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -575,6 +575,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
 	mutex_destroy(&dev->clientlist_mutex);
 	mutex_destroy(&dev->filelist_mutex);
 	mutex_destroy(&dev->struct_mutex);
+	mutex_destroy(&dev->debugfs_mutex);
 	drm_legacy_destroy_members(dev);
 }
 
@@ -608,12 +609,14 @@ static int drm_dev_init(struct drm_device *dev,
 	INIT_LIST_HEAD(&dev->filelist_internal);
 	INIT_LIST_HEAD(&dev->clientlist);
 	INIT_LIST_HEAD(&dev->vblank_event_list);
+	INIT_LIST_HEAD(&dev->debugfs_list);
 
 	spin_lock_init(&dev->event_lock);
 	mutex_init(&dev->struct_mutex);
 	mutex_init(&dev->filelist_mutex);
 	mutex_init(&dev->clientlist_mutex);
 	mutex_init(&dev->master_mutex);
+	mutex_init(&dev->debugfs_mutex);
 
 	ret = drmm_add_action_or_reset(dev, drm_dev_init_release, NULL);
 	if (ret)
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
index 2188dc83957f..53b7297260a5 100644
--- a/include/drm/drm_debugfs.h
+++ b/include/drm/drm_debugfs.h
@@ -79,12 +79,43 @@ struct drm_info_node {
 	struct dentry *dent;
 };
 
+/**
+ * struct drm_debugfs_info - debugfs info list entry
+ *
+ * This structure represents a debugfs file to be created by the drm
+ * core.
+ */
+struct drm_debugfs_info {
+	const char *name;
+	int (*show)(struct seq_file*, void*);
+	u32 driver_features;
+	void *data;
+};
+
+/**
+ * struct drm_debugfs_entry - Per-device debugfs node structure
+ *
+ * This structure represents a debugfs file, as an instantiation of a &struct
+ * drm_debugfs_info on a &struct drm_device.
+ */
+struct drm_debugfs_entry {
+	struct drm_device *dev;
+	struct drm_debugfs_info file;
+	struct list_head list;
+};
+
 #if defined(CONFIG_DEBUG_FS)
 void drm_debugfs_create_files(const struct drm_info_list *files,
 			      int count, struct dentry *root,
 			      struct drm_minor *minor);
 int drm_debugfs_remove_files(const struct drm_info_list *files,
 			     int count, struct drm_minor *minor);
+
+void drm_debugfs_add_file(struct drm_device *dev, const char *name,
+			  int (*show)(struct seq_file*, void*), void *data);
+
+void drm_debugfs_add_files(struct drm_device *dev,
+			   const struct drm_debugfs_info *files, int count);
 #else
 static inline void drm_debugfs_create_files(const struct drm_info_list *files,
 					    int count, struct dentry *root,
@@ -96,6 +127,16 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
 {
 	return 0;
 }
+
+static inline void drm_debugfs_add_file(struct drm_device *dev, const char *name,
+					int (*show)(struct seq_file*, void*),
+					void *data)
+{}
+
+static inline void drm_debugfs_add_files(struct drm_device *dev,
+					 const struct drm_debugfs_info *files,
+					 int count)
+{}
 #endif
 
 #endif /* _DRM_DEBUGFS_H_ */
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 9923c7a6885e..fa6af1d57929 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -295,6 +295,21 @@ struct drm_device {
 	 */
 	struct drm_fb_helper *fb_helper;
 
+	/**
+	 * @debugfs_mutex:
+	 *
+	 * Protects &debugfs_list access.
+	 */
+	struct mutex debugfs_mutex;
+
+	/**
+	 * @debugfs_list:
+	 *
+	 * List of debugfs files to be created by the DRM device. The files
+	 * must be added during drm_dev_register().
+	 */
+	struct list_head debugfs_list;
+
 	/* Everything below here is for legacy driver, never use! */
 	/* private: */
 #if IS_ENABLED(CONFIG_DRM_LEGACY)
-- 
2.38.1


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

* [PATCH v4 1/7] drm/debugfs: create device-centered debugfs functions
@ 2022-12-19 12:06   ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen, Maxime Ripard, Wambui Karuga

Introduce the ability to track requests for the addition of DRM debugfs
files at any time and have them added all at once during
drm_dev_register().

Drivers can add DRM debugfs files to a device-managed list and, during
drm_dev_register(), all added files will be created at once.

Now, the drivers can use the functions drm_debugfs_add_file() and
drm_debugfs_add_files() to create DRM debugfs files instead of using the
drm_debugfs_create_files() function.

Co-developed-by: Wambui Karuga <wambui.karugax@gmail.com>
Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_debugfs.c | 70 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_drv.c     |  3 ++
 include/drm/drm_debugfs.h     | 41 ++++++++++++++++++++
 include/drm/drm_device.h      | 15 ++++++++
 4 files changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index ee445f4605ba..988fc07b94b4 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -38,6 +38,7 @@
 #include <drm/drm_edid.h>
 #include <drm/drm_file.h>
 #include <drm/drm_gem.h>
+#include <drm/drm_managed.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -151,6 +152,21 @@ static int drm_debugfs_open(struct inode *inode, struct file *file)
 	return single_open(file, node->info_ent->show, node);
 }
 
+static int drm_debugfs_entry_open(struct inode *inode, struct file *file)
+{
+	struct drm_debugfs_entry *entry = inode->i_private;
+	struct drm_debugfs_info *node = &entry->file;
+
+	return single_open(file, node->show, entry);
+}
+
+static const struct file_operations drm_debugfs_entry_fops = {
+	.owner = THIS_MODULE,
+	.open = drm_debugfs_entry_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
 
 static const struct file_operations drm_debugfs_fops = {
 	.owner = THIS_MODULE,
@@ -207,6 +223,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		     struct dentry *root)
 {
 	struct drm_device *dev = minor->dev;
+	struct drm_debugfs_entry *entry, *tmp;
 	char name[64];
 
 	INIT_LIST_HEAD(&minor->debugfs_list);
@@ -230,6 +247,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	if (dev->driver->debugfs_init)
 		dev->driver->debugfs_init(minor);
 
+	list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) {
+		debugfs_create_file(entry->file.name, S_IFREG | S_IRUGO,
+				    minor->debugfs_root, entry, &drm_debugfs_entry_fops);
+		list_del(&entry->list);
+	}
+
 	return 0;
 }
 
@@ -281,6 +304,53 @@ void drm_debugfs_cleanup(struct drm_minor *minor)
 	minor->debugfs_root = NULL;
 }
 
+/**
+ * drm_debugfs_add_file - Add a given file to the DRM device debugfs file list
+ * @dev: drm device for the ioctl
+ * @name: debugfs file name
+ * @show: show callback
+ * @data: driver-private data, should not be device-specific
+ *
+ * Add a given file entry to the DRM device debugfs file list to be created on
+ * drm_debugfs_init.
+ */
+void drm_debugfs_add_file(struct drm_device *dev, const char *name,
+			  int (*show)(struct seq_file*, void*), void *data)
+{
+	struct drm_debugfs_entry *entry = drmm_kzalloc(dev, sizeof(*entry), GFP_KERNEL);
+
+	if (!entry)
+		return;
+
+	entry->file.name = name;
+	entry->file.show = show;
+	entry->file.data = data;
+	entry->dev = dev;
+
+	mutex_lock(&dev->debugfs_mutex);
+	list_add(&entry->list, &dev->debugfs_list);
+	mutex_unlock(&dev->debugfs_mutex);
+}
+EXPORT_SYMBOL(drm_debugfs_add_file);
+
+/**
+ * drm_debugfs_add_files - Add an array of files to the DRM device debugfs file list
+ * @dev: drm device for the ioctl
+ * @files: The array of files to create
+ * @count: The number of files given
+ *
+ * Add a given set of debugfs files represented by an array of
+ * &struct drm_debugfs_info in the DRM device debugfs file list.
+ */
+void drm_debugfs_add_files(struct drm_device *dev, const struct drm_debugfs_info *files, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++)
+		drm_debugfs_add_file(dev, files[i].name, files[i].show, files[i].data);
+}
+EXPORT_SYMBOL(drm_debugfs_add_files);
+
 static int connector_show(struct seq_file *m, void *data)
 {
 	struct drm_connector *connector = m->private;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 203bf8d6c34c..c287488c776f 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -575,6 +575,7 @@ static void drm_dev_init_release(struct drm_device *dev, void *res)
 	mutex_destroy(&dev->clientlist_mutex);
 	mutex_destroy(&dev->filelist_mutex);
 	mutex_destroy(&dev->struct_mutex);
+	mutex_destroy(&dev->debugfs_mutex);
 	drm_legacy_destroy_members(dev);
 }
 
@@ -608,12 +609,14 @@ static int drm_dev_init(struct drm_device *dev,
 	INIT_LIST_HEAD(&dev->filelist_internal);
 	INIT_LIST_HEAD(&dev->clientlist);
 	INIT_LIST_HEAD(&dev->vblank_event_list);
+	INIT_LIST_HEAD(&dev->debugfs_list);
 
 	spin_lock_init(&dev->event_lock);
 	mutex_init(&dev->struct_mutex);
 	mutex_init(&dev->filelist_mutex);
 	mutex_init(&dev->clientlist_mutex);
 	mutex_init(&dev->master_mutex);
+	mutex_init(&dev->debugfs_mutex);
 
 	ret = drmm_add_action_or_reset(dev, drm_dev_init_release, NULL);
 	if (ret)
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
index 2188dc83957f..53b7297260a5 100644
--- a/include/drm/drm_debugfs.h
+++ b/include/drm/drm_debugfs.h
@@ -79,12 +79,43 @@ struct drm_info_node {
 	struct dentry *dent;
 };
 
+/**
+ * struct drm_debugfs_info - debugfs info list entry
+ *
+ * This structure represents a debugfs file to be created by the drm
+ * core.
+ */
+struct drm_debugfs_info {
+	const char *name;
+	int (*show)(struct seq_file*, void*);
+	u32 driver_features;
+	void *data;
+};
+
+/**
+ * struct drm_debugfs_entry - Per-device debugfs node structure
+ *
+ * This structure represents a debugfs file, as an instantiation of a &struct
+ * drm_debugfs_info on a &struct drm_device.
+ */
+struct drm_debugfs_entry {
+	struct drm_device *dev;
+	struct drm_debugfs_info file;
+	struct list_head list;
+};
+
 #if defined(CONFIG_DEBUG_FS)
 void drm_debugfs_create_files(const struct drm_info_list *files,
 			      int count, struct dentry *root,
 			      struct drm_minor *minor);
 int drm_debugfs_remove_files(const struct drm_info_list *files,
 			     int count, struct drm_minor *minor);
+
+void drm_debugfs_add_file(struct drm_device *dev, const char *name,
+			  int (*show)(struct seq_file*, void*), void *data);
+
+void drm_debugfs_add_files(struct drm_device *dev,
+			   const struct drm_debugfs_info *files, int count);
 #else
 static inline void drm_debugfs_create_files(const struct drm_info_list *files,
 					    int count, struct dentry *root,
@@ -96,6 +127,16 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
 {
 	return 0;
 }
+
+static inline void drm_debugfs_add_file(struct drm_device *dev, const char *name,
+					int (*show)(struct seq_file*, void*),
+					void *data)
+{}
+
+static inline void drm_debugfs_add_files(struct drm_device *dev,
+					 const struct drm_debugfs_info *files,
+					 int count)
+{}
 #endif
 
 #endif /* _DRM_DEBUGFS_H_ */
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 9923c7a6885e..fa6af1d57929 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -295,6 +295,21 @@ struct drm_device {
 	 */
 	struct drm_fb_helper *fb_helper;
 
+	/**
+	 * @debugfs_mutex:
+	 *
+	 * Protects &debugfs_list access.
+	 */
+	struct mutex debugfs_mutex;
+
+	/**
+	 * @debugfs_list:
+	 *
+	 * List of debugfs files to be created by the DRM device. The files
+	 * must be added during drm_dev_register().
+	 */
+	struct list_head debugfs_list;
+
 	/* Everything below here is for legacy driver, never use! */
 	/* private: */
 #if IS_ENABLED(CONFIG_DRM_LEGACY)
-- 
2.38.1


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

* [PATCH v4 2/7] drm: use new debugfs device-centered functions on DRM core files
  2022-12-19 12:06 ` Maíra Canal
@ 2022-12-19 12:06   ` Maíra Canal
  -1 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal,
	Maxime Ripard

Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function in all DRM core files, centering the
debugfs files management on the drm_device instead of drm_minor.

Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_atomic.c          | 11 +++++------
 drivers/gpu/drm/drm_client.c          | 11 +++++------
 drivers/gpu/drm/drm_debugfs.c         | 18 ++++++++----------
 drivers/gpu/drm/drm_framebuffer.c     | 11 +++++------
 drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++------
 5 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index e666799a46d5..5457c02ca1ab 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1756,8 +1756,8 @@ EXPORT_SYMBOL(drm_state_dump);
 #ifdef CONFIG_DEBUG_FS
 static int drm_state_info(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 
 	__drm_state_dump(dev, &p, true);
@@ -1766,14 +1766,13 @@ static int drm_state_info(struct seq_file *m, void *data)
 }
 
 /* any use in debugfs files to dump individual planes/crtc/etc? */
-static const struct drm_info_list drm_atomic_debugfs_list[] = {
+static const struct drm_debugfs_info drm_atomic_debugfs_list[] = {
 	{"state", drm_state_info, 0},
 };
 
 void drm_atomic_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(drm_atomic_debugfs_list,
-				 ARRAY_SIZE(drm_atomic_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_atomic_debugfs_list,
+			      ARRAY_SIZE(drm_atomic_debugfs_list));
 }
 #endif
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index fd67efe37c63..262ec64d4397 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -480,8 +480,8 @@ EXPORT_SYMBOL(drm_client_framebuffer_flush);
 #ifdef CONFIG_DEBUG_FS
 static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 	struct drm_client_dev *client;
 
@@ -493,14 +493,13 @@ static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list drm_client_debugfs_list[] = {
+static const struct drm_debugfs_info drm_client_debugfs_list[] = {
 	{ "internal_clients", drm_client_debugfs_internal_clients, 0 },
 };
 
 void drm_client_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(drm_client_debugfs_list,
-				 ARRAY_SIZE(drm_client_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_client_debugfs_list,
+			      ARRAY_SIZE(drm_client_debugfs_list));
 }
 #endif
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 988fc07b94b4..d9d3ed7acc80 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -51,9 +51,8 @@
 
 static int drm_name_info(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_minor *minor = node->minor;
-	struct drm_device *dev = minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_master *master;
 
 	mutex_lock(&dev->master_mutex);
@@ -73,8 +72,8 @@ static int drm_name_info(struct seq_file *m, void *data)
 
 static int drm_clients_info(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_file *priv;
 	kuid_t uid;
 
@@ -125,8 +124,8 @@ static int drm_gem_one_name_info(int id, void *ptr, void *data)
 
 static int drm_gem_name_info(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 
 	seq_printf(m, "  name     size handles refcount\n");
 
@@ -137,7 +136,7 @@ static int drm_gem_name_info(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list drm_debugfs_list[] = {
+static const struct drm_debugfs_info drm_debugfs_list[] = {
 	{"name", drm_name_info, 0},
 	{"clients", drm_clients_info, 0},
 	{"gem_names", drm_gem_name_info, DRIVER_GEM},
@@ -231,8 +230,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	sprintf(name, "%d", minor_id);
 	minor->debugfs_root = debugfs_create_dir(name, root);
 
-	drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_debugfs_list, DRM_DEBUGFS_ENTRIES);
 
 	if (drm_drv_uses_atomic_modeset(dev)) {
 		drm_atomic_debugfs_init(minor);
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 2dd97473ca10..aff3746dedfb 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -1203,8 +1203,8 @@ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
 #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_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 	struct drm_framebuffer *fb;
 
@@ -1218,14 +1218,13 @@ static int drm_framebuffer_info(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list drm_framebuffer_debugfs_list[] = {
+static const struct drm_debugfs_info drm_framebuffer_debugfs_list[] = {
 	{ "framebuffer", drm_framebuffer_info, 0 },
 };
 
 void drm_framebuffer_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(drm_framebuffer_debugfs_list,
-				 ARRAY_SIZE(drm_framebuffer_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_framebuffer_debugfs_list,
+			      ARRAY_SIZE(drm_framebuffer_debugfs_list));
 }
 #endif
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index f59adffd938a..d40b3edb52d0 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -957,8 +957,8 @@ static struct ttm_device_funcs bo_driver = {
 
 static int drm_vram_mm_debugfs(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_vram_mm *vmm = node->minor->dev->vram_mm;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_vram_mm *vmm = entry->dev->vram_mm;
 	struct ttm_resource_manager *man = ttm_manager_type(&vmm->bdev, TTM_PL_VRAM);
 	struct drm_printer p = drm_seq_file_printer(m);
 
@@ -966,7 +966,7 @@ static int drm_vram_mm_debugfs(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
+static const struct drm_debugfs_info drm_vram_mm_debugfs_list[] = {
 	{ "vram-mm", drm_vram_mm_debugfs, 0, NULL },
 };
 
@@ -978,9 +978,8 @@ static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
  */
 void drm_vram_mm_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(drm_vram_mm_debugfs_list,
-				 ARRAY_SIZE(drm_vram_mm_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_vram_mm_debugfs_list,
+			      ARRAY_SIZE(drm_vram_mm_debugfs_list));
 }
 EXPORT_SYMBOL(drm_vram_mm_debugfs_init);
 
-- 
2.38.1


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

* [PATCH v4 2/7] drm: use new debugfs device-centered functions on DRM core files
@ 2022-12-19 12:06   ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen, Maxime Ripard

Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function in all DRM core files, centering the
debugfs files management on the drm_device instead of drm_minor.

Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_atomic.c          | 11 +++++------
 drivers/gpu/drm/drm_client.c          | 11 +++++------
 drivers/gpu/drm/drm_debugfs.c         | 18 ++++++++----------
 drivers/gpu/drm/drm_framebuffer.c     | 11 +++++------
 drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++------
 5 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index e666799a46d5..5457c02ca1ab 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1756,8 +1756,8 @@ EXPORT_SYMBOL(drm_state_dump);
 #ifdef CONFIG_DEBUG_FS
 static int drm_state_info(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 
 	__drm_state_dump(dev, &p, true);
@@ -1766,14 +1766,13 @@ static int drm_state_info(struct seq_file *m, void *data)
 }
 
 /* any use in debugfs files to dump individual planes/crtc/etc? */
-static const struct drm_info_list drm_atomic_debugfs_list[] = {
+static const struct drm_debugfs_info drm_atomic_debugfs_list[] = {
 	{"state", drm_state_info, 0},
 };
 
 void drm_atomic_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(drm_atomic_debugfs_list,
-				 ARRAY_SIZE(drm_atomic_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_atomic_debugfs_list,
+			      ARRAY_SIZE(drm_atomic_debugfs_list));
 }
 #endif
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index fd67efe37c63..262ec64d4397 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -480,8 +480,8 @@ EXPORT_SYMBOL(drm_client_framebuffer_flush);
 #ifdef CONFIG_DEBUG_FS
 static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 	struct drm_client_dev *client;
 
@@ -493,14 +493,13 @@ static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list drm_client_debugfs_list[] = {
+static const struct drm_debugfs_info drm_client_debugfs_list[] = {
 	{ "internal_clients", drm_client_debugfs_internal_clients, 0 },
 };
 
 void drm_client_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(drm_client_debugfs_list,
-				 ARRAY_SIZE(drm_client_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_client_debugfs_list,
+			      ARRAY_SIZE(drm_client_debugfs_list));
 }
 #endif
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 988fc07b94b4..d9d3ed7acc80 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -51,9 +51,8 @@
 
 static int drm_name_info(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_minor *minor = node->minor;
-	struct drm_device *dev = minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_master *master;
 
 	mutex_lock(&dev->master_mutex);
@@ -73,8 +72,8 @@ static int drm_name_info(struct seq_file *m, void *data)
 
 static int drm_clients_info(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_file *priv;
 	kuid_t uid;
 
@@ -125,8 +124,8 @@ static int drm_gem_one_name_info(int id, void *ptr, void *data)
 
 static int drm_gem_name_info(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 
 	seq_printf(m, "  name     size handles refcount\n");
 
@@ -137,7 +136,7 @@ static int drm_gem_name_info(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list drm_debugfs_list[] = {
+static const struct drm_debugfs_info drm_debugfs_list[] = {
 	{"name", drm_name_info, 0},
 	{"clients", drm_clients_info, 0},
 	{"gem_names", drm_gem_name_info, DRIVER_GEM},
@@ -231,8 +230,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	sprintf(name, "%d", minor_id);
 	minor->debugfs_root = debugfs_create_dir(name, root);
 
-	drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_debugfs_list, DRM_DEBUGFS_ENTRIES);
 
 	if (drm_drv_uses_atomic_modeset(dev)) {
 		drm_atomic_debugfs_init(minor);
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 2dd97473ca10..aff3746dedfb 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -1203,8 +1203,8 @@ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
 #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_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 	struct drm_framebuffer *fb;
 
@@ -1218,14 +1218,13 @@ static int drm_framebuffer_info(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list drm_framebuffer_debugfs_list[] = {
+static const struct drm_debugfs_info drm_framebuffer_debugfs_list[] = {
 	{ "framebuffer", drm_framebuffer_info, 0 },
 };
 
 void drm_framebuffer_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(drm_framebuffer_debugfs_list,
-				 ARRAY_SIZE(drm_framebuffer_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_framebuffer_debugfs_list,
+			      ARRAY_SIZE(drm_framebuffer_debugfs_list));
 }
 #endif
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index f59adffd938a..d40b3edb52d0 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -957,8 +957,8 @@ static struct ttm_device_funcs bo_driver = {
 
 static int drm_vram_mm_debugfs(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct drm_vram_mm *vmm = node->minor->dev->vram_mm;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_vram_mm *vmm = entry->dev->vram_mm;
 	struct ttm_resource_manager *man = ttm_manager_type(&vmm->bdev, TTM_PL_VRAM);
 	struct drm_printer p = drm_seq_file_printer(m);
 
@@ -966,7 +966,7 @@ static int drm_vram_mm_debugfs(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
+static const struct drm_debugfs_info drm_vram_mm_debugfs_list[] = {
 	{ "vram-mm", drm_vram_mm_debugfs, 0, NULL },
 };
 
@@ -978,9 +978,8 @@ static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
  */
 void drm_vram_mm_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(drm_vram_mm_debugfs_list,
-				 ARRAY_SIZE(drm_vram_mm_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, drm_vram_mm_debugfs_list,
+			      ARRAY_SIZE(drm_vram_mm_debugfs_list));
 }
 EXPORT_SYMBOL(drm_vram_mm_debugfs_init);
 
-- 
2.38.1


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

* [PATCH v4 3/7] drm/debugfs: create debugfs late register functions
  2022-12-19 12:06 ` Maíra Canal
@ 2022-12-19 12:06   ` Maíra Canal
  -1 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal

Although the device-centered debugfs functions can track requests for
the addition of DRM debugfs files at any time and have them added all
at once during drm_dev_register(), they are not able to create debugfs
files for modeset components, as they are registered after the primary
and the render drm_minor are registered.

So, create a drm_debugfs_late_register() function, which is responsible
for dealing with the creation of all the debugfs files for modeset
components at once. Therefore, the functions drm_debugfs_add_file()
and drm_debugfs_add_files() can be used in late_register hooks.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_debugfs.c     | 14 ++++++++++++++
 drivers/gpu/drm/drm_internal.h    |  5 +++++
 drivers/gpu/drm/drm_mode_config.c |  2 ++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index d9d3ed7acc80..51e3772e2e2b 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -254,6 +254,20 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	return 0;
 }
 
+void drm_debugfs_late_register(struct drm_device *dev)
+{
+	struct drm_minor *minor = dev->primary;
+	struct drm_debugfs_entry *entry, *tmp;
+
+	if (minor == NULL)
+		return;
+
+	list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) {
+		debugfs_create_file(entry->file.name, S_IFREG | S_IRUGO,
+				    minor->debugfs_root, entry, &drm_debugfs_entry_fops);
+		list_del(&entry->list);
+	}
+}
 
 int drm_debugfs_remove_files(const struct drm_info_list *files, int count,
 			     struct drm_minor *minor)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 5ea5e260118c..ed2103ee272c 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -186,6 +186,7 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev,
 int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		     struct dentry *root);
 void drm_debugfs_cleanup(struct drm_minor *minor);
+void drm_debugfs_late_register(struct drm_device *dev);
 void drm_debugfs_connector_add(struct drm_connector *connector);
 void drm_debugfs_connector_remove(struct drm_connector *connector);
 void drm_debugfs_crtc_add(struct drm_crtc *crtc);
@@ -202,6 +203,10 @@ static inline void drm_debugfs_cleanup(struct drm_minor *minor)
 {
 }
 
+static inline void drm_debugfs_late_register(struct drm_device *dev)
+{
+}
+
 static inline void drm_debugfs_connector_add(struct drm_connector *connector)
 {
 }
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 8525ef851540..87eb591fe9b5 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -54,6 +54,8 @@ int drm_modeset_register_all(struct drm_device *dev)
 	if (ret)
 		goto err_connector;
 
+	drm_debugfs_late_register(dev);
+
 	return 0;
 
 err_connector:
-- 
2.38.1


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

* [PATCH v4 3/7] drm/debugfs: create debugfs late register functions
@ 2022-12-19 12:06   ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen

Although the device-centered debugfs functions can track requests for
the addition of DRM debugfs files at any time and have them added all
at once during drm_dev_register(), they are not able to create debugfs
files for modeset components, as they are registered after the primary
and the render drm_minor are registered.

So, create a drm_debugfs_late_register() function, which is responsible
for dealing with the creation of all the debugfs files for modeset
components at once. Therefore, the functions drm_debugfs_add_file()
and drm_debugfs_add_files() can be used in late_register hooks.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_debugfs.c     | 14 ++++++++++++++
 drivers/gpu/drm/drm_internal.h    |  5 +++++
 drivers/gpu/drm/drm_mode_config.c |  2 ++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index d9d3ed7acc80..51e3772e2e2b 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -254,6 +254,20 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	return 0;
 }
 
+void drm_debugfs_late_register(struct drm_device *dev)
+{
+	struct drm_minor *minor = dev->primary;
+	struct drm_debugfs_entry *entry, *tmp;
+
+	if (minor == NULL)
+		return;
+
+	list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) {
+		debugfs_create_file(entry->file.name, S_IFREG | S_IRUGO,
+				    minor->debugfs_root, entry, &drm_debugfs_entry_fops);
+		list_del(&entry->list);
+	}
+}
 
 int drm_debugfs_remove_files(const struct drm_info_list *files, int count,
 			     struct drm_minor *minor)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 5ea5e260118c..ed2103ee272c 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -186,6 +186,7 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev,
 int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 		     struct dentry *root);
 void drm_debugfs_cleanup(struct drm_minor *minor);
+void drm_debugfs_late_register(struct drm_device *dev);
 void drm_debugfs_connector_add(struct drm_connector *connector);
 void drm_debugfs_connector_remove(struct drm_connector *connector);
 void drm_debugfs_crtc_add(struct drm_crtc *crtc);
@@ -202,6 +203,10 @@ static inline void drm_debugfs_cleanup(struct drm_minor *minor)
 {
 }
 
+static inline void drm_debugfs_late_register(struct drm_device *dev)
+{
+}
+
 static inline void drm_debugfs_connector_add(struct drm_connector *connector)
 {
 }
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 8525ef851540..87eb591fe9b5 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -54,6 +54,8 @@ int drm_modeset_register_all(struct drm_device *dev)
 	if (ret)
 		goto err_connector;
 
+	drm_debugfs_late_register(dev);
+
 	return 0;
 
 err_connector:
-- 
2.38.1


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

* [PATCH v4 4/7] drm/vc4: use new debugfs device-centered functions
  2022-12-19 12:06 ` Maíra Canal
@ 2022-12-19 12:06   ` Maíra Canal
  -1 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal,
	Maxime Ripard

Currently, vc4 has its own debugfs infrastructure that adds the debugfs
files on drm_dev_register(). With the introduction of the new debugfs,
functions, replace the vc4 debugfs structure with the DRM debugfs
device-centered function, drm_debugfs_add_file().

Moreover, remove the explicit error handling of debugfs related functions,
considering that the only failure mode is -ENOMEM and also that error
handling is not recommended for debugfs functions, as pointed out in [1].

[1] https://lore.kernel.org/all/YWAmZdRwnAt6wh9B@kroah.com/

Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_bo.c      | 10 +++------
 drivers/gpu/drm/vc4/vc4_crtc.c    |  7 ++----
 drivers/gpu/drm/vc4/vc4_debugfs.c | 36 ++++++-------------------------
 drivers/gpu/drm/vc4/vc4_dpi.c     |  5 +----
 drivers/gpu/drm/vc4/vc4_drv.c     |  1 -
 drivers/gpu/drm/vc4/vc4_drv.h     | 32 ++++++---------------------
 drivers/gpu/drm/vc4/vc4_dsi.c     |  6 +-----
 drivers/gpu/drm/vc4/vc4_hdmi.c    | 12 ++++-------
 drivers/gpu/drm/vc4/vc4_hvs.c     | 24 ++++++---------------
 drivers/gpu/drm/vc4/vc4_v3d.c     | 14 ++++--------
 drivers/gpu/drm/vc4/vc4_vec.c     |  6 +-----
 11 files changed, 37 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 43d9b3a6a352..c2b7573bd92b 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -69,8 +69,8 @@ static void vc4_bo_stats_print(struct drm_printer *p, struct vc4_dev *vc4)
 
 static int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct drm_printer p = drm_seq_file_printer(m);
 
@@ -993,15 +993,11 @@ int vc4_bo_debugfs_init(struct drm_minor *minor)
 {
 	struct drm_device *drm = minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(drm);
-	int ret;
 
 	if (!vc4->v3d)
 		return -ENODEV;
 
-	ret = vc4_debugfs_add_file(minor, "bo_stats",
-				   vc4_bo_stats_debugfs, NULL);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, "bo_stats", vc4_bo_stats_debugfs, NULL);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index a1a3465948c4..35a6b5907278 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1090,12 +1090,9 @@ int vc4_crtc_late_register(struct drm_crtc *crtc)
 	struct drm_device *drm = crtc->dev;
 	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
 	const struct vc4_crtc_data *crtc_data = vc4_crtc_to_vc4_crtc_data(vc4_crtc);
-	int ret;
 
-	ret = vc4_debugfs_add_regset32(drm->primary, crtc_data->debugfs_name,
-				       &vc4_crtc->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, crtc_data->debugfs_name,
+				 &vc4_crtc->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
index 19cda4f91a82..fac624a663ea 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -34,9 +34,9 @@ vc4_debugfs_init(struct drm_minor *minor)
 
 static int vc4_debugfs_regset32(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *drm = node->minor->dev;
-	struct debugfs_regset32 *regset = node->info_ent->data;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *drm = entry->dev;
+	struct debugfs_regset32 *regset = entry->file.data;
 	struct drm_printer p = drm_seq_file_printer(m);
 	int idx;
 
@@ -50,31 +50,9 @@ static int vc4_debugfs_regset32(struct seq_file *m, void *unused)
 	return 0;
 }
 
-int vc4_debugfs_add_file(struct drm_minor *minor,
-			 const char *name,
-			 int (*show)(struct seq_file*, void*),
-			 void *data)
+void vc4_debugfs_add_regset32(struct drm_device *drm,
+			      const char *name,
+			      struct debugfs_regset32 *regset)
 {
-	struct drm_device *dev = minor->dev;
-	struct dentry *root = minor->debugfs_root;
-	struct drm_info_list *file;
-
-	file = drmm_kzalloc(dev, sizeof(*file), GFP_KERNEL);
-	if (!file)
-		return -ENOMEM;
-
-	file->name = name;
-	file->show = show;
-	file->data = data;
-
-	drm_debugfs_create_files(file, 1, root, minor);
-
-	return 0;
-}
-
-int vc4_debugfs_add_regset32(struct drm_minor *minor,
-			     const char *name,
-			     struct debugfs_regset32 *regset)
-{
-	return vc4_debugfs_add_file(minor, name, vc4_debugfs_regset32, regset);
+	drm_debugfs_add_file(drm, name, vc4_debugfs_regset32, regset);
 }
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index d31a02af1e53..f518d6e59ed6 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -267,11 +267,8 @@ static int vc4_dpi_late_register(struct drm_encoder *encoder)
 {
 	struct drm_device *drm = encoder->dev;
 	struct vc4_dpi *dpi = to_vc4_dpi(encoder);
-	int ret;
 
-	ret = vc4_debugfs_add_regset32(drm->primary, "dpi_regs", &dpi->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, "dpi_regs", &dpi->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 3b0ae2c3e33c..0ccaee57fe9a 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -320,7 +320,6 @@ static int vc4_drm_bind(struct device *dev)
 
 	drm = &vc4->base;
 	platform_set_drvdata(pdev, drm);
-	INIT_LIST_HEAD(&vc4->debugfs_list);
 
 	if (!is_vc5) {
 		ret = drmm_mutex_init(drm, &vc4->bin_bo_lock);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 78fda5332cb3..95069bb16821 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -226,11 +226,6 @@ struct vc4_dev {
 	struct drm_private_obj hvs_channels;
 	struct drm_private_obj load_tracker;
 
-	/* List of vc4_debugfs_info_entry for adding to debugfs once
-	 * the minor is available (after drm_dev_register()).
-	 */
-	struct list_head debugfs_list;
-
 	/* Mutex for binner bo allocation. */
 	struct mutex bin_bo_lock;
 	/* Reference count for our binner bo. */
@@ -971,28 +966,15 @@ void vc4_crtc_get_margins(struct drm_crtc_state *state,
 /* vc4_debugfs.c */
 void vc4_debugfs_init(struct drm_minor *minor);
 #ifdef CONFIG_DEBUG_FS
-int vc4_debugfs_add_file(struct drm_minor *minor,
-			 const char *filename,
-			 int (*show)(struct seq_file*, void*),
-			 void *data);
-int vc4_debugfs_add_regset32(struct drm_minor *minor,
-			     const char *filename,
-			     struct debugfs_regset32 *regset);
+void vc4_debugfs_add_regset32(struct drm_device *drm,
+			      const char *filename,
+			      struct debugfs_regset32 *regset);
 #else
-static inline int vc4_debugfs_add_file(struct drm_minor *minor,
-				       const char *filename,
-				       int (*show)(struct seq_file*, void*),
-				       void *data)
-{
-	return 0;
-}
 
-static inline int vc4_debugfs_add_regset32(struct drm_minor *minor,
-					   const char *filename,
-					   struct debugfs_regset32 *regset)
-{
-	return 0;
-}
+static inline void vc4_debugfs_add_regset32(struct drm_device *drm,
+					    const char *filename,
+					    struct debugfs_regset32 *regset)
+{}
 #endif
 
 /* vc4_drv.c */
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 0ee9ada428ab..e05be9a34156 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -1427,12 +1427,8 @@ static int vc4_dsi_late_register(struct drm_encoder *encoder)
 {
 	struct drm_device *drm = encoder->dev;
 	struct vc4_dsi *dsi = to_vc4_dsi(encoder);
-	int ret;
 
-	ret = vc4_debugfs_add_regset32(drm->primary, dsi->variant->debugfs_name,
-				       &dsi->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, dsi->variant->debugfs_name, &dsi->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 12a00d644b61..0d3313c71f2f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -160,8 +160,8 @@ static bool vc4_hdmi_is_full_range_rgb(struct vc4_hdmi *vc4_hdmi,
 
 static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct vc4_hdmi *vc4_hdmi = node->info_ent->data;
+	struct drm_debugfs_entry *entry = m->private;
+	struct vc4_hdmi *vc4_hdmi = entry->file.data;
 	struct drm_device *drm = vc4_hdmi->connector.dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 	int idx;
@@ -1995,13 +1995,9 @@ static int vc4_hdmi_late_register(struct drm_encoder *encoder)
 	struct drm_device *drm = encoder->dev;
 	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 	const struct vc4_hdmi_variant *variant = vc4_hdmi->variant;
-	int ret;
 
-	ret = vc4_debugfs_add_file(drm->primary, variant->debugfs_name,
-				   vc4_hdmi_debugfs_regs,
-				   vc4_hdmi);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, variant->debugfs_name,
+			     vc4_hdmi_debugfs_regs, vc4_hdmi);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 94c29f8547bb..024a2cdff5b2 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -93,8 +93,8 @@ void vc4_hvs_dump_state(struct vc4_hvs *hvs)
 
 static int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct drm_printer p = drm_seq_file_printer(m);
 
@@ -105,8 +105,8 @@ static int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data)
 
 static int vc4_hvs_debugfs_dlist(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct vc4_hvs *hvs = vc4->hvs;
 	struct drm_printer p = drm_seq_file_printer(m);
@@ -740,7 +740,6 @@ int vc4_hvs_debugfs_init(struct drm_minor *minor)
 	struct drm_device *drm = minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	struct vc4_hvs *hvs = vc4->hvs;
-	int ret;
 
 	if (!vc4->hvs)
 		return -ENODEV;
@@ -750,20 +749,11 @@ int vc4_hvs_debugfs_init(struct drm_minor *minor)
 				    minor->debugfs_root,
 				    &vc4->load_tracker_enabled);
 
-	ret = vc4_debugfs_add_file(minor, "hvs_dlists",
-				   vc4_hvs_debugfs_dlist, NULL);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, "hvs_dlists", vc4_hvs_debugfs_dlist, NULL);
 
-	ret = vc4_debugfs_add_file(minor, "hvs_underrun",
-				   vc4_hvs_debugfs_underrun, NULL);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, "hvs_underrun", vc4_hvs_debugfs_underrun, NULL);
 
-	ret = vc4_debugfs_add_regset32(minor, "hvs_regs",
-				       &hvs->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, "hvs_regs", &hvs->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 56abb0d6bc39..29a664c8bf44 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -96,8 +96,8 @@ static const struct debugfs_reg32 v3d_regs[] = {
 
 static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	int ret = vc4_v3d_pm_get(vc4);
 
@@ -404,19 +404,13 @@ int vc4_v3d_debugfs_init(struct drm_minor *minor)
 	struct drm_device *drm = minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	struct vc4_v3d *v3d = vc4->v3d;
-	int ret;
 
 	if (!vc4->v3d)
 		return -ENODEV;
 
-	ret = vc4_debugfs_add_file(minor, "v3d_ident",
-				   vc4_v3d_debugfs_ident, NULL);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, "v3d_ident", vc4_v3d_debugfs_ident, NULL);
 
-	ret = vc4_debugfs_add_regset32(minor, "v3d_regs", &v3d->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, "v3d_regs", &v3d->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index f589ab918f4d..d8bf00b442e9 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -716,12 +716,8 @@ static int vc4_vec_late_register(struct drm_encoder *encoder)
 {
 	struct drm_device *drm = encoder->dev;
 	struct vc4_vec *vec = encoder_to_vc4_vec(encoder);
-	int ret;
 
-	ret = vc4_debugfs_add_regset32(drm->primary, "vec_regs",
-				       &vec->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, "vec_regs", &vec->regset);
 
 	return 0;
 }
-- 
2.38.1


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

* [PATCH v4 4/7] drm/vc4: use new debugfs device-centered functions
@ 2022-12-19 12:06   ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen, Maxime Ripard

Currently, vc4 has its own debugfs infrastructure that adds the debugfs
files on drm_dev_register(). With the introduction of the new debugfs,
functions, replace the vc4 debugfs structure with the DRM debugfs
device-centered function, drm_debugfs_add_file().

Moreover, remove the explicit error handling of debugfs related functions,
considering that the only failure mode is -ENOMEM and also that error
handling is not recommended for debugfs functions, as pointed out in [1].

[1] https://lore.kernel.org/all/YWAmZdRwnAt6wh9B@kroah.com/

Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vc4/vc4_bo.c      | 10 +++------
 drivers/gpu/drm/vc4/vc4_crtc.c    |  7 ++----
 drivers/gpu/drm/vc4/vc4_debugfs.c | 36 ++++++-------------------------
 drivers/gpu/drm/vc4/vc4_dpi.c     |  5 +----
 drivers/gpu/drm/vc4/vc4_drv.c     |  1 -
 drivers/gpu/drm/vc4/vc4_drv.h     | 32 ++++++---------------------
 drivers/gpu/drm/vc4/vc4_dsi.c     |  6 +-----
 drivers/gpu/drm/vc4/vc4_hdmi.c    | 12 ++++-------
 drivers/gpu/drm/vc4/vc4_hvs.c     | 24 ++++++---------------
 drivers/gpu/drm/vc4/vc4_v3d.c     | 14 ++++--------
 drivers/gpu/drm/vc4/vc4_vec.c     |  6 +-----
 11 files changed, 37 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 43d9b3a6a352..c2b7573bd92b 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -69,8 +69,8 @@ static void vc4_bo_stats_print(struct drm_printer *p, struct vc4_dev *vc4)
 
 static int vc4_bo_stats_debugfs(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct drm_printer p = drm_seq_file_printer(m);
 
@@ -993,15 +993,11 @@ int vc4_bo_debugfs_init(struct drm_minor *minor)
 {
 	struct drm_device *drm = minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(drm);
-	int ret;
 
 	if (!vc4->v3d)
 		return -ENODEV;
 
-	ret = vc4_debugfs_add_file(minor, "bo_stats",
-				   vc4_bo_stats_debugfs, NULL);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, "bo_stats", vc4_bo_stats_debugfs, NULL);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index a1a3465948c4..35a6b5907278 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -1090,12 +1090,9 @@ int vc4_crtc_late_register(struct drm_crtc *crtc)
 	struct drm_device *drm = crtc->dev;
 	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
 	const struct vc4_crtc_data *crtc_data = vc4_crtc_to_vc4_crtc_data(vc4_crtc);
-	int ret;
 
-	ret = vc4_debugfs_add_regset32(drm->primary, crtc_data->debugfs_name,
-				       &vc4_crtc->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, crtc_data->debugfs_name,
+				 &vc4_crtc->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
index 19cda4f91a82..fac624a663ea 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -34,9 +34,9 @@ vc4_debugfs_init(struct drm_minor *minor)
 
 static int vc4_debugfs_regset32(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *drm = node->minor->dev;
-	struct debugfs_regset32 *regset = node->info_ent->data;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *drm = entry->dev;
+	struct debugfs_regset32 *regset = entry->file.data;
 	struct drm_printer p = drm_seq_file_printer(m);
 	int idx;
 
@@ -50,31 +50,9 @@ static int vc4_debugfs_regset32(struct seq_file *m, void *unused)
 	return 0;
 }
 
-int vc4_debugfs_add_file(struct drm_minor *minor,
-			 const char *name,
-			 int (*show)(struct seq_file*, void*),
-			 void *data)
+void vc4_debugfs_add_regset32(struct drm_device *drm,
+			      const char *name,
+			      struct debugfs_regset32 *regset)
 {
-	struct drm_device *dev = minor->dev;
-	struct dentry *root = minor->debugfs_root;
-	struct drm_info_list *file;
-
-	file = drmm_kzalloc(dev, sizeof(*file), GFP_KERNEL);
-	if (!file)
-		return -ENOMEM;
-
-	file->name = name;
-	file->show = show;
-	file->data = data;
-
-	drm_debugfs_create_files(file, 1, root, minor);
-
-	return 0;
-}
-
-int vc4_debugfs_add_regset32(struct drm_minor *minor,
-			     const char *name,
-			     struct debugfs_regset32 *regset)
-{
-	return vc4_debugfs_add_file(minor, name, vc4_debugfs_regset32, regset);
+	drm_debugfs_add_file(drm, name, vc4_debugfs_regset32, regset);
 }
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index d31a02af1e53..f518d6e59ed6 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -267,11 +267,8 @@ static int vc4_dpi_late_register(struct drm_encoder *encoder)
 {
 	struct drm_device *drm = encoder->dev;
 	struct vc4_dpi *dpi = to_vc4_dpi(encoder);
-	int ret;
 
-	ret = vc4_debugfs_add_regset32(drm->primary, "dpi_regs", &dpi->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, "dpi_regs", &dpi->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 3b0ae2c3e33c..0ccaee57fe9a 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -320,7 +320,6 @@ static int vc4_drm_bind(struct device *dev)
 
 	drm = &vc4->base;
 	platform_set_drvdata(pdev, drm);
-	INIT_LIST_HEAD(&vc4->debugfs_list);
 
 	if (!is_vc5) {
 		ret = drmm_mutex_init(drm, &vc4->bin_bo_lock);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 78fda5332cb3..95069bb16821 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -226,11 +226,6 @@ struct vc4_dev {
 	struct drm_private_obj hvs_channels;
 	struct drm_private_obj load_tracker;
 
-	/* List of vc4_debugfs_info_entry for adding to debugfs once
-	 * the minor is available (after drm_dev_register()).
-	 */
-	struct list_head debugfs_list;
-
 	/* Mutex for binner bo allocation. */
 	struct mutex bin_bo_lock;
 	/* Reference count for our binner bo. */
@@ -971,28 +966,15 @@ void vc4_crtc_get_margins(struct drm_crtc_state *state,
 /* vc4_debugfs.c */
 void vc4_debugfs_init(struct drm_minor *minor);
 #ifdef CONFIG_DEBUG_FS
-int vc4_debugfs_add_file(struct drm_minor *minor,
-			 const char *filename,
-			 int (*show)(struct seq_file*, void*),
-			 void *data);
-int vc4_debugfs_add_regset32(struct drm_minor *minor,
-			     const char *filename,
-			     struct debugfs_regset32 *regset);
+void vc4_debugfs_add_regset32(struct drm_device *drm,
+			      const char *filename,
+			      struct debugfs_regset32 *regset);
 #else
-static inline int vc4_debugfs_add_file(struct drm_minor *minor,
-				       const char *filename,
-				       int (*show)(struct seq_file*, void*),
-				       void *data)
-{
-	return 0;
-}
 
-static inline int vc4_debugfs_add_regset32(struct drm_minor *minor,
-					   const char *filename,
-					   struct debugfs_regset32 *regset)
-{
-	return 0;
-}
+static inline void vc4_debugfs_add_regset32(struct drm_device *drm,
+					    const char *filename,
+					    struct debugfs_regset32 *regset)
+{}
 #endif
 
 /* vc4_drv.c */
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 0ee9ada428ab..e05be9a34156 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -1427,12 +1427,8 @@ static int vc4_dsi_late_register(struct drm_encoder *encoder)
 {
 	struct drm_device *drm = encoder->dev;
 	struct vc4_dsi *dsi = to_vc4_dsi(encoder);
-	int ret;
 
-	ret = vc4_debugfs_add_regset32(drm->primary, dsi->variant->debugfs_name,
-				       &dsi->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, dsi->variant->debugfs_name, &dsi->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 12a00d644b61..0d3313c71f2f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -160,8 +160,8 @@ static bool vc4_hdmi_is_full_range_rgb(struct vc4_hdmi *vc4_hdmi,
 
 static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct vc4_hdmi *vc4_hdmi = node->info_ent->data;
+	struct drm_debugfs_entry *entry = m->private;
+	struct vc4_hdmi *vc4_hdmi = entry->file.data;
 	struct drm_device *drm = vc4_hdmi->connector.dev;
 	struct drm_printer p = drm_seq_file_printer(m);
 	int idx;
@@ -1995,13 +1995,9 @@ static int vc4_hdmi_late_register(struct drm_encoder *encoder)
 	struct drm_device *drm = encoder->dev;
 	struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
 	const struct vc4_hdmi_variant *variant = vc4_hdmi->variant;
-	int ret;
 
-	ret = vc4_debugfs_add_file(drm->primary, variant->debugfs_name,
-				   vc4_hdmi_debugfs_regs,
-				   vc4_hdmi);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, variant->debugfs_name,
+			     vc4_hdmi_debugfs_regs, vc4_hdmi);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 94c29f8547bb..024a2cdff5b2 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -93,8 +93,8 @@ void vc4_hvs_dump_state(struct vc4_hvs *hvs)
 
 static int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct drm_printer p = drm_seq_file_printer(m);
 
@@ -105,8 +105,8 @@ static int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data)
 
 static int vc4_hvs_debugfs_dlist(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	struct vc4_hvs *hvs = vc4->hvs;
 	struct drm_printer p = drm_seq_file_printer(m);
@@ -740,7 +740,6 @@ int vc4_hvs_debugfs_init(struct drm_minor *minor)
 	struct drm_device *drm = minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	struct vc4_hvs *hvs = vc4->hvs;
-	int ret;
 
 	if (!vc4->hvs)
 		return -ENODEV;
@@ -750,20 +749,11 @@ int vc4_hvs_debugfs_init(struct drm_minor *minor)
 				    minor->debugfs_root,
 				    &vc4->load_tracker_enabled);
 
-	ret = vc4_debugfs_add_file(minor, "hvs_dlists",
-				   vc4_hvs_debugfs_dlist, NULL);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, "hvs_dlists", vc4_hvs_debugfs_dlist, NULL);
 
-	ret = vc4_debugfs_add_file(minor, "hvs_underrun",
-				   vc4_hvs_debugfs_underrun, NULL);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, "hvs_underrun", vc4_hvs_debugfs_underrun, NULL);
 
-	ret = vc4_debugfs_add_regset32(minor, "hvs_regs",
-				       &hvs->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, "hvs_regs", &hvs->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index 56abb0d6bc39..29a664c8bf44 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -96,8 +96,8 @@ static const struct debugfs_reg32 v3d_regs[] = {
 
 static int vc4_v3d_debugfs_ident(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	int ret = vc4_v3d_pm_get(vc4);
 
@@ -404,19 +404,13 @@ int vc4_v3d_debugfs_init(struct drm_minor *minor)
 	struct drm_device *drm = minor->dev;
 	struct vc4_dev *vc4 = to_vc4_dev(drm);
 	struct vc4_v3d *v3d = vc4->v3d;
-	int ret;
 
 	if (!vc4->v3d)
 		return -ENODEV;
 
-	ret = vc4_debugfs_add_file(minor, "v3d_ident",
-				   vc4_v3d_debugfs_ident, NULL);
-	if (ret)
-		return ret;
+	drm_debugfs_add_file(drm, "v3d_ident", vc4_v3d_debugfs_ident, NULL);
 
-	ret = vc4_debugfs_add_regset32(minor, "v3d_regs", &v3d->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, "v3d_regs", &v3d->regset);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index f589ab918f4d..d8bf00b442e9 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -716,12 +716,8 @@ static int vc4_vec_late_register(struct drm_encoder *encoder)
 {
 	struct drm_device *drm = encoder->dev;
 	struct vc4_vec *vec = encoder_to_vc4_vec(encoder);
-	int ret;
 
-	ret = vc4_debugfs_add_regset32(drm->primary, "vec_regs",
-				       &vec->regset);
-	if (ret)
-		return ret;
+	vc4_debugfs_add_regset32(drm, "vec_regs", &vec->regset);
 
 	return 0;
 }
-- 
2.38.1


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

* [PATCH v4 5/7] drm/v3d: use new debugfs device-centered functions
  2022-12-19 12:06 ` Maíra Canal
@ 2022-12-19 12:06   ` Maíra Canal
  -1 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal

Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function, which centers the debugfs files
management on the drm_device instead of drm_minor.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_debugfs.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
index efbde124c296..330669f51fa7 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -79,8 +79,8 @@ static const struct v3d_reg_def v3d_csd_reg_defs[] = {
 
 static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 	int i, core;
 
@@ -126,8 +126,8 @@ static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
 
 static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 	u32 ident0, ident1, ident2, ident3, cores;
 	int core;
@@ -188,8 +188,8 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
 
 static int v3d_debugfs_bo_stats(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 
 	mutex_lock(&v3d->bo_lock);
@@ -204,8 +204,8 @@ static int v3d_debugfs_bo_stats(struct seq_file *m, void *unused)
 
 static int v3d_measure_clock(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 	uint32_t cycles;
 	int core = 0;
@@ -236,7 +236,7 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
 	return 0;
 }
 
-static const struct drm_info_list v3d_debugfs_list[] = {
+static const struct drm_debugfs_info v3d_debugfs_list[] = {
 	{"v3d_ident", v3d_v3d_debugfs_ident, 0},
 	{"v3d_regs", v3d_v3d_debugfs_regs, 0},
 	{"measure_clock", v3d_measure_clock, 0},
@@ -246,7 +246,5 @@ static const struct drm_info_list v3d_debugfs_list[] = {
 void
 v3d_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(v3d_debugfs_list,
-				 ARRAY_SIZE(v3d_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, v3d_debugfs_list, ARRAY_SIZE(v3d_debugfs_list));
 }
-- 
2.38.1


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

* [PATCH v4 5/7] drm/v3d: use new debugfs device-centered functions
@ 2022-12-19 12:06   ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen

Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function, which centers the debugfs files
management on the drm_device instead of drm_minor.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_debugfs.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
index efbde124c296..330669f51fa7 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -79,8 +79,8 @@ static const struct v3d_reg_def v3d_csd_reg_defs[] = {
 
 static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 	int i, core;
 
@@ -126,8 +126,8 @@ static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
 
 static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 	u32 ident0, ident1, ident2, ident3, cores;
 	int core;
@@ -188,8 +188,8 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
 
 static int v3d_debugfs_bo_stats(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 
 	mutex_lock(&v3d->bo_lock);
@@ -204,8 +204,8 @@ static int v3d_debugfs_bo_stats(struct seq_file *m, void *unused)
 
 static int v3d_measure_clock(struct seq_file *m, void *unused)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct v3d_dev *v3d = to_v3d_dev(dev);
 	uint32_t cycles;
 	int core = 0;
@@ -236,7 +236,7 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
 	return 0;
 }
 
-static const struct drm_info_list v3d_debugfs_list[] = {
+static const struct drm_debugfs_info v3d_debugfs_list[] = {
 	{"v3d_ident", v3d_v3d_debugfs_ident, 0},
 	{"v3d_regs", v3d_v3d_debugfs_regs, 0},
 	{"measure_clock", v3d_measure_clock, 0},
@@ -246,7 +246,5 @@ static const struct drm_info_list v3d_debugfs_list[] = {
 void
 v3d_debugfs_init(struct drm_minor *minor)
 {
-	drm_debugfs_create_files(v3d_debugfs_list,
-				 ARRAY_SIZE(v3d_debugfs_list),
-				 minor->debugfs_root, minor);
+	drm_debugfs_add_files(minor->dev, v3d_debugfs_list, ARRAY_SIZE(v3d_debugfs_list));
 }
-- 
2.38.1


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

* [PATCH v4 6/7] drm/vkms: use new debugfs device-centered functions
  2022-12-19 12:06 ` Maíra Canal
@ 2022-12-19 12:06   ` Maíra Canal
  -1 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen

Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function, which centers the debugfs files
management on the drm_device instead of drm_minor. Moreover, remove the
debugfs_init hook and add the debugfs files directly on vkms_create(),
before drm_dev_register().

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vkms/vkms_drv.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 69346906ec81..6d3a2d57d992 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -92,8 +92,8 @@ static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
 
 static int vkms_config_show(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
 
 	seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
@@ -103,24 +103,16 @@ static int vkms_config_show(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list vkms_config_debugfs_list[] = {
+static const struct drm_debugfs_info vkms_config_debugfs_list[] = {
 	{ "vkms_config", vkms_config_show, 0 },
 };
 
-static void vkms_config_debugfs_init(struct drm_minor *minor)
-{
-	drm_debugfs_create_files(vkms_config_debugfs_list, ARRAY_SIZE(vkms_config_debugfs_list),
-				 minor->debugfs_root, minor);
-}
-
 static const struct drm_driver vkms_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
 	.release		= vkms_release,
 	.fops			= &vkms_driver_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
 
-	.debugfs_init           = vkms_config_debugfs_init,
-
 	.name			= DRIVER_NAME,
 	.desc			= DRIVER_DESC,
 	.date			= DRIVER_DATE,
@@ -202,6 +194,9 @@ static int vkms_create(struct vkms_config *config)
 	if (ret)
 		goto out_devres;
 
+	drm_debugfs_add_files(&vkms_device->drm, vkms_config_debugfs_list,
+			      ARRAY_SIZE(vkms_config_debugfs_list));
+
 	ret = drm_dev_register(&vkms_device->drm, 0);
 	if (ret)
 		goto out_devres;
-- 
2.38.1


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

* [PATCH v4 6/7] drm/vkms: use new debugfs device-centered functions
@ 2022-12-19 12:06   ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal

Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function, which centers the debugfs files
management on the drm_device instead of drm_minor. Moreover, remove the
debugfs_init hook and add the debugfs files directly on vkms_create(),
before drm_dev_register().

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vkms/vkms_drv.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 69346906ec81..6d3a2d57d992 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -92,8 +92,8 @@ static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
 
 static int vkms_config_show(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct drm_device *dev = node->minor->dev;
+	struct drm_debugfs_entry *entry = m->private;
+	struct drm_device *dev = entry->dev;
 	struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
 
 	seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
@@ -103,24 +103,16 @@ static int vkms_config_show(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const struct drm_info_list vkms_config_debugfs_list[] = {
+static const struct drm_debugfs_info vkms_config_debugfs_list[] = {
 	{ "vkms_config", vkms_config_show, 0 },
 };
 
-static void vkms_config_debugfs_init(struct drm_minor *minor)
-{
-	drm_debugfs_create_files(vkms_config_debugfs_list, ARRAY_SIZE(vkms_config_debugfs_list),
-				 minor->debugfs_root, minor);
-}
-
 static const struct drm_driver vkms_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
 	.release		= vkms_release,
 	.fops			= &vkms_driver_fops,
 	DRM_GEM_SHMEM_DRIVER_OPS,
 
-	.debugfs_init           = vkms_config_debugfs_init,
-
 	.name			= DRIVER_NAME,
 	.desc			= DRIVER_DESC,
 	.date			= DRIVER_DATE,
@@ -202,6 +194,9 @@ static int vkms_create(struct vkms_config *config)
 	if (ret)
 		goto out_devres;
 
+	drm_debugfs_add_files(&vkms_device->drm, vkms_config_debugfs_list,
+			      ARRAY_SIZE(vkms_config_debugfs_list));
+
 	ret = drm_dev_register(&vkms_device->drm, 0);
 	if (ret)
 		goto out_devres;
-- 
2.38.1


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

* [PATCH v4 7/7] drm/todo: update the debugfs clean up task
  2022-12-19 12:06 ` Maíra Canal
@ 2022-12-19 12:06   ` Maíra Canal
  -1 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal

The structs drm_debugfs_info and drm_debugfs_entry introduced a new
debugfs structure to DRM, centered on drm_device instead of drm_minor.
Therefore, remove the tasks related to create a new device-centered
debugfs structure and add a new task to replace the use of
drm_debugfs_create_files() for the use of drm_debugfs_add_file() and
drm_debugfs_add_files().

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 Documentation/gpu/todo.rst | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index b2c6aaf1edf2..f64abf69f341 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -508,17 +508,14 @@ 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.
+- Convert drivers to support the drm_debugfs_add_files() function instead of
+  the drm_debugfs_create_files() function.
 
 - 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
@@ -527,8 +524,6 @@ There's a bunch of issues with it:
   this (together with the drm_minor->drm_device move) would allow us to remove
   debugfs_init.
 
-Previous RFC that hasn't landed yet: https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/
-
 Contact: Daniel Vetter
 
 Level: Intermediate
-- 
2.38.1


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

* [PATCH v4 7/7] drm/todo: update the debugfs clean up task
@ 2022-12-19 12:06   ` Maíra Canal
  0 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-19 12:06 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen

The structs drm_debugfs_info and drm_debugfs_entry introduced a new
debugfs structure to DRM, centered on drm_device instead of drm_minor.
Therefore, remove the tasks related to create a new device-centered
debugfs structure and add a new task to replace the use of
drm_debugfs_create_files() for the use of drm_debugfs_add_file() and
drm_debugfs_add_files().

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 Documentation/gpu/todo.rst | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index b2c6aaf1edf2..f64abf69f341 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -508,17 +508,14 @@ 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.
+- Convert drivers to support the drm_debugfs_add_files() function instead of
+  the drm_debugfs_create_files() function.
 
 - 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
@@ -527,8 +524,6 @@ There's a bunch of issues with it:
   this (together with the drm_minor->drm_device move) would allow us to remove
   debugfs_init.
 
-Previous RFC that hasn't landed yet: https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/
-
 Contact: Daniel Vetter
 
 Level: Intermediate
-- 
2.38.1


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

* Re: [PATCH v4 0/7] Introduce debugfs device-centered functions
  2022-12-19 12:06 ` Maíra Canal
@ 2022-12-19 12:49   ` Melissa Wen
  -1 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2022-12-19 12:49 UTC (permalink / raw)
  To: Maíra Canal
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Oded Gabbay, Jani Nikula,
	André Almeida, Emma Anholt, Rodrigo Siqueira, Wambui Karuga,
	dri-devel, linux-kernel

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

On 12/19, Maíra Canal wrote:
> This series introduces the initial structure to make DRM debugfs more
> device-centered and it is the first step to drop the
> drm_driver->debugfs_init hooks in the future [1].
> 
> Currently, DRM debugfs files are created using drm_debugfs_create_files()
> on request. The first patch of this series makes it possible for DRM devices
> for creating debugfs files during drm_dev_register(). For it, it introduces
> two new functions that can be used by the drivers: drm_debugfs_add_files()
> and drm_debugfs_add_file(). The requests are added to a list and are created
> all at once during drm_dev_register(). Moreover, the first patch was based on
> this RFC series [2].
> 
> The main difference between the RFC series and the current series is the
> creation of a new fops structure to accommodate the new structs and, also,
> the creation of a new drm_debugfs_open. Moreover, the new series uses
> device-managed allocation, returns memory allocation errors, and converts
> more drivers to the new structure.
> 
> Moreover, since v3, the ability to create debugfs files at late_register hooks was
> added. In previous versions, modeset components weren't able to create debugfs
> files at late_register hooks as the registration of drm_minor happens before the
> registration of the modeset abstractions. So, the third patch fixes this problem
> by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
> catching this problem!
> 
> Apart from the third patch, the series looks similiar from its last version.
> 
> [1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
> [2] https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/
> 
> Best Regards,
> - Maíra Canal
> 
> ---
> 
> v1 -> v2: https://lore.kernel.org/dri-devel/20221122190314.185015-1-mcanal@igalia.com/T/#t
> 
> - Fix compilation errors in the second patch (kernel test robot).
> - Drop debugfs_init hook from vkms (Maíra Canal).
> - Remove return values and error handling to debugfs related
> functions (Jani Nikula).
> - Remove entry from list after the file is created, so that drm_debugfs_init
> can be called more than once (Maíra Canal).
> 
> v2 -> v3: https://lore.kernel.org/dri-devel/20221123220725.1272155-1-mcanal@igalia.com/
> 
> - Rebase on top of drm-misc-next
> 
> v3 -> v4: https://lore.kernel.org/dri-devel/20221207132325.140393-1-mcanal@igalia.com/
> 
> - Add Maxime's Reviewed-by tags
> - Add the ability to create debugfs files at late_register hooks (Melissa Wen).

Hi Maíra,

Thanks for addressing all comments.

Maybe Danvet has some inputs for the late_register approach.

Anyway, LGTM and the entire series is:

Reviewed-by: Melissa Wen <mwen@igalia.com>

> 
> ---
> 
> Maíra Canal (7):
>   drm/debugfs: create device-centered debugfs functions
>   drm: use new debugfs device-centered functions on DRM core files
>   drm/debugfs: create debugfs late register functions
>   drm/vc4: use new debugfs device-centered functions
>   drm/v3d: use new debugfs device-centered functions
>   drm/vkms: use new debugfs device-centered functions
>   drm/todo: update the debugfs clean up task
> 
>  Documentation/gpu/todo.rst            |   9 +--
>  drivers/gpu/drm/drm_atomic.c          |  11 ++-
>  drivers/gpu/drm/drm_client.c          |  11 ++-
>  drivers/gpu/drm/drm_debugfs.c         | 102 +++++++++++++++++++++++---
>  drivers/gpu/drm/drm_drv.c             |   3 +
>  drivers/gpu/drm/drm_framebuffer.c     |  11 ++-
>  drivers/gpu/drm/drm_gem_vram_helper.c |  11 ++-
>  drivers/gpu/drm/drm_internal.h        |   5 ++
>  drivers/gpu/drm/drm_mode_config.c     |   2 +
>  drivers/gpu/drm/v3d/v3d_debugfs.c     |  22 +++---
>  drivers/gpu/drm/vc4/vc4_bo.c          |  10 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c        |   7 +-
>  drivers/gpu/drm/vc4/vc4_debugfs.c     |  36 ++-------
>  drivers/gpu/drm/vc4/vc4_dpi.c         |   5 +-
>  drivers/gpu/drm/vc4/vc4_drv.c         |   1 -
>  drivers/gpu/drm/vc4/vc4_drv.h         |  32 ++------
>  drivers/gpu/drm/vc4/vc4_dsi.c         |   6 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c        |  12 +--
>  drivers/gpu/drm/vc4/vc4_hvs.c         |  24 ++----
>  drivers/gpu/drm/vc4/vc4_v3d.c         |  14 +---
>  drivers/gpu/drm/vc4/vc4_vec.c         |   6 +-
>  drivers/gpu/drm/vkms/vkms_drv.c       |  17 ++---
>  include/drm/drm_debugfs.h             |  41 +++++++++++
>  include/drm/drm_device.h              |  15 ++++
>  24 files changed, 233 insertions(+), 180 deletions(-)
> 
> -- 
> 2.38.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v4 0/7] Introduce debugfs device-centered functions
@ 2022-12-19 12:49   ` Melissa Wen
  0 siblings, 0 replies; 23+ messages in thread
From: Melissa Wen @ 2022-12-19 12:49 UTC (permalink / raw)
  To: Maíra Canal
  Cc: dri-devel, André Almeida, Thomas Zimmermann, Emma Anholt,
	Oded Gabbay, linux-kernel, Wambui Karuga, Rodrigo Siqueira

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

On 12/19, Maíra Canal wrote:
> This series introduces the initial structure to make DRM debugfs more
> device-centered and it is the first step to drop the
> drm_driver->debugfs_init hooks in the future [1].
> 
> Currently, DRM debugfs files are created using drm_debugfs_create_files()
> on request. The first patch of this series makes it possible for DRM devices
> for creating debugfs files during drm_dev_register(). For it, it introduces
> two new functions that can be used by the drivers: drm_debugfs_add_files()
> and drm_debugfs_add_file(). The requests are added to a list and are created
> all at once during drm_dev_register(). Moreover, the first patch was based on
> this RFC series [2].
> 
> The main difference between the RFC series and the current series is the
> creation of a new fops structure to accommodate the new structs and, also,
> the creation of a new drm_debugfs_open. Moreover, the new series uses
> device-managed allocation, returns memory allocation errors, and converts
> more drivers to the new structure.
> 
> Moreover, since v3, the ability to create debugfs files at late_register hooks was
> added. In previous versions, modeset components weren't able to create debugfs
> files at late_register hooks as the registration of drm_minor happens before the
> registration of the modeset abstractions. So, the third patch fixes this problem
> by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
> catching this problem!
> 
> Apart from the third patch, the series looks similiar from its last version.
> 
> [1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
> [2] https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/
> 
> Best Regards,
> - Maíra Canal
> 
> ---
> 
> v1 -> v2: https://lore.kernel.org/dri-devel/20221122190314.185015-1-mcanal@igalia.com/T/#t
> 
> - Fix compilation errors in the second patch (kernel test robot).
> - Drop debugfs_init hook from vkms (Maíra Canal).
> - Remove return values and error handling to debugfs related
> functions (Jani Nikula).
> - Remove entry from list after the file is created, so that drm_debugfs_init
> can be called more than once (Maíra Canal).
> 
> v2 -> v3: https://lore.kernel.org/dri-devel/20221123220725.1272155-1-mcanal@igalia.com/
> 
> - Rebase on top of drm-misc-next
> 
> v3 -> v4: https://lore.kernel.org/dri-devel/20221207132325.140393-1-mcanal@igalia.com/
> 
> - Add Maxime's Reviewed-by tags
> - Add the ability to create debugfs files at late_register hooks (Melissa Wen).

Hi Maíra,

Thanks for addressing all comments.

Maybe Danvet has some inputs for the late_register approach.

Anyway, LGTM and the entire series is:

Reviewed-by: Melissa Wen <mwen@igalia.com>

> 
> ---
> 
> Maíra Canal (7):
>   drm/debugfs: create device-centered debugfs functions
>   drm: use new debugfs device-centered functions on DRM core files
>   drm/debugfs: create debugfs late register functions
>   drm/vc4: use new debugfs device-centered functions
>   drm/v3d: use new debugfs device-centered functions
>   drm/vkms: use new debugfs device-centered functions
>   drm/todo: update the debugfs clean up task
> 
>  Documentation/gpu/todo.rst            |   9 +--
>  drivers/gpu/drm/drm_atomic.c          |  11 ++-
>  drivers/gpu/drm/drm_client.c          |  11 ++-
>  drivers/gpu/drm/drm_debugfs.c         | 102 +++++++++++++++++++++++---
>  drivers/gpu/drm/drm_drv.c             |   3 +
>  drivers/gpu/drm/drm_framebuffer.c     |  11 ++-
>  drivers/gpu/drm/drm_gem_vram_helper.c |  11 ++-
>  drivers/gpu/drm/drm_internal.h        |   5 ++
>  drivers/gpu/drm/drm_mode_config.c     |   2 +
>  drivers/gpu/drm/v3d/v3d_debugfs.c     |  22 +++---
>  drivers/gpu/drm/vc4/vc4_bo.c          |  10 +--
>  drivers/gpu/drm/vc4/vc4_crtc.c        |   7 +-
>  drivers/gpu/drm/vc4/vc4_debugfs.c     |  36 ++-------
>  drivers/gpu/drm/vc4/vc4_dpi.c         |   5 +-
>  drivers/gpu/drm/vc4/vc4_drv.c         |   1 -
>  drivers/gpu/drm/vc4/vc4_drv.h         |  32 ++------
>  drivers/gpu/drm/vc4/vc4_dsi.c         |   6 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c        |  12 +--
>  drivers/gpu/drm/vc4/vc4_hvs.c         |  24 ++----
>  drivers/gpu/drm/vc4/vc4_v3d.c         |  14 +---
>  drivers/gpu/drm/vc4/vc4_vec.c         |   6 +-
>  drivers/gpu/drm/vkms/vkms_drv.c       |  17 ++---
>  include/drm/drm_debugfs.h             |  41 +++++++++++
>  include/drm/drm_device.h              |  15 ++++
>  24 files changed, 233 insertions(+), 180 deletions(-)
> 
> -- 
> 2.38.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v4 0/7] Introduce debugfs device-centered functions
  2022-12-19 12:49   ` Melissa Wen
@ 2022-12-22 17:20     ` Daniel Vetter
  -1 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2022-12-22 17:20 UTC (permalink / raw)
  To: Melissa Wen
  Cc: Maíra Canal, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter, Oded Gabbay,
	Jani Nikula, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel

On Mon, Dec 19, 2022 at 11:49:47AM -0100, Melissa Wen wrote:
> On 12/19, Maíra Canal wrote:
> > This series introduces the initial structure to make DRM debugfs more
> > device-centered and it is the first step to drop the
> > drm_driver->debugfs_init hooks in the future [1].
> > 
> > Currently, DRM debugfs files are created using drm_debugfs_create_files()
> > on request. The first patch of this series makes it possible for DRM devices
> > for creating debugfs files during drm_dev_register(). For it, it introduces
> > two new functions that can be used by the drivers: drm_debugfs_add_files()
> > and drm_debugfs_add_file(). The requests are added to a list and are created
> > all at once during drm_dev_register(). Moreover, the first patch was based on
> > this RFC series [2].
> > 
> > The main difference between the RFC series and the current series is the
> > creation of a new fops structure to accommodate the new structs and, also,
> > the creation of a new drm_debugfs_open. Moreover, the new series uses
> > device-managed allocation, returns memory allocation errors, and converts
> > more drivers to the new structure.
> > 
> > Moreover, since v3, the ability to create debugfs files at late_register hooks was
> > added. In previous versions, modeset components weren't able to create debugfs
> > files at late_register hooks as the registration of drm_minor happens before the
> > registration of the modeset abstractions. So, the third patch fixes this problem
> > by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
> > catching this problem!
> > 
> > Apart from the third patch, the series looks similiar from its last version.
> > 
> > [1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
> > [2] https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/
> > 
> > Best Regards,
> > - Maíra Canal
> > 
> > ---
> > 
> > v1 -> v2: https://lore.kernel.org/dri-devel/20221122190314.185015-1-mcanal@igalia.com/T/#t
> > 
> > - Fix compilation errors in the second patch (kernel test robot).
> > - Drop debugfs_init hook from vkms (Maíra Canal).
> > - Remove return values and error handling to debugfs related
> > functions (Jani Nikula).
> > - Remove entry from list after the file is created, so that drm_debugfs_init
> > can be called more than once (Maíra Canal).
> > 
> > v2 -> v3: https://lore.kernel.org/dri-devel/20221123220725.1272155-1-mcanal@igalia.com/
> > 
> > - Rebase on top of drm-misc-next
> > 
> > v3 -> v4: https://lore.kernel.org/dri-devel/20221207132325.140393-1-mcanal@igalia.com/
> > 
> > - Add Maxime's Reviewed-by tags
> > - Add the ability to create debugfs files at late_register hooks (Melissa Wen).
> 
> Hi Maíra,
> 
> Thanks for addressing all comments.
> 
> Maybe Danvet has some inputs for the late_register approach.

I think as a stop-gap (really need to get this stuff landed so people can
start to use it) this is ok, but long term I think the right fix is to
roll out the same pre-register infrastructure for connector and crtc too.
That way drivers don't need to split their setup code into init and
register anymore, which is the point of this entire rework.

If you want, you can adjust the todo accordingly, but we do already have
the paragraph about connector/crtc.

But we can do that later on, because this is definitely a great way
forward. Thanks a lot for pushing this forward!

> Anyway, LGTM and the entire series is:
> 
> Reviewed-by: Melissa Wen <mwen@igalia.com>

On the series: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > 
> > ---
> > 
> > Maíra Canal (7):
> >   drm/debugfs: create device-centered debugfs functions
> >   drm: use new debugfs device-centered functions on DRM core files
> >   drm/debugfs: create debugfs late register functions
> >   drm/vc4: use new debugfs device-centered functions
> >   drm/v3d: use new debugfs device-centered functions
> >   drm/vkms: use new debugfs device-centered functions
> >   drm/todo: update the debugfs clean up task
> > 
> >  Documentation/gpu/todo.rst            |   9 +--
> >  drivers/gpu/drm/drm_atomic.c          |  11 ++-
> >  drivers/gpu/drm/drm_client.c          |  11 ++-
> >  drivers/gpu/drm/drm_debugfs.c         | 102 +++++++++++++++++++++++---
> >  drivers/gpu/drm/drm_drv.c             |   3 +
> >  drivers/gpu/drm/drm_framebuffer.c     |  11 ++-
> >  drivers/gpu/drm/drm_gem_vram_helper.c |  11 ++-
> >  drivers/gpu/drm/drm_internal.h        |   5 ++
> >  drivers/gpu/drm/drm_mode_config.c     |   2 +
> >  drivers/gpu/drm/v3d/v3d_debugfs.c     |  22 +++---
> >  drivers/gpu/drm/vc4/vc4_bo.c          |  10 +--
> >  drivers/gpu/drm/vc4/vc4_crtc.c        |   7 +-
> >  drivers/gpu/drm/vc4/vc4_debugfs.c     |  36 ++-------
> >  drivers/gpu/drm/vc4/vc4_dpi.c         |   5 +-
> >  drivers/gpu/drm/vc4/vc4_drv.c         |   1 -
> >  drivers/gpu/drm/vc4/vc4_drv.h         |  32 ++------
> >  drivers/gpu/drm/vc4/vc4_dsi.c         |   6 +-
> >  drivers/gpu/drm/vc4/vc4_hdmi.c        |  12 +--
> >  drivers/gpu/drm/vc4/vc4_hvs.c         |  24 ++----
> >  drivers/gpu/drm/vc4/vc4_v3d.c         |  14 +---
> >  drivers/gpu/drm/vc4/vc4_vec.c         |   6 +-
> >  drivers/gpu/drm/vkms/vkms_drv.c       |  17 ++---
> >  include/drm/drm_debugfs.h             |  41 +++++++++++
> >  include/drm/drm_device.h              |  15 ++++
> >  24 files changed, 233 insertions(+), 180 deletions(-)
> > 
> > -- 
> > 2.38.1
> > 



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v4 0/7] Introduce debugfs device-centered functions
@ 2022-12-22 17:20     ` Daniel Vetter
  0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2022-12-22 17:20 UTC (permalink / raw)
  To: Melissa Wen
  Cc: dri-devel, André Almeida, Thomas Zimmermann, Emma Anholt,
	Oded Gabbay, Maíra Canal, linux-kernel, Wambui Karuga,
	Rodrigo Siqueira

On Mon, Dec 19, 2022 at 11:49:47AM -0100, Melissa Wen wrote:
> On 12/19, Maíra Canal wrote:
> > This series introduces the initial structure to make DRM debugfs more
> > device-centered and it is the first step to drop the
> > drm_driver->debugfs_init hooks in the future [1].
> > 
> > Currently, DRM debugfs files are created using drm_debugfs_create_files()
> > on request. The first patch of this series makes it possible for DRM devices
> > for creating debugfs files during drm_dev_register(). For it, it introduces
> > two new functions that can be used by the drivers: drm_debugfs_add_files()
> > and drm_debugfs_add_file(). The requests are added to a list and are created
> > all at once during drm_dev_register(). Moreover, the first patch was based on
> > this RFC series [2].
> > 
> > The main difference between the RFC series and the current series is the
> > creation of a new fops structure to accommodate the new structs and, also,
> > the creation of a new drm_debugfs_open. Moreover, the new series uses
> > device-managed allocation, returns memory allocation errors, and converts
> > more drivers to the new structure.
> > 
> > Moreover, since v3, the ability to create debugfs files at late_register hooks was
> > added. In previous versions, modeset components weren't able to create debugfs
> > files at late_register hooks as the registration of drm_minor happens before the
> > registration of the modeset abstractions. So, the third patch fixes this problem
> > by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
> > catching this problem!
> > 
> > Apart from the third patch, the series looks similiar from its last version.
> > 
> > [1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
> > [2] https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/
> > 
> > Best Regards,
> > - Maíra Canal
> > 
> > ---
> > 
> > v1 -> v2: https://lore.kernel.org/dri-devel/20221122190314.185015-1-mcanal@igalia.com/T/#t
> > 
> > - Fix compilation errors in the second patch (kernel test robot).
> > - Drop debugfs_init hook from vkms (Maíra Canal).
> > - Remove return values and error handling to debugfs related
> > functions (Jani Nikula).
> > - Remove entry from list after the file is created, so that drm_debugfs_init
> > can be called more than once (Maíra Canal).
> > 
> > v2 -> v3: https://lore.kernel.org/dri-devel/20221123220725.1272155-1-mcanal@igalia.com/
> > 
> > - Rebase on top of drm-misc-next
> > 
> > v3 -> v4: https://lore.kernel.org/dri-devel/20221207132325.140393-1-mcanal@igalia.com/
> > 
> > - Add Maxime's Reviewed-by tags
> > - Add the ability to create debugfs files at late_register hooks (Melissa Wen).
> 
> Hi Maíra,
> 
> Thanks for addressing all comments.
> 
> Maybe Danvet has some inputs for the late_register approach.

I think as a stop-gap (really need to get this stuff landed so people can
start to use it) this is ok, but long term I think the right fix is to
roll out the same pre-register infrastructure for connector and crtc too.
That way drivers don't need to split their setup code into init and
register anymore, which is the point of this entire rework.

If you want, you can adjust the todo accordingly, but we do already have
the paragraph about connector/crtc.

But we can do that later on, because this is definitely a great way
forward. Thanks a lot for pushing this forward!

> Anyway, LGTM and the entire series is:
> 
> Reviewed-by: Melissa Wen <mwen@igalia.com>

On the series: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> > 
> > ---
> > 
> > Maíra Canal (7):
> >   drm/debugfs: create device-centered debugfs functions
> >   drm: use new debugfs device-centered functions on DRM core files
> >   drm/debugfs: create debugfs late register functions
> >   drm/vc4: use new debugfs device-centered functions
> >   drm/v3d: use new debugfs device-centered functions
> >   drm/vkms: use new debugfs device-centered functions
> >   drm/todo: update the debugfs clean up task
> > 
> >  Documentation/gpu/todo.rst            |   9 +--
> >  drivers/gpu/drm/drm_atomic.c          |  11 ++-
> >  drivers/gpu/drm/drm_client.c          |  11 ++-
> >  drivers/gpu/drm/drm_debugfs.c         | 102 +++++++++++++++++++++++---
> >  drivers/gpu/drm/drm_drv.c             |   3 +
> >  drivers/gpu/drm/drm_framebuffer.c     |  11 ++-
> >  drivers/gpu/drm/drm_gem_vram_helper.c |  11 ++-
> >  drivers/gpu/drm/drm_internal.h        |   5 ++
> >  drivers/gpu/drm/drm_mode_config.c     |   2 +
> >  drivers/gpu/drm/v3d/v3d_debugfs.c     |  22 +++---
> >  drivers/gpu/drm/vc4/vc4_bo.c          |  10 +--
> >  drivers/gpu/drm/vc4/vc4_crtc.c        |   7 +-
> >  drivers/gpu/drm/vc4/vc4_debugfs.c     |  36 ++-------
> >  drivers/gpu/drm/vc4/vc4_dpi.c         |   5 +-
> >  drivers/gpu/drm/vc4/vc4_drv.c         |   1 -
> >  drivers/gpu/drm/vc4/vc4_drv.h         |  32 ++------
> >  drivers/gpu/drm/vc4/vc4_dsi.c         |   6 +-
> >  drivers/gpu/drm/vc4/vc4_hdmi.c        |  12 +--
> >  drivers/gpu/drm/vc4/vc4_hvs.c         |  24 ++----
> >  drivers/gpu/drm/vc4/vc4_v3d.c         |  14 +---
> >  drivers/gpu/drm/vc4/vc4_vec.c         |   6 +-
> >  drivers/gpu/drm/vkms/vkms_drv.c       |  17 ++---
> >  include/drm/drm_debugfs.h             |  41 +++++++++++
> >  include/drm/drm_device.h              |  15 ++++
> >  24 files changed, 233 insertions(+), 180 deletions(-)
> > 
> > -- 
> > 2.38.1
> > 



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v4 0/7] Introduce debugfs device-centered functions
  2022-12-22 17:20     ` Daniel Vetter
  (?)
@ 2022-12-23 18:12     ` Maíra Canal
  -1 siblings, 0 replies; 23+ messages in thread
From: Maíra Canal @ 2022-12-23 18:12 UTC (permalink / raw)
  To: Melissa Wen, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Oded Gabbay, Jani Nikula, André Almeida,
	Emma Anholt, Rodrigo Siqueira, Wambui Karuga, dri-devel,
	linux-kernel

On 12/22/22 14:20, Daniel Vetter wrote:
> On Mon, Dec 19, 2022 at 11:49:47AM -0100, Melissa Wen wrote:
>> On 12/19, Maíra Canal wrote:
>>> This series introduces the initial structure to make DRM debugfs more
>>> device-centered and it is the first step to drop the
>>> drm_driver->debugfs_init hooks in the future [1].
>>>
>>> Currently, DRM debugfs files are created using drm_debugfs_create_files()
>>> on request. The first patch of this series makes it possible for DRM devices
>>> for creating debugfs files during drm_dev_register(). For it, it introduces
>>> two new functions that can be used by the drivers: drm_debugfs_add_files()
>>> and drm_debugfs_add_file(). The requests are added to a list and are created
>>> all at once during drm_dev_register(). Moreover, the first patch was based on
>>> this RFC series [2].
>>>
>>> The main difference between the RFC series and the current series is the
>>> creation of a new fops structure to accommodate the new structs and, also,
>>> the creation of a new drm_debugfs_open. Moreover, the new series uses
>>> device-managed allocation, returns memory allocation errors, and converts
>>> more drivers to the new structure.
>>>
>>> Moreover, since v3, the ability to create debugfs files at late_register hooks was
>>> added. In previous versions, modeset components weren't able to create debugfs
>>> files at late_register hooks as the registration of drm_minor happens before the
>>> registration of the modeset abstractions. So, the third patch fixes this problem
>>> by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
>>> catching this problem!
>>>
>>> Apart from the third patch, the series looks similiar from its last version.
>>>
>>> [1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
>>> [2] https://lore.kernel.org/dri-devel/20200513114130.28641-2-wambui.karugax@gmail.com/
>>>
>>> Best Regards,
>>> - Maíra Canal
>>>
>>> ---
>>>
>>> v1 -> v2: https://lore.kernel.org/dri-devel/20221122190314.185015-1-mcanal@igalia.com/T/#t
>>>
>>> - Fix compilation errors in the second patch (kernel test robot).
>>> - Drop debugfs_init hook from vkms (Maíra Canal).
>>> - Remove return values and error handling to debugfs related
>>> functions (Jani Nikula).
>>> - Remove entry from list after the file is created, so that drm_debugfs_init
>>> can be called more than once (Maíra Canal).
>>>
>>> v2 -> v3: https://lore.kernel.org/dri-devel/20221123220725.1272155-1-mcanal@igalia.com/
>>>
>>> - Rebase on top of drm-misc-next
>>>
>>> v3 -> v4: https://lore.kernel.org/dri-devel/20221207132325.140393-1-mcanal@igalia.com/
>>>
>>> - Add Maxime's Reviewed-by tags
>>> - Add the ability to create debugfs files at late_register hooks (Melissa Wen).
>>
>> Hi Maíra,
>>
>> Thanks for addressing all comments.
>>
>> Maybe Danvet has some inputs for the late_register approach.
> 
> I think as a stop-gap (really need to get this stuff landed so people can
> start to use it) this is ok, but long term I think the right fix is to
> roll out the same pre-register infrastructure for connector and crtc too.
> That way drivers don't need to split their setup code into init and
> register anymore, which is the point of this entire rework.
> 
> If you want, you can adjust the todo accordingly, but we do already have
> the paragraph about connector/crtc.
> 
> But we can do that later on, because this is definitely a great way
> forward. Thanks a lot for pushing this forward!
> 
>> Anyway, LGTM and the entire series is:
>>
>> Reviewed-by: Melissa Wen <mwen@igalia.com>
> 
> On the series: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Applied this series to drm-misc-next.

Best Regards,
- Maíra Canal

>>
>>>
>>> ---
>>>
>>> Maíra Canal (7):
>>>    drm/debugfs: create device-centered debugfs functions
>>>    drm: use new debugfs device-centered functions on DRM core files
>>>    drm/debugfs: create debugfs late register functions
>>>    drm/vc4: use new debugfs device-centered functions
>>>    drm/v3d: use new debugfs device-centered functions
>>>    drm/vkms: use new debugfs device-centered functions
>>>    drm/todo: update the debugfs clean up task
>>>
>>>   Documentation/gpu/todo.rst            |   9 +--
>>>   drivers/gpu/drm/drm_atomic.c          |  11 ++-
>>>   drivers/gpu/drm/drm_client.c          |  11 ++-
>>>   drivers/gpu/drm/drm_debugfs.c         | 102 +++++++++++++++++++++++---
>>>   drivers/gpu/drm/drm_drv.c             |   3 +
>>>   drivers/gpu/drm/drm_framebuffer.c     |  11 ++-
>>>   drivers/gpu/drm/drm_gem_vram_helper.c |  11 ++-
>>>   drivers/gpu/drm/drm_internal.h        |   5 ++
>>>   drivers/gpu/drm/drm_mode_config.c     |   2 +
>>>   drivers/gpu/drm/v3d/v3d_debugfs.c     |  22 +++---
>>>   drivers/gpu/drm/vc4/vc4_bo.c          |  10 +--
>>>   drivers/gpu/drm/vc4/vc4_crtc.c        |   7 +-
>>>   drivers/gpu/drm/vc4/vc4_debugfs.c     |  36 ++-------
>>>   drivers/gpu/drm/vc4/vc4_dpi.c         |   5 +-
>>>   drivers/gpu/drm/vc4/vc4_drv.c         |   1 -
>>>   drivers/gpu/drm/vc4/vc4_drv.h         |  32 ++------
>>>   drivers/gpu/drm/vc4/vc4_dsi.c         |   6 +-
>>>   drivers/gpu/drm/vc4/vc4_hdmi.c        |  12 +--
>>>   drivers/gpu/drm/vc4/vc4_hvs.c         |  24 ++----
>>>   drivers/gpu/drm/vc4/vc4_v3d.c         |  14 +---
>>>   drivers/gpu/drm/vc4/vc4_vec.c         |   6 +-
>>>   drivers/gpu/drm/vkms/vkms_drv.c       |  17 ++---
>>>   include/drm/drm_debugfs.h             |  41 +++++++++++
>>>   include/drm/drm_device.h              |  15 ++++
>>>   24 files changed, 233 insertions(+), 180 deletions(-)
>>>
>>> -- 
>>> 2.38.1
>>>
> 
> 
> 

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

* Re: [PATCH v4 1/7] drm/debugfs: create device-centered debugfs functions
  2022-12-19 12:06   ` Maíra Canal
@ 2023-01-05 12:44     ` Jani Nikula
  -1 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2023-01-05 12:44 UTC (permalink / raw)
  To: Maíra Canal, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter, Oded Gabbay
  Cc: Melissa Wen, André Almeida, Emma Anholt, Rodrigo Siqueira,
	Wambui Karuga, dri-devel, linux-kernel, Maíra Canal,
	Wambui Karuga, Maxime Ripard

On Mon, 19 Dec 2022, Maíra Canal <mcanal@igalia.com> wrote:
> @@ -230,6 +247,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  	if (dev->driver->debugfs_init)
>  		dev->driver->debugfs_init(minor);
>  
> +	list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) {
> +		debugfs_create_file(entry->file.name, S_IFREG | S_IRUGO,

I know this was merged already, but S_IFREG is redundant, and the octal
values are preferred over S_IRUGO. See checkpatch SYMBOLIC_PERMS.

This would be just 0444.


BR,
Jani.


> +				    minor->debugfs_root, entry, &drm_debugfs_entry_fops);
> +		list_del(&entry->list);
> +	}
> +
>  	return 0;
>  }
>  

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH v4 1/7] drm/debugfs: create device-centered debugfs functions
@ 2023-01-05 12:44     ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2023-01-05 12:44 UTC (permalink / raw)
  To: Maíra Canal, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Daniel Vetter, Oded Gabbay
  Cc: André Almeida, Emma Anholt, Rodrigo Siqueira,
	Maíra Canal, linux-kernel, dri-devel, Wambui Karuga,
	Melissa Wen, Maxime Ripard, Wambui Karuga

On Mon, 19 Dec 2022, Maíra Canal <mcanal@igalia.com> wrote:
> @@ -230,6 +247,12 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  	if (dev->driver->debugfs_init)
>  		dev->driver->debugfs_init(minor);
>  
> +	list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) {
> +		debugfs_create_file(entry->file.name, S_IFREG | S_IRUGO,

I know this was merged already, but S_IFREG is redundant, and the octal
values are preferred over S_IRUGO. See checkpatch SYMBOLIC_PERMS.

This would be just 0444.


BR,
Jani.


> +				    minor->debugfs_root, entry, &drm_debugfs_entry_fops);
> +		list_del(&entry->list);
> +	}
> +
>  	return 0;
>  }
>  

-- 
Jani Nikula, Intel Open Source Graphics Center

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

end of thread, other threads:[~2023-01-05 12:44 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-19 12:06 [PATCH v4 0/7] Introduce debugfs device-centered functions Maíra Canal
2022-12-19 12:06 ` Maíra Canal
2022-12-19 12:06 ` [PATCH v4 1/7] drm/debugfs: create device-centered debugfs functions Maíra Canal
2022-12-19 12:06   ` Maíra Canal
2023-01-05 12:44   ` Jani Nikula
2023-01-05 12:44     ` Jani Nikula
2022-12-19 12:06 ` [PATCH v4 2/7] drm: use new debugfs device-centered functions on DRM core files Maíra Canal
2022-12-19 12:06   ` Maíra Canal
2022-12-19 12:06 ` [PATCH v4 3/7] drm/debugfs: create debugfs late register functions Maíra Canal
2022-12-19 12:06   ` Maíra Canal
2022-12-19 12:06 ` [PATCH v4 4/7] drm/vc4: use new debugfs device-centered functions Maíra Canal
2022-12-19 12:06   ` Maíra Canal
2022-12-19 12:06 ` [PATCH v4 5/7] drm/v3d: " Maíra Canal
2022-12-19 12:06   ` Maíra Canal
2022-12-19 12:06 ` [PATCH v4 6/7] drm/vkms: " Maíra Canal
2022-12-19 12:06   ` Maíra Canal
2022-12-19 12:06 ` [PATCH v4 7/7] drm/todo: update the debugfs clean up task Maíra Canal
2022-12-19 12:06   ` Maíra Canal
2022-12-19 12:49 ` [PATCH v4 0/7] Introduce debugfs device-centered functions Melissa Wen
2022-12-19 12:49   ` Melissa Wen
2022-12-22 17:20   ` Daniel Vetter
2022-12-22 17:20     ` Daniel Vetter
2022-12-23 18:12     ` Maíra Canal

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.