[{"id":3401,"web_url":"https://patchwork.libcamera.org/comment/3401/","msgid":"<20200110232745.GD4859@pendragon.ideasonboard.com>","date":"2020-01-10T23:27:45","subject":"Re: [libcamera-devel] [PATCH v3 05/33] v4l2: camera: Handle memory\n\tmapping of buffers directly","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Fri, Jan 10, 2020 at 08:37:40PM +0100, Niklas Söderlund wrote:\n> In the upcoming FrameBuffer API the memory mapping of buffers will be\n> left to the user of the FrameBuffer objects. Prepare the V4L2\n> compatibility layer to this upcoming change to ease conversion to the\n> new API.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/v4l2/v4l2_camera.cpp | 12 ++++++------\n> src/v4l2/v4l2_camera.h | 4 ++--\n> src/v4l2/v4l2_camera_proxy.cpp | 20 +++++++++++++++-----\n> src/v4l2/v4l2_camera_proxy.h | 2 +-\n> src/v4l2/v4l2_compat_manager.cpp | 2 +-\n> 5 files changed, 25 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> index 2a507b9bb8318025..ba3783876101b284 100644\n> --- a/src/v4l2/v4l2_camera.cpp\n> +++ b/src/v4l2/v4l2_camera.cpp\n> @@ -121,12 +121,6 @@ int V4L2Camera::configure(StreamConfiguration *streamConfigOut,\n> \treturn 0;\n> }\n> \n> -void *V4L2Camera::mmap(unsigned int index)\n> -{\n> -\tStream *stream = *camera_->streams().begin();\n> -\treturn stream->buffers()[index].planes()[0].mem();\n> -}\n> -\n> int V4L2Camera::allocBuffers(unsigned int count)\n> {\n> \tint ret = camera_->allocateBuffers();\n> @@ -138,6 +132,12 @@ void V4L2Camera::freeBuffers()\n> \tcamera_->freeBuffers();\n> }\n> \n> +int V4L2Camera::getBufferFd(unsigned int index)\n\nHow about returning a FileDescriptor instance, especially given that\nfurther in the series you will get a FileDescriptor from FrameBuffer.\n\n> +{\n> +\tStream *stream = *camera_->streams().begin();\n> +\treturn stream->buffers()[index].planes()[0].dmabuf();\n> +}\n> +\n> int V4L2Camera::streamOn()\n> {\n> \tif (isRunning_)\n> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h\n> index 81f7908e5e8a6beb..afa20a232235e778 100644\n> --- a/src/v4l2/v4l2_camera.h\n> +++ b/src/v4l2/v4l2_camera.h\n> @@ -53,14 +53,14 @@ public:\n> \tvoid getStreamConfig(StreamConfiguration *streamConfig);\n> \tstd::vector completedBuffers();\n> \n> -\tvoid *mmap(unsigned int index);\n> -\n> \tint configure(StreamConfiguration *streamConfigOut,\n> \t\t const Size &size, PixelFormat pixelformat,\n> \t\t unsigned int bufferCount);\n> \n> \tint allocBuffers(unsigned int count);\n> \tvoid freeBuffers();\n> +\tint getBufferFd(unsigned int index);\n> +\n> \tint streamOn();\n> \tint streamOff();\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 52f8468cdaa06a7a..3c8eba6b29be4811 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -75,7 +75,8 @@ void V4L2CameraProxy::close()\n> \tvcam_->invokeMethod(&V4L2Camera::close, ConnectionTypeBlocking);\n> }\n> \n> -void *V4L2CameraProxy::mmap(size_t length, int prot, int flags, off_t offset)\n> +void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,\n> +\t\t\t off_t offset)\n> {\n> \tLOG(V4L2Compat, Debug) << \"Servicing mmap\";\n> \n> @@ -91,13 +92,22 @@ void *V4L2CameraProxy::mmap(size_t length, int prot, int flags, off_t offset)\n> \t\treturn MAP_FAILED;\n> \t}\n> \n> -\tvoid *val = vcam_->invokeMethod(&V4L2Camera::mmap,\n> -\t\t\t\t\tConnectionTypeBlocking, index);\n> +\tint fd = vcam_->invokeMethod(&V4L2Camera::getBufferFd,\n> +\t\t\t\t ConnectionTypeBlocking, index);\n> +\tif (fd < 0) {\n> +\t\terrno = -fd;\n> +\t\treturn MAP_FAILED;\n> +\t}\n> +\n> +\tvoid *map = V4L2CompatManager::instance()->fops().mmap(addr, length, prot,\n> +\t\t\t\t\t\t\t flags, fd, 0);\n> +\tif (map == MAP_FAILED)\n> +\t\treturn map;\n> \n> \tbuffers_[index].flags |= V4L2_BUF_FLAG_MAPPED;\n> -\tmmaps_[val] = index;\n> +\tmmaps_[map] = index;\n> \n> -\treturn val;\n> +\treturn map;\n> }\n> \n> int V4L2CameraProxy::munmap(void *addr, size_t length)\n\nWe also need to patch munmap() to call fops().munmap(). This should be\nfairly trivial as V4L2CompatManager::munmap() doesn't allow partial\nunmapping, so we don't need to care about that. You just need to call\nfops().munmap() in V4L2CameraProxy::munmap() without real error handling\n(but logging an Error message would be useful).\n\nReviewed-by: Laurent Pinchart \n\n> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> index b59d19d707590d88..c8e61adf80f1b93b 100644\n> --- a/src/v4l2/v4l2_camera_proxy.h\n> +++ b/src/v4l2/v4l2_camera_proxy.h\n> @@ -28,7 +28,7 @@ public:\n> \tint open(bool nonBlocking);\n> \tvoid dup();\n> \tvoid close();\n> -\tvoid *mmap(size_t length, int prot, int flags, off_t offset);\n> +\tvoid *mmap(void *addr, size_t length, int prot, int flags, off_t offset);\n> \tint munmap(void *addr, size_t length);\n> \n> \tint ioctl(unsigned long request, void *arg);\n> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> index fe453445a9fb2cdd..f5a7b2ac4229d5d5 100644\n> --- a/src/v4l2/v4l2_compat_manager.cpp\n> +++ b/src/v4l2/v4l2_compat_manager.cpp\n> @@ -223,7 +223,7 @@ void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,\n> \tif (!proxy)\n> \t\treturn fops_.mmap(addr, length, prot, flags, fd, offset);\n> \n> -\tvoid *map = proxy->mmap(length, prot, flags, offset);\n> +\tvoid *map = proxy->mmap(addr, length, prot, flags, offset);\n> \tif (map == MAP_FAILED)\n> \t\treturn map;\n>","headers":{"Return-Path":"","Received":["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 453526067C\n\tfor ;\n\tSat, 11 Jan 2020 00:27:59 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B385052F;\n\tSat, 11 Jan 2020 00:27:58 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578698878;\n\tbh=2WQMaZmqp0vEFBh+t+8/Y4EhZ6/kjZ9oADhuULy4rM4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=twtYO3bk+eQnJl5Bz2m+RNKPJAy04coNRltwoLVEY5KoOpcB7DHwPq9EgNiTSl0gI\n\tB17PylWpE6hH24l9lPEX6CprCUJ3uTwpMUqqwoVaMby8NT+fnFncoS3v5oHKO5cpd3\n\tCn0dJvy1Yfdi9iiJZTLO5Q34cvnSpZFWq4C2R4Ls=","Date":"Sat, 11 Jan 2020 01:27:45 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200110232745.GD4859@pendragon.ideasonboard.com>","References":"<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>\n\t<20200110193808.2266294-6-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200110193808.2266294-6-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 05/33] v4l2: camera: Handle memory\n\tmapping of buffers directly","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 10 Jan 2020 23:27:59 -0000"}}]