[libcamera-devel,RFC,v2,1/1] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
diff mbox series

Message ID 20211124103447.102069-2-hpa@redhat.com
State Superseded
Headers show
Series
  • ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM
Related show

Commit Message

Kate Hsuan Nov. 24, 2021, 10:34 a.m. UTC
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
state and a appropriate focus value.

Changes since v2:
1. Add grid configuration interface.
2. Add AF accelerator configuration.
3. Move default focus area to the center of the image.

Signed-off-by: Kate Hsuan <hpa@redhat.com>
---
 src/ipa/ipu3/algorithms/af.cpp      | 284 ++++++++++++++++++++++++++++
 src/ipa/ipu3/algorithms/af.h        |  54 ++++++
 src/ipa/ipu3/algorithms/agc.cpp     |   2 +-
 src/ipa/ipu3/algorithms/meson.build |   3 +-
 src/ipa/ipu3/ipa_context.cpp        |  26 +++
 src/ipa/ipu3/ipa_context.h          |  11 ++
 src/ipa/ipu3/ipu3.cpp               |   2 +
 7 files changed, 380 insertions(+), 2 deletions(-)
 create mode 100644 src/ipa/ipu3/algorithms/af.cpp
 create mode 100644 src/ipa/ipu3/algorithms/af.h

Comments

Jean-Michel Hautbois Nov. 24, 2021, 1:40 p.m. UTC | #1
Hi Kate,

Thanks for the patch.

On 24/11/2021 11:34, 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
> state and a appropriate focus value.
> 
> Changes since v2:
> 1. Add grid configuration interface.
> 2. Add AF accelerator configuration.
> 3. Move default focus area to the center of the image.
> 
> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> ---
>   src/ipa/ipu3/algorithms/af.cpp      | 284 ++++++++++++++++++++++++++++
>   src/ipa/ipu3/algorithms/af.h        |  54 ++++++
>   src/ipa/ipu3/algorithms/agc.cpp     |   2 +-
>   src/ipa/ipu3/algorithms/meson.build |   3 +-
>   src/ipa/ipu3/ipa_context.cpp        |  26 +++
>   src/ipa/ipu3/ipa_context.h          |  11 ++
>   src/ipa/ipu3/ipu3.cpp               |   2 +
>   7 files changed, 380 insertions(+), 2 deletions(-)
>   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..b0359721
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -0,0 +1,284 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Red Hat
> + *
> + * af.cpp - IPU3 auto focus control
> + */
> +
> +#include "af.h"
> +
> +#include <algorithm>
> +#include <chrono>
> +#include <cmath>
> +#include <fcntl.h>
> +#include <numeric>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include <linux/videodev2.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 {
> +
> +/**
> + * \class Af
> + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
> + *
> + * This algorithm is used to determine the position of the lens and get a
> + * focused image. The IPU3 AF accelerator computes the statistics, composed
> + * by high pass and low pass filtered value and stores in a AF buffer.
> + * Typically, for a focused image, it has relative high contrast than a
> + * blurred image, i.e. an out of focus image. Therefore, if an image with the
> + * highest contrast can be found from the AF scan, the lens' position is the
> + * best step of the focus.
> + *
> + */
> +
> +LOG_DEFINE_CATEGORY(IPU3Af)
> +
> +/**
> + * Maximum focus value of the VCM control
> + * \todo should be obtained from the VCM driver
> + */
> +static constexpr uint32_t MaxFocusSteps_ = 1023;
> +
> +/* Minimum focus step for searching appropriate focus*/
> +static constexpr uint32_t MinSearchStep_ = 5;
> +
> +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
> +static constexpr double MaxChange_ = 0.8;
> +
> +Af::Af()
> +	: focus_(0), currentVariance_(0.0)
> +{
> +	/**
> +	 * For surface Go 2 back camera VCM (dw9719)
> +	 * \todo move to control class
> +	*/
> +	vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);

This shouldn't be here :-).

> +}
> +
> +Af::~Af()
> +{
> +	if (vcmFd_ != -1)
> +		close(vcmFd_);
> +}
> +
> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
> +{
> +	/* AF grid config */
> +	params->acc_param.af.grid_cfg.width = 16;
> +	params->acc_param.af.grid_cfg.height = 16;
> +	params->acc_param.af.grid_cfg.block_height_log2 = 3;
> +	params->acc_param.af.grid_cfg.block_width_log2 = 3;
> +	params->acc_param.af.grid_cfg.height_per_slice = 2;
> +	/* Start position of AF area */
> +	params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
> +	params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;

That is interesting... and probably not right :-).
You are setting the ImgU to use the same default grid it is using if you 
are not setting params->use.acc_af.

Meaning, you could simplify this by using the default table here:
https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9579

You can use what is done in IPU3::Awb for instance, we are using the 
default BNR table from 
https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9313

And then you can only change the desired start_x and start_y (I have 
comments on those later).

> +
> +	params->acc_param.af.filter_config.y1_sign_vec = 0;
> +	params->acc_param.af.filter_config.y2_sign_vec = 0;
> +
> +	/* b + gb + gr + r = 32 */
> +	params->acc_param.af.filter_config.y_calc.y_gen_rate_b = 8;
> +	params->acc_param.af.filter_config.y_calc.y_gen_rate_gb = 8;
> +	params->acc_param.af.filter_config.y_calc.y_gen_rate_gr = 8;
> +	params->acc_param.af.filter_config.y_calc.y_gen_rate_r = 8;
> +
> +	/* 2^7 = 128,  a1 + a2 + ... + a12 = 128, log2 of sum of a1 to a12*/
> +	params->acc_param.af.filter_config.nf.y1_nf = 7;
> +	params->acc_param.af.filter_config.nf.y2_nf = 7;
> +
> +	/* Low pass filter configuration (y1_coeff_n) */
> +	params->acc_param.af.filter_config.y1_coeff_0.a1 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_0.a2 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_0.a3 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_0.a4 = 0;
> +
> +	params->acc_param.af.filter_config.y1_coeff_1.a5 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_1.a6 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_1.a7 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_1.a8 = 0;
> +
> +	params->acc_param.af.filter_config.y1_coeff_2.a9 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_2.a10 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_2.a11 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_2.a12 = 128;
> +
> +	/* High pass filter configuration (y2_coeff_n) */
> +	params->acc_param.af.filter_config.y2_coeff_0.a1 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_0.a2 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_0.a3 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_0.a4 = 0;
> +
> +	params->acc_param.af.filter_config.y2_coeff_1.a5 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_1.a6 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_1.a7 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_1.a8 = 0;
> +
> +	params->acc_param.af.filter_config.y2_coeff_2.a9 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_2.a10 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_2.a11 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_2.a12 = 128;
> +
> +	/* Enable AF accelerator */
> +	params->use.acc_af = 1;
> +}
> +
> +/**
> + * \brief Configure the Af given a configInfo
> + * \param[in] context The shared IPA context
> + * \param[in] configInfo The IPA configuration data
> + *
> + * \return 0
> + */
> +int Af::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)
> +{
> +	/* Determined focus value i.e. current focus value */
> +	context.frameContext.af.focus = 0;
> +	/* Maximum variance of the AF statistics */
> +	context.frameContext.af.maxVariance = 0;
> +	/* is focused? if it is true, the AF should be in a stable state. */
> +	context.frameContext.af.stable = false;
> +	/* Frame to be ignored before start to estimate AF variance. */
> +	ignoreFrame_ = 10;
> +
> +	/*
> +	 * AF default area configuration
> +	 * Move AF area to the center of the image.
> +	 */
> +	/* AF width is 16x8 = 128 */
> +	context.configuration.af.start_x = (1280 / 2) - 64;
> +	context.configuration.af.start_y = (720 / 2) - 64;

This is good, only if the frame output is set to 1280x720.
If I call:
'cam -c2 -C -s width=1920,height=1280'

Then you won't be centered.
You can get the configuration directly from configInfo.bdsOutputSize (I 
think af is using the BDS output) and then remove the [[maybe_unused]].

> +
> +	return 0;
> +}
> +
> +/**
> + * \brief Send focus step to the VCM.
> + * \param[in] value Set lens position.
> + * \todo It is hardcoded here for the dw9717 VCM and will be moved to the
> + * subdev control in the future.
> + */
> +int Af::vcmSet(int value)
> +{
> +	int ret;
> +	struct v4l2_control ctrl;
> +	if (vcmFd_ == -1)
> +		return -EINVAL;
> +	memset(&ctrl, 0, sizeof(struct v4l2_control));
> +	ctrl.id = V4L2_CID_FOCUS_ABSOLUTE;
> +	ctrl.value = value;
> +	ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
> +	return ret;
> +}

This does not belong to the algorithm.

> +
> +/**
> + * \brief Determine the max contrast image and lens position. y_table is the
> + * statictic data from IPU3 and is composed of low pass and high pass filtered
> + * value. High pass filtered value also represents the sharpness of the image.
> + * Based on this, if the image with highest variance of the high pass filtered
> + * value (contrast) during the AF scan, the position of the len should be the
> + * best focus.
> + * \param[in] context The shared IPA context.
> + * \param[in] stats The statistic buffer of 3A from the IPU3.
> + */
> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> +{
> +	uint32_t total = 0;
> +	double mean;
> +	uint64_t var_sum = 0;
> +	y_table_item_t *y_item;
> +	int z = 0;
> +
> +	y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
> +
> +	/**
> +	 * Calculate the mean of each non-zero AF statistics, since IPU3 only determine the AF value
> +	 * for a given grid.
> +	 */
> +	for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
> +		printf("%d, ", y_item[z].y2_avg);
> +		total = total + y_item[z].y2_avg;
> +		if (y_item[z].y2_avg == 0)
> +			break;
> +	}
> +	mean = total / z;
> +
> +	/* Calculate the variance of every AF statistic value. */
> +	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;
> +	}
> +
> +	/* Determine the average variance of the frame. */
> +	currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
> +	LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
> +
> +	if (context.frameContext.af.stable == true) {
> +		const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
> +		const double var_ratio = diff_var / context.frameContext.af.maxVariance;
> +		LOG(IPU3Af, Debug) << "Change ratio: "
> +				   << var_ratio
> +				   << " current focus: "
> +				   << context.frameContext.af.focus;
> +		/**
> +		 * If the change ratio of contrast is over Maxchange_ (out of focus),
> +		 * trigger AF again.
> +		 */
> +		if (var_ratio > MaxChange_) {
> +			if (ignoreFrame_ == 0) {
> +				context.frameContext.af.maxVariance = 0;
> +				context.frameContext.af.focus = 0;
> +				focus_ = 0;
> +				context.frameContext.af.stable = false;
> +				ignoreFrame_ = 60;
> +			} else
> +				ignoreFrame_--;
> +		} else
> +			ignoreFrame_ = 10;
> +	} else {
> +		if (ignoreFrame_ != 0)
> +			ignoreFrame_--;
> +		else {
> +			/* Find the maximum variance during the AF scan using a greedy strategy */
> +			if (currentVariance_ > context.frameContext.af.maxVariance) {
> +				context.frameContext.af.maxVariance = currentVariance_;
> +				context.frameContext.af.focus = focus_;
> +			}
> +
> +			if (focus_ > MaxFocusSteps_) {
> +				/* If reach the max step, move lens to the position and set "focus stable". */
> +				context.frameContext.af.stable = true;
> +				vcmSet(context.frameContext.af.focus);
> +			} else {
> +				focus_ += MinSearchStep_;
> +				vcmSet(focus_);
> +			}
> +			LOG(IPU3Af, Debug) << "Focus searching max variance is: "
> +					   << context.frameContext.af.maxVariance
> +					   << " Focus step is "
> +					   << context.frameContext.af.focus;
> +		}
> +	}
> +}

I won't comment the algorithm yet, as you already have some changes to 
make according to Kieran's comments ;-).

> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> new file mode 100644
> index 00000000..b5c11874
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/af.h
> @@ -0,0 +1,54 @@
> +/* 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
> +{
> +	/* The format of y_table. From ipu3-ipa repo */
> +	typedef struct y_table_item {
> +		uint16_t y1_avg;
> +		uint16_t y2_avg;
> +	} y_table_item_t;
> +
> +public:
> +	Af();
> +	~Af();
> +
> +	void prepare(IPAContext &context, ipu3_uapi_params *params) override;
> +	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> +	void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> +
> +private:
> +	int vcmSet(int value);
> +
> +	int vcmFd_;
> +	/* Used for focus scan. */
> +	uint32_t focus_;
> +	/* Recent AF statistic variance. */
> +	double currentVariance_;
> +	/* The frames to be ignore before starting measuring. */
> +	uint32_t ignoreFrame_;
> +};
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index b5d736c1..70ae4e59 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -138,7 +138,7 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>   	}
>   
>   	/* Estimate the quantile mean of the top 2% of the histogram */
> -	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> +	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.7, 1.0);

Why are you changing this ? It is the top 30% of the cumulative histogram ?

>   }
>   
>   /**
> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> index 3ec42f72..8574416e 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -1,9 +1,10 @@
>   # SPDX-License-Identifier: CC0-1.0
>   
>   ipu3_ipa_algorithms = files([
> +    'af.cpp',
>       'agc.cpp',
>       'algorithm.cpp',
>       'awb.cpp',
>       'blc.cpp',
> -    'tone_mapping.cpp',
> +    'tone_mapping.cpp'
>   ])
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 2355a9c7..1064d62d 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -69,6 +69,17 @@ namespace libcamera::ipa::ipu3 {
>    * \brief Number of cells on one line including the ImgU padding
>    */
>   
> +/**
> + * \var IPASessionConfiguration::af
> + * \brief AF parameters configuration of the IPA
> + *
> + * \var IPASessionConfiguration::af.start_x
> + * \brief The start X position of the AF area
> + *
> + * \var IPASessionConfiguration::af.start_y
> + * \brief The start Y position of the AF area
> + */
> +
>   /**
>    * \var IPASessionConfiguration::agc
>    * \brief AGC parameters configuration of the IPA
> @@ -86,6 +97,21 @@ namespace libcamera::ipa::ipu3 {
>    * \brief Maximum analogue gain supported with the configured sensor
>    */
>   
> +/**
> + * \var IPAFrameContext::af
> + * \brief Context for the Automatic Focus algorithm
> + *
> + * \struct  IPAFrameContext::af
> + * \var IPAFrameContext::af.focus
> + * \brief Current position of the lens
> + *
> + * \var IPAFrameContext::af.maxVariance
> + * \brief The maximum variance of the current image.
> + *
> + * \var IPAFrameContext::af.stable
> + * \brief is the image focused?
> + */
> +
>   /**
>    * \var IPAFrameContext::agc
>    * \brief Context for the Automatic Gain Control algorithm
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 1e46c61a..a7ffcbed 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -31,6 +31,11 @@ struct IPASessionConfiguration {
>   		double minAnalogueGain;
>   		double maxAnalogueGain;
>   	} agc;
> +
> +	struct {
> +		uint32_t start_x;
> +		uint32_t start_y;
> +	} af;

Those are __u16 in the kernel header file, maybe can we keep the same type ?

>   };
>   
>   struct IPAFrameContext {
> @@ -47,6 +52,12 @@ struct IPAFrameContext {
>   		} gains;
>   	} awb;
>   
> +	struct {
> +		uint32_t focus;
> +		double maxVariance;
> +		bool stable;
> +	} af;
> +
>   	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..f19d0059 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -30,6 +30,7 @@
>   
>   #include "libcamera/internal/mapped_framebuffer.h"
>   
> +#include "algorithms/af.h"
>   #include "algorithms/agc.h"
>   #include "algorithms/algorithm.h"
>   #include "algorithms/awb.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>());
>
Daniel Scally Nov. 24, 2021, 11:51 p.m. UTC | #2
Hi Kate

On 24/11/2021 10:34, 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
> state and a appropriate focus value.
>
> Changes since v2:
> 1. Add grid configuration interface.
> 2. Add AF accelerator configuration.
> 3. Move default focus area to the center of the image.
>
> Signed-off-by: Kate Hsuan <hpa@redhat.com>


Thanks a lot for this, it's very cool. I tried it out and observed the
following:


1. It seems to focus too close initially, then transition to a
progressively further away focus (passing through the sweet spot where
the items on my desk behind the Go2 are in focus) and then resets and
goes back close focus again. It repeats this a couple times , before
eventually settling in to an ok image. So, seems to work but takes a
little time to get there.

2. When the "reset" happens, and when stream stops, there's an audible
click as the VCM changes position a large amount in one go. I think (at
least for stream off) that's my fault in the driver; most of them have a
section in .suspend() that gradually relaxes on power off, I'll update
the dw9719 to do that too. Not sure that that's the cause of the click
on the reset though

3. There's _a lot_ of noise written to the terminal


> ---
>  src/ipa/ipu3/algorithms/af.cpp      | 284 ++++++++++++++++++++++++++++
>  src/ipa/ipu3/algorithms/af.h        |  54 ++++++
>  src/ipa/ipu3/algorithms/agc.cpp     |   2 +-
>  src/ipa/ipu3/algorithms/meson.build |   3 +-
>  src/ipa/ipu3/ipa_context.cpp        |  26 +++
>  src/ipa/ipu3/ipa_context.h          |  11 ++
>  src/ipa/ipu3/ipu3.cpp               |   2 +
>  7 files changed, 380 insertions(+), 2 deletions(-)
>  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..b0359721
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -0,0 +1,284 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Red Hat
> + *
> + * af.cpp - IPU3 auto focus control
> + */
> +
> +#include "af.h"
> +
> +#include <algorithm>
> +#include <chrono>
> +#include <cmath>
> +#include <fcntl.h>
> +#include <numeric>
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include <linux/videodev2.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 {
> +
> +/**
> + * \class Af
> + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
> + *
> + * This algorithm is used to determine the position of the lens and get a
> + * focused image. The IPU3 AF accelerator computes the statistics, composed
> + * by high pass and low pass filtered value and stores in a AF buffer.
> + * Typically, for a focused image, it has relative high contrast than a
> + * blurred image, i.e. an out of focus image. Therefore, if an image with the
> + * highest contrast can be found from the AF scan, the lens' position is the
> + * best step of the focus.
> + *
> + */
> +
> +LOG_DEFINE_CATEGORY(IPU3Af)
> +
> +/**
> + * Maximum focus value of the VCM control
> + * \todo should be obtained from the VCM driver
> + */
> +static constexpr uint32_t MaxFocusSteps_ = 1023;
> +
> +/* Minimum focus step for searching appropriate focus*/
> +static constexpr uint32_t MinSearchStep_ = 5;
> +
> +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
> +static constexpr double MaxChange_ = 0.8;
> +
> +Af::Af()
> +	: focus_(0), currentVariance_(0.0)
> +{
> +	/**
> +	 * For surface Go 2 back camera VCM (dw9719)
> +	 * \todo move to control class
> +	*/
> +	vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);
> +}
> +
> +Af::~Af()
> +{
> +	if (vcmFd_ != -1)
> +		close(vcmFd_);
> +}


