[PATCH 1/2] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu

[PATCH 1/2] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu

[ming.qian]

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

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>
---
 drivers/pmdomain/imx/imx8m-blk-ctrl.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
index 5c83e5599f1e..1f07ff041295 100644
--- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
+++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
@@ -852,16 +852,12 @@ static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[]
 		.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.34.1

[PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[ming.qian]

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 led 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.
Then this allows for successful multi-instance decoding of H.264 and HEVC.

Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
 drivers/media/platform/verisilicon/hantro.h   |  1 +
 .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
 .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
 3 files changed, 31 insertions(+)

diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
index e0fdc4535b2d..8baebf767d26 100644
--- a/drivers/media/platform/verisilicon/hantro.h
+++ b/drivers/media/platform/verisilicon/hantro.h
@@ -101,6 +101,7 @@ struct hantro_variant {
 	unsigned int double_buffer : 1;
 	unsigned int legacy_regs : 1;
 	unsigned int late_postproc : 1;
+	atomic_t *shared_resource;
 };
 
 /**
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index 60b95b5d8565..036ce43c9359 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <linux/videodev2.h>
 #include <linux/workqueue.h>
+#include <linux/iopoll.h>
 #include <media/v4l2-event.h>
 #include <media/v4l2-mem2mem.h>
 #include <media/videobuf2-core.h>
@@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
 
 	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
 
+	if (vpu->variant->shared_resource)
+		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
+
 	hantro_job_finish_no_pm(vpu, ctx, result);
 }
 
@@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx *ctx)
 			      msecs_to_jiffies(2000));
 }
 
+static int hantro_wait_shared_resource(struct hantro_dev *vpu)
+{
+	u32 data;
+	int ret;
+
+	if (!vpu->variant->shared_resource)
+		return 0;
+
+	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 * NSEC_PER_MSEC, false,
+				vpu->variant->shared_resource, 1, 0);
+	if (ret) {
+		dev_err(vpu->dev, "Failed to wait shared resource\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static void device_run(void *priv)
 {
 	struct hantro_ctx *ctx = priv;
 	struct vb2_v4l2_buffer *src, *dst;
 	int ret;
 
+	ret = hantro_wait_shared_resource(ctx->dev);
+	if (ret < 0)
+		goto err_cancel_job;
+
 	src = hantro_get_src_buf(ctx);
 	dst = hantro_get_dst_buf(ctx);
 
diff --git a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
index 5be0e2e76882..1b3a0bfbf890 100644
--- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
+++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
@@ -324,6 +324,8 @@ static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" };
 static const char * const imx8mq_g1_clk_names[] = { "g1" };
 static const char * const imx8mq_g2_clk_names[] = { "g2" };
 
+static atomic_t imx8mq_shared_resource = ATOMIC_INIT(1);
+
 const struct hantro_variant imx8mq_vpu_variant = {
 	.dec_fmts = imx8m_vpu_dec_fmts,
 	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
@@ -356,6 +358,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_resource = &imx8mq_shared_resource,
 };
 
 const struct hantro_variant imx8mq_vpu_g2_variant = {
@@ -371,6 +374,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_resource = &imx8mq_shared_resource,
 };
 
 const struct hantro_variant imx8mm_vpu_g1_variant = {
-- 
2.34.1

Re: [PATCH 1/2] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu

[Benjamin Gaignard]

Le 21/11/2025 à 09:19, ming.qian@oss.nxp.com a écrit :
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> 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>

Thanks for the patch.

Reviewed-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

> ---
>   drivers/pmdomain/imx/imx8m-blk-ctrl.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> index 5c83e5599f1e..1f07ff041295 100644
> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> @@ -852,16 +852,12 @@ static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[]
>   		.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),
>   	},
>   };
>   

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Benjamin Gaignard]

Le 21/11/2025 à 09:19, ming.qian@oss.nxp.com a écrit :
> 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 led 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.
> Then this allows for successful multi-instance decoding of H.264 and HEVC.
> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>

Thanks for the patch.

Reviewed-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>

> ---
>   drivers/media/platform/verisilicon/hantro.h   |  1 +
>   .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
>   .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
>   3 files changed, 31 insertions(+)
>
> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> index e0fdc4535b2d..8baebf767d26 100644
> --- a/drivers/media/platform/verisilicon/hantro.h
> +++ b/drivers/media/platform/verisilicon/hantro.h
> @@ -101,6 +101,7 @@ struct hantro_variant {
>   	unsigned int double_buffer : 1;
>   	unsigned int legacy_regs : 1;
>   	unsigned int late_postproc : 1;
> +	atomic_t *shared_resource;
>   };
>   
>   /**
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 60b95b5d8565..036ce43c9359 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -19,6 +19,7 @@
>   #include <linux/slab.h>
>   #include <linux/videodev2.h>
>   #include <linux/workqueue.h>
> +#include <linux/iopoll.h>
>   #include <media/v4l2-event.h>
>   #include <media/v4l2-mem2mem.h>
>   #include <media/videobuf2-core.h>
> @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
>   
>   	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>   
> +	if (vpu->variant->shared_resource)
> +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
> +
>   	hantro_job_finish_no_pm(vpu, ctx, result);
>   }
>   
> @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx *ctx)
>   			      msecs_to_jiffies(2000));
>   }
>   
> +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
> +{
> +	u32 data;
> +	int ret;
> +
> +	if (!vpu->variant->shared_resource)
> +		return 0;
> +
> +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 * NSEC_PER_MSEC, false,
> +				vpu->variant->shared_resource, 1, 0);
> +	if (ret) {
> +		dev_err(vpu->dev, "Failed to wait shared resource\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>   static void device_run(void *priv)
>   {
>   	struct hantro_ctx *ctx = priv;
>   	struct vb2_v4l2_buffer *src, *dst;
>   	int ret;
>   
> +	ret = hantro_wait_shared_resource(ctx->dev);
> +	if (ret < 0)
> +		goto err_cancel_job;
> +
>   	src = hantro_get_src_buf(ctx);
>   	dst = hantro_get_dst_buf(ctx);
>   
> diff --git a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> index 5be0e2e76882..1b3a0bfbf890 100644
> --- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> @@ -324,6 +324,8 @@ static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" };
>   static const char * const imx8mq_g1_clk_names[] = { "g1" };
>   static const char * const imx8mq_g2_clk_names[] = { "g2" };
>   
> +static atomic_t imx8mq_shared_resource = ATOMIC_INIT(1);
> +
>   const struct hantro_variant imx8mq_vpu_variant = {
>   	.dec_fmts = imx8m_vpu_dec_fmts,
>   	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
> @@ -356,6 +358,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_resource = &imx8mq_shared_resource,
>   };
>   
>   const struct hantro_variant imx8mq_vpu_g2_variant = {
> @@ -371,6 +374,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_resource = &imx8mq_shared_resource,
>   };
>   
>   const struct hantro_variant imx8mm_vpu_g1_variant = {

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Frank Li]

On Fri, Nov 21, 2025 at 04:19:09PM +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 led 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.
> Then this allows for successful multi-instance decoding of H.264 and HEVC.
>
> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> ---
>  drivers/media/platform/verisilicon/hantro.h   |  1 +
>  .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
>  .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
>  3 files changed, 31 insertions(+)
>
...
>  #include <linux/workqueue.h>
> +#include <linux/iopoll.h>
>  #include <media/v4l2-event.h>
>  #include <media/v4l2-mem2mem.h>
>  #include <media/videobuf2-core.h>
> @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
>
>  	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>
> +	if (vpu->variant->shared_resource)
> +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
> +
>  	hantro_job_finish_no_pm(vpu, ctx, result);
>  }
>
> @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx *ctx)
>  			      msecs_to_jiffies(2000));
>  }
>
> +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
> +{
> +	u32 data;
> +	int ret;
> +
> +	if (!vpu->variant->shared_resource)
> +		return 0;
> +
> +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 * NSEC_PER_MSEC, false,
> +				vpu->variant->shared_resource, 1, 0);
> +	if (ret) {
> +		dev_err(vpu->dev, "Failed to wait shared resource\n");
> +		return -EINVAL;
> +	}

why not use a mutex?

mutex() lock here, unlock at hantro_job_finish(), if second instance
run to here, mutex() will block thread, until previous hantro_job_finish()
finish.

Frank
> +
> +	return 0;
> +}
> +
>  static void device_run(void *priv)
>  {
>  	struct hantro_ctx *ctx = priv;
>  	struct vb2_v4l2_buffer *src, *dst;
>  	int ret;
>
> +	ret = hantro_wait_shared_resource(ctx->dev);
> +	if (ret < 0)
> +		goto err_cancel_job;
> +
>  	src = hantro_get_src_buf(ctx);
>  	dst = hantro_get_dst_buf(ctx);
...

>
> --
> 2.34.1
>

Re: [PATCH 1/2] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu

[Frank Li]

On Fri, Nov 21, 2025 at 04:19:08PM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> 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>
> ---
>  drivers/pmdomain/imx/imx8m-blk-ctrl.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> index 5c83e5599f1e..1f07ff041295 100644
> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> @@ -852,16 +852,12 @@ static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[]
>  		.clk_names = (const char *[]){ "g1", },
>  		.num_clks = 1,
>  		.gpc_name = "g1",
> -		.rst_mask = BIT(1),
> -		.clk_mask = BIT(1),

Does this bit not exist or just put VPU's reset bit here previously?

Frank
>  	},
>  	[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.34.1
>

Re: [PATCH 1/2] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu

[Nicolas Dufresne]

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

Hi Ming,

thanks a lot for working on this.

Le vendredi 21 novembre 2025 à 16:19 +0800, ming.qian@oss.nxp.com a écrit :
> From: Ming Qian <ming.qian@oss.nxp.com>
> 
> 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>
> ---
>  drivers/pmdomain/imx/imx8m-blk-ctrl.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> index 5c83e5599f1e..1f07ff041295 100644
> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> @@ -852,16 +852,12 @@ static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[]
>  		.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),
>  	},

That was also our impression, but we could not get information about this HW.
One question here, how do we ensure that we don't reset twice on power on ?

Nicolas

>  };
>  

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

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Ming Qian(OSS)]

Hi Frank,

On 11/22/2025 12:08 AM, Frank Li wrote:
> On Fri, Nov 21, 2025 at 04:19:09PM +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 led 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.
>> Then this allows for successful multi-instance decoding of H.264 and HEVC.
>>
>> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>>   drivers/media/platform/verisilicon/hantro.h   |  1 +
>>   .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
>>   .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
>>   3 files changed, 31 insertions(+)
>>
> ...
>>   #include <linux/workqueue.h>
>> +#include <linux/iopoll.h>
>>   #include <media/v4l2-event.h>
>>   #include <media/v4l2-mem2mem.h>
>>   #include <media/videobuf2-core.h>
>> @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
>>
>>   	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>>
>> +	if (vpu->variant->shared_resource)
>> +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
>> +
>>   	hantro_job_finish_no_pm(vpu, ctx, result);
>>   }
>>
>> @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx *ctx)
>>   			      msecs_to_jiffies(2000));
>>   }
>>
>> +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
>> +{
>> +	u32 data;
>> +	int ret;
>> +
>> +	if (!vpu->variant->shared_resource)
>> +		return 0;
>> +
>> +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 * NSEC_PER_MSEC, false,
>> +				vpu->variant->shared_resource, 1, 0);
>> +	if (ret) {
>> +		dev_err(vpu->dev, "Failed to wait shared resource\n");
>> +		return -EINVAL;
>> +	}
> 
> why not use a mutex?
> 
> mutex() lock here, unlock at hantro_job_finish(), if second instance
> run to here, mutex() will block thread, until previous hantro_job_finish()
> finish.
> 
> Frank

G1 and G2 are two different devices. If I were to use a mutex, I would
need to define a global mutex. Therefore, to avoid using a global mutex,
I only define a static atomic variable.

If a static mutex is acceptable, I think I can change it to a mutex.

Regards,
Ming

>> +
>> +	return 0;
>> +}
>> +
>>   static void device_run(void *priv)
>>   {
>>   	struct hantro_ctx *ctx = priv;
>>   	struct vb2_v4l2_buffer *src, *dst;
>>   	int ret;
>>
>> +	ret = hantro_wait_shared_resource(ctx->dev);
>> +	if (ret < 0)
>> +		goto err_cancel_job;
>> +
>>   	src = hantro_get_src_buf(ctx);
>>   	dst = hantro_get_dst_buf(ctx);
> ...
> 
>>
>> --
>> 2.34.1
>>

Re: [PATCH 1/2] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu

[Ming Qian(OSS)]

Hi Frank,

On 11/22/2025 12:11 AM, Frank Li wrote:
> On Fri, Nov 21, 2025 at 04:19:08PM +0800, ming.qian@oss.nxp.com wrote:
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> 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>
>> ---
>>   drivers/pmdomain/imx/imx8m-blk-ctrl.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
>> index 5c83e5599f1e..1f07ff041295 100644
>> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
>> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
>> @@ -852,16 +852,12 @@ static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[]
>>   		.clk_names = (const char *[]){ "g1", },
>>   		.num_clks = 1,
>>   		.gpc_name = "g1",
>> -		.rst_mask = BIT(1),
>> -		.clk_mask = BIT(1),
> 
> Does this bit not exist or just put VPU's reset bit here previously?
> 
> Frank

In NXP's internal VPU solution of i.MX8MQ, this vpu-blk-ctrl is not used 
directly.
The internal driver always reset the G1 and G2 VPU together.
Just like imx8mq_vpu_power_notifier() does.

Regards,
Ming

>>   	},
>>   	[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.34.1
>>

Re: [PATCH 1/2] pmdomain: imx8m-blk-ctrl: Remove separate rst and clk mask for 8mq vpu

[Ming Qian(OSS)]

Hi Nicolas,

On 11/22/2025 2:07 AM, Nicolas Dufresne wrote:
> Hi Ming,
> 
> thanks a lot for working on this.
> 
> Le vendredi 21 novembre 2025 à 16:19 +0800, ming.qian@oss.nxp.com a écrit :
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> 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>
>> ---
>>   drivers/pmdomain/imx/imx8m-blk-ctrl.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
>> index 5c83e5599f1e..1f07ff041295 100644
>> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
>> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
>> @@ -852,16 +852,12 @@ static const struct imx8m_blk_ctrl_domain_data imx8mq_vpu_blk_ctl_domain_data[]
>>   		.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),
>>   	},
> 
> That was also our impression, but we could not get information about this HW.
> One question here, how do we ensure that we don't reset twice on power on ?
> 
> Nicolas

The imx8mq_vpu_power_notifier() that is the power notifier callback of
imx8mq_vpu_blk_ctl_dev_data will do the real reset of G1 and G2.
	/*
	 * The ADB in the VPUMIX domain has no separate reset and clock
	 * enable bits, but is ungated and reset together with the VPUs. The
	 * reset and clock enable inputs to the ADB is a logical OR of the
	 * VPU bits. In order to set the G2 fuse bits, the G2 clock must
	 * also be enabled.
	 */
	regmap_set_bits(bc->regmap, BLK_SFT_RSTN, BIT(0) | BIT(1));
	regmap_set_bits(bc->regmap, BLK_CLK_EN, BIT(0) | BIT(1));

Thie notifier callback is set to bus power dev of vpu-blk-ctrl.

If we remove the reset mask from the G1/G2 blk-ctrl domain, then the VPU
is only set when the bus power domain power on.

I think the bus power domain can ensure that reset will not be executed
twice.

Regards,
Ming


> 
>>   };
>>   

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Frank Li]

On Mon, Nov 24, 2025 at 09:38:15AM +0800, Ming Qian(OSS) wrote:
> Hi Frank,
>
> On 11/22/2025 12:08 AM, Frank Li wrote:
> > On Fri, Nov 21, 2025 at 04:19:09PM +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 led 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.
> > > Then this allows for successful multi-instance decoding of H.264 and HEVC.
> > >
> > > Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> > > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> > > ---
> > >   drivers/media/platform/verisilicon/hantro.h   |  1 +
> > >   .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
> > >   .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
> > >   3 files changed, 31 insertions(+)
> > >
> > ...
> > >   #include <linux/workqueue.h>
> > > +#include <linux/iopoll.h>
> > >   #include <media/v4l2-event.h>
> > >   #include <media/v4l2-mem2mem.h>
> > >   #include <media/videobuf2-core.h>
> > > @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
> > >
> > >   	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
> > >
> > > +	if (vpu->variant->shared_resource)
> > > +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
> > > +
> > >   	hantro_job_finish_no_pm(vpu, ctx, result);
> > >   }
> > >
> > > @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx *ctx)
> > >   			      msecs_to_jiffies(2000));
> > >   }
> > >
> > > +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
> > > +{
> > > +	u32 data;
> > > +	int ret;
> > > +
> > > +	if (!vpu->variant->shared_resource)
> > > +		return 0;
> > > +
> > > +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 * NSEC_PER_MSEC, false,
> > > +				vpu->variant->shared_resource, 1, 0);
> > > +	if (ret) {
> > > +		dev_err(vpu->dev, "Failed to wait shared resource\n");
> > > +		return -EINVAL;
> > > +	}
> >
> > why not use a mutex?
> >
> > mutex() lock here, unlock at hantro_job_finish(), if second instance
> > run to here, mutex() will block thread, until previous hantro_job_finish()
> > finish.
> >
> > Frank
>
> G1 and G2 are two different devices. If I were to use a mutex, I would
> need to define a global mutex. Therefore, to avoid using a global mutex,
> I only define a static atomic variable.

static atomic varible also is global.  Global mutex is allowed if it is
really needed.

>
> If a static mutex is acceptable, I think I can change it to a mutex.

ref to
https://elixir.bootlin.com/linux/v6.18-rc6/source/drivers/base/core.c#L43

Frank
>
> Regards,
> Ming
>
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   static void device_run(void *priv)
> > >   {
> > >   	struct hantro_ctx *ctx = priv;
> > >   	struct vb2_v4l2_buffer *src, *dst;
> > >   	int ret;
> > >
> > > +	ret = hantro_wait_shared_resource(ctx->dev);
> > > +	if (ret < 0)
> > > +		goto err_cancel_job;
> > > +
> > >   	src = hantro_get_src_buf(ctx);
> > >   	dst = hantro_get_dst_buf(ctx);
> > ...
> >
> > >
> > > --
> > > 2.34.1
> > >

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Nicolas Dufresne]

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

Hi,

Le lundi 24 novembre 2025 à 10:49 -0500, Frank Li a écrit :
> On Mon, Nov 24, 2025 at 09:38:15AM +0800, Ming Qian(OSS) wrote:
> > Hi Frank,
> > 
> > On 11/22/2025 12:08 AM, Frank Li wrote:
> > > On Fri, Nov 21, 2025 at 04:19:09PM +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 led 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.
> > > > Then this allows for successful multi-instance decoding of H.264 and
> > > > HEVC.
> > > > 
> > > > Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> > > > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> > > > ---
> > > >   drivers/media/platform/verisilicon/hantro.h   |  1 +
> > > >   .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
> > > >   .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
> > > >   3 files changed, 31 insertions(+)
> > > > 
> > > ...
> > > >   #include <linux/workqueue.h>
> > > > +#include <linux/iopoll.h>
> > > >   #include <media/v4l2-event.h>
> > > >   #include <media/v4l2-mem2mem.h>
> > > >   #include <media/videobuf2-core.h>
> > > > @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
> > > > 
> > > >   	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
> > > > 
> > > > +	if (vpu->variant->shared_resource)
> > > > +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
> > > > +
> > > >   	hantro_job_finish_no_pm(vpu, ctx, result);
> > > >   }
> > > > 
> > > > @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx
> > > > *ctx)
> > > >   			      msecs_to_jiffies(2000));
> > > >   }
> > > > 
> > > > +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
> > > > +{
> > > > +	u32 data;
> > > > +	int ret;
> > > > +
> > > > +	if (!vpu->variant->shared_resource)
> > > > +		return 0;
> > > > +
> > > > +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 *
> > > > NSEC_PER_MSEC, false,
> > > > +				vpu->variant->shared_resource, 1, 0);
> > > > +	if (ret) {
> > > > +		dev_err(vpu->dev, "Failed to wait shared resource\n");
> > > > +		return -EINVAL;
> > > > +	}
> > > 
> > > why not use a mutex?
> > > 
> > > mutex() lock here, unlock at hantro_job_finish(), if second instance
> > > run to here, mutex() will block thread, until previous hantro_job_finish()
> > > finish.
> > > 
> > > Frank
> > 
> > G1 and G2 are two different devices. If I were to use a mutex, I would
> > need to define a global mutex. Therefore, to avoid using a global mutex,
> > I only define a static atomic variable.
> 
> static atomic varible also is global.  Global mutex is allowed if it is
> really needed.
> 
> > 
> > If a static mutex is acceptable, I think I can change it to a mutex.
> 
> ref to
> https://elixir.bootlin.com/linux/v6.18-rc6/source/drivers/base/core.c#L43

My main concern with either of these approaches is that it kills the ability to
rmmod the driver forever. The only working approach would be that both drivers
depends on a third driver that provide the synchronization services.

Nicolas

> 
> Frank
> > 
> > Regards,
> > Ming
> > 
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > >   static void device_run(void *priv)
> > > >   {
> > > >   	struct hantro_ctx *ctx = priv;
> > > >   	struct vb2_v4l2_buffer *src, *dst;
> > > >   	int ret;
> > > > 
> > > > +	ret = hantro_wait_shared_resource(ctx->dev);
> > > > +	if (ret < 0)
> > > > +		goto err_cancel_job;
> > > > +
> > > >   	src = hantro_get_src_buf(ctx);
> > > >   	dst = hantro_get_dst_buf(ctx);
> > > ...
> > > 
> > > > 
> > > > --
> > > > 2.34.1
> > > > 

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

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Nicolas Dufresne]

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

Le lundi 24 novembre 2025 à 11:39 -0500, Nicolas Dufresne a écrit :
> Hi,
> 
> Le lundi 24 novembre 2025 à 10:49 -0500, Frank Li a écrit :
> > On Mon, Nov 24, 2025 at 09:38:15AM +0800, Ming Qian(OSS) wrote:
> > > Hi Frank,
> > > 
> > > On 11/22/2025 12:08 AM, Frank Li wrote:
> > > > On Fri, Nov 21, 2025 at 04:19:09PM +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 led 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.
> > > > > Then this allows for successful multi-instance decoding of H.264 and
> > > > > HEVC.
> > > > > 
> > > > > Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> > > > > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> > > > > ---
> > > > >   drivers/media/platform/verisilicon/hantro.h   |  1 +
> > > > >   .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
> > > > >   .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
> > > > >   3 files changed, 31 insertions(+)
> > > > > 
> > > > ...
> > > > >   #include <linux/workqueue.h>
> > > > > +#include <linux/iopoll.h>
> > > > >   #include <media/v4l2-event.h>
> > > > >   #include <media/v4l2-mem2mem.h>
> > > > >   #include <media/videobuf2-core.h>
> > > > > @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
> > > > > 
> > > > >   	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
> > > > > 
> > > > > +	if (vpu->variant->shared_resource)
> > > > > +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
> > > > > +
> > > > >   	hantro_job_finish_no_pm(vpu, ctx, result);
> > > > >   }
> > > > > 
> > > > > @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx
> > > > > *ctx)
> > > > >   			      msecs_to_jiffies(2000));
> > > > >   }
> > > > > 
> > > > > +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
> > > > > +{
> > > > > +	u32 data;
> > > > > +	int ret;
> > > > > +
> > > > > +	if (!vpu->variant->shared_resource)
> > > > > +		return 0;
> > > > > +
> > > > > +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 *
> > > > > NSEC_PER_MSEC, false,
> > > > > +				vpu->variant->shared_resource, 1, 0);
> > > > > +	if (ret) {
> > > > > +		dev_err(vpu->dev, "Failed to wait shared resource\n");
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > 
> > > > why not use a mutex?
> > > > 
> > > > mutex() lock here, unlock at hantro_job_finish(), if second instance
> > > > run to here, mutex() will block thread, until previous hantro_job_finish()
> > > > finish.
> > > > 
> > > > Frank
> > > 
> > > G1 and G2 are two different devices. If I were to use a mutex, I would
> > > need to define a global mutex. Therefore, to avoid using a global mutex,
> > > I only define a static atomic variable.
> > 
> > static atomic varible also is global.  Global mutex is allowed if it is
> > really needed.
> > 
> > > 
> > > If a static mutex is acceptable, I think I can change it to a mutex.
> > 
> > ref to
> > https://elixir.bootlin.com/linux/v6.18-rc6/source/drivers/base/core.c#L43
> 
> My main concern with either of these approaches is that it kills the ability to
> rmmod the driver forever. The only working approach would be that both drivers
> depends on a third driver that provide the synchronization services.

I do realize after the fact that my answer is a little off considering its a
drivers against itself (not cross-driver, that would be a huge pain if it was
the case).

Checking further, the ref to the counter (or mutex) should cleanly be gone by
the time the driver is removed, so perhaps its fine, though best to test it.
Though, in both cases, I'm not happy to see code that will wait for multiple
milliseconds on either home made mutex or a real mutex. Adding another arbitrary
timeout is also not very nice either. The current software watchdog already get
in the way when testing simulated IP.

I know its work, but what about a recounted singleton, with a notifier so we can
schedule work when the resource is free ?

Nicolas

> 
> Nicolas
> 
> > 
> > Frank
> > > 
> > > Regards,
> > > Ming
> > > 
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > >   static void device_run(void *priv)
> > > > >   {
> > > > >   	struct hantro_ctx *ctx = priv;
> > > > >   	struct vb2_v4l2_buffer *src, *dst;
> > > > >   	int ret;
> > > > > 
> > > > > +	ret = hantro_wait_shared_resource(ctx->dev);
> > > > > +	if (ret < 0)
> > > > > +		goto err_cancel_job;
> > > > > +
> > > > >   	src = hantro_get_src_buf(ctx);
> > > > >   	dst = hantro_get_dst_buf(ctx);
> > > > ...
> > > > 
> > > > > 
> > > > > --
> > > > > 2.34.1
> > > > > 

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

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Lucas Stach]

Hi Ming,

Am Freitag, dem 21.11.2025 um 16:19 +0800 schrieb
ming.qian@oss.nxp.com:
> 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 led 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.
> Then this allows for successful multi-instance decoding of H.264 and HEVC.
> 
Is there any more info available on what's actually causing the hang?
Is there some kind of overflow of the interconnect request capacity?

I'm asking to understand the issue a bit better, as locking both
decoder instances against each other seems like a pretty big hammer.

Regards,
Lucas

> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> ---
>  drivers/media/platform/verisilicon/hantro.h   |  1 +
>  .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
>  .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> index e0fdc4535b2d..8baebf767d26 100644
> --- a/drivers/media/platform/verisilicon/hantro.h
> +++ b/drivers/media/platform/verisilicon/hantro.h
> @@ -101,6 +101,7 @@ struct hantro_variant {
>  	unsigned int double_buffer : 1;
>  	unsigned int legacy_regs : 1;
>  	unsigned int late_postproc : 1;
> +	atomic_t *shared_resource;
>  };
>  
>  /**
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 60b95b5d8565..036ce43c9359 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -19,6 +19,7 @@
>  #include <linux/slab.h>
>  #include <linux/videodev2.h>
>  #include <linux/workqueue.h>
> +#include <linux/iopoll.h>
>  #include <media/v4l2-event.h>
>  #include <media/v4l2-mem2mem.h>
>  #include <media/videobuf2-core.h>
> @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
>  
>  	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>  
> +	if (vpu->variant->shared_resource)
> +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
> +
>  	hantro_job_finish_no_pm(vpu, ctx, result);
>  }
>  
> @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx *ctx)
>  			      msecs_to_jiffies(2000));
>  }
>  
> +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
> +{
> +	u32 data;
> +	int ret;
> +
> +	if (!vpu->variant->shared_resource)
> +		return 0;
> +
> +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 * NSEC_PER_MSEC, false,
> +				vpu->variant->shared_resource, 1, 0);
> +	if (ret) {
> +		dev_err(vpu->dev, "Failed to wait shared resource\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static void device_run(void *priv)
>  {
>  	struct hantro_ctx *ctx = priv;
>  	struct vb2_v4l2_buffer *src, *dst;
>  	int ret;
>  
> +	ret = hantro_wait_shared_resource(ctx->dev);
> +	if (ret < 0)
> +		goto err_cancel_job;
> +
>  	src = hantro_get_src_buf(ctx);
>  	dst = hantro_get_dst_buf(ctx);
>  
> diff --git a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> index 5be0e2e76882..1b3a0bfbf890 100644
> --- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> @@ -324,6 +324,8 @@ static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" };
>  static const char * const imx8mq_g1_clk_names[] = { "g1" };
>  static const char * const imx8mq_g2_clk_names[] = { "g2" };
>  
> +static atomic_t imx8mq_shared_resource = ATOMIC_INIT(1);
> +
>  const struct hantro_variant imx8mq_vpu_variant = {
>  	.dec_fmts = imx8m_vpu_dec_fmts,
>  	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
> @@ -356,6 +358,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_resource = &imx8mq_shared_resource,
>  };
>  
>  const struct hantro_variant imx8mq_vpu_g2_variant = {
> @@ -371,6 +374,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_resource = &imx8mq_shared_resource,
>  };
>  
>  const struct hantro_variant imx8mm_vpu_g1_variant = {

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Ming Qian(OSS)]

Hi Nicolas and Frank,

On 11/25/2025 12:55 AM, Nicolas Dufresne wrote:
> Le lundi 24 novembre 2025 à 11:39 -0500, Nicolas Dufresne a écrit :
>> Hi,
>>
>> Le lundi 24 novembre 2025 à 10:49 -0500, Frank Li a écrit :
>>> On Mon, Nov 24, 2025 at 09:38:15AM +0800, Ming Qian(OSS) wrote:
>>>> Hi Frank,
>>>>
>>>> On 11/22/2025 12:08 AM, Frank Li wrote:
>>>>> On Fri, Nov 21, 2025 at 04:19:09PM +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 led 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.
>>>>>> Then this allows for successful multi-instance decoding of H.264 and
>>>>>> HEVC.
>>>>>>
>>>>>> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
>>>>>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>>>>>> ---
>>>>>>    drivers/media/platform/verisilicon/hantro.h   |  1 +
>>>>>>    .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
>>>>>>    .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
>>>>>>    3 files changed, 31 insertions(+)
>>>>>>
>>>>> ...
>>>>>>    #include <linux/workqueue.h>
>>>>>> +#include <linux/iopoll.h>
>>>>>>    #include <media/v4l2-event.h>
>>>>>>    #include <media/v4l2-mem2mem.h>
>>>>>>    #include <media/videobuf2-core.h>
>>>>>> @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
>>>>>>
>>>>>>    	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>>>>>>
>>>>>> +	if (vpu->variant->shared_resource)
>>>>>> +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
>>>>>> +
>>>>>>    	hantro_job_finish_no_pm(vpu, ctx, result);
>>>>>>    }
>>>>>>
>>>>>> @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx
>>>>>> *ctx)
>>>>>>    			      msecs_to_jiffies(2000));
>>>>>>    }
>>>>>>
>>>>>> +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
>>>>>> +{
>>>>>> +	u32 data;
>>>>>> +	int ret;
>>>>>> +
>>>>>> +	if (!vpu->variant->shared_resource)
>>>>>> +		return 0;
>>>>>> +
>>>>>> +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 *
>>>>>> NSEC_PER_MSEC, false,
>>>>>> +				vpu->variant->shared_resource, 1, 0);
>>>>>> +	if (ret) {
>>>>>> +		dev_err(vpu->dev, "Failed to wait shared resource\n");
>>>>>> +		return -EINVAL;
>>>>>> +	}
>>>>>
>>>>> why not use a mutex?
>>>>>
>>>>> mutex() lock here, unlock at hantro_job_finish(), if second instance
>>>>> run to here, mutex() will block thread, until previous hantro_job_finish()
>>>>> finish.
>>>>>
>>>>> Frank
>>>>
>>>> G1 and G2 are two different devices. If I were to use a mutex, I would
>>>> need to define a global mutex. Therefore, to avoid using a global mutex,
>>>> I only define a static atomic variable.
>>>
>>> static atomic varible also is global.  Global mutex is allowed if it is
>>> really needed.
>>>
>>>>
>>>> If a static mutex is acceptable, I think I can change it to a mutex.
>>>
>>> ref to
>>> https://elixir.bootlin.com/linux/v6.18-rc6/source/drivers/base/core.c#L43
>>
>> My main concern with either of these approaches is that it kills the ability to
>> rmmod the driver forever. The only working approach would be that both drivers
>> depends on a third driver that provide the synchronization services.
> 
> I do realize after the fact that my answer is a little off considering its a
> drivers against itself (not cross-driver, that would be a huge pain if it was
> the case).
> 
> Checking further, the ref to the counter (or mutex) should cleanly be gone by
> the time the driver is removed, so perhaps its fine, though best to test it.
> Though, in both cases, I'm not happy to see code that will wait for multiple
> milliseconds on either home made mutex or a real mutex. Adding another arbitrary
> timeout is also not very nice either. The current software watchdog already get
> in the way when testing simulated IP.
> 
> I know its work, but what about a recounted singleton, with a notifier so we can
> schedule work when the resource is free ?
> 
> Nicolas
> 

Thank you for your comments. I will consider a better solution.

Regards,
Ming

>>
>> Nicolas
>>
>>>
>>> Frank
>>>>
>>>> Regards,
>>>> Ming
>>>>
>>>>>> +
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +
>>>>>>    static void device_run(void *priv)
>>>>>>    {
>>>>>>    	struct hantro_ctx *ctx = priv;
>>>>>>    	struct vb2_v4l2_buffer *src, *dst;
>>>>>>    	int ret;
>>>>>>
>>>>>> +	ret = hantro_wait_shared_resource(ctx->dev);
>>>>>> +	if (ret < 0)
>>>>>> +		goto err_cancel_job;
>>>>>> +
>>>>>>    	src = hantro_get_src_buf(ctx);
>>>>>>    	dst = hantro_get_dst_buf(ctx);
>>>>> ...
>>>>>
>>>>>>
>>>>>> --
>>>>>> 2.34.1
>>>>>>

Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while decoding H.264 and HEVC

[Ming Qian(OSS)]

Hi Lucas,

On 11/25/2025 1:42 AM, Lucas Stach wrote:
> Hi Ming,
> 
> Am Freitag, dem 21.11.2025 um 16:19 +0800 schrieb
> ming.qian@oss.nxp.com:
>> 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 led 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.
>> Then this allows for successful multi-instance decoding of H.264 and HEVC.
>>
> Is there any more info available on what's actually causing the hang?
> Is there some kind of overflow of the interconnect request capacity?
> 
> I'm asking to understand the issue a bit better, as locking both
> decoder instances against each other seems like a pretty big hammer.
> 
> Regards,
> Lucas
> 

In NXP's internal VPU solution, i.MX8MQ combines g1 and g2 into a single
device for processing. However, i.MX8MP separates g1 and g2 into two
separate devices.
However, I cannot find relevant documentation.
I can only guess that there are some hardware signals on the i.MX8MQ
that might interfere with each other, such as G1 and G2 VPU.

We recently encountered this problem when trying to switch to upstream
device nodes. So we tried applying our previous strategy.

I agree that waiting or locking is not nice enough, I will try a better
solution.

Regards,
Ming

>> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>>   drivers/media/platform/verisilicon/hantro.h   |  1 +
>>   .../media/platform/verisilicon/hantro_drv.c   | 26 +++++++++++++++++++
>>   .../media/platform/verisilicon/imx8m_vpu_hw.c |  4 +++
>>   3 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
>> index e0fdc4535b2d..8baebf767d26 100644
>> --- a/drivers/media/platform/verisilicon/hantro.h
>> +++ b/drivers/media/platform/verisilicon/hantro.h
>> @@ -101,6 +101,7 @@ struct hantro_variant {
>>   	unsigned int double_buffer : 1;
>>   	unsigned int legacy_regs : 1;
>>   	unsigned int late_postproc : 1;
>> +	atomic_t *shared_resource;
>>   };
>>   
>>   /**
>> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
>> index 60b95b5d8565..036ce43c9359 100644
>> --- a/drivers/media/platform/verisilicon/hantro_drv.c
>> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/slab.h>
>>   #include <linux/videodev2.h>
>>   #include <linux/workqueue.h>
>> +#include <linux/iopoll.h>
>>   #include <media/v4l2-event.h>
>>   #include <media/v4l2-mem2mem.h>
>>   #include <media/videobuf2-core.h>
>> @@ -93,6 +94,9 @@ static void hantro_job_finish(struct hantro_dev *vpu,
>>   
>>   	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>>   
>> +	if (vpu->variant->shared_resource)
>> +		atomic_cmpxchg(vpu->variant->shared_resource, 0, 1);
>> +
>>   	hantro_job_finish_no_pm(vpu, ctx, result);
>>   }
>>   
>> @@ -166,12 +170,34 @@ void hantro_end_prepare_run(struct hantro_ctx *ctx)
>>   			      msecs_to_jiffies(2000));
>>   }
>>   
>> +static int hantro_wait_shared_resource(struct hantro_dev *vpu)
>> +{
>> +	u32 data;
>> +	int ret;
>> +
>> +	if (!vpu->variant->shared_resource)
>> +		return 0;
>> +
>> +	ret = read_poll_timeout(atomic_cmpxchg, data, data, 10, 300 * NSEC_PER_MSEC, false,
>> +				vpu->variant->shared_resource, 1, 0);
>> +	if (ret) {
>> +		dev_err(vpu->dev, "Failed to wait shared resource\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static void device_run(void *priv)
>>   {
>>   	struct hantro_ctx *ctx = priv;
>>   	struct vb2_v4l2_buffer *src, *dst;
>>   	int ret;
>>   
>> +	ret = hantro_wait_shared_resource(ctx->dev);
>> +	if (ret < 0)
>> +		goto err_cancel_job;
>> +
>>   	src = hantro_get_src_buf(ctx);
>>   	dst = hantro_get_dst_buf(ctx);
>>   
>> diff --git a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
>> index 5be0e2e76882..1b3a0bfbf890 100644
>> --- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
>> +++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
>> @@ -324,6 +324,8 @@ static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" };
>>   static const char * const imx8mq_g1_clk_names[] = { "g1" };
>>   static const char * const imx8mq_g2_clk_names[] = { "g2" };
>>   
>> +static atomic_t imx8mq_shared_resource = ATOMIC_INIT(1);
>> +
>>   const struct hantro_variant imx8mq_vpu_variant = {
>>   	.dec_fmts = imx8m_vpu_dec_fmts,
>>   	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
>> @@ -356,6 +358,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_resource = &imx8mq_shared_resource,
>>   };
>>   
>>   const struct hantro_variant imx8mq_vpu_g2_variant = {
>> @@ -371,6 +374,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_resource = &imx8mq_shared_resource,
>>   };
>>   
>>   const struct hantro_variant imx8mm_vpu_g1_variant = {
>