diff mbox series

[v10,06/16] drm/mediatek: gamma: Use bitfield macros

Message ID	20230804072850.89365-7-angelogioacchino.delregno@collabora.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id C858E66071BE; Fri, 4 Aug 2023 08:29:01 +0100 (BST) From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> To: chunkuang.hu@kernel.org Cc: p.zabel@pengutronix.de, airlied@gmail.com, daniel@ffwll.ch, matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com, dri-devel@lists.freedesktop.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, wenst@chromium.org, kernel@collabora.com, ehristev@collabora.com, "Jason-JH . Lin" <jason-jh.lin@mediatek.com>, Alexandre Mergnat <amergnat@baylibre.com> Subject: [PATCH v10 06/16] drm/mediatek: gamma: Use bitfield macros Date: Fri, 4 Aug 2023 09:28:40 +0200 Message-ID: <20230804072850.89365-7-angelogioacchino.delregno@collabora.com> In-Reply-To: <20230804072850.89365-1-angelogioacchino.delregno@collabora.com> References: <20230804072850.89365-1-angelogioacchino.delregno@collabora.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	MediaTek DDP GAMMA - 12-bit LUT support \| [v10,00/16] MediaTek DDP GAMMA - 12-bit LUT support [v10,01/16] drm/mediatek: gamma: Adjust mtk_drm_gamma_set_common parameters [v10,02/16] drm/mediatek: gamma: Reduce indentation in mtk_gamma_set_common() [v10,03/16] drm/mediatek: gamma: Support SoC specific LUT size [v10,04/16] drm/mediatek: gamma: Improve and simplify HW LUT calculation [v10,05/16] drm/mediatek: gamma: Enable the Gamma LUT table only after programming [v10,06/16] drm/mediatek: gamma: Use bitfield macros [v10,07/16] drm/mediatek: aal: Use bitfield macros [v10,08/16] drm/mediatek: De-commonize disp_aal/disp_gamma gamma_set functions [v10,09/16] drm/mediatek: gamma: Support specifying number of bits per LUT component [v10,10/16] drm/mediatek: gamma: Support multi-bank gamma LUT [v10,11/16] drm/mediatek: gamma: Add support for 12-bit LUT and MT8195 [v10,12/16] drm/mediatek: gamma: Make sure relay mode is disabled [v10,13/16] drm/mediatek: gamma: Program gamma LUT type for descending or rising [v10,14/16] drm/mediatek: aal: Rewrite kerneldoc for struct mtk_disp_aal [v10,15/16] drm/mediatek: gamma: Add kerneldoc for struct mtk_disp_gamma [v10,16/16] drm/mediatek: aal: Compress of_device_id entries and add sentinel

Commit Message

AngeloGioacchino Del Regno Aug. 4, 2023, 7:28 a.m. UTC

  Make the code more robust and improve readability by using bitfield
macros instead of open coding bit operations.
While at it, also add a definition for LUT_BITS_DEFAULT.

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 | 41 ++++++++++++++---------
 1 file changed, 26 insertions(+), 15 deletions(-)

Comments

