[libcamera-devel] [PATCH v2] ipa: ipu3: Consolidate querying of exposure and gain limits

[Kieran Bingham]

Quoting Umang Jain (2022-03-01 07:59:19)
> The exposure and gain limits are required for AGC configuration
> handled in IPAIPU3::updateSessionConfiguration(), which is happening
> already. Therefore the max/min private members in IPAIPU3 class for
> exposure/gain serve no use except setting initial values of exposure_
> and gain_ members.
> 
> Drop the max/min private members from IPAIPU3 class and set initial
> gain_ and exposure_ in  IPAIPU3::updateSessionConfiguration().


Great, even better to drop them if they aren't used.

This looks like we're on the path to removing the private variables for
controls from the IPAIPU3 now, as they should all be stored in the
context if there's any relationship with the algorithms.

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

> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> Changes in v2:
>  - Split into separate patch
>  - Prepped over "v4 ipa: ipu3: Misc clean up"
>  - Don't include max/min exposure/gain members in
>    IPASessionConfiguration, as they aren't used anywhere. 
> ---
>  src/ipa/ipu3/ipu3.cpp | 26 ++------------------------
>  1 file changed, 2 insertions(+), 24 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 17324616..4b6852a7 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -167,11 +167,7 @@ private:
>         /* Camera sensor controls. */
>         uint32_t defVBlank_;
>         uint32_t exposure_;
> -       uint32_t minExposure_;
> -       uint32_t maxExposure_;
>         uint32_t gain_;
> -       uint32_t minGain_;
> -       uint32_t maxGain_;

What's required to get rid of gain_, exposure_ and defVBlank_ from here
too?

--
Kieran


>  
>         /* Interface to the Camera Helper */
>         std::unique_ptr<CameraSensorHelper> camHelper_;
> @@ -192,10 +188,12 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)
>         const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;
>         int32_t minExposure = v4l2Exposure.min().get<int32_t>();
>         int32_t maxExposure = v4l2Exposure.max().get<int32_t>();
> +       exposure_ = minExposure;
>  
>         const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;
>         int32_t minGain = v4l2Gain.min().get<int32_t>();
>         int32_t maxGain = v4l2Gain.max().get<int32_t>();
> +       gain_ = minGain;
>  
>         /*
>          * When the AGC computes the new exposure values for a frame, it needs
> @@ -429,32 +427,12 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>          */
>         ctrls_ = configInfo.sensorControls;
>  
> -       const auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);
> -       if (itExp == ctrls_.end()) {
> -               LOG(IPAIPU3, Error) << "Can't find exposure control";
> -               return -EINVAL;
> -       }
> -
> -       const auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);
> -       if (itGain == ctrls_.end()) {
> -               LOG(IPAIPU3, Error) << "Can't find gain control";
> -               return -EINVAL;
> -       }
> -
>         const auto itVBlank = ctrls_.find(V4L2_CID_VBLANK);
>         if (itVBlank == ctrls_.end()) {
>                 LOG(IPAIPU3, Error) << "Can't find VBLANK control";
>                 return -EINVAL;
>         }
>  
> -       minExposure_ = itExp->second.min().get<int32_t>();
> -       maxExposure_ = itExp->second.max().get<int32_t>();
> -       exposure_ = minExposure_;
> -
> -       minGain_ = itGain->second.min().get<int32_t>();
> -       maxGain_ = itGain->second.max().get<int32_t>();
> -       gain_ = minGain_;
> -
>         defVBlank_ = itVBlank->second.def().get<int32_t>();
>  
>         calculateBdsGrid(configInfo.bdsOutputSize);
> -- 
> 2.31.1
>