[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6fd6510b-dfbd-417e-9f64-6c6265fd1a64@huawei.com>
Date: Sat, 22 Feb 2025 18:33:15 +0800
From: Yongbang Shi <shiyongbang@...wei.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
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>,
<shiyongbang@...wei.com>
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.
Okay!
>> + 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;
>> +}
Powered by blists - more mailing lists