[PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Enric Balletbo i Serra]

Dear all,

Those patches are intended to solve an old standing issue on some
Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
to the precedent series.

Up to now both drivers, clock and drm are probed with the same device tree
compatible. But only the first driver get probed, which in effect breaks
graphics on those devices.

The version eight of the series tries to solve the problem with a
different approach than the previous series but similar to how is solved
on other Mediatek devices.

The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
control clock gates (which is used in the clk driver) and some registers
to set the routing and enable the differnet blocks of the display
and MDP (Media Data Path) subsystem. On this series the clk driver is
not a pure clock controller but a system controller that can provide
access to the shared registers between the different drivers that need
it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
this version, clk driver is the entry point (parent) which will trigger
the probe of the corresponding mediatek-drm driver and pass its MMSYS
platform data for display configuration.

All this series was tested on the Acer R13 Chromebook only.

For reference, here are the links to the old discussions:

* v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
* v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
* v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
* v4:
  * https://patchwork.kernel.org/patch/10530871/
  * https://patchwork.kernel.org/patch/10530883/
  * https://patchwork.kernel.org/patch/10530885/
  * https://patchwork.kernel.org/patch/10530911/
  * https://patchwork.kernel.org/patch/10530913/
* v3:
  * https://patchwork.kernel.org/patch/10367857/
  * https://patchwork.kernel.org/patch/10367861/
  * https://patchwork.kernel.org/patch/10367877/
  * https://patchwork.kernel.org/patch/10367875/
  * https://patchwork.kernel.org/patch/10367885/
  * https://patchwork.kernel.org/patch/10367883/
  * https://patchwork.kernel.org/patch/10367889/
  * https://patchwork.kernel.org/patch/10367907/
  * https://patchwork.kernel.org/patch/10367909/
  * https://patchwork.kernel.org/patch/10367905/
* v2: No relevant discussion, see v3
* v1:
  * https://patchwork.kernel.org/patch/10016497/
  * https://patchwork.kernel.org/patch/10016499/
  * https://patchwork.kernel.org/patch/10016505/
  * https://patchwork.kernel.org/patch/10016507/

Best regards,
 Enric

Changes in v8:
- Be a builtin_platform_driver like other mediatek mmsys drivers.
- New patches introduced in this series.

Changes in v7:
- Add R-by from CK
- Add R-by from CK
- Fix check of return value of of_clk_get
- Fix identation
- Free clk_data->clks as well
- Get rid of private data structure

Enric Balletbo i Serra (2):
  drm/mediatek: Move MMSYS configuration to include/linux/platform_data
  clk/drm: mediatek: Fix mediatek-drm device probing

Matthias Brugger (4):
  drm/mediatek: Use regmap for register access
  drm/mediatek: Omit warning on probe defers
  media: mtk-mdp: Check return value of of_clk_get
  clk: mediatek: mt8173: Switch MMSYS to platform driver

 drivers/clk/mediatek/Kconfig                  |   6 +
 drivers/clk/mediatek/Makefile                 |   1 +
 drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
 drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
 drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
 drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
 drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
 drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
 drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
 drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
 drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
 drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
 drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
 include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
 20 files changed, 401 insertions(+), 317 deletions(-)
 create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
 create mode 100644 include/linux/platform_data/mtk_mmsys.h

-- 
2.25.0

[PATCH v8 1/6] drm/mediatek: Use regmap for register access

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

The mmsys memory space is shared between the drm and the
clk driver. Use regmap to access it.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v8: None
Changes in v7:
- Add R-by from CK

 drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  4 +-
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 50 +++++++++++--------------
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h  |  4 +-
 drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 13 ++-----
 drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  2 +-
 5 files changed, 30 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 5ee74d7ce35c..a236499123aa 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -28,7 +28,7 @@
  * @enabled: records whether crtc_enable succeeded
  * @planes: array of 4 drm_plane structures, one for each overlay plane
  * @pending_planes: whether any plane has pending changes to be applied
- * @config_regs: memory mapped mmsys configuration register space
+ * @config_regs: regmap mapped mmsys configuration register space
  * @mutex: handle to one of the ten disp_mutex streams
  * @ddp_comp_nr: number of components in ddp_comp
  * @ddp_comp: array of pointers the mtk_ddp_comp structures used by this crtc
@@ -50,7 +50,7 @@ struct mtk_drm_crtc {
 	u32				cmdq_event;
 #endif
 
-	void __iomem			*config_regs;
+	struct regmap			*config_regs;
 	struct mtk_disp_mutex		*mutex;
 	unsigned int			ddp_comp_nr;
 	struct mtk_ddp_comp		**ddp_comp;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 13035c906035..302753744cc6 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -383,61 +383,53 @@ 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,
+static void mtk_ddp_sout_sel(struct regmap *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);
+		regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
+				BLS_TO_DSI_RDMA1_TO_DPI1);
 	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DPI0) {
-		writel_relaxed(BLS_TO_DPI_RDMA1_TO_DSI,
-			       config_regs + DISP_REG_CONFIG_OUT_SEL);
-		writel_relaxed(DSI_SEL_IN_RDMA,
-			       config_regs + DISP_REG_CONFIG_DSI_SEL);
-		writel_relaxed(DPI_SEL_IN_BLS,
-			       config_regs + DISP_REG_CONFIG_DPI_SEL);
+		regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
+				BLS_TO_DPI_RDMA1_TO_DSI);
+		regmap_write(config_regs, DISP_REG_CONFIG_DSI_SEL,
+				DSI_SEL_IN_RDMA);
+		regmap_write(config_regs, DISP_REG_CONFIG_DPI_SEL,
+				DPI_SEL_IN_BLS);
 	}
 }
 
-void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
+void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
 			      enum mtk_ddp_comp_id cur,
 			      enum mtk_ddp_comp_id next)
 {
-	unsigned int addr, value, reg;
+	unsigned int addr, value;
 
 	value = mtk_ddp_mout_en(cur, next, &addr);
-	if (value) {
-		reg = readl_relaxed(config_regs + addr) | value;
-		writel_relaxed(reg, config_regs + addr);
-	}
+	if (value)
+		regmap_update_bits(config_regs, addr, value, value);
 
 	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;
-		writel_relaxed(reg, config_regs + addr);
-	}
+	if (value)
+		regmap_update_bits(config_regs, addr, value, value);
 }
 
-void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
+void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
 				   enum mtk_ddp_comp_id cur,
 				   enum mtk_ddp_comp_id next)
 {
-	unsigned int addr, value, reg;
+	unsigned int addr, value;
 
 	value = mtk_ddp_mout_en(cur, next, &addr);
-	if (value) {
-		reg = readl_relaxed(config_regs + addr) & ~value;
-		writel_relaxed(reg, config_regs + addr);
-	}
+	if (value)
+		regmap_update_bits(config_regs, addr, value, 0);
 
 	value = mtk_ddp_sel_in(cur, next, &addr);
-	if (value) {
-		reg = readl_relaxed(config_regs + addr) & ~value;
-		writel_relaxed(reg, config_regs + addr);
-	}
+	if (value)
+		regmap_update_bits(config_regs, addr, value, 0);
 }
 
 struct mtk_disp_mutex *mtk_disp_mutex_get(struct device *dev, unsigned int id)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
index 827be424a148..01ff8b68881f 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
@@ -12,10 +12,10 @@ struct regmap;
 struct device;
 struct mtk_disp_mutex;
 
-void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
+void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
 			      enum mtk_ddp_comp_id cur,
 			      enum mtk_ddp_comp_id next);
-void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
+void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
 				   enum mtk_ddp_comp_id cur,
 				   enum mtk_ddp_comp_id next);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 0563c6813333..b68837ea02b3 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -6,6 +6,7 @@
 
 #include <linux/component.h>
 #include <linux/iommu.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
@@ -425,7 +426,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct mtk_drm_private *private;
-	struct resource *mem;
 	struct device_node *node;
 	struct component_match *match = NULL;
 	int ret;
@@ -437,14 +437,9 @@ static int mtk_drm_probe(struct platform_device *pdev)
 
 	private->data = of_device_get_match_data(dev);
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	private->config_regs = devm_ioremap_resource(dev, mem);
-	if (IS_ERR(private->config_regs)) {
-		ret = PTR_ERR(private->config_regs);
-		dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
-			ret);
-		return ret;
-	}
+	private->config_regs = syscon_node_to_regmap(dev->of_node);
+	if (IS_ERR(private->config_regs))
+		return PTR_ERR(private->config_regs);
 
 	/* Iterate over sibling DISP function blocks */
 	for_each_child_of_node(dev->of_node->parent, node) {
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 17bc99b9f5d4..03201080688d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -39,7 +39,7 @@ struct mtk_drm_private {
 
 	struct device_node *mutex_node;
 	struct device *mutex_dev;
-	void __iomem *config_regs;
+	struct regmap *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;
-- 
2.25.0

Re: [PATCH v8 1/6] drm/mediatek: Use regmap for register access

[Randy Dunlap]

On 2/20/20 9:21 AM, Enric Balletbo i Serra wrote:
> From: Matthias Brugger <mbrugger@suse.com>
> 
> The mmsys memory space is shared between the drm and the
> clk driver. Use regmap to access it.
> 
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
> Changes in v8: None
> Changes in v7:
> - Add R-by from CK
> 
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  4 +-
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 50 +++++++++++--------------
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h  |  4 +-
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 13 ++-----
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  2 +-
>  5 files changed, 30 insertions(+), 43 deletions(-)

Hi. Just a quick question:

Do you need to select REGMAP or one of its derivatives to make sure
that the proper interfaces are available for this driver?

thanks.
-- 
~Randy

Re: [PATCH v8 1/6] drm/mediatek: Use regmap for register access

[Enric Balletbo i Serra]

Hi Randy,

On 21/2/20 0:48, Randy Dunlap wrote:
> On 2/20/20 9:21 AM, Enric Balletbo i Serra wrote:
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> The mmsys memory space is shared between the drm and the
>> clk driver. Use regmap to access it.
>>
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Reviewed-by: CK Hu <ck.hu@mediatek.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>> Changes in v8: None
>> Changes in v7:
>> - Add R-by from CK
>>
>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  4 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 50 +++++++++++--------------
>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h  |  4 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 13 ++-----
>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  2 +-
>>  5 files changed, 30 insertions(+), 43 deletions(-)
> 
> Hi. Just a quick question:
> 
> Do you need to select REGMAP or one of its derivatives to make sure
> that the proper interfaces are available for this driver?
> 

Right, I will fix this in next version.

> thanks.
> 

[PATCH v8 2/6] drm/mediatek: Omit warning on probe defers

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

It can happen that the mmsys clock drivers aren't probed before the
platform driver gets invoked. The platform driver used to print a warning
that the driver failed to get the clocks. Omit this error on
the defered probe path.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v8: None
Changes in v7:
- Add Rv-by from CK

 drivers/gpu/drm/mediatek/mtk_disp_color.c |  5 ++++-
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c   |  5 ++++-
 drivers/gpu/drm/mediatek/mtk_disp_rdma.c  |  5 ++++-
 drivers/gpu/drm/mediatek/mtk_dpi.c        | 12 +++++++++---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c    |  3 ++-
 drivers/gpu/drm/mediatek/mtk_dsi.c        |  8 ++++++--
 drivers/gpu/drm/mediatek/mtk_hdmi.c       |  4 +++-
 7 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c
index 6fb0d6983a4a..3ae9c810845b 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_color.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c
@@ -119,7 +119,10 @@ static int mtk_disp_color_probe(struct platform_device *pdev)
 	ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
 				&mtk_disp_color_funcs);
 	if (ret) {
-		dev_err(dev, "Failed to initialize component: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to initialize component: %d\n",
+				ret);
+
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 891d80c73e04..28651bc579bc 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -386,7 +386,10 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev)
 	ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
 				&mtk_disp_ovl_funcs);
 	if (ret) {
-		dev_err(dev, "Failed to initialize component: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to initialize component: %d\n",
+				ret);
+
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
index 0cb848d64206..e04319fedf46 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
@@ -294,7 +294,10 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev)
 	ret = mtk_ddp_comp_init(dev, dev->of_node, &priv->ddp_comp, comp_id,
 				&mtk_disp_rdma_funcs);
 	if (ret) {
-		dev_err(dev, "Failed to initialize component: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to initialize component: %d\n",
+				ret);
+
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 01fa8b8d763d..1b219edef541 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -701,21 +701,27 @@ static int mtk_dpi_probe(struct platform_device *pdev)
 	dpi->engine_clk = devm_clk_get(dev, "engine");
 	if (IS_ERR(dpi->engine_clk)) {
 		ret = PTR_ERR(dpi->engine_clk);
-		dev_err(dev, "Failed to get engine clock: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get engine clock: %d\n", ret);
+
 		return ret;
 	}
 
 	dpi->pixel_clk = devm_clk_get(dev, "pixel");
 	if (IS_ERR(dpi->pixel_clk)) {
 		ret = PTR_ERR(dpi->pixel_clk);
-		dev_err(dev, "Failed to get pixel clock: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get pixel clock: %d\n", ret);
+
 		return ret;
 	}
 
 	dpi->tvd_clk = devm_clk_get(dev, "pll");
 	if (IS_ERR(dpi->tvd_clk)) {
 		ret = PTR_ERR(dpi->tvd_clk);
-		dev_err(dev, "Failed to get tvdpll clock: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get tvdpll clock: %d\n", ret);
+
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 302753744cc6..39700b9428b9 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -620,7 +620,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
 	if (!ddp->data->no_clk) {
 		ddp->clk = devm_clk_get(dev, NULL);
 		if (IS_ERR(ddp->clk)) {
-			dev_err(dev, "Failed to get clock\n");
+			if (PTR_ERR(ddp->clk) != -EPROBE_DEFER)
+				dev_err(dev, "Failed to get clock\n");
 			return PTR_ERR(ddp->clk);
 		}
 	}
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 5fa1073cf26b..a45ed0270531 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1194,14 +1194,18 @@ static int mtk_dsi_probe(struct platform_device *pdev)
 	dsi->engine_clk = devm_clk_get(dev, "engine");
 	if (IS_ERR(dsi->engine_clk)) {
 		ret = PTR_ERR(dsi->engine_clk);
-		dev_err(dev, "Failed to get engine clock: %d\n", ret);
+
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get engine clock: %d\n", ret);
 		goto err_unregister_host;
 	}
 
 	dsi->digital_clk = devm_clk_get(dev, "digital");
 	if (IS_ERR(dsi->digital_clk)) {
 		ret = PTR_ERR(dsi->digital_clk);
-		dev_err(dev, "Failed to get digital clock: %d\n", ret);
+
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get digital clock: %d\n", ret);
 		goto err_unregister_host;
 	}
 
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 5e4a4dbda443..69c6a146c561 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1451,7 +1451,9 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
 
 	ret = mtk_hdmi_get_all_clk(hdmi, np);
 	if (ret) {
-		dev_err(dev, "Failed to get clocks: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get clocks: %d\n", ret);
+
 		return ret;
 	}
 
-- 
2.25.0

[PATCH v8 3/6] media: mtk-mdp: Check return value of of_clk_get

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

Check the return value of of_clk_get and print an error
message if not EPROBE_DEFER.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v8: None
Changes in v7:
- Fix check of return value of of_clk_get
- Fix identation

 drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
index 0c4788af78dd..58abfbdfb82d 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
@@ -110,6 +110,12 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node,
 
 	for (i = 0; i < ARRAY_SIZE(comp->clk); i++) {
 		comp->clk[i] = of_clk_get(node, i);
+		if (IS_ERR(comp->clk[i])) {
+			if (PTR_ERR(comp->clk[i]) != -EPROBE_DEFER)
+				dev_err(dev, "Failed to get clock\n");
+
+			return PTR_ERR(comp->clk[i]);
+		}
 
 		/* Only RDMA needs two clocks */
 		if (comp->type != MTK_MDP_RDMA)
-- 
2.25.0

[PATCH v8 4/6] clk: mediatek: mt8173: Switch MMSYS to platform driver

[Enric Balletbo i Serra]

From: Matthias Brugger <mbrugger@suse.com>

There is no strong reason for this to use CLK_OF_DECLARE instead of
being a platform driver. Plus, this driver provides clocks but also
a shared register space for the mediatek-drm and the mediatek-mdp
driver. So move to a platform driver. This will now be aligned with
other Mediatek MMSYS drivers.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v8:
- Be a builtin_platform_driver like other mediatek mmsys drivers.

Changes in v7:
- Free clk_data->clks as well
- Get rid of private data structure

 drivers/clk/mediatek/Kconfig         |   6 ++
 drivers/clk/mediatek/Makefile        |   1 +
 drivers/clk/mediatek/clk-mt8173-mm.c | 137 +++++++++++++++++++++++++++
 drivers/clk/mediatek/clk-mt8173.c    | 104 --------------------
 4 files changed, 144 insertions(+), 104 deletions(-)
 create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c

diff --git a/drivers/clk/mediatek/Kconfig b/drivers/clk/mediatek/Kconfig
index ea3c70d1307e..9a6548c552d3 100644
--- a/drivers/clk/mediatek/Kconfig
+++ b/drivers/clk/mediatek/Kconfig
@@ -274,6 +274,12 @@ config COMMON_CLK_MT8173
 	---help---
 	  This driver supports MediaTek MT8173 clocks.
 
+config COMMON_CLK_MT8173_MMSYS
+	bool "Clock driver for MediaTek MT8173 mmsys"
+	depends on COMMON_CLK_MT8173
+	help
+	  This driver supports MediaTek MT8173 mmsys clocks.
+
 config COMMON_CLK_MT8183
 	bool "Clock driver for MediaTek MT8183"
 	depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
index 8cdb76a5cd71..bb0536942075 100644
--- a/drivers/clk/mediatek/Makefile
+++ b/drivers/clk/mediatek/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_COMMON_CLK_MT7629_ETHSYS) += clk-mt7629-eth.o
 obj-$(CONFIG_COMMON_CLK_MT7629_HIFSYS) += clk-mt7629-hif.o
 obj-$(CONFIG_COMMON_CLK_MT8135) += clk-mt8135.o
 obj-$(CONFIG_COMMON_CLK_MT8173) += clk-mt8173.o
+obj-$(CONFIG_COMMON_CLK_MT8173_MMSYS) += clk-mt8173-mm.o
 obj-$(CONFIG_COMMON_CLK_MT8183) += clk-mt8183.o
 obj-$(CONFIG_COMMON_CLK_MT8183_AUDIOSYS) += clk-mt8183-audio.o
 obj-$(CONFIG_COMMON_CLK_MT8183_CAMSYS) += clk-mt8183-cam.o
diff --git a/drivers/clk/mediatek/clk-mt8173-mm.c b/drivers/clk/mediatek/clk-mt8173-mm.c
new file mode 100644
index 000000000000..83884fd5a750
--- /dev/null
+++ b/drivers/clk/mediatek/clk-mt8173-mm.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ * Author: James Liao <jamesjj.liao@mediatek.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+
+#include "clk-mtk.h"
+#include "clk-gate.h"
+
+#include <dt-bindings/clock/mt8173-clk.h>
+
+static const struct mtk_gate_regs mm0_cg_regs = {
+	.set_ofs = 0x0104,
+	.clr_ofs = 0x0108,
+	.sta_ofs = 0x0100,
+};
+
+static const struct mtk_gate_regs mm1_cg_regs = {
+	.set_ofs = 0x0114,
+	.clr_ofs = 0x0118,
+	.sta_ofs = 0x0110,
+};
+
+#define GATE_MM0(_id, _name, _parent, _shift) {			\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &mm0_cg_regs,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr,		\
+	}
+
+#define GATE_MM1(_id, _name, _parent, _shift) {			\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &mm1_cg_regs,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr,		\
+	}
+
+static const struct mtk_gate mm_clks[] = {
+	/* MM0 */
+	GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
+	GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
+	GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
+	GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
+	GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
+	GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
+	GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
+	GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
+	GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
+	GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
+	GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
+	GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
+	GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
+	GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
+	GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
+	GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
+	GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
+	GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
+	GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
+	GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
+	GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
+	GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
+	GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
+	GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
+	GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
+	GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
+	GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
+	GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
+	GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
+	GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
+	GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
+	/* MM1 */
+	GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
+	GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
+	GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
+	GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
+	GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
+	GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "dsi0_dig", 5),
+	GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
+	GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "dsi1_dig", 7),
+	GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
+	GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
+	GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "lvds_pxl", 10),
+	GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
+	GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
+	GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
+	GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
+	GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
+	GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "lvds_pxl", 16),
+	GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "lvds_cts", 17),
+	GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
+	GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
+	GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
+};
+
+static int clk_mt8173_mm_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct clk_onecell_data *clk_data;
+	int ret;
+
+	clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
+	if (!clk_data)
+		return -ENOMEM;
+
+	ret = mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
+				     clk_data);
+	if (ret)
+		return ret;
+
+	ret = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id of_match_clk_mt8173_mm[] = {
+	{ .compatible = "mediatek,mt8173-mmsys", },
+	{ }
+};
+
+static struct platform_driver clk_mt8173_mm_drv = {
+	.driver = {
+		.name = "clk-mt8173-mm",
+		.of_match_table = of_match_clk_mt8173_mm,
+	},
+	.probe = clk_mt8173_mm_probe,
+};
+
+builtin_platform_driver(clk_mt8173_mm_drv);
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 537a7f49b0f7..8f898ac476c0 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -753,93 +753,6 @@ static const struct mtk_gate img_clks[] __initconst = {
 	GATE_IMG(CLK_IMG_FD, "img_fd", "mm_sel", 11),
 };
 