FYI I'm going to work on moving these bits out so that the actual
setting of the v4l2 control is done via an instance of the CameraLens
class from Han-Lin's recent series. Ditto the calls to the vcmSet()
function in ::process(). This should mean it's nicely agnostic (i.e. no
hardcoded v4l-subdev, nor sensor/vcm pairs)


> +
> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
> +{
> +	/* AF grid config */
> +	params->acc_param.af.grid_cfg.width = 16;
> +	params->acc_param.af.grid_cfg.height = 16;
> +	params->acc_param.af.grid_cfg.block_height_log2 = 3;
> +	params->acc_param.af.grid_cfg.block_width_log2 = 3;
> +	params->acc_param.af.grid_cfg.height_per_slice = 2;
> +	/* Start position of AF area */
> +	params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
> +	params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;
> +
> +	params->acc_param.af.filter_config.y1_sign_vec = 0;
> +	params->acc_param.af.filter_config.y2_sign_vec = 0;
> +
> +	/* b + gb + gr + r = 32 */
> +	params->acc_param.af.filter_config.y_calc.y_gen_rate_b = 8;
> +	params->acc_param.af.filter_config.y_calc.y_gen_rate_gb = 8;
> +	params->acc_param.af.filter_config.y_calc.y_gen_rate_gr = 8;
> +	params->acc_param.af.filter_config.y_calc.y_gen_rate_r = 8;
> +
> +	/* 2^7 = 128,  a1 + a2 + ... + a12 = 128, log2 of sum of a1 to a12*/
> +	params->acc_param.af.filter_config.nf.y1_nf = 7;
> +	params->acc_param.af.filter_config.nf.y2_nf = 7;
> +
> +	/* Low pass filter configuration (y1_coeff_n) */
> +	params->acc_param.af.filter_config.y1_coeff_0.a1 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_0.a2 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_0.a3 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_0.a4 = 0;
> +
> +	params->acc_param.af.filter_config.y1_coeff_1.a5 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_1.a6 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_1.a7 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_1.a8 = 0;
> +
> +	params->acc_param.af.filter_config.y1_coeff_2.a9 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_2.a10 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_2.a11 = 0;
> +	params->acc_param.af.filter_config.y1_coeff_2.a12 = 128;
> +
> +	/* High pass filter configuration (y2_coeff_n) */
> +	params->acc_param.af.filter_config.y2_coeff_0.a1 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_0.a2 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_0.a3 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_0.a4 = 0;
> +
> +	params->acc_param.af.filter_config.y2_coeff_1.a5 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_1.a6 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_1.a7 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_1.a8 = 0;
> +
> +	params->acc_param.af.filter_config.y2_coeff_2.a9 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_2.a10 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_2.a11 = 0;
> +	params->acc_param.af.filter_config.y2_coeff_2.a12 = 128;
> +
> +	/* Enable AF accelerator */
> +	params->use.acc_af = 1;
> +}
> +
> +/**
> + * \brief Configure the Af given a configInfo
> + * \param[in] context The shared IPA context
> + * \param[in] configInfo The IPA configuration data
> + *
> + * \return 0
> + */
> +int Af::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)
> +{
> +	/* Determined focus value i.e. current focus value */
> +	context.frameContext.af.focus = 0;
> +	/* Maximum variance of the AF statistics */
> +	context.frameContext.af.maxVariance = 0;
> +	/* is focused? if it is true, the AF should be in a stable state. */
> +	context.frameContext.af.stable = false;
> +	/* Frame to be ignored before start to estimate AF variance. */
> +	ignoreFrame_ = 10;
> +
> +	/*
> +	 * AF default area configuration
> +	 * Move AF area to the center of the image.
> +	 */
> +	/* AF width is 16x8 = 128 */
> +	context.configuration.af.start_x = (1280 / 2) - 64;
> +	context.configuration.af.start_y = (720 / 2) - 64;
> +
> +	return 0;
> +}
> +
> +/**
> + * \brief Send focus step to the VCM.
> + * \param[in] value Set lens position.
> + * \todo It is hardcoded here for the dw9717 VCM and will be moved to the
> + * subdev control in the future.
> + */
> +int Af::vcmSet(int value)
> +{
> +	int ret;
> +	struct v4l2_control ctrl;
> +	if (vcmFd_ == -1)
> +		return -EINVAL;
> +	memset(&ctrl, 0, sizeof(struct v4l2_control));
> +	ctrl.id = V4L2_CID_FOCUS_ABSOLUTE;
> +	ctrl.value = value;
> +	ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
> +	return ret;
> +}
> +
> +/**
> + * \brief Determine the max contrast image and lens position. y_table is the
> + * statictic data from IPU3 and is composed of low pass and high pass filtered
> + * value. High pass filtered value also represents the sharpness of the image.
> + * Based on this, if the image with highest variance of the high pass filtered
> + * value (contrast) during the AF scan, the position of the len should be the
> + * best focus.
> + * \param[in] context The shared IPA context.
> + * \param[in] stats The statistic buffer of 3A from the IPU3.
> + */
> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> +{
> +	uint32_t total = 0;
> +	double mean;
> +	uint64_t var_sum = 0;
> +	y_table_item_t *y_item;
> +	int z = 0;
> +
> +	y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;


I get a warning here when compiling:


../src/ipa/ipu3/algorithms/af.cpp: In member function ‘virtual void
libcamera::ipa::ipu3::algorithms::Af::process(libcamera::ipa::ipu3::IPAContext&,
const ipu3_uapi_stats_3a*)’:
../src/ipa/ipu3/algorithms/af.cpp:209:50: error: taking address of
packed member of ‘ipu3_uapi_stats_3a’ may result in an unaligned pointer
value [-Werror=address-of-packed-member]
  209 |  y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
      |                             ~~~~~~~~~~~~~~~~~~~~~^~~~~~~

> +
> +	/**
> +	 * Calculate the mean of each non-zero AF statistics, since IPU3 only determine the AF value
> +	 * for a given grid.
> +	 */
> +	for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
> +		printf("%d, ", y_item[z].y2_avg);
> +		total = total + y_item[z].y2_avg;
> +		if (y_item[z].y2_avg == 0)
> +			break;
> +	}
> +	mean = total / z;
> +
> +	/* Calculate the variance of every AF statistic value. */
> +	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;
> +	}
> +
> +	/* Determine the average variance of the frame. */
> +	currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
> +	LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
> +
> +	if (context.frameContext.af.stable == true) {
> +		const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
> +		const double var_ratio = diff_var / context.frameContext.af.maxVariance;
> +		LOG(IPU3Af, Debug) << "Change ratio: "
> +				   << var_ratio
> +				   << " current focus: "
> +				   << context.frameContext.af.focus;
> +		/**
> +		 * If the change ratio of contrast is over Maxchange_ (out of focus),
> +		 * trigger AF again.
> +		 */
> +		if (var_ratio > MaxChange_) {
> +			if (ignoreFrame_ == 0) {
> +				context.frameContext.af.maxVariance = 0;
> +				context.frameContext.af.focus = 0;
> +				focus_ = 0;
> +				context.frameContext.af.stable = false;
> +				ignoreFrame_ = 60;
> +			} else
> +				ignoreFrame_--;
> +		} else
> +			ignoreFrame_ = 10;
> +	} else {
> +		if (ignoreFrame_ != 0)
> +			ignoreFrame_--;
> +		else {
> +			/* Find the maximum variance during the AF scan using a greedy strategy */
> +			if (currentVariance_ > context.frameContext.af.maxVariance) {
> +				context.frameContext.af.maxVariance = currentVariance_;
> +				context.frameContext.af.focus = focus_;
> +			}
> +
> +			if (focus_ > MaxFocusSteps_) {
> +				/* If reach the max step, move lens to the position and set "focus stable". */
> +				context.frameContext.af.stable = true;
> +				vcmSet(context.frameContext.af.focus);
> +			} else {
> +				focus_ += MinSearchStep_;
> +				vcmSet(focus_);
> +			}
> +			LOG(IPU3Af, Debug) << "Focus searching max variance is: "
> +					   << context.frameContext.af.maxVariance
> +					   << " Focus step is "
> +					   << context.frameContext.af.focus;
> +		}
> +	}
> +}
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> new file mode 100644
> index 00000000..b5c11874
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/af.h
> @@ -0,0 +1,54 @@
> +/* 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
> +{
> +	/* The format of y_table. From ipu3-ipa repo */
> +	typedef struct y_table_item {
> +		uint16_t y1_avg;
> +		uint16_t y2_avg;
> +	} y_table_item_t;
> +
> +public:
> +	Af();
> +	~Af();
> +
> +	void prepare(IPAContext &context, ipu3_uapi_params *params) override;
> +	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> +	void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> +
> +private:
> +	int vcmSet(int value);
> +
> +	int vcmFd_;
> +	/* Used for focus scan. */
> +	uint32_t focus_;
> +	/* Recent AF statistic variance. */
> +	double currentVariance_;
> +	/* The frames to be ignore before starting measuring. */
> +	uint32_t ignoreFrame_;
> +};
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index b5d736c1..70ae4e59 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -138,7 +138,7 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>  	}
>  
>  	/* Estimate the quantile mean of the top 2% of the histogram */
> -	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> +	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.7, 1.0);
>  }
>  
>  /**
> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> index 3ec42f72..8574416e 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -1,9 +1,10 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  ipu3_ipa_algorithms = files([
> +    'af.cpp',
>      'agc.cpp',
>      'algorithm.cpp',
>      'awb.cpp',
>      'blc.cpp',
> -    'tone_mapping.cpp',
> +    'tone_mapping.cpp'
>  ])
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 2355a9c7..1064d62d 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -69,6 +69,17 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Number of cells on one line including the ImgU padding
>   */
>  
> +/**
> + * \var IPASessionConfiguration::af
> + * \brief AF parameters configuration of the IPA
> + *
> + * \var IPASessionConfiguration::af.start_x
> + * \brief The start X position of the AF area
> + *
> + * \var IPASessionConfiguration::af.start_y
> + * \brief The start Y position of the AF area
> + */
> +
>  /**
>   * \var IPASessionConfiguration::agc
>   * \brief AGC parameters configuration of the IPA
> @@ -86,6 +97,21 @@ namespace libcamera::ipa::ipu3 {
>   * \brief Maximum analogue gain supported with the configured sensor
>   */
>  
> +/**
> + * \var IPAFrameContext::af
> + * \brief Context for the Automatic Focus algorithm
> + *
> + * \struct  IPAFrameContext::af
> + * \var IPAFrameContext::af.focus
> + * \brief Current position of the lens
> + *
> + * \var IPAFrameContext::af.maxVariance
> + * \brief The maximum variance of the current image.
> + *
> + * \var IPAFrameContext::af.stable
> + * \brief is the image focused?
> + */
> +
>  /**
>   * \var IPAFrameContext::agc
>   * \brief Context for the Automatic Gain Control algorithm
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 1e46c61a..a7ffcbed 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -31,6 +31,11 @@ struct IPASessionConfiguration {
>  		double minAnalogueGain;
>  		double maxAnalogueGain;
>  	} agc;
> +
> +	struct {
> +		uint32_t start_x;
> +		uint32_t start_y;
> +	} af;
>  };
>  
>  struct IPAFrameContext {
> @@ -47,6 +52,12 @@ struct IPAFrameContext {
>  		} gains;
>  	} awb;
>  
> +	struct {
> +		uint32_t focus;
> +		double maxVariance;
> +		bool stable;
> +	} af;
> +
>  	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..f19d0059 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -30,6 +30,7 @@
>  
>  #include "libcamera/internal/mapped_framebuffer.h"
>  
> +#include "algorithms/af.h"
>  #include "algorithms/agc.h"
>  #include "algorithms/algorithm.h"
>  #include "algorithms/awb.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>());
Daniel Scally Nov. 24, 2021, 11:55 p.m. UTC | #3
On 24/11/2021 23:51, Daniel Scally wrote:
> Hi Kate
>
> On 24/11/2021 10:34, 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
>> state and a appropriate focus value.
>>
>> Changes since v2:
>> 1. Add grid configuration interface.
>> 2. Add AF accelerator configuration.
>> 3. Move default focus area to the center of the image.
>>
>> Signed-off-by: Kate Hsuan <hpa@redhat.com>
>
> Thanks a lot for this, it's very cool. I tried it out and observed the
> following:
>
>
> 1. It seems to focus too close initially, then transition to a
> progressively further away focus (passing through the sweet spot where
> the items on my desk behind the Go2 are in focus) and then resets and
> goes back close focus again. It repeats this a couple times , before
> eventually settling in to an ok image. So, seems to work but takes a
> little time to get there.
>
> 2. When the "reset" happens, and when stream stops, there's an audible
> click as the VCM changes position a large amount in one go. I think (at
> least for stream off) that's my fault in the driver; most of them have a
> section in .suspend() that gradually relaxes on power off, I'll update
> the dw9719 to do that too. Not sure that that's the cause of the click
> on the reset though
>
> 3. There's _a lot_ of noise written to the terminal


Pressed send by accident so this came off a little abrupt; sorry :). I
meant to say that I can't quite figure out where it's coming from, but
it's a constant string of integers being written out.
Kate Hsuan Nov. 26, 2021, 7:23 a.m. UTC | #4
Hi Jean-Michel,

On Wed, Nov 24, 2021 at 9:40 PM Jean-Michel Hautbois
<jeanmichel.hautbois@ideasonboard.com> wrote:
>
> Hi Kate,
>
> Thanks for the patch.
>
> On 24/11/2021 11:34, 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
> > state and a appropriate focus value.
> >
> > Changes since v2:
> > 1. Add grid configuration interface.
> > 2. Add AF accelerator configuration.
> > 3. Move default focus area to the center of the image.
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> > ---
> >   src/ipa/ipu3/algorithms/af.cpp      | 284 ++++++++++++++++++++++++++++
> >   src/ipa/ipu3/algorithms/af.h        |  54 ++++++
> >   src/ipa/ipu3/algorithms/agc.cpp     |   2 +-
> >   src/ipa/ipu3/algorithms/meson.build |   3 +-
> >   src/ipa/ipu3/ipa_context.cpp        |  26 +++
> >   src/ipa/ipu3/ipa_context.h          |  11 ++
> >   src/ipa/ipu3/ipu3.cpp               |   2 +
> >   7 files changed, 380 insertions(+), 2 deletions(-)
> >   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..b0359721
> > --- /dev/null
> > +++ b/src/ipa/ipu3/algorithms/af.cpp
> > @@ -0,0 +1,284 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Red Hat
> > + *
> > + * af.cpp - IPU3 auto focus control
> > + */
> > +
> > +#include "af.h"
> > +
> > +#include <algorithm>
> > +#include <chrono>
> > +#include <cmath>
> > +#include <fcntl.h>
> > +#include <numeric>
> > +#include <sys/ioctl.h>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +
> > +#include <linux/videodev2.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 {
> > +
> > +/**
> > + * \class Af
> > + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
> > + *
> > + * This algorithm is used to determine the position of the lens and get a
> > + * focused image. The IPU3 AF accelerator computes the statistics, composed
> > + * by high pass and low pass filtered value and stores in a AF buffer.
> > + * Typically, for a focused image, it has relative high contrast than a
> > + * blurred image, i.e. an out of focus image. Therefore, if an image with the
> > + * highest contrast can be found from the AF scan, the lens' position is the
> > + * best step of the focus.
> > + *
> > + */
> > +
> > +LOG_DEFINE_CATEGORY(IPU3Af)
> > +
> > +/**
> > + * Maximum focus value of the VCM control
> > + * \todo should be obtained from the VCM driver
> > + */
> > +static constexpr uint32_t MaxFocusSteps_ = 1023;
> > +
> > +/* Minimum focus step for searching appropriate focus*/
> > +static constexpr uint32_t MinSearchStep_ = 5;
> > +
> > +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
> > +static constexpr double MaxChange_ = 0.8;
> > +
> > +Af::Af()
> > +     : focus_(0), currentVariance_(0.0)
> > +{
> > +     /**
> > +      * For surface Go 2 back camera VCM (dw9719)
> > +      * \todo move to control class
> > +     */
> > +     vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);
>
> This shouldn't be here :-).
>
> > +}
> > +
> > +Af::~Af()
> > +{
> > +     if (vcmFd_ != -1)
> > +             close(vcmFd_);
> > +}
> > +
> > +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
> > +{
> > +     /* AF grid config */
> > +     params->acc_param.af.grid_cfg.width = 16;
> > +     params->acc_param.af.grid_cfg.height = 16;
> > +     params->acc_param.af.grid_cfg.block_height_log2 = 3;
> > +     params->acc_param.af.grid_cfg.block_width_log2 = 3;
> > +     params->acc_param.af.grid_cfg.height_per_slice = 2;
> > +     /* Start position of AF area */
> > +     params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
> > +     params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;
>
> That is interesting... and probably not right :-).
> You are setting the ImgU to use the same default grid it is using if you
> are not setting params->use.acc_af.
>
> Meaning, you could simplify this by using the default table here:
> https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9579
>
> You can use what is done in IPU3::Awb for instance, we are using the
> default BNR table from
> https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9313
>
> And then you can only change the desired start_x and start_y (I have
> comments on those later).

