[{"id":21289,"web_url":"https://patchwork.libcamera.org/comment/21289/","msgid":"<YaKOPCEF4Qrrw4O6@pendragon.ideasonboard.com>","date":"2021-11-27T19:59:56","subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Vedant,\n\nThank you for the patch.\n\nOn Sun, Nov 28, 2021 at 12:54:40AM +0530, Vedant Paranjape wrote:\n> To support DMABUF as one of the memory buffer, we need to implement\n> EXPBUF in the v4l2 compat layer.\n> \n> This patch implements vidioc_expbuf as one of the supported ioctls.\n> \n> Reference issue: https://bugs.libcamera.org/show_bug.cgi?id=89\n\nWe use\n\nBug: https://bugs.libcamera.org/show_bug.cgi?id=89\n\n> Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> ---\n>  src/v4l2/v4l2_camera_proxy.cpp | 41 +++++++++++++++++++++++++++++++++-\n>  src/v4l2/v4l2_camera_proxy.h   |  1 +\n>  2 files changed, 41 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 3610e63cade3..03e3564bfed5 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -160,7 +160,8 @@ bool V4L2CameraProxy::validateBufferType(uint32_t type)\n>  \n>  bool V4L2CameraProxy::validateMemoryType(uint32_t memory)\n>  {\n> -\treturn memory == V4L2_MEMORY_MMAP;\n> +\treturn (memory == V4L2_MEMORY_MMAP) ||\n> +\t\t\t(memory == V4L2_MEMORY_DMABUF);\n\nThis has the side effect that, for instance, REQBUFS will accept the\nmemory type DMABUF, but without DMABUF support actually implemented. Why\ndo you need this change as part of this patch ?\n\n>  }\n>  \n>  void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)\n> @@ -624,6 +625,40 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n>  \treturn 0;\n>  }\n>  \n> +int V4L2CameraProxy::vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg)\n> +{\n> +\tLOG(V4L2Compat, Debug) << \"Servicing vidioc_expbuf fd = \" << file->efd();\n> +\t/* \\todo Verify that the memory type is MMAP when adding DMABUF support */\n> +\tif (!validateBufferType(arg->type))\n> +\t\treturn -EINVAL;\n> +\n> +\tif (arg->index >= bufferCount_)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (arg->flags & ~(O_CLOEXEC | O_ACCMODE))\n> +\t\treturn -EINVAL;\n> +\n> +\tLOG(V4L2Compat, Debug) << arg->index << \" : index of the buffer\";\n> +\n> +\tif (file->priority() < maxPriority())\n> +\t\treturn -EBUSY;\n> +\n> +\tif (!hasOwnership(file) && owner_)\n> +\t\treturn -EBUSY;\n\nI don't think this is right, you can't EXPBUF without calling REQBUFS\nfirst, which has an impact on ownership. No cargo cult copy&paste\nplease.\n\n> +\n> +\tmemset(arg->reserved, 0, sizeof(arg->reserved));\n> +\n> +\targ->fd = fcntl(buffers_[arg->index].m.fd,\n> +\t\t\targ->flags & O_CLOEXEC ? F_DUPFD_CLOEXEC : F_DUPFD, 0);\n> +\targ->fd = fcntl(arg->fd, F_SETFL, 0);\n\n0 ?\n\n> +\n> +\tLOG(V4L2Compat, Debug) << \"Exported buffer at index: \" << arg->index;\n> +\n> +\tacquire(file);\n\nSame comment regarding ownership.\n\n> +\n> +\treturn 0;\n> +}\n> +\n>  int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << file->efd();\n> @@ -685,6 +720,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n>  \tVIDIOC_QUERYBUF,\n>  \tVIDIOC_QBUF,\n>  \tVIDIOC_DQBUF,\n> +\tVIDIOC_EXPBUF,\n>  \tVIDIOC_STREAMON,\n>  \tVIDIOC_STREAMOFF,\n>  };\n> @@ -755,6 +791,9 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar\n>  \tcase VIDIOC_DQBUF:\n>  \t\tret = vidioc_dqbuf(file, static_cast<struct v4l2_buffer *>(arg), &locker);\n>  \t\tbreak;\n> +\tcase VIDIOC_EXPBUF:\n> +\t\tret = vidioc_expbuf(file, static_cast<struct v4l2_exportbuffer *>(arg));\n> +\t\tbreak;\n>  \tcase VIDIOC_STREAMON:\n>  \t\tret = vidioc_streamon(file, static_cast<int *>(arg));\n>  \t\tbreak;\n> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> index fccec241879d..81ef7788e9fe 100644\n> --- a/src/v4l2/v4l2_camera_proxy.h\n> +++ b/src/v4l2/v4l2_camera_proxy.h\n> @@ -58,6 +58,7 @@ private:\n>  \tint vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n>  \tint vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n>  \t\t\t libcamera::MutexLocker *locker);\n> +\tint vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg);\n>  \tint vidioc_streamon(V4L2CameraFile *file, int *arg);\n>  \tint vidioc_streamoff(V4L2CameraFile *file, int *arg);\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0A3BCBDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 27 Nov 2021 20:00:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4F78C60574;\n\tSat, 27 Nov 2021 21:00:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 678CF6011E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Nov 2021 21:00:19 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D6C362C5;\n\tSat, 27 Nov 2021 21:00:18 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BFIoJNiC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638043219;\n\tbh=gcNFHLwXFXpC62eIhNIdqxSsixbZpUBXs/0UQRGsS0Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BFIoJNiCFZ2tKPz6+gEfbm2TCkQwRFKP3O6snDmsJAsGTPcY2Zrrvs3qB2md/c+5z\n\tUxL+riTFPFfk8Y9d+M9XgQE2vZmlRbr65r2v+pYfR4KIBgfVdPT+aBKwxMD8QgmVFX\n\tZEbgy5phhmuWTdgUdZRh+G5R9+Ijvi8JPh3PtdZk=","Date":"Sat, 27 Nov 2021 21:59:56 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Message-ID":"<YaKOPCEF4Qrrw4O6@pendragon.ideasonboard.com>","References":"<20211127192440.389551-1-vedantparanjape160201@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211127192440.389551-1-vedantparanjape160201@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":21290,"web_url":"https://patchwork.libcamera.org/comment/21290/","msgid":"<CACGrz-O6prgGBV=+oUYSUnwmvoBrtaQJqYuZT1=rSarxzJX2=A@mail.gmail.com>","date":"2021-11-27T20:22:55","subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"Hello Laurent,\nThanks for the review.\n\nOn Sun, 28 Nov, 2021, 01:30 Laurent Pinchart,\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Vedant,\n>\n> Thank you for the patch.\n>\n> On Sun, Nov 28, 2021 at 12:54:40AM +0530, Vedant Paranjape wrote:\n> > To support DMABUF as one of the memory buffer, we need to implement\n> > EXPBUF in the v4l2 compat layer.\n> >\n> > This patch implements vidioc_expbuf as one of the supported ioctls.\n> >\n> > Reference issue: https://bugs.libcamera.org/show_bug.cgi?id=89\n>\n> We use\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=89\n\nNoted.\n\n>\n> > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> > ---\n> >  src/v4l2/v4l2_camera_proxy.cpp | 41 +++++++++++++++++++++++++++++++++-\n> >  src/v4l2/v4l2_camera_proxy.h   |  1 +\n> >  2 files changed, 41 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > index 3610e63cade3..03e3564bfed5 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > @@ -160,7 +160,8 @@ bool V4L2CameraProxy::validateBufferType(uint32_t type)\n> >\n> >  bool V4L2CameraProxy::validateMemoryType(uint32_t memory)\n> >  {\n> > -     return memory == V4L2_MEMORY_MMAP;\n> > +     return (memory == V4L2_MEMORY_MMAP) ||\n> > +                     (memory == V4L2_MEMORY_DMABUF);\n>\n> This has the side effect that, for instance, REQBUFS will accept the\n> memory type DMABUF, but without DMABUF support actually implemented. Why\n> do you need this change as part of this patch ?\n\nRight, will remove it.\n\n> >  }\n> >\n> >  void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)\n> > @@ -624,6 +625,40 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> >       return 0;\n> >  }\n> >\n> > +int V4L2CameraProxy::vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg)\n> > +{\n> > +     LOG(V4L2Compat, Debug) << \"Servicing vidioc_expbuf fd = \" << file->efd();\n> > +     /* \\todo Verify that the memory type is MMAP when adding DMABUF support */\n> > +     if (!validateBufferType(arg->type))\n> > +             return -EINVAL;\n> > +\n> > +     if (arg->index >= bufferCount_)\n> > +             return -EINVAL;\n> > +\n> > +     if (arg->flags & ~(O_CLOEXEC | O_ACCMODE))\n> > +             return -EINVAL;\n> > +\n> > +     LOG(V4L2Compat, Debug) << arg->index << \" : index of the buffer\";\n> > +\n> > +     if (file->priority() < maxPriority())\n> > +             return -EBUSY;\n> > +\n> > +     if (!hasOwnership(file) && owner_)\n> > +             return -EBUSY;\n>\n> I don't think this is right, you can't EXPBUF without calling REQBUFS\n> first, which has an impact on ownership.\n\nBut, there's no way to know that reqbuf is the owner and was called before this.\n\n> No cargo cult copy&paste please.\nLMAO, okay ;)\n\n> > +\n> > +     memset(arg->reserved, 0, sizeof(arg->reserved));\n> > +\n> > +     arg->fd = fcntl(buffers_[arg->index].m.fd,\n> > +                     arg->flags & O_CLOEXEC ? F_DUPFD_CLOEXEC : F_DUPFD, 0);\n> > +     arg->fd = fcntl(arg->fd, F_SETFL, 0);\n>\n> 0 ?\n>\n> > +\n> > +     LOG(V4L2Compat, Debug) << \"Exported buffer at index: \" << arg->index;\n> > +\n> > +     acquire(file);\n>\n> Same comment regarding ownership.\n>\n> > +\n> > +     return 0;\n> > +}\n> > +\n> >  int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)\n> >  {\n> >       LOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << file->efd();\n> > @@ -685,6 +720,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n> >       VIDIOC_QUERYBUF,\n> >       VIDIOC_QBUF,\n> >       VIDIOC_DQBUF,\n> > +     VIDIOC_EXPBUF,\n> >       VIDIOC_STREAMON,\n> >       VIDIOC_STREAMOFF,\n> >  };\n> > @@ -755,6 +791,9 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar\n> >       case VIDIOC_DQBUF:\n> >               ret = vidioc_dqbuf(file, static_cast<struct v4l2_buffer *>(arg), &locker);\n> >               break;\n> > +     case VIDIOC_EXPBUF:\n> > +             ret = vidioc_expbuf(file, static_cast<struct v4l2_exportbuffer *>(arg));\n> > +             break;\n> >       case VIDIOC_STREAMON:\n> >               ret = vidioc_streamon(file, static_cast<int *>(arg));\n> >               break;\n> > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> > index fccec241879d..81ef7788e9fe 100644\n> > --- a/src/v4l2/v4l2_camera_proxy.h\n> > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > @@ -58,6 +58,7 @@ private:\n> >       int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> >       int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> >                        libcamera::MutexLocker *locker);\n> > +     int vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg);\n> >       int vidioc_streamon(V4L2CameraFile *file, int *arg);\n> >       int vidioc_streamoff(V4L2CameraFile *file, int *arg);\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n\nRegards,\nVedant Paranjape","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 28EB7BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 27 Nov 2021 20:23:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7EA586011D;\n\tSat, 27 Nov 2021 21:23:10 +0100 (CET)","from mail-yb1-xb31.google.com (mail-yb1-xb31.google.com\n\t[IPv6:2607:f8b0:4864:20::b31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 037C86011D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Nov 2021 21:23:08 +0100 (CET)","by mail-yb1-xb31.google.com with SMTP id f9so29665312ybq.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Nov 2021 12:23:08 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"bIidd/0+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=mXG8x16xAbw1mjMT2nyn/nmD9s69nXBz/+e2D9iBBOo=;\n\tb=bIidd/0+5Wt2Li3B9PXmyj2bTGvHVP29YW++IEW3a9Z5fcPfp3+3n6v/afmAfC3brw\n\tKNMt9rMFPmUe4W3i2Nkj264CeroBm5Gn6d040gX9ia+cd6j6DtRW8QsrxH07DuITWGTC\n\t1oQW48PJZ1VbJaMo+zPkT7WXNAP/YkkczU6J85SNgYkw7qQ4C9DsZSS7+CmHy/jRxQLY\n\tuI23VivV2yEHyLJ7LnPXrIVxZzVo63UC8WcyLsY3TDPx2OOj9ASMjyQn0fZQf4SYOqpc\n\taS77oP2LI4S72TNtzDq0avP2G0XIDVou3xXRMmfFSRoPe4DLo6uTG7nA7Ftpd5pvroJX\n\tSAaw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=mXG8x16xAbw1mjMT2nyn/nmD9s69nXBz/+e2D9iBBOo=;\n\tb=56Se8tflUVLL7mNd3c8GsFCiHHojwXpYqq2wMemkaq9P6Hj7tuTJ7V1IUiZ48R/tOP\n\tOKPym58Pyg1UpZmKHFRBxKmWxVA2OhAKKTDkYZnAG7YsBSZispCmXaNGxufk7tUKqzS+\n\tp0rt8WhjYRxX3Gry5fx5wI/JN9J1gvrwYozdMQlDe1Pk90COiKUczFxFbUCaXIc1HlPE\n\t+D/ZnI09nqo17OALNxNXRzT3UaaHAwzQwxhwihdO8Z85VfPNcqob6D9CuVQrtC3DnWsf\n\tv9lP9Q+8x4Q6TkEke2RjA8eMpsOhWM0TKVJdsNF/OiTrrbkMsd082rzV+JkWKY4u3dTo\n\tm7ZQ==","X-Gm-Message-State":"AOAM532BS3drbOnKv0Su5zNBurPAp3kRRYh3azw2iNCYUhLKPt1RTyTy\n\txgNtsBvBFnqI6QMYEaRbObPy0caBtA5qtCencpQ=","X-Google-Smtp-Source":"ABdhPJyyoHw1WtCC3HkpaSoCQfoxGN1LI6jyb049Wbl1pqdK6AsnhjGvkLe13FfzBfu1exqorrDfI5RqXICwBf6WrGU=","X-Received":"by 2002:a25:bd45:: with SMTP id\n\tp5mr27286359ybm.213.1638044587256; \n\tSat, 27 Nov 2021 12:23:07 -0800 (PST)","MIME-Version":"1.0","References":"<20211127192440.389551-1-vedantparanjape160201@gmail.com>\n\t<YaKOPCEF4Qrrw4O6@pendragon.ideasonboard.com>","In-Reply-To":"<YaKOPCEF4Qrrw4O6@pendragon.ideasonboard.com>","From":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Date":"Sun, 28 Nov 2021 01:52:55 +0530","Message-ID":"<CACGrz-O6prgGBV=+oUYSUnwmvoBrtaQJqYuZT1=rSarxzJX2=A@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":21291,"web_url":"https://patchwork.libcamera.org/comment/21291/","msgid":"<YaKUyPCjn2IVDtHT@pendragon.ideasonboard.com>","date":"2021-11-27T20:27:52","subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Vedant,\n\nOn Sun, Nov 28, 2021 at 01:52:55AM +0530, Vedant Paranjape wrote:\n> On Sun, 28 Nov, 2021, 01:30 Laurent Pinchart wrote:\n> > On Sun, Nov 28, 2021 at 12:54:40AM +0530, Vedant Paranjape wrote:\n> > > To support DMABUF as one of the memory buffer, we need to implement\n> > > EXPBUF in the v4l2 compat layer.\n> > >\n> > > This patch implements vidioc_expbuf as one of the supported ioctls.\n> > >\n> > > Reference issue: https://bugs.libcamera.org/show_bug.cgi?id=89\n> >\n> > We use\n> >\n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=89\n> \n> Noted.\n> \n> > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> > > ---\n> > >  src/v4l2/v4l2_camera_proxy.cpp | 41 +++++++++++++++++++++++++++++++++-\n> > >  src/v4l2/v4l2_camera_proxy.h   |  1 +\n> > >  2 files changed, 41 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > index 3610e63cade3..03e3564bfed5 100644\n> > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > @@ -160,7 +160,8 @@ bool V4L2CameraProxy::validateBufferType(uint32_t type)\n> > >\n> > >  bool V4L2CameraProxy::validateMemoryType(uint32_t memory)\n> > >  {\n> > > -     return memory == V4L2_MEMORY_MMAP;\n> > > +     return (memory == V4L2_MEMORY_MMAP) ||\n> > > +                     (memory == V4L2_MEMORY_DMABUF);\n> >\n> > This has the side effect that, for instance, REQBUFS will accept the\n> > memory type DMABUF, but without DMABUF support actually implemented. Why\n> > do you need this change as part of this patch ?\n> \n> Right, will remove it.\n> \n> > >  }\n> > >\n> > >  void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)\n> > > @@ -624,6 +625,40 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > >       return 0;\n> > >  }\n> > >\n> > > +int V4L2CameraProxy::vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg)\n> > > +{\n> > > +     LOG(V4L2Compat, Debug) << \"Servicing vidioc_expbuf fd = \" << file->efd();\n> > > +     /* \\todo Verify that the memory type is MMAP when adding DMABUF support */\n> > > +     if (!validateBufferType(arg->type))\n> > > +             return -EINVAL;\n> > > +\n> > > +     if (arg->index >= bufferCount_)\n> > > +             return -EINVAL;\n> > > +\n> > > +     if (arg->flags & ~(O_CLOEXEC | O_ACCMODE))\n> > > +             return -EINVAL;\n> > > +\n> > > +     LOG(V4L2Compat, Debug) << arg->index << \" : index of the buffer\";\n> > > +\n> > > +     if (file->priority() < maxPriority())\n> > > +             return -EBUSY;\n> > > +\n> > > +     if (!hasOwnership(file) && owner_)\n> > > +             return -EBUSY;\n> >\n> > I don't think this is right, you can't EXPBUF without calling REQBUFS\n> > first, which has an impact on ownership.\n> \n> But, there's no way to know that reqbuf is the owner and was called before this.\n\nIsn't there ?\n\n> > No cargo cult copy&paste please.\n>\n> LMAO, okay ;)\n> \n> > > +\n> > > +     memset(arg->reserved, 0, sizeof(arg->reserved));\n> > > +\n> > > +     arg->fd = fcntl(buffers_[arg->index].m.fd,\n> > > +                     arg->flags & O_CLOEXEC ? F_DUPFD_CLOEXEC : F_DUPFD, 0);\n> > > +     arg->fd = fcntl(arg->fd, F_SETFL, 0);\n> >\n> > 0 ?\n> >\n> > > +\n> > > +     LOG(V4L2Compat, Debug) << \"Exported buffer at index: \" << arg->index;\n> > > +\n> > > +     acquire(file);\n> >\n> > Same comment regarding ownership.\n> >\n> > > +\n> > > +     return 0;\n> > > +}\n> > > +\n> > >  int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)\n> > >  {\n> > >       LOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << file->efd();\n> > > @@ -685,6 +720,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n> > >       VIDIOC_QUERYBUF,\n> > >       VIDIOC_QBUF,\n> > >       VIDIOC_DQBUF,\n> > > +     VIDIOC_EXPBUF,\n> > >       VIDIOC_STREAMON,\n> > >       VIDIOC_STREAMOFF,\n> > >  };\n> > > @@ -755,6 +791,9 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar\n> > >       case VIDIOC_DQBUF:\n> > >               ret = vidioc_dqbuf(file, static_cast<struct v4l2_buffer *>(arg), &locker);\n> > >               break;\n> > > +     case VIDIOC_EXPBUF:\n> > > +             ret = vidioc_expbuf(file, static_cast<struct v4l2_exportbuffer *>(arg));\n> > > +             break;\n> > >       case VIDIOC_STREAMON:\n> > >               ret = vidioc_streamon(file, static_cast<int *>(arg));\n> > >               break;\n> > > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> > > index fccec241879d..81ef7788e9fe 100644\n> > > --- a/src/v4l2/v4l2_camera_proxy.h\n> > > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > > @@ -58,6 +58,7 @@ private:\n> > >       int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > >       int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > >                        libcamera::MutexLocker *locker);\n> > > +     int vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg);\n> > >       int vidioc_streamon(V4L2CameraFile *file, int *arg);\n> > >       int vidioc_streamoff(V4L2CameraFile *file, int *arg);\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2F69ABDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 27 Nov 2021 20:28:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 516436057B;\n\tSat, 27 Nov 2021 21:28:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1CAE26011D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Nov 2021 21:28:16 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 817F4340;\n\tSat, 27 Nov 2021 21:28:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gKIK3zI+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638044895;\n\tbh=vZ1a7O73UngLVJehTXm3SpfkU1VwvVT2hqxFnfdEYDU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gKIK3zI+7JJJf+FMVItk8fbWFFjBev3+IIEEdf6wz3fpJs0ca5JAyFZSmDB609oC0\n\tnqeD9nhkO9mVpJ6EGUJKFHtyCgjPk2it4++2OA+ZNgB+8YHX2OvOZYYfoI/D762QH1\n\tTbfyluWj4JKEpWFhNWuLjKGH3zP8hlFbz3BcGyrw=","Date":"Sat, 27 Nov 2021 22:27:52 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Message-ID":"<YaKUyPCjn2IVDtHT@pendragon.ideasonboard.com>","References":"<20211127192440.389551-1-vedantparanjape160201@gmail.com>\n\t<YaKOPCEF4Qrrw4O6@pendragon.ideasonboard.com>\n\t<CACGrz-O6prgGBV=+oUYSUnwmvoBrtaQJqYuZT1=rSarxzJX2=A@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CACGrz-O6prgGBV=+oUYSUnwmvoBrtaQJqYuZT1=rSarxzJX2=A@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":21292,"web_url":"https://patchwork.libcamera.org/comment/21292/","msgid":"<CACGrz-Psqt5Bz7a4ozpqabLXFmcByrnDWBRu5Rq3ksx+1SQZ=g@mail.gmail.com>","date":"2021-11-27T20:41:28","subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","submitter":{"id":85,"url":"https://patchwork.libcamera.org/api/people/85/","name":"Vedant Paranjape","email":"vedantparanjape160201@gmail.com"},"content":"On Sun, Nov 28, 2021 at 1:58 AM Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Vedant,\n>\n> On Sun, Nov 28, 2021 at 01:52:55AM +0530, Vedant Paranjape wrote:\n> > On Sun, 28 Nov, 2021, 01:30 Laurent Pinchart wrote:\n> > > On Sun, Nov 28, 2021 at 12:54:40AM +0530, Vedant Paranjape wrote:\n> > > > To support DMABUF as one of the memory buffer, we need to implement\n> > > > EXPBUF in the v4l2 compat layer.\n> > > >\n> > > > This patch implements vidioc_expbuf as one of the supported ioctls.\n> > > >\n> > > > Reference issue: https://bugs.libcamera.org/show_bug.cgi?id=89\n> > >\n> > > We use\n> > >\n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=89\n> >\n> > Noted.\n> >\n> > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> > > > ---\n> > > >  src/v4l2/v4l2_camera_proxy.cpp | 41 +++++++++++++++++++++++++++++++++-\n> > > >  src/v4l2/v4l2_camera_proxy.h   |  1 +\n> > > >  2 files changed, 41 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > index 3610e63cade3..03e3564bfed5 100644\n> > > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > @@ -160,7 +160,8 @@ bool V4L2CameraProxy::validateBufferType(uint32_t type)\n> > > >\n> > > >  bool V4L2CameraProxy::validateMemoryType(uint32_t memory)\n> > > >  {\n> > > > -     return memory == V4L2_MEMORY_MMAP;\n> > > > +     return (memory == V4L2_MEMORY_MMAP) ||\n> > > > +                     (memory == V4L2_MEMORY_DMABUF);\n> > >\n> > > This has the side effect that, for instance, REQBUFS will accept the\n> > > memory type DMABUF, but without DMABUF support actually implemented. Why\n> > > do you need this change as part of this patch ?\n> >\n> > Right, will remove it.\n> >\n> > > >  }\n> > > >\n> > > >  void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)\n> > > > @@ -624,6 +625,40 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > > >       return 0;\n> > > >  }\n> > > >\n> > > > +int V4L2CameraProxy::vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg)\n> > > > +{\n> > > > +     LOG(V4L2Compat, Debug) << \"Servicing vidioc_expbuf fd = \" << file->efd();\n> > > > +     /* \\todo Verify that the memory type is MMAP when adding DMABUF support */\n> > > > +     if (!validateBufferType(arg->type))\n> > > > +             return -EINVAL;\n> > > > +\n> > > > +     if (arg->index >= bufferCount_)\n> > > > +             return -EINVAL;\n> > > > +\n> > > > +     if (arg->flags & ~(O_CLOEXEC | O_ACCMODE))\n> > > > +             return -EINVAL;\n> > > > +\n> > > > +     LOG(V4L2Compat, Debug) << arg->index << \" : index of the buffer\";\n> > > > +\n> > > > +     if (file->priority() < maxPriority())\n> > > > +             return -EBUSY;\n> > > > +\n> > > > +     if (!hasOwnership(file) && owner_)\n> > > > +             return -EBUSY;\n> > >\n> > > I don't think this is right, you can't EXPBUF without calling REQBUFS\n> > > first, which has an impact on ownership.\n> >\n> > But, there's no way to know that reqbuf is the owner and was called before this.\n>\n> Isn't there ?\n\nI am a bit confused as to how this will work. Can you hint it out please.\n\n>\n> > > No cargo cult copy&paste please.\n> >\n> > LMAO, okay ;)\n> >\n> > > > +\n> > > > +     memset(arg->reserved, 0, sizeof(arg->reserved));\n> > > > +\n> > > > +     arg->fd = fcntl(buffers_[arg->index].m.fd,\n> > > > +                     arg->flags & O_CLOEXEC ? F_DUPFD_CLOEXEC : F_DUPFD, 0);\n> > > > +     arg->fd = fcntl(arg->fd, F_SETFL, 0);\n> > >\n> > > 0 ?\n> > >\n> > > > +\n> > > > +     LOG(V4L2Compat, Debug) << \"Exported buffer at index: \" << arg->index;\n> > > > +\n> > > > +     acquire(file);\n> > >\n> > > Same comment regarding ownership.\n> > >\n> > > > +\n> > > > +     return 0;\n> > > > +}\n> > > > +\n> > > >  int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)\n> > > >  {\n> > > >       LOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << file->efd();\n> > > > @@ -685,6 +720,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n> > > >       VIDIOC_QUERYBUF,\n> > > >       VIDIOC_QBUF,\n> > > >       VIDIOC_DQBUF,\n> > > > +     VIDIOC_EXPBUF,\n> > > >       VIDIOC_STREAMON,\n> > > >       VIDIOC_STREAMOFF,\n> > > >  };\n> > > > @@ -755,6 +791,9 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar\n> > > >       case VIDIOC_DQBUF:\n> > > >               ret = vidioc_dqbuf(file, static_cast<struct v4l2_buffer *>(arg), &locker);\n> > > >               break;\n> > > > +     case VIDIOC_EXPBUF:\n> > > > +             ret = vidioc_expbuf(file, static_cast<struct v4l2_exportbuffer *>(arg));\n> > > > +             break;\n> > > >       case VIDIOC_STREAMON:\n> > > >               ret = vidioc_streamon(file, static_cast<int *>(arg));\n> > > >               break;\n> > > > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> > > > index fccec241879d..81ef7788e9fe 100644\n> > > > --- a/src/v4l2/v4l2_camera_proxy.h\n> > > > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > > > @@ -58,6 +58,7 @@ private:\n> > > >       int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > > >       int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > > >                        libcamera::MutexLocker *locker);\n> > > > +     int vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg);\n> > > >       int vidioc_streamon(V4L2CameraFile *file, int *arg);\n> > > >       int vidioc_streamoff(V4L2CameraFile *file, int *arg);\n> > > >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9C4C1BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 27 Nov 2021 20:41:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D671B6057F;\n\tSat, 27 Nov 2021 21:41:42 +0100 (CET)","from mail-yb1-xb34.google.com (mail-yb1-xb34.google.com\n\t[IPv6:2607:f8b0:4864:20::b34])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6A6C56011D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Nov 2021 21:41:41 +0100 (CET)","by mail-yb1-xb34.google.com with SMTP id d10so29835297ybe.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Nov 2021 12:41:41 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"htHvh/sy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=7jPxGWvm7788HJDpEJI4eBH23acTmf+EQHUH4q6OvYs=;\n\tb=htHvh/syCz2PnmyGckBvCoyROlzzFN6XF2ZzbCw+6gaFMpDMCUpwgbNgqarqHTgZ9L\n\t6qcLTB5faI18qTHA4ur0ytDE3Vpx8pahaYhz+ADCfsKHiOcp45caCyr/SAbSgpKhLtKK\n\t+uYtAWRxZqOXxBtgej951b9pfRL8uLdSzAV5abqyFj5RWxvPCQGlzZ9m88Qa7uM3IVhu\n\t/w4DXNdw47ZhBbLAE9ucRU98PNNb8mpWyj15tgFhr/PwmuKbQOex5jYbgXbLtIXEEE5Z\n\t+oCxBDrrh9w8SZ90Zsn/B1KuDbGKyjMUGDN+eymV7FsQGmf4RMu3y4jNUwndGqt8TWBT\n\tC6bw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=7jPxGWvm7788HJDpEJI4eBH23acTmf+EQHUH4q6OvYs=;\n\tb=DdbPI3xV5Dd0B7PKbzxt6dcxcFbuNQbfQRb1VfxD7pJS9bfYK4Q0pDM4VFwMrZHT/X\n\tiTa1+CGISwOLICBHf876Scw1OInNta8V6jabwLDhaghpjwqXpUqGLjdCI2RDHDfZA/GE\n\twrZDZh8CFxmbmr6SsutpxDj2fDiQ0MNo6jpdbeH/sPegm22blZzcjZ4YxmRqExgN9Pr6\n\tO+sNkwjLlFwH7jYy/c1S75SpReai8D5GaktKwcZqfWp2jXBo0bsFjp4DDXRMsxs8dRwg\n\tmLMudWEWY9aZkZtO+BiFZUJC51StCWsFfGFP+Vs8uJT353NI+POc3UlR2Zxzm5/qyQYR\n\tInUA==","X-Gm-Message-State":"AOAM5300EMYP47uPnc0WmTph5sEFZKIzofzeqEd/56sZID0gabefwKsF\n\tWMMVaSE0zmwxb3b/EQbU2KfwbQFjAXoqqHPZXgQ=","X-Google-Smtp-Source":"ABdhPJxdaYM2v8VezI36KXnXG9QWnM4BFeR5F3FaDiKFU8+u1gnCqy2LLzP46S9cs/e1oX2RB6G175FsVKhOl4OBjj8=","X-Received":"by 2002:a25:2395:: with SMTP id\n\tj143mr24771516ybj.415.1638045700195; \n\tSat, 27 Nov 2021 12:41:40 -0800 (PST)","MIME-Version":"1.0","References":"<20211127192440.389551-1-vedantparanjape160201@gmail.com>\n\t<YaKOPCEF4Qrrw4O6@pendragon.ideasonboard.com>\n\t<CACGrz-O6prgGBV=+oUYSUnwmvoBrtaQJqYuZT1=rSarxzJX2=A@mail.gmail.com>\n\t<YaKUyPCjn2IVDtHT@pendragon.ideasonboard.com>","In-Reply-To":"<YaKUyPCjn2IVDtHT@pendragon.ideasonboard.com>","From":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Date":"Sun, 28 Nov 2021 02:11:28 +0530","Message-ID":"<CACGrz-Psqt5Bz7a4ozpqabLXFmcByrnDWBRu5Rq3ksx+1SQZ=g@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":21293,"web_url":"https://patchwork.libcamera.org/comment/21293/","msgid":"<YaKeUbrSw+9P19m6@pendragon.ideasonboard.com>","date":"2021-11-27T21:08:33","subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sun, Nov 28, 2021 at 02:11:28AM +0530, Vedant Paranjape wrote:\n> On Sun, Nov 28, 2021 at 1:58 AM Laurent Pinchart wrote:\n> > On Sun, Nov 28, 2021 at 01:52:55AM +0530, Vedant Paranjape wrote:\n> > > On Sun, 28 Nov, 2021, 01:30 Laurent Pinchart wrote:\n> > > > On Sun, Nov 28, 2021 at 12:54:40AM +0530, Vedant Paranjape wrote:\n> > > > > To support DMABUF as one of the memory buffer, we need to implement\n> > > > > EXPBUF in the v4l2 compat layer.\n> > > > >\n> > > > > This patch implements vidioc_expbuf as one of the supported ioctls.\n> > > > >\n> > > > > Reference issue: https://bugs.libcamera.org/show_bug.cgi?id=89\n> > > >\n> > > > We use\n> > > >\n> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=89\n> > >\n> > > Noted.\n> > >\n> > > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>\n> > > > > ---\n> > > > >  src/v4l2/v4l2_camera_proxy.cpp | 41 +++++++++++++++++++++++++++++++++-\n> > > > >  src/v4l2/v4l2_camera_proxy.h   |  1 +\n> > > > >  2 files changed, 41 insertions(+), 1 deletion(-)\n> > > > >\n> > > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > index 3610e63cade3..03e3564bfed5 100644\n> > > > > --- a/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> > > > > @@ -160,7 +160,8 @@ bool V4L2CameraProxy::validateBufferType(uint32_t type)\n> > > > >\n> > > > >  bool V4L2CameraProxy::validateMemoryType(uint32_t memory)\n> > > > >  {\n> > > > > -     return memory == V4L2_MEMORY_MMAP;\n> > > > > +     return (memory == V4L2_MEMORY_MMAP) ||\n> > > > > +                     (memory == V4L2_MEMORY_DMABUF);\n> > > >\n> > > > This has the side effect that, for instance, REQBUFS will accept the\n> > > > memory type DMABUF, but without DMABUF support actually implemented. Why\n> > > > do you need this change as part of this patch ?\n> > >\n> > > Right, will remove it.\n> > >\n> > > > >  }\n> > > > >\n> > > > >  void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)\n> > > > > @@ -624,6 +625,40 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > > > >       return 0;\n> > > > >  }\n> > > > >\n> > > > > +int V4L2CameraProxy::vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg)\n> > > > > +{\n> > > > > +     LOG(V4L2Compat, Debug) << \"Servicing vidioc_expbuf fd = \" << file->efd();\n> > > > > +     /* \\todo Verify that the memory type is MMAP when adding DMABUF support */\n> > > > > +     if (!validateBufferType(arg->type))\n> > > > > +             return -EINVAL;\n> > > > > +\n> > > > > +     if (arg->index >= bufferCount_)\n> > > > > +             return -EINVAL;\n> > > > > +\n> > > > > +     if (arg->flags & ~(O_CLOEXEC | O_ACCMODE))\n> > > > > +             return -EINVAL;\n> > > > > +\n> > > > > +     LOG(V4L2Compat, Debug) << arg->index << \" : index of the buffer\";\n> > > > > +\n> > > > > +     if (file->priority() < maxPriority())\n> > > > > +             return -EBUSY;\n> > > > > +\n> > > > > +     if (!hasOwnership(file) && owner_)\n> > > > > +             return -EBUSY;\n> > > >\n> > > > I don't think this is right, you can't EXPBUF without calling REQBUFS\n> > > > first, which has an impact on ownership.\n> > >\n> > > But, there's no way to know that reqbuf is the owner and was called before this.\n> >\n> > Isn't there ?\n> \n> I am a bit confused as to how this will work. Can you hint it out please.\n\nYou already check if arg->index >= bufferCount_ and fail if it is. This\nwill catch cases where REQBUFS hasn't been called, as bufferCount_ will\nbe equal to 0 in that case.\n\nFor EXPBUF to succeed, REQBUFS must have been called, so someone has\nownership already. There's no need to acquire it here, you can check if\nyou're the owner, and if not, return -EBUSY.\n\nTo match the behaviour of the kernel, the ownership check should come\nfirst.\n\n> > > > No cargo cult copy&paste please.\n> > >\n> > > LMAO, okay ;)\n> > >\n> > > > > +\n> > > > > +     memset(arg->reserved, 0, sizeof(arg->reserved));\n> > > > > +\n> > > > > +     arg->fd = fcntl(buffers_[arg->index].m.fd,\n> > > > > +                     arg->flags & O_CLOEXEC ? F_DUPFD_CLOEXEC : F_DUPFD, 0);\n> > > > > +     arg->fd = fcntl(arg->fd, F_SETFL, 0);\n> > > >\n> > > > 0 ?\n> > > >\n> > > > > +\n> > > > > +     LOG(V4L2Compat, Debug) << \"Exported buffer at index: \" << arg->index;\n> > > > > +\n> > > > > +     acquire(file);\n> > > >\n> > > > Same comment regarding ownership.\n> > > >\n> > > > > +\n> > > > > +     return 0;\n> > > > > +}\n> > > > > +\n> > > > >  int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)\n> > > > >  {\n> > > > >       LOG(V4L2Compat, Debug) << \"Servicing vidioc_streamon fd = \" << file->efd();\n> > > > > @@ -685,6 +720,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {\n> > > > >       VIDIOC_QUERYBUF,\n> > > > >       VIDIOC_QBUF,\n> > > > >       VIDIOC_DQBUF,\n> > > > > +     VIDIOC_EXPBUF,\n> > > > >       VIDIOC_STREAMON,\n> > > > >       VIDIOC_STREAMOFF,\n> > > > >  };\n> > > > > @@ -755,6 +791,9 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar\n> > > > >       case VIDIOC_DQBUF:\n> > > > >               ret = vidioc_dqbuf(file, static_cast<struct v4l2_buffer *>(arg), &locker);\n> > > > >               break;\n> > > > > +     case VIDIOC_EXPBUF:\n> > > > > +             ret = vidioc_expbuf(file, static_cast<struct v4l2_exportbuffer *>(arg));\n> > > > > +             break;\n> > > > >       case VIDIOC_STREAMON:\n> > > > >               ret = vidioc_streamon(file, static_cast<int *>(arg));\n> > > > >               break;\n> > > > > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> > > > > index fccec241879d..81ef7788e9fe 100644\n> > > > > --- a/src/v4l2/v4l2_camera_proxy.h\n> > > > > +++ b/src/v4l2/v4l2_camera_proxy.h\n> > > > > @@ -58,6 +58,7 @@ private:\n> > > > >       int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);\n> > > > >       int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,\n> > > > >                        libcamera::MutexLocker *locker);\n> > > > > +     int vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg);\n> > > > >       int vidioc_streamon(V4L2CameraFile *file, int *arg);\n> > > > >       int vidioc_streamoff(V4L2CameraFile *file, int *arg);\n> > > > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A74D0BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 27 Nov 2021 21:08:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0AF4C6057B;\n\tSat, 27 Nov 2021 22:08:58 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 24CE26011D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 27 Nov 2021 22:08:57 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8AA442C5;\n\tSat, 27 Nov 2021 22:08:56 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"s+lGtXmv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638047336;\n\tbh=/MmbFGHZ4rg2ZMorNJu3Jed3CKmQ/+EnUSeLm3mOyWs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=s+lGtXmvTFDTMLABK+V6wihu09+VeGH0SZvlI+Lji87E7GOGnEL6NI2cXoTn5qzs0\n\t20EKpCVUz3yO6uFW3pLLJ2KRmWpCzVK+muHtm7qWjlppTkmgE1gdKV+WZZSi0kOGMa\n\t0OaEhaqV96yHFOQd3efnv9KU9U4GMSbn27glrzYI=","Date":"Sat, 27 Nov 2021 23:08:33 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Vedant Paranjape <vedantparanjape160201@gmail.com>","Message-ID":"<YaKeUbrSw+9P19m6@pendragon.ideasonboard.com>","References":"<20211127192440.389551-1-vedantparanjape160201@gmail.com>\n\t<YaKOPCEF4Qrrw4O6@pendragon.ideasonboard.com>\n\t<CACGrz-O6prgGBV=+oUYSUnwmvoBrtaQJqYuZT1=rSarxzJX2=A@mail.gmail.com>\n\t<YaKUyPCjn2IVDtHT@pendragon.ideasonboard.com>\n\t<CACGrz-Psqt5Bz7a4ozpqabLXFmcByrnDWBRu5Rq3ksx+1SQZ=g@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CACGrz-Psqt5Bz7a4ozpqabLXFmcByrnDWBRu5Rq3ksx+1SQZ=g@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH] v4l2: V4L2CameraProxy: Add EXPBUF as\n\tone of the supported ioctl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]