All of lore.kernel.org
 help / color / mirror / Atom feed
* [v2 0/5] support dsi for mt8183
@ 2019-04-16  6:04 ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: Jitao Shi, Thierry Reding, Ajay Kumar, Inki Dae, Rahul Sharma,
	Sean Paul, Vincent Palatin, Andy Yan, Philipp Zabel,
	Russell King, devicetree, linux-kernel, dri-devel,
	linux-arm-kernel, linux-mediatek, srv_heupstream, Sascha Hauer,
	yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh, ck.hu,
	stonea168

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


^ permalink raw reply	[flat|nested] 49+ messages in thread

* [v2 0/5] support dsi for mt8183
@ 2019-04-16  6:04 ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, bibby.hsieh, ck.hu, Russell King, Thierry Reding,
	devicetree, Jitao Shi, Philipp Zabel, Inki Dae, linux-mediatek,
	yingjoe.chen, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Sascha Hauer, Sean Paul

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [v2 0/5] support dsi for mt8183
@ 2019-04-16  6:04 ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, bibby.hsieh, ck.hu, Russell King, Thierry Reding,
	devicetree, Jitao Shi, Philipp Zabel, Inki Dae, linux-mediatek,
	yingjoe.chen, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Sascha Hauer, Sean Paul

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
  2019-04-16  6:04 ` Jitao Shi
  (?)
@ 2019-04-16  6:04   ` Jitao Shi
  -1 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: Jitao Shi, Thierry Reding, Ajay Kumar, Inki Dae, Rahul Sharma,
	Sean Paul, Vincent Palatin, Andy Yan, Philipp Zabel,
	Russell King, devicetree, linux-kernel, dri-devel,
	linux-arm-kernel, linux-mediatek, srv_heupstream, Sascha Hauer,
	yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh, ck.hu,
	stonea168

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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
@ 2019-04-16  6:04   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: Jitao Shi, Thierry Reding, Ajay Kumar, Inki Dae, Rahul Sharma,
	Sean Paul, Vincent Palatin, Andy Yan, Philipp Zabel,
	Russell King, devicetree, linux-kernel, dri-devel,
	linux-arm-kernel, linux-mediatek, srv_heupstream, Sascha Hauer,
	yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh, ck.hu

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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
@ 2019-04-16  6:04   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, bibby.hsieh, ck.hu, Russell King, Thierry Reding,
	devicetree, Jitao Shi, Philipp Zabel, Inki Dae, linux-mediatek,
	yingjoe.chen, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Sascha Hauer, Sean Paul

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
  2019-04-16  6:04 ` Jitao Shi
  (?)
@ 2019-04-16  6:04   ` Jitao Shi
  -1 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: Jitao Shi, Thierry Reding, Ajay Kumar, Inki Dae, Rahul Sharma,
	Sean Paul, Vincent Palatin, Andy Yan, Philipp Zabel,
	Russell King, devicetree, linux-kernel, dri-devel,
	linux-arm-kernel, linux-mediatek, srv_heupstream, Sascha Hauer,
	yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh, ck.hu,
	stonea168

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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-04-16  6:04   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: Jitao Shi, Thierry Reding, Ajay Kumar, Inki Dae, Rahul Sharma,
	Sean Paul, Vincent Palatin, Andy Yan, Philipp Zabel,
	Russell King, devicetree, linux-kernel, dri-devel,
	linux-arm-kernel, linux-mediatek, srv_heupstream, Sascha Hauer,
	yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh, ck.hu

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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-04-16  6:04   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, bibby.hsieh, ck.hu, Russell King, Thierry Reding,
	devicetree, Jitao Shi, Philipp Zabel, Inki Dae, linux-mediatek,
	yingjoe.chen, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Sascha Hauer, Sean Paul

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 3/5] drm/mediatek: add dsi reg commit control
  2019-04-16  6:04 ` Jitao Shi
  (?)
@ 2019-04-16  6:04   ` Jitao Shi
  -1 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: Jitao Shi, Thierry Reding, Ajay Kumar, Inki Dae, Rahul Sharma,
	Sean Paul, Vincent Palatin, Andy Yan, Philipp Zabel,
	Russell King, devicetree, linux-kernel, dri-devel,
	linux-arm-kernel, linux-mediatek, srv_heupstream, Sascha Hauer,
	yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh, ck.hu,
	stonea168

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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 3/5] drm/mediatek: add dsi reg commit control
@ 2019-04-16  6:04   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, bibby.hsieh, ck.hu, Russell King, Thierry Reding,
	devicetree, Jitao Shi, Philipp Zabel, Inki Dae, linux-mediatek,
	yingjoe.chen, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Sascha Hauer, Sean Paul

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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 3/5] drm/mediatek: add dsi reg commit control
@ 2019-04-16  6:04   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:04 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, bibby.hsieh, ck.hu, Russell King, Thierry Reding,
	devicetree, Jitao Shi, Philipp Zabel, Inki Dae, linux-mediatek,
	yingjoe.chen, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Sascha Hauer, Sean Paul

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 4/5] drm/mediatek: add frame size control
  2019-04-16  6:04 ` Jitao Shi
  (?)
@ 2019-04-16  6:05   ` Jitao Shi
  -1 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:05 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: Jitao Shi, Thierry Reding, Ajay Kumar, Inki Dae, Rahul Sharma,
	Sean Paul, Vincent Palatin, Andy Yan, Philipp Zabel,
	Russell King, devicetree, linux-kernel, dri-devel,
	linux-arm-kernel, linux-mediatek, srv_heupstream, Sascha Hauer,
	yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh, ck.hu,
	stonea168

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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 4/5] drm/mediatek: add frame size control
@ 2019-04-16  6:05   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:05 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, Russell King, Thierry Reding, devicetree, Jitao Shi,
	linux-mediatek, yingjoe.chen, eddie.huang, linux-arm-kernel,
	Rahul Sharma, srv_heupstream, linux-kernel, Sascha Hauer,
	Sean Paul

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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 4/5] drm/mediatek: add frame size control
@ 2019-04-16  6:05   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:05 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, bibby.hsieh, ck.hu, Russell King, Thierry Reding,
	devicetree, Jitao Shi, Philipp Zabel, Inki Dae, linux-mediatek,
	yingjoe.chen, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Sascha Hauer, Sean Paul

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 5/5] drm/mediatek: add mt8183 dsi driver support
  2019-04-16  6:04 ` Jitao Shi
  (?)
@ 2019-04-16  6:05   ` Jitao Shi
  -1 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:05 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: Jitao Shi, Thierry Reding, Ajay Kumar, Inki Dae, Rahul Sharma,
	Sean Paul, Vincent Palatin, Andy Yan, Philipp Zabel,
	Russell King, devicetree, linux-kernel, dri-devel,
	linux-arm-kernel, linux-mediatek, srv_heupstream, Sascha Hauer,
	yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh, ck.hu,
	stonea168

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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 5/5] drm/mediatek: add mt8183 dsi driver support
@ 2019-04-16  6:05   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:05 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, Russell King, Thierry Reding, devicetree, Jitao Shi,
	linux-mediatek, yingjoe.chen, eddie.huang, linux-arm-kernel,
	Rahul Sharma, srv_heupstream, linux-kernel, Sascha Hauer,
	Sean Paul

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

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [v2 5/5] drm/mediatek: add mt8183 dsi driver support
@ 2019-04-16  6:05   ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-04-16  6:05 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger
  Cc: stonea168, dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin,
	cawa.cheng, bibby.hsieh, ck.hu, Russell King, Thierry Reding,
	devicetree, Jitao Shi, Philipp Zabel, Inki Dae, linux-mediatek,
	yingjoe.chen, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Sascha Hauer, Sean Paul

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
  2019-04-16  6:04   ` Jitao Shi
  (?)
@ 2019-04-17  4:58     ` kbuild test robot
  -1 siblings, 0 replies; 49+ messages in thread
From: kbuild test robot @ 2019-04-17  4:58 UTC (permalink / raw)
  To: Jitao Shi
  Cc: kbuild-all, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, linux-pwm, David Airlie, Matthias Brugger, stonea168,
	dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin, cawa.cheng,
	Russell King, Thierry Reding, devicetree, Jitao Shi,
	linux-mediatek, yingjoe.chen, eddie.huang, linux-arm-kernel,
	Rahul Sharma, srv_heupstream, linux-kernel, Sascha Hauer,
	Sean Paul

[-- 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 --]

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-04-17  4:58     ` kbuild test robot
  0 siblings, 0 replies; 49+ messages in thread
From: kbuild test robot @ 2019-04-17  4:58 UTC (permalink / raw)
  Cc: kbuild-all, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, linux-pwm, David Airlie, Matthias Brugger, stonea168,
	dri-devel, Andy Yan, Ajay Kumar, Vincent Palatin, cawa.cheng,
	Russell King, Thierry Reding, devicetree, Jitao Shi,
	linux-mediatek, yingjoe.chen, eddie.huang, linux-arm-kernel

[-- 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 --]

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-04-17  4:58     ` kbuild test robot
  0 siblings, 0 replies; 49+ messages in thread
From: kbuild test robot @ 2019-04-17  4:58 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	Ajay Kumar, Vincent Palatin, cawa.cheng, Russell King,
	Thierry Reding, Sean Paul, linux-pwm, Jitao Shi, Pawel Moll,
	Ian Campbell, Rob Herring, linux-mediatek, yingjoe.chen,
	Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, kbuild-all, Sascha Hauer,
	Kumar Gala, Andy Yan

