[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SLXP216MB11482E2C69022D56A2DE42CFED09A@SLXP216MB1148.KORP216.PROD.OUTLOOK.COM>
Date: Thu, 11 Sep 2025 02:04:41 +0000
From: jackson.lee <jackson.lee@...psnmedia.com>
To: Nicolas Dufresne <nicolas.dufresne@...labora.com>, "mchehab@...nel.org"
<mchehab@...nel.org>, "hverkuil-cisco@...all.nl" <hverkuil-cisco@...all.nl>,
"bob.beckett@...labora.com" <bob.beckett@...labora.com>
CC: "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, lafley.kim
<lafley.kim@...psnmedia.com>, "b-brnich@...com" <b-brnich@...com>,
"hverkuil@...all.nl" <hverkuil@...all.nl>, Nas Chung
<nas.chung@...psnmedia.com>
Subject: RE: [PATCH v3 0/4] Performance improvement of decoder
Hi Nicolas
> -----Original Message-----
> From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
> Sent: Wednesday, September 10, 2025 9:57 PM
> To: jackson.lee <jackson.lee@...psnmedia.com>; mchehab@...nel.org;
> hverkuil-cisco@...all.nl; bob.beckett@...labora.com
> Cc: linux-media@...r.kernel.org; linux-kernel@...r.kernel.org; lafley.kim
> <lafley.kim@...psnmedia.com>; b-brnich@...com; hverkuil@...all.nl; Nas
> Chung <nas.chung@...psnmedia.com>
> Subject: Re: [PATCH v3 0/4] Performance improvement of decoder
>
> Hi Jackson,
>
> Le mercredi 10 septembre 2025 à 06:59 +0000, jackson.lee a écrit :
> [...]
>
> > I have reproduced the stall problem, I can see it with the latest
> Gstreamer version.
> > The root cause is we checked an incorrect return value while flushing,
> so in spite of not finished flushing, the checking loop if the flushing
> was finished was exited.
> > When stop streaming was called and the instance queue count was 1, the
> checking function put infinite loop, so the stall problem happened.
> >
> > The below patch should be needed.
> >
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > index edbe69540ef1..2e0128cd0e4d 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > @@ -52,6 +52,7 @@ int wave5_vpu_init_with_bitcode(struct device *dev,
> > u8 *bitcode, size_t size)
> > int wave5_vpu_flush_instance(struct vpu_instance *inst)
> > {
> > int ret = 0;
> > + int mutex_ret = 0;
> > int retry = 0;
> >
> > ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> > @@ -80,9 +81,9 @@ int wave5_vpu_flush_instance(struct vpu_instance
> > *inst)
> >
> > mutex_unlock(&inst->dev->hw_lock);
> > wave5_vpu_dec_get_output_info(inst,
> > &dec_info);
> > - ret =
> > mutex_lock_interruptible(&inst->dev->hw_lock);
> > - if (ret)
> > - return ret;
> > + mutex_ret =
> > +mutex_lock_interruptible(&inst->dev->hw_lock);
> > + if (mutex_ret)
> > + return mutex_ret;
> > if (dec_info.index_frame_display > 0)
> > wave5_vpu_dec_set_disp_flag(inst,
> > dec_info.index_frame_display);
> > }
>
> Good catch, unfortunately it does not completely fix the problem for me.
> You can find a the end of this message the patch I actually tested. Note
> I ,ove the mutex_ret in a close scope, and fixed other occurence of this
> pattern, except one that I highlighted to you with a FIXME.
>
> Some new information, I had this trace from GStreamer when the bug occured
> on forward seeks (very rare):
" gst-play-1.0 --audiosink=fakesink --videosink=fakevideosink long-drc.h264"
When the above command is executed, to do forward seeks, I can use "shift key + '+'key " this key combination?
Thanks
Jackson
>
> ** (gst-play-1.0:604): WARNING **: 00:03:59.965: v4l2h264dec0: Too old
> frames, bug in decoder -- please file a bug
>
> [root@...into nicolas]# echo w > /proc/sysrq-trigger [ 335.116289] sysrq:
> Show Blocked State
> [ 335.120054] task:typefind:sink state:D stack:0 pid:607 tgid:604
> ppid:543 task_flags:0x40044c flags:0x00000019
> [ 335.131147] Call trace:
> [ 335.133584] __switch_to+0xf0/0x1c0 (T) [ 335.137442]
> __schedule+0x35c/0x9bc [ 335.140935] schedule+0x34/0x110 [ 335.144162]
> schedule_timeout+0x80/0x104 [ 335.148081]
> wait_for_completion_timeout+0x74/0x158
> [ 335.152955] wave5_vpu_wait_interrupt+0x28/0x60 [wave5] [ 335.158252]
> wave5_vpu_dec_stop_streaming+0x68/0x28c [wave5] [ 335.163915]
> __vb2_queue_cancel+0x2c/0x2d4 [videobuf2_common] [ 335.169668]
> vb2_core_queue_release+0x20/0x74 [videobuf2_common] [ 335.175678]
> vb2_queue_release+0x10/0x1c [videobuf2_v4l2] [ 335.181081]
> v4l2_m2m_ctx_release+0x20/0x40 [v4l2_mem2mem] [ 335.186567]
> wave5_vpu_release_device+0x44/0x150 [wave5] [ 335.191879]
> wave5_vpu_dec_release+0x20/0x2c [wave5] [ 335.196841]
> v4l2_release+0xb4/0xf0 [videodev] [ 335.201709] __fput+0xd0/0x2e0
> [ 335.205090] ____fput+0x14/0x20 [ 335.208468] task_work_run+0x64/0xd4
> [ 335.212164] do_exit+0x240/0x8e0 [ 335.215552] do_group_exit+0x30/0xa4
> [ 335.219177] get_signal+0x790/0x860 [ 335.222676] do_signal+0x94/0x394
> [ 335.225986] do_notify_resume+0xd0/0x14c [ 335.229910]
> el0_svc+0xe4/0xe8 [ 335.232967] el0t_64_sync_handler+0xa0/0xe4
> [ 335.237154] el0t_64_sync+0x198/0x19c
>
> regards,
> Nicolas
>
> ---
>
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> index edbe69540ef1e..2faca2eee41fe 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> @@ -77,12 +77,13 @@ int wave5_vpu_flush_instance(struct vpu_instance *inst)
> return -ETIMEDOUT;
> } else if (ret == -EBUSY) {
> struct dec_output_info dec_info;
> + int ret_mutex;
>
> mutex_unlock(&inst->dev->hw_lock);
> wave5_vpu_dec_get_output_info(inst, &dec_info);
> - ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> - if (ret)
> - return ret;
> + ret_mutex = mutex_lock_interruptible(&inst->dev-
> >hw_lock);
> + if (ret_mutex)
> + return ret_mutex;
> if (dec_info.index_frame_display > 0)
> wave5_vpu_dec_set_disp_flag(inst,
> dec_info.index_frame_display);
> }
> @@ -222,6 +223,8 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32
> *fail_res)
> }
>
> do {
> + int ret_mutex;
> +
> ret = wave5_vpu_dec_finish_seq(inst, fail_res);
> if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING) {
> dev_warn(inst->dev->dev, "dec_finish_seq timed out\n");
> @@ -243,10 +246,10 @@ int wave5_vpu_dec_close(struct vpu_instance *inst,
> u32 *fail_res)
>
> mutex_unlock(&vpu_dev->hw_lock);
> wave5_vpu_dec_get_output_info(inst, &dec_info);
> - ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> - if (ret) {
> + ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
> + if (ret_mutex) {
> pm_runtime_put_sync(inst->dev->dev);
> - return ret;
> + return ret_mutex;
> }
> } while (ret != 0);
>
> @@ -482,6 +485,7 @@ dma_addr_t wave5_vpu_dec_get_rd_ptr(struct
> vpu_instance *inst)
>
> ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> if (ret)
> + // FIXME this return type is wrong
> return ret;
>
> rd_ptr = wave5_dec_get_rd_ptr(inst);
> nicolas@...tebuilder:~/Sources/TI/jacinto/linux$ git diff diff --git
> a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> index edbe69540ef1e..2faca2eee41fe 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> @@ -77,12 +77,13 @@ int wave5_vpu_flush_instance(struct vpu_instance *inst)
> return -ETIMEDOUT;
> } else if (ret == -EBUSY) {
> struct dec_output_info dec_info;
> + int ret_mutex;
>
> mutex_unlock(&inst->dev->hw_lock);
> wave5_vpu_dec_get_output_info(inst, &dec_info);
> - ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> - if (ret)
> - return ret;
> + ret_mutex = mutex_lock_interruptible(&inst->dev-
> >hw_lock);
> + if (ret_mutex)
> + return ret_mutex;
> if (dec_info.index_frame_display > 0)
> wave5_vpu_dec_set_disp_flag(inst,
> dec_info.index_frame_display);
> }
> @@ -222,6 +223,8 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32
> *fail_res)
> }
>
> do {
> + int ret_mutex;
> +
> ret = wave5_vpu_dec_finish_seq(inst, fail_res);
> if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING) {
> dev_warn(inst->dev->dev, "dec_finish_seq timed out\n");
> @@ -243,10 +246,10 @@ int wave5_vpu_dec_close(struct vpu_instance *inst,
> u32 *fail_res)
>
> mutex_unlock(&vpu_dev->hw_lock);
> wave5_vpu_dec_get_output_info(inst, &dec_info);
> - ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> - if (ret) {
> + ret_mutex = mutex_lock_interruptible(&vpu_dev->hw_lock);
> + if (ret_mutex) {
> pm_runtime_put_sync(inst->dev->dev);
> - return ret;
> + return ret_mutex;
> }
> } while (ret != 0);
>
> @@ -482,6 +485,7 @@ dma_addr_t wave5_vpu_dec_get_rd_ptr(struct
> vpu_instance *inst)
>
> ret = mutex_lock_interruptible(&inst->dev->hw_lock);
> if (ret)
> + // FIXME this return type is wrong
> return ret;
>
> rd_ptr = wave5_dec_get_rd_ptr(inst);
Powered by blists - more mailing lists