[PATCH 0/4] soc: mediatek: Prepare MMSYS for DDP routing using tables

[PATCH 0/4] soc: mediatek: Prepare MMSYS for DDP routing using tables

[Enric Balletbo i Serra]

Dear all,

The following series are intended to prepare the mtk-mmsys driver to
allow different DDP (Data Display Path) routing tables per SoC. Note
that the series has been tested only on MT8173 platform and could break
the display on MT2701 and MT2712 based devices. I kindly ask for someone
having these devices to provide a tested routing table (unfortunately I
didn't have enough documentation to figure out this myself).

For the other devices (MT8183, MT6779 and MT6797) DRM support is not in
mainline yet so nothing will break.

Thanks,
  Enric


CK Hu (2):
  soc: mediatek: mmsys: Create struct mtk_mmsys to store context data
  soc: mediatek: mmsys: Use an array for setting the routing registers

Enric Balletbo i Serra (1):
  soc: mediatek: mmsys: Use devm_platform_ioremap_resource()

Yongqiang Niu (1):
  soc / drm: mediatek: Move DDP component defines into mtk-mmsys.h

 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  34 +-
 drivers/soc/mediatek/mtk-mmsys.c            | 429 +++++++++++---------
 include/linux/soc/mediatek/mtk-mmsys.h      |  33 ++
 3 files changed, 263 insertions(+), 233 deletions(-)

-- 
2.28.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 1/4] soc / drm: mediatek: Move DDP component defines into mtk-mmsys.h

[Enric Balletbo i Serra]

From: Yongqiang Niu <yongqiang.niu@mediatek.com>

MMSYS is the driver which controls the routing of these DDP components,
so the definition of the mtk_ddp_comp_id enum should be placed in mtk-mmsys.h

Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
This patch was previously part of another series, but has no
dependencies and can be applied independently. As the latest version
sent is from two months ago, I resent this patch because the next patches
of this series depends on it to apply cleanly.

[1] https://patchwork.kernel.org/patch/11706243

 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 34 +--------------------
 drivers/soc/mediatek/mtk-mmsys.c            |  4 +--
 include/linux/soc/mediatek/mtk-mmsys.h      | 33 ++++++++++++++++++++
 3 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index debe36395fe7..161201fe5179 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -7,6 +7,7 @@
 #define MTK_DRM_DDP_COMP_H
 
 #include <linux/io.h>
+#include <linux/soc/mediatek/mtk-mmsys.h>
 
 struct device;
 struct device_node;
@@ -35,39 +36,6 @@ enum mtk_ddp_comp_type {
 	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 {
diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index a55f25511173..36ad66bb221b 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -5,13 +5,11 @@
  */
 
 #include <linux/device.h>
+#include <linux/io.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/soc/mediatek/mtk-mmsys.h>
 
-#include "../../gpu/drm/mediatek/mtk_drm_ddp.h"
-#include "../../gpu/drm/mediatek/mtk_drm_ddp_comp.h"
-
 #define DISP_REG_CONFIG_DISP_OVL0_MOUT_EN	0x040
 #define DISP_REG_CONFIG_DISP_OVL1_MOUT_EN	0x044
 #define DISP_REG_CONFIG_DISP_OD_MOUT_EN		0x048
diff --git a/include/linux/soc/mediatek/mtk-mmsys.h b/include/linux/soc/mediatek/mtk-mmsys.h
index 7bab5d9a3d31..2228bf6133da 100644
--- a/include/linux/soc/mediatek/mtk-mmsys.h
+++ b/include/linux/soc/mediatek/mtk-mmsys.h
@@ -9,6 +9,39 @@
 enum mtk_ddp_comp_id;
 struct device;
 
+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,
+};
+
 void mtk_mmsys_ddp_connect(struct device *dev,
 			   enum mtk_ddp_comp_id cur,
 			   enum mtk_ddp_comp_id next);
-- 
2.28.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 1/4] soc / drm: mediatek: Move DDP component defines into mtk-mmsys.h

[Matthias Brugger]

On 06/10/2020 21:33, Enric Balletbo i Serra wrote:
> From: Yongqiang Niu <yongqiang.niu@mediatek.com>
> 
> MMSYS is the driver which controls the routing of these DDP components,
> so the definition of the mtk_ddp_comp_id enum should be placed in mtk-mmsys.h
> 
> Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> This patch was previously part of another series, but has no
> dependencies and can be applied independently. As the latest version
> sent is from two months ago, I resent this patch because the next patches
> of this series depends on it to apply cleanly.
> 

Applied to v5.10-next/soc

Thanks

> [1] https://patchwork.kernel.org/patch/11706243
> 
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 34 +--------------------
>   drivers/soc/mediatek/mtk-mmsys.c            |  4 +--
>   include/linux/soc/mediatek/mtk-mmsys.h      | 33 ++++++++++++++++++++
>   3 files changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> index debe36395fe7..161201fe5179 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
> @@ -7,6 +7,7 @@
>   #define MTK_DRM_DDP_COMP_H
>   
>   #include <linux/io.h>
> +#include <linux/soc/mediatek/mtk-mmsys.h>
>   
>   struct device;
>   struct device_node;
> @@ -35,39 +36,6 @@ enum mtk_ddp_comp_type {
>   	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 {
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index a55f25511173..36ad66bb221b 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -5,13 +5,11 @@
>    */
>   
>   #include <linux/device.h>
> +#include <linux/io.h>
>   #include <linux/of_device.h>
>   #include <linux/platform_device.h>
>   #include <linux/soc/mediatek/mtk-mmsys.h>
>   
> -#include "../../gpu/drm/mediatek/mtk_drm_ddp.h"
> -#include "../../gpu/drm/mediatek/mtk_drm_ddp_comp.h"
> -
>   #define DISP_REG_CONFIG_DISP_OVL0_MOUT_EN	0x040
>   #define DISP_REG_CONFIG_DISP_OVL1_MOUT_EN	0x044
>   #define DISP_REG_CONFIG_DISP_OD_MOUT_EN		0x048
> diff --git a/include/linux/soc/mediatek/mtk-mmsys.h b/include/linux/soc/mediatek/mtk-mmsys.h
> index 7bab5d9a3d31..2228bf6133da 100644
> --- a/include/linux/soc/mediatek/mtk-mmsys.h
> +++ b/include/linux/soc/mediatek/mtk-mmsys.h
> @@ -9,6 +9,39 @@
>   enum mtk_ddp_comp_id;
>   struct device;
>   
> +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,
> +};
> +
>   void mtk_mmsys_ddp_connect(struct device *dev,
>   			   enum mtk_ddp_comp_id cur,
>   			   enum mtk_ddp_comp_id next);
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 2/4] soc: mediatek: mmsys: Use devm_platform_ioremap_resource()

[Enric Balletbo i Serra]

For the common platform_get_resource()+devm_platform_ioremap() combination,
there is a helper, so use it and make the code a bit more compact.

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

 drivers/soc/mediatek/mtk-mmsys.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 36ad66bb221b..18f93979e14a 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -306,15 +306,12 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
 	struct platform_device *clks;
 	struct platform_device *drm;
 	void __iomem *config_regs;
-	struct resource *mem;
 	int ret;
 
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	config_regs = devm_ioremap_resource(dev, mem);
+	config_regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(config_regs)) {
 		ret = PTR_ERR(config_regs);
-		dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
-			ret);
+		dev_err(dev, "Failed to ioremap mmsys registers: %d\n", ret);
 		return ret;
 	}
 
-- 
2.28.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/4] soc: mediatek: mmsys: Use devm_platform_ioremap_resource()

[Chun-Kuang Hu]

Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年10月7日 週三 上午3:33寫道：
>
> For the common platform_get_resource()+devm_platform_ioremap() combination,
> there is a helper, so use it and make the code a bit more compact.

Reviewed-by: Chun-Kuang Hu <chunkuang.hu@mediatek.com>

>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
>  drivers/soc/mediatek/mtk-mmsys.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index 36ad66bb221b..18f93979e14a 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -306,15 +306,12 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
>         struct platform_device *clks;
>         struct platform_device *drm;
>         void __iomem *config_regs;
> -       struct resource *mem;
>         int ret;
>
> -       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -       config_regs = devm_ioremap_resource(dev, mem);
> +       config_regs = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(config_regs)) {
>                 ret = PTR_ERR(config_regs);
> -               dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
> -                       ret);
> +               dev_err(dev, "Failed to ioremap mmsys registers: %d\n", ret);
>                 return ret;
>         }
>
> --
> 2.28.0
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/4] soc: mediatek: mmsys: Use devm_platform_ioremap_resource()

[Chun-Kuang Hu]

Chun-Kuang Hu <chunkuang.hu@kernel.org> 於 2020年10月7日 週三 上午7:21寫道：
>
> Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年10月7日 週三 上午3:33寫道：
> >
> > For the common platform_get_resource()+devm_platform_ioremap() combination,
> > there is a helper, so use it and make the code a bit more compact.
>
> Reviewed-by: Chun-Kuang Hu <chunkuang.hu@mediatek.com>
>

Sorry for typo,

Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>

> >
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > ---
> >
> >  drivers/soc/mediatek/mtk-mmsys.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> > index 36ad66bb221b..18f93979e14a 100644
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > @@ -306,15 +306,12 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
> >         struct platform_device *clks;
> >         struct platform_device *drm;
> >         void __iomem *config_regs;
> > -       struct resource *mem;
> >         int ret;
> >
> > -       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -       config_regs = devm_ioremap_resource(dev, mem);
> > +       config_regs = devm_platform_ioremap_resource(pdev, 0);
> >         if (IS_ERR(config_regs)) {
> >                 ret = PTR_ERR(config_regs);
> > -               dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
> > -                       ret);
> > +               dev_err(dev, "Failed to ioremap mmsys registers: %d\n", ret);
> >                 return ret;
> >         }
> >
> > --
> > 2.28.0
> >

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 2/4] soc: mediatek: mmsys: Use devm_platform_ioremap_resource()

[Matthias Brugger]

On 06/10/2020 21:33, Enric Balletbo i Serra wrote:
> For the common platform_get_resource()+devm_platform_ioremap() combination,
> there is a helper, so use it and make the code a bit more compact.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Applied to v5.10-next/soc

Thanks!

> ---
> 
>   drivers/soc/mediatek/mtk-mmsys.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index 36ad66bb221b..18f93979e14a 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -306,15 +306,12 @@ static int mtk_mmsys_probe(struct platform_device *pdev)
>   	struct platform_device *clks;
>   	struct platform_device *drm;
>   	void __iomem *config_regs;
> -	struct resource *mem;
>   	int ret;
>   
> -	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	config_regs = devm_ioremap_resource(dev, mem);
> +	config_regs = devm_platform_ioremap_resource(pdev, 0);
>   	if (IS_ERR(config_regs)) {
>   		ret = PTR_ERR(config_regs);
> -		dev_err(dev, "Failed to ioremap mmsys-config resource: %d\n",
> -			ret);
> +		dev_err(dev, "Failed to ioremap mmsys registers: %d\n", ret);
>   		return ret;
>   	}
>   
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 3/4] soc: mediatek: mmsys: Create struct mtk_mmsys to store context data

[Enric Balletbo i Serra]

From: CK Hu <ck.hu@mediatek.com>

Apart from the driver data, in order to extend the driver to support more
and more SoCs, we will need to store other configuration data. So, create
a mtk_mmsys struct to encapsulate all that information.

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

 drivers/soc/mediatek/mtk-mmsys.c | 47 ++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index 18f93979e14a..da2de8f6969e 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -101,6 +101,11 @@ static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
 	.clk_driver = "clk-mt8183-mm",
 };
 
+struct mtk_mmsys {
+	void __iomem *regs;
+	const struct mtk_mmsys_driver_data *data;
+};
+
 static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur,
 					  enum mtk_ddp_comp_id next,
 					  unsigned int *addr)
@@ -259,21 +264,21 @@ void mtk_mmsys_ddp_connect(struct device *dev,
 			   enum mtk_ddp_comp_id cur,
 			   enum mtk_ddp_comp_id next)
 {
-	void __iomem *config_regs = dev_get_drvdata(dev);
+	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
 	unsigned int addr, value, reg;
 
 	value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
 	if (value) {
-		reg = readl_relaxed(config_regs + addr) | value;
-		writel_relaxed(reg, config_regs + addr);
+		reg = readl_relaxed(mmsys->regs + addr) | value;
+		writel_relaxed(reg, mmsys->regs + addr);
 	}
 
-	mtk_mmsys_ddp_sout_sel(config_regs, cur, next);
+	mtk_mmsys_ddp_sout_sel(mmsys->regs, cur, next);
 
 	value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
 	if (value) {
-		reg = readl_relaxed(config_regs + addr) | value;
-		writel_relaxed(reg, config_regs + addr);
+		reg = readl_relaxed(mmsys->regs + addr) | value;
+		writel_relaxed(reg, mmsys->regs + addr);
 	}
 }
 EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect);
@@ -282,44 +287,46 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
 			      enum mtk_ddp_comp_id cur,
 			      enum mtk_ddp_comp_id next)
 {
-	void __iomem *config_regs = dev_get_drvdata(dev);
+	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
 	unsigned int addr, value, reg;
 
 	value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
 	if (value) {
-		reg = readl_relaxed(config_regs + addr) & ~value;
-		writel_relaxed(reg, config_regs + addr);
+		reg = readl_relaxed(mmsys->regs + addr) & ~value;
+		writel_relaxed(reg, mmsys->regs + addr);
 	}
 
 	value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
 	if (value) {
-		reg = readl_relaxed(config_regs + addr) & ~value;
-		writel_relaxed(reg, config_regs + addr);
+		reg = readl_relaxed(mmsys->regs + addr) & ~value;
+		writel_relaxed(reg, mmsys->regs + addr);
 	}
 }
 EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
 
 static int mtk_mmsys_probe(struct platform_device *pdev)
 {
-	const struct mtk_mmsys_driver_data *data;
 	struct device *dev = &pdev->dev;
 	struct platform_device *clks;
 	struct platform_device *drm;
-	void __iomem *config_regs;
+	struct mtk_mmsys *mmsys;
 	int ret;
 
-	config_regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(config_regs)) {
-		ret = PTR_ERR(config_regs);
+	mmsys = devm_kzalloc(dev, sizeof(*mmsys), GFP_KERNEL);
+	if (!mmsys)
+		return -ENOMEM;
+
+	mmsys->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(mmsys->regs)) {
+		ret = PTR_ERR(mmsys->regs);
 		dev_err(dev, "Failed to ioremap mmsys registers: %d\n", ret);
 		return ret;
 	}
 
