2020-05-13 11:44:23

by Wambui Karuga

[permalink] [raw]
Subject: [RFC PATCH 0/3] drm: introduce new method of creating debugfs files.

Hi All,

Currently drm debugfs files are created using drm_debugfs_create_files()
on request.
This series introduces new functions and infrastructure that will enable
the mass creation of debugfs files during drm_dev_register(). Drivers can
request for the creation of debugfs files at any time using two new
functions: drm_debugfs_add_files()/drm_debugfs_add_file(). The file
requests are added to a list and tracked until drm_dev_register() when
the files are created all at once.

This concept was already in use by the drm/vc4 driver and this series
tries to introduce the same infrastructure in drm core.
Patch 2 removes vc4's functions doing the same, and replaces it
with the new drm core functions.
Patch 3 is an example of how the new functions and structs can replace
the existing drm_info_list and drm_debugfs_create_files() usage.

Hoping to get some feedback or comments on the current progress of this
series.

Thanks,
wambui karuga

Wambui Karuga (3):
drm/debugfs: create debugfs files during drm_dev_register().
drm/vc4: use new debugfs functions for file creation.
drm: use new debugfs functions for various files.

drivers/gpu/drm/drm_atomic.c | 11 +++---
drivers/gpu/drm/drm_client.c | 11 +++---
drivers/gpu/drm/drm_debugfs.c | 59 ++++++++++++++++++++++++++++---
drivers/gpu/drm/drm_drv.c | 2 ++
drivers/gpu/drm/drm_framebuffer.c | 11 +++---
drivers/gpu/drm/vc4/vc4_bo.c | 4 +--
drivers/gpu/drm/vc4/vc4_debugfs.c | 38 +++++---------------
drivers/gpu/drm/vc4/vc4_hdmi.c | 4 +--
drivers/gpu/drm/vc4/vc4_hvs.c | 4 +--
drivers/gpu/drm/vc4/vc4_v3d.c | 4 +--
include/drm/drm_debugfs.h | 38 ++++++++++++++++++++
include/drm/drm_device.h | 12 +++++++
12 files changed, 138 insertions(+), 60 deletions(-)

--
2.26.2


2020-05-13 11:44:41

by Wambui Karuga

[permalink] [raw]
Subject: [RFC PATCH 2/3] drm/vc4: use new debugfs functions for file creation.

Currently, vc4 delays adding of debugfs files until drm_dev_register()
calls vc4_debugfs_init() on each registered minor.

This change removes this infrastructure and uses the new
drm_debugfs_add_file() function and drm_device->debugfs_list to track
debugfs files which are added at drm_dev_register() time on each minor.

Signed-off-by: Wambui Karuga <[email protected]>
---
drivers/gpu/drm/vc4/vc4_bo.c | 4 ++--
drivers/gpu/drm/vc4/vc4_debugfs.c | 38 +++++++------------------------
drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++--
drivers/gpu/drm/vc4/vc4_hvs.c | 4 ++--
drivers/gpu/drm/vc4/vc4_v3d.c | 4 ++--
5 files changed, 16 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
index 72d30d90b856..ff332bac25d1 100644
--- a/drivers/gpu/drm/vc4/vc4_bo.c
+++ b/drivers/gpu/drm/vc4/vc4_bo.c
@@ -65,8 +65,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_simple_info_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);

diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
index 4fbbf980a299..e31e400c8cfc 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -11,10 +11,6 @@
#include "vc4_drv.h"
#include "vc4_regs.h"

-struct vc4_debugfs_info_entry {
- struct list_head link;
- struct drm_info_list info;
-};

