[PATCH v4 4/9] ipa: rkisp1: Add ISP parameters abstraction class
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Tue Aug 27 09:49:07 CEST 2024
On Tue, Aug 27, 2024 at 10:43:39AM GMT, Laurent Pinchart wrote:
> On Tue, Aug 27, 2024 at 08:53:44AM +0200, Jacopo Mondi wrote:
> > Hi Laurent
> > On Tue, Aug 27, 2024 at 04:40:38AM GMT, Laurent Pinchart wrote:
> > > Individual algorithms of the rkisp1 IPA module access their
> > > corresponding ISP parameters through the top-level structure
> > > rkisp1_params_cfg. This will not work anymore with the new parameters
> > > format. In order to ease the transition to the new format, abstract the
> > > ISP parameters in a new RkISP1Params class that offers the same
> > > interface regardless of the format.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > > Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > > ---
> > > Changes since v3:
> > >
> > > - Update to latest kernel API
> > >
> >
> > I don't get it, what kernel API was the previous version based on ?
>
> This should have been in the "changes since v2" section, sorry. I'm
> referring to the usage of the .flags field, replacing .enabled.
>
Ah ok, I was scared I have been testing against an older kernel
revision ;)
> > I tested against your v6.11/merge branch which if I'm not mistaken has
> > media-stage merged in.
> >
> > > Changes since v2:
> > >
> > > - Rename Block to BlockType
> > > - Fix grammer in comment
> > >
> > > Changes since v1:
> > >
> > > - Fix module enable and update fields for legacy format
> > > - Log messages when block allocation fails
> > > - Use uint32_t type for enableBit
> > > - Reword comment explaining block caching
> > > - Fix State::Disable enumerator value
> > > - Disable copying of RkISP1ParamsBlock class
> > > - Move the params enabled state handling to a setEnabled() function
> > > ---
> > > src/ipa/rkisp1/meson.build | 1 +
> > > src/ipa/rkisp1/params.cpp | 219 +++++++++++++++++++++++++++++++++++++
> > > src/ipa/rkisp1/params.h | 157 ++++++++++++++++++++++++++
> > > 3 files changed, 377 insertions(+)
> > > create mode 100644 src/ipa/rkisp1/params.cpp
> > > create mode 100644 src/ipa/rkisp1/params.h
> > >
> > > diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build
> > > index 160ef52dd52e..34844f1498f9 100644
> > > --- a/src/ipa/rkisp1/meson.build
> > > +++ b/src/ipa/rkisp1/meson.build
> > > @@ -7,6 +7,7 @@ ipa_name = 'ipa_rkisp1'
> > >
> > > rkisp1_ipa_sources = files([
> > > 'ipa_context.cpp',
> > > + 'params.cpp',
> > > 'rkisp1.cpp',
> > > 'utils.cpp',
> > > ])
> > > diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp
> > > new file mode 100644
> > > index 000000000000..6bc6f89919fb
> > > --- /dev/null
> > > +++ b/src/ipa/rkisp1/params.cpp
> > > @@ -0,0 +1,219 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2024, Ideas On Board
> > > + *
> > > + * RkISP1 ISP Parameters
> > > + */
> > > +
> > > +#include "params.h"
> > > +
> > > +#include <map>
> > > +#include <stddef.h>
> > > +#include <string.h>
> > > +
> > > +#include <linux/rkisp1-config.h>
> > > +#include <linux/videodev2.h>
> > > +
> > > +#include <libcamera/base/log.h>
> > > +#include <libcamera/base/utils.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +LOG_DEFINE_CATEGORY(RkISP1Params)
> > > +
> > > +namespace ipa::rkisp1 {
> > > +
> > > +namespace {
> > > +
> > > +struct BlockTypeInfo {
> > > + enum rkisp1_ext_params_block_type type;
> > > + size_t size;
> > > + size_t offset;
> > > + uint32_t enableBit;
> > > +};
> > > +
> > > +#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), \
> > > + offsetof(struct rkisp1_params_cfg, category.type##_config), \
> > > + RKISP1_CIF_ISP_MODULE_##bit, \
> > > + } }
> > > +
> > > +#define RKISP1_BLOCK_TYPE_ENTRY_MEAS(block, id, type) \
> > > + RKISP1_BLOCK_TYPE_ENTRY(block, id##_MEAS, type, meas, id)
> > > +
> > > +#define RKISP1_BLOCK_TYPE_ENTRY_OTHERS(block, id, type) \
> > > + RKISP1_BLOCK_TYPE_ENTRY(block, id, type, others, id)
> > > +
> > > +#define RKISP1_BLOCK_TYPE_ENTRY_EXT(block, id, type) \
> > > + { BlockType::block, { \
> > > + RKISP1_EXT_PARAMS_BLOCK_TYPE_##id, \
> > > + sizeof(struct rkisp1_cif_isp_##type##_config), \
> > > + 0, 0, \
> > > + } }
> > > +
> > > +const std::map<BlockType, BlockTypeInfo> kBlockTypeInfo = {
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Bls, BLS, bls),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Dpcc, DPCC, dpcc),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Sdg, SDG, sdg),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(AwbGain, AWB_GAIN, awb_gain),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Flt, FLT, flt),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Bdm, BDM, bdm),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Ctk, CTK, ctk),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Goc, GOC, goc),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Dpf, DPF, dpf),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(DpfStrength, DPF_STRENGTH, dpf_strength),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Cproc, CPROC, cproc),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Ie, IE, ie),
> > > + RKISP1_BLOCK_TYPE_ENTRY_OTHERS(Lsc, LSC, lsc),
> > > + RKISP1_BLOCK_TYPE_ENTRY_MEAS(Awb, AWB, awb_meas),
> > > + RKISP1_BLOCK_TYPE_ENTRY_MEAS(Hst, HST, hst),
> > > + RKISP1_BLOCK_TYPE_ENTRY_MEAS(Aec, AEC, aec),
> > > + RKISP1_BLOCK_TYPE_ENTRY_MEAS(Afc, AFC, afc),
> > > +};
> > > +
> > > +} /* 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);
> > > +
> > > + struct rkisp1_params_cfg *cfg =
> > > + reinterpret_cast<struct rkisp1_params_cfg *>(data_.data());
> > > + if (enabled)
> > > + cfg->module_ens |= info.enableBit;
> > > + else
> > > + cfg->module_ens &= ~info.enableBit;
> > > +}
> > > +
> > > +Span<uint8_t> RkISP1Params::block(BlockType type)
> > > +{
> > > + auto infoIt = kBlockTypeInfo.find(type);
> > > + if (infoIt == kBlockTypeInfo.end()) {
> > > + LOG(RkISP1Params, Error)
> > > + << "Invalid parameters block type "
> > > + << utils::to_underlying(type);
> > > + return {};
> > > + }
> > > +
> > > + const BlockTypeInfo &info = infoIt->second;
> > > +
> > > + /*
> > > + * For the legacy format, return a block referencing the fixed location
> > > + * of the data.
> > > + */
> > > + if (format_ == V4L2_META_FMT_RK_ISP1_PARAMS) {
> > > + /*
> > > + * Blocks available only in extended parameters have an offset
> > > + * of 0. Return nullptr in that case.
> > > + */
> > > + if (info.offset == 0) {
> > > + LOG(RkISP1Params, Error)
> > > + << "Block type " << utils::to_underlying(type)
> > > + << " unavailable in fixed parameters format";
> > > + return {};
> > > + }
> > > +
> > > + struct rkisp1_params_cfg *cfg =
> > > + reinterpret_cast<struct rkisp1_params_cfg *>(data_.data());
> > > +
> > > + cfg->module_cfg_update |= info.enableBit;
> > > + cfg->module_en_update |= info.enableBit;
> > > +
> > > + 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;
> > > +}
> > > +
> > > +} /* namespace ipa::rkisp1 */
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/ipa/rkisp1/params.h b/src/ipa/rkisp1/params.h
> > > new file mode 100644
> > > index 000000000000..28a781bc447c
> > > --- /dev/null
> > > +++ b/src/ipa/rkisp1/params.h
> > > @@ -0,0 +1,157 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2024, Ideas On Board
> > > + *
> > > + * RkISP1 ISP Parameters
> > > + */
> > > +
> > > +#pragma once
> > > +
> > > +#include <map>
> > > +#include <stdint.h>
> > > +
> > > +#include <linux/rkisp1-config.h>
> > > +
> > > +#include <libcamera/base/class.h>
> > > +#include <libcamera/base/span.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +namespace ipa::rkisp1 {
> > > +
> > > +enum class BlockType {
> > > + Bls,
> > > + Dpcc,
> > > + Sdg,
> > > + AwbGain,
> > > + Flt,
> > > + Bdm,
> > > + Ctk,
> > > + Goc,
> > > + Dpf,
> > > + DpfStrength,
> > > + Cproc,
> > > + Ie,
> > > + Lsc,
> > > + Awb,
> > > + Hst,
> > > + Aec,
> > > + Afc,
> > > +};
> > > +
> > > +namespace details {
> > > +
> > > +template<BlockType B>
> > > +struct block_type {
> > > +};
> > > +
> > > +#define RKISP1_DEFINE_BLOCK_TYPE(blockType, blockStruct) \
> > > +template<> \
> > > +struct block_type<BlockType::blockType> { \
> > > + using type = struct rkisp1_cif_isp_##blockStruct##_config; \
> > > +};
> > > +
> > > +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)
> > > +
> > > +} /* namespace details */
> > > +
> > > +class RkISP1Params;
> > > +
> > > +class RkISP1ParamsBlockBase
> > > +{
> > > +public:
> > > + RkISP1ParamsBlockBase(RkISP1Params *params, BlockType type,
> > > + const Span<uint8_t> &data);
> > > +
> > > + Span<uint8_t> data() const { return data_; }
> > > +
> > > + void setEnabled(bool enabled);
> > > +
> > > +private:
> > > + LIBCAMERA_DISABLE_COPY(RkISP1ParamsBlockBase)
> > > +
> > > + RkISP1Params *params_;
> > > + BlockType type_;
> > > + Span<uint8_t> header_;
> > > + Span<uint8_t> data_;
> > > +};
> > > +
> > > +template<BlockType B>
> > > +class RkISP1ParamsBlock : public RkISP1ParamsBlockBase
> > > +{
> > > +public:
> > > + using Type = typename details::block_type<B>::type;
> > > +
> > > + RkISP1ParamsBlock(RkISP1Params *params, const Span<uint8_t> &data)
> > > + : RkISP1ParamsBlockBase(params, B, data)
> > > + {
> > > + }
> > > +
> > > + const Type *operator->() const
> > > + {
> > > + return reinterpret_cast<const Type *>(data().data());
> > > + }
> > > +
> > > + Type *operator->()
> > > + {
> > > + return reinterpret_cast<Type *>(data().data());
> > > + }
> > > +
> > > + const Type &operator*() const &
> > > + {
> > > + return *reinterpret_cast<const Type *>(data().data());
> > > + }
> > > +
> > > + Type &operator*() &
> > > + {
> > > + return *reinterpret_cast<Type *>(data().data());
> > > + }
> > > +};
> > > +
> > > +class RkISP1Params
> > > +{
> > > +public:
> > > + RkISP1Params(uint32_t format, Span<uint8_t> data);
> > > +
> > > + template<BlockType B>
> > > + RkISP1ParamsBlock<B> block()
> > > + {
> > > + return RkISP1ParamsBlock<B>(this, block(B));
> > > + }
> > > +
> > > + 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_;
> > > +};
> > > +
> > > +} /* namespace ipa::rkisp1 */
> > > +
> > > +} /* namespace libcamera*/
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list