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

["Ming Qian(OSS)" <ming.qian@oss.nxp.com>]

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 = {
>