Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on
@ 2025-12-05  1:54 ming.qian
  2025-12-05  1:54 ` [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure ming.qian
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: ming.qian @ 2025-12-05  1:54 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil-cisco, nicolas, benjamin.gaignard, p.zabel,
	sebastian.fricke, shawnguo, ulf.hansson, s.hauer, kernel,
	festevam, linux-imx, l.stach, Frank.li, peng.fan, eagle.zhou, imx,
	linux-pm, linux-kernel, linux-arm-kernel

From: Ming Qian <ming.qian@oss.nxp.com>

On the i.MX8MQ, we encountered some concurrency issues with H264 and HEVC
decoding.

There are two main reasons:
1. The vpu blk-ctrl don't have separate reset and clock enable bits.
2. The g1 VPU and g2 VPU cannot decode simultaneously.

We attempted to make corresponding fix to address these two issues.

Ming Qian (2):
  pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu
  media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

Nicolas Dufresne (1):
  media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure

 drivers/media/platform/verisilicon/hantro.h   |  2 +
 .../media/platform/verisilicon/hantro_drv.c   | 42 +++++++++++++++++--
 .../media/platform/verisilicon/imx8m_vpu_hw.c |  8 ++++
 drivers/media/v4l2-core/v4l2-mem2mem.c        | 23 ++++++++++
 drivers/pmdomain/imx/imx8m-blk-ctrl.c         | 11 +++--
 include/media/v4l2-mem2mem.h                  | 21 ++++++++++
 6 files changed, 100 insertions(+), 7 deletions(-)

-- 
2.52.0



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

* [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure
  2025-12-05  1:54 [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on ming.qian
@ 2025-12-05  1:54 ` ming.qian
  2025-12-05 15:08   ` Frank Li
  2025-12-30 13:39   ` Ulf Hansson
  2025-12-05  1:54 ` [PATCH v4 2/3] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu ming.qian
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: ming.qian @ 2025-12-05  1:54 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil-cisco, nicolas, benjamin.gaignard, p.zabel,
	sebastian.fricke, shawnguo, ulf.hansson, s.hauer, kernel,
	festevam, linux-imx, l.stach, Frank.li, peng.fan, eagle.zhou, imx,
	linux-pm, linux-kernel, linux-arm-kernel

From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Adding a reference count to the v4l2_m2m_dev structure allow safely
sharing it across multiple hardware nodes. This can be used to prevent
running jobs concurrently on m2m cores that have some internal resource
sharing.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
v4
- Add my Signed-off-by

 drivers/media/v4l2-core/v4l2-mem2mem.c | 23 +++++++++++++++++++++++
 include/media/v4l2-mem2mem.h           | 21 +++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index fec93c1a9231..ae0de54d4c3e 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -90,6 +90,7 @@ static const char * const m2m_entity_name[] = {
  * @job_work:		worker to run queued jobs.
  * @job_queue_flags:	flags of the queue status, %QUEUE_PAUSED.
  * @m2m_ops:		driver callbacks
+ * @kref:		device reference count
  */
 struct v4l2_m2m_dev {
 	struct v4l2_m2m_ctx	*curr_ctx;
@@ -109,6 +110,8 @@ struct v4l2_m2m_dev {
 	unsigned long		job_queue_flags;
 
 	const struct v4l2_m2m_ops *m2m_ops;
+
+	struct kref kref;
 };
 
 static struct v4l2_m2m_queue_ctx *get_queue_ctx(struct v4l2_m2m_ctx *m2m_ctx,
@@ -1200,6 +1203,7 @@ struct v4l2_m2m_dev *v4l2_m2m_init(const struct v4l2_m2m_ops *m2m_ops)
 	INIT_LIST_HEAD(&m2m_dev->job_queue);
 	spin_lock_init(&m2m_dev->job_spinlock);
 	INIT_WORK(&m2m_dev->job_work, v4l2_m2m_device_run_work);
+	kref_init(&m2m_dev->kref);
 
 	return m2m_dev;
 }
@@ -1211,6 +1215,25 @@ void v4l2_m2m_release(struct v4l2_m2m_dev *m2m_dev)
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_release);
 
+void v4l2_m2m_get(struct v4l2_m2m_dev *m2m_dev)
+{
+	kref_get(&m2m_dev->kref);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_get);
+
+static void v4l2_m2m_release_from_kref(struct kref *kref)
+{
+	struct v4l2_m2m_dev *m2m_dev = container_of(kref, struct v4l2_m2m_dev, kref);
+
+	v4l2_m2m_release(m2m_dev);
+}
+
+void v4l2_m2m_put(struct v4l2_m2m_dev *m2m_dev)
+{
+	kref_put(&m2m_dev->kref, v4l2_m2m_release_from_kref);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_put);
+
 struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
 		void *drv_priv,
 		int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq))
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index bf6a09a04dcf..ca295c660c7f 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -547,6 +547,27 @@ v4l2_m2m_register_media_controller(struct v4l2_m2m_dev *m2m_dev,
  */
 void v4l2_m2m_release(struct v4l2_m2m_dev *m2m_dev);
 
+/**
+ * v4l2_m2m_get() - take a reference to the m2m_dev structure
+ *
+ * @m2m_dev: opaque pointer to the internal data to handle M2M context
+ *
+ * This is used to share the M2M device across multiple devices. This
+ * can be used to avoid scheduling two hardware nodes concurrently.
+ */
+void v4l2_m2m_get(struct v4l2_m2m_dev *m2m_dev);
+
+/**
+ * v4l2_m2m_put() - remove a reference to the m2m_dev structure
+ *
+ * @m2m_dev: opaque pointer to the internal data to handle M2M context
+ *
+ * Once the M2M device have no more references, v4l2_m2m_realse() will be
+ * called automatically. Users of this method should never call
+ * v4l2_m2m_release() directly. See v4l2_m2m_get() for more details.
+ */
+void v4l2_m2m_put(struct v4l2_m2m_dev *m2m_dev);
+
 /**
  * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
  *
-- 
2.52.0



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

* [PATCH v4 2/3] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu
  2025-12-05  1:54 [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on ming.qian
  2025-12-05  1:54 ` [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure ming.qian
@ 2025-12-05  1:54 ` ming.qian
  2026-01-03 10:02   ` Ulf Hansson
  2025-12-05  1:54 ` [PATCH v4 3/3] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC ming.qian
  2025-12-30 13:42 ` [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on Ulf Hansson
  3 siblings, 1 reply; 13+ messages in thread
From: ming.qian @ 2025-12-05  1:54 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil-cisco, nicolas, benjamin.gaignard, p.zabel,
	sebastian.fricke, shawnguo, ulf.hansson, s.hauer, kernel,
	festevam, linux-imx, l.stach, Frank.li, peng.fan, eagle.zhou, imx,
	linux-pm, linux-kernel, linux-arm-kernel

From: Ming Qian <ming.qian@oss.nxp.com>

For i.MX8MQ platform, the ADB in the VPUMIX domain has no separate reset
and clock enable bits, but is ungated and reset together with the VPUs.
So we can't reset G1 or G2 separately, it may led to the system hang.
Remove rst_mask and clk_mask of imx8mq_vpu_blk_ctl_domain_data.
Let imx8mq_vpu_power_notifier() do really vpu reset.

Fixes: 608d7c325e85 ("soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl")
Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
v3
- Add some comments
v2
- Update commit message

 drivers/pmdomain/imx/imx8m-blk-ctrl.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
index 5c83e5599f1e..74bf4936991d 100644
--- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
+++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
@@ -846,22 +846,25 @@ static int imx8mq_vpu_power_notifier(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
+/*
+ * For i.MX8MQ, the ADB in the VPUMIX domain has no separate reset and clock
+ * enable bits, but is ungated and reset together with the VPUs.
+ * Resetting G1 or G2 separately may led to system hang.
+ * Remove the rst_mask and clk_mask from the domain data of G1 and G2,
+ * Let imx8mq_vpu_power_notifier() do really vpu reset.
+ */
 static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[] = {
 	[IMX8MQ_VPUBLK_PD_G1] = {
 		.name = "vpublk-g1",
 		.clk_names = (const char *[]){ "g1", },
 		.num_clks = 1,
 		.gpc_name = "g1",
-		.rst_mask = BIT(1),
-		.clk_mask = BIT(1),
 	},
 	[IMX8MQ_VPUBLK_PD_G2] = {
 		.name = "vpublk-g2",
 		.clk_names = (const char *[]){ "g2", },
 		.num_clks = 1,
 		.gpc_name = "g2",
-		.rst_mask = BIT(0),
-		.clk_mask = BIT(0),
 	},
 };
 
-- 
2.52.0



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

* [PATCH v4 3/3] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC
  2025-12-05  1:54 [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on ming.qian
  2025-12-05  1:54 ` [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure ming.qian
  2025-12-05  1:54 ` [PATCH v4 2/3] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu ming.qian
@ 2025-12-05  1:54 ` ming.qian
  2025-12-05 15:09   ` Frank Li
  2025-12-30 13:42 ` [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on Ulf Hansson
  3 siblings, 1 reply; 13+ messages in thread
From: ming.qian @ 2025-12-05  1:54 UTC (permalink / raw)
  To: linux-media
  Cc: mchehab, hverkuil-cisco, nicolas, benjamin.gaignard, p.zabel,
	sebastian.fricke, shawnguo, ulf.hansson, s.hauer, kernel,
	festevam, linux-imx, l.stach, Frank.li, peng.fan, eagle.zhou, imx,
	linux-pm, linux-kernel, linux-arm-kernel

From: Ming Qian <ming.qian@oss.nxp.com>

For the i.MX8MQ platform, there is a hardware limitation: the g1 VPU and
g2 VPU cannot decode simultaneously; otherwise, it will cause below bus
error and produce corrupted pictures, even potentially lead to system hang.

[  110.527986] hantro-vpu 38310000.video-codec: frame decode timed out.
[  110.583517] hantro-vpu 38310000.video-codec: bus error detected.

Therefore, it is necessary to ensure that g1 and g2 operate alternately.
This allows for successful multi-instance decoding of H.264 and HEVC.

To achieve this, g1 and g2 share the same v4l2_m2m_dev, and then the
v4l2_m2m_dev can handle the scheduling.

Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
Co-developed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
v4
- Avoid double put device node in for_each_matching_node
v3
- Add some comments
v2
- Update commit message

 drivers/media/platform/verisilicon/hantro.h   |  2 +
 .../media/platform/verisilicon/hantro_drv.c   | 42 +++++++++++++++++--
 .../media/platform/verisilicon/imx8m_vpu_hw.c |  8 ++++
 3 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
index e0fdc4535b2d..0353de154a1e 100644
--- a/drivers/media/platform/verisilicon/hantro.h
+++ b/drivers/media/platform/verisilicon/hantro.h
@@ -77,6 +77,7 @@ struct hantro_irq {
  * @double_buffer:		core needs double buffering
  * @legacy_regs:		core uses legacy register set
  * @late_postproc:		postproc must be set up at the end of the job
+ * @shared_devices:		an array of device ids that cannot run concurrently
  */
 struct hantro_variant {
 	unsigned int enc_offset;
@@ -101,6 +102,7 @@ struct hantro_variant {
 	unsigned int double_buffer : 1;
 	unsigned int legacy_regs : 1;
 	unsigned int late_postproc : 1;
+	const struct of_device_id *shared_devices;
 };
 
 /**
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index 60b95b5d8565..94f58f4e4a4e 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -13,6 +13,7 @@
 #include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
@@ -1035,6 +1036,41 @@ static int hantro_disable_multicore(struct hantro_dev *vpu)
 	return 0;
 }
 
+static struct v4l2_m2m_dev *hantro_get_v4l2_m2m_dev(struct hantro_dev *vpu)
+{
+	struct device_node *node;
+	struct hantro_dev *shared_vpu;
+
+	if (!vpu->variant || !vpu->variant->shared_devices)
+		goto init_new_m2m_dev;
+
+	for_each_matching_node(node, vpu->variant->shared_devices) {
+		struct platform_device *pdev;
+		struct v4l2_m2m_dev *m2m_dev;
+
+		pdev = of_find_device_by_node(node);
+		if (!pdev)
+			continue;
+
+		shared_vpu = platform_get_drvdata(pdev);
+		if (IS_ERR_OR_NULL(shared_vpu) || shared_vpu == vpu) {
+			platform_device_put(pdev);
+			continue;
+		}
+
+		v4l2_m2m_get(shared_vpu->m2m_dev);
+		m2m_dev = shared_vpu->m2m_dev;
+		platform_device_put(pdev);
+
+		of_node_put(node);
+
+		return m2m_dev;
+	}
+
+init_new_m2m_dev:
+	return v4l2_m2m_init(&vpu_m2m_ops);
+}
+
 static int hantro_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
@@ -1186,7 +1222,7 @@ static int hantro_probe(struct platform_device *pdev)
 	}
 	platform_set_drvdata(pdev, vpu);
 
-	vpu->m2m_dev = v4l2_m2m_init(&vpu_m2m_ops);
+	vpu->m2m_dev = hantro_get_v4l2_m2m_dev(vpu);
 	if (IS_ERR(vpu->m2m_dev)) {
 		v4l2_err(&vpu->v4l2_dev, "Failed to init mem2mem device\n");
 		ret = PTR_ERR(vpu->m2m_dev);
@@ -1225,7 +1261,7 @@ static int hantro_probe(struct platform_device *pdev)
 	hantro_remove_enc_func(vpu);
 err_m2m_rel:
 	media_device_cleanup(&vpu->mdev);
-	v4l2_m2m_release(vpu->m2m_dev);
+	v4l2_m2m_put(vpu->m2m_dev);
 err_v4l2_unreg:
 	v4l2_device_unregister(&vpu->v4l2_dev);
 err_clk_unprepare:
@@ -1248,7 +1284,7 @@ static void hantro_remove(struct platform_device *pdev)
 	hantro_remove_dec_func(vpu);
 	hantro_remove_enc_func(vpu);
 	media_device_cleanup(&vpu->mdev);
-	v4l2_m2m_release(vpu->m2m_dev);
+	v4l2_m2m_put(vpu->m2m_dev);
 	v4l2_device_unregister(&vpu->v4l2_dev);
 	clk_bulk_unprepare(vpu->variant->num_clocks, vpu->clocks);
 	reset_control_assert(vpu->resets);
diff --git a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
index 5be0e2e76882..6f8e43b7f157 100644
--- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
+++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
@@ -343,6 +343,12 @@ const struct hantro_variant imx8mq_vpu_variant = {
 	.num_regs = ARRAY_SIZE(imx8mq_reg_names)
 };
 
+static const struct of_device_id imx8mq_vpu_shared_resources[] __initconst = {
+	{ .compatible = "nxp,imx8mq-vpu-g1", },
+	{ .compatible = "nxp,imx8mq-vpu-g2", },
+	{ /* sentinel */ }
+};
+
 const struct hantro_variant imx8mq_vpu_g1_variant = {
 	.dec_fmts = imx8m_vpu_dec_fmts,
 	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
@@ -356,6 +362,7 @@ const struct hantro_variant imx8mq_vpu_g1_variant = {
 	.num_irqs = ARRAY_SIZE(imx8mq_irqs),
 	.clk_names = imx8mq_g1_clk_names,
 	.num_clocks = ARRAY_SIZE(imx8mq_g1_clk_names),
+	.shared_devices = imx8mq_vpu_shared_resources,
 };
 
 const struct hantro_variant imx8mq_vpu_g2_variant = {
@@ -371,6 +378,7 @@ const struct hantro_variant imx8mq_vpu_g2_variant = {
 	.num_irqs = ARRAY_SIZE(imx8mq_g2_irqs),
 	.clk_names = imx8mq_g2_clk_names,
 	.num_clocks = ARRAY_SIZE(imx8mq_g2_clk_names),
+	.shared_devices = imx8mq_vpu_shared_resources,
 };
 
 const struct hantro_variant imx8mm_vpu_g1_variant = {
-- 
2.52.0



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

* Re: [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure
  2025-12-05  1:54 ` [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure ming.qian
@ 2025-12-05 15:08   ` Frank Li
  2025-12-30 13:39   ` Ulf Hansson
  1 sibling, 0 replies; 13+ messages in thread
From: Frank Li @ 2025-12-05 15:08 UTC (permalink / raw)
  To: ming.qian
  Cc: linux-media, mchehab, hverkuil-cisco, nicolas, benjamin.gaignard,
	p.zabel, sebastian.fricke, shawnguo, ulf.hansson, s.hauer, kernel,
	festevam, linux-imx, l.stach, peng.fan, eagle.zhou, imx, linux-pm,
	linux-kernel, linux-arm-kernel

On Fri, Dec 05, 2025 at 09:54:24AM +0800, ming.qian@oss.nxp.com wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
> Adding a reference count to the v4l2_m2m_dev structure allow safely
> sharing it across multiple hardware nodes. This can be used to prevent
> running jobs concurrently on m2m cores that have some internal resource
> sharing.
>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> v4
> - Add my Signed-off-by
>
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 23 +++++++++++++++++++++++
>  include/media/v4l2-mem2mem.h           | 21 +++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index fec93c1a9231..ae0de54d4c3e 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -90,6 +90,7 @@ static const char * const m2m_entity_name[] = {
>   * @job_work:		worker to run queued jobs.
>   * @job_queue_flags:	flags of the queue status, %QUEUE_PAUSED.
>   * @m2m_ops:		driver callbacks
> + * @kref:		device reference count
>   */
>  struct v4l2_m2m_dev {
>  	struct v4l2_m2m_ctx	*curr_ctx;
> @@ -109,6 +110,8 @@ struct v4l2_m2m_dev {
>  	unsigned long		job_queue_flags;
>
>  	const struct v4l2_m2m_ops *m2m_ops;
> +
> +	struct kref kref;
>  };
>
>  static struct v4l2_m2m_queue_ctx *get_queue_ctx(struct v4l2_m2m_ctx *m2m_ctx,
> @@ -1200,6 +1203,7 @@ struct v4l2_m2m_dev *v4l2_m2m_init(const struct v4l2_m2m_ops *m2m_ops)
>  	INIT_LIST_HEAD(&m2m_dev->job_queue);
>  	spin_lock_init(&m2m_dev->job_spinlock);
>  	INIT_WORK(&m2m_dev->job_work, v4l2_m2m_device_run_work);
> +	kref_init(&m2m_dev->kref);
>
>  	return m2m_dev;
>  }
> @@ -1211,6 +1215,25 @@ void v4l2_m2m_release(struct v4l2_m2m_dev *m2m_dev)
>  }
>  EXPORT_SYMBOL_GPL(v4l2_m2m_release);
>
> +void v4l2_m2m_get(struct v4l2_m2m_dev *m2m_dev)
> +{
> +	kref_get(&m2m_dev->kref);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_get);
> +
> +static void v4l2_m2m_release_from_kref(struct kref *kref)
> +{
> +	struct v4l2_m2m_dev *m2m_dev = container_of(kref, struct v4l2_m2m_dev, kref);
> +
> +	v4l2_m2m_release(m2m_dev);
> +}
> +
> +void v4l2_m2m_put(struct v4l2_m2m_dev *m2m_dev)
> +{
> +	kref_put(&m2m_dev->kref, v4l2_m2m_release_from_kref);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_put);
> +
>  struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>  		void *drv_priv,
>  		int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq))
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index bf6a09a04dcf..ca295c660c7f 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -547,6 +547,27 @@ v4l2_m2m_register_media_controller(struct v4l2_m2m_dev *m2m_dev,
>   */
>  void v4l2_m2m_release(struct v4l2_m2m_dev *m2m_dev);
>
> +/**
> + * v4l2_m2m_get() - take a reference to the m2m_dev structure
> + *
> + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> + *
> + * This is used to share the M2M device across multiple devices. This
> + * can be used to avoid scheduling two hardware nodes concurrently.
> + */
> +void v4l2_m2m_get(struct v4l2_m2m_dev *m2m_dev);
> +
> +/**
> + * v4l2_m2m_put() - remove a reference to the m2m_dev structure
> + *
> + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> + *
> + * Once the M2M device have no more references, v4l2_m2m_realse() will be
> + * called automatically. Users of this method should never call
> + * v4l2_m2m_release() directly. See v4l2_m2m_get() for more details.
> + */
> +void v4l2_m2m_put(struct v4l2_m2m_dev *m2m_dev);
> +
>  /**
>   * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
>   *
> --
> 2.52.0
>


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

* Re: [PATCH v4 3/3] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC
  2025-12-05  1:54 ` [PATCH v4 3/3] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC ming.qian
@ 2025-12-05 15:09   ` Frank Li
  0 siblings, 0 replies; 13+ messages in thread
From: Frank Li @ 2025-12-05 15:09 UTC (permalink / raw)
  To: ming.qian
  Cc: linux-media, mchehab, hverkuil-cisco, nicolas, benjamin.gaignard,
	p.zabel, sebastian.fricke, shawnguo, ulf.hansson, s.hauer, kernel,
	festevam, linux-imx, l.stach, peng.fan, eagle.zhou, imx, linux-pm,
	linux-kernel, linux-arm-kernel

On Fri, Dec 05, 2025 at 09:54:26AM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> For the i.MX8MQ platform, there is a hardware limitation: the g1 VPU and
> g2 VPU cannot decode simultaneously; otherwise, it will cause below bus
> error and produce corrupted pictures, even potentially lead to system hang.
>
> [  110.527986] hantro-vpu 38310000.video-codec: frame decode timed out.
> [  110.583517] hantro-vpu 38310000.video-codec: bus error detected.
>
> Therefore, it is necessary to ensure that g1 and g2 operate alternately.
> This allows for successful multi-instance decoding of H.264 and HEVC.
>
> To achieve this, g1 and g2 share the same v4l2_m2m_dev, and then the
> v4l2_m2m_dev can handle the scheduling.
>
> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> Co-developed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> v4
> - Avoid double put device node in for_each_matching_node
> v3
> - Add some comments
> v2
> - Update commit message
>
>  drivers/media/platform/verisilicon/hantro.h   |  2 +
>  .../media/platform/verisilicon/hantro_drv.c   | 42 +++++++++++++++++--
>  .../media/platform/verisilicon/imx8m_vpu_hw.c |  8 ++++
>  3 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> index e0fdc4535b2d..0353de154a1e 100644
> --- a/drivers/media/platform/verisilicon/hantro.h
> +++ b/drivers/media/platform/verisilicon/hantro.h
> @@ -77,6 +77,7 @@ struct hantro_irq {
>   * @double_buffer:		core needs double buffering
>   * @legacy_regs:		core uses legacy register set
>   * @late_postproc:		postproc must be set up at the end of the job
> + * @shared_devices:		an array of device ids that cannot run concurrently
>   */
>  struct hantro_variant {
>  	unsigned int enc_offset;
> @@ -101,6 +102,7 @@ struct hantro_variant {
>  	unsigned int double_buffer : 1;
>  	unsigned int legacy_regs : 1;
>  	unsigned int late_postproc : 1;
> +	const struct of_device_id *shared_devices;
>  };
>
>  /**
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 60b95b5d8565..94f58f4e4a4e 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -13,6 +13,7 @@
>  #include <linux/clk.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
> @@ -1035,6 +1036,41 @@ static int hantro_disable_multicore(struct hantro_dev *vpu)
>  	return 0;
>  }
>
> +static struct v4l2_m2m_dev *hantro_get_v4l2_m2m_dev(struct hantro_dev *vpu)
> +{
> +	struct device_node *node;
> +	struct hantro_dev *shared_vpu;
> +
> +	if (!vpu->variant || !vpu->variant->shared_devices)
> +		goto init_new_m2m_dev;
> +
> +	for_each_matching_node(node, vpu->variant->shared_devices) {
> +		struct platform_device *pdev;
> +		struct v4l2_m2m_dev *m2m_dev;
> +
> +		pdev = of_find_device_by_node(node);
> +		if (!pdev)
> +			continue;
> +
> +		shared_vpu = platform_get_drvdata(pdev);
> +		if (IS_ERR_OR_NULL(shared_vpu) || shared_vpu == vpu) {
> +			platform_device_put(pdev);
> +			continue;
> +		}
> +
> +		v4l2_m2m_get(shared_vpu->m2m_dev);
> +		m2m_dev = shared_vpu->m2m_dev;
> +		platform_device_put(pdev);
> +
> +		of_node_put(node);
> +
> +		return m2m_dev;
> +	}
> +
> +init_new_m2m_dev:
> +	return v4l2_m2m_init(&vpu_m2m_ops);
> +}
> +
>  static int hantro_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *match;
> @@ -1186,7 +1222,7 @@ static int hantro_probe(struct platform_device *pdev)
>  	}
>  	platform_set_drvdata(pdev, vpu);
>
> -	vpu->m2m_dev = v4l2_m2m_init(&vpu_m2m_ops);
> +	vpu->m2m_dev = hantro_get_v4l2_m2m_dev(vpu);
>  	if (IS_ERR(vpu->m2m_dev)) {
>  		v4l2_err(&vpu->v4l2_dev, "Failed to init mem2mem device\n");
>  		ret = PTR_ERR(vpu->m2m_dev);
> @@ -1225,7 +1261,7 @@ static int hantro_probe(struct platform_device *pdev)
>  	hantro_remove_enc_func(vpu);
>  err_m2m_rel:
>  	media_device_cleanup(&vpu->mdev);
> -	v4l2_m2m_release(vpu->m2m_dev);
> +	v4l2_m2m_put(vpu->m2m_dev);
>  err_v4l2_unreg:
>  	v4l2_device_unregister(&vpu->v4l2_dev);
>  err_clk_unprepare:
> @@ -1248,7 +1284,7 @@ static void hantro_remove(struct platform_device *pdev)
>  	hantro_remove_dec_func(vpu);
>  	hantro_remove_enc_func(vpu);
>  	media_device_cleanup(&vpu->mdev);
> -	v4l2_m2m_release(vpu->m2m_dev);
> +	v4l2_m2m_put(vpu->m2m_dev);
>  	v4l2_device_unregister(&vpu->v4l2_dev);
>  	clk_bulk_unprepare(vpu->variant->num_clocks, vpu->clocks);
>  	reset_control_assert(vpu->resets);
> diff --git a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> index 5be0e2e76882..6f8e43b7f157 100644
> --- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> @@ -343,6 +343,12 @@ const struct hantro_variant imx8mq_vpu_variant = {
>  	.num_regs = ARRAY_SIZE(imx8mq_reg_names)
>  };
>
> +static const struct of_device_id imx8mq_vpu_shared_resources[] __initconst = {
> +	{ .compatible = "nxp,imx8mq-vpu-g1", },
> +	{ .compatible = "nxp,imx8mq-vpu-g2", },
> +	{ /* sentinel */ }
> +};
> +
>  const struct hantro_variant imx8mq_vpu_g1_variant = {
>  	.dec_fmts = imx8m_vpu_dec_fmts,
>  	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
> @@ -356,6 +362,7 @@ const struct hantro_variant imx8mq_vpu_g1_variant = {
>  	.num_irqs = ARRAY_SIZE(imx8mq_irqs),
>  	.clk_names = imx8mq_g1_clk_names,
>  	.num_clocks = ARRAY_SIZE(imx8mq_g1_clk_names),
> +	.shared_devices = imx8mq_vpu_shared_resources,
>  };
>
>  const struct hantro_variant imx8mq_vpu_g2_variant = {
> @@ -371,6 +378,7 @@ const struct hantro_variant imx8mq_vpu_g2_variant = {
>  	.num_irqs = ARRAY_SIZE(imx8mq_g2_irqs),
>  	.clk_names = imx8mq_g2_clk_names,
>  	.num_clocks = ARRAY_SIZE(imx8mq_g2_clk_names),
> +	.shared_devices = imx8mq_vpu_shared_resources,
>  };
>
>  const struct hantro_variant imx8mm_vpu_g1_variant = {
> --
> 2.52.0
>


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

* Re: [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure
  2025-12-05  1:54 ` [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure ming.qian
  2025-12-05 15:08   ` Frank Li
@ 2025-12-30 13:39   ` Ulf Hansson
  2026-01-05 18:08     ` Nicolas Dufresne
  1 sibling, 1 reply; 13+ messages in thread
From: Ulf Hansson @ 2025-12-30 13:39 UTC (permalink / raw)
  To: ming.qian
  Cc: linux-media, mchehab, hverkuil-cisco, nicolas, benjamin.gaignard,
	p.zabel, sebastian.fricke, shawnguo, s.hauer, kernel, festevam,
	linux-imx, l.stach, Frank.li, peng.fan, eagle.zhou, imx, linux-pm,
	linux-kernel, linux-arm-kernel

On Fri, 5 Dec 2025 at 02:55, <ming.qian@oss.nxp.com> wrote:
>
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>
> Adding a reference count to the v4l2_m2m_dev structure allow safely
> sharing it across multiple hardware nodes. This can be used to prevent
> running jobs concurrently on m2m cores that have some internal resource
> sharing.
>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>

I certainly don't have the complete picture for how this needs to work.

However, I was thinking that rather than using a kref and having to
share two specific functions to update it (v4l2_m2m_get|put), couldn't
we just use a device-link instead?

Kind regards
Uffe

> ---
> v4
> - Add my Signed-off-by
>
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 23 +++++++++++++++++++++++
>  include/media/v4l2-mem2mem.h           | 21 +++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index fec93c1a9231..ae0de54d4c3e 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -90,6 +90,7 @@ static const char * const m2m_entity_name[] = {
>   * @job_work:          worker to run queued jobs.
>   * @job_queue_flags:   flags of the queue status, %QUEUE_PAUSED.
>   * @m2m_ops:           driver callbacks
> + * @kref:              device reference count
>   */
>  struct v4l2_m2m_dev {
>         struct v4l2_m2m_ctx     *curr_ctx;
> @@ -109,6 +110,8 @@ struct v4l2_m2m_dev {
>         unsigned long           job_queue_flags;
>
>         const struct v4l2_m2m_ops *m2m_ops;
> +
> +       struct kref kref;
>  };
>
>  static struct v4l2_m2m_queue_ctx *get_queue_ctx(struct v4l2_m2m_ctx *m2m_ctx,
> @@ -1200,6 +1203,7 @@ struct v4l2_m2m_dev *v4l2_m2m_init(const struct v4l2_m2m_ops *m2m_ops)
>         INIT_LIST_HEAD(&m2m_dev->job_queue);
>         spin_lock_init(&m2m_dev->job_spinlock);
>         INIT_WORK(&m2m_dev->job_work, v4l2_m2m_device_run_work);
> +       kref_init(&m2m_dev->kref);
>
>         return m2m_dev;
>  }
> @@ -1211,6 +1215,25 @@ void v4l2_m2m_release(struct v4l2_m2m_dev *m2m_dev)
>  }
>  EXPORT_SYMBOL_GPL(v4l2_m2m_release);
>
> +void v4l2_m2m_get(struct v4l2_m2m_dev *m2m_dev)
> +{
> +       kref_get(&m2m_dev->kref);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_get);
> +
> +static void v4l2_m2m_release_from_kref(struct kref *kref)
> +{
> +       struct v4l2_m2m_dev *m2m_dev = container_of(kref, struct v4l2_m2m_dev, kref);
> +
> +       v4l2_m2m_release(m2m_dev);
> +}
> +
> +void v4l2_m2m_put(struct v4l2_m2m_dev *m2m_dev)
> +{
> +       kref_put(&m2m_dev->kref, v4l2_m2m_release_from_kref);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_put);
> +
>  struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>                 void *drv_priv,
>                 int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq))
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index bf6a09a04dcf..ca295c660c7f 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -547,6 +547,27 @@ v4l2_m2m_register_media_controller(struct v4l2_m2m_dev *m2m_dev,
>   */
>  void v4l2_m2m_release(struct v4l2_m2m_dev *m2m_dev);
>
> +/**
> + * v4l2_m2m_get() - take a reference to the m2m_dev structure
> + *
> + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> + *
> + * This is used to share the M2M device across multiple devices. This
> + * can be used to avoid scheduling two hardware nodes concurrently.
> + */
> +void v4l2_m2m_get(struct v4l2_m2m_dev *m2m_dev);
> +
> +/**
> + * v4l2_m2m_put() - remove a reference to the m2m_dev structure
> + *
> + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> + *
> + * Once the M2M device have no more references, v4l2_m2m_realse() will be
> + * called automatically. Users of this method should never call
> + * v4l2_m2m_release() directly. See v4l2_m2m_get() for more details.
> + */
> +void v4l2_m2m_put(struct v4l2_m2m_dev *m2m_dev);
> +
>  /**
>   * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
>   *
> --
> 2.52.0
>


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

* Re: [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on
  2025-12-05  1:54 [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on ming.qian
                   ` (2 preceding siblings ...)
  2025-12-05  1:54 ` [PATCH v4 3/3] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC ming.qian
@ 2025-12-30 13:42 ` Ulf Hansson
  2025-12-31  5:37   ` Ming Qian(OSS)
  2026-01-05 18:10   ` Nicolas Dufresne
  3 siblings, 2 replies; 13+ messages in thread
From: Ulf Hansson @ 2025-12-30 13:42 UTC (permalink / raw)
  To: ming.qian
  Cc: linux-media, mchehab, hverkuil-cisco, nicolas, benjamin.gaignard,
	p.zabel, sebastian.fricke, shawnguo, s.hauer, kernel, festevam,
	linux-imx, l.stach, Frank.li, peng.fan, eagle.zhou, imx, linux-pm,
	linux-kernel, linux-arm-kernel

On Fri, 5 Dec 2025 at 02:55, <ming.qian@oss.nxp.com> wrote:
>
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> On the i.MX8MQ, we encountered some concurrency issues with H264 and HEVC
> decoding.
>
> There are two main reasons:
> 1. The vpu blk-ctrl don't have separate reset and clock enable bits.
> 2. The g1 VPU and g2 VPU cannot decode simultaneously.
>
> We attempted to make corresponding fix to address these two issues.
>
> Ming Qian (2):
>   pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu
>   media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC
>
> Nicolas Dufresne (1):
>   media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure
>
>  drivers/media/platform/verisilicon/hantro.h   |  2 +
>  .../media/platform/verisilicon/hantro_drv.c   | 42 +++++++++++++++++--
>  .../media/platform/verisilicon/imx8m_vpu_hw.c |  8 ++++
>  drivers/media/v4l2-core/v4l2-mem2mem.c        | 23 ++++++++++
>  drivers/pmdomain/imx/imx8m-blk-ctrl.c         | 11 +++--
>  include/media/v4l2-mem2mem.h                  | 21 ++++++++++
>  6 files changed, 100 insertions(+), 7 deletions(-)
>

Can I pick the pmdomain patch separately from the others? Or does this
need to go together?

Kind regards
Uffe


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

* Re: [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on
  2025-12-30 13:42 ` [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on Ulf Hansson
@ 2025-12-31  5:37   ` Ming Qian(OSS)
  2026-01-05 18:10   ` Nicolas Dufresne
  1 sibling, 0 replies; 13+ messages in thread
From: Ming Qian(OSS) @ 2025-12-31  5:37 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-media, mchehab, hverkuil-cisco, nicolas, benjamin.gaignard,
	p.zabel, sebastian.fricke, shawnguo, s.hauer, kernel, festevam,
	linux-imx, l.stach, Frank.li, peng.fan, eagle.zhou, imx, linux-pm,
	linux-kernel, linux-arm-kernel

Hi Uffe,

On 12/30/2025 9:42 PM, Ulf Hansson wrote:
> On Fri, 5 Dec 2025 at 02:55, <ming.qian@oss.nxp.com> wrote:
>>
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> On the i.MX8MQ, we encountered some concurrency issues with H264 and HEVC
>> decoding.
>>
>> There are two main reasons:
>> 1. The vpu blk-ctrl don't have separate reset and clock enable bits.
>> 2. The g1 VPU and g2 VPU cannot decode simultaneously.
>>
>> We attempted to make corresponding fix to address these two issues.
>>
>> Ming Qian (2):
>>    pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu
>>    media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC
>>
>> Nicolas Dufresne (1):
>>    media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure
>>
>>   drivers/media/platform/verisilicon/hantro.h   |  2 +
>>   .../media/platform/verisilicon/hantro_drv.c   | 42 +++++++++++++++++--
>>   .../media/platform/verisilicon/imx8m_vpu_hw.c |  8 ++++
>>   drivers/media/v4l2-core/v4l2-mem2mem.c        | 23 ++++++++++
>>   drivers/pmdomain/imx/imx8m-blk-ctrl.c         | 11 +++--
>>   include/media/v4l2-mem2mem.h                  | 21 ++++++++++
>>   6 files changed, 100 insertions(+), 7 deletions(-)
>>
> 
> Can I pick the pmdomain patch separately from the others? Or does this
> need to go together?
> 
> Kind regards
> Uffe

I think you can pick the pmdomain patch separately.

Regards,
Ming



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

* Re: [PATCH v4 2/3] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu
  2025-12-05  1:54 ` [PATCH v4 2/3] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu ming.qian
@ 2026-01-03 10:02   ` Ulf Hansson
  0 siblings, 0 replies; 13+ messages in thread
From: Ulf Hansson @ 2026-01-03 10:02 UTC (permalink / raw)
  To: ming.qian
  Cc: linux-media, mchehab, hverkuil-cisco, nicolas, benjamin.gaignard,
	p.zabel, sebastian.fricke, shawnguo, s.hauer, kernel, festevam,
	linux-imx, l.stach, Frank.li, peng.fan, eagle.zhou, imx, linux-pm,
	linux-kernel, linux-arm-kernel

On Fri, 5 Dec 2025 at 02:55, <ming.qian@oss.nxp.com> wrote:
>
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> For i.MX8MQ platform, the ADB in the VPUMIX domain has no separate reset
> and clock enable bits, but is ungated and reset together with the VPUs.
> So we can't reset G1 or G2 separately, it may led to the system hang.
> Remove rst_mask and clk_mask of imx8mq_vpu_blk_ctl_domain_data.
> Let imx8mq_vpu_power_notifier() do really vpu reset.
>
> Fixes: 608d7c325e85 ("soc: imx: imx8m-blk-ctrl: add i.MX8MQ VPU blk-ctrl")
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> Reviewed-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>

Applied for fixes and by adding a stable tag, thanks!

Kind regards
Uffe


> ---
> v3
> - Add some comments
> v2
> - Update commit message
>
>  drivers/pmdomain/imx/imx8m-blk-ctrl.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> index 5c83e5599f1e..74bf4936991d 100644
> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> @@ -846,22 +846,25 @@ static int imx8mq_vpu_power_notifier(struct notifier_block *nb,
>         return NOTIFY_OK;
>  }
>
> +/*
> + * For i.MX8MQ, the ADB in the VPUMIX domain has no separate reset and clock
> + * enable bits, but is ungated and reset together with the VPUs.
> + * Resetting G1 or G2 separately may led to system hang.
> + * Remove the rst_mask and clk_mask from the domain data of G1 and G2,
> + * Let imx8mq_vpu_power_notifier() do really vpu reset.
> + */
>  static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[] = {
>         [IMX8MQ_VPUBLK_PD_G1] = {
>                 .name = "vpublk-g1",
>                 .clk_names = (const char *[]){ "g1", },
>                 .num_clks = 1,
>                 .gpc_name = "g1",
> -               .rst_mask = BIT(1),
> -               .clk_mask = BIT(1),
>         },
>         [IMX8MQ_VPUBLK_PD_G2] = {
>                 .name = "vpublk-g2",
>                 .clk_names = (const char *[]){ "g2", },
>                 .num_clks = 1,
>                 .gpc_name = "g2",
> -               .rst_mask = BIT(0),
> -               .clk_mask = BIT(0),
>         },
>  };
>
> --
> 2.52.0
>


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

* Re: [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure
  2025-12-30 13:39   ` Ulf Hansson
@ 2026-01-05 18:08     ` Nicolas Dufresne
  2026-01-06 13:28       ` Ulf Hansson
  0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Dufresne @ 2026-01-05 18:08 UTC (permalink / raw)
  To: Ulf Hansson, ming.qian
  Cc: linux-media, mchehab, hverkuil-cisco, benjamin.gaignard, p.zabel,
	sebastian.fricke, shawnguo, s.hauer, kernel, festevam, linux-imx,
	l.stach, Frank.li, peng.fan, eagle.zhou, imx, linux-pm,
	linux-kernel, linux-arm-kernel

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

Hi,

Le mardi 30 décembre 2025 à 14:39 +0100, Ulf Hansson a écrit :
> On Fri, 5 Dec 2025 at 02:55, <ming.qian@oss.nxp.com> wrote:
> > 
> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > Adding a reference count to the v4l2_m2m_dev structure allow safely
> > sharing it across multiple hardware nodes. This can be used to prevent
> > running jobs concurrently on m2m cores that have some internal resource
> > sharing.
> > 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> 
> I certainly don't have the complete picture for how this needs to work.
> 
> However, I was thinking that rather than using a kref and having to
> share two specific functions to update it (v4l2_m2m_get|put), couldn't
> we just use a device-link instead?

The device link seems to be intended for power management, I'm effectively not
familiar with it, so thanks for the information. However, my impression is that
this is indented to describe power management dependencies on siblings, but this
was done differently on this SoC (way before my time here), the DT seems to
describe a block controller which is common to the two (defective) cores. So
power management wise, its already handled.

Be aware that my intent is to rename v4l2_m2m_dev into v4l2_m2m_sched. Its not a
dev wrapper, but a helper object that will schedule work coming from N
v4l2_m2m_ctx (per open() calls). Making this helper object ref counted makes the
live-time management a lot simpler, and since its not a dev wrapper, this patch
is not creating a second refcount for the same object.

regards,
Nicolas


> 
> Kind regards
> Uffe
> 
> > ---
> > v4
> > - Add my Signed-off-by
> > 
> >  drivers/media/v4l2-core/v4l2-mem2mem.c | 23 +++++++++++++++++++++++
> >  include/media/v4l2-mem2mem.h           | 21 +++++++++++++++++++++
> >  2 files changed, 44 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-
> > core/v4l2-mem2mem.c
> > index fec93c1a9231..ae0de54d4c3e 100644
> > --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> > +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> > @@ -90,6 +90,7 @@ static const char * const m2m_entity_name[] = {
> >   * @job_work:          worker to run queued jobs.
> >   * @job_queue_flags:   flags of the queue status, %QUEUE_PAUSED.
> >   * @m2m_ops:           driver callbacks
> > + * @kref:              device reference count
> >   */
> >  struct v4l2_m2m_dev {
> >         struct v4l2_m2m_ctx     *curr_ctx;
> > @@ -109,6 +110,8 @@ struct v4l2_m2m_dev {
> >         unsigned long           job_queue_flags;
> > 
> >         const struct v4l2_m2m_ops *m2m_ops;
> > +
> > +       struct kref kref;
> >  };
> > 
> >  static struct v4l2_m2m_queue_ctx *get_queue_ctx(struct v4l2_m2m_ctx
> > *m2m_ctx,
> > @@ -1200,6 +1203,7 @@ struct v4l2_m2m_dev *v4l2_m2m_init(const struct
> > v4l2_m2m_ops *m2m_ops)
> >         INIT_LIST_HEAD(&m2m_dev->job_queue);
> >         spin_lock_init(&m2m_dev->job_spinlock);
> >         INIT_WORK(&m2m_dev->job_work, v4l2_m2m_device_run_work);
> > +       kref_init(&m2m_dev->kref);
> > 
> >         return m2m_dev;
> >  }
> > @@ -1211,6 +1215,25 @@ void v4l2_m2m_release(struct v4l2_m2m_dev *m2m_dev)
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_m2m_release);
> > 
> > +void v4l2_m2m_get(struct v4l2_m2m_dev *m2m_dev)
> > +{
> > +       kref_get(&m2m_dev->kref);
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_m2m_get);
> > +
> > +static void v4l2_m2m_release_from_kref(struct kref *kref)
> > +{
> > +       struct v4l2_m2m_dev *m2m_dev = container_of(kref, struct
> > v4l2_m2m_dev, kref);
> > +
> > +       v4l2_m2m_release(m2m_dev);
> > +}
> > +
> > +void v4l2_m2m_put(struct v4l2_m2m_dev *m2m_dev)
> > +{
> > +       kref_put(&m2m_dev->kref, v4l2_m2m_release_from_kref);
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_m2m_put);
> > +
> >  struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
> >                 void *drv_priv,
> >                 int (*queue_init)(void *priv, struct vb2_queue *src_vq,
> > struct vb2_queue *dst_vq))
> > diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> > index bf6a09a04dcf..ca295c660c7f 100644
> > --- a/include/media/v4l2-mem2mem.h
> > +++ b/include/media/v4l2-mem2mem.h
> > @@ -547,6 +547,27 @@ v4l2_m2m_register_media_controller(struct v4l2_m2m_dev
> > *m2m_dev,
> >   */
> >  void v4l2_m2m_release(struct v4l2_m2m_dev *m2m_dev);
> > 
> > +/**
> > + * v4l2_m2m_get() - take a reference to the m2m_dev structure
> > + *
> > + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> > + *
> > + * This is used to share the M2M device across multiple devices. This
> > + * can be used to avoid scheduling two hardware nodes concurrently.
> > + */
> > +void v4l2_m2m_get(struct v4l2_m2m_dev *m2m_dev);
> > +
> > +/**
> > + * v4l2_m2m_put() - remove a reference to the m2m_dev structure
> > + *
> > + * @m2m_dev: opaque pointer to the internal data to handle M2M context
> > + *
> > + * Once the M2M device have no more references, v4l2_m2m_realse() will be
> > + * called automatically. Users of this method should never call
> > + * v4l2_m2m_release() directly. See v4l2_m2m_get() for more details.
> > + */
> > +void v4l2_m2m_put(struct v4l2_m2m_dev *m2m_dev);
> > +
> >  /**
> >   * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
> >   *
> > --
> > 2.52.0
> > 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on
  2025-12-30 13:42 ` [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on Ulf Hansson
  2025-12-31  5:37   ` Ming Qian(OSS)
@ 2026-01-05 18:10   ` Nicolas Dufresne
  1 sibling, 0 replies; 13+ messages in thread
From: Nicolas Dufresne @ 2026-01-05 18:10 UTC (permalink / raw)
  To: Ulf Hansson, ming.qian
  Cc: linux-media, mchehab, hverkuil-cisco, benjamin.gaignard, p.zabel,
	sebastian.fricke, shawnguo, s.hauer, kernel, festevam, linux-imx,
	l.stach, Frank.li, peng.fan, eagle.zhou, imx, linux-pm,
	linux-kernel, linux-arm-kernel

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

Hi,

Le mardi 30 décembre 2025 à 14:42 +0100, Ulf Hansson a écrit :
> On Fri, 5 Dec 2025 at 02:55, <ming.qian@oss.nxp.com> wrote:
> > 
> > From: Ming Qian <ming.qian@oss.nxp.com>
> > 
> > On the i.MX8MQ, we encountered some concurrency issues with H264 and HEVC
> > decoding.
> > 
> > There are two main reasons:
> > 1. The vpu blk-ctrl don't have separate reset and clock enable bits.
> > 2. The g1 VPU and g2 VPU cannot decode simultaneously.
> > 
> > We attempted to make corresponding fix to address these two issues.
> > 
> > Ming Qian (2):
> >   pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu
> >   media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC
> > 
> > Nicolas Dufresne (1):
> >   media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure
> > 
> >  drivers/media/platform/verisilicon/hantro.h   |  2 +
> >  .../media/platform/verisilicon/hantro_drv.c   | 42 +++++++++++++++++--
> >  .../media/platform/verisilicon/imx8m_vpu_hw.c |  8 ++++
> >  drivers/media/v4l2-core/v4l2-mem2mem.c        | 23 ++++++++++
> >  drivers/pmdomain/imx/imx8m-blk-ctrl.c         | 11 +++--
> >  include/media/v4l2-mem2mem.h                  | 21 ++++++++++
> >  6 files changed, 100 insertions(+), 7 deletions(-)
> > 
> 
> Can I pick the pmdomain patch separately from the others? Or does this
> need to go together?

I believe you did already, and its perfect, we have picked the rest in the media
tree now.

regards,
Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure
  2026-01-05 18:08     ` Nicolas Dufresne
@ 2026-01-06 13:28       ` Ulf Hansson
  0 siblings, 0 replies; 13+ messages in thread
From: Ulf Hansson @ 2026-01-06 13:28 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: ming.qian, linux-media, mchehab, hverkuil-cisco,
	benjamin.gaignard, p.zabel, sebastian.fricke, shawnguo, s.hauer,
	kernel, festevam, linux-imx, l.stach, Frank.li, peng.fan,
	eagle.zhou, imx, linux-pm, linux-kernel, linux-arm-kernel

On Mon, 5 Jan 2026 at 19:08, Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Hi,
>
> Le mardi 30 décembre 2025 à 14:39 +0100, Ulf Hansson a écrit :
> > On Fri, 5 Dec 2025 at 02:55, <ming.qian@oss.nxp.com> wrote:
> > >
> > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > >
> > > Adding a reference count to the v4l2_m2m_dev structure allow safely
> > > sharing it across multiple hardware nodes. This can be used to prevent
> > > running jobs concurrently on m2m cores that have some internal resource
> > > sharing.
> > >
> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> >
> > I certainly don't have the complete picture for how this needs to work.
> >
> > However, I was thinking that rather than using a kref and having to
> > share two specific functions to update it (v4l2_m2m_get|put), couldn't
> > we just use a device-link instead?
>
> The device link seems to be intended for power management, I'm effectively not
> familiar with it, so thanks for the information. However, my impression is that
> this is indented to describe power management dependencies on siblings, but this
> was done differently on this SoC (way before my time here), the DT seems to
> describe a block controller which is common to the two (defective) cores. So
> power management wise, its already handled.
>
> Be aware that my intent is to rename v4l2_m2m_dev into v4l2_m2m_sched. Its not a
> dev wrapper, but a helper object that will schedule work coming from N
> v4l2_m2m_ctx (per open() calls). Making this helper object ref counted makes the
> live-time management a lot simpler, and since its not a dev wrapper, this patch
> is not creating a second refcount for the same object.

Okay, fair enough. No objections from my side,

[...]

Kind regards
Uffe


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

end of thread, other threads:[~2026-01-06 13:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-05  1:54 [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on ming.qian
2025-12-05  1:54 ` [PATCH v4 1/3] media: v4l2-mem2mem: Add a kref to the v4l2_m2m_dev structure ming.qian
2025-12-05 15:08   ` Frank Li
2025-12-30 13:39   ` Ulf Hansson
2026-01-05 18:08     ` Nicolas Dufresne
2026-01-06 13:28       ` Ulf Hansson
2025-12-05  1:54 ` [PATCH v4 2/3] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu ming.qian
2026-01-03 10:02   ` Ulf Hansson
2025-12-05  1:54 ` [PATCH v4 3/3] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC ming.qian
2025-12-05 15:09   ` Frank Li
2025-12-30 13:42 ` [PATCH v4 0/3] Fix concurrency issues between G1 and G2 on Ulf Hansson
2025-12-31  5:37   ` Ming Qian(OSS)
2026-01-05 18:10   ` Nicolas Dufresne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox