Re: [PATCH v7 1/5] drm: move the debugfs accel driver code to drm layer

[Khatri, Sunil]

On 6/30/2025 8:49 PM, Jeff Hugo wrote:
On 6/30/2025 8:36 AM, Sunil Khatri wrote:

I don't see a cover letter on list. Surely there should be one?
Yes there is one with the first version of patches.

Looks like you didn't send this to the Accel maintainer. Did you 
forget to run the get_maintainers/pl script?
Let me check and add more. Christian is already there who did make 
changes to code around that area where i made but surely let me add more.
Move the debugfs accel driver code to the drm layer
and it is an intermediate step to move all debugfs
related handling into drm_debugfs.c

This is missing the answer to the most important question - why?
Updated in V8.


Signed-off-by: Sunil Khatri <sunil.kha...@amd.com>
Reviewed-by: Christian König <christian.koe...@amd.com>
---
  drivers/accel/drm_accel.c | 16 ----------------
  drivers/gpu/drm/drm_drv.c |  6 +++++-
  include/drm/drm_accel.h   |  5 -----
  3 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/accel/drm_accel.c b/drivers/accel/drm_accel.c
index aa826033b0ce..ca3357acd127 100644
--- a/drivers/accel/drm_accel.c
+++ b/drivers/accel/drm_accel.c
@@ -20,8 +20,6 @@
    DEFINE_XARRAY_ALLOC(accel_minors_xa);
  -static struct dentry *accel_debugfs_root;
-
  static const struct device_type accel_sysfs_device_minor = {
      .name = "accel_minor"
  };
@@ -73,17 +71,6 @@ static const struct drm_info_list 
accel_debugfs_list[] = {
  };
  #define ACCEL_DEBUGFS_ENTRIES ARRAY_SIZE(accel_debugfs_list)
  -/**
- * accel_debugfs_init() - Initialize debugfs for device
- * @dev: Pointer to the device instance.
- *
- * This function creates a root directory for the device in debugfs.
- */
-void accel_debugfs_init(struct drm_device *dev)
-{
-    drm_debugfs_dev_init(dev, accel_debugfs_root);
-}
-
  /**
   * accel_debugfs_register() - Register debugfs for device
   * @dev: Pointer to the device instance.
@@ -194,7 +181,6 @@ static const struct file_operations 
accel_stub_fops = {
  void accel_core_exit(void)
  {
      unregister_chrdev(ACCEL_MAJOR, "accel");
-    debugfs_remove(accel_debugfs_root);
      accel_sysfs_destroy();
      WARN_ON(!xa_empty(&accel_minors_xa));
  }
@@ -209,8 +195,6 @@ int __init accel_core_init(void)
          goto error;
      }
  -    accel_debugfs_root = debugfs_create_dir("accel", NULL);
-
      ret = register_chrdev(ACCEL_MAJOR, "accel", &accel_stub_fops);
      if (ret < 0)
          DRM_ERROR("Cannot register ACCEL major: %d\n", ret);
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 17fc5dc708f4..5d57b622f9aa 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -70,6 +70,7 @@ 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);
  @@ -752,7 +753,7 @@ static int drm_dev_init(struct drm_device *dev,
      }
        if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL))
-        accel_debugfs_init(dev);
+        drm_debugfs_dev_init(dev, accel_debugfs_root);
      else
          drm_debugfs_dev_init(dev, drm_debugfs_root);
  @@ -1166,6 +1167,7 @@ static void drm_core_exit(void)
  {
      drm_privacy_screen_lookup_exit();
      drm_panic_exit();
+    debugfs_remove(accel_debugfs_root);
      accel_core_exit();
      unregister_chrdev(DRM_MAJOR, "drm");
      debugfs_remove(drm_debugfs_root);
@@ -1193,6 +1195,8 @@ static int __init drm_core_init(void)
      if (ret < 0)
          goto error;
  +    accel_debugfs_root = debugfs_create_dir("accel", NULL);

Did you test this with CONFIG_DRM_ACCEL=n?
It looks like you change the behavior with this change in that we'll 
have an accel debugfs directory created, even when ACCEL is not built 
into DRM.

Thanks for catching that out. Added support based on the configuration now.

Regards
Sunil Khatri