CK Hu (胡俊光) Oct. 12, 2023, 4:12 a.m. UTC | #1

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Make the code more robust and improve readability by using bitfield
> macros instead of open coding bit operations.
> While at it, also add a definition for LUT_BITS_DEFAULT.
> 
> 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 | 41 ++++++++++++++-------
> --
>  1 file changed, 26 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 18b102bef370..ea91d3619716 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -3,6 +3,7 @@
>   * Copyright (c) 2021 MediaTek Inc.
>   */
>  
> +#include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/component.h>
>  #include <linux/module.h>
> @@ -21,9 +22,16 @@
>  #define GAMMA_LUT_EN					BIT(1)
>  #define GAMMA_DITHERING					BIT(2)
>  #define DISP_GAMMA_SIZE				0x0030
> +#define DISP_GAMMA_SIZE_HSIZE				GENMASK
> (28, 16)
> +#define DISP_GAMMA_SIZE_VSIZE				GENMASK
> (12, 0)
>  #define DISP_GAMMA_LUT				0x0700
>  
> +#define DISP_GAMMA_LUT_10BIT_R			GENMASK(29, 20)
> +#define DISP_GAMMA_LUT_10BIT_G			GENMASK(19, 10)
> +#define DISP_GAMMA_LUT_10BIT_B			GENMASK(9, 0)
> +
>  #define LUT_10BIT_MASK				0x03ff
> +#define LUT_BITS_DEFAULT			10
>  #define LUT_SIZE_DEFAULT			512
>  
>  struct mtk_disp_gamma_data {
> @@ -96,33 +104,33 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  	for (i = 0; i < lut_size; i++) {
>  		struct drm_color_lut diff, hwlut;
>  
> -		hwlut.red = drm_color_lut_extract(lut[i].red, 10);
> -		hwlut.green = drm_color_lut_extract(lut[i].green, 10);
> -		hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
> +		hwlut.red = drm_color_lut_extract(lut[i].red,
> LUT_BITS_DEFAULT);

You finally drop LUT_BITS_DEFAULT, so it's not necessary to introduce
it in this patch. Let this patch only use bitfield macros.

Regards,
CK

> +		hwlut.green = drm_color_lut_extract(lut[i].green,
> LUT_BITS_DEFAULT);
> +		hwlut.blue = drm_color_lut_extract(lut[i].blue,
> LUT_BITS_DEFAULT);
>  
>  		if (!lut_diff || (i % 2 == 0)) {
> -			word = hwlut.red << 20 +
> -			       hwlut.green << 10 +
> -			       hwlut.red;
> +			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, 10);
> +			diff.red = drm_color_lut_extract(diff.red,
> LUT_BITS_DEFAULT);
>  
>  			diff.green = lut[i].green - lut[i - 1].green;
> -			diff.green = drm_color_lut_extract(diff.green,
> 10);
> +			diff.green = drm_color_lut_extract(diff.green,
> LUT_BITS_DEFAULT);
>  
>  			diff.blue = lut[i].blue - lut[i - 1].blue;
> -			diff.blue = drm_color_lut_extract(diff.blue,
> 10);
> +			diff.blue = drm_color_lut_extract(diff.blue,
> LUT_BITS_DEFAULT);
>  
> -			word = diff.blue << 20 +
> -			       diff.green << 10 +
> -			       diff.red;
> +			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));
>  	}
>  
>  	/* Enable the gamma table */
> -	cfg_val = cfg_val | GAMMA_LUT_EN;
> +	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
>  
>  	writel(cfg_val, regs + DISP_GAMMA_CFG);
>  }
> @@ -139,9 +147,12 @@ void mtk_gamma_config(struct device *dev,
> unsigned int w,
>  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
>  {
>  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +	u32 sz;
> +
> +	sz = FIELD_PREP(DISP_GAMMA_SIZE_HSIZE, w);
> +	sz |= FIELD_PREP(DISP_GAMMA_SIZE_VSIZE, h);
>  
> -	mtk_ddp_write(cmdq_pkt, h << 16 | w, &gamma->cmdq_reg, gamma-
> >regs,
> -		      DISP_GAMMA_SIZE);
> +	mtk_ddp_write(cmdq_pkt, sz, &gamma->cmdq_reg, gamma->regs,
> DISP_GAMMA_SIZE);
>  	if (gamma->data && gamma->data->has_dither)
>  		mtk_dither_set_common(gamma->regs, &gamma->cmdq_reg,
> bpc,
>  				      DISP_GAMMA_CFG, GAMMA_DITHERING,
> cmdq_pkt);

diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 18b102bef370..ea91d3619716 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -3,6 +3,7 @@ 
  * Copyright (c) 2021 MediaTek Inc.
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <linux/module.h>
@@ -21,9 +22,16 @@ 
 #define GAMMA_LUT_EN					BIT(1)
 #define GAMMA_DITHERING					BIT(2)
 #define DISP_GAMMA_SIZE				0x0030
+#define DISP_GAMMA_SIZE_HSIZE				GENMASK(28, 16)
+#define DISP_GAMMA_SIZE_VSIZE				GENMASK(12, 0)
 #define DISP_GAMMA_LUT				0x0700
 
+#define DISP_GAMMA_LUT_10BIT_R			GENMASK(29, 20)
+#define DISP_GAMMA_LUT_10BIT_G			GENMASK(19, 10)
+#define DISP_GAMMA_LUT_10BIT_B			GENMASK(9, 0)
+
 #define LUT_10BIT_MASK				0x03ff
+#define LUT_BITS_DEFAULT			10
 #define LUT_SIZE_DEFAULT			512
 
 struct mtk_disp_gamma_data {
@@ -96,33 +104,33 @@  void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	for (i = 0; i < lut_size; i++) {
 		struct drm_color_lut diff, hwlut;
 
-		hwlut.red = drm_color_lut_extract(lut[i].red, 10);
-		hwlut.green = drm_color_lut_extract(lut[i].green, 10);
-		hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
+		hwlut.red = drm_color_lut_extract(lut[i].red, LUT_BITS_DEFAULT);
+		hwlut.green = drm_color_lut_extract(lut[i].green, LUT_BITS_DEFAULT);
+		hwlut.blue = drm_color_lut_extract(lut[i].blue, LUT_BITS_DEFAULT);
 
 		if (!lut_diff || (i % 2 == 0)) {
-			word = hwlut.red << 20 +
-			       hwlut.green << 10 +
-			       hwlut.red;
+			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, 10);
+			diff.red = drm_color_lut_extract(diff.red, LUT_BITS_DEFAULT);
 
 			diff.green = lut[i].green - lut[i - 1].green;
-			diff.green = drm_color_lut_extract(diff.green, 10);
+			diff.green = drm_color_lut_extract(diff.green, LUT_BITS_DEFAULT);
 
 			diff.blue = lut[i].blue - lut[i - 1].blue;
-			diff.blue = drm_color_lut_extract(diff.blue, 10);
+			diff.blue = drm_color_lut_extract(diff.blue, LUT_BITS_DEFAULT);
 
-			word = diff.blue << 20 +
-			       diff.green << 10 +
-			       diff.red;
+			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));
 	}
 
 	/* Enable the gamma table */
-	cfg_val = cfg_val | GAMMA_LUT_EN;
+	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
 
 	writel(cfg_val, regs + DISP_GAMMA_CFG);
 }
@@ -139,9 +147,12 @@  void mtk_gamma_config(struct device *dev, unsigned int w,
 		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
 {
 	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
+	u32 sz;
+
+	sz = FIELD_PREP(DISP_GAMMA_SIZE_HSIZE, w);
+	sz |= FIELD_PREP(DISP_GAMMA_SIZE_VSIZE, h);
 
-	mtk_ddp_write(cmdq_pkt, h << 16 | w, &gamma->cmdq_reg, gamma->regs,
-		      DISP_GAMMA_SIZE);
+	mtk_ddp_write(cmdq_pkt, sz, &gamma->cmdq_reg, gamma->regs, DISP_GAMMA_SIZE);
 	if (gamma->data && gamma->data->has_dither)
 		mtk_dither_set_common(gamma->regs, &gamma->cmdq_reg, bpc,
 				      DISP_GAMMA_CFG, GAMMA_DITHERING, cmdq_pkt);