[libcamera-devel,v3,10/11] ipa: rkisp1: agc: Introduce prepare call

14722 diff mbox series

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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@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(-)

Quoting Jean-Michel Hautbois (2021-11-23 15:04:22)
> 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>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Tempted to add in the commit message that we no longer disable the AE
algorithm based on the controls::AeEnable, which will be handled in a
different manner later.

Reviewed-by: Kieran Bingham <kieran.bingham@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 9c6d312e..0433813a 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -260,6 +260,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 83159743..ac95dea5 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 89d98b6c..59e868ff 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -270,19 +270,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;
> -- 
> 2.32.0
>