[PATCH v2 0/3] Add gamma lut support for mt8195

[PATCH v2 0/3] Add gamma lut support for mt8195

[zheng-yan.chen]

Since the gamma_set_common() function for previous SoC,
such as  mt8173 and mt8183, is designed for 9bit-to-10bit
conversion.
mt8195 is using 10bit-to-12bit conversion, which is
not compatible with the previous function.

Thus, need to update the function to fit the need of mt8195.

---
Base on series [1]:
[1]: Add driver nodes for MT8195 SoC
- https://patchwork.kernel.org/project/linux-mediatek/list/?series=666741
---

Change in v2:
        1. fix global variable in mtk_disp_gamma.c
        2. update mask calculation and delete not-necessary variables
        3. fix shift-bit problem.
        4. fix commit messages for dt-binding
        5. fix dts gamma label.


zheng-yan.chen (3):
  dt-bindings: mediatek: Add gamma compatible for mt8195
  drm/mediatek: Add gamma lut support for mt8195
  arm64: dts: Modify gamma compatible for mt8195

 .../display/mediatek/mediatek,gamma.yaml      |   3 +-
 arch/arm64/boot/dts/mediatek/mt8195.dtsi      |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_aal.c       |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h       |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c     | 102 +++++++++++++-----
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   5 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h       |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        |   2 +
 10 files changed, 88 insertions(+), 34 deletions(-)

-- 
2.18.0

[PATCH v2 0/3] Add gamma lut support for mt8195

[zheng-yan.chen]

Since the gamma_set_common() function for previous SoC,
such as  mt8173 and mt8183, is designed for 9bit-to-10bit
conversion.
mt8195 is using 10bit-to-12bit conversion, which is
not compatible with the previous function.

Thus, need to update the function to fit the need of mt8195.

---
Base on series [1]:
[1]: Add driver nodes for MT8195 SoC
- https://patchwork.kernel.org/project/linux-mediatek/list/?series=666741
---

Change in v2:
        1. fix global variable in mtk_disp_gamma.c
        2. update mask calculation and delete not-necessary variables
        3. fix shift-bit problem.
        4. fix commit messages for dt-binding
        5. fix dts gamma label.


zheng-yan.chen (3):
  dt-bindings: mediatek: Add gamma compatible for mt8195
  drm/mediatek: Add gamma lut support for mt8195
  arm64: dts: Modify gamma compatible for mt8195

 .../display/mediatek/mediatek,gamma.yaml      |   3 +-
 arch/arm64/boot/dts/mediatek/mt8195.dtsi      |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_aal.c       |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h       |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c     | 102 +++++++++++++-----
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   5 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h       |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        |   2 +
 10 files changed, 88 insertions(+), 34 deletions(-)

-- 
2.18.0

[PATCH v2 0/3] Add gamma lut support for mt8195

[zheng-yan.chen]

Since the gamma_set_common() function for previous SoC,
such as  mt8173 and mt8183, is designed for 9bit-to-10bit
conversion.
mt8195 is using 10bit-to-12bit conversion, which is
not compatible with the previous function.

Thus, need to update the function to fit the need of mt8195.

---
Base on series [1]:
[1]: Add driver nodes for MT8195 SoC
- https://patchwork.kernel.org/project/linux-mediatek/list/?series=666741
---

Change in v2:
        1. fix global variable in mtk_disp_gamma.c
        2. update mask calculation and delete not-necessary variables
        3. fix shift-bit problem.
        4. fix commit messages for dt-binding
        5. fix dts gamma label.


zheng-yan.chen (3):
  dt-bindings: mediatek: Add gamma compatible for mt8195
  drm/mediatek: Add gamma lut support for mt8195
  arm64: dts: Modify gamma compatible for mt8195

 .../display/mediatek/mediatek,gamma.yaml      |   3 +-
 arch/arm64/boot/dts/mediatek/mt8195.dtsi      |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_aal.c       |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h       |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c     | 102 +++++++++++++-----
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   5 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h       |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        |   2 +
 10 files changed, 88 insertions(+), 34 deletions(-)

-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[zheng-yan.chen]

mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
current 9bit-to-10bit gamma-LUT.

This patch thus add constant compatible for mt8195, which means that
mt8195 should only use specified mt8195 gamma driver data.

Also, delete related compatible from enum, to ensure that
mt8195 will not accidentally get others' gamma driver data and thus
cause fatal error.

Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
index a89ea0ea7542..fbd7b9664a78 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
@@ -25,11 +25,12 @@ properties:
           - const: mediatek,mt8173-disp-gamma
       - items:
           - const: mediatek,mt8183-disp-gamma
+      - items:
+          - const: mediatek,mt8195-disp-gamma
       - items:
           - enum:
               - mediatek,mt8186-disp-gamma
               - mediatek,mt8192-disp-gamma
-              - mediatek,mt8195-disp-gamma
           - const: mediatek,mt8183-disp-gamma
 
   reg:
-- 
2.18.0

[PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[zheng-yan.chen]

mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
current 9bit-to-10bit gamma-LUT.

This patch thus add constant compatible for mt8195, which means that
mt8195 should only use specified mt8195 gamma driver data.

Also, delete related compatible from enum, to ensure that
mt8195 will not accidentally get others' gamma driver data and thus
cause fatal error.

Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
index a89ea0ea7542..fbd7b9664a78 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
@@ -25,11 +25,12 @@ properties:
           - const: mediatek,mt8173-disp-gamma
       - items:
           - const: mediatek,mt8183-disp-gamma
+      - items:
+          - const: mediatek,mt8195-disp-gamma
       - items:
           - enum:
               - mediatek,mt8186-disp-gamma
               - mediatek,mt8192-disp-gamma
-              - mediatek,mt8195-disp-gamma
           - const: mediatek,mt8183-disp-gamma
 
   reg:
-- 
2.18.0

[PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[zheng-yan.chen]

mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
current 9bit-to-10bit gamma-LUT.

This patch thus add constant compatible for mt8195, which means that
mt8195 should only use specified mt8195 gamma driver data.

Also, delete related compatible from enum, to ensure that
mt8195 will not accidentally get others' gamma driver data and thus
cause fatal error.

Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
index a89ea0ea7542..fbd7b9664a78 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
@@ -25,11 +25,12 @@ properties:
           - const: mediatek,mt8173-disp-gamma
       - items:
           - const: mediatek,mt8183-disp-gamma
+      - items:
+          - const: mediatek,mt8195-disp-gamma
       - items:
           - enum:
               - mediatek,mt8186-disp-gamma
               - mediatek,mt8192-disp-gamma
-              - mediatek,mt8195-disp-gamma
           - const: mediatek,mt8183-disp-gamma
 
   reg:
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[zheng-yan.chen]

Since the previous gamma_set_common() function is designed for
9bit-to-10bit conversion, which is not feasible for the
10bit-to-12bit conversion in mt8195.

Update the function to fit the need of mt8195.

Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0 support for mt8195")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-----
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
 8 files changed, 85 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
 }
 
 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 33e61a136bbc..c1269fce9a66 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -51,8 +51,9 @@ 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_size(struct device *dev);
 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev);
 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 bbd558a036ec..0409e15fceb3 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -18,18 +18,22 @@
 #define DISP_GAMMA_EN				0x0000
 #define GAMMA_EN					BIT(0)
 #define DISP_GAMMA_CFG				0x0020
+#define RELAY_MODE					BIT(0)
 #define GAMMA_LUT_EN					BIT(1)
 #define GAMMA_DITHERING					BIT(2)
 #define DISP_GAMMA_SIZE				0x0030
+#define DISP_GAMMA_BANK				0x0100
 #define DISP_GAMMA_LUT				0x0700
-
-#define LUT_10BIT_MASK				0x03ff
-
+#define DISP_GAMMA_LUT1				0x0b00
+#define TABLE_9BIT_SIZE				512
+#define TABLE_10BIT_SIZE			1024
+#define BANK_SIZE				256
 struct mtk_disp_gamma_data {
 	bool has_dither;
 	bool lut_diff;
+	unsigned int lut_size;
+	unsigned int lut_bits;
 };
-
 /*
  * struct mtk_disp_gamma - DISP_GAMMA driver structure
  */
@@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev)
 {
-	unsigned int i, reg;
-	struct drm_color_lut *lut;
+	unsigned int i, reg, idx;
 	void __iomem *lut_base;
-	u32 word;
-	u32 diff[3] = {0};
+	void __iomem *lut1_base;
+	u32 word, word1;
+	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
 
 	if (state->gamma_lut) {
+		u32 table_size;
+		u32 mask;
+		u32 lut_bits;
+		u32 shift_bits;
+		bool lut_diff;
+		struct drm_color_lut color, color_rec;
+		struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
+
+		table_size = gamma->data->lut_size;
+		lut_bits = gamma->data->lut_bits;
+		lut_diff = gamma->data->lut_diff;
+		shift_bits = (lut_bits == 12) ? 4 : 6;
+		mask = GENMASK(lut_bits - 1, 0);
 		reg = readl(regs + DISP_GAMMA_CFG);
+		reg = reg & ~RELAY_MODE;
 		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);
+		lut1_base = regs + DISP_GAMMA_LUT1;
+		for (i = 0; i < table_size; i++) {
+			color.red = (lut[i].red >> shift_bits) & mask;
+			color.green = (lut[i].green >> shift_bits) & mask;
+			color.blue = (lut[i].blue >> shift_bits) & mask;
+			if (lut_diff && (i % 2)) {
+				word = (lut_bits == 12) ?
+						      (((color.green - color_rec.green) << 12) +
+						      (color.red - color_rec.red))
+							:
+						      (((color.red - color_rec.red) << 20) +
+						      ((color.green - color_rec.green) << 10) +
+						      (color.blue - color_rec.blue));
+				word1 = (color.blue - color_rec.blue);
 			} 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);
+				word = (lut_bits == 12) ?
+						      ((color.green << 12) + color.red)
+							:
+						      ((color.red << 20) +
+						      (color.green << 10) + color.blue);
+				word1 = color.blue;
+				color_rec = color;
 			}
-			writel(word, (lut_base + i * 4));
+			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
+			writel(word, (lut_base + idx * 4));
+			if (!(i % BANK_SIZE) && lut_bits == 12)
+				writel((i / BANK_SIZE), regs + DISP_GAMMA_BANK);
+			if (lut_bits == 12)
+				writel(word1, (lut1_base + idx * 4));
 		}
 	}
 }
 
+unsigned int mtk_gamma_size(struct device *dev)
+{
+	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
+
+	if (gamma->data)
+		return gamma->data->lut_size;
+	else
+		return 0;
+}
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 {
 	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
@@ -95,8 +134,7 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 
 	if (gamma->data)
 		lut_diff = gamma->data->lut_diff;
-
-	mtk_gamma_set_common(gamma->regs, state, lut_diff);
+	mtk_gamma_set_common(gamma->regs, state, dev);
 }
 
 void mtk_gamma_config(struct device *dev, unsigned int w,
@@ -191,10 +229,20 @@ 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,
+	.lut_bits = 10,
 };
 
 static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
 	.lut_diff = true,
+	.lut_size = 512,
+	.lut_bits = 10,
+};
+
+static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
+	.lut_diff = true,
+	.lut_size = 1024,
+	.lut_bits = 12,
 };
 
 static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
@@ -202,6 +250,8 @@ static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
 	  .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);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 42cc7052b050..2a6513259562 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -930,9 +930,8 @@ 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_size)
+				gamma_lut_size = comp->funcs->gamma_size(comp->dev);
 			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 cb9a36c48d4f..1799853ef89a 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 2d72cc5ddaba..4c6538a17b88 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma = {
 	.clk_enable = mtk_gamma_clk_enable,
 	.clk_disable = mtk_gamma_clk_disable,
 	.gamma_set = mtk_gamma_set,
+	.gamma_size = mtk_gamma_size,
 	.config = mtk_gamma_config,
 	.start = mtk_gamma_start,
 	.stop = mtk_gamma_stop,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 2d0052c23dcb..bf0cf7f86010 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
 	void (*disable_vblank)(struct device *dev);
 	unsigned int (*supported_rotations)(struct device *dev);
 	unsigned int (*layer_nr)(struct device *dev);
+	unsigned int (*gamma_size)(struct device *dev);
 	int (*layer_check)(struct device *dev,
 			   unsigned int idx,
 			   struct mtk_plane_state *state);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 0e4c77724b05..473766be56e1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -567,6 +567,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
 	  .data = (void *)MTK_DISP_GAMMA, },
 	{ .compatible = "mediatek,mt8183-disp-gamma",
 	  .data = (void *)MTK_DISP_GAMMA, },
+	{ .compatible = "mediatek,mt8195-disp-gamma",
+	  .data = (void *)MTK_DISP_GAMMA, },
 	{ .compatible = "mediatek,mt8195-disp-merge",
 	  .data = (void *)MTK_DISP_MERGE },
 	{ .compatible = "mediatek,mt2701-disp-mutex",
-- 
2.18.0

[PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[zheng-yan.chen]

Since the previous gamma_set_common() function is designed for
9bit-to-10bit conversion, which is not feasible for the
10bit-to-12bit conversion in mt8195.

Update the function to fit the need of mt8195.

Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0 support for mt8195")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-----
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
 8 files changed, 85 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
 }
 
 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 33e61a136bbc..c1269fce9a66 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -51,8 +51,9 @@ 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_size(struct device *dev);
 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev);
 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 bbd558a036ec..0409e15fceb3 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -18,18 +18,22 @@
 #define DISP_GAMMA_EN				0x0000
 #define GAMMA_EN					BIT(0)
 #define DISP_GAMMA_CFG				0x0020
+#define RELAY_MODE					BIT(0)
 #define GAMMA_LUT_EN					BIT(1)
 #define GAMMA_DITHERING					BIT(2)
 #define DISP_GAMMA_SIZE				0x0030
+#define DISP_GAMMA_BANK				0x0100
 #define DISP_GAMMA_LUT				0x0700
-
-#define LUT_10BIT_MASK				0x03ff
-
+#define DISP_GAMMA_LUT1				0x0b00
+#define TABLE_9BIT_SIZE				512
+#define TABLE_10BIT_SIZE			1024
+#define BANK_SIZE				256
 struct mtk_disp_gamma_data {
 	bool has_dither;
 	bool lut_diff;
+	unsigned int lut_size;
+	unsigned int lut_bits;
 };
-
 /*
  * struct mtk_disp_gamma - DISP_GAMMA driver structure
  */
@@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev)
 {
-	unsigned int i, reg;
-	struct drm_color_lut *lut;
+	unsigned int i, reg, idx;
 	void __iomem *lut_base;
-	u32 word;
-	u32 diff[3] = {0};
+	void __iomem *lut1_base;
+	u32 word, word1;
+	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
 
 	if (state->gamma_lut) {
+		u32 table_size;
+		u32 mask;
+		u32 lut_bits;
+		u32 shift_bits;
+		bool lut_diff;
+		struct drm_color_lut color, color_rec;
+		struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
+
+		table_size = gamma->data->lut_size;
+		lut_bits = gamma->data->lut_bits;
+		lut_diff = gamma->data->lut_diff;
+		shift_bits = (lut_bits == 12) ? 4 : 6;
+		mask = GENMASK(lut_bits - 1, 0);
 		reg = readl(regs + DISP_GAMMA_CFG);
+		reg = reg & ~RELAY_MODE;
 		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);
+		lut1_base = regs + DISP_GAMMA_LUT1;
+		for (i = 0; i < table_size; i++) {
+			color.red = (lut[i].red >> shift_bits) & mask;
+			color.green = (lut[i].green >> shift_bits) & mask;
+			color.blue = (lut[i].blue >> shift_bits) & mask;
+			if (lut_diff && (i % 2)) {
+				word = (lut_bits == 12) ?
+						      (((color.green - color_rec.green) << 12) +
+						      (color.red - color_rec.red))
+							:
+						      (((color.red - color_rec.red) << 20) +
+						      ((color.green - color_rec.green) << 10) +
+						      (color.blue - color_rec.blue));
+				word1 = (color.blue - color_rec.blue);
 			} 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);
+				word = (lut_bits == 12) ?
+						      ((color.green << 12) + color.red)
+							:
+						      ((color.red << 20) +
+						      (color.green << 10) + color.blue);
+				word1 = color.blue;
+				color_rec = color;
 			}
