[PATCH v1 0/7] bug fixes

[PATCH v1 0/7] bug fixes

[Jackson.lee]

From: Jackson Lee <jackson.lee@chipsnmedia.com>

A few independent fixes for the Wave5 VPU driver, covering decode setup, m2m scheduling and stop_streaming robustness.

v4l2-compliance results:
========================
v4l2-compliance 1.33.0-5476, 64 bits, 64-bit time_t

Buffer ioctls:
                warn: ../utils/v4l2-compliance/v4l2-test-buffers.cpp(813): VIDIOC_CREATE_BUFS not supported
                warn: ../utils/v4l2-compliance/v4l2-test-buffers.cpp(813): VIDIOC_CREATE_BUFS not supported
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test CREATE_BUFS maximum buffers: OK
        test VIDIOC_REMOVE_BUFS: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Total for wave5-dec device /dev/video0: 48, Succeeded: 48, Failed: 0, Warnings: 2
Total for wave5-enc device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0

Change since v0:
	* For [PATCH v1 3/7] media: chips-media: wave5: avoid skipping device_run while VPU has work
	  - Remove to declare q_status
	* For [PATCH v1 4/7]  media: chips-media: wave5: Add timeout while stop_streaming
	  -  Bound the queue-drain polling loop with a timeout

Jackson Lee (7):
  media: chips-media: wave5: Guard bit depth check with
    initial_info_obtained
  media: chips-media: wave5: Set inst->std during default format
    initialization
  media: chips-media: wave5: avoid skipping device_run while VPU has
    work
  media: chips-media: wave5: Add timeout while stop_streaming
  media: chips-media: wave5: Defer job_finish() only when a DEC_PIC was
    queued
  media: chips-media: wave5: Fix pipeline stall when queuing fails
  media: chips-media: wave5: Resume device before setting EOS flag

 .../chips-media/wave5/wave5-vpu-dec.c         | 64 +++++++++++++++----
 .../chips-media/wave5/wave5-vpu-enc.c         |  6 +-
 .../chips-media/wave5/wave5-vpuconfig.h       |  2 +-
 3 files changed, 57 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH v1 1/7] media: chips-media: wave5: Guard bit depth check with initial_info_obtained

[Jackson.lee]

From: Jackson Lee <jackson.lee@chipsnmedia.com>

When CAPTURE STREAMON is called before the VPU has completed sequence
initialization (initial_info_obtained == false), the initial_info fields
contain uninitialized data. The driver checks
luma_bitdepth and rejects anything other than 8-bit, so garbage values
(e.g. 15) cause STREAMON to fail spuriously.

This is reproducible with the following multi-threaded test scenario:
  1. Allocate 2 CAPTURE buffers.
  2. Call STREAMON on the CAPTURE queue.
  3. Call DQBUF, which blocks waiting for a decoded frame.
  4. A second thread calls STREAMOFF on the CAPTURE queue.
  5. The blocked DQBUF should be released, allowing graceful termination.

At step 2, STREAMON reads uninitialized luma_bitdepth and rejects the
stream, causing the test to fail.

Fix this by checking initial_info_obtained before accessing the bit
depth fields, so the validation is only performed when the sequence
info has actually been parsed by the VPU.

Fixes: 035371c9e509 ("media: chips-media: wave5: Fix timeout while testing 10bit hevc fluster")
Cc: stable@vger.kernel.org
Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index bb2ba9204a83..01d1368b2965 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -1403,6 +1403,7 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
 	} else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
 		struct dec_initial_info *initial_info =
 			&inst->codec_info->dec_info.initial_info;
+		struct dec_info *p_dec_info = &inst->codec_info->dec_info;
 
 		if (inst->state == VPU_INST_STATE_STOP)
 			ret = switch_state(inst, VPU_INST_STATE_INIT_SEQ);
@@ -1410,6 +1411,7 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
 			goto return_buffers;
 
 		if (inst->state == VPU_INST_STATE_INIT_SEQ &&
+		    p_dec_info->initial_info_obtained &&
 		    inst->dev->product_code == WAVE521C_CODE) {
 			if (initial_info->luma_bitdepth != 8) {
 				dev_info(inst->dev->dev, "%s: no support for %d bit depth",
@@ -1418,7 +1420,6 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
 				goto return_buffers;
 			}
 		}
-
 	}
 	pm_runtime_put_autosuspend(inst->dev->dev);
 	return ret;
-- 
2.43.0

[PATCH v1 2/7] media: chips-media: wave5: Set inst->std during default format initialization

[Jackson.lee]

From: Jackson Lee <jackson.lee@chipsnmedia.com>

When the encoder is opened, wave5_set_default_format() sets up the
default capture format (e.g. H.264) but does not initialize inst->std.
As a result, inst->std remains zero, which does not match any valid
encoder codec.

If STREAMON is called before the user explicitly calls S_FMT on the
capture queue — as v4l2-compliance does in testBlockingDQBuf — the
codec/product check in wave5_vpu_enc_init_seq() fails with
"Unsupported encoder-codec & product combination" because inst->std
is neither W_HEVC_ENC nor W_AVC_ENC, returning -EOPNOTSUPP.

Fix this by setting inst->std via wave5_to_vpu_std() in
wave5_set_default_format(), so that the codec type is always consistent
with the default capture pixel format from the moment the instance is
opened.

Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
Cc: stable@vger.kernel.org
Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
index e6c94b6f2671..f9fcdf4c224b 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
@@ -1494,7 +1494,8 @@ static const struct vb2_ops wave5_vpu_enc_vb2_ops = {
 	.stop_streaming = wave5_vpu_enc_stop_streaming,
 };
 
-static void wave5_set_default_format(struct v4l2_pix_format_mplane *src_fmt,
+static void wave5_set_default_format(struct vpu_instance *inst,
+				     struct v4l2_pix_format_mplane *src_fmt,
 				     struct v4l2_pix_format_mplane *dst_fmt)
 {
 	src_fmt->pixelformat = enc_fmt_list[VPU_FMT_TYPE_RAW][0].v4l2_pix_fmt;
@@ -1506,6 +1507,7 @@ static void wave5_set_default_format(struct v4l2_pix_format_mplane *src_fmt,
 	wave5_update_pix_fmt(dst_fmt, VPU_FMT_TYPE_CODEC,
 			     W5_DEF_ENC_PIC_WIDTH, W5_DEF_ENC_PIC_HEIGHT,
 			     &enc_frmsize[VPU_FMT_TYPE_CODEC]);
+	inst->std = wave5_to_vpu_std(dst_fmt->pixelformat, inst->type);
 }
 
 static int wave5_vpu_enc_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
@@ -1770,7 +1772,7 @@ static int wave5_vpu_open_enc(struct file *filp)
 	inst->v4l2_fh.ctrl_handler = v4l2_ctrl_hdl;
 	v4l2_ctrl_handler_setup(v4l2_ctrl_hdl);
 
-	wave5_set_default_format(&inst->src_fmt, &inst->dst_fmt);
+	wave5_set_default_format(inst, &inst->src_fmt, &inst->dst_fmt);
 	inst->conf_win.width = inst->dst_fmt.width;
 	inst->conf_win.height = inst->dst_fmt.height;
 	inst->colorspace = V4L2_COLORSPACE_REC709;
-- 
2.43.0

[PATCH v1 3/7] media: chips-media: wave5: avoid skipping device_run while VPU has work

[Jackson.lee]

From: Jackson Lee <jackson.lee@chipsnmedia.com>

The decoder stalls because empty_queue is set to true even when the
m2m context still has pending commands. As a result, device_run is
never invoked, the m2m source queue fills up, and userspace (e.g.
Chromium) can no longer queue new bitstream buffers to the V4L2
driver.

Fix this by querying the VPU queue status via DEC_GET_QUEUE_STATUS
before deciding whether to skip device_run. Only skip when the
VPU's instance_queue_count equals the number of ready source
buffers in the v4l2-m2m context, which indicates that there is
genuinely no new work to perform. Otherwise, proceed with issuing
a decode command so that the VPU can continue draining its internal
queue.

Fixes: a176ac5e701f ("media: chips-media: wave5: Improve performance of decoder")
Cc: stable@vger.kernel.org
Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 .../media/platform/chips-media/wave5/wave5-vpu-dec.c   | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 01d1368b2965..6c6e86b09b40 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -1663,9 +1663,13 @@ static void wave5_vpu_dec_device_run(void *priv)
 		} else if (!inst->eos &&
 				inst->queuing_num == 0 &&
 				inst->state == VPU_INST_STATE_PIC_RUN) {
-			dev_dbg(inst->dev->dev, "%s: no bitstream for feeding, so skip ", __func__);
-			inst->empty_queue = true;
-			goto finish_job_and_return;
+			wave5_vpu_dec_give_command(inst, DEC_GET_QUEUE_STATUS, &q_status);
+			if (q_status.instance_queue_count == v4l2_m2m_num_src_bufs_ready(m2m_ctx)) {
+				dev_dbg(inst->dev->dev, "%s: no bitstream, skip\n",
+					__func__);
+				inst->empty_queue = true;
+				goto finish_job_and_return;
+			}
 		}
 	}
 
-- 
2.43.0

[PATCH v1 4/7] media: chips-media: wave5: Add timeout while stop_streaming

[Jackson.lee]

From: Jackson Lee <jackson.lee@chipsnmedia.com>

When stop_streaming is called, an infinite loop may occur in some cases.
Add a bounded poll of the queue status: loop until the queues drain,
sleeping briefly between polls, and bail out once VPU_DEC_STOP_TIMEOUT
elapses.

Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
Cc: stable@vger.kernel.org
Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 .../platform/chips-media/wave5/wave5-vpu-dec.c    | 15 +++++++++------
 .../platform/chips-media/wave5/wave5-vpuconfig.h  |  2 +-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 6c6e86b09b40..93f7b724d86c 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2021-2023 CHIPS&MEDIA INC
  */
 
+#include <linux/delay.h>
 #include <linux/pm_runtime.h>
 #include "wave5-helper.h"
 
@@ -1537,15 +1538,15 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
 {
 	struct vpu_instance *inst = vb2_get_drv_priv(q);
 	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
-
-	bool check_cmd = TRUE;
+	unsigned long timeout;
 
 	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
 	pm_runtime_resume_and_get(inst->dev->dev);
 	inst->empty_queue = true;
-	while (check_cmd) {
+
+	timeout = jiffies + msecs_to_jiffies(VPU_DEC_STOP_TIMEOUT);
+	while (true) {
 		struct queue_status_info q_status;
-		struct dec_output_info dec_output_info;
 
 		wave5_vpu_dec_give_command(inst, DEC_GET_QUEUE_STATUS, &q_status);
 		if ((inst->state == VPU_INST_STATE_STOP ||
@@ -1554,8 +1555,10 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
 			q_status.report_queue_count == 0)
 			break;
 
-		if (wave5_vpu_dec_get_output_info(inst, &dec_output_info))
-			dev_dbg(inst->dev->dev, "there is no output info\n");
+		if (time_after(jiffies, timeout))
+			break;
+
+		usleep_range(1000, 2000);
 	}
 
 	v4l2_m2m_update_stop_streaming_state(m2m_ctx, q);
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
index 4ebd48d5550e..e04f2dbf3b65 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
@@ -59,7 +59,7 @@
 //  application specific configuration
 #define VPU_ENC_TIMEOUT                 60000
 #define VPU_DEC_TIMEOUT                 60000
-#define VPU_DEC_STOP_TIMEOUT            10
+#define VPU_DEC_STOP_TIMEOUT            300
 
 // for WAVE encoder
 #define USE_SRC_PRP_AXI         0
-- 
2.43.0

[PATCH v1 5/7] media: chips-media: wave5: Defer job_finish() only when a DEC_PIC was queued

[Jackson.lee]

From: Jackson Lee <jackson.lee@chipsnmedia.com>

Decoder instances sharing a VPU also share one v4l2_m2m job slot, released
when the running context calls v4l2_m2m_job_finish(). While draining,
device_run() defers job_finish() once EOS is sent (sent_eos), expecting a
later finish_decode() (from a DEC_PIC completion IRQ) to release the slot.

But the m2m core checks job_ready() only when a job is queued, not when it
is dispatched. A job queued while draining can run after finish_decode()
has already moved the instance to STOP and sent EOS. device_run() then runs
in STOP, issues no DEC_PIC, yet still skips job_finish() - so no IRQ, no
finish_decode(), and the shared slot is leaked, stalling every instance.
With several v4l2h264dec instances in parallel, GStreamer hangs at EOS.

Track whether the run actually queued a DEC_PIC (cmd_issued) and defer
job_finish() only then. Otherwise finish the job immediately

Fixes: a176ac5e701f ("media: chips-media: wave5: Improve performance of decoder")
Cc: stable@vger.kernel.org
Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 .../media/platform/chips-media/wave5/wave5-vpu-dec.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 93f7b724d86c..f33c00cb801b 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -1655,6 +1655,7 @@ static void wave5_vpu_dec_device_run(void *priv)
 	struct queue_status_info q_status;
 	u32 fail_res = 0;
 	int ret = 0;
+	bool cmd_issued = false;
 
 	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new bitstream data", __func__);
 	pm_runtime_resume_and_get(inst->dev->dev);
@@ -1752,6 +1753,7 @@ static void wave5_vpu_dec_device_run(void *priv)
 			inst->retry = false;
 			if (!inst->eos)
 				inst->queuing_num--;
+			cmd_issued = true;
 		}
 		break;
 	default:
@@ -1769,8 +1771,16 @@ static void wave5_vpu_dec_device_run(void *priv)
 	 * in power and CPU time.
 	 * If EOS is passed, device_run will not call job_finish no more, it is called
 	 * only if HW is idle status in order to reduce overhead.
+	 *
+	 * Deferring job_finish() is only safe when this run actually queued a
+	 * DEC_PIC command (cmd_issued): that guarantees a completion IRQ, and
+	 * thus a later finish_decode(), will release the shared job slot. When
+	 * device_run() is entered with no command to issue (e.g. a job that was
+	 * queued while draining but reached the STOP state by the time it ran),
+	 * no IRQ follows, so finish the job here to avoid leaking the slot and
+	 * stalling every instance sharing the VPU.
 	 */
-	if (!inst->sent_eos)
+	if (!inst->sent_eos || !cmd_issued)
 		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
 }
 
-- 
2.43.0

[PATCH v1 6/7] media: chips-media: wave5: Fix pipeline stall when queuing fails

[Jackson.lee]

From: Jackson Lee <jackson.lee@chipsnmedia.com>

The Wave5 decoder calls v4l2_m2m_job_finish() immediately in device_run()
after submitting frames to firmware. When the firmware completes those
frames and the queue drains to zero, finish_decode() has no active M2M
job to finish, so v4l2_m2m_schedule_next_job() is never called and the
decoder stalls.

Call v4l2_m2m_try_schedule() in finish_decode() when the firmware queue
empties to ensure the framework always schedules the next device_run().

Fixes: a176ac5e701f ("media: chips-media: wave5: Improve performance of decoder")
Cc: stable@vger.kernel.org
Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index f33c00cb801b..1817b83c5884 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -475,7 +475,10 @@ static void wave5_vpu_dec_finish_decode(struct vpu_instance *inst)
 			v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
 	}
 
-	inst->queuing_fail = false;
+	if (inst->queuing_fail) {
+		inst->queuing_fail = false;
+		v4l2_m2m_try_schedule(m2m_ctx);
+	}
 }
 
 static int wave5_vpu_dec_querycap(struct file *file, void *fh, struct v4l2_capability *cap)
-- 
2.43.0

[PATCH v1 7/7] media: chips-media: wave5: Resume device before setting EOS flag

[Jackson.lee]

From: Jackson Lee <jackson.lee@chipsnmedia.com>

Setting the EOS flag talks to the firmware via send_firmware_command(),
which accesses VPU registers. Both the STREAMOFF path
(wave5_vpu_dec_job_abort()) and the V4L2_DEC_CMD_STOP path
(wave5_vpu_dec_stop()) can run while the device is runtime suspended, so
those register accesses hit powered-down hardware and the SoC raises an
asynchronous SError, panicking the kernel:

  SError Interrupt on CPU3, code 0x00000000bf000000 -- SError
   send_firmware_command+0x2c/0x160 [wave5]
   wave5_vpu_dec_set_bitstream_flag+0x6c/0x80 [wave5]
   wave5_vpu_dec_update_bitstream_buffer+0x80/0xec [wave5]
   wave5_vpu_dec_job_abort+0x44/0xa0 [wave5]
   v4l2_m2m_cancel_job+0x110/0x19c [v4l2_mem2mem]
   v4l2_m2m_streamoff+0x24/0x140 [v4l2_mem2mem]

Resume the device with pm_runtime_resume_and_get() around the EOS
firmware command and release it with pm_runtime_put_autosuspend(),
matching the runtime PM handling already done in
wave5_vpu_dec_device_run().

Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
Cc: stable@vger.kernel.org
Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 .../chips-media/wave5/wave5-vpu-dec.c         | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 1817b83c5884..6564cf3ec739 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -823,7 +823,15 @@ static int wave5_vpu_dec_stop(struct vpu_instance *inst)
 		 * calls do not block on a mutex while inside this spinlock.
 		 */
 		spin_unlock_irqrestore(&inst->state_spinlock, flags);
+		/*
+		 * V4L2_DEC_CMD_STOP can arrive while the device is runtime
+		 * suspended (e.g. on pipeline teardown). Setting the EOS flag
+		 * accesses VPU registers via send_firmware_command(), so the
+		 * device must be resumed first to avoid an asynchronous SError.
+		 */
+		pm_runtime_resume_and_get(inst->dev->dev);
 		ret = wave5_vpu_dec_set_eos_on_firmware(inst);
+		pm_runtime_put_autosuspend(inst->dev->dev);
 		if (ret)
 			return ret;
 
@@ -1797,11 +1805,22 @@ static void wave5_vpu_dec_job_abort(void *priv)
 	if (ret)
 		return;
 
+	/*
+	 * job_abort() runs from the STREAMOFF path and may be called while the
+	 * device is runtime suspended. Setting the EOS flag talks to the
+	 * firmware (send_firmware_command() accesses VPU registers), so the
+	 * device must be resumed first; otherwise the register access faults
+	 * with an asynchronous SError.
+	 */
+	pm_runtime_resume_and_get(inst->dev->dev);
+
 	ret = wave5_vpu_dec_set_eos_on_firmware(inst);
 	if (ret)
 		dev_warn(inst->dev->dev,
 			 "Setting EOS for the bitstream, fail: %d\n", ret);
 
+	pm_runtime_put_autosuspend(inst->dev->dev);
+
 	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
 }
 
-- 
2.43.0

Re: [PATCH v1 1/7] media: chips-media: wave5: Guard bit depth check with initial_info_obtained

[Nicolas Dufresne]

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

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@chipsnmedia.com>
> 
> When CAPTURE STREAMON is called before the VPU has completed sequence
> initialization (initial_info_obtained == false), the initial_info fields
> contain uninitialized data. The driver checks
> luma_bitdepth and rejects anything other than 8-bit, so garbage values
> (e.g. 15) cause STREAMON to fail spuriously.
> 
> This is reproducible with the following multi-threaded test scenario:
>   1. Allocate 2 CAPTURE buffers.
>   2. Call STREAMON on the CAPTURE queue.
>   3. Call DQBUF, which blocks waiting for a decoded frame.
>   4. A second thread calls STREAMOFF on the CAPTURE queue.
>   5. The blocked DQBUF should be released, allowing graceful termination.
> 
> At step 2, STREAMON reads uninitialized luma_bitdepth and rejects the
> stream, causing the test to fail.
> 
> Fix this by checking initial_info_obtained before accessing the bit
> depth fields, so the validation is only performed when the sequence
> info has actually been parsed by the VPU.
> 
> Fixes: 035371c9e509 ("media: chips-media: wave5: Fix timeout while testing 10bit hevc fluster")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index bb2ba9204a83..01d1368b2965 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -1403,6 +1403,7 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
>  	} else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>  		struct dec_initial_info *initial_info =
>  			&inst->codec_info->dec_info.initial_info;
> +		struct dec_info *p_dec_info = &inst->codec_info->dec_info;
>  
>  		if (inst->state == VPU_INST_STATE_STOP)
>  			ret = switch_state(inst, VPU_INST_STATE_INIT_SEQ);
> @@ -1410,6 +1411,7 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
>  			goto return_buffers;
>  
>  		if (inst->state == VPU_INST_STATE_INIT_SEQ &&
> +		    p_dec_info->initial_info_obtained &&
>  		    inst->dev->product_code == WAVE521C_CODE) {
>  			if (initial_info->luma_bitdepth != 8) {
>  				dev_info(inst->dev->dev, "%s: no support for %d bit depth",
> @@ -1418,7 +1420,6 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
>  				goto return_buffers;
>  			}
>  		}
> -
>  	}
>  	pm_runtime_put_autosuspend(inst->dev->dev);
>  	return ret;

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

Re: [PATCH v1 2/7] media: chips-media: wave5: Set inst->std during default format initialization

[Nicolas Dufresne]

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

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@chipsnmedia.com>
> 
> When the encoder is opened, wave5_set_default_format() sets up the
> default capture format (e.g. H.264) but does not initialize inst->std.
> As a result, inst->std remains zero, which does not match any valid
> encoder codec.
> 
> If STREAMON is called before the user explicitly calls S_FMT on the
> capture queue — as v4l2-compliance does in testBlockingDQBuf — the
> codec/product check in wave5_vpu_enc_init_seq() fails with
> "Unsupported encoder-codec & product combination" because inst->std
> is neither W_HEVC_ENC nor W_AVC_ENC, returning -EOPNOTSUPP.
> 
> Fix this by setting inst->std via wave5_to_vpu_std() in
> wave5_set_default_format(), so that the codec type is always consistent
> with the default capture pixel format from the moment the instance is
> opened.
> 
> Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> index e6c94b6f2671..f9fcdf4c224b 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> @@ -1494,7 +1494,8 @@ static const struct vb2_ops wave5_vpu_enc_vb2_ops = {
>  	.stop_streaming = wave5_vpu_enc_stop_streaming,
>  };
>  
> -static void wave5_set_default_format(struct v4l2_pix_format_mplane *src_fmt,
> +static void wave5_set_default_format(struct vpu_instance *inst,
> +				     struct v4l2_pix_format_mplane *src_fmt,
>  				     struct v4l2_pix_format_mplane *dst_fmt)
>  {
>  	src_fmt->pixelformat = enc_fmt_list[VPU_FMT_TYPE_RAW][0].v4l2_pix_fmt;
> @@ -1506,6 +1507,7 @@ static void wave5_set_default_format(struct v4l2_pix_format_mplane *src_fmt,
>  	wave5_update_pix_fmt(dst_fmt, VPU_FMT_TYPE_CODEC,
>  			     W5_DEF_ENC_PIC_WIDTH, W5_DEF_ENC_PIC_HEIGHT,
>  			     &enc_frmsize[VPU_FMT_TYPE_CODEC]);
> +	inst->std = wave5_to_vpu_std(dst_fmt->pixelformat, inst->type);
>  }
>  
>  static int wave5_vpu_enc_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> @@ -1770,7 +1772,7 @@ static int wave5_vpu_open_enc(struct file *filp)
>  	inst->v4l2_fh.ctrl_handler = v4l2_ctrl_hdl;
>  	v4l2_ctrl_handler_setup(v4l2_ctrl_hdl);
>  
> -	wave5_set_default_format(&inst->src_fmt, &inst->dst_fmt);
> +	wave5_set_default_format(inst, &inst->src_fmt, &inst->dst_fmt);
>  	inst->conf_win.width = inst->dst_fmt.width;
>  	inst->conf_win.height = inst->dst_fmt.height;
>  	inst->colorspace = V4L2_COLORSPACE_REC709;

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

Re: [PATCH v1 3/7] media: chips-media: wave5: avoid skipping device_run while VPU has work

[Nicolas Dufresne]

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

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@chipsnmedia.com>
> 
> The decoder stalls because empty_queue is set to true even when the
> m2m context still has pending commands. As a result, device_run is
> never invoked, the m2m source queue fills up, and userspace (e.g.
> Chromium) can no longer queue new bitstream buffers to the V4L2
> driver.
> 
> Fix this by querying the VPU queue status via DEC_GET_QUEUE_STATUS
> before deciding whether to skip device_run. Only skip when the
> VPU's instance_queue_count equals the number of ready source
> buffers in the v4l2-m2m context, which indicates that there is
> genuinely no new work to perform. Otherwise, proceed with issuing
> a decode command so that the VPU can continue draining its internal
> queue.
> 
> Fixes: a176ac5e701f ("media: chips-media: wave5: Improve performance of decoder")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  .../media/platform/chips-media/wave5/wave5-vpu-dec.c   | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index 01d1368b2965..6c6e86b09b40 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -1663,9 +1663,13 @@ static void wave5_vpu_dec_device_run(void *priv)
>  		} else if (!inst->eos &&
>  				inst->queuing_num == 0 &&
>  				inst->state == VPU_INST_STATE_PIC_RUN) {
> -			dev_dbg(inst->dev->dev, "%s: no bitstream for feeding, so skip ", __func__);
> -			inst->empty_queue = true;
> -			goto finish_job_and_return;
> +			wave5_vpu_dec_give_command(inst, DEC_GET_QUEUE_STATUS, &q_status);
> +			if (q_status.instance_queue_count == v4l2_m2m_num_src_bufs_ready(m2m_ctx)) {
> +				dev_dbg(inst->dev->dev, "%s: no bitstream, skip\n",
> +					__func__);
> +				inst->empty_queue = true;
> +				goto finish_job_and_return;
> +			}
>  		}
>  	}
>  

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

Re: [PATCH v1 4/7] media: chips-media: wave5: Add timeout while stop_streaming

[Nicolas Dufresne]

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

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@chipsnmedia.com>
> 
> When stop_streaming is called, an infinite loop may occur in some cases.
> Add a bounded poll of the queue status: loop until the queues drain,
> sleeping briefly between polls, and bail out once VPU_DEC_STOP_TIMEOUT
> elapses.
> 
> Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>

This is not the nicest thing to do, but I don't really have a better idea. We'd
need something from the firmware to signal this instead. The code was clearly
prone to spinning and CPU hugging, even if the HW was behaving, so I'll admit
this is an improvement.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  .../platform/chips-media/wave5/wave5-vpu-dec.c    | 15 +++++++++------
>  .../platform/chips-media/wave5/wave5-vpuconfig.h  |  2 +-
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index 6c6e86b09b40..93f7b724d86c 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
>   */
>  
> +#include <linux/delay.h>
>  #include <linux/pm_runtime.h>
>  #include "wave5-helper.h"
>  
> @@ -1537,15 +1538,15 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
>  {
>  	struct vpu_instance *inst = vb2_get_drv_priv(q);
>  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> -
> -	bool check_cmd = TRUE;
> +	unsigned long timeout;
>  
>  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
>  	pm_runtime_resume_and_get(inst->dev->dev);
>  	inst->empty_queue = true;
> -	while (check_cmd) {
> +
> +	timeout = jiffies + msecs_to_jiffies(VPU_DEC_STOP_TIMEOUT);
> +	while (true) {
>  		struct queue_status_info q_status;
> -		struct dec_output_info dec_output_info;
>  
>  		wave5_vpu_dec_give_command(inst, DEC_GET_QUEUE_STATUS, &q_status);
>  		if ((inst->state == VPU_INST_STATE_STOP ||
> @@ -1554,8 +1555,10 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
>  			q_status.report_queue_count == 0)
>  			break;
>  
> -		if (wave5_vpu_dec_get_output_info(inst, &dec_output_info))
> -			dev_dbg(inst->dev->dev, "there is no output info\n");
> +		if (time_after(jiffies, timeout))
> +			break;
> +
> +		usleep_range(1000, 2000);
>  	}
>  
>  	v4l2_m2m_update_stop_streaming_state(m2m_ctx, q);
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> index 4ebd48d5550e..e04f2dbf3b65 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuconfig.h
> @@ -59,7 +59,7 @@
>  //  application specific configuration
>  #define VPU_ENC_TIMEOUT                 60000
>  #define VPU_DEC_TIMEOUT                 60000
> -#define VPU_DEC_STOP_TIMEOUT            10
> +#define VPU_DEC_STOP_TIMEOUT            300
>  
>  // for WAVE encoder
>  #define USE_SRC_PRP_AXI         0

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

Re: [PATCH v1 5/7] media: chips-media: wave5: Defer job_finish() only when a DEC_PIC was queued

[Nicolas Dufresne]

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

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@chipsnmedia.com>
> 
> Decoder instances sharing a VPU also share one v4l2_m2m job slot, released
> when the running context calls v4l2_m2m_job_finish(). While draining,
> device_run() defers job_finish() once EOS is sent (sent_eos), expecting a
> later finish_decode() (from a DEC_PIC completion IRQ) to release the slot.
> 
> But the m2m core checks job_ready() only when a job is queued, not when it
> is dispatched. A job queued while draining can run after finish_decode()
> has already moved the instance to STOP and sent EOS. device_run() then runs
> in STOP, issues no DEC_PIC, yet still skips job_finish() - so no IRQ, no
> finish_decode(), and the shared slot is leaked, stalling every instance.
> With several v4l2h264dec instances in parallel, GStreamer hangs at EOS.
> 
> Track whether the run actually queued a DEC_PIC (cmd_issued) and defer
> job_finish() only then. Otherwise finish the job immediately
> 
> Fixes: a176ac5e701f ("media: chips-media: wave5: Improve performance of decoder")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  .../media/platform/chips-media/wave5/wave5-vpu-dec.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index 93f7b724d86c..f33c00cb801b 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -1655,6 +1655,7 @@ static void wave5_vpu_dec_device_run(void *priv)
>  	struct queue_status_info q_status;
>  	u32 fail_res = 0;
>  	int ret = 0;
> +	bool cmd_issued = false;
>  
>  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new bitstream data", __func__);
>  	pm_runtime_resume_and_get(inst->dev->dev);
> @@ -1752,6 +1753,7 @@ static void wave5_vpu_dec_device_run(void *priv)
>  			inst->retry = false;
>  			if (!inst->eos)
>  				inst->queuing_num--;
> +			cmd_issued = true;
>  		}
>  		break;
>  	default:
> @@ -1769,8 +1771,16 @@ static void wave5_vpu_dec_device_run(void *priv)
>  	 * in power and CPU time.
>  	 * If EOS is passed, device_run will not call job_finish no more, it is called
>  	 * only if HW is idle status in order to reduce overhead.
> +	 *
> +	 * Deferring job_finish() is only safe when this run actually queued a
> +	 * DEC_PIC command (cmd_issued): that guarantees a completion IRQ, and
> +	 * thus a later finish_decode(), will release the shared job slot. When
> +	 * device_run() is entered with no command to issue (e.g. a job that was
> +	 * queued while draining but reached the STOP state by the time it ran),
> +	 * no IRQ follows, so finish the job here to avoid leaking the slot and
> +	 * stalling every instance sharing the VPU.
>  	 */
> -	if (!inst->sent_eos)
> +	if (!inst->sent_eos || !cmd_issued)
>  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>  }
>  

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

Re: [PATCH v1 6/7] media: chips-media: wave5: Fix pipeline stall when queuing fails

[Nicolas Dufresne]

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

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@chipsnmedia.com>
> 
> The Wave5 decoder calls v4l2_m2m_job_finish() immediately in device_run()
> after submitting frames to firmware. When the firmware completes those
> frames and the queue drains to zero, finish_decode() has no active M2M
> job to finish, so v4l2_m2m_schedule_next_job() is never called and the
> decoder stalls.
> 
> Call v4l2_m2m_try_schedule() in finish_decode() when the firmware queue
> empties to ensure the framework always schedules the next device_run().
> 
> Fixes: a176ac5e701f ("media: chips-media: wave5: Improve performance of decoder")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index f33c00cb801b..1817b83c5884 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -475,7 +475,10 @@ static void wave5_vpu_dec_finish_decode(struct vpu_instance *inst)
>  			v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>  	}
>  
> -	inst->queuing_fail = false;
> +	if (inst->queuing_fail) {
> +		inst->queuing_fail = false;
> +		v4l2_m2m_try_schedule(m2m_ctx);
> +	}
>  }
>  
>  static int wave5_vpu_dec_querycap(struct file *file, void *fh, struct v4l2_capability *cap)

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

Re: [PATCH v1 7/7] media: chips-media: wave5: Resume device before setting EOS flag

[Nicolas Dufresne]

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

Hi,

Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> From: Jackson Lee <jackson.lee@chipsnmedia.com>
> 
> Setting the EOS flag talks to the firmware via send_firmware_command(),
> which accesses VPU registers. Both the STREAMOFF path
> (wave5_vpu_dec_job_abort()) and the V4L2_DEC_CMD_STOP path
> (wave5_vpu_dec_stop()) can run while the device is runtime suspended, so
> those register accesses hit powered-down hardware and the SoC raises an
> asynchronous SError, panicking the kernel:
> 
>   SError Interrupt on CPU3, code 0x00000000bf000000 -- SError
>    send_firmware_command+0x2c/0x160 [wave5]
>    wave5_vpu_dec_set_bitstream_flag+0x6c/0x80 [wave5]
>    wave5_vpu_dec_update_bitstream_buffer+0x80/0xec [wave5]
>    wave5_vpu_dec_job_abort+0x44/0xa0 [wave5]
>    v4l2_m2m_cancel_job+0x110/0x19c [v4l2_mem2mem]
>    v4l2_m2m_streamoff+0x24/0x140 [v4l2_mem2mem]
> 
> Resume the device with pm_runtime_resume_and_get() around the EOS
> firmware command and release it with pm_runtime_put_autosuspend(),
> matching the runtime PM handling already done in
> wave5_vpu_dec_device_run().
> 
> Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> ---
>  .../chips-media/wave5/wave5-vpu-dec.c         | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index 1817b83c5884..6564cf3ec739 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -823,7 +823,15 @@ static int wave5_vpu_dec_stop(struct vpu_instance *inst)
>  		 * calls do not block on a mutex while inside this spinlock.
>  		 */
>  		spin_unlock_irqrestore(&inst->state_spinlock, flags);
> +		/*
> +		 * V4L2_DEC_CMD_STOP can arrive while the device is runtime
> +		 * suspended (e.g. on pipeline teardown). Setting the EOS
> flag
> +		 * accesses VPU registers via send_firmware_command(), so the
> +		 * device must be resumed first to avoid an asynchronous
> SError.
> +		 */
> +		pm_runtime_resume_and_get(inst->dev->dev);

I'll take this one, as it improves things a lot, but please follow-up with
fixing all the missing return value heck for pm_runtime_resume_and_get().

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

>  		ret = wave5_vpu_dec_set_eos_on_firmware(inst);
> +		pm_runtime_put_autosuspend(inst->dev->dev);
>  		if (ret)
>  			return ret;
>  
> @@ -1797,11 +1805,22 @@ static void wave5_vpu_dec_job_abort(void *priv)
>  	if (ret)
>  		return;
>  
> +	/*
> +	 * job_abort() runs from the STREAMOFF path and may be called while
> the
> +	 * device is runtime suspended. Setting the EOS flag talks to the
> +	 * firmware (send_firmware_command() accesses VPU registers), so the
> +	 * device must be resumed first; otherwise the register access faults
> +	 * with an asynchronous SError.
> +	 */
> +	pm_runtime_resume_and_get(inst->dev->dev);
> +
>  	ret = wave5_vpu_dec_set_eos_on_firmware(inst);
>  	if (ret)
>  		dev_warn(inst->dev->dev,
>  			 "Setting EOS for the bitstream, fail: %d\n", ret);
>  
> +	pm_runtime_put_autosuspend(inst->dev->dev);
> +
>  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>  }
>  

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

RE: [PATCH v1 7/7] media: chips-media: wave5: Resume device before setting EOS flag

[jackson.lee]

Hi Nicolas

> -----Original Message-----
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Sent: Thursday, July 16, 2026 11:19 AM
> To: jackson.lee <jackson.lee@chipsnmedia.com>; mchehab@kernel.org;
> hverkuil-cisco@xs4all.nl; bob.beckett@collabora.com
> Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; lafley.kim
> <lafley.kim@chipsnmedia.com>; b-brnich@ti.com; hverkuil@xs4all.nl; Nas
> Chung <nas.chung@chipsnmedia.com>; stable@vger.kernel.org
> Subject: Re: [PATCH v1 7/7] media: chips-media: wave5: Resume device
> before setting EOS flag
> 
> Hi,
> 
> Le vendredi 26 juin 2026 à 10:22 +0900, Jackson.lee a écrit :
> > From: Jackson Lee <jackson.lee@chipsnmedia.com>
> >
> > Setting the EOS flag talks to the firmware via
> > send_firmware_command(), which accesses VPU registers. Both the
> > STREAMOFF path
> > (wave5_vpu_dec_job_abort()) and the V4L2_DEC_CMD_STOP path
> > (wave5_vpu_dec_stop()) can run while the device is runtime suspended,
> > so those register accesses hit powered-down hardware and the SoC
> > raises an asynchronous SError, panicking the kernel:
> >
> >   SError Interrupt on CPU3, code 0x00000000bf000000 -- SError
> >    send_firmware_command+0x2c/0x160 [wave5]
> >    wave5_vpu_dec_set_bitstream_flag+0x6c/0x80 [wave5]
> >    wave5_vpu_dec_update_bitstream_buffer+0x80/0xec [wave5]
> >    wave5_vpu_dec_job_abort+0x44/0xa0 [wave5]
> >    v4l2_m2m_cancel_job+0x110/0x19c [v4l2_mem2mem]
> >    v4l2_m2m_streamoff+0x24/0x140 [v4l2_mem2mem]
> >
> > Resume the device with pm_runtime_resume_and_get() around the EOS
> > firmware command and release it with pm_runtime_put_autosuspend(),
> > matching the runtime PM handling already done in
> > wave5_vpu_dec_device_run().
> >
> > Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jackson Lee <jackson.lee@chipsnmedia.com>
> > Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
> > ---
> >  .../chips-media/wave5/wave5-vpu-dec.c         | 19
> > +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > index 1817b83c5884..6564cf3ec739 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > @@ -823,7 +823,15 @@ static int wave5_vpu_dec_stop(struct vpu_instance
> *inst)
> >  		 * calls do not block on a mutex while inside this spinlock.
> >  		 */
> >  		spin_unlock_irqrestore(&inst->state_spinlock, flags);
> > +		/*
> > +		 * V4L2_DEC_CMD_STOP can arrive while the device is runtime
> > +		 * suspended (e.g. on pipeline teardown). Setting the EOS
> > flag
> > +		 * accesses VPU registers via send_firmware_command(), so the
> > +		 * device must be resumed first to avoid an asynchronous
> > SError.
> > +		 */
> > +		pm_runtime_resume_and_get(inst->dev->dev);
> 
> I'll take this one, as it improves things a lot, but please follow-up with
> fixing all the missing return value heck for pm_runtime_resume_and_get().
> 

Thanks for your review.
I will follow up with a separate patch that adds the missing return value for all pm_runtime_resume_and_get() calls.

Thanks
Jackson

> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>



> 
> >  		ret = wave5_vpu_dec_set_eos_on_firmware(inst);
> > +		pm_runtime_put_autosuspend(inst->dev->dev);
> >  		if (ret)
> >  			return ret;
> >
> > @@ -1797,11 +1805,22 @@ static void wave5_vpu_dec_job_abort(void *priv)
> >  	if (ret)
> >  		return;
> >
> > +	/*
> > +	 * job_abort() runs from the STREAMOFF path and may be called while
> > the
> > +	 * device is runtime suspended. Setting the EOS flag talks to the
> > +	 * firmware (send_firmware_command() accesses VPU registers), so
> the
> > +	 * device must be resumed first; otherwise the register access
> faults
> > +	 * with an asynchronous SError.
> > +	 */
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> > +
> >  	ret = wave5_vpu_dec_set_eos_on_firmware(inst);
> >  	if (ret)
> >  		dev_warn(inst->dev->dev,
> >  			 "Setting EOS for the bitstream, fail: %d\n", ret);
> >
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > +
> >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> >  }
> >