<pre>
Hi, Angelo:

On Thu, 2023-10-12 at 11:57 +0200, AngeloGioacchino Del Regno wrote:
> New SoCs, like MT8195, not only may support bigger lookup tables, but
> have got a different register layout to support bigger precision:
> support specifying the number of `lut_bits` for each SoC and use it
> in mtk_gamma_set_common() to perform the right calculations and add
> support for 12-bit gamma lookup tables.
>
> While at it, also reorder the variables in mtk_gamma_set_common()
> and rename `lut_base` to `lut0_base` to improve readability.

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

>
> Reviewed-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 83 +++++++++++++++++--
> ----
> 1 file changed, 64 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 911468984ad5..6305cd95e6d4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -26,17 +26,26 @@
> #define DISP_GAMMA_SIZE_VSIZEGENMASK
> (12, 0)
> #define DISP_GAMMA_BANK0x0100
> #define DISP_GAMMA_BANK_BANKGENMASK(1, 0)
> +#define DISP_GAMMA_BANK_DATA_MODEBIT(2)
> #define DISP_GAMMA_LUT0x0700
> +#define DISP_GAMMA_LUT10x0b00
>
> +/* For 10 bit LUT layout, R/G/B are in the same register */
> #define DISP_GAMMA_LUT_10BIT_RGENMASK(29, 20)
> #define DISP_GAMMA_LUT_10BIT_GGENMASK(19, 10)
> #define DISP_GAMMA_LUT_10BIT_BGENMASK(9, 0)
>
> +/* For 12 bit LUT layout, R/G are in LUT, B is in LUT1 */
> +#define DISP_GAMMA_LUT_12BIT_RGENMASK(11, 0)
> +#define DISP_GAMMA_LUT_12BIT_GGENMASK(23, 12)
> +#define DISP_GAMMA_LUT_12BIT_BGENMASK(11, 0)
> +
> struct mtk_disp_gamma_data {
> bool has_dither;
> bool lut_diff;
> u16 lut_bank_size;
> u16 lut_size;
> +u8 lut_bits;
> };
>
> /*
> @@ -72,28 +81,48 @@ unsigned int mtk_gamma_get_lut_size(struct device
> *dev)
> return 0;
> }
>
> +/*
> + * 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
> + * ignore the support for 10-bits in this driver and always use 12-
> bits.
> + *
> + * Summarizing:
> + * - SoC HW support 9/10-bits LUT only
> + * - Old register layout
> + * - 10-bits LUT supported
> + * - 9-bits LUT not supported
> + * - SoC HW support both 10/12bits LUT
> + * - New register layout
> + * - 12-bits LUT supported
> + * - 10-its LUT not supported
> + */
> void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> {
> struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> -unsigned int i;
> -struct drm_color_lut *lut;
> -void __iomem *lut_base;
> -u32 cfg_val, lbank_val, word;
> +void __iomem *lut0_base = gamma->regs + DISP_GAMMA_LUT;
> +void __iomem *lut1_base = gamma->regs + DISP_GAMMA_LUT1;
> +u32 cfg_val, data_mode, lbank_val, word[2];
> +u8 lut_bits = gamma->data->lut_bits;
> int cur_bank, num_lut_banks;
> +struct drm_color_lut *lut;
> +unsigned int i;
>
> /* If there's no gamma lut there's nothing to do here. */
> if (!state->gamma_lut)
> return;
>
> num_lut_banks = gamma->data->lut_size / gamma->data-
> >lut_bank_size;
> -lut_base = gamma->regs + DISP_GAMMA_LUT;
> lut = (struct drm_color_lut *)state->gamma_lut->data;
>
> +/* Switch to 12 bits data mode if supported */
> +data_mode = FIELD_PREP(DISP_GAMMA_BANK_DATA_MODE, !!(lut_bits
> == 12));
> +
> for (cur_bank = 0; cur_bank < num_lut_banks; cur_bank++) {
>
> /* Switch gamma bank and set data mode before writing
> LUT */
> if (num_lut_banks > 1) {
> lbank_val = FIELD_PREP(DISP_GAMMA_BANK_BANK,
> cur_bank);
> +lbank_val |= data_mode;
> writel(lbank_val, gamma->regs +
> DISP_GAMMA_BANK);
> }
>
> @@ -101,29 +130,43 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
> int n = cur_bank * gamma->data->lut_bank_size +
> i;
> struct drm_color_lut diff, hwlut;
>
> -hwlut.red = drm_color_lut_extract(lut[n].red,
> 10);
> -hwlut.green =
> drm_color_lut_extract(lut[n].green, 10);
> -hwlut.blue = drm_color_lut_extract(lut[n].blue,
> 10);
> +hwlut.red = drm_color_lut_extract(lut[n].red,
> lut_bits);
> +hwlut.green =
> drm_color_lut_extract(lut[n].green, lut_bits);
> +hwlut.blue = drm_color_lut_extract(lut[n].blue,
> lut_bits);
>
> if (!gamma->data->lut_diff || (i % 2 == 0)) {
> -word =
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
> -word |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
> -word |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
> +if (lut_bits == 12) {
> +word[0] =
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_R, hwlut.red);
> +word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_G, hwlut.green);
> +word[1] =
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_B, hwlut.blue);
> +} else {
> +word[0] =
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
> +word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
> +word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
> +}
> } else {
> diff.red = lut[n].red - lut[n - 1].red;
> -diff.red =
> drm_color_lut_extract(diff.red, 10);
> +diff.red =
> drm_color_lut_extract(diff.red, lut_bits);
>
> diff.green = lut[n].green - lut[n -
> 1].green;
> -diff.green =
> drm_color_lut_extract(diff.green, 10);
> +diff.green =
> drm_color_lut_extract(diff.green, lut_bits);
>
> diff.blue = lut[n].blue - lut[n -
> 1].blue;
> -diff.blue =
> drm_color_lut_extract(diff.blue, 10);
> -
> -word =
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
> -word |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
> -word |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
> +diff.blue =
> drm_color_lut_extract(diff.blue, lut_bits);
> +
> +if (lut_bits == 12) {
> +word[0] =
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_R, diff.red);
> +word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_G, diff.green);
> +word[1] =
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_B, diff.blue);
> +} else {
> +word[0] =
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
> +word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
> +word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
> +}
> }
> -writel(word, lut_base + i * 4);
> +writel(word[0], lut0_base + i * 4);
> +if (lut_bits == 12)
> +writel(word[1], lut1_base + i * 4);
> }
> }
>
> @@ -225,11 +268,13 @@ static void mtk_disp_gamma_remove(struct
> platform_device *pdev)
> static const struct mtk_disp_gamma_data mt8173_gamma_driver_data = {
> .has_dither = true,
> .lut_bank_size = 512,
> +.lut_bits = 10,
> .lut_size = 512,
> };
>
> static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
> .lut_bank_size = 512,
> +.lut_bits = 10,
> .lut_diff = true,
> .lut_size = 512,
> };

</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><!--}-->