| Message ID | 20211119111654.68445-8-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel, Thank you for the patch. On Fri, Nov 19, 2021 at 12:16:53PM +0100, Jean-Michel Hautbois wrote: > When a new parameter buffer needs to be queued, we need to specify which > algorithm is activated or not in the ISP. Add a simple prepare function > in AGC for that, which may later evolve to take the exposure locking > into account. For that function to be called, we also need to add the > loop on the algorithms in IPARkISP1::queueRequest. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/agc.cpp | 7 +++++++ > src/ipa/rkisp1/algorithms/agc.h | 1 + > src/ipa/rkisp1/rkisp1.cpp | 12 +++--------- > 3 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 3d4a17ff..2b7c6fda 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -244,6 +244,13 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats) > frameCount_++; > } > > +void Agc::prepare([[maybe_unused]] IPAContext &context, > + rkisp1_params_cfg *params) > +{ > + params->module_ens |= RKISP1_CIF_ISP_MODULE_AEC; > + params->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC; There would be a tiny performance improvement if you set the RKISP1_CIF_ISP_MODULE_AEC flag in module_en_update only when the AEC configuration changed. Currently, that would be on the first frame only, as we don't support disabling AE anymore after this patch. It's so tiny that it doesn't matter much at the moment, but once we implement support for manual exposure, it would be nice to improve this. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +} > + > } /* namespace ipa::rkisp1::algorithms */ > > } /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h > index fbb8ea98..934a455e 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -28,6 +28,7 @@ public: > ~Agc() = default; > > int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; > + void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; > > private: > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 1783f91f..b1bfb8b6 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -271,19 +271,13 @@ void IPARkISP1::processEvent(const RkISP1Event &event) > } > > void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params, > - const ControlList &controls) > + [[maybe_unused]] const ControlList &controls) > { > /* Prepare parameters buffer. */ > memset(params, 0, sizeof(*params)); > > - /* Auto Exposure on/off. */ > - if (controls.contains(controls::AeEnable)) { > - autoExposure_ = controls.get(controls::AeEnable); > - if (autoExposure_) > - params->module_ens = RKISP1_CIF_ISP_MODULE_AEC; > - > - params->module_en_update = RKISP1_CIF_ISP_MODULE_AEC; > - } > + for (auto const &algo : algorithms_) > + algo->prepare(context_, params); > > RkISP1Action op; > op.op = ActionParamFilled;
diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 3d4a17ff..2b7c6fda 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -244,6 +244,13 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats) frameCount_++; } +void Agc::prepare([[maybe_unused]] IPAContext &context, + rkisp1_params_cfg *params) +{ + params->module_ens |= RKISP1_CIF_ISP_MODULE_AEC; + params->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC; +} + } /* namespace ipa::rkisp1::algorithms */ } /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h index fbb8ea98..934a455e 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -28,6 +28,7 @@ public: ~Agc() = default; int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override; + void prepare(IPAContext &context, rkisp1_params_cfg *params) override; void process(IPAContext &context, const rkisp1_stat_buffer *stats) override; private: diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 1783f91f..b1bfb8b6 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -271,19 +271,13 @@ void IPARkISP1::processEvent(const RkISP1Event &event) } void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params, - const ControlList &controls) + [[maybe_unused]] const ControlList &controls) { /* Prepare parameters buffer. */ memset(params, 0, sizeof(*params)); - /* Auto Exposure on/off. */ - if (controls.contains(controls::AeEnable)) { - autoExposure_ = controls.get(controls::AeEnable); - if (autoExposure_) - params->module_ens = RKISP1_CIF_ISP_MODULE_AEC; - - params->module_en_update = RKISP1_CIF_ISP_MODULE_AEC; - } + for (auto const &algo : algorithms_) + algo->prepare(context_, params); RkISP1Action op; op.op = ActionParamFilled;
When a new parameter buffer needs to be queued, we need to specify which algorithm is activated or not in the ISP. Add a simple prepare function in AGC for that, which may later evolve to take the exposure locking into account. For that function to be called, we also need to add the loop on the algorithms in IPARkISP1::queueRequest. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/rkisp1/algorithms/agc.cpp | 7 +++++++ src/ipa/rkisp1/algorithms/agc.h | 1 + src/ipa/rkisp1/rkisp1.cpp | 12 +++--------- 3 files changed, 11 insertions(+), 9 deletions(-)