[libcamera-devel] [RFC v1 5/5] ipa: ipu3: af: Remove hardcoded maximum VCM steps

[Kieran Bingham]

Quoting Kate Hsuan via libcamera-devel (2022-04-14 08:43:42)
> The hardcoded VCM step variable was removed and was replaced by the
> context configuration.
> 
> Signed-off-by: Kate Hsuan<hpa at redhat.com>
> ---
>  src/ipa/ipu3/algorithms/af.cpp | 6 +++---
>  src/ipa/ipu3/ipu3.cpp          | 1 +
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index addf98af..95fb39f5 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -179,7 +179,7 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>         grid.y_start |= IPU3_UAPI_GRID_Y_START_EN;
>  
>         /* Initial max focus step */
> -       maxStep_ = kMaxFocusSteps;
> +       maxStep_ = context.configuration.af.maxVcmSteps;

Aha I see (from below) you're setting
context.configuration.af.maxVcmSteps just to assign it here, but you
have direct access to configInfo.sensorInfo.maxVcmSteps.

Seems like the af.maxVcmSteps addition might not be needed?

>  
>         /* Initial focus value */
>         context.frameContext.af.focus = 0;
> @@ -214,7 +214,7 @@ void Af::afCoarseScan(IPAContext &context)
>                 context.frameContext.af.focus = focus_;
>                 previousVariance_ = 0;
>                 maxStep_ = std::clamp(focus_ + static_cast<uint32_t>((focus_ * kFineRange)),
> -                                     0U, kMaxFocusSteps);
> +                                     0U, static_cast<uint32_t>(context.configuration.af.maxVcmSteps));

The static_cast shouldn't be needed here. I think maxVcmSteps should be
uint32_t.

>         }
>  }
>  
> @@ -259,7 +259,7 @@ void Af::afReset(IPAContext &context)
>         previousVariance_ = 0.0;
>         coarseCompleted_ = false;
>         fineCompleted_ = false;
> -       maxStep_ = kMaxFocusSteps;
> +       maxStep_ = context.configuration.af.maxVcmSteps;
>  }
>  
>  /**
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index dd6cfd79..d46f7853 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -456,6 +456,7 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
>  
>         /* Clean frameContext at each reconfiguration. */
>         context_.frameContext = {};
> +       context_.configuration.af.maxVcmSteps = configInfo.sensorInfo.maxVcmSteps;

This should be in the configure call of the AF, which will then get
called by the "for (auto const &algo : algorithms_)" loop at the bottom
of this function.

>         if (!validateSensorControls()) {
>                 LOG(IPAIPU3, Error) << "Sensor control validation failed.";
> -- 
> 2.35.1
>