-			writel(word, (lut_base + i * 4));
+			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
+			writel(word, (lut_base + idx * 4));
+			if (!(i % BANK_SIZE) && lut_bits == 12)
+				writel((i / BANK_SIZE), regs + DISP_GAMMA_BANK);
+			if (lut_bits == 12)
+				writel(word1, (lut1_base + idx * 4));
 		}
 	}
 }
 
+unsigned int mtk_gamma_size(struct device *dev)
+{
+	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
+
+	if (gamma->data)
+		return gamma->data->lut_size;
+	else
+		return 0;
+}
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 {
 	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
@@ -95,8 +134,7 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 
 	if (gamma->data)
 		lut_diff = gamma->data->lut_diff;
-
-	mtk_gamma_set_common(gamma->regs, state, lut_diff);
+	mtk_gamma_set_common(gamma->regs, state, dev);
 }
 
 void mtk_gamma_config(struct device *dev, unsigned int w,
@@ -191,10 +229,20 @@ 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,
+	.lut_bits = 10,
 };
 
 static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
 	.lut_diff = true,
+	.lut_size = 512,
+	.lut_bits = 10,
+};
+
+static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
+	.lut_diff = true,
+	.lut_size = 1024,
+	.lut_bits = 12,
 };
 
 static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
@@ -202,6 +250,8 @@ static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
 	  .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);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 42cc7052b050..2a6513259562 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -930,9 +930,8 @@ 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_size)
+				gamma_lut_size = comp->funcs->gamma_size(comp->dev);
 			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 cb9a36c48d4f..1799853ef89a 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 2d72cc5ddaba..4c6538a17b88 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma = {
 	.clk_enable = mtk_gamma_clk_enable,
 	.clk_disable = mtk_gamma_clk_disable,
 	.gamma_set = mtk_gamma_set,
+	.gamma_size = mtk_gamma_size,
 	.config = mtk_gamma_config,
 	.start = mtk_gamma_start,
 	.stop = mtk_gamma_stop,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 2d0052c23dcb..bf0cf7f86010 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
 	void (*disable_vblank)(struct device *dev);
 	unsigned int (*supported_rotations)(struct device *dev);
 	unsigned int (*layer_nr)(struct device *dev);
+	unsigned int (*gamma_size)(struct device *dev);
 	int (*layer_check)(struct device *dev,
 			   unsigned int idx,
 			   struct mtk_plane_state *state);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 0e4c77724b05..473766be56e1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -567,6 +567,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
 	  .data = (void *)MTK_DISP_GAMMA, },
 	{ .compatible = "mediatek,mt8183-disp-gamma",
 	  .data = (void *)MTK_DISP_GAMMA, },
+	{ .compatible = "mediatek,mt8195-disp-gamma",
+	  .data = (void *)MTK_DISP_GAMMA, },
 	{ .compatible = "mediatek,mt8195-disp-merge",
 	  .data = (void *)MTK_DISP_MERGE },
 	{ .compatible = "mediatek,mt2701-disp-mutex",
-- 
2.18.0

[PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[zheng-yan.chen]

Since the previous gamma_set_common() function is designed for
9bit-to-10bit conversion, which is not feasible for the
10bit-to-12bit conversion in mt8195.

Update the function to fit the need of mt8195.

Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0 support for mt8195")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
 drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-----
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
 8 files changed, 85 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
 }
 
 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 33e61a136bbc..c1269fce9a66 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -51,8 +51,9 @@ 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_size(struct device *dev);
 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev);
 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 bbd558a036ec..0409e15fceb3 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -18,18 +18,22 @@
 #define DISP_GAMMA_EN				0x0000
 #define GAMMA_EN					BIT(0)
 #define DISP_GAMMA_CFG				0x0020
+#define RELAY_MODE					BIT(0)
 #define GAMMA_LUT_EN					BIT(1)
 #define GAMMA_DITHERING					BIT(2)
 #define DISP_GAMMA_SIZE				0x0030
+#define DISP_GAMMA_BANK				0x0100
 #define DISP_GAMMA_LUT				0x0700
-
-#define LUT_10BIT_MASK				0x03ff
-
+#define DISP_GAMMA_LUT1				0x0b00
+#define TABLE_9BIT_SIZE				512
+#define TABLE_10BIT_SIZE			1024
+#define BANK_SIZE				256
 struct mtk_disp_gamma_data {
 	bool has_dither;
 	bool lut_diff;
+	unsigned int lut_size;
+	unsigned int lut_bits;
 };
-
 /*
  * struct mtk_disp_gamma - DISP_GAMMA driver structure
  */
@@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev)
 {
-	unsigned int i, reg;
-	struct drm_color_lut *lut;
+	unsigned int i, reg, idx;
 	void __iomem *lut_base;
-	u32 word;
-	u32 diff[3] = {0};
+	void __iomem *lut1_base;
+	u32 word, word1;
+	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
 
 	if (state->gamma_lut) {
+		u32 table_size;
+		u32 mask;
+		u32 lut_bits;
+		u32 shift_bits;
+		bool lut_diff;
+		struct drm_color_lut color, color_rec;
+		struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
+
+		table_size = gamma->data->lut_size;
+		lut_bits = gamma->data->lut_bits;
+		lut_diff = gamma->data->lut_diff;
+		shift_bits = (lut_bits == 12) ? 4 : 6;
+		mask = GENMASK(lut_bits - 1, 0);
 		reg = readl(regs + DISP_GAMMA_CFG);
+		reg = reg & ~RELAY_MODE;
 		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);
+		lut1_base = regs + DISP_GAMMA_LUT1;
+		for (i = 0; i < table_size; i++) {
+			color.red = (lut[i].red >> shift_bits) & mask;
+			color.green = (lut[i].green >> shift_bits) & mask;
+			color.blue = (lut[i].blue >> shift_bits) & mask;
+			if (lut_diff && (i % 2)) {
+				word = (lut_bits == 12) ?
+						      (((color.green - color_rec.green) << 12) +
+						      (color.red - color_rec.red))
+							:
+						      (((color.red - color_rec.red) << 20) +
+						      ((color.green - color_rec.green) << 10) +
+						      (color.blue - color_rec.blue));
+				word1 = (color.blue - color_rec.blue);
 			} 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);
+				word = (lut_bits == 12) ?
+						      ((color.green << 12) + color.red)
+							:
+						      ((color.red << 20) +
+						      (color.green << 10) + color.blue);
+				word1 = color.blue;
+				color_rec = color;
 			}
-			writel(word, (lut_base + i * 4));
+			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
+			writel(word, (lut_base + idx * 4));
+			if (!(i % BANK_SIZE) && lut_bits == 12)
+				writel((i / BANK_SIZE), regs + DISP_GAMMA_BANK);
+			if (lut_bits == 12)
+				writel(word1, (lut1_base + idx * 4));
 		}
 	}
 }
 
+unsigned int mtk_gamma_size(struct device *dev)
+{
+	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
+
+	if (gamma->data)
+		return gamma->data->lut_size;
+	else
+		return 0;
+}
 void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 {
 	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
@@ -95,8 +134,7 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
 
 	if (gamma->data)
 		lut_diff = gamma->data->lut_diff;
-
-	mtk_gamma_set_common(gamma->regs, state, lut_diff);
+	mtk_gamma_set_common(gamma->regs, state, dev);
 }
 
 void mtk_gamma_config(struct device *dev, unsigned int w,
@@ -191,10 +229,20 @@ 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,
+	.lut_bits = 10,
 };
 
 static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
 	.lut_diff = true,
+	.lut_size = 512,
+	.lut_bits = 10,
+};
+
+static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
+	.lut_diff = true,
+	.lut_size = 1024,
+	.lut_bits = 12,
 };
 
 static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
@@ -202,6 +250,8 @@ static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
 	  .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);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 42cc7052b050..2a6513259562 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -930,9 +930,8 @@ 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_size)
+				gamma_lut_size = comp->funcs->gamma_size(comp->dev);
 			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 cb9a36c48d4f..1799853ef89a 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 2d72cc5ddaba..4c6538a17b88 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma = {
 	.clk_enable = mtk_gamma_clk_enable,
 	.clk_disable = mtk_gamma_clk_disable,
 	.gamma_set = mtk_gamma_set,
+	.gamma_size = mtk_gamma_size,
 	.config = mtk_gamma_config,
 	.start = mtk_gamma_start,
 	.stop = mtk_gamma_stop,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 2d0052c23dcb..bf0cf7f86010 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
 	void (*disable_vblank)(struct device *dev);
 	unsigned int (*supported_rotations)(struct device *dev);
 	unsigned int (*layer_nr)(struct device *dev);
+	unsigned int (*gamma_size)(struct device *dev);
 	int (*layer_check)(struct device *dev,
 			   unsigned int idx,
 			   struct mtk_plane_state *state);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 0e4c77724b05..473766be56e1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -567,6 +567,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
 	  .data = (void *)MTK_DISP_GAMMA, },
 	{ .compatible = "mediatek,mt8183-disp-gamma",
 	  .data = (void *)MTK_DISP_GAMMA, },
+	{ .compatible = "mediatek,mt8195-disp-gamma",
+	  .data = (void *)MTK_DISP_GAMMA, },
 	{ .compatible = "mediatek,mt8195-disp-merge",
 	  .data = (void *)MTK_DISP_MERGE },
 	{ .compatible = "mediatek,mt2701-disp-mutex",
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

Modify gamma compatible for mt8195.

Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index a50ebb5d145f..d4110f6fac62 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -2022,7 +2022,7 @@
 		};
 
 		gamma0: gamma@1c006000 {
-			compatible = "mediatek,mt8195-disp-gamma", "mediatek,mt8183-disp-gamma";
+			compatible = "mediatek,mt8195-disp-gamma";
 			reg = <0 0x1c006000 0 0x1000>;
 			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
 			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
-- 
2.18.0

[PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

Modify gamma compatible for mt8195.

Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index a50ebb5d145f..d4110f6fac62 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -2022,7 +2022,7 @@
 		};
 
 		gamma0: gamma@1c006000 {
-			compatible = "mediatek,mt8195-disp-gamma", "mediatek,mt8183-disp-gamma";
+			compatible = "mediatek,mt8195-disp-gamma";
 			reg = <0 0x1c006000 0 0x1000>;
 			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
 			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
-- 
2.18.0

[PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

Modify gamma compatible for mt8195.

Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index a50ebb5d145f..d4110f6fac62 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -2022,7 +2022,7 @@
 		};
 
 		gamma0: gamma@1c006000 {
-			compatible = "mediatek,mt8195-disp-gamma", "mediatek,mt8183-disp-gamma";
+			compatible = "mediatek,mt8195-disp-gamma";
 			reg = <0 0x1c006000 0 0x1000>;
 			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
 			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> Since the previous gamma_set_common() function is designed for
> 9bit-to-10bit conversion, which is not feasible for the
> 10bit-to-12bit conversion in mt8195.
> 
> Update the function to fit the need of mt8195.
> 
> Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0 support for mt8195")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
>   drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
>   drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-----
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
>   8 files changed, 85 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
>   }
>   
>   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 33e61a136bbc..c1269fce9a66 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -51,8 +51,9 @@ 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_size(struct device *dev);
>   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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev);
>   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 bbd558a036ec..0409e15fceb3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -18,18 +18,22 @@
>   #define DISP_GAMMA_EN				0x0000
>   #define GAMMA_EN					BIT(0)
>   #define DISP_GAMMA_CFG				0x0020
> +#define RELAY_MODE					BIT(0)
>   #define GAMMA_LUT_EN					BIT(1)
>   #define GAMMA_DITHERING					BIT(2)
>   #define DISP_GAMMA_SIZE				0x0030
> +#define DISP_GAMMA_BANK				0x0100
>   #define DISP_GAMMA_LUT				0x0700
> -
> -#define LUT_10BIT_MASK				0x03ff
> -
> +#define DISP_GAMMA_LUT1				0x0b00

> +#define TABLE_9BIT_SIZE				512
> +#define TABLE_10BIT_SIZE			1024

Please remove these two unused definitions.

> +#define BANK_SIZE				256
>   struct mtk_disp_gamma_data {
>   	bool has_dither;
>   	bool lut_diff;
> +	unsigned int lut_size;
> +	unsigned int lut_bits;

We can reduce the size of this structure by using

	u16 lut_size;
	u8 lut_bits;

>   };
> -
>   /*
>    * struct mtk_disp_gamma - DISP_GAMMA driver structure
>    */
> @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev)
>   {
> -	unsigned int i, reg;
> -	struct drm_color_lut *lut;
> +	unsigned int i, reg, idx;
>   	void __iomem *lut_base;
> -	u32 word;
> -	u32 diff[3] = {0};
> +	void __iomem *lut1_base;
> +	u32 word, word1;
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);

Please reorder these definitions, (columns) longest first, shortest last

>   
>   	if (state->gamma_lut) {
> +		u32 table_size;
> +		u32 mask;
> +		u32 lut_bits;
> +		u32 shift_bits;
> +		bool lut_diff;
> +		struct drm_color_lut color, color_rec;
> +		struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;


struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
struct drm_color_lut color, color_rec;
u32 lut_bits, shift_bits;
u32 mask, table_size;
bool lut_diff;

> +
> +		table_size = gamma->data->lut_size;
> +		lut_bits = gamma->data->lut_bits;
> +		lut_diff = gamma->data->lut_diff;
> +		shift_bits = (lut_bits == 12) ? 4 : 6;
> +		mask = GENMASK(lut_bits - 1, 0);
>   		reg = readl(regs + DISP_GAMMA_CFG);
> +		reg = reg & ~RELAY_MODE;
>   		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);
> +		lut1_base = regs + DISP_GAMMA_LUT1;
> +		for (i = 0; i < table_size; i++) {
> +			color.red = (lut[i].red >> shift_bits) & mask;
> +			color.green = (lut[i].green >> shift_bits) & mask;
> +			color.blue = (lut[i].blue >> shift_bits) & mask;
> +			if (lut_diff && (i % 2)) {
> +				word = (lut_bits == 12) ?
> +						      (((color.green - color_rec.green) << 12) +
> +						      (color.red - color_rec.red))
> +							:
> +						      (((color.red - color_rec.red) << 20) +
> +						      ((color.green - color_rec.green) << 10) +
> +						      (color.blue - color_rec.blue));
> +				word1 = (color.blue - color_rec.blue);
>   			} 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);
> +				word = (lut_bits == 12) ?
> +						      ((color.green << 12) + color.red)
> +							:
> +						      ((color.red << 20) +
> +						      (color.green << 10) + color.blue);
> +				word1 = color.blue;
> +				color_rec = color;
>   			}
> -			writel(word, (lut_base + i * 4));
> +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> +			writel(word, (lut_base + idx * 4));
> +			if (!(i % BANK_SIZE) && lut_bits == 12)
> +				writel((i / BANK_SIZE), regs + DISP_GAMMA_BANK);
> +			if (lut_bits == 12)
> +				writel(word1, (lut1_base + idx * 4));
>   		}
>   	}
>   }
>   
> +unsigned int mtk_gamma_size(struct device *dev)

