[PATCH v10 00/16] MediaTek DDP GAMMA - 12-bit LUT support

[PATCH v10 00/16] MediaTek DDP GAMMA - 12-bit LUT support

[AngeloGioacchino Del Regno]

Changes in v10:
 - Moved snippet from patch [7/15] to patch [6/15] as that was
   intended to be there instead; fixes build issue for patch [6/15]
   as pointed out by the kernel text robot (oops, sorry!)

Changes in v9:
 - As per previous conversation with CK Hu, added a commit that
   de-commonizes the gamma setting function that was used in
   both DISP_AAL and DISP_GAMMA, now each of them have their
   own .gamma_set() callback (mtk_disp_gamma_set_common() has
   been removed).
 - Added a change to use bitfield macros in mtk_disp_aal.c
 - Added a change to compress of_device_id entries in mtk_disp_aal.c
 - Tested again on MT6795, MT8173, MT8186, MT8192, MT8195

Changes in v8:
 - Changed lut_size to be a mtk_disp_gamma_set_common() function
   parameter to pass lut size from AAL

Changes in v7:
 - Added check for NULL dev for AAL-gamma case
 - Added get_lut_size callback for AAL-gamma
 - Added comment to clarify SoC 10/12 bits support and old vs new
   register layout as suggested by Alexandre M.

Changes in v6:
 - Fixed smatch warning in patch 11/11, ref.:
   https://lore.kernel.org/all/202306101458.lRXHEE0Z-lkp@intel.com/

Changes in v5:
 - Removed incorrect comment on default LUT size and bits
 - Removed useless check for num_lut_banks
 - Added comment about CMDQ implementation on patch 5
 - Evaluated passing lut size/bits from AAL, idea discarded as
   the implementation would be rather tricky while bringing no
   benefits.

Changes in v4:
 - Fixed assignment typo appeared in v3

Changes in v3:
 - Fixed issues due to variables renaming during cleanup (oops)
 - This is actually the right series, since v2 was taken from the
   wrong kernel tree.... :-)

Changes in v2:
 - Added explicit inclusion of linux/bitfield.h in patch [06/11]

This series adds support for GAMMA IP requiring and/or supporting
a 12-bits LUT using a slightly different register layout and programming
sequence for multiple LUT banks: this IP version is currently found
on a number of SoCs, not only including the Chromebook/IoT oriented
Kompanio 1200/1380 MT8195/MT8195T, but also Smartphone chips such as
the Dimensity 9200 (MT6985) and others.

This series was tested on MT8195, MT8192, MT8173, MT6795:
 * MT6795, MT8192, MT8173: No regression, works fine.
 * MT8195: Color correction is finally working!

AngeloGioacchino Del Regno (15):
  drm/mediatek: gamma: Reduce indentation in mtk_gamma_set_common()
  drm/mediatek: gamma: Support SoC specific LUT size
  drm/mediatek: gamma: Improve and simplify HW LUT calculation
  drm/mediatek: gamma: Enable the Gamma LUT table only after programming
  drm/mediatek: gamma: Use bitfield macros
  drm/mediatek: aal: Use bitfield macros
  drm/mediatek: De-commonize disp_aal/disp_gamma gamma_set functions
  drm/mediatek: gamma: Support specifying number of bits per LUT
    component
  drm/mediatek: gamma: Support multi-bank gamma LUT
  drm/mediatek: gamma: Add support for 12-bit LUT and MT8195
  drm/mediatek: gamma: Make sure relay mode is disabled
  drm/mediatek: gamma: Program gamma LUT type for descending or rising
  drm/mediatek: aal: Rewrite kerneldoc for struct mtk_disp_aal
  drm/mediatek: gamma: Add kerneldoc for struct mtk_disp_gamma
  drm/mediatek: aal: Compress of_device_id entries and add sentinel

Jason-JH.Lin (1):
  drm/mediatek: gamma: Adjust mtk_drm_gamma_set_common parameters

 drivers/gpu/drm/mediatek/mtk_disp_aal.c     |  84 ++++++--
 drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 203 ++++++++++++++++----
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   8 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   2 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   9 +
 7 files changed, 256 insertions(+), 54 deletions(-)

-- 
2.41.0

[PATCH v10 01/16] drm/mediatek: gamma: Adjust mtk_drm_gamma_set_common parameters

[AngeloGioacchino Del Regno]

From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>

Adjust the parameters in mtk_drm_gamma_set_common()
  - add (struct device *dev) to get lut_diff from gamma's driver data
  - remove (bool lut_diff) and use false as default value in the function

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  2 +-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 18 ++++++++++++------
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index cdbec79474d1..2f602f1f1c49 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -66,7 +66,7 @@ void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
 	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
 
 	if (aal->data && aal->data->has_gamma)
-		mtk_gamma_set_common(aal->regs, state, false);
+		mtk_gamma_set_common(NULL, aal->regs, state);
 }
 
 void mtk_aal_start(struct device *dev)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index 2254038519e1..75045932353e 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -54,7 +54,7 @@ void mtk_gamma_config(struct device *dev, unsigned int w,
 		      unsigned int h, unsigned int vrefresh,
 		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
-void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state, bool lut_diff);
+void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state);
 void mtk_gamma_start(struct device *dev);
 void mtk_gamma_stop(struct device *dev);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 7746dceadb20..d42cc0698d83 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -54,14 +54,24 @@ void mtk_gamma_clk_disable(struct device *dev)
 	clk_disable_unprepare(gamma->clk);
 }
 
