{"id":14157,"url":"https://patchwork.libcamera.org/api/patches/14157/?format=json","web_url":"https://patchwork.libcamera.org/patch/14157/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20211015141656.242247-1-kieran.bingham@ideasonboard.com>","date":"2021-10-15T14:16:56","name":"[libcamera-devel,IPU3-IPA] libcamera-helpers: Integrate latest MappedFrameBuffer","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"f2baa95a669025ccea0004d7513c78bcb0c40f28","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/?format=json","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/14157/mbox/","series":[{"id":2632,"url":"https://patchwork.libcamera.org/api/series/2632/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2632","date":"2021-10-15T14:16:56","name":"[libcamera-devel,IPU3-IPA] libcamera-helpers: Integrate latest MappedFrameBuffer","version":1,"mbox":"https://patchwork.libcamera.org/series/2632/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/14157/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/14157/checks/","tags":{},"headers":{"Return-Path":"","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 75189C323E\n\tfor ;\n\tFri, 15 Oct 2021 14:17:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED0C568F4A;\n\tFri, 15 Oct 2021 16:17:01 +0200 (CEST)","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 C346568546\n\tfor ;\n\tFri, 15 Oct 2021 16:16:59 +0200 (CEST)","from Monstersaurus.ksquared.org.uk.beta.tailscale.net\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 48DB22E3;\n\tFri, 15 Oct 2021 16:16:59 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uHMBe/sR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634307419;\n\tbh=5ddUBykmljV5vQYmyN6RPcjAPfjW40C1ab9rHPzyWnw=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=uHMBe/sRcKl5YlAppws3NU+a+DuuzzHpLp0hxjSjOkIWsCmn1jT5ZYMN3yQ78MsjS\n\tOjOh3btePu1+MGOkY0VeVA5nI8YEs40aVvjxK/fdgm+LUQnpPmFMaR/KT0dtJi7Vnc\n\t4F7y5pOVD2YX564wZQ215q3eDbYgVpmXA2gnasmg=","From":"Kieran Bingham ","To":"libcamera devel ","Date":"Fri, 15 Oct 2021 15:16:56 +0100","Message-Id":"<20211015141656.242247-1-kieran.bingham@ideasonboard.com>","X-Mailer":"git-send-email 2.30.2","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [IPU3-IPA PATCH] libcamera-helpers: Integrate\n\tlatest MappedFrameBuffer","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The MappedFrameBuffer helper class has been updated in the libcamera\nsource code.\n\nThis makes use of the new enum MapFlags type, and corrects the mapping\nchanges that were made during 8708904fad6f (\"libcamera:\nmapped_framebuffer: Return plane begin address by MappedBuffer::maps()\")\n\nThis update also brings back isolated IPA functionality to this external\nIPA, which is otherwise broken due to the offset/plane changes.\n\nThe files are renamed to mapped_framebuffer to match the naming in\nlibcamera, but are kept within the 'libcamera-helper' hierarchy of the\nIPA.\n\nAlso, a minor todo is added to IPAIPU3::mapBuffers, to highlight that we\ncould potentially map Statistics buffers as read only rather than\nread/write if we could correctly identify them.\n\nSigned-off-by: Kieran Bingham \n---\n include/libcamera-helpers/mapped_buffer.h | 53 ---------\n .../libcamera-helpers/mapped_framebuffer.h | 65 +++++++++++\n ipu3.cpp | 8 +-\n ...pped_buffer.cpp => mapped_framebuffer.cpp} | 110 +++++++++++++++---\n src/libcamera-helpers/meson.build | 2 +-\n 5 files changed, 163 insertions(+), 75 deletions(-)\n delete mode 100644 include/libcamera-helpers/mapped_buffer.h\n create mode 100644 include/libcamera-helpers/mapped_framebuffer.h\n rename src/libcamera-helpers/{mapped_buffer.cpp => mapped_framebuffer.cpp} (56%)","diff":"diff --git a/include/libcamera-helpers/mapped_buffer.h b/include/libcamera-helpers/mapped_buffer.h\ndeleted file mode 100644\nindex 6cfc57217d75..000000000000\n--- a/include/libcamera-helpers/mapped_buffer.h\n+++ /dev/null\n@@ -1,53 +0,0 @@\n-/* SPDX-License-Identifier: LGPL-2.1-or-later */\n-/*\n- * Copyright (C) 2020, Google Inc.\n- *\n- * buffer.h - Internal buffer handling\n- */\n-#ifndef __LIBCAMERA_MAPPED_BUFFER_H__\n-#define __LIBCAMERA_MAPPED_BUFFER_H__\n-\n-#include \n-#include \n-\n-#include \n-#include \n-\n-#include \n-\n-namespace libcamera {\n-\n-class MappedBuffer\n-{\n-public:\n- using Plane = Span;\n-\n- ~MappedBuffer();\n-\n- MappedBuffer(MappedBuffer &&other);\n- MappedBuffer &operator=(MappedBuffer &&other);\n-\n- bool isValid() const { return error_ == 0; }\n- int error() const { return error_; }\n- const std::vector &maps() const { return maps_; }\n-\n-protected:\n- MappedBuffer();\n-\n- int error_;\n- std::vector maps_;\n-\n-private:\n- LIBCAMERA_DISABLE_COPY(MappedBuffer)\n-};\n-\n-class MappedFrameBuffer : public MappedBuffer\n-{\n-public:\n- MappedFrameBuffer(const FrameBuffer *buffer, int flags);\n-};\n-\n-} /* namespace libcamera */\n-\n-#endif /* __LIBCAMERA_MAPPED_BUFFER_H__ */\n-\ndiff --git a/include/libcamera-helpers/mapped_framebuffer.h b/include/libcamera-helpers/mapped_framebuffer.h\nnew file mode 100644\nindex 000000000000..42155279d44d\n--- /dev/null\n+++ b/include/libcamera-helpers/mapped_framebuffer.h\n@@ -0,0 +1,65 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2021, Google Inc.\n+ *\n+ * mapped_framebuffer.h - Frame buffer memory mapping support\n+ */\n+#ifndef __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__\n+#define __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__\n+\n+#include \n+#include \n+\n+#include \n+#include \n+#include \n+\n+#include \n+\n+namespace libcamera {\n+\n+class MappedBuffer\n+{\n+public:\n+\tusing Plane = Span;\n+\n+\t~MappedBuffer();\n+\n+\tMappedBuffer(MappedBuffer &&other);\n+\tMappedBuffer &operator=(MappedBuffer &&other);\n+\n+\tbool isValid() const { return error_ == 0; }\n+\tint error() const { return error_; }\n+\t/* \\todo rename to planes(). */\n+\tconst std::vector &maps() const { return planes_; }\n+\n+protected:\n+\tMappedBuffer();\n+\n+\tint error_;\n+\tstd::vector planes_;\n+\tstd::vector maps_;\n+\n+private:\n+\tLIBCAMERA_DISABLE_COPY(MappedBuffer)\n+};\n+\n+class MappedFrameBuffer : public MappedBuffer\n+{\n+public:\n+\tenum class MapFlag {\n+\t\tRead = 1 << 0,\n+\t\tWrite = 1 << 1,\n+\t\tReadWrite = Read | Write,\n+\t};\n+\n+\tusing MapFlags = Flags;\n+\n+\tMappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags);\n+};\n+\n+LIBCAMERA_FLAGS_ENABLE_OPERATORS(MappedFrameBuffer::MapFlag)\n+\n+} /* namespace libcamera */\n+\n+#endif /* __LIBCAMERA_HELPER_MAPPED_FRAMEBUFFER_H__ */\ndiff --git a/ipu3.cpp b/ipu3.cpp\nindex 3e89e6dd4e02..c1a254517845 100644\n--- a/ipu3.cpp\n+++ b/ipu3.cpp\n@@ -20,7 +20,7 @@\n \n #include \n \n-#include \"libcamera-helpers/mapped_buffer.h\"\n+#include \"libcamera-helpers/mapped_framebuffer.h\"\n \n /* IA AIQ Wrapper API */\n #include \"aic/aic.h\"\n@@ -258,10 +258,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n \n void IPAIPU3::mapBuffers(const std::vector &buffers)\n {\n+\t/*\n+\t * todo: Statistics buffers could be mapped read-only if they\n+\t * could be easily identified.\n+\t */\n \tfor (const IPABuffer &buffer : buffers) {\n \t\tconst FrameBuffer fb(buffer.planes);\n \t\tbuffers_.emplace(buffer.id,\n-\t\t\t\t MappedFrameBuffer(&fb, PROT_READ | PROT_WRITE));\n+\t\t\t\t MappedFrameBuffer(&fb, MappedFrameBuffer::MapFlag::ReadWrite));\n \t}\n }\n \ndiff --git a/src/libcamera-helpers/mapped_buffer.cpp b/src/libcamera-helpers/mapped_framebuffer.cpp\nsimilarity index 56%\nrename from src/libcamera-helpers/mapped_buffer.cpp\nrename to src/libcamera-helpers/mapped_framebuffer.cpp\nindex 6f3248e1b31b..a65740831331 100644\n--- a/src/libcamera-helpers/mapped_buffer.cpp\n+++ b/src/libcamera-helpers/mapped_framebuffer.cpp\n@@ -2,26 +2,27 @@\n /*\n * Copyright (C) 2021, Google Inc.\n *\n- * mapped_buffer.cpp - Mapped Buffer handling\n+ * mapped_framebuffer.cpp - Mapped Framebuffer support\n */\n \n-#include \"libcamera-helpers/mapped_buffer.h\"\n+#include \"libcamera-helpers/mapped_framebuffer.h\"\n \n+#include \n #include \n-#include \n+#include \n #include \n #include \n \n #include \n \n /**\n- * \\file libcamera-helpers/mapped_buffer.h\n- * \\brief Mapped Buffer handling\n+ * \\file libcamera-helpers/mapped_framebuffer.h\n+ * \\brief Frame buffer memory mapping support\n */\n \n namespace libcamera {\n \n-LOG_DEFINE_CATEGORY(MappedBuffer)\n+LOG_DECLARE_CATEGORY(Buffer)\n \n /**\n * \\class MappedBuffer\n@@ -81,6 +82,7 @@ MappedBuffer::MappedBuffer(MappedBuffer &&other)\n MappedBuffer &MappedBuffer::operator=(MappedBuffer &&other)\n {\n \terror_ = other.error_;\n+\tplanes_ = std::move(other.planes_);\n \tmaps_ = std::move(other.maps_);\n \tother.error_ = -ENOENT;\n \n@@ -129,10 +131,18 @@ MappedBuffer::~MappedBuffer()\n */\n \n /**\n- * \\var MappedBuffer::maps_\n+ * \\var MappedBuffer::planes_\n * \\brief Stores the internal mapped planes\n *\n * MappedBuffer derived classes shall store the mappings they create in this\n+ * vector which points the beginning of mapped plane addresses.\n+ */\n+\n+/**\n+ * \\var MappedBuffer::maps_\n+ * \\brief Stores the mapped buffer\n+ *\n+ * MappedBuffer derived classes shall store the mappings they create in this\n * vector which is parsed during destruct to unmap any memory mappings which\n * completed successfully.\n */\n@@ -142,29 +152,91 @@ MappedBuffer::~MappedBuffer()\n * \\brief Map a FrameBuffer using the MappedBuffer interface\n */\n \n+/**\n+ * \\enum MappedFrameBuffer::MapFlag\n+ * \\brief Specify the mapping mode for the FrameBuffer\n+ * \\var MappedFrameBuffer::Read\n+ * \\brief Create a read-only mapping\n+ * \\var MappedFrameBuffer::Write\n+ * \\brief Create a write-only mapping\n+ * \\var MappedFrameBuffer::ReadWrite\n+ * \\brief Create a mapping that can be both read and written\n+ */\n+\n+/**\n+ * \\typedef MappedFrameBuffer::MapFlags\n+ * \\brief A bitwise combination of MappedFrameBuffer::MapFlag values\n+ */\n+\n /**\n * \\brief Map all planes of a FrameBuffer\n * \\param[in] buffer FrameBuffer to be mapped\n * \\param[in] flags Protection flags to apply to map\n *\n- * Construct an object to map a frame buffer for CPU access.\n- * The flags are passed directly to mmap and should be either PROT_READ,\n- * PROT_WRITE, or a bitwise-or combination of both.\n+ * Construct an object to map a frame buffer for CPU access. The mapping can be\n+ * made as Read only, Write only or support Read and Write operations by setting\n+ * the MapFlag flags accordingly.\n */\n-MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, int flags)\n+MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags)\n {\n-\tmaps_.reserve(buffer->planes().size());\n+\tASSERT(!buffer->planes().empty());\n+\tplanes_.reserve(buffer->planes().size());\n+\n+\tint mmapFlags = 0;\n+\n+\tif (flags & MapFlag::Read)\n+\t\tmmapFlags |= PROT_READ;\n+\n+\tif (flags & MapFlag::Write)\n+\t\tmmapFlags |= PROT_WRITE;\n+\n+\tstruct MappedBufferInfo {\n+\t\tuint8_t *address = nullptr;\n+\t\tsize_t mapLength = 0;\n+\t\tsize_t dmabufLength = 0;\n+\t};\n+\tstd::map mappedBuffers;\n+\n+\tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n+\t\tconst int fd = plane.fd.fd();\n+\t\tif (mappedBuffers.find(fd) == mappedBuffers.end()) {\n+\t\t\tconst size_t length = lseek(fd, 0, SEEK_END);\n+\t\t\tmappedBuffers[fd] = MappedBufferInfo{ nullptr, 0, length };\n+\t\t}\n+\n+\t\tconst size_t length = mappedBuffers[fd].dmabufLength;\n+\n+\t\tif (plane.offset > length ||\n+\t\t plane.offset + plane.length > length) {\n+\t\t\tLOG(Buffer, Fatal) << \"plane is out of buffer: \"\n+\t\t\t\t\t << \"buffer length=\" << length\n+\t\t\t\t\t << \", plane offset=\" << plane.offset\n+\t\t\t\t\t << \", plane length=\" << plane.length;\n+\t\t\treturn;\n+\t\t}\n+\t\tsize_t &mapLength = mappedBuffers[fd].mapLength;\n+\t\tmapLength = std::max(mapLength,\n+\t\t\t\t static_cast(plane.offset + plane.length));\n+\t}\n \n \tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n-\t\tvoid *address = mmap(nullptr, plane.length, flags,\n-\t\t\t\t MAP_SHARED, plane.fd.fd(), 0);\n-\t\tif (address == MAP_FAILED) {\n-\t\t\terror_ = -errno;\n-\t\t\tLOG(MappedBuffer, Error) << \"Failed to mmap plane\";\n-\t\t\tbreak;\n+\t\tconst int fd = plane.fd.fd();\n+\t\tauto &info = mappedBuffers[fd];\n+\t\tif (!info.address) {\n+\t\t\tvoid *address = mmap(nullptr, info.mapLength, mmapFlags,\n+\t\t\t\t\t MAP_SHARED, fd, 0);\n+\t\t\tif (address == MAP_FAILED) {\n+\t\t\t\terror_ = -errno;\n+\t\t\t\tLOG(Buffer, Error) << \"Failed to mmap plane: \"\n+\t\t\t\t\t\t << strerror(-error_);\n+\t\t\t\treturn;\n+\t\t\t}\n+\n+\t\t\tinfo.address = static_cast(address);\n+\t\t\tmaps_.emplace_back(info.address, info.mapLength);\n \t\t}\n \n-\t\tmaps_.emplace_back(static_cast(address), plane.length);\n+\t\tplanes_.emplace_back(info.address + plane.offset, plane.length);\n \t}\n }\n \ndiff --git a/src/libcamera-helpers/meson.build b/src/libcamera-helpers/meson.build\nindex 444f21204d5d..084bf65345ae 100644\n--- a/src/libcamera-helpers/meson.build\n+++ b/src/libcamera-helpers/meson.build\n@@ -2,5 +2,5 @@\n \n # Implementation of internal libcamera internals\n libcamera_helpers = files([\n- 'mapped_buffer.cpp',\n+ 'mapped_framebuffer.cpp',\n ])\n","prefixes":["libcamera-devel","IPU3-IPA"]}