diff mbox series

[03/13] drm/debugfs: Create a debugfs infrastructure for connectors

Message ID 20230111173748.752659-4-mcanal@igalia.com (mailing list archive)
State New, archived
Headers show
Series drm/debugfs: Create a debugfs infrastructure for kms objects | expand

Commit Message

Maíra Canal Jan. 11, 2023, 5:37 p.m. UTC
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@igalia.com>
---
 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(+)

Comments

Jani Nikula Jan. 12, 2023, 9:07 a.m. UTC | #1
On Wed, 11 Jan 2023, Maíra Canal <mcanal@igalia.com> 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@igalia.com>
> ---
>  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);

In another mail, I suggested adding a struct wrapper for debugfs_list
and debugfs_mutex. I think those should also be initialized by debugfs
code.

The initializer would not have to be connector/encoder/crtc specific, it
could be:

	drm_debugfs_something_init(&connector->debugfs_something);

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

Ditto for cleanup.

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

Just perhaps this should be called _register()? The name gives a strong
feeling at which stage it is called, and the init here feels like it
should be moved earlier, to connector init, instead of connector
register.

> +
>  	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;
> +};
> +
>  #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_ */
Daniel Vetter Jan. 12, 2023, 9:14 a.m. UTC | #2
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@igalia.com>
> ---
>  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
>
Jani Nikula Jan. 12, 2023, 9:21 a.m. UTC | #3
On Thu, 12 Jan 2023, Daniel Vetter <daniel@ffwll.ch> wrote:
> 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@igalia.com>
>> ---
>>  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

See also https://lore.kernel.org/r/878ri8glee.fsf@intel.com

>
>> +
>>  #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
>>
diff mbox series

Patch

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;
+};
+
 #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_ */