[RFC 0/1] drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor

[RFC 0/1] drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor

[Jagath Jog J]

This patch updates the MIPI DBI driver to use drm_device.debugfs_root
instead of drm_minor for creating debugfs files. The debugfs setup is now
done earlier in probe(), before drm_dev_register(), and the drivers can
avoid using the .debugfs_init callback.

This is an initial version, and only a few drivers are updated for now.

I noticed that some newer drivers or patches still use
drm_debugfs_create_files(), which relies on drm_minor. I was wondering if
there is a specific reason for this, or if there's a plan to switch to
drm_debugfs_add_files? I can send patches to update more drivers if
that helps.

This patch helps move toward the debugfs cleanup task listed here:
https://docs.kernel.org/gpu/todo.html#clean-up-the-debugfs-support

Looking forward to your feedback.

Jagath Jog J (1):
  drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor and
    .debugfs_init

 drivers/gpu/drm/drm_mipi_dbi.c        | 8 ++++----
 drivers/gpu/drm/tiny/ili9163.c        | 3 ++-
 drivers/gpu/drm/tiny/panel-mipi-dbi.c | 3 ++-
 include/drm/drm_mipi_dbi.h            | 4 ++--
 4 files changed, 10 insertions(+), 8 deletions(-)

-- 
2.20.1

[RFC 1/1] drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor and .debugfs_init

[Jagath Jog J]

Refactor to use drm_device.debugfs_root instead of drm_minor for
debugfs file creation. The driver can now initialize debugfs directly
in probe(), before drm_dev_register(). This also removes the use of
.debugfs_init callback.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/gpu/drm/drm_mipi_dbi.c        | 8 ++++----
 drivers/gpu/drm/tiny/ili9163.c        | 3 ++-
 drivers/gpu/drm/tiny/panel-mipi-dbi.c | 3 ++-
 include/drm/drm_mipi_dbi.h            | 4 ++--
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 89e05a5bed1d..66f292c48a78 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -1488,17 +1488,17 @@ static const struct file_operations mipi_dbi_debugfs_command_fops = {
  *
  * This function creates a 'command' debugfs file for sending commands to the
  * controller or getting the read command values.
- * Drivers can use this as their &drm_driver->debugfs_init callback.
+ * Drivers can call this function before registering their device to drm.
  *
  */
-void mipi_dbi_debugfs_init(struct drm_minor *minor)
+void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev)
 {
-	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(minor->dev);
 	umode_t mode = S_IFREG | S_IWUSR;
 
 	if (dbidev->dbi.read_commands)
 		mode |= S_IRUGO;
-	debugfs_create_file("command", mode, minor->debugfs_root, dbidev,
+
+	debugfs_create_file("command", mode, dbidev->drm.debugfs_root, dbidev,
 			    &mipi_dbi_debugfs_command_fops);
 }
 EXPORT_SYMBOL(mipi_dbi_debugfs_init);
diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
index 62cadf5e033d..351d2a5b9f27 100644
--- a/drivers/gpu/drm/tiny/ili9163.c
+++ b/drivers/gpu/drm/tiny/ili9163.c
@@ -115,7 +115,6 @@ static struct drm_driver ili9163_driver = {
 	.fops			= &ili9163_fops,
 	DRM_GEM_DMA_DRIVER_OPS_VMAP,
 	DRM_FBDEV_DMA_DRIVER_OPS,
-	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "ili9163",
 	.desc			= "Ilitek ILI9163",
 	.major			= 1,
@@ -182,6 +181,8 @@ static int ili9163_probe(struct spi_device *spi)
 
 	drm_mode_config_reset(drm);
 
+	mipi_dbi_debugfs_init(dbidev);
+
 	ret = drm_dev_register(drm, 0);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
index 0460ecaef4bd..94466dd8db9f 100644
--- a/drivers/gpu/drm/tiny/panel-mipi-dbi.c
+++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
@@ -267,7 +267,6 @@ static const struct drm_driver panel_mipi_dbi_driver = {
 	.fops			= &panel_mipi_dbi_fops,
 	DRM_GEM_DMA_DRIVER_OPS_VMAP,
 	DRM_FBDEV_DMA_DRIVER_OPS,
-	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "panel-mipi-dbi",
 	.desc			= "MIPI DBI compatible display panel",
 	.major			= 1,
@@ -384,6 +383,8 @@ static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
 
 	drm_mode_config_reset(drm);
 
+	mipi_dbi_debugfs_init(dbidev);
+
 	ret = drm_dev_register(drm, 0);
 	if (ret)
 		return ret;
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index f45f9612c0bc..88a9c87a1e99 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -230,9 +230,9 @@ int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *
 })
 
 #ifdef CONFIG_DEBUG_FS
-void mipi_dbi_debugfs_init(struct drm_minor *minor);
+void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev);
 #else
