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(-)
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 wambui.karugax@gmail.com --- 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)
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 wambui.karugax@gmail.com
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)
On Wed, 13 May 2020, Thomas Zimmermann wrote:
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 wambui.karugax@gmail.com
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().
Okay, that makes sense. I can change that.
+{
- 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.
Hadn't thought of that - I can try add that.
+}
/**
- 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?
Yes. When using the new functions, drm_debugfs_create_files() should not be called at this point, but for compatibility these two lines should be put back, I think.
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.
I'll change the names.
Thanks, wambui karuga
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
On Wed, May 13, 2020 at 8:12 PM Wambui Karuga wambui.karugax@gmail.com wrote:
On Wed, 13 May 2020, Thomas Zimmermann wrote:
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 wambui.karugax@gmail.com
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().
Okay, that makes sense. I can change that.
+{
- 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.
Hadn't thought of that - I can try add that.
The function here is static, called once by the core. So no need for complicated logic, it's guaranteed to only be called once.
I guess what confused Thomas is the kerneldoc. We generally only do that for functions exported to drivers, that drivers should call. Not the case here. The function name itself is descriptive enough I think (with Thomas' suggestion even better). So I'd just remove the kerneldoc here. -Daniel
+}
/**
- 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?
Yes. When using the new functions, drm_debugfs_create_files() should not be called at this point, but for compatibility these two lines should be put back, I think.
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.
I'll change the names.
Thanks, wambui karuga
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
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 wambui.karugax@gmail.com --- 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);
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 wambui.karugax@gmail.com --- 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
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Thomas Zimmermann -
Wambui Karuga