[{"id":22125,"web_url":"https://patchwork.libcamera.org/comment/22125/","msgid":"","date":"2022-02-05T06:11:37","subject":"Re: [libcamera-devel] [RFC v5] ipa: ipu3: af: Auto focus for dw9719\n\tSurface Go2 VCM","submitter":{"id":105,"url":"https://patchwork.libcamera.org/api/people/105/","name":"Kate Hsuan","email":"hpa@redhat.com"},"content":"Hi Jean-Michel,\n\n== The second part of the mail :)\n\nOn Fri, Jan 21, 2022 at 5:52 PM Jean-Michel Hautbois\n wrote:\n>\n> Sorry, this is the second part of my review, bad key stroke :-(\n>\n> On 21/01/2022 10:26, Jean-Michel Hautbois wrote:\n> > Hi Kate,\n> >\n> > Thanks for the patch !\n> >\n> > On 20/01/2022 12:41, Kate Hsuan wrote:\n> >> Since VCM for surface Go 2 (dw9719) had been successfully\n> >> driven, this Af module can be used to control the VCM and\n> >> determine the focus value based on the IPU3 AF state.\n> >>\n> >> The variance of each focus step is determined and a greedy\n> >> approah is used to find the maximum variance of the AF\n> >> state and a appropriate focus value.\n> > s/approah/approach\n> >\n> >>\n> >> The grid configuration is implemented as a context. Also,\n> >> the grid parameter- AF_MIN_BLOCK_WIDTH is set to 4 (default\n> >> is 3) since if the default value is used, x_start\n> >> (x_start > 640) will be at an incorrect location of the\n> >> image (rightmost of the sensor).\n> >\n> > We should investigate this, as it should not be a blocker ?\n> >\n> >>\n> >> Signed-off-by: Kate Hsuan \n> >> ---\n> >> src/ipa/ipu3/algorithms/af.cpp | 367 ++++++++++++++++++++++++++++\n> >> src/ipa/ipu3/algorithms/af.h | 79 ++++++\n> >> src/ipa/ipu3/algorithms/meson.build | 3 +-\n> >> src/ipa/ipu3/ipa_context.cpp | 24 ++\n> >> src/ipa/ipu3/ipa_context.h | 10 +\n> >> src/ipa/ipu3/ipu3.cpp | 32 +++\n> >> 6 files changed, 514 insertions(+), 1 deletion(-)\n> >> create mode 100644 src/ipa/ipu3/algorithms/af.cpp\n> >> create mode 100644 src/ipa/ipu3/algorithms/af.h\n> >>\n> >> diff --git a/src/ipa/ipu3/algorithms/af.cpp\n> >> b/src/ipa/ipu3/algorithms/af.cpp\n> >> new file mode 100644\n> >> index 00000000..26c98868\n> >> --- /dev/null\n> >> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> >> @@ -0,0 +1,367 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Red Hat\n> >> + *\n> >> + * af.cpp - IPU3 auto focus control\n> >> + */\n> >> +\n> >> +#include \"af.h\"\n> >> +\n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \"libipa/histogram.h\"\n> >> +\n> >> +/**\n> >> + * \\file af.h\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def AF_MIN_GRID_WIDTH\n> >> + * \\brief the minimum width of AF grid.\n> >> + * The minimum grid horizontal dimensions, in number of grid\n> >> blocks(cells).\n> >> +*/\n> >\n> > This is true for this one and all the defines, please replace those by\n> > static constexpr instead ?\n> >\n> >> +\n> >> +/**\n> >> + * \\def AF_MIN_GRID_HEIGHT\n> >> + * \\brief the minimum height of AF grid.\n> >> + * The minimum grid horizontal dimensions, in number of grid\n> >> blocks(cells).\n> >> +*/\n> >> +\n> >> +/**\n> >> + * \\def AF_MIN_BLOCK_WIDTH\n> >> + * \\brief the minimum block size of the width.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def AF_MAX_GRID_HEIGHT\n> >> + * \\brief the minimum height of AF grid.\n> >> + * The minimum grid horizontal dimensions, in number of grid\n> >> blocks(cells).\n> >> +*/\n> >> +\n> >> +/**\n> >> + * \\def AF_MAX_BLOCK_WIDTH\n> >> + * \\brief the minimum block size of the width.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def AF_MAX_BLOCK_WIDTH\n> >> + * \\brief the maximum block size of the width.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def AF_MIN_BLOCK_HEIGHT\n> >> + * \\brief the minimum block size of the height.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def AF_MAX_BLOCK_HEIGHT\n> >> + * \\brief the maximum block size of the height.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\def AF_DEFAULT_HEIGHT_PER_SLICE\n> >> + * \\brief The default number of blocks in vertical axis per slice.\n> >> + */\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +using namespace std::literals::chrono_literals;\n> >> +\n> >> +namespace ipa::ipu3::algorithms {\n> >> +\n> >> +/**\n> >> + * \\class Af\n> >> + * \\brief A IPU3 auto-focus accelerator based auto focus algorthim\n> >\n> > 'An auto-focus algorithm based on IPU3 statistics' maybe ?\n> > This proposal should be taken with a grain of salt, as I am not a native\n> > speaker ;-).\n> >\n> >> + *\n> >> + * This algorithm is used to determine the position of the lens and\n> >> get a\n> >> + * focused image. The IPU3 AF accelerator computes the statistics,\n> >> composed\n> > s/accelerator/processing block/\n> >\n> >> + * by high pass and low pass filtered value and stores in a AF buffer.\n> >> + * Typically, for a focused image, it has relative high contrast than a\n> >> + * blurred image, i.e. an out of focus image. Therefore, if an image\n> >> with the\n> >> + * highest contrast can be found from the AF scan, the lens' position\n> >> is the\n> >> + * best step of the focus.\n> >> + *\n> >> + */\n> >> +\n> >> +LOG_DEFINE_CATEGORY(IPU3Af)\n> >> +\n> >> +/**\n> >> + * Maximum focus value of the VCM control\n> >> + * \\todo should be obtained from the VCM driver\n> >> + */\n> >> +static constexpr uint32_t MaxFocusSteps_ = 1023;\n> >\n> > Remove the trailing underscore (same below):\n> > s/MaxFocusSteps_/maxFocusSteps/\n> >\n> >> +\n> >> +/* minimum focus step for searching appropriate focus */\n> >> +static constexpr uint32_t coarseSearchStep_ = 10;\n> >> +static constexpr uint32_t fineSearchStep_ = 1;\n> >\n> > Correct me if I am wrong:\n> > - the coarse step is used to determine a first approximation of the\n> > focused image\n> > - The fine step is then used to find the optimal lens position\n> >\n> > The VCM used on SGo2 has 1024 steps, so, you may search for ~100 frames\n> > (ie ~3 seconds at 30fps) before going into the fine search step ?\n> >\n> >> +\n> >> +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0 */\n> >> +static constexpr double MaxChange_ = 0.2;\n> >> +\n> >> +/* the numbers of frame to be ignored, before performing focus scan. */\n> >> +static constexpr uint32_t ignoreFrame_ = 10;\n> >> +\n> >> +/* fine scan range 0 < findRange_ < 1 */\n> >> +static constexpr double findRange_ = 0.05;\n> >> +\n> >> +/* settings for Auto Focus from the kernel */\n> > Those filters are not the ones defined in the kernel ;-).\n> >\n> >> +static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {\n> >> + { 0, 1, 3, 7 },\n> >> + { 11, 13, 1, 2 },\n> >> + { 8, 19, 34, 242 },\n> >> + 0x7fdffbfe,\n> >> + { 0, 1, 6, 6 },\n> >> + { 13, 25, 3, 0 },\n> >> + { 25, 3, 177, 254 },\n> >> + 0x4e53ca72,\n> >> + .y_calc = { 8, 8, 8, 8 },\n> >> + .nf = { 0, 9, 0, 9, 0 },\n> >> +};\n> >\n> > Use the fields names for all the structure:\n> > static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {\n> > .y1_coeff_0 = { 0, 1, 3, 7 },\n> > .y1_coeff_1 = { 11, 13, 1, 2 },\n> > etc.\n> > };\n> >\n> >> +\n> >> +Af::Af()\n> >> + : focus_(0), goodFocus_(0), currentVariance_(0.0),\n> >> previousVariance_(0.0),\n> >> + coarseComplete_(false), fineComplete_(false)\n> >> +{\n> >> + maxStep_ = MaxFocusSteps_;\n> >> +}\n> >> +\n> >> +Af::~Af()\n> >> +{\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> >> + */\n> >> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)\n> >> +{\n> >> + const struct ipu3_uapi_grid_config &grid =\n> >> context.configuration.af.afGrid;\n> >> + params->acc_param.af.grid_cfg = grid;\n> >> + params->acc_param.af.filter_config = afFilterConfigDefault;\n> >> +\n> >> + /* enable AF acc */\n> > s/acc/processing block/\n> >\n> >> + params->use.acc_af = 1;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Configure the Af given a configInfo\n> >> + * \\param[in] context The shared IPA context\n> >> + * \\param[in] configInfo The IPA configuration data\n> >> + *\n> >> + * \\return 0\n> >> + */\n> >> +int Af::configure(IPAContext &context,\n> >> + [[maybe_unused]] const IPAConfigInfo &configInfo)\n> >\n> > Are you using the utils/hooks/ in your git repo ?\n> > If so, I think you should have a misaligned warning here ?\n> >\n> >> +{\n> >> + /* determined focus value i.e. current focus value */\n> >> + context.frameContext.af.focus = 0;\n> >> + /* maximum variance of the AF statistics */\n> >> + context.frameContext.af.maxVariance = 0;\n> >> + /* the stable AF value flag. if it is true, the AF should be in a\n> >> stable state. */\n> >> + context.frameContext.af.stable = false;\n> >> + /* AF buffer length */\n> >> + afRawBufferLen_ = context.configuration.af.afGrid.width *\n> >> + context.configuration.af.afGrid.height;\n> >\n> > AFAIK, this is only used in the process() function, maybe could it be a\n> > local variable ?\n> >\n> >> +\n> >> + return 0;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief AF coarse scan\n> >> + * \\param[in] context The shared IPA context\n> >> + *\n> >> + */\n> >> +void Af::afCoarseScan(IPAContext &context)\n> >> +{\n> >> + if (coarseComplete_ == true)\n> >> + return;\n> >> +\n> >> + if (afScan(context, coarseSearchStep_)) {\n> >> + coarseComplete_ = true;\n> >> + context.frameContext.af.maxVariance = 0;\n> >> + focus_ = context.frameContext.af.focus -\n> >> (context.frameContext.af.focus * findRange_);\n> >> + context.frameContext.af.focus = focus_;\n> >> + previousVariance_ = 0;\n> >> + maxStep_ = std::clamp(static_cast(focus_ + (focus_\n> >> * findRange_)), 0U, MaxFocusSteps_);\n> >> + }\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief AF fine scan\n> >> + *\n> >> + * Finetune the lens position with moving 1 step for each variance\n> >> computation.\n> >> + *\n> >> + * \\param[in] context The shared IPA context\n> >> + *\n> >> + */\n> >> +void Af::afFineScan(IPAContext &context)\n> >> +{\n> >> + if (coarseComplete_ != true)\n> >> + return;\n> >> +\n> >> + if (afScan(context, fineSearchStep_)) {\n> >> + context.frameContext.af.stable = true;\n> >> + fineComplete_ = true;\n> >> + }\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief AF reset\n> >> + *\n> >> + * Reset all the parameter to start over the AF process.\n> >> + *\n> >> + * \\param[in] context The shared IPA context\n> >> + *\n> >> + */\n> >> +void Af::afReset(IPAContext &context)\n> >> +{\n> >> + context.frameContext.af.maxVariance = 0;\n> >> + context.frameContext.af.focus = 0;\n> >> + focus_ = 0;\n> >> + context.frameContext.af.stable = false;\n> >> + ignoreCounter_ = ignoreFrame_;\n> >> + previousVariance_ = 0.0;\n> >> + coarseComplete_ = false;\n> >> + fineComplete_ = false;\n> >> + maxStep_ = MaxFocusSteps_;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief AF scan\n> >> + * \\param[in] context The shared IPA context\n> >> + *\n> >> + * \\return True, if it finds a AF value.\n> >> + */\n> >> +bool Af::afScan(IPAContext &context, int min_step)\n> >> +{\n> >> + /* find the maximum variance during the AF scan using a greedy\n> >> strategy */\n>\n> I am interested here: why greedy :-) ?\n>\n> >> + if (currentVariance_ > context.frameContext.af.maxVariance) {\n> >> + context.frameContext.af.maxVariance = currentVariance_;\n> >> + goodFocus_ = focus_;\n> >> + }\n> >> +\n> >> + if (focus_ > maxStep_) {\n> >> + /* if reach the max step, move lens to the position and set\n> >> \"focus stable\". */\n> >> + context.frameContext.af.focus = goodFocus_;\n> >> + return true;\n> >> + } else {\n> >> + /* check negative gradient */\n>\n> You are mixing gradient and variance terms. You are checking the\n> direction of the variance derivate, right ?\n>\n> >> + if ((currentVariance_ - context.frameContext.af.maxVariance)\n> >> > -(context.frameContext.af.maxVariance * 0.15)) {\n> >> + focus_ += min_step;\n> >> + context.frameContext.af.focus = focus_;\n> >> + } else {\n> >> + context.frameContext.af.focus = goodFocus_;\n> >> + previousVariance_ = currentVariance_;\n> >> + return true;\n> >> + }\n> >> + }\n> >> + LOG(IPU3Af, Debug) << \"Variance previous: \"\n> >> + << previousVariance_\n> >> + << \" current: \"\n> >> + << currentVariance_\n> >> + << \" Diff: \"\n> >> + << (currentVariance_ -\n> >> context.frameContext.af.maxVariance);\n> >> + previousVariance_ = currentVariance_;\n> >> + LOG(IPU3Af, Debug) << \"Focus searching max variance is: \"\n> >> + << context.frameContext.af.maxVariance\n> >> + << \" Focus step is \"\n> >> + << goodFocus_\n> >> + << \" Current scan is \"\n> >> + << focus_;\n> >> + return false;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Determine the max contrast image and lens position. y_table\n> >> is the\n> >> + * statictic data from IPU3 and is composed of low pass and high pass\n> >> filtered\n> s/statictic/statistics\n>\n> >> + * value. High pass filtered value also represents the sharpness of\n> >> the image.\n> >> + * Based on this, if the image with highest variance of the high pass\n> >> filtered\n> >> + * value (contrast) during the AF scan, the position of the lens\n> >> should be the\n> >> + * best focus.\n> >> + * \\param[in] context The shared IPA context.\n> >> + * \\param[in] stats The statistic buffer of 3A from the IPU3.\n> >> + */\n> >> +void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)\n> >> +{\n> >> + uint32_t total = 0;\n> >> + double mean;\n> >> + uint64_t var_sum = 0;\n> >> + y_table_item_t *y_item;\n> >> + uint32_t z = 0;\n> >> +\n> >> + y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;\n>\n> static_cast<> ?\n\nOK, I'll try to use static_cast.\n\n>\n> >> +\n> >> + /**\n> >> + * Calculate the mean and the varience AF statistics, since IPU3\n> >> only determine the AF value\n> s/varience/variance\n>\n> >> + * for a given grid.\n> >> + * For coarse: low pass results are used.\n> >> + * For fine: high pass results are used.\n> >> + */\n> >> + if (coarseComplete_) {\n> >> + for (z = 0; z < afRawBufferLen_; z++) {\n> >> + total = total + y_item[z].y2_avg;\n> >> + }\n> >> + mean = total / afRawBufferLen_;\n> >> +\n> >> + for (z = 0; z < afRawBufferLen_; z++) {\n> >> + var_sum = var_sum + ((y_item[z].y2_avg - mean) *\n> >> (y_item[z].y2_avg - mean));\n> >> + }\n> >> + } else {\n> >> + for (z = 0; z < afRawBufferLen_; z++) {\n> >> + total = total + y_item[z].y1_avg;\n> >> + }\n> >> + mean = total / afRawBufferLen_;\n> >> +\n> >> + for (z = 0; z < afRawBufferLen_; z++) {\n> >> + var_sum = var_sum + ((y_item[z].y1_avg - mean) *\n> >> (y_item[z].y1_avg - mean));\n> >> + }\n> >> + }\n> >> +\n> >> + /* Determine the average variance of the frame. */\n> >> + currentVariance_ = static_cast(var_sum) /\n> >> static_cast(afRawBufferLen_);\n> >> + LOG(IPU3Af, Debug) << \"variance: \" << currentVariance_;\n> >> +\n> >> + if (context.frameContext.af.stable == true) {\n> >> + const uint32_t diff_var = std::abs(currentVariance_ -\n> >> context.frameContext.af.maxVariance);\n> >> + const double var_ratio = diff_var /\n> >> context.frameContext.af.maxVariance;\n> >> + LOG(IPU3Af, Debug) << \"Rate of variance change: \"\n> >> + << var_ratio\n> >> + << \" current focus step: \"\n> >> + << context.frameContext.af.focus;\n> >> + /**\n> >> + * If the change ratio of contrast is over Maxchange_ (out of\n> >> focus),\n> >> + * trigger AF again.\n> >> + */\n> >> + if (var_ratio > MaxChange_) {\n> >> + if (ignoreCounter_ == 0) {\n> >> + afReset(context);\n> >> + } else\n> >> + ignoreCounter_--;\n> >> + } else\n> >> + ignoreCounter_ = ignoreFrame_;\n> >> + } else {\n> >> + if (ignoreCounter_ != 0)\n> >> + ignoreCounter_--;\n> >> + else {\n> >> + afCoarseScan(context);\n> >> + afFineScan(context);\n> >> + }\n> >> + }\n> >> +}\n> >> +\n> >> +} /* namespace ipa::ipu3::algorithms */\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h\n> >> new file mode 100644\n> >> index 00000000..5654a964\n> >> --- /dev/null\n> >> +++ b/src/ipa/ipu3/algorithms/af.h\n> >> @@ -0,0 +1,79 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Red Hat\n> >> + *\n> >> + * af.h - IPU3 Af control\n> >> + */\n> >> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_AF_H__\n> >> +#define __LIBCAMERA_IPU3_ALGORITHMS_AF_H__\n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \"algorithm.h\"\n> >> +\n> >> +/* Definitions from repo of chromium */\n> >> +#define AF_MIN_GRID_WIDTH 16\n> >> +#define AF_MIN_GRID_HEIGHT 16\n> >> +#define AF_MAX_GRID_WIDTH 32\n> >> +#define AF_MAX_GRID_HEIGHT 24\n> >> +#define AF_MIN_BLOCK_WIDTH 4\n> >> +#define AF_MIN_BLOCK_HEIGHT 3\n> >> +#define AF_MAX_BLOCK_WIDTH 6\n> >> +#define AF_MAX_BLOCK_HEIGHT 6\n> >> +#define AF_DEFAULT_HEIGHT_PER_SLICE 2\n>\n> Those would better be static constexpr ?\n\nOK\n\n>\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +namespace ipa::ipu3::algorithms {\n> >> +\n> >> +class Af : public Algorithm\n> >> +{\n> >> + /* The format of y_table. From ipu3-ipa repo */\n> >> + typedef struct __attribute__((packed)) y_table_item {\n> >> + uint16_t y1_avg;\n> >> + uint16_t y2_avg;\n> >> + } y_table_item_t;\n> >> +\n> >> +public:\n> >> + Af();\n> >> + ~Af();\n> >> +\n> >> + void prepare(IPAContext &context, ipu3_uapi_params *params)\n> >> override;\n> >> + int configure(IPAContext &context, const IPAConfigInfo\n> >> &configInfo) override;\n> >> + void process(IPAContext &context, const ipu3_uapi_stats_3a\n> >> *stats) override;\n> >> +\n> >> +private:\n> >> + void afCoarseScan(IPAContext &context);\n> >> + void afFineScan(IPAContext &context);\n> >> + bool afScan(IPAContext &context, int min_step);\n> >> + void afReset(IPAContext &context);\n> >> +\n> >> + /* Used for focus scan. */\n> >> + uint32_t focus_;\n> >> + /* Focus good */\n> >> + uint32_t goodFocus_;\n> >> + /* Recent AF statistic variance. */\n> >> + double currentVariance_;\n> >> + /* The frames to be ignore before starting measuring. */\n> >> + uint32_t ignoreCounter_;\n> >> + /* previous variance. it is used to determine the gradient */\n> >> + double previousVariance_;\n> >> + /* Max scan steps of each pass of AF scaning */\n> >> + uint32_t maxStep_;\n> >> + /* coarse scan stable. Complete low pass search (coarse) scan) */\n> >> + bool coarseComplete_;\n> >> + /* fine scan stable. Complete high pass scan (fine scan) */\n> >> + bool fineComplete_;\n> >> + /* Raw buffer length */\n> >> + uint32_t afRawBufferLen_;\n> >> +};\n> >> +\n> >> +} /* namespace ipa::ipu3::algorithms */\n> >> +\n> >> +} /* namespace libcamera */\n> >> +\n> >> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */\n> >> diff --git a/src/ipa/ipu3/algorithms/meson.build\n> >> b/src/ipa/ipu3/algorithms/meson.build\n> >> index 4db6ae1d..e1099169 100644\n> >> --- a/src/ipa/ipu3/algorithms/meson.build\n> >> +++ b/src/ipa/ipu3/algorithms/meson.build\n> >> @@ -1,8 +1,9 @@\n> >> # SPDX-License-Identifier: CC0-1.0\n> >> ipu3_ipa_algorithms = files([\n> >> + 'af.cpp',\n> >> 'agc.cpp',\n> >> 'awb.cpp',\n> >> 'blc.cpp',\n> >> - 'tone_mapping.cpp',\n> >> + 'tone_mapping.cpp'\n> >> ])\n> >> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> >> index 86794ac1..ecb2b90a 100644\n> >> --- a/src/ipa/ipu3/ipa_context.cpp\n> >> +++ b/src/ipa/ipu3/ipa_context.cpp\n> >> @@ -67,6 +67,30 @@ namespace libcamera::ipa::ipu3 {\n> >> *\n> >> * \\var IPASessionConfiguration::grid.stride\n> >> * \\brief Number of cells on one line including the ImgU padding\n> >> + *\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var IPASessionConfiguration::af\n> >> + * \\brief AF grid configuration of the IPA\n> >> + *\n> >> + * \\var IPASessionConfiguration::af.afGrid\n> >> + *\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\var IPAFrameContext::af\n> >> + * \\brief Context for the Automatic Focus algorithm\n> >> + *\n> >> + * \\struct IPAFrameContext::af\n> >> + * \\var IPAFrameContext::af.focus\n> >> + * \\brief Current position of the lens\n> >> + *\n> >> + * \\var IPAFrameContext::af.maxVariance\n> >> + * \\brief The maximum variance of the current image.\n> >> + *\n> >> + * \\var IPAFrameContext::af.stable\n> >> + * \\brief is the image focused?\n> >> */\n> >> /**\n> >> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> >> index c6dc0814..e643d38f 100644\n> >> --- a/src/ipa/ipu3/ipa_context.h\n> >> +++ b/src/ipa/ipu3/ipa_context.h\n> >> @@ -31,6 +31,10 @@ struct IPASessionConfiguration {\n> >> double minAnalogueGain;\n> >> double maxAnalogueGain;\n> >> } agc;\n> >> +\n> >> + struct {\n> >> + ipu3_uapi_grid_config afGrid;\n> >> + } af;\n>\n> Alphabetical order (af should be before agc).\n>\n> >> };\n> >> struct IPAFrameContext {\n> >> @@ -49,6 +53,12 @@ struct IPAFrameContext {\n> >> double temperatureK;\n> >> } awb;\n>\n> Add a blank line.\n>\n> >> + struct {\n> >> + uint32_t focus;\n> >> + double maxVariance;\n> >> + bool stable;\n> >> + } af;\n>\n> Alphabetical order.\n>\n> >> +\n> >> struct {\n> >> uint32_t exposure;\n> >> double gain;\n> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> >> index 3d307708..b5149bcd 100644\n> >> --- a/src/ipa/ipu3/ipu3.cpp\n> >> +++ b/src/ipa/ipu3/ipu3.cpp\n> >> @@ -30,6 +30,7 @@\n> >> #include \"libcamera/internal/mapped_framebuffer.h\"\n> >> +#include \"algorithms/af.h\"\n> >> #include \"algorithms/agc.h\"\n> >> #include \"algorithms/algorithm.h\"\n> >> #include \"algorithms/awb.h\"\n> >> @@ -157,6 +158,7 @@ private:\n> >> void setControls(unsigned int frame);\n> >> void calculateBdsGrid(const Size &bdsOutputSize);\n> >> + void initAfGrid(const Size &bdsOutputSize);\n> >> std::map buffers_;\n> >> @@ -294,6 +296,7 @@ int IPAIPU3::init(const IPASettings &settings,\n> >> }\n> >> /* Construct our Algorithms */\n> >> + algorithms_.push_back(std::make_unique());H\n> >> algorithms_.push_back(std::make_unique());\n> >> algorithms_.push_back(std::make_unique());\n> >>\n> >> algorithms_.push_back(std::make_unique());\n> >>\n> >> @@ -395,6 +398,33 @@ void IPAIPU3::calculateBdsGrid(const Size\n> >> &bdsOutputSize)\n> >> << (int)bdsGrid.height << \" << \" <<\n> >> (int)bdsGrid.block_height_log2 << \")\";\n> >> }\n> >> +/**\n> >> + * \\brief Configure the IPU3 AF grid\n> >> + *\n> >> + * This function gives the default values for the AF grid configuration.\n> >> + * All the parameters are set to the minimum acceptable values.\n> >> + *\n> >> + * \\param bdsOutputSize The bsd output size\n> s/bsd/Bayer Down Scaler/\n>\n> >> + */\n> >> +void IPAIPU3::initAfGrid(const Size &bdsOutputSize)\n> >> +{\n> >> + struct ipu3_uapi_grid_config &grid =\n> >> context_.configuration.af.afGrid;\n> >> + grid.width = AF_MIN_GRID_WIDTH;\n> >> + grid.height = AF_MIN_GRID_HEIGHT;\n> >> + grid.block_width_log2 = AF_MIN_BLOCK_WIDTH;\n> >> + grid.block_height_log2 = AF_MIN_BLOCK_HEIGHT;\n> >> + grid.height_per_slice = AF_DEFAULT_HEIGHT_PER_SLICE;\n> >> + /* x_start and y start are default to BDS center */\n> >> + grid.x_start = (bdsOutputSize.width / 2) -\n> >> + (((grid.width << grid.block_width_log2) / 2));\n> >> + grid.y_start = (bdsOutputSize.height / 2) -\n> >> + (((grid.height << grid.block_height_log2) / 2));\n> >> + /* make sure x_start is multiplied by 10 and y_start is\n> >> multiplied by 2 */\n> >> + grid.x_start = (grid.x_start / 10) * 10;\n> >> + grid.y_start = (grid.y_start / 2) * 2;\n>\n> I am not sure to understand why you are doing this ?\n> We nned to be sure x_start is at least 10 pixels from the left of the\n> frame, and y_start needs to be 2 pixels from the top of the frame. I am\n> not sure we need to have them as multiples of 10 (and 2) ?\n\nHere is the chrome OS mentioned.\nhttps://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/af_public.h#84\n\nunsigned short x_start; /**< default value 10, X top left corner of\nthe grid x_start is even */ (should be even)\nunsigned short y_start; /**< default value 2, Y top left corner of the\ngrid y_start is even */ (should be even)\n\nI actually set both x_start and y_start to odd numbers and the AF\ncould not work properly.\nI would double-check that behaviour and try to make sure both of them\nare even numbers.\n\nAlso, I'll keep x_start and y_start are multiplied by 2 in the next v6 patch.\n\n>\n> >> + grid.y_start = grid.y_start | IPU3_UAPI_GRID_Y_START_EN;\n> >> +}\n> >> +\n> >> /**\n> >> * \\brief Configure the IPU3 IPA\n> >> * \\param[in] configInfo The IPA configuration data, received from\n> >> the pipeline\n> >> @@ -461,6 +491,8 @@ int IPAIPU3::configure(const IPAConfigInfo\n> >> &configInfo,\n> >> lineDuration_ = sensorInfo_.lineLength * 1.0s /\n> >> sensorInfo_.pixelRate;\n> >> + initAfGrid(configInfo.bdsOutputSize);\n> >> +\n> >> /* Update the camera controls using the new sensor settings. */\n> >> updateControls(sensorInfo_, ctrls_, ipaControls);\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DD3DDBDCBF\n\tfor ;\n\tSat, 5 Feb 2022 06:12:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB26D60E5D;\n\tSat, 5 Feb 2022 07:12:00 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FAD260E4C\n\tfor ;\n\tSat, 5 Feb 2022 07:11:57 +0100 (CET)","from mail-lj1-f198.google.com (mail-lj1-f198.google.com\n\t[209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id\n\tus-mta-377-WevZfw-yNkaAbyjnnoQVjg-1; Sat, 05 Feb 2022 01:11:53 -0500","by mail-lj1-f198.google.com with SMTP id\n\ty19-20020a2e9793000000b0023f158d6cc0so1811957lji.10\n\tfor ;\n\tFri, 04 Feb 2022 22:11:52 -0800 (PST)"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"hS9euW8D\"; dkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hpa@redhat.com"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1644041516;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=ZxsHaTjhKFR5ZJmb8HzCQFmFkD/+feQjLswDXw5gMD0=;\n\tb=hS9euW8DIQLqhOpuMAFnnr7pTA7E+aQRZMwrbEjNp5tcr+ckcCllziDUrOcM+ClauNNA/Z\n\tK0mTVmNqJ1kVfpjvXlSsumnLXOXB+jBWhhqAh415kM3LeppPGktBZatwcP3C3zMErI8J5P\n\tHLb4508mLH3G51HxhIxl9k92S5ZmYXE=","X-MC-Unique":"WevZfw-yNkaAbyjnnoQVjg-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ZxsHaTjhKFR5ZJmb8HzCQFmFkD/+feQjLswDXw5gMD0=;\n\tb=kuDYhWEh1/PhB5B2MbZlg6UUdY+uLbYaWGI7Eg6HemtDuDmcxUOEIwhyCmBrbmoNOJ\n\tK5GHcw+YrKoW7vWb1LKxdpx1zNnD54fI7sESooECnSZzVsbcVEQcseXiHd4vetkKuiL8\n\tzjxNf8AEYROjO8uq2oh0ms8ZDgmieDGYxD3jNWufFh7x7WMZf9XJAmnfQg90iYMqVKKr\n\tEagKyd+l4w3yGZOVWPe0yysbAO0AdPr11ko1389/0ogdKkdOtjYuaRf6jkxsL2PXt3lJ\n\tjC0dNen3HuwrWVTTuJrwaqqyKhkfWCFeuM4C8FNhUugVkHVJXqz6iyX10fiC17wLFK36\n\tHn4Q==","X-Gm-Message-State":"AOAM531beiQbhyYE28cHEipcuvcP44xgKkdjdv/vXrKn9I/pPXccb8dD\n\tcaHrLTxPkLFq36v+pvYf3soeeHJYbpYM+lZQX4z5AjVmZG3oshscKPZWHMQNzmjeWrrIfyubo7d\n\tmRFoM2T7HapZrpcaoZl8fLTP9+53DYEAjKwW7si0zTqo8J+1QJw==","X-Received":["by 2002:a05:6512:3a85:: with SMTP id\n\tq5mr1642623lfu.539.1644041510999; \n\tFri, 04 Feb 2022 22:11:50 -0800 (PST)","by 2002:a05:6512:3a85:: with SMTP id\n\tq5mr1642601lfu.539.1644041510369; \n\tFri, 04 Feb 2022 22:11:50 -0800 (PST)"],"X-Google-Smtp-Source":"ABdhPJxTzKJMHnR6aVq0ijDjz1XiZy1VuV/ldBOZJliLZe6Zie9be6q5TGE5sbRi3Sz97QS55P3/v1ncuF1Qzcqkqok=","MIME-Version":"1.0","References":"<20220120114102.19477-1-hpa@redhat.com>\n\t<22742944-986b-2899-21b6-22b33774f352@ideasonboard.com>\n\t<2def17f3-792d-ee31-b89f-c0de43e22552@ideasonboard.com>","In-Reply-To":"<2def17f3-792d-ee31-b89f-c0de43e22552@ideasonboard.com>","From":"Kate Hsuan ","Date":"Sat, 5 Feb 2022 14:11:37 +0800","Message-ID":"","To":"Jean-Michel Hautbois ","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC v5] ipa: ipu3: af: Auto focus for dw9719\n\tSurface Go2 VCM","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]