| Message ID | 20240703225230.3530-7-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent only one comment on dpf which is the only one with a more convoluted handling. The rest looks very nice! On Thu, Jul 04, 2024 at 01:52:25AM GMT, Laurent Pinchart wrote: > Use the new ISP parameters abstraction class RkISP1Params to access the > ISP parameters in the IPA algorithms. The class replaces the pointer to > the rkisp1_params_cfg structure passed to the algorithms' prepare() > function, and is used to access individual parameters blocks. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/agc.cpp | 46 +++++++++++------------ > src/ipa/rkisp1/algorithms/agc.h | 2 +- > src/ipa/rkisp1/algorithms/awb.cpp | 56 ++++++++++++---------------- > src/ipa/rkisp1/algorithms/awb.h | 2 +- > src/ipa/rkisp1/algorithms/blc.cpp | 18 ++++----- > src/ipa/rkisp1/algorithms/blc.h | 3 +- > src/ipa/rkisp1/algorithms/ccm.cpp | 14 ++----- > src/ipa/rkisp1/algorithms/ccm.h | 4 +- > src/ipa/rkisp1/algorithms/cproc.cpp | 13 +++---- > src/ipa/rkisp1/algorithms/cproc.h | 2 +- > src/ipa/rkisp1/algorithms/dpcc.cpp | 9 ++--- > src/ipa/rkisp1/algorithms/dpcc.h | 2 +- > src/ipa/rkisp1/algorithms/dpf.cpp | 28 +++++++------- > src/ipa/rkisp1/algorithms/dpf.h | 2 +- > src/ipa/rkisp1/algorithms/filter.cpp | 51 ++++++++++++------------- > src/ipa/rkisp1/algorithms/filter.h | 2 +- > src/ipa/rkisp1/algorithms/goc.cpp | 15 ++++---- > src/ipa/rkisp1/algorithms/goc.h | 2 +- > src/ipa/rkisp1/algorithms/gsl.cpp | 20 ++++------ > src/ipa/rkisp1/algorithms/gsl.h | 2 +- > src/ipa/rkisp1/algorithms/lsc.cpp | 27 ++++++-------- > src/ipa/rkisp1/algorithms/lsc.h | 4 +- > src/ipa/rkisp1/module.h | 3 +- > src/ipa/rkisp1/rkisp1.cpp | 12 ++---- > 24 files changed, 148 insertions(+), 191 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp [snip] > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp > index abf957288550..b3f4446631fc 100644 > --- a/src/ipa/rkisp1/algorithms/dpf.cpp > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp > @@ -215,15 +215,26 @@ void Dpf::queueRequest(IPAContext &context, > * \copydoc libcamera::ipa::Algorithm::prepare > */ > void Dpf::prepare(IPAContext &context, const uint32_t frame, > - IPAFrameContext &frameContext, rkisp1_params_cfg *params) > + IPAFrameContext &frameContext, RkISP1Params *params) > { > + if (!frameContext.dpf.update && frame > 0) > + return; > + > + RkISP1Params::State state = frameContext.dpf.denoise > + ? RkISP1Params::State::Enable > + : RkISP1Params::State::Disable; > + auto config = params->block<Block::Dpf>(state); > + > if (frame == 0) { > - params->others.dpf_config = config_; > - params->others.dpf_strength_config = strengthConfig_; > + auto strengthConfig = params->block<Block::DpfStrength>(state); The dpf strength handler in the kernel ignores the enable state, and strengths are configured only when frame == 0, so I would use Enable here for clarity (even if it has not practical differences). > + *strengthConfig = strengthConfig_; > + > + *config = config_; This, however needs to be done everytime we enable the block. The driver sees that state == enable and programs the dpf block with the content of the config structure. If frame > 0 && frameContext.dpf.denoise == true, the kernel will program the block with 0-initialized data if I read it right. I would set *config = config_ for all frames if dpf.update && dpf.denoise ? Thanks j > > const auto &awb = context.configuration.awb; > const auto &lsc = context.configuration.lsc; > - auto &mode = params->others.dpf_config.gain.mode; > + > + auto &mode = config->gain.mode; > > /* > * The DPF needs to take into account the total amount of > @@ -241,15 +252,6 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame, > mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS; > else > mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED; > - > - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPF | > - RKISP1_CIF_ISP_MODULE_DPF_STRENGTH; > - } > - > - if (frameContext.dpf.update) { > - params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF; > - if (frameContext.dpf.denoise) > - params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF; > } > } > > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h > index da0115baf8f1..2dd8cd362624 100644 > --- a/src/ipa/rkisp1/algorithms/dpf.h > +++ b/src/ipa/rkisp1/algorithms/dpf.h > @@ -27,7 +27,7 @@ public: > const ControlList &controls) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) override; > + RkISP1Params *params) override; > > private: > struct rkisp1_cif_isp_dpf_config config_; > diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp > index 9752248a5965..242b781a10f6 100644 > --- a/src/ipa/rkisp1/algorithms/filter.cpp > +++ b/src/ipa/rkisp1/algorithms/filter.cpp > @@ -104,7 +104,7 @@ void Filter::queueRequest(IPAContext &context, > */ > void Filter::prepare([[maybe_unused]] IPAContext &context, > [[maybe_unused]] const uint32_t frame, > - IPAFrameContext &frameContext, rkisp1_params_cfg *params) > + IPAFrameContext &frameContext, RkISP1Params *params) > { > /* Check if the algorithm configuration has been updated. */ > if (!frameContext.filter.update) > @@ -160,23 +160,24 @@ void Filter::prepare([[maybe_unused]] IPAContext &context, > > uint8_t denoise = frameContext.filter.denoise; > uint8_t sharpness = frameContext.filter.sharpness; > - auto &flt_config = params->others.flt_config; > > - flt_config.fac_sh0 = filt_fac_sh0[sharpness]; > - flt_config.fac_sh1 = filt_fac_sh1[sharpness]; > - flt_config.fac_mid = filt_fac_mid[sharpness]; > - flt_config.fac_bl0 = filt_fac_bl0[sharpness]; > - flt_config.fac_bl1 = filt_fac_bl1[sharpness]; > + auto config = params->block<Block::Flt>(RkISP1Params::State::Enable); > > - flt_config.lum_weight = kFiltLumWeightDefault; > - flt_config.mode = kFiltModeDefault; > - flt_config.thresh_sh0 = filt_thresh_sh0[denoise]; > - flt_config.thresh_sh1 = filt_thresh_sh1[denoise]; > - flt_config.thresh_bl0 = filt_thresh_bl0[denoise]; > - flt_config.thresh_bl1 = filt_thresh_bl1[denoise]; > - flt_config.grn_stage1 = stage1_select[denoise]; > - flt_config.chr_v_mode = filt_chr_v_mode[denoise]; > - flt_config.chr_h_mode = filt_chr_h_mode[denoise]; > + config->fac_sh0 = filt_fac_sh0[sharpness]; > + config->fac_sh1 = filt_fac_sh1[sharpness]; > + config->fac_mid = filt_fac_mid[sharpness]; > + config->fac_bl0 = filt_fac_bl0[sharpness]; > + config->fac_bl1 = filt_fac_bl1[sharpness]; > + > + config->lum_weight = kFiltLumWeightDefault; > + config->mode = kFiltModeDefault; > + config->thresh_sh0 = filt_thresh_sh0[denoise]; > + config->thresh_sh1 = filt_thresh_sh1[denoise]; > + config->thresh_bl0 = filt_thresh_bl0[denoise]; > + config->thresh_bl1 = filt_thresh_bl1[denoise]; > + config->grn_stage1 = stage1_select[denoise]; > + config->chr_v_mode = filt_chr_v_mode[denoise]; > + config->chr_h_mode = filt_chr_h_mode[denoise]; > > /* > * Combined high denoising and high sharpening requires some > @@ -186,27 +187,23 @@ void Filter::prepare([[maybe_unused]] IPAContext &context, > */ > if (denoise == 9) { > if (sharpness > 3) > - flt_config.grn_stage1 = 2; > + config->grn_stage1 = 2; > } else if (denoise == 10) { > if (sharpness > 5) > - flt_config.grn_stage1 = 2; > + config->grn_stage1 = 2; > else if (sharpness > 3) > - flt_config.grn_stage1 = 1; > + config->grn_stage1 = 1; > } > > if (denoise > 7) { > if (sharpness > 7) { > - flt_config.fac_bl0 /= 2; > - flt_config.fac_bl1 /= 4; > + config->fac_bl0 /= 2; > + config->fac_bl1 /= 4; > } else if (sharpness > 4) { > - flt_config.fac_bl0 = flt_config.fac_bl0 * 3 / 4; > - flt_config.fac_bl1 /= 2; > + config->fac_bl0 = config->fac_bl0 * 3 / 4; > + config->fac_bl1 /= 2; > } > } > - > - params->module_en_update |= RKISP1_CIF_ISP_MODULE_FLT; > - params->module_ens |= RKISP1_CIF_ISP_MODULE_FLT; > - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_FLT; > } > > REGISTER_IPA_ALGORITHM(Filter, "Filter") > diff --git a/src/ipa/rkisp1/algorithms/filter.h b/src/ipa/rkisp1/algorithms/filter.h > index d595811d455f..8f858e574fa2 100644 > --- a/src/ipa/rkisp1/algorithms/filter.h > +++ b/src/ipa/rkisp1/algorithms/filter.h > @@ -26,7 +26,7 @@ public: > const ControlList &controls) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) override; > + RkISP1Params *params) override; > }; > > } /* namespace ipa::rkisp1::algorithms */ > diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp > index a82cee3bbf61..9067bf34c581 100644 > --- a/src/ipa/rkisp1/algorithms/goc.cpp > +++ b/src/ipa/rkisp1/algorithms/goc.cpp > @@ -99,11 +99,14 @@ void GammaOutCorrection::queueRequest(IPAContext &context, const uint32_t frame, > void GammaOutCorrection::prepare(IPAContext &context, > [[maybe_unused]] const uint32_t frame, > IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) > + RkISP1Params *params) > { > ASSERT(context.hw->numGammaOutSamples == > RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10); > > + if (!frameContext.goc.update) > + return; > + > /* > * The logarithmic segments as specified in the reference. > * Plus an additional 0 to make the loop easier > @@ -112,10 +115,9 @@ void GammaOutCorrection::prepare(IPAContext &context, > 64, 64, 64, 64, 128, 128, 128, 128, 256, > 256, 256, 512, 512, 512, 512, 512, 0 > }; > - __u16 *gamma_y = params->others.goc_config.gamma_y; > > - if (!frameContext.goc.update) > - return; > + auto config = params->block<Block::Goc>(RkISP1Params::State::Enable); > + __u16 *gamma_y = config->gamma_y; > > unsigned x = 0; > for (const auto [i, size] : utils::enumerate(segments)) { > @@ -123,10 +125,7 @@ void GammaOutCorrection::prepare(IPAContext &context, > x += size; > } > > - params->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC; > - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC; > - params->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC; > - params->module_ens |= RKISP1_CIF_ISP_MODULE_GOC; > + config->mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC; > } > > /** > diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h > index 0e05d7ce4a01..bb2ddfc92375 100644 > --- a/src/ipa/rkisp1/algorithms/goc.h > +++ b/src/ipa/rkisp1/algorithms/goc.h > @@ -28,7 +28,7 @@ public: > const ControlList &controls) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) override; > + RkISP1Params *params) override; > void process(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > const rkisp1_stat_buffer *stats, > diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp > index 9b056c6edd96..f1880f16cc5d 100644 > --- a/src/ipa/rkisp1/algorithms/gsl.cpp > +++ b/src/ipa/rkisp1/algorithms/gsl.cpp > @@ -119,24 +119,18 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context, > void GammaSensorLinearization::prepare([[maybe_unused]] IPAContext &context, > const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) > + RkISP1Params *params) > { > if (frame > 0) > return; > > - params->others.sdg_config.xa_pnts.gamma_dx0 = gammaDx_[0]; > - params->others.sdg_config.xa_pnts.gamma_dx1 = gammaDx_[1]; > + auto config = params->block<Block::Sdg>(RkISP1Params::State::Enable); > + config->xa_pnts.gamma_dx0 = gammaDx_[0]; > + config->xa_pnts.gamma_dx1 = gammaDx_[1]; > > - std::copy(curveYr_.begin(), curveYr_.end(), > - params->others.sdg_config.curve_r.gamma_y); > - std::copy(curveYg_.begin(), curveYg_.end(), > - params->others.sdg_config.curve_g.gamma_y); > - std::copy(curveYb_.begin(), curveYb_.end(), > - params->others.sdg_config.curve_b.gamma_y); > - > - params->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG; > - params->module_ens |= RKISP1_CIF_ISP_MODULE_SDG; > - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG; > + std::copy(curveYr_.begin(), curveYr_.end(), config->curve_r.gamma_y); > + std::copy(curveYg_.begin(), curveYg_.end(), config->curve_g.gamma_y); > + std::copy(curveYb_.begin(), curveYb_.end(), config->curve_b.gamma_y); > } > > REGISTER_IPA_ALGORITHM(GammaSensorLinearization, "GammaSensorLinearization") > diff --git a/src/ipa/rkisp1/algorithms/gsl.h b/src/ipa/rkisp1/algorithms/gsl.h > index c404105e6310..91cf6efa7925 100644 > --- a/src/ipa/rkisp1/algorithms/gsl.h > +++ b/src/ipa/rkisp1/algorithms/gsl.h > @@ -22,7 +22,7 @@ public: > int init(IPAContext &context, const YamlObject &tuningData) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) override; > + RkISP1Params *params) override; > > private: > uint32_t gammaDx_[2]; > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp > index 161183fca352..81a50e5b4396 100644 > --- a/src/ipa/rkisp1/algorithms/lsc.cpp > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp > @@ -185,18 +185,12 @@ int LensShadingCorrection::configure(IPAContext &context, > return 0; > } > > -void LensShadingCorrection::setParameters(rkisp1_params_cfg *params) > +void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config) > { > - struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config; > - > memcpy(config.x_grad_tbl, xGrad_, sizeof(config.x_grad_tbl)); > memcpy(config.y_grad_tbl, yGrad_, sizeof(config.y_grad_tbl)); > memcpy(config.x_size_tbl, xSizes_, sizeof(config.x_size_tbl)); > memcpy(config.y_size_tbl, ySizes_, sizeof(config.y_size_tbl)); > - > - params->module_en_update |= RKISP1_CIF_ISP_MODULE_LSC; > - params->module_ens |= RKISP1_CIF_ISP_MODULE_LSC; > - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_LSC; > } > > void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config, > @@ -248,10 +242,8 @@ void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config, > void LensShadingCorrection::prepare(IPAContext &context, > const uint32_t frame, > [[maybe_unused]] IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) > + RkISP1Params *params) > { > - struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config; > - > /* > * If there is only one set, the configuration has already been done > * for first frame. > @@ -264,8 +256,10 @@ void LensShadingCorrection::prepare(IPAContext &context, > * never be relevant. > */ > if (sets_.size() == 1) { > - setParameters(params); > - copyTable(config, sets_.cbegin()->second); > + auto config = params->block<Block::Lsc>(RkISP1Params::State::Enable); > + > + setParameters(*config); > + copyTable(*config, sets_.cbegin()->second); > return; > } > > @@ -294,13 +288,14 @@ void LensShadingCorrection::prepare(IPAContext &context, > (lastCt_.adjusted <= ct && ct <= lastCt_.original)) > return; > > - setParameters(params); > + auto config = params->block<Block::Lsc>(RkISP1Params::State::Enable); > + setParameters(*config); > > /* > * The color temperature matches exactly one of the available LSC tables. > */ > if (sets_.count(ct)) { > - copyTable(config, sets_[ct]); > + copyTable(*config, sets_[ct]); > lastCt_ = { ct, ct }; > return; > } > @@ -319,7 +314,7 @@ void LensShadingCorrection::prepare(IPAContext &context, > if (diff0 < threshold || diff1 < threshold) { > const Components &set = diff0 < diff1 ? set0 : set1; > LOG(RkISP1Lsc, Debug) << "using LSC table for " << set.ct; > - copyTable(config, set); > + copyTable(*config, set); > lastCt_ = { ct, set.ct }; > return; > } > @@ -331,7 +326,7 @@ void LensShadingCorrection::prepare(IPAContext &context, > LOG(RkISP1Lsc, Debug) > << "ct is " << ct << ", interpolating between " > << ct0 << " and " << ct1; > - interpolateTable(config, set0, set1, ct); > + interpolateTable(*config, set0, set1, ct); > lastCt_ = { ct, ct }; > } > > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h > index 5baf592797a6..a9c7a230e0fc 100644 > --- a/src/ipa/rkisp1/algorithms/lsc.h > +++ b/src/ipa/rkisp1/algorithms/lsc.h > @@ -25,7 +25,7 @@ public: > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > - rkisp1_params_cfg *params) override; > + RkISP1Params *params) override; > > private: > struct Components { > @@ -36,7 +36,7 @@ private: > std::vector<uint16_t> b; > }; > > - void setParameters(rkisp1_params_cfg *params); > + void setParameters(rkisp1_cif_isp_lsc_config &config); > void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0); > void interpolateTable(rkisp1_cif_isp_lsc_config &config, > const Components &set0, const Components &set1, > diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h > index 16c3e43e88df..69e9bc823720 100644 > --- a/src/ipa/rkisp1/module.h > +++ b/src/ipa/rkisp1/module.h > @@ -14,13 +14,14 @@ > #include <libipa/module.h> > > #include "ipa_context.h" > +#include "params.h" > > namespace libcamera { > > namespace ipa::rkisp1 { > > using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, > - rkisp1_params_cfg, rkisp1_stat_buffer>; > + RkISP1Params, rkisp1_stat_buffer>; > > } /* namespace ipa::rkisp1 */ > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 84ffe6cf2777..1a89eabf10b4 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -31,6 +31,7 @@ > #include "algorithms/algorithm.h" > > #include "ipa_context.h" > +#include "params.h" > > namespace libcamera { > > @@ -322,17 +323,12 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) > { > IPAFrameContext &frameContext = context_.frameContexts.get(frame); > > - rkisp1_params_cfg *params = > - reinterpret_cast<rkisp1_params_cfg *>( > - mappedBuffers_.at(bufferId).planes()[0].data()); > - > - /* Prepare parameters buffer. */ > - memset(params, 0, sizeof(*params)); > + RkISP1Params params(paramFormat_, mappedBuffers_.at(bufferId).planes()[0]); > > for (auto const &algo : algorithms()) > - algo->prepare(context_, frame, frameContext, params); > + algo->prepare(context_, frame, frameContext, ¶ms); > > - paramsBufferReady.emit(frame, sizeof(*params)); > + paramsBufferReady.emit(frame, params.size()); > } > > void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId, > -- > Regards, > > Laurent Pinchart >
On Thu, Jul 04, 2024 at 02:33:19PM +0200, Jacopo Mondi wrote: > Hi Laurent > > only one comment on dpf which is the only one with a more convoluted > handling. The rest looks very nice! > > On Thu, Jul 04, 2024 at 01:52:25AM GMT, Laurent Pinchart wrote: > > Use the new ISP parameters abstraction class RkISP1Params to access the > > ISP parameters in the IPA algorithms. The class replaces the pointer to > > the rkisp1_params_cfg structure passed to the algorithms' prepare() > > function, and is used to access individual parameters blocks. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/ipa/rkisp1/algorithms/agc.cpp | 46 +++++++++++------------ > > src/ipa/rkisp1/algorithms/agc.h | 2 +- > > src/ipa/rkisp1/algorithms/awb.cpp | 56 ++++++++++++---------------- > > src/ipa/rkisp1/algorithms/awb.h | 2 +- > > src/ipa/rkisp1/algorithms/blc.cpp | 18 ++++----- > > src/ipa/rkisp1/algorithms/blc.h | 3 +- > > src/ipa/rkisp1/algorithms/ccm.cpp | 14 ++----- > > src/ipa/rkisp1/algorithms/ccm.h | 4 +- > > src/ipa/rkisp1/algorithms/cproc.cpp | 13 +++---- > > src/ipa/rkisp1/algorithms/cproc.h | 2 +- > > src/ipa/rkisp1/algorithms/dpcc.cpp | 9 ++--- > > src/ipa/rkisp1/algorithms/dpcc.h | 2 +- > > src/ipa/rkisp1/algorithms/dpf.cpp | 28 +++++++------- > > src/ipa/rkisp1/algorithms/dpf.h | 2 +- > > src/ipa/rkisp1/algorithms/filter.cpp | 51 ++++++++++++------------- > > src/ipa/rkisp1/algorithms/filter.h | 2 +- > > src/ipa/rkisp1/algorithms/goc.cpp | 15 ++++---- > > src/ipa/rkisp1/algorithms/goc.h | 2 +- > > src/ipa/rkisp1/algorithms/gsl.cpp | 20 ++++------ > > src/ipa/rkisp1/algorithms/gsl.h | 2 +- > > src/ipa/rkisp1/algorithms/lsc.cpp | 27 ++++++-------- > > src/ipa/rkisp1/algorithms/lsc.h | 4 +- > > src/ipa/rkisp1/module.h | 3 +- > > src/ipa/rkisp1/rkisp1.cpp | 12 ++---- > > 24 files changed, 148 insertions(+), 191 deletions(-) > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > [snip] > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp > > index abf957288550..b3f4446631fc 100644 > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp > > @@ -215,15 +215,26 @@ void Dpf::queueRequest(IPAContext &context, > > * \copydoc libcamera::ipa::Algorithm::prepare > > */ > > void Dpf::prepare(IPAContext &context, const uint32_t frame, > > - IPAFrameContext &frameContext, rkisp1_params_cfg *params) > > + IPAFrameContext &frameContext, RkISP1Params *params) > > { > > + if (!frameContext.dpf.update && frame > 0) > > + return; > > + > > + RkISP1Params::State state = frameContext.dpf.denoise > > + ? RkISP1Params::State::Enable > > + : RkISP1Params::State::Disable; > > + auto config = params->block<Block::Dpf>(state); > > + > > if (frame == 0) { > > - params->others.dpf_config = config_; > > - params->others.dpf_strength_config = strengthConfig_; > > + auto strengthConfig = params->block<Block::DpfStrength>(state); > > The dpf strength handler in the kernel ignores the enable state, and > strengths are configured only when frame == 0, so I would use Enable > here for clarity (even if it has not practical differences). OK. > > + *strengthConfig = strengthConfig_; > > + > > + *config = config_; > > This, however needs to be done everytime we enable the block. The > driver sees that state == enable and programs the dpf block with the > content of the config structure. If frame > 0 && > frameContext.dpf.denoise == true, the kernel will program the block > with 0-initialized data if I read it right. > > I would set *config = config_ for all frames if dpf.update && > dpf.denoise ? Yes there's an issue indeed. This leads to an annoying question: do we need the ability to enable/disable blocks without reconfiguring them ? > > > > const auto &awb = context.configuration.awb; > > const auto &lsc = context.configuration.lsc; > > - auto &mode = params->others.dpf_config.gain.mode; > > + > > + auto &mode = config->gain.mode; > > > > /* > > * The DPF needs to take into account the total amount of > > @@ -241,15 +252,6 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame, > > mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS; > > else > > mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED; > > - > > - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPF | > > - RKISP1_CIF_ISP_MODULE_DPF_STRENGTH; > > - } > > - > > - if (frameContext.dpf.update) { > > - params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF; > > - if (frameContext.dpf.denoise) > > - params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF; > > } > > } > > [snip]
Hi Laurent On Thu, Jul 04, 2024 at 04:38:34PM GMT, Laurent Pinchart wrote: > On Thu, Jul 04, 2024 at 02:33:19PM +0200, Jacopo Mondi wrote: > > Hi Laurent > > > > only one comment on dpf which is the only one with a more convoluted > > handling. The rest looks very nice! > > > > On Thu, Jul 04, 2024 at 01:52:25AM GMT, Laurent Pinchart wrote: > > > Use the new ISP parameters abstraction class RkISP1Params to access the > > > ISP parameters in the IPA algorithms. The class replaces the pointer to > > > the rkisp1_params_cfg structure passed to the algorithms' prepare() > > > function, and is used to access individual parameters blocks. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > src/ipa/rkisp1/algorithms/agc.cpp | 46 +++++++++++------------ > > > src/ipa/rkisp1/algorithms/agc.h | 2 +- > > > src/ipa/rkisp1/algorithms/awb.cpp | 56 ++++++++++++---------------- > > > src/ipa/rkisp1/algorithms/awb.h | 2 +- > > > src/ipa/rkisp1/algorithms/blc.cpp | 18 ++++----- > > > src/ipa/rkisp1/algorithms/blc.h | 3 +- > > > src/ipa/rkisp1/algorithms/ccm.cpp | 14 ++----- > > > src/ipa/rkisp1/algorithms/ccm.h | 4 +- > > > src/ipa/rkisp1/algorithms/cproc.cpp | 13 +++---- > > > src/ipa/rkisp1/algorithms/cproc.h | 2 +- > > > src/ipa/rkisp1/algorithms/dpcc.cpp | 9 ++--- > > > src/ipa/rkisp1/algorithms/dpcc.h | 2 +- > > > src/ipa/rkisp1/algorithms/dpf.cpp | 28 +++++++------- > > > src/ipa/rkisp1/algorithms/dpf.h | 2 +- > > > src/ipa/rkisp1/algorithms/filter.cpp | 51 ++++++++++++------------- > > > src/ipa/rkisp1/algorithms/filter.h | 2 +- > > > src/ipa/rkisp1/algorithms/goc.cpp | 15 ++++---- > > > src/ipa/rkisp1/algorithms/goc.h | 2 +- > > > src/ipa/rkisp1/algorithms/gsl.cpp | 20 ++++------ > > > src/ipa/rkisp1/algorithms/gsl.h | 2 +- > > > src/ipa/rkisp1/algorithms/lsc.cpp | 27 ++++++-------- > > > src/ipa/rkisp1/algorithms/lsc.h | 4 +- > > > src/ipa/rkisp1/module.h | 3 +- > > > src/ipa/rkisp1/rkisp1.cpp | 12 ++---- > > > 24 files changed, 148 insertions(+), 191 deletions(-) > > > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > > > [snip] > > > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp > > > index abf957288550..b3f4446631fc 100644 > > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp > > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp > > > @@ -215,15 +215,26 @@ void Dpf::queueRequest(IPAContext &context, > > > * \copydoc libcamera::ipa::Algorithm::prepare > > > */ > > > void Dpf::prepare(IPAContext &context, const uint32_t frame, > > > - IPAFrameContext &frameContext, rkisp1_params_cfg *params) > > > + IPAFrameContext &frameContext, RkISP1Params *params) > > > { > > > + if (!frameContext.dpf.update && frame > 0) > > > + return; > > > + > > > + RkISP1Params::State state = frameContext.dpf.denoise > > > + ? RkISP1Params::State::Enable > > > + : RkISP1Params::State::Disable; > > > + auto config = params->block<Block::Dpf>(state); > > > + > > > if (frame == 0) { > > > - params->others.dpf_config = config_; > > > - params->others.dpf_strength_config = strengthConfig_; > > > + auto strengthConfig = params->block<Block::DpfStrength>(state); > > > > The dpf strength handler in the kernel ignores the enable state, and > > strengths are configured only when frame == 0, so I would use Enable > > here for clarity (even if it has not practical differences). > > OK. > > > > + *strengthConfig = strengthConfig_; > > > + > > > + *config = config_; > > > > This, however needs to be done everytime we enable the block. The > > driver sees that state == enable and programs the dpf block with the > > content of the config structure. If frame > 0 && > > frameContext.dpf.denoise == true, the kernel will program the block > > with 0-initialized data if I read it right. > > > > I would set *config = config_ for all frames if dpf.update && > > dpf.denoise ? > > Yes there's an issue indeed. > > This leads to an annoying question: do we need the ability to > enable/disable blocks without reconfiguring them ? > This indeed is not possible with the current uAPI. However, if to do we need to expand the number of enablment states enum rkisp1_ext_params_block_enable { RKISP1_EXT_PARAMS_BLOCK_DISABLE, RKISP1_EXT_PARAMS_BLOCK_ENABLE, }; in example by adding an additional entry as enum rkisp1_ext_params_block_enable { RKISP1_EXT_PARAMS_BLOCK_DISABLE, RKISP1_EXT_PARAMS_BLOCK_ENABLE, RKISP1_EXT_PARAMS_BLOCK_ENABLE_NO_CONFIG, }; would could do so later without breaking the existing users > > > > > > const auto &awb = context.configuration.awb; > > > const auto &lsc = context.configuration.lsc; > > > - auto &mode = params->others.dpf_config.gain.mode; > > > + > > > + auto &mode = config->gain.mode; > > > > > > /* > > > * The DPF needs to take into account the total amount of > > > @@ -241,15 +252,6 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame, > > > mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS; > > > else > > > mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED; > > > - > > > - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPF | > > > - RKISP1_CIF_ISP_MODULE_DPF_STRENGTH; > > > - } > > > - > > > - if (frameContext.dpf.update) { > > > - params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF; > > > - if (frameContext.dpf.denoise) > > > - params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF; > > > } > > > } > > > > > [snip] > > -- > Regards, > > Laurent Pinchart
On Thu, Jul 04, 2024 at 04:14:19PM +0200, Jacopo Mondi wrote: > On Thu, Jul 04, 2024 at 04:38:34PM GMT, Laurent Pinchart wrote: > > On Thu, Jul 04, 2024 at 02:33:19PM +0200, Jacopo Mondi wrote: > > > Hi Laurent > > > > > > only one comment on dpf which is the only one with a more convoluted > > > handling. The rest looks very nice! > > > > > > On Thu, Jul 04, 2024 at 01:52:25AM GMT, Laurent Pinchart wrote: > > > > Use the new ISP parameters abstraction class RkISP1Params to access the > > > > ISP parameters in the IPA algorithms. The class replaces the pointer to > > > > the rkisp1_params_cfg structure passed to the algorithms' prepare() > > > > function, and is used to access individual parameters blocks. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > src/ipa/rkisp1/algorithms/agc.cpp | 46 +++++++++++------------ > > > > src/ipa/rkisp1/algorithms/agc.h | 2 +- > > > > src/ipa/rkisp1/algorithms/awb.cpp | 56 ++++++++++++---------------- > > > > src/ipa/rkisp1/algorithms/awb.h | 2 +- > > > > src/ipa/rkisp1/algorithms/blc.cpp | 18 ++++----- > > > > src/ipa/rkisp1/algorithms/blc.h | 3 +- > > > > src/ipa/rkisp1/algorithms/ccm.cpp | 14 ++----- > > > > src/ipa/rkisp1/algorithms/ccm.h | 4 +- > > > > src/ipa/rkisp1/algorithms/cproc.cpp | 13 +++---- > > > > src/ipa/rkisp1/algorithms/cproc.h | 2 +- > > > > src/ipa/rkisp1/algorithms/dpcc.cpp | 9 ++--- > > > > src/ipa/rkisp1/algorithms/dpcc.h | 2 +- > > > > src/ipa/rkisp1/algorithms/dpf.cpp | 28 +++++++------- > > > > src/ipa/rkisp1/algorithms/dpf.h | 2 +- > > > > src/ipa/rkisp1/algorithms/filter.cpp | 51 ++++++++++++------------- > > > > src/ipa/rkisp1/algorithms/filter.h | 2 +- > > > > src/ipa/rkisp1/algorithms/goc.cpp | 15 ++++---- > > > > src/ipa/rkisp1/algorithms/goc.h | 2 +- > > > > src/ipa/rkisp1/algorithms/gsl.cpp | 20 ++++------ > > > > src/ipa/rkisp1/algorithms/gsl.h | 2 +- > > > > src/ipa/rkisp1/algorithms/lsc.cpp | 27 ++++++-------- > > > > src/ipa/rkisp1/algorithms/lsc.h | 4 +- > > > > src/ipa/rkisp1/module.h | 3 +- > > > > src/ipa/rkisp1/rkisp1.cpp | 12 ++---- > > > > 24 files changed, 148 insertions(+), 191 deletions(-) > > > > > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > > > > > [snip] > > > > > > > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp > > > > index abf957288550..b3f4446631fc 100644 > > > > --- a/src/ipa/rkisp1/algorithms/dpf.cpp > > > > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp > > > > @@ -215,15 +215,26 @@ void Dpf::queueRequest(IPAContext &context, > > > > * \copydoc libcamera::ipa::Algorithm::prepare > > > > */ > > > > void Dpf::prepare(IPAContext &context, const uint32_t frame, > > > > - IPAFrameContext &frameContext, rkisp1_params_cfg *params) > > > > + IPAFrameContext &frameContext, RkISP1Params *params) > > > > { > > > > + if (!frameContext.dpf.update && frame > 0) > > > > + return; > > > > + > > > > + RkISP1Params::State state = frameContext.dpf.denoise > > > > + ? RkISP1Params::State::Enable > > > > + : RkISP1Params::State::Disable; > > > > + auto config = params->block<Block::Dpf>(state); > > > > + > > > > if (frame == 0) { > > > > - params->others.dpf_config = config_; > > > > - params->others.dpf_strength_config = strengthConfig_; > > > > + auto strengthConfig = params->block<Block::DpfStrength>(state); > > > > > > The dpf strength handler in the kernel ignores the enable state, and > > > strengths are configured only when frame == 0, so I would use Enable > > > here for clarity (even if it has not practical differences). > > > > OK. > > > > > > + *strengthConfig = strengthConfig_; > > > > + > > > > + *config = config_; > > > > > > This, however needs to be done everytime we enable the block. The > > > driver sees that state == enable and programs the dpf block with the > > > content of the config structure. If frame > 0 && > > > frameContext.dpf.denoise == true, the kernel will program the block > > > with 0-initialized data if I read it right. > > > > > > I would set *config = config_ for all frames if dpf.update && > > > dpf.denoise ? > > > > Yes there's an issue indeed. > > > > This leads to an annoying question: do we need the ability to > > enable/disable blocks without reconfiguring them ? > > This indeed is not possible with the current uAPI. However, if to do > we need to expand the number of enablment states > > enum rkisp1_ext_params_block_enable { > RKISP1_EXT_PARAMS_BLOCK_DISABLE, > RKISP1_EXT_PARAMS_BLOCK_ENABLE, > }; > > in example by adding an additional entry as > > enum rkisp1_ext_params_block_enable { > RKISP1_EXT_PARAMS_BLOCK_DISABLE, > RKISP1_EXT_PARAMS_BLOCK_ENABLE, > RKISP1_EXT_PARAMS_BLOCK_ENABLE_NO_CONFIG, > }; And we may need the ability to configure the block while keeping it disabled :-/ > would could do so later without breaking the existing users That, or we could omit the data after the header. It would decouple enabling (set through the enable flag) and config (set through inclusion of the data after the header). This should also not break existing users, although we may want to then drop the structures that wrap the header and configuration data. It may also be more troublesome for userspace to use, as code would need to know whether or not configuration data is needed at the time the block is allocated. We would need to prototype first. > > > > > > > > const auto &awb = context.configuration.awb; > > > > const auto &lsc = context.configuration.lsc; > > > > - auto &mode = params->others.dpf_config.gain.mode; > > > > + > > > > + auto &mode = config->gain.mode; > > > > > > > > /* > > > > * The DPF needs to take into account the total amount of > > > > @@ -241,15 +252,6 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame, > > > > mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS; > > > > else > > > > mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED; > > > > - > > > > - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPF | > > > > - RKISP1_CIF_ISP_MODULE_DPF_STRENGTH; > > > > - } > > > > - > > > > - if (frameContext.dpf.update) { > > > > - params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF; > > > > - if (frameContext.dpf.denoise) > > > > - params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF; > > > > } > > > > } > > > > > > > > [snip]
diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index f12f8b60de15..32db07912695 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -281,7 +281,7 @@ void Agc::queueRequest(IPAContext &context, * \copydoc libcamera::ipa::Algorithm::prepare */ void Agc::prepare(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, rkisp1_params_cfg *params) + IPAFrameContext &frameContext, RkISP1Params *params) { if (frameContext.agc.autoEnabled) { frameContext.agc.exposure = context.activeState.agc.automatic.exposure; @@ -291,41 +291,37 @@ void Agc::prepare(IPAContext &context, const uint32_t frame, if (frame > 0 && !frameContext.agc.updateMetering) return; - /* Configure the measurement window. */ - params->meas.aec_config.meas_window = context.configuration.agc.measureWindow; - /* Use a continuous method for measure. */ - params->meas.aec_config.autostop = RKISP1_CIF_ISP_EXP_CTRL_AUTOSTOP_0; - /* Estimate Y as (R + G + B) x (85/256). */ - params->meas.aec_config.mode = RKISP1_CIF_ISP_EXP_MEASURING_MODE_1; + /* + * Configure the AEC measurements. Set the window, measure + * continuously, and estimate Y as (R + G + B) x (85/256). + */ + auto aecConfig = params->block<Block::Aec>(RkISP1Params::State::Enable); - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AEC; - params->module_ens |= RKISP1_CIF_ISP_MODULE_AEC; - params->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC; + aecConfig->meas_window = context.configuration.agc.measureWindow; + aecConfig->autostop = RKISP1_CIF_ISP_EXP_CTRL_AUTOSTOP_0; + aecConfig->mode = RKISP1_CIF_ISP_EXP_MEASURING_MODE_1; - /* Configure histogram. */ - params->meas.hst_config.meas_window = context.configuration.agc.measureWindow; - /* Produce the luminance histogram. */ - params->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM; + /* + * Configure the histogram measurement. Set the window, produce a + * luminance histogram, and set the weights and predivider. + */ + auto hstConfig = params->block<Block::Hst>(RkISP1Params::State::Enable); + + hstConfig->meas_window = context.configuration.agc.measureWindow; + hstConfig->mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM; - /* Set an average weighted histogram. */ Span<uint8_t> weights{ - params->meas.hst_config.hist_weight, + hstConfig->hist_weight, context.hw->numHistogramWeights }; std::vector<uint8_t> &modeWeights = meteringModes_.at(frameContext.agc.meteringMode); std::copy(modeWeights.begin(), modeWeights.end(), weights.begin()); - struct rkisp1_cif_isp_window window = params->meas.hst_config.meas_window; + struct rkisp1_cif_isp_window window = hstConfig->meas_window; Size windowSize = { window.h_size, window.v_size }; - params->meas.hst_config.histogram_predivider = + hstConfig->histogram_predivider = computeHistogramPredivider(windowSize, - static_cast<rkisp1_cif_isp_histogram_mode>(params->meas.hst_config.mode)); - - /* Update the configuration for histogram. */ - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST; - /* Enable the histogram measure unit. */ - params->module_ens |= RKISP1_CIF_ISP_MODULE_HST; - params->module_en_update |= RKISP1_CIF_ISP_MODULE_HST; + static_cast<rkisp1_cif_isp_histogram_mode>(hstConfig->mode)); } void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext, diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h index 9ceaa82b099e..d64ff42c1665 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -37,7 +37,7 @@ public: const ControlList &controls) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats, diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index a01fe5d90973..ef916e22602f 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -108,7 +108,7 @@ void Awb::queueRequest(IPAContext &context, * \copydoc libcamera::ipa::Algorithm::prepare */ void Awb::prepare(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, rkisp1_params_cfg *params) + IPAFrameContext &frameContext, RkISP1Params *params) { /* * This is the latest time we can read the active state. This is the @@ -120,29 +120,28 @@ void Awb::prepare(IPAContext &context, const uint32_t frame, frameContext.awb.gains.blue = context.activeState.awb.gains.automatic.blue; } - params->others.awb_gain_config.gain_green_b = 256 * frameContext.awb.gains.green; - params->others.awb_gain_config.gain_blue = 256 * frameContext.awb.gains.blue; - params->others.awb_gain_config.gain_red = 256 * frameContext.awb.gains.red; - params->others.awb_gain_config.gain_green_r = 256 * frameContext.awb.gains.green; + auto gainConfig = params->block<Block::AwbGain>(RkISP1Params::State::Enable); - /* Update the gains. */ - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; + gainConfig->gain_green_b = 256 * frameContext.awb.gains.green; + gainConfig->gain_blue = 256 * frameContext.awb.gains.blue; + gainConfig->gain_red = 256 * frameContext.awb.gains.red; + gainConfig->gain_green_r = 256 * frameContext.awb.gains.green; /* If we have already set the AWB measurement parameters, return. */ if (frame > 0) return; - rkisp1_cif_isp_awb_meas_config &awb_config = params->meas.awb_meas_config; + auto awbConfig = params->block<Block::Awb>(RkISP1Params::State::Enable); /* Configure the measure window for AWB. */ - awb_config.awb_wnd = context.configuration.awb.measureWindow; + awbConfig->awb_wnd = context.configuration.awb.measureWindow; /* Number of frames to use to estimate the means (0 means 1 frame). */ - awb_config.frames = 0; + awbConfig->frames = 0; /* Select RGB or YCbCr means measurement. */ if (rgbMode_) { - awb_config.awb_mode = RKISP1_CIF_ISP_AWB_MODE_RGB; + awbConfig->awb_mode = RKISP1_CIF_ISP_AWB_MODE_RGB; /* * For RGB-based measurements, pixels are selected with maximum @@ -150,19 +149,19 @@ void Awb::prepare(IPAContext &context, const uint32_t frame, * awb_ref_cr, awb_min_y and awb_ref_cb respectively. The other * values are not used, set them to 0. */ - awb_config.awb_ref_cr = 250; - awb_config.min_y = 250; - awb_config.awb_ref_cb = 250; + awbConfig->awb_ref_cr = 250; + awbConfig->min_y = 250; + awbConfig->awb_ref_cb = 250; - awb_config.max_y = 0; - awb_config.min_c = 0; - awb_config.max_csum = 0; + awbConfig->max_y = 0; + awbConfig->min_c = 0; + awbConfig->max_csum = 0; } else { - awb_config.awb_mode = RKISP1_CIF_ISP_AWB_MODE_YCBCR; + awbConfig->awb_mode = RKISP1_CIF_ISP_AWB_MODE_YCBCR; /* Set the reference Cr and Cb (AWB target) to white. */ - awb_config.awb_ref_cb = 128; - awb_config.awb_ref_cr = 128; + awbConfig->awb_ref_cb = 128; + awbConfig->awb_ref_cr = 128; /* * Filter out pixels based on luminance and chrominance values. @@ -170,20 +169,11 @@ void Awb::prepare(IPAContext &context, const uint32_t frame, * range, while the acceptable chroma values are specified with * a minimum of 16 and a maximum Cb+Cr sum of 250. */ - awb_config.min_y = 16; - awb_config.max_y = 250; - awb_config.min_c = 16; - awb_config.max_csum = 250; + awbConfig->min_y = 16; + awbConfig->max_y = 250; + awbConfig->min_c = 16; + awbConfig->max_csum = 250; } - - /* Enable the AWB gains. */ - params->module_en_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; - params->module_ens |= RKISP1_CIF_ISP_MODULE_AWB_GAIN; - - /* Update the AWB measurement parameters and enable the AWB module. */ - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB; - params->module_en_update |= RKISP1_CIF_ISP_MODULE_AWB; - params->module_ens |= RKISP1_CIF_ISP_MODULE_AWB; } uint32_t Awb::estimateCCT(double red, double green, double blue) diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h index 06c92896e2dc..b3b2c0bbb9ae 100644 --- a/src/ipa/rkisp1/algorithms/awb.h +++ b/src/ipa/rkisp1/algorithms/awb.h @@ -25,7 +25,7 @@ public: const ControlList &controls) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats, diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp index 871dd2047c6a..464a0e066dd4 100644 --- a/src/ipa/rkisp1/algorithms/blc.cpp +++ b/src/ipa/rkisp1/algorithms/blc.cpp @@ -113,7 +113,7 @@ int BlackLevelCorrection::init(IPAContext &context, const YamlObject &tuningData void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context, const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, - rkisp1_params_cfg *params) + RkISP1Params *params) { if (context.configuration.raw) return; @@ -124,16 +124,14 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context, if (!tuningParameters_) return; - params->others.bls_config.enable_auto = 0; - /* The rkisp1 uses 12bit based black levels. Scale down accordingly. */ - params->others.bls_config.fixed_val.r = blackLevelRed_ >> 4; - params->others.bls_config.fixed_val.gr = blackLevelGreenR_ >> 4; - params->others.bls_config.fixed_val.gb = blackLevelGreenB_ >> 4; - params->others.bls_config.fixed_val.b = blackLevelBlue_ >> 4; + auto config = params->block<Block::Bls>(RkISP1Params::State::Enable); - params->module_en_update |= RKISP1_CIF_ISP_MODULE_BLS; - params->module_ens |= RKISP1_CIF_ISP_MODULE_BLS; - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS; + config->enable_auto = 0; + /* The rkisp1 uses 12bit based black levels. Scale down accordingly. */ + config->fixed_val.r = blackLevelRed_ >> 4; + config->fixed_val.gr = blackLevelGreenR_ >> 4; + config->fixed_val.gb = blackLevelGreenB_ >> 4; + config->fixed_val.b = blackLevelBlue_ >> 4; } /** diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h index 4ecac233f88b..372f6f7d00cc 100644 --- a/src/ipa/rkisp1/algorithms/blc.h +++ b/src/ipa/rkisp1/algorithms/blc.h @@ -22,11 +22,12 @@ public: int init(IPAContext &context, const YamlObject &tuningData) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats, ControlList &metadata) override; + private: bool tuningParameters_; int16_t blackLevelRed_; diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp index c1f5403a8ab2..55bf21516c89 100644 --- a/src/ipa/rkisp1/algorithms/ccm.cpp +++ b/src/ipa/rkisp1/algorithms/ccm.cpp @@ -71,12 +71,10 @@ int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData return 0; } -void Ccm::setParameters(rkisp1_params_cfg *params, +void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config, const Matrix<float, 3, 3> &matrix, const Matrix<int16_t, 3, 1> &offsets) { - struct rkisp1_cif_isp_ctk_config &config = params->others.ctk_config; - /* * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to * +7.992 (0x3ff) @@ -92,18 +90,13 @@ void Ccm::setParameters(rkisp1_params_cfg *params, LOG(RkISP1Ccm, Debug) << "Setting matrix " << matrix; LOG(RkISP1Ccm, Debug) << "Setting offsets " << offsets; - - params->module_en_update |= RKISP1_CIF_ISP_MODULE_CTK; - params->module_ens |= RKISP1_CIF_ISP_MODULE_CTK; - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_CTK; } /** * \copydoc libcamera::ipa::Algorithm::prepare */ void Ccm::prepare(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, - rkisp1_params_cfg *params) + IPAFrameContext &frameContext, RkISP1Params *params) { uint32_t ct = context.activeState.awb.temperatureK; @@ -120,7 +113,8 @@ void Ccm::prepare(IPAContext &context, const uint32_t frame, frameContext.ccm.ccm = ccm; - setParameters(params, ccm, offsets); + auto config = params->block<Block::Ctk>(RkISP1Params::State::Enable); + setParameters(*config, ccm, offsets); } /** diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h index 30cb882180cc..9daadb6834b1 100644 --- a/src/ipa/rkisp1/algorithms/ccm.h +++ b/src/ipa/rkisp1/algorithms/ccm.h @@ -27,7 +27,7 @@ public: int init(IPAContext &context, const YamlObject &tuningData) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats, @@ -35,7 +35,7 @@ public: private: void parseYaml(const YamlObject &tuningData); - void setParameters(rkisp1_params_cfg *params, + void setParameters(struct rkisp1_cif_isp_ctk_config &config, const Matrix<float, 3, 3> &matrix, const Matrix<int16_t, 3, 1> &offsets); diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp index ef0931b20453..e850dfabc0a5 100644 --- a/src/ipa/rkisp1/algorithms/cproc.cpp +++ b/src/ipa/rkisp1/algorithms/cproc.cpp @@ -140,19 +140,16 @@ void ColorProcessing::queueRequest(IPAContext &context, void ColorProcessing::prepare([[maybe_unused]] IPAContext &context, [[maybe_unused]] const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) + RkISP1Params *params) { /* Check if the algorithm configuration has been updated. */ if (!frameContext.cproc.update) return; - params->others.cproc_config.brightness = frameContext.cproc.brightness; - params->others.cproc_config.contrast = frameContext.cproc.contrast; - params->others.cproc_config.sat = frameContext.cproc.saturation; - - params->module_en_update |= RKISP1_CIF_ISP_MODULE_CPROC; - params->module_ens |= RKISP1_CIF_ISP_MODULE_CPROC; - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_CPROC; + auto config = params->block<Block::Cproc>(RkISP1Params::State::Enable); + config->brightness = frameContext.cproc.brightness; + config->contrast = frameContext.cproc.contrast; + config->sat = frameContext.cproc.saturation; } REGISTER_IPA_ALGORITHM(ColorProcessing, "ColorProcessing") diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h index e50e7200bd73..fd38fd17e8bb 100644 --- a/src/ipa/rkisp1/algorithms/cproc.h +++ b/src/ipa/rkisp1/algorithms/cproc.h @@ -29,7 +29,7 @@ public: const ControlList &controls) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; }; } /* namespace ipa::rkisp1::algorithms */ diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp index b5a339e9137f..0b13bfac2a67 100644 --- a/src/ipa/rkisp1/algorithms/dpcc.cpp +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp @@ -232,16 +232,13 @@ int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context, void DefectPixelClusterCorrection::prepare([[maybe_unused]] IPAContext &context, const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, - rkisp1_params_cfg *params) + RkISP1Params *params) { if (frame > 0) return; - params->others.dpcc_config = config_; - - params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPCC; - params->module_ens |= RKISP1_CIF_ISP_MODULE_DPCC; - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPCC; + auto config = params->block<Block::Dpcc>(RkISP1Params::State::Enable); + *config = config_; } REGISTER_IPA_ALGORITHM(DefectPixelClusterCorrection, "DefectPixelClusterCorrection") diff --git a/src/ipa/rkisp1/algorithms/dpcc.h b/src/ipa/rkisp1/algorithms/dpcc.h index d39b7bedc1e1..b77766c300fb 100644 --- a/src/ipa/rkisp1/algorithms/dpcc.h +++ b/src/ipa/rkisp1/algorithms/dpcc.h @@ -22,7 +22,7 @@ public: int init(IPAContext &context, const YamlObject &tuningData) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; private: rkisp1_cif_isp_dpcc_config config_; diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp index abf957288550..b3f4446631fc 100644 --- a/src/ipa/rkisp1/algorithms/dpf.cpp +++ b/src/ipa/rkisp1/algorithms/dpf.cpp @@ -215,15 +215,26 @@ void Dpf::queueRequest(IPAContext &context, * \copydoc libcamera::ipa::Algorithm::prepare */ void Dpf::prepare(IPAContext &context, const uint32_t frame, - IPAFrameContext &frameContext, rkisp1_params_cfg *params) + IPAFrameContext &frameContext, RkISP1Params *params) { + if (!frameContext.dpf.update && frame > 0) + return; + + RkISP1Params::State state = frameContext.dpf.denoise + ? RkISP1Params::State::Enable + : RkISP1Params::State::Disable; + auto config = params->block<Block::Dpf>(state); + if (frame == 0) { - params->others.dpf_config = config_; - params->others.dpf_strength_config = strengthConfig_; + auto strengthConfig = params->block<Block::DpfStrength>(state); + *strengthConfig = strengthConfig_; + + *config = config_; const auto &awb = context.configuration.awb; const auto &lsc = context.configuration.lsc; - auto &mode = params->others.dpf_config.gain.mode; + + auto &mode = config->gain.mode; /* * The DPF needs to take into account the total amount of @@ -241,15 +252,6 @@ void Dpf::prepare(IPAContext &context, const uint32_t frame, mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS; else mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED; - - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPF | - RKISP1_CIF_ISP_MODULE_DPF_STRENGTH; - } - - if (frameContext.dpf.update) { - params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF; - if (frameContext.dpf.denoise) - params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF; } } diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h index da0115baf8f1..2dd8cd362624 100644 --- a/src/ipa/rkisp1/algorithms/dpf.h +++ b/src/ipa/rkisp1/algorithms/dpf.h @@ -27,7 +27,7 @@ public: const ControlList &controls) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; private: struct rkisp1_cif_isp_dpf_config config_; diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp index 9752248a5965..242b781a10f6 100644 --- a/src/ipa/rkisp1/algorithms/filter.cpp +++ b/src/ipa/rkisp1/algorithms/filter.cpp @@ -104,7 +104,7 @@ void Filter::queueRequest(IPAContext &context, */ void Filter::prepare([[maybe_unused]] IPAContext &context, [[maybe_unused]] const uint32_t frame, - IPAFrameContext &frameContext, rkisp1_params_cfg *params) + IPAFrameContext &frameContext, RkISP1Params *params) { /* Check if the algorithm configuration has been updated. */ if (!frameContext.filter.update) @@ -160,23 +160,24 @@ void Filter::prepare([[maybe_unused]] IPAContext &context, uint8_t denoise = frameContext.filter.denoise; uint8_t sharpness = frameContext.filter.sharpness; - auto &flt_config = params->others.flt_config; - flt_config.fac_sh0 = filt_fac_sh0[sharpness]; - flt_config.fac_sh1 = filt_fac_sh1[sharpness]; - flt_config.fac_mid = filt_fac_mid[sharpness]; - flt_config.fac_bl0 = filt_fac_bl0[sharpness]; - flt_config.fac_bl1 = filt_fac_bl1[sharpness]; + auto config = params->block<Block::Flt>(RkISP1Params::State::Enable); - flt_config.lum_weight = kFiltLumWeightDefault; - flt_config.mode = kFiltModeDefault; - flt_config.thresh_sh0 = filt_thresh_sh0[denoise]; - flt_config.thresh_sh1 = filt_thresh_sh1[denoise]; - flt_config.thresh_bl0 = filt_thresh_bl0[denoise]; - flt_config.thresh_bl1 = filt_thresh_bl1[denoise]; - flt_config.grn_stage1 = stage1_select[denoise]; - flt_config.chr_v_mode = filt_chr_v_mode[denoise]; - flt_config.chr_h_mode = filt_chr_h_mode[denoise]; + config->fac_sh0 = filt_fac_sh0[sharpness]; + config->fac_sh1 = filt_fac_sh1[sharpness]; + config->fac_mid = filt_fac_mid[sharpness]; + config->fac_bl0 = filt_fac_bl0[sharpness]; + config->fac_bl1 = filt_fac_bl1[sharpness]; + + config->lum_weight = kFiltLumWeightDefault; + config->mode = kFiltModeDefault; + config->thresh_sh0 = filt_thresh_sh0[denoise]; + config->thresh_sh1 = filt_thresh_sh1[denoise]; + config->thresh_bl0 = filt_thresh_bl0[denoise]; + config->thresh_bl1 = filt_thresh_bl1[denoise]; + config->grn_stage1 = stage1_select[denoise]; + config->chr_v_mode = filt_chr_v_mode[denoise]; + config->chr_h_mode = filt_chr_h_mode[denoise]; /* * Combined high denoising and high sharpening requires some @@ -186,27 +187,23 @@ void Filter::prepare([[maybe_unused]] IPAContext &context, */ if (denoise == 9) { if (sharpness > 3) - flt_config.grn_stage1 = 2; + config->grn_stage1 = 2; } else if (denoise == 10) { if (sharpness > 5) - flt_config.grn_stage1 = 2; + config->grn_stage1 = 2; else if (sharpness > 3) - flt_config.grn_stage1 = 1; + config->grn_stage1 = 1; } if (denoise > 7) { if (sharpness > 7) { - flt_config.fac_bl0 /= 2; - flt_config.fac_bl1 /= 4; + config->fac_bl0 /= 2; + config->fac_bl1 /= 4; } else if (sharpness > 4) { - flt_config.fac_bl0 = flt_config.fac_bl0 * 3 / 4; - flt_config.fac_bl1 /= 2; + config->fac_bl0 = config->fac_bl0 * 3 / 4; + config->fac_bl1 /= 2; } } - - params->module_en_update |= RKISP1_CIF_ISP_MODULE_FLT; - params->module_ens |= RKISP1_CIF_ISP_MODULE_FLT; - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_FLT; } REGISTER_IPA_ALGORITHM(Filter, "Filter") diff --git a/src/ipa/rkisp1/algorithms/filter.h b/src/ipa/rkisp1/algorithms/filter.h index d595811d455f..8f858e574fa2 100644 --- a/src/ipa/rkisp1/algorithms/filter.h +++ b/src/ipa/rkisp1/algorithms/filter.h @@ -26,7 +26,7 @@ public: const ControlList &controls) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; }; } /* namespace ipa::rkisp1::algorithms */ diff --git a/src/ipa/rkisp1/algorithms/goc.cpp b/src/ipa/rkisp1/algorithms/goc.cpp index a82cee3bbf61..9067bf34c581 100644 --- a/src/ipa/rkisp1/algorithms/goc.cpp +++ b/src/ipa/rkisp1/algorithms/goc.cpp @@ -99,11 +99,14 @@ void GammaOutCorrection::queueRequest(IPAContext &context, const uint32_t frame, void GammaOutCorrection::prepare(IPAContext &context, [[maybe_unused]] const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) + RkISP1Params *params) { ASSERT(context.hw->numGammaOutSamples == RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10); + if (!frameContext.goc.update) + return; + /* * The logarithmic segments as specified in the reference. * Plus an additional 0 to make the loop easier @@ -112,10 +115,9 @@ void GammaOutCorrection::prepare(IPAContext &context, 64, 64, 64, 64, 128, 128, 128, 128, 256, 256, 256, 512, 512, 512, 512, 512, 0 }; - __u16 *gamma_y = params->others.goc_config.gamma_y; - if (!frameContext.goc.update) - return; + auto config = params->block<Block::Goc>(RkISP1Params::State::Enable); + __u16 *gamma_y = config->gamma_y; unsigned x = 0; for (const auto [i, size] : utils::enumerate(segments)) { @@ -123,10 +125,7 @@ void GammaOutCorrection::prepare(IPAContext &context, x += size; } - params->others.goc_config.mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC; - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_GOC; - params->module_en_update |= RKISP1_CIF_ISP_MODULE_GOC; - params->module_ens |= RKISP1_CIF_ISP_MODULE_GOC; + config->mode = RKISP1_CIF_ISP_GOC_MODE_LOGARITHMIC; } /** diff --git a/src/ipa/rkisp1/algorithms/goc.h b/src/ipa/rkisp1/algorithms/goc.h index 0e05d7ce4a01..bb2ddfc92375 100644 --- a/src/ipa/rkisp1/algorithms/goc.h +++ b/src/ipa/rkisp1/algorithms/goc.h @@ -28,7 +28,7 @@ public: const ControlList &controls) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats, diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp index 9b056c6edd96..f1880f16cc5d 100644 --- a/src/ipa/rkisp1/algorithms/gsl.cpp +++ b/src/ipa/rkisp1/algorithms/gsl.cpp @@ -119,24 +119,18 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context, void GammaSensorLinearization::prepare([[maybe_unused]] IPAContext &context, const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, - rkisp1_params_cfg *params) + RkISP1Params *params) { if (frame > 0) return; - params->others.sdg_config.xa_pnts.gamma_dx0 = gammaDx_[0]; - params->others.sdg_config.xa_pnts.gamma_dx1 = gammaDx_[1]; + auto config = params->block<Block::Sdg>(RkISP1Params::State::Enable); + config->xa_pnts.gamma_dx0 = gammaDx_[0]; + config->xa_pnts.gamma_dx1 = gammaDx_[1]; - std::copy(curveYr_.begin(), curveYr_.end(), - params->others.sdg_config.curve_r.gamma_y); - std::copy(curveYg_.begin(), curveYg_.end(), - params->others.sdg_config.curve_g.gamma_y); - std::copy(curveYb_.begin(), curveYb_.end(), - params->others.sdg_config.curve_b.gamma_y); - - params->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG; - params->module_ens |= RKISP1_CIF_ISP_MODULE_SDG; - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG; + std::copy(curveYr_.begin(), curveYr_.end(), config->curve_r.gamma_y); + std::copy(curveYg_.begin(), curveYg_.end(), config->curve_g.gamma_y); + std::copy(curveYb_.begin(), curveYb_.end(), config->curve_b.gamma_y); } REGISTER_IPA_ALGORITHM(GammaSensorLinearization, "GammaSensorLinearization") diff --git a/src/ipa/rkisp1/algorithms/gsl.h b/src/ipa/rkisp1/algorithms/gsl.h index c404105e6310..91cf6efa7925 100644 --- a/src/ipa/rkisp1/algorithms/gsl.h +++ b/src/ipa/rkisp1/algorithms/gsl.h @@ -22,7 +22,7 @@ public: int init(IPAContext &context, const YamlObject &tuningData) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; private: uint32_t gammaDx_[2]; diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp index 161183fca352..81a50e5b4396 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -185,18 +185,12 @@ int LensShadingCorrection::configure(IPAContext &context, return 0; } -void LensShadingCorrection::setParameters(rkisp1_params_cfg *params) +void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config) { - struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config; - memcpy(config.x_grad_tbl, xGrad_, sizeof(config.x_grad_tbl)); memcpy(config.y_grad_tbl, yGrad_, sizeof(config.y_grad_tbl)); memcpy(config.x_size_tbl, xSizes_, sizeof(config.x_size_tbl)); memcpy(config.y_size_tbl, ySizes_, sizeof(config.y_size_tbl)); - - params->module_en_update |= RKISP1_CIF_ISP_MODULE_LSC; - params->module_ens |= RKISP1_CIF_ISP_MODULE_LSC; - params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_LSC; } void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config, @@ -248,10 +242,8 @@ void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config, void LensShadingCorrection::prepare(IPAContext &context, const uint32_t frame, [[maybe_unused]] IPAFrameContext &frameContext, - rkisp1_params_cfg *params) + RkISP1Params *params) { - struct rkisp1_cif_isp_lsc_config &config = params->others.lsc_config; - /* * If there is only one set, the configuration has already been done * for first frame. @@ -264,8 +256,10 @@ void LensShadingCorrection::prepare(IPAContext &context, * never be relevant. */ if (sets_.size() == 1) { - setParameters(params); - copyTable(config, sets_.cbegin()->second); + auto config = params->block<Block::Lsc>(RkISP1Params::State::Enable); + + setParameters(*config); + copyTable(*config, sets_.cbegin()->second); return; } @@ -294,13 +288,14 @@ void LensShadingCorrection::prepare(IPAContext &context, (lastCt_.adjusted <= ct && ct <= lastCt_.original)) return; - setParameters(params); + auto config = params->block<Block::Lsc>(RkISP1Params::State::Enable); + setParameters(*config); /* * The color temperature matches exactly one of the available LSC tables. */ if (sets_.count(ct)) { - copyTable(config, sets_[ct]); + copyTable(*config, sets_[ct]); lastCt_ = { ct, ct }; return; } @@ -319,7 +314,7 @@ void LensShadingCorrection::prepare(IPAContext &context, if (diff0 < threshold || diff1 < threshold) { const Components &set = diff0 < diff1 ? set0 : set1; LOG(RkISP1Lsc, Debug) << "using LSC table for " << set.ct; - copyTable(config, set); + copyTable(*config, set); lastCt_ = { ct, set.ct }; return; } @@ -331,7 +326,7 @@ void LensShadingCorrection::prepare(IPAContext &context, LOG(RkISP1Lsc, Debug) << "ct is " << ct << ", interpolating between " << ct0 << " and " << ct1; - interpolateTable(config, set0, set1, ct); + interpolateTable(*config, set0, set1, ct); lastCt_ = { ct, ct }; } diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h index 5baf592797a6..a9c7a230e0fc 100644 --- a/src/ipa/rkisp1/algorithms/lsc.h +++ b/src/ipa/rkisp1/algorithms/lsc.h @@ -25,7 +25,7 @@ public: int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, - rkisp1_params_cfg *params) override; + RkISP1Params *params) override; private: struct Components { @@ -36,7 +36,7 @@ private: std::vector<uint16_t> b; }; - void setParameters(rkisp1_params_cfg *params); + void setParameters(rkisp1_cif_isp_lsc_config &config); void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0); void interpolateTable(rkisp1_cif_isp_lsc_config &config, const Components &set0, const Components &set1, diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h index 16c3e43e88df..69e9bc823720 100644 --- a/src/ipa/rkisp1/module.h +++ b/src/ipa/rkisp1/module.h @@ -14,13 +14,14 @@ #include <libipa/module.h> #include "ipa_context.h" +#include "params.h" namespace libcamera { namespace ipa::rkisp1 { using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo, - rkisp1_params_cfg, rkisp1_stat_buffer>; + RkISP1Params, rkisp1_stat_buffer>; } /* namespace ipa::rkisp1 */ diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 84ffe6cf2777..1a89eabf10b4 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -31,6 +31,7 @@ #include "algorithms/algorithm.h" #include "ipa_context.h" +#include "params.h" namespace libcamera { @@ -322,17 +323,12 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) { IPAFrameContext &frameContext = context_.frameContexts.get(frame); - rkisp1_params_cfg *params = - reinterpret_cast<rkisp1_params_cfg *>( - mappedBuffers_.at(bufferId).planes()[0].data()); - - /* Prepare parameters buffer. */ - memset(params, 0, sizeof(*params)); + RkISP1Params params(paramFormat_, mappedBuffers_.at(bufferId).planes()[0]); for (auto const &algo : algorithms()) - algo->prepare(context_, frame, frameContext, params); + algo->prepare(context_, frame, frameContext, ¶ms); - paramsBufferReady.emit(frame, sizeof(*params)); + paramsBufferReady.emit(frame, params.size()); } void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,
Use the new ISP parameters abstraction class RkISP1Params to access the ISP parameters in the IPA algorithms. The class replaces the pointer to the rkisp1_params_cfg structure passed to the algorithms' prepare() function, and is used to access individual parameters blocks. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/ipa/rkisp1/algorithms/agc.cpp | 46 +++++++++++------------ src/ipa/rkisp1/algorithms/agc.h | 2 +- src/ipa/rkisp1/algorithms/awb.cpp | 56 ++++++++++++---------------- src/ipa/rkisp1/algorithms/awb.h | 2 +- src/ipa/rkisp1/algorithms/blc.cpp | 18 ++++----- src/ipa/rkisp1/algorithms/blc.h | 3 +- src/ipa/rkisp1/algorithms/ccm.cpp | 14 ++----- src/ipa/rkisp1/algorithms/ccm.h | 4 +- src/ipa/rkisp1/algorithms/cproc.cpp | 13 +++---- src/ipa/rkisp1/algorithms/cproc.h | 2 +- src/ipa/rkisp1/algorithms/dpcc.cpp | 9 ++--- src/ipa/rkisp1/algorithms/dpcc.h | 2 +- src/ipa/rkisp1/algorithms/dpf.cpp | 28 +++++++------- src/ipa/rkisp1/algorithms/dpf.h | 2 +- src/ipa/rkisp1/algorithms/filter.cpp | 51 ++++++++++++------------- src/ipa/rkisp1/algorithms/filter.h | 2 +- src/ipa/rkisp1/algorithms/goc.cpp | 15 ++++---- src/ipa/rkisp1/algorithms/goc.h | 2 +- src/ipa/rkisp1/algorithms/gsl.cpp | 20 ++++------ src/ipa/rkisp1/algorithms/gsl.h | 2 +- src/ipa/rkisp1/algorithms/lsc.cpp | 27 ++++++-------- src/ipa/rkisp1/algorithms/lsc.h | 4 +- src/ipa/rkisp1/module.h | 3 +- src/ipa/rkisp1/rkisp1.cpp | 12 ++---- 24 files changed, 148 insertions(+), 191 deletions(-)