OK, Thanks for the information, I'll try this BNR table.

>
> > +
> > +     params->acc_param.af.filter_config.y1_sign_vec = 0;
> > +     params->acc_param.af.filter_config.y2_sign_vec = 0;
> > +
> > +     /* b + gb + gr + r = 32 */
> > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_b = 8;
> > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gb = 8;
> > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gr = 8;
> > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_r = 8;
> > +
> > +     /* 2^7 = 128,  a1 + a2 + ... + a12 = 128, log2 of sum of a1 to a12*/
> > +     params->acc_param.af.filter_config.nf.y1_nf = 7;
> > +     params->acc_param.af.filter_config.nf.y2_nf = 7;
> > +
> > +     /* Low pass filter configuration (y1_coeff_n) */
> > +     params->acc_param.af.filter_config.y1_coeff_0.a1 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_0.a2 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_0.a3 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_0.a4 = 0;
> > +
> > +     params->acc_param.af.filter_config.y1_coeff_1.a5 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_1.a6 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_1.a7 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_1.a8 = 0;
> > +
> > +     params->acc_param.af.filter_config.y1_coeff_2.a9 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_2.a10 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_2.a11 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_2.a12 = 128;
> > +
> > +     /* High pass filter configuration (y2_coeff_n) */
> > +     params->acc_param.af.filter_config.y2_coeff_0.a1 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_0.a2 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_0.a3 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_0.a4 = 0;
> > +
> > +     params->acc_param.af.filter_config.y2_coeff_1.a5 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_1.a6 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_1.a7 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_1.a8 = 0;
> > +
> > +     params->acc_param.af.filter_config.y2_coeff_2.a9 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_2.a10 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_2.a11 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_2.a12 = 128;
> > +
> > +     /* Enable AF accelerator */
> > +     params->use.acc_af = 1;
> > +}
> > +
> > +/**
> > + * \brief Configure the Af given a configInfo
> > + * \param[in] context The shared IPA context
> > + * \param[in] configInfo The IPA configuration data
> > + *
> > + * \return 0
> > + */
> > +int Af::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)
> > +{
> > +     /* Determined focus value i.e. current focus value */
> > +     context.frameContext.af.focus = 0;
> > +     /* Maximum variance of the AF statistics */
> > +     context.frameContext.af.maxVariance = 0;
> > +     /* is focused? if it is true, the AF should be in a stable state. */
> > +     context.frameContext.af.stable = false;
> > +     /* Frame to be ignored before start to estimate AF variance. */
> > +     ignoreFrame_ = 10;
> > +
> > +     /*
> > +      * AF default area configuration
> > +      * Move AF area to the center of the image.
> > +      */
> > +     /* AF width is 16x8 = 128 */
> > +     context.configuration.af.start_x = (1280 / 2) - 64;
> > +     context.configuration.af.start_y = (720 / 2) - 64;
>
> This is good, only if the frame output is set to 1280x720.
> If I call:
> 'cam -c2 -C -s width=1920,height=1280'
>
> Then you won't be centered.
> You can get the configuration directly from configInfo.bdsOutputSize (I
> think af is using the BDS output) and then remove the [[maybe_unused]].

Thank you for this, I'll use the bdsOutputSize to determine the centre
of the image.

>
> > +
> > +     return 0;
> > +}
> > +
> > +/**
> > + * \brief Send focus step to the VCM.
> > + * \param[in] value Set lens position.
> > + * \todo It is hardcoded here for the dw9717 VCM and will be moved to the
> > + * subdev control in the future.
> > + */
> > +int Af::vcmSet(int value)
> > +{
> > +     int ret;
> > +     struct v4l2_control ctrl;
> > +     if (vcmFd_ == -1)
> > +             return -EINVAL;
> > +     memset(&ctrl, 0, sizeof(struct v4l2_control));
> > +     ctrl.id = V4L2_CID_FOCUS_ABSOLUTE;
> > +     ctrl.value = value;
> > +     ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
> > +     return ret;
> > +}
>
> This does not belong to the algorithm.

Sure :)

>
> > +
> > +/**
> > + * \brief Determine the max contrast image and lens position. y_table is the
> > + * statictic data from IPU3 and is composed of low pass and high pass filtered
> > + * value. High pass filtered value also represents the sharpness of the image.
> > + * Based on this, if the image with highest variance of the high pass filtered
> > + * value (contrast) during the AF scan, the position of the len should be the
> > + * best focus.
> > + * \param[in] context The shared IPA context.
> > + * \param[in] stats The statistic buffer of 3A from the IPU3.
> > + */
> > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> > +{
> > +     uint32_t total = 0;
> > +     double mean;
> > +     uint64_t var_sum = 0;
> > +     y_table_item_t *y_item;
> > +     int z = 0;
> > +
> > +     y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
> > +
> > +     /**
> > +      * Calculate the mean of each non-zero AF statistics, since IPU3 only determine the AF value
> > +      * for a given grid.
> > +      */
> > +     for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
> > +             printf("%d, ", y_item[z].y2_avg);
> > +             total = total + y_item[z].y2_avg;
> > +             if (y_item[z].y2_avg == 0)
> > +                     break;
> > +     }
> > +     mean = total / z;
> > +
> > +     /* Calculate the variance of every AF statistic value. */
> > +     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;
> > +     }
> > +
> > +     /* Determine the average variance of the frame. */
> > +     currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
> > +     LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
> > +
> > +     if (context.frameContext.af.stable == true) {
> > +             const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
> > +             const double var_ratio = diff_var / context.frameContext.af.maxVariance;
> > +             LOG(IPU3Af, Debug) << "Change ratio: "
> > +                                << var_ratio
> > +                                << " current focus: "
> > +                                << context.frameContext.af.focus;
> > +             /**
> > +              * If the change ratio of contrast is over Maxchange_ (out of focus),
> > +              * trigger AF again.
> > +              */
> > +             if (var_ratio > MaxChange_) {
> > +                     if (ignoreFrame_ == 0) {
> > +                             context.frameContext.af.maxVariance = 0;
> > +                             context.frameContext.af.focus = 0;
> > +                             focus_ = 0;
> > +                             context.frameContext.af.stable = false;
> > +                             ignoreFrame_ = 60;
> > +                     } else
> > +                             ignoreFrame_--;
> > +             } else
> > +                     ignoreFrame_ = 10;
> > +     } else {
> > +             if (ignoreFrame_ != 0)
> > +                     ignoreFrame_--;
> > +             else {
> > +                     /* Find the maximum variance during the AF scan using a greedy strategy */
> > +                     if (currentVariance_ > context.frameContext.af.maxVariance) {
> > +                             context.frameContext.af.maxVariance = currentVariance_;
> > +                             context.frameContext.af.focus = focus_;
> > +                     }
> > +
> > +                     if (focus_ > MaxFocusSteps_) {
> > +                             /* If reach the max step, move lens to the position and set "focus stable". */
> > +                             context.frameContext.af.stable = true;
> > +                             vcmSet(context.frameContext.af.focus);
> > +                     } else {
> > +                             focus_ += MinSearchStep_;
> > +                             vcmSet(focus_);
> > +                     }
> > +                     LOG(IPU3Af, Debug) << "Focus searching max variance is: "
> > +                                        << context.frameContext.af.maxVariance
> > +                                        << " Focus step is "
> > +                                        << context.frameContext.af.focus;
> > +             }
> > +     }
> > +}
>
> I won't comment the algorithm yet, as you already have some changes to
> make according to Kieran's comments ;-).
>
> > +
> > +} /* namespace ipa::ipu3::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> > new file mode 100644
> > index 00000000..b5c11874
> > --- /dev/null
> > +++ b/src/ipa/ipu3/algorithms/af.h
> > @@ -0,0 +1,54 @@
> > +/* 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
> > +{
> > +     /* The format of y_table. From ipu3-ipa repo */
> > +     typedef struct y_table_item {
> > +             uint16_t y1_avg;
> > +             uint16_t y2_avg;
> > +     } y_table_item_t;
> > +
> > +public:
> > +     Af();
> > +     ~Af();
> > +
> > +     void prepare(IPAContext &context, ipu3_uapi_params *params) override;
> > +     int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> > +     void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> > +
> > +private:
> > +     int vcmSet(int value);
> > +
> > +     int vcmFd_;
> > +     /* Used for focus scan. */
> > +     uint32_t focus_;
> > +     /* Recent AF statistic variance. */
> > +     double currentVariance_;
> > +     /* The frames to be ignore before starting measuring. */
> > +     uint32_t ignoreFrame_;
> > +};
> > +
> > +} /* namespace ipa::ipu3::algorithms */
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > index b5d736c1..70ae4e59 100644
> > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > @@ -138,7 +138,7 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
> >       }
> >
> >       /* Estimate the quantile mean of the top 2% of the histogram */
> > -     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> > +     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.7, 1.0);
>
> Why are you changing this ? It is the top 30% of the cumulative histogram ?
>

Sorry about this. It's my fault. AE connot work properly on my surface
Go 2 back camera. I try to make it more stable for the exposure. So I
expend the mean from the top 2% to 30%. It works for the front camera
before but doesn't work for the back camera.
I'll remove this experimental code in the v3.

