[{"id":21027,"web_url":"https://patchwork.libcamera.org/comment/21027/","msgid":"<163732139645.1089182.4656119205789894324@Monstersaurus>","date":"2021-11-19T11:29:56","subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-19 11:16:54)\n> The ISP can use 25 or 81 cells depending on its revision. Remove the\n> cached value in IPARkISP1 and use IPASessionConfiguration to store it\n> and pass it to AGC.\n> \n> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> ---\n>  src/ipa/ipu3/ipa_context.cpp      | 3 +++\n>  src/ipa/rkisp1/algorithms/agc.cpp | 4 +++-\n>  src/ipa/rkisp1/algorithms/agc.h   | 2 ++\n>  src/ipa/rkisp1/ipa_context.h      | 1 +\n>  src/ipa/rkisp1/rkisp1.cpp         | 5 ++---\n>  5 files changed, 11 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 99caf9ad..71a8619b 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 {\n>   *\n>   * \\var IPASessionConfiguration::grid.maxAnalogueGain\n>   * \\brief Maximum analogue gain supported with the configured sensor\n> + *\n> + * \\var IPASessionConfiguration::agc.numCells\n> + * \\brief Number of cells used by the ISP to store the Y means\n>   */\n>  \n>  /**\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 2b7c6fda..471cf584 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -82,6 +82,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>         context.frameContext.agc.gain = minAnalogueGain_;\n>         context.frameContext.agc.exposure = 10ms / lineDuration_;\n>  \n> +       numCells_ = context.configuration.agc.numCells;\n\nDoes the AGC need to store this? or can it always access the context\nwhen it needs it?\n\n> +\n>         return 0;\n>  }\n>  \n> @@ -195,7 +197,7 @@ double Agc::computeInitialY(const rkisp1_cif_isp_ae_stat *ae, double currentYGai\n>         double ySum = 0.0;\n>         unsigned int num = 0;\n>  \n> -       for (unsigned int aeCell = 0; aeCell < RKISP1_CIF_ISP_AE_MEAN_MAX_V10; aeCell++) {\n> +       for (unsigned int aeCell = 0; aeCell < numCells_; aeCell++) {\n\nAh - ok that answers that - this function isn't being given the context.\nSo this is fine for me.\n\n\n>                 ySum += ae->exp_mean[aeCell] * currentYGain;\n>                 num++;\n>         }\n> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> index 934a455e..f4293035 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.h\n> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> @@ -45,6 +45,8 @@ private:\n>         double minAnalogueGain_;\n>         double maxAnalogueGain_;\n>  \n> +       uint32_t numCells_;\n> +\n>         utils::Duration filteredExposure_;\n>         utils::Duration currentExposure_;\n>  };\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index ca48ffc5..e8e1dc47 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -24,6 +24,7 @@ struct IPASessionConfiguration {\n>                 utils::Duration maxShutterSpeed;\n>                 double minAnalogueGain;\n>                 double maxAnalogueGain;\n> +               uint32_t numCells;\n>         } agc;\n>  };\n>  \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index b1bfb8b6..cebf2c65 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -75,7 +75,6 @@ private:\n>         uint32_t maxGain_;\n>  \n>         /* revision-specific data */\n> -       unsigned int hwAeMeanMax_;\n>         unsigned int hwHistBinNMax_;\n>         unsigned int hwGammaOutMaxSamples_;\n>         unsigned int hwHistogramWeightGridsSize_;\n> @@ -98,13 +97,13 @@ int IPARkISP1::init([[maybe_unused]] const IPASettings &settings,\n>         /* \\todo Add support for other revisions */\n>         switch (hwRevision) {\n>         case RKISP1_V10:\n> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n>                 hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n>                 hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n>                 hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;\n\nI suspect each of those are going to move into the context too... the\nalgorithms shouldn't have to dig into the top level class. But .. later\n;-)\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n>                 break;\n>         case RKISP1_V12:\n> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n>                 hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n>                 hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;\n>                 hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;\n> -- \n> 2.32.0\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 C7D2ABF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 19 Nov 2021 11:30:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7CDC360376;\n\tFri, 19 Nov 2021 12:30: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 2465660233\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 19 Nov 2021 12:29:59 +0100 (CET)","from pendragon.ideasonboard.com\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 C78C61959;\n\tFri, 19 Nov 2021 12:29: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=\"bZwigif+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637321398;\n\tbh=x9Y0Ec07ijQajTlwdE+brGeuX7cld9/X7z7ucglWGYw=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=bZwigif+AJsU0p3adMSXZKK8wO02PMAPrcQ+PjMLR0i2cK+s7pkMc64Xny9MWpeBj\n\tX0sSvvw73hfPp2l98pzDg3BxWegUiiDtwpYthCbEG7G2iyrH9BTgpecAUdMjQ9IWd4\n\tAzbRaG5JB2ai0NumE4HNQKCAHqgAF36rlpngi7gA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211119111654.68445-9-jeanmichel.hautbois@ideasonboard.com>","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-9-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 19 Nov 2021 11:29:56 +0000","Message-ID":"<163732139645.1089182.4656119205789894324@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","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":21028,"web_url":"https://patchwork.libcamera.org/comment/21028/","msgid":"<6d937a48-17ea-7176-04e3-5216f798336d@ideasonboard.com>","date":"2021-11-19T11:35:22","subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"On 19/11/2021 12:29, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-11-19 11:16:54)\n>> The ISP can use 25 or 81 cells depending on its revision. Remove the\n>> cached value in IPARkISP1 and use IPASessionConfiguration to store it\n>> and pass it to AGC.\n>>\n>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n>> ---\n>>   src/ipa/ipu3/ipa_context.cpp      | 3 +++\n>>   src/ipa/rkisp1/algorithms/agc.cpp | 4 +++-\n>>   src/ipa/rkisp1/algorithms/agc.h   | 2 ++\n>>   src/ipa/rkisp1/ipa_context.h      | 1 +\n>>   src/ipa/rkisp1/rkisp1.cpp         | 5 ++---\n>>   5 files changed, 11 insertions(+), 4 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n>> index 99caf9ad..71a8619b 100644\n>> --- a/src/ipa/ipu3/ipa_context.cpp\n>> +++ b/src/ipa/ipu3/ipa_context.cpp\n>> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 {\n>>    *\n>>    * \\var IPASessionConfiguration::grid.maxAnalogueGain\n>>    * \\brief Maximum analogue gain supported with the configured sensor\n>> + *\n>> + * \\var IPASessionConfiguration::agc.numCells\n>> + * \\brief Number of cells used by the ISP to store the Y means\n>>    */\n>>   \n>>   /**\n>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n>> index 2b7c6fda..471cf584 100644\n>> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n>> @@ -82,6 +82,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>>          context.frameContext.agc.gain = minAnalogueGain_;\n>>          context.frameContext.agc.exposure = 10ms / lineDuration_;\n>>   \n>> +       numCells_ = context.configuration.agc.numCells;\n> \n> Does the AGC need to store this? or can it always access the context\n> when it needs it?\n> \n>> +\n>>          return 0;\n>>   }\n>>   \n>> @@ -195,7 +197,7 @@ double Agc::computeInitialY(const rkisp1_cif_isp_ae_stat *ae, double currentYGai\n>>          double ySum = 0.0;\n>>          unsigned int num = 0;\n>>   \n>> -       for (unsigned int aeCell = 0; aeCell < RKISP1_CIF_ISP_AE_MEAN_MAX_V10; aeCell++) {\n>> +       for (unsigned int aeCell = 0; aeCell < numCells_; aeCell++) {\n> \n> Ah - ok that answers that - this function isn't being given the context.\n> So this is fine for me.\n\nYou got me ! Yes, I will remove the cached value when AWB will be \nintroduced, as I will then pass the context to the function to get the \nawb gains applied ;-).\n\n> \n> \n>>                  ySum += ae->exp_mean[aeCell] * currentYGain;\n>>                  num++;\n>>          }\n>> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n>> index 934a455e..f4293035 100644\n>> --- a/src/ipa/rkisp1/algorithms/agc.h\n>> +++ b/src/ipa/rkisp1/algorithms/agc.h\n>> @@ -45,6 +45,8 @@ private:\n>>          double minAnalogueGain_;\n>>          double maxAnalogueGain_;\n>>   \n>> +       uint32_t numCells_;\n>> +\n>>          utils::Duration filteredExposure_;\n>>          utils::Duration currentExposure_;\n>>   };\n>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n>> index ca48ffc5..e8e1dc47 100644\n>> --- a/src/ipa/rkisp1/ipa_context.h\n>> +++ b/src/ipa/rkisp1/ipa_context.h\n>> @@ -24,6 +24,7 @@ struct IPASessionConfiguration {\n>>                  utils::Duration maxShutterSpeed;\n>>                  double minAnalogueGain;\n>>                  double maxAnalogueGain;\n>> +               uint32_t numCells;\n>>          } agc;\n>>   };\n>>   \n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index b1bfb8b6..cebf2c65 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -75,7 +75,6 @@ private:\n>>          uint32_t maxGain_;\n>>   \n>>          /* revision-specific data */\n>> -       unsigned int hwAeMeanMax_;\n>>          unsigned int hwHistBinNMax_;\n>>          unsigned int hwGammaOutMaxSamples_;\n>>          unsigned int hwHistogramWeightGridsSize_;\n>> @@ -98,13 +97,13 @@ int IPARkISP1::init([[maybe_unused]] const IPASettings &settings,\n>>          /* \\todo Add support for other revisions */\n>>          switch (hwRevision) {\n>>          case RKISP1_V10:\n>> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n>> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n>>                  hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n>>                  hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n>>                  hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;\n> \n> I suspect each of those are going to move into the context too... the\n> algorithms shouldn't have to dig into the top level class. But .. later\n> ;-)\n\nEach algorithm will remove the cached value which won't be needed anymore.\n\n> \n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n>>                  break;\n>>          case RKISP1_V12:\n>> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n>> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n>>                  hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n>>                  hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;\n>>                  hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;\n>> -- \n>> 2.32.0\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 48817BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 19 Nov 2021 11:35:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6308960376;\n\tFri, 19 Nov 2021 12:35:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B93C260233\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 19 Nov 2021 12:35:24 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:db30:8e54:a96:9838] (unknown\n\t[IPv6:2a01:e0a:169:7140:db30:8e54:a96:9838])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 66F731959;\n\tFri, 19 Nov 2021 12:35:24 +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=\"utaygAmr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637321724;\n\tbh=mxwFkWzfJdmfifosqZjJ2a+rElRL7yfXJGKkpCzOWJE=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=utaygAmrk85JNhoNrV6k4Ed38+k65I4UQyV7GrMNkvYFR4YjJG1mcYIF2ZMjHlfz+\n\tvcc7pVMaRp9M1bFDHshNiHin6LjdC0ozupMR4A242uSJ3svVeNJzpSqGVvkI4tLaSn\n\tJUWOKSBrejoUMy+wV9FgdgmcWVfhBrVvz5Yyl/Nw=","Message-ID":"<6d937a48-17ea-7176-04e3-5216f798336d@ideasonboard.com>","Date":"Fri, 19 Nov 2021 12:35:22 +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":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-9-jeanmichel.hautbois@ideasonboard.com>\n\t<163732139645.1089182.4656119205789894324@Monstersaurus>","From":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>","In-Reply-To":"<163732139645.1089182.4656119205789894324@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","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":21047,"web_url":"https://patchwork.libcamera.org/comment/21047/","msgid":"<YZeuEpwsXOwg3+Dd@pendragon.ideasonboard.com>","date":"2021-11-19T14:00:50","subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","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, Nov 19, 2021 at 12:35:22PM +0100, Jean-Michel Hautbois wrote:\n> On 19/11/2021 12:29, Kieran Bingham wrote:\n> > Quoting Jean-Michel Hautbois (2021-11-19 11:16:54)\n> >> The ISP can use 25 or 81 cells depending on its revision. Remove the\n> >> cached value in IPARkISP1 and use IPASessionConfiguration to store it\n> >> and pass it to AGC.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> >> ---\n> >>   src/ipa/ipu3/ipa_context.cpp      | 3 +++\n> >>   src/ipa/rkisp1/algorithms/agc.cpp | 4 +++-\n> >>   src/ipa/rkisp1/algorithms/agc.h   | 2 ++\n> >>   src/ipa/rkisp1/ipa_context.h      | 1 +\n> >>   src/ipa/rkisp1/rkisp1.cpp         | 5 ++---\n> >>   5 files changed, 11 insertions(+), 4 deletions(-)\n> >>\n> >> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> >> index 99caf9ad..71a8619b 100644\n> >> --- a/src/ipa/ipu3/ipa_context.cpp\n> >> +++ b/src/ipa/ipu3/ipa_context.cpp\n> >> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 {\n> >>    *\n> >>    * \\var IPASessionConfiguration::grid.maxAnalogueGain\n> >>    * \\brief Maximum analogue gain supported with the configured sensor\n> >> + *\n> >> + * \\var IPASessionConfiguration::agc.numCells\n> >> + * \\brief Number of cells used by the ISP to store the Y means\n> >>    */\n> >>   \n> >>   /**\n> >> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> >> index 2b7c6fda..471cf584 100644\n> >> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> >> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> >> @@ -82,6 +82,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> >>          context.frameContext.agc.gain = minAnalogueGain_;\n> >>          context.frameContext.agc.exposure = 10ms / lineDuration_;\n> >>   \n> >> +       numCells_ = context.configuration.agc.numCells;\n> > \n> > Does the AGC need to store this? or can it always access the context\n> > when it needs it?\n> > \n> >> +\n> >>          return 0;\n> >>   }\n> >>   \n> >> @@ -195,7 +197,7 @@ double Agc::computeInitialY(const rkisp1_cif_isp_ae_stat *ae, double currentYGai\n> >>          double ySum = 0.0;\n> >>          unsigned int num = 0;\n> >>   \n> >> -       for (unsigned int aeCell = 0; aeCell < RKISP1_CIF_ISP_AE_MEAN_MAX_V10; aeCell++) {\n> >> +       for (unsigned int aeCell = 0; aeCell < numCells_; aeCell++) {\n> > \n> > Ah - ok that answers that - this function isn't being given the context.\n> > So this is fine for me.\n> \n> You got me ! Yes, I will remove the cached value when AWB will be \n> introduced, as I will then pass the context to the function to get the \n> awb gains applied ;-).\n> \n> >>                  ySum += ae->exp_mean[aeCell] * currentYGain;\n> >>                  num++;\n> >>          }\n> >> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> >> index 934a455e..f4293035 100644\n> >> --- a/src/ipa/rkisp1/algorithms/agc.h\n> >> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> >> @@ -45,6 +45,8 @@ private:\n> >>          double minAnalogueGain_;\n> >>          double maxAnalogueGain_;\n> >>   \n> >> +       uint32_t numCells_;\n> >> +\n> >>          utils::Duration filteredExposure_;\n> >>          utils::Duration currentExposure_;\n> >>   };\n> >> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> >> index ca48ffc5..e8e1dc47 100644\n> >> --- a/src/ipa/rkisp1/ipa_context.h\n> >> +++ b/src/ipa/rkisp1/ipa_context.h\n> >> @@ -24,6 +24,7 @@ struct IPASessionConfiguration {\n> >>                  utils::Duration maxShutterSpeed;\n> >>                  double minAnalogueGain;\n> >>                  double maxAnalogueGain;\n> >> +               uint32_t numCells;\n> >>          } agc;\n> >>   };\n> >>   \n> >> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> >> index b1bfb8b6..cebf2c65 100644\n> >> --- a/src/ipa/rkisp1/rkisp1.cpp\n> >> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> >> @@ -75,7 +75,6 @@ private:\n> >>          uint32_t maxGain_;\n> >>   \n> >>          /* revision-specific data */\n> >> -       unsigned int hwAeMeanMax_;\n> >>          unsigned int hwHistBinNMax_;\n> >>          unsigned int hwGammaOutMaxSamples_;\n> >>          unsigned int hwHistogramWeightGridsSize_;\n> >> @@ -98,13 +97,13 @@ int IPARkISP1::init([[maybe_unused]] const IPASettings &settings,\n> >>          /* \\todo Add support for other revisions */\n> >>          switch (hwRevision) {\n> >>          case RKISP1_V10:\n> >> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n> >> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n> >>                  hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n> >>                  hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n> >>                  hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;\n> > \n> > I suspect each of those are going to move into the context too... the\n> > algorithms shouldn't have to dig into the top level class. But .. later\n> > ;-)\n> \n> Each algorithm will remove the cached value which won't be needed anymore.\n\nWould it make sense to store the hardware revision in the context, and\ncompute (and cache) the derived values such as numCells in the\nalgorithms ?\n\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > \n> >>                  break;\n> >>          case RKISP1_V12:\n> >> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> >> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> >>                  hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n> >>                  hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;\n> >>                  hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;","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 4B5B8BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 19 Nov 2021 14:01:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E5F560371;\n\tFri, 19 Nov 2021 15:01:15 +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 33FDF600B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 19 Nov 2021 15:01:13 +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 9782C340;\n\tFri, 19 Nov 2021 15:01:12 +0100 (CET)"],"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=\"bFIw98Ls\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637330472;\n\tbh=QUeGNxvk1Vz6jwTNv/AHw0hHMDO4ZC7i4b7/vMcpEcc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bFIw98LsrBSeHZaLXSPZ15AsmIBPcUpVFu5+0Z9RjLVpf+9sQLZYOzs4Mg4lr18Lb\n\tcIHu+p1Ia0kaJUcF/jpw35ET8lL866S9SMTb163Pg2df96j/wrX2Ja9IIEsUuE8YPM\n\tuooDafQ6YCpqAV7zb8hwZTnnrl68rnreXaMIaQ+Q=","Date":"Fri, 19 Nov 2021 16:00:50 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>","Message-ID":"<YZeuEpwsXOwg3+Dd@pendragon.ideasonboard.com>","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-9-jeanmichel.hautbois@ideasonboard.com>\n\t<163732139645.1089182.4656119205789894324@Monstersaurus>\n\t<6d937a48-17ea-7176-04e3-5216f798336d@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6d937a48-17ea-7176-04e3-5216f798336d@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","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":21052,"web_url":"https://patchwork.libcamera.org/comment/21052/","msgid":"<b23e842b-3799-6212-a702-2fd75b204726@ideasonboard.com>","date":"2021-11-19T14:41:16","subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","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 19/11/2021 15:00, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> Thank you for the patch.\n> \n> On Fri, Nov 19, 2021 at 12:35:22PM +0100, Jean-Michel Hautbois wrote:\n>> On 19/11/2021 12:29, Kieran Bingham wrote:\n>>> Quoting Jean-Michel Hautbois (2021-11-19 11:16:54)\n>>>> The ISP can use 25 or 81 cells depending on its revision. Remove the\n>>>> cached value in IPARkISP1 and use IPASessionConfiguration to store it\n>>>> and pass it to AGC.\n>>>>\n>>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n>>>> ---\n>>>>    src/ipa/ipu3/ipa_context.cpp      | 3 +++\n>>>>    src/ipa/rkisp1/algorithms/agc.cpp | 4 +++-\n>>>>    src/ipa/rkisp1/algorithms/agc.h   | 2 ++\n>>>>    src/ipa/rkisp1/ipa_context.h      | 1 +\n>>>>    src/ipa/rkisp1/rkisp1.cpp         | 5 ++---\n>>>>    5 files changed, 11 insertions(+), 4 deletions(-)\n>>>>\n>>>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n>>>> index 99caf9ad..71a8619b 100644\n>>>> --- a/src/ipa/ipu3/ipa_context.cpp\n>>>> +++ b/src/ipa/ipu3/ipa_context.cpp\n>>>> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 {\n>>>>     *\n>>>>     * \\var IPASessionConfiguration::grid.maxAnalogueGain\n>>>>     * \\brief Maximum analogue gain supported with the configured sensor\n>>>> + *\n>>>> + * \\var IPASessionConfiguration::agc.numCells\n>>>> + * \\brief Number of cells used by the ISP to store the Y means\n>>>>     */\n>>>>    \n>>>>    /**\n>>>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n>>>> index 2b7c6fda..471cf584 100644\n>>>> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n>>>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n>>>> @@ -82,6 +82,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>>>>           context.frameContext.agc.gain = minAnalogueGain_;\n>>>>           context.frameContext.agc.exposure = 10ms / lineDuration_;\n>>>>    \n>>>> +       numCells_ = context.configuration.agc.numCells;\n>>>\n>>> Does the AGC need to store this? or can it always access the context\n>>> when it needs it?\n>>>\n>>>> +\n>>>>           return 0;\n>>>>    }\n>>>>    \n>>>> @@ -195,7 +197,7 @@ double Agc::computeInitialY(const rkisp1_cif_isp_ae_stat *ae, double currentYGai\n>>>>           double ySum = 0.0;\n>>>>           unsigned int num = 0;\n>>>>    \n>>>> -       for (unsigned int aeCell = 0; aeCell < RKISP1_CIF_ISP_AE_MEAN_MAX_V10; aeCell++) {\n>>>> +       for (unsigned int aeCell = 0; aeCell < numCells_; aeCell++) {\n>>>\n>>> Ah - ok that answers that - this function isn't being given the context.\n>>> So this is fine for me.\n>>\n>> You got me ! Yes, I will remove the cached value when AWB will be\n>> introduced, as I will then pass the context to the function to get the\n>> awb gains applied ;-).\n>>\n>>>>                   ySum += ae->exp_mean[aeCell] * currentYGain;\n>>>>                   num++;\n>>>>           }\n>>>> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n>>>> index 934a455e..f4293035 100644\n>>>> --- a/src/ipa/rkisp1/algorithms/agc.h\n>>>> +++ b/src/ipa/rkisp1/algorithms/agc.h\n>>>> @@ -45,6 +45,8 @@ private:\n>>>>           double minAnalogueGain_;\n>>>>           double maxAnalogueGain_;\n>>>>    \n>>>> +       uint32_t numCells_;\n>>>> +\n>>>>           utils::Duration filteredExposure_;\n>>>>           utils::Duration currentExposure_;\n>>>>    };\n>>>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n>>>> index ca48ffc5..e8e1dc47 100644\n>>>> --- a/src/ipa/rkisp1/ipa_context.h\n>>>> +++ b/src/ipa/rkisp1/ipa_context.h\n>>>> @@ -24,6 +24,7 @@ struct IPASessionConfiguration {\n>>>>                   utils::Duration maxShutterSpeed;\n>>>>                   double minAnalogueGain;\n>>>>                   double maxAnalogueGain;\n>>>> +               uint32_t numCells;\n>>>>           } agc;\n>>>>    };\n>>>>    \n>>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>>>> index b1bfb8b6..cebf2c65 100644\n>>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>>>> @@ -75,7 +75,6 @@ private:\n>>>>           uint32_t maxGain_;\n>>>>    \n>>>>           /* revision-specific data */\n>>>> -       unsigned int hwAeMeanMax_;\n>>>>           unsigned int hwHistBinNMax_;\n>>>>           unsigned int hwGammaOutMaxSamples_;\n>>>>           unsigned int hwHistogramWeightGridsSize_;\n>>>> @@ -98,13 +97,13 @@ int IPARkISP1::init([[maybe_unused]] const IPASettings &settings,\n>>>>           /* \\todo Add support for other revisions */\n>>>>           switch (hwRevision) {\n>>>>           case RKISP1_V10:\n>>>> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n>>>> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n>>>>                   hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n>>>>                   hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n>>>>                   hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;\n>>>\n>>> I suspect each of those are going to move into the context too... the\n>>> algorithms shouldn't have to dig into the top level class. But .. later\n>>> ;-)\n>>\n>> Each algorithm will remove the cached value which won't be needed anymore.\n> \n> Would it make sense to store the hardware revision in the context, and\n> compute (and cache) the derived values such as numCells in the\n> algorithms ?\n\nIt is possible, but is there any interest in doing so (except that it \nmakes it out from IPAContext) ?\n\n> \n>>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>>\n>>>>                   break;\n>>>>           case RKISP1_V12:\n>>>> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n>>>> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n>>>>                   hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n>>>>                   hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;\n>>>>                   hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;\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 97C81BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 19 Nov 2021 14:41:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B7126037A;\n\tFri, 19 Nov 2021 15:41:20 +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 73CDA600B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 19 Nov 2021 15:41:19 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:db30:8e54:a96:9838] (unknown\n\t[IPv6:2a01:e0a:169:7140:db30:8e54:a96:9838])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 33F351959;\n\tFri, 19 Nov 2021 15:41:19 +0100 (CET)"],"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=\"R5xdFTrY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637332879;\n\tbh=66jYKy4buKcSU9yxYsgXI7iUykhIW26/mDXtkpAIu8Y=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=R5xdFTrYjcImJ4pOipniuSvcon1RGNBrNMMocz9vb9Oq7x4BTvXQCBhSE2D0gUYxk\n\tn83nQr70H92HOhDZ5y2HlVz58AX1pd1dBNkBKGQD0ZzALqT+ON+z1vsnqPK+zHu9Ay\n\tvbdUPYa/e31B5rym1dBSply7hjfjJGjyAE1xTlXU=","Message-ID":"<b23e842b-3799-6212-a702-2fd75b204726@ideasonboard.com>","Date":"Fri, 19 Nov 2021 15:41:16 +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 <laurent.pinchart@ideasonboard.com>","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-9-jeanmichel.hautbois@ideasonboard.com>\n\t<163732139645.1089182.4656119205789894324@Monstersaurus>\n\t<6d937a48-17ea-7176-04e3-5216f798336d@ideasonboard.com>\n\t<YZeuEpwsXOwg3+Dd@pendragon.ideasonboard.com>","From":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>","In-Reply-To":"<YZeuEpwsXOwg3+Dd@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","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":21054,"web_url":"https://patchwork.libcamera.org/comment/21054/","msgid":"<163733323266.1089182.15552828431671622817@Monstersaurus>","date":"2021-11-19T14:47:12","subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-19 14:41:16)\n> Hi Laurent,\n> \n> On 19/11/2021 15:00, Laurent Pinchart wrote:\n> > Hi Jean-Michel,\n> > \n> > Thank you for the patch.\n> > \n> > On Fri, Nov 19, 2021 at 12:35:22PM +0100, Jean-Michel Hautbois wrote:\n> >> On 19/11/2021 12:29, Kieran Bingham wrote:\n> >>> Quoting Jean-Michel Hautbois (2021-11-19 11:16:54)\n> >>>> The ISP can use 25 or 81 cells depending on its revision. Remove the\n> >>>> cached value in IPARkISP1 and use IPASessionConfiguration to store it\n> >>>> and pass it to AGC.\n> >>>>\n> >>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> >>>> ---\n> >>>>    src/ipa/ipu3/ipa_context.cpp      | 3 +++\n> >>>>    src/ipa/rkisp1/algorithms/agc.cpp | 4 +++-\n> >>>>    src/ipa/rkisp1/algorithms/agc.h   | 2 ++\n> >>>>    src/ipa/rkisp1/ipa_context.h      | 1 +\n> >>>>    src/ipa/rkisp1/rkisp1.cpp         | 5 ++---\n> >>>>    5 files changed, 11 insertions(+), 4 deletions(-)\n> >>>>\n> >>>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> >>>> index 99caf9ad..71a8619b 100644\n> >>>> --- a/src/ipa/ipu3/ipa_context.cpp\n> >>>> +++ b/src/ipa/ipu3/ipa_context.cpp\n> >>>> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 {\n> >>>>     *\n> >>>>     * \\var IPASessionConfiguration::grid.maxAnalogueGain\n> >>>>     * \\brief Maximum analogue gain supported with the configured sensor\n> >>>> + *\n> >>>> + * \\var IPASessionConfiguration::agc.numCells\n> >>>> + * \\brief Number of cells used by the ISP to store the Y means\n> >>>>     */\n> >>>>    \n> >>>>    /**\n> >>>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> >>>> index 2b7c6fda..471cf584 100644\n> >>>> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> >>>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> >>>> @@ -82,6 +82,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> >>>>           context.frameContext.agc.gain = minAnalogueGain_;\n> >>>>           context.frameContext.agc.exposure = 10ms / lineDuration_;\n> >>>>    \n> >>>> +       numCells_ = context.configuration.agc.numCells;\n> >>>\n> >>> Does the AGC need to store this? or can it always access the context\n> >>> when it needs it?\n> >>>\n> >>>> +\n> >>>>           return 0;\n> >>>>    }\n> >>>>    \n> >>>> @@ -195,7 +197,7 @@ double Agc::computeInitialY(const rkisp1_cif_isp_ae_stat *ae, double currentYGai\n> >>>>           double ySum = 0.0;\n> >>>>           unsigned int num = 0;\n> >>>>    \n> >>>> -       for (unsigned int aeCell = 0; aeCell < RKISP1_CIF_ISP_AE_MEAN_MAX_V10; aeCell++) {\n> >>>> +       for (unsigned int aeCell = 0; aeCell < numCells_; aeCell++) {\n> >>>\n> >>> Ah - ok that answers that - this function isn't being given the context.\n> >>> So this is fine for me.\n> >>\n> >> You got me ! Yes, I will remove the cached value when AWB will be\n> >> introduced, as I will then pass the context to the function to get the\n> >> awb gains applied ;-).\n> >>\n> >>>>                   ySum += ae->exp_mean[aeCell] * currentYGain;\n> >>>>                   num++;\n> >>>>           }\n> >>>> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> >>>> index 934a455e..f4293035 100644\n> >>>> --- a/src/ipa/rkisp1/algorithms/agc.h\n> >>>> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> >>>> @@ -45,6 +45,8 @@ private:\n> >>>>           double minAnalogueGain_;\n> >>>>           double maxAnalogueGain_;\n> >>>>    \n> >>>> +       uint32_t numCells_;\n> >>>> +\n> >>>>           utils::Duration filteredExposure_;\n> >>>>           utils::Duration currentExposure_;\n> >>>>    };\n> >>>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> >>>> index ca48ffc5..e8e1dc47 100644\n> >>>> --- a/src/ipa/rkisp1/ipa_context.h\n> >>>> +++ b/src/ipa/rkisp1/ipa_context.h\n> >>>> @@ -24,6 +24,7 @@ struct IPASessionConfiguration {\n> >>>>                   utils::Duration maxShutterSpeed;\n> >>>>                   double minAnalogueGain;\n> >>>>                   double maxAnalogueGain;\n> >>>> +               uint32_t numCells;\n> >>>>           } agc;\n> >>>>    };\n> >>>>    \n> >>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> >>>> index b1bfb8b6..cebf2c65 100644\n> >>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n> >>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> >>>> @@ -75,7 +75,6 @@ private:\n> >>>>           uint32_t maxGain_;\n> >>>>    \n> >>>>           /* revision-specific data */\n> >>>> -       unsigned int hwAeMeanMax_;\n> >>>>           unsigned int hwHistBinNMax_;\n> >>>>           unsigned int hwGammaOutMaxSamples_;\n> >>>>           unsigned int hwHistogramWeightGridsSize_;\n> >>>> @@ -98,13 +97,13 @@ int IPARkISP1::init([[maybe_unused]] const IPASettings &settings,\n> >>>>           /* \\todo Add support for other revisions */\n> >>>>           switch (hwRevision) {\n> >>>>           case RKISP1_V10:\n> >>>> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n> >>>> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n> >>>>                   hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n> >>>>                   hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n> >>>>                   hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;\n> >>>\n> >>> I suspect each of those are going to move into the context too... the\n> >>> algorithms shouldn't have to dig into the top level class. But .. later\n> >>> ;-)\n> >>\n> >> Each algorithm will remove the cached value which won't be needed anymore.\n> > \n> > Would it make sense to store the hardware revision in the context, and\n> > compute (and cache) the derived values such as numCells in the\n> > algorithms ?\n> \n> It is possible, but is there any interest in doing so (except that it \n> makes it out from IPAContext) ?\n\nWe'd have to be careful about how we clean the IPAContext too.\nCurrently it gets reset/cleared during configure...\n\nIt might make sense to be able to keep the algorithm specific\nrequirements closely associated with it's algorithm though.\n\n\n> \n> > \n> >>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >>>\n> >>>>                   break;\n> >>>>           case RKISP1_V12:\n> >>>> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> >>>> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> >>>>                   hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n> >>>>                   hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;\n> >>>>                   hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;\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 C5538BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 19 Nov 2021 14:47:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79C4460371;\n\tFri, 19 Nov 2021 15:47:16 +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 7FF0B600B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 19 Nov 2021 15:47:15 +0100 (CET)","from pendragon.ideasonboard.com\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 431291974;\n\tFri, 19 Nov 2021 15:47:15 +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=\"MPuLl43e\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637333235;\n\tbh=gXn9CI6bN4DpzRey3f+shjBrn4J/ZbOQtDEe7SPVvQ8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=MPuLl43ec53eDejoDgWEcyqgO/kQXTFM87HWfcqFqI6XIUZOxG3b260kRofr2Ngvy\n\tFgAR2cRK/vIWWhOCqPwCuBrJE7y5IJl3aZLoug4B18IjBDYkRhMZhP2cwcKMbGVQif\n\tG7Gs6c0c0YkGqWcS2ZViqJ1ExKK4vlbHzF6SA2xE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<b23e842b-3799-6212-a702-2fd75b204726@ideasonboard.com>","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-9-jeanmichel.hautbois@ideasonboard.com>\n\t<163732139645.1089182.4656119205789894324@Monstersaurus>\n\t<6d937a48-17ea-7176-04e3-5216f798336d@ideasonboard.com>\n\t<YZeuEpwsXOwg3+Dd@pendragon.ideasonboard.com>\n\t<b23e842b-3799-6212-a702-2fd75b204726@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Fri, 19 Nov 2021 14:47:12 +0000","Message-ID":"<163733323266.1089182.15552828431671622817@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","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":21056,"web_url":"https://patchwork.libcamera.org/comment/21056/","msgid":"<YZfLYjpiHa60Cy8v@pendragon.ideasonboard.com>","date":"2021-11-19T16:05:54","subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Nov 19, 2021 at 02:47:12PM +0000, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-11-19 14:41:16)\n> > On 19/11/2021 15:00, Laurent Pinchart wrote:\n> > > On Fri, Nov 19, 2021 at 12:35:22PM +0100, Jean-Michel Hautbois wrote:\n> > >> On 19/11/2021 12:29, Kieran Bingham wrote:\n> > >>> Quoting Jean-Michel Hautbois (2021-11-19 11:16:54)\n> > >>>> The ISP can use 25 or 81 cells depending on its revision. Remove the\n> > >>>> cached value in IPARkISP1 and use IPASessionConfiguration to store it\n> > >>>> and pass it to AGC.\n> > >>>>\n> > >>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> > >>>> ---\n> > >>>>    src/ipa/ipu3/ipa_context.cpp      | 3 +++\n> > >>>>    src/ipa/rkisp1/algorithms/agc.cpp | 4 +++-\n> > >>>>    src/ipa/rkisp1/algorithms/agc.h   | 2 ++\n> > >>>>    src/ipa/rkisp1/ipa_context.h      | 1 +\n> > >>>>    src/ipa/rkisp1/rkisp1.cpp         | 5 ++---\n> > >>>>    5 files changed, 11 insertions(+), 4 deletions(-)\n> > >>>>\n> > >>>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> > >>>> index 99caf9ad..71a8619b 100644\n> > >>>> --- a/src/ipa/ipu3/ipa_context.cpp\n> > >>>> +++ b/src/ipa/ipu3/ipa_context.cpp\n> > >>>> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 {\n> > >>>>     *\n> > >>>>     * \\var IPASessionConfiguration::grid.maxAnalogueGain\n> > >>>>     * \\brief Maximum analogue gain supported with the configured sensor\n> > >>>> + *\n> > >>>> + * \\var IPASessionConfiguration::agc.numCells\n> > >>>> + * \\brief Number of cells used by the ISP to store the Y means\n> > >>>>     */\n> > >>>>    \n> > >>>>    /**\n> > >>>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > >>>> index 2b7c6fda..471cf584 100644\n> > >>>> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > >>>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > >>>> @@ -82,6 +82,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> > >>>>           context.frameContext.agc.gain = minAnalogueGain_;\n> > >>>>           context.frameContext.agc.exposure = 10ms / lineDuration_;\n> > >>>>    \n> > >>>> +       numCells_ = context.configuration.agc.numCells;\n> > >>>\n> > >>> Does the AGC need to store this? or can it always access the context\n> > >>> when it needs it?\n> > >>>\n> > >>>> +\n> > >>>>           return 0;\n> > >>>>    }\n> > >>>>    \n> > >>>> @@ -195,7 +197,7 @@ double Agc::computeInitialY(const rkisp1_cif_isp_ae_stat *ae, double currentYGai\n> > >>>>           double ySum = 0.0;\n> > >>>>           unsigned int num = 0;\n> > >>>>    \n> > >>>> -       for (unsigned int aeCell = 0; aeCell < RKISP1_CIF_ISP_AE_MEAN_MAX_V10; aeCell++) {\n> > >>>> +       for (unsigned int aeCell = 0; aeCell < numCells_; aeCell++) {\n> > >>>\n> > >>> Ah - ok that answers that - this function isn't being given the context.\n> > >>> So this is fine for me.\n> > >>\n> > >> You got me ! Yes, I will remove the cached value when AWB will be\n> > >> introduced, as I will then pass the context to the function to get the\n> > >> awb gains applied ;-).\n> > >>\n> > >>>>                   ySum += ae->exp_mean[aeCell] * currentYGain;\n> > >>>>                   num++;\n> > >>>>           }\n> > >>>> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\n> > >>>> index 934a455e..f4293035 100644\n> > >>>> --- a/src/ipa/rkisp1/algorithms/agc.h\n> > >>>> +++ b/src/ipa/rkisp1/algorithms/agc.h\n> > >>>> @@ -45,6 +45,8 @@ private:\n> > >>>>           double minAnalogueGain_;\n> > >>>>           double maxAnalogueGain_;\n> > >>>>    \n> > >>>> +       uint32_t numCells_;\n> > >>>> +\n> > >>>>           utils::Duration filteredExposure_;\n> > >>>>           utils::Duration currentExposure_;\n> > >>>>    };\n> > >>>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > >>>> index ca48ffc5..e8e1dc47 100644\n> > >>>> --- a/src/ipa/rkisp1/ipa_context.h\n> > >>>> +++ b/src/ipa/rkisp1/ipa_context.h\n> > >>>> @@ -24,6 +24,7 @@ struct IPASessionConfiguration {\n> > >>>>                   utils::Duration maxShutterSpeed;\n> > >>>>                   double minAnalogueGain;\n> > >>>>                   double maxAnalogueGain;\n> > >>>> +               uint32_t numCells;\n> > >>>>           } agc;\n> > >>>>    };\n> > >>>>    \n> > >>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > >>>> index b1bfb8b6..cebf2c65 100644\n> > >>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n> > >>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > >>>> @@ -75,7 +75,6 @@ private:\n> > >>>>           uint32_t maxGain_;\n> > >>>>    \n> > >>>>           /* revision-specific data */\n> > >>>> -       unsigned int hwAeMeanMax_;\n> > >>>>           unsigned int hwHistBinNMax_;\n> > >>>>           unsigned int hwGammaOutMaxSamples_;\n> > >>>>           unsigned int hwHistogramWeightGridsSize_;\n> > >>>> @@ -98,13 +97,13 @@ int IPARkISP1::init([[maybe_unused]] const IPASettings &settings,\n> > >>>>           /* \\todo Add support for other revisions */\n> > >>>>           switch (hwRevision) {\n> > >>>>           case RKISP1_V10:\n> > >>>> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n> > >>>> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;\n> > >>>>                   hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;\n> > >>>>                   hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V10;\n> > >>>>                   hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V10;\n> > >>>\n> > >>> I suspect each of those are going to move into the context too... the\n> > >>> algorithms shouldn't have to dig into the top level class. But .. later\n> > >>> ;-)\n> > >>\n> > >> Each algorithm will remove the cached value which won't be needed anymore.\n> > > \n> > > Would it make sense to store the hardware revision in the context, and\n> > > compute (and cache) the derived values such as numCells in the\n> > > algorithms ?\n> > \n> > It is possible, but is there any interest in doing so (except that it \n> > makes it out from IPAContext) ?\n> \n> We'd have to be careful about how we clean the IPAContext too.\n> Currently it gets reset/cleared during configure...\n> \n> It might make sense to be able to keep the algorithm specific\n> requirements closely associated with it's algorithm though.\n\nThat was my idea, if all (or most) of the hw*_ fields currently stored\nin IPARkISP1 are specific to a single algorithm, they can be computed\nfrom the revision in the algorithms and stored there.\n\n> > >>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > >>>\n> > >>>>                   break;\n> > >>>>           case RKISP1_V12:\n> > >>>> -               hwAeMeanMax_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> > >>>> +               context_.configuration.agc.numCells = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;\n> > >>>>                   hwHistBinNMax_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;\n> > >>>>                   hwGammaOutMaxSamples_ = RKISP1_CIF_ISP_GAMMA_OUT_MAX_SAMPLES_V12;\n> > >>>>                   hwHistogramWeightGridsSize_ = RKISP1_CIF_ISP_HISTOGRAM_WEIGHT_GRIDS_SIZE_V12;","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 BDED5BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 19 Nov 2021 16:06:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2286F60371;\n\tFri, 19 Nov 2021 17:06:20 +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 5E5BF600B5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 19 Nov 2021 17:06:18 +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 B55FC1C19;\n\tFri, 19 Nov 2021 17:06:17 +0100 (CET)"],"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=\"IDjnQpS8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637337978;\n\tbh=iicdA0GNGN7FGwoQRSQd+pQtMXyBff8SOwEBFlAsxP4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IDjnQpS83dNCECqC+tlhE4xtUuzJdgHG0+63DtoNLh3N8jxCMXglBuXnq9I3k3tbf\n\tqIoiCLN5UFWGJ8KqtFL9mVeriqmZuVAXFETWDLZ0ifL3ElLGYxRBXtq9bZuDzPUE5d\n\tQRj775cpgQLIUMERfJ7p3YQufIck2njcscKOdMOQ=","Date":"Fri, 19 Nov 2021 18:05:54 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YZfLYjpiHa60Cy8v@pendragon.ideasonboard.com>","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-9-jeanmichel.hautbois@ideasonboard.com>\n\t<163732139645.1089182.4656119205789894324@Monstersaurus>\n\t<6d937a48-17ea-7176-04e3-5216f798336d@ideasonboard.com>\n\t<YZeuEpwsXOwg3+Dd@pendragon.ideasonboard.com>\n\t<b23e842b-3799-6212-a702-2fd75b204726@ideasonboard.com>\n\t<163733323266.1089182.15552828431671622817@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163733323266.1089182.15552828431671622817@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v1 8/8] ipa: rkisp1: agc: Configure\n\tthe number of cells used by HW","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>"}}]