[libcamera-devel] [RFC v5] ipa: ipu3: af: Auto focus for dw9719 Surface Go2 VCM

Jean-Michel Hautbois jeanmichel.hautbois at ideasonboard.com
Fri Jan 21 10:52:11 CET 2022


Sorry, this is the second part of my review, bad key stroke :-(

On 21/01/2022 10:26, Jean-Michel Hautbois wrote:
> Hi Kate,
> 
> Thanks for the patch !
> 
> On 20/01/2022 12:41, 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.
> s/approah/approach
> 
>>
>> The grid configuration is implemented as a context. Also,
>> the grid parameter- AF_MIN_BLOCK_WIDTH is set to 4 (default
>> is 3) since if the default value is used, x_start
>> (x_start > 640) will be at an incorrect location of the
>> image (rightmost of the sensor).
> 
> We should investigate this, as it should not be a blocker ?
> 
>>
>> Signed-off-by: Kate Hsuan <hpa at redhat.com>
>> ---
>>   src/ipa/ipu3/algorithms/af.cpp      | 367 ++++++++++++++++++++++++++++
>>   src/ipa/ipu3/algorithms/af.h        |  79 ++++++
>>   src/ipa/ipu3/algorithms/meson.build |   3 +-
>>   src/ipa/ipu3/ipa_context.cpp        |  24 ++
>>   src/ipa/ipu3/ipa_context.h          |  10 +
>>   src/ipa/ipu3/ipu3.cpp               |  32 +++
>>   6 files changed, 514 insertions(+), 1 deletion(-)
>>   create mode 100644 src/ipa/ipu3/algorithms/af.cpp
>>   create mode 100644 src/ipa/ipu3/algorithms/af.h
>>
>> diff --git a/src/ipa/ipu3/algorithms/af.cpp 
>> b/src/ipa/ipu3/algorithms/af.cpp
>> new file mode 100644
>> index 00000000..26c98868
>> --- /dev/null
>> +++ b/src/ipa/ipu3/algorithms/af.cpp
>> @@ -0,0 +1,367 @@
>> +/* 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
>> + */
>> +
>> +/**
>> + * \def AF_MIN_GRID_WIDTH
>> + * \brief the minimum width of AF grid.
>> + * The minimum grid horizontal dimensions, in number of grid 
>> blocks(cells).
>> +*/
> 
> This is true for this one and all the defines, please replace those by 
> static constexpr instead ?
> 
>> +
>> +/**
>> + * \def AF_MIN_GRID_HEIGHT
>> + * \brief the minimum height of AF grid.
>> + * The minimum grid horizontal dimensions, in number of grid 
>> blocks(cells).
>> +*/
>> +
>> +/**
>> + * \def AF_MIN_BLOCK_WIDTH
>> + * \brief the minimum block size of the width.
>> + */
>> +
>> +/**
>> + * \def AF_MAX_GRID_HEIGHT
>> + * \brief the minimum height of AF grid.
>> + * The minimum grid horizontal dimensions, in number of grid 
>> blocks(cells).
>> +*/
>> +
>> +/**
>> + * \def AF_MAX_BLOCK_WIDTH
>> + * \brief the minimum block size of the width.
>> + */
>> +
>> +/**
>> + * \def AF_MAX_BLOCK_WIDTH
>> + * \brief the maximum block size of the width.
>> + */
>> +
>> +/**
>> + * \def AF_MIN_BLOCK_HEIGHT
>> + * \brief the minimum block size of the height.
>> + */
>> +
>> +/**
>> + * \def AF_MAX_BLOCK_HEIGHT
>> + * \brief the maximum block size of the height.
>> + */
>> +
>> +/**
>> + * \def AF_DEFAULT_HEIGHT_PER_SLICE
>> + * \brief The default number of blocks in vertical axis per slice.
>> + */
>> +
>> +namespace libcamera {
>> +
>> +using namespace std::literals::chrono_literals;
>> +
>> +namespace ipa::ipu3::algorithms {
>> +
>> +/**
>> + * \class Af
>> + * \brief A IPU3 auto-focus accelerator based auto focus algorthim
> 
> 'An auto-focus algorithm based on IPU3 statistics' maybe ?
> This proposal should be taken with a grain of salt, as I am not a native 
> speaker ;-).
> 
>> + *
>> + * This algorithm is used to determine the position of the lens and 
>> get a
>> + * focused image. The IPU3 AF accelerator computes the statistics, 
>> composed
> s/accelerator/processing block/
> 
>> + * 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;
> 
> Remove the trailing underscore (same below):
> s/MaxFocusSteps_/maxFocusSteps/
> 
>> +
>> +/* minimum focus step for searching appropriate focus */
>> +static constexpr uint32_t coarseSearchStep_ = 10;
>> +static constexpr uint32_t fineSearchStep_ = 1;
> 
> Correct me if I am wrong:
> - the coarse step is used to determine a first approximation of the 
> focused image
> - The fine step is then used to find the optimal lens position
> 
> The VCM used on SGo2 has 1024 steps, so, you may search for ~100 frames 
> (ie ~3 seconds at 30fps) before going into the fine search step ?
> 
>> +
>> +/* max ratio of variance change, 0.0 < MaxChange_ < 1.0 */
>> +static constexpr double MaxChange_ = 0.2;
>> +
>> +/* the numbers of frame to be ignored, before performing focus scan. */
>> +static constexpr uint32_t ignoreFrame_ = 10;
>> +
>> +/* fine scan range 0 < findRange_ < 1 */
>> +static constexpr double findRange_ = 0.05;
>> +
>> +/* settings for Auto Focus from the kernel */
> Those filters are not the ones defined in the kernel ;-).
> 
>> +static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {
>> +    { 0, 1, 3, 7 },
>> +    { 11, 13, 1, 2 },
>> +    { 8, 19, 34, 242 },
>> +    0x7fdffbfe,
>> +    { 0, 1, 6, 6 },
>> +    { 13, 25, 3, 0 },
>> +    { 25, 3, 177, 254 },
>> +    0x4e53ca72,
>> +    .y_calc = { 8, 8, 8, 8 },
>> +    .nf = { 0, 9, 0, 9, 0 },
>> +};
> 
> Use the fields names for all the structure:
> static struct ipu3_uapi_af_filter_config afFilterConfigDefault = {
>      .y1_coeff_0 = { 0, 1, 3, 7 },
>      .y1_coeff_1 = { 11, 13, 1, 2 },
> etc.
> };
> 
>> +
>> +Af::Af()
>> +    : focus_(0), goodFocus_(0), currentVariance_(0.0), 
>> previousVariance_(0.0),
>> +      coarseComplete_(false), fineComplete_(false)
>> +{
>> +    maxStep_ = MaxFocusSteps_;
>> +}
>> +
>> +Af::~Af()
>> +{
>> +}
>> +
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::prepare
>> + */
>> +void Af::prepare(IPAContext &context, ipu3_uapi_params *params)
>> +{
>> +    const struct ipu3_uapi_grid_config &grid = 
>> context.configuration.af.afGrid;
>> +    params->acc_param.af.grid_cfg = grid;
>> +    params->acc_param.af.filter_config = afFilterConfigDefault;
>> +
>> +    /* enable AF acc */
> s/acc/processing block/
> 
>> +    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)
> 
> Are you using the utils/hooks/ in your git repo ?
> If so, I think you should have a misaligned warning here ?
> 
>> +{
>> +    /* determined focus value i.e. current focus value */
>> +    context.frameContext.af.focus = 0;
>> +    /* maximum variance of the AF statistics */
>> +    context.frameContext.af.maxVariance = 0;
>> +    /* the stable AF value flag. if it is true, the AF should be in a 
>> stable state. */
>> +    context.frameContext.af.stable = false;
>> +    /* AF buffer length */
>> +    afRawBufferLen_ = context.configuration.af.afGrid.width *
>> +              context.configuration.af.afGrid.height;
> 
> AFAIK, this is only used in the process() function, maybe could it be a 
> local variable ?
> 
>> +
>> +    return 0;
>> +}
>> +
>> +/**
>> + * \brief AF coarse scan
>> + * \param[in] context The shared IPA context
>> + *
>> + */
>> +void Af::afCoarseScan(IPAContext &context)
>> +{
>> +    if (coarseComplete_ == true)
>> +        return;
>> +
>> +    if (afScan(context, coarseSearchStep_)) {
>> +        coarseComplete_ = true;
>> +        context.frameContext.af.maxVariance = 0;
>> +        focus_ = context.frameContext.af.focus - 
>> (context.frameContext.af.focus * findRange_);
>> +        context.frameContext.af.focus = focus_;
>> +        previousVariance_ = 0;
>> +        maxStep_ = std::clamp(static_cast<uint32_t>(focus_ + (focus_ 
>> * findRange_)), 0U, MaxFocusSteps_);
>> +    }
>> +}
>> +
>> +/**
>> + * \brief AF fine scan
>> + *
>> + * Finetune the lens position with moving 1 step for each variance 
>> computation.
>> + *
>> + * \param[in] context The shared IPA context
>> + *
>> + */
>> +void Af::afFineScan(IPAContext &context)
>> +{
>> +    if (coarseComplete_ != true)
>> +        return;
>> +
>> +    if (afScan(context, fineSearchStep_)) {
>> +        context.frameContext.af.stable = true;
>> +        fineComplete_ = true;
>> +    }
>> +}
>> +
>> +/**
>> + * \brief AF reset
>> + *
>> + * Reset all the parameter to start over the AF process.
>> + *
>> + * \param[in] context The shared IPA context
>> + *
>> + */
>> +void Af::afReset(IPAContext &context)
>> +{
>> +    context.frameContext.af.maxVariance = 0;
>> +    context.frameContext.af.focus = 0;
>> +    focus_ = 0;
>> +    context.frameContext.af.stable = false;
>> +    ignoreCounter_ = ignoreFrame_;
>> +    previousVariance_ = 0.0;
>> +    coarseComplete_ = false;
>> +    fineComplete_ = false;
>> +    maxStep_ = MaxFocusSteps_;
>> +}
>> +
>> +/**
>> + * \brief AF scan
>> + * \param[in] context The shared IPA context
>> + *
>> + * \return True, if it finds a AF value.
>> + */
>> +bool Af::afScan(IPAContext &context, int min_step)
>> +{
>> +    /* find the maximum variance during the AF scan using a greedy 
>> strategy */

I am interested here: why greedy :-) ?

>> +    if (currentVariance_ > context.frameContext.af.maxVariance) {
>> +        context.frameContext.af.maxVariance = currentVariance_;
>> +        goodFocus_ = focus_;
>> +    }
>> +
>> +    if (focus_ > maxStep_) {
>> +        /* if reach the max step, move lens to the position and set 
>> "focus stable". */
>> +        context.frameContext.af.focus = goodFocus_;
>> +        return true;
>> +    } else {
>> +        /* check negative gradient */

You are mixing gradient and variance terms. You are checking the 
direction of the variance derivate, right ?

>> +        if ((currentVariance_ - context.frameContext.af.maxVariance) 
>> > -(context.frameContext.af.maxVariance * 0.15)) {
>> +            focus_ += min_step;
>> +            context.frameContext.af.focus = focus_;
>> +        } else {
>> +            context.frameContext.af.focus = goodFocus_;
>> +            previousVariance_ = currentVariance_;
>> +            return true;
>> +        }
>> +    }
>> +    LOG(IPU3Af, Debug) << "Variance previous: "
>> +               << previousVariance_
>> +               << " current: "
>> +               << currentVariance_
>> +               << " Diff: "
>> +               << (currentVariance_ - 
>> context.frameContext.af.maxVariance);
>> +    previousVariance_ = currentVariance_;
>> +    LOG(IPU3Af, Debug) << "Focus searching max variance is: "
>> +               << context.frameContext.af.maxVariance
>> +               << " Focus step is "
>> +               << goodFocus_
>> +               << " Current scan is "
>> +               << focus_;
>> +    return false;
>> +}
>> +
>> +/**
>> + * \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
s/statictic/statistics

>> + * 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 lens 
>> 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;
>> +    uint32_t z = 0;
>> +
>> +    y_item = (y_table_item_t *)stats->af_raw_buffer.y_table;

static_cast<> ?

>> +
>> +    /**
>> +     * Calculate the mean and the varience AF statistics, since IPU3 
>> only determine the AF value
s/varience/variance

>> +     * for a given grid.
>> +     * For coarse: low pass results are used.
>> +     * For fine: high pass results are used.
>> +     */
>> +    if (coarseComplete_) {
>> +        for (z = 0; z < afRawBufferLen_; z++) {
>> +            total = total + y_item[z].y2_avg;
>> +        }
>> +        mean = total / afRawBufferLen_;
>> +
>> +        for (z = 0; z < afRawBufferLen_; z++) {
>> +            var_sum = var_sum + ((y_item[z].y2_avg - mean) * 
>> (y_item[z].y2_avg - mean));
>> +        }
>> +    } else {
>> +        for (z = 0; z < afRawBufferLen_; z++) {
>> +            total = total + y_item[z].y1_avg;
>> +        }
>> +        mean = total / afRawBufferLen_;
>> +
>> +        for (z = 0; z < afRawBufferLen_; z++) {
>> +            var_sum = var_sum + ((y_item[z].y1_avg - mean) * 
>> (y_item[z].y1_avg - mean));
>> +        }
>> +    }
>> +
>> +    /* Determine the average variance of the frame. */
>> +    currentVariance_ = static_cast<double>(var_sum) / 
>> static_cast<double>(afRawBufferLen_);
>> +    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) << "Rate of variance change: "
>> +                   << var_ratio
>> +                   << " current focus step: "
>> +                   << context.frameContext.af.focus;
>> +        /**
>> +         * If the change ratio of contrast is over Maxchange_ (out of 
>> focus),
>> +         * trigger AF again.
>> +         */
>> +        if (var_ratio > MaxChange_) {
>> +            if (ignoreCounter_ == 0) {
>> +                afReset(context);
>> +            } else
>> +                ignoreCounter_--;
>> +        } else
>> +            ignoreCounter_ = ignoreFrame_;
>> +    } else {
>> +        if (ignoreCounter_ != 0)
>> +            ignoreCounter_--;
>> +        else {
>> +            afCoarseScan(context);
>> +            afFineScan(context);
>> +        }
>> +    }
>> +}
>> +
>> +} /* 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..5654a964
>> --- /dev/null
>> +++ b/src/ipa/ipu3/algorithms/af.h
>> @@ -0,0 +1,79 @@
>> +/* 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"
>> +
>> +/* Definitions from repo of chromium */
>> +#define AF_MIN_GRID_WIDTH 16
>> +#define AF_MIN_GRID_HEIGHT 16
>> +#define AF_MAX_GRID_WIDTH 32
>> +#define AF_MAX_GRID_HEIGHT 24
>> +#define AF_MIN_BLOCK_WIDTH 4
>> +#define AF_MIN_BLOCK_HEIGHT 3
>> +#define AF_MAX_BLOCK_WIDTH 6
>> +#define AF_MAX_BLOCK_HEIGHT 6
>> +#define AF_DEFAULT_HEIGHT_PER_SLICE 2

Those would better be static constexpr ?

>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::ipu3::algorithms {
>> +
>> +class Af : public Algorithm
>> +{
>> +    /* The format of y_table. From ipu3-ipa repo */
>> +    typedef struct __attribute__((packed)) 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:
>> +    void afCoarseScan(IPAContext &context);
>> +    void afFineScan(IPAContext &context);
>> +    bool afScan(IPAContext &context, int min_step);
>> +    void afReset(IPAContext &context);
>> +
>> +    /* Used for focus scan. */
>> +    uint32_t focus_;
>> +    /* Focus good */
>> +    uint32_t goodFocus_;
>> +    /* Recent AF statistic variance. */
>> +    double currentVariance_;
>> +    /* The frames to be ignore before starting measuring. */
>> +    uint32_t ignoreCounter_;
>> +    /* previous variance. it is used to determine the gradient */
>> +    double previousVariance_;
>> +    /* Max scan steps of each pass of AF scaning */
>> +    uint32_t maxStep_;
>> +    /* coarse scan stable. Complete low pass search (coarse) scan) */
>> +    bool coarseComplete_;
>> +    /* fine scan stable. Complete high pass scan (fine scan) */
>> +    bool fineComplete_;
>> +    /* Raw buffer length */
>> +    uint32_t afRawBufferLen_;
>> +};
>> +
>> +} /* namespace ipa::ipu3::algorithms */
>> +
>> +} /* namespace libcamera */
>> +
>> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_AF_H__ */
>> diff --git a/src/ipa/ipu3/algorithms/meson.build 
>> b/src/ipa/ipu3/algorithms/meson.build
>> index 4db6ae1d..e1099169 100644
>> --- a/src/ipa/ipu3/algorithms/meson.build
>> +++ b/src/ipa/ipu3/algorithms/meson.build
>> @@ -1,8 +1,9 @@
>>   # SPDX-License-Identifier: CC0-1.0
>>   ipu3_ipa_algorithms = files([
>> +    'af.cpp',
>>       'agc.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 86794ac1..ecb2b90a 100644
>> --- a/src/ipa/ipu3/ipa_context.cpp
>> +++ b/src/ipa/ipu3/ipa_context.cpp
>> @@ -67,6 +67,30 @@ namespace libcamera::ipa::ipu3 {
>>    *
>>    * \var IPASessionConfiguration::grid.stride
>>    * \brief Number of cells on one line including the ImgU padding
>> + *
>> + */
>> +
>> +/**
>> + * \var IPASessionConfiguration::af
>> + * \brief AF grid configuration of the IPA
>> + *
>> + * \var IPASessionConfiguration::af.afGrid
>> + *
>> + */
>> +
>> +/**
>> + * \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?
>>    */
>>   /**
>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>> index c6dc0814..e643d38f 100644
>> --- a/src/ipa/ipu3/ipa_context.h
>> +++ b/src/ipa/ipu3/ipa_context.h
>> @@ -31,6 +31,10 @@ struct IPASessionConfiguration {
>>           double minAnalogueGain;
>>           double maxAnalogueGain;
>>       } agc;
>> +
>> +    struct {
>> +        ipu3_uapi_grid_config afGrid;
>> +    } af;

Alphabetical order (af should be before agc).

>>   };
>>   struct IPAFrameContext {
>> @@ -49,6 +53,12 @@ struct IPAFrameContext {
>>           double temperatureK;
>>       } awb;

Add a blank line.

>> +    struct {
>> +        uint32_t focus;
>> +        double maxVariance;
>> +        bool stable;
>> +    } af;

Alphabetical order.

>> +
>>       struct {
>>           uint32_t exposure;
>>           double gain;
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index 3d307708..b5149bcd 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"
>> @@ -157,6 +158,7 @@ private:
>>       void setControls(unsigned int frame);
>>       void calculateBdsGrid(const Size &bdsOutputSize);
>> +    void initAfGrid(const Size &bdsOutputSize);
>>       std::map<unsigned int, MappedFrameBuffer> buffers_;
>> @@ -294,6 +296,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>()); 
>>
>> @@ -395,6 +398,33 @@ void IPAIPU3::calculateBdsGrid(const Size 
>> &bdsOutputSize)
>>                   << (int)bdsGrid.height << " << " << 
>> (int)bdsGrid.block_height_log2 << ")";
>>   }
>> +/**
>> + * \brief Configure the IPU3 AF grid
>> + *
>> + * This function gives the default values for the AF grid configuration.
>> + * All the parameters are set to the minimum acceptable values.
>> + *
>> + * \param bdsOutputSize The bsd output size
s/bsd/Bayer Down Scaler/

>> + */
>> +void IPAIPU3::initAfGrid(const Size &bdsOutputSize)
>> +{
>> +    struct ipu3_uapi_grid_config &grid = 
>> context_.configuration.af.afGrid;
>> +    grid.width = AF_MIN_GRID_WIDTH;
>> +    grid.height = AF_MIN_GRID_HEIGHT;
>> +    grid.block_width_log2 = AF_MIN_BLOCK_WIDTH;
>> +    grid.block_height_log2 = AF_MIN_BLOCK_HEIGHT;
>> +    grid.height_per_slice = AF_DEFAULT_HEIGHT_PER_SLICE;
>> +    /* x_start and y start are default to BDS center */
>> +    grid.x_start = (bdsOutputSize.width / 2) -
>> +               (((grid.width << grid.block_width_log2) / 2));
>> +    grid.y_start = (bdsOutputSize.height / 2) -
>> +               (((grid.height << grid.block_height_log2) / 2));
>> +    /* make sure x_start is multiplied by 10 and y_start is 
>> multiplied by 2 */
>> +    grid.x_start = (grid.x_start / 10) * 10;
>> +    grid.y_start = (grid.y_start / 2) * 2;

I am not sure to understand why you are doing this ?
We nned to be sure x_start is at least 10 pixels from the left of the 
frame, and y_start needs to be 2 pixels from the top of the frame. I am 
not sure we need to have them as multiples of 10 (and 2) ?

>> +    grid.y_start = grid.y_start | IPU3_UAPI_GRID_Y_START_EN;
>> +}
>> +
>>   /**
>>    * \brief Configure the IPU3 IPA
>>    * \param[in] configInfo The IPA configuration data, received from 
>> the pipeline
>> @@ -461,6 +491,8 @@ int IPAIPU3::configure(const IPAConfigInfo 
>> &configInfo,
>>       lineDuration_ = sensorInfo_.lineLength * 1.0s / 
>> sensorInfo_.pixelRate;
>> +    initAfGrid(configInfo.bdsOutputSize);
>> +
>>       /* Update the camera controls using the new sensor settings. */
>>       updateControls(sensorInfo_, ctrls_, ipaControls);


More information about the libcamera-devel mailing list