On Tue, 22 Nov 2022, Maíra Canal <mcanal@xxxxxxxxxx> wrote:
> 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@xxxxxxxxx>
> Signed-off-by: Wambui Karuga <wambui.karugax@xxxxxxxxx>
> Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_debugfs.c | 76 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_drv.c | 3 ++
> include/drm/drm_debugfs.h | 45 +++++++++++++++++++++
> include/drm/drm_device.h | 15 +++++++
> 4 files changed, 139 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index ee445f4605ba..ca27c2b05051 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;
> char name[64];
>
> INIT_LIST_HEAD(&minor->debugfs_list);
> @@ -230,6 +247,11 @@ 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(entry, &dev->debugfs_list, list) {
> + debugfs_create_file(entry->file.name, S_IFREG | S_IRUGO,
> + minor->debugfs_root, entry, &drm_debugfs_entry_fops);
> + }
> +
> return 0;
> }
>
> @@ -281,6 +303,60 @@ 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.
> + */
> +int 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 -ENOMEM;
> +
> + 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);
> +
> + return 0;
> +}
> +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.
> + */
> +int drm_debugfs_add_files(struct drm_device *dev, const struct drm_debugfs_info *files, int count)
> +{
> + int i, ret = 0, err;
> +
> + for (i = 0; i < count; i++) {
> + err = drm_debugfs_add_file(dev, files[i].name, files[i].show, files[i].data);
> + if (err)
> + ret = err;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_debugfs_add_files);
Do we want to add return values and error handling to debugfs related
functions at all?
BR,
Jani.
> +
> 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 8214a0b1ab7f..803942008fcb 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(dev, drm_dev_init_release, NULL);
> if (ret)
> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> index 2188dc83957f..c5684d6c5055 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);
> +
> +int drm_debugfs_add_file(struct drm_device *dev, const char *name,
> + int (*show)(struct seq_file*, void*), void *data);
> +
> +int 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,20 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
> {
> return 0;
> }
> +
> +static inline int drm_debugfs_add_file(struct drm_device *dev, const char *name,
> + int (*show)(struct seq_file*, void*),
> + void *data)
> +{
> + return 0;
> +}
> +
> +static inline int drm_debugfs_add_files(struct drm_device *dev,
> + const struct drm_debugfs_info *files,
> + int count)
> +{
> + return 0;
> +}
> #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)
--
Jani Nikula, Intel Open Source Graphics Center
