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.kha...@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;
>>> };
>>>
>>> /**
