public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Nicolas Dufresne <nicolas@ndufresne.ca>
Cc: Stefan Klug <stefan.klug@ideasonboard.com>,
	Xavier Roumegue <xavier.roumegue@oss.nxp.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Clark Williams <clrkwllms@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-rt-devel@lists.linux.dev, Hans Verkuil <hans@jjverkuil.nl>
Subject: Re: [PATCH 1/4] media: dw100: Implement V4L2 requests support
Date: Tue, 6 Jan 2026 17:45:17 +0200	[thread overview]
Message-ID: <20260106154517.GD26157@pendragon.ideasonboard.com> (raw)
In-Reply-To: <f4eef909a0f9b787dd4720fe005de0c4e41f5c5a.camel@ndufresne.ca>

On Tue, Jan 06, 2026 at 10:41:56AM -0500, Nicolas Dufresne wrote:
> Hi Laurent,
> 
> Le mardi 06 janvier 2026 à 16:53 +0200, Laurent Pinchart a écrit :
> > CC'ing Hans Verkuil for two questions below.
> > > 
> 
> [...]
> 
> > > The docs say "a buffer that was never queued to the driver but is
> > > associated with a queued request was canceled..." So to my understanding
> > > the only purpose of this function is to mark all controls in the request
> > > as being handled, so that the request can be finished.
> > 
> > That doesn't explain why it should be done twice per request. Hans,
> > could you clarify this ?
> 
> I explained it in another thread, it is only called if device_run() is not going
> to be called, so only once. vb2 does not have access to the the control handler,
> so it can't call the v4l2_ctrl_request_complete() fonction directly.

But the function is called per queue. If a buffer has been queued on
both the capture and the output queue for a request, won't the operation
be called twice with the same request ?

> [...]
> 
> > > Nicolas, if I go you right you mean that one might be tempted to do
> > > 
> > > v4l2_ctrl_request_setup()
> > > v4l2_m2m_buf_done(src)
> > > v4l2_m2m_buf_done(dst)
> > > v4l2_ctrl_request_complete()
> > > 
> > > which feels natural from the names of the functions, but the
> > > v4l2_m2m_buf_done(src) might complete the request if it has no
> > > associated controls and therefore the later v4l2_ctrl_request_complete()
> > > would fail...
> > > 
> > > I see that the usage of v4l2_m2m_buf_done_and_job_finish() is more
> > > compact and will use that in v2. For the sake of understanding: The only
> > > functional issue with my code is that v4l2_m2m_buf_done(src_buf) is
> > > called before v4l2_m2m_buf_done(dest_buf), right?
> > 
> > Is that an issue, why would the destination buffer need to be completed
> > first ?
> 
> The VB2 media_request_object is being removed from the request once the OUTPUT
> (src) buffer is marked done. If this was the last media_request_object, the
> request will move to completed state, which will signal its FD.

I'll reply to this separately, I need to read the code first.

> If you do that before you mark the CAPTURE (dst) buffer as done, an application
> that uses non blocking IOs may endup calling DQBUF(dst) too soon, which will
> return EBUSY. Since we really want the request FD to be used as the only
> synchronisation point, we made the rule that the request FD must be signalled
> last. Since its error prone, and its not illegal to synchronise on the device
> read/write/pri state, we made a condensed helper for it.
> 
> Alternatively, the manual request completion is being added for cases this
> implicit request completion does not work, or when it makes everything too
> complicated to adhere to this rule. This is the case for dual-stage video
> decoders (MTK/RPi).
> 
> [...]

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2026-01-06 15:45 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-05 11:35 [PATCH 0/4] media: dw100: Dynamic vertex map updates and fixes for PREEMPT_RT Stefan Klug
2026-01-05 11:35 ` [PATCH 1/4] media: dw100: Implement V4L2 requests support Stefan Klug
2026-01-05 18:46   ` Nicolas Dufresne
2026-01-06  0:33     ` Laurent Pinchart
2026-01-06 14:16       ` Stefan Klug
2026-01-06 14:35         ` Nicolas Dufresne
2026-01-06 14:59           ` Laurent Pinchart
2026-01-06 15:44             ` Nicolas Dufresne
2026-01-06 14:53         ` Laurent Pinchart
2026-01-06 15:41           ` Nicolas Dufresne
2026-01-06 15:45             ` Laurent Pinchart [this message]
2026-01-06 15:56               ` Nicolas Dufresne
2026-01-06 17:25                 ` Laurent Pinchart
2026-01-05 11:35 ` [PATCH 2/4] media: dw100: Implement dynamic vertex map update Stefan Klug
2026-01-05 18:58   ` Nicolas Dufresne
2026-01-06  0:42     ` Laurent Pinchart
2026-01-06 13:47       ` Nicolas Dufresne
2026-01-06 14:29         ` Stefan Klug
2026-01-06 15:27           ` Nicolas Dufresne
2026-01-06 17:30             ` Laurent Pinchart
2026-01-06 14:35         ` Laurent Pinchart
2026-01-05 11:35 ` [PATCH 3/4] media: dw100: Fix kernel oops with PREEMPT_RT enabled Stefan Klug
2026-01-05 19:02   ` Nicolas Dufresne
2026-01-05 23:59     ` Laurent Pinchart
2026-01-06  0:39       ` Steven Rostedt
2026-01-06  0:49         ` Laurent Pinchart
2026-01-06 17:11           ` Stefan Klug
2026-01-12 11:43             ` Sebastian Andrzej Siewior
2026-01-14 17:22               ` Stefan Klug
2026-01-23  8:24                 ` Sebastian Andrzej Siewior
2026-01-05 11:35 ` [PATCH 4/4] media: dw100: Split interrupt handler to fix timeout error Stefan Klug
2026-01-05 19:03   ` Nicolas Dufresne
2026-01-05 21:37     ` Steven Rostedt
2026-01-05 23:44       ` Laurent Pinchart
2026-01-06  0:43         ` Steven Rostedt
2026-01-06  0:51           ` Laurent Pinchart
2026-01-06  0:57             ` Steven Rostedt

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=20260106154517.GD26157@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=bigeasy@linutronix.de \
    --cc=clrkwllms@kernel.org \
    --cc=hans@jjverkuil.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=mchehab@kernel.org \
    --cc=nicolas@ndufresne.ca \
    --cc=rostedt@goodmis.org \
    --cc=stefan.klug@ideasonboard.com \
    --cc=xavier.roumegue@oss.nxp.com \
    /path/to/YOUR_REPLY

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

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