-void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state, bool lut_diff)
+void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state)
 {
+	struct mtk_disp_gamma *gamma;
 	unsigned int i, reg;
 	struct drm_color_lut *lut;
 	void __iomem *lut_base;
+	bool lut_diff;
 	u32 word;
 	u32 diff[3] = {0};
 
+	/* If we're called from AAL, dev is NULL */
+	gamma = dev ? dev_get_drvdata(dev) : NULL;
+
+	if (gamma && gamma->data)
+		lut_diff = gamma->data->lut_diff;
+	else
+		lut_diff = false;
+
 	if (state->gamma_lut) {
 		reg = readl(regs + DISP_GAMMA_CFG);
 		reg = reg | GAMMA_LUT_EN;
@@ -91,12 +101,8 @@ void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state, bool
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 {
 	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
-	bool lut_diff = false;
-
-	if (gamma->data)
-		lut_diff = gamma->data->lut_diff;
 
-	mtk_gamma_set_common(gamma->regs, state, lut_diff);
+	mtk_gamma_set_common(dev, gamma->regs, state);
 }
 
 void mtk_gamma_config(struct device *dev, unsigned int w,
-- 
2.41.0

[PATCH v10 02/16] drm/mediatek: gamma: Reduce indentation in mtk_gamma_set_common()

[AngeloGioacchino Del Regno]

Invert the check for state->gamma_lut and move it at the beginning
of the function to reduce indentation: this prepares the code for
keeping readability on later additions.

This commit brings no functional changes.

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 | 45 ++++++++++++-----------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index d42cc0698d83..47751864bd5c 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -64,6 +64,10 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	u32 word;
 	u32 diff[3] = {0};
 
+	/* If there's no gamma lut there's nothing to do here. */
+	if (!state->gamma_lut)
+		return;
+
 	/* If we're called from AAL, dev is NULL */
 	gamma = dev ? dev_get_drvdata(dev) : NULL;
 
@@ -72,29 +76,26 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	else
 		lut_diff = false;
 
-	if (state->gamma_lut) {
-		reg = readl(regs + DISP_GAMMA_CFG);
-		reg = reg | GAMMA_LUT_EN;
-		writel(reg, regs + DISP_GAMMA_CFG);
-		lut_base = regs + DISP_GAMMA_LUT;
-		lut = (struct drm_color_lut *)state->gamma_lut->data;
-		for (i = 0; i < MTK_LUT_SIZE; i++) {
-
-			if (!lut_diff || (i % 2 == 0)) {
-				word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
-					(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
-					((lut[i].blue >> 6) & LUT_10BIT_MASK);
-			} else {
-				diff[0] = (lut[i].red >> 6) - (lut[i - 1].red >> 6);
-				diff[1] = (lut[i].green >> 6) - (lut[i - 1].green >> 6);
-				diff[2] = (lut[i].blue >> 6) - (lut[i - 1].blue >> 6);
-
-				word = ((diff[0] & LUT_10BIT_MASK) << 20) +
-					((diff[1] & LUT_10BIT_MASK) << 10) +
-					(diff[2] & LUT_10BIT_MASK);
-			}
-			writel(word, (lut_base + i * 4));
+	reg = readl(regs + DISP_GAMMA_CFG);
+	reg = reg | GAMMA_LUT_EN;
+	writel(reg, regs + DISP_GAMMA_CFG);
+	lut_base = regs + DISP_GAMMA_LUT;
+	lut = (struct drm_color_lut *)state->gamma_lut->data;
+	for (i = 0; i < MTK_LUT_SIZE; i++) {
+		if (!lut_diff || (i % 2 == 0)) {
+			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
+				(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
+				((lut[i].blue >> 6) & LUT_10BIT_MASK);
+		} else {
+			diff[0] = (lut[i].red >> 6) - (lut[i - 1].red >> 6);
+			diff[1] = (lut[i].green >> 6) - (lut[i - 1].green >> 6);
+			diff[2] = (lut[i].blue >> 6) - (lut[i - 1].blue >> 6);
+
+			word = ((diff[0] & LUT_10BIT_MASK) << 20) +
+				((diff[1] & LUT_10BIT_MASK) << 10) +
+				(diff[2] & LUT_10BIT_MASK);
 		}
+		writel(word, (lut_base + i * 4));
 	}
 }
 
-- 
2.41.0

[PATCH v10 03/16] drm/mediatek: gamma: Support SoC specific LUT size

[AngeloGioacchino Del Regno]

Newer SoCs support a bigger Gamma LUT table: wire up a callback
to retrieve the correct LUT size for each different Gamma IP.

Co-developed-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
[Angelo: Rewritten commit message/description + porting]
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_aal.c     | 17 ++++++++++++++-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  2 ++
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 23 ++++++++++++++++++---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  8 +++++--
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |  1 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  2 ++
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  9 ++++++++
 7 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 2f602f1f1c49..e2e4155faf01 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -19,7 +19,7 @@
 #define AAL_EN						BIT(0)
 #define DISP_AAL_SIZE				0x0030
 #define DISP_AAL_OUTPUT_SIZE			0x04d8
-
+#define DISP_AAL_LUT_SIZE			512
 
 struct mtk_disp_aal_data {
 	bool has_gamma;
@@ -61,6 +61,21 @@ void mtk_aal_config(struct device *dev, unsigned int w,
 	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_OUTPUT_SIZE);
 }
 
+/**
+ * mtk_aal_gamma_get_lut_size() - Get gamma LUT size for AAL
+ * @dev: Pointer to struct device
+ *
+ * Return: 0 if gamma control not supported in AAL or gamma LUT size
+ */
+unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
+{
+	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+	if (aal->data && aal->data->has_gamma)
+		return DISP_AAL_LUT_SIZE;
+	return 0;
+}
+
 void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
 {
 	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index 75045932353e..ca377265e5eb 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -17,6 +17,7 @@ void mtk_aal_clk_disable(struct device *dev);
 void mtk_aal_config(struct device *dev, unsigned int w,
 		    unsigned int h, unsigned int vrefresh,
 		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
+unsigned int mtk_aal_gamma_get_lut_size(struct device *dev);
 void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
 void mtk_aal_start(struct device *dev);
 void mtk_aal_stop(struct device *dev);
@@ -53,6 +54,7 @@ void mtk_gamma_clk_disable(struct device *dev);
 void mtk_gamma_config(struct device *dev, unsigned int w,
 		      unsigned int h, unsigned int vrefresh,
 		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
+unsigned int mtk_gamma_get_lut_size(struct device *dev);
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
 void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state);
 void mtk_gamma_start(struct device *dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 47751864bd5c..7575237625d2 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -24,10 +24,12 @@
 #define DISP_GAMMA_LUT				0x0700
 
 #define LUT_10BIT_MASK				0x03ff
+#define LUT_SIZE_DEFAULT			512
 
 struct mtk_disp_gamma_data {
 	bool has_dither;
 	bool lut_diff;
+	u16 lut_size;
 };
 
 /*
@@ -54,6 +56,15 @@ void mtk_gamma_clk_disable(struct device *dev)
 	clk_disable_unprepare(gamma->clk);
 }
 
+unsigned int mtk_gamma_get_lut_size(struct device *dev)
+{
+	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
+
+	if (gamma && gamma->data)
+		return gamma->data->lut_size;
+	return LUT_SIZE_DEFAULT;
+}
+
 void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state)
 {
 	struct mtk_disp_gamma *gamma;
@@ -61,6 +72,7 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	struct drm_color_lut *lut;
 	void __iomem *lut_base;
 	bool lut_diff;
+	u16 lut_size;
 	u32 word;
 	u32 diff[3] = {0};
 
@@ -71,17 +83,20 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	/* If we're called from AAL, dev is NULL */
 	gamma = dev ? dev_get_drvdata(dev) : NULL;
 
-	if (gamma && gamma->data)
+	if (gamma && gamma->data) {
 		lut_diff = gamma->data->lut_diff;
-	else
+		lut_size = gamma->data->lut_size;
+	} else {
 		lut_diff = false;
+		lut_size = LUT_SIZE_DEFAULT;
+	}
 
 	reg = readl(regs + DISP_GAMMA_CFG);
 	reg = reg | GAMMA_LUT_EN;
 	writel(reg, regs + DISP_GAMMA_CFG);
 	lut_base = regs + DISP_GAMMA_LUT;
 	lut = (struct drm_color_lut *)state->gamma_lut->data;
-	for (i = 0; i < MTK_LUT_SIZE; i++) {
+	for (i = 0; i < lut_size; i++) {
 		if (!lut_diff || (i % 2 == 0)) {
 			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
 				(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
@@ -198,10 +213,12 @@ 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_size = 512,
 };
 
 static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
 	.lut_diff = true,
+	.lut_size = 512,
 };
 
 static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 8a43656ecc30..ebe0cc3a1a4c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -959,8 +959,12 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
 		mtk_crtc->ddp_comp[i] = comp;
 
 		if (comp->funcs) {
-			if (comp->funcs->gamma_set)
-				gamma_lut_size = MTK_LUT_SIZE;
+			if (comp->funcs->gamma_set && comp->funcs->gamma_get_lut_size) {
+				unsigned int lut_sz = mtk_ddp_gamma_get_lut_size(comp);
+
+				if (lut_sz)
+					gamma_lut_size = lut_sz;
+			}
 
 			if (comp->funcs->ctm_set)
 				has_ctm = true;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
index 3e9046993d09..b2e50292e57d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
@@ -10,7 +10,6 @@
 #include "mtk_drm_ddp_comp.h"
 #include "mtk_drm_plane.h"
 
-#define MTK_LUT_SIZE	512
 #define MTK_MAX_BPC	10
 #define MTK_MIN_BPC	3
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index f114da4d36a9..f3212e08f2cd 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -271,6 +271,7 @@ static void mtk_ufoe_start(struct device *dev)
 static const struct mtk_ddp_comp_funcs ddp_aal = {
 	.clk_enable = mtk_aal_clk_enable,
 	.clk_disable = mtk_aal_clk_disable,
+	.gamma_get_lut_size = mtk_aal_gamma_get_lut_size,
 	.gamma_set = mtk_aal_gamma_set,
 	.config = mtk_aal_config,
 	.start = mtk_aal_start,
@@ -322,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_dsi = {
 static const struct mtk_ddp_comp_funcs ddp_gamma = {
 	.clk_enable = mtk_gamma_clk_enable,
 	.clk_disable = mtk_gamma_clk_disable,
+	.gamma_get_lut_size = mtk_gamma_get_lut_size,
 	.gamma_set = mtk_gamma_set,
 	.config = mtk_gamma_config,
 	.start = mtk_gamma_start,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index febcaeef16a1..c1355960e195 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -67,6 +67,7 @@ struct mtk_ddp_comp_funcs {
 	void (*layer_config)(struct device *dev, unsigned int idx,
 			     struct mtk_plane_state *state,
 			     struct cmdq_pkt *cmdq_pkt);
+	unsigned int (*gamma_get_lut_size)(struct device *dev);
 	void (*gamma_set)(struct device *dev,
 			  struct drm_crtc_state *state);
 	void (*bgclr_in_on)(struct device *dev);
@@ -186,6 +187,14 @@ static inline void mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
 		comp->funcs->layer_config(comp->dev, idx, state, cmdq_pkt);
 }
 
+static inline unsigned int mtk_ddp_gamma_get_lut_size(struct mtk_ddp_comp *comp)
+{
+	if (comp->funcs && comp->funcs->gamma_get_lut_size)
+		return comp->funcs->gamma_get_lut_size(comp->dev);
+
+	return 0;
+}
+
 static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
 				     struct drm_crtc_state *state)
 {
-- 
2.41.0

[PATCH v10 04/16] drm/mediatek: gamma: Improve and simplify HW LUT calculation

[AngeloGioacchino Del Regno]

Use drm_color_lut_extract() to avoid open-coding the bits reduction
calculations for each color channel and use a struct drm_color_lut
to temporarily store the information instead of an array of u32.

Also, slightly improve the precision of the HW LUT calculation in the
LUT DIFF case by performing the subtractions on the 16-bits values and
doing the 10 bits conversion later.

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 | 30 +++++++++++++++--------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 7575237625d2..fd6a75a64a9f 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -74,7 +74,6 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	bool lut_diff;
 	u16 lut_size;
 	u32 word;
-	u32 diff[3] = {0};
 
 	/* If there's no gamma lut there's nothing to do here. */
 	if (!state->gamma_lut)
@@ -97,18 +96,29 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	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, 10);
+		hwlut.green = drm_color_lut_extract(lut[i].green, 10);
+		hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
+
 		if (!lut_diff || (i % 2 == 0)) {
-			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
-				(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
-				((lut[i].blue >> 6) & LUT_10BIT_MASK);
+			word = hwlut.red << 20 +
+			       hwlut.green << 10 +
+			       hwlut.red;
 		} else {
-			diff[0] = (lut[i].red >> 6) - (lut[i - 1].red >> 6);
-			diff[1] = (lut[i].green >> 6) - (lut[i - 1].green >> 6);
-			diff[2] = (lut[i].blue >> 6) - (lut[i - 1].blue >> 6);
+			diff.red = lut[i].red - lut[i - 1].red;
+			diff.red = drm_color_lut_extract(diff.red, 10);
+
+			diff.green = lut[i].green - lut[i - 1].green;
+			diff.green = drm_color_lut_extract(diff.green, 10);
+
+			diff.blue = lut[i].blue - lut[i - 1].blue;
+			diff.blue = drm_color_lut_extract(diff.blue, 10);
 
-			word = ((diff[0] & LUT_10BIT_MASK) << 20) +
-				((diff[1] & LUT_10BIT_MASK) << 10) +
-				(diff[2] & LUT_10BIT_MASK);
+			word = diff.blue << 20 +
+			       diff.green << 10 +
+			       diff.red;
 		}
 		writel(word, (lut_base + i * 4));
 	}
-- 
2.41.0

[PATCH v10 05/16] drm/mediatek: gamma: Enable the Gamma LUT table only after programming

[AngeloGioacchino Del Regno]

Move the write to DISP_GAMMA_CFG to enable the Gamma LUT to after
programming the actual table to avoid potential visual glitches during
table modification.

Note:
GAMMA should get enabled in between vblanks, but this requires many
efforts in order to make this happen, as that requires migrating all
of the writes to make use of CMDQ instead of cpu writes and that's
not trivial. For this reason, this patch only moves the LUT enable.
The CMDQ rework will come at a later time.

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 | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index fd6a75a64a9f..18b102bef370 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -68,12 +68,12 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
 void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state)
 {
 	struct mtk_disp_gamma *gamma;
-	unsigned int i, reg;
+	unsigned int i;
 	struct drm_color_lut *lut;
 	void __iomem *lut_base;
 	bool lut_diff;
 	u16 lut_size;
-	u32 word;
+	u32 cfg_val, word;
 
 	/* If there's no gamma lut there's nothing to do here. */
 	if (!state->gamma_lut)
@@ -90,9 +90,7 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 		lut_size = LUT_SIZE_DEFAULT;
 	}
 
-	reg = readl(regs + DISP_GAMMA_CFG);
-	reg = reg | GAMMA_LUT_EN;
-	writel(reg, regs + DISP_GAMMA_CFG);
+	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++) {
@@ -122,6 +120,11 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 		}
 		writel(word, (lut_base + i * 4));
 	}
+
+	/* Enable the gamma table */
+	cfg_val = cfg_val | GAMMA_LUT_EN;
+
+	writel(cfg_val, regs + DISP_GAMMA_CFG);
 }
 
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
-- 
2.41.0

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

[AngeloGioacchino Del Regno]

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);
+		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);
-- 
2.41.0

[PATCH v10 07/16] drm/mediatek: aal: Use bitfield macros

[AngeloGioacchino Del Regno]

Make the code more robust and improve readability by using bitfield
macros instead of open coding bit operations.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index e2e4155faf01..bec035780db0 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -18,6 +18,8 @@
 #define DISP_AAL_EN				0x0000
 #define AAL_EN						BIT(0)
 #define DISP_AAL_SIZE				0x0030
+#define DISP_AAL_SIZE_HSIZE				GENMASK(28, 16)
+#define DISP_AAL_SIZE_VSIZE				GENMASK(12, 0)
 #define DISP_AAL_OUTPUT_SIZE			0x04d8
 #define DISP_AAL_LUT_SIZE			512
 
@@ -56,9 +58,13 @@ void mtk_aal_config(struct device *dev, unsigned int w,
 			   unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
 {
 	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+	u32 sz;
 
-	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
-	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_OUTPUT_SIZE);
+	sz = FIELD_PREP(DISP_AAL_SIZE_HSIZE, w);
+	sz |= FIELD_PREP(DISP_AAL_SIZE_VSIZE, h);
+
+	mtk_ddp_write(cmdq_pkt, sz, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
+	mtk_ddp_write(cmdq_pkt, sz, &aal->cmdq_reg, aal->regs, DISP_AAL_OUTPUT_SIZE);
 }
 
 /**
-- 
2.41.0

[PATCH v10 08/16] drm/mediatek: De-commonize disp_aal/disp_gamma gamma_set functions

[AngeloGioacchino Del Regno]

In preparation for adding a 12-bits gamma support for the DISP_GAMMA
IP, remove the mtk_gamma_set_common() function and move the relevant
bits in mtk_gamma_set() for DISP_GAMMA and mtk_aal_gamma_set() for
DISP_AAL: since the latter has no more support for gamma manipulation
(being moved to a different IP) in newer revisions, those functions
are about to diverge and it makes no sense to keep a common one (with
all the complications of passing common data and making exclusions
for device driver data) for just a few bits.

This commit brings no functional changes.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c   | 39 +++++++++++++++++++++--
 drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  1 -
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 ++++----------------
 3 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index bec035780db0..21b25470e9b7 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -17,10 +17,18 @@
 
 #define DISP_AAL_EN				0x0000
 #define AAL_EN						BIT(0)
+#define DISP_AAL_CFG				0x0020
+#define AAL_RELAY_MODE					BIT(0)
+#define AAL_GAMMA_LUT_EN				BIT(1)
 #define DISP_AAL_SIZE				0x0030
 #define DISP_AAL_SIZE_HSIZE				GENMASK(28, 16)
 #define DISP_AAL_SIZE_VSIZE				GENMASK(12, 0)
 #define DISP_AAL_OUTPUT_SIZE			0x04d8
+#define DISP_AAL_GAMMA_LUT			0x0700
+#define DISP_AAL_GAMMA_LUT_R				GENMASK(29, 20)
+#define DISP_AAL_GAMMA_LUT_G				GENMASK(19, 10)
+#define DISP_AAL_GAMMA_LUT_B				GENMASK(9, 0)
+#define DISP_AAL_LUT_BITS			10
 #define DISP_AAL_LUT_SIZE			512
 
 struct mtk_disp_aal_data {
@@ -85,9 +93,36 @@ unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
 void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
 {
 	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+	struct drm_color_lut *lut;
+	unsigned int i;
+	u32 cfg_val;
+
+	/* If gamma is not supported in AAL, go out immediately */
+	if (!(aal->data && aal->data->has_gamma))
+		return;
+
+	/* Also, if there's no gamma lut there's nothing to do here. */
+	if (!state->gamma_lut)
+		return;
+
+	cfg_val = readl(aal->regs + DISP_AAL_CFG);
+	lut = (struct drm_color_lut *)state->gamma_lut->data;
+
+	for (i = 0; i < DISP_AAL_LUT_SIZE; i++) {
+		struct drm_color_lut hwlut = {
+			.red = drm_color_lut_extract(lut[i].red, DISP_AAL_LUT_BITS),
+			.green = drm_color_lut_extract(lut[i].green, DISP_AAL_LUT_BITS),
+			.blue = drm_color_lut_extract(lut[i].blue, DISP_AAL_LUT_BITS)
+		};
+		u32 word;
+
+		word = FIELD_PREP(DISP_AAL_GAMMA_LUT_R, hwlut.red);
+		word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_G, hwlut.green);
+		word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_B, hwlut.blue);
+		writel(word, (aal->regs + DISP_AAL_GAMMA_LUT) + (i * 4));
+	}
 
-	if (aal->data && aal->data->has_gamma)
-		mtk_gamma_set_common(NULL, aal->regs, state);
+	writel(cfg_val, aal->regs + DISP_AAL_CFG);
 }
 
 void mtk_aal_start(struct device *dev)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index ca377265e5eb..54d3712e2afd 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -56,7 +56,6 @@ void mtk_gamma_config(struct device *dev, unsigned int w,
 		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
 unsigned int mtk_gamma_get_lut_size(struct device *dev);
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
-void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state);
 void mtk_gamma_start(struct device *dev);
 void mtk_gamma_stop(struct device *dev);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index ea91d3619716..001b98694761 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -73,42 +73,29 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
 	return LUT_SIZE_DEFAULT;
 }
 
-void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state)
+void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 {
-	struct mtk_disp_gamma *gamma;
+	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
 	unsigned int i;
 	struct drm_color_lut *lut;
 	void __iomem *lut_base;
-	bool lut_diff;
-	u16 lut_size;
 	u32 cfg_val, word;
 
 	/* If there's no gamma lut there's nothing to do here. */
 	if (!state->gamma_lut)
 		return;
 
-	/* If we're called from AAL, dev is NULL */
-	gamma = dev ? dev_get_drvdata(dev) : NULL;
-
-	if (gamma && gamma->data) {
-		lut_diff = gamma->data->lut_diff;
-		lut_size = gamma->data->lut_size;
-	} else {
-		lut_diff = false;
-		lut_size = LUT_SIZE_DEFAULT;
-	}
-
-	cfg_val = readl(regs + DISP_GAMMA_CFG);
-	lut_base = regs + DISP_GAMMA_LUT;
+	cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
+	lut_base = gamma->regs + DISP_GAMMA_LUT;
 	lut = (struct drm_color_lut *)state->gamma_lut->data;
-	for (i = 0; i < lut_size; i++) {
+	for (i = 0; i < gamma->data->lut_size; i++) {
 		struct drm_color_lut diff, hwlut;
 
 		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)) {
+		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);
@@ -132,14 +119,7 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
 	/* Enable the gamma table */
 	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
 
-	writel(cfg_val, regs + DISP_GAMMA_CFG);
-}
-
-void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
-{
-	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
-
-	mtk_gamma_set_common(dev, gamma->regs, state);
+	writel(cfg_val, gamma->regs + DISP_GAMMA_CFG);
 }
 
 void mtk_gamma_config(struct device *dev, unsigned int w,
-- 
2.41.0

[PATCH v10 09/16] drm/mediatek: gamma: Support specifying number of bits per LUT component

[AngeloGioacchino Del Regno]

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 calculation.

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

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 001b98694761..1845bd326a6d 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -38,6 +38,7 @@ struct mtk_disp_gamma_data {
 	bool has_dither;
 	bool lut_diff;
 	u16 lut_size;
+	u8 lut_bits;
 };
 
 /*
@@ -91,9 +92,9 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 	for (i = 0; i < gamma->data->lut_size; i++) {
 		struct drm_color_lut diff, hwlut;
 
-		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);
+		hwlut.red = drm_color_lut_extract(lut[i].red, gamma->data->lut_bits);
+		hwlut.green = drm_color_lut_extract(lut[i].green, gamma->data->lut_bits);
+		hwlut.blue = drm_color_lut_extract(lut[i].blue, gamma->data->lut_bits);
 
 		if (!gamma->data->lut_diff || (i % 2 == 0)) {
 			word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
@@ -101,13 +102,13 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 			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_DEFAULT);
+			diff.red = drm_color_lut_extract(diff.red, gamma->data->lut_bits);
 
 			diff.green = lut[i].green - lut[i - 1].green;
-			diff.green = drm_color_lut_extract(diff.green, LUT_BITS_DEFAULT);
+			diff.green = drm_color_lut_extract(diff.green, gamma->data->lut_bits);
 
 			diff.blue = lut[i].blue - lut[i - 1].blue;
-			diff.blue = drm_color_lut_extract(diff.blue, LUT_BITS_DEFAULT);
+			diff.blue = drm_color_lut_extract(diff.blue, gamma->data->lut_bits);
 
 			word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
 			word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
@@ -217,10 +218,12 @@ 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_bits = 10,
 	.lut_size = 512,
 };
 
 static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
+	.lut_bits = 10,
 	.lut_diff = true,
 	.lut_size = 512,
 };
-- 
2.41.0

[PATCH v10 10/16] drm/mediatek: gamma: Support multi-bank gamma LUT

[AngeloGioacchino Del Regno]

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@mediatek.com>
[Angelo: Refactored original commit]
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 70 ++++++++++++++---------
 1 file changed, 44 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 1845bd326a6d..3f1c6815ea5a 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)
@@ -37,6 +39,7 @@
 struct mtk_disp_gamma_data {
 	bool has_dither;
 	bool lut_diff;
+	u16 lut_bank_size;
 	u16 lut_size;
 	u8 lut_bits;
 };
@@ -80,41 +83,54 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 	unsigned int i;
 	struct drm_color_lut *lut;
 	void __iomem *lut_base;
-	u32 cfg_val, word;
+	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)
 		return;
 
+	num_lut_banks = gamma->data->lut_size / gamma->data->lut_bank_size;
 	cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
 	lut_base = gamma->regs + DISP_GAMMA_LUT;
 	lut = (struct drm_color_lut *)state->gamma_lut->data;
-	for (i = 0; i < gamma->data->lut_size; i++) {
-		struct drm_color_lut diff, hwlut;
-
-		hwlut.red = drm_color_lut_extract(lut[i].red, gamma->data->lut_bits);
-		hwlut.green = drm_color_lut_extract(lut[i].green, gamma->data->lut_bits);
-		hwlut.blue = drm_color_lut_extract(lut[i].blue, gamma->data->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);
-		} else {
-			diff.red = lut[i].red - lut[i - 1].red;
-			diff.red = drm_color_lut_extract(diff.red, gamma->data->lut_bits);
-
-			diff.green = lut[i].green - lut[i - 1].green;
-			diff.green = drm_color_lut_extract(diff.green, gamma->data->lut_bits);
-
-			diff.blue = lut[i].blue - lut[i - 1].blue;
-			diff.blue = drm_color_lut_extract(diff.blue, gamma->data->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, gamma->regs + DISP_GAMMA_BANK);
+		}
+
+		for (i = 0; i < gamma->data->lut_bank_size; i++) {
+			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, gamma->data->lut_bits);
+			hwlut.green = drm_color_lut_extract(lut[n].green, gamma->data->lut_bits);
+			hwlut.blue = drm_color_lut_extract(lut[n].blue, gamma->data->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);
+			} else {
+				diff.red = lut[n].red - lut[n - 1].red;
+				diff.red = drm_color_lut_extract(diff.red, gamma->data->lut_bits);
+
+				diff.green = lut[n].green - lut[n - 1].green;
+				diff.green = drm_color_lut_extract(diff.green, gamma->data->lut_bits);
+
+				diff.blue = lut[n].blue - lut[n - 1].blue;
+				diff.blue = drm_color_lut_extract(diff.blue, gamma->data->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 */
@@ -218,11 +234,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,
-- 
2.41.0

[PATCH v10 11/16] drm/mediatek: gamma: Add support for 12-bit LUT and MT8195

[AngeloGioacchino Del Regno]

Add support for 12-bit gamma lookup tables and introduce the first
user for it: MT8195.
While at it, also reorder the variables in mtk_gamma_set_common()
and rename `lut_base` to `lut0_base` to improve readability.

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 | 74 +++++++++++++++++++----
 1 file changed, 62 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 3f1c6815ea5a..7d2f8042ace0 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -26,12 +26,20 @@
 #define DISP_GAMMA_SIZE_VSIZE				GENMASK(12, 0)
 #define DISP_GAMMA_BANK				0x0100
 #define DISP_GAMMA_BANK_BANK				GENMASK(1, 0)
+#define DISP_GAMMA_BANK_DATA_MODE			BIT(2)
 #define DISP_GAMMA_LUT				0x0700
+#define DISP_GAMMA_LUT1				0x0b00
 
+/* For 10 bit LUT layout, R/G/B are in the same register */
 #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)
 
+/* For 12 bit LUT layout, R/G are in LUT, B is in LUT1 */
+#define DISP_GAMMA_LUT_12BIT_R			GENMASK(11, 0)
+#define DISP_GAMMA_LUT_12BIT_G			GENMASK(23, 12)
+#define DISP_GAMMA_LUT_12BIT_B			GENMASK(11, 0)
+
 #define LUT_10BIT_MASK				0x03ff
 #define LUT_BITS_DEFAULT			10
 #define LUT_SIZE_DEFAULT			512
@@ -77,14 +85,30 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
 	return LUT_SIZE_DEFAULT;
 }
 
+/*
+ * 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];
 	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)
@@ -92,14 +116,17 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 
 	num_lut_banks = gamma->data->lut_size / gamma->data->lut_bank_size;
 	cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
-	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, !!(gamma->data->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);
 		}
 
@@ -112,9 +139,15 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 			hwlut.blue = drm_color_lut_extract(lut[n].blue, gamma->data->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 (gamma->data->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, gamma->data->lut_bits);
@@ -125,11 +158,19 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 				diff.blue = lut[n].blue - lut[n - 1].blue;
 				diff.blue = drm_color_lut_extract(diff.blue, gamma->data->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);
+				if (gamma->data->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 (gamma->data->lut_bits == 12)
+				writel(word[1], (lut1_base + i * 4));
 		}
 	}
 
@@ -246,11 +287,20 @@ static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
 	.lut_size = 512,
 };
 
+static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
+	.lut_bank_size = 256,
+	.lut_bits = 12,
+	.lut_diff = true,
+	.lut_size = 1024,
+};
+
 static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
 	{ .compatible = "mediatek,mt8173-disp-gamma",
 	  .data = &mt8173_gamma_driver_data},
 	{ .compatible = "mediatek,mt8183-disp-gamma",
 	  .data = &mt8183_gamma_driver_data},
+	{ .compatible = "mediatek,mt8195-disp-gamma",
+	  .data = &mt8195_gamma_driver_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
-- 
2.41.0

[PATCH v10 12/16] drm/mediatek: gamma: Make sure relay mode is disabled

[AngeloGioacchino Del Regno]

Disable relay mode at the end of LUT programming to make sure that the
processed image goes through in both DISP_GAMMA and DISP_AAL for gamma
setting.

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_aal.c   | 3 +++
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 21b25470e9b7..992dc1424c91 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -122,6 +122,9 @@ void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
 		writel(word, (aal->regs + DISP_AAL_GAMMA_LUT) + (i * 4));
 	}
 
+	/* Disable RELAY mode to pass the processed image */
+	cfg_val &= ~AAL_RELAY_MODE;
+
 	writel(cfg_val, aal->regs + DISP_AAL_CFG);
 }
 
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 7d2f8042ace0..fbff9f97b737 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -19,6 +19,7 @@
 #define DISP_GAMMA_EN				0x0000
 #define GAMMA_EN					BIT(0)
 #define DISP_GAMMA_CFG				0x0020
+#define GAMMA_RELAY_MODE				BIT(0)
 #define GAMMA_LUT_EN					BIT(1)
 #define GAMMA_DITHERING					BIT(2)
 #define DISP_GAMMA_SIZE				0x0030
@@ -177,6 +178,9 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 	/* Enable the gamma table */
 	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
 
+	/* Disable RELAY mode to pass the processed image */
+	cfg_val &= ~GAMMA_RELAY_MODE;
+
 	writel(cfg_val, gamma->regs + DISP_GAMMA_CFG);
 }
 
-- 
2.41.0

[PATCH v10 13/16] drm/mediatek: gamma: Program gamma LUT type for descending or rising

[AngeloGioacchino Del Regno]

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_MODE				BIT(0)
 #define GAMMA_LUT_EN					BIT(1)
 #define GAMMA_DITHERING					BIT(2)
+#define GAMMA_LUT_TYPE					BIT(2)
 #define DISP_GAMMA_SIZE				0x0030
 #define DISP_GAMMA_SIZE_HSIZE				GENMASK(28, 16)
 #define DISP_GAMMA_SIZE_VSIZE				GENMASK(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);
+		else
+			cfg_val &= ~GAMMA_LUT_TYPE;
+	}
+
 	/* Enable the gamma table */
 	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
 
-- 
2.41.0

[PATCH v10 14/16] drm/mediatek: aal: Rewrite kerneldoc for struct mtk_disp_aal

[AngeloGioacchino Del Regno]

The kerneldoc for struct mtk_disp_aal was entirely wrong: rewrite it
to actually document the structure.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 992dc1424c91..e6ab3eaa1126 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -36,9 +36,11 @@ struct mtk_disp_aal_data {
 };
 
 /**
- * struct mtk_disp_aal - DISP_AAL driver structure
- * @ddp_comp - structure containing type enum and hardware resources
- * @crtc - associated crtc to report irq events to
+ * struct mtk_disp_aal - Display Adaptive Ambient Light driver structure
+ * @clk:      clock for DISP_AAL controller
+ * @regs:     MMIO registers base
+ * @cmdq_reg: CMDQ Client register
+ * @data:     platform specific data for DISP_AAL
  */
 struct mtk_disp_aal {
 	struct clk *clk;
-- 
2.41.0

[PATCH v10 15/16] drm/mediatek: gamma: Add kerneldoc for struct mtk_disp_gamma

[AngeloGioacchino Del Regno]

The mtk_disp_gamma structure was completely undocumented: add some
kerneldoc documentation to it.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index d9a70238d524..867445a2af9e 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -54,8 +54,12 @@ struct mtk_disp_gamma_data {
 	u8 lut_bits;
 };
 
-/*
- * struct mtk_disp_gamma - DISP_GAMMA driver structure
+/**
+ * struct mtk_disp_gamma - Display Gamma driver structure
+ * @clk:      clock for DISP_GAMMA block
+ * @regs:     MMIO registers base
+ * @cmdq_reg: CMDQ Client register
+ * @data:     platform data for DISP_GAMMA
  */
 struct mtk_disp_gamma {
 	struct clk *clk;
-- 
2.41.0

[PATCH v10 16/16] drm/mediatek: aal: Compress of_device_id entries and add sentinel

[AngeloGioacchino Del Regno]

Compress the entry for mediatek,mt8173-disp-aal, as it fits in one
line, and fix the style; while at it, also add the usual sentinel
comment to the last entry.

This commit brings no functional changes.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index e6ab3eaa1126..70de5f3007e4 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -212,10 +212,9 @@ static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
 };
 
 static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-aal",
-	  .data = &mt8173_aal_driver_data},
-	{ .compatible = "mediatek,mt8183-disp-aal"},
-	{},
+	{ .compatible = "mediatek,mt8173-disp-aal", .data = &mt8173_aal_driver_data },
+	{ .compatible = "mediatek,mt8183-disp-aal" },
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_aal_driver_dt_match);
 
-- 
2.41.0

Re: [PATCH v10 11/16] drm/mediatek: gamma: Add support for 12-bit LUT and MT8195

[Chen-Yu Tsai]

On Fri, Aug 4, 2023 at 3:29 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Add support for 12-bit gamma lookup tables and introduce the first
> user for it: MT8195.
> While at it, also reorder the variables in mtk_gamma_set_common()
> and rename `lut_base` to `lut0_base` to improve readability.
>
> 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 | 74 +++++++++++++++++++----
>  1 file changed, 62 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 3f1c6815ea5a..7d2f8042ace0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -26,12 +26,20 @@
>  #define DISP_GAMMA_SIZE_VSIZE                          GENMASK(12, 0)
>  #define DISP_GAMMA_BANK                                0x0100
>  #define DISP_GAMMA_BANK_BANK                           GENMASK(1, 0)
> +#define DISP_GAMMA_BANK_DATA_MODE                      BIT(2)
>  #define DISP_GAMMA_LUT                         0x0700
> +#define DISP_GAMMA_LUT1                                0x0b00
>
> +/* For 10 bit LUT layout, R/G/B are in the same register */
>  #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)
>
> +/* For 12 bit LUT layout, R/G are in LUT, B is in LUT1 */
> +#define DISP_GAMMA_LUT_12BIT_R                 GENMASK(11, 0)
> +#define DISP_GAMMA_LUT_12BIT_G                 GENMASK(23, 12)
> +#define DISP_GAMMA_LUT_12BIT_B                 GENMASK(11, 0)
> +
>  #define LUT_10BIT_MASK                         0x03ff
>  #define LUT_BITS_DEFAULT                       10
>  #define LUT_SIZE_DEFAULT                       512
> @@ -77,14 +85,30 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
>         return LUT_SIZE_DEFAULT;
>  }
>
> +/*
> + * 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];
>         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)
> @@ -92,14 +116,17 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>
>         num_lut_banks = gamma->data->lut_size / gamma->data->lut_bank_size;
>         cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
> -       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, !!(gamma->data->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);
>                 }
>
> @@ -112,9 +139,15 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>                         hwlut.blue = drm_color_lut_extract(lut[n].blue, gamma->data->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 (gamma->data->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, gamma->data->lut_bits);
> @@ -125,11 +158,19 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>                                 diff.blue = lut[n].blue - lut[n - 1].blue;
>                                 diff.blue = drm_color_lut_extract(diff.blue, gamma->data->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);
> +                               if (gamma->data->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 (gamma->data->lut_bits == 12)
> +                               writel(word[1], (lut1_base + i * 4));
>                 }
>         }
>
> @@ -246,11 +287,20 @@ static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
>         .lut_size = 512,
>  };
>
> +static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
> +       .lut_bank_size = 256,
> +       .lut_bits = 12,
> +       .lut_diff = true,
> +       .lut_size = 1024,
> +};
> +
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
>         { .compatible = "mediatek,mt8173-disp-gamma",
>           .data = &mt8173_gamma_driver_data},
>         { .compatible = "mediatek,mt8183-disp-gamma",
>           .data = &mt8183_gamma_driver_data},
> +       { .compatible = "mediatek,mt8195-disp-gamma",
> +         .data = &mt8195_gamma_driver_data},

I assume we need matching changes for the DT binding and to the DT?

ChenYu

>         {},
>  };
>  MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
> --
> 2.41.0
>

Re: [PATCH v10 11/16] drm/mediatek: gamma: Add support for 12-bit LUT and MT8195

[AngeloGioacchino Del Regno]

Il 15/08/23 09:57, Chen-Yu Tsai ha scritto:
> On Fri, Aug 4, 2023 at 3:29 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@collabora.com> wrote:
>>
>> Add support for 12-bit gamma lookup tables and introduce the first
>> user for it: MT8195.
>> While at it, also reorder the variables in mtk_gamma_set_common()
>> and rename `lut_base` to `lut0_base` to improve readability.
>>
>> 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 | 74 +++++++++++++++++++----
>>   1 file changed, 62 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> index 3f1c6815ea5a..7d2f8042ace0 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> @@ -26,12 +26,20 @@
>>   #define DISP_GAMMA_SIZE_VSIZE                          GENMASK(12, 0)
>>   #define DISP_GAMMA_BANK                                0x0100
>>   #define DISP_GAMMA_BANK_BANK                           GENMASK(1, 0)
>> +#define DISP_GAMMA_BANK_DATA_MODE                      BIT(2)
>>   #define DISP_GAMMA_LUT                         0x0700
>> +#define DISP_GAMMA_LUT1                                0x0b00
>>
>> +/* For 10 bit LUT layout, R/G/B are in the same register */
>>   #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)
>>
>> +/* For 12 bit LUT layout, R/G are in LUT, B is in LUT1 */
>> +#define DISP_GAMMA_LUT_12BIT_R                 GENMASK(11, 0)
>> +#define DISP_GAMMA_LUT_12BIT_G                 GENMASK(23, 12)
>> +#define DISP_GAMMA_LUT_12BIT_B                 GENMASK(11, 0)
>> +
>>   #define LUT_10BIT_MASK                         0x03ff
>>   #define LUT_BITS_DEFAULT                       10
>>   #define LUT_SIZE_DEFAULT                       512
>> @@ -77,14 +85,30 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
>>          return LUT_SIZE_DEFAULT;
>>   }
>>
>> +/*
>> + * 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];
>>          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)
>> @@ -92,14 +116,17 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>>
>>          num_lut_banks = gamma->data->lut_size / gamma->data->lut_bank_size;
>>          cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
>> -       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, !!(gamma->data->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);
>>                  }
>>
>> @@ -112,9 +139,15 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>>                          hwlut.blue = drm_color_lut_extract(lut[n].blue, gamma->data->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 (gamma->data->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, gamma->data->lut_bits);
>> @@ -125,11 +158,19 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>>                                  diff.blue = lut[n].blue - lut[n - 1].blue;
>>                                  diff.blue = drm_color_lut_extract(diff.blue, gamma->data->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);
>> +                               if (gamma->data->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 (gamma->data->lut_bits == 12)
>> +                               writel(word[1], (lut1_base + i * 4));
>>                  }
>>          }
>>
>> @@ -246,11 +287,20 @@ static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
>>          .lut_size = 512,
>>   };
>>
>> +static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
>> +       .lut_bank_size = 256,
>> +       .lut_bits = 12,
>> +       .lut_diff = true,
>> +       .lut_size = 1024,
>> +};
>> +
>>   static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
>>          { .compatible = "mediatek,mt8173-disp-gamma",
>>            .data = &mt8173_gamma_driver_data},
>>          { .compatible = "mediatek,mt8183-disp-gamma",
>>            .data = &mt8183_gamma_driver_data},
>> +       { .compatible = "mediatek,mt8195-disp-gamma",
>> +         .data = &mt8195_gamma_driver_data},
> 
> I assume we need matching changes for the DT binding and to the DT?
> 

Sorry for the late reply; no, we don't need any bindings/dt changes for this,
as the bindings are matching the current devicetree node in mt8195.dtsi, and
the node has

compatible = "mediatek,mt8195-disp-gamma", "mediatek,mt8183-disp-gamma";

so, no devicetree and/or bindings modification is needed.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/mediatek/mt8195.dtsi?h=next-20230914#n2724

Regards,
Angelo

> ChenYu
> 
>>          {},
>>   };
>>   MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
>> --
>> 2.41.0
>>

Re: [PATCH v10 03/16] drm/mediatek: gamma: Support SoC specific LUT size

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 15685 bytes --]

[-- Attachment #2: Type: text/plain, Size: 9636 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Newer SoCs support a bigger Gamma LUT table: wire up a callback
> to retrieve the correct LUT size for each different Gamma IP.
> 
> Co-developed-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> [Angelo: Rewritten commit message/description + porting]
> 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_aal.c     | 17 ++++++++++++++-
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  2 ++
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 23 ++++++++++++++++++-
> --
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  8 +++++--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |  1 -
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  2 ++
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  9 ++++++++
>  7 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 2f602f1f1c49..e2e4155faf01 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -19,7 +19,7 @@
>  #define AAL_EN						BIT(0)
>  #define DISP_AAL_SIZE				0x0030
>  #define DISP_AAL_OUTPUT_SIZE			0x04d8
> -
> +#define DISP_AAL_LUT_SIZE			512
>  
>  struct mtk_disp_aal_data {
>  	bool has_gamma;
> @@ -61,6 +61,21 @@ void mtk_aal_config(struct device *dev, unsigned
> int w,
>  	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, 
> DISP_AAL_OUTPUT_SIZE);
>  }
>  
> +/**
> + * mtk_aal_gamma_get_lut_size() - Get gamma LUT size for AAL
> + * @dev: Pointer to struct device
> + *
> + * Return: 0 if gamma control not supported in AAL or gamma LUT size
> + */
> +unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
> +{
> +	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +
> +	if (aal->data && aal->data->has_gamma)
> +		return DISP_AAL_LUT_SIZE;
> +	return 0;
> +}
> +
>  void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
> *state)
>  {
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 75045932353e..ca377265e5eb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -17,6 +17,7 @@ void mtk_aal_clk_disable(struct device *dev);
>  void mtk_aal_config(struct device *dev, unsigned int w,
>  		    unsigned int h, unsigned int vrefresh,
>  		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> +unsigned int mtk_aal_gamma_get_lut_size(struct device *dev);
>  void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
>  void mtk_aal_start(struct device *dev);
>  void mtk_aal_stop(struct device *dev);
> @@ -53,6 +54,7 @@ void mtk_gamma_clk_disable(struct device *dev);
>  void mtk_gamma_config(struct device *dev, unsigned int w,
>  		      unsigned int h, unsigned int vrefresh,
>  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> +unsigned int mtk_gamma_get_lut_size(struct device *dev);
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
>  void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state);
>  void mtk_gamma_start(struct device *dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 47751864bd5c..7575237625d2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -24,10 +24,12 @@
>  #define DISP_GAMMA_LUT				0x0700
>  
>  #define LUT_10BIT_MASK				0x03ff
> +#define LUT_SIZE_DEFAULT			512
>  
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
> +	u16 lut_size;
>  };
>  
>  /*
> @@ -54,6 +56,15 @@ void mtk_gamma_clk_disable(struct device *dev)
>  	clk_disable_unprepare(gamma->clk);
>  }
>  
> +unsigned int mtk_gamma_get_lut_size(struct device *dev)
> +{
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +
> +	if (gamma && gamma->data)
> +		return gamma->data->lut_size;
> +	return LUT_SIZE_DEFAULT;
> +}
> +
>  void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma;
> @@ -61,6 +72,7 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs, struct drm_crt
>  	struct drm_color_lut *lut;
>  	void __iomem *lut_base;
>  	bool lut_diff;
> +	u16 lut_size;
>  	u32 word;
>  	u32 diff[3] = {0};
>  
> @@ -71,17 +83,20 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  	/* If we're called from AAL, dev is NULL */
>  	gamma = dev ? dev_get_drvdata(dev) : NULL;
>  
> -	if (gamma && gamma->data)
> +	if (gamma && gamma->data) {
>  		lut_diff = gamma->data->lut_diff;
> -	else
> +		lut_size = gamma->data->lut_size;
> +	} else {
>  		lut_diff = false;
> +		lut_size = LUT_SIZE_DEFAULT;

Only AAL use this definiton. As previous discussion,
mtk_gamm_set_common() would not be common and has gamma version and AAL
version.

Regards,
CK

> +	}
>  
>  	reg = readl(regs + DISP_GAMMA_CFG);
>  	reg = reg | GAMMA_LUT_EN;
>  	writel(reg, regs + DISP_GAMMA_CFG);
>  	lut_base = regs + DISP_GAMMA_LUT;
>  	lut = (struct drm_color_lut *)state->gamma_lut->data;
> -	for (i = 0; i < MTK_LUT_SIZE; i++) {
> +	for (i = 0; i < lut_size; i++) {
>  		if (!lut_diff || (i % 2 == 0)) {
>  			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> 20) +
>  				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> << 10) +
> @@ -198,10 +213,12 @@ 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_size = 512,
>  };
>  
>  static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
>  	.lut_diff = true,
> +	.lut_size = 512,
>  };
>  
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 8a43656ecc30..ebe0cc3a1a4c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -959,8 +959,12 @@ int mtk_drm_crtc_create(struct drm_device
> *drm_dev,
>  		mtk_crtc->ddp_comp[i] = comp;
>  
>  		if (comp->funcs) {
> -			if (comp->funcs->gamma_set)
> -				gamma_lut_size = MTK_LUT_SIZE;
> +			if (comp->funcs->gamma_set && comp->funcs-
> >gamma_get_lut_size) {
> +				unsigned int lut_sz =
> mtk_ddp_gamma_get_lut_size(comp);
> +
> +				if (lut_sz)
> +					gamma_lut_size = lut_sz;
> +			}
>  
>  			if (comp->funcs->ctm_set)
>  				has_ctm = true;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> index 3e9046993d09..b2e50292e57d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> @@ -10,7 +10,6 @@
>  #include "mtk_drm_ddp_comp.h"
>  #include "mtk_drm_plane.h"
>  
> -#define MTK_LUT_SIZE	512
>  #define MTK_MAX_BPC	10
>  #define MTK_MIN_BPC	3
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index f114da4d36a9..f3212e08f2cd 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -271,6 +271,7 @@ static void mtk_ufoe_start(struct device *dev)
>  static const struct mtk_ddp_comp_funcs ddp_aal = {
>  	.clk_enable = mtk_aal_clk_enable,
>  	.clk_disable = mtk_aal_clk_disable,
> +	.gamma_get_lut_size = mtk_aal_gamma_get_lut_size,
>  	.gamma_set = mtk_aal_gamma_set,
>  	.config = mtk_aal_config,
>  	.start = mtk_aal_start,
> @@ -322,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_dsi =
> {
>  static const struct mtk_ddp_comp_funcs ddp_gamma = {
>  	.clk_enable = mtk_gamma_clk_enable,
>  	.clk_disable = mtk_gamma_clk_disable,
> +	.gamma_get_lut_size = mtk_gamma_get_lut_size,
>  	.gamma_set = mtk_gamma_set,
>  	.config = mtk_gamma_config,
>  	.start = mtk_gamma_start,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index febcaeef16a1..c1355960e195 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -67,6 +67,7 @@ struct mtk_ddp_comp_funcs {
>  	void (*layer_config)(struct device *dev, unsigned int idx,
>  			     struct mtk_plane_state *state,
>  			     struct cmdq_pkt *cmdq_pkt);
> +	unsigned int (*gamma_get_lut_size)(struct device *dev);
>  	void (*gamma_set)(struct device *dev,
>  			  struct drm_crtc_state *state);
>  	void (*bgclr_in_on)(struct device *dev);
> @@ -186,6 +187,14 @@ static inline void
> mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
>  		comp->funcs->layer_config(comp->dev, idx, state,
> cmdq_pkt);
>  }
>  
> +static inline unsigned int mtk_ddp_gamma_get_lut_size(struct
> mtk_ddp_comp *comp)
> +{
> +	if (comp->funcs && comp->funcs->gamma_get_lut_size)
> +		return comp->funcs->gamma_get_lut_size(comp->dev);
> +
> +	return 0;
> +}
> +
>  static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
>  				     struct drm_crtc_state *state)
>  {

Re: [PATCH v10 03/16] drm/mediatek: gamma: Support SoC specific LUT size

[AngeloGioacchino Del Regno]

Il 18/09/23 04:30, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
> 
> On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
>> Newer SoCs support a bigger Gamma LUT table: wire up a callback
>> to retrieve the correct LUT size for each different Gamma IP.
>>
>> Co-developed-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
>> [Angelo: Rewritten commit message/description + porting]
>> 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_aal.c     | 17 ++++++++++++++-
>>   drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  2 ++
>>   drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 23 ++++++++++++++++++-
>> --
>>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  8 +++++--
>>   drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |  1 -
>>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  2 ++
>>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  9 ++++++++
>>   7 files changed, 55 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
>> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
>> index 2f602f1f1c49..e2e4155faf01 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
>> @@ -19,7 +19,7 @@
>>   #define AAL_EN						BIT(0)
>>   #define DISP_AAL_SIZE				0x0030
>>   #define DISP_AAL_OUTPUT_SIZE			0x04d8
>> -
>> +#define DISP_AAL_LUT_SIZE			512
>>   
>>   struct mtk_disp_aal_data {
>>   	bool has_gamma;
>> @@ -61,6 +61,21 @@ void mtk_aal_config(struct device *dev, unsigned
>> int w,
>>   	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs,
>> DISP_AAL_OUTPUT_SIZE);
>>   }
>>   
>> +/**
>> + * mtk_aal_gamma_get_lut_size() - Get gamma LUT size for AAL
>> + * @dev: Pointer to struct device
>> + *
>> + * Return: 0 if gamma control not supported in AAL or gamma LUT size
>> + */
>> +unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
>> +{
>> +	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
>> +
>> +	if (aal->data && aal->data->has_gamma)
>> +		return DISP_AAL_LUT_SIZE;
>> +	return 0;
>> +}
>> +
>>   void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
>> *state)
>>   {
>>   	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>> index 75045932353e..ca377265e5eb 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>> @@ -17,6 +17,7 @@ void mtk_aal_clk_disable(struct device *dev);
>>   void mtk_aal_config(struct device *dev, unsigned int w,
>>   		    unsigned int h, unsigned int vrefresh,
>>   		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>> +unsigned int mtk_aal_gamma_get_lut_size(struct device *dev);
>>   void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
>> *state);
>>   void mtk_aal_start(struct device *dev);
>>   void mtk_aal_stop(struct device *dev);
>> @@ -53,6 +54,7 @@ void mtk_gamma_clk_disable(struct device *dev);
>>   void mtk_gamma_config(struct device *dev, unsigned int w,
>>   		      unsigned int h, unsigned int vrefresh,
>>   		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>> +unsigned int mtk_gamma_get_lut_size(struct device *dev);
>>   void mtk_gamma_set(struct device *dev, struct drm_crtc_state
>> *state);
>>   void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
>> struct drm_crtc_state *state);
>>   void mtk_gamma_start(struct device *dev);
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> index 47751864bd5c..7575237625d2 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> @@ -24,10 +24,12 @@
>>   #define DISP_GAMMA_LUT				0x0700
>>   
>>   #define LUT_10BIT_MASK				0x03ff
>> +#define LUT_SIZE_DEFAULT			512
>>   
>>   struct mtk_disp_gamma_data {
>>   	bool has_dither;
>>   	bool lut_diff;
>> +	u16 lut_size;
>>   };
>>   
>>   /*
>> @@ -54,6 +56,15 @@ void mtk_gamma_clk_disable(struct device *dev)
>>   	clk_disable_unprepare(gamma->clk);
>>   }
>>   
>> +unsigned int mtk_gamma_get_lut_size(struct device *dev)
>> +{
>> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>> +
>> +	if (gamma && gamma->data)
>> +		return gamma->data->lut_size;
>> +	return LUT_SIZE_DEFAULT;
>> +}
>> +
>>   void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
>> struct drm_crtc_state *state)
>>   {
>>   	struct mtk_disp_gamma *gamma;
>> @@ -61,6 +72,7 @@ void mtk_gamma_set_common(struct device *dev, void
>> __iomem *regs, struct drm_crt
>>   	struct drm_color_lut *lut;
>>   	void __iomem *lut_base;
>>   	bool lut_diff;
>> +	u16 lut_size;
>>   	u32 word;
>>   	u32 diff[3] = {0};
>>   
>> @@ -71,17 +83,20 @@ void mtk_gamma_set_common(struct device *dev,
>> void __iomem *regs, struct drm_crt
>>   	/* If we're called from AAL, dev is NULL */
>>   	gamma = dev ? dev_get_drvdata(dev) : NULL;
>>   
>> -	if (gamma && gamma->data)
>> +	if (gamma && gamma->data) {
>>   		lut_diff = gamma->data->lut_diff;
>> -	else
>> +		lut_size = gamma->data->lut_size;
>> +	} else {
>>   		lut_diff = false;
>> +		lut_size = LUT_SIZE_DEFAULT;
> 
> Only AAL use this definiton. As previous discussion,
> mtk_gamm_set_common() would not be common and has gamma version and AAL
> version.
> 

Hello CK,

Yes, that's true, but every patch has to work on its own: this means that
I forcefully have to add this here to avoid breaking functionality!

Please check patch [08/16] where I de-commonize the gamma_set function, as
there I am removing the AAL-only definition and adding a AAL-specific
gamma_set() callback.

I've done it this way for multiple reasons, one of which is to show why a
de-commonization of this function is required.

Regards,
Angelo

> Regards,
> CK
> 

Re: [PATCH v10 16/16] drm/mediatek: aal: Compress of_device_id entries and add sentinel

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 3009 bytes --]

[-- Attachment #2: Type: text/plain, Size: 1362 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Compress the entry for mediatek,mt8173-disp-aal, as it fits in one
> line, and fix the style; while at it, also add the usual sentinel
> comment to the last entry.
> 
> This commit brings no functional changes.

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

> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index e6ab3eaa1126..70de5f3007e4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -212,10 +212,9 @@ static const struct mtk_disp_aal_data
> mt8173_aal_driver_data = {
>  };
>  
>  static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
> -	{ .compatible = "mediatek,mt8173-disp-aal",
> -	  .data = &mt8173_aal_driver_data},
> -	{ .compatible = "mediatek,mt8183-disp-aal"},
> -	{},
> +	{ .compatible = "mediatek,mt8173-disp-aal", .data =
> &mt8173_aal_driver_data },
> +	{ .compatible = "mediatek,mt8183-disp-aal" },
> +	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, mtk_disp_aal_driver_dt_match);
>  

Re: [PATCH v10 15/16] drm/mediatek: gamma: Add kerneldoc for struct mtk_disp_gamma

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 2729 bytes --]

[-- Attachment #2: Type: text/plain, Size: 1145 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> The mtk_disp_gamma structure was completely undocumented: add some
> kerneldoc documentation to it.

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

> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index d9a70238d524..867445a2af9e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -54,8 +54,12 @@ struct mtk_disp_gamma_data {
>  	u8 lut_bits;
>  };
>  
> -/*
> - * struct mtk_disp_gamma - DISP_GAMMA driver structure
> +/**
> + * struct mtk_disp_gamma - Display Gamma driver structure
> + * @clk:      clock for DISP_GAMMA block
> + * @regs:     MMIO registers base
> + * @cmdq_reg: CMDQ Client register
> + * @data:     platform data for DISP_GAMMA
>   */
>  struct mtk_disp_gamma {
>  	struct clk *clk;

Re: [PATCH v10 14/16] drm/mediatek: aal: Rewrite kerneldoc for struct mtk_disp_aal

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 2953 bytes --]

[-- Attachment #2: Type: text/plain, Size: 1265 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> The kerneldoc for struct mtk_disp_aal was entirely wrong: rewrite it
> to actually document the structure.

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

> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 992dc1424c91..e6ab3eaa1126 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -36,9 +36,11 @@ struct mtk_disp_aal_data {
>  };
>  
>  /**
> - * struct mtk_disp_aal - DISP_AAL driver structure
> - * @ddp_comp - structure containing type enum and hardware resources
> - * @crtc - associated crtc to report irq events to
> + * struct mtk_disp_aal - Display Adaptive Ambient Light driver
> structure
> + * @clk:      clock for DISP_AAL controller
> + * @regs:     MMIO registers base
> + * @cmdq_reg: CMDQ Client register
> + * @data:     platform specific data for DISP_AAL
>   */
>  struct mtk_disp_aal {
>  	struct clk *clk;

Re: [PATCH v10 00/16] MediaTek DDP GAMMA - 12-bit LUT support

[AngeloGioacchino Del Regno]

Il 04/08/23 09:28, AngeloGioacchino Del Regno ha scritto:
> Changes in v10:
>   - Moved snippet from patch [7/15] to patch [6/15] as that was
>     intended to be there instead; fixes build issue for patch [6/15]
>     as pointed out by the kernel text robot (oops, sorry!)
> 

Hello CK,

is there anything wrong about this series?

All commits do work on their own, there are no regressions as we tested this
on multiple Chromebooks (MT8173, MT8183, MT8192, MT8195) and a MT6795 Helio
X10 smartphone, and kept testing for months now.

Can you please pick it?

Thanks,
Angelo

> Changes in v9:
>   - As per previous conversation with CK Hu, added a commit that
>     de-commonizes the gamma setting function that was used in
>     both DISP_AAL and DISP_GAMMA, now each of them have their
>     own .gamma_set() callback (mtk_disp_gamma_set_common() has
>     been removed).
>   - Added a change to use bitfield macros in mtk_disp_aal.c
>   - Added a change to compress of_device_id entries in mtk_disp_aal.c
>   - Tested again on MT6795, MT8173, MT8186, MT8192, MT8195
> 
> Changes in v8:
>   - Changed lut_size to be a mtk_disp_gamma_set_common() function
>     parameter to pass lut size from AAL
> 
> Changes in v7:
>   - Added check for NULL dev for AAL-gamma case
>   - Added get_lut_size callback for AAL-gamma
>   - Added comment to clarify SoC 10/12 bits support and old vs new
>     register layout as suggested by Alexandre M.
> 
> Changes in v6:
>   - Fixed smatch warning in patch 11/11, ref.:
>     https://lore.kernel.org/all/202306101458.lRXHEE0Z-lkp@intel.com/
> 
> Changes in v5:
>   - Removed incorrect comment on default LUT size and bits
>   - Removed useless check for num_lut_banks
>   - Added comment about CMDQ implementation on patch 5
>   - Evaluated passing lut size/bits from AAL, idea discarded as
>     the implementation would be rather tricky while bringing no
>     benefits.
> 
> Changes in v4:
>   - Fixed assignment typo appeared in v3
> 
> Changes in v3:
>   - Fixed issues due to variables renaming during cleanup (oops)
>   - This is actually the right series, since v2 was taken from the
>     wrong kernel tree.... :-)
> 
> Changes in v2:
>   - Added explicit inclusion of linux/bitfield.h in patch [06/11]
> 
> This series adds support for GAMMA IP requiring and/or supporting
> a 12-bits LUT using a slightly different register layout and programming
> sequence for multiple LUT banks: this IP version is currently found
> on a number of SoCs, not only including the Chromebook/IoT oriented
> Kompanio 1200/1380 MT8195/MT8195T, but also Smartphone chips such as
> the Dimensity 9200 (MT6985) and others.
> 
> This series was tested on MT8195, MT8192, MT8173, MT6795:
>   * MT6795, MT8192, MT8173: No regression, works fine.
>   * MT8195: Color correction is finally working!
> 
> AngeloGioacchino Del Regno (15):
>    drm/mediatek: gamma: Reduce indentation in mtk_gamma_set_common()
>    drm/mediatek: gamma: Support SoC specific LUT size
>    drm/mediatek: gamma: Improve and simplify HW LUT calculation
>    drm/mediatek: gamma: Enable the Gamma LUT table only after programming
>    drm/mediatek: gamma: Use bitfield macros
>    drm/mediatek: aal: Use bitfield macros
>    drm/mediatek: De-commonize disp_aal/disp_gamma gamma_set functions
>    drm/mediatek: gamma: Support specifying number of bits per LUT
>      component
>    drm/mediatek: gamma: Support multi-bank gamma LUT
>    drm/mediatek: gamma: Add support for 12-bit LUT and MT8195
>    drm/mediatek: gamma: Make sure relay mode is disabled
>    drm/mediatek: gamma: Program gamma LUT type for descending or rising
>    drm/mediatek: aal: Rewrite kerneldoc for struct mtk_disp_aal
>    drm/mediatek: gamma: Add kerneldoc for struct mtk_disp_gamma
>    drm/mediatek: aal: Compress of_device_id entries and add sentinel
> 
> Jason-JH.Lin (1):
>    drm/mediatek: gamma: Adjust mtk_drm_gamma_set_common parameters
> 
>   drivers/gpu/drm/mediatek/mtk_disp_aal.c     |  84 ++++++--
>   drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
>   drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 203 ++++++++++++++++----
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   8 +-
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   2 +
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   9 +
>   7 files changed, 256 insertions(+), 54 deletions(-)
> 

Re: [PATCH v10 07/16] drm/mediatek: aal: Use bitfield macros

[Nícolas F. R. A. Prado]

On Fri, Aug 04, 2023 at 09:28:41AM +0200, AngeloGioacchino Del Regno wrote:
> Make the code more robust and improve readability by using bitfield
> macros instead of open coding bit operations.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas

Re: [PATCH v10 08/16] drm/mediatek: De-commonize disp_aal/disp_gamma gamma_set functions

[Nícolas F. R. A. Prado]

On Fri, Aug 04, 2023 at 09:28:42AM +0200, AngeloGioacchino Del Regno wrote:
> In preparation for adding a 12-bits gamma support for the DISP_GAMMA
> IP, remove the mtk_gamma_set_common() function and move the relevant
> bits in mtk_gamma_set() for DISP_GAMMA and mtk_aal_gamma_set() for
> DISP_AAL: since the latter has no more support for gamma manipulation
> (being moved to a different IP) in newer revisions, those functions
> are about to diverge and it makes no sense to keep a common one (with
> all the complications of passing common data and making exclusions
> for device driver data) for just a few bits.
> 
> This commit brings no functional changes.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c   | 39 +++++++++++++++++++++--
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  1 -
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 ++++----------------
>  3 files changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index bec035780db0..21b25470e9b7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -17,10 +17,18 @@
>  
>  #define DISP_AAL_EN				0x0000
>  #define AAL_EN						BIT(0)
> +#define DISP_AAL_CFG				0x0020
> +#define AAL_RELAY_MODE					BIT(0)
> +#define AAL_GAMMA_LUT_EN				BIT(1)
>  #define DISP_AAL_SIZE				0x0030
>  #define DISP_AAL_SIZE_HSIZE				GENMASK(28, 16)
>  #define DISP_AAL_SIZE_VSIZE				GENMASK(12, 0)
>  #define DISP_AAL_OUTPUT_SIZE			0x04d8
> +#define DISP_AAL_GAMMA_LUT			0x0700
> +#define DISP_AAL_GAMMA_LUT_R				GENMASK(29, 20)
> +#define DISP_AAL_GAMMA_LUT_G				GENMASK(19, 10)
> +#define DISP_AAL_GAMMA_LUT_B				GENMASK(9, 0)
> +#define DISP_AAL_LUT_BITS			10
>  #define DISP_AAL_LUT_SIZE			512
>  
>  struct mtk_disp_aal_data {
> @@ -85,9 +93,36 @@ unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
>  void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +	struct drm_color_lut *lut;
> +	unsigned int i;
> +	u32 cfg_val;
> +
> +	/* If gamma is not supported in AAL, go out immediately */
> +	if (!(aal->data && aal->data->has_gamma))
> +		return;
> +
> +	/* Also, if there's no gamma lut there's nothing to do here. */
> +	if (!state->gamma_lut)
> +		return;
> +
> +	cfg_val = readl(aal->regs + DISP_AAL_CFG);
> +	lut = (struct drm_color_lut *)state->gamma_lut->data;
> +
> +	for (i = 0; i < DISP_AAL_LUT_SIZE; i++) {
> +		struct drm_color_lut hwlut = {
> +			.red = drm_color_lut_extract(lut[i].red, DISP_AAL_LUT_BITS),
> +			.green = drm_color_lut_extract(lut[i].green, DISP_AAL_LUT_BITS),
> +			.blue = drm_color_lut_extract(lut[i].blue, DISP_AAL_LUT_BITS)
> +		};
> +		u32 word;
> +
> +		word = FIELD_PREP(DISP_AAL_GAMMA_LUT_R, hwlut.red);
> +		word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_G, hwlut.green);
> +		word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_B, hwlut.blue);
> +		writel(word, (aal->regs + DISP_AAL_GAMMA_LUT) + (i * 4));
> +	}

Hi Angelo,

it looks like you're missing a

  	cfg_val |= FIELD_PREP(AAL_GAMMA_LUT_EN, 1);

to actually enable the AAL gamma table here, like was done in the common
function.

Thanks,
Nícolas

>  
> -	if (aal->data && aal->data->has_gamma)
> -		mtk_gamma_set_common(NULL, aal->regs, state);
> +	writel(cfg_val, aal->regs + DISP_AAL_CFG);
>  }
[..]

Re: [PATCH v10 13/16] drm/mediatek: gamma: Program gamma LUT type for descending or rising

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 4727 bytes --]

[-- Attachment #2: Type: text/plain, Size: 2468 bytes --]

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_MODE				BIT(0)
>  #define GAMMA_LUT_EN					BIT(1)
>  #define GAMMA_DITHERING					BIT(2)
> +#define GAMMA_LUT_TYPE					BIT(2)
>  #define DISP_GAMMA_SIZE				0x0030
>  #define DISP_GAMMA_SIZE_HSIZE				GENMASK
> (28, 16)
>  #define DISP_GAMMA_SIZE_VSIZE				GENMASK
> (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);
>  

Re: [PATCH v10 08/16] drm/mediatek: De-commonize disp_aal/disp_gamma gamma_set functions

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 11515 bytes --]

[-- Attachment #2: Type: text/plain, Size: 7019 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> In preparation for adding a 12-bits gamma support for the DISP_GAMMA
> IP, remove the mtk_gamma_set_common() function and move the relevant
> bits in mtk_gamma_set() for DISP_GAMMA and mtk_aal_gamma_set() for
> DISP_AAL: since the latter has no more support for gamma manipulation
> (being moved to a different IP) in newer revisions, those functions
> are about to diverge and it makes no sense to keep a common one (with
> all the complications of passing common data and making exclusions
> for device driver data) for just a few bits.
> 
> This commit brings no functional changes.
> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c   | 39
> +++++++++++++++++++++--
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  1 -
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 ++++----------------
>  3 files changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index bec035780db0..21b25470e9b7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -17,10 +17,18 @@
>  
>  #define DISP_AAL_EN				0x0000
>  #define AAL_EN						BIT(0)
> +#define DISP_AAL_CFG				0x0020
> +#define AAL_RELAY_MODE					BIT(0)

You does not use this bit in this patch, so move it to [12/16].

> +#define AAL_GAMMA_LUT_EN				BIT(1)

You should enable this bit.

>  #define DISP_AAL_SIZE				0x0030
>  #define DISP_AAL_SIZE_HSIZE				GENMASK(28, 16)
>  #define DISP_AAL_SIZE_VSIZE				GENMASK(12, 0)
>  #define DISP_AAL_OUTPUT_SIZE			0x04d8
> +#define DISP_AAL_GAMMA_LUT			0x0700
> +#define DISP_AAL_GAMMA_LUT_R				GENMASK(29, 20)
> +#define DISP_AAL_GAMMA_LUT_G				GENMASK(19, 10)
> +#define DISP_AAL_GAMMA_LUT_B				GENMASK(9, 0)
> +#define DISP_AAL_LUT_BITS			10
>  #define DISP_AAL_LUT_SIZE			512
>  
>  struct mtk_disp_aal_data {
> @@ -85,9 +93,36 @@ unsigned int mtk_aal_gamma_get_lut_size(struct
> device *dev)
>  void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
> *state)
>  {
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +	struct drm_color_lut *lut;
> +	unsigned int i;
> +	u32 cfg_val;
> +
> +	/* If gamma is not supported in AAL, go out immediately */
> +	if (!(aal->data && aal->data->has_gamma))
> +		return;
> +
> +	/* Also, if there's no gamma lut there's nothing to do here. */
> +	if (!state->gamma_lut)
> +		return;
> +
> +	cfg_val = readl(aal->regs + DISP_AAL_CFG);
> +	lut = (struct drm_color_lut *)state->gamma_lut->data;
> +
> +	for (i = 0; i < DISP_AAL_LUT_SIZE; i++) {
> +		struct drm_color_lut hwlut = {
> +			.red = drm_color_lut_extract(lut[i].red,
> DISP_AAL_LUT_BITS),
> +			.green = drm_color_lut_extract(lut[i].green,
> DISP_AAL_LUT_BITS),
> +			.blue = drm_color_lut_extract(lut[i].blue,
> DISP_AAL_LUT_BITS)
> +		};
> +		u32 word;
> +
> +		word = FIELD_PREP(DISP_AAL_GAMMA_LUT_R, hwlut.red);
> +		word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_G, hwlut.green);
> +		word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_B, hwlut.blue);
> +		writel(word, (aal->regs + DISP_AAL_GAMMA_LUT) + (i *
> 4));

This could be more simple:

writel(word, aal->regs + DISP_AAL_GAMMA_LUT + i * 4);

Regards,
CK

> +	}
>  
> -	if (aal->data && aal->data->has_gamma)
> -		mtk_gamma_set_common(NULL, aal->regs, state);
> +	writel(cfg_val, aal->regs + DISP_AAL_CFG);
>  }
>  
>  void mtk_aal_start(struct device *dev)
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index ca377265e5eb..54d3712e2afd 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -56,7 +56,6 @@ void mtk_gamma_config(struct device *dev, unsigned
> int w,
>  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>  unsigned int mtk_gamma_get_lut_size(struct device *dev);
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
> -void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state);
>  void mtk_gamma_start(struct device *dev);
>  void mtk_gamma_stop(struct device *dev);
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index ea91d3619716..001b98694761 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -73,42 +73,29 @@ unsigned int mtk_gamma_get_lut_size(struct device
> *dev)
>  	return LUT_SIZE_DEFAULT;
>  }
>  
> -void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state)
> +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>  {
> -	struct mtk_disp_gamma *gamma;
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>  	unsigned int i;
>  	struct drm_color_lut *lut;
>  	void __iomem *lut_base;
> -	bool lut_diff;
> -	u16 lut_size;
>  	u32 cfg_val, word;
>  
>  	/* If there's no gamma lut there's nothing to do here. */
>  	if (!state->gamma_lut)
>  		return;
>  
> -	/* If we're called from AAL, dev is NULL */
> -	gamma = dev ? dev_get_drvdata(dev) : NULL;
> -
> -	if (gamma && gamma->data) {
> -		lut_diff = gamma->data->lut_diff;
> -		lut_size = gamma->data->lut_size;
> -	} else {
> -		lut_diff = false;
> -		lut_size = LUT_SIZE_DEFAULT;
> -	}
> -
> -	cfg_val = readl(regs + DISP_GAMMA_CFG);
> -	lut_base = regs + DISP_GAMMA_LUT;
> +	cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
> +	lut_base = gamma->regs + DISP_GAMMA_LUT;
>  	lut = (struct drm_color_lut *)state->gamma_lut->data;
> -	for (i = 0; i < lut_size; i++) {
> +	for (i = 0; i < gamma->data->lut_size; i++) {
>  		struct drm_color_lut diff, hwlut;
>  
>  		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)) {
> +		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);
> @@ -132,14 +119,7 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  	/* Enable the gamma table */
>  	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
>  
> -	writel(cfg_val, regs + DISP_GAMMA_CFG);
> -}
> -
> -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> -{
> -	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> -
> -	mtk_gamma_set_common(dev, gamma->regs, state);
> +	writel(cfg_val, gamma->regs + DISP_GAMMA_CFG);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,

Re: [PATCH v10 12/16] drm/mediatek: gamma: Make sure relay mode is disabled

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 4302 bytes --]

[-- Attachment #2: Type: text/plain, Size: 2258 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Disable relay mode at the end of LUT programming to make sure that
> the
> processed image goes through in both DISP_GAMMA and DISP_AAL for
> gamma
> setting.
> 
> 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_aal.c   | 3 +++
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 4 ++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 21b25470e9b7..992dc1424c91 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -122,6 +122,9 @@ void mtk_aal_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  		writel(word, (aal->regs + DISP_AAL_GAMMA_LUT) + (i *
> 4));
>  	}
>  
> +	/* Disable RELAY mode to pass the processed image */
> +	cfg_val &= ~AAL_RELAY_MODE;

It seems that the relay mode is default turned off. But explicitly
setting it is good to me. After adding AAL_RELAY_MODE definition in
this patch,

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

> +
>  	writel(cfg_val, aal->regs + DISP_AAL_CFG);
>  }
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 7d2f8042ace0..fbff9f97b737 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -19,6 +19,7 @@
>  #define DISP_GAMMA_EN				0x0000
>  #define GAMMA_EN					BIT(0)
>  #define DISP_GAMMA_CFG				0x0020
> +#define GAMMA_RELAY_MODE				BIT(0)
>  #define GAMMA_LUT_EN					BIT(1)
>  #define GAMMA_DITHERING					BIT(2)
>  #define DISP_GAMMA_SIZE				0x0030
> @@ -177,6 +178,9 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  	/* Enable the gamma table */
>  	cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
>  
> +	/* Disable RELAY mode to pass the processed image */
> +	cfg_val &= ~GAMMA_RELAY_MODE;
> +
>  	writel(cfg_val, gamma->regs + DISP_GAMMA_CFG);
>  }
>  

Re: [PATCH v10 11/16] drm/mediatek: gamma: Add support for 12-bit LUT and MT8195

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 11487 bytes --]

[-- Attachment #2: Type: text/plain, Size: 6936 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Add support for 12-bit gamma lookup tables and introduce the first
> user for it: MT8195.
> While at it, also reorder the variables in mtk_gamma_set_common()
> and rename `lut_base` to `lut0_base` to improve readability.

Separate 12-bit gamma support and mt8195 support to different patches.

Regards,
CK

> 
> 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 | 74 +++++++++++++++++++
> ----
>  1 file changed, 62 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 3f1c6815ea5a..7d2f8042ace0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -26,12 +26,20 @@
>  #define DISP_GAMMA_SIZE_VSIZE				GENMASK
> (12, 0)
>  #define DISP_GAMMA_BANK				0x0100
>  #define DISP_GAMMA_BANK_BANK				GENMASK(1, 0)
> +#define DISP_GAMMA_BANK_DATA_MODE			BIT(2)
>  #define DISP_GAMMA_LUT				0x0700
> +#define DISP_GAMMA_LUT1				0x0b00
>  
> +/* For 10 bit LUT layout, R/G/B are in the same register */
>  #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)
>  
> +/* For 12 bit LUT layout, R/G are in LUT, B is in LUT1 */
> +#define DISP_GAMMA_LUT_12BIT_R			GENMASK(11, 0)
> +#define DISP_GAMMA_LUT_12BIT_G			GENMASK(23, 12)
> +#define DISP_GAMMA_LUT_12BIT_B			GENMASK(11, 0)
> +
>  #define LUT_10BIT_MASK				0x03ff
>  #define LUT_BITS_DEFAULT			10
>  #define LUT_SIZE_DEFAULT			512
> @@ -77,14 +85,30 @@ unsigned int mtk_gamma_get_lut_size(struct device
> *dev)
>  	return LUT_SIZE_DEFAULT;
>  }
>  
> +/*
> + * 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];
>  	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)
> @@ -92,14 +116,17 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  
>  	num_lut_banks = gamma->data->lut_size / gamma->data-
> >lut_bank_size;
>  	cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
> -	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, !!(gamma-
> >data->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);
>  		}
>  
> @@ -112,9 +139,15 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  			hwlut.blue = drm_color_lut_extract(lut[n].blue,
> gamma->data->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 (gamma->data->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, gamma->data->lut_bits);
> @@ -125,11 +158,19 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  				diff.blue = lut[n].blue - lut[n -
> 1].blue;
>  				diff.blue =
> drm_color_lut_extract(diff.blue, gamma->data->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);
> +				if (gamma->data->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 (gamma->data->lut_bits == 12)
> +				writel(word[1], (lut1_base + i * 4));
>  		}
>  	}
>  
> @@ -246,11 +287,20 @@ static const struct mtk_disp_gamma_data
> mt8183_gamma_driver_data = {
>  	.lut_size = 512,
>  };
>  
> +static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
> +	.lut_bank_size = 256,
> +	.lut_bits = 12,
> +	.lut_diff = true,
> +	.lut_size = 1024,
> +};
> +
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
>  	{ .compatible = "mediatek,mt8173-disp-gamma",
>  	  .data = &mt8173_gamma_driver_data},
>  	{ .compatible = "mediatek,mt8183-disp-gamma",
>  	  .data = &mt8183_gamma_driver_data},
> +	{ .compatible = "mediatek,mt8195-disp-gamma",
> +	  .data = &mt8195_gamma_driver_data},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);

Re: [PATCH v10 10/16] drm/mediatek: gamma: Support multi-bank gamma LUT

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 9327 bytes --]

[-- Attachment #2: Type: text/plain, Size: 5640 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +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@mediatek.com>
> [Angelo: Refactored original commit]
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 70 ++++++++++++++-------
> --
>  1 file changed, 44 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 1845bd326a6d..3f1c6815ea5a 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)
> @@ -37,6 +39,7 @@
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
> +	u16 lut_bank_size;
>  	u16 lut_size;
>  	u8 lut_bits;
>  };
> @@ -80,41 +83,54 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  	unsigned int i;
>  	struct drm_color_lut *lut;
>  	void __iomem *lut_base;
> -	u32 cfg_val, word;
> +	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)
>  		return;
>  
> +	num_lut_banks = gamma->data->lut_size / gamma->data-
> >lut_bank_size;
>  	cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
>  	lut_base = gamma->regs + DISP_GAMMA_LUT;
>  	lut = (struct drm_color_lut *)state->gamma_lut->data;
> -	for (i = 0; i < gamma->data->lut_size; i++) {
> -		struct drm_color_lut diff, hwlut;
> -
> -		hwlut.red = drm_color_lut_extract(lut[i].red, gamma-
> >data->lut_bits);
> -		hwlut.green = drm_color_lut_extract(lut[i].green,
> gamma->data->lut_bits);
> -		hwlut.blue = drm_color_lut_extract(lut[i].blue, gamma-
> >data->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);
> -		} else {
> -			diff.red = lut[i].red - lut[i - 1].red;
> -			diff.red = drm_color_lut_extract(diff.red,
> gamma->data->lut_bits);
> -
> -			diff.green = lut[i].green - lut[i - 1].green;
> -			diff.green = drm_color_lut_extract(diff.green,
> gamma->data->lut_bits);
> -
> -			diff.blue = lut[i].blue - lut[i - 1].blue;
> -			diff.blue = drm_color_lut_extract(diff.blue,
> gamma->data->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, gamma->regs +
> DISP_GAMMA_BANK);
> +		}
> +
> +		for (i = 0; i < gamma->data->lut_bank_size; i++) {
> +			int n = (cur_bank * gamma->data->lut_bank_size) 
> + i;

int n = cur_bank * gamma->data->lut_bank_size + i;

After this modification,

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

> +			struct drm_color_lut diff, hwlut;
> +
> +			hwlut.red = drm_color_lut_extract(lut[n].red,
> gamma->data->lut_bits);
> +			hwlut.green =
> drm_color_lut_extract(lut[n].green, gamma->data->lut_bits);
> +			hwlut.blue = drm_color_lut_extract(lut[n].blue,
> gamma->data->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);
> +			} else {
> +				diff.red = lut[n].red - lut[n - 1].red;
> +				diff.red =
> drm_color_lut_extract(diff.red, gamma->data->lut_bits);
> +
> +				diff.green = lut[n].green - lut[n -
> 1].green;
> +				diff.green =
> drm_color_lut_extract(diff.green, gamma->data->lut_bits);
> +
> +				diff.blue = lut[n].blue - lut[n -
> 1].blue;
> +				diff.blue =
> drm_color_lut_extract(diff.blue, gamma->data->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 */
> @@ -218,11 +234,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,

