linux-kernel - Re: [PATCH v4 05/11] drm/mediatek: gamma: Enable the Gamma LUT table only after programming

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51a5fbd349cce69d372f4ccfff7010ea9e6e8f75.camel@mediatek.com>
Date:   Mon, 22 May 2023 10:00:11 +0000
From:   CK Hu (胡俊光) <ck.hu@...iatek.com>
To:     "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>,
        "chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "wenst@...omium.org" <wenst@...omium.org>,
        Jason-JH Lin (林睿祥) 
        <Jason-JH.Lin@...iatek.com>,
        "kernel@...labora.com" <kernel@...labora.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>
Subject: Re: [PATCH v4 05/11] drm/mediatek: gamma: Enable the Gamma LUT table
 only after programming

Hi, Angelo:

On Thu, 2023-05-18 at 12:48 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Move the write to DISP_GAMMA_CFG to enable the Gamma LUT to after
> programming the actual table to avoid potential visual glitches
> during
> table modification.

I think user could update the lut table frequently, so when do you
disable the gamma function before next update? In addition, if we
really care the glitches, update the register in vblank period which
should use cmdq to update the register. But now, I think we do not care
the glitches. You may skip this patch, or fix the problem I mention.

Regards,
CK

> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@...labora.com>
> Reviewed-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 60ccea8c1e1a..1592614b6de7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -71,12 +71,12 @@ unsigned int mtk_gamma_get_lut_size(struct device
> *dev)
>  void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state)
>  {
>         struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> -       unsigned int i, reg;
> +       unsigned int i;
>         struct drm_color_lut *lut;
>         void __iomem *lut_base;
>         bool lut_diff;
>         u16 lut_size;
> -       u32 word;
> +       u32 cfg_val, word;
> 
>         /* If there's no gamma lut there's nothing to do here. */
>         if (!state->gamma_lut)
> @@ -90,9 +90,7 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs, struct drm_crt
>                 lut_size = LUT_SIZE_DEFAULT;
>         }
> 
> -       reg = readl(regs + DISP_GAMMA_CFG);
> -       reg = reg | GAMMA_LUT_EN;
> -       writel(reg, regs + DISP_GAMMA_CFG);
> +       cfg_val = readl(regs + DISP_GAMMA_CFG);
>         lut_base = regs + DISP_GAMMA_LUT;
>         lut = (struct drm_color_lut *)state->gamma_lut->data;
>         for (i = 0; i < lut_size; i++) {
> @@ -122,6 +120,11 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>                 }
>                 writel(word, (lut_base + i * 4));
>         }
> +
> +       /* Enable the gamma table */
> +       cfg_val = cfg_val | GAMMA_LUT_EN;
> +
> +       writel(cfg_val, regs + DISP_GAMMA_CFG);
>  }
> 
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> --
> 2.40.1
>