Re: [PATCH v6 41/42] drm/mediatek: mtk_hdmi_v2: Add debugfs ops and implement ABIST

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

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.

> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.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.

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
>