All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bryan O'Donoghue <bod@kernel.org>
To: Christian Cortner <ccortner@gmail.com>,
	Vikash Garodia <vikash.garodia@oss.qualcomm.com>,
	Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Cc: Abhinav Kumar <abhinav.kumar@linux.dev>,
	Birk Skyum <birk.skyum@pm.me>,
	linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [RFC PATCH 3/5] media: iris: Signal V4L2_EVENT_EOS only at the end of a drain
Date: Tue, 27 Oct 2026 23:44:32 +0100	[thread overview]
Message-ID: <7ee16d85-9cc2-48a0-ab38-3eb131c6282f@kernel.org> (raw)
In-Reply-To: <20261027021934.22113-4-ccortner@gmail.com>

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
> 


  reply	other threads:[~2026-10-27 22:44 UTC|newest]

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

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=7ee16d85-9cc2-48a0-ab38-3eb131c6282f@kernel.org \
    --to=bod@kernel.org \
    --cc=abhinav.kumar@linux.dev \
    --cc=birk.skyum@pm.me \
    --cc=ccortner@gmail.com \
    --cc=dikshita.agarwal@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=vikash.garodia@oss.qualcomm.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is 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.