[{"id":26305,"web_url":"https://patchwork.libcamera.org/comment/26305/","msgid":"<Y82GXySn70CIPzy/@pendragon.ideasonboard.com>","date":"2023-01-22T18:54:23","subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","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 Thu, Jan 19, 2023 at 10:45:34AM +0000, Naushir Patuck via libcamera-devel wrote:\n> Pass the available lens controls to the IPA through the configure() function.\n> Validate that the V4L2_CID_FOCUS_ABSOLUTE does exist. If it doesn't, log a\n> warning message, and do not advertise focus related controls from the IPA.\n\nShouldn't this be done at init() time instead of configure() time ? I\ndon't suppose the presense of a lens controller could change between\ndifferent configurations.\n\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> Reviewed-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n>  src/ipa/raspberrypi/raspberrypi.cpp           | 21 +++++++++++++++++++\n>  .../pipeline/raspberrypi/raspberrypi.cpp      |  2 ++\n>  3 files changed, 24 insertions(+)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index 2a4821fbc0ef..bfacd1275bfb 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -38,6 +38,7 @@ struct IPAConfig {\n>  \tlibcamera.SharedFD lsTableHandle;\n>  \tlibcamera.ControlInfoMap sensorControls;\n>  \tlibcamera.ControlInfoMap ispControls;\n> +\tlibcamera.ControlInfoMap lensControls;\n>  };\n>  \n>  struct IPAConfigResult {\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index aa18ed750370..bbf3c7dc4a69 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -131,6 +131,7 @@ private:\n>  \tvoid setMode(const IPACameraSensorInfo &sensorInfo);\n>  \tbool validateSensorControls();\n>  \tbool validateIspControls();\n> +\tbool validateLensControls();\n>  \tvoid queueRequest(const ControlList &controls);\n>  \tvoid returnEmbeddedBuffer(unsigned int bufferId);\n>  \tvoid prepareISP(const ISPConfig &data);\n> @@ -155,6 +156,7 @@ private:\n>  \n>  \tControlInfoMap sensorCtrls_;\n>  \tControlInfoMap ispCtrls_;\n> +\tControlInfoMap lensCtrls_;\n>  \tbool lensPresent_;\n>  \tControlList libcameraMetadata_;\n>  \n> @@ -394,6 +396,15 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip\n>  \t\treturn -1;\n>  \t}\n>  \n> +\tif (lensPresent_) {\n> +\t\tlensCtrls_ = ipaConfig.lensControls;\n> +\t\tif (!validateLensControls()) {\n> +\t\t\tLOG(IPARPI, Warning) << \"Lens validation failed, \"\n> +\t\t\t\t\t     << \"no lens control will be available.\";\n> +\t\t\tlensPresent_ = false;\n> +\t\t}\n> +\t}\n> +\n>  \tmaxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();\n>  \n>  \t/* Setup a metadata ControlList to output metadata. */\n> @@ -648,6 +659,16 @@ bool IPARPi::validateIspControls()\n>  \treturn true;\n>  }\n>  \n> +bool IPARPi::validateLensControls()\n> +{\n> +\tif (lensCtrls_.find(V4L2_CID_FOCUS_ABSOLUTE) == lensCtrls_.end()) {\n> +\t\tLOG(IPARPI, Error) << \"Unable to find Lens control V4L2_CID_FOCUS_ABSOLUTE\";\n> +\t\treturn false;\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> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 9dd36cbaea78..249dedcd8c09 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1617,6 +1617,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::IPA\n>  \n>  \tipaConfig.sensorControls = sensor_->controls();\n>  \tipaConfig.ispControls = isp_[Isp::Input].dev()->controls();\n> +\tif (sensor_->focusLens())\n> +\t\tipaConfig.lensControls = sensor_->focusLens()->controls();\n>  \n>  \t/* Always send the user transform to the IPA. */\n>  \tipaConfig.transform = static_cast<unsigned int>(config->transform);","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 58049BDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 22 Jan 2023 18:54:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B01B4625DF;\n\tSun, 22 Jan 2023 19:54:28 +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 75847625DD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 22 Jan 2023 19:54:26 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCFE1308;\n\tSun, 22 Jan 2023 19:54:25 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674413668;\n\tbh=W1Xqzgm7QG/dQIHjbJKBtTL3TxG0C/fBQgvsxgLCNeI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=DTRlZcBHfwkR4wbVphI553eO3UNFZhocHu3MX6AZZkGsbtDFeKh3yckqDGv2spQVX\n\tuVG/7pzGFf558OYlSVvPiz+WSTgPNMRgOAbQPJIO9yzGye1NzLRgNYA4+1AGjQT/O1\n\trpqv9YWRvnuTsPm+CjkBgXzMr07+RhIb7bMp6pLUAnsKw2OQAchOLxjnMfhc1lfkoC\n\t89+xoV63qbYcTwln6O67GEmN52TpuqqlJTo7Ne9JQYPwFrS3mVn54ujJ3s7QH5Y/mH\n\tn4Y7hWQUI+z/7kPFSvzosgFMvC9knWzBVRmFfpDW+nXZPRSjeNz7CTmbcC9G7KNcu1\n\tRfYcRyNNpocag==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674413666;\n\tbh=W1Xqzgm7QG/dQIHjbJKBtTL3TxG0C/fBQgvsxgLCNeI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AzbG8lBuD0VUUx3u78TJ0Y99uU9bquhXp0Vebo12GVoxebheatZcHqQ1fqAV0z8SJ\n\t5pdFj8TFmtruIdbrhK5lWXtVTqX7NY+k5sCmoYVvfCUzPBR+jguj2N+Iknov1+eytd\n\tkI5CB1M3EEb8clG/w51vPCp/h01pcSAz5FLhTglc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AzbG8lBu\"; dkim-atps=neutral","Date":"Sun, 22 Jan 2023 20:54:23 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<Y82GXySn70CIPzy/@pendragon.ideasonboard.com>","References":"<20230119104544.9456-1-naush@raspberrypi.com>\n\t<20230119104544.9456-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230119104544.9456-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":26320,"web_url":"https://patchwork.libcamera.org/comment/26320/","msgid":"<CAEmqJPprc26qed9GpiGLmrWnmBLsxo0RLqEixruz8Vn-T0iyqg@mail.gmail.com>","date":"2023-01-23T09:02:59","subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","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 feedback!\n\nOn Sun, 22 Jan 2023 at 18:54, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Thu, Jan 19, 2023 at 10:45:34AM +0000, Naushir Patuck via\n> libcamera-devel wrote:\n> > Pass the available lens controls to the IPA through the configure()\n> function.\n> > Validate that the V4L2_CID_FOCUS_ABSOLUTE does exist. If it doesn't, log\n> a\n> > warning message, and do not advertise focus related controls from the\n> IPA.\n>\n> Shouldn't this be done at init() time instead of configure() time ? I\n> don't suppose the presense of a lens controller could change between\n> different configurations.\n>\n\nYes, this would be possible to do at init() time.  I put it in configure()\nto\nbe consistent with the sensor and ISP control validation.  I doubt lens\ncontrols would change between configure() calls, so happy to move it to\ninit()\nif folks prefer that.\n\nRegards,\nNaush\n\n\n>\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > Reviewed-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>\n> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n> >  src/ipa/raspberrypi/raspberrypi.cpp           | 21 +++++++++++++++++++\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      |  2 ++\n> >  3 files changed, 24 insertions(+)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.mojom\n> b/include/libcamera/ipa/raspberrypi.mojom\n> > index 2a4821fbc0ef..bfacd1275bfb 100644\n> > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > @@ -38,6 +38,7 @@ struct IPAConfig {\n> >       libcamera.SharedFD lsTableHandle;\n> >       libcamera.ControlInfoMap sensorControls;\n> >       libcamera.ControlInfoMap ispControls;\n> > +     libcamera.ControlInfoMap lensControls;\n> >  };\n> >\n> >  struct IPAConfigResult {\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index aa18ed750370..bbf3c7dc4a69 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -131,6 +131,7 @@ private:\n> >       void setMode(const IPACameraSensorInfo &sensorInfo);\n> >       bool validateSensorControls();\n> >       bool validateIspControls();\n> > +     bool validateLensControls();\n> >       void queueRequest(const ControlList &controls);\n> >       void returnEmbeddedBuffer(unsigned int bufferId);\n> >       void prepareISP(const ISPConfig &data);\n> > @@ -155,6 +156,7 @@ private:\n> >\n> >       ControlInfoMap sensorCtrls_;\n> >       ControlInfoMap ispCtrls_;\n> > +     ControlInfoMap lensCtrls_;\n> >       bool lensPresent_;\n> >       ControlList libcameraMetadata_;\n> >\n> > @@ -394,6 +396,15 @@ int IPARPi::configure(const IPACameraSensorInfo\n> &sensorInfo, const IPAConfig &ip\n> >               return -1;\n> >       }\n> >\n> > +     if (lensPresent_) {\n> > +             lensCtrls_ = ipaConfig.lensControls;\n> > +             if (!validateLensControls()) {\n> > +                     LOG(IPARPI, Warning) << \"Lens validation failed, \"\n> > +                                          << \"no lens control will be\n> available.\";\n> > +                     lensPresent_ = false;\n> > +             }\n> > +     }\n> > +\n> >       maxSensorGainCode_ =\n> sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();\n> >\n> >       /* Setup a metadata ControlList to output metadata. */\n> > @@ -648,6 +659,16 @@ bool IPARPi::validateIspControls()\n> >       return true;\n> >  }\n> >\n> > +bool IPARPi::validateLensControls()\n> > +{\n> > +     if (lensCtrls_.find(V4L2_CID_FOCUS_ABSOLUTE) == lensCtrls_.end()) {\n> > +             LOG(IPARPI, Error) << \"Unable to find Lens control\n> V4L2_CID_FOCUS_ABSOLUTE\";\n> > +             return false;\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> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 9dd36cbaea78..249dedcd8c09 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1617,6 +1617,8 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config, ipa::RPi::IPA\n> >\n> >       ipaConfig.sensorControls = sensor_->controls();\n> >       ipaConfig.ispControls = isp_[Isp::Input].dev()->controls();\n> > +     if (sensor_->focusLens())\n> > +             ipaConfig.lensControls = sensor_->focusLens()->controls();\n> >\n> >       /* Always send the user transform to the IPA. */\n> >       ipaConfig.transform = static_cast<unsigned int>(config->transform);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 16011BDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Jan 2023 09:03:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D214625DF;\n\tMon, 23 Jan 2023 10:03:18 +0100 (CET)","from mail-yb1-xb34.google.com (mail-yb1-xb34.google.com\n\t[IPv6:2607:f8b0:4864:20::b34])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D38AF603C0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Jan 2023 10:03:15 +0100 (CET)","by mail-yb1-xb34.google.com with SMTP id c124so13848405ybb.13\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Jan 2023 01:03:15 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674464598;\n\tbh=yFtCVpWPidkD422gsvqK5Ocm20cv7uK/dYT74HUKEs0=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=maqeAW+HZTlvj6mSF2Gddk8Fl+M8CGaRNSmSulmQMKMNmTWsX6lav+9c05LwaXiJa\n\tIxY6aMrjURhNaksPgx/ARJrMi2Pl3k71LLA6+LTRgtnQqQR0w63brW4fJjsFveoC+l\n\tUTFRUuRYy9cT49BLx4Y+wj+GX48EwAWjgAoYyQdGx8Z99uR39ysAA0fAuaUR0mOu2Z\n\t9JSSqz0GccJ9Zy8eQcfvdLyvt0rbDVYBCYWa8o80KWSafXMTwc7Jaf0XQVTErwBXT9\n\tAfy3diYNTX8zOrqxy1N8neK4PVi8qAtcqaNn0/n1FBylo62lnKTpCMjlMknwwTs0Zz\n\tcNOwsjgZjs5Sg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=M2KAwfwKb/IpsWICdxceHL6vnLX/AnodlsRJ27IjPTU=;\n\tb=dFsKdilU9IdTnIbE99lPN3QPkezVQzh9CvTzWAWIt2oKZLSSu800Vih0XVw6/ufwrW\n\t2avCpLj7PbXtFlKlXhKAQLwGdxIhMWeSBlRkZuY1aPv2TTlR6J/1QiGPjEeHVQAi3mQY\n\tzu9Ia0e6KvmgLk05k9HKgqOFZ2+KCtwF678I930JG+qxRRjVLfKArK5lOj1IJY0zjPAX\n\te0I/DIPaFUk44ofNyjPfWn2lqtXzaTH3mk/RDomN1wblBjy8fJZxlZhWqM2cgvhQ/tZq\n\tYVCaD9c3Cj2Ep/uWbb8HuzTbgYYX/7nnXRHZZmZwwwCGMx76mMibhg3YMlbOEik6FwIV\n\tx9IQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"dFsKdilU\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=M2KAwfwKb/IpsWICdxceHL6vnLX/AnodlsRJ27IjPTU=;\n\tb=JB0EKejd60Udv3336u+TtERtZhG2J6IKtkTTtg/m1U87RSq7RvJ7vImnR32iWQVYmK\n\tKMeLD/VhAdzdNZ484DidNAr3mPXTVgRTV+ZYvhKCA2pYl79fLeWF5oL0QXOOSGTbQZtR\n\tO4Hdda/hJnybV7wMLvad1o/BQPHKT8dRwmx53cROxid35ZGbjia+FgOn0/h19jhgvLXk\n\tHjHWV55pz4ehm0qsOkvZNOE5pDISBP5sIV/RWeRb4+Sef21/rj5qVPY1KZuyu+skyEWw\n\t6M5XRXLMUZWpKmr08LIVC3qd5V9kzQE9KBVUgp6xniEtFv5Gve8G+p6oasGBbPm5gB1m\n\tC1GA==","X-Gm-Message-State":"AFqh2kqgXZdg4dPL6hjO3uHb7gOpbx4tns7ZHsevSLktoT2PEHR6fd9v\n\tGQrPWo1Kh7gC7D3s9aMHhuG/++/YocX2VuiRi5OWlYmYNz6ANcDcvIw=","X-Google-Smtp-Source":"AMrXdXuwpzjCVvZt4tMDuHfIIlsZ03Rhzavj9qI910FrhCz8Iat90iBhD4G709zOEDAWYQJFMgIp2MEENdPi0B/he7U=","X-Received":"by 2002:a25:1984:0:b0:7fe:e7f5:e228 with SMTP id\n\t126-20020a251984000000b007fee7f5e228mr1029272ybz.582.1674464594532;\n\tMon, 23 Jan 2023 01:03:14 -0800 (PST)","MIME-Version":"1.0","References":"<20230119104544.9456-1-naush@raspberrypi.com>\n\t<20230119104544.9456-5-naush@raspberrypi.com>\n\t<Y82GXySn70CIPzy/@pendragon.ideasonboard.com>","In-Reply-To":"<Y82GXySn70CIPzy/@pendragon.ideasonboard.com>","Date":"Mon, 23 Jan 2023 09:02:59 +0000","Message-ID":"<CAEmqJPprc26qed9GpiGLmrWnmBLsxo0RLqEixruz8Vn-T0iyqg@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"00000000000058aee405f2eaab49\"","Subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":26323,"web_url":"https://patchwork.libcamera.org/comment/26323/","msgid":"<Y85QxfLyqWASBqFr@pendragon.ideasonboard.com>","date":"2023-01-23T09:17:57","subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Mon, Jan 23, 2023 at 09:02:59AM +0000, Naushir Patuck wrote:\n> On Sun, 22 Jan 2023 at 18:54, Laurent Pinchart wrote:\n> > On Thu, Jan 19, 2023 at 10:45:34AM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > Pass the available lens controls to the IPA through the configure() function.\n> > > Validate that the V4L2_CID_FOCUS_ABSOLUTE does exist. If it doesn't, log a\n> > > warning message, and do not advertise focus related controls from the IPA.\n> >\n> > Shouldn't this be done at init() time instead of configure() time ? I\n> > don't suppose the presense of a lens controller could change between\n> > different configurations.\n> \n> Yes, this would be possible to do at init() time.  I put it in configure() to\n> be consistent with the sensor and ISP control validation.  I doubt lens\n> controls would change between configure() calls, so happy to move it to init()\n> if folks prefer that.\n\nMaybe the sensor and ISP controls validation should also move to init()\ntime then ? :-) This can be done on top of this series, it's not a\nblocker.\n\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > Reviewed-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>\n> > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > > ---\n> > >  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n> > >  src/ipa/raspberrypi/raspberrypi.cpp           | 21 +++++++++++++++++++\n> > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  2 ++\n> > >  3 files changed, 24 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> > > index 2a4821fbc0ef..bfacd1275bfb 100644\n> > > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > > @@ -38,6 +38,7 @@ struct IPAConfig {\n> > >       libcamera.SharedFD lsTableHandle;\n> > >       libcamera.ControlInfoMap sensorControls;\n> > >       libcamera.ControlInfoMap ispControls;\n> > > +     libcamera.ControlInfoMap lensControls;\n> > >  };\n> > >\n> > >  struct IPAConfigResult {\n> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > index aa18ed750370..bbf3c7dc4a69 100644\n> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > @@ -131,6 +131,7 @@ private:\n> > >       void setMode(const IPACameraSensorInfo &sensorInfo);\n> > >       bool validateSensorControls();\n> > >       bool validateIspControls();\n> > > +     bool validateLensControls();\n> > >       void queueRequest(const ControlList &controls);\n> > >       void returnEmbeddedBuffer(unsigned int bufferId);\n> > >       void prepareISP(const ISPConfig &data);\n> > > @@ -155,6 +156,7 @@ private:\n> > >\n> > >       ControlInfoMap sensorCtrls_;\n> > >       ControlInfoMap ispCtrls_;\n> > > +     ControlInfoMap lensCtrls_;\n> > >       bool lensPresent_;\n> > >       ControlList libcameraMetadata_;\n> > >\n> > > @@ -394,6 +396,15 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip\n> > >               return -1;\n> > >       }\n> > >\n> > > +     if (lensPresent_) {\n> > > +             lensCtrls_ = ipaConfig.lensControls;\n> > > +             if (!validateLensControls()) {\n> > > +                     LOG(IPARPI, Warning) << \"Lens validation failed, \"\n> > > +                                          << \"no lens control will be available.\";\n> > > +                     lensPresent_ = false;\n> > > +             }\n> > > +     }\n> > > +\n> > >       maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();\n> > >\n> > >       /* Setup a metadata ControlList to output metadata. */\n> > > @@ -648,6 +659,16 @@ bool IPARPi::validateIspControls()\n> > >       return true;\n> > >  }\n> > >\n> > > +bool IPARPi::validateLensControls()\n> > > +{\n> > > +     if (lensCtrls_.find(V4L2_CID_FOCUS_ABSOLUTE) == lensCtrls_.end()) {\n> > > +             LOG(IPARPI, Error) << \"Unable to find Lens control V4L2_CID_FOCUS_ABSOLUTE\";\n> > > +             return false;\n> > > +     }\n> > > +\n> > > +     return 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> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 9dd36cbaea78..249dedcd8c09 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -1617,6 +1617,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::IPA\n> > >\n> > >       ipaConfig.sensorControls = sensor_->controls();\n> > >       ipaConfig.ispControls = isp_[Isp::Input].dev()->controls();\n> > > +     if (sensor_->focusLens())\n> > > +             ipaConfig.lensControls = sensor_->focusLens()->controls();\n> > >\n> > >       /* Always send the user transform to the IPA. */\n> > >       ipaConfig.transform = static_cast<unsigned int>(config->transform);","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 EBAC2BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Jan 2023 09:18:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CE83625E4;\n\tMon, 23 Jan 2023 10:18:03 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22337603C0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Jan 2023 10:18:01 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 839E5D6;\n\tMon, 23 Jan 2023 10:18:00 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674465483;\n\tbh=a+ZPpyVrXgA3F1l+bFkx0qhdzDS2dHi61aMrCKbhUhc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=B/T9JMSwHKpmXfeJvQFOU7XqP33bLBajiy/pLHJQOAqN8Gm8QtFCjulVzxscHkbcB\n\tOTJt5ftGYAi8GieW946YxiKXwjPfMxygheS6AuS4Y/daOq5Dos5i2i+m5nFEl/qc1f\n\t68qsgz8NEFjEq6AACTMGhMGqV1oaagIBsxaIef81ZLYeq87ctCoMOSTOgX1yJcv58H\n\tWTgjgy7uQRoH7GP1H3wzC3GVZS53xdBmwOup8TnZllQpVvcfaZazg516Z1atTJq4iK\n\tHZqn7GMdoWRxqjzhojWjP49wyfkZtTXk2Dt/TG9XFcDUZC5R+7dPhqA6nB+IzL8Jtl\n\t1bfdo0qj4VfUA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674465480;\n\tbh=a+ZPpyVrXgA3F1l+bFkx0qhdzDS2dHi61aMrCKbhUhc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XZLnYHe5iWZtJychHseQzz5XvF65jJ2Vze6Nv8Ynwsfrfp5kajJMmG577IiUeywKe\n\tfk1rW3O9Zwal4xkmb74pxf+Q9fs4dKhzkGWTCCd7rKZufH9ASo14w37vlJhioEQICZ\n\tpBMmmWm7xRo5uDjH3/prYZ7i8bcKwURGY5jbNQxk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"XZLnYHe5\"; dkim-atps=neutral","Date":"Mon, 23 Jan 2023 11:17:57 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<Y85QxfLyqWASBqFr@pendragon.ideasonboard.com>","References":"<20230119104544.9456-1-naush@raspberrypi.com>\n\t<20230119104544.9456-5-naush@raspberrypi.com>\n\t<Y82GXySn70CIPzy/@pendragon.ideasonboard.com>\n\t<CAEmqJPprc26qed9GpiGLmrWnmBLsxo0RLqEixruz8Vn-T0iyqg@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPprc26qed9GpiGLmrWnmBLsxo0RLqEixruz8Vn-T0iyqg@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":26324,"web_url":"https://patchwork.libcamera.org/comment/26324/","msgid":"<CAEmqJPpm02Fv=u4h1g-5P8ZxqnJ03eiXGe8i6yvkOvOEERujDg@mail.gmail.com>","date":"2023-01-23T09:22:10","subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Mon, 23 Jan 2023 at 09:18, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Mon, Jan 23, 2023 at 09:02:59AM +0000, Naushir Patuck wrote:\n> > On Sun, 22 Jan 2023 at 18:54, Laurent Pinchart wrote:\n> > > On Thu, Jan 19, 2023 at 10:45:34AM +0000, Naushir Patuck via\n> libcamera-devel wrote:\n> > > > Pass the available lens controls to the IPA through the configure()\n> function.\n> > > > Validate that the V4L2_CID_FOCUS_ABSOLUTE does exist. If it doesn't,\n> log a\n> > > > warning message, and do not advertise focus related controls from\n> the IPA.\n> > >\n> > > Shouldn't this be done at init() time instead of configure() time ? I\n> > > don't suppose the presense of a lens controller could change between\n> > > different configurations.\n> >\n> > Yes, this would be possible to do at init() time.  I put it in\n> configure() to\n> > be consistent with the sensor and ISP control validation.  I doubt lens\n> > controls would change between configure() calls, so happy to move it to\n> init()\n> > if folks prefer that.\n>\n> Maybe the sensor and ISP controls validation should also move to init()\n> time then ? :-) This can be done on top of this series, it's not a\n> blocker.\n>\n\nISP controls can (and possibly should) move to init, but sensor controls\nneed\nto remain in configure().  Things like vblank/hblank/exposure/gain limits\ncan\nand will change across modes.\n\nI'll make the change as part of this series, it's not going to be too\ndisruptive.\n\nRegards,\nNaush\n\n\n\n>\n> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > Reviewed-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>\n> > > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > ---\n> > > >  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n> > > >  src/ipa/raspberrypi/raspberrypi.cpp           | 21\n> +++++++++++++++++++\n> > > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  2 ++\n> > > >  3 files changed, 24 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/ipa/raspberrypi.mojom\n> b/include/libcamera/ipa/raspberrypi.mojom\n> > > > index 2a4821fbc0ef..bfacd1275bfb 100644\n> > > > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > > > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > > > @@ -38,6 +38,7 @@ struct IPAConfig {\n> > > >       libcamera.SharedFD lsTableHandle;\n> > > >       libcamera.ControlInfoMap sensorControls;\n> > > >       libcamera.ControlInfoMap ispControls;\n> > > > +     libcamera.ControlInfoMap lensControls;\n> > > >  };\n> > > >\n> > > >  struct IPAConfigResult {\n> > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > index aa18ed750370..bbf3c7dc4a69 100644\n> > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > @@ -131,6 +131,7 @@ private:\n> > > >       void setMode(const IPACameraSensorInfo &sensorInfo);\n> > > >       bool validateSensorControls();\n> > > >       bool validateIspControls();\n> > > > +     bool validateLensControls();\n> > > >       void queueRequest(const ControlList &controls);\n> > > >       void returnEmbeddedBuffer(unsigned int bufferId);\n> > > >       void prepareISP(const ISPConfig &data);\n> > > > @@ -155,6 +156,7 @@ private:\n> > > >\n> > > >       ControlInfoMap sensorCtrls_;\n> > > >       ControlInfoMap ispCtrls_;\n> > > > +     ControlInfoMap lensCtrls_;\n> > > >       bool lensPresent_;\n> > > >       ControlList libcameraMetadata_;\n> > > >\n> > > > @@ -394,6 +396,15 @@ int IPARPi::configure(const IPACameraSensorInfo\n> &sensorInfo, const IPAConfig &ip\n> > > >               return -1;\n> > > >       }\n> > > >\n> > > > +     if (lensPresent_) {\n> > > > +             lensCtrls_ = ipaConfig.lensControls;\n> > > > +             if (!validateLensControls()) {\n> > > > +                     LOG(IPARPI, Warning) << \"Lens validation\n> failed, \"\n> > > > +                                          << \"no lens control will\n> be available.\";\n> > > > +                     lensPresent_ = false;\n> > > > +             }\n> > > > +     }\n> > > > +\n> > > >       maxSensorGainCode_ =\n> sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();\n> > > >\n> > > >       /* Setup a metadata ControlList to output metadata. */\n> > > > @@ -648,6 +659,16 @@ bool IPARPi::validateIspControls()\n> > > >       return true;\n> > > >  }\n> > > >\n> > > > +bool IPARPi::validateLensControls()\n> > > > +{\n> > > > +     if (lensCtrls_.find(V4L2_CID_FOCUS_ABSOLUTE) ==\n> lensCtrls_.end()) {\n> > > > +             LOG(IPARPI, Error) << \"Unable to find Lens control\n> V4L2_CID_FOCUS_ABSOLUTE\";\n> > > > +             return false;\n> > > > +     }\n> > > > +\n> > > > +     return true;\n> > > > +}\n> > > > +\n> > > >  /*\n> > > >   * Converting between enums (used in the libcamera API) and the\n> names that\n> > > >   * we use to identify different modes. Unfortunately, the\n> conversion tables\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index 9dd36cbaea78..249dedcd8c09 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -1617,6 +1617,8 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config, ipa::RPi::IPA\n> > > >\n> > > >       ipaConfig.sensorControls = sensor_->controls();\n> > > >       ipaConfig.ispControls = isp_[Isp::Input].dev()->controls();\n> > > > +     if (sensor_->focusLens())\n> > > > +             ipaConfig.lensControls =\n> sensor_->focusLens()->controls();\n> > > >\n> > > >       /* Always send the user transform to the IPA. */\n> > > >       ipaConfig.transform = static_cast<unsigned\n> int>(config->transform);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 049FBBDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Jan 2023 09:22:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 73737603C0;\n\tMon, 23 Jan 2023 10:22:28 +0100 (CET)","from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com\n\t[IPv6:2607:f8b0:4864:20::b32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF6A0603C0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Jan 2023 10:22:26 +0100 (CET)","by mail-yb1-xb32.google.com with SMTP id a9so13976535ybb.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Jan 2023 01:22:26 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674465748;\n\tbh=Ei+lC+UGjrMiR6xBh3MD2+03mBFOuq6G5F/ETnELKEw=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tC3U0ySzYDHJZIC+3QIBSwU0haBiUoa+ts+mobhYq4lrInV2Cm8WveuokWJPFLQoJ\n\txRWdaLpsglErszmNQiSKvfV6kdCGEm82VXxwNfgRI0/9JGFo0J9Uxv4fnKJbdAk//V\n\t/rgMSmwB3evy6vU5PWOAlrwZGr7t2wzb6tneNmJmJf7bES/rovFQ/BZxjc8dWPTCur\n\tsebXNwDv1J1ZY68ozYiRa9yHojTExRWNLSniT9RzVqMKfKbFUZ0olU9IknJ1KPNhOF\n\teklsB3DZU7Ftq4xhfx+ZsSXbcnDFZO0xVrKyjH4k/8k/GLGwQ/urQiyVz/PLSpsEeg\n\tTl5r7PYZI9ypg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=h8Io+hzHMpJcjruhs9YDWnaiNXAd1bxxlr0WVFUzrT8=;\n\tb=dxxw23x1/IP85SBc/ewGXV0s8LBwDb14iGE8XF7vkvwOYe+IvxTwa7LHDrZGwVeety\n\tl+QL3hyGNd3Udvy6t+tpwYlBukVOw7Fp14+FTKmlHpcpkCDPTF4efDwaJOzXpyfPWGfK\n\tsjYQzrkiAPGOl9GYUpYjhaHI0GZVObcbFxaxIZgpEtO1rSKA7eevLGfZLXNtICjRDKWi\n\tCI5mQd65+/wu7/w0eCGlYK6O9LJftw2OzQFR9yU++9Wr5aRBKMJC0yWt8uDw21GJWwhK\n\tAyYwC0jD9zeKa9eKIqcHjn2vtzqsEVVI9DDK9upvne5AG0+JD7frd0VIujnTbo4+KbaO\n\tvNvQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"dxxw23x1\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=h8Io+hzHMpJcjruhs9YDWnaiNXAd1bxxlr0WVFUzrT8=;\n\tb=hIv0Gv8yqrP91y2gEV15c3SLRv+GXLwMn5+opiZHQGm6ETnkCNpUrw7FQnb1w+tsyZ\n\tdk3zVnv2FrHK7c1uTEFL5xn+8IKJ28xCyAcnMeKFKcXuXxvLU6Z2UQp4DPPABwxw3AtO\n\ttt6CLJqrHwrXlmv5SPHEGHj4RP/TtQfHhEySUTibquIHfye3wDH12H11zvehjThNub+a\n\t8z2LrnSZHYFoqFIHWra3JLLWB63xXcwUfDGjSYS+vjFzuc0fx54SRPUgEXXtySChcBac\n\tOjH8V5ht1r1pCnLatabjDzs+zTSVLwsOxq7AVVv6NF5BgTBN6QTdzL8JpouEdr1UT1kk\n\tLgxA==","X-Gm-Message-State":"AFqh2kq8/JGsa5MVte1VBN0SgYMV0XdZq2S6zjcYoZ+JGwtHFHxI8uFy\n\t3KN9+bifabr/fzCqdCgbSYeJL+G6/nVZi5IS1fBQWA==","X-Google-Smtp-Source":"AMrXdXuixKRgxTMyjkrhQx9Gjbweb6DqeNfPCXJ6xI1xKnehzSvWRcGlmWLyxHOlGkvnnmA9hzy+kurf9469Rtv9sZE=","X-Received":"by 2002:a25:8a91:0:b0:7ff:fd5f:f4c3 with SMTP id\n\th17-20020a258a91000000b007fffd5ff4c3mr1018714ybl.223.1674465745752;\n\tMon, 23 Jan 2023 01:22:25 -0800 (PST)","MIME-Version":"1.0","References":"<20230119104544.9456-1-naush@raspberrypi.com>\n\t<20230119104544.9456-5-naush@raspberrypi.com>\n\t<Y82GXySn70CIPzy/@pendragon.ideasonboard.com>\n\t<CAEmqJPprc26qed9GpiGLmrWnmBLsxo0RLqEixruz8Vn-T0iyqg@mail.gmail.com>\n\t<Y85QxfLyqWASBqFr@pendragon.ideasonboard.com>","In-Reply-To":"<Y85QxfLyqWASBqFr@pendragon.ideasonboard.com>","Date":"Mon, 23 Jan 2023 09:22:10 +0000","Message-ID":"<CAEmqJPpm02Fv=u4h1g-5P8ZxqnJ03eiXGe8i6yvkOvOEERujDg@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"000000000000f6f35005f2eaefae\"","Subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":26328,"web_url":"https://patchwork.libcamera.org/comment/26328/","msgid":"<CAEmqJPpJmZ4nfpZqgRiyeFj0BOyy3zdx5sXQFmXnMKE3d4CVLg@mail.gmail.com>","date":"2023-01-23T10:39:16","subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Mon, 23 Jan 2023 at 09:22, Naushir Patuck <naush@raspberrypi.com> wrote:\n\n> Hi Laurent,\n>\n> On Mon, 23 Jan 2023 at 09:18, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n>\n>> Hi Naush,\n>>\n>> On Mon, Jan 23, 2023 at 09:02:59AM +0000, Naushir Patuck wrote:\n>> > On Sun, 22 Jan 2023 at 18:54, Laurent Pinchart wrote:\n>> > > On Thu, Jan 19, 2023 at 10:45:34AM +0000, Naushir Patuck via\n>> libcamera-devel wrote:\n>> > > > Pass the available lens controls to the IPA through the configure()\n>> function.\n>> > > > Validate that the V4L2_CID_FOCUS_ABSOLUTE does exist. If it\n>> doesn't, log a\n>> > > > warning message, and do not advertise focus related controls from\n>> the IPA.\n>> > >\n>> > > Shouldn't this be done at init() time instead of configure() time ? I\n>> > > don't suppose the presense of a lens controller could change between\n>> > > different configurations.\n>> >\n>> > Yes, this would be possible to do at init() time.  I put it in\n>> configure() to\n>> > be consistent with the sensor and ISP control validation.  I doubt lens\n>> > controls would change between configure() calls, so happy to move it to\n>> init()\n>> > if folks prefer that.\n>>\n>> Maybe the sensor and ISP controls validation should also move to init()\n>> time then ? :-) This can be done on top of this series, it's not a\n>> blocker.\n>>\n>\n> ISP controls can (and possibly should) move to init, but sensor controls\n> need\n> to remain in configure().  Things like vblank/hblank/exposure/gain limits\n> can\n> and will change across modes.\n>\n> I'll make the change as part of this series, it's not going to be too\n> disruptive.\n>\n\nCan I backtrack on my statement please? :-)\n\nThere is a bit more restructuring involved to make this change -\nspecifically\nthe devices are not yet opened at the point of ipa init(), so there are no\ncontrols to be passed in.  Nothing too troublesome, but I think it makes\nsense\nto do this as a separate piece of work on top.\n\nNaush\n\n\n\n>\n> Regards,\n> Naush\n>\n>\n>\n>>\n>> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n>> > > > Reviewed-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>\n>> > > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n>> > > > ---\n>> > > >  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n>> > > >  src/ipa/raspberrypi/raspberrypi.cpp           | 21\n>> +++++++++++++++++++\n>> > > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  2 ++\n>> > > >  3 files changed, 24 insertions(+)\n>> > > >\n>> > > > diff --git a/include/libcamera/ipa/raspberrypi.mojom\n>> b/include/libcamera/ipa/raspberrypi.mojom\n>> > > > index 2a4821fbc0ef..bfacd1275bfb 100644\n>> > > > --- a/include/libcamera/ipa/raspberrypi.mojom\n>> > > > +++ b/include/libcamera/ipa/raspberrypi.mojom\n>> > > > @@ -38,6 +38,7 @@ struct IPAConfig {\n>> > > >       libcamera.SharedFD lsTableHandle;\n>> > > >       libcamera.ControlInfoMap sensorControls;\n>> > > >       libcamera.ControlInfoMap ispControls;\n>> > > > +     libcamera.ControlInfoMap lensControls;\n>> > > >  };\n>> > > >\n>> > > >  struct IPAConfigResult {\n>> > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n>> b/src/ipa/raspberrypi/raspberrypi.cpp\n>> > > > index aa18ed750370..bbf3c7dc4a69 100644\n>> > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>> > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>> > > > @@ -131,6 +131,7 @@ private:\n>> > > >       void setMode(const IPACameraSensorInfo &sensorInfo);\n>> > > >       bool validateSensorControls();\n>> > > >       bool validateIspControls();\n>> > > > +     bool validateLensControls();\n>> > > >       void queueRequest(const ControlList &controls);\n>> > > >       void returnEmbeddedBuffer(unsigned int bufferId);\n>> > > >       void prepareISP(const ISPConfig &data);\n>> > > > @@ -155,6 +156,7 @@ private:\n>> > > >\n>> > > >       ControlInfoMap sensorCtrls_;\n>> > > >       ControlInfoMap ispCtrls_;\n>> > > > +     ControlInfoMap lensCtrls_;\n>> > > >       bool lensPresent_;\n>> > > >       ControlList libcameraMetadata_;\n>> > > >\n>> > > > @@ -394,6 +396,15 @@ int IPARPi::configure(const\n>> IPACameraSensorInfo &sensorInfo, const IPAConfig &ip\n>> > > >               return -1;\n>> > > >       }\n>> > > >\n>> > > > +     if (lensPresent_) {\n>> > > > +             lensCtrls_ = ipaConfig.lensControls;\n>> > > > +             if (!validateLensControls()) {\n>> > > > +                     LOG(IPARPI, Warning) << \"Lens validation\n>> failed, \"\n>> > > > +                                          << \"no lens control will\n>> be available.\";\n>> > > > +                     lensPresent_ = false;\n>> > > > +             }\n>> > > > +     }\n>> > > > +\n>> > > >       maxSensorGainCode_ =\n>> sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();\n>> > > >\n>> > > >       /* Setup a metadata ControlList to output metadata. */\n>> > > > @@ -648,6 +659,16 @@ bool IPARPi::validateIspControls()\n>> > > >       return true;\n>> > > >  }\n>> > > >\n>> > > > +bool IPARPi::validateLensControls()\n>> > > > +{\n>> > > > +     if (lensCtrls_.find(V4L2_CID_FOCUS_ABSOLUTE) ==\n>> lensCtrls_.end()) {\n>> > > > +             LOG(IPARPI, Error) << \"Unable to find Lens control\n>> V4L2_CID_FOCUS_ABSOLUTE\";\n>> > > > +             return false;\n>> > > > +     }\n>> > > > +\n>> > > > +     return true;\n>> > > > +}\n>> > > > +\n>> > > >  /*\n>> > > >   * Converting between enums (used in the libcamera API) and the\n>> names that\n>> > > >   * we use to identify different modes. Unfortunately, the\n>> conversion tables\n>> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > > > index 9dd36cbaea78..249dedcd8c09 100644\n>> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> > > > @@ -1617,6 +1617,8 @@ int RPiCameraData::configureIPA(const\n>> CameraConfiguration *config, ipa::RPi::IPA\n>> > > >\n>> > > >       ipaConfig.sensorControls = sensor_->controls();\n>> > > >       ipaConfig.ispControls = isp_[Isp::Input].dev()->controls();\n>> > > > +     if (sensor_->focusLens())\n>> > > > +             ipaConfig.lensControls =\n>> sensor_->focusLens()->controls();\n>> > > >\n>> > > >       /* Always send the user transform to the IPA. */\n>> > > >       ipaConfig.transform = static_cast<unsigned\n>> int>(config->transform);\n>>\n>> --\n>> Regards,\n>>\n>> Laurent Pinchart\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 14CC9BDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Jan 2023 10:39:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 754C0625DF;\n\tMon, 23 Jan 2023 11:39:35 +0100 (CET)","from mail-yw1-x112c.google.com (mail-yw1-x112c.google.com\n\t[IPv6:2607:f8b0:4864:20::112c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E5CF603C0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Jan 2023 11:39:33 +0100 (CET)","by mail-yw1-x112c.google.com with SMTP id\n\t00721157ae682-501c3a414acso101026857b3.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Jan 2023 02:39:33 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674470375;\n\tbh=AmHdfCIvJ31qi9MwCu20ZUnyt0USDZB0GXXtAqVQomE=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Z/ULZeHZVwtxzQAZCaojYVd2A31upMqFDRzRN8G9b6Lwvpk8JsyjKdqdwzLxqqprS\n\tV3UZo0fIZSTiCdoOOe60dm+r0COb4Eg3EgWoSALCFZ/mU5ACPbv0D5nVNhYgaKuvnM\n\tt/NcJTiL2s0D0oVLwYNfKXC7yVFW9q+CeB+LJV4jr19uOzOg1fshcXjHgwJmm2Xd0O\n\t6s6a1hezXKWuF1DHLj0TwM0E3bpwcp+6lQHmRHIvpTYHKKpvUc0Y+HRkpAS7qdueSc\n\tdDmc8bY0zyfgKb6+SJD+1EMr+5HbFdp2h6jYF5g2jd/+MfvZDEwDMWnjfqbVnUCfp6\n\toLD2GlJCbpdZA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=cLEQPchdxQx8KkLRIFlZ2Dh/DypGpU3ZGcz9ARx4KnM=;\n\tb=ZYR9KW0M4w3Fzgci8x+FSqbUjObdbEcnrNribB070wl63KjsAhuyHJm1XglitUX2dU\n\t+kmChxpewnzJVNBgtrnFi8o6GKJJLdDjDfMxCHV7c4MASN31t984zUIZTHUb+Wcq/vvX\n\t+SV9caHjofff7BCJCo3KPsvrh/j/n4KhkcKEAlFIIZ0NX502rxfU1mD9aVv1gl+OpOlh\n\tjVim/F+F9qQNK3aVCC8Z2a6rfMZ1SMNDz833kJdlqa82S2Lki6MYbkOBKk55xFQr1CDy\n\tjzdpIywpIEAHg2saHDsPrfSd6IWlCFD/vfrnRM0ScFFCgTb2xLpePPAWfv8G9LItYPJs\n\tBdCg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"ZYR9KW0M\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=cLEQPchdxQx8KkLRIFlZ2Dh/DypGpU3ZGcz9ARx4KnM=;\n\tb=63FmXzWHgU1rwDsB8rD/ZpDIZujtzhaOlZlkHNn2JaI1JKtatiSIt/j3yElGiPWv4F\n\tKbJKWK2//qW4fMucQss4078A1OyUUjXsuErn4iQpFf32StSryndYV68oQ3YoQW1/BT7N\n\tPK5DwmSVgbJrE9+YMUFt4DJSdDAwrg7o8LYlbC+vTc3BtHREyiQAAZQpQ2XcZtkDcX1J\n\t/njydpQ4fe0C2Zmof4tpL7cqK1whsM9DeUJ4WRhA3cpaOF181nf95Tr/ANjSVU4vvsHG\n\tRPsAW6Qp7tkSzMTQrs123RTKK4/NYsTYbGF4GI+8LDQuIzCm3Dtlp2oVoKTqvSORkKzL\n\t36gw==","X-Gm-Message-State":"AFqh2kquNwYAnz14oSoZmF76IJgIiG1aN9yKJOckVZF395h6Aaux0jV+\n\tVCqusDa7eIIkAs1Gd3ySDCzM7cVcn+edrDcKWefNxQ==","X-Google-Smtp-Source":"AMrXdXtB37Y08K2HBe+ZGAD9D2q1FGFUe47xjsIfZS3TLzNpRypL6eqApdz2662lKtxx9hpylr4IZczww801A+BWCa4=","X-Received":"by 2002:a81:f908:0:b0:4fa:fa3a:72e with SMTP id\n\tx8-20020a81f908000000b004fafa3a072emr1708272ywm.231.1674470372218;\n\tMon, 23 Jan 2023 02:39:32 -0800 (PST)","MIME-Version":"1.0","References":"<20230119104544.9456-1-naush@raspberrypi.com>\n\t<20230119104544.9456-5-naush@raspberrypi.com>\n\t<Y82GXySn70CIPzy/@pendragon.ideasonboard.com>\n\t<CAEmqJPprc26qed9GpiGLmrWnmBLsxo0RLqEixruz8Vn-T0iyqg@mail.gmail.com>\n\t<Y85QxfLyqWASBqFr@pendragon.ideasonboard.com>\n\t<CAEmqJPpm02Fv=u4h1g-5P8ZxqnJ03eiXGe8i6yvkOvOEERujDg@mail.gmail.com>","In-Reply-To":"<CAEmqJPpm02Fv=u4h1g-5P8ZxqnJ03eiXGe8i6yvkOvOEERujDg@mail.gmail.com>","Date":"Mon, 23 Jan 2023 10:39:16 +0000","Message-ID":"<CAEmqJPpJmZ4nfpZqgRiyeFj0BOyy3zdx5sXQFmXnMKE3d4CVLg@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"000000000000b92fcd05f2ec0382\"","Subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","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>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":26329,"web_url":"https://patchwork.libcamera.org/comment/26329/","msgid":"<Y85qgupArr0RpPRt@pendragon.ideasonboard.com>","date":"2023-01-23T11:07:46","subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Mon, Jan 23, 2023 at 10:39:16AM +0000, Naushir Patuck wrote:\n> On Mon, 23 Jan 2023 at 09:22, Naushir Patuck wrote:\n> > On Mon, 23 Jan 2023 at 09:18, Laurent Pinchart wrote:\n> >> On Mon, Jan 23, 2023 at 09:02:59AM +0000, Naushir Patuck wrote:\n> >> > On Sun, 22 Jan 2023 at 18:54, Laurent Pinchart wrote:\n> >> > > On Thu, Jan 19, 2023 at 10:45:34AM +0000, Naushir Patuck via libcamera-devel wrote:\n> >> > > > Pass the available lens controls to the IPA through the configure() function.\n> >> > > > Validate that the V4L2_CID_FOCUS_ABSOLUTE does exist. If it doesn't, log a\n> >> > > > warning message, and do not advertise focus related controls from the IPA.\n> >> > >\n> >> > > Shouldn't this be done at init() time instead of configure() time ? I\n> >> > > don't suppose the presense of a lens controller could change between\n> >> > > different configurations.\n> >> >\n> >> > Yes, this would be possible to do at init() time.  I put it in configure() to\n> >> > be consistent with the sensor and ISP control validation.  I doubt lens\n> >> > controls would change between configure() calls, so happy to move it to init()\n> >> > if folks prefer that.\n> >>\n> >> Maybe the sensor and ISP controls validation should also move to init()\n> >> time then ? :-) This can be done on top of this series, it's not a\n> >> blocker.\n> >\n> > ISP controls can (and possibly should) move to init, but sensor controls need\n> > to remain in configure().  Things like vblank/hblank/exposure/gain limits can\n> > and will change across modes.\n> >\n> > I'll make the change as part of this series, it's not going to be too\n> > disruptive.\n> \n> Can I backtrack on my statement please? :-)\n> \n> There is a bit more restructuring involved to make this change - specifically\n> the devices are not yet opened at the point of ipa init(), so there are no\n> controls to be passed in.  Nothing too troublesome, but I think it makes sense\n> to do this as a separate piece of work on top.\n\nSure, no problem, this can be done on top. You can just add a todo\ncomment in the next version of this patch.\n\n> >> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> >> > > > Reviewed-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>\n> >> > > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> >> > > > ---\n> >> > > >  include/libcamera/ipa/raspberrypi.mojom       |  1 +\n> >> > > >  src/ipa/raspberrypi/raspberrypi.cpp           | 21 +++++++++++++++++++\n> >> > > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  2 ++\n> >> > > >  3 files changed, 24 insertions(+)\n> >> > > >\n> >> > > > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> >> > > > index 2a4821fbc0ef..bfacd1275bfb 100644\n> >> > > > --- a/include/libcamera/ipa/raspberrypi.mojom\n> >> > > > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> >> > > > @@ -38,6 +38,7 @@ struct IPAConfig {\n> >> > > >       libcamera.SharedFD lsTableHandle;\n> >> > > >       libcamera.ControlInfoMap sensorControls;\n> >> > > >       libcamera.ControlInfoMap ispControls;\n> >> > > > +     libcamera.ControlInfoMap lensControls;\n> >> > > >  };\n> >> > > >\n> >> > > >  struct IPAConfigResult {\n> >> > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> >> > > > index aa18ed750370..bbf3c7dc4a69 100644\n> >> > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> >> > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> >> > > > @@ -131,6 +131,7 @@ private:\n> >> > > >       void setMode(const IPACameraSensorInfo &sensorInfo);\n> >> > > >       bool validateSensorControls();\n> >> > > >       bool validateIspControls();\n> >> > > > +     bool validateLensControls();\n> >> > > >       void queueRequest(const ControlList &controls);\n> >> > > >       void returnEmbeddedBuffer(unsigned int bufferId);\n> >> > > >       void prepareISP(const ISPConfig &data);\n> >> > > > @@ -155,6 +156,7 @@ private:\n> >> > > >\n> >> > > >       ControlInfoMap sensorCtrls_;\n> >> > > >       ControlInfoMap ispCtrls_;\n> >> > > > +     ControlInfoMap lensCtrls_;\n> >> > > >       bool lensPresent_;\n> >> > > >       ControlList libcameraMetadata_;\n> >> > > >\n> >> > > > @@ -394,6 +396,15 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, const IPAConfig &ip\n> >> > > >               return -1;\n> >> > > >       }\n> >> > > >\n> >> > > > +     if (lensPresent_) {\n> >> > > > +             lensCtrls_ = ipaConfig.lensControls;\n> >> > > > +             if (!validateLensControls()) {\n> >> > > > +                     LOG(IPARPI, Warning) << \"Lens validation failed, \"\n> >> > > > +                                          << \"no lens control will be available.\";\n> >> > > > +                     lensPresent_ = false;\n> >> > > > +             }\n> >> > > > +     }\n> >> > > > +\n> >> > > >       maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();\n> >> > > >\n> >> > > >       /* Setup a metadata ControlList to output metadata. */\n> >> > > > @@ -648,6 +659,16 @@ bool IPARPi::validateIspControls()\n> >> > > >       return true;\n> >> > > >  }\n> >> > > >\n> >> > > > +bool IPARPi::validateLensControls()\n> >> > > > +{\n> >> > > > +     if (lensCtrls_.find(V4L2_CID_FOCUS_ABSOLUTE) == lensCtrls_.end()) {\n> >> > > > +             LOG(IPARPI, Error) << \"Unable to find Lens control V4L2_CID_FOCUS_ABSOLUTE\";\n> >> > > > +             return false;\n> >> > > > +     }\n> >> > > > +\n> >> > > > +     return 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> >> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > > > index 9dd36cbaea78..249dedcd8c09 100644\n> >> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> > > > @@ -1617,6 +1617,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::IPA\n> >> > > >\n> >> > > >       ipaConfig.sensorControls = sensor_->controls();\n> >> > > >       ipaConfig.ispControls = isp_[Isp::Input].dev()->controls();\n> >> > > > +     if (sensor_->focusLens())\n> >> > > > +             ipaConfig.lensControls = sensor_->focusLens()->controls();\n> >> > > >\n> >> > > >       /* Always send the user transform to the IPA. */\n> >> > > >       ipaConfig.transform = static_cast<unsigned int>(config->transform);","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 53612BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Jan 2023 11:07:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B7F1A625D8;\n\tMon, 23 Jan 2023 12:07:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C5023603C7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Jan 2023 12:07:50 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1561F10B;\n\tMon, 23 Jan 2023 12:07:49 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674472072;\n\tbh=+iXRbuBWsN/ok89OL6qONI5IKzuuh+Qtpj0Vl/MRSIg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=vBeF0wwc0p5rYjfAdQb8VcItwCDKZ1VdXoosdSvSrB8oaEJvIz87pXVAGu8pqae2c\n\toPmc260nuhvAtm72bdjzYWlcYWzu9dIbcWFelDsg0j+5ByCJeXZvFT9fG1eXrtqvQg\n\tx2wGLLeLqJ2+si2Zt1UOCWGMMdLwMRp+QxGIgwh5cayIMWGk6jF5rHJxungNs2P6mw\n\t0Tt6vBhcMpENQjLAQZNC9rH+6RXbZYxG+ydQVqZrssFL2+Pn+vHHXGB5RoLyUQm+rd\n\t+kBQ/2mLYIz/RxQLXXcXxUCulz7aMBf78EJNQRKkyoDwPndBC5MnWP6W1JToSsPb5z\n\tX7+9FYKDq+BKg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674472070;\n\tbh=+iXRbuBWsN/ok89OL6qONI5IKzuuh+Qtpj0Vl/MRSIg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L2w8hkymYD6AWx/io9fG8PWG5Vu+X6SEI58YuGVtA9xOEl/FvY7A5TwlyIId90iR5\n\tJ75pJBeErw6UsRTvIO4UTDhvpO2D8Q+emZeqHiuFlbjwq8lsji4n4anVYaKEVqWu+s\n\tN5msMwEr9JgbonlFPhaMf8CXHrb4RBTT3QzOfmOE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"L2w8hkym\"; dkim-atps=neutral","Date":"Mon, 23 Jan 2023 13:07:46 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<Y85qgupArr0RpPRt@pendragon.ideasonboard.com>","References":"<20230119104544.9456-1-naush@raspberrypi.com>\n\t<20230119104544.9456-5-naush@raspberrypi.com>\n\t<Y82GXySn70CIPzy/@pendragon.ideasonboard.com>\n\t<CAEmqJPprc26qed9GpiGLmrWnmBLsxo0RLqEixruz8Vn-T0iyqg@mail.gmail.com>\n\t<Y85QxfLyqWASBqFr@pendragon.ideasonboard.com>\n\t<CAEmqJPpm02Fv=u4h1g-5P8ZxqnJ03eiXGe8i6yvkOvOEERujDg@mail.gmail.com>\n\t<CAEmqJPpJmZ4nfpZqgRiyeFj0BOyy3zdx5sXQFmXnMKE3d4CVLg@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPpJmZ4nfpZqgRiyeFj0BOyy3zdx5sXQFmXnMKE3d4CVLg@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v1 04/14] pipeline: ipa: raspberrypi:\n\tValidate lens controls","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]