Hi Dmitry Thanks for the reply
On Mon, Apr 21, 2025 at 3:59 PM Dmitry Baryshkov <dmitry.barysh...@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-mca...@igalia.com v3d - https://patchwork.freedesktop.org/patch/msgid/20221219120621.15086-6-mca...@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 <jagathjog1...@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