-	platform_set_drvdata(pdev, config_regs);
-
-	data = of_device_get_match_data(&pdev->dev);
+	mmsys->data = of_device_get_match_data(&pdev->dev);
+	platform_set_drvdata(pdev, mmsys);
 
-	clks = platform_device_register_data(&pdev->dev, data->clk_driver,
+	clks = platform_device_register_data(&pdev->dev, mmsys->data->clk_driver,
 					     PLATFORM_DEVID_AUTO, NULL, 0);
 	if (IS_ERR(clks))
 		return PTR_ERR(clks);
-- 
2.28.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 3/4] soc: mediatek: mmsys: Create struct mtk_mmsys to store context data

[Chun-Kuang Hu]

Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年10月7日 週三 上午3:33寫道：
>
> From: CK Hu <ck.hu@mediatek.com>
>
> Apart from the driver data, in order to extend the driver to support more
> and more SoCs, we will need to store other configuration data. So, create
> a mtk_mmsys struct to encapsulate all that information.

Reviewed-by: Chun-Kuang Hu <chunkuang.hu@mediatek.com>

>
> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
>  drivers/soc/mediatek/mtk-mmsys.c | 47 ++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index 18f93979e14a..da2de8f6969e 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -101,6 +101,11 @@ static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
>         .clk_driver = "clk-mt8183-mm",
>  };
>
> +struct mtk_mmsys {
> +       void __iomem *regs;
> +       const struct mtk_mmsys_driver_data *data;
> +};
> +
>  static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur,
>                                           enum mtk_ddp_comp_id next,
>                                           unsigned int *addr)
> @@ -259,21 +264,21 @@ void mtk_mmsys_ddp_connect(struct device *dev,
>                            enum mtk_ddp_comp_id cur,
>                            enum mtk_ddp_comp_id next)
>  {
> -       void __iomem *config_regs = dev_get_drvdata(dev);
> +       struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
>         unsigned int addr, value, reg;
>
>         value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
>         if (value) {
> -               reg = readl_relaxed(config_regs + addr) | value;
> -               writel_relaxed(reg, config_regs + addr);
> +               reg = readl_relaxed(mmsys->regs + addr) | value;
> +               writel_relaxed(reg, mmsys->regs + addr);
>         }
>
> -       mtk_mmsys_ddp_sout_sel(config_regs, cur, next);
> +       mtk_mmsys_ddp_sout_sel(mmsys->regs, cur, next);
>
>         value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
>         if (value) {
> -               reg = readl_relaxed(config_regs + addr) | value;
> -               writel_relaxed(reg, config_regs + addr);
> +               reg = readl_relaxed(mmsys->regs + addr) | value;
> +               writel_relaxed(reg, mmsys->regs + addr);
>         }
>  }
>  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect);
> @@ -282,44 +287,46 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
>                               enum mtk_ddp_comp_id cur,
>                               enum mtk_ddp_comp_id next)
>  {
> -       void __iomem *config_regs = dev_get_drvdata(dev);
> +       struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
>         unsigned int addr, value, reg;
>
>         value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
>         if (value) {
> -               reg = readl_relaxed(config_regs + addr) & ~value;
> -               writel_relaxed(reg, config_regs + addr);
> +               reg = readl_relaxed(mmsys->regs + addr) & ~value;
> +               writel_relaxed(reg, mmsys->regs + addr);
>         }
>
>         value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
>         if (value) {
> -               reg = readl_relaxed(config_regs + addr) & ~value;
> -               writel_relaxed(reg, config_regs + addr);
> +               reg = readl_relaxed(mmsys->regs + addr) & ~value;
> +               writel_relaxed(reg, mmsys->regs + addr);
>         }
>  }
>  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
>
>  static int mtk_mmsys_probe(struct platform_device *pdev)
>  {
> -       const struct mtk_mmsys_driver_data *data;
>         struct device *dev = &pdev->dev;
>         struct platform_device *clks;
>         struct platform_device *drm;
> -       void __iomem *config_regs;
> +       struct mtk_mmsys *mmsys;
>         int ret;
>
> -       config_regs = devm_platform_ioremap_resource(pdev, 0);
> -       if (IS_ERR(config_regs)) {
> -               ret = PTR_ERR(config_regs);
> +       mmsys = devm_kzalloc(dev, sizeof(*mmsys), GFP_KERNEL);
> +       if (!mmsys)
> +               return -ENOMEM;
> +
> +       mmsys->regs = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(mmsys->regs)) {
> +               ret = PTR_ERR(mmsys->regs);
>                 dev_err(dev, "Failed to ioremap mmsys registers: %d\n", ret);
>                 return ret;
>         }
>
> -       platform_set_drvdata(pdev, config_regs);
> -
> -       data = of_device_get_match_data(&pdev->dev);
> +       mmsys->data = of_device_get_match_data(&pdev->dev);
> +       platform_set_drvdata(pdev, mmsys);
>
> -       clks = platform_device_register_data(&pdev->dev, data->clk_driver,
> +       clks = platform_device_register_data(&pdev->dev, mmsys->data->clk_driver,
>                                              PLATFORM_DEVID_AUTO, NULL, 0);
>         if (IS_ERR(clks))
>                 return PTR_ERR(clks);
> --
> 2.28.0
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 3/4] soc: mediatek: mmsys: Create struct mtk_mmsys to store context data

[Chun-Kuang Hu]

Chun-Kuang Hu <chunkuang.hu@kernel.org> 於 2020年10月7日 週三 上午7:24寫道：
>
> Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年10月7日 週三 上午3:33寫道：
> >
> > From: CK Hu <ck.hu@mediatek.com>
> >
> > Apart from the driver data, in order to extend the driver to support more
> > and more SoCs, we will need to store other configuration data. So, create
> > a mtk_mmsys struct to encapsulate all that information.
>
> Reviewed-by: Chun-Kuang Hu <chunkuang.hu@mediatek.com>

Sorry for typo:

Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>

>
> >
> > Signed-off-by: CK Hu <ck.hu@mediatek.com>
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > ---
> >
> >  drivers/soc/mediatek/mtk-mmsys.c | 47 ++++++++++++++++++--------------
> >  1 file changed, 27 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> > index 18f93979e14a..da2de8f6969e 100644
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > @@ -101,6 +101,11 @@ static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
> >         .clk_driver = "clk-mt8183-mm",
> >  };
> >
> > +struct mtk_mmsys {
> > +       void __iomem *regs;
> > +       const struct mtk_mmsys_driver_data *data;
> > +};
> > +
> >  static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur,
> >                                           enum mtk_ddp_comp_id next,
> >                                           unsigned int *addr)
> > @@ -259,21 +264,21 @@ void mtk_mmsys_ddp_connect(struct device *dev,
> >                            enum mtk_ddp_comp_id cur,
> >                            enum mtk_ddp_comp_id next)
> >  {
> > -       void __iomem *config_regs = dev_get_drvdata(dev);
> > +       struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> >         unsigned int addr, value, reg;
> >
> >         value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
> >         if (value) {
> > -               reg = readl_relaxed(config_regs + addr) | value;
> > -               writel_relaxed(reg, config_regs + addr);
> > +               reg = readl_relaxed(mmsys->regs + addr) | value;
> > +               writel_relaxed(reg, mmsys->regs + addr);
> >         }
> >
> > -       mtk_mmsys_ddp_sout_sel(config_regs, cur, next);
> > +       mtk_mmsys_ddp_sout_sel(mmsys->regs, cur, next);
> >
> >         value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
> >         if (value) {
> > -               reg = readl_relaxed(config_regs + addr) | value;
> > -               writel_relaxed(reg, config_regs + addr);
> > +               reg = readl_relaxed(mmsys->regs + addr) | value;
> > +               writel_relaxed(reg, mmsys->regs + addr);
> >         }
> >  }
> >  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect);
> > @@ -282,44 +287,46 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
> >                               enum mtk_ddp_comp_id cur,
> >                               enum mtk_ddp_comp_id next)
> >  {
> > -       void __iomem *config_regs = dev_get_drvdata(dev);
> > +       struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> >         unsigned int addr, value, reg;
> >
> >         value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
> >         if (value) {
> > -               reg = readl_relaxed(config_regs + addr) & ~value;
> > -               writel_relaxed(reg, config_regs + addr);
> > +               reg = readl_relaxed(mmsys->regs + addr) & ~value;
> > +               writel_relaxed(reg, mmsys->regs + addr);
> >         }
> >
> >         value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
> >         if (value) {
> > -               reg = readl_relaxed(config_regs + addr) & ~value;
> > -               writel_relaxed(reg, config_regs + addr);
> > +               reg = readl_relaxed(mmsys->regs + addr) & ~value;
> > +               writel_relaxed(reg, mmsys->regs + addr);
> >         }
> >  }
> >  EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
> >
> >  static int mtk_mmsys_probe(struct platform_device *pdev)
> >  {
> > -       const struct mtk_mmsys_driver_data *data;
> >         struct device *dev = &pdev->dev;
> >         struct platform_device *clks;
> >         struct platform_device *drm;
> > -       void __iomem *config_regs;
> > +       struct mtk_mmsys *mmsys;
> >         int ret;
> >
> > -       config_regs = devm_platform_ioremap_resource(pdev, 0);
> > -       if (IS_ERR(config_regs)) {
> > -               ret = PTR_ERR(config_regs);
> > +       mmsys = devm_kzalloc(dev, sizeof(*mmsys), GFP_KERNEL);
> > +       if (!mmsys)
> > +               return -ENOMEM;
> > +
> > +       mmsys->regs = devm_platform_ioremap_resource(pdev, 0);
> > +       if (IS_ERR(mmsys->regs)) {
> > +               ret = PTR_ERR(mmsys->regs);
> >                 dev_err(dev, "Failed to ioremap mmsys registers: %d\n", ret);
> >                 return ret;
> >         }
> >
> > -       platform_set_drvdata(pdev, config_regs);
> > -
> > -       data = of_device_get_match_data(&pdev->dev);
> > +       mmsys->data = of_device_get_match_data(&pdev->dev);
> > +       platform_set_drvdata(pdev, mmsys);
> >
> > -       clks = platform_device_register_data(&pdev->dev, data->clk_driver,
> > +       clks = platform_device_register_data(&pdev->dev, mmsys->data->clk_driver,
> >                                              PLATFORM_DEVID_AUTO, NULL, 0);
> >         if (IS_ERR(clks))
> >                 return PTR_ERR(clks);
> > --
> > 2.28.0
> >

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH 4/4] soc: mediatek: mmsys: Use an array for setting the routing registers

[Enric Balletbo i Serra]

From: CK Hu <ck.hu@mediatek.com>

Actually, setting the registers for routing, use multiple 'if-else' for different
routes, but this code would be more and more complicated while we
support more and more SoCs. Change that and use a table per SoC so the
code will be more portable and clear.

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

 drivers/soc/mediatek/mtk-mmsys.c | 393 +++++++++++++++++--------------
 1 file changed, 210 insertions(+), 183 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
index da2de8f6969e..f00d6d08c9c5 100644
--- a/drivers/soc/mediatek/mtk-mmsys.c
+++ b/drivers/soc/mediatek/mtk-mmsys.c
@@ -42,39 +42,61 @@
 #define RDMA0_SOUT_DSI1				0x1
 #define RDMA0_SOUT_DSI2				0x4
 #define RDMA0_SOUT_DSI3				0x5
+#define RDMA0_SOUT_MASK				0x7
 #define RDMA1_SOUT_DPI0				0x2
 #define RDMA1_SOUT_DPI1				0x3
 #define RDMA1_SOUT_DSI1				0x1
 #define RDMA1_SOUT_DSI2				0x4
 #define RDMA1_SOUT_DSI3				0x5
+#define RDMA1_SOUT_MASK				0x7
 #define RDMA2_SOUT_DPI0				0x2
 #define RDMA2_SOUT_DPI1				0x3
 #define RDMA2_SOUT_DSI1				0x1
 #define RDMA2_SOUT_DSI2				0x4
 #define RDMA2_SOUT_DSI3				0x5
+#define RDMA2_SOUT_MASK				0x7
 #define DPI0_SEL_IN_RDMA1			0x1
 #define DPI0_SEL_IN_RDMA2			0x3
+#define DPI0_SEL_IN_MASK			0x3
 #define DPI1_SEL_IN_RDMA1			(0x1 << 8)
 #define DPI1_SEL_IN_RDMA2			(0x3 << 8)
+#define DPI1_SEL_IN_MASK			(0x3 << 8)
 #define DSI0_SEL_IN_RDMA1			0x1
 #define DSI0_SEL_IN_RDMA2			0x4
+#define DSI0_SEL_IN_MASK			0x7
 #define DSI1_SEL_IN_RDMA1			0x1
 #define DSI1_SEL_IN_RDMA2			0x4
+#define DSI1_SEL_IN_MASK			0x7
 #define DSI2_SEL_IN_RDMA1			(0x1 << 16)
 #define DSI2_SEL_IN_RDMA2			(0x4 << 16)
+#define DSI2_SEL_IN_MASK			(0x7 << 16)
 #define DSI3_SEL_IN_RDMA1			(0x1 << 16)
 #define DSI3_SEL_IN_RDMA2			(0x4 << 16)
+#define DSI3_SEL_IN_MASK			(0x7 << 16)
 #define COLOR1_SEL_IN_OVL1			0x1
 
 #define OVL_MOUT_EN_RDMA			0x1
 #define BLS_TO_DSI_RDMA1_TO_DPI1		0x8
 #define BLS_TO_DPI_RDMA1_TO_DSI			0x2
+#define BLS_RDMA1_DSI_DPI_MASK			0xf
 #define DSI_SEL_IN_BLS				0x0
 #define DPI_SEL_IN_BLS				0x0
+#define DPI_SEL_IN_MASK				0x1
 #define DSI_SEL_IN_RDMA				0x1
+#define DSI_SEL_IN_MASK				0x1
+
+struct mtk_mmsys_routes {
+	u32 from_comp;
+	u32 to_comp;
+	u32 addr;
+	u32 mask;
+	u32 val;
+};
 
 struct mtk_mmsys_driver_data {
 	const char *clk_driver;
+	const struct mtk_mmsys_routes *routes;
+	const unsigned int num_routes;
 };
 
 static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
@@ -93,10 +115,6 @@ static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
 	.clk_driver = "clk-mt6797-mm",
 };
 
-static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
-	.clk_driver = "clk-mt8173-mm",
-};
-
 static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
 	.clk_driver = "clk-mt8183-mm",
 };