unsigned int mtk_gamma_get_lut_size(struct device *dev)

that looks a bit more readable and perfectly explains this function :-)

> +{
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +
> +	if (gamma->data)
> +		return gamma->data->lut_size;
> +	else
> +		return 0;
> +}
>   void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>   {
>   	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);

..snip..

> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 2d72cc5ddaba..4c6538a17b88 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma = {
>   	.clk_enable = mtk_gamma_clk_enable,
>   	.clk_disable = mtk_gamma_clk_disable,
>   	.gamma_set = mtk_gamma_set,
> +	.gamma_size = mtk_gamma_size,
>   	.config = mtk_gamma_config,
>   	.start = mtk_gamma_start,
>   	.stop = mtk_gamma_stop,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 2d0052c23dcb..bf0cf7f86010 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
>   	void (*disable_vblank)(struct device *dev);
>   	unsigned int (*supported_rotations)(struct device *dev);
>   	unsigned int (*layer_nr)(struct device *dev);
> +	unsigned int (*gamma_size)(struct device *dev);

	unsigned int (*gamma_get_lut_size)(struct device *dev);

>   	int (*layer_check)(struct device *dev,
>   			   unsigned int idx,
>   			   struct mtk_plane_state *state);

Regards,
Angelo

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> Since the previous gamma_set_common() function is designed for
> 9bit-to-10bit conversion, which is not feasible for the
> 10bit-to-12bit conversion in mt8195.
> 
> Update the function to fit the need of mt8195.
> 
> Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0 support for mt8195")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
>   drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
>   drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-----
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
>   8 files changed, 85 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
>   }
>   
>   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 33e61a136bbc..c1269fce9a66 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -51,8 +51,9 @@ 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_size(struct device *dev);
>   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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev);
>   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 bbd558a036ec..0409e15fceb3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -18,18 +18,22 @@
>   #define DISP_GAMMA_EN				0x0000
>   #define GAMMA_EN					BIT(0)
>   #define DISP_GAMMA_CFG				0x0020
> +#define RELAY_MODE					BIT(0)
>   #define GAMMA_LUT_EN					BIT(1)
>   #define GAMMA_DITHERING					BIT(2)
>   #define DISP_GAMMA_SIZE				0x0030
> +#define DISP_GAMMA_BANK				0x0100
>   #define DISP_GAMMA_LUT				0x0700
> -
> -#define LUT_10BIT_MASK				0x03ff
> -
> +#define DISP_GAMMA_LUT1				0x0b00

> +#define TABLE_9BIT_SIZE				512
> +#define TABLE_10BIT_SIZE			1024

Please remove these two unused definitions.

> +#define BANK_SIZE				256
>   struct mtk_disp_gamma_data {
>   	bool has_dither;
>   	bool lut_diff;
> +	unsigned int lut_size;
> +	unsigned int lut_bits;

We can reduce the size of this structure by using

	u16 lut_size;
	u8 lut_bits;

>   };
> -
>   /*
>    * struct mtk_disp_gamma - DISP_GAMMA driver structure
>    */
> @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev)
>   {
> -	unsigned int i, reg;
> -	struct drm_color_lut *lut;
> +	unsigned int i, reg, idx;
>   	void __iomem *lut_base;
> -	u32 word;
> -	u32 diff[3] = {0};
> +	void __iomem *lut1_base;
> +	u32 word, word1;
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);

Please reorder these definitions, (columns) longest first, shortest last

>   
>   	if (state->gamma_lut) {
> +		u32 table_size;
> +		u32 mask;
> +		u32 lut_bits;
> +		u32 shift_bits;
> +		bool lut_diff;
> +		struct drm_color_lut color, color_rec;
> +		struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;


struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
struct drm_color_lut color, color_rec;
u32 lut_bits, shift_bits;
u32 mask, table_size;
bool lut_diff;

> +
> +		table_size = gamma->data->lut_size;
> +		lut_bits = gamma->data->lut_bits;
> +		lut_diff = gamma->data->lut_diff;
> +		shift_bits = (lut_bits == 12) ? 4 : 6;
> +		mask = GENMASK(lut_bits - 1, 0);
>   		reg = readl(regs + DISP_GAMMA_CFG);
> +		reg = reg & ~RELAY_MODE;
>   		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);
> +		lut1_base = regs + DISP_GAMMA_LUT1;
> +		for (i = 0; i < table_size; i++) {
> +			color.red = (lut[i].red >> shift_bits) & mask;
> +			color.green = (lut[i].green >> shift_bits) & mask;
> +			color.blue = (lut[i].blue >> shift_bits) & mask;
> +			if (lut_diff && (i % 2)) {
> +				word = (lut_bits == 12) ?
> +						      (((color.green - color_rec.green) << 12) +
> +						      (color.red - color_rec.red))
> +							:
> +						      (((color.red - color_rec.red) << 20) +
> +						      ((color.green - color_rec.green) << 10) +
> +						      (color.blue - color_rec.blue));
> +				word1 = (color.blue - color_rec.blue);
>   			} 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);
> +				word = (lut_bits == 12) ?
> +						      ((color.green << 12) + color.red)
> +							:
> +						      ((color.red << 20) +
> +						      (color.green << 10) + color.blue);
> +				word1 = color.blue;
> +				color_rec = color;
>   			}
> -			writel(word, (lut_base + i * 4));
> +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> +			writel(word, (lut_base + idx * 4));
> +			if (!(i % BANK_SIZE) && lut_bits == 12)
> +				writel((i / BANK_SIZE), regs + DISP_GAMMA_BANK);
> +			if (lut_bits == 12)
> +				writel(word1, (lut1_base + idx * 4));
>   		}
>   	}
>   }
>   
> +unsigned int mtk_gamma_size(struct device *dev)

unsigned int mtk_gamma_get_lut_size(struct device *dev)

that looks a bit more readable and perfectly explains this function :-)

> +{
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +
> +	if (gamma->data)
> +		return gamma->data->lut_size;
> +	else
> +		return 0;
> +}
>   void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>   {
>   	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);

..snip..

> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 2d72cc5ddaba..4c6538a17b88 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma = {
>   	.clk_enable = mtk_gamma_clk_enable,
>   	.clk_disable = mtk_gamma_clk_disable,
>   	.gamma_set = mtk_gamma_set,
> +	.gamma_size = mtk_gamma_size,
>   	.config = mtk_gamma_config,
>   	.start = mtk_gamma_start,
>   	.stop = mtk_gamma_stop,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 2d0052c23dcb..bf0cf7f86010 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
>   	void (*disable_vblank)(struct device *dev);
>   	unsigned int (*supported_rotations)(struct device *dev);
>   	unsigned int (*layer_nr)(struct device *dev);
> +	unsigned int (*gamma_size)(struct device *dev);

	unsigned int (*gamma_get_lut_size)(struct device *dev);

>   	int (*layer_check)(struct device *dev,
>   			   unsigned int idx,
>   			   struct mtk_plane_state *state);

Regards,
Angelo

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> Since the previous gamma_set_common() function is designed for
> 9bit-to-10bit conversion, which is not feasible for the
> 10bit-to-12bit conversion in mt8195.
> 
> Update the function to fit the need of mt8195.
> 
> Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0 support for mt8195")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>   drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
>   drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
>   drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-----
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
>   8 files changed, 85 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
>   }
>   
>   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 33e61a136bbc..c1269fce9a66 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -51,8 +51,9 @@ 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_size(struct device *dev);
>   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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev);
>   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 bbd558a036ec..0409e15fceb3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -18,18 +18,22 @@
>   #define DISP_GAMMA_EN				0x0000
>   #define GAMMA_EN					BIT(0)
>   #define DISP_GAMMA_CFG				0x0020
> +#define RELAY_MODE					BIT(0)
>   #define GAMMA_LUT_EN					BIT(1)
>   #define GAMMA_DITHERING					BIT(2)
>   #define DISP_GAMMA_SIZE				0x0030
> +#define DISP_GAMMA_BANK				0x0100
>   #define DISP_GAMMA_LUT				0x0700
> -
> -#define LUT_10BIT_MASK				0x03ff
> -
> +#define DISP_GAMMA_LUT1				0x0b00

> +#define TABLE_9BIT_SIZE				512
> +#define TABLE_10BIT_SIZE			1024

Please remove these two unused definitions.

> +#define BANK_SIZE				256
>   struct mtk_disp_gamma_data {
>   	bool has_dither;
>   	bool lut_diff;
> +	unsigned int lut_size;
> +	unsigned int lut_bits;

We can reduce the size of this structure by using

	u16 lut_size;
	u8 lut_bits;

>   };
> -
>   /*
>    * struct mtk_disp_gamma - DISP_GAMMA driver structure
>    */
> @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state *state, struct device *dev)
>   {
> -	unsigned int i, reg;
> -	struct drm_color_lut *lut;
> +	unsigned int i, reg, idx;
>   	void __iomem *lut_base;
> -	u32 word;
> -	u32 diff[3] = {0};
> +	void __iomem *lut1_base;
> +	u32 word, word1;
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);

Please reorder these definitions, (columns) longest first, shortest last

>   
>   	if (state->gamma_lut) {
> +		u32 table_size;
> +		u32 mask;
> +		u32 lut_bits;
> +		u32 shift_bits;
> +		bool lut_diff;
> +		struct drm_color_lut color, color_rec;
> +		struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;


struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
struct drm_color_lut color, color_rec;
u32 lut_bits, shift_bits;
u32 mask, table_size;
bool lut_diff;

> +
> +		table_size = gamma->data->lut_size;
> +		lut_bits = gamma->data->lut_bits;
> +		lut_diff = gamma->data->lut_diff;
> +		shift_bits = (lut_bits == 12) ? 4 : 6;
> +		mask = GENMASK(lut_bits - 1, 0);
>   		reg = readl(regs + DISP_GAMMA_CFG);
> +		reg = reg & ~RELAY_MODE;
>   		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);
> +		lut1_base = regs + DISP_GAMMA_LUT1;
> +		for (i = 0; i < table_size; i++) {
> +			color.red = (lut[i].red >> shift_bits) & mask;
> +			color.green = (lut[i].green >> shift_bits) & mask;
> +			color.blue = (lut[i].blue >> shift_bits) & mask;
> +			if (lut_diff && (i % 2)) {
> +				word = (lut_bits == 12) ?
> +						      (((color.green - color_rec.green) << 12) +
> +						      (color.red - color_rec.red))
> +							:
> +						      (((color.red - color_rec.red) << 20) +
> +						      ((color.green - color_rec.green) << 10) +
> +						      (color.blue - color_rec.blue));
> +				word1 = (color.blue - color_rec.blue);
>   			} 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);
> +				word = (lut_bits == 12) ?
> +						      ((color.green << 12) + color.red)
> +							:
> +						      ((color.red << 20) +
> +						      (color.green << 10) + color.blue);
> +				word1 = color.blue;
> +				color_rec = color;
>   			}
> -			writel(word, (lut_base + i * 4));
> +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> +			writel(word, (lut_base + idx * 4));
> +			if (!(i % BANK_SIZE) && lut_bits == 12)
> +				writel((i / BANK_SIZE), regs + DISP_GAMMA_BANK);
> +			if (lut_bits == 12)
> +				writel(word1, (lut1_base + idx * 4));
>   		}
>   	}
>   }
>   
> +unsigned int mtk_gamma_size(struct device *dev)

unsigned int mtk_gamma_get_lut_size(struct device *dev)

that looks a bit more readable and perfectly explains this function :-)

> +{
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +
> +	if (gamma->data)
> +		return gamma->data->lut_size;
> +	else
> +		return 0;
> +}
>   void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>   {
>   	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);

..snip..

> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 2d72cc5ddaba..4c6538a17b88 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma = {
>   	.clk_enable = mtk_gamma_clk_enable,
>   	.clk_disable = mtk_gamma_clk_disable,
>   	.gamma_set = mtk_gamma_set,
> +	.gamma_size = mtk_gamma_size,
>   	.config = mtk_gamma_config,
>   	.start = mtk_gamma_start,
>   	.stop = mtk_gamma_stop,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 2d0052c23dcb..bf0cf7f86010 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
>   	void (*disable_vblank)(struct device *dev);
>   	unsigned int (*supported_rotations)(struct device *dev);
>   	unsigned int (*layer_nr)(struct device *dev);
> +	unsigned int (*gamma_size)(struct device *dev);

	unsigned int (*gamma_get_lut_size)(struct device *dev);

>   	int (*layer_check)(struct device *dev,
>   			   unsigned int idx,
>   			   struct mtk_plane_state *state);

Regards,
Angelo



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> current 9bit-to-10bit gamma-LUT.
> 
> This patch thus add constant compatible for mt8195, which means that
> mt8195 should only use specified mt8195 gamma driver data.
> 
> Also, delete related compatible from enum, to ensure that
> mt8195 will not accidentally get others' gamma driver data and thus
> cause fatal error.
> 
> Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> current 9bit-to-10bit gamma-LUT.
> 
> This patch thus add constant compatible for mt8195, which means that
> mt8195 should only use specified mt8195 gamma driver data.
> 
> Also, delete related compatible from enum, to ensure that
> mt8195 will not accidentally get others' gamma driver data and thus
> cause fatal error.
> 
> Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> current 9bit-to-10bit gamma-LUT.
> 
> This patch thus add constant compatible for mt8195, which means that
> mt8195 should only use specified mt8195 gamma driver data.
> 
> Also, delete related compatible from enum, to ensure that
> mt8195 will not accidentally get others' gamma driver data and thus
> cause fatal error.
> 
> Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> Modify gamma compatible for mt8195.
> 
> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> Modify gamma compatible for mt8195.
> 
> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> Modify gamma compatible for mt8195.
> 
> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 09:47 +0200, AngeloGioacchino Del Regno wrote:
> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> > Since the previous gamma_set_common() function is designed for
> > 9bit-to-10bit conversion, which is not feasible for the
> > 10bit-to-12bit conversion in mt8195.
> > 
> > Update the function to fit the need of mt8195.
> > 
> > Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> > support for mt8195")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >   drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
> >   drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
> >   drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102
> > +++++++++++++++-----
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
> >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
> >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
> >   drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
> >   8 files changed, 85 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
> >   }
> >   
> >   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 33e61a136bbc..c1269fce9a66 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -51,8 +51,9 @@ 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_size(struct device *dev);
> >   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(void __iomem *regs, struct
> > drm_crtc_state *state, struct device *dev);
> >   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 bbd558a036ec..0409e15fceb3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -18,18 +18,22 @@
> >   #define DISP_GAMMA_EN				0x0000
> >   #define GAMMA_EN					BIT(0)
> >   #define DISP_GAMMA_CFG				0x0020
> > +#define RELAY_MODE					BIT(0)
> >   #define GAMMA_LUT_EN					BIT(1)
> >   #define GAMMA_DITHERING					BIT(2)
> >   #define DISP_GAMMA_SIZE				0x0030
> > +#define DISP_GAMMA_BANK				0x0100
> >   #define DISP_GAMMA_LUT				0x0700
> > -
> > -#define LUT_10BIT_MASK				0x03ff
> > -
> > +#define DISP_GAMMA_LUT1				0x0b00
> > +#define TABLE_9BIT_SIZE				512
> > +#define TABLE_10BIT_SIZE			1024
> 
> Please remove these two unused definitions.
> 
> > +#define BANK_SIZE				256
> >   struct mtk_disp_gamma_data {
> >   	bool has_dither;
> >   	bool lut_diff;
> > +	unsigned int lut_size;
> > +	unsigned int lut_bits;
> 
> We can reduce the size of this structure by using
> 
> 	u16 lut_size;
> 	u8 lut_bits;
> 
> >   };

