[{"id":14225,"web_url":"https://patchwork.libcamera.org/comment/14225/","msgid":"","date":"2020-12-11T21:00:45","subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: Only validate ISP\n\tand Unicam controls during configure","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Wed, Dec 09, 2020 at 09:53:39AM +0000, Naushir Patuck wrote:\n> There is no need to validate all the ISP and Unicam V4L2 controls on\n> every frame. Simply validate them once during the IPA configuration, and\n> fail if a required control is not available.\n> \n> Also add some whitespace for readability.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/raspberrypi.cpp | 131 ++++++++++++++--------------\n> 1 file changed, 64 insertions(+), 67 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 60cfdc27..e5d2b41e 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -92,6 +92,8 @@ public:\n> \n> private:\n> \tvoid setMode(const CameraSensorInfo &sensorInfo);\n> +\tbool validateUnicamControls();\n> +\tbool validateIspControls();\n> \tvoid queueRequest(const ControlList &controls);\n> \tvoid returnEmbeddedBuffer(unsigned int bufferId);\n> \tvoid prepareISP(unsigned int bufferId);\n> @@ -238,6 +240,16 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \tunicamCtrls_ = entityControls.at(0);\n> \tispCtrls_ = entityControls.at(1);\n> \n> +\tif (!validateUnicamControls()) {\n> +\t\tLOG(IPARPI, Error) << \"Unicam control validation failed.\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tif (!validateIspControls()) {\n> +\t\tLOG(IPARPI, Error) << \"ISP control validation failed.\";\n> +\t\treturn -EINVAL;\n> +\t}\n\nconfigure() is a void function. Rebase issue ?\n\n> +\n> \t/* Setup a metadata ControlList to output metadata. */\n> \tlibcameraMetadata_ = ControlList(controls::controls);\n> \n> @@ -473,6 +485,51 @@ void IPARPi::reportMetadata()\n> \t}\n> }\n> \n> +bool IPARPi::validateUnicamControls()\n> +{\n> +\tconst uint32_t ctrls[] = {\n\nstatic const ? Same below.\n\n> +\t\tV4L2_CID_ANALOGUE_GAIN,\n> +\t\tV4L2_CID_EXPOSURE\n> +\t};\n> +\n> +\tfor (auto c : ctrls) {\n> +\t\tif (unicamCtrls_.find(c) == unicamCtrls_.end()) {\n> +\t\t\tLOG(IPARPI, Error) << \"Unable to find Unicam control \"\n> +\t\t\t\t\t << c;\n\nI'd use utils::hex(c) as otherwise the value is hard to read. Same\nbelow.\n\n> +\t\t\treturn false;\n> +\t\t}\n> +\t}\n> +\n> +\treturn true;\n> +}\n\nIsn't this sensor controls, not unicam controls ? While at it, we could\nrename unicamCtrls_ to sensorCtrls_.\n\nThe rest looks good to me.\n\n> +\n> +bool IPARPi::validateIspControls()\n> +{\n> +\tconst uint32_t ctrls[] = {\n> +\t\tV4L2_CID_RED_BALANCE,\n> +\t\tV4L2_CID_BLUE_BALANCE,\n> +\t\tV4L2_CID_DIGITAL_GAIN,\n> +\t\tV4L2_CID_USER_BCM2835_ISP_CC_MATRIX,\n> +\t\tV4L2_CID_USER_BCM2835_ISP_GAMMA,\n> +\t\tV4L2_CID_USER_BCM2835_ISP_BLACK_LEVEL,\n> +\t\tV4L2_CID_USER_BCM2835_ISP_GEQ,\n> +\t\tV4L2_CID_USER_BCM2835_ISP_DENOISE,\n> +\t\tV4L2_CID_USER_BCM2835_ISP_SHARPEN,\n> +\t\tV4L2_CID_USER_BCM2835_ISP_DPC,\n> +\t\tV4L2_CID_USER_BCM2835_ISP_LENS_SHADING\n> +\t};\n> +\n> +\tfor (auto c : ctrls) {\n> +\t\tif (ispCtrls_.find(c) == ispCtrls_.end()) {\n> +\t\t\tLOG(IPARPI, Error) << \"Unable to find ISP control \"\n> +\t\t\t\t\t << c;\n> +\t\t\treturn false;\n> +\t\t}\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> /*\n> * Converting between enums (used in the libcamera API) and the names that\n> * we use to identify different modes. Unfortunately, the conversion tables\n> @@ -858,18 +915,6 @@ void IPARPi::processStats(unsigned int bufferId)\n> \n> void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> {\n> -\tconst auto gainR = ispCtrls_.find(V4L2_CID_RED_BALANCE);\n> -\tif (gainR == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find red gain control\";\n> -\t\treturn;\n> -\t}\n> -\n> -\tconst auto gainB = ispCtrls_.find(V4L2_CID_BLUE_BALANCE);\n> -\tif (gainB == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find blue gain control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tLOG(IPARPI, Debug) << \"Applying WB R: \" << awbStatus->gain_r << \" B: \"\n> \t\t\t << awbStatus->gain_b;\n> \n> @@ -884,16 +929,6 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n> \tint32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n> \tint32_t exposureLines = helper_->ExposureLines(agcStatus->shutter_time);\n> \n> -\tif (unicamCtrls_.find(V4L2_CID_ANALOGUE_GAIN) == unicamCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find analogue gain control\";\n> -\t\treturn;\n> -\t}\n> -\n> -\tif (unicamCtrls_.find(V4L2_CID_EXPOSURE) == unicamCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find exposure control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tLOG(IPARPI, Debug) << \"Applying AGC Exposure: \" << agcStatus->shutter_time\n> \t\t\t << \" (Shutter lines: \" << exposureLines << \") Gain: \"\n> \t\t\t << agcStatus->analogue_gain << \" (Gain Code: \"\n> @@ -905,23 +940,14 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n> \n> void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_DIGITAL_GAIN) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find digital gain control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tctrls.set(V4L2_CID_DIGITAL_GAIN,\n> \t\t static_cast(dgStatus->digital_gain * 1000));\n> }\n> \n> void IPARPi::applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_CC_MATRIX) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find CCM control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tbcm2835_isp_custom_ccm ccm;\n> +\n> \tfor (int i = 0; i < 9; i++) {\n> \t\tccm.ccm.ccm[i / 3][i % 3].den = 1000;\n> \t\tccm.ccm.ccm[i / 3][i % 3].num = 1000 * ccmStatus->matrix[i];\n> @@ -937,12 +963,8 @@ void IPARPi::applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls)\n> \n> void IPARPi::applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_GAMMA) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find Gamma control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tstruct bcm2835_isp_gamma gamma;\n> +\n> \tgamma.enabled = 1;\n> \tfor (int i = 0; i < CONTRAST_NUM_POINTS; i++) {\n> \t\tgamma.x[i] = contrastStatus->points[i].x;\n> @@ -956,12 +978,8 @@ void IPARPi::applyGamma(const struct ContrastStatus *contrastStatus, ControlList\n> \n> void IPARPi::applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_BLACK_LEVEL) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find black level control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tbcm2835_isp_black_level blackLevel;\n> +\n> \tblackLevel.enabled = 1;\n> \tblackLevel.black_level_r = blackLevelStatus->black_level_r;\n> \tblackLevel.black_level_g = blackLevelStatus->black_level_g;\n> @@ -974,12 +992,8 @@ void IPARPi::applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, Co\n> \n> void IPARPi::applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_GEQ) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find geq control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tbcm2835_isp_geq geq;\n> +\n> \tgeq.enabled = 1;\n> \tgeq.offset = geqStatus->offset;\n> \tgeq.slope.den = 1000;\n> @@ -992,12 +1006,8 @@ void IPARPi::applyGEQ(const struct GeqStatus *geqStatus, ControlList &ctrls)\n> \n> void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_DENOISE) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find denoise control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tbcm2835_isp_denoise denoise;\n> +\n> \tdenoise.enabled = 1;\n> \tdenoise.constant = denoiseStatus->noise_constant;\n> \tdenoise.slope.num = 1000 * denoiseStatus->noise_slope;\n> @@ -1012,12 +1022,8 @@ void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus, ControlList &ct\n> \n> void IPARPi::applySharpen(const struct SharpenStatus *sharpenStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_SHARPEN) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find sharpen control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tbcm2835_isp_sharpen sharpen;\n> +\n> \tsharpen.enabled = 1;\n> \tsharpen.threshold.num = 1000 * sharpenStatus->threshold;\n> \tsharpen.threshold.den = 1000;\n> @@ -1033,12 +1039,8 @@ void IPARPi::applySharpen(const struct SharpenStatus *sharpenStatus, ControlList\n> \n> void IPARPi::applyDPC(const struct DpcStatus *dpcStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_DPC) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find DPC control\";\n> -\t\treturn;\n> -\t}\n> -\n> \tbcm2835_isp_dpc dpc;\n> +\n> \tdpc.enabled = 1;\n> \tdpc.strength = dpcStatus->strength;\n> \n> @@ -1049,11 +1051,6 @@ void IPARPi::applyDPC(const struct DpcStatus *dpcStatus, ControlList &ctrls)\n> \n> void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls)\n> {\n> -\tif (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING) == ispCtrls_.end()) {\n> -\t\tLOG(IPARPI, Error) << \"Can't find LS control\";\n> -\t\treturn;\n> -\t}\n> -\n> \t/*\n> \t * Program lens shading tables into pipeline.\n> \t * Choose smallest cell size that won't exceed 63x48 cells.","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 21434BD80A\n\tfor ;\n\tFri, 11 Dec 2020 21:00:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 820E067FB2;\n\tFri, 11 Dec 2020 22:00:52 +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 B490E608A7\n\tfor ;\n\tFri, 11 Dec 2020 22:00:51 +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 3771999;\n\tFri, 11 Dec 2020 22:00:51 +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=\"Yx+2/pR1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1607720451;\n\tbh=QSZUEJPpDdavMZGLxJIxeGNPl3optesT0rczOwvq2Uk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Yx+2/pR1TZRDAive7vRTcTFI4ZECOTYztlzQv1myQnwQYwdRQWRYvHuKbyt/UmiaV\n\tbEPn6etkn7tSplBTPFhnt7Oru4iijp8KI0IImgb/uyNi5xh0yN1TLgByf5KH25PHTK\n\tWr5B2wcUiG3iBoZc9VyytvFRH6xatnkDbQ2Dhw1I=","Date":"Fri, 11 Dec 2020 23:00:45 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20201209095339.4107-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201209095339.4107-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: Only validate ISP\n\tand Unicam controls during configure","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14228,"web_url":"https://patchwork.libcamera.org/comment/14228/","msgid":"","date":"2020-12-11T21:41:16","subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: Only validate ISP\n\tand Unicam controls during configure","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your review comments.\n\n\n\n\nOn Fri, 11 Dec 2020, 9:00 pm Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Wed, Dec 09, 2020 at 09:53:39AM +0000, Naushir Patuck wrote:\n> > There is no need to validate all the ISP and Unicam V4L2 controls on\n> > every frame. Simply validate them once during the IPA configuration, and\n> > fail if a required control is not available.\n> >\n> > Also add some whitespace for readability.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/raspberrypi/raspberrypi.cpp | 131 ++++++++++++++--------------\n> > 1 file changed, 64 insertions(+), 67 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 60cfdc27..e5d2b41e 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -92,6 +92,8 @@ public:\n> >\n> > private:\n> > void setMode(const CameraSensorInfo &sensorInfo);\n> > + bool validateUnicamControls();\n> > + bool validateIspControls();\n> > void queueRequest(const ControlList &controls);\n> > void returnEmbeddedBuffer(unsigned int bufferId);\n> > void prepareISP(unsigned int bufferId);\n> > @@ -238,6 +240,16 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> > unicamCtrls_ = entityControls.at(0);\n> > ispCtrls_ = entityControls.at(1);\n> >\n> > + if (!validateUnicamControls()) {\n> > + LOG(IPARPI, Error) << \"Unicam control validation failed.\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + if (!validateIspControls()) {\n> > + LOG(IPARPI, Error) << \"ISP control validation failed.\";\n> > + return -EINVAL;\n> > + }\n>\n> configure() is a void function. Rebase issue ?\n>\n\nI'm sorry I really should have withdrawn this patch. It was based off your\nchanges for returning error codes from configure(). I jumped the gun a bit\nin submitting this.\n\nNow that we are using my alternate mechanism, I will resubmit with the\nappropriate changes. I will also address the points below.\n\nRegards,\nNaush\n\n\n> > +\n> > /* Setup a metadata ControlList to output metadata. */\n> > libcameraMetadata_ = ControlList(controls::controls);\n> >\n> > @@ -473,6 +485,51 @@ void IPARPi::reportMetadata()\n> > }\n> > }\n> >\n> > +bool IPARPi::validateUnicamControls()\n> > +{\n> > + const uint32_t ctrls[] = {\n>\n> static const ? Same below.\n>\n> > + V4L2_CID_ANALOGUE_GAIN,\n> > + V4L2_CID_EXPOSURE\n> > + };\n> > +\n> > + for (auto c : ctrls) {\n> > + if (unicamCtrls_.find(c) == unicamCtrls_.end()) {\n> > + LOG(IPARPI, Error) << \"Unable to find Unicam\n> control \"\n> > + << c;\n>\n> I'd use utils::hex(c) as otherwise the value is hard to read. Same\n> below.\n>\n> > + return false;\n> > + }\n> > + }\n> > +\n> > + return true;\n> > +}\n>\n> Isn't this sensor controls, not unicam controls ? While at it, we could\n> rename unicamCtrls_ to sensorCtrls_.\n>\n> The rest looks good to me.\n>\n> > +\n> > +bool IPARPi::validateIspControls()\n> > +{\n> > + const uint32_t ctrls[] = {\n> > + V4L2_CID_RED_BALANCE,\n> > + V4L2_CID_BLUE_BALANCE,\n> > + V4L2_CID_DIGITAL_GAIN,\n> > + V4L2_CID_USER_BCM2835_ISP_CC_MATRIX,\n> > + V4L2_CID_USER_BCM2835_ISP_GAMMA,\n> > + V4L2_CID_USER_BCM2835_ISP_BLACK_LEVEL,\n> > + V4L2_CID_USER_BCM2835_ISP_GEQ,\n> > + V4L2_CID_USER_BCM2835_ISP_DENOISE,\n> > + V4L2_CID_USER_BCM2835_ISP_SHARPEN,\n> > + V4L2_CID_USER_BCM2835_ISP_DPC,\n> > + V4L2_CID_USER_BCM2835_ISP_LENS_SHADING\n> > + };\n> > +\n> > + for (auto c : ctrls) {\n> > + if (ispCtrls_.find(c) == ispCtrls_.end()) {\n> > + LOG(IPARPI, Error) << \"Unable to find ISP control \"\n> > + << c;\n> > + return false;\n> > + }\n> > + }\n> > +\n> > + return true;\n> > +}\n> > +\n> > /*\n> > * Converting between enums (used in the libcamera API) and the names\n> that\n> > * we use to identify different modes. Unfortunately, the conversion\n> tables\n> > @@ -858,18 +915,6 @@ void IPARPi::processStats(unsigned int bufferId)\n> >\n> > void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList\n> &ctrls)\n> > {\n> > - const auto gainR = ispCtrls_.find(V4L2_CID_RED_BALANCE);\n> > - if (gainR == ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find red gain control\";\n> > - return;\n> > - }\n> > -\n> > - const auto gainB = ispCtrls_.find(V4L2_CID_BLUE_BALANCE);\n> > - if (gainB == ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find blue gain control\";\n> > - return;\n> > - }\n> > -\n> > LOG(IPARPI, Debug) << \"Applying WB R: \" << awbStatus->gain_r << \"\n> B: \"\n> > << awbStatus->gain_b;\n> >\n> > @@ -884,16 +929,6 @@ void IPARPi::applyAGC(const struct AgcStatus\n> *agcStatus, ControlList &ctrls)\n> > int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n> > int32_t exposureLines =\n> helper_->ExposureLines(agcStatus->shutter_time);\n> >\n> > - if (unicamCtrls_.find(V4L2_CID_ANALOGUE_GAIN) ==\n> unicamCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find analogue gain control\";\n> > - return;\n> > - }\n> > -\n> > - if (unicamCtrls_.find(V4L2_CID_EXPOSURE) == unicamCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find exposure control\";\n> > - return;\n> > - }\n> > -\n> > LOG(IPARPI, Debug) << \"Applying AGC Exposure: \" <<\n> agcStatus->shutter_time\n> > << \" (Shutter lines: \" << exposureLines << \")\n> Gain: \"\n> > << agcStatus->analogue_gain << \" (Gain Code: \"\n> > @@ -905,23 +940,14 @@ void IPARPi::applyAGC(const struct AgcStatus\n> *agcStatus, ControlList &ctrls)\n> >\n> > void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList\n> &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_DIGITAL_GAIN) == ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find digital gain control\";\n> > - return;\n> > - }\n> > -\n> > ctrls.set(V4L2_CID_DIGITAL_GAIN,\n> > static_cast(dgStatus->digital_gain * 1000));\n> > }\n> >\n> > void IPARPi::applyCCM(const struct CcmStatus *ccmStatus, ControlList\n> &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_CC_MATRIX) ==\n> ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find CCM control\";\n> > - return;\n> > - }\n> > -\n> > bcm2835_isp_custom_ccm ccm;\n> > +\n> > for (int i = 0; i < 9; i++) {\n> > ccm.ccm.ccm[i / 3][i % 3].den = 1000;\n> > ccm.ccm.ccm[i / 3][i % 3].num = 1000 *\n> ccmStatus->matrix[i];\n> > @@ -937,12 +963,8 @@ void IPARPi::applyCCM(const struct CcmStatus\n> *ccmStatus, ControlList &ctrls)\n> >\n> > void IPARPi::applyGamma(const struct ContrastStatus *contrastStatus,\n> ControlList &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_GAMMA) ==\n> ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find Gamma control\";\n> > - return;\n> > - }\n> > -\n> > struct bcm2835_isp_gamma gamma;\n> > +\n> > gamma.enabled = 1;\n> > for (int i = 0; i < CONTRAST_NUM_POINTS; i++) {\n> > gamma.x[i] = contrastStatus->points[i].x;\n> > @@ -956,12 +978,8 @@ void IPARPi::applyGamma(const struct ContrastStatus\n> *contrastStatus, ControlList\n> >\n> > void IPARPi::applyBlackLevel(const struct BlackLevelStatus\n> *blackLevelStatus, ControlList &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_BLACK_LEVEL) ==\n> ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find black level control\";\n> > - return;\n> > - }\n> > -\n> > bcm2835_isp_black_level blackLevel;\n> > +\n> > blackLevel.enabled = 1;\n> > blackLevel.black_level_r = blackLevelStatus->black_level_r;\n> > blackLevel.black_level_g = blackLevelStatus->black_level_g;\n> > @@ -974,12 +992,8 @@ void IPARPi::applyBlackLevel(const struct\n> BlackLevelStatus *blackLevelStatus, Co\n> >\n> > void IPARPi::applyGEQ(const struct GeqStatus *geqStatus, ControlList\n> &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_GEQ) ==\n> ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find geq control\";\n> > - return;\n> > - }\n> > -\n> > bcm2835_isp_geq geq;\n> > +\n> > geq.enabled = 1;\n> > geq.offset = geqStatus->offset;\n> > geq.slope.den = 1000;\n> > @@ -992,12 +1006,8 @@ void IPARPi::applyGEQ(const struct GeqStatus\n> *geqStatus, ControlList &ctrls)\n> >\n> > void IPARPi::applyDenoise(const struct SdnStatus *denoiseStatus,\n> ControlList &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_DENOISE) ==\n> ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find denoise control\";\n> > - return;\n> > - }\n> > -\n> > bcm2835_isp_denoise denoise;\n> > +\n> > denoise.enabled = 1;\n> > denoise.constant = denoiseStatus->noise_constant;\n> > denoise.slope.num = 1000 * denoiseStatus->noise_slope;\n> > @@ -1012,12 +1022,8 @@ void IPARPi::applyDenoise(const struct SdnStatus\n> *denoiseStatus, ControlList &ct\n> >\n> > void IPARPi::applySharpen(const struct SharpenStatus *sharpenStatus,\n> ControlList &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_SHARPEN) ==\n> ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find sharpen control\";\n> > - return;\n> > - }\n> > -\n> > bcm2835_isp_sharpen sharpen;\n> > +\n> > sharpen.enabled = 1;\n> > sharpen.threshold.num = 1000 * sharpenStatus->threshold;\n> > sharpen.threshold.den = 1000;\n> > @@ -1033,12 +1039,8 @@ void IPARPi::applySharpen(const struct\n> SharpenStatus *sharpenStatus, ControlList\n> >\n> > void IPARPi::applyDPC(const struct DpcStatus *dpcStatus, ControlList\n> &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_DPC) ==\n> ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find DPC control\";\n> > - return;\n> > - }\n> > -\n> > bcm2835_isp_dpc dpc;\n> > +\n> > dpc.enabled = 1;\n> > dpc.strength = dpcStatus->strength;\n> >\n> > @@ -1049,11 +1051,6 @@ void IPARPi::applyDPC(const struct DpcStatus\n> *dpcStatus, ControlList &ctrls)\n> >\n> > void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList\n> &ctrls)\n> > {\n> > - if (ispCtrls_.find(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING) ==\n> ispCtrls_.end()) {\n> > - LOG(IPARPI, Error) << \"Can't find LS control\";\n> > - return;\n> > - }\n> > -\n> > /*\n> > * Program lens shading tables into pipeline.\n> > * Choose smallest cell size that won't exceed 63x48 cells.\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 3F7D4BD80A\n\tfor ;\n\tFri, 11 Dec 2020 21:41:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB72A67FB0;\n\tFri, 11 Dec 2020 22:41:30 +0100 (CET)","from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com\n\t[IPv6:2a00:1450:4864:20::12f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 72196608A7\n\tfor ;\n\tFri, 11 Dec 2020 22:41:29 +0100 (CET)","by mail-lf1-x12f.google.com with SMTP id h19so15265552lfc.12\n\tfor ;\n\tFri, 11 Dec 2020 13:41:29 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"FeDjBR9t\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=nNIQRKpWxfuXluO7WYirHt1KP9FDZ1ppMhtHrmGiCsE=;\n\tb=FeDjBR9tW31skywUfry4Ah2Gf/7C+2N5UpmkUVUS4JjX59jU3RVglOn59wGIp/EeNH\n\tA2cIsw4GR5dzr4wn82mbh5I8TJDLe81teMcjT1xkpTu2qQpTKHFCnLSbIl45znfR0uAL\n\tRCdwxwSmu+onPl283Qs8amV0RkYzs20o99vISf/z8CdWswWDUZpMhQJKHXMAT4s4xV/0\n\tg7+xO9B22me+occZ123RpvLYt1B8f8DHtw+VUG+zMAzDXvCSSrspS72aYt7spoB35ZtY\n\tPnXCirYvPrIQ3kX6xLFXAoh/08733UiKAXhQVnBqprYI1103ILc4HCkONsHQ30Fsq3CQ\n\t9R8A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=nNIQRKpWxfuXluO7WYirHt1KP9FDZ1ppMhtHrmGiCsE=;\n\tb=XYcNKFg6bvVVs4i8i6Ggdua3PKA/6yOq01NT5G6QOf/K+m69tGtL43nwmrgEsij8sh\n\tMtldRGiXc64PVMNDuOq9IYg+bIMNzJh7TqVPtt0drmMuo6M4G8DXa//1WVR8gKKdCfHL\n\t+E8mYieV07LcRiJUZd0IDNQau6kScCFmj1Droe6q8P6bL/zcJmGfJqXvdTIFo0z9YuQk\n\tLVUkQrSoBXUTBDIwk2vMwIi6zTOYT7hHnLcW54dBD38ydtggf2DHHmi4JrhTmDjr6jDX\n\tKUFSY++WmLZALbxZh1PyI+LGLY44AR7zK4vRcTZHbaMYjvC9nRMf/HEt6ODTBfxMy9WC\n\tYcpw==","X-Gm-Message-State":"AOAM532z4EylJTC8Rly2cOvwVdDq9f30ZUFEL315STWhhnWWsuzaF5Bl\n\trqLUOnVx8DcxvdBcsE8P6U3eRrQtygE+qrWo+ifOcKWwe1w=","X-Google-Smtp-Source":"ABdhPJy+9nUaNYRQQjpkZ2X5VX8KpirqeymbXCAWBCfB8Fu0ykKkOLWCP8RY8oY+XbCPg4vD9JemuTcJ+Ifl+BY4hF4=","X-Received":"by 2002:a19:5ca:: with SMTP id 193mr5456704lff.375.1607722888603;\n\tFri, 11 Dec 2020 13:41:28 -0800 (PST)","MIME-Version":"1.0","References":"<20201209095339.4107-1-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 11 Dec 2020 21:41:16 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: Only validate ISP\n\tand Unicam controls during configure","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============5644686513368184607==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]