@@ -106,180 +124,192 @@ struct mtk_mmsys {
 	const struct mtk_mmsys_driver_data *data;
 };
 
-static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur,
-					  enum mtk_ddp_comp_id next,
-					  unsigned int *addr)
-{
-	unsigned int value;
-
-	if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
-		*addr = DISP_REG_CONFIG_DISP_OVL0_MOUT_EN;
-		value = OVL0_MOUT_EN_COLOR0;
-	} else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) {
-		*addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN;
-		value = OVL_MOUT_EN_RDMA;
-	} else if (cur == DDP_COMPONENT_OD0 && next == DDP_COMPONENT_RDMA0) {
-		*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
-		value = OD_MOUT_EN_RDMA0;
-	} else if (cur == DDP_COMPONENT_UFOE && next == DDP_COMPONENT_DSI0) {
-		*addr = DISP_REG_CONFIG_DISP_UFOE_MOUT_EN;
-		value = UFOE_MOUT_EN_DSI0;
-	} else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
-		*addr = DISP_REG_CONFIG_DISP_OVL1_MOUT_EN;
-		value = OVL1_MOUT_EN_COLOR1;
-	} else if (cur == DDP_COMPONENT_GAMMA && next == DDP_COMPONENT_RDMA1) {
-		*addr = DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN;
-		value = GAMMA_MOUT_EN_RDMA1;
-	} else if (cur == DDP_COMPONENT_OD1 && next == DDP_COMPONENT_RDMA1) {
-		*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
-		value = OD1_MOUT_EN_RDMA1;
-	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DPI0) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
-		value = RDMA0_SOUT_DPI0;
-	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DPI1) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
-		value = RDMA0_SOUT_DPI1;
-	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI1) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
-		value = RDMA0_SOUT_DSI1;
-	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI2) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
-		value = RDMA0_SOUT_DSI2;
-	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI3) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
-		value = RDMA0_SOUT_DSI3;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI1) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
-		value = RDMA1_SOUT_DSI1;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI2) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
-		value = RDMA1_SOUT_DSI2;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI3) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
-		value = RDMA1_SOUT_DSI3;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
-		value = RDMA1_SOUT_DPI0;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
-		value = RDMA1_SOUT_DPI1;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
-		value = RDMA2_SOUT_DPI0;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
-		value = RDMA2_SOUT_DPI1;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI1) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
-		value = RDMA2_SOUT_DSI1;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI2) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
-		value = RDMA2_SOUT_DSI2;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI3) {
-		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
-		value = RDMA2_SOUT_DSI3;
-	} else {
-		value = 0;
-	}
-
-	return value;
-}
-
-static unsigned int mtk_mmsys_ddp_sel_in(enum mtk_ddp_comp_id cur,
-					 enum mtk_ddp_comp_id next,
-					 unsigned int *addr)
-{
-	unsigned int value;
-
-	if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
-		*addr = DISP_REG_CONFIG_DISP_COLOR0_SEL_IN;
-		value = COLOR0_SEL_IN_OVL0;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
-		*addr = DISP_REG_CONFIG_DPI_SEL_IN;
-		value = DPI0_SEL_IN_RDMA1;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) {
-		*addr = DISP_REG_CONFIG_DPI_SEL_IN;
-		value = DPI1_SEL_IN_RDMA1;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI0) {
-		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
-		value = DSI0_SEL_IN_RDMA1;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI1) {
-		*addr = DISP_REG_CONFIG_DSIO_SEL_IN;
-		value = DSI1_SEL_IN_RDMA1;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI2) {
-		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
-		value = DSI2_SEL_IN_RDMA1;
-	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI3) {
-		*addr = DISP_REG_CONFIG_DSIO_SEL_IN;
-		value = DSI3_SEL_IN_RDMA1;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) {
-		*addr = DISP_REG_CONFIG_DPI_SEL_IN;
-		value = DPI0_SEL_IN_RDMA2;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) {
-		*addr = DISP_REG_CONFIG_DPI_SEL_IN;
-		value = DPI1_SEL_IN_RDMA2;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI0) {
-		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
-		value = DSI0_SEL_IN_RDMA2;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI1) {
-		*addr = DISP_REG_CONFIG_DSIO_SEL_IN;
-		value = DSI1_SEL_IN_RDMA2;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI2) {
-		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
-		value = DSI2_SEL_IN_RDMA2;
-	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI3) {
-		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
-		value = DSI3_SEL_IN_RDMA2;
-	} else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
-		*addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN;
-		value = COLOR1_SEL_IN_OVL1;
-	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
-		*addr = DISP_REG_CONFIG_DSI_SEL;
-		value = DSI_SEL_IN_BLS;
-	} else {
-		value = 0;
+static const struct mtk_mmsys_routes mt8173_mmsys_routing_table[] = {
+	{
+		DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
+		DISP_REG_CONFIG_OUT_SEL,
+		BLS_RDMA1_DSI_DPI_MASK, BLS_TO_DSI_RDMA1_TO_DPI1
+	}, {
+		DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
+		DISP_REG_CONFIG_DSI_SEL,
+		DSI_SEL_IN_MASK, DSI_SEL_IN_BLS
+	}, {
+		DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
+		DISP_REG_CONFIG_OUT_SEL,
+		BLS_RDMA1_DSI_DPI_MASK, BLS_TO_DPI_RDMA1_TO_DSI
+	}, {
+		DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
+		DISP_REG_CONFIG_DSI_SEL,
+		DSI_SEL_IN_MASK, DSI_SEL_IN_RDMA
+	}, {
+		DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
+		DISP_REG_CONFIG_DPI_SEL,
+		DPI_SEL_IN_MASK, DPI_SEL_IN_BLS
+	}, {
+		DDP_COMPONENT_GAMMA, DDP_COMPONENT_RDMA1,
+		DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN,
+		GAMMA_MOUT_EN_RDMA1, GAMMA_MOUT_EN_RDMA1
+	}, {
+		DDP_COMPONENT_OD0, DDP_COMPONENT_RDMA0,
+		DISP_REG_CONFIG_DISP_OD_MOUT_EN,
+		OD_MOUT_EN_RDMA0, OD_MOUT_EN_RDMA0
+	}, {
+		DDP_COMPONENT_OD1, DDP_COMPONENT_RDMA1,
+		DISP_REG_CONFIG_DISP_OD_MOUT_EN,
+		OD1_MOUT_EN_RDMA1, OD1_MOUT_EN_RDMA1
+	}, {
+		DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0,
+		DISP_REG_CONFIG_DISP_OVL0_MOUT_EN,
+		OVL0_MOUT_EN_COLOR0, OVL0_MOUT_EN_COLOR0
+	}, {
+		DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0,
+		DISP_REG_CONFIG_DISP_COLOR0_SEL_IN,
+		COLOR0_SEL_IN_OVL0, COLOR0_SEL_IN_OVL0
+	}, {
+		DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0,
+		DISP_REG_CONFIG_DISP_OVL_MOUT_EN,
+		OVL_MOUT_EN_RDMA, OVL_MOUT_EN_RDMA
+	}, {
+		DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1,
+		DISP_REG_CONFIG_DISP_OVL1_MOUT_EN,
+		OVL1_MOUT_EN_COLOR1, OVL1_MOUT_EN_COLOR1
+	}, {
+		DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1,
+		DISP_REG_CONFIG_DISP_COLOR1_SEL_IN,
+		COLOR1_SEL_IN_OVL1, COLOR1_SEL_IN_OVL1
+	}, {
+		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DPI0,
+		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
+		RDMA0_SOUT_MASK, RDMA0_SOUT_DPI0
+	}, {
+		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DPI1,
+		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
+		RDMA0_SOUT_MASK, RDMA0_SOUT_DPI1
+	}, {
+		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI1,
+		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
+		RDMA0_SOUT_MASK, RDMA0_SOUT_DSI1
+	}, {
+		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI2,
+		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
+		RDMA0_SOUT_MASK, RDMA0_SOUT_DSI2
+	}, {
+		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI3,
+		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
+		RDMA0_SOUT_MASK, RDMA0_SOUT_DSI3
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,
+		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
+		RDMA1_SOUT_MASK, RDMA1_SOUT_DPI0
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,
+		DISP_REG_CONFIG_DPI_SEL_IN,
+		DPI0_SEL_IN_MASK, DPI0_SEL_IN_RDMA1
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI1,
+		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
+		RDMA1_SOUT_MASK, RDMA1_SOUT_DPI1
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI1,
+		DISP_REG_CONFIG_DPI_SEL_IN,
+		DPI1_SEL_IN_MASK, DPI1_SEL_IN_RDMA1
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI0,
+		DISP_REG_CONFIG_DSIE_SEL_IN,
+		DSI0_SEL_IN_MASK, DSI0_SEL_IN_RDMA1
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI1,
+		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
+		RDMA1_SOUT_MASK, RDMA1_SOUT_DSI1
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI1,
+		DISP_REG_CONFIG_DSIO_SEL_IN,
+		DSI1_SEL_IN_MASK, DSI1_SEL_IN_RDMA1
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI2,
+		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
+		RDMA1_SOUT_MASK, RDMA1_SOUT_DSI2
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI2,
+		DISP_REG_CONFIG_DSIE_SEL_IN,
+		DSI2_SEL_IN_MASK, DSI2_SEL_IN_RDMA1
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI3,
+		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
+		RDMA1_SOUT_MASK, RDMA1_SOUT_DSI3
+	}, {
+		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI3,
+		DISP_REG_CONFIG_DSIO_SEL_IN,
+		DSI3_SEL_IN_MASK, DSI3_SEL_IN_RDMA1
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI0,
+		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
+		RDMA2_SOUT_MASK, RDMA2_SOUT_DPI0
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI0,
+		DISP_REG_CONFIG_DPI_SEL_IN,
+		DPI0_SEL_IN_MASK, DPI0_SEL_IN_RDMA2
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI1,
+		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
+		RDMA2_SOUT_MASK, RDMA2_SOUT_DPI1
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI1,
+		DISP_REG_CONFIG_DPI_SEL_IN,
+		DPI1_SEL_IN_MASK, DPI1_SEL_IN_RDMA2
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI0,
+		DISP_REG_CONFIG_DSIE_SEL_IN,
+		DSI0_SEL_IN_MASK, DSI0_SEL_IN_RDMA2
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI1,
+		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
+		RDMA2_SOUT_MASK, RDMA2_SOUT_DSI1
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI1,
+		DISP_REG_CONFIG_DSIO_SEL_IN,
+		DSI1_SEL_IN_MASK, DSI1_SEL_IN_RDMA2
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI2,
+		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
+		RDMA2_SOUT_MASK, RDMA2_SOUT_DSI2
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI2,
+		DISP_REG_CONFIG_DSIE_SEL_IN,
+		DSI2_SEL_IN_MASK, DSI2_SEL_IN_RDMA2
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI3,
+		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
+		RDMA2_SOUT_MASK, RDMA2_SOUT_DSI3
+	}, {
+		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI3,
+		DISP_REG_CONFIG_DSIO_SEL_IN,
+		DSI3_SEL_IN_MASK, DSI3_SEL_IN_RDMA2
 	}
+};
 
-	return value;
-}
-
-static void mtk_mmsys_ddp_sout_sel(void __iomem *config_regs,
-				   enum mtk_ddp_comp_id cur,
-				   enum mtk_ddp_comp_id next)
-{
-	if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
-		writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
-			       config_regs + DISP_REG_CONFIG_OUT_SEL);
-	} 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);
-	}
-}
+static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
+	.clk_driver = "clk-mt8173-mm",
+	.routes = mt8173_mmsys_routing_table,
+	.num_routes = ARRAY_SIZE(mt8173_mmsys_routing_table),
+};
 
 void mtk_mmsys_ddp_connect(struct device *dev,
 			   enum mtk_ddp_comp_id cur,
 			   enum mtk_ddp_comp_id next)
 {
 	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
-	unsigned int addr, value, reg;
-
-	value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
-	if (value) {
-		reg = readl_relaxed(mmsys->regs + addr) | value;
-		writel_relaxed(reg, mmsys->regs + addr);
-	}
-
-	mtk_mmsys_ddp_sout_sel(mmsys->regs, cur, next);
-
-	value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
-	if (value) {
-		reg = readl_relaxed(mmsys->regs + addr) | value;
-		writel_relaxed(reg, mmsys->regs + addr);
-	}
+	const struct mtk_mmsys_routes *routes = mmsys->data->routes;
+	u32 reg;
+	int i;
+
+	for (i = 0; i < mmsys->data->num_routes; i++)
+		if (cur == routes[i].from_comp && next == routes[i].to_comp) {
+			reg = readl(mmsys->regs + routes[i].addr);
+			reg &= ~routes[i].mask;
+			reg |= routes[i].val;
+			writel(reg, mmsys->regs + routes[i].addr);
+		}
 }
 EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_connect);
 
@@ -288,19 +318,16 @@ void mtk_mmsys_ddp_disconnect(struct device *dev,
 			      enum mtk_ddp_comp_id next)
 {
 	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
-	unsigned int addr, value, reg;
-
-	value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
-	if (value) {
-		reg = readl_relaxed(mmsys->regs + addr) & ~value;
-		writel_relaxed(reg, mmsys->regs + addr);
-	}
-
-	value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
-	if (value) {
-		reg = readl_relaxed(mmsys->regs + addr) & ~value;
-		writel_relaxed(reg, mmsys->regs + addr);
-	}
+	const struct mtk_mmsys_routes *routes = mmsys->data->routes;
+	u32 reg;
+	int i;
+
+	for (i = 0; i < mmsys->data->num_routes; i++)
+		if (cur == routes[i].from_comp && next == routes[i].to_comp) {
+			reg = readl(mmsys->regs + routes[i].addr);
+			reg &= ~routes[i].mask;
+			writel(reg, mmsys->regs + routes[i].addr);
+		}
 }
 EXPORT_SYMBOL_GPL(mtk_mmsys_ddp_disconnect);
 
-- 
2.28.0


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 4/4] soc: mediatek: mmsys: Use an array for setting the routing registers

[Chun-Kuang Hu]

Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年10月7日 週三 上午3:33寫道：
>
> From: CK Hu <ck.hu@mediatek.com>
>
> Actually, setting the registers for routing, use multiple 'if-else' for different
> routes, but this code would be more and more complicated while we
> support more and more SoCs. Change that and use a table per SoC so the
> code will be more portable and clear.

Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>

>
> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
>  drivers/soc/mediatek/mtk-mmsys.c | 393 +++++++++++++++++--------------
>  1 file changed, 210 insertions(+), 183 deletions(-)
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 4/4] soc: mediatek: mmsys: Use an array for setting the routing registers

[Chun-Kuang Hu]

Hi, Enric:

Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年10月7日 週三 上午3:33寫道：
>
> From: CK Hu <ck.hu@mediatek.com>
>
> Actually, setting the registers for routing, use multiple 'if-else' for different
> routes, but this code would be more and more complicated while we
> support more and more SoCs. Change that and use a table per SoC so the
> code will be more portable and clear.
>
> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
>  drivers/soc/mediatek/mtk-mmsys.c | 393 +++++++++++++++++--------------
>  1 file changed, 210 insertions(+), 183 deletions(-)
>

[snip]

>
>  static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
> @@ -93,10 +115,6 @@ static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
>         .clk_driver = "clk-mt6797-mm",
>  };
>
> -static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> -       .clk_driver = "clk-mt8173-mm",
> -};
> -
>  static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
>         .clk_driver = "clk-mt8183-mm",
>  };
> @@ -106,180 +124,192 @@ struct mtk_mmsys {
>         const struct mtk_mmsys_driver_data *data;
>  };
>

[snip]

> +static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> +       .clk_driver = "clk-mt8173-mm",
> +       .routes = mt8173_mmsys_routing_table,
> +       .num_routes = ARRAY_SIZE(mt8173_mmsys_routing_table),
> +};
>

I remove my Reviewed-by tag. You does not set routes for mt2701 and
mt2712, but these two SoC need that. Maybe now they use the same table
as mt8173.

Regards,
Chun-Kuang.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 4/4] soc: mediatek: mmsys: Use an array for setting the routing registers

[Enric Balletbo i Serra]

Hi Chun-Kuang,

On 8/10/20 2:01, Chun-Kuang Hu wrote:
> Hi, Enric:
> 
> Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年10月7日 週三 上午3:33寫道：
>>
>> From: CK Hu <ck.hu@mediatek.com>
>>
>> Actually, setting the registers for routing, use multiple 'if-else' for different
>> routes, but this code would be more and more complicated while we
>> support more and more SoCs. Change that and use a table per SoC so the
>> code will be more portable and clear.
>>
>> Signed-off-by: CK Hu <ck.hu@mediatek.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>>  drivers/soc/mediatek/mtk-mmsys.c | 393 +++++++++++++++++--------------
>>  1 file changed, 210 insertions(+), 183 deletions(-)
>>
> 
> [snip]
> 
>>
>>  static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
>> @@ -93,10 +115,6 @@ static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
>>         .clk_driver = "clk-mt6797-mm",
>>  };
>>
>> -static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>> -       .clk_driver = "clk-mt8173-mm",
>> -};
>> -
>>  static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
>>         .clk_driver = "clk-mt8183-mm",
>>  };
>> @@ -106,180 +124,192 @@ struct mtk_mmsys {
>>         const struct mtk_mmsys_driver_data *data;
>>  };
>>
> 
> [snip]
> 
>> +static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>> +       .clk_driver = "clk-mt8173-mm",
>> +       .routes = mt8173_mmsys_routing_table,
>> +       .num_routes = ARRAY_SIZE(mt8173_mmsys_routing_table),
>> +};
>>
> 
> I remove my Reviewed-by tag. You does not set routes for mt2701 and
> mt2712, but these two SoC need that. Maybe now they use the same table
> as mt8173.
> 

I did that on purpose as explained in the cover letter, and asked for someone
with the hardware to provide me a working routing table. But, if you think the
same routing should work on those devices I'm fine to use the same for all the
current devices. I don't have that hardware, so anyway, will need to test.

Thanks,
 Enric

> Regards,
> Chun-Kuang.
> 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 4/4] soc: mediatek: mmsys: Use an array for setting the routing registers

[Matthias Brugger]

On 08/10/2020 09:49, Enric Balletbo i Serra wrote:
> Hi Chun-Kuang,
> 
> On 8/10/20 2:01, Chun-Kuang Hu wrote:
>> Hi, Enric:
>>
>> Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年10月7日 週三 上午3:33寫道：
>>>
>>> From: CK Hu <ck.hu@mediatek.com>
>>>
>>> Actually, setting the registers for routing, use multiple 'if-else' for different
>>> routes, but this code would be more and more complicated while we
>>> support more and more SoCs. Change that and use a table per SoC so the
>>> code will be more portable and clear.
>>>
>>> Signed-off-by: CK Hu <ck.hu@mediatek.com>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> ---
>>>
>>>   drivers/soc/mediatek/mtk-mmsys.c | 393 +++++++++++++++++--------------
>>>   1 file changed, 210 insertions(+), 183 deletions(-)
>>>
>>
>> [snip]
>>
>>>
>>>   static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
>>> @@ -93,10 +115,6 @@ static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
>>>          .clk_driver = "clk-mt6797-mm",
>>>   };
>>>
>>> -static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>>> -       .clk_driver = "clk-mt8173-mm",
>>> -};
>>> -
>>>   static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
>>>          .clk_driver = "clk-mt8183-mm",
>>>   };
>>> @@ -106,180 +124,192 @@ struct mtk_mmsys {
>>>          const struct mtk_mmsys_driver_data *data;
>>>   };
>>>
>>
>> [snip]
>>
>>> +static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>>> +       .clk_driver = "clk-mt8173-mm",
>>> +       .routes = mt8173_mmsys_routing_table,
>>> +       .num_routes = ARRAY_SIZE(mt8173_mmsys_routing_table),
>>> +};
>>>
>>
>> I remove my Reviewed-by tag. You does not set routes for mt2701 and
>> mt2712, but these two SoC need that. Maybe now they use the same table
>> as mt8173.
>>
> 
> I did that on purpose as explained in the cover letter, and asked for someone
> with the hardware to provide me a working routing table. But, if you think the
> same routing should work on those devices I'm fine to use the same for all the
> current devices. I don't have that hardware, so anyway, will need to test.
> 

But you could deduce the routes needed by having a look into the components in 
mtk_drm_drv.c, correct?
Well see my other email, but defining them twice sounds like not a good approach 
to me.

Regards,
Matthias

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 4/4] soc: mediatek: mmsys: Use an array for setting the routing registers

[Matthias Brugger]

Hi Enric and CK,

On 06/10/2020 21:33, Enric Balletbo i Serra wrote:
> From: CK Hu <ck.hu@mediatek.com>
> 
> Actually, setting the registers for routing, use multiple 'if-else' for different
> routes, but this code would be more and more complicated while we
> support more and more SoCs. Change that and use a table per SoC so the
> code will be more portable and clear.
> 

I'd like to have some clarifications on this patch. Now we use one big if-else 
structure to decide which value and address we need. As this values work for a 
large set of SoCs I suppose the hardware block is the same.

When we add new HW like mt8183, mt8192 or mt6779 we will need different values. 
But my question is, if the HW between theses new platforms will be different 
(read values change for every SoC). Or will it be the same HW block for all 
these SoCs so that we could add three new functions ddp_mout_en, ddp_sout_sel 
and ddp_sel_in.

Why I'm asking? Because right now it seems like double the work we have to do 
for every SoC. We will need to define the components in the DRM driver and then 
we need to add the values for the routing.

More comments below.

> Signed-off-by: CK Hu <ck.hu@mediatek.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>   drivers/soc/mediatek/mtk-mmsys.c | 393 +++++++++++++++++--------------
>   1 file changed, 210 insertions(+), 183 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> index da2de8f6969e..f00d6d08c9c5 100644
> --- a/drivers/soc/mediatek/mtk-mmsys.c
> +++ b/drivers/soc/mediatek/mtk-mmsys.c
> @@ -42,39 +42,61 @@
>   #define RDMA0_SOUT_DSI1				0x1
>   #define RDMA0_SOUT_DSI2				0x4
>   #define RDMA0_SOUT_DSI3				0x5
> +#define RDMA0_SOUT_MASK				0x7
>   #define RDMA1_SOUT_DPI0				0x2
>   #define RDMA1_SOUT_DPI1				0x3
>   #define RDMA1_SOUT_DSI1				0x1
>   #define RDMA1_SOUT_DSI2				0x4
>   #define RDMA1_SOUT_DSI3				0x5
> +#define RDMA1_SOUT_MASK				0x7
>   #define RDMA2_SOUT_DPI0				0x2
>   #define RDMA2_SOUT_DPI1				0x3
>   #define RDMA2_SOUT_DSI1				0x1
>   #define RDMA2_SOUT_DSI2				0x4
>   #define RDMA2_SOUT_DSI3				0x5
> +#define RDMA2_SOUT_MASK				0x7
>   #define DPI0_SEL_IN_RDMA1			0x1
>   #define DPI0_SEL_IN_RDMA2			0x3
> +#define DPI0_SEL_IN_MASK			0x3
>   #define DPI1_SEL_IN_RDMA1			(0x1 << 8)
>   #define DPI1_SEL_IN_RDMA2			(0x3 << 8)
> +#define DPI1_SEL_IN_MASK			(0x3 << 8)
>   #define DSI0_SEL_IN_RDMA1			0x1
>   #define DSI0_SEL_IN_RDMA2			0x4
> +#define DSI0_SEL_IN_MASK			0x7
>   #define DSI1_SEL_IN_RDMA1			0x1
>   #define DSI1_SEL_IN_RDMA2			0x4
> +#define DSI1_SEL_IN_MASK			0x7
>   #define DSI2_SEL_IN_RDMA1			(0x1 << 16)
>   #define DSI2_SEL_IN_RDMA2			(0x4 << 16)
> +#define DSI2_SEL_IN_MASK			(0x7 << 16)
>   #define DSI3_SEL_IN_RDMA1			(0x1 << 16)
>   #define DSI3_SEL_IN_RDMA2			(0x4 << 16)
> +#define DSI3_SEL_IN_MASK			(0x7 << 16)
>   #define COLOR1_SEL_IN_OVL1			0x1
>   
>   #define OVL_MOUT_EN_RDMA			0x1
>   #define BLS_TO_DSI_RDMA1_TO_DPI1		0x8
>   #define BLS_TO_DPI_RDMA1_TO_DSI			0x2
> +#define BLS_RDMA1_DSI_DPI_MASK			0xf
>   #define DSI_SEL_IN_BLS				0x0
>   #define DPI_SEL_IN_BLS				0x0
> +#define DPI_SEL_IN_MASK				0x1
>   #define DSI_SEL_IN_RDMA				0x1
> +#define DSI_SEL_IN_MASK				0x1
> +
> +struct mtk_mmsys_routes {
> +	u32 from_comp;
> +	u32 to_comp;
> +	u32 addr;
> +	u32 mask;

We didn't need the mask up to now, if needed in the future, we should add it 
when needed. This will make the code cleaner for now.

> +	u32 val;
> +};
>   
>   struct mtk_mmsys_driver_data {
>   	const char *clk_driver;
> +	const struct mtk_mmsys_routes *routes;
> +	const unsigned int num_routes;
>   };
>   
>   static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
> @@ -93,10 +115,6 @@ static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
>   	.clk_driver = "clk-mt6797-mm",
>   };
>   
> -static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> -	.clk_driver = "clk-mt8173-mm",
> -};
> -
>   static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
>   	.clk_driver = "clk-mt8183-mm",
>   };
> @@ -106,180 +124,192 @@ struct mtk_mmsys {
>   	const struct mtk_mmsys_driver_data *data;
>   };
>   
> -static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur,
> -					  enum mtk_ddp_comp_id next,
> -					  unsigned int *addr)
> -{
> -	unsigned int value;
> -
> -	if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
> -		*addr = DISP_REG_CONFIG_DISP_OVL0_MOUT_EN;
> -		value = OVL0_MOUT_EN_COLOR0;
> -	} else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) {
> -		*addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN;
> -		value = OVL_MOUT_EN_RDMA;
> -	} else if (cur == DDP_COMPONENT_OD0 && next == DDP_COMPONENT_RDMA0) {
> -		*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
> -		value = OD_MOUT_EN_RDMA0;
> -	} else if (cur == DDP_COMPONENT_UFOE && next == DDP_COMPONENT_DSI0) {
> -		*addr = DISP_REG_CONFIG_DISP_UFOE_MOUT_EN;
> -		value = UFOE_MOUT_EN_DSI0;
> -	} else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
> -		*addr = DISP_REG_CONFIG_DISP_OVL1_MOUT_EN;
> -		value = OVL1_MOUT_EN_COLOR1;
> -	} else if (cur == DDP_COMPONENT_GAMMA && next == DDP_COMPONENT_RDMA1) {
> -		*addr = DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN;
> -		value = GAMMA_MOUT_EN_RDMA1;
> -	} else if (cur == DDP_COMPONENT_OD1 && next == DDP_COMPONENT_RDMA1) {
> -		*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
> -		value = OD1_MOUT_EN_RDMA1;
> -	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DPI0) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> -		value = RDMA0_SOUT_DPI0;
> -	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DPI1) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> -		value = RDMA0_SOUT_DPI1;
> -	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI1) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> -		value = RDMA0_SOUT_DSI1;
> -	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI2) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> -		value = RDMA0_SOUT_DSI2;
> -	} else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI3) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> -		value = RDMA0_SOUT_DSI3;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI1) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> -		value = RDMA1_SOUT_DSI1;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI2) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> -		value = RDMA1_SOUT_DSI2;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI3) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> -		value = RDMA1_SOUT_DSI3;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> -		value = RDMA1_SOUT_DPI0;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> -		value = RDMA1_SOUT_DPI1;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> -		value = RDMA2_SOUT_DPI0;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> -		value = RDMA2_SOUT_DPI1;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI1) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> -		value = RDMA2_SOUT_DSI1;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI2) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> -		value = RDMA2_SOUT_DSI2;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI3) {
> -		*addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> -		value = RDMA2_SOUT_DSI3;
> -	} else {
> -		value = 0;
> -	}
> -
> -	return value;
> -}
> -
> -static unsigned int mtk_mmsys_ddp_sel_in(enum mtk_ddp_comp_id cur,
> -					 enum mtk_ddp_comp_id next,
> -					 unsigned int *addr)
> -{
> -	unsigned int value;
> -
> -	if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
> -		*addr = DISP_REG_CONFIG_DISP_COLOR0_SEL_IN;
> -		value = COLOR0_SEL_IN_OVL0;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
> -		*addr = DISP_REG_CONFIG_DPI_SEL_IN;
> -		value = DPI0_SEL_IN_RDMA1;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) {
> -		*addr = DISP_REG_CONFIG_DPI_SEL_IN;
> -		value = DPI1_SEL_IN_RDMA1;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI0) {
> -		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> -		value = DSI0_SEL_IN_RDMA1;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI1) {
> -		*addr = DISP_REG_CONFIG_DSIO_SEL_IN;
> -		value = DSI1_SEL_IN_RDMA1;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI2) {
> -		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> -		value = DSI2_SEL_IN_RDMA1;
> -	} else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI3) {
> -		*addr = DISP_REG_CONFIG_DSIO_SEL_IN;
> -		value = DSI3_SEL_IN_RDMA1;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) {
> -		*addr = DISP_REG_CONFIG_DPI_SEL_IN;
> -		value = DPI0_SEL_IN_RDMA2;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) {
> -		*addr = DISP_REG_CONFIG_DPI_SEL_IN;
> -		value = DPI1_SEL_IN_RDMA2;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI0) {
> -		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> -		value = DSI0_SEL_IN_RDMA2;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI1) {
> -		*addr = DISP_REG_CONFIG_DSIO_SEL_IN;
> -		value = DSI1_SEL_IN_RDMA2;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI2) {
> -		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> -		value = DSI2_SEL_IN_RDMA2;
> -	} else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI3) {
> -		*addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> -		value = DSI3_SEL_IN_RDMA2;
> -	} else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
> -		*addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN;
> -		value = COLOR1_SEL_IN_OVL1;
> -	} else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> -		*addr = DISP_REG_CONFIG_DSI_SEL;
> -		value = DSI_SEL_IN_BLS;
> -	} else {
> -		value = 0;
> +static const struct mtk_mmsys_routes mt8173_mmsys_routing_table[] = {
> +	{
> +		DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
> +		DISP_REG_CONFIG_OUT_SEL,
> +		BLS_RDMA1_DSI_DPI_MASK, BLS_TO_DSI_RDMA1_TO_DPI1
> +	}, {
> +		DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
> +		DISP_REG_CONFIG_DSI_SEL,
> +		DSI_SEL_IN_MASK, DSI_SEL_IN_BLS
> +	}, {
> +		DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
> +		DISP_REG_CONFIG_OUT_SEL,
> +		BLS_RDMA1_DSI_DPI_MASK, BLS_TO_DPI_RDMA1_TO_DSI
> +	}, {
> +		DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
> +		DISP_REG_CONFIG_DSI_SEL,
> +		DSI_SEL_IN_MASK, DSI_SEL_IN_RDMA
> +	}, {
> +		DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
> +		DISP_REG_CONFIG_DPI_SEL,
> +		DPI_SEL_IN_MASK, DPI_SEL_IN_BLS

I wonder if we could make the data structure easier to read by grouping actions 
for the same component tuple together.

> +	}, {
> +		DDP_COMPONENT_GAMMA, DDP_COMPONENT_RDMA1,
> +		DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN,
> +		GAMMA_MOUT_EN_RDMA1, GAMMA_MOUT_EN_RDMA1
> +	}, {
> +		DDP_COMPONENT_OD0, DDP_COMPONENT_RDMA0,
> +		DISP_REG_CONFIG_DISP_OD_MOUT_EN,
> +		OD_MOUT_EN_RDMA0, OD_MOUT_EN_RDMA0
> +	}, {
> +		DDP_COMPONENT_OD1, DDP_COMPONENT_RDMA1,
> +		DISP_REG_CONFIG_DISP_OD_MOUT_EN,
> +		OD1_MOUT_EN_RDMA1, OD1_MOUT_EN_RDMA1
> +	}, {
> +		DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0,
> +		DISP_REG_CONFIG_DISP_OVL0_MOUT_EN,
> +		OVL0_MOUT_EN_COLOR0, OVL0_MOUT_EN_COLOR0
> +	}, {
> +		DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0,
> +		DISP_REG_CONFIG_DISP_COLOR0_SEL_IN,
> +		COLOR0_SEL_IN_OVL0, COLOR0_SEL_IN_OVL0
> +	}, {
> +		DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0,
> +		DISP_REG_CONFIG_DISP_OVL_MOUT_EN,
> +		OVL_MOUT_EN_RDMA, OVL_MOUT_EN_RDMA
> +	}, {
> +		DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1,
> +		DISP_REG_CONFIG_DISP_OVL1_MOUT_EN,
> +		OVL1_MOUT_EN_COLOR1, OVL1_MOUT_EN_COLOR1
> +	}, {
> +		DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1,
> +		DISP_REG_CONFIG_DISP_COLOR1_SEL_IN,
> +		COLOR1_SEL_IN_OVL1, COLOR1_SEL_IN_OVL1
> +	}, {
> +		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DPI0,
> +		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> +		RDMA0_SOUT_MASK, RDMA0_SOUT_DPI0
> +	}, {
> +		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DPI1,
> +		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> +		RDMA0_SOUT_MASK, RDMA0_SOUT_DPI1
> +	}, {
> +		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI1,
> +		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> +		RDMA0_SOUT_MASK, RDMA0_SOUT_DSI1
> +	}, {
> +		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI2,
> +		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> +		RDMA0_SOUT_MASK, RDMA0_SOUT_DSI2
> +	}, {
> +		DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI3,
> +		DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> +		RDMA0_SOUT_MASK, RDMA0_SOUT_DSI3
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,
> +		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> +		RDMA1_SOUT_MASK, RDMA1_SOUT_DPI0
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,
> +		DISP_REG_CONFIG_DPI_SEL_IN,
> +		DPI0_SEL_IN_MASK, DPI0_SEL_IN_RDMA1
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI1,
> +		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> +		RDMA1_SOUT_MASK, RDMA1_SOUT_DPI1
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI1,
> +		DISP_REG_CONFIG_DPI_SEL_IN,
> +		DPI1_SEL_IN_MASK, DPI1_SEL_IN_RDMA1
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI0,
> +		DISP_REG_CONFIG_DSIE_SEL_IN,
> +		DSI0_SEL_IN_MASK, DSI0_SEL_IN_RDMA1
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI1,
> +		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> +		RDMA1_SOUT_MASK, RDMA1_SOUT_DSI1
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI1,
> +		DISP_REG_CONFIG_DSIO_SEL_IN,
> +		DSI1_SEL_IN_MASK, DSI1_SEL_IN_RDMA1
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI2,
> +		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> +		RDMA1_SOUT_MASK, RDMA1_SOUT_DSI2
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI2,
> +		DISP_REG_CONFIG_DSIE_SEL_IN,
> +		DSI2_SEL_IN_MASK, DSI2_SEL_IN_RDMA1
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI3,
> +		DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> +		RDMA1_SOUT_MASK, RDMA1_SOUT_DSI3
> +	}, {
> +		DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI3,
> +		DISP_REG_CONFIG_DSIO_SEL_IN,
> +		DSI3_SEL_IN_MASK, DSI3_SEL_IN_RDMA1
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI0,
> +		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> +		RDMA2_SOUT_MASK, RDMA2_SOUT_DPI0
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI0,
> +		DISP_REG_CONFIG_DPI_SEL_IN,
> +		DPI0_SEL_IN_MASK, DPI0_SEL_IN_RDMA2
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI1,
> +		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> +		RDMA2_SOUT_MASK, RDMA2_SOUT_DPI1
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI1,
> +		DISP_REG_CONFIG_DPI_SEL_IN,
> +		DPI1_SEL_IN_MASK, DPI1_SEL_IN_RDMA2
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI0,
> +		DISP_REG_CONFIG_DSIE_SEL_IN,
> +		DSI0_SEL_IN_MASK, DSI0_SEL_IN_RDMA2
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI1,
> +		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> +		RDMA2_SOUT_MASK, RDMA2_SOUT_DSI1
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI1,
> +		DISP_REG_CONFIG_DSIO_SEL_IN,
> +		DSI1_SEL_IN_MASK, DSI1_SEL_IN_RDMA2
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI2,
> +		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> +		RDMA2_SOUT_MASK, RDMA2_SOUT_DSI2
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI2,
> +		DISP_REG_CONFIG_DSIE_SEL_IN,
> +		DSI2_SEL_IN_MASK, DSI2_SEL_IN_RDMA2
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI3,
> +		DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> +		RDMA2_SOUT_MASK, RDMA2_SOUT_DSI3
> +	}, {
> +		DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI3,
> +		DISP_REG_CONFIG_DSIO_SEL_IN,
> +		DSI3_SEL_IN_MASK, DSI3_SEL_IN_RDMA2
>   	}
> +};
>   
> -	return value;
> -}
> -
> -static void mtk_mmsys_ddp_sout_sel(void __iomem *config_regs,
> -				   enum mtk_ddp_comp_id cur,
> -				   enum mtk_ddp_comp_id next)
> -{
> -	if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> -		writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
> -			       config_regs + DISP_REG_CONFIG_OUT_SEL);
> -	} 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);
> -	}
> -}
> +static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> +	.clk_driver = "clk-mt8173-mm",
> +	.routes = mt8173_mmsys_routing_table,
> +	.num_routes = ARRAY_SIZE(mt8173_mmsys_routing_table),
> +};
>   
>   void mtk_mmsys_ddp_connect(struct device *dev,
>   			   enum mtk_ddp_comp_id cur,
>   			   enum mtk_ddp_comp_id next)
>   {
>   	struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> -	unsigned int addr, value, reg;
> -
> -	value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
> -	if (value) {
> -		reg = readl_relaxed(mmsys->regs + addr) | value;
> -		writel_relaxed(reg, mmsys->regs + addr);
> -	}
> -
> -	mtk_mmsys_ddp_sout_sel(mmsys->regs, cur, next);
> -
> -	value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
> -	if (value) {
> -		reg = readl_relaxed(mmsys->regs + addr) | value;
> -		writel_relaxed(reg, mmsys->regs + addr);
> -	}

What I don't like of this new approach is the fact, that we loose information 
about the different grouping we had up to now, mout_en, sout_sel and sel_in.

Regards,
Matthias

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 4/4] soc: mediatek: mmsys: Use an array for setting the routing registers

[Chun-Kuang Hu]

Hi, Matthias:

Matthias Brugger <matthias.bgg@gmail.com> 於 2020年10月8日 週四 下午6:22寫道：
>
> Hi Enric and CK,
>
> On 06/10/2020 21:33, Enric Balletbo i Serra wrote:
> > From: CK Hu <ck.hu@mediatek.com>
> >
> > Actually, setting the registers for routing, use multiple 'if-else' for different
> > routes, but this code would be more and more complicated while we
> > support more and more SoCs. Change that and use a table per SoC so the
> > code will be more portable and clear.
> >
>
> I'd like to have some clarifications on this patch. Now we use one big if-else
> structure to decide which value and address we need. As this values work for a
> large set of SoCs I suppose the hardware block is the same.
>
> When we add new HW like mt8183, mt8192 or mt6779 we will need different values.
> But my question is, if the HW between theses new platforms will be different
> (read values change for every SoC). Or will it be the same HW block for all
> these SoCs so that we could add three new functions ddp_mout_en, ddp_sout_sel
> and ddp_sel_in.
>

As I know, routes in mt8173, mt2701, mt2712 are different. That means
in the same register address, it control different input/output
selection for each SoC. But they use a very tricky way to use the same
table: some register's default value (the default routes) meet their
requirement, they do not set it and just set the register whose
default value does not meet the requirement. But this tricky way fail
when support more SoC, so mt8183 and mt8192 need an independent route.
So we have better have different route for each SoC, but we don't have
the complete route information for these three SoC, so just keep them
in the same table. After we've more information, we could separate
mt2701, mt2712 to an independent table.

> Why I'm asking? Because right now it seems like double the work we have to do
> for every SoC. We will need to define the components in the DRM driver and then
> we need to add the values for the routing.

The double work are two different function work. mmsys driver provide
full routing control and drm choose the route according to its
application. For example:

mmsys provide the three route:

rdma0 -> dsi0
rdma1 -> dsi0
rdma2 -> dsi0

and drm could only choose one at some moment. Even drm now just choose
rdma0 -> dsi0, I think mmsys still should provide the full route
control like clock driver (even though some clock is not use by client
driver, clock driver should implement all clock control function)

>
> More comments below.
>
> > Signed-off-by: CK Hu <ck.hu@mediatek.com>
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > ---
> >
> >   drivers/soc/mediatek/mtk-mmsys.c | 393 +++++++++++++++++--------------
> >   1 file changed, 210 insertions(+), 183 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
> > index da2de8f6969e..f00d6d08c9c5 100644
> > --- a/drivers/soc/mediatek/mtk-mmsys.c
> > +++ b/drivers/soc/mediatek/mtk-mmsys.c
> > @@ -42,39 +42,61 @@
> >   #define RDMA0_SOUT_DSI1                             0x1
> >   #define RDMA0_SOUT_DSI2                             0x4
> >   #define RDMA0_SOUT_DSI3                             0x5
> > +#define RDMA0_SOUT_MASK                              0x7
> >   #define RDMA1_SOUT_DPI0                             0x2
> >   #define RDMA1_SOUT_DPI1                             0x3
> >   #define RDMA1_SOUT_DSI1                             0x1
> >   #define RDMA1_SOUT_DSI2                             0x4
> >   #define RDMA1_SOUT_DSI3                             0x5
> > +#define RDMA1_SOUT_MASK                              0x7
> >   #define RDMA2_SOUT_DPI0                             0x2
> >   #define RDMA2_SOUT_DPI1                             0x3
> >   #define RDMA2_SOUT_DSI1                             0x1
> >   #define RDMA2_SOUT_DSI2                             0x4
> >   #define RDMA2_SOUT_DSI3                             0x5
> > +#define RDMA2_SOUT_MASK                              0x7
> >   #define DPI0_SEL_IN_RDMA1                   0x1
> >   #define DPI0_SEL_IN_RDMA2                   0x3
> > +#define DPI0_SEL_IN_MASK                     0x3
> >   #define DPI1_SEL_IN_RDMA1                   (0x1 << 8)
> >   #define DPI1_SEL_IN_RDMA2                   (0x3 << 8)
> > +#define DPI1_SEL_IN_MASK                     (0x3 << 8)
> >   #define DSI0_SEL_IN_RDMA1                   0x1
> >   #define DSI0_SEL_IN_RDMA2                   0x4
> > +#define DSI0_SEL_IN_MASK                     0x7
> >   #define DSI1_SEL_IN_RDMA1                   0x1
> >   #define DSI1_SEL_IN_RDMA2                   0x4
> > +#define DSI1_SEL_IN_MASK                     0x7
> >   #define DSI2_SEL_IN_RDMA1                   (0x1 << 16)
> >   #define DSI2_SEL_IN_RDMA2                   (0x4 << 16)
> > +#define DSI2_SEL_IN_MASK                     (0x7 << 16)
> >   #define DSI3_SEL_IN_RDMA1                   (0x1 << 16)
> >   #define DSI3_SEL_IN_RDMA2                   (0x4 << 16)
> > +#define DSI3_SEL_IN_MASK                     (0x7 << 16)
> >   #define COLOR1_SEL_IN_OVL1                  0x1
> >
> >   #define OVL_MOUT_EN_RDMA                    0x1
> >   #define BLS_TO_DSI_RDMA1_TO_DPI1            0x8
> >   #define BLS_TO_DPI_RDMA1_TO_DSI                     0x2
> > +#define BLS_RDMA1_DSI_DPI_MASK                       0xf
> >   #define DSI_SEL_IN_BLS                              0x0
> >   #define DPI_SEL_IN_BLS                              0x0
> > +#define DPI_SEL_IN_MASK                              0x1
> >   #define DSI_SEL_IN_RDMA                             0x1
> > +#define DSI_SEL_IN_MASK                              0x1
> > +
> > +struct mtk_mmsys_routes {
> > +     u32 from_comp;
> > +     u32 to_comp;
> > +     u32 addr;
> > +     u32 mask;
>
> We didn't need the mask up to now, if needed in the future, we should add it
> when needed. This will make the code cleaner for now.

Agree.

>
> > +     u32 val;
> > +};
> >
> >   struct mtk_mmsys_driver_data {
> >       const char *clk_driver;
> > +     const struct mtk_mmsys_routes *routes;
> > +     const unsigned int num_routes;
> >   };
> >
> >   static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
> > @@ -93,10 +115,6 @@ static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
> >       .clk_driver = "clk-mt6797-mm",
> >   };
> >
> > -static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> > -     .clk_driver = "clk-mt8173-mm",
> > -};
> > -
> >   static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
> >       .clk_driver = "clk-mt8183-mm",
> >   };
> > @@ -106,180 +124,192 @@ struct mtk_mmsys {
> >       const struct mtk_mmsys_driver_data *data;
> >   };
> >
> > -static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur,
> > -                                       enum mtk_ddp_comp_id next,
> > -                                       unsigned int *addr)
> > -{
> > -     unsigned int value;
> > -
> > -     if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
> > -             *addr = DISP_REG_CONFIG_DISP_OVL0_MOUT_EN;
> > -             value = OVL0_MOUT_EN_COLOR0;
> > -     } else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) {
> > -             *addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN;
> > -             value = OVL_MOUT_EN_RDMA;
> > -     } else if (cur == DDP_COMPONENT_OD0 && next == DDP_COMPONENT_RDMA0) {
> > -             *addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
> > -             value = OD_MOUT_EN_RDMA0;
> > -     } else if (cur == DDP_COMPONENT_UFOE && next == DDP_COMPONENT_DSI0) {
> > -             *addr = DISP_REG_CONFIG_DISP_UFOE_MOUT_EN;
> > -             value = UFOE_MOUT_EN_DSI0;
> > -     } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
> > -             *addr = DISP_REG_CONFIG_DISP_OVL1_MOUT_EN;
> > -             value = OVL1_MOUT_EN_COLOR1;
> > -     } else if (cur == DDP_COMPONENT_GAMMA && next == DDP_COMPONENT_RDMA1) {
> > -             *addr = DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN;
> > -             value = GAMMA_MOUT_EN_RDMA1;
> > -     } else if (cur == DDP_COMPONENT_OD1 && next == DDP_COMPONENT_RDMA1) {
> > -             *addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
> > -             value = OD1_MOUT_EN_RDMA1;
> > -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DPI0) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> > -             value = RDMA0_SOUT_DPI0;
> > -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DPI1) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> > -             value = RDMA0_SOUT_DPI1;
> > -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI1) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> > -             value = RDMA0_SOUT_DSI1;
> > -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI2) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> > -             value = RDMA0_SOUT_DSI2;
> > -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI3) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
> > -             value = RDMA0_SOUT_DSI3;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI1) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> > -             value = RDMA1_SOUT_DSI1;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI2) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> > -             value = RDMA1_SOUT_DSI2;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI3) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> > -             value = RDMA1_SOUT_DSI3;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> > -             value = RDMA1_SOUT_DPI0;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
> > -             value = RDMA1_SOUT_DPI1;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> > -             value = RDMA2_SOUT_DPI0;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> > -             value = RDMA2_SOUT_DPI1;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI1) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> > -             value = RDMA2_SOUT_DSI1;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI2) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> > -             value = RDMA2_SOUT_DSI2;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI3) {
> > -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
> > -             value = RDMA2_SOUT_DSI3;
> > -     } else {
> > -             value = 0;
> > -     }
> > -
> > -     return value;
> > -}
> > -
> > -static unsigned int mtk_mmsys_ddp_sel_in(enum mtk_ddp_comp_id cur,
> > -                                      enum mtk_ddp_comp_id next,
> > -                                      unsigned int *addr)
> > -{
> > -     unsigned int value;
> > -
> > -     if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
> > -             *addr = DISP_REG_CONFIG_DISP_COLOR0_SEL_IN;
> > -             value = COLOR0_SEL_IN_OVL0;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
> > -             *addr = DISP_REG_CONFIG_DPI_SEL_IN;
> > -             value = DPI0_SEL_IN_RDMA1;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) {
> > -             *addr = DISP_REG_CONFIG_DPI_SEL_IN;
> > -             value = DPI1_SEL_IN_RDMA1;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI0) {
> > -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> > -             value = DSI0_SEL_IN_RDMA1;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI1) {
> > -             *addr = DISP_REG_CONFIG_DSIO_SEL_IN;
> > -             value = DSI1_SEL_IN_RDMA1;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI2) {
> > -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> > -             value = DSI2_SEL_IN_RDMA1;
> > -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI3) {
> > -             *addr = DISP_REG_CONFIG_DSIO_SEL_IN;
> > -             value = DSI3_SEL_IN_RDMA1;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) {
> > -             *addr = DISP_REG_CONFIG_DPI_SEL_IN;
> > -             value = DPI0_SEL_IN_RDMA2;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) {
> > -             *addr = DISP_REG_CONFIG_DPI_SEL_IN;
> > -             value = DPI1_SEL_IN_RDMA2;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI0) {
> > -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> > -             value = DSI0_SEL_IN_RDMA2;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI1) {
> > -             *addr = DISP_REG_CONFIG_DSIO_SEL_IN;
> > -             value = DSI1_SEL_IN_RDMA2;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI2) {
> > -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> > -             value = DSI2_SEL_IN_RDMA2;
> > -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI3) {
> > -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
> > -             value = DSI3_SEL_IN_RDMA2;
> > -     } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
> > -             *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN;
> > -             value = COLOR1_SEL_IN_OVL1;
> > -     } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> > -             *addr = DISP_REG_CONFIG_DSI_SEL;
> > -             value = DSI_SEL_IN_BLS;
> > -     } else {
> > -             value = 0;
> > +static const struct mtk_mmsys_routes mt8173_mmsys_routing_table[] = {
> > +     {
> > +             DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
> > +             DISP_REG_CONFIG_OUT_SEL,
> > +             BLS_RDMA1_DSI_DPI_MASK, BLS_TO_DSI_RDMA1_TO_DPI1
> > +     }, {
> > +             DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
> > +             DISP_REG_CONFIG_DSI_SEL,
> > +             DSI_SEL_IN_MASK, DSI_SEL_IN_BLS
> > +     }, {
> > +             DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
> > +             DISP_REG_CONFIG_OUT_SEL,
> > +             BLS_RDMA1_DSI_DPI_MASK, BLS_TO_DPI_RDMA1_TO_DSI
> > +     }, {
> > +             DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
> > +             DISP_REG_CONFIG_DSI_SEL,
> > +             DSI_SEL_IN_MASK, DSI_SEL_IN_RDMA
> > +     }, {
> > +             DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
> > +             DISP_REG_CONFIG_DPI_SEL,
> > +             DPI_SEL_IN_MASK, DPI_SEL_IN_BLS
>
> I wonder if we could make the data structure easier to read by grouping actions
> for the same component tuple together.

Maybe if-else is the simple way to group then. Two layer array could
achieve this but it's more complicated.

>
> > +     }, {
> > +             DDP_COMPONENT_GAMMA, DDP_COMPONENT_RDMA1,
> > +             DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN,
> > +             GAMMA_MOUT_EN_RDMA1, GAMMA_MOUT_EN_RDMA1
> > +     }, {
> > +             DDP_COMPONENT_OD0, DDP_COMPONENT_RDMA0,
> > +             DISP_REG_CONFIG_DISP_OD_MOUT_EN,
> > +             OD_MOUT_EN_RDMA0, OD_MOUT_EN_RDMA0
> > +     }, {
> > +             DDP_COMPONENT_OD1, DDP_COMPONENT_RDMA1,
> > +             DISP_REG_CONFIG_DISP_OD_MOUT_EN,
> > +             OD1_MOUT_EN_RDMA1, OD1_MOUT_EN_RDMA1
> > +     }, {
> > +             DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0,
> > +             DISP_REG_CONFIG_DISP_OVL0_MOUT_EN,
> > +             OVL0_MOUT_EN_COLOR0, OVL0_MOUT_EN_COLOR0
> > +     }, {
> > +             DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0,
> > +             DISP_REG_CONFIG_DISP_COLOR0_SEL_IN,
> > +             COLOR0_SEL_IN_OVL0, COLOR0_SEL_IN_OVL0
> > +     }, {
> > +             DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0,
> > +             DISP_REG_CONFIG_DISP_OVL_MOUT_EN,
> > +             OVL_MOUT_EN_RDMA, OVL_MOUT_EN_RDMA
> > +     }, {
> > +             DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1,
> > +             DISP_REG_CONFIG_DISP_OVL1_MOUT_EN,
> > +             OVL1_MOUT_EN_COLOR1, OVL1_MOUT_EN_COLOR1
> > +     }, {
> > +             DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1,
> > +             DISP_REG_CONFIG_DISP_COLOR1_SEL_IN,
> > +             COLOR1_SEL_IN_OVL1, COLOR1_SEL_IN_OVL1
> > +     }, {
> > +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DPI0,
> > +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> > +             RDMA0_SOUT_MASK, RDMA0_SOUT_DPI0
> > +     }, {
> > +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DPI1,
> > +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> > +             RDMA0_SOUT_MASK, RDMA0_SOUT_DPI1
> > +     }, {
> > +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI1,
> > +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> > +             RDMA0_SOUT_MASK, RDMA0_SOUT_DSI1
> > +     }, {
> > +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI2,
> > +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> > +             RDMA0_SOUT_MASK, RDMA0_SOUT_DSI2
> > +     }, {
> > +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI3,
> > +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
> > +             RDMA0_SOUT_MASK, RDMA0_SOUT_DSI3
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,
> > +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> > +             RDMA1_SOUT_MASK, RDMA1_SOUT_DPI0
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,
> > +             DISP_REG_CONFIG_DPI_SEL_IN,
> > +             DPI0_SEL_IN_MASK, DPI0_SEL_IN_RDMA1
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI1,
> > +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> > +             RDMA1_SOUT_MASK, RDMA1_SOUT_DPI1
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI1,
> > +             DISP_REG_CONFIG_DPI_SEL_IN,
> > +             DPI1_SEL_IN_MASK, DPI1_SEL_IN_RDMA1
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI0,
> > +             DISP_REG_CONFIG_DSIE_SEL_IN,
> > +             DSI0_SEL_IN_MASK, DSI0_SEL_IN_RDMA1
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI1,
> > +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> > +             RDMA1_SOUT_MASK, RDMA1_SOUT_DSI1
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI1,
> > +             DISP_REG_CONFIG_DSIO_SEL_IN,
> > +             DSI1_SEL_IN_MASK, DSI1_SEL_IN_RDMA1
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI2,
> > +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> > +             RDMA1_SOUT_MASK, RDMA1_SOUT_DSI2
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI2,
> > +             DISP_REG_CONFIG_DSIE_SEL_IN,
> > +             DSI2_SEL_IN_MASK, DSI2_SEL_IN_RDMA1
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI3,
> > +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
> > +             RDMA1_SOUT_MASK, RDMA1_SOUT_DSI3
> > +     }, {
> > +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI3,
> > +             DISP_REG_CONFIG_DSIO_SEL_IN,
> > +             DSI3_SEL_IN_MASK, DSI3_SEL_IN_RDMA1
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI0,
> > +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> > +             RDMA2_SOUT_MASK, RDMA2_SOUT_DPI0
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI0,
> > +             DISP_REG_CONFIG_DPI_SEL_IN,
> > +             DPI0_SEL_IN_MASK, DPI0_SEL_IN_RDMA2
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI1,
> > +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> > +             RDMA2_SOUT_MASK, RDMA2_SOUT_DPI1
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI1,
> > +             DISP_REG_CONFIG_DPI_SEL_IN,
> > +             DPI1_SEL_IN_MASK, DPI1_SEL_IN_RDMA2
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI0,
> > +             DISP_REG_CONFIG_DSIE_SEL_IN,
> > +             DSI0_SEL_IN_MASK, DSI0_SEL_IN_RDMA2
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI1,
> > +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> > +             RDMA2_SOUT_MASK, RDMA2_SOUT_DSI1
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI1,
> > +             DISP_REG_CONFIG_DSIO_SEL_IN,
> > +             DSI1_SEL_IN_MASK, DSI1_SEL_IN_RDMA2
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI2,
> > +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> > +             RDMA2_SOUT_MASK, RDMA2_SOUT_DSI2
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI2,
> > +             DISP_REG_CONFIG_DSIE_SEL_IN,
> > +             DSI2_SEL_IN_MASK, DSI2_SEL_IN_RDMA2
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI3,
> > +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
> > +             RDMA2_SOUT_MASK, RDMA2_SOUT_DSI3
> > +     }, {
> > +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI3,
> > +             DISP_REG_CONFIG_DSIO_SEL_IN,
> > +             DSI3_SEL_IN_MASK, DSI3_SEL_IN_RDMA2
> >       }
> > +};
> >
> > -     return value;
> > -}
> > -
> > -static void mtk_mmsys_ddp_sout_sel(void __iomem *config_regs,
> > -                                enum mtk_ddp_comp_id cur,
> > -                                enum mtk_ddp_comp_id next)
> > -{
> > -     if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
> > -             writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
> > -                            config_regs + DISP_REG_CONFIG_OUT_SEL);
> > -     } 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);
> > -     }
> > -}
> > +static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
> > +     .clk_driver = "clk-mt8173-mm",
> > +     .routes = mt8173_mmsys_routing_table,
> > +     .num_routes = ARRAY_SIZE(mt8173_mmsys_routing_table),
> > +};
> >
> >   void mtk_mmsys_ddp_connect(struct device *dev,
> >                          enum mtk_ddp_comp_id cur,
> >                          enum mtk_ddp_comp_id next)
> >   {
> >       struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
> > -     unsigned int addr, value, reg;
> > -
> > -     value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
> > -     if (value) {
> > -             reg = readl_relaxed(mmsys->regs + addr) | value;
> > -             writel_relaxed(reg, mmsys->regs + addr);
> > -     }
> > -
> > -     mtk_mmsys_ddp_sout_sel(mmsys->regs, cur, next);
> > -
> > -     value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
> > -     if (value) {
> > -             reg = readl_relaxed(mmsys->regs + addr) | value;
> > -             writel_relaxed(reg, mmsys->regs + addr);
> > -     }
>
> What I don't like of this new approach is the fact, that we loose information
> about the different grouping we had up to now, mout_en, sout_sel and sel_in.

Agree. Maybe we should keep if-else model in mt8173, mt2701, mt2712.
And ask mt8183 [1] , mt8192 [2] to use if-else model.

[1] https://patchwork.kernel.org/patch/11706231/
[2] https://patchwork.kernel.org/patch/11725595/

Regards,
Chun-Kuang.

>
> Regards,
> Matthias

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 4/4] soc: mediatek: mmsys: Use an array for setting the routing registers

[Enric Balletbo i Serra]

Hi Matthias and Chun-Kuang,

On 11/10/20 4:22, Chun-Kuang Hu wrote:
> Hi, Matthias:
> 
> Matthias Brugger <matthias.bgg@gmail.com> 於 2020年10月8日 週四 下午6:22寫道：
>>
>> Hi Enric and CK,
>>
>> On 06/10/2020 21:33, Enric Balletbo i Serra wrote:
>>> From: CK Hu <ck.hu@mediatek.com>
>>>
>>> Actually, setting the registers for routing, use multiple 'if-else' for different
>>> routes, but this code would be more and more complicated while we
>>> support more and more SoCs. Change that and use a table per SoC so the
>>> code will be more portable and clear.
>>>
>>
>> I'd like to have some clarifications on this patch. Now we use one big if-else
>> structure to decide which value and address we need. As this values work for a
>> large set of SoCs I suppose the hardware block is the same.
>>
>> When we add new HW like mt8183, mt8192 or mt6779 we will need different values.
>> But my question is, if the HW between theses new platforms will be different
>> (read values change for every SoC). Or will it be the same HW block for all
>> these SoCs so that we could add three new functions ddp_mout_en, ddp_sout_sel
>> and ddp_sel_in.
>>
> 
> As I know, routes in mt8173, mt2701, mt2712 are different. That means
> in the same register address, it control different input/output
> selection for each SoC. But they use a very tricky way to use the same
> table: some register's default value (the default routes) meet their
> requirement, they do not set it and just set the register whose
> default value does not meet the requirement. But this tricky way fail
> when support more SoC, so mt8183 and mt8192 need an independent route.
> So we have better have different route for each SoC, but we don't have
> the complete route information for these three SoC, so just keep them
> in the same table. After we've more information, we could separate
> mt2701, mt2712 to an independent table.
> 
>> Why I'm asking? Because right now it seems like double the work we have to do
>> for every SoC. We will need to define the components in the DRM driver and then
>> we need to add the values for the routing.
> 
> The double work are two different function work. mmsys driver provide
> full routing control and drm choose the route according to its
> application. For example:
> 
> mmsys provide the three route:
> 
> rdma0 -> dsi0
> rdma1 -> dsi0
> rdma2 -> dsi0
> 
> and drm could only choose one at some moment. Even drm now just choose
> rdma0 -> dsi0, I think mmsys still should provide the full route
> control like clock driver (even though some clock is not use by client
> driver, clock driver should implement all clock control function)
> 
>>
>> More comments below.
>>
>>> Signed-off-by: CK Hu <ck.hu@mediatek.com>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> ---
>>>
>>>   drivers/soc/mediatek/mtk-mmsys.c | 393 +++++++++++++++++--------------
>>>   1 file changed, 210 insertions(+), 183 deletions(-)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
>>> index da2de8f6969e..f00d6d08c9c5 100644
>>> --- a/drivers/soc/mediatek/mtk-mmsys.c
>>> +++ b/drivers/soc/mediatek/mtk-mmsys.c
>>> @@ -42,39 +42,61 @@
>>>   #define RDMA0_SOUT_DSI1                             0x1
>>>   #define RDMA0_SOUT_DSI2                             0x4
>>>   #define RDMA0_SOUT_DSI3                             0x5
>>> +#define RDMA0_SOUT_MASK                              0x7
>>>   #define RDMA1_SOUT_DPI0                             0x2
>>>   #define RDMA1_SOUT_DPI1                             0x3
>>>   #define RDMA1_SOUT_DSI1                             0x1
>>>   #define RDMA1_SOUT_DSI2                             0x4
>>>   #define RDMA1_SOUT_DSI3                             0x5
>>> +#define RDMA1_SOUT_MASK                              0x7
>>>   #define RDMA2_SOUT_DPI0                             0x2
>>>   #define RDMA2_SOUT_DPI1                             0x3
>>>   #define RDMA2_SOUT_DSI1                             0x1
>>>   #define RDMA2_SOUT_DSI2                             0x4
>>>   #define RDMA2_SOUT_DSI3                             0x5
>>> +#define RDMA2_SOUT_MASK                              0x7
>>>   #define DPI0_SEL_IN_RDMA1                   0x1
>>>   #define DPI0_SEL_IN_RDMA2                   0x3
>>> +#define DPI0_SEL_IN_MASK                     0x3
>>>   #define DPI1_SEL_IN_RDMA1                   (0x1 << 8)
>>>   #define DPI1_SEL_IN_RDMA2                   (0x3 << 8)
>>> +#define DPI1_SEL_IN_MASK                     (0x3 << 8)
>>>   #define DSI0_SEL_IN_RDMA1                   0x1
>>>   #define DSI0_SEL_IN_RDMA2                   0x4
>>> +#define DSI0_SEL_IN_MASK                     0x7
>>>   #define DSI1_SEL_IN_RDMA1                   0x1
>>>   #define DSI1_SEL_IN_RDMA2                   0x4
>>> +#define DSI1_SEL_IN_MASK                     0x7
>>>   #define DSI2_SEL_IN_RDMA1                   (0x1 << 16)
>>>   #define DSI2_SEL_IN_RDMA2                   (0x4 << 16)
>>> +#define DSI2_SEL_IN_MASK                     (0x7 << 16)
>>>   #define DSI3_SEL_IN_RDMA1                   (0x1 << 16)
>>>   #define DSI3_SEL_IN_RDMA2                   (0x4 << 16)
>>> +#define DSI3_SEL_IN_MASK                     (0x7 << 16)
>>>   #define COLOR1_SEL_IN_OVL1                  0x1
>>>
>>>   #define OVL_MOUT_EN_RDMA                    0x1
>>>   #define BLS_TO_DSI_RDMA1_TO_DPI1            0x8
>>>   #define BLS_TO_DPI_RDMA1_TO_DSI                     0x2
>>> +#define BLS_RDMA1_DSI_DPI_MASK                       0xf
>>>   #define DSI_SEL_IN_BLS                              0x0
>>>   #define DPI_SEL_IN_BLS                              0x0
>>> +#define DPI_SEL_IN_MASK                              0x1
>>>   #define DSI_SEL_IN_RDMA                             0x1
>>> +#define DSI_SEL_IN_MASK                              0x1
>>> +
>>> +struct mtk_mmsys_routes {
>>> +     u32 from_comp;
>>> +     u32 to_comp;
>>> +     u32 addr;
>>> +     u32 mask;
>>
>> We didn't need the mask up to now, if needed in the future, we should add it
>> when needed. This will make the code cleaner for now.
> 
> Agree.
> 
>>
>>> +     u32 val;
>>> +};
>>>
>>>   struct mtk_mmsys_driver_data {
>>>       const char *clk_driver;
>>> +     const struct mtk_mmsys_routes *routes;
>>> +     const unsigned int num_routes;
>>>   };
>>>
>>>   static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = {
>>> @@ -93,10 +115,6 @@ static const struct mtk_mmsys_driver_data mt6797_mmsys_driver_data = {
>>>       .clk_driver = "clk-mt6797-mm",
>>>   };
>>>
>>> -static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>>> -     .clk_driver = "clk-mt8173-mm",
>>> -};
>>> -
>>>   static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
>>>       .clk_driver = "clk-mt8183-mm",
>>>   };
>>> @@ -106,180 +124,192 @@ struct mtk_mmsys {
>>>       const struct mtk_mmsys_driver_data *data;
>>>   };
>>>
>>> -static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur,
>>> -                                       enum mtk_ddp_comp_id next,
>>> -                                       unsigned int *addr)
>>> -{
>>> -     unsigned int value;
>>> -
>>> -     if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
>>> -             *addr = DISP_REG_CONFIG_DISP_OVL0_MOUT_EN;
>>> -             value = OVL0_MOUT_EN_COLOR0;
>>> -     } else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) {
>>> -             *addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN;
>>> -             value = OVL_MOUT_EN_RDMA;
>>> -     } else if (cur == DDP_COMPONENT_OD0 && next == DDP_COMPONENT_RDMA0) {
>>> -             *addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
>>> -             value = OD_MOUT_EN_RDMA0;
>>> -     } else if (cur == DDP_COMPONENT_UFOE && next == DDP_COMPONENT_DSI0) {
>>> -             *addr = DISP_REG_CONFIG_DISP_UFOE_MOUT_EN;
>>> -             value = UFOE_MOUT_EN_DSI0;
>>> -     } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_OVL1_MOUT_EN;
>>> -             value = OVL1_MOUT_EN_COLOR1;
>>> -     } else if (cur == DDP_COMPONENT_GAMMA && next == DDP_COMPONENT_RDMA1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN;
>>> -             value = GAMMA_MOUT_EN_RDMA1;
>>> -     } else if (cur == DDP_COMPONENT_OD1 && next == DDP_COMPONENT_RDMA1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
>>> -             value = OD1_MOUT_EN_RDMA1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DPI0) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
>>> -             value = RDMA0_SOUT_DPI0;
>>> -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DPI1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
>>> -             value = RDMA0_SOUT_DPI1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
>>> -             value = RDMA0_SOUT_DSI1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI2) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
>>> -             value = RDMA0_SOUT_DSI2;
>>> -     } else if (cur == DDP_COMPONENT_RDMA0 && next == DDP_COMPONENT_DSI3) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN;
>>> -             value = RDMA0_SOUT_DSI3;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
>>> -             value = RDMA1_SOUT_DSI1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI2) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
>>> -             value = RDMA1_SOUT_DSI2;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI3) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
>>> -             value = RDMA1_SOUT_DSI3;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
>>> -             value = RDMA1_SOUT_DPI0;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN;
>>> -             value = RDMA1_SOUT_DPI1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
>>> -             value = RDMA2_SOUT_DPI0;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
>>> -             value = RDMA2_SOUT_DPI1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
>>> -             value = RDMA2_SOUT_DSI1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI2) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
>>> -             value = RDMA2_SOUT_DSI2;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI3) {
>>> -             *addr = DISP_REG_CONFIG_DISP_RDMA2_SOUT;
>>> -             value = RDMA2_SOUT_DSI3;
>>> -     } else {
>>> -             value = 0;
>>> -     }
>>> -
>>> -     return value;
>>> -}
>>> -
>>> -static unsigned int mtk_mmsys_ddp_sel_in(enum mtk_ddp_comp_id cur,
>>> -                                      enum mtk_ddp_comp_id next,
>>> -                                      unsigned int *addr)
>>> -{
>>> -     unsigned int value;
>>> -
>>> -     if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
>>> -             *addr = DISP_REG_CONFIG_DISP_COLOR0_SEL_IN;
>>> -             value = COLOR0_SEL_IN_OVL0;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI0) {
>>> -             *addr = DISP_REG_CONFIG_DPI_SEL_IN;
>>> -             value = DPI0_SEL_IN_RDMA1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DPI1) {
>>> -             *addr = DISP_REG_CONFIG_DPI_SEL_IN;
>>> -             value = DPI1_SEL_IN_RDMA1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI0) {
>>> -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
>>> -             value = DSI0_SEL_IN_RDMA1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI1) {
>>> -             *addr = DISP_REG_CONFIG_DSIO_SEL_IN;
>>> -             value = DSI1_SEL_IN_RDMA1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI2) {
>>> -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
>>> -             value = DSI2_SEL_IN_RDMA1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA1 && next == DDP_COMPONENT_DSI3) {
>>> -             *addr = DISP_REG_CONFIG_DSIO_SEL_IN;
>>> -             value = DSI3_SEL_IN_RDMA1;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI0) {
>>> -             *addr = DISP_REG_CONFIG_DPI_SEL_IN;
>>> -             value = DPI0_SEL_IN_RDMA2;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DPI1) {
>>> -             *addr = DISP_REG_CONFIG_DPI_SEL_IN;
>>> -             value = DPI1_SEL_IN_RDMA2;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI0) {
>>> -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
>>> -             value = DSI0_SEL_IN_RDMA2;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI1) {
>>> -             *addr = DISP_REG_CONFIG_DSIO_SEL_IN;
>>> -             value = DSI1_SEL_IN_RDMA2;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI2) {
>>> -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
>>> -             value = DSI2_SEL_IN_RDMA2;
>>> -     } else if (cur == DDP_COMPONENT_RDMA2 && next == DDP_COMPONENT_DSI3) {
>>> -             *addr = DISP_REG_CONFIG_DSIE_SEL_IN;
>>> -             value = DSI3_SEL_IN_RDMA2;
>>> -     } else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
>>> -             *addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN;
>>> -             value = COLOR1_SEL_IN_OVL1;
>>> -     } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
>>> -             *addr = DISP_REG_CONFIG_DSI_SEL;
>>> -             value = DSI_SEL_IN_BLS;
>>> -     } else {
>>> -             value = 0;
>>> +static const struct mtk_mmsys_routes mt8173_mmsys_routing_table[] = {
>>> +     {
>>> +             DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
>>> +             DISP_REG_CONFIG_OUT_SEL,
>>> +             BLS_RDMA1_DSI_DPI_MASK, BLS_TO_DSI_RDMA1_TO_DPI1
>>> +     }, {
>>> +             DDP_COMPONENT_BLS, DDP_COMPONENT_DSI0,
>>> +             DISP_REG_CONFIG_DSI_SEL,
>>> +             DSI_SEL_IN_MASK, DSI_SEL_IN_BLS
>>> +     }, {
>>> +             DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
>>> +             DISP_REG_CONFIG_OUT_SEL,
>>> +             BLS_RDMA1_DSI_DPI_MASK, BLS_TO_DPI_RDMA1_TO_DSI
>>> +     }, {
>>> +             DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
>>> +             DISP_REG_CONFIG_DSI_SEL,
>>> +             DSI_SEL_IN_MASK, DSI_SEL_IN_RDMA
>>> +     }, {
>>> +             DDP_COMPONENT_BLS, DDP_COMPONENT_DPI0,
>>> +             DISP_REG_CONFIG_DPI_SEL,
>>> +             DPI_SEL_IN_MASK, DPI_SEL_IN_BLS
>>
>> I wonder if we could make the data structure easier to read by grouping actions
>> for the same component tuple together.
> 
> Maybe if-else is the simple way to group then. Two layer array could
> achieve this but it's more complicated.
> 
>>
>>> +     }, {
>>> +             DDP_COMPONENT_GAMMA, DDP_COMPONENT_RDMA1,
>>> +             DISP_REG_CONFIG_DISP_GAMMA_MOUT_EN,
>>> +             GAMMA_MOUT_EN_RDMA1, GAMMA_MOUT_EN_RDMA1
>>> +     }, {
>>> +             DDP_COMPONENT_OD0, DDP_COMPONENT_RDMA0,
>>> +             DISP_REG_CONFIG_DISP_OD_MOUT_EN,
>>> +             OD_MOUT_EN_RDMA0, OD_MOUT_EN_RDMA0
>>> +     }, {
>>> +             DDP_COMPONENT_OD1, DDP_COMPONENT_RDMA1,
>>> +             DISP_REG_CONFIG_DISP_OD_MOUT_EN,
>>> +             OD1_MOUT_EN_RDMA1, OD1_MOUT_EN_RDMA1
>>> +     }, {
>>> +             DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0,
>>> +             DISP_REG_CONFIG_DISP_OVL0_MOUT_EN,
>>> +             OVL0_MOUT_EN_COLOR0, OVL0_MOUT_EN_COLOR0
>>> +     }, {
>>> +             DDP_COMPONENT_OVL0, DDP_COMPONENT_COLOR0,
>>> +             DISP_REG_CONFIG_DISP_COLOR0_SEL_IN,
>>> +             COLOR0_SEL_IN_OVL0, COLOR0_SEL_IN_OVL0
>>> +     }, {
>>> +             DDP_COMPONENT_OVL0, DDP_COMPONENT_RDMA0,
>>> +             DISP_REG_CONFIG_DISP_OVL_MOUT_EN,
>>> +             OVL_MOUT_EN_RDMA, OVL_MOUT_EN_RDMA
>>> +     }, {
>>> +             DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1,
>>> +             DISP_REG_CONFIG_DISP_OVL1_MOUT_EN,
>>> +             OVL1_MOUT_EN_COLOR1, OVL1_MOUT_EN_COLOR1
>>> +     }, {
>>> +             DDP_COMPONENT_OVL1, DDP_COMPONENT_COLOR1,
>>> +             DISP_REG_CONFIG_DISP_COLOR1_SEL_IN,
>>> +             COLOR1_SEL_IN_OVL1, COLOR1_SEL_IN_OVL1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DPI0,
>>> +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
>>> +             RDMA0_SOUT_MASK, RDMA0_SOUT_DPI0
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DPI1,
>>> +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
>>> +             RDMA0_SOUT_MASK, RDMA0_SOUT_DPI1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI1,
>>> +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
>>> +             RDMA0_SOUT_MASK, RDMA0_SOUT_DSI1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI2,
>>> +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
>>> +             RDMA0_SOUT_MASK, RDMA0_SOUT_DSI2
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA0, DDP_COMPONENT_DSI3,
>>> +             DISP_REG_CONFIG_DISP_RDMA0_SOUT_EN,
>>> +             RDMA0_SOUT_MASK, RDMA0_SOUT_DSI3
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,
>>> +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
>>> +             RDMA1_SOUT_MASK, RDMA1_SOUT_DPI0
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI0,
>>> +             DISP_REG_CONFIG_DPI_SEL_IN,
>>> +             DPI0_SEL_IN_MASK, DPI0_SEL_IN_RDMA1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI1,
>>> +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
>>> +             RDMA1_SOUT_MASK, RDMA1_SOUT_DPI1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DPI1,
>>> +             DISP_REG_CONFIG_DPI_SEL_IN,
>>> +             DPI1_SEL_IN_MASK, DPI1_SEL_IN_RDMA1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI0,
>>> +             DISP_REG_CONFIG_DSIE_SEL_IN,
>>> +             DSI0_SEL_IN_MASK, DSI0_SEL_IN_RDMA1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI1,
>>> +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
>>> +             RDMA1_SOUT_MASK, RDMA1_SOUT_DSI1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI1,
>>> +             DISP_REG_CONFIG_DSIO_SEL_IN,
>>> +             DSI1_SEL_IN_MASK, DSI1_SEL_IN_RDMA1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI2,
>>> +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
>>> +             RDMA1_SOUT_MASK, RDMA1_SOUT_DSI2
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI2,
>>> +             DISP_REG_CONFIG_DSIE_SEL_IN,
>>> +             DSI2_SEL_IN_MASK, DSI2_SEL_IN_RDMA1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI3,
>>> +             DISP_REG_CONFIG_DISP_RDMA1_SOUT_EN,
>>> +             RDMA1_SOUT_MASK, RDMA1_SOUT_DSI3
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA1, DDP_COMPONENT_DSI3,
>>> +             DISP_REG_CONFIG_DSIO_SEL_IN,
>>> +             DSI3_SEL_IN_MASK, DSI3_SEL_IN_RDMA1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI0,
>>> +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
>>> +             RDMA2_SOUT_MASK, RDMA2_SOUT_DPI0
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI0,
>>> +             DISP_REG_CONFIG_DPI_SEL_IN,
>>> +             DPI0_SEL_IN_MASK, DPI0_SEL_IN_RDMA2
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI1,
>>> +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
>>> +             RDMA2_SOUT_MASK, RDMA2_SOUT_DPI1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DPI1,
>>> +             DISP_REG_CONFIG_DPI_SEL_IN,
>>> +             DPI1_SEL_IN_MASK, DPI1_SEL_IN_RDMA2
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI0,
>>> +             DISP_REG_CONFIG_DSIE_SEL_IN,
>>> +             DSI0_SEL_IN_MASK, DSI0_SEL_IN_RDMA2
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI1,
>>> +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
>>> +             RDMA2_SOUT_MASK, RDMA2_SOUT_DSI1
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI1,
>>> +             DISP_REG_CONFIG_DSIO_SEL_IN,
>>> +             DSI1_SEL_IN_MASK, DSI1_SEL_IN_RDMA2
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI2,
>>> +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
>>> +             RDMA2_SOUT_MASK, RDMA2_SOUT_DSI2
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI2,
>>> +             DISP_REG_CONFIG_DSIE_SEL_IN,
>>> +             DSI2_SEL_IN_MASK, DSI2_SEL_IN_RDMA2
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI3,
>>> +             DISP_REG_CONFIG_DISP_RDMA2_SOUT,
>>> +             RDMA2_SOUT_MASK, RDMA2_SOUT_DSI3
>>> +     }, {
>>> +             DDP_COMPONENT_RDMA2, DDP_COMPONENT_DSI3,
>>> +             DISP_REG_CONFIG_DSIO_SEL_IN,
>>> +             DSI3_SEL_IN_MASK, DSI3_SEL_IN_RDMA2
>>>       }
>>> +};
>>>
>>> -     return value;
>>> -}
>>> -
>>> -static void mtk_mmsys_ddp_sout_sel(void __iomem *config_regs,
>>> -                                enum mtk_ddp_comp_id cur,
>>> -                                enum mtk_ddp_comp_id next)
>>> -{
>>> -     if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
>>> -             writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
>>> -                            config_regs + DISP_REG_CONFIG_OUT_SEL);
>>> -     } 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);
>>> -     }
>>> -}
>>> +static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>>> +     .clk_driver = "clk-mt8173-mm",
>>> +     .routes = mt8173_mmsys_routing_table,
>>> +     .num_routes = ARRAY_SIZE(mt8173_mmsys_routing_table),
>>> +};
>>>
>>>   void mtk_mmsys_ddp_connect(struct device *dev,
>>>                          enum mtk_ddp_comp_id cur,
>>>                          enum mtk_ddp_comp_id next)
>>>   {
>>>       struct mtk_mmsys *mmsys = dev_get_drvdata(dev);
>>> -     unsigned int addr, value, reg;
>>> -
>>> -     value = mtk_mmsys_ddp_mout_en(cur, next, &addr);
>>> -     if (value) {
>>> -             reg = readl_relaxed(mmsys->regs + addr) | value;
>>> -             writel_relaxed(reg, mmsys->regs + addr);
>>> -     }
>>> -
>>> -     mtk_mmsys_ddp_sout_sel(mmsys->regs, cur, next);
>>> -
>>> -     value = mtk_mmsys_ddp_sel_in(cur, next, &addr);
>>> -     if (value) {
>>> -             reg = readl_relaxed(mmsys->regs + addr) | value;
>>> -             writel_relaxed(reg, mmsys->regs + addr);
>>> -     }
>>
>> What I don't like of this new approach is the fact, that we loose information
>> about the different grouping we had up to now, mout_en, sout_sel and sel_in.
> 
> Agree. Maybe we should keep if-else model in mt8173, mt2701, mt2712.
> And ask mt8183 [1] , mt8192 [2] to use if-else model.
> 
> [1] https://patchwork.kernel.org/patch/11706231/
> [2] https://patchwork.kernel.org/patch/11725595/
> 

Ok, I'll give a second round of these patches trying to address your comments.
Matthias, patch 1 and 2 are kind of independent/unrelated patches and can be
applied. I just added here to avoid conflicts. So I am wondering if you are fine
with them, if you can pick these two.

Thanks,
  Enric


> Regards,
> Chun-Kuang.
> 
>>
>> Regards,
>> Matthias

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH 0/4] soc: mediatek: Prepare MMSYS for DDP routing using tables

[Enric Balletbo i Serra]

Hi Matthias,

On 6/10/20 21:33, Enric Balletbo i Serra wrote:
> Dear all,
> 
> The following series are intended to prepare the mtk-mmsys driver to
> allow different DDP (Data Display Path) routing tables per SoC. Note
> that the series has been tested only on MT8173 platform and could break
> the display on MT2701 and MT2712 based devices. I kindly ask for someone
> having these devices to provide a tested routing table (unfortunately I
> didn't have enough documentation to figure out this myself).
> 
> For the other devices (MT8183, MT6779 and MT6797) DRM support is not in
> mainline yet so nothing will break.
> 
> Thanks,
>   Enric
> 
> 
> CK Hu (2):
>   soc: mediatek: mmsys: Create struct mtk_mmsys to store context data
>   soc: mediatek: mmsys: Use an array for setting the routing registers
> 
> Enric Balletbo i Serra (1):
>   soc: mediatek: mmsys: Use devm_platform_ioremap_resource()
> 
> Yongqiang Niu (1):
>   soc / drm: mediatek: Move DDP component defines into mtk-mmsys.h
> 
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  34 +-
>  drivers/soc/mediatek/mtk-mmsys.c            | 429 +++++++++++---------
>  include/linux/soc/mediatek/mtk-mmsys.h      |  33 ++
>  3 files changed, 263 insertions(+), 233 deletions(-)
> 

Although the patches 3 and 4 are controversial, and I'll work on it, I am
wondering if 1 and 2 are ready to be picked, as they are independent, so I can
send next version without these two patches.

Thanks,
  Enric



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek