[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c58be715-a93e-4cda-9473-c2e862fc4537@huawei.com>
Date: Fri, 14 Feb 2025 11:59:41 +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 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
Hi Dmitry,
Thanks for your much advising! I will consider all of them and fix them with detailed explanations.
And I will try to make it in drm_dp_helper.c for here.
Sincerely,
Baihan Li
>> 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.
Ok.
>> 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;
>> +}
>> +
Powered by blists - more mailing lists