Re: [PATCH v8 08/13] drm/mediatek: gamma: Support multi-bank gamma LUT

[CK Hu (胡俊光)]

Hi, Angelo:

On Tue, 2023-08-01 at 13:58 +0200, AngeloGioacchino Del Regno wrote:
> Newer Gamma IP have got multiple LUT banks: support specifying the
> size of the LUT banks and handle bank-switching before programming
> the LUT in mtk_gamma_set_common() in preparation for adding support
> for MT8195 and newer SoCs.
> 
> Suggested-by: Jason-JH.Lin <jason-jh.lin@xxxxxxxxxxxx>
> [Angelo: Refactored original commit]
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@xxxxxxxxxxxxx>
> Reviewed-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 73 +++++++++++++++----
> ----
>  1 file changed, 47 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index a6f7af1a9e8e..fb7c3650a9f7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -24,6 +24,8 @@
>  #define DISP_GAMMA_SIZE				0x0030
>  #define DISP_GAMMA_SIZE_HSIZE				GENMASK
> (28, 16)
>  #define DISP_GAMMA_SIZE_VSIZE				GENMASK
> (12, 0)
> +#define DISP_GAMMA_BANK				0x0100
> +#define DISP_GAMMA_BANK_BANK				GENMASK(1, 0)
>  #define DISP_GAMMA_LUT				0x0700
>  
>  #define DISP_GAMMA_LUT_10BIT_R			GENMASK(29, 20)
> @@ -32,10 +34,12 @@
>  
>  #define LUT_10BIT_MASK				0x03ff
>  #define LUT_BITS_DEFAULT			10
> +#define LUT_BANK_SIZE_DEFAULT			512
>  
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
> +	u16 lut_bank_size;
>  	u16 lut_size;
>  	u8 lut_bits;
>  };
> @@ -80,7 +84,9 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs,
>  	void __iomem *lut_base;
>  	bool lut_diff;
>  	u8 lut_bits;
> -	u32 cfg_val, word;
> +	u16 lut_bank_size;
> +	u32 cfg_val, lbank_val, word;
> +	int cur_bank, num_lut_banks;
>  
>  	/* If there's no gamma lut there's nothing to do here. */
>  	if (!state->gamma_lut)
> @@ -91,41 +97,54 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs,
>  
>  	if (gamma && gamma->data) {
>  		lut_diff = gamma->data->lut_diff;
> +		lut_bank_size = gamma->data->lut_bank_size;
>  		lut_bits = gamma->data->lut_bits;
>  	} else {
>  		lut_diff = false;
> +		lut_bank_size = LUT_BANK_SIZE_DEFAULT;

LUT_BANK_SIZE_DEFAULT is only for AAL driver, so place it in AAL driver
and pass it into this function.

Regards,
CK

>  		lut_bits = LUT_BITS_DEFAULT;
>  	}
> +	num_lut_banks = lut_size / lut_bank_size;
>  
>  	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++) {
> -		struct drm_color_lut diff, hwlut;
> -
> -		hwlut.red = drm_color_lut_extract(lut[i].red,
> lut_bits);
> -		hwlut.green = drm_color_lut_extract(lut[i].green,
> lut_bits);
> -		hwlut.blue = drm_color_lut_extract(lut[i].blue,
> lut_bits);
> -
> -		if (!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);
> -		} else {
> -			diff.red = lut[i].red - lut[i - 1].red;
> -			diff.red = drm_color_lut_extract(diff.red,
> lut_bits);
> -
> -			diff.green = lut[i].green - lut[i - 1].green;
> -			diff.green = drm_color_lut_extract(diff.green,
> lut_bits);
> -
> -			diff.blue = lut[i].blue - lut[i - 1].blue;
> -			diff.blue = drm_color_lut_extract(diff.blue,
> lut_bits);
> -
> -			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);
> +	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);
> +			writel(lbank_val, regs + DISP_GAMMA_BANK);
> +		}
> +
> +		for (i = 0; i < lut_bank_size; i++) {
> +			int n = (cur_bank * lut_bank_size) + i;
> +			struct drm_color_lut diff, hwlut;
> +
> +			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 (!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);
> +			} else {
> +				diff.red = lut[n].red - lut[n - 1].red;
> +				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, lut_bits);
> +
> +				diff.blue = lut[n].blue - lut[n -
> 1].blue;
> +				diff.blue =
> drm_color_lut_extract(diff.blue, lut_bits);
> +
> +				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);
> +			}
> +			writel(word, (lut_base + i * 4));
>  		}
> -		writel(word, (lut_base + i * 4));
>  	}
>  
>  	/* Enable the gamma table */
> @@ -236,11 +255,13 @@ static int 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,