On 6/16/2025 6:26 PM, Christian König wrote:
On 6/16/25 14:25, Khatri, Sunil wrote:
On 6/16/2025 5:41 PM, Christian König wrote:
On 6/16/25 12:05, 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 | 1 +
drivers/gpu/drm/drm_file.c | 26 ++++++++++++++++++++++++++
include/drm/drm_device.h | 4 ++++
include/drm/drm_file.h | 7 +++++++
4 files changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 2d43bda82887..b4956960ae76 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -296,6 +296,7 @@ EXPORT_SYMBOL(drm_debugfs_remove_files);
void drm_debugfs_dev_init(struct drm_device *dev, struct dentry *root)
{
dev->debugfs_root = debugfs_create_dir(dev->unique, root);
+ dev->drm_debugfs_root = root;
We should probably just move drm_debugfs_root into drm_debugfs.c instead of
keeping that around per device.
root node above is needed in the drm_file.c function drm_alloc and there is
nothing above drm_device where i can get the root information. that is the
reason i mentioned it as drm_debugfs_root as root node of the drm subsystem
itself.
drm_debugfs_root is currently a global variable in drm_drv.c, but if we move it
to drm_debugfs.c all functions in that file could use it.
The global variable drm_debugfs_root is global variable in drm_drv.c and
a lot of function there are dependent on it. So no matter where we move
the variable we need to have a reference of dentry in drm_drv.c too and
drm_device is the only thing that is being used in drm_drv.c.
eg:
drm_core_init is using it and there is no structure there to use to have
a reference to this node.
drm_minor_register also uses the same root to create the device nodes
and we cant move all the code from drm_drv.c so we are stuck again how
to get the reference in drm_debugfs.c
I am trying to explore if its possible but if there is anything you
could suggest appreciate that. What is in the current patch is we have a
reference of root in drm_device itself like drm core is parent to every
drm device, could use a different name like parent_debugfs or something
like that.
Regards
Sunil Khatri
Including the new functions for creating the per-client debugfs directory.
Regards,
Christian.
~Sunil
}
/**
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 06ba6dcbf5ae..32012e39dcb4 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -39,6 +39,7 @@
#include <linux/poll.h>
#include <linux/slab.h>
#include <linux/vga_switcheroo.h>
+#include <linux/debugfs.h>
#include <drm/drm_client_event.h>
#include <drm/drm_drv.h>
@@ -133,6 +134,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
struct drm_device *dev = minor->dev;
struct drm_file *file;
int ret;
+ char *dir_name, *drm_name, *symlink;
file = kzalloc(sizeof(*file), GFP_KERNEL);
if (!file)
@@ -143,6 +145,27 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor)
rcu_assign_pointer(file->pid, get_pid(task_tgid(current)));
file->minor = minor;
+ dir_name = kasprintf(GFP_KERNEL, "client-%llu", file->client_id);
+ if (!dir_name)
+ return ERR_PTR(-ENOMEM);
+
+ /* Create a debugfs directory for the client in root on drm debugfs */
+ file->debugfs_client = debugfs_create_dir(dir_name, dev->drm_debugfs_root);
+ kfree(dir_name);
+
+ drm_name = kasprintf(GFP_KERNEL, "%d", minor->index);
+ if (!drm_name)
+ return ERR_PTR(-ENOMEM);
+
+ symlink = kasprintf(GFP_KERNEL, "../%d", minor->index);
Better use dev->unique here, minor->index is also only a symlink.
Thats a good suggestion and doable. Will update in next version
~Sunil
+ if (!symlink)
+ return ERR_PTR(-ENOMEM);
+
+ /* Create a link from client_id to the drm device this client id belongs
to */
+ debugfs_create_symlink(drm_name, file->debugfs_client, symlink);
+ kfree(drm_name);
+ kfree(symlink);
+
Move all that debugfs handling into a function in drm_debugfs.c
Sure, let me try that and push another patch.
/* for compatibility root is always authenticated */
file->authenticated = capable(CAP_SYS_ADMIN);
@@ -237,6 +260,9 @@ void drm_file_free(struct drm_file *file)
drm_events_release(file);
+ debugfs_remove_recursive(file->debugfs_client);
+ file->debugfs_client = NULL;
+
Same here, move to drm_debugfs.c
Sure, let me try that if there are not challenges.
~sunil
Apart from that looks solid to me.
Regards,
Christian.
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
drm_fb_release(file);
drm_property_destroy_user_blobs(dev, file);
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 6ea54a578cda..ec20b777b3cc 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -325,6 +325,10 @@ struct drm_device {
* Root directory for debugfs files.
*/
struct dentry *debugfs_root;
+ /**
+ * @drm_debugfs_root;
+ */
+ struct dentry *drm_debugfs_root;
};
#endif
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;
};
/**