[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <358623c0-a81a-4a17-8979-f11de5b71675@collabora.com>
Date: Mon, 17 Feb 2025 16:10:42 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: CK Hu (胡俊光) <ck.hu@...iatek.com>,
"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
Cc: "robh@...nel.org" <robh@...nel.org>,
"jie.qiu@...iatek.com" <jie.qiu@...iatek.com>,
"tzimmermann@...e.de" <tzimmermann@...e.de>,
"simona@...ll.ch" <simona@...ll.ch>, "mripard@...nel.org"
<mripard@...nel.org>, Jitao Shi (石记涛)
<jitao.shi@...iatek.com>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"kernel@...labora.com" <kernel@...labora.com>,
"dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
Lewis Liao (廖柏鈞) <Lewis.Liao@...iatek.com>,
"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
TommyYL Chen (陳彥良) <TommyYL.Chen@...iatek.com>,
Ives Chenjh (陳俊弘) <Ives.Chenjh@...iatek.com>,
"airlied@...il.com" <airlied@...il.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>,
"junzhi.zhao@...iatek.com" <junzhi.zhao@...iatek.com>
Subject: Re: [PATCH v6 41/42] drm/mediatek: mtk_hdmi_v2: Add debugfs ops and
implement ABIST
Il 14/02/25 06:21, CK Hu (胡俊光) ha scritto:
> On Tue, 2025-02-11 at 12:34 +0100, AngeloGioacchino Del Regno wrote:
>> External email : Please do not click links or open attachments until you have verified the sender or the content.
>>
>>
>> Implement the Automated Built-In Self-Test ABIST functionality
>> provided by the HDMIv2 IP and expose it through the "hdmi_abist"
>> debugfs file.
>>
>> Write "1" to this file to activate ABIST, or "0" to deactivate.
>
> Describe what we would see when activate ABIST.
> Give some example of when to use it.
>
Sure, I will.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>> ---
>> drivers/gpu/drm/mediatek/mtk_hdmi_v2.c | 123 +++++++++++++++++++++++++
>> 1 file changed, 123 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c b/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c
>> index 36b7f8d8d218..f4a086b92dae 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi_v2.c
>> @@ -1170,6 +1170,128 @@ static int mtk_hdmi_v2_hdmi_write_infoframe(struct drm_bridge *bridge,
>> return 0;
>> }
>>
>> +static int mtk_hdmi_v2_set_abist(struct mtk_hdmi *hdmi, bool enable)
>> +{
>> + struct drm_display_mode *mode = &hdmi->mode;
>> + int abist_format = -EINVAL;
>> + bool interlaced;
>> +
>> + if (!enable) {
>> + regmap_clear_bits(hdmi->regs, TOP_CFG00, HDMI_ABIST_ENABLE);
>> + return 0;
>> + }
>> +
>> + if (!mode->hdisplay || !mode->vdisplay)
>> + return -EINVAL;
>> +
>> + interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE;
>> +
>> + switch (mode->hdisplay) {
>> + case 720:
>> + if (mode->vdisplay == 480)
>> + abist_format = 2;
>> + else if (mode->vdisplay == 576)
>> + abist_format = 11;
>> + break;
>> + case 1280:
>> + if (mode->vdisplay == 720)
>> + abist_format = 3;
>> + break;
>> + case 1440:
>> + if (mode->vdisplay == 480)
>> + abist_format = interlaced ? 5 : 9;
>> + else if (mode->vdisplay == 576)
>> + abist_format = interlaced ? 14 : 18;
>> + break;
>> + case 1920:
>> + if (mode->vdisplay == 1080)
>> + abist_format = interlaced ? 4 : 10;
>> + break;
>> + case 3840:
>> + if (mode->vdisplay == 2160)
>> + abist_format = 25;
>> + break;
>> + case 4096:
>> + if (mode->vdisplay == 2160)
>> + abist_format = 26;
>> + break;
>> + default:
>> + break;
>> + }
>> + if (!abist_format)
>
> abist_format's initial value is -EINVAL, so this checking would never be true.
> Let abist_format's initial value be zero.
>
No, because abist_format 0 is a valid format.
I'll change this to
if (abist_format < 0)
return abist_format;
Thanks!
Regards,
Angelo
> Regards,
> CK
>
>> + return -EINVAL;
>> +
>> + regmap_update_bits(hdmi->regs, TOP_CFG00, HDMI_ABIST_VIDEO_FORMAT,
>> + FIELD_PREP(HDMI_ABIST_VIDEO_FORMAT, abist_format));
>> + regmap_set_bits(hdmi->regs, TOP_CFG00, HDMI_ABIST_ENABLE);
>> + return 0;
>> +}
>> +
>> +static int mtk_hdmi_v2_debug_abist_show(struct seq_file *m, void *arg)
>> +{
>> + struct mtk_hdmi *hdmi = m->private;
>> + bool en;
>> + u32 val;
>> + int ret;
>> +
>> + if (!hdmi)
>> + return -EINVAL;
>> +
>> + ret = regmap_read(hdmi->regs, TOP_CFG00, &val);
>> + if (ret)
>> + return ret;
>> +
>> + en = FIELD_GET(HDMI_ABIST_ENABLE, val);
>> +
>> + seq_printf(m, "HDMI Automated Built-In Self Test: %s\n",
>> + en ? "Enabled" : "Disabled");
>> +
>> + return 0;
>> +}
>> +
>> +static ssize_t mtk_hdmi_v2_debug_abist_write(struct file *file,
>> + const char __user *ubuf,
>> + size_t len, loff_t *offp)
>> +{
>> + struct seq_file *m = file->private_data;
>> + int ret;
>> + u32 en;
>> +
>> + if (!m || !m->private || *offp)
>> + return -EINVAL;
>> +
>> + ret = kstrtouint_from_user(ubuf, len, 0, &en);
>> + if (ret)
>> + return ret;
>> +
>> + if (en < 0 || en > 1)
>> + return -EINVAL;
>> +
>> + mtk_hdmi_v2_set_abist((struct mtk_hdmi *)m->private, en);
>> + return len;
>> +}
>> +
>> +static int mtk_hdmi_v2_debug_abist_open(struct inode *inode, struct file *file)
>> +{
>> + return single_open(file, mtk_hdmi_v2_debug_abist_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations mtk_hdmi_debug_abist_fops = {
>> + .owner = THIS_MODULE,
>> + .open = mtk_hdmi_v2_debug_abist_open,
>> + .read = seq_read,
>> + .write = mtk_hdmi_v2_debug_abist_write,
>> + .llseek = seq_lseek,
>> + .release = single_release,
>> +};
>> +
>> +static void mtk_hdmi_v2_debugfs_init(struct drm_bridge *bridge, struct dentry *root)
>> +{
>> + struct mtk_hdmi *dpi = hdmi_ctx_from_bridge(bridge);
>> +
>> + debugfs_create_file("hdmi_abist", 0640, root, dpi, &mtk_hdmi_debug_abist_fops);
>> +}
>> +
>> static const struct drm_bridge_funcs mtk_v2_hdmi_bridge_funcs = {
>> .attach = mtk_hdmi_v2_bridge_attach,
>> .detach = mtk_hdmi_v2_bridge_detach,
>> @@ -1189,6 +1311,7 @@ static const struct drm_bridge_funcs mtk_v2_hdmi_bridge_funcs = {
>> .hdmi_tmds_char_rate_valid = mtk_hdmi_v2_hdmi_tmds_char_rate_valid,
>> .hdmi_clear_infoframe = mtk_hdmi_v2_hdmi_clear_infoframe,
>> .hdmi_write_infoframe = mtk_hdmi_v2_hdmi_write_infoframe,
>> + .debugfs_init = mtk_hdmi_v2_debugfs_init,
>> };
>>
>> /*
>> --
>> 2.48.1
>>
>
Powered by blists - more mailing lists