-static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
+static inline void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev) {}
 #endif
 
 /**
-- 
2.20.1

Re: [RFC 1/1] drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor and .debugfs_init

[Dmitry Baryshkov]

On Mon, Apr 21, 2025 at 02:29:07PM +0530, Jagath Jog J wrote:
> Refactor to use drm_device.debugfs_root instead of drm_minor for
> debugfs file creation. The driver can now initialize debugfs directly
> in probe(), before drm_dev_register(). This also removes the use of
> .debugfs_init callback.

Why? The callback was designed to add debugfs files. Likewise most if
not all DRM drivers add files under the corresponding minor node.

> 
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> ---
>  drivers/gpu/drm/drm_mipi_dbi.c        | 8 ++++----
>  drivers/gpu/drm/tiny/ili9163.c        | 3 ++-
>  drivers/gpu/drm/tiny/panel-mipi-dbi.c | 3 ++-
>  include/drm/drm_mipi_dbi.h            | 4 ++--
>  4 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index 89e05a5bed1d..66f292c48a78 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -1488,17 +1488,17 @@ static const struct file_operations mipi_dbi_debugfs_command_fops = {
>   *
>   * This function creates a 'command' debugfs file for sending commands to the
>   * controller or getting the read command values.
> - * Drivers can use this as their &drm_driver->debugfs_init callback.
> + * Drivers can call this function before registering their device to drm.
>   *
>   */
> -void mipi_dbi_debugfs_init(struct drm_minor *minor)
> +void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev)
>  {
> -	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(minor->dev);
>  	umode_t mode = S_IFREG | S_IWUSR;
>  
>  	if (dbidev->dbi.read_commands)
>  		mode |= S_IRUGO;
> -	debugfs_create_file("command", mode, minor->debugfs_root, dbidev,
> +
> +	debugfs_create_file("command", mode, dbidev->drm.debugfs_root, dbidev,
>  			    &mipi_dbi_debugfs_command_fops);
>  }
>  EXPORT_SYMBOL(mipi_dbi_debugfs_init);
> diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
> index 62cadf5e033d..351d2a5b9f27 100644
> --- a/drivers/gpu/drm/tiny/ili9163.c
> +++ b/drivers/gpu/drm/tiny/ili9163.c
> @@ -115,7 +115,6 @@ static struct drm_driver ili9163_driver = {
>  	.fops			= &ili9163_fops,
>  	DRM_GEM_DMA_DRIVER_OPS_VMAP,
>  	DRM_FBDEV_DMA_DRIVER_OPS,
> -	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "ili9163",
>  	.desc			= "Ilitek ILI9163",
>  	.major			= 1,
> @@ -182,6 +181,8 @@ static int ili9163_probe(struct spi_device *spi)
>  
>  	drm_mode_config_reset(drm);
>  
> +	mipi_dbi_debugfs_init(dbidev);
> +
>  	ret = drm_dev_register(drm, 0);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> index 0460ecaef4bd..94466dd8db9f 100644
> --- a/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> @@ -267,7 +267,6 @@ static const struct drm_driver panel_mipi_dbi_driver = {
>  	.fops			= &panel_mipi_dbi_fops,
>  	DRM_GEM_DMA_DRIVER_OPS_VMAP,
>  	DRM_FBDEV_DMA_DRIVER_OPS,
> -	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "panel-mipi-dbi",
>  	.desc			= "MIPI DBI compatible display panel",
>  	.major			= 1,
> @@ -384,6 +383,8 @@ static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
>  
>  	drm_mode_config_reset(drm);
>  
> +	mipi_dbi_debugfs_init(dbidev);
> +
>  	ret = drm_dev_register(drm, 0);
>  	if (ret)
>  		return ret;
> diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
> index f45f9612c0bc..88a9c87a1e99 100644
> --- a/include/drm/drm_mipi_dbi.h
> +++ b/include/drm/drm_mipi_dbi.h
> @@ -230,9 +230,9 @@ int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *
>  })
>  
>  #ifdef CONFIG_DEBUG_FS
> -void mipi_dbi_debugfs_init(struct drm_minor *minor);
> +void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev);
>  #else
> -static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
> +static inline void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev) {}
>  #endif
>  
>  /**
> -- 
> 2.20.1
> 

-- 
With best wishes
Dmitry

Re: [RFC 1/1] drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor and .debugfs_init

[Jagath Jog J]

Hi Dmitry

Thanks for the reply

On Mon, Apr 21, 2025 at 3:59 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Mon, Apr 21, 2025 at 02:29:07PM +0530, Jagath Jog J wrote:
> > Refactor to use drm_device.debugfs_root instead of drm_minor for
> > debugfs file creation. The driver can now initialize debugfs directly
> > in probe(), before drm_dev_register(). This also removes the use of
> > .debugfs_init callback.
>
> Why? The callback was designed to add debugfs files. Likewise most if
> not all DRM drivers add files under the corresponding minor node.

My intention here was to follow the direction suggested in the DRM TODO list
https://docs.kernel.org/gpu/todo.html#clean-up-the-debugfs-support

It recommends using drm_device instead of drm_minor, transitioning towards
drm_device.debugfs_root, and avoiding .debugfs_init. The RFC patch was
an initial
step to gather feedback.

Related to this todo, some drivers use drm_debugfs_add_files() instead of
drm_debugfs_create_files(). For example

hdlcd - https://patchwork.freedesktop.org/patch/msgid/20221226155029.244355-4-mcanal@igalia.com
v3d - https://patchwork.freedesktop.org/patch/msgid/20221219120621.15086-6-mcanal@igalia.com
https://elixir.bootlin.com/linux/v6.14.3/source/include/drm/drm_device.h#L92

Please let me know your thoughts on this.


Regards
Jagath
>
> >
> > Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_mipi_dbi.c        | 8 ++++----
> >  drivers/gpu/drm/tiny/ili9163.c        | 3 ++-
> >  drivers/gpu/drm/tiny/panel-mipi-dbi.c | 3 ++-
> >  include/drm/drm_mipi_dbi.h            | 4 ++--
> >  4 files changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> > index 89e05a5bed1d..66f292c48a78 100644
> > --- a/drivers/gpu/drm/drm_mipi_dbi.c
> > +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> > @@ -1488,17 +1488,17 @@ static const struct file_operations mipi_dbi_debugfs_command_fops = {
> >   *
> >   * This function creates a 'command' debugfs file for sending commands to the
> >   * controller or getting the read command values.
> > - * Drivers can use this as their &drm_driver->debugfs_init callback.
> > + * Drivers can call this function before registering their device to drm.
> >   *
> >   */
> > -void mipi_dbi_debugfs_init(struct drm_minor *minor)
> > +void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev)
> >  {
> > -     struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(minor->dev);
> >       umode_t mode = S_IFREG | S_IWUSR;
> >
> >       if (dbidev->dbi.read_commands)
> >               mode |= S_IRUGO;
> > -     debugfs_create_file("command", mode, minor->debugfs_root, dbidev,
> > +
> > +     debugfs_create_file("command", mode, dbidev->drm.debugfs_root, dbidev,
> >                           &mipi_dbi_debugfs_command_fops);
> >  }
> >  EXPORT_SYMBOL(mipi_dbi_debugfs_init);
> > diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
> > index 62cadf5e033d..351d2a5b9f27 100644
> > --- a/drivers/gpu/drm/tiny/ili9163.c
> > +++ b/drivers/gpu/drm/tiny/ili9163.c
> > @@ -115,7 +115,6 @@ static struct drm_driver ili9163_driver = {
> >       .fops                   = &ili9163_fops,
> >       DRM_GEM_DMA_DRIVER_OPS_VMAP,
> >       DRM_FBDEV_DMA_DRIVER_OPS,
> > -     .debugfs_init           = mipi_dbi_debugfs_init,
> >       .name                   = "ili9163",
> >       .desc                   = "Ilitek ILI9163",
> >       .major                  = 1,
> > @@ -182,6 +181,8 @@ static int ili9163_probe(struct spi_device *spi)
> >
> >       drm_mode_config_reset(drm);
> >
> > +     mipi_dbi_debugfs_init(dbidev);
> > +
> >       ret = drm_dev_register(drm, 0);
> >       if (ret)
> >               return ret;
> > diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> > index 0460ecaef4bd..94466dd8db9f 100644
> > --- a/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> > +++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
> > @@ -267,7 +267,6 @@ static const struct drm_driver panel_mipi_dbi_driver = {
> >       .fops                   = &panel_mipi_dbi_fops,
> >       DRM_GEM_DMA_DRIVER_OPS_VMAP,
> >       DRM_FBDEV_DMA_DRIVER_OPS,
> > -     .debugfs_init           = mipi_dbi_debugfs_init,
> >       .name                   = "panel-mipi-dbi",
> >       .desc                   = "MIPI DBI compatible display panel",
> >       .major                  = 1,
> > @@ -384,6 +383,8 @@ static int panel_mipi_dbi_spi_probe(struct spi_device *spi)
> >
> >       drm_mode_config_reset(drm);
> >
> > +     mipi_dbi_debugfs_init(dbidev);
> > +
> >       ret = drm_dev_register(drm, 0);
> >       if (ret)
> >               return ret;
> > diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
> > index f45f9612c0bc..88a9c87a1e99 100644
> > --- a/include/drm/drm_mipi_dbi.h
> > +++ b/include/drm/drm_mipi_dbi.h
> > @@ -230,9 +230,9 @@ int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *
> >  })
> >
> >  #ifdef CONFIG_DEBUG_FS
> > -void mipi_dbi_debugfs_init(struct drm_minor *minor);
> > +void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev);
> >  #else
> > -static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
> > +static inline void mipi_dbi_debugfs_init(struct mipi_dbi_dev *dbidev) {}
> >  #endif
> >
> >  /**
> > --
> > 2.20.1
> >
>
> --
> With best wishes
> Dmitry

Re: [RFC 0/1] drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor

[Maíra Canal]

Hi Jagath,

On 21/04/25 05:59, Jagath Jog J wrote:
> This patch updates the MIPI DBI driver to use drm_device.debugfs_root
> instead of drm_minor for creating debugfs files. The debugfs setup is now
> done earlier in probe(), before drm_dev_register(), and the drivers can
> avoid using the .debugfs_init callback.
> 
> This is an initial version, and only a few drivers are updated for now.
> 
> I noticed that some newer drivers or patches still use
> drm_debugfs_create_files(), which relies on drm_minor. I was wondering if
> there is a specific reason for this, or if there's a plan to switch to
> drm_debugfs_add_files? I can send patches to update more drivers if
> that helps.

Yes, there was a plan to switch to drm_debugfs_add_files(), as it
centers the debugfs files management on the drm_device instead of
drm_minor.

But, in the end, we concluded that we needed a few more changes in the
infrastructure to make things more generic. Here are some discussions
[1][2] and this was my plan to make things more generic [3], which
needed some improvements.

It would be create if you help us to cleanup debugfs and feel free to
use my patches as a base for it.

[1] 
https://lore.kernel.org/dri-devel/20230209081838.45273-1-christian.koenig@amd.com/
[2] https://lore.kernel.org/dri-devel/87eds0gm9b.fsf@intel.com/
[3] 
https://lore.kernel.org/dri-devel/20230131195825.677487-1-mcanal@igalia.com/

Best Regards,
- Maíra

> 
> This patch helps move toward the debugfs cleanup task listed here:
> https://docs.kernel.org/gpu/todo.html#clean-up-the-debugfs-support
> 
> Looking forward to your feedback.
> 
> Jagath Jog J (1):
>    drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor and
>      .debugfs_init
> 
>   drivers/gpu/drm/drm_mipi_dbi.c        | 8 ++++----
>   drivers/gpu/drm/tiny/ili9163.c        | 3 ++-
>   drivers/gpu/drm/tiny/panel-mipi-dbi.c | 3 ++-
>   include/drm/drm_mipi_dbi.h            | 4 ++--
>   4 files changed, 10 insertions(+), 8 deletions(-)
> 

Re: [RFC 0/1] drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor

[Jagath Jog J]

Hi Maíra,

On Wed, Apr 23, 2025 at 4:22 PM Maíra Canal <mcanal@igalia.com> wrote:
>
> Hi Jagath,
>
> On 21/04/25 05:59, Jagath Jog J wrote:
> > This patch updates the MIPI DBI driver to use drm_device.debugfs_root
> > instead of drm_minor for creating debugfs files. The debugfs setup is now
> > done earlier in probe(), before drm_dev_register(), and the drivers can
> > avoid using the .debugfs_init callback.
> >
> > This is an initial version, and only a few drivers are updated for now.
> >
> > I noticed that some newer drivers or patches still use
> > drm_debugfs_create_files(), which relies on drm_minor. I was wondering if
> > there is a specific reason for this, or if there's a plan to switch to
> > drm_debugfs_add_files? I can send patches to update more drivers if
> > that helps.
>
> Yes, there was a plan to switch to drm_debugfs_add_files(), as it
> centers the debugfs files management on the drm_device instead of
> drm_minor.
>
> But, in the end, we concluded that we needed a few more changes in the
> infrastructure to make things more generic. Here are some discussions
> [1][2] and this was my plan to make things more generic [3], which
> needed some improvements.
>
> It would be create if you help us to cleanup debugfs and feel free to
> use my patches as a base for it.
>
> [1]
> https://lore.kernel.org/dri-devel/20230209081838.45273-1-christian.koenig@amd.com/
> [2] https://lore.kernel.org/dri-devel/87eds0gm9b.fsf@intel.com/
> [3]
> https://lore.kernel.org/dri-devel/20230131195825.677487-1-mcanal@igalia.com/

Thanks for sharing the links. I had missed these discussions.
I will go through them and get back soon. Since these touch core DRM infra,
I will study the changes more closely before proceeding.
Thanks for sharing your patches as a base, that should help a lot.


Regards
Jagath

>
> Best Regards,
> - Maíra
>
> >
> > This patch helps move toward the debugfs cleanup task listed here:
> > https://docs.kernel.org/gpu/todo.html#clean-up-the-debugfs-support
> >
> > Looking forward to your feedback.
> >
> > Jagath Jog J (1):
> >    drm/mipi-dbi: Use drm_device for debugfs, drop drm_minor and
> >      .debugfs_init
> >
> >   drivers/gpu/drm/drm_mipi_dbi.c        | 8 ++++----
> >   drivers/gpu/drm/tiny/ili9163.c        | 3 ++-
> >   drivers/gpu/drm/tiny/panel-mipi-dbi.c | 3 ++-
> >   include/drm/drm_mipi_dbi.h            | 4 ++--
> >   4 files changed, 10 insertions(+), 8 deletions(-)
> >
>