[v2 0/5] support dsi for mt8183

[v2 0/5] support dsi for mt8183

[Jitao Shi]

Changes since v1:
 - separate frame size and reg commit control independent patches.
 - fix some return values in probe
 - remove DSI_CMDW0 in "CMDQ reg address of mt8173 is different with mt2701"

Jitao Shi (5):
  drm/mediatek: move mipi_dsi_host_register to probe
  drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
  drm/mediatek: add dsi reg commit control
  drm/mediatek: add frame size control
  drm/mediatek: add mt8183 dsi driver support

 drivers/gpu/drm/mediatek/mtk_dsi.c | 112 +++++++++++++++++++++--------
 1 file changed, 83 insertions(+), 29 deletions(-)

-- 
2.21.0

[v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe

[Jitao Shi]

DSI panel driver need attach function which is inculde in
mipi_dsi_host_ops.

If mipi_dsi_host_register is not in probe, dsi panel will
probe fail or more delay.

So move the mipi_dsi_host_register to probe from bind.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 50 ++++++++++++++++++------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index b00eb2d2e086..6c4ac37f983d 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1045,12 +1045,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 	}
 
-	ret = mipi_dsi_host_register(&dsi->host);
-	if (ret < 0) {
-		dev_err(dev, "failed to register DSI host: %d\n", ret);
-		goto err_ddp_comp_unregister;
-	}
-
 	ret = mtk_dsi_create_conn_enc(drm, dsi);
 	if (ret) {
 		DRM_ERROR("Encoder create failed with %d\n", ret);
@@ -1060,8 +1054,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
 	return 0;
 
 err_unregister:
-	mipi_dsi_host_unregister(&dsi->host);
-err_ddp_comp_unregister:
 	mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
 	return ret;
 }
@@ -1097,31 +1089,37 @@ static int mtk_dsi_probe(struct platform_device *pdev)
 
 	dsi->host.ops = &mtk_dsi_ops;
 	dsi->host.dev = dev;
+	dsi->dev = dev;
+	ret = mipi_dsi_host_register(&dsi->host);
+	if (ret < 0) {
+		dev_err(dev, "failed to register DSI host: %d\n", ret);
+		return ret;
+	}
 
 	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
 					  &dsi->panel, &dsi->bridge);
 	if (ret)
-		return ret;
+		goto err_unregister_host;
 
 	dsi->engine_clk = devm_clk_get(dev, "engine");
 	if (IS_ERR(dsi->engine_clk)) {
 		ret = PTR_ERR(dsi->engine_clk);
 		dev_err(dev, "Failed to get engine clock: %d\n", ret);
-		return ret;
+		goto err_unregister_host;
 	}
 
 	dsi->digital_clk = devm_clk_get(dev, "digital");
 	if (IS_ERR(dsi->digital_clk)) {
 		ret = PTR_ERR(dsi->digital_clk);
 		dev_err(dev, "Failed to get digital clock: %d\n", ret);
-		return ret;
+		goto err_unregister_host;
 	}
 
 	dsi->hs_clk = devm_clk_get(dev, "hs");
 	if (IS_ERR(dsi->hs_clk)) {
 		ret = PTR_ERR(dsi->hs_clk);
 		dev_err(dev, "Failed to get hs clock: %d\n", ret);
-		return ret;
+		goto err_unregister_host;
 	}
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1129,33 +1127,35 @@ static int mtk_dsi_probe(struct platform_device *pdev)
 	if (IS_ERR(dsi->regs)) {
 		ret = PTR_ERR(dsi->regs);
 		dev_err(dev, "Failed to ioremap memory: %d\n", ret);
-		return ret;
+		goto err_unregister_host;
 	}
 
 	dsi->phy = devm_phy_get(dev, "dphy");
 	if (IS_ERR(dsi->phy)) {
 		ret = PTR_ERR(dsi->phy);
 		dev_err(dev, "Failed to get MIPI-DPHY: %d\n", ret);
-		return ret;
+		goto err_unregister_host;
 	}
 
 	comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DSI);
 	if (comp_id < 0) {
 		dev_err(dev, "Failed to identify by alias: %d\n", comp_id);
-		return comp_id;
+		ret = comp_id;
+		goto err_unregister_host;
 	}
 
 	ret = mtk_ddp_comp_init(dev, dev->of_node, &dsi->ddp_comp, comp_id,
 				&mtk_dsi_funcs);
 	if (ret) {
 		dev_err(dev, "Failed to initialize component: %d\n", ret);
-		return ret;
+		goto err_unregister_host;
 	}
 
 	irq_num = platform_get_irq(pdev, 0);
 	if (irq_num < 0) {
-		dev_err(&pdev->dev, "failed to request dsi irq resource\n");
-		return -EPROBE_DEFER;
+		dev_err(&pdev->dev, "failed to get dsi irq_num: %d\n", irq_num);
+		ret = irq_num;
+		goto err_unregister_host;
 	}
 
 	irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
@@ -1163,14 +1163,24 @@ static int mtk_dsi_probe(struct platform_device *pdev)
 			       IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to request mediatek dsi irq\n");
-		return -EPROBE_DEFER;
+		goto err_unregister_host;
 	}
 
 	init_waitqueue_head(&dsi->irq_wait_queue);
 
 	platform_set_drvdata(pdev, dsi);
 
-	return component_add(&pdev->dev, &mtk_dsi_component_ops);
+	ret = component_add(&pdev->dev, &mtk_dsi_component_ops);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add component: %d\n", ret);
+		goto err_unregister_host;
+	}
+
+	return 0;
+
+err_unregister_host:
+	mipi_dsi_host_unregister(&dsi->host);
+	return ret;
 }
 
 static int mtk_dsi_remove(struct platform_device *pdev)
-- 
2.21.0

Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe

[CK Hu]

Hi, Jitao:

On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> DSI panel driver need attach function which is inculde in
> mipi_dsi_host_ops.
> 
> If mipi_dsi_host_register is not in probe, dsi panel will
> probe fail or more delay.

I think this patch just prevent delay, not to prevent dsi panel probe
fail. In [1], you mention mipi_dsi_attach() is called in
panel_simple_dsi_probe(), but panel_simple_dsi_probe() is trigger by
mipi_dsi_host_register(), so the probe would success.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-simple.c?h=v5.0-rc6#n2987


> 
> So move the mipi_dsi_host_register to probe from bind.
> 
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 50 ++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index b00eb2d2e086..6c4ac37f983d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1045,12 +1045,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
>  		return ret;
>  	}
>  
> -	ret = mipi_dsi_host_register(&dsi->host);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to register DSI host: %d\n", ret);
> -		goto err_ddp_comp_unregister;
> -	}
> -
>  	ret = mtk_dsi_create_conn_enc(drm, dsi);
>  	if (ret) {
>  		DRM_ERROR("Encoder create failed with %d\n", ret);
> @@ -1060,8 +1054,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
>  	return 0;
>  
>  err_unregister:
> -	mipi_dsi_host_unregister(&dsi->host);
> -err_ddp_comp_unregister:
>  	mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
>  	return ret;
>  }
> @@ -1097,31 +1089,37 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>  
>  	dsi->host.ops = &mtk_dsi_ops;
>  	dsi->host.dev = dev;
> +	dsi->dev = dev;

Why do this?

Regards,
CK

> +	ret = mipi_dsi_host_register(&dsi->host);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to register DSI host: %d\n", ret);
> +		return ret;
> +	}
>  
>  	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
>  					  &dsi->panel, &dsi->bridge);
>  	if (ret)
> -		return ret;
> +		goto err_unregister_host;
>  
>  	dsi->engine_clk = devm_clk_get(dev, "engine");
>  	if (IS_ERR(dsi->engine_clk)) {
>  		ret = PTR_ERR(dsi->engine_clk);
>  		dev_err(dev, "Failed to get engine clock: %d\n", ret);
> -		return ret;
> +		goto err_unregister_host;
>  	}
>  
>  	dsi->digital_clk = devm_clk_get(dev, "digital");
>  	if (IS_ERR(dsi->digital_clk)) {
>  		ret = PTR_ERR(dsi->digital_clk);
>  		dev_err(dev, "Failed to get digital clock: %d\n", ret);
> -		return ret;
> +		goto err_unregister_host;
>  	}
>  
>  	dsi->hs_clk = devm_clk_get(dev, "hs");
>  	if (IS_ERR(dsi->hs_clk)) {
>  		ret = PTR_ERR(dsi->hs_clk);
>  		dev_err(dev, "Failed to get hs clock: %d\n", ret);
> -		return ret;
> +		goto err_unregister_host;
>  	}
>  
>  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);

Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe

[Jitao Shi]

On Tue, 2019-05-07 at 17:52 +0800, CK Hu wrote:
> Hi, Jitao:
> 
> On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> > DSI panel driver need attach function which is inculde in
> > mipi_dsi_host_ops.
> > 
> > If mipi_dsi_host_register is not in probe, dsi panel will
> > probe fail or more delay.
> 
> I think this patch just prevent delay, not to prevent dsi panel probe
> fail. In [1], you mention mipi_dsi_attach() is called in
> panel_simple_dsi_probe(), but panel_simple_dsi_probe() is trigger by
> mipi_dsi_host_register(), so the probe would success.
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-simple.c?h=v5.0-rc6#n2987
> 
> 

Yes, this just prevent delay.

> > 
> > So move the mipi_dsi_host_register to probe from bind.
> > 
> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c | 50 ++++++++++++++++++------------
> >  1 file changed, 30 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index b00eb2d2e086..6c4ac37f983d 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -1045,12 +1045,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> >  		return ret;
> >  	}
> >  
> > -	ret = mipi_dsi_host_register(&dsi->host);
> > -	if (ret < 0) {
> > -		dev_err(dev, "failed to register DSI host: %d\n", ret);
> > -		goto err_ddp_comp_unregister;
> > -	}
> > -
> >  	ret = mtk_dsi_create_conn_enc(drm, dsi);
> >  	if (ret) {
> >  		DRM_ERROR("Encoder create failed with %d\n", ret);
> > @@ -1060,8 +1054,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> >  	return 0;
> >  
> >  err_unregister:
> > -	mipi_dsi_host_unregister(&dsi->host);
> > -err_ddp_comp_unregister:
> >  	mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
> >  	return ret;
> >  }
> > @@ -1097,31 +1089,37 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> >  
> >  	dsi->host.ops = &mtk_dsi_ops;
> >  	dsi->host.dev = dev;
> > +	dsi->dev = dev;
> 
> Why do this?
> 
> Regards,
> CK
> 

There are some error message require this poweron().

> > +	ret = mipi_dsi_host_register(&dsi->host);
> > +	if (ret < 0) {
> > +		dev_err(dev, "failed to register DSI host: %d\n", ret);
> > +		return ret;
> > +	}
> >  
> >  	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> >  					  &dsi->panel, &dsi->bridge);
> >  	if (ret)
> > -		return ret;
> > +		goto err_unregister_host;
> >  
> >  	dsi->engine_clk = devm_clk_get(dev, "engine");
> >  	if (IS_ERR(dsi->engine_clk)) {
> >  		ret = PTR_ERR(dsi->engine_clk);
> >  		dev_err(dev, "Failed to get engine clock: %d\n", ret);
> > -		return ret;
> > +		goto err_unregister_host;
> >  	}
> >  
> >  	dsi->digital_clk = devm_clk_get(dev, "digital");
> >  	if (IS_ERR(dsi->digital_clk)) {
> >  		ret = PTR_ERR(dsi->digital_clk);
> >  		dev_err(dev, "Failed to get digital clock: %d\n", ret);
> > -		return ret;
> > +		goto err_unregister_host;
> >  	}
> >  
> >  	dsi->hs_clk = devm_clk_get(dev, "hs");
> >  	if (IS_ERR(dsi->hs_clk)) {
> >  		ret = PTR_ERR(dsi->hs_clk);
> >  		dev_err(dev, "Failed to get hs clock: %d\n", ret);
> > -		return ret;
> > +		goto err_unregister_host;
> >  	}
> >  
> >  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 
> 
> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe

[CK Hu]

On Sun, 2019-05-19 at 17:36 +0800, Jitao Shi wrote:
> On Tue, 2019-05-07 at 17:52 +0800, CK Hu wrote:
> > Hi, Jitao:
> > 
> > On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> > > DSI panel driver need attach function which is inculde in
> > > mipi_dsi_host_ops.
> > > 
> > > If mipi_dsi_host_register is not in probe, dsi panel will
> > > probe fail or more delay.
> > 
> > I think this patch just prevent delay, not to prevent dsi panel probe
> > fail. In [1], you mention mipi_dsi_attach() is called in
> > panel_simple_dsi_probe(), but panel_simple_dsi_probe() is trigger by
> > mipi_dsi_host_register(), so the probe would success.
> > 
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-simple.c?h=v5.0-rc6#n2987
> > 
> > 
> 
> Yes, this just prevent delay.
> 
> > > 
> > > So move the mipi_dsi_host_register to probe from bind.
> > > 
> > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> > > ---
> > >  drivers/gpu/drm/mediatek/mtk_dsi.c | 50 ++++++++++++++++++------------
> > >  1 file changed, 30 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > index b00eb2d2e086..6c4ac37f983d 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > @@ -1045,12 +1045,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> > >  		return ret;
> > >  	}
> > >  
> > > -	ret = mipi_dsi_host_register(&dsi->host);
> > > -	if (ret < 0) {
> > > -		dev_err(dev, "failed to register DSI host: %d\n", ret);
> > > -		goto err_ddp_comp_unregister;
> > > -	}
> > > -
> > >  	ret = mtk_dsi_create_conn_enc(drm, dsi);
> > >  	if (ret) {
> > >  		DRM_ERROR("Encoder create failed with %d\n", ret);
> > > @@ -1060,8 +1054,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> > >  	return 0;
> > >  
> > >  err_unregister:
> > > -	mipi_dsi_host_unregister(&dsi->host);
> > > -err_ddp_comp_unregister:
> > >  	mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
> > >  	return ret;
> > >  }
> > > @@ -1097,31 +1089,37 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> > >  
> > >  	dsi->host.ops = &mtk_dsi_ops;
> > >  	dsi->host.dev = dev;
> > > +	dsi->dev = dev;
> > 
> > Why do this?
> > 
> > Regards,
> > CK
> > 
> 
> There are some error message require this poweron().

So this should not be in this patch. This patch is related to the timing
of mipi_dsi_host_register().

Regards,
CK

> 
> > > +	ret = mipi_dsi_host_register(&dsi->host);
> > > +	if (ret < 0) {
> > > +		dev_err(dev, "failed to register DSI host: %d\n", ret);
> > > +		return ret;
> > > +	}
> > >  
> > >  	ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> > >  					  &dsi->panel, &dsi->bridge);
> > >  	if (ret)
> > > -		return ret;
> > > +		goto err_unregister_host;
> > >  
> > >  	dsi->engine_clk = devm_clk_get(dev, "engine");
> > >  	if (IS_ERR(dsi->engine_clk)) {
> > >  		ret = PTR_ERR(dsi->engine_clk);
> > >  		dev_err(dev, "Failed to get engine clock: %d\n", ret);
> > > -		return ret;
> > > +		goto err_unregister_host;
> > >  	}
> > >  
> > >  	dsi->digital_clk = devm_clk_get(dev, "digital");
> > >  	if (IS_ERR(dsi->digital_clk)) {
> > >  		ret = PTR_ERR(dsi->digital_clk);
> > >  		dev_err(dev, "Failed to get digital clock: %d\n", ret);
> > > -		return ret;
> > > +		goto err_unregister_host;
> > >  	}
> > >  
> > >  	dsi->hs_clk = devm_clk_get(dev, "hs");
> > >  	if (IS_ERR(dsi->hs_clk)) {
> > >  		ret = PTR_ERR(dsi->hs_clk);
> > >  		dev_err(dev, "Failed to get hs clock: %d\n", ret);
> > > -		return ret;
> > > +		goto err_unregister_host;
> > >  	}
> > >  
> > >  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > 
> > 
> > 
> 
> 

[v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701

[Jitao Shi]

Config the different CMDQ reg address in driver data.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 6c4ac37f983d..573e6bec6d36 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -131,7 +131,6 @@
 #define VM_CMD_EN			BIT(0)
 #define TS_VFP_EN			BIT(5)
 
-#define DSI_CMDQ0		0x180
 #define CONFIG				(0xff << 0)
 #define SHORT_PACKET			0
 #define LONG_PACKET			2
@@ -156,6 +155,10 @@
 
 struct phy;
 
+struct mtk_dsi_driver_data {
+	const u32 reg_cmdq_off;
+};
+
 struct mtk_dsi {
 	struct mtk_ddp_comp ddp_comp;
 	struct device *dev;
@@ -182,6 +185,7 @@ struct mtk_dsi {
 	bool enabled;
 	u32 irq_data;
 	wait_queue_head_t irq_wait_queue;
+	struct mtk_dsi_driver_data *driver_data;
 };
 
 static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
@@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
 	const char *tx_buf = msg->tx_buf;
 	u8 config, cmdq_size, cmdq_off, type = msg->type;
 	u32 reg_val, cmdq_mask, i;
+	u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
 
 	if (MTK_DSI_HOST_IS_READ(type))
 		config = BTA;
@@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
 	}
 
 	for (i = 0; i < msg->tx_len; i++)
-		writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
+		mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
+			     (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
+			     tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));
 
-	mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
+	mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
 	mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
 }
 
@@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
 	.unbind = mtk_dsi_unbind,
 };
 
+static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
+	.reg_cmdq_off = 0x200,
+};
+
+static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
+	.reg_cmdq_off = 0x180,
+};
+
+static const struct of_device_id mtk_dsi_of_match[] = {
+	{ .compatible = "mediatek,mt2701-dsi",
+	  .data = &mt2701_dsi_driver_data },
+	{ .compatible = "mediatek,mt8173-dsi",
+	  .data = &mt8173_dsi_driver_data },
+	{ },
+};
+
 static int mtk_dsi_probe(struct platform_device *pdev)
 {
 	struct mtk_dsi *dsi;
 	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
 	struct resource *regs;
 	int irq_num;
 	int comp_id;
@@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_unregister_host;
 
+	of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
+	dsi->driver_data = of_id->data;
+
 	dsi->engine_clk = devm_clk_get(dev, "engine");
 	if (IS_ERR(dsi->engine_clk)) {
 		ret = PTR_ERR(dsi->engine_clk);
@@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id mtk_dsi_of_match[] = {
-	{ .compatible = "mediatek,mt2701-dsi" },
-	{ .compatible = "mediatek,mt8173-dsi" },
-	{ },
-};
-
 struct platform_driver mtk_dsi_driver = {
 	.probe = mtk_dsi_probe,
 	.remove = mtk_dsi_remove,
-- 
2.21.0

Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4967 bytes --]

Hi Jitao,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.1-rc5 next-20190416]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jitao-Shi/drm-mediatek-move-mipi_dsi_host_register-to-probe/20190417-100619
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

   drivers/gpu//drm/mediatek/mtk_dsi.c: In function 'mtk_dsi_probe':
>> drivers/gpu//drm/mediatek/mtk_dsi.c:1129:19: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     dsi->driver_data = of_id->data;
                      ^

vim +/const +1129 drivers/gpu//drm/mediatek/mtk_dsi.c

  1099	
  1100	static int mtk_dsi_probe(struct platform_device *pdev)
  1101	{
  1102		struct mtk_dsi *dsi;
  1103		struct device *dev = &pdev->dev;
  1104		const struct of_device_id *of_id;
  1105		struct resource *regs;
  1106		int irq_num;
  1107		int comp_id;
  1108		int ret;
  1109	
  1110		dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
  1111		if (!dsi)
  1112			return -ENOMEM;
  1113	
  1114		dsi->host.ops = &mtk_dsi_ops;
  1115		dsi->host.dev = dev;
  1116		dsi->dev = dev;
  1117		ret = mipi_dsi_host_register(&dsi->host);
  1118		if (ret < 0) {
  1119			dev_err(dev, "failed to register DSI host: %d\n", ret);
  1120			return ret;
  1121		}
  1122	
  1123		ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
  1124						  &dsi->panel, &dsi->bridge);
  1125		if (ret)
  1126			goto err_unregister_host;
  1127	
  1128		of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> 1129		dsi->driver_data = of_id->data;
  1130	
  1131		dsi->engine_clk = devm_clk_get(dev, "engine");
  1132		if (IS_ERR(dsi->engine_clk)) {
  1133			ret = PTR_ERR(dsi->engine_clk);
  1134			dev_err(dev, "Failed to get engine clock: %d\n", ret);
  1135			goto err_unregister_host;
  1136		}
  1137	
  1138		dsi->digital_clk = devm_clk_get(dev, "digital");
  1139		if (IS_ERR(dsi->digital_clk)) {
  1140			ret = PTR_ERR(dsi->digital_clk);
  1141			dev_err(dev, "Failed to get digital clock: %d\n", ret);
  1142			goto err_unregister_host;
  1143		}
  1144	
  1145		dsi->hs_clk = devm_clk_get(dev, "hs");
  1146		if (IS_ERR(dsi->hs_clk)) {
  1147			ret = PTR_ERR(dsi->hs_clk);
  1148			dev_err(dev, "Failed to get hs clock: %d\n", ret);
  1149			goto err_unregister_host;
  1150		}
  1151	
  1152		regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  1153		dsi->regs = devm_ioremap_resource(dev, regs);
  1154		if (IS_ERR(dsi->regs)) {
  1155			ret = PTR_ERR(dsi->regs);
  1156			dev_err(dev, "Failed to ioremap memory: %d\n", ret);
  1157			goto err_unregister_host;
  1158		}
  1159	
  1160		dsi->phy = devm_phy_get(dev, "dphy");
  1161		if (IS_ERR(dsi->phy)) {
  1162			ret = PTR_ERR(dsi->phy);
  1163			dev_err(dev, "Failed to get MIPI-DPHY: %d\n", ret);
  1164			goto err_unregister_host;
  1165		}
  1166	
  1167		comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DSI);
  1168		if (comp_id < 0) {
  1169			dev_err(dev, "Failed to identify by alias: %d\n", comp_id);
  1170			ret = comp_id;
  1171			goto err_unregister_host;
  1172		}
  1173	
  1174		ret = mtk_ddp_comp_init(dev, dev->of_node, &dsi->ddp_comp, comp_id,
  1175					&mtk_dsi_funcs);
  1176		if (ret) {
  1177			dev_err(dev, "Failed to initialize component: %d\n", ret);
  1178			goto err_unregister_host;
  1179		}
  1180	
  1181		irq_num = platform_get_irq(pdev, 0);
  1182		if (irq_num < 0) {
  1183			dev_err(&pdev->dev, "failed to get dsi irq_num: %d\n", irq_num);
  1184			ret = irq_num;
  1185			goto err_unregister_host;
  1186		}
  1187	
  1188		irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
  1189		ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq,
  1190				       IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi);
  1191		if (ret) {
  1192			dev_err(&pdev->dev, "failed to request mediatek dsi irq\n");
  1193			goto err_unregister_host;
  1194		}
  1195	
  1196		init_waitqueue_head(&dsi->irq_wait_queue);
  1197	
  1198		platform_set_drvdata(pdev, dsi);
  1199	
  1200		ret = component_add(&pdev->dev, &mtk_dsi_component_ops);
  1201		if (ret) {
  1202			dev_err(&pdev->dev, "failed to add component: %d\n", ret);
  1203			goto err_unregister_host;
  1204		}
  1205	
  1206		return 0;
  1207	
  1208	err_unregister_host:
  1209		mipi_dsi_host_unregister(&dsi->host);
  1210		return ret;
  1211	}
  1212	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68322 bytes --]

Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701

[Nicolas Boichat]

On Tue, Apr 16, 2019 at 2:05 PM Jitao Shi <jitao.shi@mediatek.com> wrote:
>
> Config the different CMDQ reg address in driver data.
>
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 6c4ac37f983d..573e6bec6d36 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -131,7 +131,6 @@
>  #define VM_CMD_EN                      BIT(0)
>  #define TS_VFP_EN                      BIT(5)
>
> -#define DSI_CMDQ0              0x180
>  #define CONFIG                         (0xff << 0)
>  #define SHORT_PACKET                   0
>  #define LONG_PACKET                    2
> @@ -156,6 +155,10 @@
>
>  struct phy;
>
> +struct mtk_dsi_driver_data {
> +       const u32 reg_cmdq_off;
> +};
> +
>  struct mtk_dsi {
>         struct mtk_ddp_comp ddp_comp;
>         struct device *dev;
> @@ -182,6 +185,7 @@ struct mtk_dsi {
>         bool enabled;
>         u32 irq_data;
>         wait_queue_head_t irq_wait_queue;
> +       struct mtk_dsi_driver_data *driver_data;

As highlighted by kbuild, you're missing a const here.

>  };
>
>  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> @@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
>         const char *tx_buf = msg->tx_buf;
>         u8 config, cmdq_size, cmdq_off, type = msg->type;
>         u32 reg_val, cmdq_mask, i;
> +       u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
>
>         if (MTK_DSI_HOST_IS_READ(type))
>                 config = BTA;
> @@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
>         }
>
>         for (i = 0; i < msg->tx_len; i++)
> -               writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
> +               mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
> +                            (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
> +                            tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));

The writeb call looked significantly cleaner...

writeb(tx_buf[i], dsi->regs + reg_cmdq_off + cmdq_off + i);
should just work, right?

>
> -       mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
> +       mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
>         mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
>  }
>
> @@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
>         .unbind = mtk_dsi_unbind,
>  };
>
> +static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
> +       .reg_cmdq_off = 0x200,
> +};
> +
> +static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
> +       .reg_cmdq_off = 0x180,
> +};
> +
> +static const struct of_device_id mtk_dsi_of_match[] = {
> +       { .compatible = "mediatek,mt2701-dsi",
> +         .data = &mt2701_dsi_driver_data },
> +       { .compatible = "mediatek,mt8173-dsi",
> +         .data = &mt8173_dsi_driver_data },
> +       { },
> +};
> +
>  static int mtk_dsi_probe(struct platform_device *pdev)
>  {
>         struct mtk_dsi *dsi;
>         struct device *dev = &pdev->dev;
> +       const struct of_device_id *of_id;
>         struct resource *regs;
>         int irq_num;
>         int comp_id;
> @@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_unregister_host;
>
> +       of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> +       dsi->driver_data = of_id->data;
> +
>         dsi->engine_clk = devm_clk_get(dev, "engine");
>         if (IS_ERR(dsi->engine_clk)) {
>                 ret = PTR_ERR(dsi->engine_clk);
> @@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> -static const struct of_device_id mtk_dsi_of_match[] = {
> -       { .compatible = "mediatek,mt2701-dsi" },
> -       { .compatible = "mediatek,mt8173-dsi" },
> -       { },
> -};
> -
>  struct platform_driver mtk_dsi_driver = {
>         .probe = mtk_dsi_probe,
>         .remove = mtk_dsi_remove,
> --
> 2.21.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701

[CK Hu]

On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> Config the different CMDQ reg address in driver data.
> 
For MT8173, you change reg_cmd_off from 0x180 to 0x200, so this patch is
a bug fix. You should add a 'Fixes' tag.

> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 6c4ac37f983d..573e6bec6d36 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -131,7 +131,6 @@
>  #define VM_CMD_EN			BIT(0)
>  #define TS_VFP_EN			BIT(5)
>  
> -#define DSI_CMDQ0		0x180
>  #define CONFIG				(0xff << 0)
>  #define SHORT_PACKET			0
>  #define LONG_PACKET			2
> @@ -156,6 +155,10 @@
>  
>  struct phy;
>  
> +struct mtk_dsi_driver_data {
> +	const u32 reg_cmdq_off;
> +};
> +
>  struct mtk_dsi {
>  	struct mtk_ddp_comp ddp_comp;
>  	struct device *dev;
> @@ -182,6 +185,7 @@ struct mtk_dsi {
>  	bool enabled;
>  	u32 irq_data;
>  	wait_queue_head_t irq_wait_queue;
> +	struct mtk_dsi_driver_data *driver_data;
>  };
>  
>  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> @@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
>  	const char *tx_buf = msg->tx_buf;
>  	u8 config, cmdq_size, cmdq_off, type = msg->type;
>  	u32 reg_val, cmdq_mask, i;
> +	u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
>  
>  	if (MTK_DSI_HOST_IS_READ(type))
>  		config = BTA;
> @@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
>  	}
>  
>  	for (i = 0; i < msg->tx_len; i++)
> -		writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
> +		mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
> +			     (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
> +			     tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));

You say you would follow Nicolas' suggestion here.

>  
> -	mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
> +	mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
>  	mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
>  }
>  
> @@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
>  	.unbind = mtk_dsi_unbind,
>  };
>  
> +static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
> +	.reg_cmdq_off = 0x200,
> +};
> +
> +static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
> +	.reg_cmdq_off = 0x180,
> +};
> +
> +static const struct of_device_id mtk_dsi_of_match[] = {
> +	{ .compatible = "mediatek,mt2701-dsi",
> +	  .data = &mt2701_dsi_driver_data },
> +	{ .compatible = "mediatek,mt8173-dsi",
> +	  .data = &mt8173_dsi_driver_data },
> +	{ },
> +};
> +
>  static int mtk_dsi_probe(struct platform_device *pdev)
>  {
>  	struct mtk_dsi *dsi;
>  	struct device *dev = &pdev->dev;
> +	const struct of_device_id *of_id;
>  	struct resource *regs;
>  	int irq_num;
>  	int comp_id;
> @@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_unregister_host;
>  
> +	of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> +	dsi->driver_data = of_id->data;

Maybe use of_device_get_match_data() is a more simple way. You could
refer to [1].

[1]
https://elixir.bootlin.com/linux/v5.1/source/drivers/gpu/drm/mediatek/mtk_disp_ovl.c#L300

Regards,
CK

> +
>  	dsi->engine_clk = devm_clk_get(dev, "engine");
>  	if (IS_ERR(dsi->engine_clk)) {
>  		ret = PTR_ERR(dsi->engine_clk);
> @@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static const struct of_device_id mtk_dsi_of_match[] = {
> -	{ .compatible = "mediatek,mt2701-dsi" },
> -	{ .compatible = "mediatek,mt8173-dsi" },
> -	{ },
> -};
> -
>  struct platform_driver mtk_dsi_driver = {
>  	.probe = mtk_dsi_probe,
>  	.remove = mtk_dsi_remove,


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701

[Jitao Shi]

On Wed, 2019-05-08 at 10:39 +0800, CK Hu wrote:
> On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> > Config the different CMDQ reg address in driver data.
> > 
> For MT8173, you change reg_cmd_off from 0x180 to 0x200, so this patch is
> a bug fix. You should add a 'Fixes' tag.
> 
> > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> > ---
> >  drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
> >  1 file changed, 30 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > index 6c4ac37f983d..573e6bec6d36 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > @@ -131,7 +131,6 @@
> >  #define VM_CMD_EN			BIT(0)
> >  #define TS_VFP_EN			BIT(5)
> >  
> > -#define DSI_CMDQ0		0x180
> >  #define CONFIG				(0xff << 0)
> >  #define SHORT_PACKET			0
> >  #define LONG_PACKET			2
> > @@ -156,6 +155,10 @@
> >  
> >  struct phy;
> >  
> > +struct mtk_dsi_driver_data {
> > +	const u32 reg_cmdq_off;
> > +};
> > +
> >  struct mtk_dsi {
> >  	struct mtk_ddp_comp ddp_comp;
> >  	struct device *dev;
> > @@ -182,6 +185,7 @@ struct mtk_dsi {
> >  	bool enabled;
> >  	u32 irq_data;
> >  	wait_queue_head_t irq_wait_queue;
> > +	struct mtk_dsi_driver_data *driver_data;
> >  };
> >  
> >  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> > @@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> >  	const char *tx_buf = msg->tx_buf;
> >  	u8 config, cmdq_size, cmdq_off, type = msg->type;
> >  	u32 reg_val, cmdq_mask, i;
> > +	u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
> >  
> >  	if (MTK_DSI_HOST_IS_READ(type))
> >  		config = BTA;
> > @@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> >  	}
> >  
> >  	for (i = 0; i < msg->tx_len; i++)
> > -		writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
> > +		mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
> > +			     (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
> > +			     tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));
> 
> You say you would follow Nicolas' suggestion here.
> 

If i replace mtk_dsi_mask with writeb, i can't get right value from
registers. I don't know why this.

> >  
> > -	mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
> > +	mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
> >  	mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
> >  }
> >  
> > @@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
> >  	.unbind = mtk_dsi_unbind,
> >  };
> >  
> > +static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
> > +	.reg_cmdq_off = 0x200,
> > +};
> > +
> > +static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
> > +	.reg_cmdq_off = 0x180,
> > +};
> > +
> > +static const struct of_device_id mtk_dsi_of_match[] = {
> > +	{ .compatible = "mediatek,mt2701-dsi",
> > +	  .data = &mt2701_dsi_driver_data },
> > +	{ .compatible = "mediatek,mt8173-dsi",
> > +	  .data = &mt8173_dsi_driver_data },
> > +	{ },
> > +};
> > +
> >  static int mtk_dsi_probe(struct platform_device *pdev)
> >  {
> >  	struct mtk_dsi *dsi;
> >  	struct device *dev = &pdev->dev;
> > +	const struct of_device_id *of_id;
> >  	struct resource *regs;
> >  	int irq_num;
> >  	int comp_id;
> > @@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		goto err_unregister_host;
> >  
> > +	of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> > +	dsi->driver_data = of_id->data;
> 
> Maybe use of_device_get_match_data() is a more simple way. You could
> refer to [1].
> 
> [1]
> https://elixir.bootlin.com/linux/v5.1/source/drivers/gpu/drm/mediatek/mtk_disp_ovl.c#L300
> 
> Regards,
> CK
> 

I'll fix it next version.

> > +
> >  	dsi->engine_clk = devm_clk_get(dev, "engine");
> >  	if (IS_ERR(dsi->engine_clk)) {
> >  		ret = PTR_ERR(dsi->engine_clk);
> > @@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > -static const struct of_device_id mtk_dsi_of_match[] = {
> > -	{ .compatible = "mediatek,mt2701-dsi" },
> > -	{ .compatible = "mediatek,mt8173-dsi" },
> > -	{ },
> > -};
> > -
> >  struct platform_driver mtk_dsi_driver = {
> >  	.probe = mtk_dsi_probe,
> >  	.remove = mtk_dsi_remove,
> 
> 

Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701

[CK Hu]

Hi, Jitao:

On Sun, 2019-05-19 at 17:33 +0800, Jitao Shi wrote:
> On Wed, 2019-05-08 at 10:39 +0800, CK Hu wrote:
> > On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> > > Config the different CMDQ reg address in driver data.
> > > 
> > For MT8173, you change reg_cmd_off from 0x180 to 0x200, so this patch is
> > a bug fix. You should add a 'Fixes' tag.
> > 
> > > Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> > > ---
> > >  drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
> > >  1 file changed, 30 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > index 6c4ac37f983d..573e6bec6d36 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > @@ -131,7 +131,6 @@
> > >  #define VM_CMD_EN			BIT(0)
> > >  #define TS_VFP_EN			BIT(5)
> > >  
> > > -#define DSI_CMDQ0		0x180
> > >  #define CONFIG				(0xff << 0)
> > >  #define SHORT_PACKET			0
> > >  #define LONG_PACKET			2
> > > @@ -156,6 +155,10 @@
> > >  
> > >  struct phy;
> > >  
> > > +struct mtk_dsi_driver_data {
> > > +	const u32 reg_cmdq_off;
> > > +};
> > > +
> > >  struct mtk_dsi {
> > >  	struct mtk_ddp_comp ddp_comp;
> > >  	struct device *dev;
> > > @@ -182,6 +185,7 @@ struct mtk_dsi {
> > >  	bool enabled;
> > >  	u32 irq_data;
> > >  	wait_queue_head_t irq_wait_queue;
> > > +	struct mtk_dsi_driver_data *driver_data;
> > >  };
> > >  
> > >  static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> > > @@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> > >  	const char *tx_buf = msg->tx_buf;
> > >  	u8 config, cmdq_size, cmdq_off, type = msg->type;
> > >  	u32 reg_val, cmdq_mask, i;
> > > +	u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
> > >  
> > >  	if (MTK_DSI_HOST_IS_READ(type))
> > >  		config = BTA;
> > > @@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> > >  	}
> > >  
> > >  	for (i = 0; i < msg->tx_len; i++)
> > > -		writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
> > > +		mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
> > > +			     (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
> > > +			     tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));
> > 
> > You say you would follow Nicolas' suggestion here.
> > 
> 
> If i replace mtk_dsi_mask with writeb, i can't get right value from
> registers. I don't know why this.

I remember that Shaoming has also meet some error about writeb(), but he
finally fix this bug. You may get some hint from him. If we can not use
writeb(), this modification should be two patches: one is changing
DSI_CMDQ0 to reg_cmdq_off, another one is changing writeb() to
mtk_dsi_mask().

Regards,
CK

> 
> > >  
> > > -	mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
> > > +	mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
> > >  	mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
> > >  }
> > >  
> > > @@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
> > >  	.unbind = mtk_dsi_unbind,
> > >  };
> > >  
> > > +static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
> > > +	.reg_cmdq_off = 0x200,
> > > +};
> > > +
> > > +static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
> > > +	.reg_cmdq_off = 0x180,
> > > +};
> > > +
> > > +static const struct of_device_id mtk_dsi_of_match[] = {
> > > +	{ .compatible = "mediatek,mt2701-dsi",
> > > +	  .data = &mt2701_dsi_driver_data },
> > > +	{ .compatible = "mediatek,mt8173-dsi",
> > > +	  .data = &mt8173_dsi_driver_data },
> > > +	{ },
> > > +};
> > > +
> > >  static int mtk_dsi_probe(struct platform_device *pdev)
> > >  {
> > >  	struct mtk_dsi *dsi;
> > >  	struct device *dev = &pdev->dev;
> > > +	const struct of_device_id *of_id;
> > >  	struct resource *regs;
> > >  	int irq_num;
> > >  	int comp_id;
> > > @@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> > >  	if (ret)
> > >  		goto err_unregister_host;
> > >  
> > > +	of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> > > +	dsi->driver_data = of_id->data;
> > 
> > Maybe use of_device_get_match_data() is a more simple way. You could
> > refer to [1].
> > 
> > [1]
> > https://elixir.bootlin.com/linux/v5.1/source/drivers/gpu/drm/mediatek/mtk_disp_ovl.c#L300
> > 
> > Regards,
> > CK
> > 
> 
> I'll fix it next version.
> 
> > > +
> > >  	dsi->engine_clk = devm_clk_get(dev, "engine");
> > >  	if (IS_ERR(dsi->engine_clk)) {
> > >  		ret = PTR_ERR(dsi->engine_clk);
> > > @@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
> > >  	return 0;
> > >  }
> > >  
> > > -static const struct of_device_id mtk_dsi_of_match[] = {
> > > -	{ .compatible = "mediatek,mt2701-dsi" },
> > > -	{ .compatible = "mediatek,mt8173-dsi" },
> > > -	{ },
> > > -};
> > > -
> > >  struct platform_driver mtk_dsi_driver = {
> > >  	.probe = mtk_dsi_probe,
> > >  	.remove = mtk_dsi_remove,
> > 
> > 
> 
> 

[v2 3/5] drm/mediatek: add dsi reg commit control

[Jitao Shi]

New DSI IP has shadow register and working reg. The register
values are writen to shadow register. And then trigger with
commit reg, the register values will be moved working register.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 573e6bec6d36..be42405a0a78 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -131,6 +131,10 @@
 #define VM_CMD_EN			BIT(0)
 #define TS_VFP_EN			BIT(5)
 
+#define DSI_SHADOW_DEBUG	0x190U
+#define FORCE_COMMIT		BIT(0)
+#define BYPASS_SHADOW		BIT(1)
+
 #define CONFIG				(0xff << 0)
 #define SHORT_PACKET			0
 #define LONG_PACKET			2
@@ -157,6 +161,7 @@ struct phy;
 
 struct mtk_dsi_driver_data {
 	const u32 reg_cmdq_off;
+	bool has_shadow_ctl;
 };
 
 struct mtk_dsi {
@@ -594,6 +599,11 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
 	}
 
 	mtk_dsi_enable(dsi);
+
+	if (dsi->driver_data->has_shadow_ctl)
+		writel(FORCE_COMMIT | BYPASS_SHADOW,
+		       dsi->regs + DSI_SHADOW_DEBUG);
+
 	mtk_dsi_reset_engine(dsi);
 	mtk_dsi_phy_timconfig(dsi);
 
-- 
2.21.0

Re: [v2 3/5] drm/mediatek: add dsi reg commit control

[CK Hu]

Hi, Jitao:

On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> New DSI IP has shadow register and working reg. The register
> values are writen to shadow register. And then trigger with
> commit reg, the register values will be moved working register.

This patch looks good, but the message is not complete. The message make
us believe you use shadow register to work, but actually, shadow
register is default turn on in new DSI IP and you want to turn off it.

Regards,
CK

> 
> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 573e6bec6d36..be42405a0a78 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -131,6 +131,10 @@
>  #define VM_CMD_EN			BIT(0)
>  #define TS_VFP_EN			BIT(5)
>  
> +#define DSI_SHADOW_DEBUG	0x190U
> +#define FORCE_COMMIT		BIT(0)
> +#define BYPASS_SHADOW		BIT(1)
> +
>  #define CONFIG				(0xff << 0)
>  #define SHORT_PACKET			0
>  #define LONG_PACKET			2
> @@ -157,6 +161,7 @@ struct phy;
>  
>  struct mtk_dsi_driver_data {
>  	const u32 reg_cmdq_off;
> +	bool has_shadow_ctl;
>  };
>  
>  struct mtk_dsi {
> @@ -594,6 +599,11 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
>  	}
>  
>  	mtk_dsi_enable(dsi);
> +
> +	if (dsi->driver_data->has_shadow_ctl)
> +		writel(FORCE_COMMIT | BYPASS_SHADOW,
> +		       dsi->regs + DSI_SHADOW_DEBUG);
> +
>  	mtk_dsi_reset_engine(dsi);
>  	mtk_dsi_phy_timconfig(dsi);
>  


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[v2 4/5] drm/mediatek: add frame size control

[Jitao Shi]

Our new DSI chip has frame size control.
So add the driver data to control for different chips.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index be42405a0a78..458a700ce74c 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -78,6 +78,7 @@
 #define DSI_VBP_NL		0x24
 #define DSI_VFP_NL		0x28
 #define DSI_VACT_NL		0x2C
+#define DSI_SIZE_CON		0x38
 #define DSI_HSA_WC		0x50
 #define DSI_HBP_WC		0x54
 #define DSI_HFP_WC		0x58
@@ -162,6 +163,7 @@ struct phy;
 struct mtk_dsi_driver_data {
 	const u32 reg_cmdq_off;
 	bool has_shadow_ctl;
+	bool has_size_ctl;
 };
 
 struct mtk_dsi {
@@ -430,6 +432,9 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
 	writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
 	writel(vm->vactive, dsi->regs + DSI_VACT_NL);
 
+	if (dsi->driver_data->has_size_ctl)
+		writel(vm->vactive << 16 | vm->hactive, dsi->regs + DSI_SIZE_CON);
+
 	horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10);
 
 	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
-- 
2.21.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 4/5] drm/mediatek: add frame size control

[CK Hu]

Hi, Jitao:

On Tue, 2019-04-16 at 14:05 +0800, Jitao Shi wrote:
> Our new DSI chip has frame size control.
> So add the driver data to control for different chips.
> 

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index be42405a0a78..458a700ce74c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -78,6 +78,7 @@
>  #define DSI_VBP_NL		0x24
>  #define DSI_VFP_NL		0x28
>  #define DSI_VACT_NL		0x2C
> +#define DSI_SIZE_CON		0x38
>  #define DSI_HSA_WC		0x50
>  #define DSI_HBP_WC		0x54
>  #define DSI_HFP_WC		0x58
> @@ -162,6 +163,7 @@ struct phy;
>  struct mtk_dsi_driver_data {
>  	const u32 reg_cmdq_off;
>  	bool has_shadow_ctl;
> +	bool has_size_ctl;
>  };
>  
>  struct mtk_dsi {
> @@ -430,6 +432,9 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
>  	writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
>  	writel(vm->vactive, dsi->regs + DSI_VACT_NL);
>  
> +	if (dsi->driver_data->has_size_ctl)
> +		writel(vm->vactive << 16 | vm->hactive, dsi->regs + DSI_SIZE_CON);
> +
>  	horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10);
>  
>  	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[v2 5/5] drm/mediatek: add mt8183 dsi driver support

[Jitao Shi]

Add mt8183 dsi driver data. Enable size control and
reg commit control.

Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_dsi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 458a700ce74c..f0b36ec38e4f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1104,11 +1104,19 @@ static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
 	.reg_cmdq_off = 0x180,
 };
 
+static const struct mtk_dsi_driver_data mt8183_dsi_driver_data = {
+	.reg_cmdq_off = 0x200,
+	.has_shadow_ctl = true,
+	.has_size_ctl = true,
+};
+
 static const struct of_device_id mtk_dsi_of_match[] = {
 	{ .compatible = "mediatek,mt2701-dsi",
 	  .data = &mt2701_dsi_driver_data },
 	{ .compatible = "mediatek,mt8173-dsi",
 	  .data = &mt8173_dsi_driver_data },
+	{ .compatible = "mediatek,mt8183-dsi",
+	  .data = &mt8183_dsi_driver_data },
 	{ },
 };
 
-- 
2.21.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2 5/5] drm/mediatek: add mt8183 dsi driver support

[CK Hu]

Hi, Jitao:

On Tue, 2019-04-16 at 14:05 +0800, Jitao Shi wrote:
> Add mt8183 dsi driver data. Enable size control and
> reg commit control.
> 

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_dsi.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 458a700ce74c..f0b36ec38e4f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1104,11 +1104,19 @@ static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
>  	.reg_cmdq_off = 0x180,
>  };
>  
> +static const struct mtk_dsi_driver_data mt8183_dsi_driver_data = {
> +	.reg_cmdq_off = 0x200,
> +	.has_shadow_ctl = true,
> +	.has_size_ctl = true,
> +};
> +
>  static const struct of_device_id mtk_dsi_of_match[] = {
>  	{ .compatible = "mediatek,mt2701-dsi",
>  	  .data = &mt2701_dsi_driver_data },
>  	{ .compatible = "mediatek,mt8173-dsi",
>  	  .data = &mt8173_dsi_driver_data },
> +	{ .compatible = "mediatek,mt8183-dsi",
> +	  .data = &mt8183_dsi_driver_data },
>  	{ },
>  };
>