[{"id":21560,"web_url":"https://patchwork.libcamera.org/comment/21560/","msgid":"","date":"2021-12-03T02:24:32","subject":"Re: [libcamera-devel] [PATCH v1 1/6] ipa: rkisp1: Introduce sensor\n\tdegamma","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 Thu, Dec 02, 2021 at 07:04:05PM +0100, Jean-Michel Hautbois wrote:\n> The RkISP1 has a sensor degamma control. Introduce the algorithm, but\n> only implement the prepare function as it is a static table to be set.\n> The table used is the one found in the imx219 tuning data in RPi as this\n> is the only sensor I have right now and it looks like a decent default\n> table.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> src/ipa/rkisp1/algorithms/sdg.cpp | 49 +++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/sdg.h | 30 ++++++++++++++++\n> src/ipa/rkisp1/rkisp1.cpp | 2 ++\n> 4 files changed, 82 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/sdg.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/sdg.h\n> \n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> index a19c1a4fa..0678518d8 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -2,4 +2,5 @@\n> \n> rkisp1_ipa_algorithms = files([\n> 'agc.cpp',\n> + 'sdg.cpp',\n> ])\n> diff --git a/src/ipa/rkisp1/algorithms/sdg.cpp b/src/ipa/rkisp1/algorithms/sdg.cpp\n> new file mode 100644\n> index 000000000..3f6b3e205\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/sdg.cpp\n> @@ -0,0 +1,49 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * sdg.cpp - Sensor degamma control algorithm\n> + */\n> +\n> +#include \"sdg.h\"\n> +\n> +/**\n> + * \\file sdg.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class SensorDeGamma\n> + * \\brief A sensor degamma algorithm\n> + */\n> +\n> +static const uint16_t imx219DeGammaCurve[] = { 0, 583, 957, 1299, 1609, 1877,\n> +\t\t\t\t\t 2123, 2350, 2540, 2859, 3101, 3293,\n> +\t\t\t\t\t 3429, 3666, 3823, 3963, 4095 };\n\nWhere does this come from ?\n\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void SensorDeGamma::prepare([[maybe_unused]] IPAContext &context,\n> +\t\t\t rkisp1_params_cfg *params)\n> +{\n> +\tfor (uint32_t i = 0; i < RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE; i++) {\n\nThat's dangerous.\n\nnamespace {\n\nconst std::array imx219DeGammaCurve = {\n\t0, 583, 957, 1299, 1609, 1877, 2123, 2350, 2540,\n\t2859, 3101, 3293, 3429, 3666, 3823, 3963, 4095\n};\n\n} /* namespace */\n\n...\n\n\tfor (const auto &[i, value] : utils::enumerate(imx219DeGammaCurve)) {\n\t\tparams->others.sdg_config.curve_r.gamma_y[i] = value;\n\t\tparams->others.sdg_config.curve_b.gamma_y[i] = value;\n\t\tparams->others.sdg_config.curve_g.gamma_y[i] = value;\n\t}\n\n(utils::enumerate is really just because we can, the important part is\nto avoid a potential array overflow, you can use\nstd::size(imx219DeGammaCurve) instead).\n\n> +\t\tparams->others.sdg_config.curve_r.gamma_y[i] = imx219DeGammaCurve[i];\n> +\t\tparams->others.sdg_config.curve_b.gamma_y[i] = imx219DeGammaCurve[i];\n> +\t\tparams->others.sdg_config.curve_g.gamma_y[i] = imx219DeGammaCurve[i];\n> +\t}\n> +\n> +\tparams->others.sdg_config.xa_pnts.gamma_dx0 = 0x44444444;\n> +\tparams->others.sdg_config.xa_pnts.gamma_dx1 = 0x44444444;\n\nWhat is this ?\n\n> +\n> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG;\n\nAs the array doesn't change, can you update the contents for the first\nframe only ?\n\n> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_SDG;\n> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG;\n> +}\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/sdg.h b/src/ipa/rkisp1/algorithms/sdg.h\n> new file mode 100644\n> index 000000000..24c41627a\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/sdg.h\n> @@ -0,0 +1,30 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * sdg.h - Sensor degamma control algorithm\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +struct IPACameraSensorInfo;\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class SensorDeGamma : public Algorithm\n> +{\n> +public:\n> +\tSensorDeGamma() = default;\n> +\t~SensorDeGamma() = default;\n> +\n> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> +};\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 2d79f15fe..07f1f1c87 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -27,6 +27,7 @@\n> \n> #include \"algorithms/agc.h\"\n> #include \"algorithms/algorithm.h\"\n> +#include \"algorithms/sdg.h\"\n> #include \"libipa/camera_sensor_helper.h\"\n> \n> #include \"ipa_context.h\"\n> @@ -126,6 +127,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n> \n> \t/* Construct our Algorithms */\n> \talgorithms_.push_back(std::make_unique());\n> +\talgorithms_.push_back(std::make_unique());\n> \n> \treturn 0;\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 EF578BDB13\n\tfor ;\n\tFri, 3 Dec 2021 02:25:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE330607DE;\n\tFri, 3 Dec 2021 03:25:00 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B7CF60118\n\tfor ;\n\tFri, 3 Dec 2021 03:24:59 +0100 (CET)","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 B696CA59;\n\tFri, 3 Dec 2021 03:24:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nlW7p/D3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638498299;\n\tbh=1gT+tZ3lpImqBR8lKTRRnEXac2t/TmH36FA+kOgGMZc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nlW7p/D3OKsp7tOHFpC5FcZRnzb4jLJ6T0buawr5KihtJ1F9LfNxF032Dcqbj2yWL\n\tq9yZVafxlvMPfHy6ot6z00rmeK+/afeyoUsh3xLNNb6SVP6W3B2PRC84ve4756T9cs\n\t6eKPttb9OaYdumWUJNRlAq8ZKLAfN6CZNkpQykEI=","Date":"Fri, 3 Dec 2021 04:24:32 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211202180410.518232-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211202180410.518232-2-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211202180410.518232-2-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 1/6] ipa: rkisp1: Introduce sensor\n\tdegamma","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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21564,"web_url":"https://patchwork.libcamera.org/comment/21564/","msgid":"","date":"2021-12-03T06:27:17","subject":"Re: [libcamera-devel] [PATCH v1 1/6] ipa: rkisp1: Introduce sensor\n\tdegamma","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 03/12/2021 03:24, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> Thank you for the patch.\n> \n> On Thu, Dec 02, 2021 at 07:04:05PM +0100, Jean-Michel Hautbois wrote:\n>> The RkISP1 has a sensor degamma control. Introduce the algorithm, but\n>> only implement the prepare function as it is a static table to be set.\n>> The table used is the one found in the imx219 tuning data in RPi as this\n>> is the only sensor I have right now and it looks like a decent default\n>> table.\n>>\n>> Signed-off-by: Jean-Michel Hautbois \n>> ---\n>> src/ipa/rkisp1/algorithms/meson.build | 1 +\n>> src/ipa/rkisp1/algorithms/sdg.cpp | 49 +++++++++++++++++++++++++++\n>> src/ipa/rkisp1/algorithms/sdg.h | 30 ++++++++++++++++\n>> src/ipa/rkisp1/rkisp1.cpp | 2 ++\n>> 4 files changed, 82 insertions(+)\n>> create mode 100644 src/ipa/rkisp1/algorithms/sdg.cpp\n>> create mode 100644 src/ipa/rkisp1/algorithms/sdg.h\n>>\n>> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n>> index a19c1a4fa..0678518d8 100644\n>> --- a/src/ipa/rkisp1/algorithms/meson.build\n>> +++ b/src/ipa/rkisp1/algorithms/meson.build\n>> @@ -2,4 +2,5 @@\n>> \n>> rkisp1_ipa_algorithms = files([\n>> 'agc.cpp',\n>> + 'sdg.cpp',\n>> ])\n>> diff --git a/src/ipa/rkisp1/algorithms/sdg.cpp b/src/ipa/rkisp1/algorithms/sdg.cpp\n>> new file mode 100644\n>> index 000000000..3f6b3e205\n>> --- /dev/null\n>> +++ b/src/ipa/rkisp1/algorithms/sdg.cpp\n>> @@ -0,0 +1,49 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Ideas On Board\n>> + *\n>> + * sdg.cpp - Sensor degamma control algorithm\n>> + */\n>> +\n>> +#include \"sdg.h\"\n>> +\n>> +/**\n>> + * \\file sdg.h\n>> + */\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::rkisp1::algorithms {\n>> +\n>> +/**\n>> + * \\class SensorDeGamma\n>> + * \\brief A sensor degamma algorithm\n>> + */\n>> +\n>> +static const uint16_t imx219DeGammaCurve[] = { 0, 583, 957, 1299, 1609, 1877,\n>> +\t\t\t\t\t 2123, 2350, 2540, 2859, 3101, 3293,\n>> +\t\t\t\t\t 3429, 3666, 3823, 3963, 4095 };\n> \n> Where does this come from ?\n\nAs stated in the commit message, this is coming from the imx219 tuning file.\nI could have been more precise, it is the rpi.contrast.gamma_curve table \nadapted to the RkISP1.\n\n> \n>> +\n>> +/**\n>> + * \\copydoc libcamera::ipa::Algorithm::prepare\n>> + */\n>> +void SensorDeGamma::prepare([[maybe_unused]] IPAContext &context,\n>> +\t\t\t rkisp1_params_cfg *params)\n>> +{\n>> +\tfor (uint32_t i = 0; i < RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE; i++) {\n> \n> That's dangerous.\n> \n> namespace {\n> \n> const std::array imx219DeGammaCurve = {\n> \t0, 583, 957, 1299, 1609, 1877, 2123, 2350, 2540,\n> \t2859, 3101, 3293, 3429, 3666, 3823, 3963, 4095\n> };\n> \n> } /* namespace */\n> \n> ...\n> \n> \tfor (const auto &[i, value] : utils::enumerate(imx219DeGammaCurve)) {\n> \t\tparams->others.sdg_config.curve_r.gamma_y[i] = value;\n> \t\tparams->others.sdg_config.curve_b.gamma_y[i] = value;\n> \t\tparams->others.sdg_config.curve_g.gamma_y[i] = value;\n> \t}\n> \n> (utils::enumerate is really just because we can, the important part is\n> to avoid a potential array overflow, you can use\n> std::size(imx219DeGammaCurve) instead).\n\nI don't always have the C++ reflex indeed :-), thanks.\n\n> \n>> +\t\tparams->others.sdg_config.curve_r.gamma_y[i] = imx219DeGammaCurve[i];\n>> +\t\tparams->others.sdg_config.curve_b.gamma_y[i] = imx219DeGammaCurve[i];\n>> +\t\tparams->others.sdg_config.curve_g.gamma_y[i] = imx219DeGammaCurve[i];\n>> +\t}\n>> +\n>> +\tparams->others.sdg_config.xa_pnts.gamma_dx0 = 0x44444444;\n>> +\tparams->others.sdg_config.xa_pnts.gamma_dx1 = 0x44444444;\n> \n> What is this ?\n> \n>> +\n>> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG;\n> \n> As the array doesn't change, can you update the contents for the first\n> frame only ?\n\nOnly if the params is no more forced to 0 in IPARkISP1 then :-). I can \nchange this.\n\n> \n>> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_SDG;\n>> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG;\n>> +}\n>> +\n>> +} /* namespace ipa::rkisp1::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/rkisp1/algorithms/sdg.h b/src/ipa/rkisp1/algorithms/sdg.h\n>> new file mode 100644\n>> index 000000000..24c41627a\n>> --- /dev/null\n>> +++ b/src/ipa/rkisp1/algorithms/sdg.h\n>> @@ -0,0 +1,30 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Ideas On Board\n>> + *\n>> + * sdg.h - Sensor degamma control algorithm\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +struct IPACameraSensorInfo;\n>> +\n>> +namespace ipa::rkisp1::algorithms {\n>> +\n>> +class SensorDeGamma : public Algorithm\n>> +{\n>> +public:\n>> +\tSensorDeGamma() = default;\n>> +\t~SensorDeGamma() = default;\n>> +\n>> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n>> +};\n>> +\n>> +} /* namespace ipa::rkisp1::algorithms */\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index 2d79f15fe..07f1f1c87 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -27,6 +27,7 @@\n>> \n>> #include \"algorithms/agc.h\"\n>> #include \"algorithms/algorithm.h\"\n>> +#include \"algorithms/sdg.h\"\n>> #include \"libipa/camera_sensor_helper.h\"\n>> \n>> #include \"ipa_context.h\"\n>> @@ -126,6 +127,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n>> \n>> \t/* Construct our Algorithms */\n>> \talgorithms_.push_back(std::make_unique());\n>> +\talgorithms_.push_back(std::make_unique());\n>> \n>> \treturn 0;\n>> }\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 3C9CCBDB13\n\tfor ;\n\tFri, 3 Dec 2021 06:27:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68ED0607DE;\n\tFri, 3 Dec 2021 07:27:22 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BACFC60118\n\tfor ;\n\tFri, 3 Dec 2021 07:27:20 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:458d:6388:c3ac:74bf] (unknown\n\t[IPv6:2a01:e0a:169:7140:458d:6388:c3ac:74bf])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4DA57A59;\n\tFri, 3 Dec 2021 07:27:20 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"es8GFygM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638512840;\n\tbh=BoiOQr3zg1bW/HR287YUjyxXWR4vpgsi9l7Paxzr17Q=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=es8GFygMWjUPUt4nGzoNVzHpbkTESTkFxrdSChgwlRoKzcgt98ojas7CLhrJyWDMx\n\tsLFmez3kGAC1iIVKGD40h9GzViY1SCLxKei2ogomyZK72WQyTwaPpa6lQ5NynTl4as\n\tGuGbA2GCthmJ6jQrd4xoLTq/U+GOxbGb9tW/PXzc=","Message-ID":"","Date":"Fri, 3 Dec 2021 07:27:17 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.3.1","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20211202180410.518232-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211202180410.518232-2-jeanmichel.hautbois@ideasonboard.com>\n\t","From":"Jean-Michel Hautbois ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v1 1/6] ipa: rkisp1: Introduce sensor\n\tdegamma","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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21568,"web_url":"https://patchwork.libcamera.org/comment/21568/","msgid":"","date":"2021-12-03T09:26:17","subject":"Re: [libcamera-devel] [PATCH v1 1/6] ipa: rkisp1: Introduce sensor\n\tdegamma","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Fri, Dec 03, 2021 at 07:27:17AM +0100, Jean-Michel Hautbois wrote:\n> On 03/12/2021 03:24, Laurent Pinchart wrote:\n> > On Thu, Dec 02, 2021 at 07:04:05PM +0100, Jean-Michel Hautbois wrote:\n> >> The RkISP1 has a sensor degamma control. Introduce the algorithm, but\n> >> only implement the prepare function as it is a static table to be set.\n> >> The table used is the one found in the imx219 tuning data in RPi as this\n> >> is the only sensor I have right now and it looks like a decent default\n> >> table.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois \n> >> ---\n> >> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> >> src/ipa/rkisp1/algorithms/sdg.cpp | 49 +++++++++++++++++++++++++++\n> >> src/ipa/rkisp1/algorithms/sdg.h | 30 ++++++++++++++++\n> >> src/ipa/rkisp1/rkisp1.cpp | 2 ++\n> >> 4 files changed, 82 insertions(+)\n> >> create mode 100644 src/ipa/rkisp1/algorithms/sdg.cpp\n> >> create mode 100644 src/ipa/rkisp1/algorithms/sdg.h\n> >>\n> >> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> >> index a19c1a4fa..0678518d8 100644\n> >> --- a/src/ipa/rkisp1/algorithms/meson.build\n> >> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> >> @@ -2,4 +2,5 @@\n> >> \n> >> rkisp1_ipa_algorithms = files([\n> >> 'agc.cpp',\n> >> + 'sdg.cpp',\n> >> ])\n> >> diff --git a/src/ipa/rkisp1/algorithms/sdg.cpp b/src/ipa/rkisp1/algorithms/sdg.cpp\n> >> new file mode 100644\n> >> index 000000000..3f6b3e205\n> >> --- /dev/null\n> >> +++ b/src/ipa/rkisp1/algorithms/sdg.cpp\n> >> @@ -0,0 +1,49 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Ideas On Board\n> >> + *\n> >> + * sdg.cpp - Sensor degamma control algorithm\n> >> + */\n> >> +\n> >> +#include \"sdg.h\"\n> >> +\n> >> +/**\n> >> + * \\file sdg.h\n> >> + */\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +namespace ipa::rkisp1::algorithms {\n> >> +\n> >> +/**\n> >> + * \\class SensorDeGamma\n> >> + * \\brief A sensor degamma algorithm\n> >> + */\n> >> +\n> >> +static const uint16_t imx219DeGammaCurve[] = { 0, 583, 957, 1299, 1609, 1877,\n> >> +\t\t\t\t\t 2123, 2350, 2540, 2859, 3101, 3293,\n> >> +\t\t\t\t\t 3429, 3666, 3823, 3963, 4095 };\n> > \n> > Where does this come from ?\n> \n> As stated in the commit message, this is coming from the imx219 tuning file.\n> I could have been more precise, it is the rpi.contrast.gamma_curve table \n> adapted to the RkISP1.\n\nThat's what I thought looking at imx219.json, but I wasn't sure. The two\ncurves don't seem to match when plotting this one linearly. You may have\nmissed that the horizontal step in rpi.contrast.gamma_curve isn't\nconstant.\n\nThis brings me to the next question then: the above is a contrast curve\nto be applied at the end of the pipeline to enhance contrast, while the\nSDG is a curved meant to linearize the sensor output. Those are two\nentirely different things.\n\n> >> +\n> >> +/**\n> >> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> >> + */\n> >> +void SensorDeGamma::prepare([[maybe_unused]] IPAContext &context,\n> >> +\t\t\t rkisp1_params_cfg *params)\n> >> +{\n> >> +\tfor (uint32_t i = 0; i < RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE; i++) {\n> > \n> > That's dangerous.\n> > \n> > namespace {\n> > \n> > const std::array imx219DeGammaCurve = {\n> > \t0, 583, 957, 1299, 1609, 1877, 2123, 2350, 2540,\n> > \t2859, 3101, 3293, 3429, 3666, 3823, 3963, 4095\n> > };\n> > \n> > } /* namespace */\n> > \n> > ...\n> > \n> > \tfor (const auto &[i, value] : utils::enumerate(imx219DeGammaCurve)) {\n> > \t\tparams->others.sdg_config.curve_r.gamma_y[i] = value;\n> > \t\tparams->others.sdg_config.curve_b.gamma_y[i] = value;\n> > \t\tparams->others.sdg_config.curve_g.gamma_y[i] = value;\n> > \t}\n> > \n> > (utils::enumerate is really just because we can, the important part is\n> > to avoid a potential array overflow, you can use\n> > std::size(imx219DeGammaCurve) instead).\n> \n> I don't always have the C++ reflex indeed :-), thanks.\n> \n> >> +\t\tparams->others.sdg_config.curve_r.gamma_y[i] = imx219DeGammaCurve[i];\n> >> +\t\tparams->others.sdg_config.curve_b.gamma_y[i] = imx219DeGammaCurve[i];\n> >> +\t\tparams->others.sdg_config.curve_g.gamma_y[i] = imx219DeGammaCurve[i];\n> >> +\t}\n> >> +\n> >> +\tparams->others.sdg_config.xa_pnts.gamma_dx0 = 0x44444444;\n> >> +\tparams->others.sdg_config.xa_pnts.gamma_dx1 = 0x44444444;\n> > \n> > What is this ?\n> > \n> >> +\n> >> +\tparams->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG;\n> > \n> > As the array doesn't change, can you update the contents for the first\n> > frame only ?\n> \n> Only if the params is no more forced to 0 in IPARkISP1 then :-). I can \n> change this.\n\nI mean that if you skip setting module_cfg_update on all frames after\nthe first, the driver will not update the SDG configuration. You can\nread the driver code to see how it works.\n\n> >> +\tparams->module_ens |= RKISP1_CIF_ISP_MODULE_SDG;\n> >> +\tparams->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG;\n> >> +}\n> >> +\n> >> +} /* namespace ipa::rkisp1::algorithms */\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/ipa/rkisp1/algorithms/sdg.h b/src/ipa/rkisp1/algorithms/sdg.h\n> >> new file mode 100644\n> >> index 000000000..24c41627a\n> >> --- /dev/null\n> >> +++ b/src/ipa/rkisp1/algorithms/sdg.h\n> >> @@ -0,0 +1,30 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Ideas On Board\n> >> + *\n> >> + * sdg.h - Sensor degamma control algorithm\n> >> + */\n> >> +\n> >> +#pragma once\n> >> +\n> >> +#include \n> >> +\n> >> +#include \"algorithm.h\"\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +struct IPACameraSensorInfo;\n> >> +\n> >> +namespace ipa::rkisp1::algorithms {\n> >> +\n> >> +class SensorDeGamma : public Algorithm\n> >> +{\n> >> +public:\n> >> +\tSensorDeGamma() = default;\n> >> +\t~SensorDeGamma() = default;\n> >> +\n> >> +\tvoid prepare(IPAContext &context, rkisp1_params_cfg *params) override;\n> >> +};\n> >> +\n> >> +} /* namespace ipa::rkisp1::algorithms */\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> >> index 2d79f15fe..07f1f1c87 100644\n> >> --- a/src/ipa/rkisp1/rkisp1.cpp\n> >> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> >> @@ -27,6 +27,7 @@\n> >> \n> >> #include \"algorithms/agc.h\"\n> >> #include \"algorithms/algorithm.h\"\n> >> +#include \"algorithms/sdg.h\"\n> >> #include \"libipa/camera_sensor_helper.h\"\n> >> \n> >> #include \"ipa_context.h\"\n> >> @@ -126,6 +127,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n> >> \n> >> \t/* Construct our Algorithms */\n> >> \talgorithms_.push_back(std::make_unique());\n> >> +\talgorithms_.push_back(std::make_unique());\n> >> \n> >> \treturn 0;\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 E4581BDB13\n\tfor ;\n\tFri, 3 Dec 2021 09:26:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 368DC607DE;\n\tFri, 3 Dec 2021 10:26:47 +0100 (CET)","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 17F6C60710\n\tfor ;\n\tFri, 3 Dec 2021 10:26:45 +0100 (CET)","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 6BE7BA59;\n\tFri, 3 Dec 2021 10:26:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BhE34ZrW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638523604;\n\tbh=qZqkSbmol9LHwlkwKGZUcqi/oaR6lEuFlO7QMSwuL6s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BhE34ZrWqaNjzUj8WGIlIyZBIuJoKmPvf6aNTeI5ImSaA20+L9vM5eAj3Bv5IHvol\n\tDCvqK8Pe3ZqttHe6pKVNsr2tSPb66h4Ay4KxKTsswzfx/rIwcOLTszg3NMdYeOxzXA\n\tRvovMSL3PHwrdLiuMmRWcY3SxSjmyRVkhDveVHx4=","Date":"Fri, 3 Dec 2021 11:26:17 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211202180410.518232-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211202180410.518232-2-jeanmichel.hautbois@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1 1/6] ipa: rkisp1: Introduce sensor\n\tdegamma","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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]