OK, I will fix it.
> > -
> >   /*
> >    * struct mtk_disp_gamma - DISP_GAMMA driver structure
> >    */
> > @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct
> > drm_crtc_state *state, struct device *dev)
> >   {
> > -	unsigned int i, reg;
> > -	struct drm_color_lut *lut;
> > +	unsigned int i, reg, idx;
> >   	void __iomem *lut_base;
> > -	u32 word;
> > -	u32 diff[3] = {0};
> > +	void __iomem *lut1_base;
> > +	u32 word, word1;
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> 
> Please reorder these definitions, (columns) longest first, shortest
> last
> 
OK, I will fix it.
> >   
> >   	if (state->gamma_lut) {
> > +		u32 table_size;
> > +		u32 mask;
> > +		u32 lut_bits;
> > +		u32 shift_bits;
> > +		bool lut_diff;
> > +		struct drm_color_lut color, color_rec;
> > +		struct drm_color_lut *lut = (struct drm_color_lut
> > *)state->gamma_lut;
> 
> 
> struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
> struct drm_color_lut color, color_rec;
> u32 lut_bits, shift_bits;
> u32 mask, table_size;
> bool lut_diff;
> 
> > +
> > +		table_size = gamma->data->lut_size;
> > +		lut_bits = gamma->data->lut_bits;
> > +		lut_diff = gamma->data->lut_diff;
> > +		shift_bits = (lut_bits == 12) ? 4 : 6;
> > +		mask = GENMASK(lut_bits - 1, 0);
> >   		reg = readl(regs + DISP_GAMMA_CFG);
> > +		reg = reg & ~RELAY_MODE;
> >   		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);
> > +		lut1_base = regs + DISP_GAMMA_LUT1;
> > +		for (i = 0; i < table_size; i++) {
> > +			color.red = (lut[i].red >> shift_bits) & mask;
> > +			color.green = (lut[i].green >> shift_bits) &
> > mask;
> > +			color.blue = (lut[i].blue >> shift_bits) &
> > mask;
> > +			if (lut_diff && (i % 2)) {
> > +				word = (lut_bits == 12) ?
> > +						      (((color.green -
> > color_rec.green) << 12) +
> > +						      (color.red -
> > color_rec.red))
> > +							:
> > +						      (((color.red -
> > color_rec.red) << 20) +
> > +						      ((color.green -
> > color_rec.green) << 10) +
> > +						      (color.blue -
> > color_rec.blue));
> > +				word1 = (color.blue - color_rec.blue);
> >   			} 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);
> > +				word = (lut_bits == 12) ?
> > +						      ((color.green <<
> > 12) + color.red)
> > +							:
> > +						      ((color.red <<
> > 20) +
> > +						      (color.green <<
> > 10) + color.blue);
> > +				word1 = color.blue;
> > +				color_rec = color;
> >   			}
> > -			writel(word, (lut_base + i * 4));
> > +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> > +			writel(word, (lut_base + idx * 4));
> > +			if (!(i % BANK_SIZE) && lut_bits == 12)
> > +				writel((i / BANK_SIZE), regs +
> > DISP_GAMMA_BANK);
> > +			if (lut_bits == 12)
> > +				writel(word1, (lut1_base + idx * 4));
> >   		}
> >   	}
> >   }
> >   
> > +unsigned int mtk_gamma_size(struct device *dev)
> 
> unsigned int mtk_gamma_get_lut_size(struct device *dev)
> 
> that looks a bit more readable and perfectly explains this function
> :-)

OK, I will fix it.

> 
> > +{
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> > +
> > +	if (gamma->data)
> > +		return gamma->data->lut_size;
> > +	else
> > +		return 0;
> > +}
> >   void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > *state)
> >   {
> >   	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> 
> ..snip..
> 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > index 2d72cc5ddaba..4c6538a17b88 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs
> > ddp_gamma = {
> >   	.clk_enable = mtk_gamma_clk_enable,
> >   	.clk_disable = mtk_gamma_clk_disable,
> >   	.gamma_set = mtk_gamma_set,
> > +	.gamma_size = mtk_gamma_size,
> >   	.config = mtk_gamma_config,
> >   	.start = mtk_gamma_start,
> >   	.stop = mtk_gamma_stop,
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index 2d0052c23dcb..bf0cf7f86010 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
> >   	void (*disable_vblank)(struct device *dev);
> >   	unsigned int (*supported_rotations)(struct device *dev);
> >   	unsigned int (*layer_nr)(struct device *dev);
> > +	unsigned int (*gamma_size)(struct device *dev);
> 
> 	unsigned int (*gamma_get_lut_size)(struct device *dev);
> 
> >   	int (*layer_check)(struct device *dev,
> >   			   unsigned int idx,
> >   			   struct mtk_plane_state *state);
> 
> Regards,
> Angelo
> 
> 

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 09:47 +0200, AngeloGioacchino Del Regno wrote:
> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> > Since the previous gamma_set_common() function is designed for
> > 9bit-to-10bit conversion, which is not feasible for the
> > 10bit-to-12bit conversion in mt8195.
> > 
> > Update the function to fit the need of mt8195.
> > 
> > Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> > support for mt8195")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >   drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
> >   drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
> >   drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102
> > +++++++++++++++-----
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
> >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
> >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
> >   drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
> >   8 files changed, 85 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
> >   }
> >   
> >   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 33e61a136bbc..c1269fce9a66 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -51,8 +51,9 @@ 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_size(struct device *dev);
> >   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(void __iomem *regs, struct
> > drm_crtc_state *state, struct device *dev);
> >   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 bbd558a036ec..0409e15fceb3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -18,18 +18,22 @@
> >   #define DISP_GAMMA_EN				0x0000
> >   #define GAMMA_EN					BIT(0)
> >   #define DISP_GAMMA_CFG				0x0020
> > +#define RELAY_MODE					BIT(0)
> >   #define GAMMA_LUT_EN					BIT(1)
> >   #define GAMMA_DITHERING					BIT(2)
> >   #define DISP_GAMMA_SIZE				0x0030
> > +#define DISP_GAMMA_BANK				0x0100
> >   #define DISP_GAMMA_LUT				0x0700
> > -
> > -#define LUT_10BIT_MASK				0x03ff
> > -
> > +#define DISP_GAMMA_LUT1				0x0b00
> > +#define TABLE_9BIT_SIZE				512
> > +#define TABLE_10BIT_SIZE			1024
> 
> Please remove these two unused definitions.
> 
> > +#define BANK_SIZE				256
> >   struct mtk_disp_gamma_data {
> >   	bool has_dither;
> >   	bool lut_diff;
> > +	unsigned int lut_size;
> > +	unsigned int lut_bits;
> 
> We can reduce the size of this structure by using
> 
> 	u16 lut_size;
> 	u8 lut_bits;
> 
> >   };

OK, I will fix it.
> > -
> >   /*
> >    * struct mtk_disp_gamma - DISP_GAMMA driver structure
> >    */
> > @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct
> > drm_crtc_state *state, struct device *dev)
> >   {
> > -	unsigned int i, reg;
> > -	struct drm_color_lut *lut;
> > +	unsigned int i, reg, idx;
> >   	void __iomem *lut_base;
> > -	u32 word;
> > -	u32 diff[3] = {0};
> > +	void __iomem *lut1_base;
> > +	u32 word, word1;
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> 
> Please reorder these definitions, (columns) longest first, shortest
> last
> 
OK, I will fix it.
> >   
> >   	if (state->gamma_lut) {
> > +		u32 table_size;
> > +		u32 mask;
> > +		u32 lut_bits;
> > +		u32 shift_bits;
> > +		bool lut_diff;
> > +		struct drm_color_lut color, color_rec;
> > +		struct drm_color_lut *lut = (struct drm_color_lut
> > *)state->gamma_lut;
> 
> 
> struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
> struct drm_color_lut color, color_rec;
> u32 lut_bits, shift_bits;
> u32 mask, table_size;
> bool lut_diff;
> 
> > +
> > +		table_size = gamma->data->lut_size;
> > +		lut_bits = gamma->data->lut_bits;
> > +		lut_diff = gamma->data->lut_diff;
> > +		shift_bits = (lut_bits == 12) ? 4 : 6;
> > +		mask = GENMASK(lut_bits - 1, 0);
> >   		reg = readl(regs + DISP_GAMMA_CFG);
> > +		reg = reg & ~RELAY_MODE;
> >   		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);
> > +		lut1_base = regs + DISP_GAMMA_LUT1;
> > +		for (i = 0; i < table_size; i++) {
> > +			color.red = (lut[i].red >> shift_bits) & mask;
> > +			color.green = (lut[i].green >> shift_bits) &
> > mask;
> > +			color.blue = (lut[i].blue >> shift_bits) &
> > mask;
> > +			if (lut_diff && (i % 2)) {
> > +				word = (lut_bits == 12) ?
> > +						      (((color.green -
> > color_rec.green) << 12) +
> > +						      (color.red -
> > color_rec.red))
> > +							:
> > +						      (((color.red -
> > color_rec.red) << 20) +
> > +						      ((color.green -
> > color_rec.green) << 10) +
> > +						      (color.blue -
> > color_rec.blue));
> > +				word1 = (color.blue - color_rec.blue);
> >   			} 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);
> > +				word = (lut_bits == 12) ?
> > +						      ((color.green <<
> > 12) + color.red)
> > +							:
> > +						      ((color.red <<
> > 20) +
> > +						      (color.green <<
> > 10) + color.blue);
> > +				word1 = color.blue;
> > +				color_rec = color;
> >   			}
> > -			writel(word, (lut_base + i * 4));
> > +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> > +			writel(word, (lut_base + idx * 4));
> > +			if (!(i % BANK_SIZE) && lut_bits == 12)
> > +				writel((i / BANK_SIZE), regs +
> > DISP_GAMMA_BANK);
> > +			if (lut_bits == 12)
> > +				writel(word1, (lut1_base + idx * 4));
> >   		}
> >   	}
> >   }
> >   
> > +unsigned int mtk_gamma_size(struct device *dev)
> 
> unsigned int mtk_gamma_get_lut_size(struct device *dev)
> 
> that looks a bit more readable and perfectly explains this function
> :-)

OK, I will fix it.

> 
> > +{
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> > +
> > +	if (gamma->data)
> > +		return gamma->data->lut_size;
> > +	else
> > +		return 0;
> > +}
> >   void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > *state)
> >   {
> >   	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> 
> ..snip..
> 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > index 2d72cc5ddaba..4c6538a17b88 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs
> > ddp_gamma = {
> >   	.clk_enable = mtk_gamma_clk_enable,
> >   	.clk_disable = mtk_gamma_clk_disable,
> >   	.gamma_set = mtk_gamma_set,
> > +	.gamma_size = mtk_gamma_size,
> >   	.config = mtk_gamma_config,
> >   	.start = mtk_gamma_start,
> >   	.stop = mtk_gamma_stop,
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index 2d0052c23dcb..bf0cf7f86010 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
> >   	void (*disable_vblank)(struct device *dev);
> >   	unsigned int (*supported_rotations)(struct device *dev);
> >   	unsigned int (*layer_nr)(struct device *dev);
> > +	unsigned int (*gamma_size)(struct device *dev);
> 
> 	unsigned int (*gamma_get_lut_size)(struct device *dev);
> 
> >   	int (*layer_check)(struct device *dev,
> >   			   unsigned int idx,
> >   			   struct mtk_plane_state *state);
> 
> Regards,
> Angelo
> 
> 

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 09:47 +0200, AngeloGioacchino Del Regno wrote:
> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> > Since the previous gamma_set_common() function is designed for
> > 9bit-to-10bit conversion, which is not feasible for the
> > 10bit-to-12bit conversion in mt8195.
> > 
> > Update the function to fit the need of mt8195.
> > 
> > Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> > support for mt8195")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >   drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
> >   drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
> >   drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102
> > +++++++++++++++-----
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
> >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
> >   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
> >   drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
> >   8 files changed, 85 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
> >   }
> >   
> >   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 33e61a136bbc..c1269fce9a66 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -51,8 +51,9 @@ 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_size(struct device *dev);
> >   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(void __iomem *regs, struct
> > drm_crtc_state *state, struct device *dev);
> >   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 bbd558a036ec..0409e15fceb3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -18,18 +18,22 @@
> >   #define DISP_GAMMA_EN				0x0000
> >   #define GAMMA_EN					BIT(0)
> >   #define DISP_GAMMA_CFG				0x0020
> > +#define RELAY_MODE					BIT(0)
> >   #define GAMMA_LUT_EN					BIT(1)
> >   #define GAMMA_DITHERING					BIT(2)
> >   #define DISP_GAMMA_SIZE				0x0030
> > +#define DISP_GAMMA_BANK				0x0100
> >   #define DISP_GAMMA_LUT				0x0700
> > -
> > -#define LUT_10BIT_MASK				0x03ff
> > -
> > +#define DISP_GAMMA_LUT1				0x0b00
> > +#define TABLE_9BIT_SIZE				512
> > +#define TABLE_10BIT_SIZE			1024
> 
> Please remove these two unused definitions.
> 
> > +#define BANK_SIZE				256
> >   struct mtk_disp_gamma_data {
> >   	bool has_dither;
> >   	bool lut_diff;
> > +	unsigned int lut_size;
> > +	unsigned int lut_bits;
> 
> We can reduce the size of this structure by using
> 
> 	u16 lut_size;
> 	u8 lut_bits;
> 
> >   };

OK, I will fix it.
> > -
> >   /*
> >    * struct mtk_disp_gamma - DISP_GAMMA driver structure
> >    */
> > @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct
> > drm_crtc_state *state, struct device *dev)
> >   {
> > -	unsigned int i, reg;
> > -	struct drm_color_lut *lut;
> > +	unsigned int i, reg, idx;
> >   	void __iomem *lut_base;
> > -	u32 word;
> > -	u32 diff[3] = {0};
> > +	void __iomem *lut1_base;
> > +	u32 word, word1;
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> 
> Please reorder these definitions, (columns) longest first, shortest
> last
> 
OK, I will fix it.
> >   
> >   	if (state->gamma_lut) {
> > +		u32 table_size;
> > +		u32 mask;
> > +		u32 lut_bits;
> > +		u32 shift_bits;
> > +		bool lut_diff;
> > +		struct drm_color_lut color, color_rec;
> > +		struct drm_color_lut *lut = (struct drm_color_lut
> > *)state->gamma_lut;
> 
> 
> struct drm_color_lut *lut = (struct drm_color_lut *)state->gamma_lut;
> struct drm_color_lut color, color_rec;
> u32 lut_bits, shift_bits;
> u32 mask, table_size;
> bool lut_diff;
> 
> > +
> > +		table_size = gamma->data->lut_size;
> > +		lut_bits = gamma->data->lut_bits;
> > +		lut_diff = gamma->data->lut_diff;
> > +		shift_bits = (lut_bits == 12) ? 4 : 6;
> > +		mask = GENMASK(lut_bits - 1, 0);
> >   		reg = readl(regs + DISP_GAMMA_CFG);
> > +		reg = reg & ~RELAY_MODE;
> >   		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);
> > +		lut1_base = regs + DISP_GAMMA_LUT1;
> > +		for (i = 0; i < table_size; i++) {
> > +			color.red = (lut[i].red >> shift_bits) & mask;
> > +			color.green = (lut[i].green >> shift_bits) &
> > mask;
> > +			color.blue = (lut[i].blue >> shift_bits) &
> > mask;
> > +			if (lut_diff && (i % 2)) {
> > +				word = (lut_bits == 12) ?
> > +						      (((color.green -
> > color_rec.green) << 12) +
> > +						      (color.red -
> > color_rec.red))
> > +							:
> > +						      (((color.red -
> > color_rec.red) << 20) +
> > +						      ((color.green -
> > color_rec.green) << 10) +
> > +						      (color.blue -
> > color_rec.blue));
> > +				word1 = (color.blue - color_rec.blue);
> >   			} 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);
> > +				word = (lut_bits == 12) ?
> > +						      ((color.green <<
> > 12) + color.red)
> > +							:
> > +						      ((color.red <<
> > 20) +
> > +						      (color.green <<
> > 10) + color.blue);
> > +				word1 = color.blue;
> > +				color_rec = color;
> >   			}
> > -			writel(word, (lut_base + i * 4));
> > +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> > +			writel(word, (lut_base + idx * 4));
> > +			if (!(i % BANK_SIZE) && lut_bits == 12)
> > +				writel((i / BANK_SIZE), regs +
> > DISP_GAMMA_BANK);
> > +			if (lut_bits == 12)
> > +				writel(word1, (lut1_base + idx * 4));
> >   		}
> >   	}
> >   }
> >   
> > +unsigned int mtk_gamma_size(struct device *dev)
> 
> unsigned int mtk_gamma_get_lut_size(struct device *dev)
> 
> that looks a bit more readable and perfectly explains this function
> :-)