> >   }
> >
> >   /**
> > diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> > index 3ec42f72..8574416e 100644
> > --- a/src/ipa/ipu3/algorithms/meson.build
> > +++ b/src/ipa/ipu3/algorithms/meson.build
> > @@ -1,9 +1,10 @@
> >   # SPDX-License-Identifier: CC0-1.0
> >
> >   ipu3_ipa_algorithms = files([
> > +    'af.cpp',
> >       'agc.cpp',
> >       'algorithm.cpp',
> >       'awb.cpp',
> >       'blc.cpp',
> > -    'tone_mapping.cpp',
> > +    'tone_mapping.cpp'
> >   ])
> > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> > index 2355a9c7..1064d62d 100644
> > --- a/src/ipa/ipu3/ipa_context.cpp
> > +++ b/src/ipa/ipu3/ipa_context.cpp
> > @@ -69,6 +69,17 @@ namespace libcamera::ipa::ipu3 {
> >    * \brief Number of cells on one line including the ImgU padding
> >    */
> >
> > +/**
> > + * \var IPASessionConfiguration::af
> > + * \brief AF parameters configuration of the IPA
> > + *
> > + * \var IPASessionConfiguration::af.start_x
> > + * \brief The start X position of the AF area
> > + *
> > + * \var IPASessionConfiguration::af.start_y
> > + * \brief The start Y position of the AF area
> > + */
> > +
> >   /**
> >    * \var IPASessionConfiguration::agc
> >    * \brief AGC parameters configuration of the IPA
> > @@ -86,6 +97,21 @@ namespace libcamera::ipa::ipu3 {
> >    * \brief Maximum analogue gain supported with the configured sensor
> >    */
> >
> > +/**
> > + * \var IPAFrameContext::af
> > + * \brief Context for the Automatic Focus algorithm
> > + *
> > + * \struct  IPAFrameContext::af
> > + * \var IPAFrameContext::af.focus
> > + * \brief Current position of the lens
> > + *
> > + * \var IPAFrameContext::af.maxVariance
> > + * \brief The maximum variance of the current image.
> > + *
> > + * \var IPAFrameContext::af.stable
> > + * \brief is the image focused?
> > + */
> > +
> >   /**
> >    * \var IPAFrameContext::agc
> >    * \brief Context for the Automatic Gain Control algorithm
> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > index 1e46c61a..a7ffcbed 100644
> > --- a/src/ipa/ipu3/ipa_context.h
> > +++ b/src/ipa/ipu3/ipa_context.h
> > @@ -31,6 +31,11 @@ struct IPASessionConfiguration {
> >               double minAnalogueGain;
> >               double maxAnalogueGain;
> >       } agc;
> > +
> > +     struct {
> > +             uint32_t start_x;
> > +             uint32_t start_y;
> > +     } af;
>
> Those are __u16 in the kernel header file, maybe can we keep the same type ?

Sure, I'll modify it in my v3.

>
> >   };
> >
> >   struct IPAFrameContext {
> > @@ -47,6 +52,12 @@ struct IPAFrameContext {
> >               } gains;
> >       } awb;
> >
> > +     struct {
> > +             uint32_t focus;
> > +             double maxVariance;
> > +             bool stable;
> > +     } af;
> > +
> >       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..f19d0059 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -30,6 +30,7 @@
> >
> >   #include "libcamera/internal/mapped_framebuffer.h"
> >
> > +#include "algorithms/af.h"
> >   #include "algorithms/agc.h"
> >   #include "algorithms/algorithm.h"
> >   #include "algorithms/awb.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>());
> >
>


Thank you :)
Jean-Michel Hautbois Nov. 26, 2021, 7:58 a.m. UTC | #5
Hi Kate,

On 26/11/2021 08:23, Kate Hsuan wrote:
> Hi Jean-Michel,
> 
> On Wed, Nov 24, 2021 at 9:40 PM Jean-Michel Hautbois
> <jeanmichel.hautbois@ideasonboard.com> wrote:
>>
>> Hi Kate,
>>
>> Thanks for the patch.
>>
>> On 24/11/2021 11:34, 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
>>> state and a appropriate focus value.
>>>
>>> Changes since v2:
>>> 1. Add grid configuration interface.
>>> 2. Add AF accelerator configuration.
>>> 3. Move default focus area to the center of the image.
>>>
>>> Signed-off-by: Kate Hsuan <hpa@redhat.com>
>>> ---
>>>    src/ipa/ipu3/algorithms/af.cpp      | 284 ++++++++++++++++++++++++++++
>>>    src/ipa/ipu3/algorithms/af.h        |  54 ++++++
>>>    src/ipa/ipu3/algorithms/agc.cpp     |   2 +-
>>>    src/ipa/ipu3/algorithms/meson.build |   3 +-
>>>    src/ipa/ipu3/ipa_context.cpp        |  26 +++
>>>    src/ipa/ipu3/ipa_context.h          |  11 ++
>>>    src/ipa/ipu3/ipu3.cpp               |   2 +
>>>    7 files changed, 380 insertions(+), 2 deletions(-)
>>>    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..b0359721
>>> --- /dev/null
>>> +++ b/src/ipa/ipu3/algorithms/af.cpp
>>> @@ -0,0 +1,284 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2021, Red Hat
>>> + *
>>> + * af.cpp - IPU3 auto focus control
>>> + */
>>> +
>>> +#include "af.h"
>>> +
>>> +#include <algorithm>
>>> +#include <chrono>
>>> +#include <cmath>
>>> +#include <fcntl.h>
>>> +#include <numeric>
>>> +#include <sys/ioctl.h>
>>> +#include <sys/stat.h>
>>> +#include <sys/types.h>
>>> +#include <unistd.h>
>>> +
>>> +#include <linux/videodev2.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 {
>>> +
>>> +/**
>>> + * \class Af
>>> + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
>>> + *
>>> + * This algorithm is used to determine the position of the lens and get a
>>> + * focused image. The IPU3 AF accelerator computes the statistics, composed
>>> + * by high pass and low pass filtered value and stores in a AF buffer.
>>> + * Typically, for a focused image, it has relative high contrast than a
>>> + * blurred image, i.e. an out of focus image. Therefore, if an image with the
>>> + * highest contrast can be found from the AF scan, the lens' position is the
>>> + * best step of the focus.
>>> + *
>>> + */
>>> +
>>> +LOG_DEFINE_CATEGORY(IPU3Af)
>>> +
>>> +/**
>>> + * Maximum focus value of the VCM control
>>> + * \todo should be obtained from the VCM driver
>>> + */
>>> +static constexpr uint32_t MaxFocusSteps_ = 1023;
>>> +
>>> +/* Minimum focus step for searching appropriate focus*/
>>> +static constexpr uint32_t MinSearchStep_ = 5;
>>> +
>>> +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
>>> +static constexpr double MaxChange_ = 0.8;
>>> +
>>> +Af::Af()
>>> +     : focus_(0), currentVariance_(0.0)
>>> +{
>>> +     /**
>>> +      * For surface Go 2 back camera VCM (dw9719)
>>> +      * \todo move to control class
>>> +     */
>>> +     vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);
>>
>> This shouldn't be here :-).
>>
>>> +}
>>> +
>>> +Af::~Af()
>>> +{
>>> +     if (vcmFd_ != -1)
>>> +             close(vcmFd_);
>>> +}
>>> +
>>> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
>>> +{
>>> +     /* AF grid config */
>>> +     params->acc_param.af.grid_cfg.width = 16;
>>> +     params->acc_param.af.grid_cfg.height = 16;
>>> +     params->acc_param.af.grid_cfg.block_height_log2 = 3;
>>> +     params->acc_param.af.grid_cfg.block_width_log2 = 3;
>>> +     params->acc_param.af.grid_cfg.height_per_slice = 2;
>>> +     /* Start position of AF area */
>>> +     params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
>>> +     params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;
>>
>> That is interesting... and probably not right :-).
>> You are setting the ImgU to use the same default grid it is using if you
>> are not setting params->use.acc_af.
>>
>> Meaning, you could simplify this by using the default table here:
>> https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9579
>>
>> You can use what is done in IPU3::Awb for instance, we are using the
>> default BNR table from
>> https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9313
>>
>> And then you can only change the desired start_x and start_y (I have
>> comments on those later).
> 
> OK, Thanks for the information, I'll try this BNR table.
> 
>>
>>> +
>>> +     params->acc_param.af.filter_config.y1_sign_vec = 0;
>>> +     params->acc_param.af.filter_config.y2_sign_vec = 0;
>>> +
>>> +     /* b + gb + gr + r = 32 */
>>> +     params->acc_param.af.filter_config.y_calc.y_gen_rate_b = 8;
>>> +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gb = 8;
>>> +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gr = 8;
>>> +     params->acc_param.af.filter_config.y_calc.y_gen_rate_r = 8;
>>> +
>>> +     /* 2^7 = 128,  a1 + a2 + ... + a12 = 128, log2 of sum of a1 to a12*/
>>> +     params->acc_param.af.filter_config.nf.y1_nf = 7;
>>> +     params->acc_param.af.filter_config.nf.y2_nf = 7;
>>> +
>>> +     /* Low pass filter configuration (y1_coeff_n) */
>>> +     params->acc_param.af.filter_config.y1_coeff_0.a1 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_0.a2 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_0.a3 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_0.a4 = 0;
>>> +
>>> +     params->acc_param.af.filter_config.y1_coeff_1.a5 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_1.a6 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_1.a7 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_1.a8 = 0;
>>> +
>>> +     params->acc_param.af.filter_config.y1_coeff_2.a9 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_2.a10 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_2.a11 = 0;
>>> +     params->acc_param.af.filter_config.y1_coeff_2.a12 = 128;
>>> +
>>> +     /* High pass filter configuration (y2_coeff_n) */
>>> +     params->acc_param.af.filter_config.y2_coeff_0.a1 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_0.a2 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_0.a3 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_0.a4 = 0;
>>> +
>>> +     params->acc_param.af.filter_config.y2_coeff_1.a5 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_1.a6 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_1.a7 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_1.a8 = 0;
>>> +
>>> +     params->acc_param.af.filter_config.y2_coeff_2.a9 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_2.a10 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_2.a11 = 0;
>>> +     params->acc_param.af.filter_config.y2_coeff_2.a12 = 128;
>>> +
>>> +     /* Enable AF accelerator */
>>> +     params->use.acc_af = 1;
>>> +}
>>> +
>>> +/**
>>> + * \brief Configure the Af given a configInfo
>>> + * \param[in] context The shared IPA context
>>> + * \param[in] configInfo The IPA configuration data
>>> + *
>>> + * \return 0
>>> + */
>>> +int Af::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)
>>> +{
>>> +     /* Determined focus value i.e. current focus value */
>>> +     context.frameContext.af.focus = 0;
>>> +     /* Maximum variance of the AF statistics */
>>> +     context.frameContext.af.maxVariance = 0;
>>> +     /* is focused? if it is true, the AF should be in a stable state. */
>>> +     context.frameContext.af.stable = false;
>>> +     /* Frame to be ignored before start to estimate AF variance. */
>>> +     ignoreFrame_ = 10;
>>> +
>>> +     /*
>>> +      * AF default area configuration
>>> +      * Move AF area to the center of the image.
>>> +      */
>>> +     /* AF width is 16x8 = 128 */
>>> +     context.configuration.af.start_x = (1280 / 2) - 64;
>>> +     context.configuration.af.start_y = (720 / 2) - 64;
>>
>> This is good, only if the frame output is set to 1280x720.
>> If I call:
>> 'cam -c2 -C -s width=1920,height=1280'
>>
>> Then you won't be centered.
>> You can get the configuration directly from configInfo.bdsOutputSize (I
>> think af is using the BDS output) and then remove the [[maybe_unused]].
> 
> Thank you for this, I'll use the bdsOutputSize to determine the centre
> of the image.
> 
>>
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +/**
>>> + * \brief Send focus step to the VCM.
>>> + * \param[in] value Set lens position.
>>> + * \todo It is hardcoded here for the dw9717 VCM and will be moved to the
>>> + * subdev control in the future.
>>> + */
>>> +int Af::vcmSet(int value)
>>> +{
>>> +     int ret;
>>> +     struct v4l2_control ctrl;
>>> +     if (vcmFd_ == -1)
>>> +             return -EINVAL;
>>> +     memset(&ctrl, 0, sizeof(struct v4l2_control));
>>> +     ctrl.id = V4L2_CID_FOCUS_ABSOLUTE;
>>> +     ctrl.value = value;
>>> +     ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
>>> +     return ret;
>>> +}
>>
>> This does not belong to the algorithm.
> 
> Sure :)
> 
>>
>>> +
>>> +/**
>>> + * \brief Determine the max contrast image and lens position. y_table is the
>>> + * statictic data from IPU3 and is composed of low pass and high pass filtered
>>> + * value. High pass filtered value also represents the sharpness of the image.
>>> + * Based on this, if the image with highest variance of the high pass filtered
>>> + * value (contrast) during the AF scan, the position of the len should be the
>>> + * best focus.
>>> + * \param[in] context The shared IPA context.
>>> + * \param[in] stats The statistic buffer of 3A from the IPU3.
>>> + */
>>> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>>> +{
>>> +     uint32_t total = 0;
>>> +     double mean;
>>> +     uint64_t var_sum = 0;
>>> +     y_table_item_t *y_item;
>>> +     int z = 0;
>>> +
>>> +     y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
>>> +
>>> +     /**
>>> +      * Calculate the mean of each non-zero AF statistics, since IPU3 only determine the AF value
>>> +      * for a given grid.
>>> +      */
>>> +     for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
>>> +             printf("%d, ", y_item[z].y2_avg);
>>> +             total = total + y_item[z].y2_avg;
>>> +             if (y_item[z].y2_avg == 0)
>>> +                     break;
>>> +     }
>>> +     mean = total / z;
>>> +
>>> +     /* Calculate the variance of every AF statistic value. */
>>> +     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;
>>> +     }
>>> +
>>> +     /* Determine the average variance of the frame. */
>>> +     currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
>>> +     LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
>>> +
>>> +     if (context.frameContext.af.stable == true) {
>>> +             const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
>>> +             const double var_ratio = diff_var / context.frameContext.af.maxVariance;
>>> +             LOG(IPU3Af, Debug) << "Change ratio: "
>>> +                                << var_ratio
>>> +                                << " current focus: "
>>> +                                << context.frameContext.af.focus;
>>> +             /**
>>> +              * If the change ratio of contrast is over Maxchange_ (out of focus),
>>> +              * trigger AF again.
>>> +              */
>>> +             if (var_ratio > MaxChange_) {
>>> +                     if (ignoreFrame_ == 0) {
>>> +                             context.frameContext.af.maxVariance = 0;
>>> +                             context.frameContext.af.focus = 0;
>>> +                             focus_ = 0;
>>> +                             context.frameContext.af.stable = false;
>>> +                             ignoreFrame_ = 60;
>>> +                     } else
>>> +                             ignoreFrame_--;
>>> +             } else
>>> +                     ignoreFrame_ = 10;
>>> +     } else {
>>> +             if (ignoreFrame_ != 0)
>>> +                     ignoreFrame_--;
>>> +             else {
>>> +                     /* Find the maximum variance during the AF scan using a greedy strategy */
>>> +                     if (currentVariance_ > context.frameContext.af.maxVariance) {
>>> +                             context.frameContext.af.maxVariance = currentVariance_;
>>> +                             context.frameContext.af.focus = focus_;
>>> +                     }
>>> +
>>> +                     if (focus_ > MaxFocusSteps_) {
>>> +                             /* If reach the max step, move lens to the position and set "focus stable". */
>>> +                             context.frameContext.af.stable = true;
>>> +                             vcmSet(context.frameContext.af.focus);
>>> +                     } else {
>>> +                             focus_ += MinSearchStep_;
>>> +                             vcmSet(focus_);
>>> +                     }
>>> +                     LOG(IPU3Af, Debug) << "Focus searching max variance is: "
>>> +                                        << context.frameContext.af.maxVariance
>>> +                                        << " Focus step is "
>>> +                                        << context.frameContext.af.focus;
>>> +             }
>>> +     }
>>> +}
>>
>> I won't comment the algorithm yet, as you already have some changes to
>> make according to Kieran's comments ;-).
>>
>>> +
>>> +} /* namespace ipa::ipu3::algorithms */
>>> +
>>> +} /* namespace libcamera */
>>> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
>>> new file mode 100644
>>> index 00000000..b5c11874
>>> --- /dev/null
>>> +++ b/src/ipa/ipu3/algorithms/af.h
>>> @@ -0,0 +1,54 @@
>>> +/* 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
>>> +{
>>> +     /* The format of y_table. From ipu3-ipa repo */
>>> +     typedef struct y_table_item {
>>> +             uint16_t y1_avg;
>>> +             uint16_t y2_avg;
>>> +     } y_table_item_t;
>>> +
>>> +public:
>>> +     Af();
>>> +     ~Af();
>>> +
>>> +     void prepare(IPAContext &context, ipu3_uapi_params *params) override;
>>> +     int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
>>> +     void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
>>> +
>>> +private:
>>> +     int vcmSet(int value);
>>> +
>>> +     int vcmFd_;
>>> +     /* Used for focus scan. */
>>> +     uint32_t focus_;
>>> +     /* Recent AF statistic variance. */
>>> +     double currentVariance_;
>>> +     /* The frames to be ignore before starting measuring. */
>>> +     uint32_t ignoreFrame_;
>>> +};
>>> +
>>> +} /* namespace ipa::ipu3::algorithms */
>>> +
>>> +} /* namespace libcamera */
>>> +
>>> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
>>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>>> index b5d736c1..70ae4e59 100644
>>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>>> @@ -138,7 +138,7 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
>>>        }
>>>
>>>        /* Estimate the quantile mean of the top 2% of the histogram */
>>> -     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
>>> +     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.7, 1.0);
>>
>> Why are you changing this ? It is the top 30% of the cumulative histogram ?
>>
> 
> Sorry about this. It's my fault. AE connot work properly on my surface
> Go 2 back camera. I try to make it more stable for the exposure. So I
> expend the mean from the top 2% to 30%. It works for the front camera
> before but doesn't work for the back camera.
> I'll remove this experimental code in the v3.

Have you this patch ?
https://github.com/linux-surface/kernel/pull/113

I don't know which kernel you are using, but back camera has an issue 
when approaching the max exposure, maybe is it your issue ?

> 
>>>    }
>>>
>>>    /**
>>> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
>>> index 3ec42f72..8574416e 100644
>>> --- a/src/ipa/ipu3/algorithms/meson.build
>>> +++ b/src/ipa/ipu3/algorithms/meson.build
>>> @@ -1,9 +1,10 @@
>>>    # SPDX-License-Identifier: CC0-1.0
>>>
>>>    ipu3_ipa_algorithms = files([
>>> +    'af.cpp',
>>>        'agc.cpp',
>>>        'algorithm.cpp',
>>>        'awb.cpp',
>>>        'blc.cpp',
>>> -    'tone_mapping.cpp',
>>> +    'tone_mapping.cpp'
>>>    ])
>>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
>>> index 2355a9c7..1064d62d 100644
>>> --- a/src/ipa/ipu3/ipa_context.cpp
>>> +++ b/src/ipa/ipu3/ipa_context.cpp
>>> @@ -69,6 +69,17 @@ namespace libcamera::ipa::ipu3 {
>>>     * \brief Number of cells on one line including the ImgU padding
>>>     */
>>>
>>> +/**
>>> + * \var IPASessionConfiguration::af
>>> + * \brief AF parameters configuration of the IPA
>>> + *
>>> + * \var IPASessionConfiguration::af.start_x
>>> + * \brief The start X position of the AF area
>>> + *
>>> + * \var IPASessionConfiguration::af.start_y
>>> + * \brief The start Y position of the AF area
>>> + */
>>> +
>>>    /**
>>>     * \var IPASessionConfiguration::agc
>>>     * \brief AGC parameters configuration of the IPA
>>> @@ -86,6 +97,21 @@ namespace libcamera::ipa::ipu3 {
>>>     * \brief Maximum analogue gain supported with the configured sensor
>>>     */
>>>
>>> +/**
>>> + * \var IPAFrameContext::af
>>> + * \brief Context for the Automatic Focus algorithm
>>> + *
>>> + * \struct  IPAFrameContext::af
>>> + * \var IPAFrameContext::af.focus
>>> + * \brief Current position of the lens
>>> + *
>>> + * \var IPAFrameContext::af.maxVariance
>>> + * \brief The maximum variance of the current image.
>>> + *
>>> + * \var IPAFrameContext::af.stable
>>> + * \brief is the image focused?
>>> + */
>>> +
>>>    /**
>>>     * \var IPAFrameContext::agc
>>>     * \brief Context for the Automatic Gain Control algorithm
>>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>>> index 1e46c61a..a7ffcbed 100644
>>> --- a/src/ipa/ipu3/ipa_context.h
>>> +++ b/src/ipa/ipu3/ipa_context.h
>>> @@ -31,6 +31,11 @@ struct IPASessionConfiguration {
>>>                double minAnalogueGain;
>>>                double maxAnalogueGain;
>>>        } agc;
>>> +
>>> +     struct {
>>> +             uint32_t start_x;
>>> +             uint32_t start_y;
>>> +     } af;
>>
>> Those are __u16 in the kernel header file, maybe can we keep the same type ?
> 
> Sure, I'll modify it in my v3.
> 
>>
>>>    };
>>>
>>>    struct IPAFrameContext {
>>> @@ -47,6 +52,12 @@ struct IPAFrameContext {
>>>                } gains;
>>>        } awb;
>>>
>>> +     struct {
>>> +             uint32_t focus;
>>> +             double maxVariance;
>>> +             bool stable;
>>> +     } af;
>>> +
>>>        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..f19d0059 100644
>>> --- a/src/ipa/ipu3/ipu3.cpp
>>> +++ b/src/ipa/ipu3/ipu3.cpp
>>> @@ -30,6 +30,7 @@
>>>
>>>    #include "libcamera/internal/mapped_framebuffer.h"
>>>
>>> +#include "algorithms/af.h"
>>>    #include "algorithms/agc.h"
>>>    #include "algorithms/algorithm.h"
>>>    #include "algorithms/awb.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>());
>>>
>>
> 
> 
> Thank you :)
>
Kate Hsuan Nov. 26, 2021, 8:55 a.m. UTC | #6
Hi Daniel,

On Thu, Nov 25, 2021 at 7:52 AM Daniel Scally <djrscally@gmail.com> wrote:
>
> Hi Kate
>
> On 24/11/2021 10:34, 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
> > state and a appropriate focus value.
> >
> > Changes since v2:
> > 1. Add grid configuration interface.
> > 2. Add AF accelerator configuration.
> > 3. Move default focus area to the center of the image.
> >
> > Signed-off-by: Kate Hsuan <hpa@redhat.com>
>
>
> Thanks a lot for this, it's very cool. I tried it out and observed the
> following:
>
>
> 1. It seems to focus too close initially, then transition to a
> progressively further away focus (passing through the sweet spot where
> the items on my desk behind the Go2 are in focus) and then resets and
> goes back close focus again. It repeats this a couple times , before
> eventually settling in to an ok image. So, seems to work but takes a
> little time to get there.

The linear scan is always slow. :( I'll try to improve the AF speed.
Now, I manually configure the exposure time and gain to get a stable
AE result then start the AF scan. It is faster than using a floating
AE result. Also, the minimal VCM step is 5 for each AF sampling, some
of the images may require a higher resolution of the VCM step to get a
clear image.

>
> 2. When the "reset" happens, and when stream stops, there's an audible
> click as the VCM changes position a large amount in one go. I think (at
> least for stream off) that's my fault in the driver; most of them have a
> section in .suspend() that gradually relaxes on power off, I'll update
> the dw9719 to do that too. Not sure that that's the cause of the click
> on the reset though

Yes. When I close() the VCM fd, I can hear the "click" from the VCM.

>
> 3. There's _a lot_ of noise written to the terminal

Sorry about that. I would like to observe the AF statistic value of
some high-contrast images or objects.
I'll remove all terminal output in v3.

>
>
> > ---
> >  src/ipa/ipu3/algorithms/af.cpp      | 284 ++++++++++++++++++++++++++++
> >  src/ipa/ipu3/algorithms/af.h        |  54 ++++++
> >  src/ipa/ipu3/algorithms/agc.cpp     |   2 +-
> >  src/ipa/ipu3/algorithms/meson.build |   3 +-
> >  src/ipa/ipu3/ipa_context.cpp        |  26 +++
> >  src/ipa/ipu3/ipa_context.h          |  11 ++
> >  src/ipa/ipu3/ipu3.cpp               |   2 +
> >  7 files changed, 380 insertions(+), 2 deletions(-)
> >  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..b0359721
> > --- /dev/null
> > +++ b/src/ipa/ipu3/algorithms/af.cpp
> > @@ -0,0 +1,284 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Red Hat
> > + *
> > + * af.cpp - IPU3 auto focus control
> > + */
> > +
> > +#include "af.h"
> > +
> > +#include <algorithm>
> > +#include <chrono>
> > +#include <cmath>
> > +#include <fcntl.h>
> > +#include <numeric>
> > +#include <sys/ioctl.h>
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +
> > +#include <linux/videodev2.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 {
> > +
> > +/**
> > + * \class Af
> > + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
> > + *
> > + * This algorithm is used to determine the position of the lens and get a
> > + * focused image. The IPU3 AF accelerator computes the statistics, composed
> > + * by high pass and low pass filtered value and stores in a AF buffer.
> > + * Typically, for a focused image, it has relative high contrast than a
> > + * blurred image, i.e. an out of focus image. Therefore, if an image with the
> > + * highest contrast can be found from the AF scan, the lens' position is the
> > + * best step of the focus.
> > + *
> > + */
> > +
> > +LOG_DEFINE_CATEGORY(IPU3Af)
> > +
> > +/**
> > + * Maximum focus value of the VCM control
> > + * \todo should be obtained from the VCM driver
> > + */
> > +static constexpr uint32_t MaxFocusSteps_ = 1023;
> > +
> > +/* Minimum focus step for searching appropriate focus*/
> > +static constexpr uint32_t MinSearchStep_ = 5;
> > +
> > +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
> > +static constexpr double MaxChange_ = 0.8;
> > +
> > +Af::Af()
> > +     : focus_(0), currentVariance_(0.0)
> > +{
> > +     /**
> > +      * For surface Go 2 back camera VCM (dw9719)
> > +      * \todo move to control class
> > +     */
> > +     vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);
> > +}
> > +
> > +Af::~Af()
> > +{
> > +     if (vcmFd_ != -1)
> > +             close(vcmFd_);
> > +}
>
>
> FYI I'm going to work on moving these bits out so that the actual
> setting of the v4l2 control is done via an instance of the CameraLens
> class from Han-Lin's recent series. Ditto the calls to the vcmSet()
> function in ::process(). This should mean it's nicely agnostic (i.e. no
> hardcoded v4l-subdev, nor sensor/vcm pairs)
>

Great. Nice to know this :) and I saw the patch about it. Thanks a lot!!

>
> > +
> > +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
> > +{
> > +     /* AF grid config */
> > +     params->acc_param.af.grid_cfg.width = 16;
> > +     params->acc_param.af.grid_cfg.height = 16;
> > +     params->acc_param.af.grid_cfg.block_height_log2 = 3;
> > +     params->acc_param.af.grid_cfg.block_width_log2 = 3;
> > +     params->acc_param.af.grid_cfg.height_per_slice = 2;
> > +     /* Start position of AF area */
> > +     params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
> > +     params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;
> > +
> > +     params->acc_param.af.filter_config.y1_sign_vec = 0;
> > +     params->acc_param.af.filter_config.y2_sign_vec = 0;
> > +
> > +     /* b + gb + gr + r = 32 */
> > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_b = 8;
> > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gb = 8;
> > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gr = 8;
> > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_r = 8;
> > +
> > +     /* 2^7 = 128,  a1 + a2 + ... + a12 = 128, log2 of sum of a1 to a12*/
> > +     params->acc_param.af.filter_config.nf.y1_nf = 7;
> > +     params->acc_param.af.filter_config.nf.y2_nf = 7;
> > +
> > +     /* Low pass filter configuration (y1_coeff_n) */
> > +     params->acc_param.af.filter_config.y1_coeff_0.a1 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_0.a2 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_0.a3 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_0.a4 = 0;
> > +
> > +     params->acc_param.af.filter_config.y1_coeff_1.a5 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_1.a6 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_1.a7 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_1.a8 = 0;
> > +
> > +     params->acc_param.af.filter_config.y1_coeff_2.a9 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_2.a10 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_2.a11 = 0;
> > +     params->acc_param.af.filter_config.y1_coeff_2.a12 = 128;
> > +
> > +     /* High pass filter configuration (y2_coeff_n) */
> > +     params->acc_param.af.filter_config.y2_coeff_0.a1 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_0.a2 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_0.a3 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_0.a4 = 0;
> > +
> > +     params->acc_param.af.filter_config.y2_coeff_1.a5 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_1.a6 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_1.a7 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_1.a8 = 0;
> > +
> > +     params->acc_param.af.filter_config.y2_coeff_2.a9 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_2.a10 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_2.a11 = 0;
> > +     params->acc_param.af.filter_config.y2_coeff_2.a12 = 128;
> > +
> > +     /* Enable AF accelerator */
> > +     params->use.acc_af = 1;
> > +}
> > +
> > +/**
> > + * \brief Configure the Af given a configInfo
> > + * \param[in] context The shared IPA context
> > + * \param[in] configInfo The IPA configuration data
> > + *
> > + * \return 0
> > + */
> > +int Af::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)
> > +{
> > +     /* Determined focus value i.e. current focus value */
> > +     context.frameContext.af.focus = 0;
> > +     /* Maximum variance of the AF statistics */
> > +     context.frameContext.af.maxVariance = 0;
> > +     /* is focused? if it is true, the AF should be in a stable state. */
> > +     context.frameContext.af.stable = false;
> > +     /* Frame to be ignored before start to estimate AF variance. */
> > +     ignoreFrame_ = 10;
> > +
> > +     /*
> > +      * AF default area configuration
> > +      * Move AF area to the center of the image.
> > +      */
> > +     /* AF width is 16x8 = 128 */
> > +     context.configuration.af.start_x = (1280 / 2) - 64;
> > +     context.configuration.af.start_y = (720 / 2) - 64;
> > +
> > +     return 0;
> > +}
> > +
> > +/**
> > + * \brief Send focus step to the VCM.
> > + * \param[in] value Set lens position.
> > + * \todo It is hardcoded here for the dw9717 VCM and will be moved to the
> > + * subdev control in the future.
> > + */
> > +int Af::vcmSet(int value)
> > +{
> > +     int ret;
> > +     struct v4l2_control ctrl;
> > +     if (vcmFd_ == -1)
> > +             return -EINVAL;
> > +     memset(&ctrl, 0, sizeof(struct v4l2_control));
> > +     ctrl.id = V4L2_CID_FOCUS_ABSOLUTE;
> > +     ctrl.value = value;
> > +     ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
> > +     return ret;
> > +}
> > +
> > +/**
> > + * \brief Determine the max contrast image and lens position. y_table is the
> > + * statictic data from IPU3 and is composed of low pass and high pass filtered
> > + * value. High pass filtered value also represents the sharpness of the image.
> > + * Based on this, if the image with highest variance of the high pass filtered
> > + * value (contrast) during the AF scan, the position of the len should be the
> > + * best focus.
> > + * \param[in] context The shared IPA context.
> > + * \param[in] stats The statistic buffer of 3A from the IPU3.
> > + */
> > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> > +{
> > +     uint32_t total = 0;
> > +     double mean;
> > +     uint64_t var_sum = 0;
> > +     y_table_item_t *y_item;
> > +     int z = 0;
> > +
> > +     y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
>
>
> I get a warning here when compiling:
>
>
> ../src/ipa/ipu3/algorithms/af.cpp: In member function ‘virtual void
> libcamera::ipa::ipu3::algorithms::Af::process(libcamera::ipa::ipu3::IPAContext&,
> const ipu3_uapi_stats_3a*)’:
> ../src/ipa/ipu3/algorithms/af.cpp:209:50: error: taking address of
> packed member of ‘ipu3_uapi_stats_3a’ may result in an unaligned pointer
> value [-Werror=address-of-packed-member]
>   209 |  y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
>       |                             ~~~~~~~~~~~~~~~~~~~~~^~~~~~~
>

I'll double-check this and fix it in my v3.



> > +
> > +     /**
> > +      * Calculate the mean of each non-zero AF statistics, since IPU3 only determine the AF value
> > +      * for a given grid.
> > +      */
> > +     for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
> > +             printf("%d, ", y_item[z].y2_avg);
> > +             total = total + y_item[z].y2_avg;
> > +             if (y_item[z].y2_avg == 0)
> > +                     break;
> > +     }
> > +     mean = total / z;
> > +
> > +     /* Calculate the variance of every AF statistic value. */
> > +     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;
> > +     }
> > +
> > +     /* Determine the average variance of the frame. */
> > +     currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
> > +     LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
> > +
> > +     if (context.frameContext.af.stable == true) {
> > +             const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
> > +             const double var_ratio = diff_var / context.frameContext.af.maxVariance;
> > +             LOG(IPU3Af, Debug) << "Change ratio: "
> > +                                << var_ratio
> > +                                << " current focus: "
> > +                                << context.frameContext.af.focus;
> > +             /**
> > +              * If the change ratio of contrast is over Maxchange_ (out of focus),
> > +              * trigger AF again.
> > +              */
> > +             if (var_ratio > MaxChange_) {
> > +                     if (ignoreFrame_ == 0) {
> > +                             context.frameContext.af.maxVariance = 0;
> > +                             context.frameContext.af.focus = 0;
> > +                             focus_ = 0;
> > +                             context.frameContext.af.stable = false;
> > +                             ignoreFrame_ = 60;
> > +                     } else
> > +                             ignoreFrame_--;
> > +             } else
> > +                     ignoreFrame_ = 10;
> > +     } else {
> > +             if (ignoreFrame_ != 0)
> > +                     ignoreFrame_--;
> > +             else {
> > +                     /* Find the maximum variance during the AF scan using a greedy strategy */
> > +                     if (currentVariance_ > context.frameContext.af.maxVariance) {
> > +                             context.frameContext.af.maxVariance = currentVariance_;
> > +                             context.frameContext.af.focus = focus_;
> > +                     }
> > +
> > +                     if (focus_ > MaxFocusSteps_) {
> > +                             /* If reach the max step, move lens to the position and set "focus stable". */
> > +                             context.frameContext.af.stable = true;
> > +                             vcmSet(context.frameContext.af.focus);
> > +                     } else {
> > +                             focus_ += MinSearchStep_;
> > +                             vcmSet(focus_);
> > +                     }
> > +                     LOG(IPU3Af, Debug) << "Focus searching max variance is: "
> > +                                        << context.frameContext.af.maxVariance
> > +                                        << " Focus step is "
> > +                                        << context.frameContext.af.focus;
> > +             }
> > +     }
> > +}
> > +
> > +} /* namespace ipa::ipu3::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> > new file mode 100644
> > index 00000000..b5c11874
> > --- /dev/null
> > +++ b/src/ipa/ipu3/algorithms/af.h
> > @@ -0,0 +1,54 @@
> > +/* 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
> > +{
> > +     /* The format of y_table. From ipu3-ipa repo */
> > +     typedef struct y_table_item {
> > +             uint16_t y1_avg;
> > +             uint16_t y2_avg;
> > +     } y_table_item_t;
> > +
> > +public:
> > +     Af();
> > +     ~Af();
> > +
> > +     void prepare(IPAContext &context, ipu3_uapi_params *params) override;
> > +     int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> > +     void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> > +
> > +private:
> > +     int vcmSet(int value);
> > +
> > +     int vcmFd_;
> > +     /* Used for focus scan. */
> > +     uint32_t focus_;
> > +     /* Recent AF statistic variance. */
> > +     double currentVariance_;
> > +     /* The frames to be ignore before starting measuring. */
> > +     uint32_t ignoreFrame_;
> > +};
> > +
> > +} /* namespace ipa::ipu3::algorithms */
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
> > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > index b5d736c1..70ae4e59 100644
> > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > @@ -138,7 +138,7 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
> >       }
> >
> >       /* Estimate the quantile mean of the top 2% of the histogram */
> > -     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> > +     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.7, 1.0);
> >  }
> >
> >  /**
> > diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> > index 3ec42f72..8574416e 100644
> > --- a/src/ipa/ipu3/algorithms/meson.build
> > +++ b/src/ipa/ipu3/algorithms/meson.build
> > @@ -1,9 +1,10 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >
> >  ipu3_ipa_algorithms = files([
> > +    'af.cpp',
> >      'agc.cpp',
> >      'algorithm.cpp',
> >      'awb.cpp',
> >      'blc.cpp',
> > -    'tone_mapping.cpp',
> > +    'tone_mapping.cpp'
> >  ])
> > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> > index 2355a9c7..1064d62d 100644
> > --- a/src/ipa/ipu3/ipa_context.cpp
> > +++ b/src/ipa/ipu3/ipa_context.cpp
> > @@ -69,6 +69,17 @@ namespace libcamera::ipa::ipu3 {
> >   * \brief Number of cells on one line including the ImgU padding
> >   */
> >
> > +/**
> > + * \var IPASessionConfiguration::af
> > + * \brief AF parameters configuration of the IPA
> > + *
> > + * \var IPASessionConfiguration::af.start_x
> > + * \brief The start X position of the AF area
> > + *
> > + * \var IPASessionConfiguration::af.start_y
> > + * \brief The start Y position of the AF area
> > + */
> > +
> >  /**
> >   * \var IPASessionConfiguration::agc
> >   * \brief AGC parameters configuration of the IPA
> > @@ -86,6 +97,21 @@ namespace libcamera::ipa::ipu3 {
> >   * \brief Maximum analogue gain supported with the configured sensor
> >   */
> >
> > +/**
> > + * \var IPAFrameContext::af
> > + * \brief Context for the Automatic Focus algorithm
> > + *
> > + * \struct  IPAFrameContext::af
> > + * \var IPAFrameContext::af.focus
> > + * \brief Current position of the lens
> > + *
> > + * \var IPAFrameContext::af.maxVariance
> > + * \brief The maximum variance of the current image.
> > + *
> > + * \var IPAFrameContext::af.stable
> > + * \brief is the image focused?
> > + */
> > +
> >  /**
> >   * \var IPAFrameContext::agc
> >   * \brief Context for the Automatic Gain Control algorithm
> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > index 1e46c61a..a7ffcbed 100644
> > --- a/src/ipa/ipu3/ipa_context.h
> > +++ b/src/ipa/ipu3/ipa_context.h
> > @@ -31,6 +31,11 @@ struct IPASessionConfiguration {
> >               double minAnalogueGain;
> >               double maxAnalogueGain;
> >       } agc;
> > +
> > +     struct {
> > +             uint32_t start_x;
> > +             uint32_t start_y;
> > +     } af;
> >  };
> >
> >  struct IPAFrameContext {
> > @@ -47,6 +52,12 @@ struct IPAFrameContext {
> >               } gains;
> >       } awb;
> >
> > +     struct {
> > +             uint32_t focus;
> > +             double maxVariance;
> > +             bool stable;
> > +     } af;
> > +
> >       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..f19d0059 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -30,6 +30,7 @@
> >
> >  #include "libcamera/internal/mapped_framebuffer.h"
> >
> > +#include "algorithms/af.h"
> >  #include "algorithms/agc.h"
> >  #include "algorithms/algorithm.h"
> >  #include "algorithms/awb.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
Kate Hsuan Nov. 26, 2021, 8:57 a.m. UTC | #7
On Thu, Nov 25, 2021 at 7:55 AM Daniel Scally <djrscally@gmail.com> wrote:
>
>
> On 24/11/2021 23:51, Daniel Scally wrote:
> > Hi Kate
> >
> > On 24/11/2021 10:34, 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
> >> state and a appropriate focus value.
> >>
> >> Changes since v2:
> >> 1. Add grid configuration interface.
> >> 2. Add AF accelerator configuration.
> >> 3. Move default focus area to the center of the image.
> >>
> >> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> >
> > Thanks a lot for this, it's very cool. I tried it out and observed the
> > following:
> >
> >
> > 1. It seems to focus too close initially, then transition to a
> > progressively further away focus (passing through the sweet spot where
> > the items on my desk behind the Go2 are in focus) and then resets and
> > goes back close focus again. It repeats this a couple times , before
> > eventually settling in to an ok image. So, seems to work but takes a
> > little time to get there.
> >
> > 2. When the "reset" happens, and when stream stops, there's an audible
> > click as the VCM changes position a large amount in one go. I think (at
> > least for stream off) that's my fault in the driver; most of them have a
> > section in .suspend() that gradually relaxes on power off, I'll update
> > the dw9719 to do that too. Not sure that that's the cause of the click
> > on the reset though
> >
> > 3. There's _a lot_ of noise written to the terminal
>
>
> Pressed send by accident so this came off a little abrupt; sorry :). I
> meant to say that I can't quite figure out where it's coming from, but
> it's a constant string of integers being written out.
>

Maybe it is my debug output. I'll check and remove them in v3.

Thank you :)
Kieran Bingham Nov. 26, 2021, 10:24 a.m. UTC | #8
Hi all,

Quoting Kate Hsuan (2021-11-26 08:55:06)
> Hi Daniel,
> 
> On Thu, Nov 25, 2021 at 7:52 AM Daniel Scally <djrscally@gmail.com> wrote:
> >
> > Hi Kate
> >
> > On 24/11/2021 10:34, 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
> > > state and a appropriate focus value.
> > >
> > > Changes since v2:
> > > 1. Add grid configuration interface.
> > > 2. Add AF accelerator configuration.
> > > 3. Move default focus area to the center of the image.
> > >
> > > Signed-off-by: Kate Hsuan <hpa@redhat.com>
> >
> >
> > Thanks a lot for this, it's very cool. I tried it out and observed the
> > following:
> >
> >
> > 1. It seems to focus too close initially, then transition to a
> > progressively further away focus (passing through the sweet spot where
> > the items on my desk behind the Go2 are in focus) and then resets and
> > goes back close focus again. It repeats this a couple times , before
> > eventually settling in to an ok image. So, seems to work but takes a
> > little time to get there.
> 
> The linear scan is always slow. :( I'll try to improve the AF speed.
> Now, I manually configure the exposure time and gain to get a stable
> AE result then start the AF scan. It is faster than using a floating
> AE result. Also, the minimal VCM step is 5 for each AF sampling, some
> of the images may require a higher resolution of the VCM step to get a
> clear image.
> 
> >
> > 2. When the "reset" happens, and when stream stops, there's an audible
> > click as the VCM changes position a large amount in one go. I think (at
> > least for stream off) that's my fault in the driver; most of them have a
> > section in .suspend() that gradually relaxes on power off, I'll update
> > the dw9719 to do that too. Not sure that that's the cause of the click
> > on the reset though
> 
> Yes. When I close() the VCM fd, I can hear the "click" from the VCM.
> 
> >
> > 3. There's _a lot_ of noise written to the terminal
> 
> Sorry about that. I would like to observe the AF statistic value of
> some high-contrast images or objects.
> I'll remove all terminal output in v3.

It's a rabbit-hole but we already support tracepoints with lttng. We can
add tracepoints to the IPA algorithms, and extract lots of data through
that.

I would envision that data capture being parsed by python scripts to
then generate graphs, potentially in realtime while the commands are
being run if we get really into it...

--
Kieran



> > > ---
> > >  src/ipa/ipu3/algorithms/af.cpp      | 284 ++++++++++++++++++++++++++++
> > >  src/ipa/ipu3/algorithms/af.h        |  54 ++++++
> > >  src/ipa/ipu3/algorithms/agc.cpp     |   2 +-
> > >  src/ipa/ipu3/algorithms/meson.build |   3 +-
> > >  src/ipa/ipu3/ipa_context.cpp        |  26 +++
> > >  src/ipa/ipu3/ipa_context.h          |  11 ++
> > >  src/ipa/ipu3/ipu3.cpp               |   2 +
> > >  7 files changed, 380 insertions(+), 2 deletions(-)
> > >  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..b0359721
> > > --- /dev/null
> > > +++ b/src/ipa/ipu3/algorithms/af.cpp
> > > @@ -0,0 +1,284 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2021, Red Hat
> > > + *
> > > + * af.cpp - IPU3 auto focus control
> > > + */
> > > +
> > > +#include "af.h"
> > > +
> > > +#include <algorithm>
> > > +#include <chrono>
> > > +#include <cmath>
> > > +#include <fcntl.h>
> > > +#include <numeric>
> > > +#include <sys/ioctl.h>
> > > +#include <sys/stat.h>
> > > +#include <sys/types.h>
> > > +#include <unistd.h>
> > > +
> > > +#include <linux/videodev2.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 {
> > > +
> > > +/**
> > > + * \class Af
> > > + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
> > > + *
> > > + * This algorithm is used to determine the position of the lens and get a
> > > + * focused image. The IPU3 AF accelerator computes the statistics, composed
> > > + * by high pass and low pass filtered value and stores in a AF buffer.
> > > + * Typically, for a focused image, it has relative high contrast than a
> > > + * blurred image, i.e. an out of focus image. Therefore, if an image with the
> > > + * highest contrast can be found from the AF scan, the lens' position is the
> > > + * best step of the focus.
> > > + *
> > > + */
> > > +
> > > +LOG_DEFINE_CATEGORY(IPU3Af)
> > > +
> > > +/**
> > > + * Maximum focus value of the VCM control
> > > + * \todo should be obtained from the VCM driver
> > > + */
> > > +static constexpr uint32_t MaxFocusSteps_ = 1023;
> > > +
> > > +/* Minimum focus step for searching appropriate focus*/
> > > +static constexpr uint32_t MinSearchStep_ = 5;
> > > +
> > > +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
> > > +static constexpr double MaxChange_ = 0.8;
> > > +
> > > +Af::Af()
> > > +     : focus_(0), currentVariance_(0.0)
> > > +{
> > > +     /**
> > > +      * For surface Go 2 back camera VCM (dw9719)
> > > +      * \todo move to control class
> > > +     */
> > > +     vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);
> > > +}
> > > +
> > > +Af::~Af()
> > > +{
> > > +     if (vcmFd_ != -1)
> > > +             close(vcmFd_);
> > > +}
> >
> >
> > FYI I'm going to work on moving these bits out so that the actual
> > setting of the v4l2 control is done via an instance of the CameraLens
> > class from Han-Lin's recent series. Ditto the calls to the vcmSet()
> > function in ::process(). This should mean it's nicely agnostic (i.e. no
> > hardcoded v4l-subdev, nor sensor/vcm pairs)
> >
> 
> Great. Nice to know this :) and I saw the patch about it. Thanks a lot!!
> 
> >
> > > +
> > > +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
> > > +{
> > > +     /* AF grid config */
> > > +     params->acc_param.af.grid_cfg.width = 16;
> > > +     params->acc_param.af.grid_cfg.height = 16;
> > > +     params->acc_param.af.grid_cfg.block_height_log2 = 3;
> > > +     params->acc_param.af.grid_cfg.block_width_log2 = 3;
> > > +     params->acc_param.af.grid_cfg.height_per_slice = 2;
> > > +     /* Start position of AF area */
> > > +     params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
> > > +     params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;
> > > +
> > > +     params->acc_param.af.filter_config.y1_sign_vec = 0;
> > > +     params->acc_param.af.filter_config.y2_sign_vec = 0;
> > > +
> > > +     /* b + gb + gr + r = 32 */
> > > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_b = 8;
> > > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gb = 8;
> > > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gr = 8;
> > > +     params->acc_param.af.filter_config.y_calc.y_gen_rate_r = 8;
> > > +
> > > +     /* 2^7 = 128,  a1 + a2 + ... + a12 = 128, log2 of sum of a1 to a12*/
> > > +     params->acc_param.af.filter_config.nf.y1_nf = 7;
> > > +     params->acc_param.af.filter_config.nf.y2_nf = 7;
> > > +
> > > +     /* Low pass filter configuration (y1_coeff_n) */
> > > +     params->acc_param.af.filter_config.y1_coeff_0.a1 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_0.a2 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_0.a3 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_0.a4 = 0;
> > > +
> > > +     params->acc_param.af.filter_config.y1_coeff_1.a5 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_1.a6 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_1.a7 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_1.a8 = 0;
> > > +
> > > +     params->acc_param.af.filter_config.y1_coeff_2.a9 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_2.a10 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_2.a11 = 0;
> > > +     params->acc_param.af.filter_config.y1_coeff_2.a12 = 128;
> > > +
> > > +     /* High pass filter configuration (y2_coeff_n) */
> > > +     params->acc_param.af.filter_config.y2_coeff_0.a1 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_0.a2 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_0.a3 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_0.a4 = 0;
> > > +
> > > +     params->acc_param.af.filter_config.y2_coeff_1.a5 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_1.a6 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_1.a7 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_1.a8 = 0;
> > > +
> > > +     params->acc_param.af.filter_config.y2_coeff_2.a9 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_2.a10 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_2.a11 = 0;
> > > +     params->acc_param.af.filter_config.y2_coeff_2.a12 = 128;
> > > +
> > > +     /* Enable AF accelerator */
> > > +     params->use.acc_af = 1;
> > > +}
> > > +
> > > +/**
> > > + * \brief Configure the Af given a configInfo
> > > + * \param[in] context The shared IPA context
> > > + * \param[in] configInfo The IPA configuration data
> > > + *
> > > + * \return 0
> > > + */
> > > +int Af::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)
> > > +{
> > > +     /* Determined focus value i.e. current focus value */
> > > +     context.frameContext.af.focus = 0;
> > > +     /* Maximum variance of the AF statistics */
> > > +     context.frameContext.af.maxVariance = 0;
> > > +     /* is focused? if it is true, the AF should be in a stable state. */
> > > +     context.frameContext.af.stable = false;
> > > +     /* Frame to be ignored before start to estimate AF variance. */
> > > +     ignoreFrame_ = 10;
> > > +
> > > +     /*
> > > +      * AF default area configuration
> > > +      * Move AF area to the center of the image.
> > > +      */
> > > +     /* AF width is 16x8 = 128 */
> > > +     context.configuration.af.start_x = (1280 / 2) - 64;
> > > +     context.configuration.af.start_y = (720 / 2) - 64;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +/**
> > > + * \brief Send focus step to the VCM.
> > > + * \param[in] value Set lens position.
> > > + * \todo It is hardcoded here for the dw9717 VCM and will be moved to the
> > > + * subdev control in the future.
> > > + */
> > > +int Af::vcmSet(int value)
> > > +{
> > > +     int ret;
> > > +     struct v4l2_control ctrl;
> > > +     if (vcmFd_ == -1)
> > > +             return -EINVAL;
> > > +     memset(&ctrl, 0, sizeof(struct v4l2_control));
> > > +     ctrl.id = V4L2_CID_FOCUS_ABSOLUTE;
> > > +     ctrl.value = value;
> > > +     ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
> > > +     return ret;
> > > +}
> > > +
> > > +/**
> > > + * \brief Determine the max contrast image and lens position. y_table is the
> > > + * statictic data from IPU3 and is composed of low pass and high pass filtered
> > > + * value. High pass filtered value also represents the sharpness of the image.
> > > + * Based on this, if the image with highest variance of the high pass filtered
> > > + * value (contrast) during the AF scan, the position of the len should be the
> > > + * best focus.
> > > + * \param[in] context The shared IPA context.
> > > + * \param[in] stats The statistic buffer of 3A from the IPU3.
> > > + */
> > > +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> > > +{
> > > +     uint32_t total = 0;
> > > +     double mean;
> > > +     uint64_t var_sum = 0;
> > > +     y_table_item_t *y_item;
> > > +     int z = 0;
> > > +
> > > +     y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
> >
> >
> > I get a warning here when compiling:
> >
> >
> > ../src/ipa/ipu3/algorithms/af.cpp: In member function ‘virtual void
> > libcamera::ipa::ipu3::algorithms::Af::process(libcamera::ipa::ipu3::IPAContext&,
> > const ipu3_uapi_stats_3a*)’:
> > ../src/ipa/ipu3/algorithms/af.cpp:209:50: error: taking address of
> > packed member of ‘ipu3_uapi_stats_3a’ may result in an unaligned pointer
> > value [-Werror=address-of-packed-member]
> >   209 |  y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
> >       |                             ~~~~~~~~~~~~~~~~~~~~~^~~~~~~
> >
> 
> I'll double-check this and fix it in my v3.
> 
> 
> 
> > > +
> > > +     /**
> > > +      * Calculate the mean of each non-zero AF statistics, since IPU3 only determine the AF value
> > > +      * for a given grid.
> > > +      */
> > > +     for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
> > > +             printf("%d, ", y_item[z].y2_avg);
> > > +             total = total + y_item[z].y2_avg;
> > > +             if (y_item[z].y2_avg == 0)
> > > +                     break;
> > > +     }
> > > +     mean = total / z;
> > > +
> > > +     /* Calculate the variance of every AF statistic value. */
> > > +     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;
> > > +     }
> > > +
> > > +     /* Determine the average variance of the frame. */
> > > +     currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
> > > +     LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
> > > +
> > > +     if (context.frameContext.af.stable == true) {
> > > +             const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
> > > +             const double var_ratio = diff_var / context.frameContext.af.maxVariance;
> > > +             LOG(IPU3Af, Debug) << "Change ratio: "
> > > +                                << var_ratio
> > > +                                << " current focus: "
> > > +                                << context.frameContext.af.focus;
> > > +             /**
> > > +              * If the change ratio of contrast is over Maxchange_ (out of focus),
> > > +              * trigger AF again.
> > > +              */
> > > +             if (var_ratio > MaxChange_) {
> > > +                     if (ignoreFrame_ == 0) {
> > > +                             context.frameContext.af.maxVariance = 0;
> > > +                             context.frameContext.af.focus = 0;
> > > +                             focus_ = 0;
> > > +                             context.frameContext.af.stable = false;
> > > +                             ignoreFrame_ = 60;
> > > +                     } else
> > > +                             ignoreFrame_--;
> > > +             } else
> > > +                     ignoreFrame_ = 10;
> > > +     } else {
> > > +             if (ignoreFrame_ != 0)
> > > +                     ignoreFrame_--;
> > > +             else {
> > > +                     /* Find the maximum variance during the AF scan using a greedy strategy */
> > > +                     if (currentVariance_ > context.frameContext.af.maxVariance) {
> > > +                             context.frameContext.af.maxVariance = currentVariance_;
> > > +                             context.frameContext.af.focus = focus_;
> > > +                     }
> > > +
> > > +                     if (focus_ > MaxFocusSteps_) {
> > > +                             /* If reach the max step, move lens to the position and set "focus stable". */
> > > +                             context.frameContext.af.stable = true;
> > > +                             vcmSet(context.frameContext.af.focus);
> > > +                     } else {
> > > +                             focus_ += MinSearchStep_;
> > > +                             vcmSet(focus_);
> > > +                     }
> > > +                     LOG(IPU3Af, Debug) << "Focus searching max variance is: "
> > > +                                        << context.frameContext.af.maxVariance
> > > +                                        << " Focus step is "
> > > +                                        << context.frameContext.af.focus;
> > > +             }
> > > +     }
> > > +}
> > > +
> > > +} /* namespace ipa::ipu3::algorithms */
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> > > new file mode 100644
> > > index 00000000..b5c11874
> > > --- /dev/null
> > > +++ b/src/ipa/ipu3/algorithms/af.h
> > > @@ -0,0 +1,54 @@
> > > +/* 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
> > > +{
> > > +     /* The format of y_table. From ipu3-ipa repo */
> > > +     typedef struct y_table_item {
> > > +             uint16_t y1_avg;
> > > +             uint16_t y2_avg;
> > > +     } y_table_item_t;
> > > +
> > > +public:
> > > +     Af();
> > > +     ~Af();
> > > +
> > > +     void prepare(IPAContext &context, ipu3_uapi_params *params) override;
> > > +     int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> > > +     void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> > > +
> > > +private:
> > > +     int vcmSet(int value);
> > > +
> > > +     int vcmFd_;
> > > +     /* Used for focus scan. */
> > > +     uint32_t focus_;
> > > +     /* Recent AF statistic variance. */
> > > +     double currentVariance_;
> > > +     /* The frames to be ignore before starting measuring. */
> > > +     uint32_t ignoreFrame_;
> > > +};
> > > +
> > > +} /* namespace ipa::ipu3::algorithms */
> > > +
> > > +} /* namespace libcamera */
> > > +
> > > +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > > index b5d736c1..70ae4e59 100644
> > > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > > @@ -138,7 +138,7 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
> > >       }
> > >
> > >       /* Estimate the quantile mean of the top 2% of the histogram */
> > > -     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> > > +     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.7, 1.0);
> > >  }
> > >
> > >  /**
> > > diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> > > index 3ec42f72..8574416e 100644
> > > --- a/src/ipa/ipu3/algorithms/meson.build
> > > +++ b/src/ipa/ipu3/algorithms/meson.build
> > > @@ -1,9 +1,10 @@
> > >  # SPDX-License-Identifier: CC0-1.0
> > >
> > >  ipu3_ipa_algorithms = files([
> > > +    'af.cpp',
> > >      'agc.cpp',
> > >      'algorithm.cpp',
> > >      'awb.cpp',
> > >      'blc.cpp',
> > > -    'tone_mapping.cpp',
> > > +    'tone_mapping.cpp'
> > >  ])
> > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> > > index 2355a9c7..1064d62d 100644
> > > --- a/src/ipa/ipu3/ipa_context.cpp
> > > +++ b/src/ipa/ipu3/ipa_context.cpp
> > > @@ -69,6 +69,17 @@ namespace libcamera::ipa::ipu3 {
> > >   * \brief Number of cells on one line including the ImgU padding
> > >   */
> > >
> > > +/**
> > > + * \var IPASessionConfiguration::af
> > > + * \brief AF parameters configuration of the IPA
> > > + *
> > > + * \var IPASessionConfiguration::af.start_x
> > > + * \brief The start X position of the AF area
> > > + *
> > > + * \var IPASessionConfiguration::af.start_y
> > > + * \brief The start Y position of the AF area
> > > + */
> > > +
> > >  /**
> > >   * \var IPASessionConfiguration::agc
> > >   * \brief AGC parameters configuration of the IPA
> > > @@ -86,6 +97,21 @@ namespace libcamera::ipa::ipu3 {
> > >   * \brief Maximum analogue gain supported with the configured sensor
> > >   */
> > >
> > > +/**
> > > + * \var IPAFrameContext::af
> > > + * \brief Context for the Automatic Focus algorithm
> > > + *
> > > + * \struct  IPAFrameContext::af
> > > + * \var IPAFrameContext::af.focus
> > > + * \brief Current position of the lens
> > > + *
> > > + * \var IPAFrameContext::af.maxVariance
> > > + * \brief The maximum variance of the current image.
> > > + *
> > > + * \var IPAFrameContext::af.stable
> > > + * \brief is the image focused?
> > > + */
> > > +
> > >  /**
> > >   * \var IPAFrameContext::agc
> > >   * \brief Context for the Automatic Gain Control algorithm
> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > > index 1e46c61a..a7ffcbed 100644
> > > --- a/src/ipa/ipu3/ipa_context.h
> > > +++ b/src/ipa/ipu3/ipa_context.h
> > > @@ -31,6 +31,11 @@ struct IPASessionConfiguration {
> > >               double minAnalogueGain;
> > >               double maxAnalogueGain;
> > >       } agc;
> > > +
> > > +     struct {
> > > +             uint32_t start_x;
> > > +             uint32_t start_y;
> > > +     } af;
> > >  };
> > >
> > >  struct IPAFrameContext {
> > > @@ -47,6 +52,12 @@ struct IPAFrameContext {
> > >               } gains;
> > >       } awb;
> > >
> > > +     struct {
> > > +             uint32_t focus;
> > > +             double maxVariance;
> > > +             bool stable;
> > > +     } af;
> > > +
> > >       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..f19d0059 100644
> > > --- a/src/ipa/ipu3/ipu3.cpp
> > > +++ b/src/ipa/ipu3/ipu3.cpp
> > > @@ -30,6 +30,7 @@
> > >
> > >  #include "libcamera/internal/mapped_framebuffer.h"
> > >
> > > +#include "algorithms/af.h"
> > >  #include "algorithms/agc.h"
> > >  #include "algorithms/algorithm.h"
> > >  #include "algorithms/awb.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
>
Kate Hsuan Dec. 1, 2021, 6:11 a.m. UTC | #9
Hi Jean-Michel,


On Fri, Nov 26, 2021 at 3:58 PM Jean-Michel Hautbois
<jeanmichel.hautbois@ideasonboard.com> wrote:
>
> Hi Kate,
>
> On 26/11/2021 08:23, Kate Hsuan wrote:
> > Hi Jean-Michel,
> >
> > On Wed, Nov 24, 2021 at 9:40 PM Jean-Michel Hautbois
> > <jeanmichel.hautbois@ideasonboard.com> wrote:
> >>
> >> Hi Kate,
> >>
> >> Thanks for the patch.
> >>
> >> On 24/11/2021 11:34, 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
> >>> state and a appropriate focus value.
> >>>
> >>> Changes since v2:
> >>> 1. Add grid configuration interface.
> >>> 2. Add AF accelerator configuration.
> >>> 3. Move default focus area to the center of the image.
> >>>
> >>> Signed-off-by: Kate Hsuan <hpa@redhat.com>
> >>> ---
> >>>    src/ipa/ipu3/algorithms/af.cpp      | 284 ++++++++++++++++++++++++++++
> >>>    src/ipa/ipu3/algorithms/af.h        |  54 ++++++
> >>>    src/ipa/ipu3/algorithms/agc.cpp     |   2 +-
> >>>    src/ipa/ipu3/algorithms/meson.build |   3 +-
> >>>    src/ipa/ipu3/ipa_context.cpp        |  26 +++
> >>>    src/ipa/ipu3/ipa_context.h          |  11 ++
> >>>    src/ipa/ipu3/ipu3.cpp               |   2 +
> >>>    7 files changed, 380 insertions(+), 2 deletions(-)
> >>>    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..b0359721
> >>> --- /dev/null
> >>> +++ b/src/ipa/ipu3/algorithms/af.cpp
> >>> @@ -0,0 +1,284 @@
> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >>> +/*
> >>> + * Copyright (C) 2021, Red Hat
> >>> + *
> >>> + * af.cpp - IPU3 auto focus control
> >>> + */
> >>> +
> >>> +#include "af.h"
> >>> +
> >>> +#include <algorithm>
> >>> +#include <chrono>
> >>> +#include <cmath>
> >>> +#include <fcntl.h>
> >>> +#include <numeric>
> >>> +#include <sys/ioctl.h>
> >>> +#include <sys/stat.h>
> >>> +#include <sys/types.h>
> >>> +#include <unistd.h>
> >>> +
> >>> +#include <linux/videodev2.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 {
> >>> +
> >>> +/**
> >>> + * \class Af
> >>> + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
> >>> + *
> >>> + * This algorithm is used to determine the position of the lens and get a
> >>> + * focused image. The IPU3 AF accelerator computes the statistics, composed
> >>> + * by high pass and low pass filtered value and stores in a AF buffer.
> >>> + * Typically, for a focused image, it has relative high contrast than a
> >>> + * blurred image, i.e. an out of focus image. Therefore, if an image with the
> >>> + * highest contrast can be found from the AF scan, the lens' position is the
> >>> + * best step of the focus.
> >>> + *
> >>> + */
> >>> +
> >>> +LOG_DEFINE_CATEGORY(IPU3Af)
> >>> +
> >>> +/**
> >>> + * Maximum focus value of the VCM control
> >>> + * \todo should be obtained from the VCM driver
> >>> + */
> >>> +static constexpr uint32_t MaxFocusSteps_ = 1023;
> >>> +
> >>> +/* Minimum focus step for searching appropriate focus*/
> >>> +static constexpr uint32_t MinSearchStep_ = 5;
> >>> +
> >>> +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
> >>> +static constexpr double MaxChange_ = 0.8;
> >>> +
> >>> +Af::Af()
> >>> +     : focus_(0), currentVariance_(0.0)
> >>> +{
> >>> +     /**
> >>> +      * For surface Go 2 back camera VCM (dw9719)
> >>> +      * \todo move to control class
> >>> +     */
> >>> +     vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);
> >>
> >> This shouldn't be here :-).
> >>
> >>> +}
> >>> +
> >>> +Af::~Af()
> >>> +{
> >>> +     if (vcmFd_ != -1)
> >>> +             close(vcmFd_);
> >>> +}
> >>> +
> >>> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
> >>> +{
> >>> +     /* AF grid config */
> >>> +     params->acc_param.af.grid_cfg.width = 16;
> >>> +     params->acc_param.af.grid_cfg.height = 16;
> >>> +     params->acc_param.af.grid_cfg.block_height_log2 = 3;
> >>> +     params->acc_param.af.grid_cfg.block_width_log2 = 3;
> >>> +     params->acc_param.af.grid_cfg.height_per_slice = 2;
> >>> +     /* Start position of AF area */
> >>> +     params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
> >>> +     params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;
> >>
> >> That is interesting... and probably not right :-).
> >> You are setting the ImgU to use the same default grid it is using if you
> >> are not setting params->use.acc_af.
> >>
> >> Meaning, you could simplify this by using the default table here:
> >> https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9579
> >>
> >> You can use what is done in IPU3::Awb for instance, we are using the
> >> default BNR table from
> >> https://elixir.bootlin.com/linux/latest/source/drivers/staging/media/ipu3/ipu3-tables.c#L9313
> >>
> >> And then you can only change the desired start_x and start_y (I have
> >> comments on those later).
> >
> > OK, Thanks for the information, I'll try this BNR table.
> >
> >>
> >>> +
> >>> +     params->acc_param.af.filter_config.y1_sign_vec = 0;
> >>> +     params->acc_param.af.filter_config.y2_sign_vec = 0;
> >>> +
> >>> +     /* b + gb + gr + r = 32 */
> >>> +     params->acc_param.af.filter_config.y_calc.y_gen_rate_b = 8;
> >>> +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gb = 8;
> >>> +     params->acc_param.af.filter_config.y_calc.y_gen_rate_gr = 8;
> >>> +     params->acc_param.af.filter_config.y_calc.y_gen_rate_r = 8;
> >>> +
> >>> +     /* 2^7 = 128,  a1 + a2 + ... + a12 = 128, log2 of sum of a1 to a12*/
> >>> +     params->acc_param.af.filter_config.nf.y1_nf = 7;
> >>> +     params->acc_param.af.filter_config.nf.y2_nf = 7;
> >>> +
> >>> +     /* Low pass filter configuration (y1_coeff_n) */
> >>> +     params->acc_param.af.filter_config.y1_coeff_0.a1 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_0.a2 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_0.a3 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_0.a4 = 0;
> >>> +
> >>> +     params->acc_param.af.filter_config.y1_coeff_1.a5 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_1.a6 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_1.a7 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_1.a8 = 0;
> >>> +
> >>> +     params->acc_param.af.filter_config.y1_coeff_2.a9 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_2.a10 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_2.a11 = 0;
> >>> +     params->acc_param.af.filter_config.y1_coeff_2.a12 = 128;
> >>> +
> >>> +     /* High pass filter configuration (y2_coeff_n) */
> >>> +     params->acc_param.af.filter_config.y2_coeff_0.a1 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_0.a2 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_0.a3 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_0.a4 = 0;
> >>> +
> >>> +     params->acc_param.af.filter_config.y2_coeff_1.a5 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_1.a6 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_1.a7 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_1.a8 = 0;
> >>> +
> >>> +     params->acc_param.af.filter_config.y2_coeff_2.a9 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_2.a10 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_2.a11 = 0;
> >>> +     params->acc_param.af.filter_config.y2_coeff_2.a12 = 128;
> >>> +
> >>> +     /* Enable AF accelerator */
> >>> +     params->use.acc_af = 1;
> >>> +}
> >>> +
> >>> +/**
> >>> + * \brief Configure the Af given a configInfo
> >>> + * \param[in] context The shared IPA context
> >>> + * \param[in] configInfo The IPA configuration data
> >>> + *
> >>> + * \return 0
> >>> + */
> >>> +int Af::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)
> >>> +{
> >>> +     /* Determined focus value i.e. current focus value */
> >>> +     context.frameContext.af.focus = 0;
> >>> +     /* Maximum variance of the AF statistics */
> >>> +     context.frameContext.af.maxVariance = 0;
> >>> +     /* is focused? if it is true, the AF should be in a stable state. */
> >>> +     context.frameContext.af.stable = false;
> >>> +     /* Frame to be ignored before start to estimate AF variance. */
> >>> +     ignoreFrame_ = 10;
> >>> +
> >>> +     /*
> >>> +      * AF default area configuration
> >>> +      * Move AF area to the center of the image.
> >>> +      */
> >>> +     /* AF width is 16x8 = 128 */
> >>> +     context.configuration.af.start_x = (1280 / 2) - 64;
> >>> +     context.configuration.af.start_y = (720 / 2) - 64;
> >>
> >> This is good, only if the frame output is set to 1280x720.
> >> If I call:
> >> 'cam -c2 -C -s width=1920,height=1280'
> >>
> >> Then you won't be centered.
> >> You can get the configuration directly from configInfo.bdsOutputSize (I
> >> think af is using the BDS output) and then remove the [[maybe_unused]].
> >
> > Thank you for this, I'll use the bdsOutputSize to determine the centre
> > of the image.
> >
> >>
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +/**
> >>> + * \brief Send focus step to the VCM.
> >>> + * \param[in] value Set lens position.
> >>> + * \todo It is hardcoded here for the dw9717 VCM and will be moved to the
> >>> + * subdev control in the future.
> >>> + */
> >>> +int Af::vcmSet(int value)
> >>> +{
> >>> +     int ret;
> >>> +     struct v4l2_control ctrl;
> >>> +     if (vcmFd_ == -1)
> >>> +             return -EINVAL;
> >>> +     memset(&ctrl, 0, sizeof(struct v4l2_control));
> >>> +     ctrl.id = V4L2_CID_FOCUS_ABSOLUTE;
> >>> +     ctrl.value = value;
> >>> +     ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
> >>> +     return ret;
> >>> +}
> >>
> >> This does not belong to the algorithm.
> >
> > Sure :)
> >
> >>
> >>> +
> >>> +/**
> >>> + * \brief Determine the max contrast image and lens position. y_table is the
> >>> + * statictic data from IPU3 and is composed of low pass and high pass filtered
> >>> + * value. High pass filtered value also represents the sharpness of the image.
> >>> + * Based on this, if the image with highest variance of the high pass filtered
> >>> + * value (contrast) during the AF scan, the position of the len should be the
> >>> + * best focus.
> >>> + * \param[in] context The shared IPA context.
> >>> + * \param[in] stats The statistic buffer of 3A from the IPU3.
> >>> + */
> >>> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
> >>> +{
> >>> +     uint32_t total = 0;
> >>> +     double mean;
> >>> +     uint64_t var_sum = 0;
> >>> +     y_table_item_t *y_item;
> >>> +     int z = 0;
> >>> +
> >>> +     y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
> >>> +
> >>> +     /**
> >>> +      * Calculate the mean of each non-zero AF statistics, since IPU3 only determine the AF value
> >>> +      * for a given grid.
> >>> +      */
> >>> +     for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
> >>> +             printf("%d, ", y_item[z].y2_avg);
> >>> +             total = total + y_item[z].y2_avg;
> >>> +             if (y_item[z].y2_avg == 0)
> >>> +                     break;
> >>> +     }
> >>> +     mean = total / z;
> >>> +
> >>> +     /* Calculate the variance of every AF statistic value. */
> >>> +     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;
> >>> +     }
> >>> +
> >>> +     /* Determine the average variance of the frame. */
> >>> +     currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
> >>> +     LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
> >>> +
> >>> +     if (context.frameContext.af.stable == true) {
> >>> +             const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
> >>> +             const double var_ratio = diff_var / context.frameContext.af.maxVariance;
> >>> +             LOG(IPU3Af, Debug) << "Change ratio: "
> >>> +                                << var_ratio
> >>> +                                << " current focus: "
> >>> +                                << context.frameContext.af.focus;
> >>> +             /**
> >>> +              * If the change ratio of contrast is over Maxchange_ (out of focus),
> >>> +              * trigger AF again.
> >>> +              */
> >>> +             if (var_ratio > MaxChange_) {
> >>> +                     if (ignoreFrame_ == 0) {
> >>> +                             context.frameContext.af.maxVariance = 0;
> >>> +                             context.frameContext.af.focus = 0;
> >>> +                             focus_ = 0;
> >>> +                             context.frameContext.af.stable = false;
> >>> +                             ignoreFrame_ = 60;
> >>> +                     } else
> >>> +                             ignoreFrame_--;
> >>> +             } else
> >>> +                     ignoreFrame_ = 10;
> >>> +     } else {
> >>> +             if (ignoreFrame_ != 0)
> >>> +                     ignoreFrame_--;
> >>> +             else {
> >>> +                     /* Find the maximum variance during the AF scan using a greedy strategy */
> >>> +                     if (currentVariance_ > context.frameContext.af.maxVariance) {
> >>> +                             context.frameContext.af.maxVariance = currentVariance_;
> >>> +                             context.frameContext.af.focus = focus_;
> >>> +                     }
> >>> +
> >>> +                     if (focus_ > MaxFocusSteps_) {
> >>> +                             /* If reach the max step, move lens to the position and set "focus stable". */
> >>> +                             context.frameContext.af.stable = true;
> >>> +                             vcmSet(context.frameContext.af.focus);
> >>> +                     } else {
> >>> +                             focus_ += MinSearchStep_;
> >>> +                             vcmSet(focus_);
> >>> +                     }
> >>> +                     LOG(IPU3Af, Debug) << "Focus searching max variance is: "
> >>> +                                        << context.frameContext.af.maxVariance
> >>> +                                        << " Focus step is "
> >>> +                                        << context.frameContext.af.focus;
> >>> +             }
> >>> +     }
> >>> +}
> >>
> >> I won't comment the algorithm yet, as you already have some changes to
> >> make according to Kieran's comments ;-).
> >>
> >>> +
> >>> +} /* namespace ipa::ipu3::algorithms */
> >>> +
> >>> +} /* namespace libcamera */
> >>> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> >>> new file mode 100644
> >>> index 00000000..b5c11874
> >>> --- /dev/null
> >>> +++ b/src/ipa/ipu3/algorithms/af.h
> >>> @@ -0,0 +1,54 @@
> >>> +/* 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
> >>> +{
> >>> +     /* The format of y_table. From ipu3-ipa repo */
> >>> +     typedef struct y_table_item {
> >>> +             uint16_t y1_avg;
> >>> +             uint16_t y2_avg;
> >>> +     } y_table_item_t;
> >>> +
> >>> +public:
> >>> +     Af();
> >>> +     ~Af();
> >>> +
> >>> +     void prepare(IPAContext &context, ipu3_uapi_params *params) override;
> >>> +     int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> >>> +     void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
> >>> +
> >>> +private:
> >>> +     int vcmSet(int value);
> >>> +
> >>> +     int vcmFd_;
> >>> +     /* Used for focus scan. */
> >>> +     uint32_t focus_;
> >>> +     /* Recent AF statistic variance. */
> >>> +     double currentVariance_;
> >>> +     /* The frames to be ignore before starting measuring. */
> >>> +     uint32_t ignoreFrame_;
> >>> +};
> >>> +
> >>> +} /* namespace ipa::ipu3::algorithms */
> >>> +
> >>> +} /* namespace libcamera */
> >>> +
> >>> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
> >>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> >>> index b5d736c1..70ae4e59 100644
> >>> --- a/src/ipa/ipu3/algorithms/agc.cpp
> >>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> >>> @@ -138,7 +138,7 @@ void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
> >>>        }
> >>>
> >>>        /* Estimate the quantile mean of the top 2% of the histogram */
> >>> -     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> >>> +     iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.7, 1.0);
> >>
> >> Why are you changing this ? It is the top 30% of the cumulative histogram ?
> >>
> >
> > Sorry about this. It's my fault. AE connot work properly on my surface
> > Go 2 back camera. I try to make it more stable for the exposure. So I
> > expend the mean from the top 2% to 30%. It works for the front camera
> > before but doesn't work for the back camera.
> > I'll remove this experimental code in the v3.
>
> Have you this patch ?
> https://github.com/linux-surface/kernel/pull/113
>
> I don't know which kernel you are using, but back camera has an issue
> when approaching the max exposure, maybe is it your issue ?
>

maybe yes. The image will be all dark when the sensor is in some low
light environment or black background.
I have used this patch. The exposure is better than before.

Thank you :)

> >
> >>>    }
> >>>
> >>>    /**
> >>> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> >>> index 3ec42f72..8574416e 100644
> >>> --- a/src/ipa/ipu3/algorithms/meson.build
> >>> +++ b/src/ipa/ipu3/algorithms/meson.build
> >>> @@ -1,9 +1,10 @@
> >>>    # SPDX-License-Identifier: CC0-1.0
> >>>
> >>>    ipu3_ipa_algorithms = files([
> >>> +    'af.cpp',
> >>>        'agc.cpp',
> >>>        'algorithm.cpp',
> >>>        'awb.cpp',
> >>>        'blc.cpp',
> >>> -    'tone_mapping.cpp',
> >>> +    'tone_mapping.cpp'
> >>>    ])
> >>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> >>> index 2355a9c7..1064d62d 100644
> >>> --- a/src/ipa/ipu3/ipa_context.cpp
> >>> +++ b/src/ipa/ipu3/ipa_context.cpp
> >>> @@ -69,6 +69,17 @@ namespace libcamera::ipa::ipu3 {
> >>>     * \brief Number of cells on one line including the ImgU padding
> >>>     */
> >>>
> >>> +/**
> >>> + * \var IPASessionConfiguration::af
> >>> + * \brief AF parameters configuration of the IPA
> >>> + *
> >>> + * \var IPASessionConfiguration::af.start_x
> >>> + * \brief The start X position of the AF area
> >>> + *
> >>> + * \var IPASessionConfiguration::af.start_y
> >>> + * \brief The start Y position of the AF area
> >>> + */
> >>> +
> >>>    /**
> >>>     * \var IPASessionConfiguration::agc
> >>>     * \brief AGC parameters configuration of the IPA
> >>> @@ -86,6 +97,21 @@ namespace libcamera::ipa::ipu3 {
> >>>     * \brief Maximum analogue gain supported with the configured sensor
> >>>     */
> >>>
> >>> +/**
> >>> + * \var IPAFrameContext::af
> >>> + * \brief Context for the Automatic Focus algorithm
> >>> + *
> >>> + * \struct  IPAFrameContext::af
> >>> + * \var IPAFrameContext::af.focus
> >>> + * \brief Current position of the lens
> >>> + *
> >>> + * \var IPAFrameContext::af.maxVariance
> >>> + * \brief The maximum variance of the current image.
> >>> + *
> >>> + * \var IPAFrameContext::af.stable
> >>> + * \brief is the image focused?
> >>> + */
> >>> +
> >>>    /**
> >>>     * \var IPAFrameContext::agc
> >>>     * \brief Context for the Automatic Gain Control algorithm
> >>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> >>> index 1e46c61a..a7ffcbed 100644
> >>> --- a/src/ipa/ipu3/ipa_context.h
> >>> +++ b/src/ipa/ipu3/ipa_context.h
> >>> @@ -31,6 +31,11 @@ struct IPASessionConfiguration {
> >>>                double minAnalogueGain;
> >>>                double maxAnalogueGain;
> >>>        } agc;
> >>> +
> >>> +     struct {
> >>> +             uint32_t start_x;
> >>> +             uint32_t start_y;
> >>> +     } af;
> >>
> >> Those are __u16 in the kernel header file, maybe can we keep the same type ?
> >
> > Sure, I'll modify it in my v3.
> >
> >>
> >>>    };
> >>>
> >>>    struct IPAFrameContext {
> >>> @@ -47,6 +52,12 @@ struct IPAFrameContext {
> >>>                } gains;
> >>>        } awb;
> >>>
> >>> +     struct {
> >>> +             uint32_t focus;
> >>> +             double maxVariance;
> >>> +             bool stable;
> >>> +     } af;
> >>> +
> >>>        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..f19d0059 100644
> >>> --- a/src/ipa/ipu3/ipu3.cpp
> >>> +++ b/src/ipa/ipu3/ipu3.cpp
> >>> @@ -30,6 +30,7 @@
> >>>
> >>>    #include "libcamera/internal/mapped_framebuffer.h"
> >>>
> >>> +#include "algorithms/af.h"
> >>>    #include "algorithms/agc.h"
> >>>    #include "algorithms/algorithm.h"
> >>>    #include "algorithms/awb.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>());
> >>>
> >>
> >
> >
> > Thank you :)
> >
>

Patch
diff mbox series

diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
new file mode 100644
index 00000000..b0359721
--- /dev/null
+++ b/src/ipa/ipu3/algorithms/af.cpp
@@ -0,0 +1,284 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2021, Red Hat
+ *
+ * af.cpp - IPU3 auto focus control
+ */
+
+#include "af.h"
+
+#include <algorithm>
+#include <chrono>
+#include <cmath>
+#include <fcntl.h>
+#include <numeric>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <linux/videodev2.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 {
+
+/**
+ * \class Af
+ * \brief A IPU3 auto-focus accelerator based auto focus algorthim
+ *
+ * This algorithm is used to determine the position of the lens and get a
+ * focused image. The IPU3 AF accelerator computes the statistics, composed
+ * by high pass and low pass filtered value and stores in a AF buffer.
+ * Typically, for a focused image, it has relative high contrast than a
+ * blurred image, i.e. an out of focus image. Therefore, if an image with the
+ * highest contrast can be found from the AF scan, the lens' position is the
+ * best step of the focus.
+ *
+ */
+
+LOG_DEFINE_CATEGORY(IPU3Af)
+
+/**
+ * Maximum focus value of the VCM control
+ * \todo should be obtained from the VCM driver
+ */
+static constexpr uint32_t MaxFocusSteps_ = 1023;
+
+/* Minimum focus step for searching appropriate focus*/
+static constexpr uint32_t MinSearchStep_ = 5;
+
+/* max ratio of variance change, 0.0 < MaxChange_ < 1.0*/
+static constexpr double MaxChange_ = 0.8;
+
+Af::Af()
+	: focus_(0), currentVariance_(0.0)
+{
+	/**
+	 * For surface Go 2 back camera VCM (dw9719)
+	 * \todo move to control class
+	*/
+	vcmFd_ = open("/dev/v4l-subdev8", O_RDWR);
+}
+
+Af::~Af()
+{
+	if (vcmFd_ != -1)
+		close(vcmFd_);
+}
+
+void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
+{
+	/* AF grid config */
+	params->acc_param.af.grid_cfg.width = 16;
+	params->acc_param.af.grid_cfg.height = 16;
+	params->acc_param.af.grid_cfg.block_height_log2 = 3;
+	params->acc_param.af.grid_cfg.block_width_log2 = 3;
+	params->acc_param.af.grid_cfg.height_per_slice = 2;
+	/* Start position of AF area */
+	params->acc_param.af.grid_cfg.x_start = context.configuration.af.start_x;
+	params->acc_param.af.grid_cfg.y_start = context.configuration.af.start_y | IPU3_UAPI_GRID_Y_START_EN;
+
+	params->acc_param.af.filter_config.y1_sign_vec = 0;
+	params->acc_param.af.filter_config.y2_sign_vec = 0;
+
+	/* b + gb + gr + r = 32 */
+	params->acc_param.af.filter_config.y_calc.y_gen_rate_b = 8;
+	params->acc_param.af.filter_config.y_calc.y_gen_rate_gb = 8;
+	params->acc_param.af.filter_config.y_calc.y_gen_rate_gr = 8;
+	params->acc_param.af.filter_config.y_calc.y_gen_rate_r = 8;
+
+	/* 2^7 = 128,  a1 + a2 + ... + a12 = 128, log2 of sum of a1 to a12*/
+	params->acc_param.af.filter_config.nf.y1_nf = 7;
+	params->acc_param.af.filter_config.nf.y2_nf = 7;
+
+	/* Low pass filter configuration (y1_coeff_n) */
+	params->acc_param.af.filter_config.y1_coeff_0.a1 = 0;
+	params->acc_param.af.filter_config.y1_coeff_0.a2 = 0;
+	params->acc_param.af.filter_config.y1_coeff_0.a3 = 0;
+	params->acc_param.af.filter_config.y1_coeff_0.a4 = 0;
+
+	params->acc_param.af.filter_config.y1_coeff_1.a5 = 0;
+	params->acc_param.af.filter_config.y1_coeff_1.a6 = 0;
+	params->acc_param.af.filter_config.y1_coeff_1.a7 = 0;
+	params->acc_param.af.filter_config.y1_coeff_1.a8 = 0;
+
+	params->acc_param.af.filter_config.y1_coeff_2.a9 = 0;
+	params->acc_param.af.filter_config.y1_coeff_2.a10 = 0;
+	params->acc_param.af.filter_config.y1_coeff_2.a11 = 0;
+	params->acc_param.af.filter_config.y1_coeff_2.a12 = 128;
+
+	/* High pass filter configuration (y2_coeff_n) */
+	params->acc_param.af.filter_config.y2_coeff_0.a1 = 0;
+	params->acc_param.af.filter_config.y2_coeff_0.a2 = 0;
+	params->acc_param.af.filter_config.y2_coeff_0.a3 = 0;
+	params->acc_param.af.filter_config.y2_coeff_0.a4 = 0;
+
+	params->acc_param.af.filter_config.y2_coeff_1.a5 = 0;
+	params->acc_param.af.filter_config.y2_coeff_1.a6 = 0;
+	params->acc_param.af.filter_config.y2_coeff_1.a7 = 0;
+	params->acc_param.af.filter_config.y2_coeff_1.a8 = 0;
+
+	params->acc_param.af.filter_config.y2_coeff_2.a9 = 0;
+	params->acc_param.af.filter_config.y2_coeff_2.a10 = 0;
+	params->acc_param.af.filter_config.y2_coeff_2.a11 = 0;
+	params->acc_param.af.filter_config.y2_coeff_2.a12 = 128;
+
+	/* Enable AF accelerator */
+	params->use.acc_af = 1;
+}
+
+/**
+ * \brief Configure the Af given a configInfo
+ * \param[in] context The shared IPA context
+ * \param[in] configInfo The IPA configuration data
+ *
+ * \return 0
+ */
+int Af::configure(IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)
+{
+	/* Determined focus value i.e. current focus value */
+	context.frameContext.af.focus = 0;
+	/* Maximum variance of the AF statistics */
+	context.frameContext.af.maxVariance = 0;
+	/* is focused? if it is true, the AF should be in a stable state. */
+	context.frameContext.af.stable = false;
+	/* Frame to be ignored before start to estimate AF variance. */
+	ignoreFrame_ = 10;
+
+	/*
+	 * AF default area configuration
+	 * Move AF area to the center of the image.
+	 */
+	/* AF width is 16x8 = 128 */
+	context.configuration.af.start_x = (1280 / 2) - 64;
+	context.configuration.af.start_y = (720 / 2) - 64;
+
+	return 0;
+}
+
+/**
+ * \brief Send focus step to the VCM.
+ * \param[in] value Set lens position.
+ * \todo It is hardcoded here for the dw9717 VCM and will be moved to the
+ * subdev control in the future.
+ */
+int Af::vcmSet(int value)
+{
+	int ret;
+	struct v4l2_control ctrl;
+	if (vcmFd_ == -1)
+		return -EINVAL;
+	memset(&ctrl, 0, sizeof(struct v4l2_control));
+	ctrl.id = V4L2_CID_FOCUS_ABSOLUTE;
+	ctrl.value = value;
+	ret = ioctl(vcmFd_, VIDIOC_S_CTRL, &ctrl);
+	return ret;
+}
+
+/**
+ * \brief Determine the max contrast image and lens position. y_table is the
+ * statictic data from IPU3 and is composed of low pass and high pass filtered
+ * value. High pass filtered value also represents the sharpness of the image.
+ * Based on this, if the image with highest variance of the high pass filtered
+ * value (contrast) during the AF scan, the position of the len should be the
+ * best focus.
+ * \param[in] context The shared IPA context.
+ * \param[in] stats The statistic buffer of 3A from the IPU3.
+ */
+void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
+{
+	uint32_t total = 0;
+	double mean;
+	uint64_t var_sum = 0;
+	y_table_item_t *y_item;
+	int z = 0;
+
+	y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;
+
+	/**
+	 * Calculate the mean of each non-zero AF statistics, since IPU3 only determine the AF value
+	 * for a given grid.
+	 */
+	for (z = 0; z < (IPU3_UAPI_AF_Y_TABLE_MAX_SIZE) / 4; z++) {
+		printf("%d, ", y_item[z].y2_avg);
+		total = total + y_item[z].y2_avg;
+		if (y_item[z].y2_avg == 0)
+			break;
+	}
+	mean = total / z;
+
+	/* Calculate the variance of every AF statistic value. */
+	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;
+	}
+
+	/* Determine the average variance of the frame. */
+	currentVariance_ = static_cast<double>(var_sum) / static_cast<double>(z);
+	LOG(IPU3Af, Debug) << "variance: " << currentVariance_;
+
+	if (context.frameContext.af.stable == true) {
+		const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance);
+		const double var_ratio = diff_var / context.frameContext.af.maxVariance;
+		LOG(IPU3Af, Debug) << "Change ratio: "
+				   << var_ratio
+				   << " current focus: "
+				   << context.frameContext.af.focus;
+		/**
+		 * If the change ratio of contrast is over Maxchange_ (out of focus),
+		 * trigger AF again.
+		 */
+		if (var_ratio > MaxChange_) {
+			if (ignoreFrame_ == 0) {
+				context.frameContext.af.maxVariance = 0;
+				context.frameContext.af.focus = 0;
+				focus_ = 0;
+				context.frameContext.af.stable = false;
+				ignoreFrame_ = 60;
+			} else
+				ignoreFrame_--;
+		} else
+			ignoreFrame_ = 10;
+	} else {
+		if (ignoreFrame_ != 0)
+			ignoreFrame_--;
+		else {
+			/* Find the maximum variance during the AF scan using a greedy strategy */
+			if (currentVariance_ > context.frameContext.af.maxVariance) {
+				context.frameContext.af.maxVariance = currentVariance_;
+				context.frameContext.af.focus = focus_;
+			}
+
+			if (focus_ > MaxFocusSteps_) {
+				/* If reach the max step, move lens to the position and set "focus stable". */
+				context.frameContext.af.stable = true;
+				vcmSet(context.frameContext.af.focus);
+			} else {
+				focus_ += MinSearchStep_;
+				vcmSet(focus_);
+			}
+			LOG(IPU3Af, Debug) << "Focus searching max variance is: "
+					   << context.frameContext.af.maxVariance
+					   << " Focus step is "
+					   << context.frameContext.af.focus;
+		}
+	}
+}
+
+} /* namespace ipa::ipu3::algorithms */
+
+} /* namespace libcamera */
diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
new file mode 100644
index 00000000..b5c11874
--- /dev/null
+++ b/src/ipa/ipu3/algorithms/af.h
@@ -0,0 +1,54 @@ 
+/* 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
+{
+	/* The format of y_table. From ipu3-ipa repo */
+	typedef struct y_table_item {
+		uint16_t y1_avg;
+		uint16_t y2_avg;
+	} y_table_item_t;
+
+public:
+	Af();
+	~Af();
+
+	void prepare(IPAContext &context, ipu3_uapi_params *params) override;
+	int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
+	void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override;
+
+private:
+	int vcmSet(int value);
+
+	int vcmFd_;
+	/* Used for focus scan. */
+	uint32_t focus_;
+	/* Recent AF statistic variance. */
+	double currentVariance_;
+	/* The frames to be ignore before starting measuring. */
+	uint32_t ignoreFrame_;
+};
+
+} /* namespace ipa::ipu3::algorithms */
+
+} /* namespace libcamera */
+
+#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
index b5d736c1..70ae4e59 100644
--- a/src/ipa/ipu3/algorithms/agc.cpp
+++ b/src/ipa/ipu3/algorithms/agc.cpp
@@ -138,7 +138,7 @@  void Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
 	}
 
 	/* Estimate the quantile mean of the top 2% of the histogram */
-	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
+	iqMean_ = Histogram(Span<uint32_t>(hist)).interQuantileMean(0.7, 1.0);
 }
 
 /**
diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
index 3ec42f72..8574416e 100644
--- a/src/ipa/ipu3/algorithms/meson.build
+++ b/src/ipa/ipu3/algorithms/meson.build
@@ -1,9 +1,10 @@ 
 # SPDX-License-Identifier: CC0-1.0
 
 ipu3_ipa_algorithms = files([
+    'af.cpp',
     'agc.cpp',
     'algorithm.cpp',
     'awb.cpp',
     'blc.cpp',
-    'tone_mapping.cpp',
+    'tone_mapping.cpp'
 ])
diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
index 2355a9c7..1064d62d 100644
--- a/src/ipa/ipu3/ipa_context.cpp
+++ b/src/ipa/ipu3/ipa_context.cpp
@@ -69,6 +69,17 @@  namespace libcamera::ipa::ipu3 {
  * \brief Number of cells on one line including the ImgU padding
  */
 
+/**
+ * \var IPASessionConfiguration::af
+ * \brief AF parameters configuration of the IPA
+ *
+ * \var IPASessionConfiguration::af.start_x
+ * \brief The start X position of the AF area
+ *
+ * \var IPASessionConfiguration::af.start_y
+ * \brief The start Y position of the AF area
+ */
+
 /**
  * \var IPASessionConfiguration::agc
  * \brief AGC parameters configuration of the IPA
@@ -86,6 +97,21 @@  namespace libcamera::ipa::ipu3 {
  * \brief Maximum analogue gain supported with the configured sensor
  */
 