-static const struct mtk_gate_regs mm0_cg_regs __initconst = {
-	.set_ofs = 0x0104,
-	.clr_ofs = 0x0108,
-	.sta_ofs = 0x0100,
-};
-
-static const struct mtk_gate_regs mm1_cg_regs __initconst = {
-	.set_ofs = 0x0114,
-	.clr_ofs = 0x0118,
-	.sta_ofs = 0x0110,
-};
-
-#define GATE_MM0(_id, _name, _parent, _shift) {			\
-		.id = _id,					\
-		.name = _name,					\
-		.parent_name = _parent,				\
-		.regs = &mm0_cg_regs,				\
-		.shift = _shift,				\
-		.ops = &mtk_clk_gate_ops_setclr,		\
-	}
-
-#define GATE_MM1(_id, _name, _parent, _shift) {			\
-		.id = _id,					\
-		.name = _name,					\
-		.parent_name = _parent,				\
-		.regs = &mm1_cg_regs,				\
-		.shift = _shift,				\
-		.ops = &mtk_clk_gate_ops_setclr,		\
-	}
-
-static const struct mtk_gate mm_clks[] __initconst = {
-	/* MM0 */
-	GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
-	GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
-	GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
-	GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
-	GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
-	GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
-	GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
-	GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
-	GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
-	GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
-	GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
-	GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
-	GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
-	GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
-	GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
-	GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
-	GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
-	GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
-	GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
-	GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
-	GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
-	GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
-	GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
-	GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
-	GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
-	GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
-	GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
-	GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
-	GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
-	GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
-	GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
-	/* MM1 */
-	GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
-	GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
-	GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
-	GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
-	GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
-	GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "dsi0_dig", 5),
-	GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
-	GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "dsi1_dig", 7),
-	GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
-	GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
-	GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "lvds_pxl", 10),
-	GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
-	GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
-	GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
-	GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
-	GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
-	GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "lvds_pxl", 16),
-	GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "lvds_cts", 17),
-	GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
-	GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
-	GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
-};
-
 static const struct mtk_gate_regs vdec0_cg_regs __initconst = {
 	.set_ofs = 0x0000,
 	.clr_ofs = 0x0004,
@@ -1144,23 +1057,6 @@ static void __init mtk_imgsys_init(struct device_node *node)
 }
 CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init);
 
-static void __init mtk_mmsys_init(struct device_node *node)
-{
-	struct clk_onecell_data *clk_data;
-	int r;
-
-	clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
-
-	mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
-						clk_data);
-
-	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
-	if (r)
-		pr_err("%s(): could not register clock provider: %d\n",
-			__func__, r);
-}
-CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init);
-
 static void __init mtk_vdecsys_init(struct device_node *node)
 {
 	struct clk_onecell_data *clk_data;
-- 
2.25.0

[PATCH v8 5/6] drm/mediatek: Move MMSYS configuration to include/linux/platform_data

[Enric Balletbo i Serra]

The MMSYS platform data and DDP enums will be needed to compile other
drivers, so move it to a global location.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v8:
- New patch introduced in this series.

Changes in v7: None

 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 56 +---------------
 drivers/gpu/drm/mediatek/mtk_drm_drv.h      | 11 ----
 include/linux/platform_data/mtk_mmsys.h     | 73 +++++++++++++++++++++
 3 files changed, 75 insertions(+), 65 deletions(-)
 create mode 100644 include/linux/platform_data/mtk_mmsys.h

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index debe36395fe7..dc45edbec3bd 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -6,6 +6,7 @@
 #ifndef MTK_DRM_DDP_COMP_H
 #define MTK_DRM_DDP_COMP_H
 
+#include <linux/platform_data/mtk_mmsys.h>
 #include <linux/io.h>
 
 struct device;
@@ -14,62 +15,9 @@ struct drm_crtc;
 struct drm_device;
 struct mtk_plane_state;
 struct drm_crtc_state;
-
-enum mtk_ddp_comp_type {
-	MTK_DISP_OVL,
-	MTK_DISP_OVL_2L,
-	MTK_DISP_RDMA,
-	MTK_DISP_WDMA,
-	MTK_DISP_COLOR,
-	MTK_DISP_CCORR,
-	MTK_DISP_DITHER,
-	MTK_DISP_AAL,
-	MTK_DISP_GAMMA,
-	MTK_DISP_UFOE,
-	MTK_DSI,
-	MTK_DPI,
-	MTK_DISP_PWM,
-	MTK_DISP_MUTEX,
-	MTK_DISP_OD,
-	MTK_DISP_BLS,
-	MTK_DDP_COMP_TYPE_MAX,
-};
-
-enum mtk_ddp_comp_id {
-	DDP_COMPONENT_AAL0,
-	DDP_COMPONENT_AAL1,
-	DDP_COMPONENT_BLS,
-	DDP_COMPONENT_CCORR,
-	DDP_COMPONENT_COLOR0,
-	DDP_COMPONENT_COLOR1,
-	DDP_COMPONENT_DITHER,
-	DDP_COMPONENT_DPI0,
-	DDP_COMPONENT_DPI1,
-	DDP_COMPONENT_DSI0,
-	DDP_COMPONENT_DSI1,
-	DDP_COMPONENT_DSI2,
-	DDP_COMPONENT_DSI3,
-	DDP_COMPONENT_GAMMA,
-	DDP_COMPONENT_OD0,
-	DDP_COMPONENT_OD1,
-	DDP_COMPONENT_OVL0,
-	DDP_COMPONENT_OVL_2L0,
-	DDP_COMPONENT_OVL_2L1,
-	DDP_COMPONENT_OVL1,
-	DDP_COMPONENT_PWM0,
-	DDP_COMPONENT_PWM1,
-	DDP_COMPONENT_PWM2,
-	DDP_COMPONENT_RDMA0,
-	DDP_COMPONENT_RDMA1,
-	DDP_COMPONENT_RDMA2,
-	DDP_COMPONENT_UFOE,
-	DDP_COMPONENT_WDMA0,
-	DDP_COMPONENT_WDMA1,
-	DDP_COMPONENT_ID_MAX,
-};
-
 struct mtk_ddp_comp;
 struct cmdq_pkt;
+
 struct mtk_ddp_comp_funcs {
 	void (*config)(struct mtk_ddp_comp *comp, unsigned int w,
 		       unsigned int h, unsigned int vrefresh,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 03201080688d..317a2a26ad1d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -20,17 +20,6 @@ struct drm_fb_helper;
 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;
-	const enum mtk_ddp_comp_id *third_path;
-	unsigned int third_len;
-
-	bool shadow_register;
-};
-
 struct mtk_drm_private {
 	struct drm_device *drm;
 	struct device *dma_dev;
diff --git a/include/linux/platform_data/mtk_mmsys.h b/include/linux/platform_data/mtk_mmsys.h
new file mode 100644
index 000000000000..292f8e72d94f
--- /dev/null
+++ b/include/linux/platform_data/mtk_mmsys.h
@@ -0,0 +1,73 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ */
+
+#ifndef __LINUX_MTK_MMSYS_H
+#define __LINUX_MTK_MMSYS_H
+
+enum mtk_ddp_comp_type {
+	MTK_DISP_OVL,
+	MTK_DISP_OVL_2L,
+	MTK_DISP_RDMA,
+	MTK_DISP_WDMA,
+	MTK_DISP_COLOR,
+	MTK_DISP_CCORR,
+	MTK_DISP_DITHER,
+	MTK_DISP_AAL,
+	MTK_DISP_GAMMA,
+	MTK_DISP_UFOE,
+	MTK_DSI,
+	MTK_DPI,
+	MTK_DISP_PWM,
+	MTK_DISP_MUTEX,
+	MTK_DISP_OD,
+	MTK_DISP_BLS,
+	MTK_DDP_COMP_TYPE_MAX,
+};
+
+enum mtk_ddp_comp_id {
+	DDP_COMPONENT_AAL0,
+	DDP_COMPONENT_AAL1,
+	DDP_COMPONENT_BLS,
+	DDP_COMPONENT_CCORR,
+	DDP_COMPONENT_COLOR0,
+	DDP_COMPONENT_COLOR1,
+	DDP_COMPONENT_DITHER,
+	DDP_COMPONENT_DPI0,
+	DDP_COMPONENT_DPI1,
+	DDP_COMPONENT_DSI0,
+	DDP_COMPONENT_DSI1,
+	DDP_COMPONENT_DSI2,
+	DDP_COMPONENT_DSI3,
+	DDP_COMPONENT_GAMMA,
+	DDP_COMPONENT_OD0,
+	DDP_COMPONENT_OD1,
+	DDP_COMPONENT_OVL0,
+	DDP_COMPONENT_OVL_2L0,
+	DDP_COMPONENT_OVL_2L1,
+	DDP_COMPONENT_OVL1,
+	DDP_COMPONENT_PWM0,
+	DDP_COMPONENT_PWM1,
+	DDP_COMPONENT_PWM2,
+	DDP_COMPONENT_RDMA0,
+	DDP_COMPONENT_RDMA1,
+	DDP_COMPONENT_RDMA2,
+	DDP_COMPONENT_UFOE,
+	DDP_COMPONENT_WDMA0,
+	DDP_COMPONENT_WDMA1,
+	DDP_COMPONENT_ID_MAX,
+};
+
+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;
+	const enum mtk_ddp_comp_id *third_path;
+	unsigned int third_len;
+
+	bool shadow_register;
+};
+
+#endif /* __LINUX_MTK_MMSYS_H */
-- 
2.25.0

[PATCH v8 6/6] clk/drm: mediatek: Fix mediatek-drm device probing

[Enric Balletbo i Serra]

In the actual implementation the same compatible string
"mediatek,<chip>-mmsys" is used to bind the clock drivers
(drivers/clk/mediatek) as well as to the gpu driver
(drivers/gpu/drm/mediatek/mtk_drm_drv.c). This ends with the problem
that the only probed driver is the clock driver and there is no display
at all.

In any case having the same compatible string for two drivers is not
correct and should be fixed. To fix this, and maintain backward
compatibility, we can consider that the clk-<chip>-mm driver is the
top-level entry point for the MMSYS subsystem, so is not a pure clock
controller but a system controller, and the drm driver is instantiated
by that MMSYS driver.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes in v8:
- New patch introduced in this series.

Changes in v7: None

 drivers/clk/mediatek/clk-mt2701-mm.c   |  30 ++++++++
 drivers/clk/mediatek/clk-mt2712-mm.c   |  44 +++++++++++
 drivers/clk/mediatek/clk-mt8173-mm.c   |  35 +++++++++
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 102 ++-----------------------
 4 files changed, 115 insertions(+), 96 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt2701-mm.c b/drivers/clk/mediatek/clk-mt2701-mm.c
index 054b597d4a73..b1281680d5bf 100644
--- a/drivers/clk/mediatek/clk-mt2701-mm.c
+++ b/drivers/clk/mediatek/clk-mt2701-mm.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/platform_data/mtk_mmsys.h>
 #include <linux/platform_device.h>
 
 #include "clk-mtk.h"
@@ -79,6 +80,27 @@ static const struct mtk_gate mm_clks[] = {
 	GATE_DISP1(CLK_MM_TVE_FMM, "mm_tve_fmm", "mm_sel", 14),
 };
 
+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 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 of_device_id of_match_clk_mt2701_mm[] = {
 	{ .compatible = "mediatek,mt2701-mmsys", },
 	{}
@@ -87,6 +109,7 @@ static const struct of_device_id of_match_clk_mt2701_mm[] = {
 static int clk_mt2701_mm_probe(struct platform_device *pdev)
 {
 	struct clk_onecell_data *clk_data;
+	struct platform_device *drm;
 	int r;
 	struct device_node *node = pdev->dev.of_node;
 
@@ -101,6 +124,13 @@ static int clk_mt2701_mm_probe(struct platform_device *pdev)
 			"could not register clock provider: %s: %d\n",
 			pdev->name, r);
 
+	platform_set_drvdata(pdev, &mt2701_mmsys_driver_data);
+
+	drm = platform_device_register_data(&pdev->dev, "mediatek-drm",
+					    PLATFORM_DEVID_NONE, NULL, 0);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
 	return r;
 }
 
diff --git a/drivers/clk/mediatek/clk-mt2712-mm.c b/drivers/clk/mediatek/clk-mt2712-mm.c
index 1c5948be35f3..0ae971783997 100644
--- a/drivers/clk/mediatek/clk-mt2712-mm.c
+++ b/drivers/clk/mediatek/clk-mt2712-mm.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/platform_data/mtk_mmsys.h>
 #include <linux/platform_device.h>
 
 #include "clk-mtk.h"
@@ -126,9 +127,45 @@ static const struct mtk_gate mm_clks[] = {
 	GATE_MM2(CLK_MM_DSI3_DIGITAL, "mm_dsi3_digital", "dsi1_lntc", 6),
 };
 
+static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
+	DDP_COMPONENT_OVL0,
+	DDP_COMPONENT_COLOR0,
+	DDP_COMPONENT_AAL0,
+	DDP_COMPONENT_OD0,
+	DDP_COMPONENT_RDMA0,
+	DDP_COMPONENT_DPI0,
+	DDP_COMPONENT_PWM0,
+};
+
+static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = {
+	DDP_COMPONENT_OVL1,
+	DDP_COMPONENT_COLOR1,
+	DDP_COMPONENT_AAL1,
+	DDP_COMPONENT_OD1,
+	DDP_COMPONENT_RDMA1,
+	DDP_COMPONENT_DPI1,
+	DDP_COMPONENT_PWM1,
+};
+
+static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = {
+	DDP_COMPONENT_RDMA2,
+	DDP_COMPONENT_DSI3,
+	DDP_COMPONENT_PWM2,
+};
+
+static struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
+	.main_path = mt2712_mtk_ddp_main,
+	.main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
+	.ext_path = mt2712_mtk_ddp_ext,
+	.ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext),
+	.third_path = mt2712_mtk_ddp_third,
+	.third_len = ARRAY_SIZE(mt2712_mtk_ddp_third),
+};
+
 static int clk_mt2712_mm_probe(struct platform_device *pdev)
 {
 	struct clk_onecell_data *clk_data;
+	struct platform_device *drm;
 	int r;
 	struct device_node *node = pdev->dev.of_node;
 
@@ -143,6 +180,13 @@ static int clk_mt2712_mm_probe(struct platform_device *pdev)
 		pr_err("%s(): could not register clock provider: %d\n",
 			__func__, r);
 
+	platform_set_drvdata(pdev, &mt2712_mmsys_driver_data);
+
+	drm = platform_device_register_data(&pdev->dev, "mediatek-drm",
+					    PLATFORM_DEVID_NONE, NULL, 0);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
 	return r;
 }
 
diff --git a/drivers/clk/mediatek/clk-mt8173-mm.c b/drivers/clk/mediatek/clk-mt8173-mm.c
index 83884fd5a750..9136c7f543f1 100644
--- a/drivers/clk/mediatek/clk-mt8173-mm.c
+++ b/drivers/clk/mediatek/clk-mt8173-mm.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/platform_data/mtk_mmsys.h>
 #include <linux/platform_device.h>
 
 #include "clk-mtk.h"
@@ -99,10 +100,37 @@ static const struct mtk_gate mm_clks[] = {
 	GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
 };
 
+static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
+	DDP_COMPONENT_OVL0,
+	DDP_COMPONENT_COLOR0,
+	DDP_COMPONENT_AAL0,
+	DDP_COMPONENT_OD0,
+	DDP_COMPONENT_RDMA0,
+	DDP_COMPONENT_UFOE,
+	DDP_COMPONENT_DSI0,
+	DDP_COMPONENT_PWM0,
+};
+
+static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
+	DDP_COMPONENT_OVL1,
+	DDP_COMPONENT_COLOR1,
+	DDP_COMPONENT_GAMMA,
+	DDP_COMPONENT_RDMA1,
+	DDP_COMPONENT_DPI0,
+};
+
+static 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 clk_mt8173_mm_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
 	struct clk_onecell_data *clk_data;
+	struct platform_device *drm;
 	int ret;
 
 	clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
@@ -118,6 +146,13 @@ static int clk_mt8173_mm_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	platform_set_drvdata(pdev, &mt8173_mmsys_driver_data);
+
+	drm = platform_device_register_data(&pdev->dev, "mediatek-drm",
+					    PLATFORM_DEVID_NONE, NULL, 0);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index b68837ea02b3..5b60f6b7d710 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -61,88 +61,6 @@ static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
 	.atomic_commit = drm_atomic_helper_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 mt2712_mtk_ddp_main[] = {
-	DDP_COMPONENT_OVL0,
-	DDP_COMPONENT_COLOR0,
-	DDP_COMPONENT_AAL0,
-	DDP_COMPONENT_OD0,
-	DDP_COMPONENT_RDMA0,
-	DDP_COMPONENT_DPI0,
-	DDP_COMPONENT_PWM0,
-};
-
-static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = {
-	DDP_COMPONENT_OVL1,
-	DDP_COMPONENT_COLOR1,
-	DDP_COMPONENT_AAL1,
-	DDP_COMPONENT_OD1,
-	DDP_COMPONENT_RDMA1,
-	DDP_COMPONENT_DPI1,
-	DDP_COMPONENT_PWM1,
-};
-
-static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = {
-	DDP_COMPONENT_RDMA2,
-	DDP_COMPONENT_DSI3,
-	DDP_COMPONENT_PWM2,
-};
-
-static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
-	DDP_COMPONENT_OVL0,
-	DDP_COMPONENT_COLOR0,
-	DDP_COMPONENT_AAL0,
-	DDP_COMPONENT_OD0,
-	DDP_COMPONENT_RDMA0,
-	DDP_COMPONENT_UFOE,
-	DDP_COMPONENT_DSI0,
-	DDP_COMPONENT_PWM0,
-};
-
-static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
-	DDP_COMPONENT_OVL1,
-	DDP_COMPONENT_COLOR1,
-	DDP_COMPONENT_GAMMA,
-	DDP_COMPONENT_RDMA1,
-	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 mt2712_mmsys_driver_data = {
-	.main_path = mt2712_mtk_ddp_main,
-	.main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
-	.ext_path = mt2712_mtk_ddp_ext,
-	.ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext),
-	.third_path = mt2712_mtk_ddp_third,
-	.third_len = ARRAY_SIZE(mt2712_mtk_ddp_third),
-};
-
-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;
@@ -425,6 +343,7 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
 static int mtk_drm_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *phandle = dev->parent->of_node;
 	struct mtk_drm_private *private;
 	struct device_node *node;
 	struct component_match *match = NULL;
@@ -435,14 +354,16 @@ static int mtk_drm_probe(struct platform_device *pdev)
 	if (!private)
 		return -ENOMEM;
 
-	private->data = of_device_get_match_data(dev);
+	private->data = dev_get_drvdata(dev->parent);
+	if (!private->data)
+		return -ENODEV;
 
-	private->config_regs = syscon_node_to_regmap(dev->of_node);
+	private->config_regs = syscon_node_to_regmap(phandle);
 	if (IS_ERR(private->config_regs))
 		return PTR_ERR(private->config_regs);
 
 	/* Iterate over sibling DISP function blocks */
-	for_each_child_of_node(dev->of_node->parent, node) {
+	for_each_child_of_node(phandle->parent, node) {
 		const struct of_device_id *of_id;
 		enum mtk_ddp_comp_type comp_type;
 		int comp_id;
@@ -576,22 +497,11 @@ static int mtk_drm_sys_resume(struct device *dev)
 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,mt2712-mmsys",
-	  .data = &mt2712_mmsys_driver_data},
-	{ .compatible = "mediatek,mt8173-mmsys",
-	  .data = &mt8173_mmsys_driver_data},
-	{ }
-};
-
 static struct platform_driver mtk_drm_platform_driver = {
 	.probe	= mtk_drm_probe,
 	.remove	= mtk_drm_remove,
 	.driver	= {
 		.name	= "mediatek-drm",
-		.of_match_table = mtk_drm_of_ids,
 		.pm     = &mtk_drm_pm_ops,
 	},
 };
-- 
2.25.0

Re: [PATCH v8 6/6] clk/drm: mediatek: Fix mediatek-drm device probing

[CK Hu]

Hi, Enric:

On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
> In the actual implementation the same compatible string
> "mediatek,<chip>-mmsys" is used to bind the clock drivers
> (drivers/clk/mediatek) as well as to the gpu driver
> (drivers/gpu/drm/mediatek/mtk_drm_drv.c). This ends with the problem
> that the only probed driver is the clock driver and there is no display
> at all.
> 
> In any case having the same compatible string for two drivers is not
> correct and should be fixed. To fix this, and maintain backward
> compatibility, we can consider that the clk-<chip>-mm driver is the
> top-level entry point for the MMSYS subsystem, so is not a pure clock
> controller but a system controller, and the drm driver is instantiated
> by that MMSYS driver.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
> Changes in v8:
> - New patch introduced in this series.
> 
> Changes in v7: None
> 
>  drivers/clk/mediatek/clk-mt2701-mm.c   |  30 ++++++++
>  drivers/clk/mediatek/clk-mt2712-mm.c   |  44 +++++++++++
>  drivers/clk/mediatek/clk-mt8173-mm.c   |  35 +++++++++
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 102 ++-----------------------
>  4 files changed, 115 insertions(+), 96 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt2701-mm.c b/drivers/clk/mediatek/clk-mt2701-mm.c
> index 054b597d4a73..b1281680d5bf 100644
> --- a/drivers/clk/mediatek/clk-mt2701-mm.c
> +++ b/drivers/clk/mediatek/clk-mt2701-mm.c
> @@ -5,6 +5,7 @@
>   */

[snip]

>  
>  
> diff --git a/drivers/clk/mediatek/clk-mt2712-mm.c b/drivers/clk/mediatek/clk-mt2712-mm.c
> index 1c5948be35f3..0ae971783997 100644
> --- a/drivers/clk/mediatek/clk-mt2712-mm.c
> +++ b/drivers/clk/mediatek/clk-mt2712-mm.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/clk-provider.h>
> +#include <linux/platform_data/mtk_mmsys.h>
>  #include <linux/platform_device.h>
>  
>  #include "clk-mtk.h"
> @@ -126,9 +127,45 @@ static const struct mtk_gate mm_clks[] = {
>  	GATE_MM2(CLK_MM_DSI3_DIGITAL, "mm_dsi3_digital", "dsi1_lntc", 6),
>  };
>  
> +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
> +	DDP_COMPONENT_OVL0,
> +	DDP_COMPONENT_COLOR0,
> +	DDP_COMPONENT_AAL0,
> +	DDP_COMPONENT_OD0,
> +	DDP_COMPONENT_RDMA0,
> +	DDP_COMPONENT_DPI0,
> +	DDP_COMPONENT_PWM0,
> +};
> +
> +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = {
> +	DDP_COMPONENT_OVL1,
> +	DDP_COMPONENT_COLOR1,
> +	DDP_COMPONENT_AAL1,
> +	DDP_COMPONENT_OD1,
> +	DDP_COMPONENT_RDMA1,
> +	DDP_COMPONENT_DPI1,
> +	DDP_COMPONENT_PWM1,
> +};
> +
> +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = {
> +	DDP_COMPONENT_RDMA2,
> +	DDP_COMPONENT_DSI3,
> +	DDP_COMPONENT_PWM2,
> +};
> +
> +static struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
> +	.main_path = mt2712_mtk_ddp_main,
> +	.main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
> +	.ext_path = mt2712_mtk_ddp_ext,
> +	.ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext),
> +	.third_path = mt2712_mtk_ddp_third,
> +	.third_len = ARRAY_SIZE(mt2712_mtk_ddp_third),
> +};
> +

[snip]

>  
> diff --git a/drivers/clk/mediatek/clk-mt8173-mm.c b/drivers/clk/mediatek/clk-mt8173-mm.c
> index 83884fd5a750..9136c7f543f1 100644
> --- a/drivers/clk/mediatek/clk-mt8173-mm.c
> +++ b/drivers/clk/mediatek/clk-mt8173-mm.c
> @@ -5,6 +5,7 @@
>   */
>  
>  #include <linux/clk-provider.h>
> +#include <linux/platform_data/mtk_mmsys.h>
>  #include <linux/platform_device.h>
>  
>  #include "clk-mtk.h"
> @@ -99,10 +100,37 @@ static const struct mtk_gate mm_clks[] = {
>  	GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
>  };
>  
> +static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
> +	DDP_COMPONENT_OVL0,
> +	DDP_COMPONENT_COLOR0,
> +	DDP_COMPONENT_AAL0,
> +	DDP_COMPONENT_OD0,
> +	DDP_COMPONENT_RDMA0,
> +	DDP_COMPONENT_UFOE,
> +	DDP_COMPONENT_DSI0,
> +	DDP_COMPONENT_PWM0,
> +};
> +
> +static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
> +	DDP_COMPONENT_OVL1,
> +	DDP_COMPONENT_COLOR1,
> +	DDP_COMPONENT_GAMMA,
> +	DDP_COMPONENT_RDMA1,
> +	DDP_COMPONENT_DPI0,
> +};
> +
> +static 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 clk_mt8173_mm_probe(struct platform_device *pdev)
>  {
>  	struct device_node *node = pdev->dev.of_node;
>  	struct clk_onecell_data *clk_data;
> +	struct platform_device *drm;
>  	int ret;
>  
>  	clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
> @@ -118,6 +146,13 @@ static int clk_mt8173_mm_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	platform_set_drvdata(pdev, &mt8173_mmsys_driver_data);
> +
> +	drm = platform_device_register_data(&pdev->dev, "mediatek-drm",
> +					    PLATFORM_DEVID_NONE, NULL, 0);
> +	if (IS_ERR(drm))
> +		return PTR_ERR(drm);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index b68837ea02b3..5b60f6b7d710 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -61,88 +61,6 @@ static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
>  	.atomic_commit = drm_atomic_helper_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 mt2712_mtk_ddp_main[] = {
> -	DDP_COMPONENT_OVL0,
> -	DDP_COMPONENT_COLOR0,
> -	DDP_COMPONENT_AAL0,
> -	DDP_COMPONENT_OD0,
> -	DDP_COMPONENT_RDMA0,
> -	DDP_COMPONENT_DPI0,
> -	DDP_COMPONENT_PWM0,
> -};
> -
> -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = {
> -	DDP_COMPONENT_OVL1,
> -	DDP_COMPONENT_COLOR1,
> -	DDP_COMPONENT_AAL1,
> -	DDP_COMPONENT_OD1,
> -	DDP_COMPONENT_RDMA1,
> -	DDP_COMPONENT_DPI1,
> -	DDP_COMPONENT_PWM1,
> -};
> -
> -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = {
> -	DDP_COMPONENT_RDMA2,
> -	DDP_COMPONENT_DSI3,
> -	DDP_COMPONENT_PWM2,
> -};
> -
> -static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
> -	DDP_COMPONENT_OVL0,
> -	DDP_COMPONENT_COLOR0,
> -	DDP_COMPONENT_AAL0,
> -	DDP_COMPONENT_OD0,
> -	DDP_COMPONENT_RDMA0,
> -	DDP_COMPONENT_UFOE,
> -	DDP_COMPONENT_DSI0,
> -	DDP_COMPONENT_PWM0,
> -};
> -
> -static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
> -	DDP_COMPONENT_OVL1,
> -	DDP_COMPONENT_COLOR1,
> -	DDP_COMPONENT_GAMMA,
> -	DDP_COMPONENT_RDMA1,
> -	DDP_COMPONENT_DPI0,
> -};

I prefer that display routing is placed in drm driver. If you want to
move display routing into mmsys driver, I think you should move
mtk_ddp_add_comp_to_path() and mtk_ddp_remove_comp_from_path() into
mmsys driver because that is the register configuration part. This array
could be changed by display driver according to its application. For
example, the another routing could be:

static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
	DDP_COMPONENT_OVL0,
	DDP_COMPONENT_COLOR0,
	DDP_COMPONENT_AAL0,
	DDP_COMPONENT_OD0,
	DDP_COMPONENT_RDMA0,
	DDP_COMPONENT_UFOE,
	DDP_COMPONENT_DPI0,
};

static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
	DDP_COMPONENT_OVL1,
	DDP_COMPONENT_COLOR1,
	DDP_COMPONENT_GAMMA,
	DDP_COMPONENT_RDMA1,
	DDP_COMPONENT_DSI0,
	DDP_COMPONENT_PWM0,
};

I exchange the dsi and dpi component for the two display output. This
array is how display driver want to route, so I think this should be
kept in display driver.

Regards,
CK

> -
> -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 mt2712_mmsys_driver_data = {
> -	.main_path = mt2712_mtk_ddp_main,
> -	.main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
> -	.ext_path = mt2712_mtk_ddp_ext,
> -	.ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext),
> -	.third_path = mt2712_mtk_ddp_third,
> -	.third_len = ARRAY_SIZE(mt2712_mtk_ddp_third),
> -};
> -
> -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),
> -};
> -

Re: [PATCH v8 6/6] clk/drm: mediatek: Fix mediatek-drm device probing

[Enric Balletbo i Serra]

Hi CK,

Thanks for your quick feedback.

On 21/2/20 6:18, CK Hu wrote:
> Hi, Enric:
> 
> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
>> In the actual implementation the same compatible string
>> "mediatek,<chip>-mmsys" is used to bind the clock drivers
>> (drivers/clk/mediatek) as well as to the gpu driver
>> (drivers/gpu/drm/mediatek/mtk_drm_drv.c). This ends with the problem
>> that the only probed driver is the clock driver and there is no display
>> at all.
>>
>> In any case having the same compatible string for two drivers is not
>> correct and should be fixed. To fix this, and maintain backward
>> compatibility, we can consider that the clk-<chip>-mm driver is the
>> top-level entry point for the MMSYS subsystem, so is not a pure clock
>> controller but a system controller, and the drm driver is instantiated
>> by that MMSYS driver.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>> Changes in v8:
>> - New patch introduced in this series.
>>
>> Changes in v7: None
>>
>>  drivers/clk/mediatek/clk-mt2701-mm.c   |  30 ++++++++
>>  drivers/clk/mediatek/clk-mt2712-mm.c   |  44 +++++++++++
>>  drivers/clk/mediatek/clk-mt8173-mm.c   |  35 +++++++++
>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 102 ++-----------------------
>>  4 files changed, 115 insertions(+), 96 deletions(-)
>>
>> diff --git a/drivers/clk/mediatek/clk-mt2701-mm.c b/drivers/clk/mediatek/clk-mt2701-mm.c
>> index 054b597d4a73..b1281680d5bf 100644
>> --- a/drivers/clk/mediatek/clk-mt2701-mm.c
>> +++ b/drivers/clk/mediatek/clk-mt2701-mm.c
>> @@ -5,6 +5,7 @@
>>   */
> 
> [snip]
> 
>>  
>>  
>> diff --git a/drivers/clk/mediatek/clk-mt2712-mm.c b/drivers/clk/mediatek/clk-mt2712-mm.c
>> index 1c5948be35f3..0ae971783997 100644
>> --- a/drivers/clk/mediatek/clk-mt2712-mm.c
>> +++ b/drivers/clk/mediatek/clk-mt2712-mm.c
>> @@ -5,6 +5,7 @@
>>   */
>>  
>>  #include <linux/clk-provider.h>
>> +#include <linux/platform_data/mtk_mmsys.h>
>>  #include <linux/platform_device.h>
>>  
>>  #include "clk-mtk.h"
>> @@ -126,9 +127,45 @@ static const struct mtk_gate mm_clks[] = {
>>  	GATE_MM2(CLK_MM_DSI3_DIGITAL, "mm_dsi3_digital", "dsi1_lntc", 6),
>>  };
>>  
>> +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_main[] = {
>> +	DDP_COMPONENT_OVL0,
>> +	DDP_COMPONENT_COLOR0,
>> +	DDP_COMPONENT_AAL0,
>> +	DDP_COMPONENT_OD0,
>> +	DDP_COMPONENT_RDMA0,
>> +	DDP_COMPONENT_DPI0,
>> +	DDP_COMPONENT_PWM0,
>> +};
>> +
>> +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = {
>> +	DDP_COMPONENT_OVL1,
>> +	DDP_COMPONENT_COLOR1,
>> +	DDP_COMPONENT_AAL1,
>> +	DDP_COMPONENT_OD1,
>> +	DDP_COMPONENT_RDMA1,
>> +	DDP_COMPONENT_DPI1,
>> +	DDP_COMPONENT_PWM1,
>> +};
>> +
>> +static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = {
>> +	DDP_COMPONENT_RDMA2,
>> +	DDP_COMPONENT_DSI3,
>> +	DDP_COMPONENT_PWM2,
>> +};
>> +
>> +static struct mtk_mmsys_driver_data mt2712_mmsys_driver_data = {
>> +	.main_path = mt2712_mtk_ddp_main,
>> +	.main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
>> +	.ext_path = mt2712_mtk_ddp_ext,
>> +	.ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext),
>> +	.third_path = mt2712_mtk_ddp_third,
>> +	.third_len = ARRAY_SIZE(mt2712_mtk_ddp_third),
>> +};
>> +
> 
> [snip]
> 
>>  
>> diff --git a/drivers/clk/mediatek/clk-mt8173-mm.c b/drivers/clk/mediatek/clk-mt8173-mm.c
>> index 83884fd5a750..9136c7f543f1 100644
>> --- a/drivers/clk/mediatek/clk-mt8173-mm.c
>> +++ b/drivers/clk/mediatek/clk-mt8173-mm.c
>> @@ -5,6 +5,7 @@
>>   */
>>  
>>  #include <linux/clk-provider.h>
>> +#include <linux/platform_data/mtk_mmsys.h>
>>  #include <linux/platform_device.h>
>>  
>>  #include "clk-mtk.h"
>> @@ -99,10 +100,37 @@ static const struct mtk_gate mm_clks[] = {
>>  	GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
>>  };
>>  
>> +static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
>> +	DDP_COMPONENT_OVL0,
>> +	DDP_COMPONENT_COLOR0,
>> +	DDP_COMPONENT_AAL0,
>> +	DDP_COMPONENT_OD0,
>> +	DDP_COMPONENT_RDMA0,
>> +	DDP_COMPONENT_UFOE,
>> +	DDP_COMPONENT_DSI0,
>> +	DDP_COMPONENT_PWM0,
>> +};
>> +
>> +static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
>> +	DDP_COMPONENT_OVL1,
>> +	DDP_COMPONENT_COLOR1,
>> +	DDP_COMPONENT_GAMMA,
>> +	DDP_COMPONENT_RDMA1,
>> +	DDP_COMPONENT_DPI0,
>> +};
>> +
>> +static 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 clk_mt8173_mm_probe(struct platform_device *pdev)
>>  {
>>  	struct device_node *node = pdev->dev.of_node;
>>  	struct clk_onecell_data *clk_data;
>> +	struct platform_device *drm;
>>  	int ret;
>>  
>>  	clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
>> @@ -118,6 +146,13 @@ static int clk_mt8173_mm_probe(struct platform_device *pdev)
>>  	if (ret)
>>  		return ret;
>>  
>> +	platform_set_drvdata(pdev, &mt8173_mmsys_driver_data);
>> +
>> +	drm = platform_device_register_data(&pdev->dev, "mediatek-drm",
>> +					    PLATFORM_DEVID_NONE, NULL, 0);
>> +	if (IS_ERR(drm))
>> +		return PTR_ERR(drm);
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> index b68837ea02b3..5b60f6b7d710 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
>> @@ -61,88 +61,6 @@ static const struct drm_mode_config_funcs mtk_drm_mode_config_funcs = {
>>  	.atomic_commit = drm_atomic_helper_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 mt2712_mtk_ddp_main[] = {
>> -	DDP_COMPONENT_OVL0,
>> -	DDP_COMPONENT_COLOR0,
>> -	DDP_COMPONENT_AAL0,
>> -	DDP_COMPONENT_OD0,
>> -	DDP_COMPONENT_RDMA0,
>> -	DDP_COMPONENT_DPI0,
>> -	DDP_COMPONENT_PWM0,
>> -};
>> -
>> -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_ext[] = {
>> -	DDP_COMPONENT_OVL1,
>> -	DDP_COMPONENT_COLOR1,
>> -	DDP_COMPONENT_AAL1,
>> -	DDP_COMPONENT_OD1,
>> -	DDP_COMPONENT_RDMA1,
>> -	DDP_COMPONENT_DPI1,
>> -	DDP_COMPONENT_PWM1,
>> -};
>> -
>> -static const enum mtk_ddp_comp_id mt2712_mtk_ddp_third[] = {
>> -	DDP_COMPONENT_RDMA2,
>> -	DDP_COMPONENT_DSI3,
>> -	DDP_COMPONENT_PWM2,
>> -};
>> -
>> -static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
>> -	DDP_COMPONENT_OVL0,
>> -	DDP_COMPONENT_COLOR0,
>> -	DDP_COMPONENT_AAL0,
>> -	DDP_COMPONENT_OD0,
>> -	DDP_COMPONENT_RDMA0,
>> -	DDP_COMPONENT_UFOE,
>> -	DDP_COMPONENT_DSI0,
>> -	DDP_COMPONENT_PWM0,
>> -};
>> -
>> -static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
>> -	DDP_COMPONENT_OVL1,
>> -	DDP_COMPONENT_COLOR1,
>> -	DDP_COMPONENT_GAMMA,
>> -	DDP_COMPONENT_RDMA1,
>> -	DDP_COMPONENT_DPI0,
>> -};
> 
> I prefer that display routing is placed in drm driver. If you want to
> move display routing into mmsys driver, I think you should move
> mtk_ddp_add_comp_to_path() and mtk_ddp_remove_comp_from_path() into
> mmsys driver because that is the register configuration part. This array
> could be changed by display driver according to its application. For
> example, the another routing could be:
> 
> static const enum mtk_ddp_comp_id mt8173_mtk_ddp_main[] = {
> 	DDP_COMPONENT_OVL0,
> 	DDP_COMPONENT_COLOR0,
> 	DDP_COMPONENT_AAL0,
> 	DDP_COMPONENT_OD0,
> 	DDP_COMPONENT_RDMA0,
> 	DDP_COMPONENT_UFOE,
> 	DDP_COMPONENT_DPI0,
> };
> 
> static const enum mtk_ddp_comp_id mt8173_mtk_ddp_ext[] = {
> 	DDP_COMPONENT_OVL1,
> 	DDP_COMPONENT_COLOR1,
> 	DDP_COMPONENT_GAMMA,
> 	DDP_COMPONENT_RDMA1,
> 	DDP_COMPONENT_DSI0,
> 	DDP_COMPONENT_PWM0,
> };
> 
> I exchange the dsi and dpi component for the two display output. This
> array is how display driver want to route, so I think this should be
> kept in display driver.
> 
> Regards,
> CK
> 

I think that what I can do is leave all this part in the drm driver and get the
compatible string of the parent to match the data with the proper SoC. I'll try
to do this in next version.

Thanks.

>> -
>> -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 mt2712_mmsys_driver_data = {
>> -	.main_path = mt2712_mtk_ddp_main,
>> -	.main_len = ARRAY_SIZE(mt2712_mtk_ddp_main),
>> -	.ext_path = mt2712_mtk_ddp_ext,
>> -	.ext_len = ARRAY_SIZE(mt2712_mtk_ddp_ext),
>> -	.third_path = mt2712_mtk_ddp_third,
>> -	.third_len = ARRAY_SIZE(mt2712_mtk_ddp_third),
>> -};
>> -
>> -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),
>> -};
>> -
> 
> 

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[CK Hu]

Hi, Enric:

On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
> Dear all,
> 
> Those patches are intended to solve an old standing issue on some
> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
> to the precedent series.
> 
> Up to now both drivers, clock and drm are probed with the same device tree
> compatible. But only the first driver get probed, which in effect breaks
> graphics on those devices.
> 
> The version eight of the series tries to solve the problem with a
> different approach than the previous series but similar to how is solved
> on other Mediatek devices.
> 
> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
> control clock gates (which is used in the clk driver) and some registers
> to set the routing and enable the differnet blocks of the display
> and MDP (Media Data Path) subsystem. On this series the clk driver is
> not a pure clock controller but a system controller that can provide
> access to the shared registers between the different drivers that need
> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
> this version, clk driver is the entry point (parent) which will trigger
> the probe of the corresponding mediatek-drm driver and pass its MMSYS
> platform data for display configuration.

When mmsys is a system controller, I prefer to place mmsys in
drivers/soc/mediatek, and it share registers for clock, display, and mdp
driver. This means the probe function is placed in
drivers/soc/mediatek ,its display clock function, mdp clock function are
placed in drivers/clk, display routing are placed in drivers/gpu/drm,
and mdp routing are placed in dirvers/video.

Regards,
CK

> 
> All this series was tested on the Acer R13 Chromebook only.
> 
> For reference, here are the links to the old discussions:
> 
> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
> * v4:
>   * https://patchwork.kernel.org/patch/10530871/
>   * https://patchwork.kernel.org/patch/10530883/
>   * https://patchwork.kernel.org/patch/10530885/
>   * https://patchwork.kernel.org/patch/10530911/
>   * https://patchwork.kernel.org/patch/10530913/
> * v3:
>   * https://patchwork.kernel.org/patch/10367857/
>   * https://patchwork.kernel.org/patch/10367861/
>   * https://patchwork.kernel.org/patch/10367877/
>   * https://patchwork.kernel.org/patch/10367875/
>   * https://patchwork.kernel.org/patch/10367885/
>   * https://patchwork.kernel.org/patch/10367883/
>   * https://patchwork.kernel.org/patch/10367889/
>   * https://patchwork.kernel.org/patch/10367907/
>   * https://patchwork.kernel.org/patch/10367909/
>   * https://patchwork.kernel.org/patch/10367905/
> * v2: No relevant discussion, see v3
> * v1:
>   * https://patchwork.kernel.org/patch/10016497/
>   * https://patchwork.kernel.org/patch/10016499/
>   * https://patchwork.kernel.org/patch/10016505/
>   * https://patchwork.kernel.org/patch/10016507/
> 
> Best regards,
>  Enric
> 
> Changes in v8:
> - Be a builtin_platform_driver like other mediatek mmsys drivers.
> - New patches introduced in this series.
> 
> Changes in v7:
> - Add R-by from CK
> - Add R-by from CK
> - Fix check of return value of of_clk_get
> - Fix identation
> - Free clk_data->clks as well
> - Get rid of private data structure
> 
> Enric Balletbo i Serra (2):
>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
>   clk/drm: mediatek: Fix mediatek-drm device probing
> 
> Matthias Brugger (4):
>   drm/mediatek: Use regmap for register access
>   drm/mediatek: Omit warning on probe defers
>   media: mtk-mdp: Check return value of of_clk_get
>   clk: mediatek: mt8173: Switch MMSYS to platform driver
> 
>  drivers/clk/mediatek/Kconfig                  |   6 +
>  drivers/clk/mediatek/Makefile                 |   1 +
>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
>  20 files changed, 401 insertions(+), 317 deletions(-)
>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
> 

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Enric Balletbo i Serra]

Hi CK,

Thanks for your quick answer.

On 21/2/20 5:39, CK Hu wrote:
> Hi, Enric:
> 
> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
>> Dear all,
>>
>> Those patches are intended to solve an old standing issue on some
>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
>> to the precedent series.
>>
>> Up to now both drivers, clock and drm are probed with the same device tree
>> compatible. But only the first driver get probed, which in effect breaks
>> graphics on those devices.
>>
>> The version eight of the series tries to solve the problem with a
>> different approach than the previous series but similar to how is solved
>> on other Mediatek devices.
>>
>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
>> control clock gates (which is used in the clk driver) and some registers
>> to set the routing and enable the differnet blocks of the display
>> and MDP (Media Data Path) subsystem. On this series the clk driver is
>> not a pure clock controller but a system controller that can provide
>> access to the shared registers between the different drivers that need
>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
>> this version, clk driver is the entry point (parent) which will trigger
>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
>> platform data for display configuration.
> 
> When mmsys is a system controller, I prefer to place mmsys in
> drivers/soc/mediatek, and it share registers for clock, display, and mdp
> driver. This means the probe function is placed in
> drivers/soc/mediatek ,its display clock function, mdp clock function are
> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
> and mdp routing are placed in dirvers/video.
> 

I understand what you mean but I am not sure this makes the code clearer and
useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
of a "simple-mfd" device (a driver that simply matches with
"mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
"mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
device-tree).

It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
beginning representing how really hardwware is, but I think that, change this
now, will break backward compatibility.

IMHO I think that considering the clk driver as entry point is fine, but this is
something that the clock maintainers should decide.

Also note that this is not only a MT8173 problem I am seeing the same problem on
all other Mediatek SoCs.

Thanks.

> Regards,
> CK
> 
>>
>> All this series was tested on the Acer R13 Chromebook only.
>>
>> For reference, here are the links to the old discussions:
>>
>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
>> * v4:
>>   * https://patchwork.kernel.org/patch/10530871/
>>   * https://patchwork.kernel.org/patch/10530883/
>>   * https://patchwork.kernel.org/patch/10530885/
>>   * https://patchwork.kernel.org/patch/10530911/
>>   * https://patchwork.kernel.org/patch/10530913/
>> * v3:
>>   * https://patchwork.kernel.org/patch/10367857/
>>   * https://patchwork.kernel.org/patch/10367861/
>>   * https://patchwork.kernel.org/patch/10367877/
>>   * https://patchwork.kernel.org/patch/10367875/
>>   * https://patchwork.kernel.org/patch/10367885/
>>   * https://patchwork.kernel.org/patch/10367883/
>>   * https://patchwork.kernel.org/patch/10367889/
>>   * https://patchwork.kernel.org/patch/10367907/
>>   * https://patchwork.kernel.org/patch/10367909/
>>   * https://patchwork.kernel.org/patch/10367905/
>> * v2: No relevant discussion, see v3
>> * v1:
>>   * https://patchwork.kernel.org/patch/10016497/
>>   * https://patchwork.kernel.org/patch/10016499/
>>   * https://patchwork.kernel.org/patch/10016505/
>>   * https://patchwork.kernel.org/patch/10016507/
>>
>> Best regards,
>>  Enric
>>
>> Changes in v8:
>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>> - New patches introduced in this series.
>>
>> Changes in v7:
>> - Add R-by from CK
>> - Add R-by from CK
>> - Fix check of return value of of_clk_get
>> - Fix identation
>> - Free clk_data->clks as well
>> - Get rid of private data structure
>>
>> Enric Balletbo i Serra (2):
>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
>>   clk/drm: mediatek: Fix mediatek-drm device probing
>>
>> Matthias Brugger (4):
>>   drm/mediatek: Use regmap for register access
>>   drm/mediatek: Omit warning on probe defers
>>   media: mtk-mdp: Check return value of of_clk_get
>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
>>
>>  drivers/clk/mediatek/Kconfig                  |   6 +
>>  drivers/clk/mediatek/Makefile                 |   1 +
>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
>>  20 files changed, 401 insertions(+), 317 deletions(-)
>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
>>
> 

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[CK Hu]

Hi, Enric:

On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
> Hi CK,
> 
> Thanks for your quick answer.
> 
> On 21/2/20 5:39, CK Hu wrote:
> > Hi, Enric:
> > 
> > On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
> >> Dear all,
> >>
> >> Those patches are intended to solve an old standing issue on some
> >> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
> >> to the precedent series.
> >>
> >> Up to now both drivers, clock and drm are probed with the same device tree
> >> compatible. But only the first driver get probed, which in effect breaks
> >> graphics on those devices.
> >>
> >> The version eight of the series tries to solve the problem with a
> >> different approach than the previous series but similar to how is solved
> >> on other Mediatek devices.
> >>
> >> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
> >> control clock gates (which is used in the clk driver) and some registers
> >> to set the routing and enable the differnet blocks of the display
> >> and MDP (Media Data Path) subsystem. On this series the clk driver is
> >> not a pure clock controller but a system controller that can provide
> >> access to the shared registers between the different drivers that need
> >> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
> >> this version, clk driver is the entry point (parent) which will trigger
> >> the probe of the corresponding mediatek-drm driver and pass its MMSYS
> >> platform data for display configuration.
> > 
> > When mmsys is a system controller, I prefer to place mmsys in
> > drivers/soc/mediatek, and it share registers for clock, display, and mdp
> > driver. This means the probe function is placed in
> > drivers/soc/mediatek ,its display clock function, mdp clock function are
> > placed in drivers/clk, display routing are placed in drivers/gpu/drm,
> > and mdp routing are placed in dirvers/video.
> > 
> 
> I understand what you mean but I am not sure this makes the code clearer and
> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
> of a "simple-mfd" device (a driver that simply matches with
> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
> device-tree).
> 

It's clear that mmsys is neither a pure clock controller nor a pure
routing controller for display and mdp. 

> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
> beginning representing how really hardwware is, but I think that, change this
> now, will break backward compatibility.

Maybe this is a solution. Current device tree would work only on old
kernel version with a bug, so this mean there is no any device tree
works on kernel version without bug. Why do we compatible with such
device tree?

Regards,
CK

> 
> IMHO I think that considering the clk driver as entry point is fine, but this is
> something that the clock maintainers should decide.
> 
> Also note that this is not only a MT8173 problem I am seeing the same problem on
> all other Mediatek SoCs.
> 
> Thanks.
> 
> > Regards,
> > CK
> > 
> >>
> >> All this series was tested on the Acer R13 Chromebook only.
> >>
> >> For reference, here are the links to the old discussions:
> >>
> >> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
> >> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
> >> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
> >> * v4:
> >>   * https://patchwork.kernel.org/patch/10530871/
> >>   * https://patchwork.kernel.org/patch/10530883/
> >>   * https://patchwork.kernel.org/patch/10530885/
> >>   * https://patchwork.kernel.org/patch/10530911/
> >>   * https://patchwork.kernel.org/patch/10530913/
> >> * v3:
> >>   * https://patchwork.kernel.org/patch/10367857/
> >>   * https://patchwork.kernel.org/patch/10367861/
> >>   * https://patchwork.kernel.org/patch/10367877/
> >>   * https://patchwork.kernel.org/patch/10367875/
> >>   * https://patchwork.kernel.org/patch/10367885/
> >>   * https://patchwork.kernel.org/patch/10367883/
> >>   * https://patchwork.kernel.org/patch/10367889/
> >>   * https://patchwork.kernel.org/patch/10367907/
> >>   * https://patchwork.kernel.org/patch/10367909/
> >>   * https://patchwork.kernel.org/patch/10367905/
> >> * v2: No relevant discussion, see v3
> >> * v1:
> >>   * https://patchwork.kernel.org/patch/10016497/
> >>   * https://patchwork.kernel.org/patch/10016499/
> >>   * https://patchwork.kernel.org/patch/10016505/
> >>   * https://patchwork.kernel.org/patch/10016507/
> >>
> >> Best regards,
> >>  Enric
> >>
> >> Changes in v8:
> >> - Be a builtin_platform_driver like other mediatek mmsys drivers.
> >> - New patches introduced in this series.
> >>
> >> Changes in v7:
> >> - Add R-by from CK
> >> - Add R-by from CK
> >> - Fix check of return value of of_clk_get
> >> - Fix identation
> >> - Free clk_data->clks as well
> >> - Get rid of private data structure
> >>
> >> Enric Balletbo i Serra (2):
> >>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
> >>   clk/drm: mediatek: Fix mediatek-drm device probing
> >>
> >> Matthias Brugger (4):
> >>   drm/mediatek: Use regmap for register access
> >>   drm/mediatek: Omit warning on probe defers
> >>   media: mtk-mdp: Check return value of of_clk_get
> >>   clk: mediatek: mt8173: Switch MMSYS to platform driver
> >>
> >>  drivers/clk/mediatek/Kconfig                  |   6 +
> >>  drivers/clk/mediatek/Makefile                 |   1 +
> >>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
> >>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
> >>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
> >>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
> >>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
> >>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
> >>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
> >>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
> >>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
> >>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
> >>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
> >>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
> >>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
> >>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
> >>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
> >>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
> >>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
> >>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
> >>  20 files changed, 401 insertions(+), 317 deletions(-)
> >>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
> >>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
> >>
> > 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Matthias Brugger]

On 21/02/2020 10:27, CK Hu wrote:
> Hi, Enric:
> 
> On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
>> Hi CK,
>>
>> Thanks for your quick answer.
>>
>> On 21/2/20 5:39, CK Hu wrote:
>>> Hi, Enric:
>>>
>>> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
>>>> Dear all,
>>>>
>>>> Those patches are intended to solve an old standing issue on some
>>>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
>>>> to the precedent series.
>>>>
>>>> Up to now both drivers, clock and drm are probed with the same device tree
>>>> compatible. But only the first driver get probed, which in effect breaks
>>>> graphics on those devices.
>>>>
>>>> The version eight of the series tries to solve the problem with a
>>>> different approach than the previous series but similar to how is solved
>>>> on other Mediatek devices.
>>>>
>>>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
>>>> control clock gates (which is used in the clk driver) and some registers
>>>> to set the routing and enable the differnet blocks of the display
>>>> and MDP (Media Data Path) subsystem. On this series the clk driver is
>>>> not a pure clock controller but a system controller that can provide
>>>> access to the shared registers between the different drivers that need
>>>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
>>>> this version, clk driver is the entry point (parent) which will trigger
>>>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
>>>> platform data for display configuration.
>>>
>>> When mmsys is a system controller, I prefer to place mmsys in
>>> drivers/soc/mediatek, and it share registers for clock, display, and mdp
>>> driver. This means the probe function is placed in
>>> drivers/soc/mediatek ,its display clock function, mdp clock function are
>>> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
>>> and mdp routing are placed in dirvers/video.
>>>
>>
>> I understand what you mean but I am not sure this makes the code clearer and
>> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
>> of a "simple-mfd" device (a driver that simply matches with
>> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
>> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
>> device-tree).
>>
> 
> It's clear that mmsys is neither a pure clock controller nor a pure
> routing controller for display and mdp. 
> 
>> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
>> beginning representing how really hardwware is, but I think that, change this
>> now, will break backward compatibility.
> 
> Maybe this is a solution. Current device tree would work only on old
> kernel version with a bug, so this mean there is no any device tree
> works on kernel version without bug. Why do we compatible with such
> device tree?
> 

The idea behind this is, that the device-tree could be passed by some boot
firmware, so that the OS do not care about it. For this we need a stable DTS as
otherwise newer kernel with older FW would break.

DTS is supposed to be just a different description of the HW like ACPI. So it
has to be compatible (newer kernel with older DTS and if possible vice versa).

Regards,
Matthias

> Regards,
> CK
> 
>>
>> IMHO I think that considering the clk driver as entry point is fine, but this is
>> something that the clock maintainers should decide.
>>
>> Also note that this is not only a MT8173 problem I am seeing the same problem on
>> all other Mediatek SoCs.
>>
>> Thanks.
>>
>>> Regards,
>>> CK
>>>
>>>>
>>>> All this series was tested on the Acer R13 Chromebook only.
>>>>
>>>> For reference, here are the links to the old discussions:
>>>>
>>>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
>>>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
>>>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
>>>> * v4:
>>>>   * https://patchwork.kernel.org/patch/10530871/
>>>>   * https://patchwork.kernel.org/patch/10530883/
>>>>   * https://patchwork.kernel.org/patch/10530885/
>>>>   * https://patchwork.kernel.org/patch/10530911/
>>>>   * https://patchwork.kernel.org/patch/10530913/
>>>> * v3:
>>>>   * https://patchwork.kernel.org/patch/10367857/
>>>>   * https://patchwork.kernel.org/patch/10367861/
>>>>   * https://patchwork.kernel.org/patch/10367877/
>>>>   * https://patchwork.kernel.org/patch/10367875/
>>>>   * https://patchwork.kernel.org/patch/10367885/
>>>>   * https://patchwork.kernel.org/patch/10367883/
>>>>   * https://patchwork.kernel.org/patch/10367889/
>>>>   * https://patchwork.kernel.org/patch/10367907/
>>>>   * https://patchwork.kernel.org/patch/10367909/
>>>>   * https://patchwork.kernel.org/patch/10367905/
>>>> * v2: No relevant discussion, see v3
>>>> * v1:
>>>>   * https://patchwork.kernel.org/patch/10016497/
>>>>   * https://patchwork.kernel.org/patch/10016499/
>>>>   * https://patchwork.kernel.org/patch/10016505/
>>>>   * https://patchwork.kernel.org/patch/10016507/
>>>>
>>>> Best regards,
>>>>  Enric
>>>>
>>>> Changes in v8:
>>>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>>>> - New patches introduced in this series.
>>>>
>>>> Changes in v7:
>>>> - Add R-by from CK
>>>> - Add R-by from CK
>>>> - Fix check of return value of of_clk_get
>>>> - Fix identation
>>>> - Free clk_data->clks as well
>>>> - Get rid of private data structure
>>>>
>>>> Enric Balletbo i Serra (2):
>>>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
>>>>   clk/drm: mediatek: Fix mediatek-drm device probing
>>>>
>>>> Matthias Brugger (4):
>>>>   drm/mediatek: Use regmap for register access
>>>>   drm/mediatek: Omit warning on probe defers
>>>>   media: mtk-mdp: Check return value of of_clk_get
>>>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
>>>>
>>>>  drivers/clk/mediatek/Kconfig                  |   6 +
>>>>  drivers/clk/mediatek/Makefile                 |   1 +
>>>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
>>>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
>>>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
>>>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
>>>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
>>>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
>>>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
>>>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
>>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
>>>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
>>>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
>>>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
>>>>  20 files changed, 401 insertions(+), 317 deletions(-)
>>>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
>>>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
>>>>
>>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> 

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[CK Hu]

Hi, Matthias:

On Fri, 2020-02-21 at 11:24 +0100, Matthias Brugger wrote:
> 
> On 21/02/2020 10:27, CK Hu wrote:
> > Hi, Enric:
> > 
> > On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
> >> Hi CK,
> >>
> >> Thanks for your quick answer.
> >>
> >> On 21/2/20 5:39, CK Hu wrote:
> >>> Hi, Enric:
> >>>
> >>> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
> >>>> Dear all,
> >>>>
> >>>> Those patches are intended to solve an old standing issue on some
> >>>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
> >>>> to the precedent series.
> >>>>
> >>>> Up to now both drivers, clock and drm are probed with the same device tree
> >>>> compatible. But only the first driver get probed, which in effect breaks
> >>>> graphics on those devices.
> >>>>
> >>>> The version eight of the series tries to solve the problem with a
> >>>> different approach than the previous series but similar to how is solved
> >>>> on other Mediatek devices.
> >>>>
> >>>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
> >>>> control clock gates (which is used in the clk driver) and some registers
> >>>> to set the routing and enable the differnet blocks of the display
> >>>> and MDP (Media Data Path) subsystem. On this series the clk driver is
> >>>> not a pure clock controller but a system controller that can provide
> >>>> access to the shared registers between the different drivers that need
> >>>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
> >>>> this version, clk driver is the entry point (parent) which will trigger
> >>>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
> >>>> platform data for display configuration.
> >>>
> >>> When mmsys is a system controller, I prefer to place mmsys in
> >>> drivers/soc/mediatek, and it share registers for clock, display, and mdp
> >>> driver. This means the probe function is placed in
> >>> drivers/soc/mediatek ,its display clock function, mdp clock function are
> >>> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
> >>> and mdp routing are placed in dirvers/video.
> >>>
> >>
> >> I understand what you mean but I am not sure this makes the code clearer and
> >> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
> >> of a "simple-mfd" device (a driver that simply matches with
> >> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
> >> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
> >> device-tree).
> >>
> > 
> > It's clear that mmsys is neither a pure clock controller nor a pure
> > routing controller for display and mdp. 
> > 
> >> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
> >> beginning representing how really hardwware is, but I think that, change this
> >> now, will break backward compatibility.
> > 
> > Maybe this is a solution. Current device tree would work only on old
> > kernel version with a bug, so this mean there is no any device tree
> > works on kernel version without bug. Why do we compatible with such
> > device tree?
> > 
> 
> The idea behind this is, that the device-tree could be passed by some boot
> firmware, so that the OS do not care about it. For this we need a stable DTS as
> otherwise newer kernel with older FW would break.
> 
> DTS is supposed to be just a different description of the HW like ACPI. So it
> has to be compatible (newer kernel with older DTS and if possible vice versa).

In my view, there is no FW (except some bug-inside FW) which works on
this dts, so this dts is in a initial state. I think the compatibility
is based on that dts correctly describe the HW. If we find this dts does
not correctly describe the HW and it's in a initial state, should we
still make it compatible?

If you have better solution, just let's forget this.

Regards,
CK

> 
> Regards,
> Matthias
> 
> > Regards,
> > CK
> > 
> >>
> >> IMHO I think that considering the clk driver as entry point is fine, but this is
> >> something that the clock maintainers should decide.
> >>
> >> Also note that this is not only a MT8173 problem I am seeing the same problem on
> >> all other Mediatek SoCs.
> >>
> >> Thanks.
> >>
> >>> Regards,
> >>> CK
> >>>
> >>>>
> >>>> All this series was tested on the Acer R13 Chromebook only.
> >>>>
> >>>> For reference, here are the links to the old discussions:
> >>>>
> >>>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
> >>>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
> >>>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
> >>>> * v4:
> >>>>   * https://patchwork.kernel.org/patch/10530871/
> >>>>   * https://patchwork.kernel.org/patch/10530883/
> >>>>   * https://patchwork.kernel.org/patch/10530885/
> >>>>   * https://patchwork.kernel.org/patch/10530911/
> >>>>   * https://patchwork.kernel.org/patch/10530913/
> >>>> * v3:
> >>>>   * https://patchwork.kernel.org/patch/10367857/
> >>>>   * https://patchwork.kernel.org/patch/10367861/
> >>>>   * https://patchwork.kernel.org/patch/10367877/
> >>>>   * https://patchwork.kernel.org/patch/10367875/
> >>>>   * https://patchwork.kernel.org/patch/10367885/
> >>>>   * https://patchwork.kernel.org/patch/10367883/
> >>>>   * https://patchwork.kernel.org/patch/10367889/
> >>>>   * https://patchwork.kernel.org/patch/10367907/
> >>>>   * https://patchwork.kernel.org/patch/10367909/
> >>>>   * https://patchwork.kernel.org/patch/10367905/
> >>>> * v2: No relevant discussion, see v3
> >>>> * v1:
> >>>>   * https://patchwork.kernel.org/patch/10016497/
> >>>>   * https://patchwork.kernel.org/patch/10016499/
> >>>>   * https://patchwork.kernel.org/patch/10016505/
> >>>>   * https://patchwork.kernel.org/patch/10016507/
> >>>>
> >>>> Best regards,
> >>>>  Enric
> >>>>
> >>>> Changes in v8:
> >>>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
> >>>> - New patches introduced in this series.
> >>>>
> >>>> Changes in v7:
> >>>> - Add R-by from CK
> >>>> - Add R-by from CK
> >>>> - Fix check of return value of of_clk_get
> >>>> - Fix identation
> >>>> - Free clk_data->clks as well
> >>>> - Get rid of private data structure
> >>>>
> >>>> Enric Balletbo i Serra (2):
> >>>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
> >>>>   clk/drm: mediatek: Fix mediatek-drm device probing
> >>>>
> >>>> Matthias Brugger (4):
> >>>>   drm/mediatek: Use regmap for register access
> >>>>   drm/mediatek: Omit warning on probe defers
> >>>>   media: mtk-mdp: Check return value of of_clk_get
> >>>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
> >>>>
> >>>>  drivers/clk/mediatek/Kconfig                  |   6 +
> >>>>  drivers/clk/mediatek/Makefile                 |   1 +
> >>>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
> >>>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
> >>>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
> >>>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
> >>>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
> >>>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
> >>>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
> >>>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
> >>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
> >>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
> >>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
> >>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
> >>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
> >>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
> >>>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
> >>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
> >>>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
> >>>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
> >>>>  20 files changed, 401 insertions(+), 317 deletions(-)
> >>>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
> >>>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
> >>>>
> >>>
> >>
> >> _______________________________________________
> >> Linux-mediatek mailing list
> >> Linux-mediatek@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> > 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Enric Balletbo i Serra]

Hi CK and Matthias,

On 21/2/20 12:11, CK Hu wrote:
> Hi, Matthias:
> 
> On Fri, 2020-02-21 at 11:24 +0100, Matthias Brugger wrote:
>>
>> On 21/02/2020 10:27, CK Hu wrote:
>>> Hi, Enric:
>>>
>>> On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
>>>> Hi CK,
>>>>
>>>> Thanks for your quick answer.
>>>>
>>>> On 21/2/20 5:39, CK Hu wrote:
>>>>> Hi, Enric:
>>>>>
>>>>> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
>>>>>> Dear all,
>>>>>>
>>>>>> Those patches are intended to solve an old standing issue on some
>>>>>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
>>>>>> to the precedent series.
>>>>>>
>>>>>> Up to now both drivers, clock and drm are probed with the same device tree
>>>>>> compatible. But only the first driver get probed, which in effect breaks
>>>>>> graphics on those devices.
>>>>>>
>>>>>> The version eight of the series tries to solve the problem with a
>>>>>> different approach than the previous series but similar to how is solved
>>>>>> on other Mediatek devices.
>>>>>>
>>>>>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
>>>>>> control clock gates (which is used in the clk driver) and some registers
>>>>>> to set the routing and enable the differnet blocks of the display
>>>>>> and MDP (Media Data Path) subsystem. On this series the clk driver is
>>>>>> not a pure clock controller but a system controller that can provide
>>>>>> access to the shared registers between the different drivers that need
>>>>>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
>>>>>> this version, clk driver is the entry point (parent) which will trigger
>>>>>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
>>>>>> platform data for display configuration.
>>>>>
>>>>> When mmsys is a system controller, I prefer to place mmsys in
>>>>> drivers/soc/mediatek, and it share registers for clock, display, and mdp
>>>>> driver. This means the probe function is placed in
>>>>> drivers/soc/mediatek ,its display clock function, mdp clock function are
>>>>> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
>>>>> and mdp routing are placed in dirvers/video.
>>>>>
>>>>
>>>> I understand what you mean but I am not sure this makes the code clearer and
>>>> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
>>>> of a "simple-mfd" device (a driver that simply matches with
>>>> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
>>>> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
>>>> device-tree).
>>>>
>>>
>>> It's clear that mmsys is neither a pure clock controller nor a pure
>>> routing controller for display and mdp. 
>>>
>>>> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
>>>> beginning representing how really hardwware is, but I think that, change this
>>>> now, will break backward compatibility.
>>>
>>> Maybe this is a solution. Current device tree would work only on old
>>> kernel version with a bug, so this mean there is no any device tree
>>> works on kernel version without bug. Why do we compatible with such
>>> device tree?
>>>
>>

So the only reason why current DT worked at some point is because there was a
kernel bug?

If that's the case I think we shouldn't worry about break DT compatibility (I'm
sorry for those that having a buggy kernel makes display working)

>> The idea behind this is, that the device-tree could be passed by some boot
>> firmware, so that the OS do not care about it. For this we need a stable DTS as
>> otherwise newer kernel with older FW would break.
>>
>> DTS is supposed to be just a different description of the HW like ACPI. So it
>> has to be compatible (newer kernel with older DTS and if possible vice versa).
> 
> In my view, there is no FW (except some bug-inside FW) which works on
> this dts, so this dts is in a initial state. I think the compatibility
> is based on that dts correctly describe the HW. If we find this dts does
> not correctly describe the HW and it's in a initial state, should we
> still make it compatible?
> 

In this case I think we don't need to worry about buggy kernels, the only thing
that we need to take in consideration is that mmsys is instantiated on both (the
old DT and the new DT), we shouldn't expect display working (because never
worked, right?)

With that in mind, I agree that is a good opportunity to fix properly the HW
topology.

What thing that worries me is that I see this pattern on all Mediatek SoCs, does
this mean that display was never well supported for Mediatek SoCs?

Thanks.

> If you have better solution, just let's forget this.
> 
> Regards,
> CK
> 
>>
>> Regards,
>> Matthias
>>
>>> Regards,
>>> CK
>>>
>>>>
>>>> IMHO I think that considering the clk driver as entry point is fine, but this is
>>>> something that the clock maintainers should decide.
>>>>
>>>> Also note that this is not only a MT8173 problem I am seeing the same problem on
>>>> all other Mediatek SoCs.
>>>>
>>>> Thanks.
>>>>
>>>>> Regards,
>>>>> CK
>>>>>
>>>>>>
>>>>>> All this series was tested on the Acer R13 Chromebook only.
>>>>>>
>>>>>> For reference, here are the links to the old discussions:
>>>>>>
>>>>>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
>>>>>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
>>>>>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
>>>>>> * v4:
>>>>>>   * https://patchwork.kernel.org/patch/10530871/
>>>>>>   * https://patchwork.kernel.org/patch/10530883/
>>>>>>   * https://patchwork.kernel.org/patch/10530885/
>>>>>>   * https://patchwork.kernel.org/patch/10530911/
>>>>>>   * https://patchwork.kernel.org/patch/10530913/
>>>>>> * v3:
>>>>>>   * https://patchwork.kernel.org/patch/10367857/
>>>>>>   * https://patchwork.kernel.org/patch/10367861/
>>>>>>   * https://patchwork.kernel.org/patch/10367877/
>>>>>>   * https://patchwork.kernel.org/patch/10367875/
>>>>>>   * https://patchwork.kernel.org/patch/10367885/
>>>>>>   * https://patchwork.kernel.org/patch/10367883/
>>>>>>   * https://patchwork.kernel.org/patch/10367889/
>>>>>>   * https://patchwork.kernel.org/patch/10367907/
>>>>>>   * https://patchwork.kernel.org/patch/10367909/
>>>>>>   * https://patchwork.kernel.org/patch/10367905/
>>>>>> * v2: No relevant discussion, see v3
>>>>>> * v1:
>>>>>>   * https://patchwork.kernel.org/patch/10016497/
>>>>>>   * https://patchwork.kernel.org/patch/10016499/
>>>>>>   * https://patchwork.kernel.org/patch/10016505/
>>>>>>   * https://patchwork.kernel.org/patch/10016507/
>>>>>>
>>>>>> Best regards,
>>>>>>  Enric
>>>>>>
>>>>>> Changes in v8:
>>>>>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>>>>>> - New patches introduced in this series.
>>>>>>
>>>>>> Changes in v7:
>>>>>> - Add R-by from CK
>>>>>> - Add R-by from CK
>>>>>> - Fix check of return value of of_clk_get
>>>>>> - Fix identation
>>>>>> - Free clk_data->clks as well
>>>>>> - Get rid of private data structure
>>>>>>
>>>>>> Enric Balletbo i Serra (2):
>>>>>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
>>>>>>   clk/drm: mediatek: Fix mediatek-drm device probing
>>>>>>
>>>>>> Matthias Brugger (4):
>>>>>>   drm/mediatek: Use regmap for register access
>>>>>>   drm/mediatek: Omit warning on probe defers
>>>>>>   media: mtk-mdp: Check return value of of_clk_get
>>>>>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
>>>>>>
>>>>>>  drivers/clk/mediatek/Kconfig                  |   6 +
>>>>>>  drivers/clk/mediatek/Makefile                 |   1 +
>>>>>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
>>>>>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
>>>>>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
>>>>>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
>>>>>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
>>>>>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
>>>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
>>>>>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
>>>>>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
>>>>>>  20 files changed, 401 insertions(+), 317 deletions(-)
>>>>>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
>>>>>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
>>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-mediatek mailing list
>>>> Linux-mediatek@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> 

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Matthias Brugger]

On 21/02/2020 12:37, Enric Balletbo i Serra wrote:
> Hi CK and Matthias,
> 
> On 21/2/20 12:11, CK Hu wrote:
>> Hi, Matthias:
>>
>> On Fri, 2020-02-21 at 11:24 +0100, Matthias Brugger wrote:
>>>
>>> On 21/02/2020 10:27, CK Hu wrote:
>>>> Hi, Enric:
>>>>
>>>> On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
>>>>> Hi CK,
>>>>>
>>>>> Thanks for your quick answer.
>>>>>
>>>>> On 21/2/20 5:39, CK Hu wrote:
>>>>>> Hi, Enric:
>>>>>>
>>>>>> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
>>>>>>> Dear all,
>>>>>>>
>>>>>>> Those patches are intended to solve an old standing issue on some
>>>>>>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
>>>>>>> to the precedent series.
>>>>>>>
>>>>>>> Up to now both drivers, clock and drm are probed with the same device tree
>>>>>>> compatible. But only the first driver get probed, which in effect breaks
>>>>>>> graphics on those devices.
>>>>>>>
>>>>>>> The version eight of the series tries to solve the problem with a
>>>>>>> different approach than the previous series but similar to how is solved
>>>>>>> on other Mediatek devices.
>>>>>>>
>>>>>>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
>>>>>>> control clock gates (which is used in the clk driver) and some registers
>>>>>>> to set the routing and enable the differnet blocks of the display
>>>>>>> and MDP (Media Data Path) subsystem. On this series the clk driver is
>>>>>>> not a pure clock controller but a system controller that can provide
>>>>>>> access to the shared registers between the different drivers that need
>>>>>>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
>>>>>>> this version, clk driver is the entry point (parent) which will trigger
>>>>>>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
>>>>>>> platform data for display configuration.
>>>>>>
>>>>>> When mmsys is a system controller, I prefer to place mmsys in
>>>>>> drivers/soc/mediatek, and it share registers for clock, display, and mdp
>>>>>> driver. This means the probe function is placed in
>>>>>> drivers/soc/mediatek ,its display clock function, mdp clock function are
>>>>>> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
>>>>>> and mdp routing are placed in dirvers/video.
>>>>>>
>>>>>
>>>>> I understand what you mean but I am not sure this makes the code clearer and
>>>>> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
>>>>> of a "simple-mfd" device (a driver that simply matches with
>>>>> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
>>>>> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
>>>>> device-tree).
>>>>>
>>>>
>>>> It's clear that mmsys is neither a pure clock controller nor a pure
>>>> routing controller for display and mdp. 
>>>>
>>>>> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
>>>>> beginning representing how really hardwware is, but I think that, change this
>>>>> now, will break backward compatibility.
>>>>
>>>> Maybe this is a solution. Current device tree would work only on old
>>>> kernel version with a bug, so this mean there is no any device tree
>>>> works on kernel version without bug. Why do we compatible with such
>>>> device tree?
>>>>
>>>
> 
> So the only reason why current DT worked at some point is because there was a
> kernel bug?
> 

I'd say you can call it a kernel bug:
https://patchwork.kernel.org/patch/10367877/#22078767


> If that's the case I think we shouldn't worry about break DT compatibility (I'm
> sorry for those that having a buggy kernel makes display working)
> 
>>> The idea behind this is, that the device-tree could be passed by some boot
>>> firmware, so that the OS do not care about it. For this we need a stable DTS as
>>> otherwise newer kernel with older FW would break.
>>>
>>> DTS is supposed to be just a different description of the HW like ACPI. So it
>>> has to be compatible (newer kernel with older DTS and if possible vice versa).
>>
>> In my view, there is no FW (except some bug-inside FW) which works on
>> this dts, so this dts is in a initial state. I think the compatibility
>> is based on that dts correctly describe the HW. If we find this dts does
>> not correctly describe the HW and it's in a initial state, should we
>> still make it compatible?
>>
> 
> In this case I think we don't need to worry about buggy kernels, the only thing
> that we need to take in consideration is that mmsys is instantiated on both (the
> old DT and the new DT), we shouldn't expect display working (because never
> worked, right?)
> 
> With that in mind, I agree that is a good opportunity to fix properly the HW
> topology.
> 
> What thing that worries me is that I see this pattern on all Mediatek SoCs, does
> this mean that display was never well supported for Mediatek SoCs?
> 

That's exactly the case. Actually there were some patches for mt762x (can't
remember if mt7623 or mt7622) whcih got pushed back, because we would need to
fix the mmsys problem first.

Ok, so if we come to the conclusion that this actually only worked because of a
bug, then we can remodel the whole thing.
In this case, I think the best would be to do what CK proposed:
https://patchwork.kernel.org/patch/11381201/#23158169

Making everything below 0x14000000 a subdevice of mmsys (mdp, ovl, rdma, you
name it).

Regards,
Matthias

> Thanks.
> 
>> If you have better solution, just let's forget this.
>>
>> Regards,
>> CK
>>
>>>
>>> Regards,
>>> Matthias
>>>
>>>> Regards,
>>>> CK
>>>>
>>>>>
>>>>> IMHO I think that considering the clk driver as entry point is fine, but this is
>>>>> something that the clock maintainers should decide.
>>>>>
>>>>> Also note that this is not only a MT8173 problem I am seeing the same problem on
>>>>> all other Mediatek SoCs.
>>>>>
>>>>> Thanks.
>>>>>
>>>>>> Regards,
>>>>>> CK
>>>>>>
>>>>>>>
>>>>>>> All this series was tested on the Acer R13 Chromebook only.
>>>>>>>
>>>>>>> For reference, here are the links to the old discussions:
>>>>>>>
>>>>>>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
>>>>>>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
>>>>>>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
>>>>>>> * v4:
>>>>>>>   * https://patchwork.kernel.org/patch/10530871/
>>>>>>>   * https://patchwork.kernel.org/patch/10530883/
>>>>>>>   * https://patchwork.kernel.org/patch/10530885/
>>>>>>>   * https://patchwork.kernel.org/patch/10530911/
>>>>>>>   * https://patchwork.kernel.org/patch/10530913/
>>>>>>> * v3:
>>>>>>>   * https://patchwork.kernel.org/patch/10367857/
>>>>>>>   * https://patchwork.kernel.org/patch/10367861/
>>>>>>>   * https://patchwork.kernel.org/patch/10367877/
>>>>>>>   * https://patchwork.kernel.org/patch/10367875/
>>>>>>>   * https://patchwork.kernel.org/patch/10367885/
>>>>>>>   * https://patchwork.kernel.org/patch/10367883/
>>>>>>>   * https://patchwork.kernel.org/patch/10367889/
>>>>>>>   * https://patchwork.kernel.org/patch/10367907/
>>>>>>>   * https://patchwork.kernel.org/patch/10367909/
>>>>>>>   * https://patchwork.kernel.org/patch/10367905/
>>>>>>> * v2: No relevant discussion, see v3
>>>>>>> * v1:
>>>>>>>   * https://patchwork.kernel.org/patch/10016497/
>>>>>>>   * https://patchwork.kernel.org/patch/10016499/
>>>>>>>   * https://patchwork.kernel.org/patch/10016505/
>>>>>>>   * https://patchwork.kernel.org/patch/10016507/
>>>>>>>
>>>>>>> Best regards,
>>>>>>>  Enric
>>>>>>>
>>>>>>> Changes in v8:
>>>>>>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>>>>>>> - New patches introduced in this series.
>>>>>>>
>>>>>>> Changes in v7:
>>>>>>> - Add R-by from CK
>>>>>>> - Add R-by from CK
>>>>>>> - Fix check of return value of of_clk_get
>>>>>>> - Fix identation
>>>>>>> - Free clk_data->clks as well
>>>>>>> - Get rid of private data structure
>>>>>>>
>>>>>>> Enric Balletbo i Serra (2):
>>>>>>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
>>>>>>>   clk/drm: mediatek: Fix mediatek-drm device probing
>>>>>>>
>>>>>>> Matthias Brugger (4):
>>>>>>>   drm/mediatek: Use regmap for register access
>>>>>>>   drm/mediatek: Omit warning on probe defers
>>>>>>>   media: mtk-mdp: Check return value of of_clk_get
>>>>>>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
>>>>>>>
>>>>>>>  drivers/clk/mediatek/Kconfig                  |   6 +
>>>>>>>  drivers/clk/mediatek/Makefile                 |   1 +
>>>>>>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
>>>>>>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
>>>>>>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
>>>>>>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
>>>>>>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
>>>>>>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
>>>>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
>>>>>>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
>>>>>>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
>>>>>>>  20 files changed, 401 insertions(+), 317 deletions(-)
>>>>>>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
>>>>>>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
>>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Linux-mediatek mailing list
>>>>> Linux-mediatek@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>>>
>>>
>>> _______________________________________________
>>> Linux-mediatek mailing list
>>> Linux-mediatek@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Enric Balletbo i Serra]

Hi,

On 21/2/20 12:53, Matthias Brugger wrote:
> 
> 
> On 21/02/2020 12:37, Enric Balletbo i Serra wrote:
>> Hi CK and Matthias,
>>
>> On 21/2/20 12:11, CK Hu wrote:
>>> Hi, Matthias:
>>>
>>> On Fri, 2020-02-21 at 11:24 +0100, Matthias Brugger wrote:
>>>>
>>>> On 21/02/2020 10:27, CK Hu wrote:
>>>>> Hi, Enric:
>>>>>
>>>>> On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
>>>>>> Hi CK,
>>>>>>
>>>>>> Thanks for your quick answer.
>>>>>>
>>>>>> On 21/2/20 5:39, CK Hu wrote:
>>>>>>> Hi, Enric:
>>>>>>>
>>>>>>> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
>>>>>>>> Dear all,
>>>>>>>>
>>>>>>>> Those patches are intended to solve an old standing issue on some
>>>>>>>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
>>>>>>>> to the precedent series.
>>>>>>>>
>>>>>>>> Up to now both drivers, clock and drm are probed with the same device tree
>>>>>>>> compatible. But only the first driver get probed, which in effect breaks
>>>>>>>> graphics on those devices.
>>>>>>>>
>>>>>>>> The version eight of the series tries to solve the problem with a
>>>>>>>> different approach than the previous series but similar to how is solved
>>>>>>>> on other Mediatek devices.
>>>>>>>>
>>>>>>>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
>>>>>>>> control clock gates (which is used in the clk driver) and some registers
>>>>>>>> to set the routing and enable the differnet blocks of the display
>>>>>>>> and MDP (Media Data Path) subsystem. On this series the clk driver is
>>>>>>>> not a pure clock controller but a system controller that can provide
>>>>>>>> access to the shared registers between the different drivers that need
>>>>>>>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
>>>>>>>> this version, clk driver is the entry point (parent) which will trigger
>>>>>>>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
>>>>>>>> platform data for display configuration.
>>>>>>>
>>>>>>> When mmsys is a system controller, I prefer to place mmsys in
>>>>>>> drivers/soc/mediatek, and it share registers for clock, display, and mdp
>>>>>>> driver. This means the probe function is placed in
>>>>>>> drivers/soc/mediatek ,its display clock function, mdp clock function are
>>>>>>> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
>>>>>>> and mdp routing are placed in dirvers/video.
>>>>>>>
>>>>>>
>>>>>> I understand what you mean but I am not sure this makes the code clearer and
>>>>>> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
>>>>>> of a "simple-mfd" device (a driver that simply matches with
>>>>>> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
>>>>>> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
>>>>>> device-tree).
>>>>>>
>>>>>
>>>>> It's clear that mmsys is neither a pure clock controller nor a pure
>>>>> routing controller for display and mdp. 
>>>>>
>>>>>> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
>>>>>> beginning representing how really hardwware is, but I think that, change this
>>>>>> now, will break backward compatibility.
>>>>>
>>>>> Maybe this is a solution. Current device tree would work only on old
>>>>> kernel version with a bug, so this mean there is no any device tree
>>>>> works on kernel version without bug. Why do we compatible with such
>>>>> device tree?
>>>>>
>>>>
>>
>> So the only reason why current DT worked at some point is because there was a
>> kernel bug?
>>
> 
> I'd say you can call it a kernel bug:
> https://patchwork.kernel.org/patch/10367877/#22078767
> 
> 
>> If that's the case I think we shouldn't worry about break DT compatibility (I'm
>> sorry for those that having a buggy kernel makes display working)
>>
>>>> The idea behind this is, that the device-tree could be passed by some boot
>>>> firmware, so that the OS do not care about it. For this we need a stable DTS as
>>>> otherwise newer kernel with older FW would break.
>>>>
>>>> DTS is supposed to be just a different description of the HW like ACPI. So it
>>>> has to be compatible (newer kernel with older DTS and if possible vice versa).
>>>
>>> In my view, there is no FW (except some bug-inside FW) which works on
>>> this dts, so this dts is in a initial state. I think the compatibility
>>> is based on that dts correctly describe the HW. If we find this dts does
>>> not correctly describe the HW and it's in a initial state, should we
>>> still make it compatible?
>>>
>>
>> In this case I think we don't need to worry about buggy kernels, the only thing
>> that we need to take in consideration is that mmsys is instantiated on both (the
>> old DT and the new DT), we shouldn't expect display working (because never
>> worked, right?)
>>
>> With that in mind, I agree that is a good opportunity to fix properly the HW
>> topology.
>>
>> What thing that worries me is that I see this pattern on all Mediatek SoCs, does
>> this mean that display was never well supported for Mediatek SoCs?
>>
> 
> That's exactly the case. Actually there were some patches for mt762x (can't
> remember if mt7623 or mt7622) whcih got pushed back, because we would need to
> fix the mmsys problem first.
> 
> Ok, so if we come to the conclusion that this actually only worked because of a
> bug, then we can remodel the whole thing.
> In this case, I think the best would be to do what CK proposed:
> https://patchwork.kernel.org/patch/11381201/#23158169
> 
> Making everything below 0x14000000 a subdevice of mmsys (mdp, ovl, rdma, you
> name it).
> 


I see the MMSYS space as config registers to route the different ports in the
drm and video subsystem, so, as phandle of the display (drivers/gpur/drm) and
video (drivers/video) subsystem. What about something like this?

mmsys: syscon@14000000 {
	compatible = "mediatek,mt8173-mmsys", "syscon";
	reg = <0 0x14000000 0 0x1000>;
	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
	assigned-clock-rates = <400000000>;
	#clock-cells = <1>;
};

Basically is what we have with

* [PATCH v8 4/6] clk: mediatek: mt8173: Switch MMSYS to platform driver

And

display-subsystem {
	compatible = "mediatek,display-subsystem";
	mediatek,mmsys = <&mmsys>; /* phandle to the routing config registers */
	ports = <&ovl0>, <&ovl1>, < ... >
}

We are introducing a new compatible that is not describing hardware but how
hardware is grouped, which I think is fine.

The mediatek-drm driver will need a bit of rework but not much I guess.

What is still not clear to me is the mdp part because doesn't seem to have an
entry point like the display part, instead it uses one port as entry point.

	mdp_rdma0: rdma@14001000 {
		compatible = "mediatek,mt8173-mdp-rdma",
			     "mediatek,mt8173-mdp";

AFAICS that driver is not touching MMSYS config registers to route the mdp path,
only is getting the clocks, but I assume it will do in the future. In such case,
maybe we could do something similar to the display-subsystem node?

Regards,
 Enric


> Regards,
> Matthias
> 
>> Thanks.
>>
>>> If you have better solution, just let's forget this.
>>>
>>> Regards,
>>> CK
>>>
>>>>
>>>> Regards,
>>>> Matthias
>>>>
>>>>> Regards,
>>>>> CK
>>>>>
>>>>>>
>>>>>> IMHO I think that considering the clk driver as entry point is fine, but this is
>>>>>> something that the clock maintainers should decide.
>>>>>>
>>>>>> Also note that this is not only a MT8173 problem I am seeing the same problem on
>>>>>> all other Mediatek SoCs.
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>>> Regards,
>>>>>>> CK
>>>>>>>
>>>>>>>>
>>>>>>>> All this series was tested on the Acer R13 Chromebook only.
>>>>>>>>
>>>>>>>> For reference, here are the links to the old discussions:
>>>>>>>>
>>>>>>>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
>>>>>>>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
>>>>>>>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
>>>>>>>> * v4:
>>>>>>>>   * https://patchwork.kernel.org/patch/10530871/
>>>>>>>>   * https://patchwork.kernel.org/patch/10530883/
>>>>>>>>   * https://patchwork.kernel.org/patch/10530885/
>>>>>>>>   * https://patchwork.kernel.org/patch/10530911/
>>>>>>>>   * https://patchwork.kernel.org/patch/10530913/
>>>>>>>> * v3:
>>>>>>>>   * https://patchwork.kernel.org/patch/10367857/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367861/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367877/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367875/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367885/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367883/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367889/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367907/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367909/
>>>>>>>>   * https://patchwork.kernel.org/patch/10367905/
>>>>>>>> * v2: No relevant discussion, see v3
>>>>>>>> * v1:
>>>>>>>>   * https://patchwork.kernel.org/patch/10016497/
>>>>>>>>   * https://patchwork.kernel.org/patch/10016499/
>>>>>>>>   * https://patchwork.kernel.org/patch/10016505/
>>>>>>>>   * https://patchwork.kernel.org/patch/10016507/
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>>  Enric
>>>>>>>>
>>>>>>>> Changes in v8:
>>>>>>>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>>>>>>>> - New patches introduced in this series.
>>>>>>>>
>>>>>>>> Changes in v7:
>>>>>>>> - Add R-by from CK
>>>>>>>> - Add R-by from CK
>>>>>>>> - Fix check of return value of of_clk_get
>>>>>>>> - Fix identation
>>>>>>>> - Free clk_data->clks as well
>>>>>>>> - Get rid of private data structure
>>>>>>>>
>>>>>>>> Enric Balletbo i Serra (2):
>>>>>>>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
>>>>>>>>   clk/drm: mediatek: Fix mediatek-drm device probing
>>>>>>>>
>>>>>>>> Matthias Brugger (4):
>>>>>>>>   drm/mediatek: Use regmap for register access
>>>>>>>>   drm/mediatek: Omit warning on probe defers
>>>>>>>>   media: mtk-mdp: Check return value of of_clk_get
>>>>>>>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
>>>>>>>>
>>>>>>>>  drivers/clk/mediatek/Kconfig                  |   6 +
>>>>>>>>  drivers/clk/mediatek/Makefile                 |   1 +
>>>>>>>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
>>>>>>>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
>>>>>>>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
>>>>>>>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
>>>>>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
>>>>>>>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
>>>>>>>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
>>>>>>>>  20 files changed, 401 insertions(+), 317 deletions(-)
>>>>>>>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
>>>>>>>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Linux-mediatek mailing list
>>>>>> Linux-mediatek@lists.infradead.org
>>>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-mediatek mailing list
>>>> Linux-mediatek@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>>
> 

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[CK Hu]

Hi,

On Fri, 2020-02-21 at 18:10 +0100, Enric Balletbo i Serra wrote:
> Hi,
> 
> On 21/2/20 12:53, Matthias Brugger wrote:
> > 
> > 
> > On 21/02/2020 12:37, Enric Balletbo i Serra wrote:
> >> Hi CK and Matthias,
> >>
> >> On 21/2/20 12:11, CK Hu wrote:
> >>> Hi, Matthias:
> >>>
> >>> On Fri, 2020-02-21 at 11:24 +0100, Matthias Brugger wrote:
> >>>>
> >>>> On 21/02/2020 10:27, CK Hu wrote:
> >>>>> Hi, Enric:
> >>>>>
> >>>>> On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
> >>>>>> Hi CK,
> >>>>>>
> >>>>>> Thanks for your quick answer.
> >>>>>>
> >>>>>> On 21/2/20 5:39, CK Hu wrote:
> >>>>>>> Hi, Enric:
> >>>>>>>
> >>>>>>> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
> >>>>>>>> Dear all,
> >>>>>>>>
> >>>>>>>> Those patches are intended to solve an old standing issue on some
> >>>>>>>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
> >>>>>>>> to the precedent series.
> >>>>>>>>
> >>>>>>>> Up to now both drivers, clock and drm are probed with the same device tree
> >>>>>>>> compatible. But only the first driver get probed, which in effect breaks
> >>>>>>>> graphics on those devices.
> >>>>>>>>
> >>>>>>>> The version eight of the series tries to solve the problem with a
> >>>>>>>> different approach than the previous series but similar to how is solved
> >>>>>>>> on other Mediatek devices.
> >>>>>>>>
> >>>>>>>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
> >>>>>>>> control clock gates (which is used in the clk driver) and some registers
> >>>>>>>> to set the routing and enable the differnet blocks of the display
> >>>>>>>> and MDP (Media Data Path) subsystem. On this series the clk driver is
> >>>>>>>> not a pure clock controller but a system controller that can provide
> >>>>>>>> access to the shared registers between the different drivers that need
> >>>>>>>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
> >>>>>>>> this version, clk driver is the entry point (parent) which will trigger
> >>>>>>>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
> >>>>>>>> platform data for display configuration.
> >>>>>>>
> >>>>>>> When mmsys is a system controller, I prefer to place mmsys in
> >>>>>>> drivers/soc/mediatek, and it share registers for clock, display, and mdp
> >>>>>>> driver. This means the probe function is placed in
> >>>>>>> drivers/soc/mediatek ,its display clock function, mdp clock function are
> >>>>>>> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
> >>>>>>> and mdp routing are placed in dirvers/video.
> >>>>>>>
> >>>>>>
> >>>>>> I understand what you mean but I am not sure this makes the code clearer and
> >>>>>> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
> >>>>>> of a "simple-mfd" device (a driver that simply matches with
> >>>>>> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
> >>>>>> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
> >>>>>> device-tree).
> >>>>>>
> >>>>>
> >>>>> It's clear that mmsys is neither a pure clock controller nor a pure
> >>>>> routing controller for display and mdp. 
> >>>>>
> >>>>>> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
> >>>>>> beginning representing how really hardwware is, but I think that, change this
> >>>>>> now, will break backward compatibility.
> >>>>>
> >>>>> Maybe this is a solution. Current device tree would work only on old
> >>>>> kernel version with a bug, so this mean there is no any device tree
> >>>>> works on kernel version without bug. Why do we compatible with such
> >>>>> device tree?
> >>>>>
> >>>>
> >>
> >> So the only reason why current DT worked at some point is because there was a
> >> kernel bug?
> >>
> > 
> > I'd say you can call it a kernel bug:
> > https://patchwork.kernel.org/patch/10367877/#22078767
> > 
> > 
> >> If that's the case I think we shouldn't worry about break DT compatibility (I'm
> >> sorry for those that having a buggy kernel makes display working)
> >>
> >>>> The idea behind this is, that the device-tree could be passed by some boot
> >>>> firmware, so that the OS do not care about it. For this we need a stable DTS as
> >>>> otherwise newer kernel with older FW would break.
> >>>>
> >>>> DTS is supposed to be just a different description of the HW like ACPI. So it
> >>>> has to be compatible (newer kernel with older DTS and if possible vice versa).
> >>>
> >>> In my view, there is no FW (except some bug-inside FW) which works on
> >>> this dts, so this dts is in a initial state. I think the compatibility
> >>> is based on that dts correctly describe the HW. If we find this dts does
> >>> not correctly describe the HW and it's in a initial state, should we
> >>> still make it compatible?
> >>>
> >>
> >> In this case I think we don't need to worry about buggy kernels, the only thing
> >> that we need to take in consideration is that mmsys is instantiated on both (the
> >> old DT and the new DT), we shouldn't expect display working (because never
> >> worked, right?)
> >>
> >> With that in mind, I agree that is a good opportunity to fix properly the HW
> >> topology.
> >>
> >> What thing that worries me is that I see this pattern on all Mediatek SoCs, does
> >> this mean that display was never well supported for Mediatek SoCs?
> >>
> > 
> > That's exactly the case. Actually there were some patches for mt762x (can't
> > remember if mt7623 or mt7622) whcih got pushed back, because we would need to
> > fix the mmsys problem first.
> > 
> > Ok, so if we come to the conclusion that this actually only worked because of a
> > bug, then we can remodel the whole thing.
> > In this case, I think the best would be to do what CK proposed:
> > https://patchwork.kernel.org/patch/11381201/#23158169
> > 
> > Making everything below 0x14000000 a subdevice of mmsys (mdp, ovl, rdma, you
> > name it).
> > 
> 
> 
> I see the MMSYS space as config registers to route the different ports in the
> drm and video subsystem, so, as phandle of the display (drivers/gpur/drm) and
> video (drivers/video) subsystem. What about something like this?
> 
> mmsys: syscon@14000000 {
> 	compatible = "mediatek,mt8173-mmsys", "syscon";
> 	reg = <0 0x14000000 0 0x1000>;
> 	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> 	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
> 	assigned-clock-rates = <400000000>;
> 	#clock-cells = <1>;
> };
> 
> Basically is what we have with
> 
> * [PATCH v8 4/6] clk: mediatek: mt8173: Switch MMSYS to platform driver
> 
> And
> 
> display-subsystem {
> 	compatible = "mediatek,display-subsystem";
> 	mediatek,mmsys = <&mmsys>; /* phandle to the routing config registers */
> 	ports = <&ovl0>, <&ovl1>, < ... >
> }
> 
> We are introducing a new compatible that is not describing hardware but how
> hardware is grouped, which I think is fine.
> 
> The mediatek-drm driver will need a bit of rework but not much I guess.
> 
> What is still not clear to me is the mdp part because doesn't seem to have an
> entry point like the display part, instead it uses one port as entry point.
> 
> 	mdp_rdma0: rdma@14001000 {
> 		compatible = "mediatek,mt8173-mdp-rdma",
> 			     "mediatek,mt8173-mdp";
> 
> AFAICS that driver is not touching MMSYS config registers to route the mdp path,
> only is getting the clocks, but I assume it will do in the future. In such case,
> maybe we could do something similar to the display-subsystem node?
> 

Your proposal is to probe clock driver by mmsys device and probe display driver by another device. You have two choice to probe display driver: one is to create a virtual device that group display devices and probe display driver by this device. Another one is to choose one display device, such as OVL, to probe display driver.

I do not like a virtual device solution. In some Mediatek SoC, the routing is so flexible that one function block could be placed in display pipe or mdp pipe by different routing.

mmsys device control the display routing and display clock. OVL control. display overlay function. Both devices control partial display function, so probing display driver by which one looks the same for me.

I prefer to probe display driver by mmsys device because it has more information of display pipe line and OVL just focus on overlay function. Only when it's difficult to probe display driver by mmsys device, we have to probe display driver by OVL.

I think mmsys is really a multi-function device, and the device tree description may look like:

mmsys: system-controller@14000000 {
	compatible = "mediatek,mt8173-mmsys", "syscon", "simple-mfd";
	reg = <0 0x14000000 0 0x1000>;
	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
	assigned-clock-rates = <400000000>;
	#clock-cells = <1>;

	route {
		compatible = "mediatek,mt8173-mmsys-route";
	};

	clock {
		compatible = "mediatek,mt8173-mmsys-clk";
	};
};

But from mt6797 register map [1], mmsys have many function such as fake engine, memory delay, reset,....
Should we break each function into a sub device?
Or we do not break any function (include clock and routing) into sub device?
Or just break these two function into device, remove "simple-mfd" from mmsys, so the rest control is in mmsys top device?

[1] https://www.96boards.org/documentation/consumer/mediatekx20/additional-docs/docs/MT6797_Register_Table_Part_2.pdf

Regards,
CK


> Regards,
>  Enric
> 
> 
> > Regards,
> > Matthias
> > 
> >> Thanks.
> >>
> >>> If you have better solution, just let's forget this.
> >>>
> >>> Regards,
> >>> CK
> >>>
> >>>>
> >>>> Regards,
> >>>> Matthias
> >>>>
> >>>>> Regards,
> >>>>> CK
> >>>>>
> >>>>>>
> >>>>>> IMHO I think that considering the clk driver as entry point is fine, but this is
> >>>>>> something that the clock maintainers should decide.
> >>>>>>
> >>>>>> Also note that this is not only a MT8173 problem I am seeing the same problem on
> >>>>>> all other Mediatek SoCs.
> >>>>>>
> >>>>>> Thanks.
> >>>>>>
> >>>>>>> Regards,
> >>>>>>> CK
> >>>>>>>
> >>>>>>>>
> >>>>>>>> All this series was tested on the Acer R13 Chromebook only.
> >>>>>>>>
> >>>>>>>> For reference, here are the links to the old discussions:
> >>>>>>>>
> >>>>>>>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
> >>>>>>>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
> >>>>>>>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
> >>>>>>>> * v4:
> >>>>>>>>   * https://patchwork.kernel.org/patch/10530871/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10530883/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10530885/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10530911/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10530913/
> >>>>>>>> * v3:
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367857/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367861/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367877/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367875/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367885/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367883/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367889/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367907/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367909/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10367905/
> >>>>>>>> * v2: No relevant discussion, see v3
> >>>>>>>> * v1:
> >>>>>>>>   * https://patchwork.kernel.org/patch/10016497/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10016499/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10016505/
> >>>>>>>>   * https://patchwork.kernel.org/patch/10016507/
> >>>>>>>>
> >>>>>>>> Best regards,
> >>>>>>>>  Enric
> >>>>>>>>
> >>>>>>>> Changes in v8:
> >>>>>>>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
> >>>>>>>> - New patches introduced in this series.
> >>>>>>>>
> >>>>>>>> Changes in v7:
> >>>>>>>> - Add R-by from CK
> >>>>>>>> - Add R-by from CK
> >>>>>>>> - Fix check of return value of of_clk_get
> >>>>>>>> - Fix identation
> >>>>>>>> - Free clk_data->clks as well
> >>>>>>>> - Get rid of private data structure
> >>>>>>>>
> >>>>>>>> Enric Balletbo i Serra (2):
> >>>>>>>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
> >>>>>>>>   clk/drm: mediatek: Fix mediatek-drm device probing
> >>>>>>>>
> >>>>>>>> Matthias Brugger (4):
> >>>>>>>>   drm/mediatek: Use regmap for register access
> >>>>>>>>   drm/mediatek: Omit warning on probe defers
> >>>>>>>>   media: mtk-mdp: Check return value of of_clk_get
> >>>>>>>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
> >>>>>>>>
> >>>>>>>>  drivers/clk/mediatek/Kconfig                  |   6 +
> >>>>>>>>  drivers/clk/mediatek/Makefile                 |   1 +
> >>>>>>>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
> >>>>>>>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
> >>>>>>>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
> >>>>>>>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
> >>>>>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
> >>>>>>>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
> >>>>>>>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
> >>>>>>>>  20 files changed, 401 insertions(+), 317 deletions(-)
> >>>>>>>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
> >>>>>>>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> Linux-mediatek mailing list
> >>>>>> Linux-mediatek@lists.infradead.org
> >>>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> >>>>>
> >>>>
> >>>> _______________________________________________
> >>>> Linux-mediatek mailing list
> >>>> Linux-mediatek@lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> >>>
> > 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Enric Balletbo i Serra]

On 24/2/20 6:52, CK Hu wrote:
> 
> Hi,
> 
> On Fri, 2020-02-21 at 18:10 +0100, Enric Balletbo i Serra wrote:
>> Hi,
>>
>> On 21/2/20 12:53, Matthias Brugger wrote:
>>>
>>>
>>> On 21/02/2020 12:37, Enric Balletbo i Serra wrote:
>>>> Hi CK and Matthias,
>>>>
>>>> On 21/2/20 12:11, CK Hu wrote:
>>>>> Hi, Matthias:
>>>>>
>>>>> On Fri, 2020-02-21 at 11:24 +0100, Matthias Brugger wrote:
>>>>>>
>>>>>> On 21/02/2020 10:27, CK Hu wrote:
>>>>>>> Hi, Enric:
>>>>>>>
>>>>>>> On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
>>>>>>>> Hi CK,
>>>>>>>>
>>>>>>>> Thanks for your quick answer.
>>>>>>>>
>>>>>>>> On 21/2/20 5:39, CK Hu wrote:
>>>>>>>>> Hi, Enric:
>>>>>>>>>
>>>>>>>>> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
>>>>>>>>>> Dear all,
>>>>>>>>>>
>>>>>>>>>> Those patches are intended to solve an old standing issue on some
>>>>>>>>>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
>>>>>>>>>> to the precedent series.
>>>>>>>>>>
>>>>>>>>>> Up to now both drivers, clock and drm are probed with the same device tree
>>>>>>>>>> compatible. But only the first driver get probed, which in effect breaks
>>>>>>>>>> graphics on those devices.
>>>>>>>>>>
>>>>>>>>>> The version eight of the series tries to solve the problem with a
>>>>>>>>>> different approach than the previous series but similar to how is solved
>>>>>>>>>> on other Mediatek devices.
>>>>>>>>>>
>>>>>>>>>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
>>>>>>>>>> control clock gates (which is used in the clk driver) and some registers
>>>>>>>>>> to set the routing and enable the differnet blocks of the display
>>>>>>>>>> and MDP (Media Data Path) subsystem. On this series the clk driver is
>>>>>>>>>> not a pure clock controller but a system controller that can provide
>>>>>>>>>> access to the shared registers between the different drivers that need
>>>>>>>>>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
>>>>>>>>>> this version, clk driver is the entry point (parent) which will trigger
>>>>>>>>>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
>>>>>>>>>> platform data for display configuration.
>>>>>>>>>
>>>>>>>>> When mmsys is a system controller, I prefer to place mmsys in
>>>>>>>>> drivers/soc/mediatek, and it share registers for clock, display, and mdp
>>>>>>>>> driver. This means the probe function is placed in
>>>>>>>>> drivers/soc/mediatek ,its display clock function, mdp clock function are
>>>>>>>>> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
>>>>>>>>> and mdp routing are placed in dirvers/video.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I understand what you mean but I am not sure this makes the code clearer and
>>>>>>>> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
>>>>>>>> of a "simple-mfd" device (a driver that simply matches with
>>>>>>>> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
>>>>>>>> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
>>>>>>>> device-tree).
>>>>>>>>
>>>>>>>
>>>>>>> It's clear that mmsys is neither a pure clock controller nor a pure
>>>>>>> routing controller for display and mdp. 
>>>>>>>
>>>>>>>> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
>>>>>>>> beginning representing how really hardwware is, but I think that, change this
>>>>>>>> now, will break backward compatibility.
>>>>>>>
>>>>>>> Maybe this is a solution. Current device tree would work only on old
>>>>>>> kernel version with a bug, so this mean there is no any device tree
>>>>>>> works on kernel version without bug. Why do we compatible with such
>>>>>>> device tree?
>>>>>>>
>>>>>>
>>>>
>>>> So the only reason why current DT worked at some point is because there was a
>>>> kernel bug?
>>>>
>>>
>>> I'd say you can call it a kernel bug:
>>> https://patchwork.kernel.org/patch/10367877/#22078767
>>>
>>>
>>>> If that's the case I think we shouldn't worry about break DT compatibility (I'm
>>>> sorry for those that having a buggy kernel makes display working)
>>>>
>>>>>> The idea behind this is, that the device-tree could be passed by some boot
>>>>>> firmware, so that the OS do not care about it. For this we need a stable DTS as
>>>>>> otherwise newer kernel with older FW would break.
>>>>>>
>>>>>> DTS is supposed to be just a different description of the HW like ACPI. So it
>>>>>> has to be compatible (newer kernel with older DTS and if possible vice versa).
>>>>>
>>>>> In my view, there is no FW (except some bug-inside FW) which works on
>>>>> this dts, so this dts is in a initial state. I think the compatibility
>>>>> is based on that dts correctly describe the HW. If we find this dts does
>>>>> not correctly describe the HW and it's in a initial state, should we
>>>>> still make it compatible?
>>>>>
>>>>
>>>> In this case I think we don't need to worry about buggy kernels, the only thing
>>>> that we need to take in consideration is that mmsys is instantiated on both (the
>>>> old DT and the new DT), we shouldn't expect display working (because never
>>>> worked, right?)
>>>>
>>>> With that in mind, I agree that is a good opportunity to fix properly the HW
>>>> topology.
>>>>
>>>> What thing that worries me is that I see this pattern on all Mediatek SoCs, does
>>>> this mean that display was never well supported for Mediatek SoCs?
>>>>
>>>
>>> That's exactly the case. Actually there were some patches for mt762x (can't
>>> remember if mt7623 or mt7622) whcih got pushed back, because we would need to
>>> fix the mmsys problem first.
>>>
>>> Ok, so if we come to the conclusion that this actually only worked because of a
>>> bug, then we can remodel the whole thing.
>>> In this case, I think the best would be to do what CK proposed:
>>> https://patchwork.kernel.org/patch/11381201/#23158169
>>>
>>> Making everything below 0x14000000 a subdevice of mmsys (mdp, ovl, rdma, you
>>> name it).
>>>
>>
>>
>> I see the MMSYS space as config registers to route the different ports in the
>> drm and video subsystem, so, as phandle of the display (drivers/gpur/drm) and
>> video (drivers/video) subsystem. What about something like this?
>>
>> mmsys: syscon@14000000 {
>> 	compatible = "mediatek,mt8173-mmsys", "syscon";
>> 	reg = <0 0x14000000 0 0x1000>;
>> 	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> 	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
>> 	assigned-clock-rates = <400000000>;
>> 	#clock-cells = <1>;
>> };
>>
>> Basically is what we have with
>>
>> * [PATCH v8 4/6] clk: mediatek: mt8173: Switch MMSYS to platform driver
>>
>> And
>>
>> display-subsystem {
>> 	compatible = "mediatek,display-subsystem";
>> 	mediatek,mmsys = <&mmsys>; /* phandle to the routing config registers */
>> 	ports = <&ovl0>, <&ovl1>, < ... >
>> }
>>
>> We are introducing a new compatible that is not describing hardware but how
>> hardware is grouped, which I think is fine.
>>
>> The mediatek-drm driver will need a bit of rework but not much I guess.
>>
>> What is still not clear to me is the mdp part because doesn't seem to have an
>> entry point like the display part, instead it uses one port as entry point.
>>
>> 	mdp_rdma0: rdma@14001000 {
>> 		compatible = "mediatek,mt8173-mdp-rdma",
>> 			     "mediatek,mt8173-mdp";
>>
>> AFAICS that driver is not touching MMSYS config registers to route the mdp path,
>> only is getting the clocks, but I assume it will do in the future. In such case,
>> maybe we could do something similar to the display-subsystem node?
>>
> 
> Your proposal is to probe clock driver by mmsys device and probe display driver by another device. You have two choice to probe display driver: one is to create a virtual device that group display devices and probe display driver by this device. Another one is to choose one display device, such as OVL, to probe display driver.
> 
> I do not like a virtual device solution. In some Mediatek SoC, the routing is so flexible that one function block could be placed in display pipe or mdp pipe by different routing.
> 
> mmsys device control the display routing and display clock. OVL control. display overlay function. Both devices control partial display function, so probing display driver by which one looks the same for me.
> 
> I prefer to probe display driver by mmsys device because it has more information of display pipe line and OVL just focus on overlay function. Only when it's difficult to probe display driver by mmsys device, we have to probe display driver by OVL.
> 
> I think mmsys is really a multi-function device, and the device tree description may look like:
> 
> mmsys: system-controller@14000000 {
> 	compatible = "mediatek,mt8173-mmsys", "syscon", "simple-mfd";
> 	reg = <0 0x14000000 0 0x1000>;
> 	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> 	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
> 	assigned-clock-rates = <400000000>;
> 	#clock-cells = <1>;
> 
> 	route {
> 		compatible = "mediatek,mt8173-mmsys-route";

Are you suggesting move the mediatek-drm routing to a new mt8173-mmsys-route
driver? Or this is a more generic routing device? Is this routing device already
implemented somewhere?

> 	};
> 
> 	clock {
> 		compatible = "mediatek,mt8173-mmsys-clk";
> 	};
> };
> 
> But from mt6797 register map [1], mmsys have many function such as fake engine, memory delay, reset,....
> Should we break each function into a sub device?
> Or we do not break any function (include clock and routing) into sub device?
> Or just break these two function into device, remove "simple-mfd" from mmsys, so the rest control is in mmsys top device?
> 

How do you see move mmsys to drivers/soc/mediatek and instantiate the clk and
mediatek-drm driver

 mmsys: syscon@14000000 {
 	compatible = "mediatek,mt8173-mmsys", "syscon", "simple-mfd";
 	reg = <0 0x14000000 0 0x1000>;
 	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;

	clock-controller {
		compatible = "mediatek,clk-mt8173-mm"
		assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
	 	assigned-clock-rates = <400000000>;
 		#clock-cells = <1>;
	};

	display-subsystem {
		compatible = "mediatek,display-subsystem";
	};
 };


Regards,
 Enric

> [1] https://www.96boards.org/documentation/consumer/mediatekx20/additional-docs/docs/MT6797_Register_Table_Part_2.pdf
> 
> Regards,
> CK
> 
> 
>> Regards,
>>  Enric
>>
>>
>>> Regards,
>>> Matthias
>>>
>>>> Thanks.
>>>>
>>>>> If you have better solution, just let's forget this.
>>>>>
>>>>> Regards,
>>>>> CK
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Matthias
>>>>>>
>>>>>>> Regards,
>>>>>>> CK
>>>>>>>
>>>>>>>>
>>>>>>>> IMHO I think that considering the clk driver as entry point is fine, but this is
>>>>>>>> something that the clock maintainers should decide.
>>>>>>>>
>>>>>>>> Also note that this is not only a MT8173 problem I am seeing the same problem on
>>>>>>>> all other Mediatek SoCs.
>>>>>>>>
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> CK
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> All this series was tested on the Acer R13 Chromebook only.
>>>>>>>>>>
>>>>>>>>>> For reference, here are the links to the old discussions:
>>>>>>>>>>
>>>>>>>>>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
>>>>>>>>>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
>>>>>>>>>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
>>>>>>>>>> * v4:
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10530871/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10530883/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10530885/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10530911/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10530913/
>>>>>>>>>> * v3:
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367857/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367861/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367877/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367875/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367885/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367883/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367889/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367907/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367909/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10367905/
>>>>>>>>>> * v2: No relevant discussion, see v3
>>>>>>>>>> * v1:
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10016497/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10016499/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10016505/
>>>>>>>>>>   * https://patchwork.kernel.org/patch/10016507/
>>>>>>>>>>
>>>>>>>>>> Best regards,
>>>>>>>>>>  Enric
>>>>>>>>>>
>>>>>>>>>> Changes in v8:
>>>>>>>>>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>>>>>>>>>> - New patches introduced in this series.
>>>>>>>>>>
>>>>>>>>>> Changes in v7:
>>>>>>>>>> - Add R-by from CK
>>>>>>>>>> - Add R-by from CK
>>>>>>>>>> - Fix check of return value of of_clk_get
>>>>>>>>>> - Fix identation
>>>>>>>>>> - Free clk_data->clks as well
>>>>>>>>>> - Get rid of private data structure
>>>>>>>>>>
>>>>>>>>>> Enric Balletbo i Serra (2):
>>>>>>>>>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
>>>>>>>>>>   clk/drm: mediatek: Fix mediatek-drm device probing
>>>>>>>>>>
>>>>>>>>>> Matthias Brugger (4):
>>>>>>>>>>   drm/mediatek: Use regmap for register access
>>>>>>>>>>   drm/mediatek: Omit warning on probe defers
>>>>>>>>>>   media: mtk-mdp: Check return value of of_clk_get
>>>>>>>>>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
>>>>>>>>>>
>>>>>>>>>>  drivers/clk/mediatek/Kconfig                  |   6 +
>>>>>>>>>>  drivers/clk/mediatek/Makefile                 |   1 +
>>>>>>>>>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
>>>>>>>>>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
>>>>>>>>>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
>>>>>>>>>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
>>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
>>>>>>>>>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
>>>>>>>>>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
>>>>>>>>>>  20 files changed, 401 insertions(+), 317 deletions(-)
>>>>>>>>>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
>>>>>>>>>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Linux-mediatek mailing list
>>>>>>>> Linux-mediatek@lists.infradead.org
>>>>>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Linux-mediatek mailing list
>>>>>> Linux-mediatek@lists.infradead.org
>>>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>>>>
>>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> 
> 

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[CK Hu]

Hi, Enric:

On Tue, 2020-02-25 at 11:56 +0100, Enric Balletbo i Serra wrote:
> 
> On 24/2/20 6:52, CK Hu wrote:
> > 
> > Hi,
> > 
> > On Fri, 2020-02-21 at 18:10 +0100, Enric Balletbo i Serra wrote:
> >> Hi,
> >>
> >> On 21/2/20 12:53, Matthias Brugger wrote:
> >>>
> >>>
> >>> On 21/02/2020 12:37, Enric Balletbo i Serra wrote:
> >>>> Hi CK and Matthias,
> >>>>
> >>>> On 21/2/20 12:11, CK Hu wrote:
> >>>>> Hi, Matthias:
> >>>>>
> >>>>> On Fri, 2020-02-21 at 11:24 +0100, Matthias Brugger wrote:
> >>>>>>
> >>>>>> On 21/02/2020 10:27, CK Hu wrote:
> >>>>>>> Hi, Enric:
> >>>>>>>
> >>>>>>> On Fri, 2020-02-21 at 09:56 +0100, Enric Balletbo i Serra wrote:
> >>>>>>>> Hi CK,
> >>>>>>>>
> >>>>>>>> Thanks for your quick answer.
> >>>>>>>>
> >>>>>>>> On 21/2/20 5:39, CK Hu wrote:
> >>>>>>>>> Hi, Enric:
> >>>>>>>>>
> >>>>>>>>> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
> >>>>>>>>>> Dear all,
> >>>>>>>>>>
> >>>>>>>>>> Those patches are intended to solve an old standing issue on some
> >>>>>>>>>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
> >>>>>>>>>> to the precedent series.
> >>>>>>>>>>
> >>>>>>>>>> Up to now both drivers, clock and drm are probed with the same device tree
> >>>>>>>>>> compatible. But only the first driver get probed, which in effect breaks
> >>>>>>>>>> graphics on those devices.
> >>>>>>>>>>
> >>>>>>>>>> The version eight of the series tries to solve the problem with a
> >>>>>>>>>> different approach than the previous series but similar to how is solved
> >>>>>>>>>> on other Mediatek devices.
> >>>>>>>>>>
> >>>>>>>>>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
> >>>>>>>>>> control clock gates (which is used in the clk driver) and some registers
> >>>>>>>>>> to set the routing and enable the differnet blocks of the display
> >>>>>>>>>> and MDP (Media Data Path) subsystem. On this series the clk driver is
> >>>>>>>>>> not a pure clock controller but a system controller that can provide
> >>>>>>>>>> access to the shared registers between the different drivers that need
> >>>>>>>>>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
> >>>>>>>>>> this version, clk driver is the entry point (parent) which will trigger
> >>>>>>>>>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
> >>>>>>>>>> platform data for display configuration.
> >>>>>>>>>
> >>>>>>>>> When mmsys is a system controller, I prefer to place mmsys in
> >>>>>>>>> drivers/soc/mediatek, and it share registers for clock, display, and mdp
> >>>>>>>>> driver. This means the probe function is placed in
> >>>>>>>>> drivers/soc/mediatek ,its display clock function, mdp clock function are
> >>>>>>>>> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
> >>>>>>>>> and mdp routing are placed in dirvers/video.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> I understand what you mean but I am not sure this makes the code clearer and
> >>>>>>>> useful. The driver in drivers/soc/mediatek will be a simple dummy implementation
> >>>>>>>> of a "simple-mfd" device (a driver that simply matches with
> >>>>>>>> "mediatek,mt8173-mmsys" and instantiates the "clk-mt8173-mm" and the
> >>>>>>>> "mediatek-drm" driver (note that mediatek-mdp" is already instantiated via
> >>>>>>>> device-tree).
> >>>>>>>>
> >>>>>>>
> >>>>>>> It's clear that mmsys is neither a pure clock controller nor a pure
> >>>>>>> routing controller for display and mdp. 
> >>>>>>>
> >>>>>>>> It'd be nice had a proper device-tree with a "simple-mfd" for mmsys from the
> >>>>>>>> beginning representing how really hardwware is, but I think that, change this
> >>>>>>>> now, will break backward compatibility.
> >>>>>>>
> >>>>>>> Maybe this is a solution. Current device tree would work only on old
> >>>>>>> kernel version with a bug, so this mean there is no any device tree
> >>>>>>> works on kernel version without bug. Why do we compatible with such
> >>>>>>> device tree?
> >>>>>>>
> >>>>>>
> >>>>
> >>>> So the only reason why current DT worked at some point is because there was a
> >>>> kernel bug?
> >>>>
> >>>
> >>> I'd say you can call it a kernel bug:
> >>> https://patchwork.kernel.org/patch/10367877/#22078767
> >>>
> >>>
> >>>> If that's the case I think we shouldn't worry about break DT compatibility (I'm
> >>>> sorry for those that having a buggy kernel makes display working)
> >>>>
> >>>>>> The idea behind this is, that the device-tree could be passed by some boot
> >>>>>> firmware, so that the OS do not care about it. For this we need a stable DTS as
> >>>>>> otherwise newer kernel with older FW would break.
> >>>>>>
> >>>>>> DTS is supposed to be just a different description of the HW like ACPI. So it
> >>>>>> has to be compatible (newer kernel with older DTS and if possible vice versa).
> >>>>>
> >>>>> In my view, there is no FW (except some bug-inside FW) which works on
> >>>>> this dts, so this dts is in a initial state. I think the compatibility
> >>>>> is based on that dts correctly describe the HW. If we find this dts does
> >>>>> not correctly describe the HW and it's in a initial state, should we
> >>>>> still make it compatible?
> >>>>>
> >>>>
> >>>> In this case I think we don't need to worry about buggy kernels, the only thing
> >>>> that we need to take in consideration is that mmsys is instantiated on both (the
> >>>> old DT and the new DT), we shouldn't expect display working (because never
> >>>> worked, right?)
> >>>>
> >>>> With that in mind, I agree that is a good opportunity to fix properly the HW
> >>>> topology.
> >>>>
> >>>> What thing that worries me is that I see this pattern on all Mediatek SoCs, does
> >>>> this mean that display was never well supported for Mediatek SoCs?
> >>>>
> >>>
> >>> That's exactly the case. Actually there were some patches for mt762x (can't
> >>> remember if mt7623 or mt7622) whcih got pushed back, because we would need to
> >>> fix the mmsys problem first.
> >>>
> >>> Ok, so if we come to the conclusion that this actually only worked because of a
> >>> bug, then we can remodel the whole thing.
> >>> In this case, I think the best would be to do what CK proposed:
> >>> https://patchwork.kernel.org/patch/11381201/#23158169
> >>>
> >>> Making everything below 0x14000000 a subdevice of mmsys (mdp, ovl, rdma, you
> >>> name it).
> >>>
> >>
> >>
> >> I see the MMSYS space as config registers to route the different ports in the
> >> drm and video subsystem, so, as phandle of the display (drivers/gpur/drm) and
> >> video (drivers/video) subsystem. What about something like this?
> >>
> >> mmsys: syscon@14000000 {
> >> 	compatible = "mediatek,mt8173-mmsys", "syscon";
> >> 	reg = <0 0x14000000 0 0x1000>;
> >> 	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> >> 	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
> >> 	assigned-clock-rates = <400000000>;
> >> 	#clock-cells = <1>;
> >> };
> >>
> >> Basically is what we have with
> >>
> >> * [PATCH v8 4/6] clk: mediatek: mt8173: Switch MMSYS to platform driver
> >>
> >> And
> >>
> >> display-subsystem {
> >> 	compatible = "mediatek,display-subsystem";
> >> 	mediatek,mmsys = <&mmsys>; /* phandle to the routing config registers */
> >> 	ports = <&ovl0>, <&ovl1>, < ... >
> >> }
> >>
> >> We are introducing a new compatible that is not describing hardware but how
> >> hardware is grouped, which I think is fine.
> >>
> >> The mediatek-drm driver will need a bit of rework but not much I guess.
> >>
> >> What is still not clear to me is the mdp part because doesn't seem to have an
> >> entry point like the display part, instead it uses one port as entry point.
> >>
> >> 	mdp_rdma0: rdma@14001000 {
> >> 		compatible = "mediatek,mt8173-mdp-rdma",
> >> 			     "mediatek,mt8173-mdp";
> >>
> >> AFAICS that driver is not touching MMSYS config registers to route the mdp path,
> >> only is getting the clocks, but I assume it will do in the future. In such case,
> >> maybe we could do something similar to the display-subsystem node?
> >>
> > 
> > Your proposal is to probe clock driver by mmsys device and probe display driver by another device. You have two choice to probe display driver: one is to create a virtual device that group display devices and probe display driver by this device. Another one is to choose one display device, such as OVL, to probe display driver.
> > 
> > I do not like a virtual device solution. In some Mediatek SoC, the routing is so flexible that one function block could be placed in display pipe or mdp pipe by different routing.
> > 
> > mmsys device control the display routing and display clock. OVL control. display overlay function. Both devices control partial display function, so probing display driver by which one looks the same for me.
> > 
> > I prefer to probe display driver by mmsys device because it has more information of display pipe line and OVL just focus on overlay function. Only when it's difficult to probe display driver by mmsys device, we have to probe display driver by OVL.
> > 
> > I think mmsys is really a multi-function device, and the device tree description may look like:
> > 
> > mmsys: system-controller@14000000 {
> > 	compatible = "mediatek,mt8173-mmsys", "syscon", "simple-mfd";
> > 	reg = <0 0x14000000 0 0x1000>;
> > 	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> > 	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
> > 	assigned-clock-rates = <400000000>;
> > 	#clock-cells = <1>;
> > 
> > 	route {
> > 		compatible = "mediatek,mt8173-mmsys-route";
> 
> Are you suggesting move the mediatek-drm routing to a new mt8173-mmsys-route
> driver? Or this is a more generic routing device? Is this routing device already
> implemented somewhere?
> 

This is a more generic routing device which control routing inside mmsys
partition (include display and mdp). Now we could find display routing
in drivers/gpu/drm/mediatek/mtk_drm_ddp.c. You could search
'config_regs' and it is the register from 0x14000000 ~ 0x14000fff.

> > 	};
> > 
> > 	clock {
> > 		compatible = "mediatek,mt8173-mmsys-clk";
> > 	};
> > };
> > 
> > But from mt6797 register map [1], mmsys have many function such as fake engine, memory delay, reset,....
> > Should we break each function into a sub device?
> > Or we do not break any function (include clock and routing) into sub device?
> > Or just break these two function into device, remove "simple-mfd" from mmsys, so the rest control is in mmsys top device?
> > 
> 
> How do you see move mmsys to drivers/soc/mediatek and instantiate the clk and
> mediatek-drm driver
> 
>  mmsys: syscon@14000000 {
>  	compatible = "mediatek,mt8173-mmsys", "syscon", "simple-mfd";
>  	reg = <0 0x14000000 0 0x1000>;
>  	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> 
> 	clock-controller {
> 		compatible = "mediatek,clk-mt8173-mm"
> 		assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
> 	 	assigned-clock-rates = <400000000>;
>  		#clock-cells = <1>;
> 	};
> 
> 	display-subsystem {
> 		compatible = "mediatek,display-subsystem";
> 	};
>  };
> 

Let's start with the simple definition.

mmsys: syscon at 14000000 {
	compatible = "mediatek,mt8173-mmsys", "syscon";
	reg = <0 0x14000000 0 0x1000>;
	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
	assigned-clock-rates = <400000000>;
	#clock-cells = <1>;
};

When we break clock control to a sub device of mmsys, the reason is that
'Linux' generally categorize clock controller to a device. When we break
display control to a sub device of mmsys, the reason is that 'Linux'
generally categorize display controller to a device. All these seems
software-oriented reason, so I think we do not break any sub device and
keep mmsys simple.

When I search of_clk_add_provider(), I find that not all clock provider
code is in drivers/clk. Maybe when a clock controller is not an
independent device, the driver code of clock controller could be placed
within the device driver it belonged to. We could place mmsys driver in
drivers/soc/mediatek/, and it control the clock, routing, fake engine,
memory delay,.... I would like drm driver to be placed in
drivers/gpu/drm/ because display function block, such as OVL, does not
belong to mmsys device. And finally let mmsys driver to probe
mediatek-drm driver.

Regards,
CK

> 
> Regards,
>  Enric
> 
> > [1] https://www.96boards.org/documentation/consumer/mediatekx20/additional-docs/docs/MT6797_Register_Table_Part_2.pdf
> > 
> > Regards,
> > CK
> > 
> > 
> >> Regards,
> >>  Enric
> >>
> >>
> >>> Regards,
> >>> Matthias
> >>>
> >>>> Thanks.
> >>>>
> >>>>> If you have better solution, just let's forget this.
> >>>>>
> >>>>> Regards,
> >>>>> CK
> >>>>>
> >>>>>>
> >>>>>> Regards,
> >>>>>> Matthias
> >>>>>>
> >>>>>>> Regards,
> >>>>>>> CK
> >>>>>>>
> >>>>>>>>
> >>>>>>>> IMHO I think that considering the clk driver as entry point is fine, but this is
> >>>>>>>> something that the clock maintainers should decide.
> >>>>>>>>
> >>>>>>>> Also note that this is not only a MT8173 problem I am seeing the same problem on
> >>>>>>>> all other Mediatek SoCs.
> >>>>>>>>
> >>>>>>>> Thanks.
> >>>>>>>>
> >>>>>>>>> Regards,
> >>>>>>>>> CK
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> All this series was tested on the Acer R13 Chromebook only.
> >>>>>>>>>>
> >>>>>>>>>> For reference, here are the links to the old discussions:
> >>>>>>>>>>
> >>>>>>>>>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
> >>>>>>>>>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
> >>>>>>>>>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
> >>>>>>>>>> * v4:
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10530871/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10530883/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10530885/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10530911/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10530913/
> >>>>>>>>>> * v3:
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367857/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367861/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367877/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367875/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367885/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367883/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367889/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367907/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367909/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10367905/
> >>>>>>>>>> * v2: No relevant discussion, see v3
> >>>>>>>>>> * v1:
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10016497/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10016499/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10016505/
> >>>>>>>>>>   * https://patchwork.kernel.org/patch/10016507/
> >>>>>>>>>>
> >>>>>>>>>> Best regards,
> >>>>>>>>>>  Enric
> >>>>>>>>>>
> >>>>>>>>>> Changes in v8:
> >>>>>>>>>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
> >>>>>>>>>> - New patches introduced in this series.
> >>>>>>>>>>
> >>>>>>>>>> Changes in v7:
> >>>>>>>>>> - Add R-by from CK
> >>>>>>>>>> - Add R-by from CK
> >>>>>>>>>> - Fix check of return value of of_clk_get
> >>>>>>>>>> - Fix identation
> >>>>>>>>>> - Free clk_data->clks as well
> >>>>>>>>>> - Get rid of private data structure
> >>>>>>>>>>
> >>>>>>>>>> Enric Balletbo i Serra (2):
> >>>>>>>>>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
> >>>>>>>>>>   clk/drm: mediatek: Fix mediatek-drm device probing
> >>>>>>>>>>
> >>>>>>>>>> Matthias Brugger (4):
> >>>>>>>>>>   drm/mediatek: Use regmap for register access
> >>>>>>>>>>   drm/mediatek: Omit warning on probe defers
> >>>>>>>>>>   media: mtk-mdp: Check return value of of_clk_get
> >>>>>>>>>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
> >>>>>>>>>>
> >>>>>>>>>>  drivers/clk/mediatek/Kconfig                  |   6 +
> >>>>>>>>>>  drivers/clk/mediatek/Makefile                 |   1 +
> >>>>>>>>>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
> >>>>>>>>>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
> >>>>>>>>>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
> >>>>>>>>>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
> >>>>>>>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
> >>>>>>>>>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
> >>>>>>>>>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
> >>>>>>>>>>  20 files changed, 401 insertions(+), 317 deletions(-)
> >>>>>>>>>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
> >>>>>>>>>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> _______________________________________________
> >>>>>>>> Linux-mediatek mailing list
> >>>>>>>> Linux-mediatek@lists.infradead.org
> >>>>>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> >>>>>>>
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> Linux-mediatek mailing list
> >>>>>> Linux-mediatek@lists.infradead.org
> >>>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> >>>>>
> >>>
> >>
> >> _______________________________________________
> >> Linux-mediatek mailing list
> >> Linux-mediatek@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> > 
> > 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Enric Balletbo i Serra]

Hi CK,

On 26/2/20 6:32, CK Hu wrote:

[snip]

>>
>> How do you see move mmsys to drivers/soc/mediatek and instantiate the clk and
>> mediatek-drm driver
>>
>>  mmsys: syscon@14000000 {
>>  	compatible = "mediatek,mt8173-mmsys", "syscon", "simple-mfd";
>>  	reg = <0 0x14000000 0 0x1000>;
>>  	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>>
>> 	clock-controller {
>> 		compatible = "mediatek,clk-mt8173-mm"
>> 		assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
>> 	 	assigned-clock-rates = <400000000>;
>>  		#clock-cells = <1>;
>> 	};
>>
>> 	display-subsystem {
>> 		compatible = "mediatek,display-subsystem";
>> 	};
>>  };
>>
> 
> Let's start with the simple definition.
> 
> mmsys: syscon at 14000000 {
> 	compatible = "mediatek,mt8173-mmsys", "syscon";
> 	reg = <0 0x14000000 0 0x1000>;
> 	power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> 	assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
> 	assigned-clock-rates = <400000000>;
> 	#clock-cells = <1>;
> };
> 
> When we break clock control to a sub device of mmsys, the reason is that
> 'Linux' generally categorize clock controller to a device. When we break
> display control to a sub device of mmsys, the reason is that 'Linux'
> generally categorize display controller to a device. All these seems
> software-oriented reason, so I think we do not break any sub device and
> keep mmsys simple.
> 
> When I search of_clk_add_provider(), I find that not all clock provider
> code is in drivers/clk. Maybe when a clock controller is not an
> independent device, the driver code of clock controller could be placed
> within the device driver it belonged to. We could place mmsys driver in
> drivers/soc/mediatek/, and it control the clock, routing, fake engine,
> memory delay,.... I would like drm driver to be placed in
> drivers/gpu/drm/ because display function block, such as OVL, does not
> belong to mmsys device. And finally let mmsys driver to probe
> mediatek-drm driver.
> 

You can apply the same reasoning in the clk subsystem, not all the drivers in
drivers/clk are pure clock controllers, some of them are already
system-controller or "simple-mfd" and some of them even instantiate other
subdrivers via the platform register API.

Note that moving clk-<chip>-mm drivers to drivers/soc/mediatek will imply move a
lot of code, I'll focus only on mt8173 for now because is the only platform I
can really test. Let me prepare a v9 and lets see how looks like.

Thanks,
 Enric

> Regards,
> CK

[snip]

Re: [PATCH v8 0/6] arm/arm64: mediatek: Fix mmsys device probing

[Matthias Brugger]

Adding Lee Jones.

On 21/02/2020 05:39, CK Hu wrote:
> Hi, Enric:
> 
> On Thu, 2020-02-20 at 18:21 +0100, Enric Balletbo i Serra wrote:
>> Dear all,
>>
>> Those patches are intended to solve an old standing issue on some
>> Mediatek devices (mt8173, mt2701 and mt2712) in a slightly different way
>> to the precedent series.
>>
>> Up to now both drivers, clock and drm are probed with the same device tree
>> compatible. But only the first driver get probed, which in effect breaks
>> graphics on those devices.
>>
>> The version eight of the series tries to solve the problem with a
>> different approach than the previous series but similar to how is solved
>> on other Mediatek devices.
>>
>> The MMSYS (Multimedia subsystem) in Mediatek SoCs has some registers to
>> control clock gates (which is used in the clk driver) and some registers
>> to set the routing and enable the differnet blocks of the display
>> and MDP (Media Data Path) subsystem. On this series the clk driver is
>> not a pure clock controller but a system controller that can provide
>> access to the shared registers between the different drivers that need
>> it (mediatek-drm and mediatek-mdp). And the biggest change is, that in
>> this version, clk driver is the entry point (parent) which will trigger
>> the probe of the corresponding mediatek-drm driver and pass its MMSYS
>> platform data for display configuration.
> 
> When mmsys is a system controller, I prefer to place mmsys in
> drivers/soc/mediatek, and it share registers for clock, display, and mdp
> driver. This means the probe function is placed in
> drivers/soc/mediatek ,its display clock function, mdp clock function are
> placed in drivers/clk, display routing are placed in drivers/gpu/drm,
> and mdp routing are placed in dirvers/video.
> 

Which sounds to me like a mfd device. Something we already tried and got a
NACKed by Lee Jones:
https://patchwork.kernel.org/patch/10367877/

Maybe we understand the problem better now to give more arguments why it makes
sense to create a mfd here?

Regards,
Matthias

> Regards,
> CK
> 
>>
>> All this series was tested on the Acer R13 Chromebook only.
>>
>> For reference, here are the links to the old discussions:
>>
>> * v7: https://patchwork.kernel.org/project/linux-mediatek/list/?series=241217
>> * v6: https://patchwork.kernel.org/project/linux-mediatek/list/?series=213219
>> * v5: https://patchwork.kernel.org/project/linux-mediatek/list/?series=44063
>> * v4:
>>   * https://patchwork.kernel.org/patch/10530871/
>>   * https://patchwork.kernel.org/patch/10530883/
>>   * https://patchwork.kernel.org/patch/10530885/
>>   * https://patchwork.kernel.org/patch/10530911/
>>   * https://patchwork.kernel.org/patch/10530913/
>> * v3:
>>   * https://patchwork.kernel.org/patch/10367857/
>>   * https://patchwork.kernel.org/patch/10367861/
>>   * https://patchwork.kernel.org/patch/10367877/
>>   * https://patchwork.kernel.org/patch/10367875/
>>   * https://patchwork.kernel.org/patch/10367885/
>>   * https://patchwork.kernel.org/patch/10367883/
>>   * https://patchwork.kernel.org/patch/10367889/
>>   * https://patchwork.kernel.org/patch/10367907/
>>   * https://patchwork.kernel.org/patch/10367909/
>>   * https://patchwork.kernel.org/patch/10367905/
>> * v2: No relevant discussion, see v3
>> * v1:
>>   * https://patchwork.kernel.org/patch/10016497/
>>   * https://patchwork.kernel.org/patch/10016499/
>>   * https://patchwork.kernel.org/patch/10016505/
>>   * https://patchwork.kernel.org/patch/10016507/
>>
>> Best regards,
>>  Enric
>>
>> Changes in v8:
>> - Be a builtin_platform_driver like other mediatek mmsys drivers.
>> - New patches introduced in this series.
>>
>> Changes in v7:
>> - Add R-by from CK
>> - Add R-by from CK
>> - Fix check of return value of of_clk_get
>> - Fix identation
>> - Free clk_data->clks as well
>> - Get rid of private data structure
>>
>> Enric Balletbo i Serra (2):
>>   drm/mediatek: Move MMSYS configuration to include/linux/platform_data
>>   clk/drm: mediatek: Fix mediatek-drm device probing
>>
>> Matthias Brugger (4):
>>   drm/mediatek: Use regmap for register access
>>   drm/mediatek: Omit warning on probe defers
>>   media: mtk-mdp: Check return value of of_clk_get
>>   clk: mediatek: mt8173: Switch MMSYS to platform driver
>>
>>  drivers/clk/mediatek/Kconfig                  |   6 +
>>  drivers/clk/mediatek/Makefile                 |   1 +
>>  drivers/clk/mediatek/clk-mt2701-mm.c          |  30 +++
>>  drivers/clk/mediatek/clk-mt2712-mm.c          |  44 +++++
>>  drivers/clk/mediatek/clk-mt8173-mm.c          | 172 ++++++++++++++++++
>>  drivers/clk/mediatek/clk-mt8173.c             | 104 -----------
>>  drivers/gpu/drm/mediatek/mtk_disp_color.c     |   5 +-
>>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |   5 +-
>>  drivers/gpu/drm/mediatek/mtk_disp_rdma.c      |   5 +-
>>  drivers/gpu/drm/mediatek/mtk_dpi.c            |  12 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   4 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.c        |  53 +++---
>>  drivers/gpu/drm/mediatek/mtk_drm_ddp.h        |   4 +-
>>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  56 +-----
>>  drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 113 +-----------
>>  drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  13 +-
>>  drivers/gpu/drm/mediatek/mtk_dsi.c            |   8 +-
>>  drivers/gpu/drm/mediatek/mtk_hdmi.c           |   4 +-
>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c |   6 +
>>  include/linux/platform_data/mtk_mmsys.h       |  73 ++++++++
>>  20 files changed, 401 insertions(+), 317 deletions(-)
>>  create mode 100644 drivers/clk/mediatek/clk-mt8173-mm.c
>>  create mode 100644 include/linux/platform_data/mtk_mmsys.h
>>
>