OK, I will fix it.

> 
> > +{
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> > +
> > +	if (gamma->data)
> > +		return gamma->data->lut_size;
> > +	else
> > +		return 0;
> > +}
> >   void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> > *state)
> >   {
> >   	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> 
> ..snip..
> 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > index 2d72cc5ddaba..4c6538a17b88 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs
> > ddp_gamma = {
> >   	.clk_enable = mtk_gamma_clk_enable,
> >   	.clk_disable = mtk_gamma_clk_disable,
> >   	.gamma_set = mtk_gamma_set,
> > +	.gamma_size = mtk_gamma_size,
> >   	.config = mtk_gamma_config,
> >   	.start = mtk_gamma_start,
> >   	.stop = mtk_gamma_stop,
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index 2d0052c23dcb..bf0cf7f86010 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
> >   	void (*disable_vblank)(struct device *dev);
> >   	unsigned int (*supported_rotations)(struct device *dev);
> >   	unsigned int (*layer_nr)(struct device *dev);
> > +	unsigned int (*gamma_size)(struct device *dev);
> 
> 	unsigned int (*gamma_get_lut_size)(struct device *dev);
> 
> >   	int (*layer_check)(struct device *dev,
> >   			   unsigned int idx,
> >   			   struct mtk_plane_state *state);
> 
> Regards,
> Angelo
> 
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 09:39, zheng-yan.chen wrote:
> mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> current 9bit-to-10bit gamma-LUT.
> 
> This patch thus add constant compatible for mt8195, which means that
> mt8195 should only use specified mt8195 gamma driver data.
> 
> Also, delete related compatible from enum, to ensure that
> mt8195 will not accidentally get others' gamma driver data and thus
> cause fatal error.
> 
> Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> index a89ea0ea7542..fbd7b9664a78 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> @@ -25,11 +25,12 @@ properties:
>            - const: mediatek,mt8173-disp-gamma
>        - items:
>            - const: mediatek,mt8183-disp-gamma
> +      - items:
> +          - const: mediatek,mt8195-disp-gamma

This is one item, so not a list. With all such cases this should be
dedicated enum.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 09:39, zheng-yan.chen wrote:
> mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> current 9bit-to-10bit gamma-LUT.
> 
> This patch thus add constant compatible for mt8195, which means that
> mt8195 should only use specified mt8195 gamma driver data.
> 
> Also, delete related compatible from enum, to ensure that
> mt8195 will not accidentally get others' gamma driver data and thus
> cause fatal error.
> 
> Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> index a89ea0ea7542..fbd7b9664a78 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> @@ -25,11 +25,12 @@ properties:
>            - const: mediatek,mt8173-disp-gamma
>        - items:
>            - const: mediatek,mt8183-disp-gamma
> +      - items:
> +          - const: mediatek,mt8195-disp-gamma

This is one item, so not a list. With all such cases this should be
dedicated enum.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 09:39, zheng-yan.chen wrote:
> mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> current 9bit-to-10bit gamma-LUT.
> 
> This patch thus add constant compatible for mt8195, which means that
> mt8195 should only use specified mt8195 gamma driver data.
> 
> Also, delete related compatible from enum, to ensure that
> mt8195 will not accidentally get others' gamma driver data and thus
> cause fatal error.
> 
> Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195 SoC binding for vdosys0")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> index a89ea0ea7542..fbd7b9664a78 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml
> @@ -25,11 +25,12 @@ properties:
>            - const: mediatek,mt8173-disp-gamma
>        - items:
>            - const: mediatek,mt8183-disp-gamma
> +      - items:
> +          - const: mediatek,mt8195-disp-gamma

This is one item, so not a list. With all such cases this should be
dedicated enum.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 09:39, zheng-yan.chen wrote:
> Modify gamma compatible for mt8195.

Commit should explain why. "What" we all can easily see.
> 
> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")

Your patchset is not bisectable and might cause ABI break. I doubt that
it matches the criteria of backports, especially that you did not
describe here bug being fixed.

Drop the tag and explain reasons for ABI break.

> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index a50ebb5d145f..d4110f6fac62 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -2022,7 +2022,7 @@
>  		};
>  
>  		gamma0: gamma@1c006000 {
> -			compatible = "mediatek,mt8195-disp-gamma", "mediatek,mt8183-disp-gamma";
> +			compatible = "mediatek,mt8195-disp-gamma";
>  			reg = <0 0x1c006000 0 0x1000>;
>  			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
>  			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;


Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 09:39, zheng-yan.chen wrote:
> Modify gamma compatible for mt8195.

Commit should explain why. "What" we all can easily see.
> 
> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")

Your patchset is not bisectable and might cause ABI break. I doubt that
it matches the criteria of backports, especially that you did not
describe here bug being fixed.

Drop the tag and explain reasons for ABI break.

> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index a50ebb5d145f..d4110f6fac62 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -2022,7 +2022,7 @@
>  		};
>  
>  		gamma0: gamma@1c006000 {
> -			compatible = "mediatek,mt8195-disp-gamma", "mediatek,mt8183-disp-gamma";
> +			compatible = "mediatek,mt8195-disp-gamma";
>  			reg = <0 0x1c006000 0 0x1000>;
>  			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
>  			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;


Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 09:39, zheng-yan.chen wrote:
> Modify gamma compatible for mt8195.

Commit should explain why. "What" we all can easily see.
> 
> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")

Your patchset is not bisectable and might cause ABI break. I doubt that
it matches the criteria of backports, especially that you did not
describe here bug being fixed.

Drop the tag and explain reasons for ABI break.

> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index a50ebb5d145f..d4110f6fac62 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -2022,7 +2022,7 @@
>  		};
>  
>  		gamma0: gamma@1c006000 {
> -			compatible = "mediatek,mt8195-disp-gamma", "mediatek,mt8183-disp-gamma";
> +			compatible = "mediatek,mt8195-disp-gamma";
>  			reg = <0 0x1c006000 0 0x1000>;
>  			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH 0>;
>  			power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>> Modify gamma compatible for mt8195.
>>
>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Please also perform review on the commit msg and backport status.

Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>> Modify gamma compatible for mt8195.
>>
>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Please also perform review on the commit msg and backport status.

Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>> Modify gamma compatible for mt8195.
>>
>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for vdosys0")
>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Please also perform review on the commit msg and backport status.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:13 +0300, Krzysztof Kozlowski wrote:
> > 
> On 30/08/2022 09:39, zheng-yan.chen wrote:
> > Modify gamma compatible for mt8195.
> 
> Commit should explain why. "What" we all can easily see.
> > Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
> > vdosys0")
> 
Hello Krzysztof,
Thanks for the review, 

I will edit commit message below:

Modify gamma compatible for mt8195.

    This modification is because of that
    mt8183 gamma driver data is not compatible with mt8195 gamma.

    Thus, need to delete mt8183 gamma compatible from mt8195 gamma.

> Your patchset is not bisectable and might cause ABI break. I doubt
> that
> it matches the criteria of backports, especially that you did not
> describe here bug being fixed.
> 
> Drop the tag and explain reasons for ABI break.
> 

I will drop the tag and add based-on messages below. 
Base on [1]:
    [1] arm64: dts: mt8195: Add display node for vdosys0
    - 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220811025813.21492-21-tinghan.shen@mediatek.com/

Best Regards,
Zheng-Yan Chen.
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index a50ebb5d145f..d4110f6fac62 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -2022,7 +2022,7 @@
> >  		};
> >  
> >  		gamma0: gamma@1c006000 {
> > -			compatible = "mediatek,mt8195-disp-gamma",
> > "mediatek,mt8183-disp-gamma";
> > +			compatible = "mediatek,mt8195-disp-gamma";
> >  			reg = <0 0x1c006000 0 0x1000>;
> >  			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH
> > 0>;
> >  			power-domains = <&spm
> > MT8195_POWER_DOMAIN_VDOSYS0>;
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:13 +0300, Krzysztof Kozlowski wrote:
> > 
> On 30/08/2022 09:39, zheng-yan.chen wrote:
> > Modify gamma compatible for mt8195.
> 
> Commit should explain why. "What" we all can easily see.
> > Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
> > vdosys0")
> 
Hello Krzysztof,
Thanks for the review, 

I will edit commit message below:

Modify gamma compatible for mt8195.

    This modification is because of that
    mt8183 gamma driver data is not compatible with mt8195 gamma.

    Thus, need to delete mt8183 gamma compatible from mt8195 gamma.

> Your patchset is not bisectable and might cause ABI break. I doubt
> that
> it matches the criteria of backports, especially that you did not
> describe here bug being fixed.
> 
> Drop the tag and explain reasons for ABI break.
> 

I will drop the tag and add based-on messages below. 
Base on [1]:
    [1] arm64: dts: mt8195: Add display node for vdosys0
    - 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220811025813.21492-21-tinghan.shen@mediatek.com/

Best Regards,
Zheng-Yan Chen.
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index a50ebb5d145f..d4110f6fac62 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -2022,7 +2022,7 @@
> >  		};
> >  
> >  		gamma0: gamma@1c006000 {
> > -			compatible = "mediatek,mt8195-disp-gamma",
> > "mediatek,mt8183-disp-gamma";
> > +			compatible = "mediatek,mt8195-disp-gamma";
> >  			reg = <0 0x1c006000 0 0x1000>;
> >  			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH
> > 0>;
> >  			power-domains = <&spm
> > MT8195_POWER_DOMAIN_VDOSYS0>;
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:13 +0300, Krzysztof Kozlowski wrote:
> > 
> On 30/08/2022 09:39, zheng-yan.chen wrote:
> > Modify gamma compatible for mt8195.
> 
> Commit should explain why. "What" we all can easily see.
> > Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
> > vdosys0")
> 
Hello Krzysztof,
Thanks for the review, 

I will edit commit message below:

Modify gamma compatible for mt8195.

    This modification is because of that
    mt8183 gamma driver data is not compatible with mt8195 gamma.

    Thus, need to delete mt8183 gamma compatible from mt8195 gamma.

> Your patchset is not bisectable and might cause ABI break. I doubt
> that
> it matches the criteria of backports, especially that you did not
> describe here bug being fixed.
> 
> Drop the tag and explain reasons for ABI break.
> 

I will drop the tag and add based-on messages below. 
Base on [1]:
    [1] arm64: dts: mt8195: Add display node for vdosys0
    - 
https://patchwork.kernel.org/project/linux-mediatek/patch/20220811025813.21492-21-tinghan.shen@mediatek.com/

Best Regards,
Zheng-Yan Chen.
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8195.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index a50ebb5d145f..d4110f6fac62 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -2022,7 +2022,7 @@
> >  		};
> >  
> >  		gamma0: gamma@1c006000 {
> > -			compatible = "mediatek,mt8195-disp-gamma",
> > "mediatek,mt8183-disp-gamma";
> > +			compatible = "mediatek,mt8195-disp-gamma";
> >  			reg = <0 0x1c006000 0 0x1000>;
> >  			interrupts = <GIC_SPI 642 IRQ_TYPE_LEVEL_HIGH
> > 0>;
> >  			power-domains = <&spm
> > MT8195_POWER_DOMAIN_VDOSYS0>;
> 
> 
> Best regards,
> Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:10 +0300, Krzysztof Kozlowski wrote:
> On 30/08/2022 09:39, zheng-yan.chen wrote:
> > mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> > current 9bit-to-10bit gamma-LUT.
> > 
> > This patch thus add constant compatible for mt8195, which means
> > that
> > mt8195 should only use specified mt8195 gamma driver data.
> > 
> > Also, delete related compatible from enum, to ensure that
> > mt8195 will not accidentally get others' gamma driver data and thus
> > cause fatal error.
> > 
> > Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195
> > SoC binding for vdosys0")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3
> > ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > index a89ea0ea7542..fbd7b9664a78 100644
> > ---
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > +++
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > @@ -25,11 +25,12 @@ properties:
> >            - const: mediatek,mt8173-disp-gamma
> >        - items:
> >            - const: mediatek,mt8183-disp-gamma
> > +      - items:
> > +          - const: mediatek,mt8195-disp-gamma
> 
> This is one item, so not a list. With all such cases this should be
> dedicated enum.
> 
> Best regards,
> Krzysztof
Hello Krzysztof, 
Thanks for the review,

I will fix it at the next version.

Best Regards,
Zheng-Yan Chen.

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:10 +0300, Krzysztof Kozlowski wrote:
> On 30/08/2022 09:39, zheng-yan.chen wrote:
> > mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> > current 9bit-to-10bit gamma-LUT.
> > 
> > This patch thus add constant compatible for mt8195, which means
> > that
> > mt8195 should only use specified mt8195 gamma driver data.
> > 
> > Also, delete related compatible from enum, to ensure that
> > mt8195 will not accidentally get others' gamma driver data and thus
> > cause fatal error.
> > 
> > Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195
> > SoC binding for vdosys0")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3
> > ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > index a89ea0ea7542..fbd7b9664a78 100644
> > ---
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > +++
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > @@ -25,11 +25,12 @@ properties:
> >            - const: mediatek,mt8173-disp-gamma
> >        - items:
> >            - const: mediatek,mt8183-disp-gamma
> > +      - items:
> > +          - const: mediatek,mt8195-disp-gamma
> 
> This is one item, so not a list. With all such cases this should be
> dedicated enum.
> 
> Best regards,
> Krzysztof
Hello Krzysztof, 
Thanks for the review,

I will fix it at the next version.

Best Regards,
Zheng-Yan Chen.

Re: [PATCH v2 1/3] dt-bindings: mediatek: Add gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:10 +0300, Krzysztof Kozlowski wrote:
> On 30/08/2022 09:39, zheng-yan.chen wrote:
> > mt8195 uses 10bit-to-12bit gamma-LUT, which is not compatible with
> > current 9bit-to-10bit gamma-LUT.
> > 
> > This patch thus add constant compatible for mt8195, which means
> > that
> > mt8195 should only use specified mt8195 gamma driver data.
> > 
> > Also, delete related compatible from enum, to ensure that
> > mt8195 will not accidentally get others' gamma driver data and thus
> > cause fatal error.
> > 
> > Fixes: a79257bae9bf ("dt-bindings: display: mediatek: add mt8195
> > SoC binding for vdosys0")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  .../devicetree/bindings/display/mediatek/mediatek,gamma.yaml   | 3
> > ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > index a89ea0ea7542..fbd7b9664a78 100644
> > ---
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > +++
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma
> > .yaml
> > @@ -25,11 +25,12 @@ properties:
> >            - const: mediatek,mt8173-disp-gamma
> >        - items:
> >            - const: mediatek,mt8183-disp-gamma
> > +      - items:
> > +          - const: mediatek,mt8195-disp-gamma
> 
> This is one item, so not a list. With all such cases this should be
> dedicated enum.
> 
> Best regards,
> Krzysztof
Hello Krzysztof, 
Thanks for the review,

I will fix it at the next version.

Best Regards,
Zheng-Yan Chen.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
> > Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> > > Modify gamma compatible for mt8195.
> > > 
> > > Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
> > > vdosys0")
> > > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > 
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> 
> Please also perform review on the commit msg and backport status.
> 
> Best regards,
> Krzysztof
Hello Krzysztof, 
Thanks for the review,
I will fix it at the next version.

Best Regards,
Zheng-Yan Chen.

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
> > Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> > > Modify gamma compatible for mt8195.
> > > 
> > > Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
> > > vdosys0")
> > > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > 
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> 
> Please also perform review on the commit msg and backport status.
> 
> Best regards,
> Krzysztof
Hello Krzysztof, 
Thanks for the review,
I will fix it at the next version.

Best Regards,
Zheng-Yan Chen.

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[zheng-yan.chen]

On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
> > Il 30/08/22 08:39, zheng-yan.chen ha scritto:
> > > Modify gamma compatible for mt8195.
> > > 
> > > Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
> > > vdosys0")
> > > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > 
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> 
> Please also perform review on the commit msg and backport status.
> 
> Best regards,
> Krzysztof
Hello Krzysztof, 
Thanks for the review,
I will fix it at the next version.

Best Regards,
Zheng-Yan Chen.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[CK Hu]

Hi, Zheng-yan:

On Tue, 2022-08-30 at 14:39 +0800, zheng-yan.chen wrote:
> Since the previous gamma_set_common() function is designed for
> 9bit-to-10bit conversion, which is not feasible for the
> 10bit-to-12bit conversion in mt8195.
> 
> Update the function to fit the need of mt8195.
> 
> Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> support for mt8195")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++---
> --
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
>  8 files changed, 85 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
>  }
>  
>  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 33e61a136bbc..c1269fce9a66 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -51,8 +51,9 @@ 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_size(struct device *dev);
>  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(void __iomem *regs, struct drm_crtc_state
> *state, struct device *dev);
>  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 bbd558a036ec..0409e15fceb3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -18,18 +18,22 @@
>  #define DISP_GAMMA_EN				0x0000
>  #define GAMMA_EN					BIT(0)
>  #define DISP_GAMMA_CFG				0x0020
> +#define RELAY_MODE					BIT(0)
>  #define GAMMA_LUT_EN					BIT(1)
>  #define GAMMA_DITHERING					BIT(2)
>  #define DISP_GAMMA_SIZE				0x0030
> +#define DISP_GAMMA_BANK				0x0100
>  #define DISP_GAMMA_LUT				0x0700
> -
> -#define LUT_10BIT_MASK				0x03ff
> -
> +#define DISP_GAMMA_LUT1				0x0b00
> +#define TABLE_9BIT_SIZE				512
> +#define TABLE_10BIT_SIZE			1024
> +#define BANK_SIZE				256
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
> +	unsigned int lut_size;
> +	unsigned int lut_bits;
>  };
> -
>  /*
>   * struct mtk_disp_gamma - DISP_GAMMA driver structure
>   */
> @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state
> *state, struct device *dev)
>  {
> -	unsigned int i, reg;
> -	struct drm_color_lut *lut;
> +	unsigned int i, reg, idx;
>  	void __iomem *lut_base;
> -	u32 word;
> -	u32 diff[3] = {0};
> +	void __iomem *lut1_base;
> +	u32 word, word1;
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>  
>  	if (state->gamma_lut) {
> +		u32 table_size;
> +		u32 mask;
> +		u32 lut_bits;
> +		u32 shift_bits;
> +		bool lut_diff;
> +		struct drm_color_lut color, color_rec;
> +		struct drm_color_lut *lut = (struct drm_color_lut
> *)state->gamma_lut;
> +
> +		table_size = gamma->data->lut_size;
> +		lut_bits = gamma->data->lut_bits;
> +		lut_diff = gamma->data->lut_diff;
> +		shift_bits = (lut_bits == 12) ? 4 : 6;
> +		mask = GENMASK(lut_bits - 1, 0);
>  		reg = readl(regs + DISP_GAMMA_CFG);
> +		reg = reg & ~RELAY_MODE;
>  		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);
> +		lut1_base = regs + DISP_GAMMA_LUT1;
> +		for (i = 0; i < table_size; i++) {
> +			color.red = (lut[i].red >> shift_bits) & mask;
> +			color.green = (lut[i].green >> shift_bits) &
> mask;
> +			color.blue = (lut[i].blue >> shift_bits) &
> mask;
> +			if (lut_diff && (i % 2)) {
> +				word = (lut_bits == 12) ?
> +						      (((color.green -
> color_rec.green) << 12) +
> +						      (color.red -
> color_rec.red))
> +							:
> +						      (((color.red -
> color_rec.red) << 20) +
> +						      ((color.green -
> color_rec.green) << 10) +
> +						      (color.blue -
> color_rec.blue));
> +				word1 = (color.blue - color_rec.blue);
>  			} 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);
> +				word = (lut_bits == 12) ?
> +						      ((color.green <<
> 12) + color.red)
> +							:
> +						      ((color.red <<
> 20) +
> +						      (color.green <<
> 10) + color.blue);
> +				word1 = color.blue;
> +				color_rec = color;
>  			}
> -			writel(word, (lut_base + i * 4));
> +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> +			writel(word, (lut_base + idx * 4));
> +			if (!(i % BANK_SIZE) && lut_bits == 12)
> +				writel((i / BANK_SIZE), regs +
> DISP_GAMMA_BANK);