+/**
+ * \var IPAFrameContext::af
+ * \brief Context for the Automatic Focus algorithm
+ *
+ * \struct  IPAFrameContext::af
+ * \var IPAFrameContext::af.focus
+ * \brief Current position of the lens
+ *
+ * \var IPAFrameContext::af.maxVariance
+ * \brief The maximum variance of the current image.
+ *
+ * \var IPAFrameContext::af.stable
+ * \brief is the image focused?
+ */
+
 /**
  * \var IPAFrameContext::agc
  * \brief Context for the Automatic Gain Control algorithm
diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
index 1e46c61a..a7ffcbed 100644
--- a/src/ipa/ipu3/ipa_context.h
+++ b/src/ipa/ipu3/ipa_context.h
@@ -31,6 +31,11 @@  struct IPASessionConfiguration {
 		double minAnalogueGain;
 		double maxAnalogueGain;
 	} agc;
+
+	struct {
+		uint32_t start_x;
+		uint32_t start_y;
+	} af;
 };
 
 struct IPAFrameContext {
@@ -47,6 +52,12 @@  struct IPAFrameContext {
 		} gains;
 	} awb;
 
+	struct {
+		uint32_t focus;
+		double maxVariance;
+		bool stable;
+	} af;
+
 	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..f19d0059 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -30,6 +30,7 @@ 
 
 #include "libcamera/internal/mapped_framebuffer.h"
 
+#include "algorithms/af.h"
 #include "algorithms/agc.h"
 #include "algorithms/algorithm.h"
 #include "algorithms/awb.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>());