linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1, 0/3] gamma set with cmdq
@ 2021-04-12  6:35 Yongqiang Niu
  2021-04-12  6:35 ` [PATCH v1, 1/3] drm/mediatek: Separate aal module Yongqiang Niu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Yongqiang Niu @ 2021-04-12  6:35 UTC (permalink / raw)
  To: Chun-Kuang Hu, Rob Herring
  Cc: Philipp Zabel, David Airlie, Daniel Vetter, Mark Rutland,
	Matthias Brugger, dri-devel, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel,
	Project_Global_Chrome_Upstream_Group, Yongqiang Niu

This series are based on 5.12-rc2 and provide 3 patch
to set gamma lut with cmdq

Yongqiang Niu (3):
  drm/mediatek: Separate aal module
  arm64: dts: mt8183: refine aal compatible name
  drm/mediatek: gamma set with cmdq

 arch/arm64/boot/dts/mediatek/mt8183.dtsi    |   3 +-
 drivers/gpu/drm/mediatek/Makefile           |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_aal.c     | 167 ++++++++++++++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  14 ++-
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c   |  11 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  18 +--
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  39 +------
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |   8 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   8 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.h      |   1 +
 10 files changed, 213 insertions(+), 59 deletions(-)
 create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_aal.c

-- 
1.8.1.1.dirty


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v1, 1/3] drm/mediatek: Separate aal module
  2021-04-12  6:35 [PATCH v1, 0/3] gamma set with cmdq Yongqiang Niu
@ 2021-04-12  6:35 ` Yongqiang Niu
  2021-04-12  8:26   ` CK Hu
  2021-04-12  6:35 ` [PATCH v1, 2/3] arm64: dts: mt8183: refine aal compatible name Yongqiang Niu
  2021-04-12  6:35 ` [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq Yongqiang Niu
  2 siblings, 1 reply; 8+ messages in thread
From: Yongqiang Niu @ 2021-04-12  6:35 UTC (permalink / raw)
  To: Chun-Kuang Hu, Rob Herring
  Cc: Philipp Zabel, David Airlie, Daniel Vetter, Mark Rutland,
	Matthias Brugger, dri-devel, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel,
	Project_Global_Chrome_Upstream_Group, Yongqiang Niu

mt8183 aal has no gamma function

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 drivers/gpu/drm/mediatek/Makefile           |   3 +-
 drivers/gpu/drm/mediatek/mtk_disp_aal.c     | 167 ++++++++++++++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   9 ++
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  39 +------
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   8 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.h      |   1 +
 6 files changed, 187 insertions(+), 40 deletions(-)
 create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_aal.c

diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
index dc54a7a..29098d7 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
-mediatek-drm-y := mtk_disp_ccorr.o \
+mediatek-drm-y := mtk_disp_aal.o \
+		  mtk_disp_ccorr.o \
 		  mtk_disp_color.o \
 		  mtk_disp_gamma.o \
 		  mtk_disp_ovl.o \
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
new file mode 100644
index 0000000..64b4528
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021 MediaTek Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
+
+#include "mtk_disp_drv.h"
+#include "mtk_drm_crtc.h"
+#include "mtk_drm_ddp_comp.h"
+
+#define DISP_AAL_EN				0x0000
+#define AAL_EN						BIT(0)
+#define DISP_AAL_SIZE				0x0030
+
+
+struct mtk_disp_aal_data {
+	bool has_gamma;
+};
+
+/**
+ * struct mtk_disp_aal - DISP_AAL driver structure
+ * @ddp_comp - structure containing type enum and hardware resources
+ * @crtc - associated crtc to report irq events to
+ */
+struct mtk_disp_aal {
+	struct clk *clk;
+	void __iomem *regs;
+	struct cmdq_client_reg cmdq_reg;
+	const struct mtk_disp_aal_data *data;
+};
+
+int mtk_aal_clk_enable(struct device *dev)
+{
+	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+	return clk_prepare_enable(aal->clk);
+}
+
+void mtk_aal_clk_disable(struct device *dev)
+{
+	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(aal->clk);
+}
+
+void mtk_aal_config(struct device *dev, unsigned int w,
+			   unsigned int h, unsigned int vrefresh,
+			   unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
+{
+	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
+}
+
+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);
+}
+
+void mtk_aal_start(struct device *dev)
+{
+	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+	writel(AAL_EN, aal->regs + DISP_AAL_EN);
+}
+
+void mtk_aal_stop(struct device *dev)
+{
+	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+	writel_relaxed(0x0, aal->regs + DISP_AAL_EN);
+}
+
+static int mtk_disp_aal_bind(struct device *dev, struct device *master,
+			       void *data)
+{
+	return 0;
+}
+
+static void mtk_disp_aal_unbind(struct device *dev, struct device *master,
+				  void *data)
+{
+}
+
+static const struct component_ops mtk_disp_aal_component_ops = {
+	.bind	= mtk_disp_aal_bind,
+	.unbind = mtk_disp_aal_unbind,
+};
+
+static int mtk_disp_aal_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mtk_disp_aal *priv;
+	struct resource *res;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		dev_err(dev, "failed to get aal clk\n");
+		return PTR_ERR(priv->clk);
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->regs)) {
+		dev_err(dev, "failed to ioremap aal\n");
+		return PTR_ERR(priv->regs);
+	}
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	ret = cmdq_dev_get_client_reg(dev, &priv->cmdq_reg, 0);
+	if (ret)
+		dev_dbg(dev, "get mediatek,gce-client-reg fail!\n");
+#endif
+
+	priv->data = of_device_get_match_data(dev);
+	platform_set_drvdata(pdev, priv);
+
+	ret = component_add(dev, &mtk_disp_aal_component_ops);
+	if (ret)
+		dev_err(dev, "Failed to add component: %d\n", ret);
+
+	return ret;
+}
+
+static int mtk_disp_aal_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &mtk_disp_aal_component_ops);
+
+	return 0;
+}
+
+static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
+	.has_gamma = true,
+};
+
+static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
+	{ .compatible = "mediatek,mt8173-disp-aal",
+	  .data = &mt8173_aal_driver_data},
+	{ .compatible = "mediatek,mt8183-disp-aal"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mtk_disp_aal_driver_dt_match);
+
+struct platform_driver mtk_disp_aal_driver = {
+	.probe		= mtk_disp_aal_probe,
+	.remove		= mtk_disp_aal_remove,
+	.driver		= {
+		.name	= "mediatek-disp-aal",
+		.owner	= THIS_MODULE,
+		.of_match_table = mtk_disp_aal_driver_dt_match,
+	},
+};
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index cafd9df..86c3068 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -9,6 +9,15 @@
 #include <linux/soc/mediatek/mtk-cmdq.h>
 #include "mtk_drm_plane.h"
 
+int mtk_aal_clk_enable(struct device *dev);
+void mtk_aal_clk_disable(struct device *dev);
+void mtk_aal_config(struct device *dev, unsigned int w,
+		    unsigned int h, unsigned int vrefresh,
+		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
+void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
+void mtk_aal_start(struct device *dev);
+void mtk_aal_stop(struct device *dev);
+
 void mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state *state);
 int mtk_ccorr_clk_enable(struct device *dev);
 void mtk_ccorr_clk_disable(struct device *dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 75bc00e..f367142 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -32,9 +32,6 @@
 
 #define DISP_REG_UFO_START			0x0000
 
-#define DISP_AAL_EN				0x0000
-#define DISP_AAL_SIZE				0x0030
-
 #define DISP_DITHER_EN				0x0000
 #define DITHER_EN				BIT(0)
 #define DISP_DITHER_CFG				0x0020
@@ -48,8 +45,6 @@
 
 #define UFO_BYPASS				BIT(2)
 
-#define AAL_EN					BIT(0)
-
 #define DISP_DITHERING				BIT(2)
 #define DITHER_LSB_ERR_SHIFT_R(x)		(((x) & 0x7) << 28)
 #define DITHER_OVFLW_BIT_R(x)			(((x) & 0x7) << 24)
@@ -190,36 +185,6 @@ static void mtk_ufoe_start(struct device *dev)
 	writel(UFO_BYPASS, priv->regs + DISP_REG_UFO_START);
 }
 
-static void mtk_aal_config(struct device *dev, unsigned int w,
-			   unsigned int h, unsigned int vrefresh,
-			   unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
-{
-	struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
-
-	mtk_ddp_write(cmdq_pkt, w << 16 | h, &priv->cmdq_reg, priv->regs, DISP_AAL_SIZE);
-}
-
-static void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
-{
-	struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
-
-	mtk_gamma_set_common(priv->regs, state);
-}
-
-static void mtk_aal_start(struct device *dev)
-{
-	struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
-
-	writel(AAL_EN, priv->regs + DISP_AAL_EN);
-}
-
-static void mtk_aal_stop(struct device *dev)
-{
-	struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
-
-	writel_relaxed(0x0, priv->regs + DISP_AAL_EN);
-}
-
 static void mtk_dither_config(struct device *dev, unsigned int w,
 			      unsigned int h, unsigned int vrefresh,
 			      unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
@@ -247,8 +212,8 @@ static void mtk_dither_stop(struct device *dev)
 }
 
 static const struct mtk_ddp_comp_funcs ddp_aal = {
-	.clk_enable = mtk_ddp_clk_enable,
-	.clk_disable = mtk_ddp_clk_disable,
+	.clk_enable = mtk_aal_clk_enable,
+	.clk_disable = mtk_aal_clk_disable,
 	.gamma_set = mtk_aal_gamma_set,
 	.config = mtk_aal_config,
 	.start = mtk_aal_start,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index b013d56..ae8b69d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -420,6 +420,8 @@ static void mtk_drm_unbind(struct device *dev)
 	  .data = (void *)MTK_DISP_COLOR },
 	{ .compatible = "mediatek,mt8173-disp-aal",
 	  .data = (void *)MTK_DISP_AAL},
+	{ .compatible = "mediatek,mt8183-disp-aal",
+	  .data = (void *)MTK_DISP_AAL},
 	{ .compatible = "mediatek,mt8173-disp-gamma",
 	  .data = (void *)MTK_DISP_GAMMA, },
 	{ .compatible = "mediatek,mt8183-disp-gamma",
@@ -531,11 +533,12 @@ static int mtk_drm_probe(struct platform_device *pdev)
 		private->comp_node[comp_id] = of_node_get(node);
 
 		/*
-		 * Currently only the CCORR, COLOR, GAMMA, OVL, RDMA, DSI, and DPI
+		 * Currently only the AAL, CCORR, COLOR, GAMMA, OVL, RDMA, DSI, and DPI
 		 * blocks have separate component platform drivers and initialize their own
 		 * DDP component structure. The others are initialized here.
 		 */
-		if (comp_type == MTK_DISP_CCORR ||
+		if (comp_type == MTK_DISP_AAL ||
+		    comp_type == MTK_DISP_CCORR ||
 		    comp_type == MTK_DISP_COLOR ||
 		    comp_type == MTK_DISP_GAMMA ||
 		    comp_type == MTK_DISP_OVL ||
@@ -635,6 +638,7 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
 };
 
 static struct platform_driver * const mtk_drm_drivers[] = {
+	&mtk_disp_aal_driver,
 	&mtk_disp_ccorr_driver,
 	&mtk_disp_color_driver,
 	&mtk_disp_gamma_driver,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 637f566..3e7d1e6 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -46,6 +46,7 @@ struct mtk_drm_private {
 	struct drm_atomic_state *suspend_state;
 };
 
+extern struct platform_driver mtk_disp_aal_driver;
 extern struct platform_driver mtk_disp_ccorr_driver;
 extern struct platform_driver mtk_disp_color_driver;
 extern struct platform_driver mtk_disp_gamma_driver;
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1, 2/3] arm64: dts: mt8183: refine aal compatible name
  2021-04-12  6:35 [PATCH v1, 0/3] gamma set with cmdq Yongqiang Niu
  2021-04-12  6:35 ` [PATCH v1, 1/3] drm/mediatek: Separate aal module Yongqiang Niu