Re: [PATCH v10 09/16] drm/mediatek: gamma: Support specifying number of bits per LUT component

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 5815 bytes --]

[-- Attachment #2: Type: text/plain, Size: 3275 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +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 calculation.

I would like to merge this patch with the 12-bit lut support.

> 
> 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 | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 001b98694761..1845bd326a6d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -38,6 +38,7 @@ struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
>  	u16 lut_size;
> +	u8 lut_bits;
>  };
>  
>  /*
> @@ -91,9 +92,9 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  	for (i = 0; i < gamma->data->lut_size; i++) {
>  		struct drm_color_lut diff, hwlut;
>  
> -		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);
> +		hwlut.red = drm_color_lut_extract(lut[i].red, gamma-
> >data->lut_bits);
> +		hwlut.green = drm_color_lut_extract(lut[i].green,
> gamma->data->lut_bits);
> +		hwlut.blue = drm_color_lut_extract(lut[i].blue, gamma-
> >data->lut_bits);
>  
>  		if (!gamma->data->lut_diff || (i % 2 == 0)) {
>  			word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R,
> hwlut.red);
> @@ -101,13 +102,13 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  			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_DEFAULT);
> +			diff.red = drm_color_lut_extract(diff.red,
> gamma->data->lut_bits);
>  
>  			diff.green = lut[i].green - lut[i - 1].green;
> -			diff.green = drm_color_lut_extract(diff.green,
> LUT_BITS_DEFAULT);
> +			diff.green = drm_color_lut_extract(diff.green,
> gamma->data->lut_bits);
>  
>  			diff.blue = lut[i].blue - lut[i - 1].blue;
> -			diff.blue = drm_color_lut_extract(diff.blue,
> LUT_BITS_DEFAULT);
> +			diff.blue = drm_color_lut_extract(diff.blue,
> gamma->data->lut_bits);
>  
>  			word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R,
> diff.red);
>  			word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G,
> diff.green);
> @@ -217,10 +218,12 @@ 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_bits = 10,
>  	.lut_size = 512,
>  };
>  
>  static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
> +	.lut_bits = 10,
>  	.lut_diff = true,
>  	.lut_size = 512,
>  };

Re: [PATCH v10 07/16] drm/mediatek: aal: Use bitfield macros

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 3453 bytes --]

[-- Attachment #2: Type: text/plain, Size: 1697 bytes --]

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.

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

> 
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index e2e4155faf01..bec035780db0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -18,6 +18,8 @@
>  #define DISP_AAL_EN				0x0000
>  #define AAL_EN						BIT(0)
>  #define DISP_AAL_SIZE				0x0030
> +#define DISP_AAL_SIZE_HSIZE				GENMASK(28, 16)
> +#define DISP_AAL_SIZE_VSIZE				GENMASK(12, 0)
>  #define DISP_AAL_OUTPUT_SIZE			0x04d8
>  #define DISP_AAL_LUT_SIZE			512
>  
> @@ -56,9 +58,13 @@ void mtk_aal_config(struct device *dev, unsigned
> int w,
>  			   unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
>  {
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +	u32 sz;
>  
> -	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, 
> DISP_AAL_SIZE);
> -	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, 
> DISP_AAL_OUTPUT_SIZE);
> +	sz = FIELD_PREP(DISP_AAL_SIZE_HSIZE, w);
> +	sz |= FIELD_PREP(DISP_AAL_SIZE_VSIZE, h);
> +
> +	mtk_ddp_write(cmdq_pkt, sz, &aal->cmdq_reg, aal->regs,
> DISP_AAL_SIZE);
> +	mtk_ddp_write(cmdq_pkt, sz, &aal->cmdq_reg, aal->regs,
> DISP_AAL_OUTPUT_SIZE);
>  }
>  
>  /**

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

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 7790 bytes --]

[-- Attachment #2: Type: text/plain, Size: 4529 bytes --]

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);

Re: [PATCH v10 05/16] drm/mediatek: gamma: Enable the Gamma LUT table only after programming

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 5151 bytes --]

[-- Attachment #2: Type: text/plain, Size: 2664 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Move the write to DISP_GAMMA_CFG to enable the Gamma LUT to after
> programming the actual table to avoid potential visual glitches
> during
> table modification.
> 
> Note:
> GAMMA should get enabled in between vblanks, but this requires many
> efforts in order to make this happen, as that requires migrating all
> of the writes to make use of CMDQ instead of cpu writes and that's
> not trivial. For this reason, this patch only moves the LUT enable.
> The CMDQ rework will come at a later time.
> 
> 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 | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index fd6a75a64a9f..18b102bef370 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -68,12 +68,12 @@ unsigned int mtk_gamma_get_lut_size(struct device
> *dev)
>  void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma;
> -	unsigned int i, reg;
> +	unsigned int i;
>  	struct drm_color_lut *lut;
>  	void __iomem *lut_base;
>  	bool lut_diff;
>  	u16 lut_size;
> -	u32 word;
> +	u32 cfg_val, word;
>  
>  	/* If there's no gamma lut there's nothing to do here. */
>  	if (!state->gamma_lut)
> @@ -90,9 +90,7 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs, struct drm_crt
>  		lut_size = LUT_SIZE_DEFAULT;
>  	}
>  
> -	reg = readl(regs + DISP_GAMMA_CFG);
> -	reg = reg | GAMMA_LUT_EN;
> -	writel(reg, regs + DISP_GAMMA_CFG);
> +	cfg_val = readl(regs + DISP_GAMMA_CFG);

