[{"id":30754,"web_url":"https://patchwork.libcamera.org/comment/30754/","msgid":"","date":"2024-08-12T14:47:12","subject":"Re: [PATCH v2 04/10] ipa: mali-c55: Add Mali-C55 ISP IPA module","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"On 09/07/2024 15:49, Daniel Scally wrote:\n> Add a barebones IPA module for the Mali-C55 ISP. In this initial\n> implementation pretty much only buffer plumbing is implemented.\n>\n> Acked-by: Nayden Kanchev \n> Co-developed-by: Jacopo Mondi \n> Signed-off-by: Jacopo Mondi \n> Signed-off-by: Daniel Scally \n> ---\n> Documentation/Doxyfile.in | 1 +\n> include/libcamera/ipa/mali-c55.mojom | 34 +++\n> include/libcamera/ipa/meson.build | 1 +\n> meson_options.txt | 2 +-\n> src/ipa/mali-c55/algorithms/algorithm.h | 24 ++\n> src/ipa/mali-c55/algorithms/meson.build | 4 +\n> src/ipa/mali-c55/data/meson.build | 8 +\n> src/ipa/mali-c55/data/uncalibrated.yaml | 6 +\n> src/ipa/mali-c55/ipa_context.cpp | 101 +++++++\n> src/ipa/mali-c55/ipa_context.h | 42 +++\n> src/ipa/mali-c55/mali-c55.cpp | 341 ++++++++++++++++++++++++\n> src/ipa/mali-c55/meson.build | 33 +++\n> src/ipa/mali-c55/module.h | 27 ++\n> 13 files changed, 623 insertions(+), 1 deletion(-)\n> create mode 100644 include/libcamera/ipa/mali-c55.mojom\n> create mode 100644 src/ipa/mali-c55/algorithms/algorithm.h\n> create mode 100644 src/ipa/mali-c55/algorithms/meson.build\n> create mode 100644 src/ipa/mali-c55/data/meson.build\n> create mode 100644 src/ipa/mali-c55/data/uncalibrated.yaml\n> create mode 100644 src/ipa/mali-c55/ipa_context.cpp\n> create mode 100644 src/ipa/mali-c55/ipa_context.h\n> create mode 100644 src/ipa/mali-c55/mali-c55.cpp\n> create mode 100644 src/ipa/mali-c55/meson.build\n> create mode 100644 src/ipa/mali-c55/module.h\n>\n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index abafcf6c..62ca622d 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -51,6 +51,7 @@ EXCLUDE = @TOP_SRCDIR@/include/libcamera/base/span.h \\\n> EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \\\n> @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h \\\n> @TOP_BUILDDIR@/include/libcamera/ipa/ipu3_*.h \\\n> + @TOP_BUILDDIR@/include/libcamera/ipa/mali-c55_*.h \\\n> @TOP_BUILDDIR@/include/libcamera/ipa/raspberrypi_*.h \\\n> @TOP_BUILDDIR@/include/libcamera/ipa/rkisp1_*.h \\\n> @TOP_BUILDDIR@/include/libcamera/ipa/vimc_*.h\n> diff --git a/include/libcamera/ipa/mali-c55.mojom b/include/libcamera/ipa/mali-c55.mojom\n> new file mode 100644\n> index 00000000..4f6f461c\n> --- /dev/null\n> +++ b/include/libcamera/ipa/mali-c55.mojom\n> @@ -0,0 +1,34 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +\n> +module ipa.mali_c55;\n> +\n> +import \"include/libcamera/ipa/core.mojom\";\n> +\n> +struct IPAConfigInfo {\n> +\tlibcamera.IPACameraSensorInfo sensorInfo;\n> +\tlibcamera.ControlInfoMap sensorControls;\n> +};\n> +\n> +interface IPAMaliC55Interface {\n> +\tinit(libcamera.IPASettings settings, IPAConfigInfo configInfo)\n> +\t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n> +\tstart() => (int32 ret);\n> +\tstop();\n> +\n> +\tconfigure(IPAConfigInfo configInfo, uint8 bayerOrder)\n> +\t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n> +\n> +\tmapBuffers(array buffers, bool readOnly);\n> +\tunmapBuffers(array buffers);\n> +\n> +\t[async] queueRequest(uint32 frame, libcamera.ControlList reqControls);\n> +\t[async] fillParams(uint32 request, uint32 bufferId);\n> +\t[async] processStats(uint32 request, uint32 bufferId,\n> +\t\t\t libcamera.ControlList sensorControls);\n> +};\n> +\n> +interface IPAMaliC55EventInterface {\n> +\tparamsComputed(uint32 request);\n> +\tstatsProcessed(uint32 request, libcamera.ControlList metadata);\n> +\tsetSensorControls(libcamera.ControlList sensorControls);\n> +};\n> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> index 3352d08f..3b872a7f 100644\n> --- a/include/libcamera/ipa/meson.build\n> +++ b/include/libcamera/ipa/meson.build\n> @@ -63,6 +63,7 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',\n> # Mapping from pipeline handler name to mojom file\n> pipeline_ipa_mojom_mapping = {\n> 'ipu3': 'ipu3.mojom',\n> + 'mali-c55': 'mali-c55.mojom',\n> 'rkisp1': 'rkisp1.mojom',\n> 'rpi/vc4': 'raspberrypi.mojom',\n> 'simple': 'soft.mojom',\n> diff --git a/meson_options.txt b/meson_options.txt\n> index 7aa41249..5365601c 100644\n> --- a/meson_options.txt\n> +++ b/meson_options.txt\n> @@ -32,7 +32,7 @@ option('gstreamer',\n> \n> option('ipas',\n> type : 'array',\n> - choices : ['ipu3', 'rkisp1', 'rpi/vc4', 'simple', 'vimc'],\n> + choices : ['ipu3', 'mali-c55', 'rkisp1', 'rpi/vc4', 'simple', 'vimc'],\n> description : 'Select which IPA modules to build')\n> \n> option('lc-compliance',\n> diff --git a/src/ipa/mali-c55/algorithms/algorithm.h b/src/ipa/mali-c55/algorithms/algorithm.h\n> new file mode 100644\n> index 00000000..afd90982\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/algorithms/algorithm.h\n> @@ -0,0 +1,24 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * algorithm.h - Mali-C55 control algorithm interface\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"module.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::mali_c55 {\n> +\n> +class Algorithm : public libcamera::ipa::Algorithm\n> +{\n> +};\n> +\n> +} /* namespace ipa::mali_c55 */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/mali-c55/algorithms/meson.build b/src/ipa/mali-c55/algorithms/meson.build\n> new file mode 100644\n> index 00000000..f2203b15\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/algorithms/meson.build\n> @@ -0,0 +1,4 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +mali_c55_ipa_algorithms = files([\n> +])\n> diff --git a/src/ipa/mali-c55/data/meson.build b/src/ipa/mali-c55/data/meson.build\n> new file mode 100644\n> index 00000000..88109d17\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/data/meson.build\n> @@ -0,0 +1,8 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +conf_files = files([\n> + 'uncalibrated.yaml'\n> +])\n> +\n> +install_data(conf_files,\n> + install_dir : ipa_data_dir / 'mali-c55')\n> diff --git a/src/ipa/mali-c55/data/uncalibrated.yaml b/src/ipa/mali-c55/data/uncalibrated.yaml\n> new file mode 100644\n> index 00000000..2cdc39a8\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/data/uncalibrated.yaml\n> @@ -0,0 +1,6 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +%YAML 1.1\n> +---\n> +version: 1\n> +algorithms:\n> +...\n> diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp\n> new file mode 100644\n> index 00000000..99f76ecd\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/ipa_context.cpp\n> @@ -0,0 +1,101 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * ipa_context.cpp - MaliC55 IPA Context\n> + */\n> +\n> +#include \"ipa_context.h\"\n> +\n> +/**\n> + * \\file ipa_context.h\n> + * \\brief Context and state information shared between the algorithms\n> + */\n> +\n> +namespace libcamera::ipa::mali_c55 {\n> +\n> +/**\n> + * \\struct IPASessionConfiguration\n> + * \\brief Session configuration for the IPA module\n> + *\n> + * The session configuration contains all IPA configuration parameters that\n> + * remain constant during the capture session, from IPA module start to stop.\n> + * It is typically set during the configure() operation of the IPA module, but\n> + * may also be updated in the start() operation.\n> + */\n> +\n> +/**\n> + * \\struct IPAActiveState\n> + * \\brief Active state for algorithms\n> + *\n> + * The active state contains all algorithm-specific data that needs to be\n> + * maintained by algorithms across frames. Unlike the session configuration,\n> + * the active state is mutable and constantly updated by algorithms. The active\n> + * state is accessible through the IPAContext structure.\n> + *\n> + * The active state stores two distinct categories of information:\n> + *\n> + * - The consolidated value of all algorithm controls. Requests passed to\n> + * the queueRequest() function store values for controls that the\n> + * application wants to modify for that particular frame, and the\n> + * queueRequest() function updates the active state with those values.\n> + * The active state thus contains a consolidated view of the value of all\n> + * controls handled by the algorithm.\n> + *\n> + * - The value of parameters computed by the algorithm when running in auto\n> + * mode. Algorithms running in auto mode compute new parameters every\n> + * time statistics buffers are received (either synchronously, or\n> + * possibly in a background thread). The latest computed value of those\n> + * parameters is stored in the active state in the process() function.\n> + *\n> + * Each of the members in the active state belongs to a specific algorithm. A\n> + * member may be read by any algorithm, but shall only be written by its owner.\n> + */\n> +\n> +/**\n> + * \\struct IPAFrameContext\n> + * \\brief Per-frame context for algorithms\n> + *\n> + * The frame context stores two distinct categories of information:\n> + *\n> + * - The value of the controls to be applied to the frame. These values are\n> + * typically set in the queueRequest() function, from the consolidated\n> + * control values stored in the active state. The frame context thus stores\n> + * values for all controls related to the algorithm, not limited to the\n> + * controls specified in the corresponding request, but consolidated from all\n> + * requests that have been queued so far.\n> + *\n> + * For controls that can be set manually or computed by an algorithm\n> + * (depending on the algorithm operation mode), such as for instance the\n> + * colour gains for the AWB algorithm, the control value will be stored in\n> + * the frame context in the queueRequest() function only when operating in\n> + * manual mode. When operating in auto mode, the values are computed by the\n> + * algorithm in process(), stored in the active state, and copied to the\n> + * frame context in prepare(), just before being stored in the ISP parameters\n> + * buffer.\n> + *\n> + * The queueRequest() function can also store ancillary data in the frame\n> + * context, such as flags to indicate if (and what) control values have\n> + * changed compared to the previous request.\n> + *\n> + * - Status information computed by the algorithm for a frame. For instance,\n> + * the colour temperature estimated by the AWB algorithm from ISP statistics\n> + * calculated on a frame is stored in the frame context for that frame in\n> + * the process() function.\n> + */\n> +\n> +/**\n> + * \\struct IPAContext\n> + * \\brief Global IPA context data shared between all algorithms\n> + *\n> + * \\var IPAContext::configuration\n> + * \\brief The IPA session configuration, immutable during the session\n> + *\n> + * \\var IPAContext::activeState\n> + * \\brief The IPA active state, storing the latest state for all algorithms\n> + *\n> + * \\var IPAContext::frameContexts\n> + * \\brief Ring buffer of per-frame contexts\n> + */\n> +\n> +} /* namespace libcamera::ipa::mali_c55 */\n> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h\n> new file mode 100644\n> index 00000000..9e408a17\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/ipa_context.h\n> @@ -0,0 +1,42 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * ipa_context.h - Mali-C55 IPA Context\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::mali_c55 {\n> +\n> +struct IPASessionConfiguration {\n> +};\n> +\n> +struct IPAActiveState {\n> +};\n> +\n> +struct IPAFrameContext : public FrameContext {\n> +\tstruct {\n> +\t\tuint32_t exposure;\n> +\t\tdouble sensorGain;\n> +\t} agc;\n> +};\n> +\n> +struct IPAContext {\n> +\tIPASessionConfiguration configuration;\n> +\tIPAActiveState activeState;\n> +\n> +\tFCQueue frameContexts;\n> +\n> +\tControlInfoMap::Map ctrlMap;\n> +};\n> +\n> +} /* namespace ipa::mali_c55 */\n> +\n> +} /* namespace libcamera*/\n> diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp\n> new file mode 100644\n> index 00000000..3daac3af\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/mali-c55.cpp\n> @@ -0,0 +1,341 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Ideas on Board Oy\n> + *\n> + * mali-c55.cpp - Mali-C55 ISP image processing algorithms\n> + */\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/mapped_framebuffer.h\"\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"algorithms/algorithm.h\"\n> +#include \"libipa/camera_sensor_helper.h\"\n> +\n> +#include \"ipa_context.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPAMaliC55)\n> +\n> +namespace ipa::mali_c55 {\n> +\n> +/* Maximum number of frame contexts to be held */\n> +static constexpr uint32_t kMaxFrameContexts = 16;\n> +\n> +class IPAMaliC55 : public IPAMaliC55Interface, public Module\n> +{\n> +public:\n> +\tIPAMaliC55();\n> +\n> +\tint init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,\n> +\t\t ControlInfoMap *ipaControls) override;\n> +\tint start() override;\n> +\tvoid stop() override;\n> +\tint configure(const IPAConfigInfo &ipaConfig, uint8_t bayerOrder,\n> +\t\t ControlInfoMap *ipaControls) override;\n> +\tvoid mapBuffers(const std::vector &buffers, bool readOnly) override;\n> +\tvoid unmapBuffers(const std::vector &buffers) override;\n> +\tvoid queueRequest(const uint32_t frame, const ControlList &controls) override;\n> +\tvoid fillParams(unsigned int request, uint32_t bufferId) override;\n> +\tvoid processStats(unsigned int request, unsigned int bufferId,\n> +\t\t\t const ControlList &sensorControls) override;\n> +\n> +protected:\n> +\tstd::string logPrefix() const override;\n> +\n> +private:\n> +\tvoid updateControls(const IPACameraSensorInfo &sensorInfo,\n> +\t\t\t const ControlInfoMap &sensorControls,\n> +\t\t\t ControlInfoMap *ipaControls);\n> +\tvoid setControls();\n> +\n> +\tstd::map buffers_;\n> +\n> +\tControlInfoMap sensorControls_;\n> +\n> +\t/* Interface to the Camera Helper */\n> +\tstd::unique_ptr camHelper_;\n> +\n> +\t/* Local parameter storage */\n> +\tstruct IPAContext context_;\n> +};\n> +\n> +static constexpr uint32_t kNumFrameContexts = 16;\n> +\n> +namespace {\n> +\n> +} /* namespace */\n> +\n> +IPAMaliC55::IPAMaliC55()\n> +\t: context_({ {}, {}, { kMaxFrameContexts }, {} })\n> +{\n> +}\n> +\n> +std::string IPAMaliC55::logPrefix() const\n> +{\n> +\treturn \"mali-c55\";\n> +}\n> +\n> +int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,\n> +\t\t ControlInfoMap *ipaControls)\n> +{\n> +\tcamHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);\n> +\tif (!camHelper_) {\n> +\t\tLOG(IPAMaliC55, Error)\n> +\t\t\t<< \"Failed to create camera sensor helper for \"\n> +\t\t\t<< settings.sensorModel;\n> +\t\treturn -ENODEV;\n> +\t}\n> +\n> +\tFile file(settings.configurationFile);\n> +\tif (!file.open(File::OpenModeFlag::ReadOnly)) {\n> +\t\tint ret = file.error();\n> +\t\tLOG(IPAMaliC55, Error)\n> +\t\t\t<< \"Failed to open configuration file \"\n> +\t\t\t<< settings.configurationFile << \": \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tstd::unique_ptr data = YamlParser::parse(file);\n> +\tif (!data)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (!data->contains(\"algorithms\")) {\n> +\t\tLOG(IPAMaliC55, Error)\n> +\t\t\t<< \"Tuning file doesn't contain any algorithm\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tint ret = createAlgorithms(context_, (*data)[\"algorithms\"]);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tupdateControls(ipaConfig.sensorInfo, ipaConfig.sensorControls, ipaControls);\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPAMaliC55::setControls()\n> +{\n> +\tControlList ctrls(sensorControls_);\n> +\n> +\tsetSensorControls.emit(ctrls);\n> +}\n> +\n> +int IPAMaliC55::start()\n> +{\n> +\treturn 0;\n> +}\n> +\n> +void IPAMaliC55::stop()\n> +{\n> +\tcontext_.frameContexts.clear();\n> +}\n> +\n> +void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,\n> +\t\t\t\tconst ControlInfoMap &sensorControls,\n> +\t\t\t\tControlInfoMap *ipaControls)\n> +{\n> +\tControlInfoMap::Map ctrlMap;\n> +\n> +\t/*\n> +\t * Compute the frame duration limits.\n> +\t *\n> +\t * The frame length is computed assuming a fixed line length combined\n> +\t * with the vertical frame sizes.\n> +\t */\n> +\tconst ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;\n> +\tuint32_t hblank = v4l2HBlank.def().get();\n> +\tuint32_t lineLength = sensorInfo.outputSize.width + hblank;\n> +\n> +\tconst ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n> +\tstd::array frameHeights{\n> +\t\tv4l2VBlank.min().get() + sensorInfo.outputSize.height,\n> +\t\tv4l2VBlank.max().get() + sensorInfo.outputSize.height,\n> +\t\tv4l2VBlank.def().get() + sensorInfo.outputSize.height,\n> +\t};\n> +\n> +\tstd::array frameDurations;\n> +\tfor (unsigned int i = 0; i < frameHeights.size(); ++i) {\n> +\t\tuint64_t frameSize = lineLength * frameHeights[i];\n> +\t\tframeDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n> +\t}\n> +\n> +\tctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n> +\t\t\t\t\t\t\t frameDurations[1],\n> +\t\t\t\t\t\t\t frameDurations[2]);\n> +\n> +\t/*\n> +\t * Compute exposure time limits from the V4L2_CID_EXPOSURE control\n> +\t * limits and the line duration.\n> +\t */\n> +\tdouble lineDuration = sensorInfo.minLineLength / sensorInfo.pixelRate;\n> +\n> +\tconst ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n> +\tint32_t minExposure = v4l2Exposure.min().get() * lineDuration;\n> +\tint32_t maxExposure = v4l2Exposure.max().get() * lineDuration;\n> +\tint32_t defExposure = v4l2Exposure.def().get() * lineDuration;\n> +\tctrlMap[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, defExposure);\n> +\n> +\t/* Compute the analogue gain limits. */\n> +\tconst ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;\n> +\tfloat minGain = camHelper_->gain(v4l2Gain.min().get());\n> +\tfloat maxGain = camHelper_->gain(v4l2Gain.max().get());\n> +\tfloat defGain = camHelper_->gain(v4l2Gain.def().get());\n> +\tctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);\n> +\n> +\t/*\n> +\t * Merge in any controls that we support either statically or from the\n> +\t * algorithms.\n> +\t */\n> +\tctrlMap.merge(context_.ctrlMap);\n> +\n> +\t*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);\n> +}\n> +\n> +int IPAMaliC55::configure(const IPAConfigInfo &ipaConfig,\n> +\t\t\t [[maybe_unused]] uint8_t bayerOrder,\n> +\t\t\t ControlInfoMap *ipaControls)\n> +{\n> +\tsensorControls_ = ipaConfig.sensorControls;\n> +\n> +\t/* Clear the IPA context before the streaming session. */\n> +\tcontext_.configuration = {};\n> +\tcontext_.activeState = {};\n> +\tcontext_.frameContexts.clear();\n> +\n> +\tconst IPACameraSensorInfo &info = ipaConfig.sensorInfo;\n> +\n> +\tupdateControls(info, ipaConfig.sensorControls, ipaControls);\n> +\n> +\tfor (auto const &a : algorithms()) {\n> +\t\tAlgorithm *algo = static_cast(a.get());\n> +\n> +\t\tint ret = algo->configure(context_, info);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPAMaliC55::mapBuffers(const std::vector &buffers, bool readOnly)\n> +{\n> +\tfor (const IPABuffer &buffer : buffers) {\n> +\t\tconst FrameBuffer fb(buffer.planes);\n> +\t\tbuffers_.emplace(\n> +\t\t\tbuffer.id,\n> +\t\t\tMappedFrameBuffer(\n> +\t\t\t\t&fb,\n> +\t\t\t\treadOnly ? MappedFrameBuffer::MapFlag::Read\n> +\t\t\t\t\t : MappedFrameBuffer::MapFlag::ReadWrite));\n> +\t}\n> +}\n> +\n> +void IPAMaliC55::unmapBuffers(const std::vector &buffers)\n> +{\n> +\tfor (const IPABuffer &buffer : buffers) {\n> +\t\tauto it = buffers_.find(buffer.id);\n> +\t\tif (it == buffers_.end())\n> +\t\t\tcontinue;\n> +\n> +\t\tbuffers_.erase(buffer.id);\n> +\t}\n> +}\n> +\n> +void IPAMaliC55::queueRequest(const uint32_t frame, const ControlList &controls)\n> +{\n> +\tIPAFrameContext &frameContext = context_.frameContexts.alloc(frame);\nThe index to FCQueue was frame here...\n> +\n> +\tfor (auto const &a : algorithms()) {\n> +\t\tAlgorithm *algo = static_cast(a.get());\n> +\n> +\t\talgo->queueRequest(context_, frame, frameContext, controls);\n> +\t}\n> +}\n> +\n> +void IPAMaliC55::fillParams(unsigned int request,\n> +\t\t\t [[maybe_unused]] uint32_t bufferId)\n> +{\n> +\tstruct mali_c55_params_buffer *params;\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(request);\n...but request here - that inconsistency is something I need to fix.\n> +\n> +\tparams = reinterpret_cast(\n> +\t\tbuffers_.at(bufferId).planes()[0].data());\n> +\tmemset(params, 0, sizeof(mali_c55_params_buffer));\n> +\n> +\tparams->version = MALI_C55_PARAM_BUFFER_V1;\n> +\n> +\tfor (auto const &algo : algorithms()) {\n> +\t\talgo->prepare(context_, request, frameContext, params);\n> +\n> +\t\tASSERT(params->total_size <= MALI_C55_PARAMS_MAX_SIZE);\n> +\t}\n> +\n> +\tparamsComputed.emit(request);\n> +}\n> +\n> +void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,\n> +\t\t\t const ControlList &sensorControls)\n> +{\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(request);\n> +\tconst mali_c55_stats_buffer *stats = nullptr;\n> +\n> +\tstats = reinterpret_cast(\n> +\t\tbuffers_.at(bufferId).planes()[0].data());\n> +\n> +\tframeContext.agc.exposure =\n> +\t\tsensorControls.get(V4L2_CID_EXPOSURE).get();\n> +\tframeContext.agc.sensorGain =\n> +\t\tcamHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get());\n> +\n> +\tControlList metadata(controls::controls);\n> +\n> +\tfor (auto const &a : algorithms()) {\n> +\t\tAlgorithm *algo = static_cast(a.get());\n> +\n> +\t\talgo->process(context_, request, frameContext, stats, metadata);\n> +\t}\n> +\n> +\tsetControls();\n> +\n> +\tstatsProcessed.emit(request, metadata);\n> +}\n> +\n> +} /* namespace ipa::mali_c55 */\n> +\n> +/*\n> + * External IPA module interface\n> + */\n> +extern \"C\" {\n> +const struct IPAModuleInfo ipaModuleInfo = {\n> +\tIPA_MODULE_API_VERSION,\n> +\t1,\n> +\t\"mali-c55\",\n> +\t\"mali-c55\",\n> +};\n> +\n> +IPAInterface *ipaCreate()\n> +{\n> +\treturn new ipa::mali_c55::IPAMaliC55();\n> +}\n> +\n> +} /* extern \"C\" */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/mali-c55/meson.build b/src/ipa/mali-c55/meson.build\n> new file mode 100644\n> index 00000000..9e9b269c\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/meson.build\n> @@ -0,0 +1,33 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +subdir('algorithms')\n> +subdir('data')\n> +\n> +ipa_name = 'ipa_mali_c55'\n> +\n> +mali_c55_ipa_sources = files([\n> + 'ipa_context.cpp',\n> + 'mali-c55.cpp'\n> +])\n> +\n> +mali_c55_ipa_sources += mali_c55_ipa_algorithms\n> +\n> +mod = shared_module(ipa_name,\n> + [mali_c55_ipa_sources, libcamera_generated_ipa_headers],\n> + name_prefix : '',\n> + include_directories : [ipa_includes, libipa_includes],\n> + dependencies : libcamera_private,\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> +\n> +ipa_names += ipa_name\n> diff --git a/src/ipa/mali-c55/module.h b/src/ipa/mali-c55/module.h\n> new file mode 100644\n> index 00000000..1d85ec1f\n> --- /dev/null\n> +++ b/src/ipa/mali-c55/module.h\n> @@ -0,0 +1,27 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas On Board\n> + *\n> + * module.h - Mali-C55 IPA Module\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"ipa_context.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::mali_c55 {\n> +\n> +using Module = ipa::Module +\t\t\t mali_c55_params_buffer, mali_c55_stats_buffer>;\n> +\n> +} /* namespace ipa::mali_c55 */\n> +\n> +} /* namespace libcamera*/","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 C9862C323E\n\tfor ;\n\tMon, 12 Aug 2024 14:47:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 231B7633B3;\n\tMon, 12 Aug 2024 16:47:18 +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 3D6DD63398\n\tfor ;\n\tMon, 12 Aug 2024 16:47:16 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B06E36B5;\n\tMon, 12 Aug 2024 16:46:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MRZYMong\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723473979;\n\tbh=DNXbihXsK0m9zLPNGuBRNP0+Ly4fknju1emgm3Nc8uo=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=MRZYMong+1mH6fJ1pY3ANMgllcHbY20qSjewFbM5QLHLlcK8lMLxCjpSpBow14cBd\n\ttqm8tN5KMTL7iJ4+LQb0v+PZaMQkMcPY27WwgF6dJrtMBEzlrMn1ubO5dM1Rpvw9uO\n\tPo6aO5PMqtkpMNuGR87ma2N6/Kx77xebzQPGqX8c=","Message-ID":"","Date":"Mon, 12 Aug 2024 15:47:12 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 04/10] ipa: mali-c55: Add Mali-C55 ISP IPA module","To":"libcamera-devel@lists.libcamera.org","Cc":"Nayden Kanchev ,\n\tJacopo Mondi ","References":"<20240709144950.3277837-1-dan.scally@ideasonboard.com>\n\t<20240709144950.3277837-5-dan.scally@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20240709144950.3277837-5-dan.scally@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}}]