[RFC v2 2/5] drm/mediatke: add support for Mediatek SoC MT2701

[YT Shen]

Hi Emil,
On Fri, 2016-05-27 at 10:35 +0100, Emil Velikov wrote:
> Hi YT Shen,
> 
> There's a typo in the commit summary - s/mediatke/mediatek/.
Ooops, even I type this word everyday, I still made a mistake...
> 
> On 20 May 2016 at 16:05,   wrote:
> > From: YT Shen 
> >
> > This patch add support for the Mediatek MT2701 DISP subsystem.
> > There is only one OVL engine in MT2701.
> >
> As is you introduce a broken driver only to fix it up with patches 3/5
> and 4/5. You really want to have this patch as 4/5, with the MT2701
> hunks from 3/5 merged in here.
OK, I will reorder the patch series in the next version.
2/5: add *driver_data for hardware specific settings
3/5: add shadow register support
4/5: add support for Mediatek SoC MT2701

> 
> Regards,
> Emil

[RFC v2 2/5] drm/mediatke: add support for Mediatek SoC MT2701

[YT Shen]

Hi CK,


On Mon, 2016-05-23 at 17:09 +0800, CK Hu wrote:
> Hi, YT:
> 
> Some comments below.
> 
> On Fri, 2016-05-20 at 23:05 +0800, yt.shen at mediatek.com wrote:
> > From: YT Shen 
> > 
> > This patch add support for the Mediatek MT2701 DISP subsystem.
> > There is only one OVL engine in MT2701.
> > 
> > Signed-off-by: YT Shen 
> >  
> > +static void mtk_ddp_mux_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);
> > +   }
> > +}
> > +
> 
> The function name 'mux' looks strange. The register written here
> controls the single output selection. I prefer to rename it as
> mtk_ddp_sout_sel().
OK, I will rename this function in the next version.
> 
> >  
> > -static const enum mtk_ddp_comp_id mtk_ddp_main[] = {
> > +static const enum mtk_ddp_comp_id mtk_ddp_main_2701[] = {
> > +   DDP_COMPONENT_OVL0,
> > +   DDP_COMPONENT_RDMA0,
> > +   DDP_COMPONENT_COLOR0,
> > +   DDP_COMPONENT_BLS,
> > +   DDP_COMPONENT_DSI0,
> > +};
> > +
> > +static const enum mtk_ddp_comp_id mtk_ddp_ext_2701[] = {
> > +   DDP_COMPONENT_OVL0,
> > +   DDP_COMPONENT_DSI0,
> > +};
> > +
> 
> These two pipelines has the same component such as OVL0 and DSI0. I
> think user program could not enable both crtc at the same time. Maybe
> MT2701 has only one crtc, so you should modify initial flow to create
> only one crtc for main display. Or it's typo for external display pipe,
> please correct it.
MT2701 hardware can support two output concurrently, but we haven't
implement DPI path yet.  We will change it like this:
static const enum mtk_ddp_comp_id mtk_ddp_ext_2701[] = {
DDP_COMPONENT_RDMA1,
DDP_COMPONENT_DPI0,
};

Thanks.
yt.shen
> 
> 
> Regards,
> CK
> 

[RFC v2 2/5] drm/mediatke: add support for Mediatek SoC MT2701

[Emil Velikov]

Hi YT Shen,

There's a typo in the commit summary - s/mediatke/mediatek/.

On 20 May 2016 at 16:05,   wrote:
> From: YT Shen 
>
> This patch add support for the Mediatek MT2701 DISP subsystem.
> There is only one OVL engine in MT2701.
>
As is you introduce a broken driver only to fix it up with patches 3/5
and 4/5. You really want to have this patch as 4/5, with the MT2701
hunks from 3/5 merged in here.

Regards,
Emil

[RFC v2 2/5] drm/mediatke: add support for Mediatek SoC MT2701

[CK Hu]

Hi, YT:

Some comments below.

On Fri, 2016-05-20 at 23:05 +0800, yt.shen at mediatek.com wrote:
> From: YT Shen 
> 
> This patch add support for the Mediatek MT2701 DISP subsystem.
> There is only one OVL engine in MT2701.
> 
> Signed-off-by: YT Shen 
>  
> +static void mtk_ddp_mux_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);
> + }
> +}
> +

The function name 'mux' looks strange. The register written here
controls the single output selection. I prefer to rename it as
mtk_ddp_sout_sel().

>  
> -static const enum mtk_ddp_comp_id mtk_ddp_main[] = {
> +static const enum mtk_ddp_comp_id mtk_ddp_main_2701[] = {
> + DDP_COMPONENT_OVL0,
> + DDP_COMPONENT_RDMA0,
> + DDP_COMPONENT_COLOR0,
> + DDP_COMPONENT_BLS,
> + DDP_COMPONENT_DSI0,
> +};
> +
> +static const enum mtk_ddp_comp_id mtk_ddp_ext_2701[] = {
> + DDP_COMPONENT_OVL0,
> + DDP_COMPONENT_DSI0,
> +};
> +

These two pipelines has the same component such as OVL0 and DSI0. I
think user program could not enable both crtc at the same time. Maybe
MT2701 has only one crtc, so you should modify initial flow to create
only one crtc for main display. Or it's typo for external display pipe,
please correct it.


Regards,
CK

[RFC v2 2/5] drm/mediatke: add support for Mediatek SoC MT2701

[yt.s...@mediatek.com]

From: YT Shen 

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

Signed-off-by: YT Shen 
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c  |   63 +---
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c |2 +
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |2 +
 drivers/gpu/drm/mediatek/mtk_drm_drv.c  |   70 ++-
 drivers/gpu/drm/mediatek/mtk_drm_drv.h  |8 +++
 5 files changed, 126 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c 
b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index d6aafd4..529569d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -31,6 +31,10 @@
 #define DISP_REG_CONFIG_DISP_RDMA1_MOUT_EN 0x0c8
 #define DISP_REG_CONFIG_MMSYS_CG_CON0  0x100

+#define DISP_REG_CONFIG_DISP_OVL_MOUT_EN   0x030
+#define DISP_REG_CONFIG_OUT_SEL0x04c
+#define DISP_REG_CONFIG_DSI_SEL0x050
+
 #define DISP_REG_MUTEX_EN(n)   (0x20 + 0x20 * (n))
 #define DISP_REG_MUTEX_RST(n)  (0x28 + 0x20 * (n))
 #define DISP_REG_MUTEX_MOD(n)  (0x2c + 0x20 * (n))
@@ -52,6 +56,13 @@
 #define MUTEX_MOD_DISP_PWM1_MT8173 BIT(24)
 #define MUTEX_MOD_DISP_OD_MT8173   BIT(25)

+#define MUTEX_MOD_DISP_OVL_MT2701  BIT(3)
+#define MUTEX_MOD_DISP_WDMA_MT2701 BIT(6)
+#define MUTEX_MOD_DISP_COLOR_MT2701BIT(7)
+#define MUTEX_MOD_DISP_BLS_MT2701  BIT(9)
+#define MUTEX_MOD_DISP_RDMA0_MT2701BIT(10)
+#define MUTEX_MOD_DISP_RDMA1_MT2701BIT(12)
+
 #define MUTEX_SOF_SINGLE_MODE  0
 #define MUTEX_SOF_DSI0 1
 #define MUTEX_SOF_DSI1 2
@@ -67,6 +78,10 @@
 #define DPI0_SEL_IN_RDMA1  0x1
 #define COLOR1_SEL_IN_OVL1 0x1

+#define OVL_MOUT_EN_RDMA   0x1
+#define BLS_TO_DSI_RDMA1_TO_DPI1   0x8
+#define DSI_SEL_IN_BLS 0x0
+
 struct mtk_disp_mutex {
int id;
bool claimed;
@@ -77,6 +92,16 @@ struct mtk_ddp {
struct clk  *clk;
void __iomem*regs;
struct mtk_disp_mutex   mutex[10];
+   const unsigned int  *mutex_mod;
+};
+
+static const unsigned int mutex_mod_mt2701[DDP_COMPONENT_ID_MAX] = {
+   [DDP_COMPONENT_BLS] = MUTEX_MOD_DISP_BLS_MT2701,
+   [DDP_COMPONENT_COLOR0] = MUTEX_MOD_DISP_COLOR_MT2701,
+   [DDP_COMPONENT_OVL0] = MUTEX_MOD_DISP_OVL_MT2701,
+   [DDP_COMPONENT_RDMA0] = MUTEX_MOD_DISP_RDMA0_MT2701,
+   [DDP_COMPONENT_RDMA1] = MUTEX_MOD_DISP_RDMA1_MT2701,
+   [DDP_COMPONENT_WDMA0] = MUTEX_MOD_DISP_WDMA_MT2701,
 };

 static const unsigned int mutex_mod_mt8173[DDP_COMPONENT_ID_MAX] = {
@@ -106,6 +131,9 @@ static unsigned int mtk_ddp_mout_en(enum mtk_ddp_comp_id 
cur,
if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_COLOR0) {
*addr = DISP_REG_CONFIG_DISP_OVL0_MOUT_EN;
value = OVL0_MOUT_EN_COLOR0;
+   } else if (cur == DDP_COMPONENT_OVL0 && next == DDP_COMPONENT_RDMA0) {
+   *addr = DISP_REG_CONFIG_DISP_OVL_MOUT_EN;
+   value = OVL_MOUT_EN_RDMA;
} else if (cur == DDP_COMPONENT_OD && next == DDP_COMPONENT_RDMA0) {
*addr = DISP_REG_CONFIG_DISP_OD_MOUT_EN;
value = OD_MOUT_EN_RDMA0;
@@ -143,6 +171,9 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id cur,
} else if (cur == DDP_COMPONENT_OVL1 && next == DDP_COMPONENT_COLOR1) {
*addr = DISP_REG_CONFIG_DISP_COLOR1_SEL_IN;
value = COLOR1_SEL_IN_OVL1;
+   } else if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
+   *addr = DISP_REG_CONFIG_DSI_SEL;
+   value = DSI_SEL_IN_BLS;
} else {
value = 0;
}
@@ -150,6 +181,15 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id 
cur,
return value;
 }

+static void mtk_ddp_mux_sel(void __iomem *config_regs,
+   enum mtk_ddp_comp_id cur, enum mtk_ddp_comp_id next)
+{
+   if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0) {
+   writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
+  config_regs + DISP_REG_CONFIG_OUT_SEL);
+   }
+}
+
 void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
  enum mtk_ddp_comp_id cur,
  enum mtk_ddp_comp_id next)
@@ -162,6 +202,8 @@ void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
writel_relaxed(reg, config_regs + addr);
}

+   mtk_ddp_mux_sel(config_regs, cur, next);
+
value = mtk_ddp_sel_in(cur, next, );
if (value) {
reg = readl_relaxed(config_regs + addr) | value;
@@ -247,7 +289,7 @@ void mtk_disp_mutex_add_comp(struct mtk_disp_mutex