[-- 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 --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
  2019-04-16  6:04   ` Jitao Shi
  (?)
@ 2019-04-18  7:51     ` Nicolas Boichat
  -1 siblings, 0 replies; 49+ messages in thread
From: Nicolas Boichat @ 2019-04-18  7:51 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, stonea168, dri-devel,
	Andy Yan, Ajay Kumar, Vincent Palatin, cawa cheng, Russell King,
	Thierry Reding, devicetree,
	moderated list:ARM/Mediatek SoC support, Yingjoe Chen,
	Eddie Huang, linux-arm Mailing List, Rahul Sharma,
	srv_heupstream, lkml, Sascha Hauer, Sean Paul

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-04-18  7:51     ` Nicolas Boichat
  0 siblings, 0 replies; 49+ messages in thread
From: Nicolas Boichat @ 2019-04-18  7:51 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	Ajay Kumar, Vincent Palatin, cawa cheng, Russell King,
	Thierry Reding, Sean Paul, linux-pwm, Pawel Moll, Ian Campbell,
	Rob Herring, moderated list:ARM/Mediatek SoC support,
	Yingjoe Chen, Matthias Brugger, Eddie Huang,
	linux-arm Mailing List, Rahul Sharma, srv_heupstream, lk

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-04-18  7:51     ` Nicolas Boichat
  0 siblings, 0 replies; 49+ messages in thread
From: Nicolas Boichat @ 2019-04-18  7:51 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	Ajay Kumar, Vincent Palatin, cawa cheng, Russell King,
	Thierry Reding, Sean Paul, linux-pwm, Pawel Moll, Ian Campbell,
	Rob Herring, moderated list:ARM/Mediatek SoC support,
	Yingjoe Chen, Matthias Brugger, Eddie Huang,
	linux-arm Mailing List, Rahul Sharma, srv_heupstream, lkml,
	Sascha Hauer, Kumar Gala, Andy Yan

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
  2019-04-16  6:04   ` Jitao Shi
  (?)
@ 2019-05-07  9:52     ` CK Hu
  -1 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-07  9:52 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
	Sascha Hauer, yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh,
	stonea168

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);




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
@ 2019-05-07  9:52     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-07  9:52 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel

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);

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
@ 2019-05-07  9:52     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-07  9:52 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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);




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
  2019-04-16  6:04   ` Jitao Shi
  (?)
@ 2019-05-08  2:39     ` CK Hu
  -1 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:39 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
	Sascha Hauer, yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh,
	stonea168

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,



^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-05-08  2:39     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:39 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	Russell King, Thierry Reding, linux-pwm, Sascha Hauer,
	Pawel Moll, Ian Campbell, Rob Herring, linux-mediatek, Andy Yan,
	Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Kumar Gala, Sean Paul

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-05-08  2:39     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:39 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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,



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 3/5] drm/mediatek: add dsi reg commit control
  2019-04-16  6:04   ` Jitao Shi
  (?)
@ 2019-05-08  2:56     ` CK Hu
  -1 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:56 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
	Sascha Hauer, yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh,
	stonea168

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);
>  



