[{"id":18753,"web_url":"https://patchwork.libcamera.org/comment/18753/","msgid":"<00b7205a-feea-8cac-1384-31e4177d5742@ideasonboard.com>","date":"2021-08-13T08:40:09","subject":"Re: [libcamera-devel] [PATCH v2 03/10] ipa: ipu3: Introduce modular\n\tgrid algorithm","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi JM,\n\n\nOn 12/08/2021 17:52, Jean-Michel Hautbois wrote:\n> Implement a new modular framework for algorithms with a common context\n> structure that is passed to each algorithm through a common API.\n> \n> The initial algorithm is chosen to configure the Bayer Down Scaler grid\n> which is moved from the IPAIPU3 class.\n> \n> This patch:\n> - adds a configure() function to the Algorithm interface\n> - creates a new Grid class implementing the computation at configure\n>   call\n> - removes all the local references from IPAIPU3\n> - implements the list of pointers and the loop at configure call on each\n>   algorithm (right now, only Grid is in the list)\n> - removes the imguCssAwbDefaults structure as it is now configured\n>   properly\n> \n\nA busy patch, but I think in this instance it is reasonable to do all of\nthat in a single patch.\n\n\n\n> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> ---\n>  src/ipa/ipu3/algorithms/algorithm.h | 11 ++++\n>  src/ipa/ipu3/algorithms/grid.cpp    | 83 +++++++++++++++++++++++++++++\n>  src/ipa/ipu3/algorithms/grid.h      | 29 ++++++++++\n>  src/ipa/ipu3/algorithms/meson.build |  1 +\n>  src/ipa/ipu3/ipa_context.h          |  8 +++\n>  src/ipa/ipu3/ipu3.cpp               | 75 ++++++--------------------\n>  src/ipa/ipu3/ipu3_awb.cpp           | 27 ++--------\n>  7 files changed, 154 insertions(+), 80 deletions(-)\n>  create mode 100644 src/ipa/ipu3/algorithms/grid.cpp\n>  create mode 100644 src/ipa/ipu3/algorithms/grid.h\n> \n> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h\n> index 072f01c4..c1b37276 100644\n> --- a/src/ipa/ipu3/algorithms/algorithm.h\n> +++ b/src/ipa/ipu3/algorithms/algorithm.h\n> @@ -7,6 +7,12 @@\n>  #ifndef __LIBCAMERA_IPA_IPU3_ALGORITHM_H__\n>  #define __LIBCAMERA_IPA_IPU3_ALGORITHM_H__\n>  \n> +#include <iostream>\n> +\n> +#include <libcamera/ipa/ipu3_ipa_interface.h>\n> +\n> +#include \"ipa_context.h\"\n> +\n>  namespace libcamera {\n>  \n>  namespace ipa::ipu3 {\n> @@ -15,6 +21,11 @@ class Algorithm\n>  {\n>  public:\n>  \tvirtual ~Algorithm() {}\n> +\n> +\tvirtual int configure([[maybe_unused]] IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)\n> +\t{\n> +\t\treturn 0;\n> +\t}\n>  };\n>  \n>  } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/algorithms/grid.cpp b/src/ipa/ipu3/algorithms/grid.cpp\n> new file mode 100644\n> index 00000000..3578f41b\n> --- /dev/null\n> +++ b/src/ipa/ipu3/algorithms/grid.cpp\n> @@ -0,0 +1,83 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * grid.cpp - IPU3 grid configuration\n> + */\n> +\n> +#include \"grid.h\"\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::ipu3::algorithms {\n> +\n> +LOG_DEFINE_CATEGORY(IPU3Grid)\n> +\n> +/* Maximum number of cells on a row */\n> +static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> +/* Maximum number of cells on a column */\n> +static constexpr uint32_t kMaxCellHeightPerSet = 56;\n> +\n> +/**\n> + * This function calculates a grid for the AWB algorithm in the IPU3 firmware.\n> + * Its input is the BDS output size calculated in the ImgU.\n> + * It is limited for now to the simplest method: find the lesser error\n> + * with the width/height and respective log2 width/height of the cells.\n> + *\n> + * \\todo The frame is divided into cells which can be 8x8 => 128x128.\n> + * As a smaller cell improves the algorithm precision, adapting the\n> + * x_start and y_start parameters of the grid would provoke a loss of\n> + * some pixels but would also result in more accurate algorithms.\n> + */\n> +int Grid::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> +{\n> +\tuint32_t minError = std::numeric_limits<uint32_t>::max();\n> +\tSize best;\n> +\tSize bestLog2;\n> +\tipu3_uapi_grid_config &bdsGrid = context.configuration.grid.bdsGrid;\n> +\n> +\tcontext.configuration.grid.bdsOutputSize = configInfo.bdsOutputSize;\n> +\tSize &bdsOutputSize = context.configuration.grid.bdsOutputSize;\n> +\n> +\tbdsGrid.x_start = 0;\n> +\tbdsGrid.y_start = 0;\n> +\n> +\tfor (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) {\n> +\t\tuint32_t width = std::min(kMaxCellWidthPerSet,\n> +\t\t\t\t\t  bdsOutputSize.width >> widthShift);\n> +\t\twidth = width << widthShift;\n> +\t\tfor (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) {\n> +\t\t\tint32_t height = std::min(kMaxCellHeightPerSet,\n> +\t\t\t\t\t\t  bdsOutputSize.height >> heightShift);\n> +\t\t\theight = height << heightShift;\n> +\t\t\tuint32_t error = std::abs(static_cast<int>(width - bdsOutputSize.width))\n> +\t\t\t\t       + std::abs(static_cast<int>(height - bdsOutputSize.height));\n> +\n> +\t\t\tif (error > minError)\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tminError = error;\n> +\t\t\tbest.width = width;\n> +\t\t\tbest.height = height;\n> +\t\t\tbestLog2.width = widthShift;\n> +\t\t\tbestLog2.height = heightShift;\n> +\t\t}\n> +\t}\n> +\n> +\tbdsGrid.width = best.width >> bestLog2.width;\n> +\tbdsGrid.block_width_log2 = bestLog2.width;\n> +\tbdsGrid.height = best.height >> bestLog2.height;\n> +\tbdsGrid.block_height_log2 = bestLog2.height;\n> +\n> +\tLOG(IPU3Grid, Debug) << \"Best grid found is: (\"\n> +\t\t\t     << (int)bdsGrid.width << \" << \" << (int)bdsGrid.block_width_log2 << \") x (\"\n> +\t\t\t     << (int)bdsGrid.height << \" << \" << (int)bdsGrid.block_height_log2 << \")\";\n> +\n> +\treturn 0;\n> +}\n> +\n> +} /* namespace ipa::ipu3::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/algorithms/grid.h b/src/ipa/ipu3/algorithms/grid.h\n> new file mode 100644\n> index 00000000..b4a51b42\n> --- /dev/null\n> +++ b/src/ipa/ipu3/algorithms/grid.h\n> @@ -0,0 +1,29 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google\n> + *\n> + * grid.h - IPU3 grid configuration\n> + */\n> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_GRID_H__\n> +#define __LIBCAMERA_IPU3_ALGORITHMS_GRID_H__\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::ipu3::algorithms {\n> +\n> +class Grid : public Algorithm\n> +{\n> +public:\n> +\t~Grid() = default;\n> +\n> +\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> +};\n> +\n> +} /* namespace ipa::ipu3::algorithms */\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_CONTRAST_H__ */\n> +\n> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n> index 67148333..3fb3ce56 100644\n> --- a/src/ipa/ipu3/algorithms/meson.build\n> +++ b/src/ipa/ipu3/algorithms/meson.build\n> @@ -1,4 +1,5 @@\n>  # SPDX-License-Identifier: CC0-1.0\n>  \n>  ipu3_ipa_algorithms = files([\n> +        'grid.cpp',\n>  ])\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 0a197a41..90b2f2c2 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -11,12 +11,20 @@\n>  \n>  #include <linux/intel-ipu3.h>\n>  \n> +#include <libcamera/geometry.h>\n> +\n>  namespace libcamera {\n>  \n>  namespace ipa::ipu3 {\n>  \n>  /* Fixed configuration of the IPA */\n>  struct IPAConfiguration {\n> +\tstruct Grid {\n> +\t\t/* Bayer Down Scaler grid plane config used by the kernel */\n> +\t\tipu3_uapi_grid_config bdsGrid;\n\nI always find structures with line comments above better with a blank\nline in-between, otherwise they 'merge together' in my eyes. But that's\njust personal preference, so not an issue.\n\n\n> +\t\t/* BDS output size configured by the pipeline handler */\n> +\t\tSize bdsOutputSize;\n> +\t} grid;\n>  };\n>  \n>  /*\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index c34fa460..ef7fec86 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -29,13 +29,12 @@\n>  \n>  #include \"libcamera/internal/mapped_framebuffer.h\"\n>  \n> +#include \"algorithms/algorithm.h\"\n> +#include \"algorithms/grid.h\"\n>  #include \"ipu3_agc.h\"\n>  #include \"ipu3_awb.h\"\n>  #include \"libipa/camera_sensor_helper.h\"\n>  \n> -static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> -static constexpr uint32_t kMaxCellHeightPerSet = 56;\n> -\n>  namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(IPAIPU3)\n> @@ -91,10 +90,12 @@ private:\n>  \t/* Interface to the Camera Helper */\n>  \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n>  \n> +\t/* Maintain the algorithms used by the IPA */\n> +\tstd::list<std::unique_ptr<ipa::ipu3::Algorithm>> algorithms_;\n> +\n>  \t/* Local parameter storage */\n> +\tstruct IPAContext context_;\n>  \tstruct ipu3_uapi_params params_;\n> -\n> -\tstruct ipu3_uapi_grid_config bdsGrid_;\n>  };\n>  \n>  /**\n> @@ -164,6 +165,8 @@ int IPAIPU3::init(const IPASettings &settings,\n>  \t\t\t\t\t\t\t       frameDurations[2]);\n>  \n>  \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\n\nI think a new line here is deserved though, Adding algorithms to the\nlist, is unrelated to initialising the controls - so they should be\nseparated with a blank line.\n\n\nTwo trivial blank lines is the most I can say ... so I think that\nsatisfies a:\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> +\t/* Construct our Algorithms */\n> +\talgorithms_.emplace_back(new algorithms::Grid());\n>  \n>  \treturn 0;\n>  }\n> @@ -175,56 +178,6 @@ int IPAIPU3::start()\n>  \treturn 0;\n>  }\n>  \n> -/**\n> - * This function calculates a grid for the AWB algorithm in the IPU3 firmware.\n> - * Its input is the BDS output size calculated in the ImgU.\n> - * It is limited for now to the simplest method: find the lesser error\n> - * with the width/height and respective log2 width/height of the cells.\n> - *\n> - * \\todo The frame is divided into cells which can be 8x8 => 128x128.\n> - * As a smaller cell improves the algorithm precision, adapting the\n> - * x_start and y_start parameters of the grid would provoke a loss of\n> - * some pixels but would also result in more accurate algorithms.\n> - */\n> -void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> -{\n> -\tuint32_t minError = std::numeric_limits<uint32_t>::max();\n> -\tSize best;\n> -\tSize bestLog2;\n> -\tbdsGrid_ = {};\n> -\n> -\tfor (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) {\n> -\t\tuint32_t width = std::min(kMaxCellWidthPerSet,\n> -\t\t\t\t\t  bdsOutputSize.width >> widthShift);\n> -\t\twidth = width << widthShift;\n> -\t\tfor (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) {\n> -\t\t\tint32_t height = std::min(kMaxCellHeightPerSet,\n> -\t\t\t\t\t\t  bdsOutputSize.height >> heightShift);\n> -\t\t\theight = height << heightShift;\n> -\t\t\tuint32_t error  = std::abs(static_cast<int>(width - bdsOutputSize.width))\n> -\t\t\t\t\t\t\t+ std::abs(static_cast<int>(height - bdsOutputSize.height));\n> -\n> -\t\t\tif (error > minError)\n> -\t\t\t\tcontinue;\n> -\n> -\t\t\tminError = error;\n> -\t\t\tbest.width = width;\n> -\t\t\tbest.height = height;\n> -\t\t\tbestLog2.width = widthShift;\n> -\t\t\tbestLog2.height = heightShift;\n> -\t\t}\n> -\t}\n> -\n> -\tbdsGrid_.width = best.width >> bestLog2.width;\n> -\tbdsGrid_.block_width_log2 = bestLog2.width;\n> -\tbdsGrid_.height = best.height >> bestLog2.height;\n> -\tbdsGrid_.block_height_log2 = bestLog2.height;\n> -\n> -\tLOG(IPAIPU3, Debug) << \"Best grid found is: (\"\n> -\t\t\t    << (int)bdsGrid_.width << \" << \" << (int)bdsGrid_.block_width_log2 << \") x (\"\n> -\t\t\t    << (int)bdsGrid_.height << \" << \" << (int)bdsGrid_.block_height_log2 << \")\";\n> -}\n> -\n>  int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n>  {\n>  \tif (configInfo.entityControls.empty()) {\n> @@ -264,15 +217,21 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n>  \n>  \tdefVBlank_ = itVBlank->second.def().get<int32_t>();\n>  \n> +\t/* Clean context and IPU3 parameters at configuration */\n>  \tparams_ = {};\n> +\tcontext_ = {};\n>  \n> -\tcalculateBdsGrid(configInfo.bdsOutputSize);\n> +\tfor (auto const &algo : algorithms_) {\n> +\t\tint ret = algo->configure(context_, configInfo);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n>  \n>  \tawbAlgo_ = std::make_unique<IPU3Awb>();\n> -\tawbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);\n> +\tawbAlgo_->initialise(params_, context_.configuration.grid.bdsOutputSize, context_.configuration.grid.bdsGrid);\n>  \n>  \tagcAlgo_ = std::make_unique<IPU3Agc>();\n> -\tagcAlgo_->initialise(bdsGrid_, sensorInfo_);\n> +\tagcAlgo_->initialise(context_.configuration.grid.bdsGrid, sensorInfo_);\n>  \n>  \treturn 0;\n>  }\n> diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp\n> index 4bb321b3..4ee5ee6f 100644\n> --- a/src/ipa/ipu3/ipu3_awb.cpp\n> +++ b/src/ipa/ipu3/ipu3_awb.cpp\n> @@ -107,25 +107,6 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {\n>  \t.opt_center_sqr = { 419904, 133956 },\n>  };\n>  \n> -/* Default settings for Auto White Balance replicated from the Kernel*/\n> -static const struct ipu3_uapi_awb_config_s imguCssAwbDefaults = {\n> -\t.rgbs_thr_gr = 8191,\n> -\t.rgbs_thr_r = 8191,\n> -\t.rgbs_thr_gb = 8191,\n> -\t.rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT,\n> -\t.grid = {\n> -\t\t.width = 160,\n> -\t\t.height = 36,\n> -\t\t.block_width_log2 = 3,\n> -\t\t.block_height_log2 = 4,\n> -\t\t.height_per_slice = 1, /* Overridden by kernel. */\n> -\t\t.x_start = 0,\n> -\t\t.y_start = 0,\n> -\t\t.x_end = 0,\n> -\t\t.y_end = 0,\n> -\t},\n> -};\n> -\n>  /* Default color correction matrix defined as an identity matrix */\n>  static const struct ipu3_uapi_ccm_mat_config imguCssCcmDefault = {\n>  \t8191, 0, 0, 0,\n> @@ -174,10 +155,12 @@ IPU3Awb::~IPU3Awb()\n>  void IPU3Awb::initialise(ipu3_uapi_params &params, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid)\n>  {\n>  \tparams.use.acc_awb = 1;\n> -\tparams.acc_param.awb.config = imguCssAwbDefaults;\n> -\n> +\tparams.acc_param.awb.config.rgbs_thr_gr = 8191;\n> +\tparams.acc_param.awb.config.rgbs_thr_r = 8191;\n> +\tparams.acc_param.awb.config.rgbs_thr_gb = 8191;\n> +\tparams.acc_param.awb.config.rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT;\n> +\tparams.acc_param.awb.config.grid = bdsGrid;\n>  \tawbGrid_ = bdsGrid;\n> -\tparams.acc_param.awb.config.grid = awbGrid_;\n>  \n>  \tparams.use.acc_bnr = 1;\n>  \tparams.acc_param.bnr = imguCssBnrDefaults;\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7943BBD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 13 Aug 2021 08:40:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CCD8668892;\n\tFri, 13 Aug 2021 10:40:13 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D60860263\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 13 Aug 2021 10:40:12 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 22EBFEE;\n\tFri, 13 Aug 2021 10:40:12 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"twzK4lUX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628844012;\n\tbh=VtVh00CJdoorq7csLH61n5efjM9Ild62WgFqa8Eic00=;\n\th=To:References:From:Subject:Date:In-Reply-To:From;\n\tb=twzK4lUXxYqJtlkVVMSozkuGaRDbAUTjBcCd/T3cZNZK6ObQn2N/v3IcA2XxKSxmz\n\to1izGHKG7j8SZwdeaNOTj1L2RdJmcD+WUocS/ab13/I3cGDjjF6BgM6VYy1kYTXrDo\n\twycnBlQLVL4t5gbfhYjm58sTDPCcWKQKSjlQlnlA=","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210812165243.276977-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210812165243.276977-4-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<00b7205a-feea-8cac-1384-31e4177d5742@ideasonboard.com>","Date":"Fri, 13 Aug 2021 09:40:09 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210812165243.276977-4-jeanmichel.hautbois@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 03/10] ipa: ipu3: Introduce modular\n\tgrid algorithm","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18804,"web_url":"https://patchwork.libcamera.org/comment/18804/","msgid":"<YRlVwwDer7zL07Hi@pendragon.ideasonboard.com>","date":"2021-08-15T17:58:27","subject":"Re: [libcamera-devel] [PATCH v2 03/10] ipa: ipu3: Introduce modular\n\tgrid algorithm","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Fri, Aug 13, 2021 at 09:40:09AM +0100, Kieran Bingham wrote:\n> On 12/08/2021 17:52, Jean-Michel Hautbois wrote:\n> > Implement a new modular framework for algorithms with a common context\n> > structure that is passed to each algorithm through a common API.\n> > \n> > The initial algorithm is chosen to configure the Bayer Down Scaler grid\n> > which is moved from the IPAIPU3 class.\n> > \n> > This patch:\n> > - adds a configure() function to the Algorithm interface\n> > - creates a new Grid class implementing the computation at configure\n> >   call\n> > - removes all the local references from IPAIPU3\n> > - implements the list of pointers and the loop at configure call on each\n> >   algorithm (right now, only Grid is in the list)\n> > - removes the imguCssAwbDefaults structure as it is now configured\n> >   properly\n> \n> A busy patch, but I think in this instance it is reasonable to do all of\n> that in a single patch.\n\nI'm sure Niklas would have disagreed ;-) I tend to like splitting things\nup too, possibly not as much as he does though.\n\n> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> > ---\n> >  src/ipa/ipu3/algorithms/algorithm.h | 11 ++++\n> >  src/ipa/ipu3/algorithms/grid.cpp    | 83 +++++++++++++++++++++++++++++\n> >  src/ipa/ipu3/algorithms/grid.h      | 29 ++++++++++\n> >  src/ipa/ipu3/algorithms/meson.build |  1 +\n> >  src/ipa/ipu3/ipa_context.h          |  8 +++\n> >  src/ipa/ipu3/ipu3.cpp               | 75 ++++++--------------------\n> >  src/ipa/ipu3/ipu3_awb.cpp           | 27 ++--------\n> >  7 files changed, 154 insertions(+), 80 deletions(-)\n> >  create mode 100644 src/ipa/ipu3/algorithms/grid.cpp\n> >  create mode 100644 src/ipa/ipu3/algorithms/grid.h\n> > \n> > diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h\n> > index 072f01c4..c1b37276 100644\n> > --- a/src/ipa/ipu3/algorithms/algorithm.h\n> > +++ b/src/ipa/ipu3/algorithms/algorithm.h\n> > @@ -7,6 +7,12 @@\n> >  #ifndef __LIBCAMERA_IPA_IPU3_ALGORITHM_H__\n> >  #define __LIBCAMERA_IPA_IPU3_ALGORITHM_H__\n> >  \n> > +#include <iostream>\n> > +\n> > +#include <libcamera/ipa/ipu3_ipa_interface.h>\n> > +\n> > +#include \"ipa_context.h\"\n> > +\n> >  namespace libcamera {\n> >  \n> >  namespace ipa::ipu3 {\n> > @@ -15,6 +21,11 @@ class Algorithm\n> >  {\n> >  public:\n> >  \tvirtual ~Algorithm() {}\n> > +\n> > +\tvirtual int configure([[maybe_unused]] IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)\n\nLine wrap.\n\n> > +\t{\n> > +\t\treturn 0;\n> > +\t}\n> >  };\n> >  \n> >  } /* namespace ipa::ipu3 */\n> > diff --git a/src/ipa/ipu3/algorithms/grid.cpp b/src/ipa/ipu3/algorithms/grid.cpp\n> > new file mode 100644\n> > index 00000000..3578f41b\n> > --- /dev/null\n> > +++ b/src/ipa/ipu3/algorithms/grid.cpp\n> > @@ -0,0 +1,83 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Ideas On Board\n> > + *\n> > + * grid.cpp - IPU3 grid configuration\n> > + */\n> > +\n> > +#include \"grid.h\"\n> > +\n> > +#include <libcamera/base/log.h>\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::ipu3::algorithms {\n> > +\n> > +LOG_DEFINE_CATEGORY(IPU3Grid)\n> > +\n> > +/* Maximum number of cells on a row */\n> > +static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> > +/* Maximum number of cells on a column */\n> > +static constexpr uint32_t kMaxCellHeightPerSet = 56;\n> > +\n> > +/**\n> > + * This function calculates a grid for the AWB algorithm in the IPU3 firmware.\n> > + * Its input is the BDS output size calculated in the ImgU.\n> > + * It is limited for now to the simplest method: find the lesser error\n> > + * with the width/height and respective log2 width/height of the cells.\n> > + *\n> > + * \\todo The frame is divided into cells which can be 8x8 => 128x128.\n> > + * As a smaller cell improves the algorithm precision, adapting the\n> > + * x_start and y_start parameters of the grid would provoke a loss of\n> > + * some pixels but would also result in more accurate algorithms.\n> > + */\n> > +int Grid::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n\nI don't think this qualifies as an \"algorithm\" I'm afraid. I'd keep it\nin the IPU3IPA class.\n\n> > +{\n> > +\tuint32_t minError = std::numeric_limits<uint32_t>::max();\n> > +\tSize best;\n> > +\tSize bestLog2;\n> > +\tipu3_uapi_grid_config &bdsGrid = context.configuration.grid.bdsGrid;\n> > +\n> > +\tcontext.configuration.grid.bdsOutputSize = configInfo.bdsOutputSize;\n> > +\tSize &bdsOutputSize = context.configuration.grid.bdsOutputSize;\n> > +\n> > +\tbdsGrid.x_start = 0;\n> > +\tbdsGrid.y_start = 0;\n> > +\n> > +\tfor (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) {\n> > +\t\tuint32_t width = std::min(kMaxCellWidthPerSet,\n> > +\t\t\t\t\t  bdsOutputSize.width >> widthShift);\n> > +\t\twidth = width << widthShift;\n> > +\t\tfor (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) {\n> > +\t\t\tint32_t height = std::min(kMaxCellHeightPerSet,\n> > +\t\t\t\t\t\t  bdsOutputSize.height >> heightShift);\n> > +\t\t\theight = height << heightShift;\n> > +\t\t\tuint32_t error = std::abs(static_cast<int>(width - bdsOutputSize.width))\n> > +\t\t\t\t       + std::abs(static_cast<int>(height - bdsOutputSize.height));\n> > +\n> > +\t\t\tif (error > minError)\n> > +\t\t\t\tcontinue;\n> > +\n> > +\t\t\tminError = error;\n> > +\t\t\tbest.width = width;\n> > +\t\t\tbest.height = height;\n> > +\t\t\tbestLog2.width = widthShift;\n> > +\t\t\tbestLog2.height = heightShift;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tbdsGrid.width = best.width >> bestLog2.width;\n> > +\tbdsGrid.block_width_log2 = bestLog2.width;\n> > +\tbdsGrid.height = best.height >> bestLog2.height;\n> > +\tbdsGrid.block_height_log2 = bestLog2.height;\n> > +\n> > +\tLOG(IPU3Grid, Debug) << \"Best grid found is: (\"\n> > +\t\t\t     << (int)bdsGrid.width << \" << \" << (int)bdsGrid.block_width_log2 << \") x (\"\n> > +\t\t\t     << (int)bdsGrid.height << \" << \" << (int)bdsGrid.block_height_log2 << \")\";\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +} /* namespace ipa::ipu3::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/ipu3/algorithms/grid.h b/src/ipa/ipu3/algorithms/grid.h\n> > new file mode 100644\n> > index 00000000..b4a51b42\n> > --- /dev/null\n> > +++ b/src/ipa/ipu3/algorithms/grid.h\n> > @@ -0,0 +1,29 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Google\n> > + *\n> > + * grid.h - IPU3 grid configuration\n> > + */\n> > +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_GRID_H__\n> > +#define __LIBCAMERA_IPU3_ALGORITHMS_GRID_H__\n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::ipu3::algorithms {\n> > +\n> > +class Grid : public Algorithm\n> > +{\n> > +public:\n> > +\t~Grid() = default;\n> > +\n> > +\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> > +};\n> > +\n> > +} /* namespace ipa::ipu3::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_CONTRAST_H__ */\n> > +\n> > diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n> > index 67148333..3fb3ce56 100644\n> > --- a/src/ipa/ipu3/algorithms/meson.build\n> > +++ b/src/ipa/ipu3/algorithms/meson.build\n> > @@ -1,4 +1,5 @@\n> >  # SPDX-License-Identifier: CC0-1.0\n> >  \n> >  ipu3_ipa_algorithms = files([\n> > +        'grid.cpp',\n> >  ])\n> > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > index 0a197a41..90b2f2c2 100644\n> > --- a/src/ipa/ipu3/ipa_context.h\n> > +++ b/src/ipa/ipu3/ipa_context.h\n> > @@ -11,12 +11,20 @@\n> >  \n> >  #include <linux/intel-ipu3.h>\n> >  \n> > +#include <libcamera/geometry.h>\n> > +\n> >  namespace libcamera {\n> >  \n> >  namespace ipa::ipu3 {\n> >  \n> >  /* Fixed configuration of the IPA */\n> >  struct IPAConfiguration {\n> > +\tstruct Grid {\n> > +\t\t/* Bayer Down Scaler grid plane config used by the kernel */\n> > +\t\tipu3_uapi_grid_config bdsGrid;\n> \n> I always find structures with line comments above better with a blank\n> line in-between, otherwise they 'merge together' in my eyes. But that's\n> just personal preference, so not an issue.\n\nAs this is meant to be a reference implementation, documentation is\nfairly important, and I'd prefer if we used Doxygen formatting. It thus\nbrings the question of whether we want to keep it here or move it to a\n.cpp file.\n\n> > +\t\t/* BDS output size configured by the pipeline handler */\n> > +\t\tSize bdsOutputSize;\n> > +\t} grid;\n> >  };\n> >  \n> >  /*\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index c34fa460..ef7fec86 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -29,13 +29,12 @@\n> >  \n> >  #include \"libcamera/internal/mapped_framebuffer.h\"\n> >  \n> > +#include \"algorithms/algorithm.h\"\n> > +#include \"algorithms/grid.h\"\n> >  #include \"ipu3_agc.h\"\n> >  #include \"ipu3_awb.h\"\n> >  #include \"libipa/camera_sensor_helper.h\"\n> >  \n> > -static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> > -static constexpr uint32_t kMaxCellHeightPerSet = 56;\n> > -\n> >  namespace libcamera {\n> >  \n> >  LOG_DEFINE_CATEGORY(IPAIPU3)\n> > @@ -91,10 +90,12 @@ private:\n> >  \t/* Interface to the Camera Helper */\n> >  \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n> >  \n> > +\t/* Maintain the algorithms used by the IPA */\n> > +\tstd::list<std::unique_ptr<ipa::ipu3::Algorithm>> algorithms_;\n> > +\n> >  \t/* Local parameter storage */\n> > +\tstruct IPAContext context_;\n> >  \tstruct ipu3_uapi_params params_;\n> > -\n> > -\tstruct ipu3_uapi_grid_config bdsGrid_;\n> >  };\n> >  \n> >  /**\n> > @@ -164,6 +165,8 @@ int IPAIPU3::init(const IPASettings &settings,\n> >  \t\t\t\t\t\t\t       frameDurations[2]);\n> >  \n> >  \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\n> \n> I think a new line here is deserved though, Adding algorithms to the\n> list, is unrelated to initialising the controls - so they should be\n> separated with a blank line.\n> \n> Two trivial blank lines is the most I can say ... so I think that\n> satisfies a:\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > +\t/* Construct our Algorithms */\n> > +\talgorithms_.emplace_back(new algorithms::Grid());\n> >  \n> >  \treturn 0;\n> >  }\n> > @@ -175,56 +178,6 @@ int IPAIPU3::start()\n> >  \treturn 0;\n> >  }\n> >  \n> > -/**\n> > - * This function calculates a grid for the AWB algorithm in the IPU3 firmware.\n> > - * Its input is the BDS output size calculated in the ImgU.\n> > - * It is limited for now to the simplest method: find the lesser error\n> > - * with the width/height and respective log2 width/height of the cells.\n> > - *\n> > - * \\todo The frame is divided into cells which can be 8x8 => 128x128.\n> > - * As a smaller cell improves the algorithm precision, adapting the\n> > - * x_start and y_start parameters of the grid would provoke a loss of\n> > - * some pixels but would also result in more accurate algorithms.\n> > - */\n> > -void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> > -{\n> > -\tuint32_t minError = std::numeric_limits<uint32_t>::max();\n> > -\tSize best;\n> > -\tSize bestLog2;\n> > -\tbdsGrid_ = {};\n> > -\n> > -\tfor (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) {\n> > -\t\tuint32_t width = std::min(kMaxCellWidthPerSet,\n> > -\t\t\t\t\t  bdsOutputSize.width >> widthShift);\n> > -\t\twidth = width << widthShift;\n> > -\t\tfor (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) {\n> > -\t\t\tint32_t height = std::min(kMaxCellHeightPerSet,\n> > -\t\t\t\t\t\t  bdsOutputSize.height >> heightShift);\n> > -\t\t\theight = height << heightShift;\n> > -\t\t\tuint32_t error  = std::abs(static_cast<int>(width - bdsOutputSize.width))\n> > -\t\t\t\t\t\t\t+ std::abs(static_cast<int>(height - bdsOutputSize.height));\n> > -\n> > -\t\t\tif (error > minError)\n> > -\t\t\t\tcontinue;\n> > -\n> > -\t\t\tminError = error;\n> > -\t\t\tbest.width = width;\n> > -\t\t\tbest.height = height;\n> > -\t\t\tbestLog2.width = widthShift;\n> > -\t\t\tbestLog2.height = heightShift;\n> > -\t\t}\n> > -\t}\n> > -\n> > -\tbdsGrid_.width = best.width >> bestLog2.width;\n> > -\tbdsGrid_.block_width_log2 = bestLog2.width;\n> > -\tbdsGrid_.height = best.height >> bestLog2.height;\n> > -\tbdsGrid_.block_height_log2 = bestLog2.height;\n> > -\n> > -\tLOG(IPAIPU3, Debug) << \"Best grid found is: (\"\n> > -\t\t\t    << (int)bdsGrid_.width << \" << \" << (int)bdsGrid_.block_width_log2 << \") x (\"\n> > -\t\t\t    << (int)bdsGrid_.height << \" << \" << (int)bdsGrid_.block_height_log2 << \")\";\n> > -}\n> > -\n> >  int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> >  {\n> >  \tif (configInfo.entityControls.empty()) {\n> > @@ -264,15 +217,21 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> >  \n> >  \tdefVBlank_ = itVBlank->second.def().get<int32_t>();\n> >  \n> > +\t/* Clean context and IPU3 parameters at configuration */\n> >  \tparams_ = {};\n> > +\tcontext_ = {};\n> >  \n> > -\tcalculateBdsGrid(configInfo.bdsOutputSize);\n> > +\tfor (auto const &algo : algorithms_) {\n> > +\t\tint ret = algo->configure(context_, configInfo);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> >  \n> >  \tawbAlgo_ = std::make_unique<IPU3Awb>();\n> > -\tawbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);\n> > +\tawbAlgo_->initialise(params_, context_.configuration.grid.bdsOutputSize, context_.configuration.grid.bdsGrid);\n> >  \n> >  \tagcAlgo_ = std::make_unique<IPU3Agc>();\n> > -\tagcAlgo_->initialise(bdsGrid_, sensorInfo_);\n> > +\tagcAlgo_->initialise(context_.configuration.grid.bdsGrid, sensorInfo_);\n> >  \n> >  \treturn 0;\n> >  }\n> > diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp\n> > index 4bb321b3..4ee5ee6f 100644\n> > --- a/src/ipa/ipu3/ipu3_awb.cpp\n> > +++ b/src/ipa/ipu3/ipu3_awb.cpp\n> > @@ -107,25 +107,6 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {\n> >  \t.opt_center_sqr = { 419904, 133956 },\n> >  };\n> >  \n> > -/* Default settings for Auto White Balance replicated from the Kernel*/\n> > -static const struct ipu3_uapi_awb_config_s imguCssAwbDefaults = {\n> > -\t.rgbs_thr_gr = 8191,\n> > -\t.rgbs_thr_r = 8191,\n> > -\t.rgbs_thr_gb = 8191,\n> > -\t.rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT,\n> > -\t.grid = {\n> > -\t\t.width = 160,\n> > -\t\t.height = 36,\n> > -\t\t.block_width_log2 = 3,\n> > -\t\t.block_height_log2 = 4,\n> > -\t\t.height_per_slice = 1, /* Overridden by kernel. */\n> > -\t\t.x_start = 0,\n> > -\t\t.y_start = 0,\n> > -\t\t.x_end = 0,\n> > -\t\t.y_end = 0,\n> > -\t},\n> > -};\n> > -\n> >  /* Default color correction matrix defined as an identity matrix */\n> >  static const struct ipu3_uapi_ccm_mat_config imguCssCcmDefault = {\n> >  \t8191, 0, 0, 0,\n> > @@ -174,10 +155,12 @@ IPU3Awb::~IPU3Awb()\n> >  void IPU3Awb::initialise(ipu3_uapi_params &params, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid)\n> >  {\n> >  \tparams.use.acc_awb = 1;\n> > -\tparams.acc_param.awb.config = imguCssAwbDefaults;\n> > -\n> > +\tparams.acc_param.awb.config.rgbs_thr_gr = 8191;\n> > +\tparams.acc_param.awb.config.rgbs_thr_r = 8191;\n> > +\tparams.acc_param.awb.config.rgbs_thr_gb = 8191;\n> > +\tparams.acc_param.awb.config.rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT;\n\nLine wrap here too.\n\n> > +\tparams.acc_param.awb.config.grid = bdsGrid;\n> >  \tawbGrid_ = bdsGrid;\n> > -\tparams.acc_param.awb.config.grid = awbGrid_;\n> >  \n> >  \tparams.use.acc_bnr = 1;\n> >  \tparams.acc_param.bnr = imguCssBnrDefaults;\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6B72ABD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 15 Aug 2021 17:58:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF40368890;\n\tSun, 15 Aug 2021 19:58:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 607F968889\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 15 Aug 2021 19:58:33 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C54EB2C5;\n\tSun, 15 Aug 2021 19:58:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"f3nP84De\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629050313;\n\tbh=WN/EeALbyN5VN1ozap2l3RYPuXzhHZ6fNAIoKJZFGL0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=f3nP84DetmNlUhfxqwwAFEqjV8hp+Erin0n4spH5jIu0mqSFYjxbTk1De9ToEdWIW\n\taSLTIzEUnTmXmEqD31ZR8MqNLwDVlNPEhRrZ8aEJfavPz0zmYYf5uh9L8F10Rh+QXa\n\tmOm96Dq4a51rzNPGiWXRxMQ8h/e3oD5ybR/VxVTw=","Date":"Sun, 15 Aug 2021 20:58:27 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>","Message-ID":"<YRlVwwDer7zL07Hi@pendragon.ideasonboard.com>","References":"<20210812165243.276977-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210812165243.276977-4-jeanmichel.hautbois@ideasonboard.com>\n\t<00b7205a-feea-8cac-1384-31e4177d5742@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<00b7205a-feea-8cac-1384-31e4177d5742@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 03/10] ipa: ipu3: Introduce modular\n\tgrid algorithm","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18806,"web_url":"https://patchwork.libcamera.org/comment/18806/","msgid":"<YRl6SJbmQX4d2D1G@oden.dyn.berto.se>","date":"2021-08-15T20:34:16","subject":"Re: [libcamera-devel] [PATCH v2 03/10] ipa: ipu3: Introduce modular\n\tgrid algorithm","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello JM,\n\nThanks for your work.\n\nOn 2021-08-15 20:58:27 +0300, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> Thank you for the patch.\n> \n> On Fri, Aug 13, 2021 at 09:40:09AM +0100, Kieran Bingham wrote:\n> > On 12/08/2021 17:52, Jean-Michel Hautbois wrote:\n> > > Implement a new modular framework for algorithms with a common context\n> > > structure that is passed to each algorithm through a common API.\n> > > \n> > > The initial algorithm is chosen to configure the Bayer Down Scaler grid\n> > > which is moved from the IPAIPU3 class.\n> > > \n> > > This patch:\n> > > - adds a configure() function to the Algorithm interface\n> > > - creates a new Grid class implementing the computation at configure\n> > >   call\n> > > - removes all the local references from IPAIPU3\n> > > - implements the list of pointers and the loop at configure call on each\n> > >   algorithm (right now, only Grid is in the list)\n> > > - removes the imguCssAwbDefaults structure as it is now configured\n> > >   properly\n> > \n> > A busy patch, but I think in this instance it is reasonable to do all of\n> > that in a single patch.\n> \n> I'm sure Niklas would have disagreed ;-) I tend to like splitting things\n> up too, possibly not as much as he does though.\n\nSo this was the burning sensation I had all day :-)\n\nMaybe the addition of the Grid class could be broken out to a separate \npatch? Over all I like the idea of the algorithms helpers.\n\n> \n> > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> > > ---\n> > >  src/ipa/ipu3/algorithms/algorithm.h | 11 ++++\n> > >  src/ipa/ipu3/algorithms/grid.cpp    | 83 +++++++++++++++++++++++++++++\n> > >  src/ipa/ipu3/algorithms/grid.h      | 29 ++++++++++\n> > >  src/ipa/ipu3/algorithms/meson.build |  1 +\n> > >  src/ipa/ipu3/ipa_context.h          |  8 +++\n> > >  src/ipa/ipu3/ipu3.cpp               | 75 ++++++--------------------\n> > >  src/ipa/ipu3/ipu3_awb.cpp           | 27 ++--------\n> > >  7 files changed, 154 insertions(+), 80 deletions(-)\n> > >  create mode 100644 src/ipa/ipu3/algorithms/grid.cpp\n> > >  create mode 100644 src/ipa/ipu3/algorithms/grid.h\n> > > \n> > > diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h\n> > > index 072f01c4..c1b37276 100644\n> > > --- a/src/ipa/ipu3/algorithms/algorithm.h\n> > > +++ b/src/ipa/ipu3/algorithms/algorithm.h\n> > > @@ -7,6 +7,12 @@\n> > >  #ifndef __LIBCAMERA_IPA_IPU3_ALGORITHM_H__\n> > >  #define __LIBCAMERA_IPA_IPU3_ALGORITHM_H__\n> > >  \n> > > +#include <iostream>\n> > > +\n> > > +#include <libcamera/ipa/ipu3_ipa_interface.h>\n> > > +\n> > > +#include \"ipa_context.h\"\n> > > +\n> > >  namespace libcamera {\n> > >  \n> > >  namespace ipa::ipu3 {\n> > > @@ -15,6 +21,11 @@ class Algorithm\n> > >  {\n> > >  public:\n> > >  \tvirtual ~Algorithm() {}\n> > > +\n> > > +\tvirtual int configure([[maybe_unused]] IPAContext &context, [[maybe_unused]] const IPAConfigInfo &configInfo)\n> \n> Line wrap.\n> \n> > > +\t{\n> > > +\t\treturn 0;\n> > > +\t}\n> > >  };\n> > >  \n> > >  } /* namespace ipa::ipu3 */\n> > > diff --git a/src/ipa/ipu3/algorithms/grid.cpp b/src/ipa/ipu3/algorithms/grid.cpp\n> > > new file mode 100644\n> > > index 00000000..3578f41b\n> > > --- /dev/null\n> > > +++ b/src/ipa/ipu3/algorithms/grid.cpp\n> > > @@ -0,0 +1,83 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2021, Ideas On Board\n> > > + *\n> > > + * grid.cpp - IPU3 grid configuration\n> > > + */\n> > > +\n> > > +#include \"grid.h\"\n> > > +\n> > > +#include <libcamera/base/log.h>\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa::ipu3::algorithms {\n> > > +\n> > > +LOG_DEFINE_CATEGORY(IPU3Grid)\n> > > +\n> > > +/* Maximum number of cells on a row */\n> > > +static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> > > +/* Maximum number of cells on a column */\n> > > +static constexpr uint32_t kMaxCellHeightPerSet = 56;\n> > > +\n> > > +/**\n> > > + * This function calculates a grid for the AWB algorithm in the IPU3 firmware.\n> > > + * Its input is the BDS output size calculated in the ImgU.\n> > > + * It is limited for now to the simplest method: find the lesser error\n> > > + * with the width/height and respective log2 width/height of the cells.\n> > > + *\n> > > + * \\todo The frame is divided into cells which can be 8x8 => 128x128.\n> > > + * As a smaller cell improves the algorithm precision, adapting the\n> > > + * x_start and y_start parameters of the grid would provoke a loss of\n> > > + * some pixels but would also result in more accurate algorithms.\n> > > + */\n> > > +int Grid::configure(IPAContext &context, const IPAConfigInfo &configInfo)\n> \n> I don't think this qualifies as an \"algorithm\" I'm afraid. I'd keep it\n> in the IPU3IPA class.\n> \n> > > +{\n> > > +\tuint32_t minError = std::numeric_limits<uint32_t>::max();\n> > > +\tSize best;\n> > > +\tSize bestLog2;\n> > > +\tipu3_uapi_grid_config &bdsGrid = context.configuration.grid.bdsGrid;\n> > > +\n> > > +\tcontext.configuration.grid.bdsOutputSize = configInfo.bdsOutputSize;\n> > > +\tSize &bdsOutputSize = context.configuration.grid.bdsOutputSize;\n> > > +\n> > > +\tbdsGrid.x_start = 0;\n> > > +\tbdsGrid.y_start = 0;\n> > > +\n> > > +\tfor (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) {\n> > > +\t\tuint32_t width = std::min(kMaxCellWidthPerSet,\n> > > +\t\t\t\t\t  bdsOutputSize.width >> widthShift);\n> > > +\t\twidth = width << widthShift;\n> > > +\t\tfor (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) {\n> > > +\t\t\tint32_t height = std::min(kMaxCellHeightPerSet,\n> > > +\t\t\t\t\t\t  bdsOutputSize.height >> heightShift);\n> > > +\t\t\theight = height << heightShift;\n> > > +\t\t\tuint32_t error = std::abs(static_cast<int>(width - bdsOutputSize.width))\n> > > +\t\t\t\t       + std::abs(static_cast<int>(height - bdsOutputSize.height));\n> > > +\n> > > +\t\t\tif (error > minError)\n> > > +\t\t\t\tcontinue;\n> > > +\n> > > +\t\t\tminError = error;\n> > > +\t\t\tbest.width = width;\n> > > +\t\t\tbest.height = height;\n> > > +\t\t\tbestLog2.width = widthShift;\n> > > +\t\t\tbestLog2.height = heightShift;\n> > > +\t\t}\n> > > +\t}\n> > > +\n> > > +\tbdsGrid.width = best.width >> bestLog2.width;\n> > > +\tbdsGrid.block_width_log2 = bestLog2.width;\n> > > +\tbdsGrid.height = best.height >> bestLog2.height;\n> > > +\tbdsGrid.block_height_log2 = bestLog2.height;\n> > > +\n> > > +\tLOG(IPU3Grid, Debug) << \"Best grid found is: (\"\n> > > +\t\t\t     << (int)bdsGrid.width << \" << \" << (int)bdsGrid.block_width_log2 << \") x (\"\n> > > +\t\t\t     << (int)bdsGrid.height << \" << \" << (int)bdsGrid.block_height_log2 << \")\";\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +} /* namespace ipa::ipu3::algorithms */\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/ipu3/algorithms/grid.h b/src/ipa/ipu3/algorithms/grid.h\n> > > new file mode 100644\n> > > index 00000000..b4a51b42\n> > > --- /dev/null\n> > > +++ b/src/ipa/ipu3/algorithms/grid.h\n> > > @@ -0,0 +1,29 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2021, Google\n> > > + *\n> > > + * grid.h - IPU3 grid configuration\n> > > + */\n> > > +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_GRID_H__\n> > > +#define __LIBCAMERA_IPU3_ALGORITHMS_GRID_H__\n> > > +\n> > > +#include \"algorithm.h\"\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa::ipu3::algorithms {\n> > > +\n> > > +class Grid : public Algorithm\n> > > +{\n> > > +public:\n> > > +\t~Grid() = default;\n> > > +\n> > > +\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> > > +};\n> > > +\n> > > +} /* namespace ipa::ipu3::algorithms */\n> > > +\n> > > +} /* namespace libcamera */\n> > > +\n> > > +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_CONTRAST_H__ */\n> > > +\n> > > diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n> > > index 67148333..3fb3ce56 100644\n> > > --- a/src/ipa/ipu3/algorithms/meson.build\n> > > +++ b/src/ipa/ipu3/algorithms/meson.build\n> > > @@ -1,4 +1,5 @@\n> > >  # SPDX-License-Identifier: CC0-1.0\n> > >  \n> > >  ipu3_ipa_algorithms = files([\n> > > +        'grid.cpp',\n> > >  ])\n> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> > > index 0a197a41..90b2f2c2 100644\n> > > --- a/src/ipa/ipu3/ipa_context.h\n> > > +++ b/src/ipa/ipu3/ipa_context.h\n> > > @@ -11,12 +11,20 @@\n> > >  \n> > >  #include <linux/intel-ipu3.h>\n> > >  \n> > > +#include <libcamera/geometry.h>\n> > > +\n> > >  namespace libcamera {\n> > >  \n> > >  namespace ipa::ipu3 {\n> > >  \n> > >  /* Fixed configuration of the IPA */\n> > >  struct IPAConfiguration {\n> > > +\tstruct Grid {\n> > > +\t\t/* Bayer Down Scaler grid plane config used by the kernel */\n> > > +\t\tipu3_uapi_grid_config bdsGrid;\n> > \n> > I always find structures with line comments above better with a blank\n> > line in-between, otherwise they 'merge together' in my eyes. But that's\n> > just personal preference, so not an issue.\n> \n> As this is meant to be a reference implementation, documentation is\n> fairly important, and I'd prefer if we used Doxygen formatting. It thus\n> brings the question of whether we want to keep it here or move it to a\n> .cpp file.\n> \n> > > +\t\t/* BDS output size configured by the pipeline handler */\n> > > +\t\tSize bdsOutputSize;\n> > > +\t} grid;\n> > >  };\n> > >  \n> > >  /*\n> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > index c34fa460..ef7fec86 100644\n> > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > @@ -29,13 +29,12 @@\n> > >  \n> > >  #include \"libcamera/internal/mapped_framebuffer.h\"\n> > >  \n> > > +#include \"algorithms/algorithm.h\"\n> > > +#include \"algorithms/grid.h\"\n> > >  #include \"ipu3_agc.h\"\n> > >  #include \"ipu3_awb.h\"\n> > >  #include \"libipa/camera_sensor_helper.h\"\n> > >  \n> > > -static constexpr uint32_t kMaxCellWidthPerSet = 160;\n> > > -static constexpr uint32_t kMaxCellHeightPerSet = 56;\n> > > -\n> > >  namespace libcamera {\n> > >  \n> > >  LOG_DEFINE_CATEGORY(IPAIPU3)\n> > > @@ -91,10 +90,12 @@ private:\n> > >  \t/* Interface to the Camera Helper */\n> > >  \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n> > >  \n> > > +\t/* Maintain the algorithms used by the IPA */\n> > > +\tstd::list<std::unique_ptr<ipa::ipu3::Algorithm>> algorithms_;\n> > > +\n> > >  \t/* Local parameter storage */\n> > > +\tstruct IPAContext context_;\n> > >  \tstruct ipu3_uapi_params params_;\n> > > -\n> > > -\tstruct ipu3_uapi_grid_config bdsGrid_;\n> > >  };\n> > >  \n> > >  /**\n> > > @@ -164,6 +165,8 @@ int IPAIPU3::init(const IPASettings &settings,\n> > >  \t\t\t\t\t\t\t       frameDurations[2]);\n> > >  \n> > >  \t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\n> > \n> > I think a new line here is deserved though, Adding algorithms to the\n> > list, is unrelated to initialising the controls - so they should be\n> > separated with a blank line.\n> > \n> > Two trivial blank lines is the most I can say ... so I think that\n> > satisfies a:\n> > \n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > \n> > > +\t/* Construct our Algorithms */\n> > > +\talgorithms_.emplace_back(new algorithms::Grid());\n> > >  \n> > >  \treturn 0;\n> > >  }\n> > > @@ -175,56 +178,6 @@ int IPAIPU3::start()\n> > >  \treturn 0;\n> > >  }\n> > >  \n> > > -/**\n> > > - * This function calculates a grid for the AWB algorithm in the IPU3 firmware.\n> > > - * Its input is the BDS output size calculated in the ImgU.\n> > > - * It is limited for now to the simplest method: find the lesser error\n> > > - * with the width/height and respective log2 width/height of the cells.\n> > > - *\n> > > - * \\todo The frame is divided into cells which can be 8x8 => 128x128.\n> > > - * As a smaller cell improves the algorithm precision, adapting the\n> > > - * x_start and y_start parameters of the grid would provoke a loss of\n> > > - * some pixels but would also result in more accurate algorithms.\n> > > - */\n> > > -void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> > > -{\n> > > -\tuint32_t minError = std::numeric_limits<uint32_t>::max();\n> > > -\tSize best;\n> > > -\tSize bestLog2;\n> > > -\tbdsGrid_ = {};\n> > > -\n> > > -\tfor (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) {\n> > > -\t\tuint32_t width = std::min(kMaxCellWidthPerSet,\n> > > -\t\t\t\t\t  bdsOutputSize.width >> widthShift);\n> > > -\t\twidth = width << widthShift;\n> > > -\t\tfor (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) {\n> > > -\t\t\tint32_t height = std::min(kMaxCellHeightPerSet,\n> > > -\t\t\t\t\t\t  bdsOutputSize.height >> heightShift);\n> > > -\t\t\theight = height << heightShift;\n> > > -\t\t\tuint32_t error  = std::abs(static_cast<int>(width - bdsOutputSize.width))\n> > > -\t\t\t\t\t\t\t+ std::abs(static_cast<int>(height - bdsOutputSize.height));\n> > > -\n> > > -\t\t\tif (error > minError)\n> > > -\t\t\t\tcontinue;\n> > > -\n> > > -\t\t\tminError = error;\n> > > -\t\t\tbest.width = width;\n> > > -\t\t\tbest.height = height;\n> > > -\t\t\tbestLog2.width = widthShift;\n> > > -\t\t\tbestLog2.height = heightShift;\n> > > -\t\t}\n> > > -\t}\n> > > -\n> > > -\tbdsGrid_.width = best.width >> bestLog2.width;\n> > > -\tbdsGrid_.block_width_log2 = bestLog2.width;\n> > > -\tbdsGrid_.height = best.height >> bestLog2.height;\n> > > -\tbdsGrid_.block_height_log2 = bestLog2.height;\n> > > -\n> > > -\tLOG(IPAIPU3, Debug) << \"Best grid found is: (\"\n> > > -\t\t\t    << (int)bdsGrid_.width << \" << \" << (int)bdsGrid_.block_width_log2 << \") x (\"\n> > > -\t\t\t    << (int)bdsGrid_.height << \" << \" << (int)bdsGrid_.block_height_log2 << \")\";\n> > > -}\n> > > -\n> > >  int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> > >  {\n> > >  \tif (configInfo.entityControls.empty()) {\n> > > @@ -264,15 +217,21 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> > >  \n> > >  \tdefVBlank_ = itVBlank->second.def().get<int32_t>();\n> > >  \n> > > +\t/* Clean context and IPU3 parameters at configuration */\n> > >  \tparams_ = {};\n> > > +\tcontext_ = {};\n> > >  \n> > > -\tcalculateBdsGrid(configInfo.bdsOutputSize);\n> > > +\tfor (auto const &algo : algorithms_) {\n> > > +\t\tint ret = algo->configure(context_, configInfo);\n> > > +\t\tif (ret)\n> > > +\t\t\treturn ret;\n> > > +\t}\n> > >  \n> > >  \tawbAlgo_ = std::make_unique<IPU3Awb>();\n> > > -\tawbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);\n> > > +\tawbAlgo_->initialise(params_, context_.configuration.grid.bdsOutputSize, context_.configuration.grid.bdsGrid);\n> > >  \n> > >  \tagcAlgo_ = std::make_unique<IPU3Agc>();\n> > > -\tagcAlgo_->initialise(bdsGrid_, sensorInfo_);\n> > > +\tagcAlgo_->initialise(context_.configuration.grid.bdsGrid, sensorInfo_);\n> > >  \n> > >  \treturn 0;\n> > >  }\n> > > diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp\n> > > index 4bb321b3..4ee5ee6f 100644\n> > > --- a/src/ipa/ipu3/ipu3_awb.cpp\n> > > +++ b/src/ipa/ipu3/ipu3_awb.cpp\n> > > @@ -107,25 +107,6 @@ static const struct ipu3_uapi_bnr_static_config imguCssBnrDefaults = {\n> > >  \t.opt_center_sqr = { 419904, 133956 },\n> > >  };\n> > >  \n> > > -/* Default settings for Auto White Balance replicated from the Kernel*/\n> > > -static const struct ipu3_uapi_awb_config_s imguCssAwbDefaults = {\n> > > -\t.rgbs_thr_gr = 8191,\n> > > -\t.rgbs_thr_r = 8191,\n> > > -\t.rgbs_thr_gb = 8191,\n> > > -\t.rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT,\n> > > -\t.grid = {\n> > > -\t\t.width = 160,\n> > > -\t\t.height = 36,\n> > > -\t\t.block_width_log2 = 3,\n> > > -\t\t.block_height_log2 = 4,\n> > > -\t\t.height_per_slice = 1, /* Overridden by kernel. */\n> > > -\t\t.x_start = 0,\n> > > -\t\t.y_start = 0,\n> > > -\t\t.x_end = 0,\n> > > -\t\t.y_end = 0,\n> > > -\t},\n> > > -};\n> > > -\n> > >  /* Default color correction matrix defined as an identity matrix */\n> > >  static const struct ipu3_uapi_ccm_mat_config imguCssCcmDefault = {\n> > >  \t8191, 0, 0, 0,\n> > > @@ -174,10 +155,12 @@ IPU3Awb::~IPU3Awb()\n> > >  void IPU3Awb::initialise(ipu3_uapi_params &params, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid)\n> > >  {\n> > >  \tparams.use.acc_awb = 1;\n> > > -\tparams.acc_param.awb.config = imguCssAwbDefaults;\n> > > -\n> > > +\tparams.acc_param.awb.config.rgbs_thr_gr = 8191;\n> > > +\tparams.acc_param.awb.config.rgbs_thr_r = 8191;\n> > > +\tparams.acc_param.awb.config.rgbs_thr_gb = 8191;\n> > > +\tparams.acc_param.awb.config.rgbs_thr_b = 8191 | IPU3_UAPI_AWB_RGBS_THR_B_EN | IPU3_UAPI_AWB_RGBS_THR_B_INCL_SAT;\n> \n> Line wrap here too.\n> \n> > > +\tparams.acc_param.awb.config.grid = bdsGrid;\n> > >  \tawbGrid_ = bdsGrid;\n> > > -\tparams.acc_param.awb.config.grid = awbGrid_;\n> > >  \n> > >  \tparams.use.acc_bnr = 1;\n> > >  \tparams.acc_param.bnr = imguCssBnrDefaults;\n> > > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 951C6BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 15 Aug 2021 20:34:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EAFAD68890;\n\tSun, 15 Aug 2021 22:34:20 +0200 (CEST)","from mail-lf1-x135.google.com (mail-lf1-x135.google.com\n\t[IPv6:2a00:1450:4864:20::135])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 43F2568889\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 15 Aug 2021 22:34:19 +0200 (CEST)","by mail-lf1-x135.google.com with SMTP id k5so2658615lfu.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 15 Aug 2021 13:34:19 -0700 (PDT)","from localhost (h-46-59-88-219.A463.priv.bahnhof.se.\n\t[46.59.88.219]) by smtp.gmail.com with ESMTPSA id\n\tl14sm753859lfc.54.2021.08.15.13.34.17\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 15 Aug 2021 13:34:17 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"ia/pD+vf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=VUeebUCnG8rutmHpRygRzbX133hslvCheSls5mLQuBw=;\n\tb=ia/pD+vfeYpsVM1y8aPUbenQ63ajlI+MCkrkmRRCKmCCWqU8w9lo9iqDVg28tBhbef\n\tNF9Q5MR/+GkYJeqD7MGSuYPema5XdeVJ3MfvaUsjHGM+Os5lNP+zXHoCURS8K8ry5DjG\n\tQrqgoLT/knM1B7haEz+1/q4D8g4OMvJS4JYpfzoPRgzEgdWwY7/vYSlpH/e/vv4CpmmY\n\tFT8yPTCzbtWc9Rgh4qmqaDosjrZRlL2fp5Vz+wLAePeIV5qOOXG03lxJ6bJeqblG67rS\n\ttg7Y6ZGCKat97ye3qW1Q+TW4ljF9nMhzHF95KMyrPOan9zeO7LdXcAZnTVUbkmwkD9l4\n\tEMAQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=VUeebUCnG8rutmHpRygRzbX133hslvCheSls5mLQuBw=;\n\tb=XGjNy48AlvEcLgJYc8Md4r3Wgu/wQr+SkQ/PpG4Gf1u63XEEB1ZgJ64Y+7SuwYFeC9\n\t599enccYj02pkEfZvEKneAdtheQXtDDq+Uhpwmfr7h9B4cTVJ+/1LCW10N8BZrWo9fVL\n\tqos+zx+xpav4GmDEUx3dXHV/7CjxFUW09YsoEjYwx1i5UfHB1ihWJCQwWxRlzegiBOE9\n\tcYORdydiEJi4mZZM3VcZxQOlAi9EKAIXrZfPmJAoNw0mdDIWHK5P46bfTTe1j5/1N/E7\n\tya4zT+YcsBGk3iM+pFnOnSUjaLcPwJUWgF9mPnzq47UsTIfx4kIkrhM39wO9Kb8uJTu2\n\tfM0Q==","X-Gm-Message-State":"AOAM532EEaRGPWDXV/2cqAQIgwMtrYWwzfkCIWMLlZZhOwAX9MOLMD1u\n\t9pQ/YXx3yAAzDpgqOlv+W4Sd/ddaXlW+n5P4","X-Google-Smtp-Source":"ABdhPJx/7oqQ1Uj5bAyx6avlJRdYXG9+L3WjTY+fYxep/zz2fljqIC1U09v7cWr6QeJIic34cgPekA==","X-Received":"by 2002:ac2:5e61:: with SMTP id a1mr9451443lfr.495.1629059658161;\n\tSun, 15 Aug 2021 13:34:18 -0700 (PDT)","Date":"Sun, 15 Aug 2021 22:34:16 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<YRl6SJbmQX4d2D1G@oden.dyn.berto.se>","References":"<20210812165243.276977-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210812165243.276977-4-jeanmichel.hautbois@ideasonboard.com>\n\t<00b7205a-feea-8cac-1384-31e4177d5742@ideasonboard.com>\n\t<YRlVwwDer7zL07Hi@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<YRlVwwDer7zL07Hi@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 03/10] ipa: ipu3: Introduce modular\n\tgrid algorithm","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]