linux-kernel - Re: [PATCH v2 drm-dp 6/7] drm/hisilicon/hibmc: Add drm debugfs functions

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: <afi5npgvnrp56oufhc7576auya26lbwgu377dprddode2kp3sb@u5ctx4o22w4v>
Date: Thu, 13 Feb 2025 02:32:20 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Yongbang Shi <shiyongbang@...wei.com>
Cc: xinliang.liu@...aro.org, tiantao6@...ilicon.com, 
	maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com, 
	daniel@...ll.ch, kong.kongxinwei@...ilicon.com, liangjian010@...wei.com, 
	chenjianmin@...wei.com, lidongming5@...wei.com, libaihan@...wei.com, 
	shenjian15@...wei.com, shaojijie@...wei.com, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 drm-dp 6/7] drm/hisilicon/hibmc: Add drm debugfs
 functions

On Mon, Feb 10, 2025 at 10:49:58PM +0800, Yongbang Shi wrote:
> From: Baihan Li <libaihan@...wei.com>
> 
> We use the previous two patches as our debug functions and
> generate two files. "hibmc-dp" and "color-bar".
> hibmc-dp: read only, print the dp link status and dpcd version

Please define a generic DP attribute for this, handle it in
drm_dp_helper.c. Other drivers then can reuse this debugfs file.
Also note drm_dp_downstream_debug(), it might also be helpful.
Also see msm_dp_debug_show() for inspiration

> color-bar: read/write
>            write: cfg color bar and enable/disable it by your input
>            read: print your current cfg info of color-bar

This really should go into your color-bar patch.

> 
> Signed-off-by: Baihan Li <libaihan@...wei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@...wei.com>
> ---
> ChangeLog:
> v1 -> v2:
>   - deleting edid decoder and its debugfs, suggested by Dmitry Baryshkov.
>   - using debugfs_init() callback, suggested by Dmitry Baryshkov.
> ---
>  drivers/gpu/drm/hisilicon/hibmc/Makefile      |   3 +-
>  .../drm/hisilicon/hibmc/hibmc_drm_debugfs.c   | 124 ++++++++++++++++++
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    |   1 +
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |   2 +
>  4 files changed, 129 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index 43de077d6769..1f65c683282f 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_i2c.o \
> -	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o dp/dp_serdes.o hibmc_drm_dp.o
> +	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o dp/dp_serdes.o hibmc_drm_dp.o \
> +	       hibmc_drm_debugfs.o
>  
>  obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c
> new file mode 100644
> index 000000000000..af2efb70d6ea
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright (c) 2024 Hisilicon Limited.
> +
> +#include <linux/debugfs.h>
> +#include <linux/device.h>
> +#include <linux/seq_file.h>
> +#include <linux/pci.h>
> +
> +#include <drm/drm_drv.h>
> +#include <drm/drm_file.h>
> +#include <drm/drm_debugfs.h>
> +#include <drm/drm_edid.h>
> +
> +#include "hibmc_drm_drv.h"
> +
> +static int hibmc_dp_show(struct seq_file *m, void *arg)
> +{
> +	struct drm_info_node *node = m->private;
> +	struct drm_device *dev = node->minor->dev;
> +	struct hibmc_drm_private *priv = to_hibmc_drm_private(dev);
> +	int idx;
> +
> +	if (!drm_dev_enter(dev, &idx))
> +		return -ENODEV;
> +
> +	seq_printf(m, "enable lanes: %u\n", hibmc_dp_get_lanes(&priv->dp));
> +	seq_printf(m, "link rate: %d\n", hibmc_dp_get_link_rate(&priv->dp) * 27);
> +	seq_printf(m, "dpcd version: 0x%x\n", hibmc_dp_get_dpcd(&priv->dp));
> +
> +	drm_dev_exit(idx);
> +
> +	return 0;
> +}
> +
> +static ssize_t hibmc_control_write(struct file *file, const char __user *user_buf,
> +				   size_t size, loff_t *ppos)
> +{
> +	struct hibmc_drm_private *priv = file_inode(file)->i_private;
> +	struct hibmc_dp_cbar_cfg *cfg = &priv->dp.cfg;
> +	u32 input = 0;
> +	int ret, idx;
> +	u8 val;
> +
> +	ret = kstrtou32_from_user(user_buf, size, 0, &input);
> +	if (ret)
> +		return ret;
> +
> +	val = FIELD_GET(GENMASK(13, 10), input);
> +	if (val > 9)
> +		return -EINVAL;
> +	cfg->pattern = val;
> +	cfg->enable = FIELD_GET(BIT(0), input);
> +	cfg->self_timing = FIELD_GET(BIT(1), input);
> +	cfg->dynamic_rate = FIELD_GET(GENMASK(9, 2), input);

Having a binary file format is really a sad idea. Can it be a text file
instead?

> +
> +	ret = drm_dev_enter(&priv->dev, &idx);
> +	if (!ret)
> +		return -ENODEV;
> +
> +	hibmc_dp_set_cbar(&priv->dp, cfg);
> +
> +	drm_dev_exit(idx);
> +
> +	return size;
> +}
> +

-- 
With best wishes
Dmitry