From: Tomasz Figa <tfiga@chromium.org>
To: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Sakari Ailus <sakari.ailus@iki.fi>,
Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [RFC 0/3] media: videobuf2: Allow driver to override vb2_queue.buf_ops
Date: Wed, 12 Jun 2024 14:45:53 +0900 [thread overview]
Message-ID: <si5iwxt3zngrlfizbrnmqjcqgiihj7gxfye45aqrej7lpb2xmk@eprxnhmikiix> (raw)
In-Reply-To: <r3xmeidoe462onnhh4oetc23kyxy5ohymema6ry2w7haqmv6de@zef57ojr7deg>
On Wed, Jun 05, 2024 at 09:37:01AM +0200, Jacopo Mondi wrote:
> Hi Laurent
>
> On Wed, Jun 05, 2024 at 09:49:09AM GMT, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > On Mon, Jun 03, 2024 at 05:25:44PM +0200, Jacopo Mondi wrote:
> > > Hello
> > >
> > > I have the need to allocate a scratch buffer to mirror the content of the
> > > vb2_buffer planes (more detail on this on request).
> > >
> > > The allocation of such 'scratch' buffer should ideally be done once, at buffer
> > > creation time (and released at buffer release time ?)
> > >
> > > Looking at the videobuf2 framework implementation I noticed that the ideal entry
> > > point for this would be vb2_queue.buf_ops.init_buffer, which is called in the
> > > __vb2_queue_alloc() call path.
> > >
> > > I have noticed that the vb2_queue.buf_ops members seems to be there to be made
> > > overridable by drivers, but are instead:
> > >
> > > 1) unconditionally set by the framework in vb2_queue_init_name()
> > > 2) the core helpers are not exported
> > >
> > > hence I was wondering if this is the result some half-baked attempt to make
> > > them ovverridable or the possibility of override them was instead deprecated.
> > > As I found no traces of this in the log, I thought it was easier to send an
> > > RFC.
> > >
> > > I also checked what other entry points I could have used to allocate backing
> > > memory for a buffer, and I have considered vb2_queue.vb2_ops.buf_init which
> > > is however called in the vb2_req_prepare() call path (I'm not using the request
> > > API atm) or in the VIDIOC_PREPARE_BUF call path, which requires ad-hoc
> > > instrumentation in user space (something I would like to avoid if possibile).
> > >
> > > What do you think ?
> >
> > I've been thinking more about this. I wonder if you could use
> > .buf_init() for your use case. It's called in three places:
> >
> > - __vb2_queue_alloc()
>
> This is only called
>
> if (memory == VB2_MEMORY_MMAP)
>
> and I originally considered it a non viable solution, as it only
> supports the MMAP use case. Now that I thought about it a few more
> seconds, I realized that MMAP it's the only actual use case where
> memory is allocated by the driver and thus the only memory management
> method that makes sense to pair with buf_init
>
> > - __prepare_userptr()
> > - __prepare_dmabuf()
>
> These, if I'm not mistaken are in VIDIOC_PREPARE_BUF call
>
> >
> > As your scratch buffer needs are limited to the ISP parameters queue,
> > which should use MMAP only, I think .buf_init() would be just fine.
> >
>
> Probably so, I'll give it a go
I agree with Laurent that .buf_init() should be a good place for this.
Please let us know if it works for you.
Best regards,
Tomasz
>
> Thanks!
>
> > > Jacopo Mondi (3):
> > > media: videobuf2: WARN if !vb2_queue.buf_ops
> > > media: Allow drivers to overwrite vb2_queue.buf_ops
> > > media: rkisp1-params: Override vb2_queue.buf_ops
> > >
> > > .../media/common/videobuf2/videobuf2-core.c | 12 ++++---
> > > .../media/common/videobuf2/videobuf2-v4l2.c | 34 +++++++++++--------
> > > .../platform/rockchip/rkisp1/rkisp1-params.c | 18 +++++++++-
> > > include/media/videobuf2-core.h | 7 ++++
> > > include/media/videobuf2-v4l2.h | 8 +++++
> > > 5 files changed, 60 insertions(+), 19 deletions(-)
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
>
prev parent reply other threads:[~2024-06-12 5:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-03 15:25 [RFC 0/3] media: videobuf2: Allow driver to override vb2_queue.buf_ops Jacopo Mondi
2024-06-03 15:25 ` [RFC 1/3] media: videobuf2: WARN if !vb2_queue.buf_ops Jacopo Mondi
2024-06-03 15:25 ` [RFC 2/3] media: Allow drivers to overwrite vb2_queue.buf_ops Jacopo Mondi
2024-06-03 15:25 ` [RFC 3/3] media: rkisp1-params: Override vb2_queue.buf_ops Jacopo Mondi
2024-06-05 6:49 ` [RFC 0/3] media: videobuf2: Allow driver to override vb2_queue.buf_ops Laurent Pinchart
2024-06-05 7:37 ` Jacopo Mondi
2024-06-12 5:45 ` Tomasz Figa [this message]
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=si5iwxt3zngrlfizbrnmqjcqgiihj7gxfye45aqrej7lpb2xmk@eprxnhmikiix \
--to=tfiga@chromium.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=jacopo.mondi@ideasonboard.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=sakari.ailus@iki.fi \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).