[v4,05/11] ipa: mali-c55: Add Mali-C55 ISP IPA module

21923 diff mbox series

Add a barebones IPA module for the Mali-C55 ISP. In this initial
implementation pretty much only buffer plumbing is implemented.

Acked-by: Nayden Kanchev  <nayden.kanchev@arm.com>
Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v4:

	- None

Changes in v3:

	- Fixed some parameter naming inconsistency

Changes in v2:

	- None

 Documentation/Doxyfile-common.in        |   1 +
 include/libcamera/ipa/mali-c55.mojom    |  34 +++
 include/libcamera/ipa/meson.build       |   1 +
 meson_options.txt                       |   2 +-
 src/ipa/mali-c55/algorithms/algorithm.h |  39 +++
 src/ipa/mali-c55/algorithms/meson.build |   4 +
 src/ipa/mali-c55/data/meson.build       |   8 +
 src/ipa/mali-c55/data/uncalibrated.yaml |   6 +
 src/ipa/mali-c55/ipa_context.cpp        | 101 +++++++
 src/ipa/mali-c55/ipa_context.h          |  42 +++
 src/ipa/mali-c55/mali-c55.cpp           | 339 ++++++++++++++++++++++++
 src/ipa/mali-c55/meson.build            |  33 +++
 src/ipa/mali-c55/module.h               |  27 ++
 13 files changed, 636 insertions(+), 1 deletion(-)
 create mode 100644 include/libcamera/ipa/mali-c55.mojom
 create mode 100644 src/ipa/mali-c55/algorithms/algorithm.h
 create mode 100644 src/ipa/mali-c55/algorithms/meson.build
 create mode 100644 src/ipa/mali-c55/data/meson.build
 create mode 100644 src/ipa/mali-c55/data/uncalibrated.yaml
 create mode 100644 src/ipa/mali-c55/ipa_context.cpp
 create mode 100644 src/ipa/mali-c55/ipa_context.h
 create mode 100644 src/ipa/mali-c55/mali-c55.cpp
 create mode 100644 src/ipa/mali-c55/meson.build
 create mode 100644 src/ipa/mali-c55/module.h

