[libcamera-devel] [PATCH] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Mon Nov 15 11:11:44 CET 2021
Kate,
On 15/11/2021 09:39, Kate Hsuan wrote:
> Hi Jean-Michel,
>
> On Mon, Nov 15, 2021 at 2:49 PM Jean-Michel Hautbois
> <jeanmichel.hautbois at ideasonboard.com
> <mailto:jeanmichel.hautbois at ideasonboard.com>> wrote:
> >
> > Hi Kate,
> >
> > Thanks for the patch !
> >
> > On 15/11/2021 06:44, Kate Hsuan wrote:
> > > Since VCM for surface Go 2 (dw9719) had been successfully
> > > driven, this Af module can be used to control the VCM and
> > > determine the focus value based on the IPU3 AF state.
> > >
> > > The variance of each focus step is determined and a greedy
> > > approah is used to find the maximum variance of the AF
> > s/approah/approach
>
> I'll correct my typo. T_T
>
> >
> > > state and a appropriate focus value.
> > >
> > > Signed-off-by: Kate Hsuan <hpa at redhat.com <mailto:hpa at redhat.com>>
> > > ---
> > > src/ipa/ipu3/algorithms/af.cpp | 165
> ++++++++++++++++++++++++++++
> > > src/ipa/ipu3/algorithms/af.h | 48 ++++++++
> > > src/ipa/ipu3/algorithms/meson.build | 3 +-
> > > src/ipa/ipu3/ipa_context.h | 6 +
> > > src/ipa/ipu3/ipu3.cpp | 2 +
> > > 5 files changed, 223 insertions(+), 1 deletion(-)
> > > create mode 100644 src/ipa/ipu3/algorithms/af.cpp
> > > create mode 100644 src/ipa/ipu3/algorithms/af.h
> > >
> > > diff --git a/src/ipa/ipu3/algorithms/af.cpp
> b/src/ipa/ipu3/algorithms/af.cpp
> > > new file mode 100644
> > > index 00000000..c276b539
> > > --- /dev/null
> > > +++ b/src/ipa/ipu3/algorithms/af.cpp
> > > @@ -0,0 +1,165 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2021, Red Hat
> > > + *
> > > + * af.cpp - IPU3 Af control
> > > + */
> > > +
> > > +#include "af.h"
> > > +
> > > +#include <algorithm>
> > > +#include <chrono>
> > > +#include <cmath>
> > > +#include <numeric>
> > > +
> > > +#include <linux/videodev2.h>
> > > +#include <sys/types.h>
> > > +#include <sys/stat.h>
> > > +#include <fcntl.h>
> > > +#include <sys/ioctl.h>
> > > +#include <unistd.h>
> > > +
> > > +#include <libcamera/base/log.h>
> > > +
> > > +#include <libcamera/ipa/core_ipa_interface.h>
> > > +
> > > +#include "libipa/histogram.h"
> > > +
> > > +/**
> > > + * \file af.h
> > > + */
> > > +
> > > +namespace libcamera {
> > > +
> > > +using namespace std::literals::chrono_literals;
> > > +
> > > +namespace ipa::ipu3::algorithms {
> > > +
> > > +LOG_DEFINE_CATEGORY(IPU3Af)
> > > +
> > > +Af::Af(): focus_(0), maxVariance_(0.0), currentVar_(0.0)
> > > +{
> > > + /* For surface Go 2 back camera VCM */
> > > + vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);
> > > +}
> >
> > IPA algorithms should not know anything about V4L2 controls, subdev,
> > etc. I know some discussion is running about this in "Fwd: Surface Go
> > VCM type (was: Need to pass acpi_enforce_resources=lax on the Surface Go
> > (version1))" on the list, and it is not yet solved afaik.
> >
> > I would say this should be done through the pipeline handler by the
> > CameraSensor class somehow.
> > And some documentation would then follow to specify it in
> > Documentation/sensor_driver_requirements.rst
>
> This is my first version of VCM control and to test the Raw AF data from
> IPU3 so I open the device in the class directly.
> Also, I have traced the codes and found the control class of
> v4l2-subdev. I'll try to update them in my v2 patch.
Maybe could you be interested by the thread "ipa: ipu3: Extend ipu3 ipa
interface for lens controls" in its v3, maybe would you like to review
it, as it sounds like the same beast here :-).
>
> >
> > > +
> > > +Af::~Af()
> > > +{
> > > + if(vcmFd_ != -1)
> > > + close(vcmFd_);
> > > +}
> > > +
> > > +int Af::configure(IPAContext &context, const IPAConfigInfo
> &configInfo)
> > > +{
> > > + const IPAConfigInfo tmp __attribute__((unused)) = configInfo;
> > > + context.frameContext.af.focus = 0;
> > > + context.frameContext.af.maxVar = 0;
> > > + context.frameContext.af.stable = false;
> > > +
> > > + maxVariance_ = 0;
> > > + ignoreFrame_ = 100;
> >
> > Those magic values should be at least commented or set as constexpr.
>
> I'll add the comments to the code.
>
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +void Af::vcmSet(int value)
> > > +{
> > > + int ret;
> > > + struct v4l2_control ctrl;
> > > + if(vcmFd_ == -1)
> > > + return;
> > > + memset(&ctrl, 0, sizeof(struct v4l2_control));
> > > + ctrl.id <http://ctrl.id> = V4L2_CID_FOCUS_ABSOLUTE;
> > > + ctrl.value = value;
> > > + ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
> >
> > Same comment, this is not something which should be done in the
> algorithm.
>
> OK, I'll try to do this in my v2 patch.
>
> >
> > > +
> > > +}
> > > +
> > > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a
> *stats )
> > > +{
> > > + typedef struct y_table_item {
> > > + uint16_t y1_avg;
> > > + uint16_t y2_avg;
> > > + }y_table_item_t;
> >
> > Not inside the function. This is the table defined in:
> >
> https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_raw_buffer
> <https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_raw_buffer>
> >
> > How do you know it is splitted in y1_avg and y2_avg ? Is it based in
> CrOs ?
> >
> https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h
> <https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h>
>
> I found the y_table format from ipu3-ipa repository
> (https://git.libcamera.org/libcamera/ipu3-ipa.git
> <https://git.libcamera.org/libcamera/ipu3-ipa.git>). Also, combined some
> keywords from the kernel, such as high and low frequency and
> convolution, the AF raw buffer may contain the result of low and high
> pass filter convolution (I guess).
> https://lxr.missinglinkelectronics.com/linux/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h#L256
> <https://lxr.missinglinkelectronics.com/linux/drivers/staging/media/ipu3/include/uapi/intel-ipu3.h#L256>
>
>
> >
> >
> > > +
> > > + uint32_t total = 0;
> > > + double mean;
> > > + uint64_t var_sum = 0;
> > > + y_table_item_t *y_item;
> > > +
> > > + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
> > > + int z = 0;
> > > +
> > > + for(z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE)/4; z++)
> > > + {
> > > + total = total + y_item[z].y2_avg;
> > > + if(y_item[z].y2_avg == 0)
> > > + break;
> > > + }
> > > + mean = total / z;
> >
> > Some comments to follow the algorithm could help :-). Is the algorithm
> > described in some book or something ? Is it a known one ? If so, may you
> > reference it ?
>
> Oh. I just calculate the variance for each non-zero y2_avg (high pass)
> value. For a clear image, the variance should be the largest value from
> each focus step. By increasing the focus step and searching for the
> maximum variance, an appropriate focus value can be found. It is
> something like the mountain climbing algorithm.
I mean, not everybody will be an image processing expert as you are, so
adding a documentation in the code could help following how AF works in
this case (looks like something like a contrast detection algorithm).
>
>
> >
> > > +
> > > + for(z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE)/4 &&
> y_item[z].y2_avg != 0; z++)
> > > + {
> > > + var_sum = var_sum + ((y_item[z].y2_avg - mean) *
> (y_item[z].y2_avg - mean));
> > > + if(y_item[z].y2_avg == 0)
> > > + break;
> > > + }
> > > + currentVar_ = static_cast<double>(var_sum)
> /static_cast<double>(z);
> > > + LOG(IPU3Af, Debug) << "variance: " << currentVar_;
> > > +
> > > + if( context.frameContext.af.stable == true )
> > > + {
> > > + const uint32_t diff_var = std::abs(currentVar_ -
> context.frameContext.af.maxVar);
> > > + const double var_ratio = diff_var /
> context.frameContext.af.maxVar;
> > > + LOG(IPU3Af, Debug) << "Change ratio: "
> > > + << var_ratio
> > > + << " current focus: "
> > > + << context.frameContext.af.focus;
> > > + if(var_ratio > 0.8)
> > > + {
> > > + if(ignoreFrame_ == 0)
> > > + {
> > > + context.frameContext.af.maxVar = 0;
> > > + context.frameContext.af.focus = 0;
> > > + focus_ = 0;
> > > + context.frameContext.af.stable = false;
> > > + ignoreFrame_ = 60;
> > > + }
> > > + else
> > > + ignoreFrame_--;
> > > + }else
> > > + ignoreFrame_ = 60;
> > > + }else
> > > + {
> > > + if(ignoreFrame_ != 0)
> > > + ignoreFrame_--;
> > > + else{
> > > + if(currentVar_ > context.frameContext.af.maxVar)
> > > + {
> > > + context.frameContext.af.maxVar =
> currentVar_;
> > > + context.frameContext.af.focus = focus_;
> > > + }
> > > +
> > > + if(focus_ > 1023)
> > 1023, because... ?
>
> It's the maximum focus step of VCM.
>
> >
> > > + {
> > > + context.frameContext.af.stable = true;
> > > + vcmSet(context.frameContext.af.focus);
> > > + LOG(IPU3Af, Debug) << "Finall Focus "
> > s/Finall/Final
>
> Sorry for my typo 😅
>
> >
> > > + <<
> context.frameContext.af.focus;
> > > + }else
> > > + {
> > > + focus_ += 5;
> > > + vcmSet(focus_);
> > > + }
> > > + LOG(IPU3Af, Debug) << "Focus searching max
> var is: "
> > > + <<
> context.frameContext.af.maxVar
> > > + << " Focus step is "
> > > + <<
> context.frameContext.af.focus;
> > > + }
> > > + }
> > > +}
> > > +
> > > +
> > > +} /* namespace ipa::ipu3::algorithms */
> > > +
> > > +} /* namespace libcamera */
> > > \ No newline at end of file
> > > diff --git a/src/ipa/ipu3/algorithms/af.h
> b/src/ipa/ipu3/algorithms/af.h
> > > new file mode 100644
> > > index 00000000..64c704b1
> > > --- /dev/null
> > > +++ b/src/ipa/ipu3/algorithms/af.h
> > > @@ -0,0 +1,48 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2021, Red Hat
> > > + *
> > > + * af.h - IPU3 Af control
> > > + */
> > > +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AF_H__
> > > +#define __LIBCAMERA_IPU3_ALGORITHMS_AF_H__
> > > +
> > > +#include <linux/intel-ipu3.h>
> > > +
> > > +#include <libcamera/base/utils.h>
> > > +
> > > +#include <libcamera/geometry.h>
> > > +
> > > +#include "algorithm.h"
> > > +
> > > +namespace libcamera {
> > > +
> > > +namespace ipa::ipu3::algorithms {
> > > +
> > > +class Af : public Algorithm
> > > +{
> > > +public:
> > > + Af();
> > > + ~Af();
> > > +
> > > + int configure(IPAContext &context, const IPAConfigInfo
> &configInfo) override;
> > > + void process(IPAContext &context, const ipu3_uapi_stats_3a
> *stats) override;
> >
> > Isn't there a need to configure the AF parameters in a prepare()
> > function ? There is a lot of tables associated, and at the very least, I
> > would have expected a grid:
> >
> >
> https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_config_s
> <https://www.kernel.org/doc/html/v5.6/media/uapi/v4l/pixfmt-meta-intel-ipu3.html#c.ipu3_uapi_af_config_s>
>
> In this version, I try to keep all the configurations in default to
> prove the algorithm and buffer format works.
> I could add grid configuration in my v2 patch.
>
> >
> > > +
> > > +private:
> > > + void filterVariance(double new_var);
> > > +
> > > + void vcmSet(int value);
> > > +
> > > + int vcmFd_;
> > > + int focus_;
> > > + unsigned int ignoreFrame_;
> > > + double maxVariance_;
> > > + double currentVar_;
> > > +
> > > +};
> > > +
> > > +} /* namespace ipa::ipu3::algorithms */
> > > +
> > > +} /* namespace libcamera */
> > > +
> > > +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
> > > diff --git a/src/ipa/ipu3/algorithms/meson.build
> b/src/ipa/ipu3/algorithms/meson.build
> > > index 3ec42f72..d32c61d2 100644
> > > --- a/src/ipa/ipu3/algorithms/meson.build
> > > +++ b/src/ipa/ipu3/algorithms/meson.build
> > > @@ -5,5 +5,6 @@ ipu3_ipa_algorithms = files([
> > > 'algorithm.cpp',
> > > 'awb.cpp',
> > > 'blc.cpp',
> > > - 'tone_mapping.cpp',
> > > + 'af.cpp',
> >
> > Alphabetical sorted
>
> I got it.
>
> >
> > > + 'tone_mapping.cpp'
> > > ])
> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > > index 1e46c61a..5d92f63c 100644
> > > --- a/src/ipa/ipu3/ipa_context.h
> > > +++ b/src/ipa/ipu3/ipa_context.h
> > > @@ -47,6 +47,12 @@ struct IPAFrameContext {
> > > } gains;
> > > } awb;
> > >
> > > + struct {
> > > + uint32_t focus;
> > > + double maxVar;
> > > + bool stable;
> > > + } af;
> >
> > Those fields are not documented in ipa_context.cpp, you probably have a
> > Doxygen warning ? If not, I suggest you activate the documentation
> > generation ;-).
>
> I don't have Doxygen warning. I guess I don't activate the documentation
> generation. I'll activate it, thank you :)
>
> >
> > > +
> > > struct {
> > > double gamma;
> > > struct ipu3_uapi_gamma_corr_lut gammaCorrection;
> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > > index 5c51607d..980815ee 100644
> > > --- a/src/ipa/ipu3/ipu3.cpp
> > > +++ b/src/ipa/ipu3/ipu3.cpp
> > > @@ -31,6 +31,7 @@
> > > #include "libcamera/internal/mapped_framebuffer.h"
> > >
> > > #include "algorithms/agc.h"
> > > +#include "algorithms/af.h"
> > > #include "algorithms/algorithm.h"
> > > #include "algorithms/awb.h"
> > > #include "algorithms/blc.h"
> > > @@ -298,6 +299,7 @@ int IPAIPU3::init(const IPASettings &settings,
> > > }
> > >
> > > /* Construct our Algorithms */
> > > + algorithms_.push_back(std::make_unique<algorithms::Af>());
> > > algorithms_.push_back(std::make_unique<algorithms::Agc>());
> > > algorithms_.push_back(std::make_unique<algorithms::Awb>());
> > >
> algorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());
> > >
> >
>
>
> --
> BR,
> Kate
More information about the libcamera-devel
mailing list