<pre>
Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> All of the SoCs that don't have dithering control in the gamma IP
> have got a GAMMA_LUT_TYPE bit that tells to the IP if the LUT is
> "descending" (bit set) or "rising" (bit cleared): make sure to set
> it correctly after programming the LUT.
>
> 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 | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index fbff9f97b737..d9a70238d524 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -22,6 +22,7 @@
> #define GAMMA_RELAY_MODEBIT(0)
> #define GAMMA_LUT_ENBIT(1)
> #define GAMMA_DITHERINGBIT(2)
> +#define GAMMA_LUT_TYPEBIT(2)
> #define DISP_GAMMA_SIZE0x0030
> #define DISP_GAMMA_SIZE_HSIZEGENMASK
> (28, 16)
> #define DISP_GAMMA_SIZE_VSIZEGENMASK
> (12, 0)
> @@ -86,6 +87,17 @@ unsigned int mtk_gamma_get_lut_size(struct device
> *dev)
> return LUT_SIZE_DEFAULT;
> }
>
> +static bool mtk_gamma_lut_is_descending(struct drm_color_lut *lut,
> u32 lut_size)
> +{
> +u64 first, last;
> +int last_entry = lut_size - 1;
> +
> +first = lut[0].red + lut[0].green + lut[0].blue;
> +last = lut[last_entry].red + lut[last_entry].green +
> lut[last_entry].blue;
> +
> +return !!(first > last);
> +}
> +
> /*
> * SoCs supporting 12-bits LUTs are using a new register layout that
> does
> * always support (by HW) both 12-bits and 10-bits LUT but, on
> those, we
> @@ -175,6 +187,14 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
> }
> }
>
> +if (!gamma->data->has_dither) {
> +/* Descending or Rising LUT */
> +if (mtk_gamma_lut_is_descending(lut, gamma->data-
> >lut_size - 1))
> +cfg_val |= FIELD_PREP(GAMMA_LUT_TYPE, 1);

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

But I'm not sure why not write it more simply as

cfg_val |= GAMMA_LUT_TYPE;

Regards,
CK

> +else
> +cfg_val &= ~GAMMA_LUT_TYPE;
> +}
> +
> /* Enable the gamma table */
> cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
>

</pre><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any 
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be 
conveyed only to the designated recipient(s). Any use, dissemination, 
distribution, printing, retaining or copying of this e-mail (including its 
attachments) by unintended recipient(s) is strictly prohibited and may 
be unlawful. If you are not an intended recipient of this e-mail, or believe 
that you have received this e-mail in error, please notify the sender 
immediately (by replying to this e-mail), delete any and all copies of 
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->