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

[Vedant Paranjape]

HI Laurent,

On Tue, Nov 30, 2021 at 4:04 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> On Tue, Nov 30, 2021 at 07:28:39PM +0900, paul.elder at ideasonboard.com wrote:
> > 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.
>
> And I don't think there's more feedback that can be provided on this
> patch alone without the rest of the implementation, we need to look at
> it in its entirety.
>
> RFC submissions should make it clear what comments are requested, and
> why they're sent as RFCs and not normal patches.

Sure, I'll keep this in mind next time I send a patch (or RFC patch).

>
> > > > >  }
> > > > >
> > > > >  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;
>
> --
> Regards,
>
> Laurent Pinchart

Regards,
Vedant Paranjape