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

[Jeff Hugo]

On 6/30/2025 8:36 AM, Sunil Khatri wrote:

I don't see a cover letter on list. Surely there should be one?

Looks like you didn't send this to the Accel maintainer. Did you forget 
to run the get_maintainers/pl script?

> 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?

> 
> Signed-off-by: Sunil Khatri <sunil.khatri at amd.com>
> Reviewed-by: Christian König <christian.koenig at 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.