[{"id":13613,"web_url":"https://patchwork.libcamera.org/comment/13613/","msgid":"<7112fb48-214f-85be-38ce-b9de1b3362c5@ideasonboard.com>","date":"2020-11-05T11:50:50","subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: ipa: ipu3: Add an\n\tIPA skeleton for the IPU3 pipeline","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 05/11/2020 00:15, Niklas Söderlund wrote:\n> Add an empty IPA skeleton for the IPU3 pipeline. The skeleton IPA do\n> handle the flow of parameter and statistic buffers but does not read or\n\ns/do handle/handles/\n\n\n> write anything in the buffers. It also allows the IPA to set sensor\n> controls but does not implement any logic to set optimal values and\n> instead set the V4L2 exposure and gain controls to max and keeps them at\n> that setting.\n> \n> This IPA is meant as a base to allow the pipeline handler to be wired up\n> to an IPA. The image algorithms can then later be added to the IPA\n> independent from also having to also add plumbing to the pipeline\n> handler.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nAs long as this doesn't break existing running of the pipeline, I'd\npretty much go for an anything goes here, to build upon ;-)\n\n> ---\n>  include/libcamera/ipa/ipu3.h |  22 ++++\n>  src/ipa/ipu3/ipu3.cpp        | 237 +++++++++++++++++++++++++++++++++++\n>  src/ipa/ipu3/meson.build     |  21 ++++\n>  src/ipa/meson.build          |   2 +-\n>  4 files changed, 281 insertions(+), 1 deletion(-)\n>  create mode 100644 include/libcamera/ipa/ipu3.h\n>  create mode 100644 src/ipa/ipu3/ipu3.cpp\n>  create mode 100644 src/ipa/ipu3/meson.build\n> \n> diff --git a/include/libcamera/ipa/ipu3.h b/include/libcamera/ipa/ipu3.h\n> new file mode 100644\n> index 0000000000000000..7c240a46b97dee28\n> --- /dev/null\n> +++ b/include/libcamera/ipa/ipu3.h\n> @@ -0,0 +1,22 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * ipu3.h - Image Processing Algorithm interface for IPU3\n> + */\n> +#ifndef __LIBCAMERA_IPA_INTERFACE_IPU3_H__\n> +#define __LIBCAMERA_IPA_INTERFACE_IPU3_H__\n> +\n> +#ifndef __DOXYGEN__\n> +\n> +enum IPU3Operations {\n> +\tIPU3_IPA_ACTION_SET_SENSOR_CONTROLS = 1,\n> +\tIPU3_IPA_ACTION_PARAM_FILLED = 2,\n> +\tIPU3_IPA_ACTION_METADATA_READY = 3,\n> +\tIPU3_IPA_EVENT_PARSE_STAT = 4,\n> +\tIPU3_IPA_EVENT_FILL_PARAMS = 5,\n> +};\n> +\n> +#endif /* __DOXYGEN__ */\n> +\n> +#endif /* __LIBCAMERA_IPA_INTERFACE_IPU3_H__ */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> new file mode 100644\n> index 0000000000000000..cdcd9b9ff013e0cc\n> --- /dev/null\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -0,0 +1,237 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * ipu3.cpp - IPU3 Image Processing Algorithms\n> + */\n> +\n> +#include <algorithm>\n> +#include <math.h>\n> +#include <queue>\n> +#include <stdint.h>\n> +#include <string.h>\n> +#include <sys/mman.h>\n> +\n> +#include <linux/intel-ipu3.h>\n> +#include <linux/v4l2-controls.h>\n> +\n> +#include <libcamera/buffer.h>\n> +#include <libcamera/control_ids.h>\n> +#include <libcamera/ipa/ipa_interface.h>\n> +#include <libcamera/ipa/ipa_module_info.h>\n> +#include <libcamera/ipa/ipu3.h>\n> +#include <libcamera/request.h>\n> +\n> +#include <libipa/ipa_interface_wrapper.h>\n> +\n> +#include \"libcamera/internal/log.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPAIPU3)\n> +\n> +class IPAIPU3 : public IPAInterface\n> +{\n> +public:\n> +\tint init([[maybe_unused]] const IPASettings &settings) override\n> +\t{\n> +\t\treturn 0;\n> +\t}\n> +\tint start() override { return 0; }\n> +\tvoid stop() override {}\n> +\n> +\tvoid configure(const CameraSensorInfo &info,\n> +\t\t       const std::map<unsigned int, IPAStream> &streamConfig,\n> +\t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> +\t\t       const IPAOperationData &ipaConfig,\n> +\t\t       IPAOperationData *response) override;\n> +\tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n> +\tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> +\tvoid processEvent(const IPAOperationData &event) override;\n> +\n> +private:\n> +\tvoid fillParams(unsigned int frame, ipu3_uapi_params *params,\n> +\t\t\tconst ControlList &controls);\n> +\n> +\tvoid parseStatistics(unsigned int frame,\n> +\t\t\t     const ipu3_uapi_stats_3a *stats);\n> +\n> +\tvoid setControls(unsigned int frame);\n> +\n> +\tstd::map<unsigned int, FrameBuffer> buffers_;\n> +\tstd::map<unsigned int, void *> buffersMemory_;\n> +\n> +\tControlInfoMap ctrls_;\n> +\n> +\t/* Camera sensor controls. */\n> +\tuint32_t exposure_;\n> +\tuint32_t minExposure_;\n> +\tuint32_t maxExposure_;\n> +\tuint32_t gain_;\n> +\tuint32_t minGain_;\n> +\tuint32_t maxGain_;\n> +};\n> +\n> +void IPAIPU3::configure([[maybe_unused]] const CameraSensorInfo &info,\n> +\t\t\t[[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,\n> +\t\t\tconst std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> +\t\t\t[[maybe_unused]] const IPAOperationData &ipaConfig,\n> +\t\t\t[[maybe_unused]] IPAOperationData *result)\n> +{\n> +\tif (entityControls.empty())\n> +\t\treturn;\n> +\n> +\tctrls_ = entityControls.at(0);\n> +\n> +\tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n> +\tif (itExp == ctrls_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Can't find exposure control\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n> +\tif (itGain == ctrls_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Can't find gain control\";\n> +\t\treturn;\n> +\t}\n\nI'm a little confused here, expecting the IPA to be dealing with\nlibcamera controls and the translation to happen in the pipeline.\n\nOtherwise, are these v4l2 controls on the sensor? or the ISP?\n\nBut maybe that doesn't matter.\n\nAnyway, anything goes still applies. This is initial skeleton, so I'll\nnot argue strongly against this if it's just prototype place holder code\nto change later.\n\n\n\n> +\n> +\tminExposure_ = std::max<uint32_t>(itExp->second.min().get<int32_t>(), 1);\n> +\tmaxExposure_ = itExp->second.max().get<int32_t>();\n> +\texposure_ = maxExposure_;\n> +\n> +\tminGain_ = std::max<uint32_t>(itGain->second.min().get<int32_t>(), 1);\n> +\tmaxGain_ = itGain->second.max().get<int32_t>();\n> +\tgain_ = maxGain_;\n> +\n> +\tsetControls(0);\n> +}\n> +\n> +void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)\n> +{\n> +\tfor (const IPABuffer &buffer : buffers) {\n> +\t\tauto elem = buffers_.emplace(std::piecewise_construct,\n> +\t\t\t\t\t     std::forward_as_tuple(buffer.id),\n> +\t\t\t\t\t     std::forward_as_tuple(buffer.planes));\n> +\t\tconst FrameBuffer &fb = elem.first->second;\n> +\n> +\t\tbuffersMemory_[buffer.id] = mmap(NULL,\n> +\t\t\t\t\t\t fb.planes()[0].length,\n> +\t\t\t\t\t\t PROT_READ | PROT_WRITE,\n> +\t\t\t\t\t\t MAP_SHARED,\n> +\t\t\t\t\t\t fb.planes()[0].fd.fd(),\n> +\t\t\t\t\t\t 0);\n> +\n> +\t\tif (buffersMemory_[buffer.id] == MAP_FAILED) {\n> +\t\t\tint ret = -errno;\n> +\t\t\tLOG(IPAIPU3, Fatal) << \"Failed to mmap buffer: \"\n> +\t\t\t\t\t    << strerror(-ret);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)\n> +{\n> +\tfor (unsigned int id : ids) {\n> +\t\tconst auto fb = buffers_.find(id);\n> +\t\tif (fb == buffers_.end())\n> +\t\t\tcontinue;\n> +\n> +\t\tmunmap(buffersMemory_[id], fb->second.planes()[0].length);\n> +\t\tbuffersMemory_.erase(id);\n> +\t\tbuffers_.erase(id);\n> +\t}\n> +}\n> +\n> +void IPAIPU3::processEvent(const IPAOperationData &event)\n> +{\n> +\tswitch (event.operation) {\n> +\tcase IPU3_IPA_EVENT_PARSE_STAT: {\n> +\t\tunsigned int frame = event.data[0];\n> +\t\tunsigned int bufferId = event.data[1];\n> +\n> +\t\tconst ipu3_uapi_stats_3a *stats =\n> +\t\t\tstatic_cast<ipu3_uapi_stats_3a *>(buffersMemory_[bufferId]);\n> +\n> +\t\tparseStatistics(frame, stats);\n> +\t\tbreak;\n> +\t}\n> +\tcase IPU3_IPA_EVENT_FILL_PARAMS: {\n> +\t\tunsigned int frame = event.data[0];\n> +\t\tunsigned int bufferId = event.data[1];\n> +\n> +\t\tipu3_uapi_params *params =\n> +\t\t\tstatic_cast<ipu3_uapi_params *>(buffersMemory_[bufferId]);\n> +\n> +\t\tfillParams(frame, params, event.controls[0]);\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tLOG(IPAIPU3, Error) << \"Unknown event \" << event.operation;\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> +void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params,\n> +\t\t\t [[maybe_unused]] const ControlList &controls)\n> +{\n> +\t/* Prepare parameters buffer. */\n> +\tmemset(params, 0, sizeof(*params));\n> +\n> +\t/* \\todo Fill in parameters buffer. */\n> +\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_PARAM_FILLED;\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +\n> +\t/* \\todo Calculate new values for exposure_ and gain_. */\n> +\tsetControls(frame);\n> +}\n> +\n> +void IPAIPU3::parseStatistics(unsigned int frame,\n> +\t\t\t      [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n> +{\n> +\tControlList ctrls(controls::controls);\n> +\n> +\t/* \\todo React to statistics and update internal state machine. */\n> +\t/* \\todo Add meta-data information to ctrls. */\n> +\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_METADATA_READY;\n> +\top.controls.push_back(ctrls);\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +}\n> +\n> +void IPAIPU3::setControls(unsigned int frame)\n> +{\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_SET_SENSOR_CONTROLS;\n> +\n> +\tControlList ctrls(ctrls_);\n> +\tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n> +\tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n> +\top.controls.push_back(ctrls);\n\nDoes this happen for every frame?\n\nAh - I see this is called from configure, with setControls(0).\n\nAnyway, I see this all as a starting point, not a finishing point.\n\nAcked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +}\n> +\n> +/*\n> + * External IPA module interface\n> + */\n> +\n> +extern \"C\" {\n> +const struct IPAModuleInfo ipaModuleInfo = {\n> +\tIPA_MODULE_API_VERSION,\n> +\t1,\n> +\t\"PipelineHandlerIPU3\",\n> +\t\"ipu3\",\n> +};\n> +\n> +struct ipa_context *ipaCreate()\n> +{\n> +\treturn new IPAInterfaceWrapper(std::make_unique<IPAIPU3>());\n> +}\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\n> new file mode 100644\n> index 0000000000000000..a7e18f06b62bef3e\n> --- /dev/null\n> +++ b/src/ipa/ipu3/meson.build\n> @@ -0,0 +1,21 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +ipa_name = 'ipa_ipu3'\n> +\n> +mod = shared_module(ipa_name,\n> +                    'ipu3.cpp',\n> +                    name_prefix : '',\n> +                    include_directories : [ipa_includes, libipa_includes],\n> +                    dependencies : libcamera_dep,\n> +                    link_with : libipa,\n> +                    install : true,\n> +                    install_dir : ipa_install_dir)\n> +\n> +if ipa_sign_module\n> +    custom_target(ipa_name + '.so.sign',\n> +                  input : mod,\n> +                  output : ipa_name + '.so.sign',\n> +                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> +                  install : false,\n> +                  build_by_default : true)\n> +endif\n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index 5a5de267c1477d24..9d623f227a1f9feb 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -19,7 +19,7 @@ subdir('libipa')\n>  \n>  ipa_sign = files('ipa-sign.sh')\n>  \n> -ipas = ['raspberrypi', 'rkisp1', 'vimc']\n> +ipas = ['ipu3', 'raspberrypi', 'rkisp1', 'vimc']\n>  ipa_names = []\n>  \n>  foreach pipeline : get_option('pipelines')\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 D3419BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  5 Nov 2020 11:50:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 62D4162D19;\n\tThu,  5 Nov 2020 12:50:54 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3B78062C7D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  5 Nov 2020 12:50:53 +0100 (CET)","from [192.168.0.20]\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 A5B7B51D;\n\tThu,  5 Nov 2020 12:50:52 +0100 (CET)"],"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=\"MDnpPlSQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604577052;\n\tbh=S3MBzGkqRhy0vSQL8ceWToveLyM3H+dD9g0Zg96j4ko=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=MDnpPlSQI1Hhv7BQkDvRvyOuYB4gabff7CNVas0B6DtTkdz0tD7yrzak6GERbY9wp\n\tDcxN9mJh26EAUNppTm+f5OUgAb2Fb0o9Fv2rvS7xcZX/tdAYO1G0LXrekvGPmx0n8T\n\ttd+kKtLReqBvAR35f9me2w4cvEThe20lbmM8PAzc=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-8-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7112fb48-214f-85be-38ce-b9de1b3362c5@ideasonboard.com>","Date":"Thu, 5 Nov 2020 11:50:50 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201105001546.1690179-8-niklas.soderlund@ragnatech.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: ipa: ipu3: Add an\n\tIPA skeleton for the IPU3 pipeline","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>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13643,"web_url":"https://patchwork.libcamera.org/comment/13643/","msgid":"<20201109115324.c6huzk6qyycg6qvu@uno.localdomain>","date":"2020-11-09T11:53:24","subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: ipa: ipu3: Add an\n\tIPA skeleton for the IPU3 pipeline","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Thu, Nov 05, 2020 at 01:15:42AM +0100, Niklas Söderlund wrote:\n> Add an empty IPA skeleton for the IPU3 pipeline. The skeleton IPA do\n> handle the flow of parameter and statistic buffers but does not read or\n> write anything in the buffers. It also allows the IPA to set sensor\n> controls but does not implement any logic to set optimal values and\n> instead set the V4L2 exposure and gain controls to max and keeps them at\n> that setting.\n>\n> This IPA is meant as a base to allow the pipeline handler to be wired up\n> to an IPA. The image algorithms can then later be added to the IPA\n> independent from also having to also add plumbing to the pipeline\n> handler.\n\nI would have expected this to be based on Paul's IPC work. Is it\nintentioanlly defined as a legacy IPA with the idea of moving it to\nthe new framework later ?\n\nPaul, as you've done the conversion of all the legacy IPAs, what would\nit be the best course of actions in your opinion ?\n\nThanks\n   j\n\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  include/libcamera/ipa/ipu3.h |  22 ++++\n>  src/ipa/ipu3/ipu3.cpp        | 237 +++++++++++++++++++++++++++++++++++\n>  src/ipa/ipu3/meson.build     |  21 ++++\n>  src/ipa/meson.build          |   2 +-\n>  4 files changed, 281 insertions(+), 1 deletion(-)\n>  create mode 100644 include/libcamera/ipa/ipu3.h\n>  create mode 100644 src/ipa/ipu3/ipu3.cpp\n>  create mode 100644 src/ipa/ipu3/meson.build\n>\n> diff --git a/include/libcamera/ipa/ipu3.h b/include/libcamera/ipa/ipu3.h\n> new file mode 100644\n> index 0000000000000000..7c240a46b97dee28\n> --- /dev/null\n> +++ b/include/libcamera/ipa/ipu3.h\n> @@ -0,0 +1,22 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * ipu3.h - Image Processing Algorithm interface for IPU3\n> + */\n> +#ifndef __LIBCAMERA_IPA_INTERFACE_IPU3_H__\n> +#define __LIBCAMERA_IPA_INTERFACE_IPU3_H__\n> +\n> +#ifndef __DOXYGEN__\n> +\n> +enum IPU3Operations {\n> +\tIPU3_IPA_ACTION_SET_SENSOR_CONTROLS = 1,\n> +\tIPU3_IPA_ACTION_PARAM_FILLED = 2,\n> +\tIPU3_IPA_ACTION_METADATA_READY = 3,\n> +\tIPU3_IPA_EVENT_PARSE_STAT = 4,\n> +\tIPU3_IPA_EVENT_FILL_PARAMS = 5,\n> +};\n> +\n> +#endif /* __DOXYGEN__ */\n> +\n> +#endif /* __LIBCAMERA_IPA_INTERFACE_IPU3_H__ */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> new file mode 100644\n> index 0000000000000000..cdcd9b9ff013e0cc\n> --- /dev/null\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -0,0 +1,237 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * ipu3.cpp - IPU3 Image Processing Algorithms\n> + */\n> +\n> +#include <algorithm>\n> +#include <math.h>\n> +#include <queue>\n> +#include <stdint.h>\n> +#include <string.h>\n> +#include <sys/mman.h>\n> +\n> +#include <linux/intel-ipu3.h>\n> +#include <linux/v4l2-controls.h>\n> +\n> +#include <libcamera/buffer.h>\n> +#include <libcamera/control_ids.h>\n> +#include <libcamera/ipa/ipa_interface.h>\n> +#include <libcamera/ipa/ipa_module_info.h>\n> +#include <libcamera/ipa/ipu3.h>\n> +#include <libcamera/request.h>\n> +\n> +#include <libipa/ipa_interface_wrapper.h>\n> +\n> +#include \"libcamera/internal/log.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPAIPU3)\n> +\n> +class IPAIPU3 : public IPAInterface\n> +{\n> +public:\n> +\tint init([[maybe_unused]] const IPASettings &settings) override\n> +\t{\n> +\t\treturn 0;\n> +\t}\n> +\tint start() override { return 0; }\n> +\tvoid stop() override {}\n> +\n> +\tvoid configure(const CameraSensorInfo &info,\n> +\t\t       const std::map<unsigned int, IPAStream> &streamConfig,\n> +\t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> +\t\t       const IPAOperationData &ipaConfig,\n> +\t\t       IPAOperationData *response) override;\n> +\tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n> +\tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> +\tvoid processEvent(const IPAOperationData &event) override;\n> +\n> +private:\n> +\tvoid fillParams(unsigned int frame, ipu3_uapi_params *params,\n> +\t\t\tconst ControlList &controls);\n> +\n> +\tvoid parseStatistics(unsigned int frame,\n> +\t\t\t     const ipu3_uapi_stats_3a *stats);\n> +\n> +\tvoid setControls(unsigned int frame);\n> +\n> +\tstd::map<unsigned int, FrameBuffer> buffers_;\n> +\tstd::map<unsigned int, void *> buffersMemory_;\n> +\n> +\tControlInfoMap ctrls_;\n> +\n> +\t/* Camera sensor controls. */\n> +\tuint32_t exposure_;\n> +\tuint32_t minExposure_;\n> +\tuint32_t maxExposure_;\n> +\tuint32_t gain_;\n> +\tuint32_t minGain_;\n> +\tuint32_t maxGain_;\n> +};\n> +\n> +void IPAIPU3::configure([[maybe_unused]] const CameraSensorInfo &info,\n> +\t\t\t[[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,\n> +\t\t\tconst std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> +\t\t\t[[maybe_unused]] const IPAOperationData &ipaConfig,\n> +\t\t\t[[maybe_unused]] IPAOperationData *result)\n> +{\n> +\tif (entityControls.empty())\n> +\t\treturn;\n> +\n> +\tctrls_ = entityControls.at(0);\n> +\n> +\tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n> +\tif (itExp == ctrls_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Can't find exposure control\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n> +\tif (itGain == ctrls_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Can't find gain control\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tminExposure_ = std::max<uint32_t>(itExp->second.min().get<int32_t>(), 1);\n> +\tmaxExposure_ = itExp->second.max().get<int32_t>();\n> +\texposure_ = maxExposure_;\n> +\n> +\tminGain_ = std::max<uint32_t>(itGain->second.min().get<int32_t>(), 1);\n> +\tmaxGain_ = itGain->second.max().get<int32_t>();\n> +\tgain_ = maxGain_;\n> +\n> +\tsetControls(0);\n> +}\n> +\n> +void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)\n> +{\n> +\tfor (const IPABuffer &buffer : buffers) {\n> +\t\tauto elem = buffers_.emplace(std::piecewise_construct,\n> +\t\t\t\t\t     std::forward_as_tuple(buffer.id),\n> +\t\t\t\t\t     std::forward_as_tuple(buffer.planes));\n> +\t\tconst FrameBuffer &fb = elem.first->second;\n> +\n> +\t\tbuffersMemory_[buffer.id] = mmap(NULL,\n> +\t\t\t\t\t\t fb.planes()[0].length,\n> +\t\t\t\t\t\t PROT_READ | PROT_WRITE,\n> +\t\t\t\t\t\t MAP_SHARED,\n> +\t\t\t\t\t\t fb.planes()[0].fd.fd(),\n> +\t\t\t\t\t\t 0);\n> +\n> +\t\tif (buffersMemory_[buffer.id] == MAP_FAILED) {\n> +\t\t\tint ret = -errno;\n> +\t\t\tLOG(IPAIPU3, Fatal) << \"Failed to mmap buffer: \"\n> +\t\t\t\t\t    << strerror(-ret);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)\n> +{\n> +\tfor (unsigned int id : ids) {\n> +\t\tconst auto fb = buffers_.find(id);\n> +\t\tif (fb == buffers_.end())\n> +\t\t\tcontinue;\n> +\n> +\t\tmunmap(buffersMemory_[id], fb->second.planes()[0].length);\n> +\t\tbuffersMemory_.erase(id);\n> +\t\tbuffers_.erase(id);\n> +\t}\n> +}\n> +\n> +void IPAIPU3::processEvent(const IPAOperationData &event)\n> +{\n> +\tswitch (event.operation) {\n> +\tcase IPU3_IPA_EVENT_PARSE_STAT: {\n> +\t\tunsigned int frame = event.data[0];\n> +\t\tunsigned int bufferId = event.data[1];\n> +\n> +\t\tconst ipu3_uapi_stats_3a *stats =\n> +\t\t\tstatic_cast<ipu3_uapi_stats_3a *>(buffersMemory_[bufferId]);\n> +\n> +\t\tparseStatistics(frame, stats);\n> +\t\tbreak;\n> +\t}\n> +\tcase IPU3_IPA_EVENT_FILL_PARAMS: {\n> +\t\tunsigned int frame = event.data[0];\n> +\t\tunsigned int bufferId = event.data[1];\n> +\n> +\t\tipu3_uapi_params *params =\n> +\t\t\tstatic_cast<ipu3_uapi_params *>(buffersMemory_[bufferId]);\n> +\n> +\t\tfillParams(frame, params, event.controls[0]);\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tLOG(IPAIPU3, Error) << \"Unknown event \" << event.operation;\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> +void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params,\n> +\t\t\t [[maybe_unused]] const ControlList &controls)\n> +{\n> +\t/* Prepare parameters buffer. */\n> +\tmemset(params, 0, sizeof(*params));\n> +\n> +\t/* \\todo Fill in parameters buffer. */\n> +\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_PARAM_FILLED;\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +\n> +\t/* \\todo Calculate new values for exposure_ and gain_. */\n> +\tsetControls(frame);\n> +}\n> +\n> +void IPAIPU3::parseStatistics(unsigned int frame,\n> +\t\t\t      [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n> +{\n> +\tControlList ctrls(controls::controls);\n> +\n> +\t/* \\todo React to statistics and update internal state machine. */\n> +\t/* \\todo Add meta-data information to ctrls. */\n> +\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_METADATA_READY;\n> +\top.controls.push_back(ctrls);\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +}\n> +\n> +void IPAIPU3::setControls(unsigned int frame)\n> +{\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_SET_SENSOR_CONTROLS;\n> +\n> +\tControlList ctrls(ctrls_);\n> +\tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n> +\tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n> +\top.controls.push_back(ctrls);\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +}\n> +\n> +/*\n> + * External IPA module interface\n> + */\n> +\n> +extern \"C\" {\n> +const struct IPAModuleInfo ipaModuleInfo = {\n> +\tIPA_MODULE_API_VERSION,\n> +\t1,\n> +\t\"PipelineHandlerIPU3\",\n> +\t\"ipu3\",\n> +};\n> +\n> +struct ipa_context *ipaCreate()\n> +{\n> +\treturn new IPAInterfaceWrapper(std::make_unique<IPAIPU3>());\n> +}\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\n> new file mode 100644\n> index 0000000000000000..a7e18f06b62bef3e\n> --- /dev/null\n> +++ b/src/ipa/ipu3/meson.build\n> @@ -0,0 +1,21 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +ipa_name = 'ipa_ipu3'\n> +\n> +mod = shared_module(ipa_name,\n> +                    'ipu3.cpp',\n> +                    name_prefix : '',\n> +                    include_directories : [ipa_includes, libipa_includes],\n> +                    dependencies : libcamera_dep,\n> +                    link_with : libipa,\n> +                    install : true,\n> +                    install_dir : ipa_install_dir)\n> +\n> +if ipa_sign_module\n> +    custom_target(ipa_name + '.so.sign',\n> +                  input : mod,\n> +                  output : ipa_name + '.so.sign',\n> +                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> +                  install : false,\n> +                  build_by_default : true)\n> +endif\n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index 5a5de267c1477d24..9d623f227a1f9feb 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -19,7 +19,7 @@ subdir('libipa')\n>\n>  ipa_sign = files('ipa-sign.sh')\n>\n> -ipas = ['raspberrypi', 'rkisp1', 'vimc']\n> +ipas = ['ipu3', 'raspberrypi', 'rkisp1', 'vimc']\n>  ipa_names = []\n>\n>  foreach pipeline : get_option('pipelines')\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 3396CBDB89\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  9 Nov 2020 11:53:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2E2363077;\n\tMon,  9 Nov 2020 12:53:23 +0100 (CET)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 44FDB63074\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  9 Nov 2020 12:53:22 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id AD50024000A;\n\tMon,  9 Nov 2020 11:53:21 +0000 (UTC)"],"Date":"Mon, 9 Nov 2020 12:53:24 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tpaul elder <paul.elder@ideasonboard.com>","Message-ID":"<20201109115324.c6huzk6qyycg6qvu@uno.localdomain>","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-8-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201105001546.1690179-8-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: ipa: ipu3: Add an\n\tIPA skeleton for the IPU3 pipeline","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13788,"web_url":"https://patchwork.libcamera.org/comment/13788/","msgid":"<20201118154109.rq4wdwxl65inhvhq@uno.localdomain>","date":"2020-11-18T15:41:09","subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: ipa: ipu3: Add an\n\tIPA skeleton for the IPU3 pipeline","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Thu, Nov 05, 2020 at 01:15:42AM +0100, Niklas Söderlund wrote:\n> Add an empty IPA skeleton for the IPU3 pipeline. The skeleton IPA do\n> handle the flow of parameter and statistic buffers but does not read or\n> write anything in the buffers. It also allows the IPA to set sensor\n> controls but does not implement any logic to set optimal values and\n> instead set the V4L2 exposure and gain controls to max and keeps them at\n> that setting.\n>\n> This IPA is meant as a base to allow the pipeline handler to be wired up\n> to an IPA. The image algorithms can then later be added to the IPA\n> independent from also having to also add plumbing to the pipeline\n> handler.\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  include/libcamera/ipa/ipu3.h |  22 ++++\n>  src/ipa/ipu3/ipu3.cpp        | 237 +++++++++++++++++++++++++++++++++++\n>  src/ipa/ipu3/meson.build     |  21 ++++\n>  src/ipa/meson.build          |   2 +-\n>  4 files changed, 281 insertions(+), 1 deletion(-)\n>  create mode 100644 include/libcamera/ipa/ipu3.h\n>  create mode 100644 src/ipa/ipu3/ipu3.cpp\n>  create mode 100644 src/ipa/ipu3/meson.build\n>\n> diff --git a/include/libcamera/ipa/ipu3.h b/include/libcamera/ipa/ipu3.h\n> new file mode 100644\n> index 0000000000000000..7c240a46b97dee28\n> --- /dev/null\n> +++ b/include/libcamera/ipa/ipu3.h\n> @@ -0,0 +1,22 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * ipu3.h - Image Processing Algorithm interface for IPU3\n> + */\n> +#ifndef __LIBCAMERA_IPA_INTERFACE_IPU3_H__\n> +#define __LIBCAMERA_IPA_INTERFACE_IPU3_H__\n> +\n> +#ifndef __DOXYGEN__\n> +\n> +enum IPU3Operations {\n> +\tIPU3_IPA_ACTION_SET_SENSOR_CONTROLS = 1,\n> +\tIPU3_IPA_ACTION_PARAM_FILLED = 2,\n> +\tIPU3_IPA_ACTION_METADATA_READY = 3,\n> +\tIPU3_IPA_EVENT_PARSE_STAT = 4,\n> +\tIPU3_IPA_EVENT_FILL_PARAMS = 5,\n> +};\n> +\n> +#endif /* __DOXYGEN__ */\n> +\n> +#endif /* __LIBCAMERA_IPA_INTERFACE_IPU3_H__ */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> new file mode 100644\n> index 0000000000000000..cdcd9b9ff013e0cc\n> --- /dev/null\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -0,0 +1,237 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * ipu3.cpp - IPU3 Image Processing Algorithms\n> + */\n> +\n> +#include <algorithm>\n> +#include <math.h>\n> +#include <queue>\n> +#include <stdint.h>\n> +#include <string.h>\n> +#include <sys/mman.h>\n> +\n> +#include <linux/intel-ipu3.h>\n> +#include <linux/v4l2-controls.h>\n> +\n> +#include <libcamera/buffer.h>\n> +#include <libcamera/control_ids.h>\n> +#include <libcamera/ipa/ipa_interface.h>\n> +#include <libcamera/ipa/ipa_module_info.h>\n> +#include <libcamera/ipa/ipu3.h>\n> +#include <libcamera/request.h>\n> +\n> +#include <libipa/ipa_interface_wrapper.h>\n> +\n> +#include \"libcamera/internal/log.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPAIPU3)\n> +\n> +class IPAIPU3 : public IPAInterface\n> +{\n> +public:\n> +\tint init([[maybe_unused]] const IPASettings &settings) override\n> +\t{\n> +\t\treturn 0;\n> +\t}\n> +\tint start() override { return 0; }\n> +\tvoid stop() override {}\n> +\n> +\tvoid configure(const CameraSensorInfo &info,\n> +\t\t       const std::map<unsigned int, IPAStream> &streamConfig,\n> +\t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> +\t\t       const IPAOperationData &ipaConfig,\n> +\t\t       IPAOperationData *response) override;\n> +\tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n> +\tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> +\tvoid processEvent(const IPAOperationData &event) override;\n> +\n> +private:\n> +\tvoid fillParams(unsigned int frame, ipu3_uapi_params *params,\n> +\t\t\tconst ControlList &controls);\n> +\n> +\tvoid parseStatistics(unsigned int frame,\n> +\t\t\t     const ipu3_uapi_stats_3a *stats);\n> +\n> +\tvoid setControls(unsigned int frame);\n> +\n> +\tstd::map<unsigned int, FrameBuffer> buffers_;\n> +\tstd::map<unsigned int, void *> buffersMemory_;\n> +\n> +\tControlInfoMap ctrls_;\n> +\n> +\t/* Camera sensor controls. */\n> +\tuint32_t exposure_;\n> +\tuint32_t minExposure_;\n> +\tuint32_t maxExposure_;\n> +\tuint32_t gain_;\n> +\tuint32_t minGain_;\n> +\tuint32_t maxGain_;\n> +};\n> +\n> +void IPAIPU3::configure([[maybe_unused]] const CameraSensorInfo &info,\n> +\t\t\t[[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,\n> +\t\t\tconst std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> +\t\t\t[[maybe_unused]] const IPAOperationData &ipaConfig,\n> +\t\t\t[[maybe_unused]] IPAOperationData *result)\n> +{\n> +\tif (entityControls.empty())\n> +\t\treturn;\n> +\n> +\tctrls_ = entityControls.at(0);\n> +\n> +\tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n\nShouldn't we use libcamera controls ? Otherwise the pipeline handler\nhas to implement the translation.\n\nThe rest is good, with the above question clarified I'll send my tag.\n\n> +\tif (itExp == ctrls_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Can't find exposure control\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n> +\tif (itGain == ctrls_.end()) {\n> +\t\tLOG(IPAIPU3, Error) << \"Can't find gain control\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tminExposure_ = std::max<uint32_t>(itExp->second.min().get<int32_t>(), 1);\n> +\tmaxExposure_ = itExp->second.max().get<int32_t>();\n> +\texposure_ = maxExposure_;\n> +\n> +\tminGain_ = std::max<uint32_t>(itGain->second.min().get<int32_t>(), 1);\n> +\tmaxGain_ = itGain->second.max().get<int32_t>();\n> +\tgain_ = maxGain_;\n> +\n> +\tsetControls(0);\n> +}\n> +\n> +void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)\n> +{\n> +\tfor (const IPABuffer &buffer : buffers) {\n> +\t\tauto elem = buffers_.emplace(std::piecewise_construct,\n> +\t\t\t\t\t     std::forward_as_tuple(buffer.id),\n> +\t\t\t\t\t     std::forward_as_tuple(buffer.planes));\n> +\t\tconst FrameBuffer &fb = elem.first->second;\n> +\n> +\t\tbuffersMemory_[buffer.id] = mmap(NULL,\n> +\t\t\t\t\t\t fb.planes()[0].length,\n> +\t\t\t\t\t\t PROT_READ | PROT_WRITE,\n> +\t\t\t\t\t\t MAP_SHARED,\n> +\t\t\t\t\t\t fb.planes()[0].fd.fd(),\n> +\t\t\t\t\t\t 0);\n> +\n> +\t\tif (buffersMemory_[buffer.id] == MAP_FAILED) {\n> +\t\t\tint ret = -errno;\n> +\t\t\tLOG(IPAIPU3, Fatal) << \"Failed to mmap buffer: \"\n> +\t\t\t\t\t    << strerror(-ret);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)\n> +{\n> +\tfor (unsigned int id : ids) {\n> +\t\tconst auto fb = buffers_.find(id);\n> +\t\tif (fb == buffers_.end())\n> +\t\t\tcontinue;\n> +\n> +\t\tmunmap(buffersMemory_[id], fb->second.planes()[0].length);\n> +\t\tbuffersMemory_.erase(id);\n> +\t\tbuffers_.erase(id);\n> +\t}\n> +}\n> +\n> +void IPAIPU3::processEvent(const IPAOperationData &event)\n> +{\n> +\tswitch (event.operation) {\n> +\tcase IPU3_IPA_EVENT_PARSE_STAT: {\n> +\t\tunsigned int frame = event.data[0];\n> +\t\tunsigned int bufferId = event.data[1];\n> +\n> +\t\tconst ipu3_uapi_stats_3a *stats =\n> +\t\t\tstatic_cast<ipu3_uapi_stats_3a *>(buffersMemory_[bufferId]);\n> +\n> +\t\tparseStatistics(frame, stats);\n> +\t\tbreak;\n> +\t}\n> +\tcase IPU3_IPA_EVENT_FILL_PARAMS: {\n> +\t\tunsigned int frame = event.data[0];\n> +\t\tunsigned int bufferId = event.data[1];\n> +\n> +\t\tipu3_uapi_params *params =\n> +\t\t\tstatic_cast<ipu3_uapi_params *>(buffersMemory_[bufferId]);\n> +\n> +\t\tfillParams(frame, params, event.controls[0]);\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tLOG(IPAIPU3, Error) << \"Unknown event \" << event.operation;\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> +void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params,\n> +\t\t\t [[maybe_unused]] const ControlList &controls)\n> +{\n> +\t/* Prepare parameters buffer. */\n> +\tmemset(params, 0, sizeof(*params));\n> +\n> +\t/* \\todo Fill in parameters buffer. */\n> +\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_PARAM_FILLED;\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +\n> +\t/* \\todo Calculate new values for exposure_ and gain_. */\n> +\tsetControls(frame);\n> +}\n> +\n> +void IPAIPU3::parseStatistics(unsigned int frame,\n> +\t\t\t      [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n> +{\n> +\tControlList ctrls(controls::controls);\n> +\n> +\t/* \\todo React to statistics and update internal state machine. */\n> +\t/* \\todo Add meta-data information to ctrls. */\n> +\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_METADATA_READY;\n> +\top.controls.push_back(ctrls);\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +}\n> +\n> +void IPAIPU3::setControls(unsigned int frame)\n> +{\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_ACTION_SET_SENSOR_CONTROLS;\n> +\n> +\tControlList ctrls(ctrls_);\n> +\tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n> +\tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n> +\top.controls.push_back(ctrls);\n> +\n> +\tqueueFrameAction.emit(frame, op);\n> +}\n> +\n> +/*\n> + * External IPA module interface\n> + */\n> +\n> +extern \"C\" {\n> +const struct IPAModuleInfo ipaModuleInfo = {\n> +\tIPA_MODULE_API_VERSION,\n> +\t1,\n> +\t\"PipelineHandlerIPU3\",\n> +\t\"ipu3\",\n> +};\n> +\n> +struct ipa_context *ipaCreate()\n> +{\n> +\treturn new IPAInterfaceWrapper(std::make_unique<IPAIPU3>());\n> +}\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\n> new file mode 100644\n> index 0000000000000000..a7e18f06b62bef3e\n> --- /dev/null\n> +++ b/src/ipa/ipu3/meson.build\n> @@ -0,0 +1,21 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +ipa_name = 'ipa_ipu3'\n> +\n> +mod = shared_module(ipa_name,\n> +                    'ipu3.cpp',\n> +                    name_prefix : '',\n> +                    include_directories : [ipa_includes, libipa_includes],\n> +                    dependencies : libcamera_dep,\n> +                    link_with : libipa,\n> +                    install : true,\n> +                    install_dir : ipa_install_dir)\n> +\n> +if ipa_sign_module\n> +    custom_target(ipa_name + '.so.sign',\n> +                  input : mod,\n> +                  output : ipa_name + '.so.sign',\n> +                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> +                  install : false,\n> +                  build_by_default : true)\n> +endif\n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index 5a5de267c1477d24..9d623f227a1f9feb 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -19,7 +19,7 @@ subdir('libipa')\n>\n>  ipa_sign = files('ipa-sign.sh')\n>\n> -ipas = ['raspberrypi', 'rkisp1', 'vimc']\n> +ipas = ['ipu3', 'raspberrypi', 'rkisp1', 'vimc']\n>  ipa_names = []\n>\n>  foreach pipeline : get_option('pipelines')\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 0E404BE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 Nov 2020 15:41:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E60F7E9B0;\n\tWed, 18 Nov 2020 16:41:08 +0100 (CET)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DDD7B632AF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 Nov 2020 16:41:06 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 559671C0003;\n\tWed, 18 Nov 2020 15:41:06 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 18 Nov 2020 16:41:09 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20201118154109.rq4wdwxl65inhvhq@uno.localdomain>","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-8-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201105001546.1690179-8-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: ipa: ipu3: Add an\n\tIPA skeleton for the IPU3 pipeline","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14117,"web_url":"https://patchwork.libcamera.org/comment/14117/","msgid":"<X87mF4bIGNYJl5dX@pendragon.ideasonboard.com>","date":"2020-12-08T02:33:59","subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: ipa: ipu3: Add an\n\tIPA skeleton for the IPU3 pipeline","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 Wed, Nov 18, 2020 at 04:41:09PM +0100, Jacopo Mondi wrote:\n> On Thu, Nov 05, 2020 at 01:15:42AM +0100, Niklas Söderlund wrote:\n> > Add an empty IPA skeleton for the IPU3 pipeline. The skeleton IPA do\n> > handle the flow of parameter and statistic buffers but does not read or\n\ns/do handle/handles/\n\n> > write anything in the buffers. It also allows the IPA to set sensor\n> > controls but does not implement any logic to set optimal values and\n> > instead set the V4L2 exposure and gain controls to max and keeps them at\n\ns/set/sets/\n\n> > that setting.\n> >\n> > This IPA is meant as a base to allow the pipeline handler to be wired up\n> > to an IPA. The image algorithms can then later be added to the IPA\n> > independent from also having to also add plumbing to the pipeline\n\ns/independent/independently/\ns/to also/to/\n\n> > handler.\n> >\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  include/libcamera/ipa/ipu3.h |  22 ++++\n> >  src/ipa/ipu3/ipu3.cpp        | 237 +++++++++++++++++++++++++++++++++++\n> >  src/ipa/ipu3/meson.build     |  21 ++++\n> >  src/ipa/meson.build          |   2 +-\n> >  4 files changed, 281 insertions(+), 1 deletion(-)\n> >  create mode 100644 include/libcamera/ipa/ipu3.h\n> >  create mode 100644 src/ipa/ipu3/ipu3.cpp\n> >  create mode 100644 src/ipa/ipu3/meson.build\n> >\n> > diff --git a/include/libcamera/ipa/ipu3.h b/include/libcamera/ipa/ipu3.h\n> > new file mode 100644\n> > index 0000000000000000..7c240a46b97dee28\n> > --- /dev/null\n> > +++ b/include/libcamera/ipa/ipu3.h\n> > @@ -0,0 +1,22 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * ipu3.h - Image Processing Algorithm interface for IPU3\n> > + */\n> > +#ifndef __LIBCAMERA_IPA_INTERFACE_IPU3_H__\n> > +#define __LIBCAMERA_IPA_INTERFACE_IPU3_H__\n> > +\n> > +#ifndef __DOXYGEN__\n> > +\n> > +enum IPU3Operations {\n> > +\tIPU3_IPA_ACTION_SET_SENSOR_CONTROLS = 1,\n> > +\tIPU3_IPA_ACTION_PARAM_FILLED = 2,\n> > +\tIPU3_IPA_ACTION_METADATA_READY = 3,\n> > +\tIPU3_IPA_EVENT_PARSE_STAT = 4,\n\nAs this is an event, should it be named stat ready instead of parse\nstat ? No big deal, this will be reworked with Paul's IPA IPC work\nanyway.\n\n> > +\tIPU3_IPA_EVENT_FILL_PARAMS = 5,\n> > +};\n> > +\n> > +#endif /* __DOXYGEN__ */\n> > +\n> > +#endif /* __LIBCAMERA_IPA_INTERFACE_IPU3_H__ */\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > new file mode 100644\n> > index 0000000000000000..cdcd9b9ff013e0cc\n> > --- /dev/null\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -0,0 +1,237 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * ipu3.cpp - IPU3 Image Processing Algorithms\n> > + */\n> > +\n> > +#include <algorithm>\n> > +#include <math.h>\n> > +#include <queue>\n> > +#include <stdint.h>\n> > +#include <string.h>\n> > +#include <sys/mman.h>\n\nSome of these headers seem unused.\n\n> > +\n> > +#include <linux/intel-ipu3.h>\n> > +#include <linux/v4l2-controls.h>\n> > +\n> > +#include <libcamera/buffer.h>\n> > +#include <libcamera/control_ids.h>\n> > +#include <libcamera/ipa/ipa_interface.h>\n> > +#include <libcamera/ipa/ipa_module_info.h>\n> > +#include <libcamera/ipa/ipu3.h>\n\nYou could include this file at the top, to ensure it gets compiled\nself-contained.\n\n> > +#include <libcamera/request.h>\n> > +\n> > +#include <libipa/ipa_interface_wrapper.h>\n> > +\n> > +#include \"libcamera/internal/log.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(IPAIPU3)\n> > +\n> > +class IPAIPU3 : public IPAInterface\n> > +{\n> > +public:\n> > +\tint init([[maybe_unused]] const IPASettings &settings) override\n> > +\t{\n> > +\t\treturn 0;\n> > +\t}\n> > +\tint start() override { return 0; }\n> > +\tvoid stop() override {}\n> > +\n> > +\tvoid configure(const CameraSensorInfo &info,\n> > +\t\t       const std::map<unsigned int, IPAStream> &streamConfig,\n> > +\t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> > +\t\t       const IPAOperationData &ipaConfig,\n> > +\t\t       IPAOperationData *response) override;\n> > +\tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n> > +\tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> > +\tvoid processEvent(const IPAOperationData &event) override;\n> > +\n> > +private:\n> > +\tvoid fillParams(unsigned int frame, ipu3_uapi_params *params,\n> > +\t\t\tconst ControlList &controls);\n> > +\n> > +\tvoid parseStatistics(unsigned int frame,\n> > +\t\t\t     const ipu3_uapi_stats_3a *stats);\n> > +\n> > +\tvoid setControls(unsigned int frame);\n> > +\n> > +\tstd::map<unsigned int, FrameBuffer> buffers_;\n> > +\tstd::map<unsigned int, void *> buffersMemory_;\n> > +\n> > +\tControlInfoMap ctrls_;\n> > +\n> > +\t/* Camera sensor controls. */\n> > +\tuint32_t exposure_;\n> > +\tuint32_t minExposure_;\n> > +\tuint32_t maxExposure_;\n> > +\tuint32_t gain_;\n> > +\tuint32_t minGain_;\n> > +\tuint32_t maxGain_;\n> > +};\n> > +\n> > +void IPAIPU3::configure([[maybe_unused]] const CameraSensorInfo &info,\n> > +\t\t\t[[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,\n> > +\t\t\tconst std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> > +\t\t\t[[maybe_unused]] const IPAOperationData &ipaConfig,\n> > +\t\t\t[[maybe_unused]] IPAOperationData *result)\n> > +{\n> > +\tif (entityControls.empty())\n> > +\t\treturn;\n> > +\n> > +\tctrls_ = entityControls.at(0);\n> > +\n> > +\tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n> \n> Shouldn't we use libcamera controls ? Otherwise the pipeline handler\n> has to implement the translation.\n> \n> The rest is good, with the above question clarified I'll send my tag.\n\nWe do the same in the RPi IPA. This should be reworked to use controls\nspecific to the CameraSensor class (we may reuse libcamera controls, but\nconceptually speaking, they're not the same, so we'll need a different\nControlInfoMap to report the correct limits, libcamera::controls won't\ndo). I'd rather fix this on top, for all IPA modules, as it requires a\nrework of the CameraSensor class.\n\n> > +\tif (itExp == ctrls_.end()) {\n> > +\t\tLOG(IPAIPU3, Error) << \"Can't find exposure control\";\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n> > +\tif (itGain == ctrls_.end()) {\n> > +\t\tLOG(IPAIPU3, Error) << \"Can't find gain control\";\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\tminExposure_ = std::max<uint32_t>(itExp->second.min().get<int32_t>(), 1);\n> > +\tmaxExposure_ = itExp->second.max().get<int32_t>();\n> > +\texposure_ = maxExposure_;\n> > +\n> > +\tminGain_ = std::max<uint32_t>(itGain->second.min().get<int32_t>(), 1);\n> > +\tmaxGain_ = itGain->second.max().get<int32_t>();\n> > +\tgain_ = maxGain_;\n> > +\n> > +\tsetControls(0);\n\nI'm not sure we need to do so at configure() time, but this is a\nskeleton anyway, so it's no big deal. Let's however be careful on how\nthis is handled on the pipeline handler side, an application could call\nconfigure() multiple times, and if this results in multiple calls to\nDelayedControls::push() before start(), I expect some trouble. This\nlikely needs to be addressed on the pipeline handler side, and it's not\na blocker as this is work in progress.\n\n> > +}\n> > +\n> > +void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)\n> > +{\n> > +\tfor (const IPABuffer &buffer : buffers) {\n> > +\t\tauto elem = buffers_.emplace(std::piecewise_construct,\n> > +\t\t\t\t\t     std::forward_as_tuple(buffer.id),\n> > +\t\t\t\t\t     std::forward_as_tuple(buffer.planes));\n> > +\t\tconst FrameBuffer &fb = elem.first->second;\n> > +\n> > +\t\tbuffersMemory_[buffer.id] = mmap(NULL,\n> > +\t\t\t\t\t\t fb.planes()[0].length,\n> > +\t\t\t\t\t\t PROT_READ | PROT_WRITE,\n> > +\t\t\t\t\t\t MAP_SHARED,\n> > +\t\t\t\t\t\t fb.planes()[0].fd.fd(),\n> > +\t\t\t\t\t\t 0);\n> > +\n> > +\t\tif (buffersMemory_[buffer.id] == MAP_FAILED) {\n> > +\t\t\tint ret = -errno;\n> > +\t\t\tLOG(IPAIPU3, Fatal) << \"Failed to mmap buffer: \"\n> > +\t\t\t\t\t    << strerror(-ret);\n> > +\t\t}\n\nHow about using MappedFrameBuffer ? The RPi IPA has moved to that since\nyou've posted this series.\n\n> > +\t}\n> > +}\n> > +\n> > +void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)\n> > +{\n> > +\tfor (unsigned int id : ids) {\n> > +\t\tconst auto fb = buffers_.find(id);\n> > +\t\tif (fb == buffers_.end())\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tmunmap(buffersMemory_[id], fb->second.planes()[0].length);\n> > +\t\tbuffersMemory_.erase(id);\n> > +\t\tbuffers_.erase(id);\n> > +\t}\n> > +}\n> > +\n> > +void IPAIPU3::processEvent(const IPAOperationData &event)\n> > +{\n> > +\tswitch (event.operation) {\n> > +\tcase IPU3_IPA_EVENT_PARSE_STAT: {\n> > +\t\tunsigned int frame = event.data[0];\n> > +\t\tunsigned int bufferId = event.data[1];\n> > +\n> > +\t\tconst ipu3_uapi_stats_3a *stats =\n> > +\t\t\tstatic_cast<ipu3_uapi_stats_3a *>(buffersMemory_[bufferId]);\n> > +\n> > +\t\tparseStatistics(frame, stats);\n> > +\t\tbreak;\n> > +\t}\n> > +\tcase IPU3_IPA_EVENT_FILL_PARAMS: {\n> > +\t\tunsigned int frame = event.data[0];\n> > +\t\tunsigned int bufferId = event.data[1];\n> > +\n> > +\t\tipu3_uapi_params *params =\n> > +\t\t\tstatic_cast<ipu3_uapi_params *>(buffersMemory_[bufferId]);\n> > +\n> > +\t\tfillParams(frame, params, event.controls[0]);\n> > +\t\tbreak;\n> > +\t}\n> > +\tdefault:\n> > +\t\tLOG(IPAIPU3, Error) << \"Unknown event \" << event.operation;\n> > +\t\tbreak;\n> > +\t}\n> > +}\n> > +\n> > +void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params,\n> > +\t\t\t [[maybe_unused]] const ControlList &controls)\n> > +{\n> > +\t/* Prepare parameters buffer. */\n> > +\tmemset(params, 0, sizeof(*params));\n> > +\n> > +\t/* \\todo Fill in parameters buffer. */\n> > +\n> > +\tIPAOperationData op;\n> > +\top.operation = IPU3_IPA_ACTION_PARAM_FILLED;\n> > +\n> > +\tqueueFrameAction.emit(frame, op);\n> > +\n> > +\t/* \\todo Calculate new values for exposure_ and gain_. */\n> > +\tsetControls(frame);\n> > +}\n> > +\n> > +void IPAIPU3::parseStatistics(unsigned int frame,\n> > +\t\t\t      [[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n> > +{\n> > +\tControlList ctrls(controls::controls);\n> > +\n> > +\t/* \\todo React to statistics and update internal state machine. */\n> > +\t/* \\todo Add meta-data information to ctrls. */\n> > +\n> > +\tIPAOperationData op;\n> > +\top.operation = IPU3_IPA_ACTION_METADATA_READY;\n> > +\top.controls.push_back(ctrls);\n> > +\n> > +\tqueueFrameAction.emit(frame, op);\n> > +}\n> > +\n> > +void IPAIPU3::setControls(unsigned int frame)\n> > +{\n> > +\tIPAOperationData op;\n> > +\top.operation = IPU3_IPA_ACTION_SET_SENSOR_CONTROLS;\n> > +\n> > +\tControlList ctrls(ctrls_);\n> > +\tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));\n> > +\tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_));\n> > +\top.controls.push_back(ctrls);\n> > +\n> > +\tqueueFrameAction.emit(frame, op);\n> > +}\n> > +\n> > +/*\n> > + * External IPA module interface\n> > + */\n> > +\n> > +extern \"C\" {\n> > +const struct IPAModuleInfo ipaModuleInfo = {\n> > +\tIPA_MODULE_API_VERSION,\n> > +\t1,\n> > +\t\"PipelineHandlerIPU3\",\n> > +\t\"ipu3\",\n> > +};\n> > +\n> > +struct ipa_context *ipaCreate()\n> > +{\n> > +\treturn new IPAInterfaceWrapper(std::make_unique<IPAIPU3>());\n> > +}\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/ipu3/meson.build b/src/ipa/ipu3/meson.build\n> > new file mode 100644\n> > index 0000000000000000..a7e18f06b62bef3e\n> > --- /dev/null\n> > +++ b/src/ipa/ipu3/meson.build\n> > @@ -0,0 +1,21 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +ipa_name = 'ipa_ipu3'\n> > +\n> > +mod = shared_module(ipa_name,\n> > +                    'ipu3.cpp',\n> > +                    name_prefix : '',\n> > +                    include_directories : [ipa_includes, libipa_includes],\n\nMissing spaces after [ and before ].\n\n> > +                    dependencies : libcamera_dep,\n> > +                    link_with : libipa,\n\nBased on 5d05418d9b53e1838692f687a6dc373dad45355c, I wonder if we should\nlink with libatomic.\n\n> > +                    install : true,\n> > +                    install_dir : ipa_install_dir)\n> > +\n> > +if ipa_sign_module\n> > +    custom_target(ipa_name + '.so.sign',\n> > +                  input : mod,\n> > +                  output : ipa_name + '.so.sign',\n> > +                  command : [ ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@' ],\n> > +                  install : false,\n> > +                  build_by_default : true)\n> > +endif\n> > diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> > index 5a5de267c1477d24..9d623f227a1f9feb 100644\n> > --- a/src/ipa/meson.build\n> > +++ b/src/ipa/meson.build\n> > @@ -19,7 +19,7 @@ subdir('libipa')\n> >\n> >  ipa_sign = files('ipa-sign.sh')\n> >\n> > -ipas = ['raspberrypi', 'rkisp1', 'vimc']\n> > +ipas = ['ipu3', 'raspberrypi', 'rkisp1', 'vimc']\n> >  ipa_names = []\n> >\n> >  foreach pipeline : get_option('pipelines')","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 7AD46BDB20\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  8 Dec 2020 02:34:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4824767E71;\n\tTue,  8 Dec 2020 03:34:04 +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 3AD5E67E6C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Dec 2020 03:34:03 +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 BBFE8DD;\n\tTue,  8 Dec 2020 03:34:02 +0100 (CET)"],"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=\"YRmsJlvT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1607394843;\n\tbh=zmPn/IXKbGjyaT3UwQo2S4ROfSNJF9JB3ruQWYBPP+A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YRmsJlvT37uODGlaB/VRg+6XbTZI4Kb0FlK0t5lp7MKMuAdhg9Mb3fBkzF9rNgO9Y\n\tspm3hCA5/VbWZtGQPgqcW8VWQvS6g4FPJX1HpniTMPanuT+deQHG5tnrmZ5TS8yMmi\n\t/80l949sV8oAv/bCEj8BIcMAAdwqzYyXrTes3ZDo=","Date":"Tue, 8 Dec 2020 04:33:59 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<X87mF4bIGNYJl5dX@pendragon.ideasonboard.com>","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-8-niklas.soderlund@ragnatech.se>\n\t<20201118154109.rq4wdwxl65inhvhq@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201118154109.rq4wdwxl65inhvhq@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: ipa: ipu3: Add an\n\tIPA skeleton for the IPU3 pipeline","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]