@ 2021-04-12  6:35 ` Yongqiang Niu
  2021-04-12  6:35 ` [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq Yongqiang Niu
  2 siblings, 0 replies; 8+ messages in thread
From: Yongqiang Niu @ 2021-04-12  6:35 UTC (permalink / raw)
  To: Chun-Kuang Hu, Rob Herring
  Cc: Philipp Zabel, David Airlie, Daniel Vetter, Mark Rutland,
	Matthias Brugger, dri-devel, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel,
	Project_Global_Chrome_Upstream_Group, Yongqiang Niu

mt8183 aal is different with mt8173
remove mt8173 compatible name for mt8183 aal

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 80519a1..ee8f87f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1061,8 +1061,7 @@
 		};
 
 		aal0: aal@14010000 {
-			compatible = "mediatek,mt8183-disp-aal",
-				     "mediatek,mt8173-disp-aal";
+			compatible = "mediatek,mt8183-disp-aal";
 			reg = <0 0x14010000 0 0x1000>;
 			interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_LOW>;
 			power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq
  2021-04-12  6:35 [PATCH v1, 0/3] gamma set with cmdq Yongqiang Niu
  2021-04-12  6:35 ` [PATCH v1, 1/3] drm/mediatek: Separate aal module Yongqiang Niu
  2021-04-12  6:35 ` [PATCH v1, 2/3] arm64: dts: mt8183: refine aal compatible name Yongqiang Niu
@ 2021-04-12  6:35 ` Yongqiang Niu
  2021-04-12  8:28   ` CK Hu
  2 siblings, 1 reply; 8+ messages in thread
From: Yongqiang Niu @ 2021-04-12  6:35 UTC (permalink / raw)
  To: Chun-Kuang Hu, Rob Herring
  Cc: Philipp Zabel, David Airlie, Daniel Vetter, Mark Rutland,
	Matthias Brugger, dri-devel, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel,
	Project_Global_Chrome_Upstream_Group, Yongqiang Niu

gamma lut set in vsync active will caused display flash issue
set gamma lut with cmdq 

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_aal.c     |  4 ++--
 drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  7 ++++---
 drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 11 ++++++-----
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 18 +++++++++++-------
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  8 +++++---
 5 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 64b4528..c8e178e 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -59,12 +59,12 @@ void mtk_aal_config(struct device *dev, unsigned int w,
 	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
 }
 
-void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
+void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
 {
 	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
 
 	if (aal->data && aal->data->has_gamma)
-		mtk_gamma_set_common(aal->regs, state);
+		mtk_gamma_set_common(aal->regs, &aal->cmdq_reg, state, cmdq_pkt);
 }
 
 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 86c3068..c2e7dcb 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -14,7 +14,7 @@
 void mtk_aal_config(struct device *dev, unsigned int w,
 		    unsigned int h, unsigned int vrefresh,
 		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
-void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
+void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
 void mtk_aal_start(struct device *dev);
 void mtk_aal_stop(struct device *dev);
 
@@ -50,8 +50,9 @@ void mtk_dither_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
 void mtk_gamma_config(struct device *dev, unsigned int w,
 		      unsigned int h, unsigned int vrefresh,
 		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
-void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
-void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state);
+void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
+void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
+			  struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
 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 3ebf91e..99a4ff3 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -55,7 +55,8 @@ 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)
+void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
+			  struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
 {
 	unsigned int i, reg;
 	struct drm_color_lut *lut;
@@ -65,23 +66,23 @@ void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state)
 	if (state->gamma_lut) {
 		reg = readl(regs + DISP_GAMMA_CFG);
 		reg = reg | GAMMA_LUT_EN;
-		writel(reg, regs + DISP_GAMMA_CFG);
+		mtk_ddp_write(cmdq_pkt, reg, cmdq_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++) {
 			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
 				(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
 				((lut[i].blue >> 6) & LUT_10BIT_MASK);
-			writel(word, (lut_base + i * 4));
+			mtk_ddp_write(cmdq_pkt, word, cmdq_reg, regs, (lut_base + i * 4));
 		}
 	}
 }
 
-void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
+void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
 {
 	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
 
-	mtk_gamma_set_common(gamma->regs, state);
+	mtk_gamma_set_common(gamma->regs, &gamma->cmdq_reg, state, cmdq_pkt);
 }
 
 void mtk_gamma_config(struct device *dev, unsigned int w,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 8b0de90..73428f0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -423,6 +423,15 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc,
 		}
 		mtk_crtc->pending_async_planes = false;
 	}
+
+	if (crtc->state->color_mgmt_changed) {
+		int i;
+
+		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
+			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state, cmdq_handle);
+			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
+		}
+	}
 }
 
 static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
@@ -464,7 +473,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
 #if IS_REACHABLE(CONFIG_MTK_CMDQ)
 	if (mtk_crtc->cmdq_client) {
 		mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
-		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
+		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, 2 * PAGE_SIZE);
 		cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
 		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
 		mtk_crtc_ddp_config(crtc, cmdq_handle);
@@ -616,15 +625,10 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 				      struct drm_atomic_state *state)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
-	int i;
 
 	if (mtk_crtc->event)
 		mtk_crtc->pending_needs_vblank = true;
-	if (crtc->state->color_mgmt_changed)
-		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
-			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
-			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
-		}
+
 	mtk_drm_crtc_hw_config(mtk_crtc);
 }
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index bb914d9..bffa58d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -60,7 +60,8 @@ struct mtk_ddp_comp_funcs {
 			     struct mtk_plane_state *state,
 			     struct cmdq_pkt *cmdq_pkt);
 	void (*gamma_set)(struct device *dev,
-			  struct drm_crtc_state *state);
+			  struct drm_crtc_state *state,
+			  struct cmdq_pkt *cmdq_pkt);
 	void (*bgclr_in_on)(struct device *dev);
 	void (*bgclr_in_off)(struct device *dev);
 	void (*ctm_set)(struct device *dev,
@@ -160,10 +161,11 @@ static inline void mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
 }
 
 static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
-				     struct drm_crtc_state *state)
+				     struct drm_crtc_state *state,
+				     struct cmdq_pkt *cmdq_pkt)
 {
 	if (comp->funcs && comp->funcs->gamma_set)
-		comp->funcs->gamma_set(comp->dev, state);
+		comp->funcs->gamma_set(comp->dev, state, cmdq_pkt);
 }
 
 static inline void mtk_ddp_comp_bgclr_in_on(struct mtk_ddp_comp *comp)
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v1, 1/3] drm/mediatek: Separate aal module
  2021-04-12  6:35 ` [PATCH v1, 1/3] drm/mediatek: Separate aal module Yongqiang Niu
@ 2021-04-12  8:26   ` CK Hu
  0 siblings, 0 replies; 8+ messages in thread
From: CK Hu @ 2021-04-12  8:26 UTC (permalink / raw)
  To: Yongqiang Niu
  Cc: Chun-Kuang Hu, Rob Herring, Philipp Zabel, David Airlie,
	Daniel Vetter, Mark Rutland, Matthias Brugger, dri-devel,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	Project_Global_Chrome_Upstream_Group

Hi, Yongqiang:

On Mon, 2021-04-12 at 14:35 +0800, Yongqiang Niu wrote:
> mt8183 aal has no gamma function

Separate this patch to two patch: one is add has_gamma config in aal.
another one is add mt8183 aal support.

Regards,
CK

> 
> Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/Makefile           |   3 +-
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c     | 167 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |   9 ++
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  39 +------
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |   8 +-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h      |   1 +
>  6 files changed, 187 insertions(+), 40 deletions(-)
>  create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_aal.c
> 
> diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
> index dc54a7a..29098d7 100644
> --- a/drivers/gpu/drm/mediatek/Makefile
> +++ b/drivers/gpu/drm/mediatek/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
> -mediatek-drm-y := mtk_disp_ccorr.o \
> +mediatek-drm-y := mtk_disp_aal.o \
> +		  mtk_disp_ccorr.o \
>  		  mtk_disp_color.o \
>  		  mtk_disp_gamma.o \
>  		  mtk_disp_ovl.o \
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> new file mode 100644
> index 0000000..64b4528
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2021 MediaTek Inc.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/soc/mediatek/mtk-cmdq.h>
> +
> +#include "mtk_disp_drv.h"
> +#include "mtk_drm_crtc.h"
> +#include "mtk_drm_ddp_comp.h"
> +
> +#define DISP_AAL_EN				0x0000
> +#define AAL_EN						BIT(0)
> +#define DISP_AAL_SIZE				0x0030
> +
> +
> +struct mtk_disp_aal_data {
> +	bool has_gamma;
> +};
> +
> +/**
> + * struct mtk_disp_aal - DISP_AAL driver structure
> + * @ddp_comp - structure containing type enum and hardware resources
> + * @crtc - associated crtc to report irq events to
> + */
> +struct mtk_disp_aal {
> +	struct clk *clk;
> +	void __iomem *regs;
> +	struct cmdq_client_reg cmdq_reg;
> +	const struct mtk_disp_aal_data *data;
> +};
> +
> +int mtk_aal_clk_enable(struct device *dev)
> +{
> +	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +
> +	return clk_prepare_enable(aal->clk);
> +}
> +
> +void mtk_aal_clk_disable(struct device *dev)
> +{
> +	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(aal->clk);
> +}
> +
> +void mtk_aal_config(struct device *dev, unsigned int w,
> +			   unsigned int h, unsigned int vrefresh,
> +			   unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> +{
> +	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +
> +	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
> +}
> +
> +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);
> +}
> +
> +void mtk_aal_start(struct device *dev)
> +{
> +	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +
> +	writel(AAL_EN, aal->regs + DISP_AAL_EN);
> +}
> +
> +void mtk_aal_stop(struct device *dev)
> +{
> +	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> +
> +	writel_relaxed(0x0, aal->regs + DISP_AAL_EN);
> +}
> +
> +static int mtk_disp_aal_bind(struct device *dev, struct device *master,
> +			       void *data)
> +{
> +	return 0;
> +}
> +
> +static void mtk_disp_aal_unbind(struct device *dev, struct device *master,
> +				  void *data)
> +{
> +}
> +
> +static const struct component_ops mtk_disp_aal_component_ops = {
> +	.bind	= mtk_disp_aal_bind,
> +	.unbind = mtk_disp_aal_unbind,
> +};
> +
> +static int mtk_disp_aal_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mtk_disp_aal *priv;
> +	struct resource *res;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		dev_err(dev, "failed to get aal clk\n");
> +		return PTR_ERR(priv->clk);
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(priv->regs)) {
> +		dev_err(dev, "failed to ioremap aal\n");
> +		return PTR_ERR(priv->regs);
> +	}
> +
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +	ret = cmdq_dev_get_client_reg(dev, &priv->cmdq_reg, 0);
> +	if (ret)
> +		dev_dbg(dev, "get mediatek,gce-client-reg fail!\n");
> +#endif
> +
> +	priv->data = of_device_get_match_data(dev);
> +	platform_set_drvdata(pdev, priv);
> +
> +	ret = component_add(dev, &mtk_disp_aal_component_ops);
> +	if (ret)
> +		dev_err(dev, "Failed to add component: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int mtk_disp_aal_remove(struct platform_device *pdev)
> +{
> +	component_del(&pdev->dev, &mtk_disp_aal_component_ops);
> +
> +	return 0;
> +}
> +
> +static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
> +	.has_gamma = true,
> +};
> +
> +static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
> +	{ .compatible = "mediatek,mt8173-disp-aal",
> +	  .data = &mt8173_aal_driver_data},
> +	{ .compatible = "mediatek,mt8183-disp-aal"},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mtk_disp_aal_driver_dt_match);
> +
> +struct platform_driver mtk_disp_aal_driver = {
> +	.probe		= mtk_disp_aal_probe,
> +	.remove		= mtk_disp_aal_remove,
> +	.driver		= {
> +		.name	= "mediatek-disp-aal",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = mtk_disp_aal_driver_dt_match,
> +	},
> +};
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index cafd9df..86c3068 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -9,6 +9,15 @@
>  #include <linux/soc/mediatek/mtk-cmdq.h>
>  #include "mtk_drm_plane.h"
>  
> +int mtk_aal_clk_enable(struct device *dev);
> +void mtk_aal_clk_disable(struct device *dev);
> +void mtk_aal_config(struct device *dev, unsigned int w,
> +		    unsigned int h, unsigned int vrefresh,
> +		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
> +void mtk_aal_start(struct device *dev);
> +void mtk_aal_stop(struct device *dev);
> +
>  void mtk_ccorr_ctm_set(struct device *dev, struct drm_crtc_state *state);
>  int mtk_ccorr_clk_enable(struct device *dev);
>  void mtk_ccorr_clk_disable(struct device *dev);
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> index 75bc00e..f367142 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> @@ -32,9 +32,6 @@
>  
>  #define DISP_REG_UFO_START			0x0000
>  
> -#define DISP_AAL_EN				0x0000
> -#define DISP_AAL_SIZE				0x0030
> -
>  #define DISP_DITHER_EN				0x0000
>  #define DITHER_EN				BIT(0)
>  #define DISP_DITHER_CFG				0x0020
> @@ -48,8 +45,6 @@
>  
>  #define UFO_BYPASS				BIT(2)
>  
> -#define AAL_EN					BIT(0)
> -
>  #define DISP_DITHERING				BIT(2)
>  #define DITHER_LSB_ERR_SHIFT_R(x)		(((x) & 0x7) << 28)
>  #define DITHER_OVFLW_BIT_R(x)			(((x) & 0x7) << 24)
> @@ -190,36 +185,6 @@ static void mtk_ufoe_start(struct device *dev)
>  	writel(UFO_BYPASS, priv->regs + DISP_REG_UFO_START);
>  }
>  
> -static void mtk_aal_config(struct device *dev, unsigned int w,
> -			   unsigned int h, unsigned int vrefresh,
> -			   unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> -{
> -	struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> -
> -	mtk_ddp_write(cmdq_pkt, w << 16 | h, &priv->cmdq_reg, priv->regs, DISP_AAL_SIZE);
> -}
> -
> -static void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
> -{
> -	struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> -
> -	mtk_gamma_set_common(priv->regs, state);
> -}
> -
> -static void mtk_aal_start(struct device *dev)
> -{
> -	struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> -
> -	writel(AAL_EN, priv->regs + DISP_AAL_EN);
> -}
> -
> -static void mtk_aal_stop(struct device *dev)
> -{
> -	struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
> -
> -	writel_relaxed(0x0, priv->regs + DISP_AAL_EN);
> -}
> -
>  static void mtk_dither_config(struct device *dev, unsigned int w,
>  			      unsigned int h, unsigned int vrefresh,
>  			      unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> @@ -247,8 +212,8 @@ static void mtk_dither_stop(struct device *dev)
>  }
>  
>  static const struct mtk_ddp_comp_funcs ddp_aal = {
> -	.clk_enable = mtk_ddp_clk_enable,
> -	.clk_disable = mtk_ddp_clk_disable,
> +	.clk_enable = mtk_aal_clk_enable,
> +	.clk_disable = mtk_aal_clk_disable,
>  	.gamma_set = mtk_aal_gamma_set,
>  	.config = mtk_aal_config,
>  	.start = mtk_aal_start,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index b013d56..ae8b69d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -420,6 +420,8 @@ static void mtk_drm_unbind(struct device *dev)
>  	  .data = (void *)MTK_DISP_COLOR },
>  	{ .compatible = "mediatek,mt8173-disp-aal",
>  	  .data = (void *)MTK_DISP_AAL},
> +	{ .compatible = "mediatek,mt8183-disp-aal",
> +	  .data = (void *)MTK_DISP_AAL},
>  	{ .compatible = "mediatek,mt8173-disp-gamma",
>  	  .data = (void *)MTK_DISP_GAMMA, },
>  	{ .compatible = "mediatek,mt8183-disp-gamma",
> @@ -531,11 +533,12 @@ static int mtk_drm_probe(struct platform_device *pdev)
>  		private->comp_node[comp_id] = of_node_get(node);
>  
>  		/*
> -		 * Currently only the CCORR, COLOR, GAMMA, OVL, RDMA, DSI, and DPI
> +		 * Currently only the AAL, CCORR, COLOR, GAMMA, OVL, RDMA, DSI, and DPI
>  		 * blocks have separate component platform drivers and initialize their own
>  		 * DDP component structure. The others are initialized here.
>  		 */
> -		if (comp_type == MTK_DISP_CCORR ||
> +		if (comp_type == MTK_DISP_AAL ||
> +		    comp_type == MTK_DISP_CCORR ||
>  		    comp_type == MTK_DISP_COLOR ||
>  		    comp_type == MTK_DISP_GAMMA ||
>  		    comp_type == MTK_DISP_OVL ||
> @@ -635,6 +638,7 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
>  };
>  
>  static struct platform_driver * const mtk_drm_drivers[] = {
> +	&mtk_disp_aal_driver,
>  	&mtk_disp_ccorr_driver,
>  	&mtk_disp_color_driver,
>  	&mtk_disp_gamma_driver,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> index 637f566..3e7d1e6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
> @@ -46,6 +46,7 @@ struct mtk_drm_private {
>  	struct drm_atomic_state *suspend_state;
>  };
>  
> +extern struct platform_driver mtk_disp_aal_driver;
>  extern struct platform_driver mtk_disp_ccorr_driver;
>  extern struct platform_driver mtk_disp_color_driver;
>  extern struct platform_driver mtk_disp_gamma_driver;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq
  2021-04-12  6:35 ` [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq Yongqiang Niu
@ 2021-04-12  8:28   ` CK Hu
  2021-04-12  8:45     ` Yongqiang Niu
  0 siblings, 1 reply; 8+ messages in thread
From: CK Hu @ 2021-04-12  8:28 UTC (permalink / raw)
  To: Yongqiang Niu
  Cc: Chun-Kuang Hu, Rob Herring, Philipp Zabel, David Airlie,
	Daniel Vetter, Mark Rutland, Matthias Brugger, dri-devel,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	Project_Global_Chrome_Upstream_Group

Hi, Yongqiang:

On Mon, 2021-04-12 at 14:35 +0800, Yongqiang Niu wrote:
> gamma lut set in vsync active will caused display flash issue
> set gamma lut with cmdq 

In MT8173, it's ok to set gammma out of vblank period. Why do you
setting gamma in vblank in this patch?

Regards,
CK

> 
> Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |  4 ++--
>  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  7 ++++---
>  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 11 ++++++-----
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 18 +++++++++++-------
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  8 +++++---
>  5 files changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 64b4528..c8e178e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -59,12 +59,12 @@ void mtk_aal_config(struct device *dev, unsigned int w,
>  	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
>  }
>  
> -void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
> +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
>  {
>  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
>  
>  	if (aal->data && aal->data->has_gamma)
> -		mtk_gamma_set_common(aal->regs, state);
> +		mtk_gamma_set_common(aal->regs, &aal->cmdq_reg, state, cmdq_pkt);
>  }
>  
>  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 86c3068..c2e7dcb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -14,7 +14,7 @@
>  void mtk_aal_config(struct device *dev, unsigned int w,
>  		    unsigned int h, unsigned int vrefresh,
>  		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> -void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
> +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
>  void mtk_aal_start(struct device *dev);
>  void mtk_aal_stop(struct device *dev);
>  
> @@ -50,8 +50,9 @@ void mtk_dither_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
>  void mtk_gamma_config(struct device *dev, unsigned int w,
>  		      unsigned int h, unsigned int vrefresh,
>  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
> -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state);
> +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
> +void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
> +			  struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
>  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 3ebf91e..99a4ff3 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -55,7 +55,8 @@ 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)
> +void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
> +			  struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
>  {
>  	unsigned int i, reg;
>  	struct drm_color_lut *lut;
> @@ -65,23 +66,23 @@ void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state)
>  	if (state->gamma_lut) {
>  		reg = readl(regs + DISP_GAMMA_CFG);
>  		reg = reg | GAMMA_LUT_EN;
> -		writel(reg, regs + DISP_GAMMA_CFG);
> +		mtk_ddp_write(cmdq_pkt, reg, cmdq_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++) {
>  			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
>  				(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
>  				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> -			writel(word, (lut_base + i * 4));
> +			mtk_ddp_write(cmdq_pkt, word, cmdq_reg, regs, (lut_base + i * 4));
>  		}
>  	}
>  }
>  
> -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
>  {
>  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>  
> -	mtk_gamma_set_common(gamma->regs, state);
> +	mtk_gamma_set_common(gamma->regs, &gamma->cmdq_reg, state, cmdq_pkt);
>  }
>  
>  void mtk_gamma_config(struct device *dev, unsigned int w,
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index 8b0de90..73428f0 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -423,6 +423,15 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc,
>  		}
>  		mtk_crtc->pending_async_planes = false;
>  	}
> +
> +	if (crtc->state->color_mgmt_changed) {
> +		int i;
> +
> +		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> +			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state, cmdq_handle);
> +			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> +		}
> +	}
>  }
>  
>  static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> @@ -464,7 +473,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
>  #if IS_REACHABLE(CONFIG_MTK_CMDQ)
>  	if (mtk_crtc->cmdq_client) {
>  		mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
> -		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
> +		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, 2 * PAGE_SIZE);
>  		cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
>  		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
>  		mtk_crtc_ddp_config(crtc, cmdq_handle);
> @@ -616,15 +625,10 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
>  				      struct drm_atomic_state *state)
>  {
>  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> -	int i;
>  
>  	if (mtk_crtc->event)
>  		mtk_crtc->pending_needs_vblank = true;
> -	if (crtc->state->color_mgmt_changed)
> -		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> -			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> -			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> -		}
> +
>  	mtk_drm_crtc_hw_config(mtk_crtc);
>  }
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index bb914d9..bffa58d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -60,7 +60,8 @@ struct mtk_ddp_comp_funcs {
>  			     struct mtk_plane_state *state,
>  			     struct cmdq_pkt *cmdq_pkt);
>  	void (*gamma_set)(struct device *dev,
> -			  struct drm_crtc_state *state);
> +			  struct drm_crtc_state *state,
> +			  struct cmdq_pkt *cmdq_pkt);
>  	void (*bgclr_in_on)(struct device *dev);
>  	void (*bgclr_in_off)(struct device *dev);
>  	void (*ctm_set)(struct device *dev,
> @@ -160,10 +161,11 @@ static inline void mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
>  }
>  
>  static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
> -				     struct drm_crtc_state *state)
> +				     struct drm_crtc_state *state,
> +				     struct cmdq_pkt *cmdq_pkt)
>  {
>  	if (comp->funcs && comp->funcs->gamma_set)
> -		comp->funcs->gamma_set(comp->dev, state);
> +		comp->funcs->gamma_set(comp->dev, state, cmdq_pkt);
>  }
>  
>  static inline void mtk_ddp_comp_bgclr_in_on(struct mtk_ddp_comp *comp)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq
  2021-04-12  8:28   ` CK Hu
@ 2021-04-12  8:45     ` Yongqiang Niu
  2021-04-12  9:05       ` CK Hu
  0 siblings, 1 reply; 8+ messages in thread
From: Yongqiang Niu @ 2021-04-12  8:45 UTC (permalink / raw)
  To: CK Hu
  Cc: Chun-Kuang Hu, Rob Herring, Philipp Zabel, David Airlie,
	Daniel Vetter, Mark Rutland, Matthias Brugger, dri-devel,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	Project_Global_Chrome_Upstream_Group

On Mon, 2021-04-12 at 16:28 +0800, CK Hu wrote:
> Hi, Yongqiang:
> 
> On Mon, 2021-04-12 at 14:35 +0800, Yongqiang Niu wrote:
> > gamma lut set in vsync active will caused display flash issue
> > set gamma lut with cmdq 
> 
> In MT8173, it's ok to set gammma out of vblank period. Why do you
> setting gamma in vblank in this patch?
> 
> Regards,
> CK
> 

mtk drm driver code has changed many since mt8173, there is no one test
this in the newest version for mt8173.

and this issue is random.
https://partnerissuetracker.corp.google.com/u/1/issues/153842418

and not all platform will set gamma lut.
some project platform will not set gamma lut from chrome os
(crhome os set gamma lut may be with some special panel, like AUO
B116XTN02.3 in
https://partnerissuetracker.corp.google.com/u/1/issues/153842418
)

> > 
> > Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |  4 ++--
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  7 ++++---
> >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 11 ++++++-----
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 18 +++++++++++-------
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  8 +++++---
> >  5 files changed, 28 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > index 64b4528..c8e178e 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > @@ -59,12 +59,12 @@ void mtk_aal_config(struct device *dev, unsigned int w,
> >  	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
> >  }
> >  
> > -void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
> > +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
> >  {
> >  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> >  
> >  	if (aal->data && aal->data->has_gamma)
> > -		mtk_gamma_set_common(aal->regs, state);
> > +		mtk_gamma_set_common(aal->regs, &aal->cmdq_reg, state, cmdq_pkt);
> >  }
> >  
> >  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 86c3068..c2e7dcb 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -14,7 +14,7 @@
> >  void mtk_aal_config(struct device *dev, unsigned int w,
> >  		    unsigned int h, unsigned int vrefresh,
> >  		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> > -void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
> > +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
> >  void mtk_aal_start(struct device *dev);
> >  void mtk_aal_stop(struct device *dev);
> >  
> > @@ -50,8 +50,9 @@ void mtk_dither_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
> >  void mtk_gamma_config(struct device *dev, unsigned int w,
> >  		      unsigned int h, unsigned int vrefresh,
> >  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> > -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
> > -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state);
> > +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
> > +void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
> > +			  struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
> >  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 3ebf91e..99a4ff3 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > @@ -55,7 +55,8 @@ 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)
> > +void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
> > +			  struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
> >  {
> >  	unsigned int i, reg;
> >  	struct drm_color_lut *lut;
> > @@ -65,23 +66,23 @@ void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state)
> >  	if (state->gamma_lut) {
> >  		reg = readl(regs + DISP_GAMMA_CFG);
> >  		reg = reg | GAMMA_LUT_EN;
> > -		writel(reg, regs + DISP_GAMMA_CFG);
> > +		mtk_ddp_write(cmdq_pkt, reg, cmdq_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++) {
> >  			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
> >  				(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
> >  				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> > -			writel(word, (lut_base + i * 4));
> > +			mtk_ddp_write(cmdq_pkt, word, cmdq_reg, regs, (lut_base + i * 4));
> >  		}
> >  	}
> >  }
> >  
> > -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> > +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
> >  {
> >  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> >  
> > -	mtk_gamma_set_common(gamma->regs, state);
> > +	mtk_gamma_set_common(gamma->regs, &gamma->cmdq_reg, state, cmdq_pkt);
> >  }
> >  
> >  void mtk_gamma_config(struct device *dev, unsigned int w,
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 8b0de90..73428f0 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -423,6 +423,15 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc,
> >  		}
> >  		mtk_crtc->pending_async_planes = false;
> >  	}
> > +
> > +	if (crtc->state->color_mgmt_changed) {
> > +		int i;
> > +
> > +		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> > +			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state, cmdq_handle);
> > +			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> > +		}
> > +	}
> >  }
> >  
> >  static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> > @@ -464,7 +473,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> >  #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> >  	if (mtk_crtc->cmdq_client) {
> >  		mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
> > -		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
> > +		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, 2 * PAGE_SIZE);
> >  		cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> >  		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
> >  		mtk_crtc_ddp_config(crtc, cmdq_handle);
> > @@ -616,15 +625,10 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> >  				      struct drm_atomic_state *state)
> >  {
> >  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > -	int i;
> >  
> >  	if (mtk_crtc->event)
> >  		mtk_crtc->pending_needs_vblank = true;
> > -	if (crtc->state->color_mgmt_changed)
> > -		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> > -			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> > -			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> > -		}
> > +
> >  	mtk_drm_crtc_hw_config(mtk_crtc);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > index bb914d9..bffa58d 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > @@ -60,7 +60,8 @@ struct mtk_ddp_comp_funcs {
> >  			     struct mtk_plane_state *state,
> >  			     struct cmdq_pkt *cmdq_pkt);
> >  	void (*gamma_set)(struct device *dev,
> > -			  struct drm_crtc_state *state);
> > +			  struct drm_crtc_state *state,
> > +			  struct cmdq_pkt *cmdq_pkt);
> >  	void (*bgclr_in_on)(struct device *dev);
> >  	void (*bgclr_in_off)(struct device *dev);
> >  	void (*ctm_set)(struct device *dev,
> > @@ -160,10 +161,11 @@ static inline void mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
> >  }
> >  
> >  static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
> > -				     struct drm_crtc_state *state)
> > +				     struct drm_crtc_state *state,
> > +				     struct cmdq_pkt *cmdq_pkt)
> >  {
> >  	if (comp->funcs && comp->funcs->gamma_set)
> > -		comp->funcs->gamma_set(comp->dev, state);
> > +		comp->funcs->gamma_set(comp->dev, state, cmdq_pkt);
> >  }
> >  
> >  static inline void mtk_ddp_comp_bgclr_in_on(struct mtk_ddp_comp *comp)
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq
  2021-04-12  8:45     ` Yongqiang Niu
@ 2021-04-12  9:05       ` CK Hu
  0 siblings, 0 replies; 8+ messages in thread
From: CK Hu @ 2021-04-12  9:05 UTC (permalink / raw)
  To: Yongqiang Niu
  Cc: Chun-Kuang Hu, Rob Herring, Philipp Zabel, David Airlie,
	Daniel Vetter, Mark Rutland, Matthias Brugger, dri-devel,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	Project_Global_Chrome_Upstream_Group

Hi, Yongqiang:

On Mon, 2021-04-12 at 16:45 +0800, Yongqiang Niu wrote:
> On Mon, 2021-04-12 at 16:28 +0800, CK Hu wrote:
> > Hi, Yongqiang:
> > 
> > On Mon, 2021-04-12 at 14:35 +0800, Yongqiang Niu wrote:
> > > gamma lut set in vsync active will caused display flash issue
> > > set gamma lut with cmdq 
> > 
> > In MT8173, it's ok to set gammma out of vblank period. Why do you
> > setting gamma in vblank in this patch?
> > 
> > Regards,
> > CK
> > 
> 
> mtk drm driver code has changed many since mt8173, there is no one test
> this in the newest version for mt8173.
> 
> and this issue is random.
> https://partnerissuetracker.corp.google.com/u/1/issues/153842418
> 
> and not all platform will set gamma lut.
> some project platform will not set gamma lut from chrome os
> (crhome os set gamma lut may be with some special panel, like AUO
> B116XTN02.3 in
> https://partnerissuetracker.corp.google.com/u/1/issues/153842418
> )
> 


I could not see the page in partnelissuetracker, If this patch fix some
bug, describe the bug in commit message. It's better that information
include how to reproduce this bug and what kind of error happen. More
information would help us to understand why setting in vblank would fix
this bug.

Regards,
CK

> > > 
> > > Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> > > ---
> > >  drivers/gpu/drm/mediatek/mtk_disp_aal.c     |  4 ++--
> > >  drivers/gpu/drm/mediatek/mtk_disp_drv.h     |  7 ++++---
> > >  drivers/gpu/drm/mediatek/mtk_disp_gamma.c   | 11 ++++++-----
> > >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 18 +++++++++++-------
> > >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  8 +++++---
> > >  5 files changed, 28 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > index 64b4528..c8e178e 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> > > @@ -59,12 +59,12 @@ void mtk_aal_config(struct device *dev, unsigned int w,
> > >  	mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
> > >  }
> > >  
> > > -void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
> > > +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
> > >  {
> > >  	struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> > >  
> > >  	if (aal->data && aal->data->has_gamma)
> > > -		mtk_gamma_set_common(aal->regs, state);
> > > +		mtk_gamma_set_common(aal->regs, &aal->cmdq_reg, state, cmdq_pkt);
> > >  }
> > >  
> > >  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 86c3068..c2e7dcb 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > > @@ -14,7 +14,7 @@
> > >  void mtk_aal_config(struct device *dev, unsigned int w,
> > >  		    unsigned int h, unsigned int vrefresh,
> > >  		    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> > > -void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
> > > +void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
> > >  void mtk_aal_start(struct device *dev);
> > >  void mtk_aal_stop(struct device *dev);
> > >  
> > > @@ -50,8 +50,9 @@ void mtk_dither_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
> > >  void mtk_gamma_config(struct device *dev, unsigned int w,
> > >  		      unsigned int h, unsigned int vrefresh,
> > >  		      unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> > > -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
> > > -void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state);
> > > +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
> > > +void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
> > > +			  struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt);
> > >  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 3ebf91e..99a4ff3 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> > > @@ -55,7 +55,8 @@ 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)
> > > +void mtk_gamma_set_common(void __iomem *regs, struct cmdq_client_reg *cmdq_reg,
> > > +			  struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
> > >  {
> > >  	unsigned int i, reg;
> > >  	struct drm_color_lut *lut;
> > > @@ -65,23 +66,23 @@ void mtk_gamma_set_common(void __iomem *regs, struct drm_crtc_state *state)
> > >  	if (state->gamma_lut) {
> > >  		reg = readl(regs + DISP_GAMMA_CFG);
> > >  		reg = reg | GAMMA_LUT_EN;
> > > -		writel(reg, regs + DISP_GAMMA_CFG);
> > > +		mtk_ddp_write(cmdq_pkt, reg, cmdq_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++) {
> > >  			word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
> > >  				(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
> > >  				((lut[i].blue >> 6) & LUT_10BIT_MASK);
> > > -			writel(word, (lut_base + i * 4));
> > > +			mtk_ddp_write(cmdq_pkt, word, cmdq_reg, regs, (lut_base + i * 4));
> > >  		}
> > >  	}
> > >  }
> > >  
> > > -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> > > +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state, struct cmdq_pkt *cmdq_pkt)
> > >  {
> > >  	struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> > >  
> > > -	mtk_gamma_set_common(gamma->regs, state);
> > > +	mtk_gamma_set_common(gamma->regs, &gamma->cmdq_reg, state, cmdq_pkt);
> > >  }
> > >  
> > >  void mtk_gamma_config(struct device *dev, unsigned int w,
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > index 8b0de90..73428f0 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > > @@ -423,6 +423,15 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc,
> > >  		}
> > >  		mtk_crtc->pending_async_planes = false;
> > >  	}
> > > +
> > > +	if (crtc->state->color_mgmt_changed) {
> > > +		int i;
> > > +
> > > +		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> > > +			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state, cmdq_handle);
> > > +			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> > > +		}
> > > +	}
> > >  }
> > >  
> > >  static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> > > @@ -464,7 +473,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc *mtk_crtc)
> > >  #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > >  	if (mtk_crtc->cmdq_client) {
> > >  		mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
> > > -		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
> > > +		cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, 2 * PAGE_SIZE);
> > >  		cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
> > >  		cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
> > >  		mtk_crtc_ddp_config(crtc, cmdq_handle);
> > > @@ -616,15 +625,10 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> > >  				      struct drm_atomic_state *state)
> > >  {
> > >  	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
> > > -	int i;
> > >  
> > >  	if (mtk_crtc->event)
> > >  		mtk_crtc->pending_needs_vblank = true;
> > > -	if (crtc->state->color_mgmt_changed)
> > > -		for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> > > -			mtk_ddp_gamma_set(mtk_crtc->ddp_comp[i], crtc->state);
> > > -			mtk_ddp_ctm_set(mtk_crtc->ddp_comp[i], crtc->state);
> > > -		}
> > > +
> > >  	mtk_drm_crtc_hw_config(mtk_crtc);
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > > index bb914d9..bffa58d 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> > > @@ -60,7 +60,8 @@ struct mtk_ddp_comp_funcs {
> > >  			     struct mtk_plane_state *state,
> > >  			     struct cmdq_pkt *cmdq_pkt);
> > >  	void (*gamma_set)(struct device *dev,
> > > -			  struct drm_crtc_state *state);
> > > +			  struct drm_crtc_state *state,
> > > +			  struct cmdq_pkt *cmdq_pkt);
> > >  	void (*bgclr_in_on)(struct device *dev);
> > >  	void (*bgclr_in_off)(struct device *dev);
> > >  	void (*ctm_set)(struct device *dev,
> > > @@ -160,10 +161,11 @@ static inline void mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
> > >  }
> > >  
> > >  static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
> > > -				     struct drm_crtc_state *state)
> > > +				     struct drm_crtc_state *state,
> > > +				     struct cmdq_pkt *cmdq_pkt)
> > >  {
> > >  	if (comp->funcs && comp->funcs->gamma_set)
> > > -		comp->funcs->gamma_set(comp->dev, state);
> > > +		comp->funcs->gamma_set(comp->dev, state, cmdq_pkt);
> > >  }
> > >  
> > >  static inline void mtk_ddp_comp_bgclr_in_on(struct mtk_ddp_comp *comp)
> > 
> > 
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-04-12  9:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12  6:35 [PATCH v1, 0/3] gamma set with cmdq Yongqiang Niu
2021-04-12  6:35 ` [PATCH v1, 1/3] drm/mediatek: Separate aal module Yongqiang Niu
2021-04-12  8:26   ` CK Hu
2021-04-12  6:35 ` [PATCH v1, 2/3] arm64: dts: mt8183: refine aal compatible name Yongqiang Niu
2021-04-12  6:35 ` [PATCH v1, 3/3] drm/mediatek: gamma set with cmdq Yongqiang Niu
2021-04-12  8:28   ` CK Hu
2021-04-12  8:45     ` Yongqiang Niu
2021-04-12  9:05       ` CK Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).