^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 3/5] drm/mediatek: add dsi reg commit control
@ 2019-05-08  2:56     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:56 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	Russell King, Thierry Reding, linux-pwm, Sascha Hauer,
	Pawel Moll, Ian Campbell, Rob Herring, linux-mediatek, Andy Yan,
	Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Kumar Gala, Sean Paul

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 3/5] drm/mediatek: add dsi reg commit control
@ 2019-05-08  2:56     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:56 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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);
>  



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 4/5] drm/mediatek: add frame size control
  2019-04-16  6:05   ` Jitao Shi
  (?)
@ 2019-05-08  2:59     ` CK Hu
  -1 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:59 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
	Sascha Hauer, yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh,
	stonea168

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)



^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 4/5] drm/mediatek: add frame size control
@ 2019-05-08  2:59     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:59 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	Russell King, Thierry Reding, linux-pwm, Sascha Hauer,
	Pawel Moll, Ian Campbell, Rob Herring, linux-mediatek, Andy Yan,
	Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Kumar Gala, Sean Paul

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 4/5] drm/mediatek: add frame size control
@ 2019-05-08  2:59     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  2:59 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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)



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 5/5] drm/mediatek: add mt8183 dsi driver support
  2019-04-16  6:05   ` Jitao Shi
  (?)
@ 2019-05-08  3:00     ` CK Hu
  -1 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  3:00 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
	Sascha Hauer, yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh,
	stonea168

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 },
>  	{ },
>  };
>  



^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 5/5] drm/mediatek: add mt8183 dsi driver support
@ 2019-05-08  3:00     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  3:00 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel

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 },
>  	{ },
>  };
>  

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 5/5] drm/mediatek: add mt8183 dsi driver support
@ 2019-05-08  3:00     ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-08  3:00 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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 },
>  	{ },
>  };
>  



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
  2019-05-08  2:39     ` CK Hu
@ 2019-05-19  9:33       ` Jitao Shi
  -1 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-05-19  9:33 UTC (permalink / raw)
  To: CK Hu
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Phi

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,
> 
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-05-19  9:33       ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-05-19  9:33 UTC (permalink / raw)
  To: CK Hu
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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,
> 
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
  2019-05-07  9:52     ` CK Hu
@ 2019-05-19  9:36       ` Jitao Shi
  -1 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-05-19  9:36 UTC (permalink / raw)
  To: CK Hu
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	Russell King, Thierry Reding, linux-pwm, Sascha Hauer,
	Pawel Moll, Ian Campbell, Rob Herring, linux-mediatek, Andy Yan,
	Matthias Brugger, eddie.huang, linux-arm-kernel, Rahul Sharma,
	srv_heupstream, linux-kernel, Kumar Gala, Sean Paul

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
@ 2019-05-19  9:36       ` Jitao Shi
  0 siblings, 0 replies; 49+ messages in thread
From: Jitao Shi @ 2019-05-19  9:36 UTC (permalink / raw)
  To: CK Hu
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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);
> 
> 
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
  2019-05-19  9:36       ` Jitao Shi
  (?)
@ 2019-05-20  7:06         ` CK Hu
  -1 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-20  7:06 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
	Sascha Hauer, yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh,
	stonea168

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);
> > 
> > 
> > 
> 
> 



^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
@ 2019-05-20  7:06         ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-20  7:06 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel

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);
> > 
> > 
> > 
> 
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe
@ 2019-05-20  7:06         ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-20  7:06 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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);
> > 
> > 
> > 
> 
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
  2019-05-19  9:33       ` Jitao Shi
  (?)
