[{"id":29200,"web_url":"https://patchwork.libcamera.org/comment/29200/","msgid":"<cb06a23f-e423-478f-ace0-6fa838acde61@ideasonboard.com>","date":"2024-04-12T07:59:01","subject":"Re: [PATCH 2/5] ipa: rkisp1: agc: Add digital gain","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul,\n\nOn 05/04/24 8:17 pm, Paul Elder wrote:\n> Add support to the rkisp1 AGC to set digital gain.\n>\n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>   src/ipa/rkisp1/algorithms/agc.cpp | 5 +++++\n>   src/ipa/rkisp1/ipa_context.h      | 3 +++\n>   src/ipa/rkisp1/rkisp1.cpp         | 2 ++\n>   3 files changed, 10 insertions(+)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index fd47ba4e..7220f00a 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -10,6 +10,7 @@\n>   #include <algorithm>\n>   #include <chrono>\n>   #include <cmath>\n> +#include <tuple>\n\nIs this required? Not sure by reading this particular patch\n\nLooks good to me otherwise. With this addressed,\n\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\n>   \n>   #include <libcamera/base/log.h>\n>   #include <libcamera/base/utils.h>\n> @@ -152,8 +153,10 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>   \tcontext.activeState.agc.automatic.gain = context.configuration.sensor.minAnalogueGain;\n>   \tcontext.activeState.agc.automatic.exposure =\n>   \t\t10ms / context.configuration.sensor.lineDuration;\n> +\tcontext.activeState.agc.automatic.dgain = 1;\n>   \tcontext.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;\n>   \tcontext.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;\n> +\tcontext.activeState.agc.manual.dgain = 1;\n>   \tcontext.activeState.agc.autoEnabled = !context.configuration.raw;\n>   \n>   \t/*\n> @@ -237,6 +240,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n>   \tif (frameContext.agc.autoEnabled) {\n>   \t\tframeContext.agc.exposure = context.activeState.agc.automatic.exposure;\n>   \t\tframeContext.agc.gain = context.activeState.agc.automatic.gain;\n> +\t\tframeContext.agc.dgain = context.activeState.agc.automatic.dgain;\n>   \t}\n>   \n>   \t/* \\todo Remove this when we can set the below with controls */\n> @@ -380,6 +384,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>   \t/* Update the estimated exposure and gain. */\n>   \tactiveState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;\n>   \tactiveState.agc.automatic.gain = aGain;\n> +\tactiveState.agc.automatic.dgain = dGain;\n>   \n>   \tfillMetadata(context, frameContext, metadata);\n>   }\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 256b75eb..a70c7eb3 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -61,10 +61,12 @@ struct IPAActiveState {\n>   \t\tstruct {\n>   \t\t\tuint32_t exposure;\n>   \t\t\tdouble gain;\n> +\t\t\tdouble dgain;\n>   \t\t} manual;\n>   \t\tstruct {\n>   \t\t\tuint32_t exposure;\n>   \t\t\tdouble gain;\n> +\t\t\tdouble dgain;\n>   \t\t} automatic;\n>   \n>   \t\tbool autoEnabled;\n> @@ -110,6 +112,7 @@ struct IPAFrameContext : public FrameContext {\n>   \tstruct {\n>   \t\tuint32_t exposure;\n>   \t\tdouble gain;\n> +\t\tdouble dgain;\n>   \t\tbool autoEnabled;\n>   \t} agc;\n>   \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index b0bbcd8c..d66dfdd7 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -446,10 +446,12 @@ void IPARkISP1::setControls(unsigned int frame)\n>   \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>   \tuint32_t exposure = frameContext.agc.exposure;\n>   \tuint32_t gain = camHelper_->gainCode(frameContext.agc.gain);\n> +\tuint32_t dgain = camHelper_->gainCode(frameContext.agc.dgain);\n>   \n>   \tControlList ctrls(sensorControls_);\n>   \tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));\n>   \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));\n> +\tctrls.set(V4L2_CID_DIGITAL_GAIN, static_cast<int32_t>(dgain));\n>   \n>   \tsetSensorControls.emit(frame, ctrls);\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 DD80CBE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Apr 2024 07:59:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B295B63352;\n\tFri, 12 Apr 2024 09:59:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E56C46333B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Apr 2024 09:59:07 +0200 (CEST)","from [192.168.1.105] (unknown [103.251.226.65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B1BAE8E1;\n\tFri, 12 Apr 2024 09:58:23 +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=\"c3nHelzt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712908704;\n\tbh=hD0s0in9pAn/BQowLtSaVobvpjUYvJLKKhtBUK3UGjg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=c3nHelztbfKsiFfR15Mo/3K1bq2griF9wzH5XoIGDOwy7nPRn17l0TT2KFTWDLrgf\n\tDa4lJ/dtYzxpw9XaKIRkEplQU+FNt2lgbod7RPhDefyoaMJ0Zirtbf1Dk3VVE08QUq\n\t6BtYUZvHlu8Nx21+2KgD39QRDrJV90HQDuz1L/Mw=","Message-ID":"<cb06a23f-e423-478f-ace0-6fa838acde61@ideasonboard.com>","Date":"Fri, 12 Apr 2024 13:29:01 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/5] ipa: rkisp1: agc: Add digital gain","Content-Language":"en-US","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240405144729.2992219-1-paul.elder@ideasonboard.com>\n\t<20240405144729.2992219-3-paul.elder@ideasonboard.com>","From":"Umang Jain <umang.jain@ideasonboard.com>","In-Reply-To":"<20240405144729.2992219-3-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":29206,"web_url":"https://patchwork.libcamera.org/comment/29206/","msgid":"<171291184578.2108906.4653031953320814146@ping.linuxembedded.co.uk>","date":"2024-04-12T08:50:45","subject":"Re: [PATCH 2/5] ipa: rkisp1: agc: Add digital gain","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2024-04-12 08:59:01)\n> Hi Paul,\n> \n> On 05/04/24 8:17 pm, Paul Elder wrote:\n> > Add support to the rkisp1 AGC to set digital gain.\n> >\n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > ---\n> >   src/ipa/rkisp1/algorithms/agc.cpp | 5 +++++\n> >   src/ipa/rkisp1/ipa_context.h      | 3 +++\n> >   src/ipa/rkisp1/rkisp1.cpp         | 2 ++\n> >   3 files changed, 10 insertions(+)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > index fd47ba4e..7220f00a 100644\n> > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > @@ -10,6 +10,7 @@\n> >   #include <algorithm>\n> >   #include <chrono>\n> >   #include <cmath>\n> > +#include <tuple>\n> \n> Is this required? Not sure by reading this particular patch\n> \n> Looks good to me otherwise. With this addressed,\n> \n> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n> \n> >   \n> >   #include <libcamera/base/log.h>\n> >   #include <libcamera/base/utils.h>\n> > @@ -152,8 +153,10 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> >       context.activeState.agc.automatic.gain = context.configuration.sensor.minAnalogueGain;\n> >       context.activeState.agc.automatic.exposure =\n> >               10ms / context.configuration.sensor.lineDuration;\n> > +     context.activeState.agc.automatic.dgain = 1;\n> >       context.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;\n> >       context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;\n> > +     context.activeState.agc.manual.dgain = 1;\n> >       context.activeState.agc.autoEnabled = !context.configuration.raw;\n> >   \n> >       /*\n> > @@ -237,6 +240,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n> >       if (frameContext.agc.autoEnabled) {\n> >               frameContext.agc.exposure = context.activeState.agc.automatic.exposure;\n> >               frameContext.agc.gain = context.activeState.agc.automatic.gain;\n> > +             frameContext.agc.dgain = context.activeState.agc.automatic.dgain;\n> >       }\n> >   \n> >       /* \\todo Remove this when we can set the below with controls */\n> > @@ -380,6 +384,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> >       /* Update the estimated exposure and gain. */\n> >       activeState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;\n> >       activeState.agc.automatic.gain = aGain;\n> > +     activeState.agc.automatic.dgain = dGain;\n> >   \n> >       fillMetadata(context, frameContext, metadata);\n> >   }\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 256b75eb..a70c7eb3 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -61,10 +61,12 @@ struct IPAActiveState {\n> >               struct {\n> >                       uint32_t exposure;\n> >                       double gain;\n> > +                     double dgain;\n> >               } manual;\n> >               struct {\n> >                       uint32_t exposure;\n> >                       double gain;\n> > +                     double dgain;\n> >               } automatic;\n> >   \n> >               bool autoEnabled;\n> > @@ -110,6 +112,7 @@ struct IPAFrameContext : public FrameContext {\n> >       struct {\n> >               uint32_t exposure;\n> >               double gain;\n> > +             double dgain;\n> >               bool autoEnabled;\n> >       } agc;\n> >   \n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index b0bbcd8c..d66dfdd7 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -446,10 +446,12 @@ void IPARkISP1::setControls(unsigned int frame)\n> >       IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> >       uint32_t exposure = frameContext.agc.exposure;\n> >       uint32_t gain = camHelper_->gainCode(frameContext.agc.gain);\n> > +     uint32_t dgain = camHelper_->gainCode(frameContext.agc.dgain);\n> >   \n> >       ControlList ctrls(sensorControls_);\n> >       ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));\n> >       ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));\n> > +     ctrls.set(V4L2_CID_DIGITAL_GAIN, static_cast<int32_t>(dgain));\n\nNot all sensors will have a digital gain, and digital gain models can be\ndifferent to analogue gain models so they would require a separate\nhelper, and can not use gainCode().\n\nI assume we can handle some digital gain on the RKISP1 right? So this\nshould be being applied through there - not the sensor.\n\n--\nKieran\n\n\n> >   \n> >       setSensorControls.emit(frame, ctrls);\n> >   }\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 79CA1BE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Apr 2024 08:50:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54A416335E;\n\tFri, 12 Apr 2024 10:50:50 +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 E37DE6333E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Apr 2024 10:50:48 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 52871A12;\n\tFri, 12 Apr 2024 10:50:05 +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=\"a6dN18f3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712911805;\n\tbh=MK+8Pw3xXMaDmL2tFBAH5VGpaZlgNC1pQx15QKQh1D8=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=a6dN18f3U1zXCdyMUP1K63aIrbo9NEsXe33tMfKiEpBJN2IAlqRlsn/zKPhTn4y6j\n\tj6j3BBdr6HgkfiqrDe7TaI5pwoAh37Bw6PfuyvXX31yQteTGMuWsZ2yfv/apAKhFv4\n\tmJz4CWtwEwTrL6Iatd9BKz1sqVYDroMan+dMKGZ8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<cb06a23f-e423-478f-ace0-6fa838acde61@ideasonboard.com>","References":"<20240405144729.2992219-1-paul.elder@ideasonboard.com>\n\t<20240405144729.2992219-3-paul.elder@ideasonboard.com>\n\t<cb06a23f-e423-478f-ace0-6fa838acde61@ideasonboard.com>","Subject":"Re: [PATCH 2/5] ipa: rkisp1: agc: Add digital gain","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tUmang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 12 Apr 2024 09:50:45 +0100","Message-ID":"<171291184578.2108906.4653031953320814146@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":29229,"web_url":"https://patchwork.libcamera.org/comment/29229/","msgid":"<20240415133817.5rvu7oddb5phi4i3@jasper>","date":"2024-04-15T13:38:17","subject":"Re: [PATCH 2/5] ipa: rkisp1: agc: Add digital gain","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nthanks for the patch.\n\nOn Fri, Apr 05, 2024 at 11:47:26PM +0900, Paul Elder wrote:\n> Add support to the rkisp1 AGC to set digital gain.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>  src/ipa/rkisp1/algorithms/agc.cpp | 5 +++++\n>  src/ipa/rkisp1/ipa_context.h      | 3 +++\n>  src/ipa/rkisp1/rkisp1.cpp         | 2 ++\n>  3 files changed, 10 insertions(+)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index fd47ba4e..7220f00a 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -10,6 +10,7 @@\n>  #include <algorithm>\n>  #include <chrono>\n>  #include <cmath>\n> +#include <tuple>\n>  \n>  #include <libcamera/base/log.h>\n>  #include <libcamera/base/utils.h>\n> @@ -152,8 +153,10 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n>  \tcontext.activeState.agc.automatic.gain = context.configuration.sensor.minAnalogueGain;\n>  \tcontext.activeState.agc.automatic.exposure =\n>  \t\t10ms / context.configuration.sensor.lineDuration;\n> +\tcontext.activeState.agc.automatic.dgain = 1;\n>  \tcontext.activeState.agc.manual.gain = context.activeState.agc.automatic.gain;\n>  \tcontext.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure;\n> +\tcontext.activeState.agc.manual.dgain = 1;\n>  \tcontext.activeState.agc.autoEnabled = !context.configuration.raw;\n>  \n>  \t/*\n> @@ -237,6 +240,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n>  \tif (frameContext.agc.autoEnabled) {\n>  \t\tframeContext.agc.exposure = context.activeState.agc.automatic.exposure;\n>  \t\tframeContext.agc.gain = context.activeState.agc.automatic.gain;\n> +\t\tframeContext.agc.dgain = context.activeState.agc.automatic.dgain;\n>  \t}\n>  \n>  \t/* \\todo Remove this when we can set the below with controls */\n> @@ -380,6 +384,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>  \t/* Update the estimated exposure and gain. */\n>  \tactiveState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;\n>  \tactiveState.agc.automatic.gain = aGain;\n> +\tactiveState.agc.automatic.dgain = dGain;\n>  \n>  \tfillMetadata(context, frameContext, metadata);\n>  }\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 256b75eb..a70c7eb3 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -61,10 +61,12 @@ struct IPAActiveState {\n>  \t\tstruct {\n>  \t\t\tuint32_t exposure;\n>  \t\t\tdouble gain;\n> +\t\t\tdouble dgain;\n>  \t\t} manual;\n>  \t\tstruct {\n>  \t\t\tuint32_t exposure;\n>  \t\t\tdouble gain;\n> +\t\t\tdouble dgain;\n>  \t\t} automatic;\n>  \n>  \t\tbool autoEnabled;\n> @@ -110,6 +112,7 @@ struct IPAFrameContext : public FrameContext {\n>  \tstruct {\n>  \t\tuint32_t exposure;\n>  \t\tdouble gain;\n> +\t\tdouble dgain;\n>  \t\tbool autoEnabled;\n>  \t} agc;\n>  \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index b0bbcd8c..d66dfdd7 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -446,10 +446,12 @@ void IPARkISP1::setControls(unsigned int frame)\n>  \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>  \tuint32_t exposure = frameContext.agc.exposure;\n>  \tuint32_t gain = camHelper_->gainCode(frameContext.agc.gain);\n> +\tuint32_t dgain = camHelper_->gainCode(frameContext.agc.dgain);\n\nI seem to be missing something here. The camHelper doesn't know if we\nare requesting digital or analog gain (or combined). This only works if\nthe gainCode is the same for analog and digital which is not generically\nthe case. Do we need this patch at the moment? I fear that it might make\nthings more difficult at the moment. There is no big benefit in digital\ngain and things will clear up a bit when the camera helpers where moved\nto the correct location.\n\nBest regards,\nStefan\n  \n>  \tControlList ctrls(sensorControls_);\n>  \tctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));\n>  \tctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));\n> +\tctrls.set(V4L2_CID_DIGITAL_GAIN, static_cast<int32_t>(dgain));\n>  \n>  \tsetSensorControls.emit(frame, ctrls);\n>  }\n> -- \n> 2.39.2\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 74F5AC3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 15 Apr 2024 13:38:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C93663352;\n\tMon, 15 Apr 2024 15:38:22 +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 BAEDF63339\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 15 Apr 2024 15:38:20 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7a0d:dd2e:881a:db83])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0318B9C1;\n\tMon, 15 Apr 2024 15:37:34 +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=\"TZBbkB9e\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713188255;\n\tbh=IH/aXqGPM2egO3oxVHCQDLyxeVCoRj45HpdBIPN5cu0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TZBbkB9eGuM6bBZmuQQVnwfQe0Zz7sXWoohU7DJV230SLa9GAg++QCXR+KiMgiRv5\n\trOm+OU+8/r04t4cVWYa21Mgs8uG+xO+/tEL0XP0oCGpiR/j+OPaOoKg1dt6N5kmNo8\n\tfjlBDhc3Y58cxXQFyhuWiZz+iuNm64o7RKVLynR0=","Date":"Mon, 15 Apr 2024 15:38:17 +0200","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/5] ipa: rkisp1: agc: Add digital gain","Message-ID":"<20240415133817.5rvu7oddb5phi4i3@jasper>","References":"<20240405144729.2992219-1-paul.elder@ideasonboard.com>\n\t<20240405144729.2992219-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240405144729.2992219-3-paul.elder@ideasonboard.com>","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>"}}]