Move this to bottom of this function. Move here in the patch which
need.

After this modification,

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

>  	lut_base = regs + DISP_GAMMA_LUT;
>  	lut = (struct drm_color_lut *)state->gamma_lut->data;
>  	for (i = 0; i < lut_size; i++) {
> @@ -122,6 +120,11 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  		}
>  		writel(word, (lut_base + i * 4));
>  	}
> +
> +	/* Enable the gamma table */
> +	cfg_val = cfg_val | GAMMA_LUT_EN;
> +
> +	writel(cfg_val, regs + DISP_GAMMA_CFG);
>  }
>  
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)

Re: [PATCH v10 04/16] drm/mediatek: gamma: Improve and simplify HW LUT calculation

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 5899 bytes --]

[-- Attachment #2: Type: text/plain, Size: 3062 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Use drm_color_lut_extract() to avoid open-coding the bits reduction
> calculations for each color channel and use a struct drm_color_lut
> to temporarily store the information instead of an array of u32.
> 
> Also, slightly improve the precision of the HW LUT calculation in the
> LUT DIFF case by performing the subtractions on the 16-bits values
> and
> doing the 10 bits conversion later.
> 
> 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 | 30 +++++++++++++++----
> ----
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 7575237625d2..fd6a75a64a9f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -74,7 +74,6 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs, struct drm_crt
>  	bool lut_diff;
>  	u16 lut_size;
>  	u32 word;
> -	u32 diff[3] = {0};
>  
>  	/* If there's no gamma lut there's nothing to do here. */
>  	if (!state->gamma_lut)
> @@ -97,18 +96,29 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  	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, 10);
> +		hwlut.green = drm_color_lut_extract(lut[i].green, 10);
> +		hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
> +
>  		if (!lut_diff || (i % 2 == 0)) {
> -			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> 20) +

