Hi Heiko,
At 2024-12-10 19:57:44, "Heiko Stübner" <he...@sntech.de> wrote:
>Hi Andy,
>
>Am Montag, 9. Dezember 2024, 13:29:13 CET schrieb Andy Yan:
>> From: Andy Yan <andy....@rock-chips.com>
>>
>> /sys/kernel/debug/dri/vop2/summary: dump vop display state
>> /sys/kernel/debug/dri/vop2/regs: dump whole vop registers
>> /sys/kernel/debug/dri/vop2/active_regs: only dump the registers of
>> activated modules
>>
>> Reviewed-by: Sascha Hauer <s.ha...@pengutronix.de>
>> Signed-off-by: Andy Yan <andy....@rock-chips.com>
>> Tested-by: Michael Riesch <michael.rie...@wolfvision.net> # on RK3568
>> Tested-by: Detlev Casanova <detlev.casan...@collabora.com>
>> ---
>
>> +static void __vop2_regs_dump(struct seq_file *s, bool active_only)
>> +{
>> + struct drm_info_node *node = s->private;
>> + struct vop2 *vop2 = node->info_ent->data;
>> + struct drm_minor *minor = node->minor;
>> + struct drm_device *drm_dev = minor->dev;
>> + const struct vop2_regs_dump *dump;
>> + unsigned int i;
>> +
>> + drm_modeset_lock_all(drm_dev);
>> +
>> + regcache_drop_region(vop2->map, 0, vop2_regmap_config.max_register);
>> +
>> + if (vop2->enable_count) {
>> + for (i = 0; i < vop2->data->regs_dump_size; i++) {
>> + dump = &vop2->data->regs_dump[i];
>> + vop2_regs_print(vop2, s, dump, active_only);
>> + }
>> + } else {
>> + seq_printf(s, "VOP disabled\n");
>> + }
>> + drm_modeset_unlock_all(drm_dev);
>> +
>
>nit: not needed empty line at the end of the function
Will fixed in V6.
>
>> +}
>> +
>
>> +static void vop2_debugfs_init(struct vop2 *vop2, struct drm_minor *minor)
>> +{
>> + struct dentry *root;
>> + unsigned int i;
>> +
>> + root = debugfs_create_dir("vop2", minor->debugfs_root);
>> + if (!IS_ERR(root)) {
>> + for (i = 0; i < ARRAY_SIZE(vop2_debugfs_list); i++)
>> + vop2_debugfs_list[i].data = vop2;
>> +
>> + drm_debugfs_create_files(vop2_debugfs_list,
>> + ARRAY_SIZE(vop2_debugfs_list),
>> + root, minor);
>> + }
>> +}
>> +
>> +static int vop2_crtc_late_register(struct drm_crtc *crtc)
>> +{
>> + struct vop2_video_port *vp = to_vop2_video_port(crtc);
>> + struct vop2 *vop2 = vp->vop2;
>> +
>> + if (drm_crtc_index(crtc) == 0)
>> + vop2_debugfs_init(vop2, crtc->dev->primary);
>> +
>> + return 0;
>> +}
>
>I'm wondering about, shoudln't there be an unregister step too?
>I.e. the late_register callback says:
>"should be unregistered in the early_unregister callback" [0].
>
>And there exists drm_debugfs_remove_files(), though it doesn't
>seem t be used much - just by tegra.
>
>I haven't managed to find drm code handling that automatically though?
Did a little digging：
rockchip_drm_unbind --》drm_dev_unregister(drm_dev)--》drm_debugfs_dev_fini
--》debugfs_remove_recursive(dev->debugfs_root);
This will remove all the debugfs file under /dri/0-->display-subsystem when we
remove rockchipdrm when it build as module.
If it is builtin, whether we remove them seems to make no difference when we
reboot or thutdown the system ?
I also searched for other platforms using similar late register hook, and it
seems they haven't remove debugfs either.
>
>
>Heiko
>
>
>[0] https://elixir.bootlin.com/linux/v6.12.4/source/include/drm/drm_crtc.h#L737
>[1]
>https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/drm_debugfs.c#L265
>
