[libcamera-devel] [PATCH] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Kate Hsuan
hpa at redhat.com
Mon Nov 15 09:39:12 CET 2021
Hi Jean-Michel,
On Mon, Nov 15, 2021 at 2:49 PM Jean-Michel Hautbois <
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>
> > ---
> > 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.
>
> > +
> > +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 = 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
>
> 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
I found the y_table format from ipu3-ipa repository (
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
>
>
> > +
> > + 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.
>
> > +
> > + 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
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211115/cbf77a8c/attachment-0001.htm>
More information about the libcamera-devel
mailing list