[libcamera-devel] [PATCH v2] v4l2: V4L2CameraProxy: Add support for DMABUF buffer I/O in REQBUF ioctl

[paul.elder at ideasonboard.com]

On Tue, Nov 30, 2021 at 03:02:15PM +0530, Vedant Paranjape wrote:
> Hi Laurent,
> I wanted to send this as an RFC, my bad.

If it was an RFC it would've be nice to know it was an RFC :D

> 
> On Tue, Nov 30, 2021 at 2:55 PM Laurent Pinchart
> <laurent.pinchart at ideasonboard.com> wrote:
> >
> > Hi Vedant,
> >
> > On Tue, Nov 30, 2021 at 02:28:30PM +0530, Vedant Paranjape wrote:
> > > To support importing DMABUF, we need to reqbufs with DMABUF mode.
> > >
> > > This patch enables using V4L2_MEMORY_DMABUF as one of the memory types
> > > in vidioc_reqbuf to initialise DMA Buffer I/O
> >
> > Have you tested that this patch enables DMABUF support ?
> >
> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=89
> > >
> > > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> > > ---
> > >  src/v4l2/v4l2_camera_proxy.cpp | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> > > index f194e06345b7..49db766d9f9d 100644
> > > --- a/src/v4l2/v4l2_camera_proxy.cpp
> > > +++ b/src/v4l2/v4l2_camera_proxy.cpp
> > > @@ -160,7 +160,8 @@ bool V4L2CameraProxy::validateBufferType(uint32_t type)
> > >
> > >  bool V4L2CameraProxy::validateMemoryType(uint32_t memory)
> > >  {
> > > -     return memory == V4L2_MEMORY_MMAP;
> > > +     return (memory == V4L2_MEMORY_MMAP) ||
> > > +            (memory == V4L2_MEMORY_DMABUF);
> >
> > No need for parentheses.
> >
> > As pointed by Paul, this introduces an issue in the EXPBUF
> > implementation.
> >
> > Please stop ignoring review comments when sending new versions, or I'll
> > have to ignore your patches.
> 
> I'm extremely sorry, didn't mean to ignore. I had a chat with Paul
> regarding solving this, I wanted to send this as an RFC instead.

So clearly this patch depends on the other stuff that you were planning
to send separately from this patch. Which means that on this patch you
need to (besides mentioning that it's RFC) mention that you depend on X
features that you'll send patches for later.


Paul

> 
> >
> > >  }
> > >
> > >  void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)
> > > @@ -468,7 +469,8 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
> > >       if (!hasOwnership(file) && owner_)
> > >               return -EBUSY;
> > >
> > > -     arg->capabilities = V4L2_BUF_CAP_SUPPORTS_MMAP;
> > > +     arg->capabilities = V4L2_BUF_CAP_SUPPORTS_MMAP |
> > > +                         V4L2_BUF_CAP_SUPPORTS_DMABUF;
> > >       memset(arg->reserved, 0, sizeof(arg->reserved));
> > >
> > >       if (arg->count == 0) {
> > > @@ -511,7 +513,7 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf
> > >               struct v4l2_buffer buf = {};
> > >               buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > >               buf.length = v4l2PixFormat_.sizeimage;
> > > -             buf.memory = V4L2_MEMORY_MMAP;
> > > +             buf.memory = arg->memory;
> > >               buf.m.offset = i * v4l2PixFormat_.sizeimage;
> > >               buf.index = i;
> > >               buf.flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> >