[PATCH v9 00/10] MT2701 DRM support

[PATCH v9 00/10] MT2701 DRM support

[YT Shen]

This is MT2701 DRM support PATCH v9, based on 4.9-rc1.
We add DSI interrupt control, transfer function for MIPI DSI panel support.
Most codes are the same, except some register changed.

For example:
 - DISP_OVL address offset changed, color format definition changed.
 - DISP_RDMA fifo size changed.
 - DISP_COLOR offset changed.
 - MIPI_TX setting changed.

We add a new component DDP_COMPONENT_BLS, and the connections are updated.
OVL -> RDMA -> COLOR -> BLS -> DSI
RDMA -> DPI
And we have shadow register support in MT2701.

We remove dts patch from the patch series, which depends on MT2701 CCF and scpsys.

Changes since v8:
- enable 3 DSI interrupts only
- move mtk_dsi_wait_for_irq_done() to the patch of irq control
- use the name BLS in DRM driver part
- move BLS declaration to a separate patch
- update mtk_dsi_switch_to_cmd_mode()
- update mtk_output_dsi_enable() and mtk_output_dsi_disable()

Changes since v7:
- Remove redundant codes
- Move the definition of DDP_COMPONENT_BLS to patch of "drm/mediatek: update display module connections"
- Move _dsi_irq_wait_queue into platform driver data
- Move mtk_dsi_irq_data_clear() to patch of "drm/mediatek: add dsi interrupt control"
- Add more descriptions in the commit messages

Changes since v6:
- Change data type of irq_data to u32
- Rewrite mtk_dsi_host_transfer() for simplify
- Move some MIPI_TX config to patch of "drm/mediatek: add *driver_data for different hardware settings".
- Remove device tree from this patch series

Changes since v5:
- Remove DPI device tree and compatible string
- Use one wait queue to handle interrupt status
- Update the interrupt check flow and DSI_INT_ALL_BITS
- Use same function for host read/write command
- various fixes

Changes since v4:
- Add messages when timeout in mtk_disp_mutex_acquire()
- Add descriptions for DISP_REG_MUTEX registers
- Move connection settings for display modules to a separate patch
- Remove 'mt2701-disp-wdma' because it is unused
- Move cleaning up and renaming to a separate patch
- Use wait_event_interruptible_timeout() to replace polling
- Remove irq_num from mtk_dsi structure
- Remove redundant and debug codes

Changes since v3:
- Add DSI support for MIPI DSI panels
- Update BLS binding to PWM nodes
- Remove ufoe device nodes
- Remove redundant parentheses
- Remove global variable initialization

Changes since v2:
- Rename mtk_ddp_mux_sel to mtk_ddp_sout_sel
- Update mt2701_mtk_ddp_ext components
- Changed to prefix naming
- Reorder the patch series
- Use of_device_get_match_data() to get driver private data
- Use iopoll macros to implement mtk_disp_mutex_acquire()
- Removed empty device tree nodes

Changes since v1:
- Removed BLS bindings and codes, which belong to pwm driver
- Moved mtk_disp_mutex_acquire() just before mtk_crtc_ddp_config()
- Split patch into smaller parts
- Added const keyword to constant structure
- Removed codes for special memory align

Thanks,
yt.shen

YT Shen (8):
  drm/mediatek: rename macros, add chip prefix
  drm/mediatek: add *driver_data for different hardware settings
  drm/mediatek: add shadow register support
  drm/mediatek: add BLS component
  drm/mediatek: update display module connections
  drm/mediatek: cleaning up and refine
  drm/mediatek: update DSI sub driver flow for sending commands to panel
  drm/mediatek: add support for Mediatek SoC MT2701

shaoming chen (2):
  drm/mediatek: add dsi interrupt control
  drm/mediatek: add dsi transfer function

 drivers/gpu/drm/mediatek/mtk_disp_ovl.c     |  33 ++-
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c    |  17 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  76 +++--
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 138 ++++++---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h      |   2 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  38 ++-
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  15 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      |  54 +++-
 drivers/gpu/drm/mediatek/mtk_drm_drv.h      |   9 +
 drivers/gpu/drm/mediatek/mtk_dsi.c          | 429 ++++++++++++++++++++++++----
 drivers/gpu/drm/mediatek/mtk_mipi_tx.c      |  70 +++--
 11 files changed, 715 insertions(+), 166 deletions(-)

-- 
1.9.1

[PATCH v9 01/10] drm/mediatek: rename macros, add chip prefix

[YT Shen]

Add MT8173 prefix for hardware related macros.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 60 +++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 17ba935..2fc4321 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -36,21 +36,21 @@
 #define DISP_REG_MUTEX_MOD(n)	(0x2c + 0x20 * (n))
 #define DISP_REG_MUTEX_SOF(n)	(0x30 + 0x20 * (n))
 
-#define MUTEX_MOD_DISP_OVL0		BIT(11)
-#define MUTEX_MOD_DISP_OVL1		BIT(12)
-#define MUTEX_MOD_DISP_RDMA0		BIT(13)
-#define MUTEX_MOD_DISP_RDMA1		BIT(14)
-#define MUTEX_MOD_DISP_RDMA2		BIT(15)
-#define MUTEX_MOD_DISP_WDMA0		BIT(16)
-#define MUTEX_MOD_DISP_WDMA1		BIT(17)
-#define MUTEX_MOD_DISP_COLOR0		BIT(18)
-#define MUTEX_MOD_DISP_COLOR1		BIT(19)
-#define MUTEX_MOD_DISP_AAL		BIT(20)
-#define MUTEX_MOD_DISP_GAMMA		BIT(21)
-#define MUTEX_MOD_DISP_UFOE		BIT(22)
-#define MUTEX_MOD_DISP_PWM0		BIT(23)
-#define MUTEX_MOD_DISP_PWM1		BIT(24)
-#define MUTEX_MOD_DISP_OD		BIT(25)
+#define MT8173_MUTEX_MOD_DISP_OVL0		BIT(11)
+#define MT8173_MUTEX_MOD_DISP_OVL1		BIT(12)
+#define MT8173_MUTEX_MOD_DISP_RDMA0		BIT(13)
+#define MT8173_MUTEX_MOD_DISP_RDMA1		BIT(14)
+#define MT8173_MUTEX_MOD_DISP_RDMA2		BIT(15)
+#define MT8173_MUTEX_MOD_DISP_WDMA0		BIT(16)
+#define MT8173_MUTEX_MOD_DISP_WDMA1		BIT(17)
+#define MT8173_MUTEX_MOD_DISP_COLOR0		BIT(18)
+#define MT8173_MUTEX_MOD_DISP_COLOR1		BIT(19)
+#define MT8173_MUTEX_MOD_DISP_AAL		BIT(20)
+#define MT8173_MUTEX_MOD_DISP_GAMMA		BIT(21)
+#define MT8173_MUTEX_MOD_DISP_UFOE		BIT(22)
+#define MT8173_MUTEX_MOD_DISP_PWM0		BIT(23)
+#define MT8173_MUTEX_MOD_DISP_PWM1		BIT(24)
+#define MT8173_MUTEX_MOD_DISP_OD		BIT(25)
 
 #define MUTEX_SOF_SINGLE_MODE		0
 #define MUTEX_SOF_DSI0			1
@@ -80,21 +80,21 @@ struct mtk_ddp {
 };
 
 static const unsigned int mutex_mod[DDP_COMPONENT_ID_MAX] = {
-	[DDP_COMPONENT_AAL] = MUTEX_MOD_DISP_AAL,
-	[DDP_COMPONENT_COLOR0] = MUTEX_MOD_DISP_COLOR0,
-	[DDP_COMPONENT_COLOR1] = MUTEX_MOD_DISP_COLOR1,
-	[DDP_COMPONENT_GAMMA] = MUTEX_MOD_DISP_GAMMA,
-	[DDP_COMPONENT_OD] = MUTEX_MOD_DISP_OD,
-	[DDP_COMPONENT_OVL0] = MUTEX_MOD_DISP_OVL0,
-	[DDP_COMPONENT_OVL1] = MUTEX_MOD_DISP_OVL1,
-	[DDP_COMPONENT_PWM0] = MUTEX_MOD_DISP_PWM0,
-	[DDP_COMPONENT_PWM1] = MUTEX_MOD_DISP_PWM1,
-	[DDP_COMPONENT_RDMA0] = MUTEX_MOD_DISP_RDMA0,
-	[DDP_COMPONENT_RDMA1] = MUTEX_MOD_DISP_RDMA1,
-	[DDP_COMPONENT_RDMA2] = MUTEX_MOD_DISP_RDMA2,
-	[DDP_COMPONENT_UFOE] = MUTEX_MOD_DISP_UFOE,
-	[DDP_COMPONENT_WDMA0] = MUTEX_MOD_DISP_WDMA0,
-	[DDP_COMPONENT_WDMA1] = MUTEX_MOD_DISP_WDMA1,
+	[DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL,
+	[DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0,
+	[DDP_COMPONENT_COLOR1] = MT8173_MUTEX_MOD_DISP_COLOR1,
+	[DDP_COMPONENT_GAMMA] = MT8173_MUTEX_MOD_DISP_GAMMA,
+	[DDP_COMPONENT_OD] = MT8173_MUTEX_MOD_DISP_OD,
+	[DDP_COMPONENT_OVL0] = MT8173_MUTEX_MOD_DISP_OVL0,
+	[DDP_COMPONENT_OVL1] = MT8173_MUTEX_MOD_DISP_OVL1,
+	[DDP_COMPONENT_PWM0] = MT8173_MUTEX_MOD_DISP_PWM0,
+	[DDP_COMPONENT_PWM1] = MT8173_MUTEX_MOD_DISP_PWM1,
+	[DDP_COMPONENT_RDMA0] = MT8173_MUTEX_MOD_DISP_RDMA0,
+	[DDP_COMPONENT_RDMA1] = MT8173_MUTEX_MOD_DISP_RDMA1,
+	[DDP_COMPONENT_RDMA2] = MT8173_MUTEX_MOD_DISP_RDMA2,
+	[DDP_COMPONENT_UFOE] = MT8173_MUTEX_MOD_DISP_UFOE,
+	[DDP_COMPONENT_WDMA0] = MT8173_MUTEX_MOD_DISP_WDMA0,
+	[DDP_COMPONENT_WDMA1] = MT8173_MUTEX_MOD_DISP_WDMA1,
 };
 
 static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id cur,
-- 
1.9.1

[PATCH v9 02/10] drm/mediatek: add *driver_data for different hardware settings

[YT Shen]

There are some hardware settings changed, between MT8173 & MT2701:
DISP_OVL address offset changed, color format definition changed.
DISP_RDMA fifo size changed.
DISP_COLOR offset changed.
MIPI_TX pll setting changed.
And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c     | 27 ++++++++++++++++-----------
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c    | 11 +++++++++--
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 11 +++++++----
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      | 25 ++++++++++++++++++-------
 drivers/gpu/drm/mediatek/mtk_drm_drv.h      |  8 ++++++++
 drivers/gpu/drm/mediatek/mtk_mipi_tx.c      | 24 +++++++++++++++++++++++-
 8 files changed, 115 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 019b7ca..1139834 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -35,13 +35,10 @@
 #define DISP_REG_OVL_PITCH(n)			(0x0044 + 0x20 * (n))
 #define DISP_REG_OVL_RDMA_CTRL(n)		(0x00c0 + 0x20 * (n))
 #define DISP_REG_OVL_RDMA_GMC(n)		(0x00c8 + 0x20 * (n))
-#define DISP_REG_OVL_ADDR(n)			(0x0f40 + 0x20 * (n))
 
 #define	OVL_RDMA_MEM_GMC	0x40402020
 
 #define OVL_CON_BYTE_SWAP	BIT(24)
-#define OVL_CON_CLRFMT_RGB565	(0 << 12)
-#define OVL_CON_CLRFMT_RGB888	(1 << 12)
 #define OVL_CON_CLRFMT_RGBA8888	(2 << 12)
 #define OVL_CON_CLRFMT_ARGB8888	(3 << 12)
 #define	OVL_CON_AEN		BIT(8)
@@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx)
 	writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
 }
 
-static unsigned int ovl_fmt_convert(unsigned int fmt)
+static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt)
 {
 	switch (fmt) {
 	default:
 	case DRM_FORMAT_RGB565:
-		return OVL_CON_CLRFMT_RGB565;
+		return comp->data->ovl.fmt_rgb565;
 	case DRM_FORMAT_BGR565:
-		return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP;
+		return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP;
 	case DRM_FORMAT_RGB888:
-		return OVL_CON_CLRFMT_RGB888;
+		return comp->data->ovl.fmt_rgb888;
 	case DRM_FORMAT_BGR888:
-		return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP;
+		return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP;
 	case DRM_FORMAT_RGBX8888:
 	case DRM_FORMAT_RGBA8888:
 		return OVL_CON_CLRFMT_ARGB8888;
@@ -178,7 +175,7 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
 	if (!pending->enable)
 		mtk_ovl_layer_off(comp, idx);
 
-	con = ovl_fmt_convert(fmt);
+	con = ovl_fmt_convert(comp, fmt);
 	if (idx != 0)
 		con |= OVL_CON_AEN | OVL_CON_ALPHA;
 
@@ -186,7 +183,8 @@ static void mtk_ovl_layer_config(struct mtk_ddp_comp *comp, unsigned int idx,
 	writel_relaxed(pitch, comp->regs + DISP_REG_OVL_PITCH(idx));
 	writel_relaxed(src_size, comp->regs + DISP_REG_OVL_SRC_SIZE(idx));
 	writel_relaxed(offset, comp->regs + DISP_REG_OVL_OFFSET(idx));
-	writel_relaxed(addr, comp->regs + DISP_REG_OVL_ADDR(idx));
+	writel_relaxed(addr, comp->regs + comp->data->ovl.addr_offset
+					+ idx * 0x20);
 
 	if (pending->enable)
 		mtk_ovl_layer_on(comp, idx);
@@ -270,6 +268,8 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	priv->ddp_comp.data = of_device_get_match_data(dev);
+
 	platform_set_drvdata(pdev, priv);
 
 	ret = component_add(dev, &mtk_disp_ovl_component_ops);
@@ -286,8 +286,13 @@ static int mtk_disp_ovl_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
+	.ovl = {0x0f40, 0, 1 << 12}
+};
+
 static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-ovl", },
+	{ .compatible = "mediatek,mt8173-disp-ovl",
+	  .data = &mt8173_ovl_driver_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_ovl_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
index 0df05f9..b4225e2 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
@@ -123,7 +123,7 @@ static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width,
 	 */
 	threshold = width * height * vrefresh * 4 * 7 / 1000000;
 	reg = RDMA_FIFO_UNDERFLOW_EN |
-	      RDMA_FIFO_PSEUDO_SIZE(SZ_8K) |
+	      RDMA_FIFO_PSEUDO_SIZE(comp->data->rdma_fifo_pseudo_size) |
 	      RDMA_OUTPUT_VALID_FIFO_THRESHOLD(threshold);
 	writel(reg, comp->regs + DISP_REG_RDMA_FIFO_CON);
 }
@@ -208,6 +208,8 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	priv->ddp_comp.data = of_device_get_match_data(dev);
+
 	platform_set_drvdata(pdev, priv);
 
 	ret = component_add(dev, &mtk_disp_rdma_component_ops);
@@ -224,8 +226,13 @@ static int mtk_disp_rdma_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_ddp_comp_driver_data mt8173_rdma_driver_data = {
+	.rdma_fifo_pseudo_size = SZ_8K,
+};
+
 static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-rdma", },
+	{ .compatible = "mediatek,mt8173-disp-rdma",
+	  .data = &mt8173_rdma_driver_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, mtk_disp_rdma_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 2fc4321..8030769 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -77,9 +77,10 @@ struct mtk_ddp {
 	struct clk			*clk;
 	void __iomem			*regs;
 	struct mtk_disp_mutex		mutex[10];
+	const unsigned int		*mutex_mod;
 };
 
-static const unsigned int mutex_mod[DDP_COMPONENT_ID_MAX] = {
+static const unsigned int mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = {
 	[DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL,
 	[DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0,
 	[DDP_COMPONENT_COLOR1] = MT8173_MUTEX_MOD_DISP_COLOR1,
@@ -247,7 +248,7 @@ void mtk_disp_mutex_add_comp(struct mtk_disp_mutex *mutex,
 		break;
 	default:
 		reg = readl_relaxed(ddp->regs + DISP_REG_MUTEX_MOD(mutex->id));
-		reg |= mutex_mod[id];
+		reg |= ddp->mutex_mod[id];
 		writel_relaxed(reg, ddp->regs + DISP_REG_MUTEX_MOD(mutex->id));
 		return;
 	}
@@ -273,7 +274,7 @@ void mtk_disp_mutex_remove_comp(struct mtk_disp_mutex *mutex,
 		break;
 	default:
 		reg = readl_relaxed(ddp->regs + DISP_REG_MUTEX_MOD(mutex->id));
-		reg &= ~mutex_mod[id];
+		reg &= ~(ddp->mutex_mod[id]);
 		writel_relaxed(reg, ddp->regs + DISP_REG_MUTEX_MOD(mutex->id));
 		break;
 	}
@@ -326,6 +327,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
 		return PTR_ERR(ddp->regs);
 	}
 
+	ddp->mutex_mod = of_device_get_match_data(dev);
+
 	platform_set_drvdata(pdev, ddp);
 
 	return 0;
@@ -337,7 +340,7 @@ static int mtk_ddp_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id ddp_driver_dt_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-mutex" },
+	{ .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod},
 	{},
 };
 MODULE_DEVICE_TABLE(of, ddp_driver_dt_match);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index df33b3c..661a4a0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -39,9 +39,8 @@
 #define DISP_REG_UFO_START			0x0000
 
 #define DISP_COLOR_CFG_MAIN			0x0400
-#define DISP_COLOR_START			0x0c00
-#define DISP_COLOR_WIDTH			0x0c50
-#define DISP_COLOR_HEIGHT			0x0c54
+#define DISP_COLOR_WIDTH			0x50
+#define DISP_COLOR_HEIGHT			0x54
 
 #define DISP_AAL_EN				0x0000
 #define DISP_AAL_SIZE				0x0030
@@ -107,15 +106,15 @@ static void mtk_color_config(struct mtk_ddp_comp *comp, unsigned int w,
 			     unsigned int h, unsigned int vrefresh,
 			     unsigned int bpc)
 {
-	writel(w, comp->regs + DISP_COLOR_WIDTH);
-	writel(h, comp->regs + DISP_COLOR_HEIGHT);
+	writel(w, comp->regs + comp->data->color_offset + DISP_COLOR_WIDTH);
+	writel(h, comp->regs + comp->data->color_offset + DISP_COLOR_HEIGHT);
 }
 
 static void mtk_color_start(struct mtk_ddp_comp *comp)
 {
 	writel(COLOR_BYPASS_ALL | COLOR_SEQ_SEL,
 	       comp->regs + DISP_COLOR_CFG_MAIN);
-	writel(0x1, comp->regs + DISP_COLOR_START);
+	writel(0x1, comp->regs + comp->data->color_offset);
 }
 
 static void mtk_od_config(struct mtk_ddp_comp *comp, unsigned int w,
@@ -264,6 +263,16 @@ struct mtk_ddp_comp_match {
 	[DDP_COMPONENT_WDMA1]	= { MTK_DISP_WDMA,	1, NULL },
 };
 
+static const struct mtk_ddp_comp_driver_data mt8173_color_driver_data = {
+	.color_offset = 0x0c00,
+};
+
+static const struct of_device_id mtk_disp_color_driver_dt_match[] = {
+	{ .compatible = "mediatek,mt8173-disp-color",
+	  .data = &mt8173_color_driver_data},
+	{},
+};
+
 int mtk_ddp_comp_get_id(struct device_node *node,
 			enum mtk_ddp_comp_type comp_type)
 {
@@ -286,6 +295,7 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
 	enum mtk_ddp_comp_type type;
 	struct device_node *larb_node;
 	struct platform_device *larb_pdev;
+	const struct of_device_id *match;
 
 	if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX)
 		return -EINVAL;
@@ -310,6 +320,11 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
 
 	type = mtk_ddp_matches[comp_id].type;
 
+	if (type == MTK_DISP_COLOR) {
+		match = of_match_node(mtk_disp_color_driver_dt_match, node);
+		comp->data = match->data;
+	}
+
 	/* Only DMA capable components need the LARB property */
 	comp->larb_dev = NULL;
 	if (type != MTK_DISP_OVL &&
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 22a33ee..2f6872a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -78,6 +78,18 @@ struct mtk_ddp_comp_funcs {
 			  struct drm_crtc_state *state);
 };
 
+struct mtk_ddp_comp_driver_data {
+	union {
+		struct ovl {
+			unsigned int addr_offset;
+			unsigned int fmt_rgb565;
+			unsigned int fmt_rgb888;
+		} ovl;
+		unsigned int rdma_fifo_pseudo_size;
+		unsigned int color_offset;
+	};
+};
+
 struct mtk_ddp_comp {
 	struct clk *clk;
 	void __iomem *regs;
@@ -85,6 +97,7 @@ struct mtk_ddp_comp {
 	struct device *larb_dev;
 	enum mtk_ddp_comp_id id;
 	const struct mtk_ddp_comp_funcs *funcs;
+	const struct mtk_ddp_comp_driver_data *data;
 };
 
 static inline void mtk_ddp_comp_config(struct mtk_ddp_comp *comp,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index cf83f65..5f9b5e8 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -126,7 +126,7 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	.atomic_commit = mtk_atomic_commit,
 };
 
-static const enum mtk_ddp_comp_id mtk_ddp_main[] = {
+static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
 	DDP_COMPONENT_OVL0,
 	DDP_COMPONENT_COLOR0,
 	DDP_COMPONENT_AAL,
@@ -137,7 +137,7 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	DDP_COMPONENT_PWM0,
 };
 
-static const enum mtk_ddp_comp_id mtk_ddp_ext[] = {
+static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
 	DDP_COMPONENT_OVL1,
 	DDP_COMPONENT_COLOR1,
 	DDP_COMPONENT_GAMMA,
@@ -145,6 +145,13 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	DDP_COMPONENT_DPI0,
 };
 
+static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
+	.main_path = mt8173_mtk_ddp_main,
+	.main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
+	.ext_path = mt8173_mtk_ddp_ext,
+	.ext_len = ARRAY_SIZE(mt8173_mtk_ddp_ext),
+};
+
 static int mtk_drm_kms_init(struct drm_device *drm)
 {
 	struct mtk_drm_private *private = drm->dev_private;
@@ -187,17 +194,19 @@ static int mtk_drm_kms_init(struct drm_device *drm)
 	 * and each statically assigned to a crtc:
 	 * OVL0 -> COLOR0 -> AAL -> OD -> RDMA0 -> UFOE -> DSI0 ...
 	 */
-	ret = mtk_drm_crtc_create(drm, mtk_ddp_main, ARRAY_SIZE(mtk_ddp_main));
+	ret = mtk_drm_crtc_create(drm, private->data->main_path,
+				  private->data->main_len);
 	if (ret < 0)
 		goto err_component_unbind;
 	/* ... and OVL1 -> COLOR1 -> GAMMA -> RDMA1 -> DPI0. */
-	ret = mtk_drm_crtc_create(drm, mtk_ddp_ext, ARRAY_SIZE(mtk_ddp_ext));
+	ret = mtk_drm_crtc_create(drm, private->data->ext_path,
+				  private->data->ext_len);
 	if (ret < 0)
 		goto err_component_unbind;
 
 	/* Use OVL device for all DMA memory allocations */
-	np = private->comp_node[mtk_ddp_main[0]] ?:
-	     private->comp_node[mtk_ddp_ext[0]];
+	np = private->comp_node[private->data->main_path[0]] ?:
+	     private->comp_node[private->data->ext_path[0]];
 	pdev = of_find_device_by_node(np);
 	if (!pdev) {
 		ret = -ENODEV;
@@ -362,6 +371,7 @@ static int mtk_drm_probe(struct platform_device *pdev)
 
 	mutex_init(&private->commit.lock);
 	INIT_WORK(&private->commit.work, mtk_atomic_work);
+	private->data = of_device_get_match_data(dev);
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	private->config_regs = devm_ioremap_resource(dev, mem);
@@ -512,7 +522,8 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
 			 mtk_drm_sys_resume);
 
 static const struct of_device_id mtk_drm_of_ids[] = {
-	{ .compatible = "mediatek,mt8173-mmsys", },
+	{ .compatible = "mediatek,mt8173-mmsys",
+	  .data = &mt8173_mmsys_driver_data},
 	{ }
 };
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index aa93894..fa0b106 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -28,6 +28,13 @@
 struct drm_property;
 struct regmap;
 
+struct mtk_mmsys_driver_data {
+	const enum mtk_ddp_comp_id *main_path;
+	unsigned int main_len;
+	const enum mtk_ddp_comp_id *ext_path;
+	unsigned int ext_len;
+};
+
 struct mtk_drm_private {
 	struct drm_device *drm;
 	struct device *dma_dev;
@@ -40,6 +47,7 @@ struct mtk_drm_private {
 	void __iomem *config_regs;
 	struct device_node *comp_node[DDP_COMPONENT_ID_MAX];
 	struct mtk_ddp_comp *ddp_comp[DDP_COMPONENT_ID_MAX];
+	const struct mtk_mmsys_driver_data *data;
 
 	struct {
 		struct drm_atomic_state *state;
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 1c366f8..935a8ef 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -16,6 +16,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
 
@@ -87,6 +88,9 @@
 
 #define MIPITX_DSI_PLL_CON2	0x58
 
+#define MIPITX_DSI_PLL_TOP	0x64
+#define RG_DSI_MPPLL_PRESERVE		(0xff << 8)
+
 #define MIPITX_DSI_PLL_PWR	0x68
 #define RG_DSI_MPPLL_SDM_PWR_ON		BIT(0)
 #define RG_DSI_MPPLL_SDM_ISO_EN		BIT(1)
@@ -123,10 +127,15 @@
 #define SW_LNT2_HSTX_PRE_OE		BIT(24)
 #define SW_LNT2_HSTX_OE			BIT(25)
 
+struct mtk_mipitx_data {
+	const u32 data;
+};
+
 struct mtk_mipi_tx {
 	struct device *dev;
 	void __iomem *regs;
 	unsigned int data_rate;
+	const struct mtk_mipitx_data *driver_data;
 	struct clk_hw pll_hw;
 	struct clk *pll;
 };
@@ -243,6 +252,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
 	mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
 			       RG_DSI_MPPLL_SDM_SSC_EN);
 
+	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_TOP,
+				RG_DSI_MPPLL_PRESERVE,
+				mipi_tx->driver_data->data);
+
 	return 0;
 }
 
@@ -255,6 +268,9 @@ static void mtk_mipi_tx_pll_unprepare(struct clk_hw *hw)
 	mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
 			       RG_DSI_MPPLL_PLL_EN);
 
+	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_TOP,
+				RG_DSI_MPPLL_PRESERVE, 0);
+
 	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR,
 				RG_DSI_MPPLL_SDM_ISO_EN |
 				RG_DSI_MPPLL_SDM_PWR_ON,
@@ -391,6 +407,7 @@ static int mtk_mipi_tx_probe(struct platform_device *pdev)
 	if (!mipi_tx)
 		return -ENOMEM;
 
+	mipi_tx->driver_data = of_device_get_match_data(dev);
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mipi_tx->regs = devm_ioremap_resource(dev, mem);
 	if (IS_ERR(mipi_tx->regs)) {
@@ -448,8 +465,13 @@ static int mtk_mipi_tx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_mipitx_data mt8173_mipitx_data = {
+	.data = (0 << 8)
+};
+
 static const struct of_device_id mtk_mipi_tx_match[] = {
-	{ .compatible = "mediatek,mt8173-mipi-tx", },
+	{ .compatible = "mediatek,mt8173-mipi-tx",
+	  .data = &mt8173_mipitx_data },
 	{},
 };
 
-- 
1.9.1

Re: [PATCH v9 02/10] drm/mediatek: add *driver_data for different hardware settings

[Daniel Kurtz]

Hi YT,

I don't see a reason to handle device_data in such a generic way at
the generic mtk_ddp_comp layer.
The device data is very component specific, so just define different
structs for different comp types, ie:

struct mtk_disp_ovl_driver_data {
    unsigned int reg_ovl_addr;
    unsigned int fmt_rgb565;
    unsigned int fmt_rgb888;
};

struct mtk_disp_rdma_driver_data {
    unsigned int fifo_pseudo_size;
};

struct mtk_disp_color_driver_data {
    unsigned int color_offset;
};

Then add typed pointers to the local structs that use them, for example:

struct mtk_disp_ovl {
        struct mtk_ddp_comp             ddp_comp;
        struct drm_crtc                 *crtc;
        const struct mtk_disp_ovl_driver_data *data;
};

And fetch the device specific driver data directly in .probe, as you
are already doing:

static int mtk_disp_ovl_probe(struct platform_device *pdev) {
  ...
  priv->data = of_device_get_match_data(dev);
  ...
}

More comments in-line...

On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
> There are some hardware settings changed, between MT8173 & MT2701:
> DISP_OVL address offset changed, color format definition changed.
> DISP_RDMA fifo size changed.
> DISP_COLOR offset changed.
> MIPI_TX pll setting changed.
> And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod.

Nit: I think it would make sense to combine this patch with
drm/mediatek: rename macros, add chip prefix

>
> Signed-off-by: YT Shen <yt.shen@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c     | 27 ++++++++++++++++-----------
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c    | 11 +++++++++--
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 11 +++++++----
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c      | 25 ++++++++++++++++++-------
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h      |  8 ++++++++
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.c      | 24 +++++++++++++++++++++++-
>  8 files changed, 115 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 019b7ca..1139834 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -35,13 +35,10 @@
>  #define DISP_REG_OVL_PITCH(n)                  (0x0044 + 0x20 * (n))
>  #define DISP_REG_OVL_RDMA_CTRL(n)              (0x00c0 + 0x20 * (n))
>  #define DISP_REG_OVL_RDMA_GMC(n)               (0x00c8 + 0x20 * (n))
> -#define DISP_REG_OVL_ADDR(n)                   (0x0f40 + 0x20 * (n))

Also, I would still use the "#define macros", for example
"DISP_REG_OVL_ADDR offsets, and use the named constant in the
driver_data:

#define DISP_REG_OVL_ADDR_MT8173  0x0f40

(and in a later patch:
#define DISP_REG_OVL_ADDR_MT2701  0x0040
)

Also, I would still use the macro rather than open coding the "0x20 *
(n)", and just pass 'ovl' to the overlay macros that depend on
hardware type.
Something like the following:

#define DISP_REG_OVL_ADDR(ovl, n)  ((ovl)->data->ovl_addr + 0x20 * (n))

>
>  #define        OVL_RDMA_MEM_GMC        0x40402020
>
>  #define OVL_CON_BYTE_SWAP      BIT(24)
> -#define OVL_CON_CLRFMT_RGB565  (0 << 12)
> -#define OVL_CON_CLRFMT_RGB888  (1 << 12)

This seems like a really random and unnecessary hardware change.
Why chip designers, why!!?!?

For this one, it seems the polarity is either one way or the other, so
we can just use a bool to distinguish:

  bool fmt_rgb565_is_0;

> +static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
> +       .ovl = { DISP_REG_OVL_ADDR_MT8173, .fmt_rgb565_is_0 = true }
> +};

For use at runtime, the defines could become:

#define OVL_CON_CLRFMT_RGB565(ovl)  ((ovl)->data->fmt_rgb565_is_0 ? 0
: OVL_CON_CLRFMT_RGB888)
#define OVL_CON_CLRFMT_RGB888(ovl)  ((ovl)->data->fmt_rgb565_is_0 ?
OVL_CON_CLRFMT_RGB888 : 0)

>  #define OVL_CON_CLRFMT_RGBA8888        (2 << 12)
>  #define OVL_CON_CLRFMT_ARGB8888        (3 << 12)
>  #define        OVL_CON_AEN             BIT(8)
> @@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx)
>         writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
>  }
>
> -static unsigned int ovl_fmt_convert(unsigned int fmt)
> +static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt)
>  {
>         switch (fmt) {
>         default:
>         case DRM_FORMAT_RGB565:
> -               return OVL_CON_CLRFMT_RGB565;
> +               return comp->data->ovl.fmt_rgb565;

It will be nice to define a helper function for converting from the
generic 'mtk_ddp_comp' to the specific 'mtk_disp_ovl':

static inline struct mtk_disp_ovl *comp_to_ovl(struct mtk_ddp_comp *comp) {
  return container_of(comp, struct mtk_disp_ovl, ddp_comp);
}

Then these could become:
   return OVL_CON_CLRFMT_RGB565(comp_to_ovl(comp));

Or maybe cleaner, do the conversion once at the top of the function:
    struct mtk_disp_ovl *ovl = comp_to_ovl(comp);

And then just:
   return OVL_CON_CLRFMT_RGB565(ovl);


>         case DRM_FORMAT_BGR565:
> -               return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP;
> +               return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP;
>         case DRM_FORMAT_RGB888:
> -               return OVL_CON_CLRFMT_RGB888;
> +               return comp->data->ovl.fmt_rgb888;
>         case DRM_FORMAT_BGR888:
> -               return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP;
> +               return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP;
>         case DRM_FORMAT_RGBX8888:
>         case DRM_FORMAT_RGBA8888:
>                 return OVL_CON_CLRFMT_ARGB8888;

[snip]


> diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> index 1c366f8..935a8ef 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> @@ -16,6 +16,7 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/phy/phy.h>
>
> @@ -87,6 +88,9 @@
>
>  #define MIPITX_DSI_PLL_CON2    0x58
>
> +#define MIPITX_DSI_PLL_TOP     0x64
> +#define RG_DSI_MPPLL_PRESERVE          (0xff << 8)
> +
>  #define MIPITX_DSI_PLL_PWR     0x68
>  #define RG_DSI_MPPLL_SDM_PWR_ON                BIT(0)
>  #define RG_DSI_MPPLL_SDM_ISO_EN                BIT(1)
> @@ -123,10 +127,15 @@
>  #define SW_LNT2_HSTX_PRE_OE            BIT(24)
>  #define SW_LNT2_HSTX_OE                        BIT(25)
>
> +struct mtk_mipitx_data {
> +       const u32 data;

Use a better name, like "mppll_preserve".

Ok, that's it for now.
Actually, the patch set in general looks pretty good.

-Dan

Re: [PATCH v9 02/10] drm/mediatek: add *driver_data for different hardware settings

[YT Shen]

Hi Daniel,

On Fri, 2016-11-18 at 12:56 +0800, Daniel Kurtz wrote:
> Hi YT,
> 
> I don't see a reason to handle device_data in such a generic way at
> the generic mtk_ddp_comp layer.
> The device data is very component specific, so just define different
> structs for different comp types, ie:
> 
> struct mtk_disp_ovl_driver_data {
>     unsigned int reg_ovl_addr;
>     unsigned int fmt_rgb565;
>     unsigned int fmt_rgb888;
> };
> 
> struct mtk_disp_rdma_driver_data {
>     unsigned int fifo_pseudo_size;
> };
> 
> struct mtk_disp_color_driver_data {
>     unsigned int color_offset;
> };
> 
> Then add typed pointers to the local structs that use them, for example:
> 
> struct mtk_disp_ovl {
>         struct mtk_ddp_comp             ddp_comp;
>         struct drm_crtc                 *crtc;
>         const struct mtk_disp_ovl_driver_data *data;
> };
> 
> And fetch the device specific driver data directly in .probe, as you
> are already doing:
> 
> static int mtk_disp_ovl_probe(struct platform_device *pdev) {
>   ...
>   priv->data = of_device_get_match_data(dev);
>   ...
> }
These suggestions make code more readable.  We will change ovl and rdma
part, and keep mtk_disp_color_driver_data in its original place.
Because ovl and rdma have its files, other modules share
mtk_drm_ddp_comp.c.

> 
> More comments in-line...
> 
> On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
> > There are some hardware settings changed, between MT8173 & MT2701:
> > DISP_OVL address offset changed, color format definition changed.
> > DISP_RDMA fifo size changed.
> > DISP_COLOR offset changed.
> > MIPI_TX pll setting changed.
> > And add prefix for mtk_ddp_main & mtk_ddp_ext & mutex_mod.
> 
> Nit: I think it would make sense to combine this patch with
> drm/mediatek: rename macros, add chip prefix
Will do.

> 
> >
> > Signed-off-by: YT Shen <yt.shen@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c     | 27 ++++++++++++++++-----------
> >  drivers/gpu/drm/mediatek/mtk_disp_rdma.c    | 11 +++++++++--
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 11 +++++++----
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 27 +++++++++++++++++++++------
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 13 +++++++++++++
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c      | 25 ++++++++++++++++++-------
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.h      |  8 ++++++++
> >  drivers/gpu/drm/mediatek/mtk_mipi_tx.c      | 24 +++++++++++++++++++++++-
> >  8 files changed, 115 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 019b7ca..1139834 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -35,13 +35,10 @@
> >  #define DISP_REG_OVL_PITCH(n)                  (0x0044 + 0x20 * (n))
> >  #define DISP_REG_OVL_RDMA_CTRL(n)              (0x00c0 + 0x20 * (n))
> >  #define DISP_REG_OVL_RDMA_GMC(n)               (0x00c8 + 0x20 * (n))
> > -#define DISP_REG_OVL_ADDR(n)                   (0x0f40 + 0x20 * (n))
> 
> Also, I would still use the "#define macros", for example
> "DISP_REG_OVL_ADDR offsets, and use the named constant in the
> driver_data:
> 
> #define DISP_REG_OVL_ADDR_MT8173  0x0f40
> 
> (and in a later patch:
> #define DISP_REG_OVL_ADDR_MT2701  0x0040
> )
> 
> Also, I would still use the macro rather than open coding the "0x20 *
> (n)", and just pass 'ovl' to the overlay macros that depend on
> hardware type.
> Something like the following:
> 
> #define DISP_REG_OVL_ADDR(ovl, n)  ((ovl)->data->ovl_addr + 0x20 * (n))
Will use the "#define macros" here.

> 
> >
> >  #define        OVL_RDMA_MEM_GMC        0x40402020
> >
> >  #define OVL_CON_BYTE_SWAP      BIT(24)
> > -#define OVL_CON_CLRFMT_RGB565  (0 << 12)
> > -#define OVL_CON_CLRFMT_RGB888  (1 << 12)
> 
> This seems like a really random and unnecessary hardware change.
> Why chip designers, why!!?!?
There are many reasons for software bugs.  Unnecessary hardware change
should be one of them...

> 
> For this one, it seems the polarity is either one way or the other, so
> we can just use a bool to distinguish:
> 
>   bool fmt_rgb565_is_0;
> 
> > +static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
> > +       .ovl = { DISP_REG_OVL_ADDR_MT8173, .fmt_rgb565_is_0 = true }
> > +};
> 
> For use at runtime, the defines could become:
> 
> #define OVL_CON_CLRFMT_RGB565(ovl)  ((ovl)->data->fmt_rgb565_is_0 ? 0
> : OVL_CON_CLRFMT_RGB888)
> #define OVL_CON_CLRFMT_RGB888(ovl)  ((ovl)->data->fmt_rgb565_is_0 ?
> OVL_CON_CLRFMT_RGB888 : 0)
OK, will do.

> 
> >  #define OVL_CON_CLRFMT_RGBA8888        (2 << 12)
> >  #define OVL_CON_CLRFMT_ARGB8888        (3 << 12)
> >  #define        OVL_CON_AEN             BIT(8)
> > @@ -137,18 +134,18 @@ static void mtk_ovl_layer_off(struct mtk_ddp_comp *comp, unsigned int idx)
> >         writel(0x0, comp->regs + DISP_REG_OVL_RDMA_CTRL(idx));
> >  }
> >
> > -static unsigned int ovl_fmt_convert(unsigned int fmt)
> > +static unsigned int ovl_fmt_convert(struct mtk_ddp_comp *comp, unsigned int fmt)
> >  {
> >         switch (fmt) {
> >         default:
> >         case DRM_FORMAT_RGB565:
> > -               return OVL_CON_CLRFMT_RGB565;
> > +               return comp->data->ovl.fmt_rgb565;
> 
> It will be nice to define a helper function for converting from the
> generic 'mtk_ddp_comp' to the specific 'mtk_disp_ovl':
> 
> static inline struct mtk_disp_ovl *comp_to_ovl(struct mtk_ddp_comp *comp) {
>   return container_of(comp, struct mtk_disp_ovl, ddp_comp);
> }
> 
> Then these could become:
>    return OVL_CON_CLRFMT_RGB565(comp_to_ovl(comp));
> 
> Or maybe cleaner, do the conversion once at the top of the function:
>     struct mtk_disp_ovl *ovl = comp_to_ovl(comp);
> 
> And then just:
>    return OVL_CON_CLRFMT_RGB565(ovl);
> 
> 
Will use a helper function for the converting.

> >         case DRM_FORMAT_BGR565:
> > -               return OVL_CON_CLRFMT_RGB565 | OVL_CON_BYTE_SWAP;
> > +               return comp->data->ovl.fmt_rgb565 | OVL_CON_BYTE_SWAP;
> >         case DRM_FORMAT_RGB888:
> > -               return OVL_CON_CLRFMT_RGB888;
> > +               return comp->data->ovl.fmt_rgb888;
> >         case DRM_FORMAT_BGR888:
> > -               return OVL_CON_CLRFMT_RGB888 | OVL_CON_BYTE_SWAP;
> > +               return comp->data->ovl.fmt_rgb888 | OVL_CON_BYTE_SWAP;
> >         case DRM_FORMAT_RGBX8888:
> >         case DRM_FORMAT_RGBA8888:
> >                 return OVL_CON_CLRFMT_ARGB8888;
> 
> [snip]
> 
> 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > index 1c366f8..935a8ef 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/delay.h>
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> > +#include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/phy/phy.h>
> >
> > @@ -87,6 +88,9 @@
> >
> >  #define MIPITX_DSI_PLL_CON2    0x58
> >
> > +#define MIPITX_DSI_PLL_TOP     0x64
> > +#define RG_DSI_MPPLL_PRESERVE          (0xff << 8)
> > +
> >  #define MIPITX_DSI_PLL_PWR     0x68
> >  #define RG_DSI_MPPLL_SDM_PWR_ON                BIT(0)
> >  #define RG_DSI_MPPLL_SDM_ISO_EN                BIT(1)
> > @@ -123,10 +127,15 @@
> >  #define SW_LNT2_HSTX_PRE_OE            BIT(24)
> >  #define SW_LNT2_HSTX_OE                        BIT(25)
> >
> > +struct mtk_mipitx_data {
> > +       const u32 data;
> 
> Use a better name, like "mppll_preserve".
OK.

Regards,
yt.shen

> 
> Ok, that's it for now.
> Actually, the patch set in general looks pretty good.
> 
> -Dan

[PATCH v9 03/10] drm/mediatek: add shadow register support

[YT Shen]

We need to acquire mutex before using the resources,
and need to release it after finished.
So we don't need to write registers in the blanking period.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 76 ++++++++++++++++++++-------------
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 25 +++++++++++
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h  |  2 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  1 +
 4 files changed, 75 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 01a21dd..a4f2b3a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -329,6 +329,42 @@ static void mtk_crtc_ddp_hw_fini(struct mtk_drm_crtc *mtk_crtc)
 	pm_runtime_put(drm->dev);
 }
 
+static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
+{
+	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
+	struct mtk_ddp_comp *ovl = mtk_crtc->ddp_comp[0];
+	unsigned int i;
+
+	/*
+	 * TODO: instead of updating the registers here, we should prepare
+	 * working registers in atomic_commit and let the hardware command
+	 * queue update module registers on vblank.
+	 */
+	if (state->pending_config) {
+		mtk_ddp_comp_config(ovl, state->pending_width,
+				    state->pending_height,
+				    state->pending_vrefresh, 0);
+
+		state->pending_config = false;
+	}
+
+	if (mtk_crtc->pending_planes) {
+		for (i = 0; i < OVL_LAYER_NR; i++) {
+			struct drm_plane *plane = &mtk_crtc->planes[i];
+			struct mtk_plane_state *plane_state;
+
+			plane_state = to_mtk_plane_state(plane->state);
+
+			if (plane_state->pending.config) {
+				mtk_ddp_comp_layer_config(ovl, i, plane_state);
+				plane_state->pending.config = false;
+			}
+		}
+		mtk_crtc->pending_planes = false;
+	}
+}
+
 static void mtk_drm_crtc_enable(struct drm_crtc *crtc)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
@@ -405,6 +441,7 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 				      struct drm_crtc_state *old_crtc_state)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	struct mtk_drm_private *priv = crtc->dev->dev_private;
 	unsigned int pending_planes = 0;
 	int i;
 
@@ -423,6 +460,13 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc *crtc,
 	}
 	if (pending_planes)
 		mtk_crtc->pending_planes = true;
+
+	if (priv->data->shadow_register) {
+		mtk_disp_mutex_acquire(mtk_crtc->mutex);
+		mtk_crtc_ddp_config(crtc);
+		mtk_disp_mutex_release(mtk_crtc->mutex);
+	}
+
 	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);
@@ -471,36 +515,10 @@ static int mtk_drm_crtc_init(struct drm_device *drm,
 void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct mtk_ddp_comp *ovl)
 {
 	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
-	struct mtk_crtc_state *state = to_mtk_crtc_state(mtk_crtc->base.state);
-	unsigned int i;
+	struct mtk_drm_private *priv = crtc->dev->dev_private;
 
-	/*
-	 * TODO: instead of updating the registers here, we should prepare
-	 * working registers in atomic_commit and let the hardware command
-	 * queue update module registers on vblank.
-	 */
-	if (state->pending_config) {
-		mtk_ddp_comp_config(ovl, state->pending_width,
-				    state->pending_height,
-				    state->pending_vrefresh, 0);
-
-		state->pending_config = false;
-	}
-
-	if (mtk_crtc->pending_planes) {
-		for (i = 0; i < OVL_LAYER_NR; i++) {
-			struct drm_plane *plane = &mtk_crtc->planes[i];
-			struct mtk_plane_state *plane_state;
-
-			plane_state = to_mtk_plane_state(plane->state);
-
-			if (plane_state->pending.config) {
-				mtk_ddp_comp_layer_config(ovl, i, plane_state);
-				plane_state->pending.config = false;
-			}
-		}
-		mtk_crtc->pending_planes = false;
-	}
+	if (!priv->data->shadow_register)
+		mtk_crtc_ddp_config(crtc);
 
 	mtk_drm_finish_page_flip(mtk_crtc);
 }
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 8030769..b77d456 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -12,6 +12,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
@@ -32,10 +33,13 @@
 #define DISP_REG_CONFIG_MMSYS_CG_CON0		0x100
 
 #define DISP_REG_MUTEX_EN(n)	(0x20 + 0x20 * (n))
+#define DISP_REG_MUTEX(n)	(0x24 + 0x20 * (n))
 #define DISP_REG_MUTEX_RST(n)	(0x28 + 0x20 * (n))
 #define DISP_REG_MUTEX_MOD(n)	(0x2c + 0x20 * (n))
 #define DISP_REG_MUTEX_SOF(n)	(0x30 + 0x20 * (n))
 
+#define INT_MUTEX				BIT(1)
+
 #define MT8173_MUTEX_MOD_DISP_OVL0		BIT(11)
 #define MT8173_MUTEX_MOD_DISP_OVL1		BIT(12)
 #define MT8173_MUTEX_MOD_DISP_RDMA0		BIT(13)
@@ -300,6 +304,27 @@ void mtk_disp_mutex_disable(struct mtk_disp_mutex *mutex)
 	writel(0, ddp->regs + DISP_REG_MUTEX_EN(mutex->id));
 }
 
+void mtk_disp_mutex_acquire(struct mtk_disp_mutex *mutex)
+{
+	struct mtk_ddp *ddp = container_of(mutex, struct mtk_ddp,
+					   mutex[mutex->id]);
+	u32 tmp;
+
+	writel(1, ddp->regs + DISP_REG_MUTEX_EN(mutex->id));
+	writel(1, ddp->regs + DISP_REG_MUTEX(mutex->id));
+	if (readl_poll_timeout_atomic(ddp->regs + DISP_REG_MUTEX(mutex->id),
+				      tmp, tmp & INT_MUTEX, 1, 10000))
+		pr_err("could not acquire mutex %d\n", mutex->id);
+}
+
+void mtk_disp_mutex_release(struct mtk_disp_mutex *mutex)
+{
+	struct mtk_ddp *ddp = container_of(mutex, struct mtk_ddp,
+					   mutex[mutex->id]);
+
+	writel(0, ddp->regs + DISP_REG_MUTEX(mutex->id));
+}
+
 static int mtk_ddp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
index 92c1175..f9a7991 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
@@ -37,5 +37,7 @@ void mtk_disp_mutex_remove_comp(struct mtk_disp_mutex *mutex,
 				enum mtk_ddp_comp_id id);
 void mtk_disp_mutex_unprepare(struct mtk_disp_mutex *mutex);
 void mtk_disp_mutex_put(struct mtk_disp_mutex *mutex);
+void mtk_disp_mutex_acquire(struct mtk_disp_mutex *mutex);
+void mtk_disp_mutex_release(struct mtk_disp_mutex *mutex);
 
 #endif /* MTK_DRM_DDP_H */
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index fa0b106..94f8b66 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -33,6 +33,7 @@ struct mtk_mmsys_driver_data {
 	unsigned int main_len;
 	const enum mtk_ddp_comp_id *ext_path;
 	unsigned int ext_len;
+	bool shadow_register;
 };
 
 struct mtk_drm_private {
-- 
1.9.1

[PATCH v9 04/10] drm/mediatek: add BLS component

[YT Shen]

Add BLS component for PWM + GAMMA function

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 5 ++++-
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 661a4a0..b78b2e6 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -235,6 +235,7 @@ static void mtk_gamma_set(struct mtk_ddp_comp *comp,
 	[MTK_DISP_PWM] = "pwm",
 	[MTK_DISP_MUTEX] = "mutex",
 	[MTK_DISP_OD] = "od",
+	[MTK_DISP_BLS] = "bls",
 };
 
 struct mtk_ddp_comp_match {
@@ -245,6 +246,7 @@ struct mtk_ddp_comp_match {
 
 static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = {
 	[DDP_COMPONENT_AAL]	= { MTK_DISP_AAL,	0, &ddp_aal },
+	[DDP_COMPONENT_BLS]	= { MTK_DISP_BLS,	0, NULL },
 	[DDP_COMPONENT_COLOR0]	= { MTK_DISP_COLOR,	0, &ddp_color },
 	[DDP_COMPONENT_COLOR1]	= { MTK_DISP_COLOR,	1, &ddp_color },
 	[DDP_COMPONENT_DPI0]	= { MTK_DPI,		0, NULL },
@@ -303,7 +305,8 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node,
 	comp->id = comp_id;
 	comp->funcs = funcs ?: mtk_ddp_matches[comp_id].funcs;
 
-	if (comp_id == DDP_COMPONENT_DPI0 ||
+	if (comp_id == DDP_COMPONENT_BLS ||
+	    comp_id == DDP_COMPONENT_DPI0 ||
 	    comp_id == DDP_COMPONENT_DSI0 ||
 	    comp_id == DDP_COMPONENT_PWM0) {
 		comp->regs = NULL;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index 2f6872a..30faf46 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -36,11 +36,13 @@ enum mtk_ddp_comp_type {
 	MTK_DISP_PWM,
 	MTK_DISP_MUTEX,
 	MTK_DISP_OD,
+	MTK_DISP_BLS,
 	MTK_DDP_COMP_TYPE_MAX,
 };
 
 enum mtk_ddp_comp_id {
 	DDP_COMPONENT_AAL,
+	DDP_COMPONENT_BLS,
 	DDP_COMPONENT_COLOR0,
 	DDP_COMPONENT_COLOR1,
 	DDP_COMPONENT_DPI0,
-- 
1.9.1

[PATCH v9 05/10] drm/mediatek: update display module connections

[YT Shen]

update connections for OVL, RDMA, BLS, DSI

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index b77d456..a9b209c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -32,6 +32,10 @@
 #define DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN	0x0c8
 #define DISP_REG_CONFIG_MMSYS_CG_CON0		0x100
 
+#define DISP_REG_CONFIG_DISP_OVL_MOUT_EN	0x030
+#define DISP_REG_CONFIG_OUT_SEL			0x04c
+#define DISP_REG_CONFIG_DSI_SEL			0x050
+
 #define DISP_REG_MUTEX_EN(n)	(0x20 + 0x20 * (n))
 #define DISP_REG_MUTEX(n)	(0x24 + 0x20 * (n))
 #define DISP_REG_MUTEX_RST(n)	(0x28 + 0x20 * (n))
@@ -71,6 +75,10 @@
 #define DPI0_SEL_IN_RDMA1		0x1
 #define COLOR1_SEL_IN_OVL1		0x1
 
+#define OVL_MOUT_EN_RDMA		0x1
+#define BLS_TO_DSI_RDMA1_TO_DPI1	0x8
+#define DSI_SEL_IN_BLS			0x0
+
 struct mtk_disp_mutex {
 	int id;
 	bool claimed;
@@ -111,6 +119,9 @@ static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id cur,
 	if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
 		*addr = DISP_REG_CONFIG_DISP_OVL0_MOUT_EN;
 		value = OVL0_MOUT_EN_COLOR0;
+	} else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) {
+		*addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN;
+		value = OVL_MOUT_EN_RDMA;
 	} else if (cur == DDP_COMPONENT_OD && next == DDP_COMPONENT_RDMA0) {
 		*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
 		value = OD_MOUT_EN_RDMA0;
@@ -148,6 +159,9 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
 	} else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
 		*addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN;
 		value = COLOR1_SEL_IN_OVL1;
+	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
+		*addr = DISP_REG_CONFIG_DSI_SEL;
+		value = DSI_SEL_IN_BLS;
 	} else {
 		value = 0;
 	}
@@ -155,6 +169,15 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
 	return value;
 }
 
+static void mtk_ddp_sout_sel(void __iomem *config_regs,
+			     enum mtk_ddp_comp_id cur,
+			     enum mtk_ddp_comp_id next)
+{
+	if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0)
+		writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
+			       config_regs + DISP_REG_CONFIG_OUT_SEL);
+}
+
 void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
 			      enum mtk_ddp_comp_id cur,
 			      enum mtk_ddp_comp_id next)
@@ -167,6 +190,8 @@ void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
 		writel_relaxed(reg, config_regs + addr);
 	}
 
+	mtk_ddp_sout_sel(config_regs, cur, next);
+
 	value = mtk_ddp_sel_in(cur, next, &addr);
 	if (value) {
 		reg = readl_relaxed(config_regs + addr) | value;
-- 
1.9.1

[PATCH v9 06/10] drm/mediatek: cleaning up and refine

[YT Shen]

cleaning up unused define and refine function name and variable

Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c     | 77 ++++++++++++++++------------------
 drivers/gpu/drm/mediatek/mtk_mipi_tx.c |  8 ++--
 2 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 28b2044..4efeb38 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -27,9 +27,6 @@
 
 #include "mtk_drm_ddp_comp.h"
 
-#define DSI_VIDEO_FIFO_DEPTH	(1920 / 4)
-#define DSI_HOST_FIFO_DEPTH	64
-
 #define DSI_START		0x00
 
 #define DSI_CON_CTRL		0x10
@@ -46,7 +43,7 @@
 #define MIX_MODE			BIT(17)
 
 #define DSI_TXRX_CTRL		0x18
-#define VC_NUM				(2 << 0)
+#define VC_NUM				BIT(1)
 #define LANE_NUM			(0xf << 2)
 #define DIS_EOT				BIT(6)
 #define NULL_EN				BIT(7)
@@ -158,11 +155,11 @@ static void mtk_dsi_mask(struct mtk_dsi *dsi, u32 offset, u32 mask, u32 data)
 	writel((temp & ~mask) | (data & mask), dsi->regs + offset);
 }
 
-static void dsi_phy_timconfig(struct mtk_dsi *dsi)
+static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
 {
 	u32 timcon0, timcon1, timcon2, timcon3;
-	unsigned int ui, cycle_time;
-	unsigned int lpx;
+	u32 ui, cycle_time;
+	u32 lpx;
 
 	ui = 1000 / dsi->data_rate + 0x01;
 	cycle_time = 8000 / dsi->data_rate + 0x01;
@@ -192,7 +189,7 @@ static void mtk_dsi_disable(struct mtk_dsi *dsi)
 	mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
 }
 
-static void mtk_dsi_reset(struct mtk_dsi *dsi)
+static void mtk_dsi_reset_engine(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_RESET, DSI_RESET);
 	mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_RESET, 0);
@@ -235,8 +232,8 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
 	}
 
 	mtk_dsi_enable(dsi);
-	mtk_dsi_reset(dsi);
-	dsi_phy_timconfig(dsi);
+	mtk_dsi_reset_engine(dsi);
+	mtk_dsi_phy_timconfig(dsi);
 
 	return 0;
 
@@ -249,33 +246,33 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
 	return ret;
 }
 
-static void dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
+static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
 	mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
 }
 
-static void dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
+static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
 	mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_WAKEUP_EN, LC_WAKEUP_EN);
 	mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_WAKEUP_EN, 0);
 }
 
-static void dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
+static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
 	mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
 }
 
-static void dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
+static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
 	mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_WAKEUP_EN, LD0_WAKEUP_EN);
 	mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_WAKEUP_EN, 0);
 }
 
-static bool dsi_clk_hs_state(struct mtk_dsi *dsi)
+static bool mtk_dsi_clk_hs_state(struct mtk_dsi *dsi)
 {
 	u32 tmp_reg1;
 
@@ -283,15 +280,15 @@ static bool dsi_clk_hs_state(struct mtk_dsi *dsi)
 	return ((tmp_reg1 & LC_HS_TX_EN) == 1) ? true : false;
 }
 
-static void dsi_clk_hs_mode(struct mtk_dsi *dsi, bool enter)
+static void mtk_dsi_clk_hs_mode(struct mtk_dsi *dsi, bool enter)
 {
-	if (enter && !dsi_clk_hs_state(dsi))
+	if (enter && !mtk_dsi_clk_hs_state(dsi))
 		mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, LC_HS_TX_EN);
-	else if (!enter && dsi_clk_hs_state(dsi))
+	else if (!enter && mtk_dsi_clk_hs_state(dsi))
 		mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
 }
 
-static void dsi_set_mode(struct mtk_dsi *dsi)
+static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
 {
 	u32 vid_mode = CMD_MODE;
 
@@ -306,7 +303,7 @@ static void dsi_set_mode(struct mtk_dsi *dsi)
 	writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
 }
 
-static void dsi_ps_control_vact(struct mtk_dsi *dsi)
+static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
 {
 	struct videomode *vm = &dsi->vm;
 	u32 dsi_buf_bpp, ps_wc;
@@ -340,7 +337,7 @@ static void dsi_ps_control_vact(struct mtk_dsi *dsi)
 	writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
 }
 
-static void dsi_rxtx_control(struct mtk_dsi *dsi)
+static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
 {
 	u32 tmp_reg;
 
@@ -365,9 +362,9 @@ static void dsi_rxtx_control(struct mtk_dsi *dsi)
 	writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
 }
 
-static void dsi_ps_control(struct mtk_dsi *dsi)
+static void mtk_dsi_ps_control(struct mtk_dsi *dsi)
 {
-	unsigned int dsi_tmp_buf_bpp;
+	u32 dsi_tmp_buf_bpp;
 	u32 tmp_reg;
 
 	switch (dsi->format) {
@@ -397,12 +394,12 @@ static void dsi_ps_control(struct mtk_dsi *dsi)
 	writel(tmp_reg, dsi->regs + DSI_PSCTRL);
 }
 
-static void dsi_config_vdo_timing(struct mtk_dsi *dsi)
+static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
 {
-	unsigned int horizontal_sync_active_byte;
-	unsigned int horizontal_backporch_byte;
-	unsigned int horizontal_frontporch_byte;
-	unsigned int dsi_tmp_buf_bpp;
+	u32 horizontal_sync_active_byte;
+	u32 horizontal_backporch_byte;
+	u32 horizontal_frontporch_byte;
+	u32 dsi_tmp_buf_bpp;
 
 	struct videomode *vm = &dsi->vm;
 
@@ -431,7 +428,7 @@ static void dsi_config_vdo_timing(struct mtk_dsi *dsi)
 	writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
 	writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
 
-	dsi_ps_control(dsi);
+	mtk_dsi_ps_control(dsi);
 }
 
 static void mtk_dsi_start(struct mtk_dsi *dsi)
@@ -448,8 +445,8 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
 	if (--dsi->refcount != 0)
 		return;
 
-	dsi_lane0_ulp_mode_enter(dsi);
-	dsi_clk_ulp_mode_enter(dsi);
+	mtk_dsi_lane0_ulp_mode_enter(dsi);
+	mtk_dsi_clk_ulp_mode_enter(dsi);
 
 	mtk_dsi_disable(dsi);
 
@@ -479,18 +476,18 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
 		return;
 	}
 
-	dsi_rxtx_control(dsi);
+	mtk_dsi_rxtx_control(dsi);
 
-	dsi_clk_ulp_mode_leave(dsi);
-	dsi_lane0_ulp_mode_leave(dsi);
-	dsi_clk_hs_mode(dsi, 0);
-	dsi_set_mode(dsi);
+	mtk_dsi_clk_ulp_mode_leave(dsi);
+	mtk_dsi_lane0_ulp_mode_leave(dsi);
+	mtk_dsi_clk_hs_mode(dsi, 0);
+	mtk_dsi_set_mode(dsi);
 
-	dsi_ps_control_vact(dsi);
-	dsi_config_vdo_timing(dsi);
+	mtk_dsi_ps_control_vact(dsi);
+	mtk_dsi_config_vdo_timing(dsi);
 
-	dsi_set_mode(dsi);
-	dsi_clk_hs_mode(dsi, 1);
+	mtk_dsi_set_mode(dsi);
+	mtk_dsi_clk_hs_mode(dsi, 1);
 
 	mtk_dsi_start(dsi);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 935a8ef..108d31a 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -134,7 +134,7 @@ struct mtk_mipitx_data {
 struct mtk_mipi_tx {
 	struct device *dev;
 	void __iomem *regs;
-	unsigned int data_rate;
+	u32 data_rate;
 	const struct mtk_mipitx_data *driver_data;
 	struct clk_hw pll_hw;
 	struct clk *pll;
@@ -172,7 +172,7 @@ static void mtk_mipi_tx_update_bits(struct mtk_mipi_tx *mipi_tx, u32 offset,
 static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
 {
 	struct mtk_mipi_tx *mipi_tx = mtk_mipi_tx_from_clk_hw(hw);
-	unsigned int txdiv, txdiv0, txdiv1;
+	u8 txdiv, txdiv0, txdiv1;
 	u64 pcw;
 
 	dev_dbg(mipi_tx->dev, "prepare: %u Hz\n", mipi_tx->data_rate);
@@ -326,7 +326,7 @@ static unsigned long mtk_mipi_tx_pll_recalc_rate(struct clk_hw *hw,
 static int mtk_mipi_tx_power_on_signal(struct phy *phy)
 {
 	struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy);
-	unsigned int reg;
+	u32 reg;
 
 	for (reg = MIPITX_DSI_CLOCK_LANE;
 	     reg <= MIPITX_DSI_DATA_LANE3; reg += 4)
@@ -357,7 +357,7 @@ static int mtk_mipi_tx_power_on(struct phy *phy)
 static void mtk_mipi_tx_power_off_signal(struct phy *phy)
 {
 	struct mtk_mipi_tx *mipi_tx = phy_get_drvdata(phy);
-	unsigned int reg;
+	u32 reg;
 
 	mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_TOP_CON,
 			     RG_DSI_PAD_TIE_LOW_EN);
-- 
1.9.1

[PATCH v9 07/10] drm/mediatek: add dsi interrupt control

[YT Shen]

From: shaoming chen <shaoming.chen@mediatek.com>

add dsi interrupt control

Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 93 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 4efeb38..e5832837 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -18,6 +18,7 @@
 #include <drm/drm_panel.h>
 #include <linux/clk.h>
 #include <linux/component.h>
+#include <linux/irq.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/of_graph.h>
@@ -29,6 +30,16 @@
 
 #define DSI_START		0x00
 
+#define DSI_INTEN		0x08
+
+#define DSI_INTSTA		0x0c
+#define LPRX_RD_RDY_INT_FLAG		BIT(0)
+#define CMD_DONE_INT_FLAG		BIT(1)
+#define TE_RDY_INT_FLAG			BIT(2)
+#define VM_DONE_INT_FLAG		BIT(3)
+#define EXT_TE_RDY_INT_FLAG		BIT(4)
+#define DSI_BUSY			BIT(31)
+
 #define DSI_CON_CTRL		0x10
 #define DSI_RESET			BIT(0)
 #define DSI_EN				BIT(1)
@@ -71,6 +82,9 @@
 
 #define DSI_HSTX_CKL_WC		0x64
 
+#define DSI_RACK		0x84
+#define RACK				BIT(0)
+
 #define DSI_PHY_LCCON		0x104
 #define LC_HS_TX_EN			BIT(0)
 #define LC_ULPM_EN			BIT(1)
@@ -131,6 +145,8 @@ struct mtk_dsi {
 	struct videomode vm;
 	int refcount;
 	bool enabled;
+	u32 irq_data;
+	wait_queue_head_t irq_wait_queue;
 };
 
 static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
@@ -437,6 +453,64 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
 	writel(1, dsi->regs + DSI_START);
 }
 
+static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
+{
+	u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
+
+	writel(inten, dsi->regs + DSI_INTEN);
+}
+
+static void mtk_dsi_irq_data_set(struct mtk_dsi *dsi, u32 irq_bit)
+{
+	dsi->irq_data |= irq_bit;
+}
+
+static __maybe_unused void mtk_dsi_irq_data_clear(struct mtk_dsi *dsi, u32 irq_bit)
+{
+	dsi->irq_data &= ~irq_bit;
+}
+
+static __maybe_unused s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
+				     unsigned int timeout)
+{
+	s32 ret = 0;
+	unsigned long jiffies = msecs_to_jiffies(timeout);
+
+	ret = wait_event_interruptible_timeout(dsi->irq_wait_queue,
+					       dsi->irq_data & irq_flag,
+					       jiffies);
+	if (ret == 0) {
+		dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
+
+		mtk_dsi_enable(dsi);
+		mtk_dsi_reset_engine(dsi);
+	}
+
+	return ret;
+}
+
+static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
+{
+	struct mtk_dsi *dsi = dev_id;
+	u32 status, tmp;
+	u32 flag = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
+
+	status = readl(dsi->regs + DSI_INTSTA) & flag;
+
+	if (status) {
+		do {
+			mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
+			tmp = readl(dsi->regs + DSI_INTSTA);
+		} while (tmp & DSI_BUSY);
+
+		mtk_dsi_mask(dsi, DSI_INTSTA, status, 0);
+		mtk_dsi_irq_data_set(dsi, status);
+		wake_up_interruptible(&dsi->irq_wait_queue);
+	}
+
+	return IRQ_HANDLED;
+}
+
 static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
 {
 	if (WARN_ON(dsi->refcount == 0))
@@ -485,6 +559,7 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
 
 	mtk_dsi_ps_control_vact(dsi);
 	mtk_dsi_config_vdo_timing(dsi);
+	mtk_dsi_set_interrupt_enable(dsi);
 
 	mtk_dsi_set_mode(dsi);
 	mtk_dsi_clk_hs_mode(dsi, 1);
@@ -793,6 +868,7 @@ static int mtk_dsi_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *remote_node, *endpoint;
 	struct resource *regs;
+	int irq_num;
 	int comp_id;
 	int ret;
 
@@ -869,6 +945,23 @@ static int mtk_dsi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	irq_num = platform_get_irq(pdev, 0);
+	if (irq_num < 0) {
+		dev_err(&pdev->dev, "failed to request dsi irq resource\n");
+		return -EPROBE_DEFER;
+	}
+
+	irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
+	ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq,
+			       IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request mediatek dsi irq\n");
+		return -EPROBE_DEFER;
+	}
+	dev_info(dev, "dsi irq num is 0x%x\n", irq_num);
+
+	init_waitqueue_head(&dsi->irq_wait_queue);
+
 	platform_set_drvdata(pdev, dsi);
 
 	return component_add(&pdev->dev, &mtk_dsi_component_ops);
-- 
1.9.1

[PATCH v9 08/10] drm/mediatek: add dsi transfer function

[YT Shen]

From: shaoming chen <shaoming.chen@mediatek.com>

add dsi read/write commands for transfer function

Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 168 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 166 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index e5832837..860b84f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -24,6 +24,7 @@
 #include <linux/of_graph.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <video/mipi_display.h>
 #include <video/videomode.h>
 
 #include "mtk_drm_ddp_comp.h"
@@ -80,8 +81,16 @@
 #define DSI_HBP_WC		0x54
 #define DSI_HFP_WC		0x58
 
+#define DSI_CMDQ_SIZE		0x60
+#define CMDQ_SIZE			0x3f
+
 #define DSI_HSTX_CKL_WC		0x64
 
+#define DSI_RX_DATA0		0x74
+#define DSI_RX_DATA1		0x78
+#define DSI_RX_DATA2		0x7c
+#define DSI_RX_DATA3		0x80
+
 #define DSI_RACK		0x84
 #define RACK				BIT(0)
 
@@ -117,8 +126,23 @@
 #define CLK_HS_POST			(0xff << 8)
 #define CLK_HS_EXIT			(0xff << 16)
 
+#define DSI_CMDQ0		0x180
+#define CONFIG				(0xff << 0)
+#define SHORT_PACKET			0
+#define LONG_PACKET			2
+#define BTA				BIT(2)
+#define DATA_ID				(0xff << 8)
+#define DATA_0				(0xff << 16)
+#define DATA_1				(0xff << 24)
+
 #define NS_TO_CYCLE(n, c)    ((n) / (c) + (((n) % (c)) ? 1 : 0))
 
+#define MTK_DSI_HOST_IS_READ(type) \
+	((type == MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM) || \
+	(type == MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM) || \
+	(type == MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM) || \
+	(type == MIPI_DSI_DCS_READ))
+
 struct phy;
 
 struct mtk_dsi {
@@ -465,12 +489,12 @@ static void mtk_dsi_irq_data_set(struct mtk_dsi *dsi, u32 irq_bit)
 	dsi->irq_data |= irq_bit;
 }
 
-static __maybe_unused void mtk_dsi_irq_data_clear(struct mtk_dsi *dsi, u32 irq_bit)
+static void mtk_dsi_irq_data_clear(struct mtk_dsi *dsi, u32 irq_bit)
 {
 	dsi->irq_data &= ~irq_bit;
 }
 
-static __maybe_unused s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
+static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
 				     unsigned int timeout)
 {
 	s32 ret = 0;
@@ -807,9 +831,149 @@ static int mtk_dsi_host_detach(struct mipi_dsi_host *host,
 	return 0;
 }
 
+static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
+{
+	u32 timeout_ms = 500000; /* total 1s ~ 2s timeout */
+
+	while (timeout_ms--) {
+		if (!(readl(dsi->regs + DSI_INTSTA) & DSI_BUSY))
+			break;
+
+		usleep_range(2, 4);
+	}
+
+	if (timeout_ms == 0) {
+		dev_info(dsi->dev, "polling dsi wait not busy timeout!\n");
+
+		mtk_dsi_enable(dsi);
+		mtk_dsi_reset_engine(dsi);
+	}
+}
+
+static u32 mtk_dsi_recv_cnt(u8 type, u8 *read_data)
+{
+	switch (type) {
+	case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_1BYTE:
+	case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
+		return 1;
+	case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_2BYTE:
+	case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_2BYTE:
+		return 2;
+	case MIPI_DSI_RX_GENERIC_LONG_READ_RESPONSE:
+	case MIPI_DSI_RX_DCS_LONG_READ_RESPONSE:
+		return read_data[1] + read_data[2] * 16;
+	case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
+		DRM_INFO("type is 0x02, try again\n");
+		break;
+	default:
+		DRM_INFO("type(0x%x) cannot be non-recognite\n", type);
+		break;
+	}
+
+	return 0;
+}
+
+static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
+{
+	const char *tx_buf = msg->tx_buf;
+	u8 config, cmdq_size, cmdq_off, type = msg->type;
+	u32 reg_val, cmdq_mask, i;
+
+	if (MTK_DSI_HOST_IS_READ(type))
+		config = BTA;
+	else
+		config = (msg->tx_len > 2) ? LONG_PACKET : SHORT_PACKET;
+
+	if (msg->tx_len > 2) {
+		cmdq_size = 1 + (msg->tx_len + 3) / 4;
+		cmdq_off = 4;
+		cmdq_mask = CONFIG | DATA_ID | DATA_0 | DATA_1;
+		reg_val = (msg->tx_len << 16) | (type << 8) | config;
+	} else {
+		cmdq_size = 1;
+		cmdq_off = 2;
+		cmdq_mask = CONFIG | DATA_ID;
+		reg_val = (type << 8) | config;
+	}
+
+	for (i = 0; i < msg->tx_len; i++)
+		writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
+
+	mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
+	mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
+}
+
+static ssize_t mtk_dsi_host_send_cmd(struct mtk_dsi *dsi,
+				     const struct mipi_dsi_msg *msg, u8 flag)
+{
+	mtk_dsi_wait_for_idle(dsi);
+	mtk_dsi_irq_data_clear(dsi, flag);
+	mtk_dsi_cmdq(dsi, msg);
+	mtk_dsi_start(dsi);
+
+	if (!mtk_dsi_wait_for_irq_done(dsi, flag, 2000))
+		return -1;
+	else
+		return 0;
+}
+
+static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
+				     const struct mipi_dsi_msg *msg)
+{
+	struct mtk_dsi *dsi = host_to_dsi(host);
+	u32 recv_cnt, i;
+	u8 read_data[16];
+	void *src_addr;
+	u8 irq_flag = CMD_DONE_INT_FLAG;
+
+	if (readl(dsi->regs + DSI_MODE_CTRL) & MODE) {
+		dev_info(dsi->dev, "dsi engine is not command mode\n");
+		return -1;
+	}
+
+	if (MTK_DSI_HOST_IS_READ(msg->type))
+		irq_flag |= LPRX_RD_RDY_INT_FLAG;
+
+	if (mtk_dsi_host_send_cmd(dsi, msg, irq_flag) < 0)
+		return -1;
+
+	if (!MTK_DSI_HOST_IS_READ(msg->type))
+		return 0;
+
+	if (!msg->rx_buf) {
+		dev_info(dsi->dev, "dsi receive buffer size may be NULL\n");
+		return -1;
+	}
+
+	for (i = 0; i < 16; i++)
+		*(read_data + i) = readb(dsi->regs + DSI_RX_DATA0 + i);
+
+	recv_cnt = mtk_dsi_recv_cnt(read_data[0], read_data);
+
+	if (recv_cnt > 2)
+		src_addr = &read_data[4];
+	else
+		src_addr = &read_data[1];
+
+	if (recv_cnt > 10)
+		recv_cnt = 10;
+
+	if (recv_cnt > msg->rx_len)
+		recv_cnt = msg->rx_len;
+
+	if (recv_cnt)
+		memcpy(msg->rx_buf, src_addr, recv_cnt);
+
+	dev_info(dsi->dev, "dsi get %d byte data from the panel address(0x%x)\n",
+		 recv_cnt, *((u8 *)(msg->tx_buf)));
+
+	return recv_cnt;
+}
+
 static const struct mipi_dsi_host_ops mtk_dsi_ops = {
 	.attach = mtk_dsi_host_attach,
 	.detach = mtk_dsi_host_detach,
+	.transfer = mtk_dsi_host_transfer,
 };
 
 static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
-- 
1.9.1

[PATCH v9 09/10] drm/mediatek: update DSI sub driver flow for sending commands to panel

[YT Shen]

This patch update enable/disable flow of DSI module and MIPI TX module.
Original flow works on there is a bridge chip: DSI -> bridge -> panel.
In this case: DSI -> panel, the DSI sub driver flow should be updated.
We need to initialize DSI first so that we can send commands to panel.

Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c     | 110 ++++++++++++++++++++++++++-------
 drivers/gpu/drm/mediatek/mtk_mipi_tx.c |  32 +++++-----
 2 files changed, 103 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 860b84f..12a1206 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -94,6 +94,8 @@
 #define DSI_RACK		0x84
 #define RACK				BIT(0)
 
+#define DSI_MEM_CONTI		0x90
+
 #define DSI_PHY_LCCON		0x104
 #define LC_HS_TX_EN			BIT(0)
 #define LC_ULPM_EN			BIT(1)
@@ -126,6 +128,10 @@
 #define CLK_HS_POST			(0xff << 8)
 #define CLK_HS_EXIT			(0xff << 16)
 
+#define DSI_VM_CMD_CON		0x130
+#define VM_CMD_EN			BIT(0)
+#define TS_VFP_EN			BIT(5)
+
 #define DSI_CMDQ0		0x180
 #define CONFIG				(0xff << 0)
 #define SHORT_PACKET			0
@@ -219,12 +225,12 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
 	writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
 }
 
-static void mtk_dsi_enable(struct mtk_dsi *dsi)
+static void mtk_dsi_engine_enable(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, DSI_EN);
 }
 
-static void mtk_dsi_disable(struct mtk_dsi *dsi)
+static void mtk_dsi_engine_disable(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
 }
@@ -249,7 +255,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
 	 * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
 	 * we set mipi_ratio is 1.05.
 	 */
-	dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
+	dsi->data_rate = dsi->vm.pixelclock * 12 * 21;
+	dsi->data_rate /= (dsi->lanes * 1000 * 10);
+	dev_info(dev, "set mipitx's data rate: %dMHz\n", dsi->data_rate);
 
 	ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
 	if (ret < 0) {
@@ -271,7 +279,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
 		goto err_disable_engine_clk;
 	}
 
-	mtk_dsi_enable(dsi);
+	mtk_dsi_engine_enable(dsi);
 	mtk_dsi_reset_engine(dsi);
 	mtk_dsi_phy_timconfig(dsi);
 
@@ -289,7 +297,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
 static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
-	mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
+	mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, LC_ULPM_EN);
 }
 
 static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
@@ -302,7 +310,7 @@ static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
 static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
 {
 	mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
-	mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
+	mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, LD0_ULPM_EN);
 }
 
 static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
@@ -338,11 +346,21 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
 		if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
 		    !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
 			vid_mode = BURST_MODE;
+		else
+			vid_mode = SYNC_EVENT_MODE;
 	}
 
 	writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
 }
 
+static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
+{
+	writel(0x3c, dsi->regs + DSI_MEM_CONTI);
+
+	mtk_dsi_mask(dsi, DSI_VM_CMD_CON, VM_CMD_EN, VM_CMD_EN);
+	mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
+}
+
 static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
 {
 	struct videomode *vm = &dsi->vm;
@@ -399,6 +417,9 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
 		break;
 	}
 
+	tmp_reg |= (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) << 6;
+	tmp_reg |= (dsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) >> 3;
+
 	writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
 }
 
@@ -477,6 +498,16 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
 	writel(1, dsi->regs + DSI_START);
 }
 
+static void mtk_dsi_stop(struct mtk_dsi *dsi)
+{
+	writel(0, dsi->regs + DSI_START);
+}
+
+static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
+{
+	writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
+}
+
 static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
 {
 	u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
@@ -506,7 +537,7 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
 	if (ret == 0) {
 		dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
 
-		mtk_dsi_enable(dsi);
+		mtk_dsi_engine_enable(dsi);
 		mtk_dsi_reset_engine(dsi);
 	}
 
@@ -535,6 +566,17 @@ static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
+{
+	mtk_dsi_irq_data_clear(dsi, irq_flag);
+	mtk_dsi_set_cmd_mode(dsi);
+
+	if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t))
+		return -1;
+	else
+		return 0;
+}
+
 static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
 {
 	if (WARN_ON(dsi->refcount == 0))
@@ -543,11 +585,6 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
 	if (--dsi->refcount != 0)
 		return;
 
-	mtk_dsi_lane0_ulp_mode_enter(dsi);
-	mtk_dsi_clk_ulp_mode_enter(dsi);
-
-	mtk_dsi_disable(dsi);
-
 	clk_disable_unprepare(dsi->engine_clk);
 	clk_disable_unprepare(dsi->digital_clk);
 
@@ -561,35 +598,45 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
 	if (dsi->enabled)
 		return;
 
-	if (dsi->panel) {
-		if (drm_panel_prepare(dsi->panel)) {
-			DRM_ERROR("failed to setup the panel\n");
-			return;
-		}
-	}
-
 	ret = mtk_dsi_poweron(dsi);
 	if (ret < 0) {
 		DRM_ERROR("failed to power on dsi\n");
 		return;
 	}
 
+	usleep_range(20000, 21000);
+
 	mtk_dsi_rxtx_control(dsi);
+	mtk_dsi_phy_timconfig(dsi);
+	mtk_dsi_ps_control_vact(dsi);
+	mtk_dsi_set_vm_cmd(dsi);
+	mtk_dsi_config_vdo_timing(dsi);
+	mtk_dsi_set_interrupt_enable(dsi);
 
+	mtk_dsi_engine_enable(dsi);
 	mtk_dsi_clk_ulp_mode_leave(dsi);
 	mtk_dsi_lane0_ulp_mode_leave(dsi);
 	mtk_dsi_clk_hs_mode(dsi, 0);
-	mtk_dsi_set_mode(dsi);
 
-	mtk_dsi_ps_control_vact(dsi);
-	mtk_dsi_config_vdo_timing(dsi);
-	mtk_dsi_set_interrupt_enable(dsi);
+	if (dsi->panel) {
+		if (drm_panel_prepare(dsi->panel)) {
+			DRM_ERROR("failed to prepare the panel\n");
+			return;
+		}
+	}
 
 	mtk_dsi_set_mode(dsi);
 	mtk_dsi_clk_hs_mode(dsi, 1);
 
 	mtk_dsi_start(dsi);
 
+	if (dsi->panel) {
+		if (drm_panel_enable(dsi->panel)) {
+			DRM_ERROR("failed to enable the panel\n");
+			return;
+		}
+	}
+
 	dsi->enabled = true;
 }
 
@@ -605,6 +652,21 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
 		}
 	}
 
+	mtk_dsi_stop(dsi);
+	mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);
+
+	if (dsi->panel) {
+		if (drm_panel_unprepare(dsi->panel)) {
+			DRM_ERROR("failed to unprepare the panel\n");
+			return;
+		}
+	}
+
+	mtk_dsi_reset_engine(dsi);
+	mtk_dsi_lane0_ulp_mode_enter(dsi);
+	mtk_dsi_clk_ulp_mode_enter(dsi);
+	mtk_dsi_engine_disable(dsi);
+
 	mtk_dsi_poweroff(dsi);
 
 	dsi->enabled = false;
@@ -845,7 +907,7 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
 	if (timeout_ms == 0) {
 		dev_info(dsi->dev, "polling dsi wait not busy timeout!\n");
 
-		mtk_dsi_enable(dsi);
+		mtk_dsi_engine_enable(dsi);
 		mtk_dsi_reset_engine(dsi);
 	}
 }
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 108d31a..34e95c6 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -177,7 +177,9 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
 
 	dev_dbg(mipi_tx->dev, "prepare: %u Hz\n", mipi_tx->data_rate);
 
-	if (mipi_tx->data_rate >= 500000000) {
+	if (mipi_tx->data_rate > 1250000000) {
+		return -EINVAL;
+	} else if (mipi_tx->data_rate >= 500000000) {
 		txdiv = 1;
 		txdiv0 = 0;
 		txdiv1 = 0;
@@ -201,6 +203,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
 		return -EINVAL;
 	}
 
+	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
+				RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
+				(8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
+
 	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_BG_CON,
 				RG_DSI_VOUT_MSK |
 				RG_DSI_BG_CKEN | RG_DSI_BG_CORE_EN,
@@ -210,24 +216,18 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
 
 	usleep_range(30, 100);
 
-	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
-				RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
-				(8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
-
-	mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_CON,
-			     RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
+	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_CON,
+				RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN,
+				RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
 
 	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR,
 				RG_DSI_MPPLL_SDM_PWR_ON |
 				RG_DSI_MPPLL_SDM_ISO_EN,
 				RG_DSI_MPPLL_SDM_PWR_ON);
 
-	mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
-			       RG_DSI_MPPLL_PLL_EN);
-
 	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
-				RG_DSI_MPPLL_TXDIV0 | RG_DSI_MPPLL_TXDIV1 |
-				RG_DSI_MPPLL_PREDIV,
+				RG_DSI_MPPLL_PREDIV | RG_DSI_MPPLL_TXDIV0 |
+				RG_DSI_MPPLL_TXDIV1 | RG_DSI_MPPLL_POSDIV,
 				(txdiv0 << 3) | (txdiv1 << 5));
 
 	/*
@@ -242,10 +242,12 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
 		      26000000);
 	writel(pcw, mipi_tx->regs + MIPITX_DSI_PLL_CON2);
 
-	mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
-			     RG_DSI_MPPLL_SDM_FRA_EN);
+	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
+				RG_DSI_MPPLL_SDM_FRA_EN,
+				RG_DSI_MPPLL_SDM_FRA_EN);
 
-	mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON0, RG_DSI_MPPLL_PLL_EN);
+	mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
+				RG_DSI_MPPLL_PLL_EN, RG_DSI_MPPLL_PLL_EN);
 
 	usleep_range(20, 100);
 
-- 
1.9.1

Re: [PATCH v9 09/10] drm/mediatek: update DSI sub driver flow for sending commands to panel

[Daniel Kurtz]

Hi YT,

Sorry for the very late review.

My biggest problem with this patch is it describes itself as adding
support for a new use case "DSI -> panel", but makes many changes to
the existing working flow "DSI -> bridge -> panel".
If these changes are really needed, or improve the existing flow, I'd
expect to see those changes added first in a preparatory patch,
followed by a second smaller, simpler
patch that adds any additional functionality required to enable the new flow.

See detailed comments inline.


On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
>
> This patch update enable/disable flow of DSI module and MIPI TX module.
> Original flow works on there is a bridge chip: DSI -> bridge -> panel.
> In this case: DSI -> panel, the DSI sub driver flow should be updated.
> We need to initialize DSI first so that we can send commands to panel.
>
> Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> Signed-off-by: YT Shen <yt.shen@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c     | 110 ++++++++++++++++++++++++++-------
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.c |  32 +++++-----
>  2 files changed, 103 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 860b84f..12a1206 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -94,6 +94,8 @@
>  #define DSI_RACK               0x84
>  #define RACK                           BIT(0)
>
> +#define DSI_MEM_CONTI          0x90
> +
>  #define DSI_PHY_LCCON          0x104
>  #define LC_HS_TX_EN                    BIT(0)
>  #define LC_ULPM_EN                     BIT(1)
> @@ -126,6 +128,10 @@
>  #define CLK_HS_POST                    (0xff << 8)
>  #define CLK_HS_EXIT                    (0xff << 16)
>
> +#define DSI_VM_CMD_CON         0x130
> +#define VM_CMD_EN                      BIT(0)
> +#define TS_VFP_EN                      BIT(5)
> +
>  #define DSI_CMDQ0              0x180
>  #define CONFIG                         (0xff << 0)
>  #define SHORT_PACKET                   0
> @@ -219,12 +225,12 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
>         writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
>  }
>
> -static void mtk_dsi_enable(struct mtk_dsi *dsi)
> +static void mtk_dsi_engine_enable(struct mtk_dsi *dsi)

I don't think we need to change these names.

>  {
>         mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, DSI_EN);
>  }
>
> -static void mtk_dsi_disable(struct mtk_dsi *dsi)
> +static void mtk_dsi_engine_disable(struct mtk_dsi *dsi)
>  {
>         mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
>  }
> @@ -249,7 +255,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
>          * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
>          * we set mipi_ratio is 1.05.
>          */
> -       dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> +       dsi->data_rate = dsi->vm.pixelclock * 12 * 21;
> +       dsi->data_rate /= (dsi->lanes * 1000 * 10);
> +       dev_info(dev, "set mipitx's data rate: %dMHz\n", dsi->data_rate);

I don't think we want to spam the log like this.  Use dev_dbg.... or
use the DRM_() messaging like elsewhere in this driver?

>
>         ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
>         if (ret < 0) {
> @@ -271,7 +279,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
>                 goto err_disable_engine_clk;
>         }
>
> -       mtk_dsi_enable(dsi);
> +       mtk_dsi_engine_enable(dsi);
>         mtk_dsi_reset_engine(dsi);
>         mtk_dsi_phy_timconfig(dsi);
>
> @@ -289,7 +297,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
>  static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
>  {
>         mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
> -       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
> +       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, LC_ULPM_EN);

What does this change do?
It looks like a pure bug fix (ie, previoulsy we were'nt actually
enabling ULP MODE before).
If so, can you please move it to a separate preliminary patch.

>  }
>
>  static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
> @@ -302,7 +310,7 @@ static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
>  static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
>  {
>         mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
> -       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
> +       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, LD0_ULPM_EN);

Same here.

>  }
>
>  static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
> @@ -338,11 +346,21 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
>                 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
>                     !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
>                         vid_mode = BURST_MODE;
> +               else
> +                       vid_mode = SYNC_EVENT_MODE;

So, when do we use SYNC_PULSE_MODE (set just before the 'if')?

>         }
>
>         writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
>  }
>
> +static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
> +{
> +       writel(0x3c, dsi->regs + DSI_MEM_CONTI);

Please use #defined constants, especially if this register is a bit field.
Also, this looks like new behavior which doesn't seem related to
changing the enable order.
If this is a general fix, please use a separate patch.

> +
> +       mtk_dsi_mask(dsi, DSI_VM_CMD_CON, VM_CMD_EN, VM_CMD_EN);
> +       mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
> +}
> +
>  static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
>  {
>         struct videomode *vm = &dsi->vm;
> @@ -399,6 +417,9 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
>                 break;
>         }
>
> +       tmp_reg |= (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) << 6;
> +       tmp_reg |= (dsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) >> 3;
> +

ditto

>         writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
>  }
>
> @@ -477,6 +498,16 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
>         writel(1, dsi->regs + DSI_START);
>  }
>
> +static void mtk_dsi_stop(struct mtk_dsi *dsi)
> +{
> +       writel(0, dsi->regs + DSI_START);
> +}
> +
> +static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
> +{
> +       writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
> +}
> +
>  static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
>  {
>         u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
> @@ -506,7 +537,7 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
>         if (ret == 0) {
>                 dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
>
> -               mtk_dsi_enable(dsi);
> +               mtk_dsi_engine_enable(dsi);
>                 mtk_dsi_reset_engine(dsi);
>         }
>
> @@ -535,6 +566,17 @@ static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
>         return IRQ_HANDLED;
>  }
>
> +static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
> +{
> +       mtk_dsi_irq_data_clear(dsi, irq_flag);
> +       mtk_dsi_set_cmd_mode(dsi);
> +
> +       if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t))
> +               return -1;

No, use a real linux errno, and return an int, and print an error
message if this is unexpected.

> +       else
> +               return 0;
> +}
> +
>  static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
>  {
>         if (WARN_ON(dsi->refcount == 0))
> @@ -543,11 +585,6 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
>         if (--dsi->refcount != 0)
>                 return;
>
> -       mtk_dsi_lane0_ulp_mode_enter(dsi);
> -       mtk_dsi_clk_ulp_mode_enter(dsi);
> -
> -       mtk_dsi_disable(dsi);
> -
>         clk_disable_unprepare(dsi->engine_clk);
>         clk_disable_unprepare(dsi->digital_clk);
>
> @@ -561,35 +598,45 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
>         if (dsi->enabled)
>                 return;
>
> -       if (dsi->panel) {
> -               if (drm_panel_prepare(dsi->panel)) {
> -                       DRM_ERROR("failed to setup the panel\n");
> -                       return;
> -               }
> -       }
> -
>         ret = mtk_dsi_poweron(dsi);
>         if (ret < 0) {
>                 DRM_ERROR("failed to power on dsi\n");
>                 return;
>         }
>
> +       usleep_range(20000, 21000);
> +

Why are you adding a 20 ms delay where there was none before?

>         mtk_dsi_rxtx_control(dsi);
> +       mtk_dsi_phy_timconfig(dsi);
> +       mtk_dsi_ps_control_vact(dsi);
> +       mtk_dsi_set_vm_cmd(dsi);
> +       mtk_dsi_config_vdo_timing(dsi);
> +       mtk_dsi_set_interrupt_enable(dsi);
>
> +       mtk_dsi_engine_enable(dsi);
>         mtk_dsi_clk_ulp_mode_leave(dsi);
>         mtk_dsi_lane0_ulp_mode_leave(dsi);
>         mtk_dsi_clk_hs_mode(dsi, 0);
> -       mtk_dsi_set_mode(dsi);
>
> -       mtk_dsi_ps_control_vact(dsi);
> -       mtk_dsi_config_vdo_timing(dsi);
> -       mtk_dsi_set_interrupt_enable(dsi);
> +       if (dsi->panel) {
> +               if (drm_panel_prepare(dsi->panel)) {
> +                       DRM_ERROR("failed to prepare the panel\n");
> +                       return;
> +               }
> +       }
>
>         mtk_dsi_set_mode(dsi);
>         mtk_dsi_clk_hs_mode(dsi, 1);
>
>         mtk_dsi_start(dsi);
>
> +       if (dsi->panel) {
> +               if (drm_panel_enable(dsi->panel)) {
> +                       DRM_ERROR("failed to enable the panel\n");

In case of error, you must undo everything done to this point.  At least:
 (1) unprepare the panel
 (2) stop dsi
 (3) poweroff dsi

> +                       return;
> +               }
> +       }
> +
>         dsi->enabled = true;
>  }
>
> @@ -605,6 +652,21 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
>                 }
>         }
>
> +       mtk_dsi_stop(dsi);
> +       mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);

This function can return an error, so please check it.  Although,
there probably isn't much you can do here about it.

> +
> +       if (dsi->panel) {
> +               if (drm_panel_unprepare(dsi->panel)) {
> +                       DRM_ERROR("failed to unprepare the panel\n");
> +                       return;

I think you should probably just ignore this error and continue
disabling dsi, since it isn't really recoverable and you can't roll
back and re-enable dsi.


> +               }
> +       }
> +
> +       mtk_dsi_reset_engine(dsi);
> +       mtk_dsi_lane0_ulp_mode_enter(dsi);
> +       mtk_dsi_clk_ulp_mode_enter(dsi);
> +       mtk_dsi_engine_disable(dsi);
> +
>         mtk_dsi_poweroff(dsi);
>
>         dsi->enabled = false;
> @@ -845,7 +907,7 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
>         if (timeout_ms == 0) {
>                 dev_info(dsi->dev, "polling dsi wait not busy timeout!\n");
>
> -               mtk_dsi_enable(dsi);
> +               mtk_dsi_engine_enable(dsi);
>                 mtk_dsi_reset_engine(dsi);
>         }
>  }
> diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> index 108d31a..34e95c6 100644
> --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> @@ -177,7 +177,9 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
>
>         dev_dbg(mipi_tx->dev, "prepare: %u Hz\n", mipi_tx->data_rate);
>
> -       if (mipi_tx->data_rate >= 500000000) {
> +       if (mipi_tx->data_rate > 1250000000) {
> +               return -EINVAL;
> +       } else if (mipi_tx->data_rate >= 500000000) {

Capping the max data rate looks like an unrelated fix.

>                 txdiv = 1;
>                 txdiv0 = 0;
>                 txdiv1 = 0;
> @@ -201,6 +203,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
>                 return -EINVAL;
>         }
>
> +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
> +                               RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
> +                               (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
> +
>         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_BG_CON,
>                                 RG_DSI_VOUT_MSK |
>                                 RG_DSI_BG_CKEN | RG_DSI_BG_CORE_EN,
> @@ -210,24 +216,18 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
>
>         usleep_range(30, 100);
>
> -       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
> -                               RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
> -                               (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
> -
> -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_CON,
> -                            RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
> +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_CON,
> +                               RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN,
> +                               RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);

Changing from set_bits to update_bits does not do anything.  Please
leave this alone.

>
>         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR,
>                                 RG_DSI_MPPLL_SDM_PWR_ON |
>                                 RG_DSI_MPPLL_SDM_ISO_EN,
>                                 RG_DSI_MPPLL_SDM_PWR_ON);
>
> -       mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> -                              RG_DSI_MPPLL_PLL_EN);
> -

Why don't you need to disable the PLL first now?

>         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> -                               RG_DSI_MPPLL_TXDIV0 | RG_DSI_MPPLL_TXDIV1 |
> -                               RG_DSI_MPPLL_PREDIV,
> +                               RG_DSI_MPPLL_PREDIV | RG_DSI_MPPLL_TXDIV0 |
> +                               RG_DSI_MPPLL_TXDIV1 | RG_DSI_MPPLL_POSDIV,
>                                 (txdiv0 << 3) | (txdiv1 << 5));

If I read this right, the only thing you are changing is clearing
"RG_DSI_MPPLL_POSDIV".
This would be more clear if you kept the field order: TXDIV0, TXDIV1, PREDIV.

And why are you making this change in this patch?


>
>         /*
> @@ -242,10 +242,12 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
>                       26000000);
>         writel(pcw, mipi_tx->regs + MIPITX_DSI_PLL_CON2);
>
> -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
> -                            RG_DSI_MPPLL_SDM_FRA_EN);
> +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
> +                               RG_DSI_MPPLL_SDM_FRA_EN,
> +                               RG_DSI_MPPLL_SDM_FRA_EN);

AFAICT, this change does not do anything but make the code more confusing.

>
> -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON0, RG_DSI_MPPLL_PLL_EN);
> +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> +                               RG_DSI_MPPLL_PLL_EN, RG_DSI_MPPLL_PLL_EN);

AFAICT, this change does not do anything but make the code more confusing.

>
>         usleep_range(20, 100);
>
> --
> 1.9.1
>

Re: [PATCH v9 09/10] drm/mediatek: update DSI sub driver flow for sending commands to panel

[YT Shen]

Hi Daniel,

Thanks for the review.

On Fri, 2016-11-18 at 11:21 +0800, Daniel Kurtz wrote:
> Hi YT,
> 
> Sorry for the very late review.
> 
> My biggest problem with this patch is it describes itself as adding
> support for a new use case "DSI -> panel", but makes many changes to
> the existing working flow "DSI -> bridge -> panel".
> If these changes are really needed, or improve the existing flow, I'd
> expect to see those changes added first in a preparatory patch,
> followed by a second smaller, simpler
> patch that adds any additional functionality required to enable the new flow.
We will split this patch into several smaller preparatory patches
necessary in the next version.

> 
> See detailed comments inline.
> 
> 
> On Fri, Nov 11, 2016 at 7:55 PM, YT Shen <yt.shen@mediatek.com> wrote:
> >
> > This patch update enable/disable flow of DSI module and MIPI TX module.
> > Original flow works on there is a bridge chip: DSI -> bridge -> panel.
> > In this case: DSI -> panel, the DSI sub driver flow should be updated.
> > We need to initialize DSI first so that we can send commands to panel.
> >
> > Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> > Signed-off-by: YT Shen <yt.shen@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c     | 110 ++++++++++++++++++++++++++-------
> >  drivers/gpu/drm/mediatek/mtk_mipi_tx.c |  32 +++++-----
> >  2 files changed, 103 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index 860b84f..12a1206 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -94,6 +94,8 @@
> >  #define DSI_RACK               0x84
> >  #define RACK                           BIT(0)
> >
> > +#define DSI_MEM_CONTI          0x90
> > +
> >  #define DSI_PHY_LCCON          0x104
> >  #define LC_HS_TX_EN                    BIT(0)
> >  #define LC_ULPM_EN                     BIT(1)
> > @@ -126,6 +128,10 @@
> >  #define CLK_HS_POST                    (0xff << 8)
> >  #define CLK_HS_EXIT                    (0xff << 16)
> >
> > +#define DSI_VM_CMD_CON         0x130
> > +#define VM_CMD_EN                      BIT(0)
> > +#define TS_VFP_EN                      BIT(5)
> > +
> >  #define DSI_CMDQ0              0x180
> >  #define CONFIG                         (0xff << 0)
> >  #define SHORT_PACKET                   0
> > @@ -219,12 +225,12 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
> >         writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
> >  }
> >
> > -static void mtk_dsi_enable(struct mtk_dsi *dsi)
> > +static void mtk_dsi_engine_enable(struct mtk_dsi *dsi)
> 
> I don't think we need to change these names.
OK.

> 
> >  {
> >         mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, DSI_EN);
> >  }
> >
> > -static void mtk_dsi_disable(struct mtk_dsi *dsi)
> > +static void mtk_dsi_engine_disable(struct mtk_dsi *dsi)
> >  {
> >         mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
> >  }
> > @@ -249,7 +255,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> >          * mipi_ratio is mipi clk coefficient for balance the pixel clk in mipi.
> >          * we set mipi_ratio is 1.05.
> >          */
> > -       dsi->data_rate = dsi->vm.pixelclock * 3 * 21 / (1 * 1000 * 10);
> > +       dsi->data_rate = dsi->vm.pixelclock * 12 * 21;
> > +       dsi->data_rate /= (dsi->lanes * 1000 * 10);
> > +       dev_info(dev, "set mipitx's data rate: %dMHz\n", dsi->data_rate);
> 
> I don't think we want to spam the log like this.  Use dev_dbg.... or
> use the DRM_() messaging like elsewhere in this driver?
OK, we will remove logs like this in the patch series.

> 
> >
> >         ret = clk_set_rate(dsi->hs_clk, dsi->data_rate * 1000000);
> >         if (ret < 0) {
> > @@ -271,7 +279,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> >                 goto err_disable_engine_clk;
> >         }
> >
> > -       mtk_dsi_enable(dsi);
> > +       mtk_dsi_engine_enable(dsi);
> >         mtk_dsi_reset_engine(dsi);
> >         mtk_dsi_phy_timconfig(dsi);
> >
> > @@ -289,7 +297,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> >  static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
> >  {
> >         mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
> > -       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
> > +       mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, LC_ULPM_EN);
> 
> What does this change do?
> It looks like a pure bug fix (ie, previoulsy we were'nt actually
> enabling ULP MODE before).
> If so, can you please move it to a separate preliminary patch.
OK.

> 
> >  }
> >
> >  static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
> > @@ -302,7 +310,7 @@ static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
> >  static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
> >  {
> >         mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
> > -       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
> > +       mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, LD0_ULPM_EN);
> 
> Same here.
> 
> >  }
> >
> >  static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
> > @@ -338,11 +346,21 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
> >                 if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
> >                     !(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
> >                         vid_mode = BURST_MODE;
> > +               else
> > +                       vid_mode = SYNC_EVENT_MODE;
> 
> So, when do we use SYNC_PULSE_MODE (set just before the 'if')?
We will update this part.

> 
> >         }
> >
> >         writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
> >  }
> >
> > +static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
> > +{
> > +       writel(0x3c, dsi->regs + DSI_MEM_CONTI);
> 
> Please use #defined constants, especially if this register is a bit field.
> Also, this looks like new behavior which doesn't seem related to
> changing the enable order.
> If this is a general fix, please use a separate patch.
We will remove this part.  This change is not necessary.

> 
> > +
> > +       mtk_dsi_mask(dsi, DSI_VM_CMD_CON, VM_CMD_EN, VM_CMD_EN);
> > +       mtk_dsi_mask(dsi, DSI_VM_CMD_CON, TS_VFP_EN, TS_VFP_EN);
> > +}
> > +
> >  static void mtk_dsi_ps_control_vact(struct mtk_dsi *dsi)
> >  {
> >         struct videomode *vm = &dsi->vm;
> > @@ -399,6 +417,9 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
> >                 break;
> >         }
> >
> > +       tmp_reg |= (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) << 6;
> > +       tmp_reg |= (dsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET) >> 3;
> > +
> 
> ditto
> 
> >         writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
> >  }
> >
> > @@ -477,6 +498,16 @@ static void mtk_dsi_start(struct mtk_dsi *dsi)
> >         writel(1, dsi->regs + DSI_START);
> >  }
> >
> > +static void mtk_dsi_stop(struct mtk_dsi *dsi)
> > +{
> > +       writel(0, dsi->regs + DSI_START);
> > +}
> > +
> > +static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
> > +{
> > +       writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
> > +}
> > +
> >  static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
> >  {
> >         u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
> > @@ -506,7 +537,7 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
> >         if (ret == 0) {
> >                 dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
> >
> > -               mtk_dsi_enable(dsi);
> > +               mtk_dsi_engine_enable(dsi);
> >                 mtk_dsi_reset_engine(dsi);
> >         }
> >
> > @@ -535,6 +566,17 @@ static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> >         return IRQ_HANDLED;
> >  }
> >
> > +static s32 mtk_dsi_switch_to_cmd_mode(struct mtk_dsi *dsi, u8 irq_flag, u32 t)
> > +{
> > +       mtk_dsi_irq_data_clear(dsi, irq_flag);
> > +       mtk_dsi_set_cmd_mode(dsi);
> > +
> > +       if (!mtk_dsi_wait_for_irq_done(dsi, irq_flag, t))
> > +               return -1;
> 
> No, use a real linux errno, and return an int, and print an error
> message if this is unexpected.
Will use a real errno: ETIME.

> 
> > +       else
> > +               return 0;
> > +}
> > +
> >  static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> >  {
> >         if (WARN_ON(dsi->refcount == 0))
> > @@ -543,11 +585,6 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
> >         if (--dsi->refcount != 0)
> >                 return;
> >
> > -       mtk_dsi_lane0_ulp_mode_enter(dsi);
> > -       mtk_dsi_clk_ulp_mode_enter(dsi);
> > -
> > -       mtk_dsi_disable(dsi);
> > -
> >         clk_disable_unprepare(dsi->engine_clk);
> >         clk_disable_unprepare(dsi->digital_clk);
> >
> > @@ -561,35 +598,45 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
> >         if (dsi->enabled)
> >                 return;
> >
> > -       if (dsi->panel) {
> > -               if (drm_panel_prepare(dsi->panel)) {
> > -                       DRM_ERROR("failed to setup the panel\n");
> > -                       return;
> > -               }
> > -       }
> > -
> >         ret = mtk_dsi_poweron(dsi);
> >         if (ret < 0) {
> >                 DRM_ERROR("failed to power on dsi\n");
> >                 return;
> >         }
> >
> > +       usleep_range(20000, 21000);
> > +
> 
> Why are you adding a 20 ms delay where there was none before?
After checking, we will remove redundant codes and the delay.

> 
> >         mtk_dsi_rxtx_control(dsi);
> > +       mtk_dsi_phy_timconfig(dsi);
> > +       mtk_dsi_ps_control_vact(dsi);
> > +       mtk_dsi_set_vm_cmd(dsi);
> > +       mtk_dsi_config_vdo_timing(dsi);
> > +       mtk_dsi_set_interrupt_enable(dsi);
> >
> > +       mtk_dsi_engine_enable(dsi);
> >         mtk_dsi_clk_ulp_mode_leave(dsi);
> >         mtk_dsi_lane0_ulp_mode_leave(dsi);
> >         mtk_dsi_clk_hs_mode(dsi, 0);
> > -       mtk_dsi_set_mode(dsi);
> >
> > -       mtk_dsi_ps_control_vact(dsi);
> > -       mtk_dsi_config_vdo_timing(dsi);
> > -       mtk_dsi_set_interrupt_enable(dsi);
> > +       if (dsi->panel) {
> > +               if (drm_panel_prepare(dsi->panel)) {
> > +                       DRM_ERROR("failed to prepare the panel\n");
> > +                       return;
> > +               }
> > +       }
> >
> >         mtk_dsi_set_mode(dsi);
> >         mtk_dsi_clk_hs_mode(dsi, 1);
> >
> >         mtk_dsi_start(dsi);
> >
> > +       if (dsi->panel) {
> > +               if (drm_panel_enable(dsi->panel)) {
> > +                       DRM_ERROR("failed to enable the panel\n");
> 
> In case of error, you must undo everything done to this point.  At least:
>  (1) unprepare the panel
>  (2) stop dsi
>  (3) poweroff dsi
OK.

> 
> > +                       return;
> > +               }
> > +       }
> > +
> >         dsi->enabled = true;
> >  }
> >
> > @@ -605,6 +652,21 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi)
> >                 }
> >         }
> >
> > +       mtk_dsi_stop(dsi);
> > +       mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);
> 
> This function can return an error, so please check it.  Although,
> there probably isn't much you can do here about it.
OK.

> 
> > +
> > +       if (dsi->panel) {
> > +               if (drm_panel_unprepare(dsi->panel)) {
> > +                       DRM_ERROR("failed to unprepare the panel\n");
> > +                       return;
> 
> I think you should probably just ignore this error and continue
> disabling dsi, since it isn't really recoverable and you can't roll
> back and re-enable dsi.
OK.

> 
> 
> > +               }
> > +       }
> > +
> > +       mtk_dsi_reset_engine(dsi);
> > +       mtk_dsi_lane0_ulp_mode_enter(dsi);
> > +       mtk_dsi_clk_ulp_mode_enter(dsi);
> > +       mtk_dsi_engine_disable(dsi);
> > +
> >         mtk_dsi_poweroff(dsi);
> >
> >         dsi->enabled = false;
> > @@ -845,7 +907,7 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
> >         if (timeout_ms == 0) {
> >                 dev_info(dsi->dev, "polling dsi wait not busy timeout!\n");
> >
> > -               mtk_dsi_enable(dsi);
> > +               mtk_dsi_engine_enable(dsi);
> >                 mtk_dsi_reset_engine(dsi);
> >         }
> >  }
> > diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > index 108d31a..34e95c6 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
> > @@ -177,7 +177,9 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> >
> >         dev_dbg(mipi_tx->dev, "prepare: %u Hz\n", mipi_tx->data_rate);
> >
> > -       if (mipi_tx->data_rate >= 500000000) {
> > +       if (mipi_tx->data_rate > 1250000000) {
> > +               return -EINVAL;
> > +       } else if (mipi_tx->data_rate >= 500000000) {
> 
> Capping the max data rate looks like an unrelated fix.
Will prepare additional patch for max data rate.

> 
> >                 txdiv = 1;
> >                 txdiv0 = 0;
> >                 txdiv1 = 0;
> > @@ -201,6 +203,10 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> >                 return -EINVAL;
> >         }
> >
> > +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
> > +                               RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
> > +                               (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
> > +
> >         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_BG_CON,
> >                                 RG_DSI_VOUT_MSK |
> >                                 RG_DSI_BG_CKEN | RG_DSI_BG_CORE_EN,
> > @@ -210,24 +216,18 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> >
> >         usleep_range(30, 100);
> >
> > -       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_TOP_CON,
> > -                               RG_DSI_LNT_IMP_CAL_CODE | RG_DSI_LNT_HS_BIAS_EN,
> > -                               (8 << 4) | RG_DSI_LNT_HS_BIAS_EN);
> > -
> > -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_CON,
> > -                            RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
> > +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_CON,
> > +                               RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN,
> > +                               RG_DSI_CKG_LDOOUT_EN | RG_DSI_LDOCORE_EN);
> 
> Changing from set_bits to update_bits does not do anything.  Please
> leave this alone.
OK.

> 
> >
> >         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_PWR,
> >                                 RG_DSI_MPPLL_SDM_PWR_ON |
> >                                 RG_DSI_MPPLL_SDM_ISO_EN,
> >                                 RG_DSI_MPPLL_SDM_PWR_ON);
> >
> > -       mtk_mipi_tx_clear_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> > -                              RG_DSI_MPPLL_PLL_EN);
> > -
> 
> Why don't you need to disable the PLL first now?
Yes, we need.  Will fix this.

> 
> >         mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> > -                               RG_DSI_MPPLL_TXDIV0 | RG_DSI_MPPLL_TXDIV1 |
> > -                               RG_DSI_MPPLL_PREDIV,
> > +                               RG_DSI_MPPLL_PREDIV | RG_DSI_MPPLL_TXDIV0 |
> > +                               RG_DSI_MPPLL_TXDIV1 | RG_DSI_MPPLL_POSDIV,
> >                                 (txdiv0 << 3) | (txdiv1 << 5));
> 
> If I read this right, the only thing you are changing is clearing
> "RG_DSI_MPPLL_POSDIV".
> This would be more clear if you kept the field order: TXDIV0, TXDIV1, PREDIV.
> 
> And why are you making this change in this patch?
Hmm, we will provide another patch for this part if necessary.
Sometimes settings are changed not in kernel stage (maybe display from
bootloader)  This change just make sure kernel have the right
configuration.

> 
> 
> >
> >         /*
> > @@ -242,10 +242,12 @@ static int mtk_mipi_tx_pll_prepare(struct clk_hw *hw)
> >                       26000000);
> >         writel(pcw, mipi_tx->regs + MIPITX_DSI_PLL_CON2);
> >
> > -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
> > -                            RG_DSI_MPPLL_SDM_FRA_EN);
> > +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON1,
> > +                               RG_DSI_MPPLL_SDM_FRA_EN,
> > +                               RG_DSI_MPPLL_SDM_FRA_EN);
> 
> AFAICT, this change does not do anything but make the code more confusing.
OK.

> 
> >
> > -       mtk_mipi_tx_set_bits(mipi_tx, MIPITX_DSI_PLL_CON0, RG_DSI_MPPLL_PLL_EN);
> > +       mtk_mipi_tx_update_bits(mipi_tx, MIPITX_DSI_PLL_CON0,
> > +                               RG_DSI_MPPLL_PLL_EN, RG_DSI_MPPLL_PLL_EN);
> 
> AFAICT, this change does not do anything but make the code more confusing.
OK.

> 
> >
> >         usleep_range(20, 100);
> >
> > --
> > 1.9.1
> >

[PATCH v9 10/10] drm/mediatek: add support for Mediatek SoC MT2701

[YT Shen]

This patch add support for the Mediatek MT2701 DISP subsystem.
There is only one OVL engine in MT2701.

Signed-off-by: YT Shen <yt.shen@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c     |  6 ++++++
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c    |  6 ++++++
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 17 +++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  6 ++++++
 drivers/gpu/drm/mediatek/mtk_drm_drv.c      | 29 +++++++++++++++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_dsi.c          |  1 +
 drivers/gpu/drm/mediatek/mtk_mipi_tx.c      |  6 ++++++
 7 files changed, 71 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 1139834..d174905 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -286,11 +286,17 @@ static int mtk_disp_ovl_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_ddp_comp_driver_data mt2701_ovl_driver_data = {
+	.ovl = {0x0040, 1 << 12, 0}
+};
+
 static const struct mtk_ddp_comp_driver_data mt8173_ovl_driver_data = {
 	.ovl = {0x0f40, 0, 1 << 12}
 };
 
 static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
+	{ .compatible = "mediatek,mt2701-disp-ovl",
+	  .data = &mt2701_ovl_driver_data},
 	{ .compatible = "mediatek,mt8173-disp-ovl",
 	  .data = &mt8173_ovl_driver_data},
 	{},
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
index b4225e2..5d363de 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
@@ -226,11 +226,17 @@ static int mtk_disp_rdma_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_ddp_comp_driver_data mt2701_rdma_driver_data = {
+	.rdma_fifo_pseudo_size = SZ_4K,
+};
+
 static const struct mtk_ddp_comp_driver_data mt8173_rdma_driver_data = {
 	.rdma_fifo_pseudo_size = SZ_8K,
 };
 
 static const struct of_device_id mtk_disp_rdma_driver_dt_match[] = {
+	{ .compatible = "mediatek,mt2701-disp-rdma",
+	  .data = &mt2701_rdma_driver_data},
 	{ .compatible = "mediatek,mt8173-disp-rdma",
 	  .data = &mt8173_rdma_driver_data},
 	{},
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index a9b209c..8130f3d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -60,6 +60,13 @@
 #define MT8173_MUTEX_MOD_DISP_PWM1		BIT(24)
 #define MT8173_MUTEX_MOD_DISP_OD		BIT(25)
 
+#define MT2701_MUTEX_MOD_DISP_OVL		BIT(3)
+#define MT2701_MUTEX_MOD_DISP_WDMA		BIT(6)
+#define MT2701_MUTEX_MOD_DISP_COLOR		BIT(7)
+#define MT2701_MUTEX_MOD_DISP_BLS		BIT(9)
+#define MT2701_MUTEX_MOD_DISP_RDMA0		BIT(10)
+#define MT2701_MUTEX_MOD_DISP_RDMA1		BIT(12)
+
 #define MUTEX_SOF_SINGLE_MODE		0
 #define MUTEX_SOF_DSI0			1
 #define MUTEX_SOF_DSI1			2
@@ -92,6 +99,15 @@ struct mtk_ddp {
 	const unsigned int		*mutex_mod;
 };
 
+static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
+	[DDP_COMPONENT_BLS] = MT2701_MUTEX_MOD_DISP_BLS,
+	[DDP_COMPONENT_COLOR0] = MT2701_MUTEX_MOD_DISP_COLOR,
+	[DDP_COMPONENT_OVL0] = MT2701_MUTEX_MOD_DISP_OVL,
+	[DDP_COMPONENT_RDMA0] = MT2701_MUTEX_MOD_DISP_RDMA0,
+	[DDP_COMPONENT_RDMA1] = MT2701_MUTEX_MOD_DISP_RDMA1,
+	[DDP_COMPONENT_WDMA0] = MT2701_MUTEX_MOD_DISP_WDMA,
+};
+
 static const unsigned int mt8173_mutex_mod[DDP_COMPONENT_ID_MAX] = {
 	[DDP_COMPONENT_AAL] = MT8173_MUTEX_MOD_DISP_AAL,
 	[DDP_COMPONENT_COLOR0] = MT8173_MUTEX_MOD_DISP_COLOR0,
@@ -390,6 +406,7 @@ static int mtk_ddp_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id ddp_driver_dt_match[] = {
+	{ .compatible = "mediatek,mt2701-disp-mutex", .data = mt2701_mutex_mod},
 	{ .compatible = "mediatek,mt8173-disp-mutex", .data = mt8173_mutex_mod},
 	{},
 };
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index b78b2e6..60d4783 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -265,11 +265,17 @@ struct mtk_ddp_comp_match {
 	[DDP_COMPONENT_WDMA1]	= { MTK_DISP_WDMA,	1, NULL },
 };
 
+static const struct mtk_ddp_comp_driver_data mt2701_color_driver_data = {
+	.color_offset = 0x0f00,
+};
+
 static const struct mtk_ddp_comp_driver_data mt8173_color_driver_data = {
 	.color_offset = 0x0c00,
 };
 
 static const struct of_device_id mtk_disp_color_driver_dt_match[] = {
+	{ .compatible = "mediatek,mt2701-disp-color",
+	  .data = &mt2701_color_driver_data},
 	{ .compatible = "mediatek,mt8173-disp-color",
 	  .data = &mt8173_color_driver_data},
 	{},
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 5f9b5e8..f5cb6f0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -126,6 +126,19 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	.atomic_commit = mtk_atomic_commit,
 };
 
+static const enum mtk_ddp_comp_id mt2701_mtk_ddp_main[] = {
+	DDP_COMPONENT_OVL0,
+	DDP_COMPONENT_RDMA0,
+	DDP_COMPONENT_COLOR0,
+	DDP_COMPONENT_BLS,
+	DDP_COMPONENT_DSI0,
+};
+
+static const enum mtk_ddp_comp_id mt2701_mtk_ddp_ext[] = {
+	DDP_COMPONENT_RDMA1,
+	DDP_COMPONENT_DPI0,
+};
+
 static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
 	DDP_COMPONENT_OVL0,
 	DDP_COMPONENT_COLOR0,
@@ -145,6 +158,14 @@ static int mtk_atomic_commit(struct drm_device *drm,
 	DDP_COMPONENT_DPI0,
 };
 
+static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
+	.main_path = mt2701_mtk_ddp_main,
+	.main_len = ARRAY_SIZE(mt2701_mtk_ddp_main),
+	.ext_path = mt2701_mtk_ddp_ext,
+	.ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext),
+	.shadow_register = true,
+};
+
 static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
 	.main_path = mt8173_mtk_ddp_main,
 	.main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
@@ -340,16 +361,22 @@ static void mtk_drm_unbind(struct device *dev)
 };
 
 static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
+	{ .compatible = "mediatek,mt2701-disp-ovl",   .data = (void *)MTK_DISP_OVL },
 	{ .compatible = "mediatek,mt8173-disp-ovl",   .data = (void *)MTK_DISP_OVL },
+	{ .compatible = "mediatek,mt2701-disp-rdma",  .data = (void *)MTK_DISP_RDMA },
 	{ .compatible = "mediatek,mt8173-disp-rdma",  .data = (void *)MTK_DISP_RDMA },
 	{ .compatible = "mediatek,mt8173-disp-wdma",  .data = (void *)MTK_DISP_WDMA },
+	{ .compatible = "mediatek,mt2701-disp-color", .data = (void *)MTK_DISP_COLOR },
 	{ .compatible = "mediatek,mt8173-disp-color", .data = (void *)MTK_DISP_COLOR },
 	{ .compatible = "mediatek,mt8173-disp-aal",   .data = (void *)MTK_DISP_AAL},
 	{ .compatible = "mediatek,mt8173-disp-gamma", .data = (void *)MTK_DISP_GAMMA, },
 	{ .compatible = "mediatek,mt8173-disp-ufoe",  .data = (void *)MTK_DISP_UFOE },
+	{ .compatible = "mediatek,mt2701-dsi",	      .data = (void *)MTK_DSI },
 	{ .compatible = "mediatek,mt8173-dsi",        .data = (void *)MTK_DSI },
 	{ .compatible = "mediatek,mt8173-dpi",        .data = (void *)MTK_DPI },
+	{ .compatible = "mediatek,mt2701-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
 	{ .compatible = "mediatek,mt8173-disp-mutex", .data = (void *)MTK_DISP_MUTEX },
+	{ .compatible = "mediatek,mt2701-disp-pwm",   .data = (void *)MTK_DISP_BLS },
 	{ .compatible = "mediatek,mt8173-disp-pwm",   .data = (void *)MTK_DISP_PWM },
 	{ .compatible = "mediatek,mt8173-disp-od",    .data = (void *)MTK_DISP_OD },
 	{ }
@@ -522,6 +549,8 @@ static SIMPLE_DEV_PM_OPS(mtk_drm_pm_ops, mtk_drm_sys_suspend,
 			 mtk_drm_sys_resume);
 
 static const struct of_device_id mtk_drm_of_ids[] = {
+	{ .compatible = "mediatek,mt2701-mmsys",
+	  .data = &mt2701_mmsys_driver_data},
 	{ .compatible = "mediatek,mt8173-mmsys",
 	  .data = &mt8173_mmsys_driver_data},
 	{ }
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 12a1206..fe9786c 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1204,6 +1204,7 @@ static int mtk_dsi_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id mtk_dsi_of_match[] = {
+	{ .compatible = "mediatek,mt2701-dsi" },
 	{ .compatible = "mediatek,mt8173-dsi" },
 	{ },
 };
diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
index 34e95c6..944fb1d 100644
--- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
+++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c
@@ -467,11 +467,17 @@ static int mtk_mipi_tx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct mtk_mipitx_data mt2701_mipitx_data = {
+	.data = (3 << 8)
+};
+
 static const struct mtk_mipitx_data mt8173_mipitx_data = {
 	.data = (0 << 8)
 };
 
 static const struct of_device_id mtk_mipi_tx_match[] = {
+	{ .compatible = "mediatek,mt2701-mipi-tx",
+	  .data = &mt2701_mipitx_data },
 	{ .compatible = "mediatek,mt8173-mipi-tx",
 	  .data = &mt8173_mipitx_data },
 	{},
-- 
1.9.1

Re: [PATCH v9 00/10] MT2701 DRM support

[CK Hu]

Hi, YT:

On Fri, 2016-11-11 at 19:55 +0800, YT Shen wrote:
> This is MT2701 DRM support PATCH v9, based on 4.9-rc1.
> We add DSI interrupt control, transfer function for MIPI DSI panel support.
> Most codes are the same, except some register changed.
> 
> For example:
>  - DISP_OVL address offset changed, color format definition changed.
>  - DISP_RDMA fifo size changed.
>  - DISP_COLOR offset changed.
>  - MIPI_TX setting changed.
> 
> We add a new component DDP_COMPONENT_BLS, and the connections are updated.
> OVL -> RDMA -> COLOR -> BLS -> DSI
> RDMA -> DPI
> And we have shadow register support in MT2701.
> 
> We remove dts patch from the patch series, which depends on MT2701 CCF and scpsys.

For this series, it looks good to me.
Acked-by: CK Hu <ck.hu@mediatek.com>

> 
> Changes since v8:
> - enable 3 DSI interrupts only
> - move mtk_dsi_wait_for_irq_done() to the patch of irq control
> - use the name BLS in DRM driver part
> - move BLS declaration to a separate patch
> - update mtk_dsi_switch_to_cmd_mode()
> - update mtk_output_dsi_enable() and mtk_output_dsi_disable()
> 
> Changes since v7:
> - Remove redundant codes
> - Move the definition of DDP_COMPONENT_BLS to patch of "drm/mediatek: update display module connections"
> - Move _dsi_irq_wait_queue into platform driver data
> - Move mtk_dsi_irq_data_clear() to patch of "drm/mediatek: add dsi interrupt control"
> - Add more descriptions in the commit messages
> 
> Changes since v6:
> - Change data type of irq_data to u32
> - Rewrite mtk_dsi_host_transfer() for simplify
> - Move some MIPI_TX config to patch of "drm/mediatek: add *driver_data for different hardware settings".
> - Remove device tree from this patch series
> 
> Changes since v5:
> - Remove DPI device tree and compatible string
> - Use one wait queue to handle interrupt status
> - Update the interrupt check flow and DSI_INT_ALL_BITS
> - Use same function for host read/write command
> - various fixes
> 
> Changes since v4:
> - Add messages when timeout in mtk_disp_mutex_acquire()
> - Add descriptions for DISP_REG_MUTEX registers
> - Move connection settings for display modules to a separate patch
> - Remove 'mt2701-disp-wdma' because it is unused
> - Move cleaning up and renaming to a separate patch
> - Use wait_event_interruptible_timeout() to replace polling
> - Remove irq_num from mtk_dsi structure
> - Remove redundant and debug codes
> 
> Changes since v3:
> - Add DSI support for MIPI DSI panels
> - Update BLS binding to PWM nodes
> - Remove ufoe device nodes
> - Remove redundant parentheses
> - Remove global variable initialization
> 
> Changes since v2:
> - Rename mtk_ddp_mux_sel to mtk_ddp_sout_sel
> - Update mt2701_mtk_ddp_ext components
> - Changed to prefix naming
> - Reorder the patch series
> - Use of_device_get_match_data() to get driver private data
> - Use iopoll macros to implement mtk_disp_mutex_acquire()
> - Removed empty device tree nodes
> 
> Changes since v1:
> - Removed BLS bindings and codes, which belong to pwm driver
> - Moved mtk_disp_mutex_acquire() just before mtk_crtc_ddp_config()
> - Split patch into smaller parts
> - Added const keyword to constant structure
> - Removed codes for special memory align
> 
> Thanks,
> yt.shen
> 
> YT Shen (8):
>   drm/mediatek: rename macros, add chip prefix
>   drm/mediatek: add *driver_data for different hardware settings
>   drm/mediatek: add shadow register support
>   drm/mediatek: add BLS component
>   drm/mediatek: update display module connections
>   drm/mediatek: cleaning up and refine
>   drm/mediatek: update DSI sub driver flow for sending commands to panel
>   drm/mediatek: add support for Mediatek SoC MT2701
> 
> shaoming chen (2):
>   drm/mediatek: add dsi interrupt control
>   drm/mediatek: add dsi transfer function
> 
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c     |  33 ++-
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c    |  17 +-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  76 +++--
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 138 ++++++---
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h      |   2 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  38 ++-
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  15 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |  54 +++-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h      |   9 +
>  drivers/gpu/drm/mediatek/mtk_dsi.c          | 429 ++++++++++++++++++++++++----
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.c      |  70 +++--
>  11 files changed, 715 insertions(+), 166 deletions(-)
> 

Re: [PATCH v9 00/10] MT2701 DRM support

[Bibby Hsieh]

Hi, YT

On Fri, 2016-11-11 at 19:55 +0800, YT Shen wrote:
> This is MT2701 DRM support PATCH v9, based on 4.9-rc1.
> We add DSI interrupt control, transfer function for MIPI DSI panel support.
> Most codes are the same, except some register changed.
> 
> For example:
>  - DISP_OVL address offset changed, color format definition changed.
>  - DISP_RDMA fifo size changed.
>  - DISP_COLOR offset changed.
>  - MIPI_TX setting changed.
> 
> We add a new component DDP_COMPONENT_BLS, and the connections are updated.
> OVL -> RDMA -> COLOR -> BLS -> DSI
> RDMA -> DPI
> And we have shadow register support in MT2701.
> 
> We remove dts patch from the patch series, which depends on MT2701 CCF and scpsys.
> 

I test this series on MT8173 platform, it looks pretty good to me,
thanks for your patches.

Tested-by: Bibby Hsieh <bibby.hsieh@mediatek.com>

> Changes since v8:
> - enable 3 DSI interrupts only
> - move mtk_dsi_wait_for_irq_done() to the patch of irq control
> - use the name BLS in DRM driver part
> - move BLS declaration to a separate patch
> - update mtk_dsi_switch_to_cmd_mode()
> - update mtk_output_dsi_enable() and mtk_output_dsi_disable()
> 
> Changes since v7:
> - Remove redundant codes
> - Move the definition of DDP_COMPONENT_BLS to patch of "drm/mediatek: update display module connections"
> - Move _dsi_irq_wait_queue into platform driver data
> - Move mtk_dsi_irq_data_clear() to patch of "drm/mediatek: add dsi interrupt control"
> - Add more descriptions in the commit messages
> 
> Changes since v6:
> - Change data type of irq_data to u32
> - Rewrite mtk_dsi_host_transfer() for simplify
> - Move some MIPI_TX config to patch of "drm/mediatek: add *driver_data for different hardware settings".
> - Remove device tree from this patch series
> 
> Changes since v5:
> - Remove DPI device tree and compatible string
> - Use one wait queue to handle interrupt status
> - Update the interrupt check flow and DSI_INT_ALL_BITS
> - Use same function for host read/write command
> - various fixes
> 
> Changes since v4:
> - Add messages when timeout in mtk_disp_mutex_acquire()
> - Add descriptions for DISP_REG_MUTEX registers
> - Move connection settings for display modules to a separate patch
> - Remove 'mt2701-disp-wdma' because it is unused
> - Move cleaning up and renaming to a separate patch
> - Use wait_event_interruptible_timeout() to replace polling
> - Remove irq_num from mtk_dsi structure
> - Remove redundant and debug codes
> 
> Changes since v3:
> - Add DSI support for MIPI DSI panels
> - Update BLS binding to PWM nodes
> - Remove ufoe device nodes
> - Remove redundant parentheses
> - Remove global variable initialization
> 
> Changes since v2:
> - Rename mtk_ddp_mux_sel to mtk_ddp_sout_sel
> - Update mt2701_mtk_ddp_ext components
> - Changed to prefix naming
> - Reorder the patch series
> - Use of_device_get_match_data() to get driver private data
> - Use iopoll macros to implement mtk_disp_mutex_acquire()
> - Removed empty device tree nodes
> 
> Changes since v1:
> - Removed BLS bindings and codes, which belong to pwm driver
> - Moved mtk_disp_mutex_acquire() just before mtk_crtc_ddp_config()
> - Split patch into smaller parts
> - Added const keyword to constant structure
> - Removed codes for special memory align
> 
> Thanks,
> yt.shen
> 
> YT Shen (8):
>   drm/mediatek: rename macros, add chip prefix
>   drm/mediatek: add *driver_data for different hardware settings
>   drm/mediatek: add shadow register support
>   drm/mediatek: add BLS component
>   drm/mediatek: update display module connections
>   drm/mediatek: cleaning up and refine
>   drm/mediatek: update DSI sub driver flow for sending commands to panel
>   drm/mediatek: add support for Mediatek SoC MT2701
> 
> shaoming chen (2):
>   drm/mediatek: add dsi interrupt control
>   drm/mediatek: add dsi transfer function
> 
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c     |  33 ++-
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c    |  17 +-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c     |  76 +++--
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c      | 138 ++++++---
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h      |   2 +
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |  38 ++-
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  15 +
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c      |  54 +++-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h      |   9 +
>  drivers/gpu/drm/mediatek/mtk_dsi.c          | 429 ++++++++++++++++++++++++----
>  drivers/gpu/drm/mediatek/mtk_mipi_tx.c      |  70 +++--
>  11 files changed, 715 insertions(+), 166 deletions(-)
> 

-- 
Bibby