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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <baec095da2b7b84be19b205b18e765f9a2305574.camel@ndufresne.ca>
Date: Mon, 24 Nov 2025 11:39:43 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Frank Li <Frank.li@....com>, "Ming Qian(OSS)" <ming.qian@....nxp.com>
Cc: linux-media@...r.kernel.org, mchehab@...nel.org,
 hverkuil-cisco@...all.nl, 	benjamin.gaignard@...labora.com,
 p.zabel@...gutronix.de, 	sebastian.fricke@...labora.com,
 shawnguo@...nel.org, ulf.hansson@...aro.org, 	s.hauer@...gutronix.de,
 kernel@...gutronix.de, festevam@...il.com, 	linux-imx@....com,
 l.stach@...gutronix.de, peng.fan@....com, eagle.zhou@....com, 
	imx@...ts.linux.dev, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, 	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] media: verisilicon: Avoid G2 bus error while
 decoding H.264 and HEVC

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@....nxp.com wrote:
> > > > From: Ming Qian <ming.qian@....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@....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
> > > > 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)