On Mon, Feb 10, 2025 at 10:49:58PM +0800, Yongbang Shi wrote:
> From: Baihan Li <libaihan@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Yongbang Shi <shiyongbang@xxxxxxxxxx>
> ---
> 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
