[v6,4/5] ipa: libipa: Introduce V4L2Params

24633 diff mbox series

The existing RkISP1Params helper class allows the RkISP1 IPA to handle
both the extensible parameters format and the legacy fixed-size format.

With the introduction of v4l2-isp.h in the Linux kernel the part of
the RkISP1Params helper class that handles extensible parameters can
be generalized so that other IPA modules can use the same helpers
to populate a parameters buffer compatible with v4l2-isp.h.

Generalize the RkISP1Params class to a new libipa component named
V4L2Params and derive the existing RkISP1Params from it, leaving
in the RkISP1-specific implementation the handling of the legacy format.

Deriving RkISP1Params from V4L2Params requires changing the size
associated to each block to include the size of v4l2_params_block_header
in the ipa:rkisp1::kBlockTypeInfo map as the V4L2Params::block()
implementation doesn't account for that as RkIS1Params::block()
implementation did.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Tested-by: Antoine Bouyer <antoine.bouyer@nxp.com>
---
 src/ipa/libipa/meson.build     |   2 +
 src/ipa/libipa/v4l2_params.cpp | 254 +++++++++++++++++++++++++++++++++++++++++
 src/ipa/libipa/v4l2_params.h   | 152 ++++++++++++++++++++++++
 src/ipa/rkisp1/params.cpp      |  93 +--------------
 src/ipa/rkisp1/params.h        | 175 ++++++++++++++++------------
 src/ipa/rkisp1/rkisp1.cpp      |   2 +-
 6 files changed, 512 insertions(+), 166 deletions(-)

Quoting Jacopo Mondi (2025-10-14 11:55:39)
> The existing RkISP1Params helper class allows the RkISP1 IPA to handle
> both the extensible parameters format and the legacy fixed-size format.
> 
> With the introduction of v4l2-isp.h in the Linux kernel the part of
> the RkISP1Params helper class that handles extensible parameters can
> be generalized so that other IPA modules can use the same helpers
> to populate a parameters buffer compatible with v4l2-isp.h.
> 
> Generalize the RkISP1Params class to a new libipa component named
> V4L2Params and derive the existing RkISP1Params from it, leaving
> in the RkISP1-specific implementation the handling of the legacy format.
> 
> Deriving RkISP1Params from V4L2Params requires changing the size
> associated to each block to include the size of v4l2_params_block_header
> in the ipa:rkisp1::kBlockTypeInfo map as the V4L2Params::block()
> implementation doesn't account for that as RkIS1Params::block()
> implementation did.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Tested-by: Antoine Bouyer <antoine.bouyer@nxp.com>

some very tiny nits below but nothing that I would consider block this:


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