/**
* Called at drm_dev_register() time on each of the minors registered
@@ -24,21 +20,15 @@ void
vc4_debugfs_init(struct drm_minor *minor)
{
struct vc4_dev *vc4 = to_vc4_dev(minor->dev);
- struct vc4_debugfs_info_entry *entry;

debugfs_create_bool("hvs_load_tracker", S_IRUGO | S_IWUSR,
minor->debugfs_root, &vc4->load_tracker_enabled);
-
- list_for_each_entry(entry, &vc4->debugfs_list, link) {
- drm_debugfs_create_files(&entry->info, 1,
- minor->debugfs_root, minor);
- }
}

static int vc4_debugfs_regset32(struct seq_file *m, void *unused)
{
- struct drm_info_node *node = (struct drm_info_node *)m->private;
- struct debugfs_regset32 *regset = node->info_ent->data;
+ struct drm_simple_info_entry *entry = m->private;
+ struct debugfs_regset32 *regset = entry->file.data;
struct drm_printer p = drm_seq_file_printer(m);

drm_print_regset32(&p, regset);
@@ -49,30 +39,18 @@ static int vc4_debugfs_regset32(struct seq_file *m, void *unused)
/**
* Registers a debugfs file with a callback function for a vc4 component.
*
- * This is like drm_debugfs_create_files(), but that can only be
- * called a given DRM minor, while the various VC4 components want to
- * register their debugfs files during the component bind process. We
- * track the request and delay it to be called on each minor during
- * vc4_debugfs_init().
+ * Various VC4 functions will register their debugfs files during the
+ * component bind process using drm_debugfs_add_file().
+ * These requests are tracked and delayed until their called on each
+ * minor during drm_dev_register().
+ *
*/
void vc4_debugfs_add_file(struct drm_device *dev,
const char *name,
int (*show)(struct seq_file*, void*),
void *data)
{
- struct vc4_dev *vc4 = to_vc4_dev(dev);
-
- struct vc4_debugfs_info_entry *entry =
- devm_kzalloc(dev->dev, sizeof(*entry), GFP_KERNEL);
-
- if (!entry)
- return;
-
- entry->info.name = name;
- entry->info.show = show;
- entry->info.data = data;
-
- list_add(&entry->link, &vc4->debugfs_list);
+ drm_debugfs_add_file(dev, name, show, data);
}

void vc4_debugfs_add_regset32(struct drm_device *drm,
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 625bfcf52dc4..05b2a3161148 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -182,8 +182,8 @@ static const struct debugfs_reg32 hd_regs[] = {

static int vc4_hdmi_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_simple_info_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct vc4_dev *vc4 = to_vc4_dev(dev);
struct vc4_hdmi *hdmi = vc4->hdmi;
struct drm_printer p = drm_seq_file_printer(m);
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 5a43659da319..4c254c027649 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -82,8 +82,8 @@ void vc4_hvs_dump_state(struct drm_device *dev)

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_simple_info_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);

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index cea77a21b205..cdef61666c42 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -98,8 +98,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_simple_info_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);

--
2.26.2

2020-05-13 11:45:04

by Wambui Karuga

[permalink] [raw]
Subject: [RFC PATCH 3/3] drm: use new debugfs functions for various files.

Replace the use of drm_debugfs_create_files with the new
drm_debugfs_add_files() to create various drm core debugfs files.

DRM debugfs files are also represented using the new drm_simple_info
struct for use with the new functions.

Signed-off-by: Wambui Karuga <[email protected]>
---
drivers/gpu/drm/drm_atomic.c | 11 +++++------
drivers/gpu/drm/drm_client.c | 11 +++++------
drivers/gpu/drm/drm_framebuffer.c | 11 +++++------
3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 965173fd0ac2..1ec8d74955fd 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1627,8 +1627,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_simple_info_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);
@@ -1637,14 +1637,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_simple_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 8cb93f5209a4..e899683f752d 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -440,8 +440,8 @@ EXPORT_SYMBOL(drm_client_framebuffer_delete);
#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_simple_info_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct drm_printer p = drm_seq_file_printer(m);
struct drm_client_dev *client;

@@ -453,14 +453,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_simple_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_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 0375b3d7f8d0..8fd346279c01 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -1188,8 +1188,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_simple_info_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct drm_printer p = drm_seq_file_printer(m);
struct drm_framebuffer *fb;

@@ -1203,14 +1203,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_simple_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
--
2.26.2

2020-05-13 20:40:54

by Wambui Karuga

[permalink] [raw]
Subject: [RFC PATCH 1/3] drm/debugfs: create debugfs files during drm_dev_register().

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 file requests to a new list tied to drm_device.
During drm_dev_register(), the new function drm_debugfs_create_file()
will iterate over the list of added files on a given minor to create
them.

Two new structs are introduced in this change: struct drm_simple_info
which represents a drm debugfs file entry and struct
drm_simple_info_entry which is used to track file requests and is the
main parameter of choice passed by functions. Each drm_simple_info_entry is
added to the new struct drm_device->debugfs_list for file requests.

