Linux Media Controller development
 help / color / mirror / Atom feed
From: Christian Cortner <ccortner@gmail.com>
To: Bryan O'Donoghue <bod@kernel.org>,
	Vikash Garodia <vikash.garodia@oss.qualcomm.com>,
	Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Cc: Abhinav Kumar <abhinav.kumar@linux.dev>,
	Birk Skyum <birk.skyum@pm.me>,
	linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: [RFC PATCH 5/5] media: iris: Resume the first source change if it fits the CAPTURE setup
Date: Mon, 26 Oct 2026 21:19:34 -0500	[thread overview]
Message-ID: <20261027021934.22113-6-ccortner@gmail.com> (raw)
In-Reply-To: <20261027021934.22113-1-ccortner@gmail.com>

The stateful decoder interface lets a client that knows the coded size
of the stream set it with VIDIOC_S_FMT(OUTPUT) and set up the CAPTURE
queue without waiting for the first source change event. A dynamic
resolution change is then only triggered "If the values configured by
the client do not match those parsed by the decoder"; vicodec, for one,
raises no source change at all when the stream matches the CAPTURE
format.

iris instead runs a full resolution change whenever the first source
change finds the CAPTURE queue streaming, even if the parsed format is
exactly the one the client configured: the client gets an empty LAST
buffer, and decoding stays stopped until it restarts CAPTURE or sends
V4L2_DEC_CMD_START. ffmpeg's h264_v4l2m2m decoder sets CAPTURE up early
like this. When VIDIOC_G_FMT(CAPTURE) after the source change event
shows an unchanged resolution, it resumes immediately with
V4L2_DEC_CMD_START, as step 6 of the Capture Setup sequence allows, but
iris rejects the command with -EBUSY until the firmware has completed
the drain. ffmpeg does not retry, and stops at the -EPIPE that follows
the LAST buffer, after a single frame.

When the first source change after S_FMT(OUTPUT) finds CAPTURE streaming
with the coded size, pixel format and stride the stream needs, and with
enough buffers that are large enough, still queue the event, so that the
client can read the parsed selection and colorspace, but resume the
firmware in the driver once the drain completes: give the empty PSC_LAST
buffer back to the firmware instead of returning it to the client, and
send the same commands as V4L2_DEC_CMD_START. Later source changes, and
any that does not fit, still take the full resolution change sequence,
so a new visible rectangle, colorspace or minimum buffer count in the
middle of a stream is not hidden from the client.

Fixes: 84e17adae3e3 ("media: iris: add support for dynamic resolution change")
Assisted-by: LLM sparse smatch
---
Not for merging, and not working for most streams: this resumes the
firmware through iris_vdec_start_cmd() while the driver's internal
buffers are still sized for four frame buffers, so streams that need
more (x264 at its default settings needs six) end in "session error
received 0x4000003: fatal error". See the cover letter and
https://lore.kernel.org/linux-media/179306719855.20070.15625203453374418392.iris-rfc-reply@gmail.com/

 .../media/platform/qcom/iris/iris_common.c    |  3 +
 .../qcom/iris/iris_hfi_gen2_response.c        | 31 ++++++++++
 .../media/platform/qcom/iris/iris_instance.h  |  4 ++
 drivers/media/platform/qcom/iris/iris_vdec.c  | 56 +++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_vdec.h  |  3 +
 5 files changed, 97 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_common.c b/drivers/media/platform/qcom/iris/iris_common.c
index 25836561bcf3..686bfe824e3a 100644
--- a/drivers/media/platform/qcom/iris/iris_common.c
+++ b/drivers/media/platform/qcom/iris/iris_common.c
@@ -204,6 +204,9 @@ int iris_session_streamoff(struct iris_inst *inst, u32 plane)
 	enum iris_buffer_type buffer_type;
 	int ret;
 
+	/* a stream stop cancels a pending iris_vdec_resume_src_change() */
+	inst->drc_auto_resume = false;
+
 	switch (plane) {
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		buffer_type = BUF_INPUT;
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
index b4fe9066fcb8..dd4200f63cdf 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
@@ -385,10 +385,25 @@ static int iris_hfi_gen2_handle_output_buffer(struct iris_inst *inst,
 	buf->data_size = hfi_buffer->data_size;
 	buf->timestamp = hfi_buffer->timestamp;
 
+	/*
+	 * The driver resumes this source change itself, see
+	 * iris_vdec_src_change_can_resume(): an empty PSC_LAST buffer goes back
+	 * to the firmware, and one with data is not the last buffer.
+	 */
+	if (inst->drc_auto_resume && !buf->data_size &&
+	    hfi_buffer->flags & HFI_BUF_FW_FLAG_PSC_LAST) {
+		buf->attr &= ~BUF_ATTR_QUEUED;
+		buf->attr |= BUF_ATTR_DEFERRED;
+		return 0;
+	}
+
 	buf->attr &= ~BUF_ATTR_QUEUED;
 	buf->attr |= BUF_ATTR_DEQUEUED;
 
 	buf->flags = iris_hfi_gen2_get_driver_buffer_flags(inst, hfi_buffer->flags);
+	if (inst->drc_auto_resume &&
+	    hfi_buffer->flags & HFI_BUF_FW_FLAG_PSC_LAST)
+		buf->flags &= ~V4L2_BUF_FLAG_LAST;
 
 	/*
 	 * The empty buffer that ends a drain (LAST) or a source change
@@ -672,16 +687,22 @@ static void iris_hfi_gen2_read_input_subcr_params(struct iris_inst *inst)
 static int iris_hfi_gen2_handle_src_change(struct iris_inst *inst,
 					   struct iris_hfi_packet *pkt)
 {
+	struct v4l2_pix_format_mplane cfg;
 	int ret;
 
 	if (pkt->port != HFI_PORT_BITSTREAM)
 		return 0;
 
+	cfg = inst->fmt_dst->fmt.pix_mp;
+
 	ret = iris_inst_sub_state_change_drc(inst);
 	if (ret)
 		return ret;
 
 	iris_hfi_gen2_read_input_subcr_params(inst);
+	inst->drc_auto_resume = inst->sub_state & IRIS_INST_SUB_DRC &&
+				iris_vdec_src_change_can_resume(inst, &cfg);
+	inst->src_change_seen = true;
 	iris_vdec_src_change(inst);
 
 	return 0;
@@ -973,6 +994,16 @@ static int iris_hfi_gen2_handle_session_response(struct iris_core *core,
 	if (dequeue)
 		iris_hfi_gen2_handle_dequeue_buffers(inst);
 
+	if (inst->drc_auto_resume && iris_drc_pending(inst) &&
+	    inst->state != IRIS_INST_ERROR) {
+		int resume_ret = iris_vdec_resume_src_change(inst);
+
+		if (resume_ret) {
+			iris_inst_change_state(inst, IRIS_INST_ERROR);
+			ret = resume_ret;
+		}
+	}
+
 	mutex_unlock(&inst->lock);
 	iris_inst_put(inst);
 
diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
index 1013ed2911a1..db472f9f745f 100644
--- a/drivers/media/platform/qcom/iris/iris_instance.h
+++ b/drivers/media/platform/qcom/iris/iris_instance.h
@@ -58,6 +58,8 @@ struct iris_hfi_session_ops;
  * @codec: codec type
  * @last_buffer_dequeued: a flag to indicate that last buffer is sent by driver
  * @drc_wq: woken when IRIS_INST_SUB_DRC_LAST is set
+ * @drc_auto_resume: the driver resumes the current source change itself
+ * @src_change_seen: a source change was reported since the last S_FMT(OUTPUT)
  * @last_buf_ns: start time of received input buffer for current one second FPS window
  * @frame_counter: input buffer counter for current one second FPS window
  * @frame_rate: frame rate of current instance
@@ -107,6 +109,8 @@ struct iris_inst {
 	u32				codec;
 	bool				last_buffer_dequeued;
 	wait_queue_head_t		drc_wq;
+	bool				drc_auto_resume;
+	bool				src_change_seen;
 	u64				last_buf_ns;
 	u32				frame_counter;
 	u32				frame_rate;
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
index 472e8e50830d..db3e506de7c2 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.c
+++ b/drivers/media/platform/qcom/iris/iris_vdec.c
@@ -299,6 +299,7 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
 		inst->crop.top = 0;
 		inst->crop.width = f->fmt.pix_mp.width;
 		inst->crop.height = f->fmt.pix_mp.height;
+		inst->src_change_seen = false;
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 		if (!check_format(inst, f->fmt.pix_mp.pixelformat, f->type))
@@ -371,6 +372,61 @@ void iris_vdec_src_change(struct iris_inst *inst)
 	v4l2_event_queue_fh(&inst->fh, &event);
 }
 
+/*
+ * dev-decoder.rst lets a client that knows the coded size of the stream set
+ * it with VIDIOC_S_FMT(OUTPUT) and set up the CAPTURE queue without waiting
+ * for the first source change event; a Dynamic Resolution Change is then only
+ * triggered "If the values configured by the client do not match those parsed
+ * by the decoder". The firmware pauses at every source change, so return true
+ * if the driver should resume it itself: this is the first source change
+ * since S_FMT(OUTPUT), CAPTURE is streaming, and @cfg, the CAPTURE format
+ * before the change, still fits the parsed stream.
+ */
+bool iris_vdec_src_change_can_resume(struct iris_inst *inst,
+				     const struct v4l2_pix_format_mplane *cfg)
+{
+	struct vb2_queue *dst_q = v4l2_m2m_get_dst_vq(inst->m2m_ctx);
+	const struct v4l2_pix_format_mplane *parsed;
+	struct vb2_buffer *vb;
+
+	parsed = &inst->fmt_dst->fmt.pix_mp;
+
+	if (inst->src_change_seen || !vb2_is_streaming(dst_q))
+		return false;
+
+	if (parsed->width != cfg->width || parsed->height != cfg->height ||
+	    parsed->pixelformat != cfg->pixelformat ||
+	    parsed->plane_fmt[0].bytesperline != cfg->plane_fmt[0].bytesperline)
+		return false;
+
+	if (vb2_get_num_buffers(dst_q) < inst->buffers[BUF_OUTPUT].min_count)
+		return false;
+
+	vb = vb2_get_buffer(dst_q, 0);
+
+	return vb && vb2_plane_size(vb, 0) >= parsed->plane_fmt[0].sizeimage;
+}
+
+/*
+ * Resume a source change accepted by iris_vdec_src_change_can_resume() once
+ * the firmware has completed its drain, with the commands of
+ * V4L2_DEC_CMD_START, and give the PSC_LAST buffer that
+ * iris_hfi_gen2_handle_output_buffer() kept back to the firmware. Called with
+ * inst->lock held.
+ */
+int iris_vdec_resume_src_change(struct iris_inst *inst)
+{
+	int ret;
+
+	inst->drc_auto_resume = false;
+
+	ret = iris_vdec_start_cmd(inst);
+	if (ret)
+		return ret;
+
+	return iris_queue_deferred_buffers(inst, BUF_OUTPUT);
+}
+
 int iris_vdec_streamon_input(struct iris_inst *inst)
 {
 	int ret;
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.h b/drivers/media/platform/qcom/iris/iris_vdec.h
index 5123d2a340e1..0270456996d0 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.h
+++ b/drivers/media/platform/qcom/iris/iris_vdec.h
@@ -15,6 +15,9 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f);
 int iris_vdec_validate_format(struct iris_inst *inst, u32 pixelformat);
 int iris_vdec_subscribe_event(struct iris_inst *inst, const struct v4l2_event_subscription *sub);
 void iris_vdec_src_change(struct iris_inst *inst);
+bool iris_vdec_src_change_can_resume(struct iris_inst *inst,
+				     const struct v4l2_pix_format_mplane *cfg);
+int iris_vdec_resume_src_change(struct iris_inst *inst);
 int iris_vdec_streamon_input(struct iris_inst *inst);
 int iris_vdec_streamon_output(struct iris_inst *inst);
 int iris_vdec_qbuf(struct iris_inst *inst, struct vb2_v4l2_buffer *vbuf);
-- 
2.56.0


  parent reply	other threads:[~2026-10-27  2:19 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <NKVWOXEaC2RjmCRG3wt6Y-1_CZxSqp_rpkiLIJV8xQg9k4sZGi5C2lZ4oIQ0qsI-Fk-wDDfmYsiCOiqKaDFnzQ==@protonmail.internalid>
2026-10-26 16:09 ` [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur Christian Cortner
2026-10-26 17:50   ` Birk Skyum
2026-10-26 21:13     ` Birk Skyum
2026-10-26 22:31   ` Bryan O'Donoghue
2026-10-27  2:19     ` Christian Cortner
2026-10-27  2:19       ` [RFC PATCH 0/5] media: iris: ffmpeg decoder flow changes, for reference only Christian Cortner
2026-10-27  2:19         ` [RFC PATCH 1/5] media: iris: Don't flag an empty source change LAST buffer as an error Christian Cortner
2026-10-27 22:26           ` Bryan O'Donoghue
2026-10-27  2:19         ` [RFC PATCH 2/5] media: iris: Update the CAPTURE stride and size on S_FMT(OUTPUT) Christian Cortner
2026-10-27 22:36           ` Bryan O'Donoghue
2026-10-27  2:19         ` [RFC PATCH 3/5] media: iris: Signal V4L2_EVENT_EOS only at the end of a drain Christian Cortner
2026-10-27 22:44           ` Bryan O'Donoghue
2026-10-27  2:19         ` [RFC PATCH 4/5] media: iris: Let a resolution change drain before stopping CAPTURE Christian Cortner
2026-10-27 22:59           ` Bryan O'Donoghue
2026-10-27  2:19         ` Christian Cortner [this message]
2026-10-27 23:04           ` [RFC PATCH 5/5] media: iris: Resume the first source change if it fits the CAPTURE setup Bryan O'Donoghue
2026-10-27 12:43       ` [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur Birk Skyum
2026-10-27 18:14         ` Christian Cortner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261027021934.22113-6-ccortner@gmail.com \
    --to=ccortner@gmail.com \
    --cc=abhinav.kumar@linux.dev \
    --cc=birk.skyum@pm.me \
    --cc=bod@kernel.org \
    --cc=dikshita.agarwal@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=vikash.garodia@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox