linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH V0 02/10] media: mtk-mdp3: fix redundant process done caused KE
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 03/10] media: mtk-mdp3: revise suspend strategy Moudy Ho
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

From: daoyuan huang <daoyuan.huang@mediatek.com>

In mdp_m2m_worker, there is m2m process done in error handle,
so add statement in mdp_cmdq_send for preventing redundant
process done in cmdq_pkt_flush return fail case.

Signed-off-by: daoyuan huang <daoyuan.huang@mediatek.com>
---
 drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
index ee81a3387cf9..f8bf8fde599c 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
@@ -454,8 +454,10 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 			mdp_dump_info(~0, 1);
 		}
 #endif
-		if (param->mdp_ctx)
-			mdp_m2m_job_finish(param->mdp_ctx);
+		if (!ret) { /* error handle in mdp_m2m_worker */
+			if (param->mdp_ctx)
+				mdp_m2m_job_finish(param->mdp_ctx);
+		}
 		cmdq_pkt_destroy(cmd.pkt);
 		for (i = 0; i < param->config->num_components; i++)
 			mdp_comp_clock_off(&mdp->pdev->dev, path.comps[i].comp);
-- 
2.18.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] 11+ messages in thread

* [RFC PATCH V0 03/10] media: mtk-mdp3: revise suspend strategy
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
  2021-06-23  7:35 ` [RFC PATCH V0 02/10] media: mtk-mdp3: fix redundant process done caused KE Moudy Ho
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 04/10] media: mtk-mdp3: add error handling in error return Moudy Ho
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

From: daoyuan huang <daoyuan.huang@mediatek.com>

1.The wait queue check for incomplete running task
  should be put in mdp_suspend instead of mdp_pm_suspend;
  Due to mdp_suspend could be called while task is running;
  Add error return value in suspend fail situation.
2.modify dma modules' power control from larb device to modules'
  device self;
  For support real mdp_pm_suspend callback.

Signed-off-by: daoyuan huang <daoyuan.huang@mediatek.com>
---
 .../media/platform/mtk-mdp3/mtk-mdp3-comp.c   | 36 ++++++++-----------
 .../media/platform/mtk-mdp3/mtk-mdp3-comp.h   |  2 +-
 .../media/platform/mtk-mdp3/mtk-mdp3-core.c   | 34 +++++-------------
 3 files changed, 23 insertions(+), 49 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
index 3033cd32340c..23dd413f3423 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
@@ -1039,8 +1039,8 @@ void mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp)
 {
 	int i, err;
 
-	if (comp->larb_dev) {
-		err = pm_runtime_get_sync(comp->larb_dev);
+	if (comp->comp_dev) {
+		err = pm_runtime_get_sync(comp->comp_dev);
 		if (err < 0)
 			dev_err(dev,
 				"Failed to get larb, err %d. type:%d id:%d\n",
@@ -1068,8 +1068,8 @@ void mdp_comp_clock_off(struct device *dev, struct mdp_comp *comp)
 		clk_disable_unprepare(comp->clks[i]);
 	}
 
-	if (comp->larb_dev)
-		pm_runtime_put(comp->larb_dev);
+	if (comp->comp_dev)
+		pm_runtime_put(comp->comp_dev);
 }
 
 static int mdp_get_subsys_id(struct device *dev, struct device_node *node,
@@ -1153,8 +1153,7 @@ static int mdp_comp_init(struct device *dev, struct mdp_dev *mdp,
 			 struct device_node *node, struct mdp_comp *comp,
 			 enum mdp_comp_id id)
 {
-	struct device_node *larb_node;
-	struct platform_device *larb_pdev;
+	struct platform_device *pdev;
 	int i;
 
 	if (id < 0 || id >= MDP_MAX_COMP_COUNT) {
@@ -1176,29 +1175,22 @@ static int mdp_comp_init(struct device *dev, struct mdp_dev *mdp,
 
 	mdp_get_subsys_id(dev, node, comp);
 
-	/* Only DMA capable components need the LARB property */
-	comp->larb_dev = NULL;
+	/* Only DMA capable components need the pm control */
+	comp->comp_dev = NULL;
 	if (comp->type != MDP_COMP_TYPE_RDMA &&
 	    comp->type != MDP_COMP_TYPE_WROT &&
 		comp->type != MDP_COMP_TYPE_WDMA)
 		return 0;
 
-	larb_node = of_parse_phandle(node, "mediatek,larb", 0);
-	if (!larb_node) {
-		dev_err(dev, "Missing mediatek,larb phandle in %pOF node\n",
-			node);
-		return -EINVAL;
-	}
-
-	larb_pdev = of_find_device_by_node(larb_node);
-	if (!larb_pdev) {
-		dev_warn(dev, "Waiting for larb device %pOF\n", larb_node);
-		of_node_put(larb_node);
-		return -EPROBE_DEFER;
+	pdev = of_find_device_by_node(node);
+	if (!pdev) {
+		dev_warn(dev, "can't find platform device of node:%s\n",
+			 node->name);
+		return -ENODEV;
 	}
-	of_node_put(larb_node);
 
-	comp->larb_dev = &larb_pdev->dev;
+	comp->comp_dev = &pdev->dev;
+	pm_runtime_enable(comp->comp_dev);
 
 	return 0;
 }
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
index 77d2c3e989c4..27d58dc51dce 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
@@ -113,7 +113,7 @@ struct mdp_comp {
 	phys_addr_t			reg_base;
 	u8				subsys_id;
 	struct clk			*clks[2];
-	struct device			*larb_dev;
+	struct device			*comp_dev;
 	enum mdp_comp_type		type;
 	enum mdp_comp_id		id;
 	u32				alias_id;
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
index 4a59dec8cfd9..eaf5b07e720f 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
@@ -152,7 +152,6 @@ static int mdp_probe(struct platform_device *pdev)
 #endif
 
 	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
-	pm_runtime_enable(dev);
 
 	ret = v4l2_device_register(dev, &mdp->v4l2_dev);
 	if (ret) {
@@ -212,7 +211,7 @@ static int mdp_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int __maybe_unused mdp_pm_suspend(struct device *dev)
+static int __maybe_unused mdp_suspend(struct device *dev)
 {
 	struct mdp_dev *mdp = dev_get_drvdata(dev);
 	int ret;
@@ -222,19 +221,19 @@ static int __maybe_unused mdp_pm_suspend(struct device *dev)
 	if (atomic_read(&mdp->job_count)) {
 		ret = wait_event_timeout(mdp->callback_wq,
 					 !atomic_read(&mdp->job_count),
-					 HZ);
-		if (ret == 0)
+					 2 * HZ);
+		if (ret == 0) {
 			dev_err(dev,
-				"%s:flushed cmdq task incomplete\n",
-				__func__);
-		else//need to remove
-			pr_err("%s:ret=%d\n", __func__, ret);
+				"%s:flushed cmdq task incomplete, count=%d\n",
+				__func__, atomic_read(&mdp->job_count));
+			return -EBUSY;
+		}
 	}
 
 	return 0;
 }
 
-static int __maybe_unused mdp_pm_resume(struct device *dev)
+static int __maybe_unused mdp_resume(struct device *dev)
 {
 	struct mdp_dev *mdp = dev_get_drvdata(dev);
 
@@ -243,25 +242,8 @@ static int __maybe_unused mdp_pm_resume(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused mdp_suspend(struct device *dev)
-{
-	if (pm_runtime_suspended(dev))
-		return 0;
-
-	return mdp_pm_suspend(dev);
-}
-
-static int __maybe_unused mdp_resume(struct device *dev)
-{
-	if (pm_runtime_suspended(dev))
-		return 0;
-
-	return mdp_pm_resume(dev);
-}
-
 static const struct dev_pm_ops mdp_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(mdp_suspend, mdp_resume)
-	SET_RUNTIME_PM_OPS(mdp_pm_suspend, mdp_pm_resume, NULL)
 };
 
 static struct platform_driver mdp_driver = {
-- 
2.18.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] 11+ messages in thread

* [RFC PATCH V0 04/10] media: mtk-mdp3: add error handling in error return
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
  2021-06-23  7:35 ` [RFC PATCH V0 02/10] media: mtk-mdp3: fix redundant process done caused KE Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 03/10] media: mtk-mdp3: revise suspend strategy Moudy Ho
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 05/10] media: mtk-mdp3: remove unnecessary Null check Moudy Ho
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