Quoting Daniel Scally (2024-11-15 12:25:34)
> Add a barebones IPA module for the Mali-C55 ISP. In this initial
> implementation pretty much only buffer plumbing is implemented.
> 
> Acked-by: Nayden Kanchev  <nayden.kanchev@arm.com>
> Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v4:
> 
>         - None
> 
> Changes in v3:
> 
>         - Fixed some parameter naming inconsistency
> 
> Changes in v2:
> 
>         - None
> 
>  Documentation/Doxyfile-common.in        |   1 +
>  include/libcamera/ipa/mali-c55.mojom    |  34 +++
>  include/libcamera/ipa/meson.build       |   1 +
>  meson_options.txt                       |   2 +-
>  src/ipa/mali-c55/algorithms/algorithm.h |  39 +++
>  src/ipa/mali-c55/algorithms/meson.build |   4 +
>  src/ipa/mali-c55/data/meson.build       |   8 +
>  src/ipa/mali-c55/data/uncalibrated.yaml |   6 +
>  src/ipa/mali-c55/ipa_context.cpp        | 101 +++++++
>  src/ipa/mali-c55/ipa_context.h          |  42 +++
>  src/ipa/mali-c55/mali-c55.cpp           | 339 ++++++++++++++++++++++++
>  src/ipa/mali-c55/meson.build            |  33 +++
>  src/ipa/mali-c55/module.h               |  27 ++
>  13 files changed, 636 insertions(+), 1 deletion(-)
>  create mode 100644 include/libcamera/ipa/mali-c55.mojom
>  create mode 100644 src/ipa/mali-c55/algorithms/algorithm.h
>  create mode 100644 src/ipa/mali-c55/algorithms/meson.build
>  create mode 100644 src/ipa/mali-c55/data/meson.build
>  create mode 100644 src/ipa/mali-c55/data/uncalibrated.yaml
>  create mode 100644 src/ipa/mali-c55/ipa_context.cpp
>  create mode 100644 src/ipa/mali-c55/ipa_context.h
>  create mode 100644 src/ipa/mali-c55/mali-c55.cpp
>  create mode 100644 src/ipa/mali-c55/meson.build
>  create mode 100644 src/ipa/mali-c55/module.h
> 
> diff --git a/Documentation/Doxyfile-common.in b/Documentation/Doxyfile-common.in
> index a70aee43..045c19dd 100644
> --- a/Documentation/Doxyfile-common.in
> +++ b/Documentation/Doxyfile-common.in
> @@ -32,6 +32,7 @@ RECURSIVE              = YES
>  EXCLUDE_PATTERNS       = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
>                           @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h \
>                           @TOP_BUILDDIR@/include/libcamera/ipa/ipu3_*.h \
> +                         @TOP_BUILDDIR@/include/libcamera/ipa/mali-c55_*.h \
>                           @TOP_BUILDDIR@/include/libcamera/ipa/raspberrypi_*.h \
>                           @TOP_BUILDDIR@/include/libcamera/ipa/rkisp1_*.h \
>                           @TOP_BUILDDIR@/include/libcamera/ipa/vimc_*.h
> diff --git a/include/libcamera/ipa/mali-c55.mojom b/include/libcamera/ipa/mali-c55.mojom
> new file mode 100644
> index 00000000..5d7eb4ee
> --- /dev/null
> +++ b/include/libcamera/ipa/mali-c55.mojom
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +
> +module ipa.mali_c55;
> +
> +import "include/libcamera/ipa/core.mojom";
> +
> +struct IPAConfigInfo {
> +       libcamera.IPACameraSensorInfo sensorInfo;
> +       libcamera.ControlInfoMap sensorControls;
> +};
> +
> +interface IPAMaliC55Interface {
> +       init(libcamera.IPASettings settings, IPAConfigInfo configInfo)
> +               => (int32 ret, libcamera.ControlInfoMap ipaControls);
> +       start() => (int32 ret);
> +       stop();
> +
> +       configure(IPAConfigInfo configInfo, uint8 bayerOrder)
> +               => (int32 ret, libcamera.ControlInfoMap ipaControls);
> +
> +       mapBuffers(array<libcamera.IPABuffer> buffers, bool readOnly);
> +       unmapBuffers(array<libcamera.IPABuffer> buffers);
> +
> +       [async] queueRequest(uint32 request, libcamera.ControlList reqControls);
> +       [async] fillParams(uint32 request, uint32 bufferId);
> +       [async] processStats(uint32 request, uint32 bufferId,
> +                            libcamera.ControlList sensorControls);
> +};
> +
> +interface IPAMaliC55EventInterface {
> +       paramsComputed(uint32 request);
> +       statsProcessed(uint32 request, libcamera.ControlList metadata);
> +       setSensorControls(libcamera.ControlList sensorControls);
> +};
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index bf55e124..3129f119 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -64,6 +64,7 @@ libcamera_ipa_headers += custom_target('core_ipa_serializer_h',
>  # Mapping from pipeline handler name to mojom file
>  pipeline_ipa_mojom_mapping = {
>      'ipu3': 'ipu3.mojom',
> +    'mali-c55': 'mali-c55.mojom',
>      'rkisp1': 'rkisp1.mojom',
>      'rpi/vc4': 'raspberrypi.mojom',
>      'simple': 'soft.mojom',
> diff --git a/meson_options.txt b/meson_options.txt
> index 7aa41249..5365601c 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -32,7 +32,7 @@ option('gstreamer',
>  
>  option('ipas',
>          type : 'array',
> -        choices : ['ipu3', 'rkisp1', 'rpi/vc4', 'simple', 'vimc'],
> +        choices : ['ipu3', 'mali-c55', 'rkisp1', 'rpi/vc4', 'simple', 'vimc'],
>          description : 'Select which IPA modules to build')
>  
>  option('lc-compliance',
> diff --git a/src/ipa/mali-c55/algorithms/algorithm.h b/src/ipa/mali-c55/algorithms/algorithm.h
> new file mode 100644
> index 00000000..36a3bff0
> --- /dev/null
> +++ b/src/ipa/mali-c55/algorithms/algorithm.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * algorithm.h - Mali-C55 control algorithm interface
> + */
> +
> +#pragma once
> +
> +#include <linux/mali-c55-config.h>
> +
> +#include <libipa/algorithm.h>
> +
> +#include "module.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::mali_c55 {
> +
> +class Algorithm : public libcamera::ipa::Algorithm<Module>
> +{
> +};
> +
> +union mali_c55_params_block {
> +       struct mali_c55_params_block_header *header;
> +       struct mali_c55_params_sensor_off_preshading *sensor_offs;
> +       struct mali_c55_params_aexp_hist *aexp_hist;
> +       struct mali_c55_params_aexp_weights *aexp_weights;
> +       struct mali_c55_params_digital_gain *digital_gain;
> +       struct mali_c55_params_awb_gains *awb_gains;
> +       struct mali_c55_params_awb_config *awb_config;
> +       struct mali_c55_params_mesh_shading_config *shading_config;
> +       struct mali_c55_params_mesh_shading_selection *shading_selection;
> +       __u8 *data;
> +};
> +
> +} /* namespace ipa::mali_c55 */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/mali-c55/algorithms/meson.build b/src/ipa/mali-c55/algorithms/meson.build
> new file mode 100644
> index 00000000..f2203b15
> --- /dev/null
> +++ b/src/ipa/mali-c55/algorithms/meson.build
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +mali_c55_ipa_algorithms = files([
> +])
> diff --git a/src/ipa/mali-c55/data/meson.build b/src/ipa/mali-c55/data/meson.build
> new file mode 100644
> index 00000000..88109d17
> --- /dev/null
> +++ b/src/ipa/mali-c55/data/meson.build
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +conf_files = files([
> +    'uncalibrated.yaml'
> +])
> +
> +install_data(conf_files,
> +             install_dir : ipa_data_dir / 'mali-c55')
> diff --git a/src/ipa/mali-c55/data/uncalibrated.yaml b/src/ipa/mali-c55/data/uncalibrated.yaml
> new file mode 100644
> index 00000000..2cdc39a8
> --- /dev/null
> +++ b/src/ipa/mali-c55/data/uncalibrated.yaml
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: CC0-1.0
> +%YAML 1.1
> +---
> +version: 1
> +algorithms:
> +...
> diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
> new file mode 100644
> index 00000000..99f76ecd
> --- /dev/null
> +++ b/src/ipa/mali-c55/ipa_context.cpp
> @@ -0,0 +1,101 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * ipa_context.cpp - MaliC55 IPA Context
> + */
> +
> +#include "ipa_context.h"
> +
> +/**
> + * \file ipa_context.h
> + * \brief Context and state information shared between the algorithms
> + */
> +
> +namespace libcamera::ipa::mali_c55 {
> +
> +/**
> + * \struct IPASessionConfiguration
> + * \brief Session configuration for the IPA module
> + *
> + * The session configuration contains all IPA configuration parameters that
> + * remain constant during the capture session, from IPA module start to stop.
> + * It is typically set during the configure() operation of the IPA module, but
> + * may also be updated in the start() operation.
> + */
> +
> +/**
> + * \struct IPAActiveState
> + * \brief Active state for algorithms
> + *
> + * The active state contains all algorithm-specific data that needs to be
> + * maintained by algorithms across frames. Unlike the session configuration,
> + * the active state is mutable and constantly updated by algorithms. The active
> + * state is accessible through the IPAContext structure.
> + *
> + * The active state stores two distinct categories of information:
> + *
> + *  - The consolidated value of all algorithm controls. Requests passed to
> + *    the queueRequest() function store values for controls that the
> + *    application wants to modify for that particular frame, and the
> + *    queueRequest() function updates the active state with those values.
> + *    The active state thus contains a consolidated view of the value of all
> + *    controls handled by the algorithm.
> + *
> + *  - The value of parameters computed by the algorithm when running in auto
> + *    mode. Algorithms running in auto mode compute new parameters every
> + *    time statistics buffers are received (either synchronously, or
> + *    possibly in a background thread). The latest computed value of those
> + *    parameters is stored in the active state in the process() function.
> + *
> + * Each of the members in the active state belongs to a specific algorithm. A
> + * member may be read by any algorithm, but shall only be written by its owner.
> + */
> +
> +/**
> + * \struct IPAFrameContext
> + * \brief Per-frame context for algorithms
> + *
> + * The frame context stores two distinct categories of information:
> + *
> + * - The value of the controls to be applied to the frame. These values are
> + *   typically set in the queueRequest() function, from the consolidated
> + *   control values stored in the active state. The frame context thus stores
> + *   values for all controls related to the algorithm, not limited to the
> + *   controls specified in the corresponding request, but consolidated from all
> + *   requests that have been queued so far.
> + *
> + *   For controls that can be set manually or computed by an algorithm
> + *   (depending on the algorithm operation mode), such as for instance the
> + *   colour gains for the AWB algorithm, the control value will be stored in
> + *   the frame context in the queueRequest() function only when operating in
> + *   manual mode. When operating in auto mode, the values are computed by the
> + *   algorithm in process(), stored in the active state, and copied to the
> + *   frame context in prepare(), just before being stored in the ISP parameters
> + *   buffer.
> + *
> + *   The queueRequest() function can also store ancillary data in the frame
> + *   context, such as flags to indicate if (and what) control values have
> + *   changed compared to the previous request.
> + *
> + * - Status information computed by the algorithm for a frame. For instance,
> + *   the colour temperature estimated by the AWB algorithm from ISP statistics
> + *   calculated on a frame is stored in the frame context for that frame in
> + *   the process() function.
> + */
> +
> +/**
> + * \struct IPAContext
> + * \brief Global IPA context data shared between all algorithms
> + *
> + * \var IPAContext::configuration
> + * \brief The IPA session configuration, immutable during the session
> + *
> + * \var IPAContext::activeState
> + * \brief The IPA active state, storing the latest state for all algorithms
> + *
> + * \var IPAContext::frameContexts
> + * \brief Ring buffer of per-frame contexts
> + */
> +
> +} /* namespace libcamera::ipa::mali_c55 */
> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
> new file mode 100644
> index 00000000..9e408a17
> --- /dev/null
> +++ b/src/ipa/mali-c55/ipa_context.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * ipa_context.h - Mali-C55 IPA Context
> + */
> +
> +#pragma once
> +
> +#include <libcamera/controls.h>
> +
> +#include <libipa/fc_queue.h>
> +
> +namespace libcamera {
> +
> +namespace ipa::mali_c55 {
> +
> +struct IPASessionConfiguration {
> +};
> +
> +struct IPAActiveState {
> +};
> +
> +struct IPAFrameContext : public FrameContext {
> +       struct {
> +               uint32_t exposure;
> +               double sensorGain;
> +       } agc;
> +};
> +
> +struct IPAContext {
> +       IPASessionConfiguration configuration;
> +       IPAActiveState activeState;
> +
> +       FCQueue<IPAFrameContext> frameContexts;
> +
> +       ControlInfoMap::Map ctrlMap;
> +};
> +
> +} /* namespace ipa::mali_c55 */
> +
> +} /* namespace libcamera*/
> diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
> new file mode 100644
> index 00000000..7efc0124
> --- /dev/null
> +++ b/src/ipa/mali-c55/mali-c55.cpp
> @@ -0,0 +1,339 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2023, Ideas on Board Oy
> + *
> + * mali-c55.cpp - Mali-C55 ISP image processing algorithms
> + */
> +
> +#include <map>
> +#include <string.h>
> +#include <vector>
> +
> +#include <linux/mali-c55-config.h>
> +#include <linux/v4l2-controls.h>
> +
> +#include <libcamera/base/file.h>
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/control_ids.h>
> +#include <libcamera/ipa/ipa_interface.h>
> +#include <libcamera/ipa/ipa_module_info.h>
> +#include <libcamera/ipa/mali-c55_ipa_interface.h>
> +
> +#include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/mapped_framebuffer.h"
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "algorithms/algorithm.h"
> +#include "libipa/camera_sensor_helper.h"
> +
> +#include "ipa_context.h"
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(IPAMaliC55)
> +
> +namespace ipa::mali_c55 {
> +
> +/* Maximum number of frame contexts to be held */
> +static constexpr uint32_t kMaxFrameContexts = 16;
> +
> +class IPAMaliC55 : public IPAMaliC55Interface, public Module
> +{
> +public:
> +       IPAMaliC55();
> +
> +       int init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
> +                ControlInfoMap *ipaControls) override;
> +       int start() override;
> +       void stop() override;
> +       int configure(const IPAConfigInfo &ipaConfig, uint8_t bayerOrder,
> +                     ControlInfoMap *ipaControls) override;
> +       void mapBuffers(const std::vector<IPABuffer> &buffers, bool readOnly) override;
> +       void unmapBuffers(const std::vector<IPABuffer> &buffers) override;
> +       void queueRequest(const uint32_t request, const ControlList &controls) override;
> +       void fillParams(unsigned int request, uint32_t bufferId) override;
> +       void processStats(unsigned int request, unsigned int bufferId,
> +                         const ControlList &sensorControls) override;
> +
> +protected:
> +       std::string logPrefix() const override;
> +
> +private:
> +       void updateControls(const IPACameraSensorInfo &sensorInfo,
> +                           const ControlInfoMap &sensorControls,
> +                           ControlInfoMap *ipaControls);
> +       void setControls();
> +
> +       std::map<unsigned int, MappedFrameBuffer> buffers_;
> +
> +       ControlInfoMap sensorControls_;
> +
> +       /* Interface to the Camera Helper */
> +       std::unique_ptr<CameraSensorHelper> camHelper_;
> +
> +       /* Local parameter storage */
> +       struct IPAContext context_;
> +};
> +
> +namespace {
> +
> +} /* namespace */
> +
> +IPAMaliC55::IPAMaliC55()
> +       : context_({ {}, {}, { kMaxFrameContexts }, {} })
> +{
> +}

This is the only quibble I'd have (but can be updated on top if you
prefer, if we don't need another version).


See https://patchwork.libcamera.org/patch/22093/


> +
> +std::string IPAMaliC55::logPrefix() const
> +{
> +       return "mali-c55";
> +}
> +
> +int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
> +                    ControlInfoMap *ipaControls)
> +{
> +       camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
> +       if (!camHelper_) {
> +               LOG(IPAMaliC55, Error)
> +                       << "Failed to create camera sensor helper for "
> +                       << settings.sensorModel;
> +               return -ENODEV;
> +       }
> +
> +       File file(settings.configurationFile);
> +       if (!file.open(File::OpenModeFlag::ReadOnly)) {
> +               int ret = file.error();
> +               LOG(IPAMaliC55, Error)
> +                       << "Failed to open configuration file "
> +                       << settings.configurationFile << ": " << strerror(-ret);
> +               return ret;
> +       }
> +
> +       std::unique_ptr<libcamera::YamlObject> data = YamlParser::parse(file);
> +       if (!data)
> +               return -EINVAL;
> +
> +       if (!data->contains("algorithms")) {
> +               LOG(IPAMaliC55, Error)
> +                       << "Tuning file doesn't contain any algorithm";
> +               return -EINVAL;
> +       }
> +
> +       int ret = createAlgorithms(context_, (*data)["algorithms"]);
> +       if (ret)
> +               return ret;
> +
> +       updateControls(ipaConfig.sensorInfo, ipaConfig.sensorControls, ipaControls);
> +
> +       return 0;
> +}
> +
> +void IPAMaliC55::setControls()
> +{
> +       ControlList ctrls(sensorControls_);
> +
> +       setSensorControls.emit(ctrls);
> +}
> +
> +int IPAMaliC55::start()
> +{
> +       return 0;
> +}
> +
> +void IPAMaliC55::stop()
> +{
> +       context_.frameContexts.clear();
> +}
> +
> +void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
> +                               const ControlInfoMap &sensorControls,
> +                               ControlInfoMap *ipaControls)
> +{
> +       ControlInfoMap::Map ctrlMap;
> +
> +       /*
> +        * Compute the frame duration limits.
> +        *
> +        * The frame length is computed assuming a fixed line length combined
> +        * with the vertical frame sizes.
> +        */
> +       const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
> +       uint32_t hblank = v4l2HBlank.def().get<int32_t>();
> +       uint32_t lineLength = sensorInfo.outputSize.width + hblank;
> +
> +       const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
> +       std::array<uint32_t, 3> frameHeights{
> +               v4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,
> +               v4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,
> +               v4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,
> +       };
> +
> +       std::array<int64_t, 3> frameDurations;
> +       for (unsigned int i = 0; i < frameHeights.size(); ++i) {
> +               uint64_t frameSize = lineLength * frameHeights[i];
> +               frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);
> +       }
> +
> +       ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
> +                                                             frameDurations[1],
> +                                                             frameDurations[2]);
> +
> +       /*
> +        * Compute exposure time limits from the V4L2_CID_EXPOSURE control
> +        * limits and the line duration.
> +        */
> +       double lineDuration = sensorInfo.minLineLength / sensorInfo.pixelRate;
> +
> +       const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
> +       int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;
> +       int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;
> +       int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;
> +       ctrlMap[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, defExposure);
> +
> +       /* Compute the analogue gain limits. */
> +       const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
> +       float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
> +       float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
> +       float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
> +       ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);

