linux-kernel - Re: [PATCH v5 01/18] drm/rockchip: vop2: Add debugfs support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4471861.MSiuQNM8U4@diego>
Date: Tue, 10 Dec 2024 12:57:44 +0100
From: Heiko Stübner <heiko@...ech.de>
To: Andy Yan <andyshrk@....com>
Cc: hjc@...k-chips.com, krzk+dt@...nel.org, s.hauer@...gutronix.de,
 devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, derek.foreman@...labora.com,
 detlev.casanova@...labora.com, Andy Yan <andy.yan@...k-chips.com>,
 Michael Riesch <michael.riesch@...fvision.net>
Subject: Re: [PATCH v5 01/18] drm/rockchip: vop2: Add debugfs support

Hi Andy,

Am Montag, 9. Dezember 2024, 13:29:13 CET schrieb Andy Yan:
> From: Andy Yan <andy.yan@...k-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.hauer@...gutronix.de>
> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>
> Tested-by: Michael Riesch <michael.riesch@...fvision.net> # on RK3568
> Tested-by: Detlev Casanova <detlev.casanova@...labora.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

> +}
> +

> +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?


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