From: daoyuan huang <daoyuan.huang@mediatek.com>

add error handling in missed places.

Signed-off-by: daoyuan huang <daoyuan.huang@mediatek.com>
---
 drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c | 14 +++++++++++++-
 drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c | 16 ++++++++++++++++
 drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h |  2 ++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
index f8bf8fde599c..42a51061677a 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
@@ -398,7 +398,17 @@ static void mdp_handle_cmdq_callback(struct cmdq_cb_data data)
 
 	cmdq_pkt_destroy(cb_param->pkt);
 	INIT_WORK(&cb_param->auto_release_work, mdp_auto_release_work);
-	queue_work(mdp->clock_wq, &cb_param->auto_release_work);
+	if (!queue_work(mdp->clock_wq, &cb_param->auto_release_work)) {
+		mdp_err("%s:queue_work fail!\n", __func__);
+		mdp_comp_clocks_off(&mdp->pdev->dev, cb_param->comps,
+				    cb_param->num_comps);
+
+		kfree(cb_param->comps);
+		kfree(cb_param);
+
+		atomic_dec(&mdp->job_count);
+		wake_up(&mdp->callback_wq);
+	}
 }
 
 int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
@@ -495,6 +505,8 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 					   (void *)cb_param);
 		if (ret) {
 			mdp_err("%s:cmdq_pkt_flush_async fail!\n", __func__);
+			mdp_comp_clocks_off(&mdp->pdev->dev, cb_param->comps,
+					    cb_param->num_comps);
 			kfree(cb_param);
 			kfree(comps);
 		}
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
index 23dd413f3423..157ef9408ed3 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
@@ -1072,6 +1072,22 @@ void mdp_comp_clock_off(struct device *dev, struct mdp_comp *comp)
 		pm_runtime_put(comp->comp_dev);
 }
 
+void mdp_comp_clocks_on(struct device *dev, struct mdp_comp *comps, int num)
+{
+	int i;
+
+	for (i = 0; i < num; i++)
+		mdp_comp_clock_on(dev, &comps[i]);
+}
+
+void mdp_comp_clocks_off(struct device *dev, struct mdp_comp *comps, int num)
+{
+	int i;
+
+	for (i = 0; i < num; i++)
+		mdp_comp_clock_off(dev, &comps[i]);
+}
+
 static int mdp_get_subsys_id(struct device *dev, struct device_node *node,
 			     struct mdp_comp *comp)
 {
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
index 27d58dc51dce..f201bced3c06 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
@@ -147,6 +147,8 @@ int mdp_component_init(struct mdp_dev *mdp);
 void mdp_component_deinit(struct mdp_dev *mdp);
 void mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp);
 void mdp_comp_clock_off(struct device *dev, struct mdp_comp *comp);
+void mdp_comp_clocks_on(struct device *dev, struct mdp_comp *comps, int num);
+void mdp_comp_clocks_off(struct device *dev, struct mdp_comp *comps, int num);
 int mdp_comp_ctx_init(struct mdp_dev *mdp, struct mdp_comp_ctx *ctx,
 		      const struct img_compparam *param,
 	const struct img_ipi_frameparam *frame);
-- 
2.18.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] 11+ messages in thread

* [RFC PATCH V0 05/10] media: mtk-mdp3: remove unnecessary Null check
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
                   ` (2 preceding siblings ...)
  2021-06-23  7:35 ` [RFC PATCH V0 04/10] media: mtk-mdp3: add error handling in error return Moudy Ho
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 06/10] media: mtk-mdp3: move clock on to precise place Moudy Ho
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

From: daoyuan huang <daoyuan.huang@mediatek.com>

Remove logically unnecessary NULL check in call back function.

Due to no NULL callback param should occur in normal case;
and if we use NULL check to jump over this clock off step,
the clock ref. count may go asymmetric.

Signed-off-by: daoyuan huang <daoyuan.huang@mediatek.com>
---
 drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
index 42a51061677a..df956ca3685c 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
@@ -349,13 +349,8 @@ static void mdp_auto_release_work(struct work_struct *work)
 				auto_release_work);
 	mdp = cb_param->mdp;
 
-	if (cb_param->comps && cb_param->num_comps) {
-		int i;
-
-		for (i = 0; i < cb_param->num_comps; i++)
-			mdp_comp_clock_off(&mdp->pdev->dev,
-					   &cb_param->comps[i]);
-	}
+	mdp_comp_clocks_off(&mdp->pdev->dev, cb_param->comps,
+			    cb_param->num_comps);
 
 	kfree(cb_param->comps);
 	kfree(cb_param);
-- 
2.18.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] 11+ messages in thread

* [RFC PATCH V0 06/10] media: mtk-mdp3: move clock on to precise place
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
                   ` (3 preceding siblings ...)
  2021-06-23  7:35 ` [RFC PATCH V0 05/10] media: mtk-mdp3: remove unnecessary Null check Moudy Ho
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 07/10] media: mtk-mdp3: Fix unpaired settings Moudy Ho
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

From: daoyuan huang <daoyuan.huang@mediatek.com>

Move clock on to the previous line of cmdq_pkt_flush.

Due to there're two ways(sync/async) to submit CMDQ task,
and for async case we need to prepare callback param struct along
with error checks and error handlings before submit,
if we move clock on after these error check steps,
we can save some clock off code in error handling.

Signed-off-by: daoyuan huang <daoyuan.huang@mediatek.com>
---
 drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
index df956ca3685c..a1bdf92d45f7 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
@@ -445,11 +445,9 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 		return ret;
 	}
 
-	// TODO: engine conflict dispatch
-	for (i = 0; i < param->config->num_components; i++)
-		mdp_comp_clock_on(&mdp->pdev->dev, path.comps[i].comp);
-
 	if (param->wait) {
+		for (i = 0; i < param->config->num_components; i++)
+			mdp_comp_clock_on(&mdp->pdev->dev, path.comps[i].comp);
 		ret = cmdq_pkt_flush(cmd.pkt);
 #ifdef MDP_DEBUG
 		if (ret) {
@@ -495,6 +493,9 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 		cb_param->num_comps = param->config->num_components;
 		cb_param->mdp_ctx = param->mdp_ctx;
 
+		mdp_comp_clocks_on(&mdp->pdev->dev, cb_param->comps,
+				   cb_param->num_comps);
+
 		ret = cmdq_pkt_flush_async(cmd.pkt,
 					   mdp_handle_cmdq_callback,
 					   (void *)cb_param);
-- 
2.18.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] 11+ messages in thread

* [RFC PATCH V0 07/10] media: mtk-mdp3: Fix unpaired settings
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
                   ` (4 preceding siblings ...)
  2021-06-23  7:35 ` [RFC PATCH V0 06/10] media: mtk-mdp3: move clock on to precise place Moudy Ho
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 08/10] media: mtk-mdp3: remove illegal device node usage Moudy Ho
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

Fix unpaired settings about:
1. clk on/off.
2. CMDQ packet kzalloc/kfree.
3. MDP job_count increase/decrease.

Also revise the redundant log.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 .../media/platform/mtk-mdp3/mtk-mdp3-cmdq.c   | 97 ++++++++++++-------
 .../media/platform/mtk-mdp3/mtk-mdp3-core.h   |  4 +-
 .../media/platform/mtk-mdp3/mtk-mdp3-m2m.c    |  2 +-
 3 files changed, 67 insertions(+), 36 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
index a1bdf92d45f7..eac10944283a 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include "mtk-mdp3-cmdq.h"
 #include "mtk-mdp3-comp.h"
 #include "mtk-mdp3-core.h"
@@ -230,6 +231,25 @@ static int mdp_path_subfrm_run(const struct mdp_path_subfrm *subfrm,
 	return 0;
 }
 
+static int mdp_path_ctx_init(struct mdp_dev *mdp, struct mdp_path *path)
+{
+	const struct img_config *config = path->config;
+	int index, ret;
+
+	if (config->num_components < 1)
+	    return -EINVAL;
+
+	for (index = 0; index < config->num_components; index++) {
+		ret = mdp_comp_ctx_init(mdp, &path->comps[index],
+					&config->components[index],
+					path->param);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int mdp_path_config_subfrm(struct mdp_cmd *cmd, struct mdp_path *path,
 				  u32 count)
 {
@@ -297,14 +317,6 @@ static int mdp_path_config(struct mdp_dev *mdp, struct mdp_cmd *cmd,
 	struct mdp_comp_ctx *ctx;
 	int index, count, ret;
 
-	for (index = 0; index < config->num_components; index++) {
-		ret = mdp_comp_ctx_init(mdp, &path->comps[index],
-					&config->components[index],
-					path->param);
-		if (ret)
-			return ret;
-	}
-
 	/* Config path frame */
 	/* Reset components */
 	for (index = 0; index < config->num_components; index++) {
@@ -410,6 +422,8 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 {
 	struct mdp_cmd cmd;
 	struct mdp_path path;
+	struct mdp_cmdq_cb_param *cb_param = NULL;
+	struct mdp_comp *comps = NULL;
 	int i, ret;
 
 	if (atomic_read(&mdp->suspended))
@@ -438,16 +452,23 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 		path.composes[i] = param->composes[i] ?
 			param->composes[i] : &path.bounds[i];
 	}
+
+	ret = mdp_path_ctx_init(mdp, &path);
+	if (ret) {
+		pr_info("%s mdp_path_ctx_init error\n", __func__);
+		goto err_destory_pkt;
+	}
+
+	for (i = 0; i < param->config->num_components; i++)
+			mdp_comp_clock_on(&mdp->pdev->dev, path.comps[i].comp);
+
 	ret = mdp_path_config(mdp, &cmd, &path);
 	if (ret) {
-		atomic_dec(&mdp->job_count);
-		wake_up(&mdp->callback_wq);
-		return ret;
+		pr_info("%s mdp_path_config error\n", __func__);
+		goto err_destory_pkt;
 	}
 
 	if (param->wait) {
-		for (i = 0; i < param->config->num_components; i++)
-			mdp_comp_clock_on(&mdp->pdev->dev, path.comps[i].comp);
 		ret = cmdq_pkt_flush(cmd.pkt);
 #ifdef MDP_DEBUG
 		if (ret) {
@@ -461,25 +482,20 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 			if (param->mdp_ctx)
 				mdp_m2m_job_finish(param->mdp_ctx);
 		}
-		cmdq_pkt_destroy(cmd.pkt);
-		for (i = 0; i < param->config->num_components; i++)
-			mdp_comp_clock_off(&mdp->pdev->dev, path.comps[i].comp);
-
-		atomic_dec(&mdp->job_count);
-		wake_up(&mdp->callback_wq);
+		goto err_clock_off;
 	} else {
-		struct mdp_cmdq_cb_param *cb_param;
-		struct mdp_comp *comps;
-
 		cb_param = kzalloc(sizeof(*cb_param), GFP_KERNEL);
-		if (!cb_param)
-			return -ENOMEM;
+		if (!cb_param) {
+			ret = -ENOMEM;
+			goto err_destory_pkt;
+		}
+
 		comps = kcalloc(param->config->num_components, sizeof(*comps),
 				GFP_KERNEL);
 		if (!comps) {
-			kfree(cb_param);
 			mdp_err("%s:comps alloc fail!\n", __func__);
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_destory_pkt;
 		}
 
 		for (i = 0; i < param->config->num_components; i++)
@@ -493,20 +509,35 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 		cb_param->num_comps = param->config->num_components;
 		cb_param->mdp_ctx = param->mdp_ctx;
 
-		mdp_comp_clocks_on(&mdp->pdev->dev, cb_param->comps,
-				   cb_param->num_comps);
-
+		cmdq_pkt_finalize(cmd.pkt);
 		ret = cmdq_pkt_flush_async(cmd.pkt,
 					   mdp_handle_cmdq_callback,
 					   (void *)cb_param);
 		if (ret) {
 			mdp_err("%s:cmdq_pkt_flush_async fail!\n", __func__);
-			mdp_comp_clocks_off(&mdp->pdev->dev, cb_param->comps,
-					    cb_param->num_comps);
-			kfree(cb_param);
-			kfree(comps);
+			goto err_clock_off;
 		}
 	}
+	return 0;
+
+err_clock_off:
+	if (param->wait) {
+		for (i = 0; i < param->config->num_components; i++)
+			mdp_comp_clock_off(&mdp->pdev->dev, path.comps[i].comp);
+	} else {
+		mdp_comp_clocks_off(&mdp->pdev->dev, cb_param->comps,
+					    cb_param->num_comps);
+	}
+err_destory_pkt:
+	cmdq_pkt_destroy(cmd.pkt);
+	atomic_dec(&mdp->job_count);
+	if (param->wait)
+		wake_up(&mdp->callback_wq);
+	if (comps)
+		kfree(comps);
+	if (cb_param)
+		kfree(cb_param);
+
 	return ret;
 }
 
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.h b/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.h
index 98cf54b1d92b..19f46da487aa 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.h
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.h
@@ -64,12 +64,12 @@ extern int mtk_mdp_debug;
 #define mdp_dbg(level, fmt, ...)\
 	do {\
 		if (mtk_mdp_debug >= (level))\
-			pr_info("[MTK-MDP3] %d %s:%d: " fmt "\n",\
+			pr_info("[MTK-MDP3] %d %s:%d: " fmt,\
 				level, __func__, __LINE__, ##__VA_ARGS__);\
 	} while (0)
 
 #define mdp_err(fmt, ...)\
-	pr_err("[MTK-MDP3][ERR] %s:%d: " fmt "\n", __func__, __LINE__,\
+	pr_err("[MTK-MDP3][ERR] %s:%d: " fmt, __func__, __LINE__,\
 		##__VA_ARGS__)
 
 #else
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c
index 1ae499f6c84b..ff80f01be82f 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c
@@ -133,7 +133,7 @@ static void mdp_m2m_worker(struct work_struct *work)
 	task.config = ctx->vpu.config;
 	task.param = &param;
 	task.composes[0] = &frame->compose;
-	task.wait = 1;
+	task.wait = 0;
 	task.cmdq_cb = NULL;
 	task.cb_data = NULL;
 	task.mdp_ctx = ctx;
-- 
2.18.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] 11+ messages in thread

* [RFC PATCH V0 08/10] media: mtk-mdp3: remove illegal device node usage
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
                   ` (5 preceding siblings ...)
  2021-06-23  7:35 ` [RFC PATCH V0 07/10] media: mtk-mdp3: Fix unpaired settings Moudy Ho
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 09/10] media: mtk-mdp3: revise error handling about get/probe MDP3 Moudy Ho
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

This patch is used for review before send upstream patch.
From Rob Herring's review comment: device node can't use same
register address as other nodes'.
Remove illegal device nodes in device tree.
MDP's sub component init need to do corresponding modification.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 .../media/platform/mtk-mdp3/mtk-mdp3-comp.c   | 224 +++++++++++++-----
 .../media/platform/mtk-mdp3/mtk-mdp3-comp.h   |   2 +-
 2 files changed, 163 insertions(+), 63 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
index 157ef9408ed3..e89fd02bb556 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
@@ -1016,23 +1016,15 @@ static const struct of_device_id mdp_comp_dt_ids[] = {
 	{}
 };
 
-static int mdp_comp_get_id(struct device *dev, struct device_node *node,
-			   enum mdp_comp_type type)
+static int mdp_comp_get_id(enum mdp_comp_type type, u32 alias_id)
 {
-	u32 alias_id;
-	int i, ret;
-
-	ret = of_property_read_u32(node, "mediatek,mdp-id", &alias_id);
-	if (ret)
-		return ret;
+	int i;
 
 	for (i = 0; i < ARRAY_SIZE(mdp_comp_matches); i++)
 		if (mdp_comp_matches[i].type == type &&
 		    mdp_comp_matches[i].alias_id == alias_id)
 			return i;
-
-	dev_err(dev, "Failed to get id. type: %d, alias: %d\n", type, alias_id);
-	return -EINVAL;
+	return -ENODEV;
 }
 
 void mdp_comp_clock_on(struct device *dev, struct mdp_comp *comp)
@@ -1143,33 +1135,10 @@ static void __mdp_comp_init(struct mdp_dev *mdp, struct device_node *node,
 	comp->reg_base = base;
 }
 
-static int mdp_mm_init(struct mdp_dev *mdp,
-		       struct mdp_comp *comp, const char *ref_name)
+static int mdp_comp_init(struct mdp_dev *mdp, struct device_node *node,
+			 struct mdp_comp *comp, enum mdp_comp_id id)
 {
-	struct device_node *node;
 	struct device *dev = &mdp->pdev->dev;
-
-	node = of_parse_phandle(dev->of_node, ref_name, 0);
-	if (!node) {
-		dev_err(dev, "Failed to parse dt %s\n", ref_name);
-		return -EINVAL;
-	}
-
-	__mdp_comp_init(mdp, node, comp);
-	mdp_get_subsys_id(dev, node, comp);
-	of_node_put(node);
-	if (!comp->reg_base) {
-		dev_err(dev, "Failed to init %s base\n", ref_name);
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static int mdp_comp_init(struct device *dev, struct mdp_dev *mdp,
-			 struct device_node *node, struct mdp_comp *comp,
-			 enum mdp_comp_id id)
-{
-	struct platform_device *pdev;
 	int i;
 
 	if (id < 0 || id >= MDP_MAX_COMP_COUNT) {
@@ -1191,32 +1160,140 @@ static int mdp_comp_init(struct device *dev, struct mdp_dev *mdp,
 
 	mdp_get_subsys_id(dev, node, comp);
 
-	/* Only DMA capable components need the pm control */
-	comp->comp_dev = NULL;
-	if (comp->type != MDP_COMP_TYPE_RDMA &&
-	    comp->type != MDP_COMP_TYPE_WROT &&
-		comp->type != MDP_COMP_TYPE_WDMA)
-		return 0;
+	return 0;
+}
 
-	pdev = of_find_device_by_node(node);
-	if (!pdev) {
-		dev_warn(dev, "can't find platform device of node:%s\n",
-			 node->name);
-		return -ENODEV;
+static struct mdp_comp *mdp_comp_create(struct mdp_dev *mdp,
+					struct device_node *node,
+					enum mdp_comp_id id)
+{
+	struct device *dev = &mdp->pdev->dev;
+	struct mdp_comp *comp;
+	int ret;
+
+	if (mdp->comp[id])
+		return ERR_PTR(-EEXIST);
+
+	comp = devm_kzalloc(dev, sizeof(*comp), GFP_KERNEL);
+	if (!comp)
+		return ERR_PTR(-ENOMEM);
+
+	ret = mdp_comp_init(mdp, node, comp, id);
+	if (ret) {
+		kfree(comp);
+		return ERR_PTR(ret);
 	}
+	mdp->comp[id] = comp;
+
+	dev_info(dev, "%s type:%d alias:%d id:%d base:%#x regs:%p\n",
+		dev->of_node->name, comp->type, comp->alias_id, id,
+		(u32)comp->reg_base, comp->regs);
+	return comp;
+}
 
-	comp->comp_dev = &pdev->dev;
-	pm_runtime_enable(comp->comp_dev);
+static int mdp_sub_comps_create(struct mdp_dev *mdp, struct device_node *node)
+{
+	struct device *dev = &mdp->pdev->dev;
+	struct property *prop;
+	const char *name;
+	int index = 0;
+
+	of_property_for_each_string(node, "mdp-comps", prop, name) {
+		const struct of_device_id *matches = mdp_comp_dt_ids;
+		enum mdp_comp_type type = MDP_COMP_NONE;
+		u32 alias_id;
+		int id, ret;
+		struct mdp_comp *comp;
 
+		for (; matches->compatible[0]; matches++) {
+			if (of_compat_cmp(name, matches->compatible,
+				strlen(matches->compatible)) == 0) {
+				type = (enum mdp_comp_type)matches->data;
+				break;
+			}
+		}
+
+		ret = of_property_read_u32_index(node, "mdp-comp-ids",
+			index, &alias_id);
+		if (ret) {
+			dev_warn(dev, "Skipping unknown component %s\n", name);
+			return ret;
+		}
+
+		id = mdp_comp_get_id(type, alias_id);
+		if (id < 0) {
+			dev_err(dev, "Failed to get component id: "
+				"%s type %d, alias %d\n", name, type, alias_id);
+			return -ENODEV;
+		}
+
+		comp = mdp_comp_create(mdp, node, id);
+		if (IS_ERR(comp))
+			return PTR_ERR(comp);
+
+		index++;
+	}
 	return 0;
 }
 
 static void mdp_comp_deinit(struct mdp_comp *comp)
 {
-	iounmap(comp->regs);
+	if (!comp)
+		return;
+
+	if (comp->regs)
+		iounmap(comp->regs);
 	/* of_node_put(comp->dev_node); */
 }
 
+static int mdp_imgi_init(struct mdp_dev *mdp, const char *ref_name)
+{
+	struct device_node *node;
+	struct device *dev = &mdp->pdev->dev;
+	int ret;
+
+	node = of_parse_phandle(dev->of_node, ref_name, 0);
+	if (!node) {
+		dev_err(dev, "Failed to parse dt %s\n", ref_name);
+		return -EINVAL;
+	}
+
+	ret = mdp_sub_comps_create(mdp, node);
+	of_node_put(node);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int mdp_mm_init(struct mdp_dev *mdp, struct mdp_comp *comp,
+		       const char *ref_name)
+{
+	struct device_node *node;
+	struct device *dev = &mdp->pdev->dev;
+	int ret;
+
+	node = of_parse_phandle(dev->of_node, ref_name, 0);
+	if (!node) {
+		dev_err(dev, "Failed to parse dt %s\n", ref_name);
+		return -EINVAL;
+	}
+
+	__mdp_comp_init(mdp, node, comp);
+	mdp_get_subsys_id(dev, node, comp);
+	if (!comp->reg_base) {
+		dev_err(dev, "Failed to init %s base\n", ref_name);
+		of_node_put(node);
+		return -EINVAL;
+	}
+
+	ret = mdp_sub_comps_create(mdp, node);
+	of_node_put(node);
+	if (ret)
+		return ret;
+	return 0;
+}
+
 void mdp_component_deinit(struct mdp_dev *mdp)
 {
 	int i;
@@ -1235,6 +1312,8 @@ int mdp_component_init(struct mdp_dev *mdp)
 {
 	struct device *dev = &mdp->pdev->dev;
 	struct device_node *node, *parent;
+	struct platform_device *pdev;
+	u32 alias_id;
 	int i, ret;
 
 	for (i = RDMA0_SOF; i < MDP_MAX_EVENT_COUNT; i++) {
@@ -1260,7 +1339,11 @@ int mdp_component_init(struct mdp_dev *mdp)
 
 	ret = mdp_mm_init(mdp, &mdp->mm_mutex, "mediatek,mm-mutex");
 	if (ret)
-		goto err_init_mm;
+		goto err_init_comps;
+
+	ret = mdp_imgi_init(mdp, "mediatek,imgsys");
+	if (ret)
+		goto err_init_comps;
 
 	parent = dev->of_node->parent;
 	/* Iterate over sibling MDP function blocks */
@@ -1281,27 +1364,44 @@ int mdp_component_init(struct mdp_dev *mdp)
 		}
 
 		type = (enum mdp_comp_type)of_id->data;
-		id = mdp_comp_get_id(dev, node, type);
-		if (id < 0) {
+		ret = of_property_read_u32(node, "mediatek,mdp-id", &alias_id);
+		if (ret) {
 			dev_warn(dev, "Skipping unknown component %pOF\n",
 				 node);
 			continue;
 		}
+		id = mdp_comp_get_id(type, alias_id);
+		if (id < 0) {
+			dev_err(dev,
+				"Fail to get component id: type %d alias %d\n",
+				type, alias_id);
+			continue;
+		}
 
-		comp = devm_kzalloc(dev, sizeof(*comp), GFP_KERNEL);
-		if (!comp) {
-			ret = -ENOMEM;
+		comp = mdp_comp_create(mdp, node, id);
+		if (IS_ERR(comp))
 			goto err_init_comps;
-		}
-		mdp->comp[id] = comp;
 
-		ret = mdp_comp_init(dev, mdp, node, comp, id);
+		ret = mdp_sub_comps_create(mdp, node);
 		if (ret)
 			goto err_init_comps;
 
-		dev_info(dev, "%s type:%d alias:%d id:%d base:%#x regs:%p\n",
-			 of_id->compatible, type, comp->alias_id, id,
-			(u32)comp->reg_base, comp->regs);
+		/* Only DMA capable components need the pm control */
+		comp->comp_dev = NULL;
+		if (comp->type != MDP_COMP_TYPE_RDMA &&
+		    comp->type != MDP_COMP_TYPE_WROT &&
+			comp->type != MDP_COMP_TYPE_WDMA)
+			continue;
+
+		pdev = of_find_device_by_node(node);
+		if (!pdev) {
+			dev_warn(dev, "can't find platform device of node:%s\n",
+				 node->name);
+			return -ENODEV;
+		}
+
+		comp->comp_dev = &pdev->dev;
+		pm_runtime_enable(comp->comp_dev);
 	}
 	return 0;
 
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
index f201bced3c06..f5d514a51236 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
@@ -112,7 +112,7 @@ struct mdp_comp {
 	void __iomem			*regs;
 	phys_addr_t			reg_base;
 	u8				subsys_id;
-	struct clk			*clks[2];
+	struct clk			*clks[4];
 	struct device			*comp_dev;
 	enum mdp_comp_type		type;
 	enum mdp_comp_id		id;
-- 
2.18.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] 11+ messages in thread

* [RFC PATCH V0 09/10] media: mtk-mdp3: revise error handling about get/probe MDP3
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
                   ` (6 preceding siblings ...)
  2021-06-23  7:35 ` [RFC PATCH V0 08/10] media: mtk-mdp3: remove illegal device node usage Moudy Ho
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23  7:35 ` [RFC PATCH V0 10/10] media: mtk-mdp3: Adjust related settings for 5.13-rc1 Moudy Ho
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

This patch is used for review before send upstream patch.
From Alexandre Courbot's review comment: Independent from the main patch.
Revise error handling about get/probe MDP3 driver to make it stable.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
index eaf5b07e720f..207b55ace97b 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
@@ -40,9 +40,9 @@ struct platform_device *mdp_get_plat_device(struct platform_device *pdev)
 	}
 
 	mdp_pdev = of_find_device_by_node(mdp_node);
+	of_node_put(mdp_node);
 	if (WARN_ON(!mdp_pdev)) {
 		dev_err(dev, "mdp pdev failed\n");
-		of_node_put(mdp_node);
 		return NULL;
 	}
 
@@ -113,7 +113,7 @@ static int mdp_probe(struct platform_device *pdev)
 	if (!mdp->job_wq) {
 		dev_err(dev, "Unable to create job workqueue\n");
 		ret = -ENOMEM;
-		goto err_destroy_job_wq;
+		goto err_deinit_comp;
 	}
 
 	mdp->clock_wq = alloc_workqueue(MDP_MODULE_NAME "-clock", WQ_FREEZABLE,
@@ -121,7 +121,7 @@ static int mdp_probe(struct platform_device *pdev)
 	if (!mdp->clock_wq) {
 		dev_err(dev, "Unable to create clock workqueue\n");
 		ret = -ENOMEM;
-		goto err_destroy_clock_wq;
+		goto err_destroy_job_wq;
 	}
 
 	mdp->scp = scp_get(pdev);
@@ -179,6 +179,8 @@ static int mdp_probe(struct platform_device *pdev)
 	destroy_workqueue(mdp->clock_wq);
 err_destroy_job_wq:
 	destroy_workqueue(mdp->job_wq);
+err_deinit_comp:
+		mdp_component_deinit(mdp);
 err_return:
 	dev_dbg(dev, "Errno %d\n", ret);
 	return ret;
-- 
2.18.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] 11+ messages in thread

* [RFC PATCH V0 10/10] media: mtk-mdp3: Adjust related settings for 5.13-rc1
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
                   ` (7 preceding siblings ...)
  2021-06-23  7:35 ` [RFC PATCH V0 09/10] media: mtk-mdp3: revise error handling about get/probe MDP3 Moudy Ho
@ 2021-06-23  7:35 ` Moudy Ho
  2021-06-23 10:41 ` [RFC PATCH V0 01/10] media: mtk-mdp3: Add Mediatek MDP Driver Chun-Kuang Hu
  2021-07-02 12:00 ` Enric Balletbo Serra
  10 siblings, 0 replies; 11+ messages in thread
From: Moudy Ho @ 2021-06-23  7:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec
  Cc: Maoguang Meng, Krzysztof Kozlowski, daoyuan huang, Ping-Hsun Wu,
	Geert Uytterhoeven, Rob Landley, Laurent Pinchart, linux-media,
	devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
	tfiga, drinkcat, acourbot, pihsun, menghui.lin, sj.huang,
	ben.lok, randy.wu, moudy.ho, srv_heupstream, frederic.chen

From: mtk18742 <moudy.ho@mediatek.com>

1. remove cmdq_pkt_flush usage.
2. remove VFL_TYPE_GRABBER enum.
3. remove undefined format
4. Adjust CMDQ API param.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 .../media/platform/mtk-mdp3/mmsys_reg_base.h  |  2 +-
 .../media/platform/mtk-mdp3/mtk-mdp3-cmdq.c   | 92 +++++++------------
 .../media/platform/mtk-mdp3/mtk-mdp3-cmdq.h   |  3 +-
 .../media/platform/mtk-mdp3/mtk-mdp3-core.c   |  2 +-
 .../media/platform/mtk-mdp3/mtk-mdp3-m2m.c    |  3 +-
 .../media/platform/mtk-mdp3/mtk-mdp3-regs.c   |  2 +-
 6 files changed, 39 insertions(+), 65 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp3/mmsys_reg_base.h b/drivers/media/platform/mtk-mdp3/mmsys_reg_base.h
index d79b82eea61b..738ecd525474 100644
--- a/drivers/media/platform/mtk-mdp3/mmsys_reg_base.h
+++ b/drivers/media/platform/mtk-mdp3/mmsys_reg_base.h
@@ -16,7 +16,7 @@
 			(0xffffffff) : (mask), ##__VA_ARGS__)
 
 #define MM_REG_WAIT(cmd, evt) \
-	cmdq_pkt_wfe(cmd->pkt, cmd->event[(evt)])
+	cmdq_pkt_wfe(cmd->pkt, cmd->event[(evt)], true)
 
 #define MM_REG_WAIT_NO_CLEAR(cmd, evt) \
 	cmdq_pkt_wait_no_clear(cmd->pkt, cmd->event[(evt)])
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
index eac10944283a..151485933eae 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
@@ -468,71 +468,48 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 		goto err_destory_pkt;
 	}
 
-	if (param->wait) {
-		ret = cmdq_pkt_flush(cmd.pkt);
-#ifdef MDP_DEBUG
-		if (ret) {
-			struct mdp_func_struct *p_func = mdp_get_func();
-
-			p_func->mdp_dump_mmsys_config();
-			mdp_dump_info(~0, 1);
-		}
-#endif
-		if (!ret) { /* error handle in mdp_m2m_worker */
-			if (param->mdp_ctx)
-				mdp_m2m_job_finish(param->mdp_ctx);
-		}
-		goto err_clock_off;
-	} else {
-		cb_param = kzalloc(sizeof(*cb_param), GFP_KERNEL);
-		if (!cb_param) {
-			ret = -ENOMEM;
-			goto err_destory_pkt;
-		}
+	cb_param = kzalloc(sizeof(*cb_param), GFP_KERNEL);
+	if (!cb_param) {
+		ret = -ENOMEM;
+		goto err_destory_pkt;
+	}
 
-		comps = kcalloc(param->config->num_components, sizeof(*comps),
-				GFP_KERNEL);
-		if (!comps) {
-			mdp_err("%s:comps alloc fail!\n", __func__);
-			ret = -ENOMEM;
-			goto err_destory_pkt;
-		}
+	comps = kcalloc(param->config->num_components, sizeof(*comps),
+			GFP_KERNEL);
+	if (!comps) {
+		mdp_err("%s:comps alloc fail!\n", __func__);
+		ret = -ENOMEM;
+		goto err_destory_pkt;
+	}
 
-		for (i = 0; i < param->config->num_components; i++)
-			memcpy(&comps[i], path.comps[i].comp,
-			       sizeof(struct mdp_comp));
-		cb_param->mdp = mdp;
-		cb_param->user_cmdq_cb = param->cmdq_cb;
-		cb_param->user_cb_data = param->cb_data;
-		cb_param->pkt = cmd.pkt;
-		cb_param->comps = comps;
-		cb_param->num_comps = param->config->num_components;
-		cb_param->mdp_ctx = param->mdp_ctx;
-
-		cmdq_pkt_finalize(cmd.pkt);
-		ret = cmdq_pkt_flush_async(cmd.pkt,
-					   mdp_handle_cmdq_callback,
-					   (void *)cb_param);
-		if (ret) {
-			mdp_err("%s:cmdq_pkt_flush_async fail!\n", __func__);
-			goto err_clock_off;
-		}
+	for (i = 0; i < param->config->num_components; i++)
+		memcpy(&comps[i], path.comps[i].comp,
+		       sizeof(struct mdp_comp));
+	cb_param->mdp = mdp;
+	cb_param->user_cmdq_cb = param->cmdq_cb;
+	cb_param->user_cb_data = param->cb_data;
+	cb_param->pkt = cmd.pkt;
+	cb_param->comps = comps;
+	cb_param->num_comps = param->config->num_components;
+	cb_param->mdp_ctx = param->mdp_ctx;
+
+	cmdq_pkt_finalize(cmd.pkt);
+	ret = cmdq_pkt_flush_async(cmd.pkt,
+				   mdp_handle_cmdq_callback,
+				   (void *)cb_param);
+	if (ret) {
+		mdp_err("%s:cmdq_pkt_flush_async fail!\n", __func__);
+		goto err_clock_off;
 	}
 	return 0;
 
 err_clock_off:
-	if (param->wait) {
-		for (i = 0; i < param->config->num_components; i++)
-			mdp_comp_clock_off(&mdp->pdev->dev, path.comps[i].comp);
-	} else {
-		mdp_comp_clocks_off(&mdp->pdev->dev, cb_param->comps,
-					    cb_param->num_comps);
-	}
+	mdp_comp_clocks_off(&mdp->pdev->dev, cb_param->comps,
+				    cb_param->num_comps);
 err_destory_pkt:
 	cmdq_pkt_destroy(cmd.pkt);
 	atomic_dec(&mdp->job_count);
-	if (param->wait)
-		wake_up(&mdp->callback_wq);
+	wake_up(&mdp->callback_wq);
 	if (comps)
 		kfree(comps);
 	if (cb_param)
@@ -543,7 +520,7 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
 
 int mdp_cmdq_sendtask(struct platform_device *pdev, struct img_config *config,
 		      struct img_ipi_frameparam *param,
-		      struct v4l2_rect *compose, unsigned int wait,
+		      struct v4l2_rect *compose,
 		      void (*cmdq_cb)(struct cmdq_cb_data data), void *cb_data)
 {
 	struct mdp_dev *mdp = platform_get_drvdata(pdev);
@@ -551,7 +528,6 @@ int mdp_cmdq_sendtask(struct platform_device *pdev, struct img_config *config,
 		.config = config,
 		.param = param,
 		.composes[0] = compose,
-		.wait = wait,
 		.cmdq_cb = cmdq_cb,
 		.cb_data = cb_data,
 	};
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.h b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.h
index 6b8b0f6b4bb5..f6394d3d0358 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.h
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.h
@@ -16,7 +16,7 @@ struct platform_device *mdp_get_plat_device(struct platform_device *pdev);
 
 int mdp_cmdq_sendtask(struct platform_device *pdev, struct img_config *config,
 		      struct img_ipi_frameparam *param,
-		      struct v4l2_rect *compose, unsigned int wait,
+		      struct v4l2_rect *compose,
 		      void (*cmdq_cb)(struct cmdq_cb_data data), void *cb_data);
 
 struct mdp_cmd {
@@ -28,7 +28,6 @@ struct mdp_cmdq_param {
 	struct img_config *config;
 	struct img_ipi_frameparam *param;
 	const struct v4l2_rect *composes[IMG_MAX_HW_OUTPUTS];
-	unsigned int wait;
 
 	void (*cmdq_cb)(struct cmdq_cb_data data);
 	void *cb_data;
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
index 207b55ace97b..a42e436d3a8c 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
@@ -137,7 +137,7 @@ static int mdp_probe(struct platform_device *pdev)
 	mutex_init(&mdp->vpu_lock);
 	mutex_init(&mdp->m2m_lock);
 
-	mdp->cmdq_clt = cmdq_mbox_create(dev, 0, 1200);
+	mdp->cmdq_clt = cmdq_mbox_create(dev, 0);
 	if (IS_ERR(mdp->cmdq_clt)) {
 		ret = PTR_ERR(mdp->cmdq_clt);
 		goto err_put_scp;
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c
index ff80f01be82f..536574f6bc32 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c
@@ -133,7 +133,6 @@ static void mdp_m2m_worker(struct work_struct *work)
 	task.config = ctx->vpu.config;
 	task.param = &param;
 	task.composes[0] = &frame->compose;
-	task.wait = 0;
 	task.cmdq_cb = NULL;
 	task.cb_data = NULL;
 	task.mdp_ctx = ctx;
@@ -769,7 +768,7 @@ int mdp_m2m_device_register(struct mdp_dev *mdp)
 		goto err_m2m_init;
 	}
 
-	ret = video_register_device(mdp->m2m_vdev, VFL_TYPE_GRABBER, 2);
+	ret = video_register_device(mdp->m2m_vdev, VFL_TYPE_VIDEO, 2);
 	if (ret) {
 		dev_err(dev, "Failed to register video device\n");
 		goto err_video_register;
diff --git a/drivers/media/platform/mtk-mdp3/mtk-mdp3-regs.c b/drivers/media/platform/mtk-mdp3/mtk-mdp3-regs.c
index 5c48a7e75efd..0c4c942b5f9c 100644
--- a/drivers/media/platform/mtk-mdp3/mtk-mdp3-regs.c
+++ b/drivers/media/platform/mtk-mdp3/mtk-mdp3-regs.c
@@ -168,7 +168,7 @@ static const struct mdp_format mdp_formats[] = {
 		.halign		= 5,
 		.flags		= MDP_FMT_FLAG_OUTPUT,
 	}, {
-		.pixelformat	= V4L2_PIX_FMT_MM21,
+		.pixelformat	= V4L2_PIX_FMT_NV12MT,
 		.mdp_color	= MDP_COLOR_420_BLK,
 		.depth		= { 8, 4 },
 		.row_depth	= { 8, 8 },
-- 
2.18.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] 11+ messages in thread

* Re: [RFC PATCH V0 01/10] media: mtk-mdp3: Add Mediatek MDP Driver
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
                   ` (8 preceding siblings ...)
  2021-06-23  7:35 ` [RFC PATCH V0 10/10] media: mtk-mdp3: Adjust related settings for 5.13-rc1 Moudy Ho
@ 2021-06-23 10:41 ` Chun-Kuang Hu
  2021-07-02 12:00 ` Enric Balletbo Serra
  10 siblings, 0 replies; 11+ messages in thread
From: Chun-Kuang Hu @ 2021-06-23 10:41 UTC (permalink / raw)
  To: Moudy Ho
  Cc: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec, Maoguang Meng, Krzysztof Kozlowski,
	daoyuan huang, Ping-Hsun Wu, Geert Uytterhoeven, Rob Landley,
	Laurent Pinchart, linux-media, DTML, Linux ARM,
	moderated list:ARM/Mediatek SoC support, linux-kernel,
	Tomasz Figa, Nicolas Boichat, acourbot, Pi-Hsun Shih,
	menghui.lin, Sj Huang (黃信璋),
	ben.lok, randy.wu, srv_heupstream,
	Frederic Chen (陳俊元)

Hi, Moudy:

Moudy Ho <moudy.ho@mediatek.com> 於 2021年6月23日 週三 下午3:46寫道:
>
> From: mtk18742 <moudy.ho@mediatek.com>
>
> Add MDP driver for MT8183
>
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---

[snip]

> +
> +#endif  // __MMSYS_CONFIG_H__
> diff --git a/drivers/media/platform/mtk-mdp3/mmsys_mutex.h b/drivers/media/platform/mtk-mdp3/mmsys_mutex.h
> new file mode 100644
> index 000000000000..fb8c179f11af
> --- /dev/null
> +++ b/drivers/media/platform/mtk-mdp3/mmsys_mutex.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
> + */
> +
> +#ifndef __MMSYS_MUTEX_H__
> +#define __MMSYS_MUTEX_H__
> +
> +#include "mmsys_reg_base.h"
> +#include "mdp-platform.h"
> +
> +#define MM_MUTEX_INTEN              0x00
> +#define MM_MUTEX_INTSTA             0x04
> +#define MM_MUTEX_CFG                0x08
> +
> +#define MM_MUTEX_EN                 (0x20 + mutex_id * 0x20)
> +#define MM_MUTEX_GET                (0x24 + mutex_id * 0x20)
> +#define MM_MUTEX_RST                (0x28 + mutex_id * 0x20)
> +#define MM_MUTEX_MOD                (MM_MUTEX_MOD_OFFSET + mutex_id * 0x20)
> +#define MM_MUTEX_SOF                (MM_MUTEX_SOF_OFFSET + mutex_id * 0x20)

mtk_mutex driver is in drivers/soc/mediatek/mtk-mutex.c, so the
mtk_mutex control should be placed there.

> +
> +// MASK
> +#define MM_MUTEX_INTEN_MASK         0x0fff
> +#define MM_MUTEX_INTSTA_MASK        0x0fff
> +#define MM_MUTEX_DEBUG_OUT_SEL_MASK 0x03
> +#define MM_MUTEX_CFG_MASK           0x01
> +
> +#define MM_MUTEX_EN_MASK            0x01
> +#define MM_MUTEX_GET_MASK           0x03
> +#define MM_MUTEX_RST_MASK           0x01
> +#define MM_MUTEX_MOD_MASK           0x07ffffff
> +#define MM_MUTEX_SOF_MASK           0x0f
> +

[snip]

> +int mdp_cmdq_send(struct mdp_dev *mdp, struct mdp_cmdq_param *param)
> +{
> +       struct mdp_cmd cmd;
> +       struct mdp_path path;
> +       int i, ret;
> +
> +       if (atomic_read(&mdp->suspended))
> +               return -ECANCELED;
> +
> +       atomic_inc(&mdp->job_count);
> +
> +       cmd.pkt = cmdq_pkt_create(mdp->cmdq_clt, SZ_16K);
> +       if (IS_ERR(cmd.pkt)) {
> +               atomic_dec(&mdp->job_count);
> +               wake_up(&mdp->callback_wq);
> +               return PTR_ERR(cmd.pkt);
> +       }
> +       cmd.event = &mdp->event[0];
> +
> +       path.mdp_dev = mdp;
> +       path.config = param->config;
> +       path.param = param->param;
> +       for (i = 0; i < param->param->num_outputs; i++) {
> +               path.bounds[i].left = 0;
> +               path.bounds[i].top = 0;
> +               path.bounds[i].width =
> +                       param->param->outputs[i].buffer.format.width;
> +               path.bounds[i].height =
> +                       param->param->outputs[i].buffer.format.height;
> +               path.composes[i] = param->composes[i] ?
> +                       param->composes[i] : &path.bounds[i];
> +       }
> +       ret = mdp_path_config(mdp, &cmd, &path);
> +       if (ret) {
> +               atomic_dec(&mdp->job_count);
> +               wake_up(&mdp->callback_wq);
> +               return ret;
> +       }
> +
> +       // TODO: engine conflict dispatch
> +       for (i = 0; i < param->config->num_components; i++)
> +               mdp_comp_clock_on(&mdp->pdev->dev, path.comps[i].comp);
> +
> +       if (param->wait) {
> +               ret = cmdq_pkt_flush(cmd.pkt);

cmdq_pkt_flush() is removed in latest kernel, please rebase this
series onto latest kernel before you send patches.

Regards,
Chun-Kuang.

> +#ifdef MDP_DEBUG
> +               if (ret) {
> +                       struct mdp_func_struct *p_func = mdp_get_func();
> +
> +                       p_func->mdp_dump_mmsys_config();
> +                       mdp_dump_info(~0, 1);
> +               }
> +#endif
> +               if (param->mdp_ctx)
> +                       mdp_m2m_job_finish(param->mdp_ctx);
> +               cmdq_pkt_destroy(cmd.pkt);
> +               for (i = 0; i < param->config->num_components; i++)
> +                       mdp_comp_clock_off(&mdp->pdev->dev, path.comps[i].comp);
> +
> +               atomic_dec(&mdp->job_count);
> +               wake_up(&mdp->callback_wq);
> +       } else {
> +               struct mdp_cmdq_cb_param *cb_param;
> +               struct mdp_comp *comps;
> +
> +               cb_param = kzalloc(sizeof(*cb_param), GFP_KERNEL);
> +               if (!cb_param)
> +                       return -ENOMEM;
> +               comps = kcalloc(param->config->num_components, sizeof(*comps),
> +                               GFP_KERNEL);
> +               if (!comps) {
> +                       kfree(cb_param);
> +                       mdp_err("%s:comps alloc fail!\n", __func__);
> +                       return -ENOMEM;
> +               }
> +
> +               for (i = 0; i < param->config->num_components; i++)
> +                       memcpy(&comps[i], path.comps[i].comp,
> +                              sizeof(struct mdp_comp));
> +               cb_param->mdp = mdp;
> +               cb_param->user_cmdq_cb = param->cmdq_cb;
> +               cb_param->user_cb_data = param->cb_data;
> +               cb_param->pkt = cmd.pkt;
> +               cb_param->comps = comps;
> +               cb_param->num_comps = param->config->num_components;
> +               cb_param->mdp_ctx = param->mdp_ctx;
> +
> +               ret = cmdq_pkt_flush_async(cmd.pkt,
> +                                          mdp_handle_cmdq_callback,
> +                                          (void *)cb_param);
> +               if (ret) {
> +                       mdp_err("%s:cmdq_pkt_flush_async fail!\n", __func__);
> +                       kfree(cb_param);
> +                       kfree(comps);
> +               }
> +       }
> +       return ret;
> +}
> +

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

_______________________________________________
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] 11+ messages in thread

* Re: [RFC PATCH V0 01/10] media: mtk-mdp3: Add Mediatek MDP Driver
       [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
                   ` (9 preceding siblings ...)
  2021-06-23 10:41 ` [RFC PATCH V0 01/10] media: mtk-mdp3: Add Mediatek MDP Driver Chun-Kuang Hu
@ 2021-07-02 12:00 ` Enric Balletbo Serra
  10 siblings, 0 replies; 11+ messages in thread
From: Enric Balletbo Serra @ 2021-07-02 12:00 UTC (permalink / raw)
  To: Moudy Ho
  Cc: Mauro Carvalho Chehab, Rob Herring, Matthias Brugger,
	Hans Verkuil, Jernej Skrabec, Maoguang Meng, Krzysztof Kozlowski,
	daoyuan huang, Ping-Hsun Wu, Geert Uytterhoeven, Rob Landley,
	Laurent Pinchart, linux-media, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, tfiga, drinkcat, acourbot, pihsun,
	menghui.lin, sj.huang, ben.lok, randy.wu, srv_heupstream,
	frederic.chen

Hi Mody Ho,

Thank you for your patch. Some comments below.

Missatge de Moudy Ho <moudy.ho@mediatek.com> del dia dc., 23 de juny
2021 a les 9:36:
>
> From: mtk18742 <moudy.ho@mediatek.com>
>

Please fix your git configuration, I assume this should be something
like Moudy Ho <moudy.ho@mediatek.com>. Note that the author and the
Signed-off must match.

> Add MDP driver for MT8183
>

In general please use a more verbose commit messages, you know what
MDP means (I probably also know, but not everyone knows what MDP
means) It is a good practice to when you introduce a new driver to
explain well what is and for what is used.

The first question that I have here is why current MDP driver (that
supports other Mediatek chips like the MT8173) can't be extended to
support MT8183. How different is this MDP driver and why can't share
the code.

This single patch is huge, and is unlikely that anyone spend time
reviewing it just for that. I'd recommend to split it and introduce
step-by-step, functionality by functionally. I also have the
impression that you're just reimplementing things that are actually
upstream or not taking advantage of what is already upstream. i.e the
access to the mmsys registers. Please use current kernel
drivers/interfaces API instead of reimplementing it.

I'd strongly recommend to look at current Mediatek mdp driver and see
if you can add support for MT8183 there, and if not, looks at it as
reference.

Best regards,
  Enric


> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
>  drivers/media/platform/Kconfig                |   17 +
>  drivers/media/platform/Makefile               |    2 +
>  drivers/media/platform/mtk-mdp3/Makefile      |    9 +
>  drivers/media/platform/mtk-mdp3/isp_reg.h     |   37 +
>  .../media/platform/mtk-mdp3/mdp-platform.h    |   58 +
>  .../media/platform/mtk-mdp3/mdp_reg_ccorr.h   |   75 +
>  .../media/platform/mtk-mdp3/mdp_reg_rdma.h    |  206 +++
>  drivers/media/platform/mtk-mdp3/mdp_reg_rsz.h |  109 ++
>  .../media/platform/mtk-mdp3/mdp_reg_wdma.h    |  125 ++
>  .../media/platform/mtk-mdp3/mdp_reg_wrot.h    |  115 ++
>  .../media/platform/mtk-mdp3/mmsys_config.h    |  188 +++
>  drivers/media/platform/mtk-mdp3/mmsys_mutex.h |   35 +
>  .../media/platform/mtk-mdp3/mmsys_reg_base.h  |   38 +
>  drivers/media/platform/mtk-mdp3/mtk-img-ipi.h |  282 ++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-cmdq.c   |  521 +++++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-cmdq.h   |   54 +
>  .../media/platform/mtk-mdp3/mtk-mdp3-comp.c   | 1329 +++++++++++++++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-comp.h   |  155 ++
>  .../media/platform/mtk-mdp3/mtk-mdp3-core.c   |  282 ++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-core.h   |   86 ++
>  .../media/platform/mtk-mdp3/mtk-mdp3-debug.c  |  973 ++++++++++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-debug.h  |   39 +
>  .../media/platform/mtk-mdp3/mtk-mdp3-m2m.c    |  804 ++++++++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-m2m.h    |   42 +
>  .../media/platform/mtk-mdp3/mtk-mdp3-regs.c   |  748 ++++++++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-regs.h   |  373 +++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-vpu.c    |  313 ++++
>  .../media/platform/mtk-mdp3/mtk-mdp3-vpu.h    |   79 +
>  28 files changed, 7094 insertions(+)
>  create mode 100644 drivers/media/platform/mtk-mdp3/Makefile
>  create mode 100644 drivers/media/platform/mtk-mdp3/isp_reg.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mdp-platform.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_ccorr.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_rdma.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_rsz.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_wdma.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mdp_reg_wrot.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mmsys_config.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mmsys_mutex.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mmsys_reg_base.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-img-ipi.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.c
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-cmdq.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.c
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-comp.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-core.c
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-core.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-debug.c
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-debug.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.c
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-m2m.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-regs.c
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-regs.h
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-vpu.c
>  create mode 100644 drivers/media/platform/mtk-mdp3/mtk-mdp3-vpu.h

 [snip]

> --
> 2.18.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] 11+ messages in thread

end of thread, other threads:[~2021-07-02 12:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210623073549.24170-1-moudy.ho@mediatek.com>
2021-06-23  7:35 ` [RFC PATCH V0 02/10] media: mtk-mdp3: fix redundant process done caused KE Moudy Ho
2021-06-23  7:35 ` [RFC PATCH V0 03/10] media: mtk-mdp3: revise suspend strategy Moudy Ho
2021-06-23  7:35 ` [RFC PATCH V0 04/10] media: mtk-mdp3: add error handling in error return Moudy Ho
2021-06-23  7:35 ` [RFC PATCH V0 05/10] media: mtk-mdp3: remove unnecessary Null check Moudy Ho
2021-06-23  7:35 ` [RFC PATCH V0 06/10] media: mtk-mdp3: move clock on to precise place Moudy Ho
2021-06-23  7:35 ` [RFC PATCH V0 07/10] media: mtk-mdp3: Fix unpaired settings Moudy Ho
2021-06-23  7:35 ` [RFC PATCH V0 08/10] media: mtk-mdp3: remove illegal device node usage Moudy Ho
2021-06-23  7:35 ` [RFC PATCH V0 09/10] media: mtk-mdp3: revise error handling about get/probe MDP3 Moudy Ho
2021-06-23  7:35 ` [RFC PATCH V0 10/10] media: mtk-mdp3: Adjust related settings for 5.13-rc1 Moudy Ho
2021-06-23 10:41 ` [RFC PATCH V0 01/10] media: mtk-mdp3: Add Mediatek MDP Driver Chun-Kuang Hu
2021-07-02 12:00 ` Enric Balletbo Serra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).