Signed-off-by: Wambui Karuga <[email protected]>
---
drivers/gpu/drm/drm_debugfs.c | 59 ++++++++++++++++++++++++++++++++---
drivers/gpu/drm/drm_drv.c | 2 ++
include/drm/drm_debugfs.h | 38 ++++++++++++++++++++++
include/drm/drm_device.h | 12 +++++++
4 files changed, 107 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 2bea22130703..03b0588ede68 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -145,9 +145,10 @@ static const struct drm_info_list drm_debugfs_list[] = {

static int drm_debugfs_open(struct inode *inode, struct file *file)
{
- struct drm_info_node *node = inode->i_private;
+ struct drm_simple_info_entry *entry = inode->i_private;
+ struct drm_simple_info *node = &entry->file;

- return single_open(file, node->info_ent->show, node);
+ return single_open(file, node->show_fn, entry);
}


@@ -159,6 +160,25 @@ static const struct file_operations drm_debugfs_fops = {
.release = single_release,
};

+/**
+ * drm_debugfs_create_file - create DRM debugfs file.
+ * @dev: drm_device that the file belongs to
+ *
+ * Create a DRM debugfs file from the list of files to be created
+ * from dev->debugfs_list.
+ */
+static void drm_debugfs_create_file(struct drm_minor *minor)
+{
+ struct drm_device *dev = minor->dev;
+ struct drm_simple_info_entry *entry;
+
+ list_for_each_entry(entry, &dev->debugfs_list, list) {
+ debugfs_create_file(entry->file.name,
+ S_IFREG | S_IRUGO, minor->debugfs_root,
+ entry,
+ &drm_debugfs_fops);
+ }
+}

/**
* drm_debugfs_create_files - Initialize a given set of debugfs files for DRM
@@ -213,8 +233,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_create_file(minor);

if (drm_drv_uses_atomic_modeset(dev)) {
drm_atomic_debugfs_init(minor);
@@ -449,4 +468,36 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
crtc->debugfs_entry = NULL;
}

+void drm_debugfs_add_file(struct drm_device *dev, const char *name,
+ drm_simple_show_t show_fn, void *data)
+{
+ struct drm_simple_info_entry *entry =
+ kzalloc(sizeof(*entry), GFP_KERNEL);
+
+ if (!entry)
+ return;
+
+ entry->file.name = name;
+ entry->file.show_fn = show_fn;
+ 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);
+
+void drm_debugfs_add_files(struct drm_device *dev,
+ const struct drm_simple_info *files, int count)
+{
+ int i;
+
+ for (i = 0; i < count; i++) {
+ drm_debugfs_add_file(dev, files[i].name, files[i].show_fn,
+ files[i].data);
+ }
+}
+EXPORT_SYMBOL(drm_debugfs_add_files);
+
#endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index bc38322f306e..c68df4e31aa0 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -646,12 +646,14 @@ 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(dev, drm_dev_init_release, NULL);
if (ret)
diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
index 2188dc83957f..bbce580c3b38 100644
--- a/include/drm/drm_debugfs.h
+++ b/include/drm/drm_debugfs.h
@@ -34,6 +34,44 @@

#include <linux/types.h>
#include <linux/seq_file.h>
+
+struct drm_device;
+
+typedef int (*drm_simple_show_t)(struct seq_file *, void *);
+
+/**
+ * struct drm_simple_info - debugfs file entry
+ *
+ * This struct represents a debugfs file to be created.
+ */
+struct drm_simple_info {
+ const char *name;
+ drm_simple_show_t show_fn;
+ u32 driver_features;
+ void *data;
+};
+
+/**
+ * struct drm_simple_info_entry - debugfs list entry
+ *
+ * This struct is used in tracking requests for new debugfs files
+ * to be created.
+ */
+struct drm_simple_info_entry {
+ struct drm_device *dev;
+ struct drm_simple_info file;
+ struct list_head list;
+};
+
+void drm_debugfs_add_file(struct drm_device *dev,
+ const char *name,
+ drm_simple_show_t show_fn,
+ void *data);
+
+void drm_debugfs_add_files(struct drm_device *dev,
+ const struct drm_simple_info *files,
+ int count);
+
/**
* struct drm_info_list - debugfs info list entry
*
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index a55874db9dd4..b84dfdac27b7 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -326,6 +326,18 @@ 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 add.
+ * Files are 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.26.2

2020-05-13 20:48:17

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] drm/debugfs: create debugfs files during drm_dev_register().

Hi

Am 13.05.20 um 13:41 schrieb 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 file requests to a new list tied to drm_device.
> During drm_dev_register(), the new function drm_debugfs_create_file()
> will iterate over the list of added files on a given minor to create
> them.
>
> Two new structs are introduced in this change: struct drm_simple_info
> which represents a drm debugfs file entry and struct
> drm_simple_info_entry which is used to track file requests and is the
> main parameter of choice passed by functions. Each drm_simple_info_entry is
> added to the new struct drm_device->debugfs_list for file requests.
>
> Signed-off-by: Wambui Karuga <[email protected]>
> ---
> drivers/gpu/drm/drm_debugfs.c | 59 ++++++++++++++++++++++++++++++++---
> drivers/gpu/drm/drm_drv.c | 2 ++
> include/drm/drm_debugfs.h | 38 ++++++++++++++++++++++
> include/drm/drm_device.h | 12 +++++++
> 4 files changed, 107 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 2bea22130703..03b0588ede68 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -145,9 +145,10 @@ static const struct drm_info_list drm_debugfs_list[] = {
>
> static int drm_debugfs_open(struct inode *inode, struct file *file)
> {
> - struct drm_info_node *node = inode->i_private;
> + struct drm_simple_info_entry *entry = inode->i_private;
> + struct drm_simple_info *node = &entry->file;
>
> - return single_open(file, node->info_ent->show, node);
> + return single_open(file, node->show_fn, entry);
> }
>
>
> @@ -159,6 +160,25 @@ static const struct file_operations drm_debugfs_fops = {
> .release = single_release,
> };
>
> +/**
> + * drm_debugfs_create_file - create DRM debugfs file.
> + * @dev: drm_device that the file belongs to
> + *
> + * Create a DRM debugfs file from the list of files to be created
> + * from dev->debugfs_list.
> + */
> +static void drm_debugfs_create_file(struct drm_minor *minor)

This function creates several files. I'd rather call it
drm_debugfs_create_added_files().

> +{
> + struct drm_device *dev = minor->dev;
> + struct drm_simple_info_entry *entry;
> +
> + list_for_each_entry(entry, &dev->debugfs_list, list) {
> + debugfs_create_file(entry->file.name,
> + S_IFREG | S_IRUGO, minor->debugfs_root,
> + entry,
> + &drm_debugfs_fops);
> + }

I think the created items should be removed from the list. That way,
drivers can call the function multiple times without recreating the same
files again.

> +}
>
> /**
> * drm_debugfs_create_files - Initialize a given set of debugfs files for DRM
> @@ -213,8 +233,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);

By removing these two lines, aren't you losing the files listed in
DRM_DEBUGFS_ENTRIES?

> + drm_debugfs_create_file(minor);
>
> if (drm_drv_uses_atomic_modeset(dev)) {
> drm_atomic_debugfs_init(minor);
> @@ -449,4 +468,36 @@ void drm_debugfs_crtc_remove(struct drm_crtc *crtc)
> crtc->debugfs_entry = NULL;
> }
>
> +void drm_debugfs_add_file(struct drm_device *dev, const char *name,
> + drm_simple_show_t show_fn, void *data)
> +{
> + struct drm_simple_info_entry *entry =
> + kzalloc(sizeof(*entry), GFP_KERNEL);
> +
> + if (!entry)
> + return;
> +
> + entry->file.name = name;
> + entry->file.show_fn = show_fn;
> + 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);
> +
> +void drm_debugfs_add_files(struct drm_device *dev,
> + const struct drm_simple_info *files, int count)
> +{
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + drm_debugfs_add_file(dev, files[i].name, files[i].show_fn,
> + files[i].data);
> + }
> +}
> +EXPORT_SYMBOL(drm_debugfs_add_files);
> +
> #endif /* CONFIG_DEBUG_FS */
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index bc38322f306e..c68df4e31aa0 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -646,12 +646,14 @@ 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(dev, drm_dev_init_release, NULL);
> if (ret)
> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> index 2188dc83957f..bbce580c3b38 100644
> --- a/include/drm/drm_debugfs.h
> +++ b/include/drm/drm_debugfs.h
> @@ -34,6 +34,44 @@
>
> #include <linux/types.h>
> #include <linux/seq_file.h>
> +
> +struct drm_device;
> +
> +typedef int (*drm_simple_show_t)(struct seq_file *, void *);
> +
> +/**
> + * struct drm_simple_info - debugfs file entry
> + *
> + * This struct represents a debugfs file to be created.
> + */
> +struct drm_simple_info {

drm_simple_info and drm_simple_info_entry seem to be misnomers. They
should probably have some reference to debugfs in their name.

Best regards
Thomas


> + const char *name;
> + drm_simple_show_t show_fn;
> + u32 driver_features;
> + void *data;
> +};
> +
> +/**
> + * struct drm_simple_info_entry - debugfs list entry
> + *
> + * This struct is used in tracking requests for new debugfs files
> + * to be created.
> + */
> +struct drm_simple_info_entry {
> + struct drm_device *dev;
> + struct drm_simple_info file;
> + struct list_head list;
> +};
> +
> +void drm_debugfs_add_file(struct drm_device *dev,
> + const char *name,
> + drm_simple_show_t show_fn,
> + void *data);
> +
> +void drm_debugfs_add_files(struct drm_device *dev,
> + const struct drm_simple_info *files,
> + int count);
> +
> /**
> * struct drm_info_list - debugfs info list entry
> *
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index a55874db9dd4..b84dfdac27b7 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -326,6 +326,18 @@ 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 add.
> + * Files are 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)
>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


Attachments:
signature.asc (499.00 B)
OpenPGP digital signature