Re: [PATCH v3 0/4] Performance improvement of decoder

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

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

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:604): WARNING **: 00:03:59.965: v4l2h264dec0: Too old frames, bug in decoder -- please file a bug

[root@jacinto 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@whitebuilder:~/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);

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