On Wed, Jan 11, 2023 at 02:37:38PM -0300, Maíra Canal wrote:
> Introduce the ability to add DRM debugfs files to a list managed by the
> connector and, during drm_connector_register(), all added files will be
> created at once.
>
> Moreover, introduce some typesafety as struct drm_debugfs_connector_entry
> holds a drm_connector instead of a drm_device. So, the drivers can get
> a connector object directly from the struct drm_debugfs_connector_entry
> in the show() callback.
>
> Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_connector.c | 5 +++++
> drivers/gpu/drm/drm_debugfs.c | 35 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_internal.h | 5 +++++
> include/drm/drm_connector.h | 15 ++++++++++++++
> include/drm/drm_debugfs.h | 26 ++++++++++++++++++++++++
> 5 files changed, 86 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 8d92777e57dd..c93655bb0edf 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -273,8 +273,10 @@ static int __drm_connector_init(struct drm_device *dev,
> INIT_LIST_HEAD(&connector->global_connector_list_entry);
> INIT_LIST_HEAD(&connector->probed_modes);
> INIT_LIST_HEAD(&connector->modes);
> + INIT_LIST_HEAD(&connector->debugfs_list);
> mutex_init(&connector->mutex);
> mutex_init(&connector->edid_override_mutex);
> + mutex_init(&connector->debugfs_mutex);
> connector->edid_blob_ptr = NULL;
> connector->epoch_counter = 0;
> connector->tile_blob_ptr = NULL;
> @@ -581,6 +583,7 @@ void drm_connector_cleanup(struct drm_connector *connector)
> connector->state);
>
> mutex_destroy(&connector->mutex);
> + mutex_destroy(&connector->debugfs_mutex);
>
> memset(connector, 0, sizeof(*connector));
>
> @@ -627,6 +630,8 @@ int drm_connector_register(struct drm_connector *connector)
> goto err_debugfs;
> }
>
> + drm_debugfs_connector_init(connector);
> +
> drm_mode_object_register(connector->dev, &connector->base);
>
> connector->registration_state = DRM_CONNECTOR_REGISTERED;
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 23f6ed7b5d68..d9ec1ed5a7ec 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -260,6 +260,17 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> return 0;
> }
>
> +void drm_debugfs_connector_init(struct drm_connector *connector)
> +{
> + struct drm_minor *minor = connector->dev->primary;
> + struct drm_debugfs_connector_entry *entry, *tmp;
> +
> + if (!minor)
> + return;
> +
> + drm_create_file_from_list(connector);
> +}
> +
> void drm_debugfs_late_register(struct drm_device *dev)
> {
> struct drm_minor *minor = dev->primary;
> @@ -369,6 +380,30 @@ void drm_debugfs_add_files(struct drm_device *dev, const struct drm_debugfs_info
> }
> EXPORT_SYMBOL(drm_debugfs_add_files);
>
> +/**
> + * drm_debugfs_connector_add_file - Add a given file to the DRM connector debugfs file list
> + * @connector: DRM connector object
> + * @name: debugfs file name
> + * @show: show callback
> + * @data: driver-private data, should not be device-specific
> + *
> + * Add a given file entry to the DRM connector debugfs file list to be created on
> + * drm_debugfs_connector_init().
> + */
> +void drm_debugfs_connector_add_file(struct drm_connector *connector, const char *name,
> + int (*show)(struct seq_file*, void*), void *data)
> +{
> + struct drm_debugfs_connector_entry *entry = drmm_kzalloc(connector->dev,
> + sizeof(*entry),
> + GFP_KERNEL);
> +
> + if (!entry)
> + return;
> +
> + drm_debugfs_add_file_to_list(connector);
> +}
> +EXPORT_SYMBOL(drm_debugfs_connector_add_file);
> +
> static int connector_show(struct seq_file *m, void *data)
> {
> struct drm_connector *connector = m->private;
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index ed2103ee272c..dd9d7b8b45bd 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -185,6 +185,7 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev,
> #if defined(CONFIG_DEBUG_FS)
> int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> struct dentry *root);
> +void drm_debugfs_connector_init(struct drm_connector *connector);
> 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);
> @@ -199,6 +200,10 @@ static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> return 0;
> }
>
> +static inline void drm_debugfs_connector_init(struct drm_connector *connector)
> +{
> +}
> +
> static inline void drm_debugfs_cleanup(struct drm_minor *minor)
> {
> }
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 9037f1317aee..51340f3162ed 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1720,6 +1720,21 @@ struct drm_connector {
> /** @debugfs_entry: debugfs directory for this connector */
> struct dentry *debugfs_entry;
>
> + /**
> + * @debugfs_mutex:
> + *
> + * Protects &debugfs_list access.
> + */
> + struct mutex debugfs_mutex;
> +
> + /**
> + * @debugfs_list:
> + *
> + * List of debugfs files to be created by the DRM connector. The files
> + * must be added during drm_connector_register().
> + */
> + struct list_head debugfs_list;
> +
> /**
> * @state:
> *
> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> index 7616f457ce70..c09c82274622 100644
> --- a/include/drm/drm_debugfs.h
> +++ b/include/drm/drm_debugfs.h
> @@ -122,6 +122,23 @@ struct drm_debugfs_entry {
> struct list_head list;
> };
>
> +/**
> + * struct drm_debugfs_connector_entry - Per-connector debugfs node structure
> + *
> + * This structure represents a debugfs file, as an instantiation of a &struct
> + * drm_debugfs_info on a &struct drm_connector.
> + */
> +struct drm_debugfs_connector_entry {
> + /** @connector: &struct drm_connector for this node. */
> + struct drm_connector *connector;
> +
> + /** @file: Template for this node. */
> + struct drm_debugfs_info file;
> +
> + /** @list: Linked list of all connector nodes. */
> + struct list_head list;
> +};
I missed it in the main api, but I'm not a big fan of exposing this struct
to driver. And I don't see the need ... if we just directly put the
drm_connector into seq_file->private (or an explicit parameter for our
show function, with some glue provided) then drivers don't need to be able
to see this? This really should be just an implementation detail I think.
-Daniel
> +
> #if defined(CONFIG_DEBUG_FS)
> void drm_debugfs_create_files(const struct drm_info_list *files,
> int count, struct dentry *root,
> @@ -134,6 +151,9 @@ void drm_debugfs_add_file(struct drm_device *dev, const char *name,
>
> void drm_debugfs_add_files(struct drm_device *dev,
> const struct drm_debugfs_info *files, int count);
> +
> +void drm_debugfs_connector_add_file(struct drm_connector *connector, const char *name,
> + int (*show)(struct seq_file*, void*), void *data);
> #else
> static inline void drm_debugfs_create_files(const struct drm_info_list *files,
> int count, struct dentry *root,
> @@ -155,6 +175,12 @@ static inline void drm_debugfs_add_files(struct drm_device *dev,
> const struct drm_debugfs_info *files,
> int count)
> {}
> +
> +static inline void drm_debugfs_connector_add_file(struct drm_connector *connector,
> + const char *name,
> + int (*show)(struct seq_file*, void*),
> + void *data)
> +{}
> #endif
>
> #endif /* _DRM_DEBUGFS_H_ */
> --
> 2.39.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