@ 2019-05-20  8:34         ` CK Hu
  -1 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-20  8:34 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel, linux-mediatek, srv_heupstream,
	Sascha Hauer, yingjoe.chen, eddie.huang, cawa.cheng, bibby.hsieh,
	stonea168

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,
> > 
> > 
> 
> 



^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-05-20  8:34         ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-20  8:34 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-pwm, David Airlie, Matthias Brugger, Thierry Reding,
	Ajay Kumar, Inki Dae, Rahul Sharma, Sean Paul, Vincent Palatin,
	Andy Yan, Philipp Zabel, Russell King, devicetree, linux-kernel,
	dri-devel, linux-arm-kernel

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,
> > 
> > 
> 
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
@ 2019-05-20  8:34         ` CK Hu
  0 siblings, 0 replies; 49+ messages in thread
From: CK Hu @ 2019-05-20  8:34 UTC (permalink / raw)
  To: Jitao Shi
  Cc: Mark Rutland, devicetree, David Airlie, stonea168, dri-devel,
	yingjoe.chen, Ajay Kumar, Vincent Palatin, cawa.cheng,
	bibby.hsieh, Russell King, Thierry Reding, linux-pwm,
	Sascha Hauer, Pawel Moll, Ian Campbell, Inki Dae, Rob Herring,
	linux-mediatek, Andy Yan, Matthias Brugger, eddie.huang,
	linux-arm-kernel, Rahul Sharma, srv_heupstream, linux-kernel,
	Philipp Zabel, Kumar Gala, Sean Paul

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,
> > 
> > 
> 
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 49+ messages in thread

end of thread, other threads:[~2019-05-20  8:34 UTC | newest]

Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16  6:04 [v2 0/5] support dsi for mt8183 Jitao Shi
2019-04-16  6:04 ` Jitao Shi
2019-04-16  6:04 ` Jitao Shi
2019-04-16  6:04 ` [v2 1/5] drm/mediatek: move mipi_dsi_host_register to probe Jitao Shi
2019-04-16  6:04   ` Jitao Shi
2019-04-16  6:04   ` Jitao Shi
2019-05-07  9:52   ` CK Hu
2019-05-07  9:52     ` CK Hu
2019-05-07  9:52     ` CK Hu
2019-05-19  9:36     ` Jitao Shi
2019-05-19  9:36       ` Jitao Shi
2019-05-20  7:06       ` CK Hu
2019-05-20  7:06         ` CK Hu
2019-05-20  7:06         ` CK Hu
2019-04-16  6:04 ` [v2 2/5] drm/mediatek: CMDQ reg address of mt8173 is different with mt2701 Jitao Shi
2019-04-16  6:04   ` Jitao Shi
2019-04-16  6:04   ` Jitao Shi
2019-04-17  4:58   ` kbuild test robot
2019-04-17  4:58     ` kbuild test robot
2019-04-17  4:58     ` kbuild test robot
2019-04-18  7:51   ` Nicolas Boichat
2019-04-18  7:51     ` Nicolas Boichat
2019-04-18  7:51     ` Nicolas Boichat
2019-05-08  2:39   ` CK Hu
2019-05-08  2:39     ` CK Hu
2019-05-08  2:39     ` CK Hu
2019-05-19  9:33     ` Jitao Shi
2019-05-19  9:33       ` Jitao Shi
2019-05-20  8:34       ` CK Hu
2019-05-20  8:34         ` CK Hu
2019-05-20  8:34         ` CK Hu
2019-04-16  6:04 ` [v2 3/5] drm/mediatek: add dsi reg commit control Jitao Shi
2019-04-16  6:04   ` Jitao Shi
2019-04-16  6:04   ` Jitao Shi
2019-05-08  2:56   ` CK Hu
2019-05-08  2:56     ` CK Hu
2019-05-08  2:56     ` CK Hu
2019-04-16  6:05 ` [v2 4/5] drm/mediatek: add frame size control Jitao Shi
2019-04-16  6:05   ` Jitao Shi
2019-04-16  6:05   ` Jitao Shi
2019-05-08  2:59   ` CK Hu
2019-05-08  2:59     ` CK Hu
2019-05-08  2:59     ` CK Hu
2019-04-16  6:05 ` [v2 5/5] drm/mediatek: add mt8183 dsi driver support Jitao Shi
2019-04-16  6:05   ` Jitao Shi
2019-04-16  6:05   ` Jitao Shi
2019-05-08  3:00   ` CK Hu
2019-05-08  3:00     ` CK Hu
2019-05-08  3:00     ` CK Hu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.