This patch looks a little messy and mt8195 gamma introduce three
different things. So I would like to separate this patch to 4 patches.

1. Add multiple bank support. For other SoC, bank size is 0 which means
no bank support.
2. Support different lut_size: For other SoC, lut_size = 512
3. Support different lut_bits: For other SoC, lut_bits = 10
4. Add MT8195 gamma support, bank_size = 256, lut_size = 1024, lut_bits
= 12

In this view point, I think the last patch is not a bug-fix but a new
feature, so just drop the Fixes tag.

Regards,
CK

> +			if (lut_bits == 12)
> +				writel(word1, (lut1_base + idx * 4));
>  		}
>  	}
>  }
>  
> +unsigned int mtk_gamma_size(struct device *dev)
> +{
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +
> +	if (gamma->data)
> +		return gamma->data->lut_size;
> +	else
> +		return 0;
> +}
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> @@ -95,8 +134,7 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  
>  	if (gamma->data)
>  		lut_diff = gamma->data->lut_diff;
> -
> -	mtk_gamma_set_common(gamma->regs, state, lut_diff);
> +	mtk_gamma_set_common(gamma->regs, state, dev);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,
> @@ -191,10 +229,20 @@ 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,
> +	.lut_bits = 10,
>  };
>  
>  static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
>  	.lut_diff = true,
> +	.lut_size = 512,
> +	.lut_bits = 10,
> +};
> +
> +static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
> +	.lut_diff = true,
> +	.lut_size = 1024,
> +	.lut_bits = 12,
>  };
>  
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
> @@ -202,6 +250,8 @@ static const struct of_device_id
> mtk_disp_gamma_driver_dt_match[] = {
>  	  .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);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 42cc7052b050..2a6513259562 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -930,9 +930,8 @@ 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_size)
> +				gamma_lut_size = comp->funcs-
> >gamma_size(comp->dev);
>  			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 cb9a36c48d4f..1799853ef89a 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 2d72cc5ddaba..4c6538a17b88 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma
> = {
>  	.clk_enable = mtk_gamma_clk_enable,
>  	.clk_disable = mtk_gamma_clk_disable,
>  	.gamma_set = mtk_gamma_set,
> +	.gamma_size = mtk_gamma_size,
>  	.config = mtk_gamma_config,
>  	.start = mtk_gamma_start,
>  	.stop = mtk_gamma_stop,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 2d0052c23dcb..bf0cf7f86010 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
>  	void (*disable_vblank)(struct device *dev);
>  	unsigned int (*supported_rotations)(struct device *dev);
>  	unsigned int (*layer_nr)(struct device *dev);
> +	unsigned int (*gamma_size)(struct device *dev);
>  	int (*layer_check)(struct device *dev,
>  			   unsigned int idx,
>  			   struct mtk_plane_state *state);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 0e4c77724b05..473766be56e1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -567,6 +567,8 @@ static const struct of_device_id
> mtk_ddp_comp_dt_ids[] = {
>  	  .data = (void *)MTK_DISP_GAMMA, },
>  	{ .compatible = "mediatek,mt8183-disp-gamma",
>  	  .data = (void *)MTK_DISP_GAMMA, },
> +	{ .compatible = "mediatek,mt8195-disp-gamma",
> +	  .data = (void *)MTK_DISP_GAMMA, },
>  	{ .compatible = "mediatek,mt8195-disp-merge",
>  	  .data = (void *)MTK_DISP_MERGE },
>  	{ .compatible = "mediatek,mt2701-disp-mutex",

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[CK Hu]

Hi, Zheng-yan:

On Tue, 2022-08-30 at 14:39 +0800, zheng-yan.chen wrote:
> Since the previous gamma_set_common() function is designed for
> 9bit-to-10bit conversion, which is not feasible for the
> 10bit-to-12bit conversion in mt8195.
> 
> Update the function to fit the need of mt8195.
> 
> Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> support for mt8195")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++---
> --
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
>  8 files changed, 85 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
>  }
>  
>  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 33e61a136bbc..c1269fce9a66 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -51,8 +51,9 @@ 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_size(struct device *dev);
>  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(void __iomem *regs, struct drm_crtc_state
> *state, struct device *dev);
>  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 bbd558a036ec..0409e15fceb3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -18,18 +18,22 @@
>  #define DISP_GAMMA_EN				0x0000
>  #define GAMMA_EN					BIT(0)
>  #define DISP_GAMMA_CFG				0x0020
> +#define RELAY_MODE					BIT(0)
>  #define GAMMA_LUT_EN					BIT(1)
>  #define GAMMA_DITHERING					BIT(2)
>  #define DISP_GAMMA_SIZE				0x0030
> +#define DISP_GAMMA_BANK				0x0100
>  #define DISP_GAMMA_LUT				0x0700
> -
> -#define LUT_10BIT_MASK				0x03ff
> -
> +#define DISP_GAMMA_LUT1				0x0b00
> +#define TABLE_9BIT_SIZE				512
> +#define TABLE_10BIT_SIZE			1024
> +#define BANK_SIZE				256
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
> +	unsigned int lut_size;
> +	unsigned int lut_bits;
>  };
> -
>  /*
>   * struct mtk_disp_gamma - DISP_GAMMA driver structure
>   */
> @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state
> *state, struct device *dev)
>  {
> -	unsigned int i, reg;
> -	struct drm_color_lut *lut;
> +	unsigned int i, reg, idx;
>  	void __iomem *lut_base;
> -	u32 word;
> -	u32 diff[3] = {0};
> +	void __iomem *lut1_base;
> +	u32 word, word1;
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>  
>  	if (state->gamma_lut) {
> +		u32 table_size;
> +		u32 mask;
> +		u32 lut_bits;
> +		u32 shift_bits;
> +		bool lut_diff;
> +		struct drm_color_lut color, color_rec;
> +		struct drm_color_lut *lut = (struct drm_color_lut
> *)state->gamma_lut;
> +
> +		table_size = gamma->data->lut_size;
> +		lut_bits = gamma->data->lut_bits;
> +		lut_diff = gamma->data->lut_diff;
> +		shift_bits = (lut_bits == 12) ? 4 : 6;
> +		mask = GENMASK(lut_bits - 1, 0);
>  		reg = readl(regs + DISP_GAMMA_CFG);
> +		reg = reg & ~RELAY_MODE;
>  		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);
> +		lut1_base = regs + DISP_GAMMA_LUT1;
> +		for (i = 0; i < table_size; i++) {
> +			color.red = (lut[i].red >> shift_bits) & mask;
> +			color.green = (lut[i].green >> shift_bits) &
> mask;
> +			color.blue = (lut[i].blue >> shift_bits) &
> mask;
> +			if (lut_diff && (i % 2)) {
> +				word = (lut_bits == 12) ?
> +						      (((color.green -
> color_rec.green) << 12) +
> +						      (color.red -
> color_rec.red))
> +							:
> +						      (((color.red -
> color_rec.red) << 20) +
> +						      ((color.green -
> color_rec.green) << 10) +
> +						      (color.blue -
> color_rec.blue));
> +				word1 = (color.blue - color_rec.blue);
>  			} 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);
> +				word = (lut_bits == 12) ?
> +						      ((color.green <<
> 12) + color.red)
> +							:
> +						      ((color.red <<
> 20) +
> +						      (color.green <<
> 10) + color.blue);
> +				word1 = color.blue;
> +				color_rec = color;
>  			}
> -			writel(word, (lut_base + i * 4));
> +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> +			writel(word, (lut_base + idx * 4));
> +			if (!(i % BANK_SIZE) && lut_bits == 12)
> +				writel((i / BANK_SIZE), regs +
> DISP_GAMMA_BANK);

This patch looks a little messy and mt8195 gamma introduce three
different things. So I would like to separate this patch to 4 patches.

1. Add multiple bank support. For other SoC, bank size is 0 which means
no bank support.
2. Support different lut_size: For other SoC, lut_size = 512
3. Support different lut_bits: For other SoC, lut_bits = 10
4. Add MT8195 gamma support, bank_size = 256, lut_size = 1024, lut_bits
= 12

In this view point, I think the last patch is not a bug-fix but a new
feature, so just drop the Fixes tag.

Regards,
CK

> +			if (lut_bits == 12)
> +				writel(word1, (lut1_base + idx * 4));
>  		}
>  	}
>  }
>  
> +unsigned int mtk_gamma_size(struct device *dev)
> +{
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +
> +	if (gamma->data)
> +		return gamma->data->lut_size;
> +	else
> +		return 0;
> +}
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> @@ -95,8 +134,7 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  
>  	if (gamma->data)
>  		lut_diff = gamma->data->lut_diff;
> -
> -	mtk_gamma_set_common(gamma->regs, state, lut_diff);
> +	mtk_gamma_set_common(gamma->regs, state, dev);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,
> @@ -191,10 +229,20 @@ 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,
> +	.lut_bits = 10,
>  };
>  
>  static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
>  	.lut_diff = true,
> +	.lut_size = 512,
> +	.lut_bits = 10,
> +};
> +
> +static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
> +	.lut_diff = true,
> +	.lut_size = 1024,
> +	.lut_bits = 12,
>  };
>  
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
> @@ -202,6 +250,8 @@ static const struct of_device_id
> mtk_disp_gamma_driver_dt_match[] = {
>  	  .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);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 42cc7052b050..2a6513259562 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -930,9 +930,8 @@ 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_size)
> +				gamma_lut_size = comp->funcs-
> >gamma_size(comp->dev);
>  			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 cb9a36c48d4f..1799853ef89a 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 2d72cc5ddaba..4c6538a17b88 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma
> = {
>  	.clk_enable = mtk_gamma_clk_enable,
>  	.clk_disable = mtk_gamma_clk_disable,
>  	.gamma_set = mtk_gamma_set,
> +	.gamma_size = mtk_gamma_size,
>  	.config = mtk_gamma_config,
>  	.start = mtk_gamma_start,
>  	.stop = mtk_gamma_stop,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 2d0052c23dcb..bf0cf7f86010 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
>  	void (*disable_vblank)(struct device *dev);
>  	unsigned int (*supported_rotations)(struct device *dev);
>  	unsigned int (*layer_nr)(struct device *dev);
> +	unsigned int (*gamma_size)(struct device *dev);
>  	int (*layer_check)(struct device *dev,
>  			   unsigned int idx,
>  			   struct mtk_plane_state *state);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 0e4c77724b05..473766be56e1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -567,6 +567,8 @@ static const struct of_device_id
> mtk_ddp_comp_dt_ids[] = {
>  	  .data = (void *)MTK_DISP_GAMMA, },
>  	{ .compatible = "mediatek,mt8183-disp-gamma",
>  	  .data = (void *)MTK_DISP_GAMMA, },
> +	{ .compatible = "mediatek,mt8195-disp-gamma",
> +	  .data = (void *)MTK_DISP_GAMMA, },
>  	{ .compatible = "mediatek,mt8195-disp-merge",
>  	  .data = (void *)MTK_DISP_MERGE },
>  	{ .compatible = "mediatek,mt2701-disp-mutex",

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[CK Hu]

Hi, Zheng-yan:

On Tue, 2022-08-30 at 14:39 +0800, zheng-yan.chen wrote:
> Since the previous gamma_set_common() function is designed for
> 9bit-to-10bit conversion, which is not feasible for the
> 10bit-to-12bit conversion in mt8195.
> 
> Update the function to fit the need of mt8195.
> 
> Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> support for mt8195")
> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++---
> --
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
>  8 files changed, 85 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
>  }
>  
>  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 33e61a136bbc..c1269fce9a66 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -51,8 +51,9 @@ 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_size(struct device *dev);
>  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(void __iomem *regs, struct drm_crtc_state
> *state, struct device *dev);
>  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 bbd558a036ec..0409e15fceb3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -18,18 +18,22 @@
>  #define DISP_GAMMA_EN				0x0000
>  #define GAMMA_EN					BIT(0)
>  #define DISP_GAMMA_CFG				0x0020
> +#define RELAY_MODE					BIT(0)
>  #define GAMMA_LUT_EN					BIT(1)
>  #define GAMMA_DITHERING					BIT(2)
>  #define DISP_GAMMA_SIZE				0x0030
> +#define DISP_GAMMA_BANK				0x0100
>  #define DISP_GAMMA_LUT				0x0700
> -
> -#define LUT_10BIT_MASK				0x03ff
> -
> +#define DISP_GAMMA_LUT1				0x0b00
> +#define TABLE_9BIT_SIZE				512
> +#define TABLE_10BIT_SIZE			1024
> +#define BANK_SIZE				256
>  struct mtk_disp_gamma_data {
>  	bool has_dither;
>  	bool lut_diff;
> +	unsigned int lut_size;
> +	unsigned int lut_bits;
>  };
> -
>  /*
>   * struct mtk_disp_gamma - DISP_GAMMA driver structure
>   */
> @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct drm_crtc_state
> *state, struct device *dev)
>  {
> -	unsigned int i, reg;
> -	struct drm_color_lut *lut;
> +	unsigned int i, reg, idx;
>  	void __iomem *lut_base;
> -	u32 word;
> -	u32 diff[3] = {0};
> +	void __iomem *lut1_base;
> +	u32 word, word1;
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>  
>  	if (state->gamma_lut) {
> +		u32 table_size;
> +		u32 mask;
> +		u32 lut_bits;
> +		u32 shift_bits;
> +		bool lut_diff;
> +		struct drm_color_lut color, color_rec;
> +		struct drm_color_lut *lut = (struct drm_color_lut
> *)state->gamma_lut;
> +
> +		table_size = gamma->data->lut_size;
> +		lut_bits = gamma->data->lut_bits;
> +		lut_diff = gamma->data->lut_diff;
> +		shift_bits = (lut_bits == 12) ? 4 : 6;
> +		mask = GENMASK(lut_bits - 1, 0);
>  		reg = readl(regs + DISP_GAMMA_CFG);
> +		reg = reg & ~RELAY_MODE;
>  		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);
> +		lut1_base = regs + DISP_GAMMA_LUT1;
> +		for (i = 0; i < table_size; i++) {
> +			color.red = (lut[i].red >> shift_bits) & mask;
> +			color.green = (lut[i].green >> shift_bits) &
> mask;
> +			color.blue = (lut[i].blue >> shift_bits) &
> mask;
> +			if (lut_diff && (i % 2)) {
> +				word = (lut_bits == 12) ?
> +						      (((color.green -
> color_rec.green) << 12) +
> +						      (color.red -
> color_rec.red))
> +							:
> +						      (((color.red -
> color_rec.red) << 20) +
> +						      ((color.green -
> color_rec.green) << 10) +
> +						      (color.blue -
> color_rec.blue));
> +				word1 = (color.blue - color_rec.blue);
>  			} 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);
> +				word = (lut_bits == 12) ?
> +						      ((color.green <<
> 12) + color.red)
> +							:
> +						      ((color.red <<
> 20) +
> +						      (color.green <<
> 10) + color.blue);
> +				word1 = color.blue;
> +				color_rec = color;
>  			}
> -			writel(word, (lut_base + i * 4));
> +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> +			writel(word, (lut_base + idx * 4));
> +			if (!(i % BANK_SIZE) && lut_bits == 12)
> +				writel((i / BANK_SIZE), regs +
> DISP_GAMMA_BANK);

This patch looks a little messy and mt8195 gamma introduce three
different things. So I would like to separate this patch to 4 patches.

1. Add multiple bank support. For other SoC, bank size is 0 which means
no bank support.
2. Support different lut_size: For other SoC, lut_size = 512
3. Support different lut_bits: For other SoC, lut_bits = 10
4. Add MT8195 gamma support, bank_size = 256, lut_size = 1024, lut_bits
= 12

In this view point, I think the last patch is not a bug-fix but a new
feature, so just drop the Fixes tag.

Regards,
CK

> +			if (lut_bits == 12)
> +				writel(word1, (lut1_base + idx * 4));
>  		}
>  	}
>  }
>  
> +unsigned int mtk_gamma_size(struct device *dev)
> +{
> +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> +
> +	if (gamma->data)
> +		return gamma->data->lut_size;
> +	else
> +		return 0;
> +}
>  void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>  {
>  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> @@ -95,8 +134,7 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
>  
>  	if (gamma->data)
>  		lut_diff = gamma->data->lut_diff;
> -
> -	mtk_gamma_set_common(gamma->regs, state, lut_diff);
> +	mtk_gamma_set_common(gamma->regs, state, dev);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,
> @@ -191,10 +229,20 @@ 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,
> +	.lut_bits = 10,
>  };
>  
>  static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
>  	.lut_diff = true,
> +	.lut_size = 512,
> +	.lut_bits = 10,
> +};
> +
> +static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
> +	.lut_diff = true,
> +	.lut_size = 1024,
> +	.lut_bits = 12,
>  };
>  
>  static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
> @@ -202,6 +250,8 @@ static const struct of_device_id
> mtk_disp_gamma_driver_dt_match[] = {
>  	  .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);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 42cc7052b050..2a6513259562 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -930,9 +930,8 @@ 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_size)
> +				gamma_lut_size = comp->funcs-
> >gamma_size(comp->dev);
>  			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 cb9a36c48d4f..1799853ef89a 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 2d72cc5ddaba..4c6538a17b88 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -323,6 +323,7 @@ static const struct mtk_ddp_comp_funcs ddp_gamma
> = {
>  	.clk_enable = mtk_gamma_clk_enable,
>  	.clk_disable = mtk_gamma_clk_disable,
>  	.gamma_set = mtk_gamma_set,
> +	.gamma_size = mtk_gamma_size,
>  	.config = mtk_gamma_config,
>  	.start = mtk_gamma_start,
>  	.stop = mtk_gamma_stop,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index 2d0052c23dcb..bf0cf7f86010 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -59,6 +59,7 @@ struct mtk_ddp_comp_funcs {
>  	void (*disable_vblank)(struct device *dev);
>  	unsigned int (*supported_rotations)(struct device *dev);
>  	unsigned int (*layer_nr)(struct device *dev);
> +	unsigned int (*gamma_size)(struct device *dev);
>  	int (*layer_check)(struct device *dev,
>  			   unsigned int idx,
>  			   struct mtk_plane_state *state);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 0e4c77724b05..473766be56e1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -567,6 +567,8 @@ static const struct of_device_id
> mtk_ddp_comp_dt_ids[] = {
>  	  .data = (void *)MTK_DISP_GAMMA, },
>  	{ .compatible = "mediatek,mt8183-disp-gamma",
>  	  .data = (void *)MTK_DISP_GAMMA, },
> +	{ .compatible = "mediatek,mt8195-disp-gamma",
> +	  .data = (void *)MTK_DISP_GAMMA, },
>  	{ .compatible = "mediatek,mt8195-disp-merge",
>  	  .data = (void *)MTK_DISP_MERGE },
>  	{ .compatible = "mediatek,mt2701-disp-mutex",


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 31/08/2022 05:29, zheng-yan.chen wrote:
> On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
>> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
>>> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>>>> Modify gamma compatible for mt8195.
>>>>
>>>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
>>>> vdosys0")
>>>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
>>>
>>> Reviewed-by: AngeloGioacchino Del Regno <
>>> angelogioacchino.delregno@collabora.com>
>>
>> Please also perform review on the commit msg and backport status.
>>
>> Best regards,
>> Krzysztof
> Hello Krzysztof, 
> Thanks for the review,
> I will fix it at the next version.

This was to AngeloGioacchino...

Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 31/08/2022 05:29, zheng-yan.chen wrote:
> On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
>> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
>>> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>>>> Modify gamma compatible for mt8195.
>>>>
>>>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
>>>> vdosys0")
>>>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
>>>
>>> Reviewed-by: AngeloGioacchino Del Regno <
>>> angelogioacchino.delregno@collabora.com>
>>
>> Please also perform review on the commit msg and backport status.
>>
>> Best regards,
>> Krzysztof
> Hello Krzysztof, 
> Thanks for the review,
> I will fix it at the next version.

This was to AngeloGioacchino...

Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[Krzysztof Kozlowski]

On 31/08/2022 05:29, zheng-yan.chen wrote:
> On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
>> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
>>> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>>>> Modify gamma compatible for mt8195.
>>>>
>>>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
>>>> vdosys0")
>>>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
>>>
>>> Reviewed-by: AngeloGioacchino Del Regno <
>>> angelogioacchino.delregno@collabora.com>
>>
>> Please also perform review on the commit msg and backport status.
>>
>> Best regards,
>> Krzysztof
> Hello Krzysztof, 
> Thanks for the review,
> I will fix it at the next version.

This was to AngeloGioacchino...

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 31/08/22 08:04, Krzysztof Kozlowski ha scritto:
> On 31/08/2022 05:29, zheng-yan.chen wrote:
>> On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
>>> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
>>>> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>>>>> Modify gamma compatible for mt8195.
>>>>>
>>>>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
>>>>> vdosys0")
>>>>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
>>>>
>>>> Reviewed-by: AngeloGioacchino Del Regno <
>>>> angelogioacchino.delregno@collabora.com>
>>>
>>> Please also perform review on the commit msg and backport status.
>>>

Ack.

Cheers,
Angelo

>>> Best regards,
>>> Krzysztof
>> Hello Krzysztof,
>> Thanks for the review,
>> I will fix it at the next version.
> 
> This was to AngeloGioacchino...
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 31/08/22 08:04, Krzysztof Kozlowski ha scritto:
> On 31/08/2022 05:29, zheng-yan.chen wrote:
>> On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
>>> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
>>>> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>>>>> Modify gamma compatible for mt8195.
>>>>>
>>>>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
>>>>> vdosys0")
>>>>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
>>>>
>>>> Reviewed-by: AngeloGioacchino Del Regno <
>>>> angelogioacchino.delregno@collabora.com>
>>>
>>> Please also perform review on the commit msg and backport status.
>>>

Ack.

Cheers,
Angelo

>>> Best regards,
>>> Krzysztof
>> Hello Krzysztof,
>> Thanks for the review,
>> I will fix it at the next version.
> 
> This was to AngeloGioacchino...
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 3/3] arm64: dts: Modify gamma compatible for mt8195

[AngeloGioacchino Del Regno]

Il 31/08/22 08:04, Krzysztof Kozlowski ha scritto:
> On 31/08/2022 05:29, zheng-yan.chen wrote:
>> On Tue, 2022-08-30 at 12:14 +0300, Krzysztof Kozlowski wrote:
>>> On 30/08/2022 10:49, AngeloGioacchino Del Regno wrote:
>>>> Il 30/08/22 08:39, zheng-yan.chen ha scritto:
>>>>> Modify gamma compatible for mt8195.
>>>>>
>>>>> Fixes: 16590e634f1d ("arm64: dts: mt8195: Add display node for
>>>>> vdosys0")
>>>>> Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
>>>>
>>>> Reviewed-by: AngeloGioacchino Del Regno <
>>>> angelogioacchino.delregno@collabora.com>
>>>
>>> Please also perform review on the commit msg and backport status.
>>>

Ack.

Cheers,
Angelo

>>> Best regards,
>>> Krzysztof
>> Hello Krzysztof,
>> Thanks for the review,
>> I will fix it at the next version.
> 
> This was to AngeloGioacchino...
> 
> Best regards,
> Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[Jason-JH Lin]

On Wed, 2022-08-31 at 10:56 +0800, CK Hu wrote:
> Hi, Zheng-yan:
> 
> On Tue, 2022-08-30 at 14:39 +0800, zheng-yan.chen wrote:
> > Since the previous gamma_set_common() function is designed for
> > 9bit-to-10bit conversion, which is not feasible for the
> > 10bit-to-12bit conversion in mt8195.
> > 
> > Update the function to fit the need of mt8195.
> > 
> > Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> > support for mt8195")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-
> > --
> > --
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
> >  8 files changed, 85 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
> >  }
> >  
> >  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 33e61a136bbc..c1269fce9a66 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -51,8 +51,9 @@ 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_size(struct device *dev);
> >  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(void __iomem *regs, struct
> > drm_crtc_state
> > *state, struct device *dev);
> >  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 bbd558a036ec..0409e15fceb3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -18,18 +18,22 @@
> >  #define DISP_GAMMA_EN				0x0000
> >  #define GAMMA_EN					BIT(0)
> >  #define DISP_GAMMA_CFG				0x0020
> > +#define RELAY_MODE					BIT(0)
> >  #define GAMMA_LUT_EN					BIT(1)
> >  #define GAMMA_DITHERING					BIT(2)
> >  #define DISP_GAMMA_SIZE				0x0030
> > +#define DISP_GAMMA_BANK				0x0100
> >  #define DISP_GAMMA_LUT				0x0700
> > -
> > -#define LUT_10BIT_MASK				0x03ff
> > -
> > +#define DISP_GAMMA_LUT1				0x0b00
> > +#define TABLE_9BIT_SIZE				512
> > +#define TABLE_10BIT_SIZE			1024
> > +#define BANK_SIZE				256
> >  struct mtk_disp_gamma_data {
> >  	bool has_dither;
> >  	bool lut_diff;
> > +	unsigned int lut_size;
> > +	unsigned int lut_bits;
> >  };
> > -
> >  /*
> >   * struct mtk_disp_gamma - DISP_GAMMA driver structure
> >   */
> > @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct
> > drm_crtc_state
> > *state, struct device *dev)
> >  {
> > -	unsigned int i, reg;
> > -	struct drm_color_lut *lut;
> > +	unsigned int i, reg, idx;
> >  	void __iomem *lut_base;
> > -	u32 word;
> > -	u32 diff[3] = {0};
> > +	void __iomem *lut1_base;
> > +	u32 word, word1;
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> >  
> >  	if (state->gamma_lut) {
> > +		u32 table_size;
> > +		u32 mask;
> > +		u32 lut_bits;
> > +		u32 shift_bits;
> > +		bool lut_diff;
> > +		struct drm_color_lut color, color_rec;
> > +		struct drm_color_lut *lut = (struct drm_color_lut
> > *)state->gamma_lut;
> > +
> > +		table_size = gamma->data->lut_size;
> > +		lut_bits = gamma->data->lut_bits;
> > +		lut_diff = gamma->data->lut_diff;
> > +		shift_bits = (lut_bits == 12) ? 4 : 6;
> > +		mask = GENMASK(lut_bits - 1, 0);
> >  		reg = readl(regs + DISP_GAMMA_CFG);
> > +		reg = reg & ~RELAY_MODE;
> >  		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);
> > +		lut1_base = regs + DISP_GAMMA_LUT1;
> > +		for (i = 0; i < table_size; i++) {
> > +			color.red = (lut[i].red >> shift_bits) & mask;
> > +			color.green = (lut[i].green >> shift_bits) &
> > mask;
> > +			color.blue = (lut[i].blue >> shift_bits) &
> > mask;
> > +			if (lut_diff && (i % 2)) {
> > +				word = (lut_bits == 12) ?
> > +						      (((color.green -
> > color_rec.green) << 12) +
> > +						      (color.red -
> > color_rec.red))
> > +							:
> > +						      (((color.red -
> > color_rec.red) << 20) +
> > +						      ((color.green -
> > color_rec.green) << 10) +
> > +						      (color.blue -
> > color_rec.blue));
> > +				word1 = (color.blue - color_rec.blue);
> >  			} 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);
> > +				word = (lut_bits == 12) ?
> > +						      ((color.green <<
> > 12) + color.red)
> > +							:
> > +						      ((color.red <<
> > 20) +
> > +						      (color.green <<
> > 10) + color.blue);
> > +				word1 = color.blue;
> > +				color_rec = color;
> >  			}
> > -			writel(word, (lut_base + i * 4));
> > +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> > +			writel(word, (lut_base + idx * 4));
> > +			if (!(i % BANK_SIZE) && lut_bits == 12)
> > +				writel((i / BANK_SIZE), regs +
> > DISP_GAMMA_BANK);
> 
> This patch looks a little messy and mt8195 gamma introduce three
> different things. So I would like to separate this patch to 4
> patches.
> 
> 1. Add multiple bank support. For other SoC, bank size is 0 which
> means
> no bank support.
> 2. Support different lut_size: For other SoC, lut_size = 512
> 3. Support different lut_bits: For other SoC, lut_bits = 10
> 4. Add MT8195 gamma support, bank_size = 256, lut_size = 1024,
> lut_bits
> = 12
> 
> In this view point, I think the last patch is not a bug-fix but a new
> feature, so just drop the Fixes tag.
> 
> Regards,
> CK


Hi CK,

Thanks for the reviews.

I'll help Zheng-yan finish this series.

I'll separate this one patch to the patches below:
drm/mediatek: Adjust mtk_drm_gamma_set_common parameters
drm/mediatek: Add gamma support different lut_size for other SoC
drm/mediatek: Add gamma support different lut_bits for other SoC
drm/mediatek: Add gamma support different bank_size for other SoC
drm/mediatek: Add gamma lut support for mt8195
drm/mediatek: Add clear RELAY_MODE bit to set gamma

Regrads,
Jason-JH.Lin

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[Jason-JH Lin]

On Wed, 2022-08-31 at 10:56 +0800, CK Hu wrote:
> Hi, Zheng-yan:
> 
> On Tue, 2022-08-30 at 14:39 +0800, zheng-yan.chen wrote:
> > Since the previous gamma_set_common() function is designed for
> > 9bit-to-10bit conversion, which is not feasible for the
> > 10bit-to-12bit conversion in mt8195.
> > 
> > Update the function to fit the need of mt8195.
> > 
> > Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> > support for mt8195")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-
> > --
> > --
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
> >  8 files changed, 85 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
> >  }
> >  
> >  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 33e61a136bbc..c1269fce9a66 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -51,8 +51,9 @@ 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_size(struct device *dev);
> >  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(void __iomem *regs, struct
> > drm_crtc_state
> > *state, struct device *dev);
> >  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 bbd558a036ec..0409e15fceb3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -18,18 +18,22 @@
> >  #define DISP_GAMMA_EN				0x0000
> >  #define GAMMA_EN					BIT(0)
> >  #define DISP_GAMMA_CFG				0x0020
> > +#define RELAY_MODE					BIT(0)
> >  #define GAMMA_LUT_EN					BIT(1)
> >  #define GAMMA_DITHERING					BIT(2)
> >  #define DISP_GAMMA_SIZE				0x0030
> > +#define DISP_GAMMA_BANK				0x0100
> >  #define DISP_GAMMA_LUT				0x0700
> > -
> > -#define LUT_10BIT_MASK				0x03ff
> > -
> > +#define DISP_GAMMA_LUT1				0x0b00
> > +#define TABLE_9BIT_SIZE				512
> > +#define TABLE_10BIT_SIZE			1024
> > +#define BANK_SIZE				256
> >  struct mtk_disp_gamma_data {
> >  	bool has_dither;
> >  	bool lut_diff;
> > +	unsigned int lut_size;
> > +	unsigned int lut_bits;
> >  };
> > -
> >  /*
> >   * struct mtk_disp_gamma - DISP_GAMMA driver structure
> >   */
> > @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct
> > drm_crtc_state
> > *state, struct device *dev)
> >  {
> > -	unsigned int i, reg;
> > -	struct drm_color_lut *lut;
> > +	unsigned int i, reg, idx;
> >  	void __iomem *lut_base;
> > -	u32 word;
> > -	u32 diff[3] = {0};
> > +	void __iomem *lut1_base;
> > +	u32 word, word1;
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> >  
> >  	if (state->gamma_lut) {
> > +		u32 table_size;
> > +		u32 mask;
> > +		u32 lut_bits;
> > +		u32 shift_bits;
> > +		bool lut_diff;
> > +		struct drm_color_lut color, color_rec;
> > +		struct drm_color_lut *lut = (struct drm_color_lut
> > *)state->gamma_lut;
> > +
> > +		table_size = gamma->data->lut_size;
> > +		lut_bits = gamma->data->lut_bits;
> > +		lut_diff = gamma->data->lut_diff;
> > +		shift_bits = (lut_bits == 12) ? 4 : 6;
> > +		mask = GENMASK(lut_bits - 1, 0);
> >  		reg = readl(regs + DISP_GAMMA_CFG);
> > +		reg = reg & ~RELAY_MODE;
> >  		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);
> > +		lut1_base = regs + DISP_GAMMA_LUT1;
> > +		for (i = 0; i < table_size; i++) {
> > +			color.red = (lut[i].red >> shift_bits) & mask;
> > +			color.green = (lut[i].green >> shift_bits) &
> > mask;
> > +			color.blue = (lut[i].blue >> shift_bits) &
> > mask;
> > +			if (lut_diff && (i % 2)) {
> > +				word = (lut_bits == 12) ?
> > +						      (((color.green -
> > color_rec.green) << 12) +
> > +						      (color.red -
> > color_rec.red))
> > +							:
> > +						      (((color.red -
> > color_rec.red) << 20) +
> > +						      ((color.green -
> > color_rec.green) << 10) +
> > +						      (color.blue -
> > color_rec.blue));
> > +				word1 = (color.blue - color_rec.blue);
> >  			} 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);
> > +				word = (lut_bits == 12) ?
> > +						      ((color.green <<
> > 12) + color.red)
> > +							:
> > +						      ((color.red <<
> > 20) +
> > +						      (color.green <<
> > 10) + color.blue);
> > +				word1 = color.blue;
> > +				color_rec = color;
> >  			}
> > -			writel(word, (lut_base + i * 4));
> > +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> > +			writel(word, (lut_base + idx * 4));
> > +			if (!(i % BANK_SIZE) && lut_bits == 12)
> > +				writel((i / BANK_SIZE), regs +
> > DISP_GAMMA_BANK);
> 
> This patch looks a little messy and mt8195 gamma introduce three
> different things. So I would like to separate this patch to 4
> patches.
> 
> 1. Add multiple bank support. For other SoC, bank size is 0 which
> means
> no bank support.
> 2. Support different lut_size: For other SoC, lut_size = 512
> 3. Support different lut_bits: For other SoC, lut_bits = 10
> 4. Add MT8195 gamma support, bank_size = 256, lut_size = 1024,
> lut_bits
> = 12
> 
> In this view point, I think the last patch is not a bug-fix but a new
> feature, so just drop the Fixes tag.
> 
> Regards,
> CK


Hi CK,

Thanks for the reviews.

I'll help Zheng-yan finish this series.

I'll separate this one patch to the patches below:
drm/mediatek: Adjust mtk_drm_gamma_set_common parameters
drm/mediatek: Add gamma support different lut_size for other SoC
drm/mediatek: Add gamma support different lut_bits for other SoC
drm/mediatek: Add gamma support different bank_size for other SoC
drm/mediatek: Add gamma lut support for mt8195
drm/mediatek: Add clear RELAY_MODE bit to set gamma

Regrads,
Jason-JH.Lin

Re: [PATCH v2 2/3] drm/mediatek: Add gamma lut support for mt8195

[Jason-JH Lin]

On Wed, 2022-08-31 at 10:56 +0800, CK Hu wrote:
> Hi, Zheng-yan:
> 
> On Tue, 2022-08-30 at 14:39 +0800, zheng-yan.chen wrote:
> > Since the previous gamma_set_common() function is designed for
> > 9bit-to-10bit conversion, which is not feasible for the
> > 10bit-to-12bit conversion in mt8195.
> > 
> > Update the function to fit the need of mt8195.
> > 
> > Fixes: 7266e90a51a3 ("drm/mediatek: Add mediatek-drm of vdosys0
> > support for mt8195")
> > Signed-off-by: zheng-yan.chen <zheng-yan.chen@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |   2 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   3 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 102 +++++++++++++++-
> > --
> > --
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |   5 +-
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |   1 -
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   2 +
> >  8 files changed, 85 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index 0f9d7efb61d7..f563eee3c330 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(aal->regs, state, dev);
> >  }
> >  
> >  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 33e61a136bbc..c1269fce9a66 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -51,8 +51,9 @@ 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_size(struct device *dev);
> >  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(void __iomem *regs, struct
> > drm_crtc_state
> > *state, struct device *dev);
> >  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 bbd558a036ec..0409e15fceb3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -18,18 +18,22 @@
> >  #define DISP_GAMMA_EN				0x0000
> >  #define GAMMA_EN					BIT(0)
> >  #define DISP_GAMMA_CFG				0x0020
> > +#define RELAY_MODE					BIT(0)
> >  #define GAMMA_LUT_EN					BIT(1)
> >  #define GAMMA_DITHERING					BIT(2)
> >  #define DISP_GAMMA_SIZE				0x0030
> > +#define DISP_GAMMA_BANK				0x0100
> >  #define DISP_GAMMA_LUT				0x0700
> > -
> > -#define LUT_10BIT_MASK				0x03ff
> > -
> > +#define DISP_GAMMA_LUT1				0x0b00
> > +#define TABLE_9BIT_SIZE				512
> > +#define TABLE_10BIT_SIZE			1024
> > +#define BANK_SIZE				256
> >  struct mtk_disp_gamma_data {
> >  	bool has_dither;
> >  	bool lut_diff;
> > +	unsigned int lut_size;
> > +	unsigned int lut_bits;
> >  };
> > -
> >  /*
> >   * struct mtk_disp_gamma - DISP_GAMMA driver structure
> >   */
> > @@ -54,40 +58,75 @@ 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(void __iomem *regs, struct
> > drm_crtc_state
> > *state, struct device *dev)
> >  {
> > -	unsigned int i, reg;
> > -	struct drm_color_lut *lut;
> > +	unsigned int i, reg, idx;
> >  	void __iomem *lut_base;
> > -	u32 word;
> > -	u32 diff[3] = {0};
> > +	void __iomem *lut1_base;
> > +	u32 word, word1;
> > +	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> >  
> >  	if (state->gamma_lut) {
> > +		u32 table_size;
> > +		u32 mask;
> > +		u32 lut_bits;
> > +		u32 shift_bits;
> > +		bool lut_diff;
> > +		struct drm_color_lut color, color_rec;
> > +		struct drm_color_lut *lut = (struct drm_color_lut
> > *)state->gamma_lut;
> > +
> > +		table_size = gamma->data->lut_size;
> > +		lut_bits = gamma->data->lut_bits;
> > +		lut_diff = gamma->data->lut_diff;
> > +		shift_bits = (lut_bits == 12) ? 4 : 6;
> > +		mask = GENMASK(lut_bits - 1, 0);
> >  		reg = readl(regs + DISP_GAMMA_CFG);
> > +		reg = reg & ~RELAY_MODE;
> >  		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);
> > +		lut1_base = regs + DISP_GAMMA_LUT1;
> > +		for (i = 0; i < table_size; i++) {
> > +			color.red = (lut[i].red >> shift_bits) & mask;
> > +			color.green = (lut[i].green >> shift_bits) &
> > mask;
> > +			color.blue = (lut[i].blue >> shift_bits) &
> > mask;
> > +			if (lut_diff && (i % 2)) {
> > +				word = (lut_bits == 12) ?
> > +						      (((color.green -
> > color_rec.green) << 12) +
> > +						      (color.red -
> > color_rec.red))
> > +							:
> > +						      (((color.red -
> > color_rec.red) << 20) +
> > +						      ((color.green -
> > color_rec.green) << 10) +
> > +						      (color.blue -
> > color_rec.blue));
> > +				word1 = (color.blue - color_rec.blue);
> >  			} 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);
> > +				word = (lut_bits == 12) ?
> > +						      ((color.green <<
> > 12) + color.red)
> > +							:
> > +						      ((color.red <<
> > 20) +
> > +						      (color.green <<
> > 10) + color.blue);
> > +				word1 = color.blue;
> > +				color_rec = color;
> >  			}
> > -			writel(word, (lut_base + i * 4));
> > +			idx = (lut_bits == 12) ? (i % BANK_SIZE) : i;
> > +			writel(word, (lut_base + idx * 4));
> > +			if (!(i % BANK_SIZE) && lut_bits == 12)
> > +				writel((i / BANK_SIZE), regs +
> > DISP_GAMMA_BANK);
> 
> This patch looks a little messy and mt8195 gamma introduce three
> different things. So I would like to separate this patch to 4
> patches.
> 
> 1. Add multiple bank support. For other SoC, bank size is 0 which
> means
> no bank support.
> 2. Support different lut_size: For other SoC, lut_size = 512
> 3. Support different lut_bits: For other SoC, lut_bits = 10
> 4. Add MT8195 gamma support, bank_size = 256, lut_size = 1024,
> lut_bits
> = 12
> 
> In this view point, I think the last patch is not a bug-fix but a new
> feature, so just drop the Fixes tag.
> 
> Regards,
> CK


Hi CK,

Thanks for the reviews.

I'll help Zheng-yan finish this series.

I'll separate this one patch to the patches below:
drm/mediatek: Adjust mtk_drm_gamma_set_common parameters
drm/mediatek: Add gamma support different lut_size for other SoC
drm/mediatek: Add gamma support different lut_bits for other SoC
drm/mediatek: Add gamma support different bank_size for other SoC
drm/mediatek: Add gamma lut support for mt8195
drm/mediatek: Add clear RELAY_MODE bit to set gamma

Regrads,
Jason-JH.Lin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel