Re: [RFC 0/3] media: videobuf2: Allow driver to override vb2_queue.buf_ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux