Re: [PATCH v5 2/5] drm: move debugfs functionality from drm_drv.c to drm_debugfs.c

[Khatri, Sunil]

On 6/26/2025 5:28 PM, Christian König wrote:

On 24.06.25 13:34, Sunil Khatri wrote:

move the debugfs functions from drm_drv.c to drm_debugfs.c

move this root node to the debugfs for easily handling
of future requirements to add more information in the
root directory and one of which is planned to have
directories for each client in the root directory
which is dri.

Suggested-by: Christian König <christian.koe...@amd.com>
Signed-off-by: Sunil Khatri <sunil.kha...@amd.com>
---
  drivers/gpu/drm/drm_debugfs.c  | 37 ++++++++++++++++++++++++++++------
  drivers/gpu/drm/drm_drv.c      | 19 ++++++-----------
  drivers/gpu/drm/drm_internal.h |  6 ++----
  include/drm/drm_drv.h          | 24 ++++++++++++++++++++--
  4 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 2d43bda82887..a227903c29c4 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -44,6 +44,9 @@
  #include "drm_crtc_internal.h"
  #include "drm_internal.h"

+static struct dentry *accel_debugfs_root;

+static struct dentry *drm_debugfs_root;
+
  /***************************************************
   * Initialization, etc.
   **************************************************/
@@ -286,16 +289,39 @@ int drm_debugfs_remove_files(const struct drm_info_list 
*files, int count,
  }
  EXPORT_SYMBOL(drm_debugfs_remove_files);

+void drm_debugfs_init_root(void)

+{
+       drm_debugfs_root = debugfs_create_dir("dri", NULL);
+}
+
+void drm_debugfs_remove_root(void)
+{
+       debugfs_remove(drm_debugfs_root);
+}
+
+void drm_debugfs_init_accel_root(void)
+{
+       accel_debugfs_root = debugfs_create_dir("accel", NULL);
+}

I think that this can be done in just the drm_debugfs_init_root() function. No 
need to split it into two functions.

Sure, Noted

Regards
Sunil

Apart from that looks good to me as well.

Regards,
Christian.

+
+void drm_debugfs_remove_accel_root(void)
+{
+       debugfs_remove(accel_debugfs_root);
+}
+
+
  /**
   * drm_debugfs_dev_init - create debugfs directory for the device
   * @dev: the device which we want to create the directory for
- * @root: the parent directory depending on the device type
   *
   * Creates the debugfs directory for the device under the given root 
directory.
   */
-void drm_debugfs_dev_init(struct drm_device *dev, struct dentry *root)
+void drm_debugfs_dev_init(struct drm_device *dev)
  {
-       dev->debugfs_root = debugfs_create_dir(dev->unique, root);
+       if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL))
+               dev->debugfs_root = debugfs_create_dir(dev->unique, 
accel_debugfs_root);
+       else
+               dev->debugfs_root = debugfs_create_dir(dev->unique, 
drm_debugfs_root);
  }

/**

@@ -322,14 +348,13 @@ void drm_debugfs_dev_register(struct drm_device *dev)
                drm_atomic_debugfs_init(dev);
  }

-int drm_debugfs_register(struct drm_minor *minor, int minor_id,

-                        struct dentry *root)
+int drm_debugfs_register(struct drm_minor *minor, int minor_id)
  {
        struct drm_device *dev = minor->dev;
        char name[64];

sprintf(name, "%d", minor_id);

-       minor->debugfs_symlink = debugfs_create_symlink(name, root,
+       minor->debugfs_symlink = debugfs_create_symlink(name, drm_debugfs_root,
                                                        dev->unique);

/* TODO: Only for compatibility with drivers */

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 5d57b622f9aa..db19aef9cfd2 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -69,9 +69,6 @@ DEFINE_XARRAY_ALLOC(drm_minors_xa);
   */
  static bool drm_core_init_complete;

-static struct dentry *drm_debugfs_root;

-static struct dentry *accel_debugfs_root;
-
  DEFINE_STATIC_SRCU(drm_unplug_srcu);

