* [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur
@ 2026-10-26 16:09 ` Christian Cortner
2026-10-26 17:50 ` Birk Skyum
2026-10-26 22:31 ` Bryan O'Donoghue
0 siblings, 2 replies; 18+ messages in thread
From: Christian Cortner @ 2026-10-26 16:09 UTC (permalink / raw)
To: Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Bryan O'Donoghue, Birk Skyum, linux-media,
linux-arm-msm
Hi,
On an ASUS Zenbook A16 (UX3607OA, Snapdragon X2 Elite / glymur),
ffmpeg's h264_v4l2m2m decoder outputs one corrupted frame and stops,
on every stream I tried. GStreamer's V4L2 decoders work. Below are the
two driver behaviours behind it (1 and 2), two more that stop ffmpeg
at a resolution change in the middle of a stream (3 and 4), and an
inconsistent CAPTURE format that ffmpeg happens to avoid (5).
I'm not a kernel developer. I tracked this down with an AI coding
assistant (Claude, by Anthropic): it traced the driver, wrote a small
V4L2 test client and tried changes to the driver. Everything below was
run on this laptop, and each local change described made its problem
go away here. I can't review or defend kernel patches myself, so I'm
reporting the problems instead of sending fixes, but I'm glad to test
patches on this machine.
Setup: the linux-msm laptops tree at 65f4ae7320b8 (v7.3-rc7), which
has the glymur iris support, plus the iris patches needed to run it
here: patch 06/13 of "media: iris: Migrate iommus to iris sub nodes"
v5, a driver-only form of the restrict-region IOVA fix, the PAS ID fix
and the runtime PM reference fix from the list, and radxa's libva-v4l2
kernel patches. Firmware: video-firmware 3.6 (aeb98f00ce9f) from the
Windows driver package. ffmpeg 8.1.3, GStreamer 1.28.8. The functions
involved are the same in media-committers next.
I could only test glymur. Birk Skyum's h264_v4l2m2m decodes on
X1E80100 matched a software decode [1], so problem 1 probably depends
on timing. Problems 2-4 are in code that runs at every resolution
change (2 and 4 on HFI gen2), so I expect them on other platforms too,
but I have not seen that.
1. A full resolution change at the start, although nothing changed
After STREAMON(OUTPUT), the firmware reports the first source change
about 2 ms later. ffmpeg has set up and started CAPTURE before that,
with a format that matches the stream. iris_inst_sub_state_change_drc()
then takes the IRIS_INST_SUB_DRC path rather than
IRIS_INST_SUB_FIRST_IPSC, so the client gets a full resolution change:
an empty LAST buffer, then no decoding until V4L2_DEC_CMD_START or a
CAPTURE restart. ffmpeg sends V4L2_DEC_CMD_START as soon as it sees the
event, gets -EBUSY because the drain is still running, as
dev-decoder.rst allows, and never sends it again after the LAST
buffer, which is a bug in ffmpeg. A test client that starts CAPTURE
early in the same way decodes all 60 frames when it sends START after
the LAST buffer, and none when it behaves like ffmpeg.
The question is whether iris should run a resolution change here at
all. The note in step 3 of the Initialization sequence says: "If the
values configured by the client do not match those parsed by the
decoder, a Dynamic Resolution Change will be triggered to reconfigure
them." vicodec raises no source change when the stream matches the
CAPTURE format. Keeping the event, but resuming the firmware in the
driver once its drain completes and giving the empty PSC_LAST buffer
back to the firmware, when this is the first source change and the
parsed format fits the CAPTURE queue, made ffmpeg decode bit-exact
here.
2. The empty LAST buffer of a source change is flagged as an error
iris_hfi_gen2_handle_output_buffer() sets V4L2_BUF_FLAG_ERROR on an
empty capture buffer unless the firmware flagged it
HFI_BUF_FW_FLAG_LAST. The empty buffer that ends a resolution change
carries HFI_BUF_FW_FLAG_PSC_LAST instead and reaches the client as
LAST|ERROR, although dev-decoder.rst says such a buffer "must be
ignored by the client". ffmpeg prints "capture: driver decode error"
for it and outputs it as a corrupted frame; that is the one frame in
problem 1. Exempting PSC_LAST the way 7adc11e6abf6 exempts LAST fixed
it here.
3. V4L2_EVENT_EOS at every resolution change
iris_vb2_buffer_done() and iris_vb2_buf_queue() queue V4L2_EVENT_EOS
and call v4l2_m2m_mark_stopped() for every LAST capture buffer,
including the one that ends a resolution change, and ffmpeg stops
decoding when it gets the event. Nothing in iris clears the stopped
mark again, so the EOS of the real drain at the end of the stream is
never sent. With a stream that changes from 640x360 to 1280x720 after
30 frames, the test client, following the spec, got an EOS at the
change and none at the end in 6 of 6 runs. Sending the event, and
marking the context stopped, only when IRIS_INST_SUB_DRAIN_LAST is set
fixed it here.
4. Stopping CAPTURE during a resolution change's drain kills the
instance
ffmpeg handles a resolution change by stopping CAPTURE about 10 us
after it dequeues the event, which dev-decoder.rst allows as a way to
abort the drain. When the stop reaches the firmware before it has
returned the PSC_LAST buffer, IRIS_INST_SUB_DRC_LAST is never set. At
the next STREAMON(CAPTURE), iris_process_streamon_output() resumes the
firmware without reallocating the internal input buffers. 1.3 ms
later the firmware returns the PSC_LAST buffer it still owed,
iris_inst_sub_state_change_drc_last() rejects it because the input is
no longer paused, and the instance goes to IRIS_INST_ERROR. Nothing is
returned after that, and the client waits in poll() forever. It is a
race: the test client imitating ffmpeg stalled in 2 of 4, 0 of 6 and
1 of 6 runs in three series, and ffmpeg stopped at the change every
time, from this or from problem 3. Waiting in
iris_vb2_stop_streaming(), for at most 500 ms, until the firmware has
returned the PSC_LAST buffer fixed it here. Completing the resolution
change at STREAMON instead did not help: the firmware then returned
nothing at all.
5. S_FMT(OUTPUT) leaves the CAPTURE stride and size stale
Setting the OUTPUT format updates the CAPTURE width and height
(8aadfd445373) but not bytesperline and sizeimage, which keep their
320x240 defaults on a new instance. For a 640x360 stream,
G_FMT(CAPTURE) then reports 640x384 NV12 with a sizeimage of 147456,
and iris_vb2_buf_prepare() rejects CAPTURE buffers smaller than 368640
bytes, so a client that allocates them from G_FMT cannot queue them.
ffmpeg is not affected, because it sets the CAPTURE format itself.
Recomputing both fields as S_FMT(CAPTURE) does fixed it here.
With all five changes
- ffmpeg h264_v4l2m2m: bit-exact against the software decoder on raw
H.264 (5 of 5 runs), MP4 (3 of 3), and across the 640x360 to
1280x720 change (2 of 2).
- GStreamer: v4l2h264dec, v4l2h265dec, v4l2vp9dec and v4l2av1dec
bit-exact at 1080p, H.264 and HEVC also at 2160p, and bit-exact
across the resolution change. The encoders still pass.
- the test client, with CAPTURE set up before or after the first
event, and in a mode that aborts the drain like ffmpeg: 27 of 27
runs bit-exact, and at the resolution change one EOS per run, at
the end (12 of 12).
- v4l2-compliance 1.33.0 (c6600a7e342e): decoder with -s5
--stream-from=<720p H.264> and encoder with -s, 55 of 55 each. This
tree predates 1facae9bc3c1, so the bus_info prefix check had to be
relaxed for these runs.
- 60 s of 2160p60 H.264 with 15 IDR frames: the firmware raised one
source change, at the start.
[1] https://lore.kernel.org/linux-media/179224901845.60580.12634525576974095945@pm.me/
Thanks,
Christian Cortner
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur
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
1 sibling, 1 reply; 18+ messages in thread
From: Birk Skyum @ 2026-10-26 17:50 UTC (permalink / raw)
To: Christian Cortner
Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Bryan O'Donoghue, linux-media, linux-arm-msm
On Mon, 26 Oct 2026 11:09:49 -0500, Christian Cortner wrote:
> I could only test glymur. Birk Skyum's h264_v4l2m2m decodes on
> X1E80100 matched a software decode [1], so problem 1 probably depends
> on timing.
It shows up on X1E80100 as well, with smaller streams than the 720p
clips I had tested. Lenovo Yoga Slim 7x, Omarchy's kernel (v7.3 with
a few Iris fixes from the list, none in the resolution change path),
ffmpeg 9.0.2, h264_v4l2m2m on 2 and 3 second libx264 clips in MP4,
compared frame by frame with a software decode:
1280x720 every frame identical in 17 of 17 runs
1920x1080 every frame identical in 3 of 3 runs
640x360 one frame, then the decode ends, in 10 of 11 runs
640x352 the same in 2 of 3 runs
640x368 the same in 2 of 3 runs
The one frame comes with "capture: driver decode error" and ffmpeg
marks it corrupt, as in your problem 2. In the other 3 runs ffmpeg
got EIO when submitting a packet and aborted:
Assertion pkt failed at fftools/ffmpeg_dec.c:753
In a series of 8 runs at 640x360 the firmware also reported a fatal
session error, five lines each time, in 2 runs, one of them the run
that aborted:
qcom-iris aa00000.video-codec: session error received 0x4000003: fatal error
I'm glad to test patches on this machine too.
Birk
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur
2026-10-26 17:50 ` Birk Skyum
@ 2026-10-26 21:13 ` Birk Skyum
0 siblings, 0 replies; 18+ messages in thread
From: Birk Skyum @ 2026-10-26 21:13 UTC (permalink / raw)
To: Christian Cortner
Cc: Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
Bryan O'Donoghue, linux-media, linux-arm-msm
A few more runs on the same Yoga. HEVC and VP9 are affected as well,
and so are some H.264 sizes above 360p, though less often. ffmpeg on
3 second libx264, libx265 and libvpx-vp9 clips, 3 runs each, compared
with a software decode:
H.264 640x480 2 identical, 1 ended after one frame
H.264 854x480 3 identical
H.264 1024x576 2 identical, 1 aborted after EIO
HEVC 640x360 1 ended after one frame, 2 aborted after EIO
VP9 640x360 3 with an extra frame first, then all 90 identical
The extra VP9 frame comes with "capture: driver decode error", so it
looks like your problem 2 without the stop. I had seen the same extra
frame at 1280x720 on 7.2.8 earlier.
mpv 0.41 with hwdec=auto-safe,v4l2m2m-copy, the setting Omarchy uses
on Snapdragon laptops, shows what this means for users. In 2 of 3
runs at 640x360 it showed one frame and ended playback as if the file
were over. It fell back to software decoding only in the third run,
where the decoder failed outright with POLLERR.
Birk
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur
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 22:31 ` Bryan O'Donoghue
2026-10-27 2:19 ` Christian Cortner
1 sibling, 1 reply; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-26 22:31 UTC (permalink / raw)
To: Christian Cortner, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
On 26/10/2026 16:09, Christian Cortner wrote:
> Hi,
>
> On an ASUS Zenbook A16 (UX3607OA, Snapdragon X2 Elite / glymur),
> ffmpeg's h264_v4l2m2m decoder outputs one corrupted frame and stops,
> on every stream I tried. GStreamer's V4L2 decoders work. Below are the
> two driver behaviours behind it (1 and 2), two more that stop ffmpeg
> at a resolution change in the middle of a stream (3 and 4), and an
> inconsistent CAPTURE format that ffmpeg happens to avoid (5).
>
> I'm not a kernel developer. I tracked this down with an AI coding
> assistant (Claude, by Anthropic): it traced the driver, wrote a small
> V4L2 test client and tried changes to the driver. Everything below was
> run on this laptop, and each local change described made its problem
> go away here. I can't review or defend kernel patches myself, so I'm
> reporting the problems instead of sending fixes, but I'm glad to test
> patches on this machine.
Err thanks for the bug report but, if you have a test-case and a
resolution - even of the code sucks and/or you don't understand it,
please post the code.
One of us can take up the task of finishing it off.
After all we are burning down the rainforest for this stuff so no sense
in wasting the already spent compute time and power.
---
bod
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC PATCH 0/5] media: iris: ffmpeg decoder flow changes, for reference only
2026-10-27 2:19 ` Christian Cortner
@ 2026-10-27 2:19 ` 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
` (4 more replies)
2026-10-27 12:43 ` [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur Birk Skyum
1 sibling, 5 replies; 18+ messages in thread
From: Christian Cortner @ 2026-10-27 2:19 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
These are the local changes behind my report "[BUG] media: iris:
resolution change handling stops ffmpeg's h264_v4l2m2m decoder on
glymur" [1], posted because Bryan asked for the code [2]. They are for
reference only, not for merging, and carry no Signed-off-by, because I
can't review or defend kernel code myself. An AI coding assistant
(Claude, by Anthropic) traced the driver, wrote the changes, their
commit messages and a test client, and ran the tests below, in a long
interactive session that started from "make hardware video decoding
work on this laptop". Each patch has an Assisted-by tag.
Patch 5 does not work for most real streams, see [3]. It 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 among them, end in a fatal
firmware error. It is included only to show where the driver could
resume a first source change itself; a working version would have to
re-create the internal buffers first, or resume only when they fit.
The commit message of patch 4 notes that re-creating them at a later
VIDIOC_STREAMON(CAPTURE), in a different situation, made the firmware
return nothing at all.
The patches and the problems in [1] they address:
1/5 Don't flag an empty source change LAST buffer as an error (2)
2/5 Update the CAPTURE stride and size on S_FMT(OUTPUT) (5)
3/5 Signal V4L2_EVENT_EOS only at the end of a drain (3)
4/5 Let a resolution change drain before stopping CAPTURE (4)
5/5 Resume the first source change if it fits the CAPTURE
setup (1)
Testing: the same changes, in the form that applies to the linux-msm
laptops tree, ran on the glymur laptop from [1]; the functions they
change are the same there and in media-committers next, apart from one
line of context. These versions, on media-committers next, are
build-tested only: each builds with W=1, sparse and smatch without new
warnings, and checkpatch --strict warns only about the missing
Signed-off-by and, because my clone is shallow, the commit ids in the
Fixes tags. The H.264 and HEVC results in [1] came from clips encoded
with -preset ultrafast. With x264-default clips at 640x360, 1024x576
and 1280x720, the test client from [3] decoded bit-exact when it set
up CAPTURE after the first event, so that patch 5 did not act. Not
tested at all: other SoCs, and HFI gen1, which patches 3 and 4 touch.
[1] https://lore.kernel.org/linux-media/20261026160952.22082-1-ccortner@gmail.com/
[2] https://lore.kernel.org/linux-media/36201ae2-1cee-4d8d-9381-dfc9013c522e@kernel.org/
[3] https://lore.kernel.org/linux-media/179306719855.20070.15625203453374418392.iris-rfc-reply@gmail.com/
Christian Cortner (5):
media: iris: Don't flag an empty source change LAST buffer as an error
media: iris: Update the CAPTURE stride and size on S_FMT(OUTPUT)
media: iris: Signal V4L2_EVENT_EOS only at the end of a drain
media: iris: Let a resolution change drain before stopping CAPTURE
media: iris: Resume the first source change if it fits the CAPTURE
setup
.../media/platform/qcom/iris/iris_buffer.c | 7 +-
.../media/platform/qcom/iris/iris_common.c | 3 +
.../qcom/iris/iris_hfi_gen2_response.c | 40 +++++-
.../media/platform/qcom/iris/iris_instance.h | 7 ++
drivers/media/platform/qcom/iris/iris_state.c | 8 +-
drivers/media/platform/qcom/iris/iris_vb2.c | 23 +++-
drivers/media/platform/qcom/iris/iris_vdec.c | 117 +++++++++++++-----
drivers/media/platform/qcom/iris/iris_vdec.h | 3 +
drivers/media/platform/qcom/iris/iris_vidc.c | 1 +
9 files changed, 176 insertions(+), 33 deletions(-)
base-commit: d7e7bca7169873b46e857c025c10f3336c1258d8
--
2.56.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC PATCH 1/5] media: iris: Don't flag an empty source change LAST buffer as an error
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 ` 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
` (3 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Christian Cortner @ 2026-10-27 2:19 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
When the firmware completes a source change, it returns the last capture
buffer from before the change flagged HFI_BUF_FW_FLAG_PSC_LAST, and that
buffer can be empty. iris_hfi_gen2_handle_output_buffer() sets
V4L2_BUF_FLAG_ERROR on every empty capture buffer returned while
streaming unless it carries HFI_BUF_FW_FLAG_LAST, the flag that ends a
drain, so the empty buffer that ends a source change reaches the client
flagged both LAST and ERROR.
The stateful decoder interface says of this buffer, as it does of the
one that ends a drain, that it "may be empty (with v4l2_buffer bytesused
= 0) and in that case it must be ignored by the client, as it does not
contain a decoded frame". With the ERROR flag, ffmpeg's h264_v4l2m2m
decoder reports "capture: driver decode error" and outputs a corrupted
frame.
Exempt HFI_BUF_FW_FLAG_PSC_LAST as well.
Fixes: 7adc11e6abf6 ("media: iris: Send V4L2_BUF_FLAG_ERROR for capture buffers with 0 filled length")
Cc: stable@vger.kernel.org
Assisted-by: LLM sparse smatch
---
Reference only, not for merging (no Signed-off-by); see the cover
letter.
drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
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 f0782c4b1e6e..b4fe9066fcb8 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
@@ -390,8 +390,13 @@ static int iris_hfi_gen2_handle_output_buffer(struct iris_inst *inst,
buf->flags = iris_hfi_gen2_get_driver_buffer_flags(inst, hfi_buffer->flags);
+ /*
+ * The empty buffer that ends a drain (LAST) or a source change
+ * (PSC_LAST) is not an error, the client just ignores it.
+ */
if (!buf->data_size && inst->state == IRIS_INST_STREAMING &&
- !(hfi_buffer->flags & HFI_BUF_FW_FLAG_LAST)) {
+ !(hfi_buffer->flags &
+ (HFI_BUF_FW_FLAG_LAST | HFI_BUF_FW_FLAG_PSC_LAST))) {
buf->flags |= V4L2_BUF_FLAG_ERROR;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [RFC PATCH 2/5] media: iris: Update the CAPTURE stride and size on S_FMT(OUTPUT)
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 2:19 ` 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
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Christian Cortner @ 2026-10-27 2:19 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
Setting the OUTPUT format also updates the width and height of the
CAPTURE format, but bytesperline and sizeimage keep their old values,
those of the 320x240 default on a new instance. For a 640x360 stream,
VIDIOC_G_FMT(CAPTURE) then reports 640x384 NV12 with a sizeimage of
147456 bytes. VIDIOC_REQBUFS(CAPTURE) allocates buffers of that size,
and iris_vb2_buf_prepare() rejects every one of them with -EINVAL,
because it checks them against the size computed from the new width
and height.
Move the per-format rules S_FMT(CAPTURE) uses into a helper and call it
for both queues, so that S_FMT(OUTPUT) leaves the CAPTURE format exactly
as S_FMT(CAPTURE) would set it for the same size.
Fixes: 8aadfd445373 ("media: iris: Update CAPTURE format info based on OUTPUT format")
Cc: stable@vger.kernel.org
Assisted-by: LLM sparse smatch
---
Reference only, not for merging (no Signed-off-by); see the cover
letter.
drivers/media/platform/qcom/iris/iris_vdec.c | 61 +++++++++++---------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
index 0dd33ff447bb..472e8e50830d 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.c
+++ b/drivers/media/platform/qcom/iris/iris_vdec.c
@@ -212,6 +212,35 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
return 0;
}
+static void iris_vdec_set_capture_size(struct iris_inst *inst, u32 width,
+ u32 height)
+{
+ struct v4l2_pix_format_mplane *pix_mp = &inst->fmt_dst->fmt.pix_mp;
+ struct iris_buffers *bufs = &inst->buffers[BUF_OUTPUT];
+
+ switch (pix_mp->pixelformat) {
+ case V4L2_PIX_FMT_P010:
+ pix_mp->width = ALIGN(width, 128);
+ pix_mp->height = ALIGN(height, 32);
+ pix_mp->plane_fmt[0].bytesperline = ALIGN(width * 2, 256);
+ break;
+ case V4L2_PIX_FMT_QC10C:
+ pix_mp->width = roundup(width, 192);
+ pix_mp->height = ALIGN(height, 16);
+ pix_mp->plane_fmt[0].bytesperline = ALIGN(width * 4 / 3, 256);
+ break;
+ case V4L2_PIX_FMT_NV12:
+ case V4L2_PIX_FMT_QC08C:
+ pix_mp->width = ALIGN(width, 128);
+ pix_mp->height = ALIGN(height, 32);
+ pix_mp->plane_fmt[0].bytesperline = ALIGN(width, 128);
+ break;
+ }
+ pix_mp->plane_fmt[0].sizeimage = iris_get_buffer_size(inst, BUF_OUTPUT);
+ bufs->min_count = iris_vpu_buf_count(inst, BUF_OUTPUT);
+ bufs->size = pix_mp->plane_fmt[0].sizeimage;
+}
+
int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
{
struct v4l2_format *fmt, *output_fmt;
@@ -263,10 +292,8 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
output_fmt->fmt.pix_mp.quantization = f->fmt.pix_mp.quantization;
/* Update capture format based on new ip w/h */
- output_fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
- output_fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
- inst->buffers[BUF_OUTPUT].min_count = iris_vpu_buf_count(inst, BUF_OUTPUT);
- inst->buffers[BUF_OUTPUT].size = iris_get_buffer_size(inst, BUF_OUTPUT);
+ iris_vdec_set_capture_size(inst, f->fmt.pix_mp.width,
+ f->fmt.pix_mp.height);
inst->crop.left = 0;
inst->crop.top = 0;
@@ -281,30 +308,8 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
fmt->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
fmt->fmt.pix_mp.num_planes = 1;
- switch (f->fmt.pix_mp.pixelformat) {
- case V4L2_PIX_FMT_P010:
- fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
- fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
- fmt->fmt.pix_mp.plane_fmt[0].bytesperline =
- ALIGN(f->fmt.pix_mp.width * 2, 256);
- break;
- case V4L2_PIX_FMT_QC10C:
- fmt->fmt.pix_mp.width = roundup(f->fmt.pix_mp.width, 192);
- fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 16);
- fmt->fmt.pix_mp.plane_fmt[0].bytesperline =
- ALIGN(f->fmt.pix_mp.width * 4 / 3, 256);
- break;
- case V4L2_PIX_FMT_NV12:
- case V4L2_PIX_FMT_QC08C:
- fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);
- fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);
- fmt->fmt.pix_mp.plane_fmt[0].bytesperline =
- ALIGN(f->fmt.pix_mp.width, 128);
- break;
- }
- fmt->fmt.pix_mp.plane_fmt[0].sizeimage = iris_get_buffer_size(inst, BUF_OUTPUT);
- inst->buffers[BUF_OUTPUT].min_count = iris_vpu_buf_count(inst, BUF_OUTPUT);
- inst->buffers[BUF_OUTPUT].size = fmt->fmt.pix_mp.plane_fmt[0].sizeimage;
+ iris_vdec_set_capture_size(inst, f->fmt.pix_mp.width,
+ f->fmt.pix_mp.height);
inst->crop.top = 0;
inst->crop.left = 0;
--
2.56.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur
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 12:43 ` [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur Birk Skyum
0 siblings, 2 replies; 18+ messages in thread
From: Christian Cortner @ 2026-10-27 2:19 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
[-- Attachment #1: Type: text/plain, Size: 4447 bytes --]
On 26/10/2026 22:31, Bryan O'Donoghue wrote:
> Err thanks for the bug report but, if you have a test-case and a
> resolution - even of the code sucks and/or you don't understand it,
> please post the code.
>
> One of us can take up the task of finishing it off.
Will do. The five changes follow as an RFC series in reply to this
mail, not for merging and without a Signed-off-by, since I can't
review them myself. First a correction to my report, and a bug that
Birk's results led to. As before, I traced this with an AI coding
assistant.
Correction
Problem 1 in my report says that a client which sets up CAPTURE early
decodes every frame when it sends V4L2_DEC_CMD_START after the LAST
buffer, and the results say that ffmpeg decodes bit-exact with all
five changes. Both were measured with x264 -preset ultrafast clips
(Constrained Baseline), for which the firmware asks for three frame
buffers. With x264's default settings, which Birk's clips probably
used, it asks for six, and then the change for problem 1 (patch 5 of
the series) fails: at 640x360 every ffmpeg run ends in five lines of
qcom-iris aa00000.video-codec: session error received 0x4000003: fatal error
and ffmpeg then gets EIO and aborts. Patch 5 resumes the firmware
through iris_vdec_start_cmd(), so it runs into the bug below. A client
that sends V4L2_DEC_CMD_START after the LAST buffer at the start
should fail the same way with these streams; I could not test that,
because on my kernel patch 5 handles that source change. At 720p
ffmpeg decodes these streams fine: there the first source change
arrives before ffmpeg starts CAPTURE, and the driver takes its normal
path.
Resuming with V4L2_DEC_CMD_START does not grow the internal buffers
When a resolution change raises the number of frame buffers a stream
needs, and the client resumes with V4L2_DEC_CMD_START after the LAST
buffer instead of reallocating CAPTURE, the firmware fails right after
the resume with the same five errors. Step 6 of the Capture Setup
sequence in dev-decoder.rst allows that resume when the CAPTURE
buffers are large enough and there are at least as many as the new
minimum, as here: 16 allocated, 6 needed.
To reproduce, make one second of an x264 ultrafast stream (three
frame buffers) followed by one second of an x264 default stream
(six), both 640x360, so that the second source change raises the
count at the same size:
ffmpeg -f lavfi -i testsrc2=duration=1:size=640x360:rate=30 \
-pix_fmt yuv420p -c:v libx264 -preset ultrafast -f h264 a.h264
ffmpeg -f lavfi -i testsrc2=duration=1:size=640x360:rate=30 \
-pix_fmt yuv420p -c:v libx264 -f h264 b.h264
cat a.h264 b.h264 > ab.h264
and decode it with the attached test client, which sets up CAPTURE
after the first event, the way GStreamer does (-d selects the device
node, default /dev/video0):
gcc -O2 -Wall -o iris-drc-test iris-drc-test.c
./iris-drc-test -i ab.h264 -s 640x360 -m late -o out.nv12
./iris-drc-test -i ab.h264 -s 640x360 -m late -A -o out.nv12
The first run resumes with V4L2_DEC_CMD_START at the second source
change: the fatal errors, and only the first 30 of 60 frames (2 of 2
runs). The second (-A) reallocates CAPTURE instead: all 60 frames,
identical to "ffmpeg -i ab.h264 -f rawvideo -pix_fmt nv12" (2 of 2).
With the halves swapped, so that the count falls from 6 to 3,
resuming with V4L2_DEC_CMD_START works.
The kernel was the one from my report, with the five changes. Patch 5
acts only on the first source change, and kprobes showed that it did
not act in these runs. None of the changes touches the resume or the
internal buffers; on this path, patch 1 drops the ERROR flag from the
LAST buffer, patch 3 the EOS event at the resolution change, and
patch 4 adds a wake-up when DRC_LAST is set. I have not run this on
an unmodified driver. Birk, if you have a moment, the client would
show whether X1E80100 behaves the same; the fatal errors you saw on
an unmodified driver might have the same cause, but I can't tell
from here.
Not verified, from reading the code: in this case
iris_vdec_start_cmd() only resumes the firmware's ports. The COMV
buffer, sized when it is created from buffers[BUF_OUTPUT].min_count
or fw_min_count (iris_vpu3x_4x_dec_comv_size(), and
HFI_PROP_COMV_BUFFER_COUNT), and in split mode the internal DPB
buffers (iris_vpu_dpb_count()) keep that count. The reallocation
path recreates both at VIDIOC_STREAMON(CAPTURE).
Thanks,
Christian Cortner
[-- Attachment #2: iris-drc-test.c --]
[-- Type: text/plain, Size: 19270 bytes --]
// SPDX-License-Identifier: GPL-2.0
/*
* iris-drc-test - a V4L2 stateful-decoder client that follows dev-decoder.rst, with switches that
* reproduce what ffmpeg's h264_v4l2m2m does, to separate client bugs from driver bugs.
*
* -m late set up CAPTURE only after the first SOURCE_CHANGE event (what GStreamer does)
* -m early set up and stream CAPTURE right after OUTPUT STREAMON (what ffmpeg does)
* -E on SOURCE_CHANGE, send V4L2_DEC_CMD_START at once (what ffmpeg does) and report it
* -R do NOT send START again after the LAST buffer (what ffmpeg does); default: send it,
* as the spec's Dynamic Resolution Change sequence requires
* -F (early mode) start CAPTURE before feeding the first access unit, so the firmware's first
* port-settings change always arrives with CAPTURE streaming (ffmpeg's losing side of the race)
* -S on a SOURCE_CHANGE whose format no longer fits, stop CAPTURE and set it up again at once, without
* waiting for the LAST buffer (what ffmpeg's reinit does; the spec's Drain section allows aborting
* the implicit drain this way); default: wait for LAST first (Capture Setup step 7)
* -A at a resolution change whose format still fits, reallocate CAPTURE anyway (STREAMOFF, REQBUFS,
* STREAMON) instead of resuming with V4L2_DEC_CMD_START: the driver's normal path, as a control
*
* Resolution changes: after every CAPTURE (re)setup the visible size is re-read (G_SELECTION, COMPOSE), so a
* stream that changes size mid-way is written at its native sizes, like ffmpeg -noautoscale.
*
* Input is fed one H.264 access unit per OUTPUT buffer. The visible NV12 frames go to -o, for an md5
* comparison with ffmpeg's software decode (-pix_fmt nv12).
*
* Usage: iris-drc-test -i s.h264 -s WxH [-m late|early] [-E] [-R] [-A] [-b capbufs] [-o out.nv12] [-d dev]
*/
#include <errno.h>
#include <fcntl.h>
#include <poll.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <time.h>
#include <unistd.h>
#include <linux/videodev2.h>
#define MAXBUF 64
#define NOUT 4
#define OUT_SIZE (2 * 1024 * 1024)
struct mbuf { void *start; size_t len; };
struct au { size_t off, len; };
static int fd;
static struct mbuf cap[MAXBUF], out[NOUT];
static unsigned ncap_req = 16, ncap;
static struct v4l2_format capfmt;
static unsigned vis_w, vis_h;
static FILE *outf;
static int xioctl(unsigned long req, void *arg)
{
int r;
do { r = ioctl(fd, req, arg); } while (r == -1 && errno == EINTR);
return r;
}
static void die(const char *what)
{
fprintf(stderr, "FAIL %s: %s\n", what, strerror(errno));
exit(2);
}
static double now(void)
{
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
return ts.tv_sec + ts.tv_nsec / 1e9;
}
/* Split an Annex-B stream into access units: a new AU starts at a non-VCL NAL (SEI/SPS/PPS/AUD)
* that follows a VCL NAL, or at a VCL NAL with first_mb_in_slice == 0 that follows a VCL NAL. */
static size_t split_aus(const uint8_t *b, size_t len, struct au *aus, size_t max)
{
size_t starts[65536], ns = 0, n = 0;
for (size_t i = 0; i + 3 <= len && ns < 65536; i++)
if (b[i] == 0 && b[i + 1] == 0 && b[i + 2] == 1) {
starts[ns++] = (i > 0 && b[i - 1] == 0) ? i - 1 : i;
i += 2;
}
size_t au_start = 0;
int seen_vcl = 0;
for (size_t k = 0; k < ns; k++) {
size_t hdr = starts[k] + (b[starts[k] + 2] == 1 ? 3 : 4);
if (hdr >= len)
break;
int type = b[hdr] & 0x1f;
int vcl = (type == 1 || type == 5);
int first_mb0 = vcl && hdr + 1 < len && (b[hdr + 1] & 0x80);
int boundary = seen_vcl && ((!vcl && (type == 6 || type == 7 || type == 8 || type == 9)) || first_mb0);
if (boundary && n < max) {
aus[n].off = au_start;
aus[n].len = starts[k] - au_start;
n++;
au_start = starts[k];
seen_vcl = 0;
}
if (vcl)
seen_vcl = 1;
}
if (seen_vcl && n < max) {
aus[n].off = au_start;
aus[n].len = len - au_start;
n++;
}
return n;
}
static int dec_cmd(unsigned cmd)
{
struct v4l2_decoder_cmd c;
memset(&c, 0, sizeof(c));
c.cmd = cmd;
return xioctl(VIDIOC_DECODER_CMD, &c) ? -errno : 0;
}
static void setup_output(unsigned w, unsigned h)
{
struct v4l2_format f;
memset(&f, 0, sizeof(f));
f.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
f.fmt.pix_mp.width = w;
f.fmt.pix_mp.height = h;
f.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_H264;
f.fmt.pix_mp.num_planes = 1;
f.fmt.pix_mp.plane_fmt[0].sizeimage = OUT_SIZE;
if (xioctl(VIDIOC_S_FMT, &f))
die("S_FMT output");
struct v4l2_requestbuffers rb = { .count = NOUT, .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, .memory = V4L2_MEMORY_MMAP };
if (xioctl(VIDIOC_REQBUFS, &rb) || rb.count < NOUT)
die("REQBUFS output");
for (unsigned i = 0; i < NOUT; i++) {
struct v4l2_buffer b; struct v4l2_plane pl[1];
memset(&b, 0, sizeof(b)); memset(pl, 0, sizeof(pl));
b.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; b.memory = V4L2_MEMORY_MMAP; b.index = i;
b.m.planes = pl; b.length = 1;
if (xioctl(VIDIOC_QUERYBUF, &b))
die("QUERYBUF output");
out[i].len = pl[0].length;
out[i].start = mmap(NULL, pl[0].length, PROT_READ | PROT_WRITE, MAP_SHARED, fd, pl[0].m.mem_offset);
if (out[i].start == MAP_FAILED)
die("mmap output");
}
}
static void qbuf_cap(unsigned i)
{
struct v4l2_buffer b; struct v4l2_plane pl[1];
memset(&b, 0, sizeof(b)); memset(pl, 0, sizeof(pl));
b.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; b.memory = V4L2_MEMORY_MMAP; b.index = i;
b.m.planes = pl; b.length = 1;
if (xioctl(VIDIOC_QBUF, &b)) {
int err = errno;
struct v4l2_format f;
memset(&f, 0, sizeof(f));
f.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
xioctl(VIDIOC_G_FMT, &f);
fprintf(stderr, "QBUF capture idx=%u failed (%s); buffer len=%zu, format now %ux%u sizeimage=%u\n", i,
strerror(err), cap[i].len, f.fmt.pix_mp.width, f.fmt.pix_mp.height, f.fmt.pix_mp.plane_fmt[0].sizeimage);
errno = err;
die("QBUF capture");
}
}
/* the visible rectangle of the decoded frames (G_SELECTION COMPOSE), kept in vis_w/vis_h */
static void read_visible(const char *why)
{
struct v4l2_selection sel;
memset(&sel, 0, sizeof(sel));
sel.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
sel.target = V4L2_SEL_TGT_COMPOSE;
if (xioctl(VIDIOC_G_SELECTION, &sel) == 0 && sel.r.width && sel.r.height) {
vis_w = sel.r.width;
vis_h = sel.r.height;
}
printf("[%s] visible %ux%u\n", why, vis_w, vis_h);
}
static void setup_capture(const char *why)
{
memset(&capfmt, 0, sizeof(capfmt));
capfmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
if (xioctl(VIDIOC_G_FMT, &capfmt))
die("G_FMT capture");
if (!strcmp(why, "early")) {
/* ffmpeg sets the CAPTURE format at init. Without this, iris's G_FMT after S_FMT(OUTPUT) reports
* the new width/height with the stale sizeimage of its 320x240 default, and QBUF fails EINVAL. */
printf("[%s] G_FMT capture: %ux%u sizeimage=%u -> S_FMT NV12 (as ffmpeg does at init)\n", why,
capfmt.fmt.pix_mp.width, capfmt.fmt.pix_mp.height, capfmt.fmt.pix_mp.plane_fmt[0].sizeimage);
capfmt.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_NV12;
if (xioctl(VIDIOC_S_FMT, &capfmt))
die("S_FMT capture NV12");
}
struct v4l2_control ctl = { .id = V4L2_CID_MIN_BUFFERS_FOR_CAPTURE };
int minb = xioctl(VIDIOC_G_CTRL, &ctl) ? -1 : ctl.value;
unsigned want = ncap_req;
if (minb > 0 && (unsigned)minb + 1 > want)
want = minb + 1;
struct v4l2_requestbuffers rb = { .count = want, .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, .memory = V4L2_MEMORY_MMAP };
printf("[%s] G_FMT capture before REQBUFS: %ux%u sizeimage=%u\n", why, capfmt.fmt.pix_mp.width,
capfmt.fmt.pix_mp.height, capfmt.fmt.pix_mp.plane_fmt[0].sizeimage);
if (xioctl(VIDIOC_REQBUFS, &rb))
die("REQBUFS capture");
ncap = rb.count;
for (unsigned i = 0; i < ncap; i++) {
struct v4l2_buffer b; struct v4l2_plane pl[1];
memset(&b, 0, sizeof(b)); memset(pl, 0, sizeof(pl));
b.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; b.memory = V4L2_MEMORY_MMAP; b.index = i;
b.m.planes = pl; b.length = 1;
if (xioctl(VIDIOC_QUERYBUF, &b))
die("QUERYBUF capture");
cap[i].len = pl[0].length;
cap[i].start = mmap(NULL, pl[0].length, PROT_READ | PROT_WRITE, MAP_SHARED, fd, pl[0].m.mem_offset);
if (cap[i].start == MAP_FAILED)
die("mmap capture");
qbuf_cap(i);
}
read_visible(why);
printf("[%s] capture %ux%u stride=%u sizeimage=%u min_buffers=%d buffers=%u\n", why,
capfmt.fmt.pix_mp.width, capfmt.fmt.pix_mp.height, capfmt.fmt.pix_mp.plane_fmt[0].bytesperline,
capfmt.fmt.pix_mp.plane_fmt[0].sizeimage, minb, ncap);
}
static void free_capture(void)
{
for (unsigned i = 0; i < ncap; i++)
munmap(cap[i].start, cap[i].len);
struct v4l2_requestbuffers rb = { .count = 0, .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, .memory = V4L2_MEMORY_MMAP };
xioctl(VIDIOC_REQBUFS, &rb);
ncap = 0;
}
static void stream(unsigned type, int on)
{
if (xioctl(on ? VIDIOC_STREAMON : VIDIOC_STREAMOFF, &type))
die(on ? "STREAMON" : "STREAMOFF");
}
static void write_frame(unsigned i)
{
const uint8_t *p = cap[i].start;
unsigned stride = capfmt.fmt.pix_mp.plane_fmt[0].bytesperline;
unsigned scan = capfmt.fmt.pix_mp.height;
for (unsigned y = 0; y < vis_h; y++)
fwrite(p + (size_t)y * stride, 1, vis_w, outf);
for (unsigned y = 0; y < vis_h / 2; y++)
fwrite(p + (size_t)stride * scan + (size_t)y * stride, 1, vis_w, outf);
}
int main(int argc, char **argv)
{
const char *dev = "/dev/video0", *infile = NULL, *outfile = "/var/tmp/iris-drc-test.nv12", *mode = "late";
int early_start = 0, no_retry = 0, feed_late = 0, abort_drain = 0, always_realloc = 0, opt;
while ((opt = getopt(argc, argv, "d:i:s:m:b:o:ERFSA")) != -1) {
switch (opt) {
case 'd': dev = optarg; break;
case 'i': infile = optarg; break;
case 'o': outfile = optarg; break;
case 'm': mode = optarg; break;
case 's': if (sscanf(optarg, "%ux%u", &vis_w, &vis_h) != 2) return 2; break;
case 'b': ncap_req = atoi(optarg); if (ncap_req < 2 || ncap_req > MAXBUF - 1) return 2; break;
case 'E': early_start = 1; break;
case 'R': no_retry = 1; break;
case 'F': feed_late = 1; break;
case 'S': abort_drain = 1; break;
case 'A': always_realloc = 1; break;
default: fprintf(stderr, "usage: %s -i s.h264 -s WxH [-m late|early] [-E] [-R] [-S] [-F] [-A] [-b n] [-o f] [-d dev]\n", argv[0]); return 2;
}
}
int early = !strcmp(mode, "early");
if (!infile || !vis_w || !vis_h || (!early && strcmp(mode, "late"))) {
fprintf(stderr, "need -i, -s and -m late|early\n");
return 2;
}
FILE *in = fopen(infile, "rb");
struct stat st;
if (!in || fstat(fileno(in), &st))
die("open input");
uint8_t *bs = malloc(st.st_size);
if (!bs || fread(bs, 1, st.st_size, in) != (size_t)st.st_size)
die("read input");
fclose(in);
static struct au aus[100000];
size_t naus = split_aus(bs, st.st_size, aus, 100000);
outf = fopen(outfile, "wb");
if (!outf)
die("open output");
printf("mode=%s early_start=%d retry_after_last=%d feed_after_capture=%d access_units=%zu\n", mode, early_start,
!no_retry, feed_late, naus);
fd = open(dev, O_RDWR | O_NONBLOCK);
if (fd < 0)
die("open device");
struct v4l2_event_subscription sub;
memset(&sub, 0, sizeof(sub));
sub.type = V4L2_EVENT_SOURCE_CHANGE;
if (xioctl(VIDIOC_SUBSCRIBE_EVENT, &sub))
die("SUBSCRIBE source change");
sub.type = V4L2_EVENT_EOS;
xioctl(VIDIOC_SUBSCRIBE_EVENT, &sub);
setup_output(vis_w, vis_h);
int out_busy[NOUT] = { 0 };
size_t next = 0;
int cap_ready = 0, drc_seen = 0, stop_sent = 0, done = 0, epipe = 0, src_changes = 0, aborted = 0;
int eos_early = 0, eos_end = 0;
int frames = 0, empty_last = 0, errors = 0, early_ret = 1, retry_ret = 1;
double t_last = now(), t0 = now();
/* first access unit, then OUTPUT STREAMON (both clients do this) */
#define QUEUE_AU(i) do { \
memcpy(out[i].start, bs + aus[next].off, aus[next].len); \
struct v4l2_buffer b; struct v4l2_plane pl[1]; \
memset(&b, 0, sizeof(b)); memset(pl, 0, sizeof(pl)); \
b.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; b.memory = V4L2_MEMORY_MMAP; b.index = i; \
b.m.planes = pl; b.length = 1; pl[0].bytesused = aus[next].len; \
b.timestamp.tv_usec = next; \
if (xioctl(VIDIOC_QBUF, &b)) die("QBUF output"); \
out_busy[i] = 1; next++; } while (0)
if (!(early && feed_late))
QUEUE_AU(0);
stream(V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, 1);
if (early) {
setup_capture("early");
stream(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, 1);
cap_ready = 1;
}
while (!done) {
for (unsigned i = 0; i < NOUT && next < naus; i++)
if (!out_busy[i])
QUEUE_AU(i);
if (next >= naus && !stop_sent && cap_ready) {
int busy = 0;
for (unsigned i = 0; i < NOUT; i++)
busy += out_busy[i];
if (!busy) {
int r = dec_cmd(V4L2_DEC_CMD_STOP);
printf("DEC_CMD_STOP (drain) -> %d\n", r);
stop_sent = 1;
}
}
struct pollfd pfd = { .fd = fd, .events = POLLIN | POLLOUT | POLLPRI };
if (poll(&pfd, 1, 500) < 0 && errno != EINTR)
die("poll");
struct v4l2_event ev;
while (xioctl(VIDIOC_DQEVENT, &ev) == 0) {
t_last = now();
if (ev.type == V4L2_EVENT_EOS) {
/* EOS ends a drain; one before our DEC_CMD_STOP marks a resolution change as EOS */
if (stop_sent)
eos_end++;
else
eos_early++;
printf("EOS event (%s)\n", stop_sent ? "after our drain" : "BEFORE our drain");
continue;
}
if (ev.type != V4L2_EVENT_SOURCE_CHANGE)
continue;
src_changes++;
printf("SOURCE_CHANGE #%d changes=0x%x at %.1f ms (%s)\n", src_changes, ev.u.src_change.changes,
(now() - t0) * 1000, cap_ready ? "CAPTURE already streaming" : "CAPTURE not set up yet");
if (!cap_ready) {
setup_capture("after event");
stream(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, 1);
cap_ready = 1;
} else {
/* With CAPTURE streaming the driver may or may not start a Dynamic Resolution
* Change; the LAST buffer, not this event, tells (handled below). */
if (abort_drain) {
struct v4l2_format f;
memset(&f, 0, sizeof(f));
f.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
if (xioctl(VIDIOC_G_FMT, &f))
die("G_FMT on event");
if (f.fmt.pix_mp.width != capfmt.fmt.pix_mp.width ||
f.fmt.pix_mp.height != capfmt.fmt.pix_mp.height ||
f.fmt.pix_mp.plane_fmt[0].sizeimage > capfmt.fmt.pix_mp.plane_fmt[0].sizeimage) {
printf(" -S: format changed (%ux%u); STREAMOFF/realloc/STREAMON CAPTURE now, before LAST\n",
f.fmt.pix_mp.width, f.fmt.pix_mp.height);
aborted++;
stream(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, 0);
free_capture();
setup_capture("realloc-on-event");
stream(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, 1);
continue;
}
}
{
/* the buffers still fit: keep them, but frames from now on use the stream's
* visible size (the first event replaces the S_FMT placeholder) */
struct v4l2_format f;
memset(&f, 0, sizeof(f));
f.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
if (xioctl(VIDIOC_G_FMT, &f) == 0 &&
f.fmt.pix_mp.width == capfmt.fmt.pix_mp.width &&
f.fmt.pix_mp.height == capfmt.fmt.pix_mp.height)
read_visible("event, buffers fit");
}
if (early_start) {
early_ret = dec_cmd(V4L2_DEC_CMD_START);
printf(" immediate DEC_CMD_START (ffmpeg) -> %d%s\n", early_ret, early_ret == -EBUSY ? " (EBUSY)" : "");
}
}
}
for (;;) {
struct v4l2_buffer b; struct v4l2_plane pl[1];
memset(&b, 0, sizeof(b)); memset(pl, 0, sizeof(pl));
b.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; b.memory = V4L2_MEMORY_MMAP; b.m.planes = pl; b.length = 1;
if (xioctl(VIDIOC_DQBUF, &b))
break;
out_busy[b.index] = 0;
t_last = now();
}
while (cap_ready) {
/* events before buffers, as ffmpeg does: a pending SOURCE_CHANGE is handled first */
struct pollfd pe = { .fd = fd, .events = POLLPRI };
if (poll(&pe, 1, 0) > 0 && (pe.revents & POLLPRI))
break;
struct v4l2_buffer b; struct v4l2_plane pl[1];
memset(&b, 0, sizeof(b)); memset(pl, 0, sizeof(pl));
b.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; b.memory = V4L2_MEMORY_MMAP; b.m.planes = pl; b.length = 1;
if (xioctl(VIDIOC_DQBUF, &b)) {
if (errno == EPIPE) {
if (!epipe)
printf("DQBUF capture -> EPIPE (after LAST; ffmpeg treats this as end of stream)\n");
epipe++;
if (no_retry)
done = 1;
}
break;
}
t_last = now();
unsigned used = pl[0].bytesused;
if (b.flags & V4L2_BUF_FLAG_ERROR)
errors++;
if (b.flags & V4L2_BUF_FLAG_LAST) {
printf("LAST buffer: bytesused=%u flags=%s%s at %.1f ms\n", used,
b.flags & V4L2_BUF_FLAG_ERROR ? "ERROR|" : "", "LAST", (now() - t0) * 1000);
if (!used)
empty_last++;
else {
write_frame(b.index);
frames++;
}
if (!stop_sent) {
/* a LAST before our drain: the decoder started a Dynamic Resolution Change */
drc_seen++;
struct v4l2_format f;
memset(&f, 0, sizeof(f));
f.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
if (xioctl(VIDIOC_G_FMT, &f))
die("G_FMT after DRC");
int same = f.fmt.pix_mp.width == capfmt.fmt.pix_mp.width &&
f.fmt.pix_mp.height == capfmt.fmt.pix_mp.height &&
f.fmt.pix_mp.plane_fmt[0].sizeimage <= capfmt.fmt.pix_mp.plane_fmt[0].sizeimage;
if (same && always_realloc)
printf(" -A: format unchanged; STREAMOFF/realloc/STREAMON CAPTURE anyway\n");
if (same && !always_realloc && no_retry) {
printf(" format unchanged; NOT sending START again (ffmpeg)\n");
} else if (same && !always_realloc) {
retry_ret = dec_cmd(V4L2_DEC_CMD_START);
printf(" format unchanged; DEC_CMD_START after LAST -> %d\n", retry_ret);
qbuf_cap(b.index);
} else {
if (!same)
printf(" format changed (%ux%u); STREAMOFF/realloc/STREAMON CAPTURE\n", f.fmt.pix_mp.width, f.fmt.pix_mp.height);
stream(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, 0);
free_capture();
setup_capture("realloc");
stream(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, 1);
break;
}
} else {
done = 1;
break;
}
continue;
}
if (used && !(b.flags & V4L2_BUF_FLAG_ERROR)) {
write_frame(b.index);
frames++;
}
qbuf_cap(b.index);
}
if (now() - t_last > 5.0) {
printf("STALLED: no buffer or event for 5 s\n");
break;
}
}
{
/* an EOS for the final drain may still be queued */
struct v4l2_event ev;
while (xioctl(VIDIOC_DQEVENT, &ev) == 0)
if (ev.type == V4L2_EVENT_EOS) {
if (stop_sent)
eos_end++;
else
eos_early++;
}
}
if (cap_ready)
stream(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, 0);
stream(V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, 0);
fclose(outf);
close(fd);
printf("RESULT mode=%s E=%d R=%d S=%d: fed %zu/%zu AUs, frames=%d, source_changes=%d, aborted_drains=%d, DRC_LAST=%d, empty_LAST=%d, ERROR-flagged=%d, "
"early_START=%d, START_after_LAST=%d, EPIPE=%d, EOS_early=%d, EOS_end=%d, %.0f ms\n", mode, early_start, no_retry,
abort_drain, next, naus, frames, src_changes, aborted, drc_seen, empty_last, errors,
early_ret == 1 ? 0 : early_ret, retry_ret == 1 ? 0 : retry_ret, epipe, eos_early, eos_end, (now() - t0) * 1000);
return frames ? 0 : 1;
}
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC PATCH 3/5] media: iris: Signal V4L2_EVENT_EOS only at the end of a drain
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 2:19 ` [RFC PATCH 2/5] media: iris: Update the CAPTURE stride and size on S_FMT(OUTPUT) Christian Cortner
@ 2026-10-27 2:19 ` 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 2:19 ` [RFC PATCH 5/5] media: iris: Resume the first source change if it fits the CAPTURE setup Christian Cortner
4 siblings, 1 reply; 18+ messages in thread
From: Christian Cortner @ 2026-10-27 2:19 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
iris_vb2_buffer_done() and iris_vb2_buf_queue() queue V4L2_EVENT_EOS and
mark the m2m context as stopped for every capture buffer flagged
V4L2_BUF_FLAG_LAST, including the one that ends a dynamic resolution
change. In the stateful decoder interface the (deprecated) EOS event is
part of the drain sequence only, and a resolution change is not the end
of the stream: a client that still acts on the event, like ffmpeg's
h264_v4l2m2m decoder, stops decoding at the first resolution change in
the stream. And since nothing in the driver clears the stopped mark
again, the EOS event of the drain at the real end of such a stream is
never sent.
Queue the event, and mark the context as stopped, only when the LAST
buffer ends a drain, that is when IRIS_INST_SUB_DRAIN_LAST is set. Both
HFI generations set it before they return the LAST buffer of a drain,
for the decoder and the encoder alike.
Fixes: 84e17adae3e3 ("media: iris: add support for dynamic resolution change")
Cc: stable@vger.kernel.org
Assisted-by: LLM sparse smatch
---
Reference only, not for merging (no Signed-off-by); see the cover
letter.
drivers/media/platform/qcom/iris/iris_buffer.c | 7 ++++++-
drivers/media/platform/qcom/iris/iris_vb2.c | 4 +++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
index eb8de60c1177..ff2a1b331ac2 100644
--- a/drivers/media/platform/qcom/iris/iris_buffer.c
+++ b/drivers/media/platform/qcom/iris/iris_buffer.c
@@ -913,7 +913,12 @@ int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf)
}
if (vbuf->flags & V4L2_BUF_FLAG_LAST) {
- if (!v4l2_m2m_has_stopped(m2m_ctx)) {
+ /*
+ * Only a drain ends with V4L2_EVENT_EOS: the LAST buffer of a
+ * source change is not the end of the stream.
+ */
+ if (inst->sub_state & IRIS_INST_SUB_DRAIN_LAST &&
+ !v4l2_m2m_has_stopped(m2m_ctx)) {
const struct v4l2_event ev = { .type = V4L2_EVENT_EOS };
v4l2_event_queue_fh(&inst->fh, &ev);
diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c
index 8faf709c26c1..b820b3e43910 100644
--- a/drivers/media/platform/qcom/iris/iris_vb2.c
+++ b/drivers/media/platform/qcom/iris/iris_vb2.c
@@ -324,7 +324,9 @@ void iris_vb2_buf_queue(struct vb2_buffer *vb2)
vbuf->field = V4L2_FIELD_NONE;
vb2_set_plane_payload(vb2, 0, 0);
v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_DONE);
- if (!v4l2_m2m_has_stopped(m2m_ctx)) {
+ /* EOS ends a drain only, see iris_vb2_buffer_done() */
+ if (inst->sub_state & IRIS_INST_SUB_DRAIN_LAST &&
+ !v4l2_m2m_has_stopped(m2m_ctx)) {
v4l2_event_queue_fh(&inst->fh, &eos);
v4l2_m2m_mark_stopped(m2m_ctx);
}
--
2.56.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [RFC PATCH 4/5] media: iris: Let a resolution change drain before stopping CAPTURE
2026-10-27 2:19 ` [RFC PATCH 0/5] media: iris: ffmpeg decoder flow changes, for reference only Christian Cortner
` (2 preceding siblings ...)
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 2:19 ` Christian Cortner
2026-10-27 22:59 ` Bryan O'Donoghue
2026-10-27 2:19 ` [RFC PATCH 5/5] media: iris: Resume the first source change if it fits the CAPTURE setup Christian Cortner
4 siblings, 1 reply; 18+ messages in thread
From: Christian Cortner @ 2026-10-27 2:19 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
The stateful decoder interface lets a client abort the implicit drain of
a dynamic resolution change by stopping the CAPTURE queue, and ffmpeg's
h264_v4l2m2m decoder does so about 10 us after it dequeues the source
change event. If the stop reaches the firmware before the firmware has
reported the end of the drain (HFI_BUF_FW_FLAG_PSC_LAST), the firmware
returns all capture buffers without that flag and
IRIS_INST_SUB_DRC_LAST is never set. At the next
VIDIOC_STREAMON(CAPTURE), iris_process_streamon_output() then finds no
completed resolution change and resumes the firmware without
reallocating the internal input buffers. The firmware still returns the
PSC_LAST buffer it owes, 1.3 ms later in our trace,
iris_inst_sub_state_change_drc_last() rejects it because the input is no
longer paused, and the instance goes to IRIS_INST_ERROR: no buffer is
returned after that, and the client waits forever. Completing the
resolution change at that STREAMON instead, by reallocating the internal
buffers before resuming the firmware, did not help: the firmware then
returned nothing at all.
Wait in iris_vb2_stop_streaming(), for at most 500 ms, until the
firmware has completed the drain of a pending resolution change before
stopping CAPTURE. The client's STREAMON then takes the normal resolution
change path. The wait queue is woken wherever IRIS_INST_SUB_DRC_LAST is
set, for both HFI generations; gen1 sets it in the same event handler
that sets IRIS_INST_SUB_DRC, so there the wait ends at once.
Fixes: 84e17adae3e3 ("media: iris: add support for dynamic resolution change")
Assisted-by: LLM sparse smatch
---
Reference only, not for merging (no Signed-off-by); see the cover
letter.
.../media/platform/qcom/iris/iris_instance.h | 3 +++
drivers/media/platform/qcom/iris/iris_state.c | 8 +++++++-
drivers/media/platform/qcom/iris/iris_vb2.c | 19 +++++++++++++++++++
drivers/media/platform/qcom/iris/iris_vidc.c | 1 +
4 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
index 771269e705d9..1013ed2911a1 100644
--- a/drivers/media/platform/qcom/iris/iris_instance.h
+++ b/drivers/media/platform/qcom/iris/iris_instance.h
@@ -7,6 +7,7 @@
#define __IRIS_INSTANCE_H__
#include <linux/kref.h>
+#include <linux/wait.h>
#include <media/v4l2-ctrls.h>
@@ -56,6 +57,7 @@ struct iris_hfi_session_ops;
* @metadata_idx: index for metadata buffer
* @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
* @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
@@ -104,6 +106,7 @@ struct iris_inst {
u32 metadata_idx;
u32 codec;
bool last_buffer_dequeued;
+ wait_queue_head_t drc_wq;
u64 last_buf_ns;
u32 frame_counter;
u32 frame_rate;
diff --git a/drivers/media/platform/qcom/iris/iris_state.c b/drivers/media/platform/qcom/iris/iris_state.c
index 5552725c614e..dd8e0093b972 100644
--- a/drivers/media/platform/qcom/iris/iris_state.c
+++ b/drivers/media/platform/qcom/iris/iris_state.c
@@ -209,6 +209,7 @@ int iris_inst_sub_state_change_drain_last(struct iris_inst *inst)
int iris_inst_sub_state_change_drc_last(struct iris_inst *inst)
{
enum iris_inst_sub_state set_sub_state;
+ int ret;
if (inst->sub_state & IRIS_INST_SUB_DRC_LAST)
return -EINVAL;
@@ -222,7 +223,12 @@ int iris_inst_sub_state_change_drc_last(struct iris_inst *inst)
set_sub_state = IRIS_INST_SUB_DRC_LAST | IRIS_INST_SUB_OUTPUT_PAUSE;
- return iris_inst_change_sub_state(inst, 0, set_sub_state);
+ ret = iris_inst_change_sub_state(inst, 0, set_sub_state);
+ /* a CAPTURE STREAMOFF may be waiting, see iris_vb2_stop_streaming() */
+ if (!ret)
+ wake_up_all(&inst->drc_wq);
+
+ return ret;
}
int iris_inst_sub_state_change_pause(struct iris_inst *inst, u32 plane)
diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c
index b820b3e43910..bc2f201fea0e 100644
--- a/drivers/media/platform/qcom/iris/iris_vb2.c
+++ b/drivers/media/platform/qcom/iris/iris_vb2.c
@@ -228,6 +228,15 @@ int iris_vb2_start_streaming(struct vb2_queue *q, unsigned int count)
return ret;
}
+/* a source change whose drain the firmware has not completed yet */
+static bool iris_drc_draining(struct iris_inst *inst)
+{
+ enum iris_inst_sub_state sub_state = READ_ONCE(inst->sub_state);
+
+ return sub_state & IRIS_INST_SUB_DRC &&
+ !(sub_state & IRIS_INST_SUB_DRC_LAST);
+}
+
void iris_vb2_stop_streaming(struct vb2_queue *q)
{
struct iris_inst *inst;
@@ -238,6 +247,16 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->state == IRIS_INST_INIT)
return;
+ /*
+ * A client may abort the drain of a resolution change by stopping
+ * CAPTURE, but if the stop reaches the firmware before the drain is
+ * complete, the resolution change can never finish and decoding
+ * stalls after the next STREAMON. Let the drain complete first.
+ */
+ if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->domain == DECODER)
+ wait_event_timeout(inst->drc_wq, !iris_drc_draining(inst),
+ msecs_to_jiffies(500));
+
mutex_lock(&inst->lock);
if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
index 05aab6e3005d..0df0a5915e16 100644
--- a/drivers/media/platform/qcom/iris/iris_vidc.c
+++ b/drivers/media/platform/qcom/iris/iris_vidc.c
@@ -201,6 +201,7 @@ int iris_open(struct file *filp)
INIT_LIST_HEAD(&inst->buffers[BUF_PARTIAL].list);
init_completion(&inst->completion);
init_completion(&inst->flush_completion);
+ init_waitqueue_head(&inst->drc_wq);
iris_v4l2_fh_init(inst, filp);
--
2.56.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [RFC PATCH 5/5] media: iris: Resume the first source change if it fits the CAPTURE setup
2026-10-27 2:19 ` [RFC PATCH 0/5] media: iris: ffmpeg decoder flow changes, for reference only Christian Cortner
` (3 preceding siblings ...)
2026-10-27 2:19 ` [RFC PATCH 4/5] media: iris: Let a resolution change drain before stopping CAPTURE Christian Cortner
@ 2026-10-27 2:19 ` Christian Cortner
2026-10-27 23:04 ` Bryan O'Donoghue
4 siblings, 1 reply; 18+ messages in thread
From: Christian Cortner @ 2026-10-27 2:19 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur
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 12:43 ` Birk Skyum
2026-10-27 18:14 ` Christian Cortner
1 sibling, 1 reply; 18+ messages in thread
From: Birk Skyum @ 2026-10-27 12:43 UTC (permalink / raw)
To: Christian Cortner, Bryan O'Donoghue, Vikash Garodia,
Dikshita Agarwal
Cc: Abhinav Kumar, linux-media, linux-arm-msm
On Mon, 26 Oct 2026 21:19:32 -0500, Christian Cortner wrote:
> Birk, if you have a moment, the client would show whether X1E80100
> behaves the same; the fatal errors you saw on an unmodified driver
> might have the same cause, but I can't tell from here.
It does. Same Yoga Slim 7x and kernel as before, so the resolution
change path is unmodified, with your client built from the attachment
and your two streams (min_buffers 3 and 6 here too):
ab, START after LAST 30 of 60 frames, five fatal errors (2 of 2)
ab, -A 60 of 60, identical to ffmpeg (2 of 2)
ba, START after LAST 30 of 60 frames, no errors (2 of 2)
ba, -A 60 of 60, identical to ffmpeg (2 of 2)
On the unmodified driver ba stops as well, without the errors. After
START the client gets a second empty LAST buffer, START then returns
EBUSY, and every DQBUF gives EPIPE, which looks like the stopped mark
from your problem 3. The fatal errors come only when the count rises.
The fatal errors in my ffmpeg runs follow the buffer count too. With
h264_v4l2m2m on 3 second 640x360 MP4 clips, 24 runs each:
x264 -preset ultrafast fatal errors in 0 of 24 runs
x264 default settings fatal errors in 5 of 24 runs
Both end after one frame in nearly every run, as in problems 1 to 3.
Birk
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [BUG] media: iris: resolution change handling stops ffmpeg's h264_v4l2m2m decoder on glymur
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
0 siblings, 0 replies; 18+ messages in thread
From: Christian Cortner @ 2026-10-27 18:14 UTC (permalink / raw)
To: Birk Skyum
Cc: Bryan O'Donoghue, Vikash Garodia, Dikshita Agarwal,
Abhinav Kumar, linux-media, linux-arm-msm
On Tue, 27 Oct 2026 12:43:32 +0000, Birk Skyum wrote:
> On the unmodified driver ba stops as well, without the errors. After
> START the client gets a second empty LAST buffer, START then returns
> EBUSY, and every DQBUF gives EPIPE, which looks like the stopped mark
> from your problem 3. The fatal errors come only when the count rises.
Thanks for running it. I booted the qcom-iris.ko of my kernel
package, which has none of the five changes, and got the same here:
when the client resumed with V4L2_DEC_CMD_START, both streams stopped
after 30 of 60 frames (2 of 2 runs each), ab with the five fatal
errors. With -A both decoded bit-exact (one run each).
With kprobes on vb2_buffer_done() (recording its caller),
iris_vb2_buf_queue(), iris_dec_cmd() and iris_vdec_start_cmd(), the
stop traces back to problem 2 rather than to the stopped mark. In all
four START runs:
1. iris_vb2_buffer_done() completes the empty PSC_LAST buffer as
VB2_BUF_STATE_ERROR (the client sees flags=ERROR|LAST). For an
ERROR buffer it returns early, before it sets
inst->last_buffer_dequeued.
2. Within 0.2 ms, before it has dequeued that buffer, the client
requeues a frame buffer. DRC_LAST is set and
inst->last_buffer_dequeued is false, so iris_vb2_buf_queue()
completes it at once as a second empty LAST. That branch also
queues the EOS event and sets the stopped mark.
3. The START for the first LAST returns 0. The one for the second
returns -EBUSY without reaching iris_vdec_start_cmd(), since the
first START already ended the resolution change. vb2 then returns
EPIPE for every DQBUF: only a START that iris accepts or a CAPTURE
STREAMOFF clears last_buffer_dequeued.
The stopped mark doesn't halt decoding: in the ba runs the firmware
completes all 16 CAPTURE buffers within 11 ms of the first START,
and the client can't dequeue any of them. With -A the second LAST is
made as well, but the client's CAPTURE STREAMOFF discards it.
On my kernel with the five changes, where patch 1 stops flagging the
PSC_LAST buffer, ba decodes all 60 frames bit-exact (2 of 2 runs). In
today's run, with the same kprobes, the client got flags=LAST and
iris_vb2_buf_queue() completed no buffer. I haven't tested patch 1 on
its own.
Christian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH 1/5] media: iris: Don't flag an empty source change LAST buffer as an error
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
0 siblings, 0 replies; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-27 22:26 UTC (permalink / raw)
To: Christian Cortner, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
On 27/10/2026 03:19, Christian Cortner wrote:
> When the firmware completes a source change, it returns the last capture
> buffer from before the change flagged HFI_BUF_FW_FLAG_PSC_LAST, and that
> buffer can be empty. iris_hfi_gen2_handle_output_buffer() sets
> V4L2_BUF_FLAG_ERROR on every empty capture buffer returned while
> streaming unless it carries HFI_BUF_FW_FLAG_LAST, the flag that ends a
> drain, so the empty buffer that ends a source change reaches the client
> flagged both LAST and ERROR.
>
> The stateful decoder interface says of this buffer, as it does of the
> one that ends a drain, that it "may be empty (with v4l2_buffer bytesused
> = 0) and in that case it must be ignored by the client, as it does not
> contain a decoded frame". With the ERROR flag, ffmpeg's h264_v4l2m2m
> decoder reports "capture: driver decode error" and outputs a corrupted
> frame.
>
> Exempt HFI_BUF_FW_FLAG_PSC_LAST as well.
I really had to read the commit log, the code and then the commit log
again to understand what this was saying.
Significant reduction of prose and more concise statements.
>
> Fixes: 7adc11e6abf6 ("media: iris: Send V4L2_BUF_FLAG_ERROR for capture buffers with 0 filled length")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM sparse smatch
> ---
> Reference only, not for merging (no Signed-off-by); see the cover
> letter.
>
> drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> 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 f0782c4b1e6e..b4fe9066fcb8 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> @@ -390,8 +390,13 @@ static int iris_hfi_gen2_handle_output_buffer(struct iris_inst *inst,
>
> buf->flags = iris_hfi_gen2_get_driver_buffer_flags(inst, hfi_buffer->flags);
>
> + /*
> + * The empty buffer that ends a drain (LAST) or a source change
> + * (PSC_LAST) is not an error, the client just ignores it.
> + */
> if (!buf->data_size && inst->state == IRIS_INST_STREAMING &&
> - !(hfi_buffer->flags & HFI_BUF_FW_FLAG_LAST)) {
> + !(hfi_buffer->flags &
> + (HFI_BUF_FW_FLAG_LAST | HFI_BUF_FW_FLAG_PSC_LAST))) {
If the analysis is correct then this fix makes sense.]
Where do we get the conclusion from though - persumably from observing
packet types returned by firmware ?
That would be a more convincing thing to put into the commit log than a
log declarative.
> buf->flags |= V4L2_BUF_FLAG_ERROR;
> }
>
> --
> 2.56.0
>
---
bod
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH 2/5] media: iris: Update the CAPTURE stride and size on S_FMT(OUTPUT)
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
0 siblings, 0 replies; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-27 22:36 UTC (permalink / raw)
To: Christian Cortner, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
On 27/10/2026 03:19, Christian Cortner wrote:
> Setting the OUTPUT format also updates the width and height of the
> CAPTURE format, but bytesperline and sizeimage keep their old values,
> those of the 320x240 default on a new instance. For a 640x360 stream,
> VIDIOC_G_FMT(CAPTURE) then reports 640x384 NV12 with a sizeimage of
> 147456 bytes. VIDIOC_REQBUFS(CAPTURE) allocates buffers of that size,
> and iris_vb2_buf_prepare() rejects every one of them with -EINVAL,
> because it checks them against the size computed from the new width
> and height.
Again the commit log is a word salad. A much clearer problem statement
should be possible.
"When updating the OUTPUT format we fail to update some variables which
we should be updating i.e. bytesperline and sizeimage."
Simple and clear, like myself ;)
> Move the per-format rules S_FMT(CAPTURE) uses into a helper and call it
> for both queues, so that S_FMT(OUTPUT) leaves the CAPTURE format exactly
> as S_FMT(CAPTURE) would set it for the same size.
>
> Fixes: 8aadfd445373 ("media: iris: Update CAPTURE format info based on OUTPUT format")
> Cc:stable@vger.kernel.org
> Assisted-by: LLM sparse smatch
On the substance here I like it.
- Functional decomposition
- Reuse
So... I'd be inclined to take this. It fixes a real bug.
---
bod
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH 3/5] media: iris: Signal V4L2_EVENT_EOS only at the end of a drain
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
0 siblings, 0 replies; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-27 22:44 UTC (permalink / raw)
To: Christian Cortner, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
On 27/10/2026 03:19, Christian Cortner wrote:
> iris_vb2_buffer_done() and iris_vb2_buf_queue() queue V4L2_EVENT_EOS and
> mark the m2m context as stopped for every capture buffer flagged
> V4L2_BUF_FLAG_LAST, including the one that ends a dynamic resolution
> change. In the stateful decoder interface the (deprecated) EOS event is
> part of the drain sequence only, and a resolution change is not the end
> of the stream: a client that still acts on the event, like ffmpeg's
> h264_v4l2m2m decoder, stops decoding at the first resolution change in
> the stream. And since nothing in the driver clears the stopped mark
> again, the EOS event of the drain at the real end of such a stream is
> never sent.
More word salad. I'd almost drop that entire paragraph - hitting you
with these stuttering paragraphs is !elucidating - kind of the opposite.
> Queue the event, and mark the context as stopped, only when the LAST
> buffer ends a drain, that is when IRIS_INST_SUB_DRAIN_LAST is set. Both
> HFI generations set it before they return the LAST buffer of a drain,
> for the decoder and the encoder alike.
This is much clearer.
> Fixes: 84e17adae3e3 ("media: iris: add support for dynamic resolution change")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM sparse smatch
> ---
> Reference only, not for merging (no Signed-off-by); see the cover
> letter.
>
> drivers/media/platform/qcom/iris/iris_buffer.c | 7 ++++++-
> drivers/media/platform/qcom/iris/iris_vb2.c | 4 +++-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index eb8de60c1177..ff2a1b331ac2 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -913,7 +913,12 @@ int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf)
> }
>
> if (vbuf->flags & V4L2_BUF_FLAG_LAST) {
> - if (!v4l2_m2m_has_stopped(m2m_ctx)) {
> + /*
> + * Only a drain ends with V4L2_EVENT_EOS: the LAST buffer of a
> + * source change is not the end of the stream.
> + */
> + if (inst->sub_state & IRIS_INST_SUB_DRAIN_LAST &&
> + !v4l2_m2m_has_stopped(m2m_ctx)) {
> const struct v4l2_event ev = { .type = V4L2_EVENT_EOS };
>
> v4l2_event_queue_fh(&inst->fh, &ev);
> diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c
> index 8faf709c26c1..b820b3e43910 100644
> --- a/drivers/media/platform/qcom/iris/iris_vb2.c
> +++ b/drivers/media/platform/qcom/iris/iris_vb2.c
> @@ -324,7 +324,9 @@ void iris_vb2_buf_queue(struct vb2_buffer *vb2)
> vbuf->field = V4L2_FIELD_NONE;
> vb2_set_plane_payload(vb2, 0, 0);
> v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_DONE);
> - if (!v4l2_m2m_has_stopped(m2m_ctx)) {
> + /* EOS ends a drain only, see iris_vb2_buffer_done() */
> + if (inst->sub_state & IRIS_INST_SUB_DRAIN_LAST &&
> + !v4l2_m2m_has_stopped(m2m_ctx)) {
Adding the flag over and over again is pretty mindless.
Add a function or wrapper to capture this logic - we had functional
decomposition in the previous patch - but distinctly lacking here.
=>
$ grep -r v4l2_m2m_has_stopped drivers/media/platform/qcom/iris/*
drivers/media/platform/qcom/iris/iris_buffer.c: if
(!v4l2_m2m_has_stopped(m2m_ctx)) {
drivers/media/platform/qcom/iris/iris_vb2.c: if
(!v4l2_m2m_has_stopped(m2m_ctx)) {
> v4l2_event_queue_fh(&inst->fh, &eos);
> v4l2_m2m_mark_stopped(m2m_ctx);
> }
> --
> 2.56.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH 4/5] media: iris: Let a resolution change drain before stopping CAPTURE
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
0 siblings, 0 replies; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-27 22:59 UTC (permalink / raw)
To: Christian Cortner, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
On 27/10/2026 03:19, Christian Cortner wrote:
> The stateful decoder interface lets a client abort the implicit drain of
> a dynamic resolution change by stopping the CAPTURE queue, and ffmpeg's
> h264_v4l2m2m decoder does so about 10 us after it dequeues the source
> change event. If the stop reaches the firmware before the firmware has
> reported the end of the drain (HFI_BUF_FW_FLAG_PSC_LAST), the firmware
> returns all capture buffers without that flag and
> IRIS_INST_SUB_DRC_LAST is never set. At the next
> VIDIOC_STREAMON(CAPTURE), iris_process_streamon_output() then finds no
> completed resolution change and resumes the firmware without
> reallocating the internal input buffers. The firmware still returns the
> PSC_LAST buffer it owes, 1.3 ms later in our trace,
> iris_inst_sub_state_change_drc_last() rejects it because the input is no
> longer paused, and the instance goes to IRIS_INST_ERROR: no buffer is
> returned after that, and the client waits forever. Completing the
> resolution change at that STREAMON instead, by reallocating the internal
> buffers before resuming the firmware, did not help: the firmware then
> returned nothing at all.
I didn't even read the above part. Sorry like Pavlov's dog at this stage
I'm conditioned, its going to be more than I or really anybody reading
the log can grok, pun intended.
> Wait in iris_vb2_stop_streaming(), for at most 500 ms, until the
> firmware has completed the drain of a pending resolution change before
> stopping CAPTURE. The client's STREAMON then takes the normal resolution
> change path. The wait queue is woken wherever IRIS_INST_SUB_DRC_LAST is
> set, for both HFI generations; gen1 sets it in the same event handler
> that sets IRIS_INST_SUB_DRC, so there the wait ends at once.
>
> Fixes: 84e17adae3e3 ("media: iris: add support for dynamic resolution change")
> Assisted-by: LLM sparse smatch> Reference only, not for merging (no Signed-off-by); see the cover
> letter.
>
> .../media/platform/qcom/iris/iris_instance.h | 3 +++
> drivers/media/platform/qcom/iris/iris_state.c | 8 +++++++-
> drivers/media/platform/qcom/iris/iris_vb2.c | 19 +++++++++++++++++++
> drivers/media/platform/qcom/iris/iris_vidc.c | 1 +
> 4 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
> index 771269e705d9..1013ed2911a1 100644
> --- a/drivers/media/platform/qcom/iris/iris_instance.h
> +++ b/drivers/media/platform/qcom/iris/iris_instance.h
> @@ -7,6 +7,7 @@
> #define __IRIS_INSTANCE_H__
>
> #include <linux/kref.h>
> +#include <linux/wait.h>
>
> #include <media/v4l2-ctrls.h>
>
> @@ -56,6 +57,7 @@ struct iris_hfi_session_ops;
> * @metadata_idx: index for metadata buffer
> * @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
> * @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
> @@ -104,6 +106,7 @@ struct iris_inst {
> u32 metadata_idx;
> u32 codec;
> bool last_buffer_dequeued;
> + wait_queue_head_t drc_wq;
> u64 last_buf_ns;
> u32 frame_counter;
> u32 frame_rate;
> diff --git a/drivers/media/platform/qcom/iris/iris_state.c b/drivers/media/platform/qcom/iris/iris_state.c
> index 5552725c614e..dd8e0093b972 100644
> --- a/drivers/media/platform/qcom/iris/iris_state.c
> +++ b/drivers/media/platform/qcom/iris/iris_state.c
> @@ -209,6 +209,7 @@ int iris_inst_sub_state_change_drain_last(struct iris_inst *inst)
> int iris_inst_sub_state_change_drc_last(struct iris_inst *inst)
> {
> enum iris_inst_sub_state set_sub_state;
> + int ret;
>
> if (inst->sub_state & IRIS_INST_SUB_DRC_LAST)
> return -EINVAL;
> @@ -222,7 +223,12 @@ int iris_inst_sub_state_change_drc_last(struct iris_inst *inst)
>
> set_sub_state = IRIS_INST_SUB_DRC_LAST | IRIS_INST_SUB_OUTPUT_PAUSE;
>
> - return iris_inst_change_sub_state(inst, 0, set_sub_state);
> + ret = iris_inst_change_sub_state(inst, 0, set_sub_state);
> + /* a CAPTURE STREAMOFF may be waiting, see iris_vb2_stop_streaming() */
> + if (!ret)
> + wake_up_all(&inst->drc_wq);
> +
> + return ret;
> }
>
> int iris_inst_sub_state_change_pause(struct iris_inst *inst, u32 plane)
> diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c
> index b820b3e43910..bc2f201fea0e 100644
> --- a/drivers/media/platform/qcom/iris/iris_vb2.c
> +++ b/drivers/media/platform/qcom/iris/iris_vb2.c
> @@ -228,6 +228,15 @@ int iris_vb2_start_streaming(struct vb2_queue *q, unsigned int count)
> return ret;
> }
>
> +/* a source change whose drain the firmware has not completed yet */
> +static bool iris_drc_draining(struct iris_inst *inst)
> +{
> + enum iris_inst_sub_state sub_state = READ_ONCE(inst->sub_state);
> +
> + return sub_state & IRIS_INST_SUB_DRC &&
> + !(sub_state & IRIS_INST_SUB_DRC_LAST);
> +}
> +
> void iris_vb2_stop_streaming(struct vb2_queue *q)
> {
> struct iris_inst *inst;
> @@ -238,6 +247,16 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
> if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->state == IRIS_INST_INIT)
> return;
>
> + /*
> + * A client may abort the drain of a resolution change by stopping
> + * CAPTURE, but if the stop reaches the firmware before the drain is
> + * complete, the resolution change can never finish and decoding
> + * stalls after the next STREAMON. Let the drain complete first.
> + */
> + if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->domain == DECODER)
> + wait_event_timeout(inst->drc_wq, !iris_drc_draining(inst),
> + msecs_to_jiffies(500));
The comment and the code are in conflict - it doesn't unconditionally
let the drain complete. Its within half a second.
And actually why should domain == DECODER be required ?
Seems to me also if the timeout is exceeded then that's a thing worth
complaining about in the kernel log.
These also seem like nits I'm picking. Introducing a rendezvous is
potentially the right solution here.
Certainly warrants further thinking about.
> +
> mutex_lock(&inst->lock);
>
> if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
> index 05aab6e3005d..0df0a5915e16 100644
> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
> @@ -201,6 +201,7 @@ int iris_open(struct file *filp)
> INIT_LIST_HEAD(&inst->buffers[BUF_PARTIAL].list);
> init_completion(&inst->completion);
> init_completion(&inst->flush_completion);
> + init_waitqueue_head(&inst->drc_wq);
>
> iris_v4l2_fh_init(inst, filp);
>
> --
> 2.56.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH 5/5] media: iris: Resume the first source change if it fits the CAPTURE setup
2026-10-27 2:19 ` [RFC PATCH 5/5] media: iris: Resume the first source change if it fits the CAPTURE setup Christian Cortner
@ 2026-10-27 23:04 ` Bryan O'Donoghue
0 siblings, 0 replies; 18+ messages in thread
From: Bryan O'Donoghue @ 2026-10-27 23:04 UTC (permalink / raw)
To: Christian Cortner, Vikash Garodia, Dikshita Agarwal
Cc: Abhinav Kumar, Birk Skyum, linux-media, linux-arm-msm
On 27/10/2026 03:19, Christian Cortner wrote:
> 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/
Right I was about to say this seems like _alot_ of code to be sending
back to -stable
"Not working for most streams" - fair enough thanks for sharing all the
same.
---
bod
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-10-27 23:04 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[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 ` [RFC PATCH 5/5] media: iris: Resume the first source change if it fits the CAPTURE setup Christian Cortner
2026-10-27 23:04 ` 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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.