This series introduces the initial structure to make DRM debugfs more
device-centered and it is the first step to drop the
drm_driver->debugfs_init hooks in the future [1].
Currently, DRM debugfs files are created using drm_debugfs_create_files()
on request. The first patch of this series makes it possible for DRM devices
for creating debugfs files during drm_dev_register(). For it, it introduces
two new functions that can be used by the drivers: drm_debugfs_add_files()
and drm_debugfs_add_file(). The requests are added to a list and are created
all at once during drm_dev_register(). Moreover, the first patch was based on
this RFC series [2].
The main difference between the RFC series and the current series is the
creation of a new fops structure to accommodate the new structs and, also,
the creation of a new drm_debugfs_open. Moreover, the new series uses
device-managed allocation, returns memory allocation errors, and converts
more drivers to the new structure.
Moreover, since v3, the ability to create debugfs files at late_register hooks was
added. In previous versions, modeset components weren't able to create debugfs
files at late_register hooks as the registration of drm_minor happens before the
registration of the modeset abstractions. So, the third patch fixes this problem
by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
catching this problem!
Apart from the third patch, the series looks similiar from its last version.
[1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
[2] https://lore.kernel.org/dri-devel/[email protected]/
Best Regards,
- Maíra Canal
---
v1 -> v2: https://lore.kernel.org/dri-devel/[email protected]/T/#t
- Fix compilation errors in the second patch (kernel test robot).
- Drop debugfs_init hook from vkms (Maíra Canal).
- Remove return values and error handling to debugfs related
functions (Jani Nikula).
- Remove entry from list after the file is created, so that drm_debugfs_init
can be called more than once (Maíra Canal).
v2 -> v3: https://lore.kernel.org/dri-devel/[email protected]/
- Rebase on top of drm-misc-next
v3 -> v4: https://lore.kernel.org/dri-devel/[email protected]/
- Add Maxime's Reviewed-by tags
- Add the ability to create debugfs files at late_register hooks (Melissa Wen).
---
Maíra Canal (7):
drm/debugfs: create device-centered debugfs functions
drm: use new debugfs device-centered functions on DRM core files
drm/debugfs: create debugfs late register functions
drm/vc4: use new debugfs device-centered functions
drm/v3d: use new debugfs device-centered functions
drm/vkms: use new debugfs device-centered functions
drm/todo: update the debugfs clean up task
Documentation/gpu/todo.rst | 9 +--
drivers/gpu/drm/drm_atomic.c | 11 ++-
drivers/gpu/drm/drm_client.c | 11 ++-
drivers/gpu/drm/drm_debugfs.c | 102 +++++++++++++++++++++++---
drivers/gpu/drm/drm_drv.c | 3 +
drivers/gpu/drm/drm_framebuffer.c | 11 ++-
drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++-
drivers/gpu/drm/drm_internal.h | 5 ++
drivers/gpu/drm/drm_mode_config.c | 2 +
drivers/gpu/drm/v3d/v3d_debugfs.c | 22 +++---
drivers/gpu/drm/vc4/vc4_bo.c | 10 +--
drivers/gpu/drm/vc4/vc4_crtc.c | 7 +-
drivers/gpu/drm/vc4/vc4_debugfs.c | 36 ++-------
drivers/gpu/drm/vc4/vc4_dpi.c | 5 +-
drivers/gpu/drm/vc4/vc4_drv.c | 1 -
drivers/gpu/drm/vc4/vc4_drv.h | 32 ++------
drivers/gpu/drm/vc4/vc4_dsi.c | 6 +-
drivers/gpu/drm/vc4/vc4_hdmi.c | 12 +--
drivers/gpu/drm/vc4/vc4_hvs.c | 24 ++----
drivers/gpu/drm/vc4/vc4_v3d.c | 14 +---
drivers/gpu/drm/vc4/vc4_vec.c | 6 +-
drivers/gpu/drm/vkms/vkms_drv.c | 17 ++---
include/drm/drm_debugfs.h | 41 +++++++++++
include/drm/drm_device.h | 15 ++++
24 files changed, 233 insertions(+), 180 deletions(-)
--
2.38.1
Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function in all DRM core files, centering the
debugfs files management on the drm_device instead of drm_minor.
Reviewed-by: Maxime Ripard <[email protected]>
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/drm_atomic.c | 11 +++++------
drivers/gpu/drm/drm_client.c | 11 +++++------
drivers/gpu/drm/drm_debugfs.c | 18 ++++++++----------
drivers/gpu/drm/drm_framebuffer.c | 11 +++++------
drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++------
5 files changed, 28 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index e666799a46d5..5457c02ca1ab 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1756,8 +1756,8 @@ EXPORT_SYMBOL(drm_state_dump);
#ifdef CONFIG_DEBUG_FS
static int drm_state_info(struct seq_file *m, void *data)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct drm_printer p = drm_seq_file_printer(m);
__drm_state_dump(dev, &p, true);
@@ -1766,14 +1766,13 @@ static int drm_state_info(struct seq_file *m, void *data)
}
/* any use in debugfs files to dump individual planes/crtc/etc? */
-static const struct drm_info_list drm_atomic_debugfs_list[] = {
+static const struct drm_debugfs_info drm_atomic_debugfs_list[] = {
{"state", drm_state_info, 0},
};
void drm_atomic_debugfs_init(struct drm_minor *minor)
{
- drm_debugfs_create_files(drm_atomic_debugfs_list,
- ARRAY_SIZE(drm_atomic_debugfs_list),
- minor->debugfs_root, minor);
+ drm_debugfs_add_files(minor->dev, drm_atomic_debugfs_list,
+ ARRAY_SIZE(drm_atomic_debugfs_list));
}
#endif
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index fd67efe37c63..262ec64d4397 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -480,8 +480,8 @@ EXPORT_SYMBOL(drm_client_framebuffer_flush);
#ifdef CONFIG_DEBUG_FS
static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
{
- struct drm_info_node *node = m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct drm_printer p = drm_seq_file_printer(m);
struct drm_client_dev *client;
@@ -493,14 +493,13 @@ static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
return 0;
}
-static const struct drm_info_list drm_client_debugfs_list[] = {
+static const struct drm_debugfs_info drm_client_debugfs_list[] = {
{ "internal_clients", drm_client_debugfs_internal_clients, 0 },
};
void drm_client_debugfs_init(struct drm_minor *minor)
{
- drm_debugfs_create_files(drm_client_debugfs_list,
- ARRAY_SIZE(drm_client_debugfs_list),
- minor->debugfs_root, minor);
+ drm_debugfs_add_files(minor->dev, drm_client_debugfs_list,
+ ARRAY_SIZE(drm_client_debugfs_list));
}
#endif
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 988fc07b94b4..d9d3ed7acc80 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -51,9 +51,8 @@
static int drm_name_info(struct seq_file *m, void *data)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_minor *minor = node->minor;
- struct drm_device *dev = minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct drm_master *master;
mutex_lock(&dev->master_mutex);
@@ -73,8 +72,8 @@ static int drm_name_info(struct seq_file *m, void *data)
static int drm_clients_info(struct seq_file *m, void *data)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct drm_file *priv;
kuid_t uid;
@@ -125,8 +124,8 @@ static int drm_gem_one_name_info(int id, void *ptr, void *data)
static int drm_gem_name_info(struct seq_file *m, void *data)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
seq_printf(m, " name size handles refcount\n");
@@ -137,7 +136,7 @@ static int drm_gem_name_info(struct seq_file *m, void *data)
return 0;
}
-static const struct drm_info_list drm_debugfs_list[] = {
+static const struct drm_debugfs_info drm_debugfs_list[] = {
{"name", drm_name_info, 0},
{"clients", drm_clients_info, 0},
{"gem_names", drm_gem_name_info, DRIVER_GEM},
@@ -231,8 +230,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
sprintf(name, "%d", minor_id);
minor->debugfs_root = debugfs_create_dir(name, root);
- drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
- minor->debugfs_root, minor);
+ drm_debugfs_add_files(minor->dev, drm_debugfs_list, DRM_DEBUGFS_ENTRIES);
if (drm_drv_uses_atomic_modeset(dev)) {
drm_atomic_debugfs_init(minor);
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 2dd97473ca10..aff3746dedfb 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -1203,8 +1203,8 @@ void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
#ifdef CONFIG_DEBUG_FS
static int drm_framebuffer_info(struct seq_file *m, void *data)
{
- struct drm_info_node *node = m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct drm_printer p = drm_seq_file_printer(m);
struct drm_framebuffer *fb;
@@ -1218,14 +1218,13 @@ static int drm_framebuffer_info(struct seq_file *m, void *data)
return 0;
}
-static const struct drm_info_list drm_framebuffer_debugfs_list[] = {
+static const struct drm_debugfs_info drm_framebuffer_debugfs_list[] = {
{ "framebuffer", drm_framebuffer_info, 0 },
};
void drm_framebuffer_debugfs_init(struct drm_minor *minor)
{
- drm_debugfs_create_files(drm_framebuffer_debugfs_list,
- ARRAY_SIZE(drm_framebuffer_debugfs_list),
- minor->debugfs_root, minor);
+ drm_debugfs_add_files(minor->dev, drm_framebuffer_debugfs_list,
+ ARRAY_SIZE(drm_framebuffer_debugfs_list));
}
#endif
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index f59adffd938a..d40b3edb52d0 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -957,8 +957,8 @@ static struct ttm_device_funcs bo_driver = {
static int drm_vram_mm_debugfs(struct seq_file *m, void *data)
{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_vram_mm *vmm = node->minor->dev->vram_mm;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_vram_mm *vmm = entry->dev->vram_mm;
struct ttm_resource_manager *man = ttm_manager_type(&vmm->bdev, TTM_PL_VRAM);
struct drm_printer p = drm_seq_file_printer(m);
@@ -966,7 +966,7 @@ static int drm_vram_mm_debugfs(struct seq_file *m, void *data)
return 0;
}
-static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
+static const struct drm_debugfs_info drm_vram_mm_debugfs_list[] = {
{ "vram-mm", drm_vram_mm_debugfs, 0, NULL },
};
@@ -978,9 +978,8 @@ static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
*/
void drm_vram_mm_debugfs_init(struct drm_minor *minor)
{
- drm_debugfs_create_files(drm_vram_mm_debugfs_list,
- ARRAY_SIZE(drm_vram_mm_debugfs_list),
- minor->debugfs_root, minor);
+ drm_debugfs_add_files(minor->dev, drm_vram_mm_debugfs_list,
+ ARRAY_SIZE(drm_vram_mm_debugfs_list));
}
EXPORT_SYMBOL(drm_vram_mm_debugfs_init);
--
2.38.1
Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function, which centers the debugfs files
management on the drm_device instead of drm_minor.
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/v3d/v3d_debugfs.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
index efbde124c296..330669f51fa7 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -79,8 +79,8 @@ static const struct v3d_reg_def v3d_csd_reg_defs[] = {
static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
{
- struct drm_info_node *node = (struct drm_info_node *)m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct v3d_dev *v3d = to_v3d_dev(dev);
int i, core;
@@ -126,8 +126,8 @@ static int v3d_v3d_debugfs_regs(struct seq_file *m, void *unused)
static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
{
- struct drm_info_node *node = (struct drm_info_node *)m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct v3d_dev *v3d = to_v3d_dev(dev);
u32 ident0, ident1, ident2, ident3, cores;
int core;
@@ -188,8 +188,8 @@ static int v3d_v3d_debugfs_ident(struct seq_file *m, void *unused)
static int v3d_debugfs_bo_stats(struct seq_file *m, void *unused)
{
- struct drm_info_node *node = (struct drm_info_node *)m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct v3d_dev *v3d = to_v3d_dev(dev);
mutex_lock(&v3d->bo_lock);
@@ -204,8 +204,8 @@ static int v3d_debugfs_bo_stats(struct seq_file *m, void *unused)
static int v3d_measure_clock(struct seq_file *m, void *unused)
{
- struct drm_info_node *node = (struct drm_info_node *)m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct v3d_dev *v3d = to_v3d_dev(dev);
uint32_t cycles;
int core = 0;
@@ -236,7 +236,7 @@ static int v3d_measure_clock(struct seq_file *m, void *unused)
return 0;
}
-static const struct drm_info_list v3d_debugfs_list[] = {
+static const struct drm_debugfs_info v3d_debugfs_list[] = {
{"v3d_ident", v3d_v3d_debugfs_ident, 0},
{"v3d_regs", v3d_v3d_debugfs_regs, 0},
{"measure_clock", v3d_measure_clock, 0},
@@ -246,7 +246,5 @@ static const struct drm_info_list v3d_debugfs_list[] = {
void
v3d_debugfs_init(struct drm_minor *minor)
{
- drm_debugfs_create_files(v3d_debugfs_list,
- ARRAY_SIZE(v3d_debugfs_list),
- minor->debugfs_root, minor);
+ drm_debugfs_add_files(minor->dev, v3d_debugfs_list, ARRAY_SIZE(v3d_debugfs_list));
}
--
2.38.1
The structs drm_debugfs_info and drm_debugfs_entry introduced a new
debugfs structure to DRM, centered on drm_device instead of drm_minor.
Therefore, remove the tasks related to create a new device-centered
debugfs structure and add a new task to replace the use of
drm_debugfs_create_files() for the use of drm_debugfs_add_file() and
drm_debugfs_add_files().
Signed-off-by: Maíra Canal <[email protected]>
---
Documentation/gpu/todo.rst | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index b2c6aaf1edf2..f64abf69f341 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -508,17 +508,14 @@ Clean up the debugfs support
There's a bunch of issues with it:
-- The drm_info_list ->show() function doesn't even bother to cast to the drm
- structure for you. This is lazy.
+- Convert drivers to support the drm_debugfs_add_files() function instead of
+ the drm_debugfs_create_files() function.
- We probably want to have some support for debugfs files on crtc/connectors and
maybe other kms objects directly in core. There's even drm_print support in
the funcs for these objects to dump kms state, so it's all there. And then the
->show() functions should obviously give you a pointer to the right object.
-- The drm_info_list stuff is centered on drm_minor instead of drm_device. For
- anything we want to print drm_device (or maybe drm_file) is the right thing.
-
- The drm_driver->debugfs_init hooks we have is just an artifact of the old
midlayered load sequence. DRM debugfs should work more like sysfs, where you
can create properties/files for an object anytime you want, and the core
@@ -527,8 +524,6 @@ There's a bunch of issues with it:
this (together with the drm_minor->drm_device move) would allow us to remove
debugfs_init.
-Previous RFC that hasn't landed yet: https://lore.kernel.org/dri-devel/[email protected]/
-
Contact: Daniel Vetter
Level: Intermediate
--
2.38.1
Replace the use of drm_debugfs_create_files() with the new
drm_debugfs_add_files() function, which centers the debugfs files
management on the drm_device instead of drm_minor. Moreover, remove the
debugfs_init hook and add the debugfs files directly on vkms_create(),
before drm_dev_register().
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/vkms/vkms_drv.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 69346906ec81..6d3a2d57d992 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -92,8 +92,8 @@ static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
static int vkms_config_show(struct seq_file *m, void *data)
{
- struct drm_info_node *node = (struct drm_info_node *)m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_debugfs_entry *entry = m->private;
+ struct drm_device *dev = entry->dev;
struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
@@ -103,24 +103,16 @@ static int vkms_config_show(struct seq_file *m, void *data)
return 0;
}
-static const struct drm_info_list vkms_config_debugfs_list[] = {
+static const struct drm_debugfs_info vkms_config_debugfs_list[] = {
{ "vkms_config", vkms_config_show, 0 },
};
-static void vkms_config_debugfs_init(struct drm_minor *minor)
-{
- drm_debugfs_create_files(vkms_config_debugfs_list, ARRAY_SIZE(vkms_config_debugfs_list),
- minor->debugfs_root, minor);
-}
-
static const struct drm_driver vkms_driver = {
.driver_features = DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
.release = vkms_release,
.fops = &vkms_driver_fops,
DRM_GEM_SHMEM_DRIVER_OPS,
- .debugfs_init = vkms_config_debugfs_init,
-
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
.date = DRIVER_DATE,
@@ -202,6 +194,9 @@ static int vkms_create(struct vkms_config *config)
if (ret)
goto out_devres;
+ drm_debugfs_add_files(&vkms_device->drm, vkms_config_debugfs_list,
+ ARRAY_SIZE(vkms_config_debugfs_list));
+
ret = drm_dev_register(&vkms_device->drm, 0);
if (ret)
goto out_devres;
--
2.38.1
On 12/19, Ma?ra Canal wrote:
> This series introduces the initial structure to make DRM debugfs more
> device-centered and it is the first step to drop the
> drm_driver->debugfs_init hooks in the future [1].
>
> Currently, DRM debugfs files are created using drm_debugfs_create_files()
> on request. The first patch of this series makes it possible for DRM devices
> for creating debugfs files during drm_dev_register(). For it, it introduces
> two new functions that can be used by the drivers: drm_debugfs_add_files()
> and drm_debugfs_add_file(). The requests are added to a list and are created
> all at once during drm_dev_register(). Moreover, the first patch was based on
> this RFC series [2].
>
> The main difference between the RFC series and the current series is the
> creation of a new fops structure to accommodate the new structs and, also,
> the creation of a new drm_debugfs_open. Moreover, the new series uses
> device-managed allocation, returns memory allocation errors, and converts
> more drivers to the new structure.
>
> Moreover, since v3, the ability to create debugfs files at late_register hooks was
> added. In previous versions, modeset components weren't able to create debugfs
> files at late_register hooks as the registration of drm_minor happens before the
> registration of the modeset abstractions. So, the third patch fixes this problem
> by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
> catching this problem!
>
> Apart from the third patch, the series looks similiar from its last version.
>
> [1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
> [2] https://lore.kernel.org/dri-devel/[email protected]/
>
> Best Regards,
> - Ma?ra Canal
>
> ---
>
> v1 -> v2: https://lore.kernel.org/dri-devel/[email protected]/T/#t
>
> - Fix compilation errors in the second patch (kernel test robot).
> - Drop debugfs_init hook from vkms (Ma?ra Canal).
> - Remove return values and error handling to debugfs related
> functions (Jani Nikula).
> - Remove entry from list after the file is created, so that drm_debugfs_init
> can be called more than once (Ma?ra Canal).
>
> v2 -> v3: https://lore.kernel.org/dri-devel/[email protected]/
>
> - Rebase on top of drm-misc-next
>
> v3 -> v4: https://lore.kernel.org/dri-devel/[email protected]/
>
> - Add Maxime's Reviewed-by tags
> - Add the ability to create debugfs files at late_register hooks (Melissa Wen).
Hi Ma?ra,
Thanks for addressing all comments.
Maybe Danvet has some inputs for the late_register approach.
Anyway, LGTM and the entire series is:
Reviewed-by: Melissa Wen <[email protected]>
>
> ---
>
> Ma?ra Canal (7):
> drm/debugfs: create device-centered debugfs functions
> drm: use new debugfs device-centered functions on DRM core files
> drm/debugfs: create debugfs late register functions
> drm/vc4: use new debugfs device-centered functions
> drm/v3d: use new debugfs device-centered functions
> drm/vkms: use new debugfs device-centered functions
> drm/todo: update the debugfs clean up task
>
> Documentation/gpu/todo.rst | 9 +--
> drivers/gpu/drm/drm_atomic.c | 11 ++-
> drivers/gpu/drm/drm_client.c | 11 ++-
> drivers/gpu/drm/drm_debugfs.c | 102 +++++++++++++++++++++++---
> drivers/gpu/drm/drm_drv.c | 3 +
> drivers/gpu/drm/drm_framebuffer.c | 11 ++-
> drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++-
> drivers/gpu/drm/drm_internal.h | 5 ++
> drivers/gpu/drm/drm_mode_config.c | 2 +
> drivers/gpu/drm/v3d/v3d_debugfs.c | 22 +++---
> drivers/gpu/drm/vc4/vc4_bo.c | 10 +--
> drivers/gpu/drm/vc4/vc4_crtc.c | 7 +-
> drivers/gpu/drm/vc4/vc4_debugfs.c | 36 ++-------
> drivers/gpu/drm/vc4/vc4_dpi.c | 5 +-
> drivers/gpu/drm/vc4/vc4_drv.c | 1 -
> drivers/gpu/drm/vc4/vc4_drv.h | 32 ++------
> drivers/gpu/drm/vc4/vc4_dsi.c | 6 +-
> drivers/gpu/drm/vc4/vc4_hdmi.c | 12 +--
> drivers/gpu/drm/vc4/vc4_hvs.c | 24 ++----
> drivers/gpu/drm/vc4/vc4_v3d.c | 14 +---
> drivers/gpu/drm/vc4/vc4_vec.c | 6 +-
> drivers/gpu/drm/vkms/vkms_drv.c | 17 ++---
> include/drm/drm_debugfs.h | 41 +++++++++++
> include/drm/drm_device.h | 15 ++++
> 24 files changed, 233 insertions(+), 180 deletions(-)
>
> --
> 2.38.1
>
On Mon, Dec 19, 2022 at 11:49:47AM -0100, Melissa Wen wrote:
> On 12/19, Ma?ra Canal wrote:
> > This series introduces the initial structure to make DRM debugfs more
> > device-centered and it is the first step to drop the
> > drm_driver->debugfs_init hooks in the future [1].
> >
> > Currently, DRM debugfs files are created using drm_debugfs_create_files()
> > on request. The first patch of this series makes it possible for DRM devices
> > for creating debugfs files during drm_dev_register(). For it, it introduces
> > two new functions that can be used by the drivers: drm_debugfs_add_files()
> > and drm_debugfs_add_file(). The requests are added to a list and are created
> > all at once during drm_dev_register(). Moreover, the first patch was based on
> > this RFC series [2].
> >
> > The main difference between the RFC series and the current series is the
> > creation of a new fops structure to accommodate the new structs and, also,
> > the creation of a new drm_debugfs_open. Moreover, the new series uses
> > device-managed allocation, returns memory allocation errors, and converts
> > more drivers to the new structure.
> >
> > Moreover, since v3, the ability to create debugfs files at late_register hooks was
> > added. In previous versions, modeset components weren't able to create debugfs
> > files at late_register hooks as the registration of drm_minor happens before the
> > registration of the modeset abstractions. So, the third patch fixes this problem
> > by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
> > catching this problem!
> >
> > Apart from the third patch, the series looks similiar from its last version.
> >
> > [1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
> > [2] https://lore.kernel.org/dri-devel/[email protected]/
> >
> > Best Regards,
> > - Ma?ra Canal
> >
> > ---
> >
> > v1 -> v2: https://lore.kernel.org/dri-devel/[email protected]/T/#t
> >
> > - Fix compilation errors in the second patch (kernel test robot).
> > - Drop debugfs_init hook from vkms (Ma?ra Canal).
> > - Remove return values and error handling to debugfs related
> > functions (Jani Nikula).
> > - Remove entry from list after the file is created, so that drm_debugfs_init
> > can be called more than once (Ma?ra Canal).
> >
> > v2 -> v3: https://lore.kernel.org/dri-devel/[email protected]/
> >
> > - Rebase on top of drm-misc-next
> >
> > v3 -> v4: https://lore.kernel.org/dri-devel/[email protected]/
> >
> > - Add Maxime's Reviewed-by tags
> > - Add the ability to create debugfs files at late_register hooks (Melissa Wen).
>
> Hi Ma?ra,
>
> Thanks for addressing all comments.
>
> Maybe Danvet has some inputs for the late_register approach.
I think as a stop-gap (really need to get this stuff landed so people can
start to use it) this is ok, but long term I think the right fix is to
roll out the same pre-register infrastructure for connector and crtc too.
That way drivers don't need to split their setup code into init and
register anymore, which is the point of this entire rework.
If you want, you can adjust the todo accordingly, but we do already have
the paragraph about connector/crtc.
But we can do that later on, because this is definitely a great way
forward. Thanks a lot for pushing this forward!
> Anyway, LGTM and the entire series is:
>
> Reviewed-by: Melissa Wen <[email protected]>
On the series: Acked-by: Daniel Vetter <[email protected]>
>
> >
> > ---
> >
> > Ma?ra Canal (7):
> > drm/debugfs: create device-centered debugfs functions
> > drm: use new debugfs device-centered functions on DRM core files
> > drm/debugfs: create debugfs late register functions
> > drm/vc4: use new debugfs device-centered functions
> > drm/v3d: use new debugfs device-centered functions
> > drm/vkms: use new debugfs device-centered functions
> > drm/todo: update the debugfs clean up task
> >
> > Documentation/gpu/todo.rst | 9 +--
> > drivers/gpu/drm/drm_atomic.c | 11 ++-
> > drivers/gpu/drm/drm_client.c | 11 ++-
> > drivers/gpu/drm/drm_debugfs.c | 102 +++++++++++++++++++++++---
> > drivers/gpu/drm/drm_drv.c | 3 +
> > drivers/gpu/drm/drm_framebuffer.c | 11 ++-
> > drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++-
> > drivers/gpu/drm/drm_internal.h | 5 ++
> > drivers/gpu/drm/drm_mode_config.c | 2 +
> > drivers/gpu/drm/v3d/v3d_debugfs.c | 22 +++---
> > drivers/gpu/drm/vc4/vc4_bo.c | 10 +--
> > drivers/gpu/drm/vc4/vc4_crtc.c | 7 +-
> > drivers/gpu/drm/vc4/vc4_debugfs.c | 36 ++-------
> > drivers/gpu/drm/vc4/vc4_dpi.c | 5 +-
> > drivers/gpu/drm/vc4/vc4_drv.c | 1 -
> > drivers/gpu/drm/vc4/vc4_drv.h | 32 ++------
> > drivers/gpu/drm/vc4/vc4_dsi.c | 6 +-
> > drivers/gpu/drm/vc4/vc4_hdmi.c | 12 +--
> > drivers/gpu/drm/vc4/vc4_hvs.c | 24 ++----
> > drivers/gpu/drm/vc4/vc4_v3d.c | 14 +---
> > drivers/gpu/drm/vc4/vc4_vec.c | 6 +-
> > drivers/gpu/drm/vkms/vkms_drv.c | 17 ++---
> > include/drm/drm_debugfs.h | 41 +++++++++++
> > include/drm/drm_device.h | 15 ++++
> > 24 files changed, 233 insertions(+), 180 deletions(-)
> >
> > --
> > 2.38.1
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On 12/22/22 14:20, Daniel Vetter wrote:
> On Mon, Dec 19, 2022 at 11:49:47AM -0100, Melissa Wen wrote:
>> On 12/19, Maíra Canal wrote:
>>> This series introduces the initial structure to make DRM debugfs more
>>> device-centered and it is the first step to drop the
>>> drm_driver->debugfs_init hooks in the future [1].
>>>
>>> Currently, DRM debugfs files are created using drm_debugfs_create_files()
>>> on request. The first patch of this series makes it possible for DRM devices
>>> for creating debugfs files during drm_dev_register(). For it, it introduces
>>> two new functions that can be used by the drivers: drm_debugfs_add_files()
>>> and drm_debugfs_add_file(). The requests are added to a list and are created
>>> all at once during drm_dev_register(). Moreover, the first patch was based on
>>> this RFC series [2].
>>>
>>> The main difference between the RFC series and the current series is the
>>> creation of a new fops structure to accommodate the new structs and, also,
>>> the creation of a new drm_debugfs_open. Moreover, the new series uses
>>> device-managed allocation, returns memory allocation errors, and converts
>>> more drivers to the new structure.
>>>
>>> Moreover, since v3, the ability to create debugfs files at late_register hooks was
>>> added. In previous versions, modeset components weren't able to create debugfs
>>> files at late_register hooks as the registration of drm_minor happens before the
>>> registration of the modeset abstractions. So, the third patch fixes this problem
>>> by adding a drm_debugfs_late_register() function. Thanks to Melissa Wen for
>>> catching this problem!
>>>
>>> Apart from the third patch, the series looks similiar from its last version.
>>>
>>> [1] https://cgit.freedesktop.org/drm/drm/tree/Documentation/gpu/todo.rst#n506
>>> [2] https://lore.kernel.org/dri-devel/[email protected]/
>>>
>>> Best Regards,
>>> - Maíra Canal
>>>
>>> ---
>>>
>>> v1 -> v2: https://lore.kernel.org/dri-devel/[email protected]/T/#t
>>>
>>> - Fix compilation errors in the second patch (kernel test robot).
>>> - Drop debugfs_init hook from vkms (Maíra Canal).
>>> - Remove return values and error handling to debugfs related
>>> functions (Jani Nikula).
>>> - Remove entry from list after the file is created, so that drm_debugfs_init
>>> can be called more than once (Maíra Canal).
>>>
>>> v2 -> v3: https://lore.kernel.org/dri-devel/[email protected]/
>>>
>>> - Rebase on top of drm-misc-next
>>>
>>> v3 -> v4: https://lore.kernel.org/dri-devel/[email protected]/
>>>
>>> - Add Maxime's Reviewed-by tags
>>> - Add the ability to create debugfs files at late_register hooks (Melissa Wen).
>>
>> Hi Maíra,
>>
>> Thanks for addressing all comments.
>>
>> Maybe Danvet has some inputs for the late_register approach.
>
> I think as a stop-gap (really need to get this stuff landed so people can
> start to use it) this is ok, but long term I think the right fix is to
> roll out the same pre-register infrastructure for connector and crtc too.
> That way drivers don't need to split their setup code into init and
> register anymore, which is the point of this entire rework.
>
> If you want, you can adjust the todo accordingly, but we do already have
> the paragraph about connector/crtc.
>
> But we can do that later on, because this is definitely a great way
> forward. Thanks a lot for pushing this forward!
>
>> Anyway, LGTM and the entire series is:
>>
>> Reviewed-by: Melissa Wen <[email protected]>
>
> On the series: Acked-by: Daniel Vetter <[email protected]>
Applied this series to drm-misc-next.
Best Regards,
- Maíra Canal
>>
>>>
>>> ---
>>>
>>> Maíra Canal (7):
>>> drm/debugfs: create device-centered debugfs functions
>>> drm: use new debugfs device-centered functions on DRM core files
>>> drm/debugfs: create debugfs late register functions
>>> drm/vc4: use new debugfs device-centered functions
>>> drm/v3d: use new debugfs device-centered functions
>>> drm/vkms: use new debugfs device-centered functions
>>> drm/todo: update the debugfs clean up task
>>>
>>> Documentation/gpu/todo.rst | 9 +--
>>> drivers/gpu/drm/drm_atomic.c | 11 ++-
>>> drivers/gpu/drm/drm_client.c | 11 ++-
>>> drivers/gpu/drm/drm_debugfs.c | 102 +++++++++++++++++++++++---
>>> drivers/gpu/drm/drm_drv.c | 3 +
>>> drivers/gpu/drm/drm_framebuffer.c | 11 ++-
>>> drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++-
>>> drivers/gpu/drm/drm_internal.h | 5 ++
>>> drivers/gpu/drm/drm_mode_config.c | 2 +
>>> drivers/gpu/drm/v3d/v3d_debugfs.c | 22 +++---
>>> drivers/gpu/drm/vc4/vc4_bo.c | 10 +--
>>> drivers/gpu/drm/vc4/vc4_crtc.c | 7 +-
>>> drivers/gpu/drm/vc4/vc4_debugfs.c | 36 ++-------
>>> drivers/gpu/drm/vc4/vc4_dpi.c | 5 +-
>>> drivers/gpu/drm/vc4/vc4_drv.c | 1 -
>>> drivers/gpu/drm/vc4/vc4_drv.h | 32 ++------
>>> drivers/gpu/drm/vc4/vc4_dsi.c | 6 +-
>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 12 +--
>>> drivers/gpu/drm/vc4/vc4_hvs.c | 24 ++----
>>> drivers/gpu/drm/vc4/vc4_v3d.c | 14 +---
>>> drivers/gpu/drm/vc4/vc4_vec.c | 6 +-
>>> drivers/gpu/drm/vkms/vkms_drv.c | 17 ++---
>>> include/drm/drm_debugfs.h | 41 +++++++++++
>>> include/drm/drm_device.h | 15 ++++
>>> 24 files changed, 233 insertions(+), 180 deletions(-)
>>>
>>> --
>>> 2.38.1
>>>
>
>
>