If all of that is the same as every other libipa IPA then I think theres
some opportunity to refactor into a helper here, or otherwise into the
common AEGC components ... but can be on top still.


> +
> +       /*
> +        * Merge in any controls that we support either statically or from the
> +        * algorithms.
> +        */
> +       ctrlMap.merge(context_.ctrlMap);
> +
> +       *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
> +}
> +
> +int IPAMaliC55::configure(const IPAConfigInfo &ipaConfig,
> +                         [[maybe_unused]] uint8_t bayerOrder,
> +                         ControlInfoMap *ipaControls)
> +{
> +       sensorControls_ = ipaConfig.sensorControls;
> +
> +       /* Clear the IPA context before the streaming session. */
> +       context_.configuration = {};
> +       context_.activeState = {};
> +       context_.frameContexts.clear();
> +
> +       const IPACameraSensorInfo &info = ipaConfig.sensorInfo;
> +
> +       updateControls(info, ipaConfig.sensorControls, ipaControls);
> +
> +       for (auto const &a : algorithms()) {
> +               Algorithm *algo = static_cast<Algorithm *>(a.get());
> +
> +               int ret = algo->configure(context_, info);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +void IPAMaliC55::mapBuffers(const std::vector<IPABuffer> &buffers, bool readOnly)
> +{
> +       for (const IPABuffer &buffer : buffers) {
> +               const FrameBuffer fb(buffer.planes);

I think the MappedFrameBuffer should probably be split out here and
error checked at time of mapping, to at least report a warning/Error
message, but we're in a void function ... hrm.


Probably something to add to a todo list ?

But we don't expect these to fail...


Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +               buffers_.emplace(
> +                       buffer.id,
> +                       MappedFrameBuffer(
> +                               &fb,
> +                               readOnly ? MappedFrameBuffer::MapFlag::Read
> +                                        : MappedFrameBuffer::MapFlag::ReadWrite));
> +       }
> +}
> +
> +void IPAMaliC55::unmapBuffers(const std::vector<IPABuffer> &buffers)
> +{
> +       for (const IPABuffer &buffer : buffers) {
> +               auto it = buffers_.find(buffer.id);
> +               if (it == buffers_.end())
> +                       continue;
> +
> +               buffers_.erase(buffer.id);
> +       }
> +}
> +
> +void IPAMaliC55::queueRequest(const uint32_t request, const ControlList &controls)
> +{
> +       IPAFrameContext &frameContext = context_.frameContexts.alloc(request);
> +
> +       for (auto const &a : algorithms()) {
> +               Algorithm *algo = static_cast<Algorithm *>(a.get());
> +
> +               algo->queueRequest(context_, request, frameContext, controls);
> +       }
> +}
> +
> +void IPAMaliC55::fillParams(unsigned int request,
> +                           [[maybe_unused]] uint32_t bufferId)
> +{
> +       struct mali_c55_params_buffer *params;
> +       IPAFrameContext &frameContext = context_.frameContexts.get(request);
> +
> +       params = reinterpret_cast<mali_c55_params_buffer *>(
> +               buffers_.at(bufferId).planes()[0].data());
> +       memset(params, 0, sizeof(mali_c55_params_buffer));
> +
> +       params->version = MALI_C55_PARAM_BUFFER_V1;
> +
> +       for (auto const &algo : algorithms()) {
> +               algo->prepare(context_, request, frameContext, params);
> +
> +               ASSERT(params->total_size <= MALI_C55_PARAMS_MAX_SIZE);
> +       }
> +
> +       paramsComputed.emit(request);
> +}
> +
> +void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
> +                             const ControlList &sensorControls)
> +{
> +       IPAFrameContext &frameContext = context_.frameContexts.get(request);
> +       const mali_c55_stats_buffer *stats = nullptr;
> +
> +       stats = reinterpret_cast<mali_c55_stats_buffer *>(
> +               buffers_.at(bufferId).planes()[0].data());
> +
> +       frameContext.agc.exposure =
> +               sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> +       frameContext.agc.sensorGain =
> +               camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +
> +       ControlList metadata(controls::controls);
> +
> +       for (auto const &a : algorithms()) {
> +               Algorithm *algo = static_cast<Algorithm *>(a.get());
> +
> +               algo->process(context_, request, frameContext, stats, metadata);
> +       }
> +
> +       setControls();
> +
> +       statsProcessed.emit(request, metadata);
> +}
> +
> +} /* namespace ipa::mali_c55 */
> +
> +/*
> + * External IPA module interface
> + */
> +extern "C" {
> +const struct IPAModuleInfo ipaModuleInfo = {
> +       IPA_MODULE_API_VERSION,
> +       1,
> +       "mali-c55",
> +       "mali-c55",
> +};
> +
> +IPAInterface *ipaCreate()
> +{
> +       return new ipa::mali_c55::IPAMaliC55();
> +}
> +
> +} /* extern "C" */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/mali-c55/meson.build b/src/ipa/mali-c55/meson.build
> new file mode 100644
> index 00000000..864d90ec
> --- /dev/null
> +++ b/src/ipa/mali-c55/meson.build
> @@ -0,0 +1,33 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +subdir('algorithms')
> +subdir('data')
> +
> +ipa_name = 'ipa_mali_c55'
> +
> +mali_c55_ipa_sources = files([
> +    'ipa_context.cpp',
> +    'mali-c55.cpp'
> +])
> +
> +mali_c55_ipa_sources += mali_c55_ipa_algorithms
> +
> +mod = shared_module(ipa_name,
> +                    mali_c55_ipa_sources,
> +                    name_prefix : '',
> +                    include_directories : [ipa_includes, libipa_includes],
> +                    dependencies : libcamera_private,
> +                    link_with : libipa,
> +                    install : true,
> +                    install_dir : ipa_install_dir)
> +
> +if ipa_sign_module
> +    custom_target(ipa_name + '.so.sign',
> +                  input : mod,
> +                  output : ipa_name + '.so.sign',
> +                  command : [ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@'],
> +                  install : false,
> +                  build_by_default : true)
> +endif
> +
> +ipa_names += ipa_name
> diff --git a/src/ipa/mali-c55/module.h b/src/ipa/mali-c55/module.h
> new file mode 100644
> index 00000000..1d85ec1f
> --- /dev/null
> +++ b/src/ipa/mali-c55/module.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * module.h - Mali-C55 IPA Module
> + */
> +
> +#pragma once
> +
> +#include <linux/mali-c55-config.h>
> +
> +#include <libcamera/ipa/mali-c55_ipa_interface.h>
> +
> +#include <libipa/module.h>
> +
> +#include "ipa_context.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::mali_c55 {
> +
> +using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo,
> +                          mali_c55_params_buffer, mali_c55_stats_buffer>;
> +
> +} /* namespace ipa::mali_c55 */
> +
> +} /* namespace libcamera*/
> -- 
> 2.30.2
>

Hi Kieran - thanks for the review

On 06/12/2024 13:51, Kieran Bingham wrote:
> Quoting Daniel Scally (2024-11-15 12:25:34)
>> Add a barebones IPA module for the Mali-C55 ISP. In this initial
>> implementation pretty much only buffer plumbing is implemented.
>>
>> Acked-by: Nayden Kanchev  <nayden.kanchev@arm.com>
>> Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>> Changes in v4:
>>
>>          - None
>>
>> Changes in v3:
>>
>>          - Fixed some parameter naming inconsistency
>>
>> Changes in v2:
>>
>>          - None
>>
>>   Documentation/Doxyfile-common.in        |   1 +
>>   include/libcamera/ipa/mali-c55.mojom    |  34 +++
>>   include/libcamera/ipa/meson.build       |   1 +
>>   meson_options.txt                       |   2 +-
>>   src/ipa/mali-c55/algorithms/algorithm.h |  39 +++
>>   src/ipa/mali-c55/algorithms/meson.build |   4 +
>>   src/ipa/mali-c55/data/meson.build       |   8 +
>>   src/ipa/mali-c55/data/uncalibrated.yaml |   6 +
>>   src/ipa/mali-c55/ipa_context.cpp        | 101 +++++++
>>   src/ipa/mali-c55/ipa_context.h          |  42 +++
>>   src/ipa/mali-c55/mali-c55.cpp           | 339 ++++++++++++++++++++++++
>>   src/ipa/mali-c55/meson.build            |  33 +++
>>   src/ipa/mali-c55/module.h               |  27 ++
>>   13 files changed, 636 insertions(+), 1 deletion(-)
>>   create mode 100644 include/libcamera/ipa/mali-c55.mojom
>>   create mode 100644 src/ipa/mali-c55/algorithms/algorithm.h
>>   create mode 100644 src/ipa/mali-c55/algorithms/meson.build
>>   create mode 100644 src/ipa/mali-c55/data/meson.build
>>   create mode 100644 src/ipa/mali-c55/data/uncalibrated.yaml
>>   create mode 100644 src/ipa/mali-c55/ipa_context.cpp
>>   create mode 100644 src/ipa/mali-c55/ipa_context.h
>>   create mode 100644 src/ipa/mali-c55/mali-c55.cpp
>>   create mode 100644 src/ipa/mali-c55/meson.build
>>   create mode 100644 src/ipa/mali-c55/module.h
>>
>> diff --git a/Documentation/Doxyfile-common.in b/Documentation/Doxyfile-common.in
>> index a70aee43..045c19dd 100644
>> --- a/Documentation/Doxyfile-common.in
>> +++ b/Documentation/Doxyfile-common.in
>> @@ -32,6 +32,7 @@ RECURSIVE              = YES
>>   EXCLUDE_PATTERNS       = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
>>                            @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h \
>>                            @TOP_BUILDDIR@/include/libcamera/ipa/ipu3_*.h \
>> +                         @TOP_BUILDDIR@/include/libcamera/ipa/mali-c55_*.h \
>>                            @TOP_BUILDDIR@/include/libcamera/ipa/raspberrypi_*.h \
>>                            @TOP_BUILDDIR@/include/libcamera/ipa/rkisp1_*.h \
>>                            @TOP_BUILDDIR@/include/libcamera/ipa/vimc_*.h
>> diff --git a/include/libcamera/ipa/mali-c55.mojom b/include/libcamera/ipa/mali-c55.mojom
>> new file mode 100644
>> index 00000000..5d7eb4ee
>> --- /dev/null
>> +++ b/include/libcamera/ipa/mali-c55.mojom
>> @@ -0,0 +1,34 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +
>> +module ipa.mali_c55;
>> +
>> +import "include/libcamera/ipa/core.mojom";
>> +
>> +struct IPAConfigInfo {
>> +       libcamera.IPACameraSensorInfo sensorInfo;
>> +       libcamera.ControlInfoMap sensorControls;
>> +};
>> +
>> +interface IPAMaliC55Interface {
>> +       init(libcamera.IPASettings settings, IPAConfigInfo configInfo)
>> +               => (int32 ret, libcamera.ControlInfoMap ipaControls);
>> +       start() => (int32 ret);
>> +       stop();
>> +
>> +       configure(IPAConfigInfo configInfo, uint8 bayerOrder)
>> +               => (int32 ret, libcamera.ControlInfoMap ipaControls);
>> +
>> +       mapBuffers(array<libcamera.IPABuffer> buffers, bool readOnly);
>> +       unmapBuffers(array<libcamera.IPABuffer> buffers);
>> +
>> +       [async] queueRequest(uint32 request, libcamera.ControlList reqControls);
>> +       [async] fillParams(uint32 request, uint32 bufferId);
>> +       [async] processStats(uint32 request, uint32 bufferId,
>> +                            libcamera.ControlList sensorControls);
>> +};
>> +
>> +interface IPAMaliC55EventInterface {
>> +       paramsComputed(uint32 request);
>> +       statsProcessed(uint32 request, libcamera.ControlList metadata);
>> +       setSensorControls(libcamera.ControlList sensorControls);
>> +};
>> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
>> index bf55e124..3129f119 100644
>> --- a/include/libcamera/ipa/meson.build
>> +++ b/include/libcamera/ipa/meson.build
>> @@ -64,6 +64,7 @@ libcamera_ipa_headers += custom_target('core_ipa_serializer_h',
>>   # Mapping from pipeline handler name to mojom file
>>   pipeline_ipa_mojom_mapping = {
>>       'ipu3': 'ipu3.mojom',
>> +    'mali-c55': 'mali-c55.mojom',
>>       'rkisp1': 'rkisp1.mojom',
>>       'rpi/vc4': 'raspberrypi.mojom',
>>       'simple': 'soft.mojom',
>> diff --git a/meson_options.txt b/meson_options.txt
>> index 7aa41249..5365601c 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -32,7 +32,7 @@ option('gstreamer',
>>   
>>   option('ipas',
>>           type : 'array',
>> -        choices : ['ipu3', 'rkisp1', 'rpi/vc4', 'simple', 'vimc'],
>> +        choices : ['ipu3', 'mali-c55', 'rkisp1', 'rpi/vc4', 'simple', 'vimc'],
>>           description : 'Select which IPA modules to build')
>>   
>>   option('lc-compliance',
>> diff --git a/src/ipa/mali-c55/algorithms/algorithm.h b/src/ipa/mali-c55/algorithms/algorithm.h
>> new file mode 100644
>> index 00000000..36a3bff0
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/algorithms/algorithm.h
>> @@ -0,0 +1,39 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024, Ideas On Board
>> + *
>> + * algorithm.h - Mali-C55 control algorithm interface
>> + */
>> +
>> +#pragma once
>> +
>> +#include <linux/mali-c55-config.h>
>> +
>> +#include <libipa/algorithm.h>
>> +
>> +#include "module.h"
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::mali_c55 {
>> +
>> +class Algorithm : public libcamera::ipa::Algorithm<Module>
>> +{
>> +};
>> +
>> +union mali_c55_params_block {
>> +       struct mali_c55_params_block_header *header;
>> +       struct mali_c55_params_sensor_off_preshading *sensor_offs;
>> +       struct mali_c55_params_aexp_hist *aexp_hist;
>> +       struct mali_c55_params_aexp_weights *aexp_weights;
>> +       struct mali_c55_params_digital_gain *digital_gain;
>> +       struct mali_c55_params_awb_gains *awb_gains;
>> +       struct mali_c55_params_awb_config *awb_config;
>> +       struct mali_c55_params_mesh_shading_config *shading_config;
>> +       struct mali_c55_params_mesh_shading_selection *shading_selection;
>> +       __u8 *data;
>> +};
>> +
>> +} /* namespace ipa::mali_c55 */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/mali-c55/algorithms/meson.build b/src/ipa/mali-c55/algorithms/meson.build
>> new file mode 100644
>> index 00000000..f2203b15
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/algorithms/meson.build
>> @@ -0,0 +1,4 @@
>> +# SPDX-License-Identifier: CC0-1.0
>> +
>> +mali_c55_ipa_algorithms = files([
>> +])
>> diff --git a/src/ipa/mali-c55/data/meson.build b/src/ipa/mali-c55/data/meson.build
>> new file mode 100644
>> index 00000000..88109d17
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/data/meson.build
>> @@ -0,0 +1,8 @@
>> +# SPDX-License-Identifier: CC0-1.0
>> +
>> +conf_files = files([
>> +    'uncalibrated.yaml'
>> +])
>> +
>> +install_data(conf_files,
>> +             install_dir : ipa_data_dir / 'mali-c55')
>> diff --git a/src/ipa/mali-c55/data/uncalibrated.yaml b/src/ipa/mali-c55/data/uncalibrated.yaml
>> new file mode 100644
>> index 00000000..2cdc39a8
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/data/uncalibrated.yaml
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: CC0-1.0
>> +%YAML 1.1
>> +---
>> +version: 1
>> +algorithms:
>> +...
>> diff --git a/src/ipa/mali-c55/ipa_context.cpp b/src/ipa/mali-c55/ipa_context.cpp
>> new file mode 100644
>> index 00000000..99f76ecd
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/ipa_context.cpp
>> @@ -0,0 +1,101 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024, Ideas On Board
>> + *
>> + * ipa_context.cpp - MaliC55 IPA Context
>> + */
>> +
>> +#include "ipa_context.h"
>> +
>> +/**
>> + * \file ipa_context.h
>> + * \brief Context and state information shared between the algorithms
>> + */
>> +
>> +namespace libcamera::ipa::mali_c55 {
>> +
>> +/**
>> + * \struct IPASessionConfiguration
>> + * \brief Session configuration for the IPA module
>> + *
>> + * The session configuration contains all IPA configuration parameters that
>> + * remain constant during the capture session, from IPA module start to stop.
>> + * It is typically set during the configure() operation of the IPA module, but
>> + * may also be updated in the start() operation.
>> + */
>> +
>> +/**
>> + * \struct IPAActiveState
>> + * \brief Active state for algorithms
>> + *
>> + * The active state contains all algorithm-specific data that needs to be
>> + * maintained by algorithms across frames. Unlike the session configuration,
>> + * the active state is mutable and constantly updated by algorithms. The active
>> + * state is accessible through the IPAContext structure.
>> + *
>> + * The active state stores two distinct categories of information:
>> + *
>> + *  - The consolidated value of all algorithm controls. Requests passed to
>> + *    the queueRequest() function store values for controls that the
>> + *    application wants to modify for that particular frame, and the
>> + *    queueRequest() function updates the active state with those values.
>> + *    The active state thus contains a consolidated view of the value of all
>> + *    controls handled by the algorithm.
>> + *
>> + *  - The value of parameters computed by the algorithm when running in auto
>> + *    mode. Algorithms running in auto mode compute new parameters every
>> + *    time statistics buffers are received (either synchronously, or
>> + *    possibly in a background thread). The latest computed value of those
>> + *    parameters is stored in the active state in the process() function.
>> + *
>> + * Each of the members in the active state belongs to a specific algorithm. A
>> + * member may be read by any algorithm, but shall only be written by its owner.
>> + */
>> +
>> +/**
>> + * \struct IPAFrameContext
>> + * \brief Per-frame context for algorithms
>> + *
>> + * The frame context stores two distinct categories of information:
>> + *
>> + * - The value of the controls to be applied to the frame. These values are
>> + *   typically set in the queueRequest() function, from the consolidated
>> + *   control values stored in the active state. The frame context thus stores
>> + *   values for all controls related to the algorithm, not limited to the
>> + *   controls specified in the corresponding request, but consolidated from all
>> + *   requests that have been queued so far.
>> + *
>> + *   For controls that can be set manually or computed by an algorithm
>> + *   (depending on the algorithm operation mode), such as for instance the
>> + *   colour gains for the AWB algorithm, the control value will be stored in
>> + *   the frame context in the queueRequest() function only when operating in
>> + *   manual mode. When operating in auto mode, the values are computed by the
>> + *   algorithm in process(), stored in the active state, and copied to the
>> + *   frame context in prepare(), just before being stored in the ISP parameters
>> + *   buffer.
>> + *
>> + *   The queueRequest() function can also store ancillary data in the frame
>> + *   context, such as flags to indicate if (and what) control values have
>> + *   changed compared to the previous request.
>> + *
>> + * - Status information computed by the algorithm for a frame. For instance,
>> + *   the colour temperature estimated by the AWB algorithm from ISP statistics
>> + *   calculated on a frame is stored in the frame context for that frame in
>> + *   the process() function.
>> + */
>> +
>> +/**
>> + * \struct IPAContext
>> + * \brief Global IPA context data shared between all algorithms
>> + *
>> + * \var IPAContext::configuration
>> + * \brief The IPA session configuration, immutable during the session
>> + *
>> + * \var IPAContext::activeState
>> + * \brief The IPA active state, storing the latest state for all algorithms
>> + *
>> + * \var IPAContext::frameContexts
>> + * \brief Ring buffer of per-frame contexts
>> + */
>> +
>> +} /* namespace libcamera::ipa::mali_c55 */
>> diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h
>> new file mode 100644
>> index 00000000..9e408a17
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/ipa_context.h
>> @@ -0,0 +1,42 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024, Ideas On Board
>> + *
>> + * ipa_context.h - Mali-C55 IPA Context
>> + */
>> +
>> +#pragma once
>> +
>> +#include <libcamera/controls.h>
>> +
>> +#include <libipa/fc_queue.h>
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::mali_c55 {
>> +
>> +struct IPASessionConfiguration {
>> +};
>> +
>> +struct IPAActiveState {
>> +};
>> +
>> +struct IPAFrameContext : public FrameContext {
>> +       struct {
>> +               uint32_t exposure;
>> +               double sensorGain;
>> +       } agc;
>> +};
>> +
>> +struct IPAContext {
>> +       IPASessionConfiguration configuration;
>> +       IPAActiveState activeState;
>> +
>> +       FCQueue<IPAFrameContext> frameContexts;
>> +
>> +       ControlInfoMap::Map ctrlMap;
>> +};
>> +
>> +} /* namespace ipa::mali_c55 */
>> +
>> +} /* namespace libcamera*/
>> diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
>> new file mode 100644
>> index 00000000..7efc0124
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/mali-c55.cpp
>> @@ -0,0 +1,339 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2023, Ideas on Board Oy
>> + *
>> + * mali-c55.cpp - Mali-C55 ISP image processing algorithms
>> + */
>> +
>> +#include <map>
>> +#include <string.h>
>> +#include <vector>
>> +
>> +#include <linux/mali-c55-config.h>
>> +#include <linux/v4l2-controls.h>
>> +
>> +#include <libcamera/base/file.h>
>> +#include <libcamera/base/log.h>
>> +
>> +#include <libcamera/control_ids.h>
>> +#include <libcamera/ipa/ipa_interface.h>
>> +#include <libcamera/ipa/ipa_module_info.h>
>> +#include <libcamera/ipa/mali-c55_ipa_interface.h>
>> +
>> +#include "libcamera/internal/bayer_format.h"
>> +#include "libcamera/internal/mapped_framebuffer.h"
>> +#include "libcamera/internal/yaml_parser.h"
>> +
>> +#include "algorithms/algorithm.h"
>> +#include "libipa/camera_sensor_helper.h"
>> +
>> +#include "ipa_context.h"
>> +
>> +namespace libcamera {
>> +
>> +LOG_DEFINE_CATEGORY(IPAMaliC55)
>> +
>> +namespace ipa::mali_c55 {
>> +
>> +/* Maximum number of frame contexts to be held */
>> +static constexpr uint32_t kMaxFrameContexts = 16;
>> +
>> +class IPAMaliC55 : public IPAMaliC55Interface, public Module
>> +{
>> +public:
>> +       IPAMaliC55();
>> +
>> +       int init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
>> +                ControlInfoMap *ipaControls) override;
>> +       int start() override;
>> +       void stop() override;
>> +       int configure(const IPAConfigInfo &ipaConfig, uint8_t bayerOrder,
>> +                     ControlInfoMap *ipaControls) override;
>> +       void mapBuffers(const std::vector<IPABuffer> &buffers, bool readOnly) override;
>> +       void unmapBuffers(const std::vector<IPABuffer> &buffers) override;
>> +       void queueRequest(const uint32_t request, const ControlList &controls) override;
>> +       void fillParams(unsigned int request, uint32_t bufferId) override;
>> +       void processStats(unsigned int request, unsigned int bufferId,
>> +                         const ControlList &sensorControls) override;
>> +
>> +protected:
>> +       std::string logPrefix() const override;
>> +
>> +private:
>> +       void updateControls(const IPACameraSensorInfo &sensorInfo,
>> +                           const ControlInfoMap &sensorControls,
>> +                           ControlInfoMap *ipaControls);
>> +       void setControls();
>> +
>> +       std::map<unsigned int, MappedFrameBuffer> buffers_;
>> +
>> +       ControlInfoMap sensorControls_;
>> +
>> +       /* Interface to the Camera Helper */
>> +       std::unique_ptr<CameraSensorHelper> camHelper_;
>> +
>> +       /* Local parameter storage */
>> +       struct IPAContext context_;
>> +};
>> +
>> +namespace {
>> +
>> +} /* namespace */
>> +
>> +IPAMaliC55::IPAMaliC55()
>> +       : context_({ {}, {}, { kMaxFrameContexts }, {} })
>> +{
>> +}
> This is the only quibble I'd have (but can be updated on top if you
> prefer, if we don't need another version).
>
>
> See https://patchwork.libcamera.org/patch/22093/
Yeah sure
>
>> +
>> +std::string IPAMaliC55::logPrefix() const
>> +{
>> +       return "mali-c55";
>> +}
>> +
>> +int IPAMaliC55::init(const IPASettings &settings, const IPAConfigInfo &ipaConfig,
>> +                    ControlInfoMap *ipaControls)
>> +{
>> +       camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel);
>> +       if (!camHelper_) {
>> +               LOG(IPAMaliC55, Error)
>> +                       << "Failed to create camera sensor helper for "
>> +                       << settings.sensorModel;
>> +               return -ENODEV;
>> +       }
>> +
>> +       File file(settings.configurationFile);
>> +       if (!file.open(File::OpenModeFlag::ReadOnly)) {
>> +               int ret = file.error();
>> +               LOG(IPAMaliC55, Error)
>> +                       << "Failed to open configuration file "
>> +                       << settings.configurationFile << ": " << strerror(-ret);
>> +               return ret;
>> +       }
>> +
>> +       std::unique_ptr<libcamera::YamlObject> data = YamlParser::parse(file);
>> +       if (!data)
>> +               return -EINVAL;
>> +
>> +       if (!data->contains("algorithms")) {
>> +               LOG(IPAMaliC55, Error)
>> +                       << "Tuning file doesn't contain any algorithm";
>> +               return -EINVAL;
>> +       }
>> +
>> +       int ret = createAlgorithms(context_, (*data)["algorithms"]);
>> +       if (ret)
>> +               return ret;
>> +
>> +       updateControls(ipaConfig.sensorInfo, ipaConfig.sensorControls, ipaControls);
>> +
>> +       return 0;
>> +}
>> +
>> +void IPAMaliC55::setControls()
>> +{
>> +       ControlList ctrls(sensorControls_);
>> +
>> +       setSensorControls.emit(ctrls);
>> +}
>> +
>> +int IPAMaliC55::start()
>> +{
>> +       return 0;
>> +}
>> +
>> +void IPAMaliC55::stop()
>> +{
>> +       context_.frameContexts.clear();
>> +}
>> +
>> +void IPAMaliC55::updateControls(const IPACameraSensorInfo &sensorInfo,
>> +                               const ControlInfoMap &sensorControls,
>> +                               ControlInfoMap *ipaControls)
>> +{
>> +       ControlInfoMap::Map ctrlMap;
>> +
>> +       /*
>> +        * Compute the frame duration limits.
>> +        *
>> +        * The frame length is computed assuming a fixed line length combined
>> +        * with the vertical frame sizes.
>> +        */
>> +       const ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;
>> +       uint32_t hblank = v4l2HBlank.def().get<int32_t>();
>> +       uint32_t lineLength = sensorInfo.outputSize.width + hblank;
>> +
>> +       const ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;
>> +       std::array<uint32_t, 3> frameHeights{
>> +               v4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,
>> +               v4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,
>> +               v4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,
>> +       };
>> +
>> +       std::array<int64_t, 3> frameDurations;
>> +       for (unsigned int i = 0; i < frameHeights.size(); ++i) {
>> +               uint64_t frameSize = lineLength * frameHeights[i];
>> +               frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);
>> +       }
>> +
>> +       ctrlMap[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],
>> +                                                             frameDurations[1],
>> +                                                             frameDurations[2]);
>> +
>> +       /*
>> +        * Compute exposure time limits from the V4L2_CID_EXPOSURE control
>> +        * limits and the line duration.
>> +        */
>> +       double lineDuration = sensorInfo.minLineLength / sensorInfo.pixelRate;
>> +
>> +       const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
>> +       int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;
>> +       int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;
>> +       int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;
>> +       ctrlMap[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, defExposure);
>> +
>> +       /* Compute the analogue gain limits. */
>> +       const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
>> +       float minGain = camHelper_->gain(v4l2Gain.min().get<int32_t>());
>> +       float maxGain = camHelper_->gain(v4l2Gain.max().get<int32_t>());
>> +       float defGain = camHelper_->gain(v4l2Gain.def().get<int32_t>());
>> +       ctrlMap[&controls::AnalogueGain] = ControlInfo(minGain, maxGain, defGain);
> If all of that is the same as every other libipa IPA then I think theres
> some opportunity to refactor into a helper here, or otherwise into the
> common AEGC components ... but can be on top still.
I will stick this on the to-do list
>
>
>> +
>> +       /*
>> +        * Merge in any controls that we support either statically or from the
>> +        * algorithms.
>> +        */
>> +       ctrlMap.merge(context_.ctrlMap);
>> +
>> +       *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
>> +}
>> +
>> +int IPAMaliC55::configure(const IPAConfigInfo &ipaConfig,
>> +                         [[maybe_unused]] uint8_t bayerOrder,
>> +                         ControlInfoMap *ipaControls)
>> +{
>> +       sensorControls_ = ipaConfig.sensorControls;
>> +
>> +       /* Clear the IPA context before the streaming session. */
>> +       context_.configuration = {};
>> +       context_.activeState = {};
>> +       context_.frameContexts.clear();
>> +
>> +       const IPACameraSensorInfo &info = ipaConfig.sensorInfo;
>> +
>> +       updateControls(info, ipaConfig.sensorControls, ipaControls);
>> +
>> +       for (auto const &a : algorithms()) {
>> +               Algorithm *algo = static_cast<Algorithm *>(a.get());
>> +
>> +               int ret = algo->configure(context_, info);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +void IPAMaliC55::mapBuffers(const std::vector<IPABuffer> &buffers, bool readOnly)
>> +{
>> +       for (const IPABuffer &buffer : buffers) {
>> +               const FrameBuffer fb(buffer.planes);
> I think the MappedFrameBuffer should probably be split out here and
> error checked at time of mapping, to at least report a warning/Error
> message, but we're in a void function ... hrm.
>
>
> Probably something to add to a todo list ?

Sure, it doesn't have to be a void though, we can change that. I suspect this is a similar problem 
for the other IPA modules too.
>
> But we don't expect these to fail...
>
>
> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


Thanks

>
>> +               buffers_.emplace(
>> +                       buffer.id,
>> +                       MappedFrameBuffer(
>> +                               &fb,
>> +                               readOnly ? MappedFrameBuffer::MapFlag::Read
>> +                                        : MappedFrameBuffer::MapFlag::ReadWrite));
>> +       }
>> +}
>> +
>> +void IPAMaliC55::unmapBuffers(const std::vector<IPABuffer> &buffers)
>> +{
>> +       for (const IPABuffer &buffer : buffers) {
>> +               auto it = buffers_.find(buffer.id);
>> +               if (it == buffers_.end())
>> +                       continue;
>> +
>> +               buffers_.erase(buffer.id);
>> +       }
>> +}
>> +
>> +void IPAMaliC55::queueRequest(const uint32_t request, const ControlList &controls)
>> +{
>> +       IPAFrameContext &frameContext = context_.frameContexts.alloc(request);
>> +
>> +       for (auto const &a : algorithms()) {
>> +               Algorithm *algo = static_cast<Algorithm *>(a.get());
>> +
>> +               algo->queueRequest(context_, request, frameContext, controls);
>> +       }
>> +}
>> +
>> +void IPAMaliC55::fillParams(unsigned int request,
>> +                           [[maybe_unused]] uint32_t bufferId)
>> +{
>> +       struct mali_c55_params_buffer *params;
>> +       IPAFrameContext &frameContext = context_.frameContexts.get(request);
>> +
>> +       params = reinterpret_cast<mali_c55_params_buffer *>(
>> +               buffers_.at(bufferId).planes()[0].data());
>> +       memset(params, 0, sizeof(mali_c55_params_buffer));
>> +
>> +       params->version = MALI_C55_PARAM_BUFFER_V1;
>> +
>> +       for (auto const &algo : algorithms()) {
>> +               algo->prepare(context_, request, frameContext, params);
>> +
>> +               ASSERT(params->total_size <= MALI_C55_PARAMS_MAX_SIZE);
>> +       }
>> +
>> +       paramsComputed.emit(request);
>> +}
>> +
>> +void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
>> +                             const ControlList &sensorControls)
>> +{
>> +       IPAFrameContext &frameContext = context_.frameContexts.get(request);
>> +       const mali_c55_stats_buffer *stats = nullptr;
>> +
>> +       stats = reinterpret_cast<mali_c55_stats_buffer *>(
>> +               buffers_.at(bufferId).planes()[0].data());
>> +
>> +       frameContext.agc.exposure =
>> +               sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>> +       frameContext.agc.sensorGain =
>> +               camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>> +
>> +       ControlList metadata(controls::controls);
>> +
>> +       for (auto const &a : algorithms()) {
>> +               Algorithm *algo = static_cast<Algorithm *>(a.get());
>> +
>> +               algo->process(context_, request, frameContext, stats, metadata);
>> +       }
>> +
>> +       setControls();
>> +
>> +       statsProcessed.emit(request, metadata);
>> +}
>> +
>> +} /* namespace ipa::mali_c55 */
>> +
>> +/*
>> + * External IPA module interface
>> + */
>> +extern "C" {
>> +const struct IPAModuleInfo ipaModuleInfo = {
>> +       IPA_MODULE_API_VERSION,
>> +       1,
>> +       "mali-c55",
>> +       "mali-c55",
>> +};
>> +
>> +IPAInterface *ipaCreate()
>> +{
>> +       return new ipa::mali_c55::IPAMaliC55();
>> +}
>> +
>> +} /* extern "C" */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/mali-c55/meson.build b/src/ipa/mali-c55/meson.build
>> new file mode 100644
>> index 00000000..864d90ec
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/meson.build
>> @@ -0,0 +1,33 @@
>> +# SPDX-License-Identifier: CC0-1.0
>> +
>> +subdir('algorithms')
>> +subdir('data')
>> +
>> +ipa_name = 'ipa_mali_c55'
>> +
>> +mali_c55_ipa_sources = files([
>> +    'ipa_context.cpp',
>> +    'mali-c55.cpp'
>> +])
>> +
>> +mali_c55_ipa_sources += mali_c55_ipa_algorithms
>> +
>> +mod = shared_module(ipa_name,
>> +                    mali_c55_ipa_sources,
>> +                    name_prefix : '',
>> +                    include_directories : [ipa_includes, libipa_includes],
>> +                    dependencies : libcamera_private,
>> +                    link_with : libipa,
>> +                    install : true,
>> +                    install_dir : ipa_install_dir)
>> +
>> +if ipa_sign_module
>> +    custom_target(ipa_name + '.so.sign',
>> +                  input : mod,
>> +                  output : ipa_name + '.so.sign',
>> +                  command : [ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@'],
>> +                  install : false,
>> +                  build_by_default : true)
>> +endif
>> +
>> +ipa_names += ipa_name
>> diff --git a/src/ipa/mali-c55/module.h b/src/ipa/mali-c55/module.h
>> new file mode 100644
>> index 00000000..1d85ec1f
>> --- /dev/null
>> +++ b/src/ipa/mali-c55/module.h
>> @@ -0,0 +1,27 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024, Ideas On Board
>> + *
>> + * module.h - Mali-C55 IPA Module
>> + */
>> +
>> +#pragma once
>> +
>> +#include <linux/mali-c55-config.h>
>> +
>> +#include <libcamera/ipa/mali-c55_ipa_interface.h>
>> +
>> +#include <libipa/module.h>
>> +
>> +#include "ipa_context.h"
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::mali_c55 {
>> +
>> +using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo,
>> +                          mali_c55_params_buffer, mali_c55_stats_buffer>;
>> +
>> +} /* namespace ipa::mali_c55 */
>> +
>> +} /* namespace libcamera*/
>> -- 
>> 2.30.2
>>