Try to address the drm_debugfs issues

Christian König ckoenig.leichtzumerken at gmail.com
Thu Feb 9 08:18:35 UTC 2023


Hello everyone,

the drm_debugfs has a couple of well known design problems.

Especially it wasn't possible to add files between initializing and registering
of DRM devices since the underlying debugfs directory wasn't created yet.

The resulting necessity of the driver->debugfs_init() callback function is a
mid-layering which is really frowned on since it creates a horrible
driver->DRM->driver design layering.

The recent patch "drm/debugfs: create device-centered debugfs functions" tried
to address those problem, but doesn't seem to work correctly. This looks like
a misunderstanding of the call flow around drm_debugfs_init(), which is called
multiple times, once for the primary and once for the render node.

So what happens now is the following:

1. drm_dev_init() initially allocates the drm_minor objects.
2. ... back to the driver ...
3. drm_dev_register() is called.

4. drm_debugfs_init() is called for the primary node.
5. drm_framebuffer_debugfs_init(), drm_client_debugfs_init() and
   drm_atomic_debugfs_init() call drm_debugfs_add_file(s)() to add the files
   for the primary node.
6. The driver->debugfs_init() callback is called to add debugfs files for the
   primary node.
7. The added files are consumed and added to the primary node debugfs directory.

8. drm_debugfs_init() is called for the render node.
9. drm_framebuffer_debugfs_init(), drm_client_debugfs_init() and
   drm_atomic_debugfs_init() call drm_debugfs_add_file(s)() to add the files
   again for the render node.
10. The driver->debugfs_init() callback is called to add debugfs files for the
    render node.
11. The added files are consumed and added to the render node debugfs directory.

12. Some more files are added through drm_debugfs_add_file().
13. drm_debugfs_late_register() add the files once more to the primary node
    debugfs directory.
14. From this point on files added through drm_debugfs_add_file() are simply ignored.
15. ... back to the driver ...

Because of this the dev->debugfs_mutex lock is also completely pointless since
any concurrent use of the interface would just randomly either add the files to
the primary or render node or just not at all.

Even worse is that this implementation nails the coffin for removing the
driver->debugfs_init() mid-layering because otherwise drivers can't control
where their debugfs (primary/render node) are actually added.

This patch set here now tries to clean this up a bit, but most likely isn't
fully complete either since I didn't audit every driver/call path.

Please comment/discuss.

Cheers,
Christian.




More information about the dri-devel mailing list