diff mbox series

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

Message ID 20231012095736.100784-7-angelogioacchino.delregno@collabora.com
State New
Headers
Series MediaTek DDP GAMMA - 12-bit LUT support |

Commit Message

AngeloGioacchino Del Regno Oct. 12, 2023, 9:57 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.

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 | 28 +++++++++++++++--------
 1 file changed, 19 insertions(+), 9 deletions(-)

Comments

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

On Thu, 2023-10-12 at 11:57 +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.

When I apply, I would remove the description of LUT_BITS_DEFAULT.

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 | 28 +++++++++++++++----
> ----
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index d35eaf6dbc2d..81c04518a5eb 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,8 +22,14 @@
>  #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)
> +
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
> @@ -97,9 +104,9 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs, struct drm_crt
>  		hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
>  
>  		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);
> @@ -110,9 +117,9 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  			diff.blue = lut[i].blue - lut[i - 1].blue;
>  			diff.blue = drm_color_lut_extract(diff.blue,
> 10);
>  
> -			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));
>  	}
> @@ -120,7 +127,7 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  	cfg_val = readl(regs + DISP_GAMMA_CFG);
>  
>  	/* Enable the gamma table */
> -	cfg_val |= GAMMA_LUT_EN;
> +	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
>  
>  	writel(cfg_val, regs + DISP_GAMMA_CFG);
>  }
> @@ -137,9 +144,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);
AngeloGioacchino Del Regno Oct. 16, 2023, 7:55 a.m. UTC | #2 
Il 13/10/23 06:00, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
> 
> On Thu, 2023-10-12 at 11:57 +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.
> 
> When I apply, I would remove the description of LUT_BITS_DEFAULT.
> 
> Reviewed-by: CK Hu <ck.hu@mediatek.com>

Sorry for forgetting about removing that from the commit description after
removing it from the code.

Thanks for removing it while applying, that simplifies my workflow.

Cheers,
Angelo
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 d35eaf6dbc2d..81c04518a5eb 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,8 +22,14 @@ 
 #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)
+
 struct mtk_disp_gamma_data {
 	bool has_dither;
 	bool lut_diff;
@@ -97,9 +104,9 @@  void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 		hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
 
 		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);
@@ -110,9 +117,9 @@  void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 			diff.blue = lut[i].blue - lut[i - 1].blue;
 			diff.blue = drm_color_lut_extract(diff.blue, 10);
 
-			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));
 	}
@@ -120,7 +127,7 @@  void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	cfg_val = readl(regs + DISP_GAMMA_CFG);
 
 	/* Enable the gamma table */
-	cfg_val |= GAMMA_LUT_EN;
+	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
 
 	writel(cfg_val, regs + DISP_GAMMA_CFG);
 }
@@ -137,9 +144,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);