/*

@@ -184,8 +181,7 @@ static int drm_minor_register(struct drm_device *dev, enum 
drm_minor_type type)
                return 0;

if (minor->type != DRM_MINOR_ACCEL) {

-               ret = drm_debugfs_register(minor, minor->index,
-                                          drm_debugfs_root);
+               ret = drm_debugfs_register(minor, minor->index);
                if (ret) {
                        DRM_ERROR("DRM: Failed to initialize 
/sys/kernel/debug/dri.\n");
                        goto err_debugfs;
@@ -752,10 +748,7 @@ static int drm_dev_init(struct drm_device *dev,
                goto err;
        }

- if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL))

-               drm_debugfs_dev_init(dev, accel_debugfs_root);
-       else
-               drm_debugfs_dev_init(dev, drm_debugfs_root);
+       drm_debugfs_dev_init(dev);

return 0; @@ -1167,10 +1160,10 @@ static void drm_core_exit(void)

  {
        drm_privacy_screen_lookup_exit();
        drm_panic_exit();
-       debugfs_remove(accel_debugfs_root);
+       drm_debugfs_remove_accel_root();
        accel_core_exit();
        unregister_chrdev(DRM_MAJOR, "drm");
-       debugfs_remove(drm_debugfs_root);
+       drm_debugfs_remove_root();
        drm_sysfs_destroy();
        WARN_ON(!xa_empty(&drm_minors_xa));
        drm_connector_ida_destroy();
@@ -1189,13 +1182,13 @@ static int __init drm_core_init(void)
                goto error;
        }

- drm_debugfs_root = debugfs_create_dir("dri", NULL);

+       drm_debugfs_init_root();

ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);

        if (ret < 0)
                goto error;

- accel_debugfs_root = debugfs_create_dir("accel", NULL);

+       drm_debugfs_init_accel_root();

ret = accel_core_init();

        if (ret < 0)
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index b2b6a8e49dda..d2d8e72f32d9 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -186,8 +186,7 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct 
iosys_map *map);
  #if defined(CONFIG_DEBUG_FS)
  void drm_debugfs_dev_fini(struct drm_device *dev);
  void drm_debugfs_dev_register(struct drm_device *dev);
-int drm_debugfs_register(struct drm_minor *minor, int minor_id,
-                        struct dentry *root);
+int drm_debugfs_register(struct drm_minor *minor, int minor_id);
  void drm_debugfs_unregister(struct drm_minor *minor);
  void drm_debugfs_connector_add(struct drm_connector *connector);
  void drm_debugfs_connector_remove(struct drm_connector *connector);
@@ -205,8 +204,7 @@ static inline void drm_debugfs_dev_register(struct 
drm_device *dev)
  {
  }

-static inline int drm_debugfs_register(struct drm_minor *minor, int minor_id,

-                                      struct dentry *root)
+static inline int drm_debugfs_register(struct drm_minor *minor, int minor_id)
  {
        return 0;
  }
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index a43d707b5f36..7964dd878144 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -566,9 +566,29 @@ static inline bool drm_firmware_drivers_only(void)
  }

#if defined(CONFIG_DEBUG_FS)

-void drm_debugfs_dev_init(struct drm_device *dev, struct dentry *root);
+void drm_debugfs_dev_init(struct drm_device *dev);
+void drm_debugfs_init_root(void);
+void drm_debugfs_remove_root(void);
+void drm_debugfs_init_accel_root(void);
+void drm_debugfs_remove_accel_root(void);
  #else
-static inline void drm_debugfs_dev_init(struct drm_device *dev, struct dentry 
*root)
+static inline void drm_debugfs_dev_init(struct drm_device *dev)
+{
+}
+
+static inline void drm_debugfs_init_root(void)
+{
+}
+
+static inline void drm_debugfs_remove_root(void)
+{
+}
+
+static inline void drm_debugfs_init_accel_root(void)
+{
+}
+
+static inline void drm_debugfs_remove_accel_root(void)
  {
  }
  #endif