After removing definition of LUT_10BIT_MASK,

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

> -				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> << 10) +
> -				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> +			word = hwlut.red << 20 +
> +			       hwlut.green << 10 +
> +			       hwlut.red;
>  		} else {
> -			diff[0] = (lut[i].red >> 6) - (lut[i - 1].red
> >> 6);
> -			diff[1] = (lut[i].green >> 6) - (lut[i -
> 1].green >> 6);
> -			diff[2] = (lut[i].blue >> 6) - (lut[i - 1].blue
> >> 6);
> +			diff.red = lut[i].red - lut[i - 1].red;
> +			diff.red = drm_color_lut_extract(diff.red, 10);
> +
> +			diff.green = lut[i].green - lut[i - 1].green;
> +			diff.green = drm_color_lut_extract(diff.green,
> 10);
> +
> +			diff.blue = lut[i].blue - lut[i - 1].blue;
> +			diff.blue = drm_color_lut_extract(diff.blue,
> 10);
>  
> -			word = ((diff[0] & LUT_10BIT_MASK) << 20) +
> -				((diff[1] & LUT_10BIT_MASK) << 10) +
> -				(diff[2] & LUT_10BIT_MASK);
> +			word = diff.blue << 20 +
> +			       diff.green << 10 +
> +			       diff.red;
>  		}
>  		writel(word, (lut_base + i * 4));
>  	}

Re: [PATCH v10 03/16] drm/mediatek: gamma: Support SoC specific LUT size

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 15648 bytes --]

