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: <aSR+eaVxBhnahOl7@lizhi-Precision-Tower-5810>
Date: Mon, 24 Nov 2025 10:49:13 -0500
From: Frank Li <Frank.li@....com>
To: "Ming Qian(OSS)" <ming.qian@....nxp.com>
Cc: linux-media@...r.kernel.org, mchehab@...nel.org,
	hverkuil-cisco@...all.nl, nicolas@...fresne.ca,
	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

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

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