> ---
>  src/ipa/libipa/meson.build     |   2 +
>  src/ipa/libipa/v4l2_params.cpp | 254 +++++++++++++++++++++++++++++++++++++++++
>  src/ipa/libipa/v4l2_params.h   | 152 ++++++++++++++++++++++++
>  src/ipa/rkisp1/params.cpp      |  93 +--------------
>  src/ipa/rkisp1/params.h        | 175 ++++++++++++++++------------
>  src/ipa/rkisp1/rkisp1.cpp      |   2 +-
>  6 files changed, 512 insertions(+), 166 deletions(-)
> 
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 660be94054fa98b714b6bc586039081e45a6b4bc..7202df869c2fe4d58586f72f0c3d7c9fb1472d23 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -17,6 +17,7 @@ libipa_headers = files([
>      'lux.h',
>      'module.h',
>      'pwl.h',
> +    'v4l2_params.h',
>  ])
>  
>  libipa_sources = files([
> @@ -36,6 +37,7 @@ libipa_sources = files([
>      'lux.cpp',
>      'module.cpp',
>      'pwl.cpp',
> +    'v4l2_params.cpp',
>  ])
>  
>  libipa_includes = include_directories('..')
> diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp
> new file mode 100644
> index 0000000000000000000000000000000000000000..ea0cc85471f1289ecd227d9833ed20d5851d404e
> --- /dev/null
> +++ b/src/ipa/libipa/v4l2_params.cpp
> @@ -0,0 +1,254 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2025, Ideas On Board
> + *
> + * V4L2 Parameters
> + */
> +
> +#include "v4l2_params.h"
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +/**
> + * \file v4l2_params.cpp
> + * \brief Helper class to populate an ISP configuration buffer compatible with
> + * the generic V4L2 ISP format
> + *
> + * The Linux kernel defines a generic buffer format for configuring ISP devices.
> + * The format describes a serialisation method for ISP parameters that allows
> + * userspace to populate a buffer of configuration data by appending blocks to
> + * the buffer with common headers but device-specific contents one after the
> + * other.
> + *
> + * The V4L2Params class implements support the V4L2 ISP parameters buffer format
> + * and allows users to populate the ISP configuration blocks, represented as
> + * V4L2ParamBlock class instances.
> + *
> + * IPA implementations using this helpers should define an enumeration of ISP

nit: Either "these helpers" or "this helper"

> + * blocks supported by the IPA module and use a set of common abstraction to
> + * help their derived implementation of V4L2Params translate the enumerated ISP
> + * block identifier to the actual type of the configuration data as defined by
> + * the kernel interface.
> + *
> + * As an example of this see the RkISP1 and Mali-C55 implementations.
> + */
> +
> +/**
> + * \class V4L2ParamsBlock
> + * \brief Helper class that represents a ISP configuration block

'an ISP'

> + *
> + * Each ISP function is associated with a set of configuration parameters
> + * defined by the kernel interface.
> + *
> + * This class represents an ISP block configuration entry. It is constructed
> + * with a reference to the memory area where the block configuration will be
> + * stored in the parameters buffer. The template parameter represents
> + * the underlying kernel-defined ISP block configuration type and allow its

allows its

> + * user to easily cast it to said type to populate and read the configuration
> + * parameters.
> + *
> + * \sa V4L2Params::block()
> + */
> +
> +/**
> + * \fn V4L2ParamsBlock::V4L2ParamsBlock()
> + * \brief Construct a V4L2ParamsBlock with memory represented by \a data
> + * \param[in] data The memory area where the ISP block is located
> + */
> +
> +/**
> + * \fn V4L2ParamsBlock::setEnabled()
> + * \brief Enable/disable an ISP configuration block
> + * \param[in] enabled The enable flag
> + */
> +
> +/**
> + * \fn V4L2ParamsBlock::operator->()
> + * \brief Access the ISP configuration block casting it to the kernel-defined
> + * ISP configuration type
> + *
> + * The V4L2ParamsBlock is templated with the kernel defined ISP configuration
> + * block type. This function allows users to easily cast a V4L2ParamsBlock to
> + * the underlying kernel-defined type in order to easily populate or read
> + * the ISP configuration data.
> + *
> + * \code{.cpp}
> + *
> + * // The kernel header defines the ISP configuration types, in example
> + * // struct my_isp_awb_config_data {
> + * //          u16 gain_ch00;
> + * //          u16 gain_ch01;
> + * //          u16 gain_ch10;
> + * //          u16 gain_ch11;
> + * //

Can probably drop this blank line

> + * //  }
> + *
> + * template<> V4L2ParamsBlock<struct my_isp_awb_config_data> awbBlock = ...
> + *
> + * awbBlock->gain_ch00 = ...;
> + * awbBlock->gain_ch01 = ...;
> + * awbBlock->gain_ch10 = ...;
> + * awbBlock->gain_ch11 = ...;
> + *
> + * \endcode
> + *
> + * Users of this class shall not create a V4L2ParamsBlock manually but should
> + * use V4L2Params::block().
> + */
> +
> +/**
> + * \fn V4L2ParamsBlock::operator->() const
> + * \copydoc V4L2ParamsBlock::operator->()
> + */
> +
> +/**
> + * \fn V4L2ParamsBlock::operator*() const
> + * \copydoc V4L2ParamsBlock::operator->()
> + */
> +
> +/**
> + * \fn V4L2ParamsBlock::operator*()
> + * \copydoc V4L2ParamsBlock::operator->()
> + */
> +
> +/**
> + * \var V4L2ParamsBlock::data_
> + * \brief Memory area reserved for the ISP configuration block
> + */
> +
> + /**
> +  * \class V4L2Params
> +  * \brief Helper class that represent an ISP configuration buffer
> +  *
> + * This class represents an ISP configuration buffer. It is constructed
> + * with a reference to the memory mapped buffer that will be queued to the ISP
> + * driver.
> + *
> + * This class is templated with the type of the enumeration of ISP blocks that
> + * each IPA module is expected to support. IPA modules are expected to derive
> + * this class by providing a 'param_traits' type that helps the class associate
> + * a block type with the actual memory area that represents the ISP
> + * configuration block.
> + *
> + * \code{.cpp}
> + *
> + * // Define the supported ISP blocks
> + * enum class myISPBlocks {
> + *     Agc,
> + *     Awb,
> + *     ...
> + * };
> + *
> + * // Maps the C++ enum type to the kernel enum type and concrete parameter type
> + * template<myISPBlocks B>
> + * struct block_type {
> + * };
> + *
> + * template<>
> + * struct block_type<myISPBlock::Agc> {
> + *     using type = struct my_isp_kernel_config_type_agc;
> + *     static constexpr kernel_enum_type blockType = MY_ISP_TYPE_AGC;
> + * };
> + *
> + * template<>
> + * struct block_type<myISPBlock::Awb> {
> + *     using type = struct my_isp_kernel_config_type_awb;
> + *     static constexpr kernel_enum_type blockType = MY_ISP_TYPE_AWB;
> + * };
> + *
> + *
> + * // Convenience type to associate a block id to the 'block_type' overload
> + * struct params_traits {
> + *     using id_type = myISPBlocks;
> + *     template<id_type Id> using id_to_details = block_type<Id>;
> + * };
> + *
> + * ...
> + *
> + * // Derive the V4L2Params class by providing params_traits
> + * class MyISPParams : public V4L2Params<params_traits>
> + * {
> + * public:
> + *     MyISPParams::MyISPParams(Span<uint8_t> data)
> + *             : V4L2Params(data, kVersion)
> + *     {
> + *     }
> + * };
> + *
> + * \endcode
> + *
> + * Users of this class can then easily access an ISP configuration block as a
> + * V4L2ParamsBlock instance.
> + *
> + * \code{.cpp}
> + *
> + * MyISPParams params(data);
> + *
> + * auto awb = params.block<myISPBlocks::AWB>();
> + * awb->gain00 = ...;
> + * awb->gain01 = ...;
> + * awb->gain10 = ...;
> + * awb->gain11 = ...;
> + * \endcode
> + */
> +
> +/**
> + * \fn V4L2Params::V4L2Params()
> + * \brief Construct an instance of V4L2Params
> + * \param[in] data Reference to the v4l2-buffer memory mapped area
> + * \param[in] version The ISP parameters version the implementation supports
> + */
> +
> +/**
> + * \fn V4L2Params::bytesused()
> + * \brief Retrieve the used size of the parameters buffer (in bytes)
> + *
> + * The parameters buffer size is mostly used to populate the v4l2_buffer
> + * bytesused field before queueing the buffer to the ISP.
> + *
> + * \return The number of bytes occupied by the ISP configuration parameters
> + */
> +
> +/**
> + * \fn V4L2Params::block()
> + * \brief Retrieve the location of an ISP configuration block a return it
> + * \return A V4L2ParamsBlock instance that points to the ISP configuration block
> + */
> +
> +/**
> + * \fn V4L2Params::block(typename Traits::id_type type, unsigned int blockType, size_t blockSize)
> + * \brief Populate an ISP configuration block a returns a reference to its
> + * memory
> + * \param[in] type The ISP block identifier enumerated by the IPA module
> + * \param[in] blockType The kernel-defined ISP block identifier, used to
> + * populate the block header
> + * \param[in] blockSize The ISP block size, used to populate the block header
> + *
> + * Initialize the block header with \a blockType and \a blockSize and
> + * returns a reference to the memory used to store an ISP configuration block.
> + *
> + * IPA modules that derive the V4L2Params class shall use this function to
> + * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T>
> + * before returning it to the caller.
> + */
> +
> +/**
> + * \var V4L2Params::data_
> + * \brief The ISP parameters buffer memory
> + */
> +
> +/**
> + * \var V4L2Params::used_
> + * \brief The number of bytes used in the parameters buffer
> + */
> +
> +/**
> + * \var V4L2Params::blocks_
> + * \brief Cache of ISP configuration blocks
> + */
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..728305c5b7b859815bee6da62ce35c44dd1ec41a
> --- /dev/null
> +++ b/src/ipa/libipa/v4l2_params.h
> @@ -0,0 +1,152 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2025, Ideas On Board
> + *
> + * V4L2 Parameters
> + */
> +
> +#pragma once
> +
> +#include <map>
> +#include <stdint.h>
> +#include <string.h>
> +
> +#include <linux/media/v4l2-isp.h>
> +
> +#include <libcamera/base/span.h>
> +#include <libcamera/base/log.h>
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +LOG_DECLARE_CATEGORY(V4L2Params)
> +
> +template<typename T>
> +class V4L2ParamsBlock
> +{
> +public:
> +       V4L2ParamsBlock(const Span<uint8_t> data)
> +               : data_(data)
> +       {
> +       }
> +
> +       virtual ~V4L2ParamsBlock() {}
> +
> +       virtual void setEnabled(bool enabled)
> +       {
> +               struct v4l2_isp_params_block_header *header =
> +                       reinterpret_cast<struct v4l2_isp_params_block_header *>(data_.data());
> +
> +               header->flags &= ~(V4L2_ISP_PARAMS_FL_BLOCK_ENABLE |
> +                                  V4L2_ISP_PARAMS_FL_BLOCK_DISABLE);
> +               header->flags |= enabled ? V4L2_ISP_PARAMS_FL_BLOCK_ENABLE
> +                                        : V4L2_ISP_PARAMS_FL_BLOCK_DISABLE;
> +       }
> +
> +       virtual const T *operator->() const
> +       {
> +               return reinterpret_cast<const T *>(data_.data());
> +       }
> +
> +       virtual T *operator->()
> +       {
> +               return reinterpret_cast<T *>(data_.data());
> +       }
> +
> +       virtual const T &operator*() const
> +       {
> +               return *reinterpret_cast<const T *>(data_.data());
> +       }
> +
> +       virtual T &operator*()
> +       {
> +               return *reinterpret_cast<T *>(data_.data());
> +       }
> +
> +protected:
> +       Span<uint8_t> data_;
> +};
> +
> +template<typename Traits>
> +class V4L2Params
> +{
> +public:
> +       V4L2Params(Span<uint8_t> data, unsigned int version)
> +               : data_(data)
> +       {
> +               struct v4l2_isp_params_buffer *params =
> +                       reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data());
> +               params->data_size = 0;
> +               params->version = version;
> +
> +               used_ = offsetof(struct v4l2_isp_params_buffer, data);
> +       }
> +
> +       size_t bytesused() const { return used_; }
> +
> +       template<typename Traits::id_type Id>
> +       auto block()
> +       {
> +               using Details = typename Traits::template id_to_details<Id>;
> +
> +               using Type = typename Details::type;
> +               constexpr auto kernelId = Details::blockType;
> +
> +               auto data = block(Id, kernelId, sizeof(Type));
> +               return V4L2ParamsBlock<Type>(data);
> +       }
> +
> +protected:
> +       Span<uint8_t> block(typename Traits::id_type type,
> +                           unsigned int blockType, size_t blockSize)
> +       {
> +               /*
> +                * Look up the block in the cache first. If an algorithm
> +                * requests the same block type twice, it should get the same
> +                * block.
> +                */
> +               auto cacheIt = blocks_.find(type);
> +               if (cacheIt != blocks_.end())
> +                       return cacheIt->second;
> +
> +               /*
> +                * Make sure we don't run out of space. Assert as otherwise
> +                * we get a segfault as soon as someone tries to access the
> +                * empty Span<> returned from here.
> +                */
> +               if (blockSize > data_.size() - used_) {
> +                       LOG(Fatal) <<
> +                               "Parameters buffer out of space; potential version mismatch between driver and libcamera";
> +                       return {};
> +               }
> +
> +               /* Allocate a new block, clear its memory, and initialize its header. */
> +               Span<uint8_t> block = data_.subspan(used_, blockSize);
> +               memset(block.data(), 0, block.size());
> +
> +               struct v4l2_isp_params_block_header *header =
> +                       reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data());
> +               header->type = blockType;
> +               header->size = block.size();
> +
> +               used_ += block.size();
> +
> +               reinterpret_cast<struct v4l2_isp_params_buffer *>
> +                       (data_.data())->data_size += block.size();
> +
> +               /* Update the cache. */
> +               blocks_[type] = block;
> +
> +               return block;
> +       }
> +
> +       Span<uint8_t> data_;
> +       size_t used_;
> +
> +       std::map<typename Traits::id_type, Span<uint8_t>> blocks_;
> +};
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp
> index 5edb36c91b87859d02c0a8b41efe977ff048def5..7040207c26557aa278050a1f7232cc6c380505b1 100644
> --- a/src/ipa/rkisp1/params.cpp
> +++ b/src/ipa/rkisp1/params.cpp
> @@ -35,7 +35,7 @@ struct BlockTypeInfo {
>  #define RKISP1_BLOCK_TYPE_ENTRY(block, id, type, category, bit)                        \
>         { BlockType::block, {                                                   \
>                 RKISP1_EXT_PARAMS_BLOCK_TYPE_##id,                              \
> -               sizeof(struct rkisp1_cif_isp_##type##_config),                  \
> +               sizeof(struct rkisp1_ext_params_##type##_config),               \
>                 offsetof(struct rkisp1_params_cfg, category.type##_config),     \
>                 RKISP1_CIF_ISP_MODULE_##bit,                                    \
>         } }
> @@ -49,7 +49,7 @@ struct BlockTypeInfo {
>  #define RKISP1_BLOCK_TYPE_ENTRY_EXT(block, id, type)                           \
>         { BlockType::block, {                                                   \
>                 RKISP1_EXT_PARAMS_BLOCK_TYPE_##id,                              \
> -               sizeof(struct rkisp1_cif_isp_##type##_config),                  \
> +               sizeof(struct rkisp1_ext_params_##type##_config),               \
>                 0, 0,                                                           \
>         } }
>  
> @@ -79,56 +79,6 @@ const std::map<BlockType, BlockTypeInfo> kBlockTypeInfo = {
>  
>  } /* namespace */
>  
> -RkISP1ParamsBlockBase::RkISP1ParamsBlockBase(RkISP1Params *params, BlockType type,
> -                                            const Span<uint8_t> &data)
> -       : params_(params), type_(type)
> -{
> -       if (params_->format() == V4L2_META_FMT_RK_ISP1_EXT_PARAMS) {
> -               header_ = data.subspan(0, sizeof(rkisp1_ext_params_block_header));
> -               data_ = data.subspan(sizeof(rkisp1_ext_params_block_header));
> -       } else {
> -               data_ = data;
> -       }
> -}
> -
> -void RkISP1ParamsBlockBase::setEnabled(bool enabled)
> -{
> -       /*
> -        * For the legacy fixed format, blocks are enabled in the top-level
> -        * header. Delegate to the RkISP1Params class.
> -        */
> -       if (params_->format() == V4L2_META_FMT_RK_ISP1_PARAMS)
> -               return params_->setBlockEnabled(type_, enabled);
> -
> -       /*
> -        * For the extensible format, set the enable and disable flags in the
> -        * block header directly.
> -        */
> -       struct rkisp1_ext_params_block_header *header =
> -               reinterpret_cast<struct rkisp1_ext_params_block_header *>(header_.data());
> -       header->flags &= ~(RKISP1_EXT_PARAMS_FL_BLOCK_ENABLE |
> -                          RKISP1_EXT_PARAMS_FL_BLOCK_DISABLE);
> -       header->flags |= enabled ? RKISP1_EXT_PARAMS_FL_BLOCK_ENABLE
> -                                : RKISP1_EXT_PARAMS_FL_BLOCK_DISABLE;
> -}
> -
> -RkISP1Params::RkISP1Params(uint32_t format, Span<uint8_t> data)
> -       : format_(format), data_(data), used_(0)
> -{
> -       if (format_ == V4L2_META_FMT_RK_ISP1_EXT_PARAMS) {
> -               struct rkisp1_ext_params_cfg *cfg =
> -                       reinterpret_cast<struct rkisp1_ext_params_cfg *>(data.data());
> -
> -               cfg->version = RKISP1_EXT_PARAM_BUFFER_V1;
> -               cfg->data_size = 0;
> -
> -               used_ += offsetof(struct rkisp1_ext_params_cfg, data);
> -       } else {
> -               memset(data.data(), 0, data.size());
> -               used_ = sizeof(struct rkisp1_params_cfg);
> -       }
> -}
> -
>  void RkISP1Params::setBlockEnabled(BlockType type, bool enabled)
>  {
>         const BlockTypeInfo &info = kBlockTypeInfo.at(type);
> @@ -178,44 +128,7 @@ Span<uint8_t> RkISP1Params::block(BlockType type)
>                 return data_.subspan(info.offset, info.size);
>         }
>  
> -       /*
> -        * For the extensible format, allocate memory for the block, including
> -        * the header. Look up the block in the cache first. If an algorithm
> -        * requests the same block type twice, it should get the same block.
> -        */
> -       auto cacheIt = blocks_.find(type);
> -       if (cacheIt != blocks_.end())
> -               return cacheIt->second;
> -
> -       /* Make sure we don't run out of space. */
> -       size_t size = sizeof(struct rkisp1_ext_params_block_header)
> -                   + ((info.size + 7) & ~7);
> -       if (size > data_.size() - used_) {
> -               LOG(RkISP1Params, Error)
> -                       << "Out of memory to allocate block type "
> -                       << utils::to_underlying(type);
> -               return {};
> -       }
> -
> -       /* Allocate a new block, clear its memory, and initialize its header. */
> -       Span<uint8_t> block = data_.subspan(used_, size);
> -       used_ += size;
> -
> -       struct rkisp1_ext_params_cfg *cfg =
> -               reinterpret_cast<struct rkisp1_ext_params_cfg *>(data_.data());
> -       cfg->data_size += size;
> -
> -       memset(block.data(), 0, block.size());
> -
> -       struct rkisp1_ext_params_block_header *header =
> -               reinterpret_cast<struct rkisp1_ext_params_block_header *>(block.data());
> -       header->type = info.type;
> -       header->size = block.size();
> -
> -       /* Update the cache. */
> -       blocks_[type] = block;
> -
> -       return block;
> +       return V4L2Params::block(type, info.type, info.size);

Well it's definitely nice to see lots of code factored out :D


>  }
>  
>  } /* namespace ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/params.h b/src/ipa/rkisp1/params.h
> index 2e60528d102ec44a31417d4b146e74cace363efa..eddb37d5c000b6b1de1c698ad915f2b42da58b81 100644
> --- a/src/ipa/rkisp1/params.h
> +++ b/src/ipa/rkisp1/params.h
> @@ -7,13 +7,10 @@
>  
>  #pragma once
>  
> -#include <map>
> -#include <stdint.h>
> -
>  #include <linux/rkisp1-config.h>
> +#include <linux/videodev2.h>
>  
> -#include <libcamera/base/class.h>
> -#include <libcamera/base/span.h>
> +#include <libipa/v4l2_params.h>
>  
>  namespace libcamera {
>  
> @@ -49,115 +46,143 @@ template<BlockType B>
>  struct block_type {
>  };
>  
> -#define RKISP1_DEFINE_BLOCK_TYPE(blockType, blockStruct)               \
> +#define RKISP1_DEFINE_BLOCK_TYPE(blockType, blockStruct, id)           \
>  template<>                                                             \
>  struct block_type<BlockType::blockType> {                              \
>         using type = struct rkisp1_cif_isp_##blockStruct##_config;      \
> +       static constexpr rkisp1_ext_params_block_type blockType =       \
> +               RKISP1_EXT_PARAMS_BLOCK_TYPE_##id;                      \
>  };
>  
> -RKISP1_DEFINE_BLOCK_TYPE(Bls, bls)
> -RKISP1_DEFINE_BLOCK_TYPE(Dpcc, dpcc)
> -RKISP1_DEFINE_BLOCK_TYPE(Sdg, sdg)
> -RKISP1_DEFINE_BLOCK_TYPE(AwbGain, awb_gain)
> -RKISP1_DEFINE_BLOCK_TYPE(Flt, flt)
> -RKISP1_DEFINE_BLOCK_TYPE(Bdm, bdm)
> -RKISP1_DEFINE_BLOCK_TYPE(Ctk, ctk)
> -RKISP1_DEFINE_BLOCK_TYPE(Goc, goc)
> -RKISP1_DEFINE_BLOCK_TYPE(Dpf, dpf)
> -RKISP1_DEFINE_BLOCK_TYPE(DpfStrength, dpf_strength)
> -RKISP1_DEFINE_BLOCK_TYPE(Cproc, cproc)
> -RKISP1_DEFINE_BLOCK_TYPE(Ie, ie)
> -RKISP1_DEFINE_BLOCK_TYPE(Lsc, lsc)
> -RKISP1_DEFINE_BLOCK_TYPE(Awb, awb_meas)
> -RKISP1_DEFINE_BLOCK_TYPE(Hst, hst)
> -RKISP1_DEFINE_BLOCK_TYPE(Aec, aec)
> -RKISP1_DEFINE_BLOCK_TYPE(Afc, afc)
> -RKISP1_DEFINE_BLOCK_TYPE(CompandBls, compand_bls)
> -RKISP1_DEFINE_BLOCK_TYPE(CompandExpand, compand_curve)
> -RKISP1_DEFINE_BLOCK_TYPE(CompandCompress, compand_curve)
> -RKISP1_DEFINE_BLOCK_TYPE(Wdr, wdr)
> +RKISP1_DEFINE_BLOCK_TYPE(Bls, bls, BLS)
> +RKISP1_DEFINE_BLOCK_TYPE(Dpcc, dpcc, DPCC)
> +RKISP1_DEFINE_BLOCK_TYPE(Sdg, sdg, SDG)
> +RKISP1_DEFINE_BLOCK_TYPE(AwbGain, awb_gain, AWB_GAIN)
> +RKISP1_DEFINE_BLOCK_TYPE(Flt, flt, FLT)
> +RKISP1_DEFINE_BLOCK_TYPE(Bdm, bdm, BDM)
> +RKISP1_DEFINE_BLOCK_TYPE(Ctk, ctk, CTK)
> +RKISP1_DEFINE_BLOCK_TYPE(Goc, goc, GOC)
> +RKISP1_DEFINE_BLOCK_TYPE(Dpf, dpf, DPF)
> +RKISP1_DEFINE_BLOCK_TYPE(DpfStrength, dpf_strength, DPF_STRENGTH)
> +RKISP1_DEFINE_BLOCK_TYPE(Cproc, cproc, CPROC)
> +RKISP1_DEFINE_BLOCK_TYPE(Ie, ie, IE)
> +RKISP1_DEFINE_BLOCK_TYPE(Lsc, lsc, LSC)
> +RKISP1_DEFINE_BLOCK_TYPE(Awb, awb_meas, AWB_MEAS)
> +RKISP1_DEFINE_BLOCK_TYPE(Hst, hst, HST_MEAS)
> +RKISP1_DEFINE_BLOCK_TYPE(Aec, aec, AEC_MEAS)
> +RKISP1_DEFINE_BLOCK_TYPE(Afc, afc, AFC_MEAS)
> +RKISP1_DEFINE_BLOCK_TYPE(CompandBls, compand_bls, COMPAND_BLS)
> +RKISP1_DEFINE_BLOCK_TYPE(CompandExpand, compand_curve, COMPAND_EXPAND)
> +RKISP1_DEFINE_BLOCK_TYPE(CompandCompress, compand_curve, COMPAND_COMPRESS)
> +RKISP1_DEFINE_BLOCK_TYPE(Wdr, wdr, WDR)
> +
> +struct params_traits {
> +       using id_type = BlockType;
> +
> +       template<id_type Id>
> +       using id_to_details = block_type<Id>;
> +};
>  
>  } /* namespace details */
>  
> -class RkISP1Params;
> +template<typename T>
> +class RkISP1ParamsBlock;
>  
> -class RkISP1ParamsBlockBase
> +class RkISP1Params : public V4L2Params<details::params_traits>
>  {
>  public:
> -       RkISP1ParamsBlockBase(RkISP1Params *params, BlockType type,
> -                             const Span<uint8_t> &data);
> +       static constexpr unsigned int kVersion = RKISP1_EXT_PARAM_BUFFER_V1;
> +
> +       RkISP1Params(uint32_t format, Span<uint8_t> data)
> +               : V4L2Params(data, kVersion), format_(format)
> +       {
> +               if (format_ == V4L2_META_FMT_RK_ISP1_PARAMS) {
> +                       memset(data.data(), 0, data.size());
> +                       used_ = sizeof(struct rkisp1_params_cfg);
> +               }
> +       }
> +
> +       template<details::params_traits::id_type id>
> +       auto block()
> +       {
> +               using Type = typename details::block_type<id>::type;
>  
> -       Span<uint8_t> data() const { return data_; }
> +               return RkISP1ParamsBlock<Type>(this, id, block(id));
> +       }
>  
> -       void setEnabled(bool enabled);
> +       uint32_t format() const { return format_; }
> +       void setBlockEnabled(BlockType type, bool enabled);
>  
>  private:
> -       LIBCAMERA_DISABLE_COPY(RkISP1ParamsBlockBase)
> +       Span<uint8_t> block(BlockType type);
>  
> -       RkISP1Params *params_;
> -       BlockType type_;
> -       Span<uint8_t> header_;
> -       Span<uint8_t> data_;
> +       uint32_t format_;
>  };
>  
> -template<BlockType B>
> -class RkISP1ParamsBlock : public RkISP1ParamsBlockBase
> +template<typename T>
> +class RkISP1ParamsBlock final : public V4L2ParamsBlock<T>
>  {
>  public:
> -       using Type = typename details::block_type<B>::type;
> -
> -       RkISP1ParamsBlock(RkISP1Params *params, const Span<uint8_t> &data)
> -               : RkISP1ParamsBlockBase(params, B, data)
> +       RkISP1ParamsBlock(RkISP1Params *params, BlockType type,
> +                         const Span<uint8_t> data)
> +               : V4L2ParamsBlock<T>(data)
>         {
> +               params_ = params;
> +               type_ = type;
> +
> +               /*
> +                * cifData_ points to the actual configuration data
> +                * (struct rkisp1_cif_isp_*) which is not prefixed by any header,
> +                * for the legacy fixed format.
> +                */
> +               if (params_->format() == V4L2_META_FMT_RK_ISP1_PARAMS)
> +                       cifData_ = data;
> +               else
> +                       cifData_ = data.subspan(sizeof(v4l2_isp_params_block_header));
>         }
>  
> -       const Type *operator->() const
> +       void setEnabled(bool enabled) override
>         {
> -               return reinterpret_cast<const Type *>(data().data());
> +               /*
> +                * For the legacy fixed format, blocks are enabled in the
> +                * top-level header. Delegate to the RkISP1Params class.
> +                */
> +               if (params_->format() == V4L2_META_FMT_RK_ISP1_PARAMS)
> +                       return params_->setBlockEnabled(type_, enabled);
> +
> +               return V4L2ParamsBlock<T>::setEnabled(enabled);
>         }
>  
> -       Type *operator->()
> +       /*
> +        * Override the dereference operators to return a reference to the
> +        * actual configuration data (struct rkisp1_cif_isp_*) skipping the
> +        * 'v4l2_isp_params_block_header' header.
> +        */
> +
> +       virtual const T *operator->() const override
>         {
> -               return reinterpret_cast<Type *>(data().data());
> +               return reinterpret_cast<const T *>(cifData_.data());
>         }
>  
> -       const Type &operator*() const &
> +       virtual T *operator->() override
>         {
> -               return *reinterpret_cast<const Type *>(data().data());
> +               return reinterpret_cast<T *>(cifData_.data());
>         }
>  
> -       Type &operator*() &
> +       virtual const T &operator*() const override
>         {
> -               return *reinterpret_cast<Type *>(data().data());
> +               return *reinterpret_cast<const T *>(cifData_.data());
>         }
> -};
> -
> -class RkISP1Params
> -{
> -public:
> -       RkISP1Params(uint32_t format, Span<uint8_t> data);
>  
> -       template<BlockType B>
> -       RkISP1ParamsBlock<B> block()
> +       virtual T &operator*() override
>         {
> -               return RkISP1ParamsBlock<B>(this, block(B));
> +               return *reinterpret_cast<T *>(cifData_.data());
>         }
>  
> -       uint32_t format() const { return format_; }
> -       size_t size() const { return used_; }
> -
>  private:
> -       friend class RkISP1ParamsBlockBase;
> -
> -       Span<uint8_t> block(BlockType type);
> -       void setBlockEnabled(BlockType type, bool enabled);
> -
> -       uint32_t format_;
> -
> -       Span<uint8_t> data_;
> -       size_t used_;
> -
> -       std::map<BlockType, Span<uint8_t>> blocks_;
> +       RkISP1Params *params_;
> +       BlockType type_;
> +       Span<uint8_t> cifData_;
>  };
>  
>  } /* namespace ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index fa22bfc349043002345d275b11a60ac983e329d7..f5f7377b63d21af11473ccc4ab18ab45294743cb 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -349,7 +349,7 @@ void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)
>         for (auto const &algo : algorithms())
>                 algo->prepare(context_, frame, frameContext, &params);
>  
> -       paramsComputed.emit(frame, params.size());
> +       paramsComputed.emit(frame, params.bytesused());
>  }
>  
>  void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
> 
> -- 
> 2.51.0
>