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

[Kate Hsuan]

Hi Kieran,

On Thu, Apr 14, 2022 at 4:55 PM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> 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?

Seems can be removed. I could get the value in configure() of the
algorithm. I'll update them in v2.


Thank you :)

>
> >
> >         /* 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
> >
>


-- 
BR,
Kate