| 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
| ||
|
Message-ID: <gv62rn7uqxplvpe6dqmfg7bypykmekdgv3abwngjcntmb7onhi@bgyfpwzx5kae>
Date: Sat, 22 Feb 2025 07:50:00 +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 v3 drm-dp 6/8] drm/hisilicon/hibmc: Add colorbar-cfg
feature and its debugfs file
On Sat, Feb 22, 2025 at 10:50:59AM +0800, Yongbang Shi wrote:
> From: Baihan Li <libaihan@...wei.com>
>
> DP controller can support generating a color bar signal over the
> DisplayPort interface. This can be useful to check for possible DDR
> or GPU problems, as the signal generator resides completely in the DP
> block. Add debugfs file that controls colorbar generator.
>
> echo: config the color bar register to display
> cat: print the color bar configuration
>
> Signed-off-by: Baihan Li <libaihan@...wei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@...wei.com>
> ---
> ChangeLog:
> v2 -> v3:
> - rewrite the commit log, suggested by Dmitry Baryshkov.
> - move colorbar debugfs entry to this patch, suggested by Dmitry Baryshkov.
> - change binary format to integer format, suggested by Dmitry Baryshkov.
> v1 -> v2:
> - add colorbar introduction in commit, suggested by Dmitry Baryshkov.
> - splittting colorbar and debugfs in different patches, suggested by Dmitry Baryshkov.
> - 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 +-
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 43 ++++++++
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 29 +++++
> drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h | 3 +
> .../drm/hisilicon/hibmc/hibmc_drm_debugfs.c | 100 ++++++++++++++++++
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 1 +
> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 2 +
> 7 files changed, 180 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/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> index 9d673f431a0e..a921b98dbf50 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> @@ -227,3 +227,46 @@ int hibmc_dp_mode_set(struct hibmc_dp *dp, struct drm_display_mode *mode)
>
> return 0;
> }
> +
> +static const struct hibmc_dp_color_raw g_rgb_raw[] = {
> + {CBAR_COLOR_BAR, 0x000, 0x000, 0x000},
> + {CBAR_WHITE, 0xfff, 0xfff, 0xfff},
> + {CBAR_RED, 0xfff, 0x000, 0x000},
> + {CBAR_ORANGE, 0xfff, 0x800, 0x000},
> + {CBAR_YELLOW, 0xfff, 0xfff, 0x000},
> + {CBAR_GREEN, 0x000, 0xfff, 0x000},
> + {CBAR_CYAN, 0x000, 0x800, 0x800},
> + {CBAR_BLUE, 0x000, 0x000, 0xfff},
> + {CBAR_PURPLE, 0x800, 0x000, 0x800},
> + {CBAR_BLACK, 0x000, 0x000, 0x000},
> +};
> +
> +void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg)
> +{
> + struct hibmc_dp_dev *dp_dev = dp->dp_dev;
> + struct hibmc_dp_color_raw raw_data;
> +
> + if (cfg->enable) {
> + hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, BIT(9),
> + cfg->self_timing);
> + hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, GENMASK(8, 1),
> + cfg->dynamic_rate);
> + if (cfg->pattern == CBAR_COLOR_BAR) {
> + hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, BIT(10), 0);
> + } else {
> + raw_data = g_rgb_raw[cfg->pattern];
> + drm_dbg_dp(dp->drm_dev, "r:%x g:%x b:%x\n", raw_data.r_value,
> + raw_data.g_value, raw_data.b_value);
> + hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, BIT(10), 1);
> + hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, GENMASK(23, 12),
> + raw_data.r_value);
> + hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL1, GENMASK(23, 12),
> + raw_data.g_value);
> + hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL1, GENMASK(11, 0),
> + raw_data.b_value);
> + }
> + }
> +
> + hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, BIT(0), cfg->enable);
> + writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL);
> +}
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> index 53b6d0beecea..83a53dae8012 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> @@ -14,6 +14,33 @@
>
> struct hibmc_dp_dev;
>
> +enum hibmc_dp_cbar_pattern {
> + CBAR_COLOR_BAR,
> + CBAR_WHITE,
> + CBAR_RED,
> + CBAR_ORANGE,
> + CBAR_YELLOW,
> + CBAR_GREEN,
> + CBAR_CYAN,
> + CBAR_BLUE,
> + CBAR_PURPLE,
> + CBAR_BLACK,
> +};
> +
> +struct hibmc_dp_color_raw {
> + enum hibmc_dp_cbar_pattern pattern;
> + u32 r_value;
> + u32 g_value;
> + u32 b_value;
> +};
> +
> +struct hibmc_dp_cbar_cfg {
> + u8 enable;
> + u8 self_timing;
> + u8 dynamic_rate; /* 0:static, 1-255(frame):dynamic */
> + enum hibmc_dp_cbar_pattern pattern;
> +};
> +
> struct hibmc_dp {
> struct hibmc_dp_dev *dp_dev;
> struct drm_device *drm_dev;
> @@ -21,10 +48,12 @@ struct hibmc_dp {
> struct drm_connector connector;
> void __iomem *mmio;
> struct drm_dp_aux aux;
> + struct hibmc_dp_cbar_cfg cfg;
> };
>
> int hibmc_dp_hw_init(struct hibmc_dp *dp);
> int hibmc_dp_mode_set(struct hibmc_dp *dp, struct drm_display_mode *mode);
> void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable);
> +void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg);
>
> #endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> index b75ac13a5ead..4c388f633081 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> @@ -67,6 +67,9 @@
> #define HIBMC_DP_CFG_STREAM_HTOTAL_SIZE GENMASK(31, 16)
> #define HIBMC_DP_CFG_STREAM_HBLANK_SIZE GENMASK(15, 0)
>
> +#define HIBMC_DP_COLOR_BAR_CTRL 0x260
> +#define HIBMC_DP_COLOR_BAR_CTRL1 0x264
> +
> #define HIBMC_DP_TIMING_GEN_CONFIG0 0x26c
> #define HIBMC_DP_CFG_TIMING_GEN0_HACTIVE GENMASK(31, 16)
> #define HIBMC_DP_CFG_TIMING_GEN0_HBLANK GENMASK(15, 0)
> 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..8d050a36946e
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c
> @@ -0,0 +1,100 @@
> +// 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"
> +
> +#define MAX_BUF_SIZE 12
> +
> +static ssize_t hibmc_control_write(struct file *file, const char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct hibmc_drm_private *priv = file_inode(file)->i_private;
> + struct hibmc_dp_cbar_cfg *cfg = &priv->dp.cfg;
> + int ret, idx;
> + u8 buf[MAX_BUF_SIZE];
> +
> + if (count >= MAX_BUF_SIZE)
> + return -EINVAL;
> +
> + if (copy_from_user(buf, user_buf, count))
> + return -EFAULT;
> +
> + buf[count] = '\0';
> +
There should be at least some documentation on the written values.
> + if (sscanf(buf, "%hhu %hhu %hhu %u", &cfg->enable, &cfg->self_timing,
> + &cfg->dynamic_rate, &cfg->pattern) != 4) {
> + return -EINVAL;
> + }
> +
> + if (cfg->pattern > 9 || cfg->enable > 1 || cfg->self_timing > 1)
> + return -EINVAL;
> +
> + ret = drm_dev_enter(&priv->dev, &idx);
> + if (!ret)
> + return -ENODEV;
> +
> + hibmc_dp_set_cbar(&priv->dp, cfg);
> +
> + drm_dev_exit(idx);
> +
> + return count;
> +}
--
With best wishes
Dmitry
Powered by blists - more mailing lists