[PATCH v10 05/16] drm/mediatek: gamma: Enable the Gamma LUT table only after programming

AngeloGioacchino Del Regno posted 16 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v10 05/16] drm/mediatek: gamma: Enable the Gamma LUT table only after programming
Posted by AngeloGioacchino Del Regno 1 year, 3 months ago
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.

Note:
GAMMA should get enabled in between vblanks, but this requires many
efforts in order to make this happen, as that requires migrating all
of the writes to make use of CMDQ instead of cpu writes and that's
not trivial. For this reason, this patch only moves the LUT enable.
The CMDQ rework will come at a later time.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.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 fd6a75a64a9f..18b102bef370 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -68,12 +68,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;
-	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.41.0
Re: [PATCH v10 05/16] drm/mediatek: gamma: Enable the Gamma LUT table only after programming
Posted by CK Hu (胡俊光) 1 year, 1 month ago
Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> 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.
> 
> Note:
> GAMMA should get enabled in between vblanks, but this requires many
> efforts in order to make this happen, as that requires migrating all
> of the writes to make use of CMDQ instead of cpu writes and that's
> not trivial. For this reason, this patch only moves the LUT enable.
> The CMDQ rework will come at a later time.
> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Reviewed-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.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 fd6a75a64a9f..18b102bef370 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -68,12 +68,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;
> -	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);

Move this to bottom of this function. Move here in the patch which
need.

After this modification,

Reviewed-by: CK Hu <ck.hu@mediatek.com>

>  	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)