[-- Attachment #2: Type: text/plain, Size: 9597 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Newer SoCs support a bigger Gamma LUT table: wire up a callback
> to retrieve the correct LUT size for each different Gamma IP.
> 
> Co-developed-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> [Angelo: Rewritten commit message/description + porting]
> 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_aal.c     | 17 ++++++++++++++-
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  2 ++
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 23 ++++++++++++++++++-
> --
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  8 +++++--
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |  1 -
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  2 ++
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  9 ++++++++
>  7 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 2f602f1f1c49..e2e4155faf01 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -19,7 +19,7 @@
>  #define AAL_EN						BIT(0)
>  #define DISP_AAL_SIZE				0x0030
>  #define DISP_AAL_OUTPUT_SIZE			0x04d8
> -
> +#define DISP_AAL_LUT_SIZE			512
>  
>  struct mtk_disp_aal_data {
>  	bool has_gamma;
> @@ -61,6 +61,21 @@ void mtk_aal_config(struct device *dev, unsigned
> int w,
>  	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, 
> DISP_AAL_OUTPUT_SIZE);
>  }
>  
> +/**
> + * mtk_aal_gamma_get_lut_size() - Get gamma LUT size for AAL
> + * @dev: Pointer to struct device
> + *
> + * Return: 0 if gamma control not supported in AAL or gamma LUT size
> + */
> +unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
> +{
> +	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +
> +	if (aal->data && aal->data->has_gamma)
> +		return DISP_AAL_LUT_SIZE;
> +	return 0;
> +}
> +
>  void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
> *state)
>  {
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 75045932353e..ca377265e5eb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -17,6 +17,7 @@ void mtk_aal_clk_disable(struct device *dev);
>  void mtk_aal_config(struct device *dev, unsigned int w,
>  		    unsigned int h, unsigned int vrefresh,
>  		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> +unsigned int mtk_aal_gamma_get_lut_size(struct device *dev);
>  void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
>  void mtk_aal_start(struct device *dev);
>  void mtk_aal_stop(struct device *dev);
> @@ -53,6 +54,7 @@ void mtk_gamma_clk_disable(struct device *dev);
>  void mtk_gamma_config(struct device *dev, unsigned int w,
>  		      unsigned int h, unsigned int vrefresh,
>  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> +unsigned int mtk_gamma_get_lut_size(struct device *dev);
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
>  void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state);
>  void mtk_gamma_start(struct device *dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 47751864bd5c..7575237625d2 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -24,10 +24,12 @@
>  #define DISP_GAMMA_LUT				0x0700
>  
>  #define LUT_10BIT_MASK				0x03ff
> +#define LUT_SIZE_DEFAULT			512

You don't need this. Every SoC has its lut_size. After remove this,

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


>  
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
> +	u16 lut_size;
>  };
>  
>  /*
> @@ -54,6 +56,15 @@ void mtk_gamma_clk_disable(struct device *dev)
>  	clk_disable_unprepare(gamma->clk);
>  }
>  
> +unsigned int mtk_gamma_get_lut_size(struct device *dev)
> +{
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +
> +	if (gamma && gamma->data)
> +		return gamma->data->lut_size;
> +	return LUT_SIZE_DEFAULT;
> +}
> +
>  void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma;
> @@ -61,6 +72,7 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs, struct drm_crt
>  	struct drm_color_lut *lut;
>  	void __iomem *lut_base;
>  	bool lut_diff;
> +	u16 lut_size;
>  	u32 word;
>  	u32 diff[3] = {0};
>  
> @@ -71,17 +83,20 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  	/* If we're called from AAL, dev is NULL */
>  	gamma = dev ? dev_get_drvdata(dev) : NULL;
>  
> -	if (gamma && gamma->data)
> +	if (gamma && gamma->data) {
>  		lut_diff = gamma->data->lut_diff;
> -	else
> +		lut_size = gamma->data->lut_size;
> +	} else {
>  		lut_diff = false;
> +		lut_size = LUT_SIZE_DEFAULT;
> +	}
>  
>  	reg = readl(regs + DISP_GAMMA_CFG);
>  	reg = reg | GAMMA_LUT_EN;
>  	writel(reg, regs + DISP_GAMMA_CFG);
>  	lut_base = regs + DISP_GAMMA_LUT;
>  	lut = (struct drm_color_lut *)state->gamma_lut->data;
> -	for (i = 0; i < MTK_LUT_SIZE; i++) {
> +	for (i = 0; i < lut_size; i++) {
>  		if (!lut_diff || (i % 2 == 0)) {
>  			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> 20) +
>  				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> << 10) +
> @@ -198,10 +213,12 @@ 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_size = 512,
>  };
>  
>  static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
>  	.lut_diff = true,
> +	.lut_size = 512,
>  };
>  
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 8a43656ecc30..ebe0cc3a1a4c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -959,8 +959,12 @@ int mtk_drm_crtc_create(struct drm_device
> *drm_dev,
>  		mtk_crtc->ddp_comp[i] = comp;
>  
>  		if (comp->funcs) {
> -			if (comp->funcs->gamma_set)
> -				gamma_lut_size = MTK_LUT_SIZE;
> +			if (comp->funcs->gamma_set && comp->funcs-
> >gamma_get_lut_size) {
> +				unsigned int lut_sz =
> mtk_ddp_gamma_get_lut_size(comp);
> +
> +				if (lut_sz)
> +					gamma_lut_size = lut_sz;
> +			}
>  
>  			if (comp->funcs->ctm_set)
>  				has_ctm = true;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> index 3e9046993d09..b2e50292e57d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
> @@ -10,7 +10,6 @@
>  #include "mtk_drm_ddp_comp.h"
>  #include "mtk_drm_plane.h"
>  
> -#define MTK_LUT_SIZE	512
>  #define MTK_MAX_BPC	10
>  #define MTK_MIN_BPC	3
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index f114da4d36a9..f3212e08f2cd 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -271,6 +271,7 @@ static void mtk_ufoe_start(struct device *dev)
>  static const struct mtk_ddp_comp_funcs ddp_aal = {
>  	.clk_enable = mtk_aal_clk_enable,
>  	.clk_disable = mtk_aal_clk_disable,
> +	.gamma_get_lut_size = mtk_aal_gamma_get_lut_size,
>  	.gamma_set = mtk_aal_gamma_set,
>  	.config = mtk_aal_config,
>  	.start = mtk_aal_start,
> @@ -322,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_dsi =
> {
>  static const struct mtk_ddp_comp_funcs ddp_gamma = {
>  	.clk_enable = mtk_gamma_clk_enable,
>  	.clk_disable = mtk_gamma_clk_disable,
> +	.gamma_get_lut_size = mtk_gamma_get_lut_size,
>  	.gamma_set = mtk_gamma_set,
>  	.config = mtk_gamma_config,
>  	.start = mtk_gamma_start,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index febcaeef16a1..c1355960e195 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -67,6 +67,7 @@ struct mtk_ddp_comp_funcs {
>  	void (*layer_config)(struct device *dev, unsigned int idx,
>  			     struct mtk_plane_state *state,
>  			     struct cmdq_pkt *cmdq_pkt);
> +	unsigned int (*gamma_get_lut_size)(struct device *dev);
>  	void (*gamma_set)(struct device *dev,
>  			  struct drm_crtc_state *state);
>  	void (*bgclr_in_on)(struct device *dev);
> @@ -186,6 +187,14 @@ static inline void
> mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
>  		comp->funcs->layer_config(comp->dev, idx, state,
> cmdq_pkt);
>  }
>  
> +static inline unsigned int mtk_ddp_gamma_get_lut_size(struct
> mtk_ddp_comp *comp)
> +{
> +	if (comp->funcs && comp->funcs->gamma_get_lut_size)
> +		return comp->funcs->gamma_get_lut_size(comp->dev);
> +
> +	return 0;
> +}
> +
>  static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
>  				     struct drm_crtc_state *state)
>  {

Re: [PATCH v10 02/16] drm/mediatek: gamma: Reduce indentation in mtk_gamma_set_common()

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 6354 bytes --]

[-- Attachment #2: Type: text/plain, Size: 3269 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> Invert the check for state->gamma_lut and move it at the beginning
> of the function to reduce indentation: this prepares the code for
> keeping readability on later additions.
> 
> This commit brings no functional changes.

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

> 
> 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 | 45 ++++++++++++---------
> --
>  1 file changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index d42cc0698d83..47751864bd5c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -64,6 +64,10 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs, struct drm_crt
>  	u32 word;
>  	u32 diff[3] = {0};
>  
> +	/* If there's no gamma lut there's nothing to do here. */
> +	if (!state->gamma_lut)
> +		return;
> +
>  	/* If we're called from AAL, dev is NULL */
>  	gamma = dev ? dev_get_drvdata(dev) : NULL;
>  
> @@ -72,29 +76,26 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
>  	else
>  		lut_diff = false;
>  
> -	if (state->gamma_lut) {
> -		reg = readl(regs + DISP_GAMMA_CFG);
> -		reg = reg | GAMMA_LUT_EN;
> -		writel(reg, regs + DISP_GAMMA_CFG);
> -		lut_base = regs + DISP_GAMMA_LUT;
> -		lut = (struct drm_color_lut *)state->gamma_lut->data;
> -		for (i = 0; i < MTK_LUT_SIZE; i++) {
> -
> -			if (!lut_diff || (i % 2 == 0)) {
> -				word = (((lut[i].red >> 6) &
> LUT_10BIT_MASK) << 20) +
> -					(((lut[i].green >> 6) &
> LUT_10BIT_MASK) << 10) +
> -					((lut[i].blue >> 6) &
> LUT_10BIT_MASK);
> -			} else {
> -				diff[0] = (lut[i].red >> 6) - (lut[i -
> 1].red >> 6);
> -				diff[1] = (lut[i].green >> 6) - (lut[i
> - 1].green >> 6);
> -				diff[2] = (lut[i].blue >> 6) - (lut[i -
> 1].blue >> 6);
> -
> -				word = ((diff[0] & LUT_10BIT_MASK) <<
> 20) +
> -					((diff[1] & LUT_10BIT_MASK) <<
> 10) +
> -					(diff[2] & LUT_10BIT_MASK);
> -			}
> -			writel(word, (lut_base + i * 4));
> +	reg = readl(regs + DISP_GAMMA_CFG);
> +	reg = reg | GAMMA_LUT_EN;
> +	writel(reg, regs + DISP_GAMMA_CFG);
> +	lut_base = regs + DISP_GAMMA_LUT;
> +	lut = (struct drm_color_lut *)state->gamma_lut->data;
> +	for (i = 0; i < MTK_LUT_SIZE; i++) {
> +		if (!lut_diff || (i % 2 == 0)) {
> +			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) <<
> 20) +
> +				(((lut[i].green >> 6) & LUT_10BIT_MASK)
> << 10) +
> +				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> +		} else {
> +			diff[0] = (lut[i].red >> 6) - (lut[i - 1].red
> >> 6);
> +			diff[1] = (lut[i].green >> 6) - (lut[i -
> 1].green >> 6);
> +			diff[2] = (lut[i].blue >> 6) - (lut[i - 1].blue
> >> 6);
> +
> +			word = ((diff[0] & LUT_10BIT_MASK) << 20) +
> +				((diff[1] & LUT_10BIT_MASK) << 10) +
> +				(diff[2] & LUT_10BIT_MASK);
>  		}
> +		writel(word, (lut_base + i * 4));
>  	}
>  }
>  

Re: [PATCH v10 01/16] drm/mediatek: gamma: Adjust mtk_drm_gamma_set_common parameters

[CK Hu (胡俊光)]

[-- Attachment #1: Type: text/html, Size: 6796 bytes --]

[-- Attachment #2: Type: text/plain, Size: 3864 bytes --]

Hi, Angelo:

On Fri, 2023-08-04 at 09:28 +0200, AngeloGioacchino Del Regno wrote:
> From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> 
> Adjust the parameters in mtk_drm_gamma_set_common()
>   - add (struct device *dev) to get lut_diff from gamma's driver data
>   - remove (bool lut_diff) and use false as default value in the
> function

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

> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c   |  2 +-
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h   |  2 +-
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 18 ++++++++++++------
>  3 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index cdbec79474d1..2f602f1f1c49 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -66,7 +66,7 @@ void mtk_aal_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
>  
>  	if (aal->data && aal->data->has_gamma)
> -		mtk_gamma_set_common(aal->regs, state, false);
> +		mtk_gamma_set_common(NULL, aal->regs, state);
>  }
>  
>  void mtk_aal_start(struct device *dev)
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index 2254038519e1..75045932353e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -54,7 +54,7 @@ void mtk_gamma_config(struct device *dev, unsigned
> int w,
>  		      unsigned int h, unsigned int vrefresh,
>  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
> -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state, bool lut_diff);
> +void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state);
>  void mtk_gamma_start(struct device *dev);
>  void mtk_gamma_stop(struct device *dev);
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 7746dceadb20..d42cc0698d83 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -54,14 +54,24 @@ void mtk_gamma_clk_disable(struct device *dev)
>  	clk_disable_unprepare(gamma->clk);
>  }
>  
> -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state
> *state, bool lut_diff)
> +void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state)
>  {
> +	struct mtk_disp_gamma *gamma;
>  	unsigned int i, reg;
>  	struct drm_color_lut *lut;
>  	void __iomem *lut_base;
> +	bool lut_diff;
>  	u32 word;
>  	u32 diff[3] = {0};
>  
> +	/* If we're called from AAL, dev is NULL */
> +	gamma = dev ? dev_get_drvdata(dev) : NULL;
> +
> +	if (gamma && gamma->data)
> +		lut_diff = gamma->data->lut_diff;
> +	else
> +		lut_diff = false;
> +
>  	if (state->gamma_lut) {
>  		reg = readl(regs + DISP_GAMMA_CFG);
>  		reg = reg | GAMMA_LUT_EN;
> @@ -91,12 +101,8 @@ void mtk_gamma_set_common(void __iomem *regs,
> struct drm_crtc_state *state, bool
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> -	bool lut_diff = false;
> -
> -	if (gamma->data)
> -		lut_diff = gamma->data->lut_diff;
>  
> -	mtk_gamma_set_common(gamma->regs, state, lut_diff);
> +	mtk_gamma_set_common(dev, gamma->regs, state);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,