[PATCH v5 3/5] drm: add debugfs support on per client-id basis
Christian König
christian.koenig at amd.com
Thu Jun 26 14:04:57 UTC 2025
On 26.06.25 15:48, Khatri, Sunil wrote:
>
> On 6/26/2025 5:34 PM, Christian König wrote:
>> On 24.06.25 13:34, Sunil Khatri wrote:
>>> add support to add a directory for each client-id
>>> with root at the dri level. Since the clients are
>>> unique and not just related to one single drm device,
>>> so it makes more sense to add all the client based
>>> nodes with root as dri.
>>>
>>> Also create a symlink back to the parent drm device
>>> from each client.
>>>
>>> Signed-off-by: Sunil Khatri <sunil.khatri at amd.com>
>>> ---
>>> drivers/gpu/drm/drm_debugfs.c | 26 ++++++++++++++++++++++++++
>>> drivers/gpu/drm/drm_file.c | 6 ++++++
>>> include/drm/drm_debugfs.h | 11 +++++++++++
>>> include/drm/drm_file.h | 7 +++++++
>>> 4 files changed, 50 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
>>> index a227903c29c4..ebdf60665b86 100644
>>> --- a/drivers/gpu/drm/drm_debugfs.c
>>> +++ b/drivers/gpu/drm/drm_debugfs.c
>>> @@ -309,6 +309,32 @@ void drm_debugfs_remove_accel_root(void)
>>> debugfs_remove(accel_debugfs_root);
>>> }
>>>
>>> +void drm_debugfs_clients_add(struct drm_file *file)
>>> +{
>>> + char *client;
>>> +
>>> + client = kasprintf(GFP_KERNEL, "client-%llu", file->client_id);
>>> + if (!client)
>>> + return;
>>> +
>>> + /* Create a debugfs directory for the client in root on drm debugfs */
>>> + file->debugfs_client = debugfs_create_dir(client, drm_debugfs_root);
>>> + kfree(client);
>>> +
>>> + client = kasprintf(GFP_KERNEL, "../%s", file->minor->dev->unique);
>>> + if (!client)
>>> + return;
>>> +
>>> + /* Create a link from client_id to the drm device this client id belongs to */
>>> + debugfs_create_symlink("device", file->debugfs_client, client);
>> Mhm, that won't work for accel devices. How should we fix that?
>
> All the clients we are creating is doing for the dri clients only and not for accel driver, Since the root itself is different for accel it cant be in dri and we could leave that for accel driver to take care if they need client
> directory for something or this we could pick later if there is a need.
>
> In the mean while what we can do in the drm_alloc/drm_free is to add an extra condition to check if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL)) or not. and create and remove client onyl for !accel
> if(!drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL))
> drm_debugfs_clients_add(file); and
> if(!drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL))
> drm_debugfs_clients_remove(file);
Works for me.
Regards,
Christian.
>
>
>>> + kfree(client);
>>> +}
>>> +
>>> +void drm_debugfs_clients_remove(struct drm_file *file)
>>> +{
>>> + debugfs_remove_recursive(file->debugfs_client);
>>> + file->debugfs_client = NULL;
>>> +}
>>>
>>> /**
>>> * drm_debugfs_dev_init - create debugfs directory for the device
>>> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
>>> index 06ba6dcbf5ae..17b803ab119e 100644
>>> --- a/drivers/gpu/drm/drm_file.c
>>> +++ b/drivers/gpu/drm/drm_file.c
>>> @@ -45,6 +45,7 @@
>>> #include <drm/drm_file.h>
>>> #include <drm/drm_gem.h>
>>> #include <drm/drm_print.h>
>>> +#include <drm/drm_debugfs.h>
>>>
>>> #include "drm_crtc_internal.h"
>>> #include "drm_internal.h"
>>> @@ -167,6 +168,8 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
>>>
>>> drm_prime_init_file_private(&file->prime);
>>>
>>> + drm_debugfs_clients_add(file);
>>> +
>>> if (dev->driver->open) {
>>> ret = dev->driver->open(dev, file);
>>> if (ret < 0)
>>> @@ -181,6 +184,8 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
>>> drm_syncobj_release(file);
>>> if (drm_core_check_feature(dev, DRIVER_GEM))
>>> drm_gem_release(dev, file);
>>> +
>>> + drm_debugfs_clients_remove(file);
>>> put_pid(rcu_access_pointer(file->pid));
>>> kfree(file);
>>>
>>> @@ -236,6 +241,7 @@ void drm_file_free(struct drm_file *file)
>>> atomic_read(&dev->open_count));
>>>
>>> drm_events_release(file);
>>> + drm_debugfs_clients_remove(file);
>> That most likely needs to come even before releasing the events.
>
> Sure will move it before events_release.
>
> Regards
> Sunil Khatri
>
>> Regards,
>> Christian.
>>
>>>
>>> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>>> drm_fb_release(file);
>>> diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
>>> index cf06cee4343f..2b3767ad8f5d 100644
>>> --- a/include/drm/drm_debugfs.h
>>> +++ b/include/drm/drm_debugfs.h
>>> @@ -153,6 +153,9 @@ void drm_debugfs_add_files(struct drm_device *dev,
>>>
>>> int drm_debugfs_gpuva_info(struct seq_file *m,
>>> struct drm_gpuvm *gpuvm);
>>> +
>>> +void drm_debugfs_clients_add(struct drm_file *file);
>>> +void drm_debugfs_clients_remove(struct drm_file *file);
>>> #else
>>> static inline void drm_debugfs_create_files(const struct drm_info_list *files,
>>> int count, struct dentry *root,
>>> @@ -181,6 +184,14 @@ static inline int drm_debugfs_gpuva_info(struct seq_file *m,
>>> {
>>> return 0;
>>> }
>>> +
>>> +static void drm_debugfs_clients_add(struct drm_file *file)
>>> +{
>>> +}
>>> +
>>> +static void drm_debugfs_clients_remove(struct drm_file *file)
>>> +{
>>> +}
>>> #endif
>>>
>>> #endif /* _DRM_DEBUGFS_H_ */
>>> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
>>> index 5c3b2aa3e69d..eab7546aad79 100644
>>> --- a/include/drm/drm_file.h
>>> +++ b/include/drm/drm_file.h
>>> @@ -400,6 +400,13 @@ struct drm_file {
>>> * @client_name_lock: Protects @client_name.
>>> */
>>> struct mutex client_name_lock;
>>> +
>>> + /**
>>> + * @debugfs_client:
>>> + *
>>> + * debugfs directory for each client under a drm node.
>>> + */
>>> + struct dentry *debugfs_client;
>>> };
>>>
>>> /**
More information about the dri-devel
mailing list