[PATCH v3 1/6] drm/debugfs: Introduce wrapper for debugfs list

[Daniel Vetter]

On Tue, Jan 31, 2023 at 04:58:21PM -0300, Maíra Canal wrote:
> Introduce a struct wrapper for all the debugfs-related stuff: the list
> of debugfs files and the mutex that protects it. This will make it
> easier to initialize all the debugfs list in a DRM object and will
> create a good abstraction for a possible implementation of the debugfs
> infrastructure for KMS objects.
> 
> Signed-off-by: Maíra Canal <mcanal at igalia.com>
> ---
>  drivers/gpu/drm/drm_debugfs.c  | 18 ++++++++++++++++++
>  drivers/gpu/drm/drm_internal.h | 12 ++++++++++++
>  include/drm/drm_debugfs.h      | 16 ++++++++++++++++
>  3 files changed, 46 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 4f643a490dc3..8658d3929ea5 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -218,6 +218,24 @@ void drm_debugfs_create_files(const struct drm_info_list *files, int count,
>  }
>  EXPORT_SYMBOL(drm_debugfs_create_files);
>  
> +struct drm_debugfs_files *drm_debugfs_files_init(void)
> +{
> +	struct drm_debugfs_files *debugfs_files;
> +
> +	debugfs_files = kzalloc(sizeof(*debugfs_files), GFP_KERNEL);
> +
> +	INIT_LIST_HEAD(&debugfs_files->list);
> +	mutex_init(&debugfs_files->mutex);
> +
> +	return debugfs_files;
> +}
> +
> +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files)
> +{
> +	mutex_destroy(&debugfs_files->mutex);
> +	kfree(debugfs_files);
> +}
> +
>  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		     struct dentry *root)
>  {
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index ed2103ee272c..f1c8766ed828 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -23,6 +23,7 @@
>  
>  #include <linux/kthread.h>
>  
> +#include <drm/drm_debugfs.h>
>  #include <drm/drm_ioctl.h>
>  #include <drm/drm_vblank.h>
>  
> @@ -183,6 +184,8 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev,
>  
>  /* drm_debugfs.c drm_debugfs_crc.c */
>  #if defined(CONFIG_DEBUG_FS)
> +struct drm_debugfs_files *drm_debugfs_files_init(void);
> +void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files);
>  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		     struct dentry *root);
>  void drm_debugfs_cleanup(struct drm_minor *minor);
> @@ -193,6 +196,15 @@ void drm_debugfs_crtc_add(struct drm_crtc *crtc);
>  void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
>  void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
>  #else
> +static inline struct drm_debugfs_files *drm_debugfs_files_init(void)
> +{
> +	return NULL;
> +}
> +
> +static inline void drm_debugfs_files_destroy(struct drm_debugfs_files *debugfs_files)
> +{
> +}
> +
>  static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  				   struct dentry *root)
>  {
> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> index 7616f457ce70..423aa3de506a 100644
> --- a/include/drm/drm_debugfs.h
> +++ b/include/drm/drm_debugfs.h
> @@ -32,6 +32,8 @@
>  #ifndef _DRM_DEBUGFS_H_
>  #define _DRM_DEBUGFS_H_
>  
> +#include <linux/list.h>
> +#include <linux/mutex.h>
>  #include <linux/types.h>
>  #include <linux/seq_file.h>
>  /**
> @@ -79,6 +81,20 @@ struct drm_info_node {
>  	struct dentry *dent;
>  };
>  
> +/**
> + * struct drm_debugfs_files - Encapsulates the debugfs list and its mutex
> + *
> + * This structure represents the debugfs list of files and is encapsulated
> + * with a mutex to protect the access of the list.
> + */
> +struct drm_debugfs_files {
> +	/** @list: List of debugfs files to be created by the DRM object. */
> +	struct list_head list;
> +
> +	/** @mutex: Protects &list access. */
> +	struct mutex mutex;

I'm not seeing any use for the mutex here? Also unless you also plan to
put like the debugfs directory pointers in this struct, I'm not sure we
need this abstraction since it's purely internal to debugfs code (so also
should really be in the headers where drivers could perhaps come up with
funny ideas).
-Daniel

> +};
> +
>  /**
>   * struct drm_debugfs_info - debugfs info list entry
>   *
> -- 
> 2.39.1
> 

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