[{"id":27190,"web_url":"https://patchwork.libcamera.org/comment/27190/","msgid":"","date":"2023-05-31T14:59:42","subject":"Re: [libcamera-devel] [PATCH v1 1/2] pipeline: ipa: rpi: Flag if\n\tthe sensor is a mono variant","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch.\n\nOn Wed, 31 May 2023 at 15:39, Naushir Patuck via libcamera-devel\n wrote:\n>\n> Add a flag to the ipa->init() interface to indicate a mono sensor\n> variant. This flag will be used in a future commit to handle controls\n> that are invalid for mono sensors.\n>\n> Signed-off-by: Naushir Patuck \n\nAll looks good to me.\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> ---\n> include/libcamera/ipa/raspberrypi.mojom | 1 +\n> src/ipa/rpi/common/ipa_base.cpp | 1 +\n> src/ipa/rpi/common/ipa_base.h | 1 +\n> src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 +++-\n> 4 files changed, 6 insertions(+), 1 deletion(-)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index ba786e647ca1..d35289ee6229 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -21,6 +21,7 @@ struct SensorConfig {\n>\n> struct InitParams {\n> bool lensPresent;\n> + bool monoSensor;\n> };\n>\n> struct InitResult {\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index db7a0eb3a1ca..150fe433d0df 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -139,6 +139,7 @@ int32_t IpaBase::init(const IPASettings &settings, const InitParams ¶ms, Ini\n> }\n>\n> lensPresent_ = params.lensPresent;\n> + monoSensor_ = params.monoSensor;\n>\n> controller_.initialise();\n>\n> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> index 6f9c46bb16b1..39d00760d012 100644\n> --- a/src/ipa/rpi/common/ipa_base.h\n> +++ b/src/ipa/rpi/common/ipa_base.h\n> @@ -87,6 +87,7 @@ private:\n> std::map buffers_;\n>\n> bool lensPresent_;\n> + bool monoSensor_;\n> ControlList libcameraMetadata_;\n>\n> std::array rpiMetadata_;\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 3bb5ec531e4f..12698056cda1 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1131,6 +1131,7 @@ int CameraData::loadPipelineConfiguration()\n> int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> {\n> ipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> + bool monoSensor = isMonoSensor(sensor_);\n>\n> if (!ipa_)\n> return -ENOENT;\n> @@ -1143,7 +1144,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_TUNING_FILE\");\n> if (!configFromEnv || *configFromEnv == '\\0') {\n> std::string model = sensor_->model();\n> - if (isMonoSensor(sensor_))\n> + if (monoSensor)\n> model += \"_mono\";\n> configurationFile = ipa_->configurationFile(model + \".json\");\n> } else {\n> @@ -1154,6 +1155,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> ipa::RPi::InitParams params;\n>\n> params.lensPresent = !!sensor_->focusLens();\n> + params.monoSensor = monoSensor;\n> int ret = platformInitIpa(params);\n> if (ret)\n> return ret;\n> --\n> 2.34.1\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 A13ECC3213\n\tfor ;\n\tWed, 31 May 2023 14:59:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 203E062722;\n\tWed, 31 May 2023 16:59:57 +0200 (CEST)","from mail-oa1-x29.google.com (mail-oa1-x29.google.com\n\t[IPv6:2001:4860:4864:20::29])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C8C7E626F8\n\tfor ;\n\tWed, 31 May 2023 16:59:54 +0200 (CEST)","by mail-oa1-x29.google.com with SMTP id\n\t586e51a60fabf-199dd37f0e4so4781414fac.2\n\tfor ;\n\tWed, 31 May 2023 07:59:54 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685545197;\n\tbh=U84w//l+FdOzI2SQRIhbZDXeNCRkbCKK+jOkxmCKcog=;\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=Q+ylj3ky+y3MY1HwhIN/rDFn49ifeoHIrwyJBJC3Ygb2c6brPrSH6Fkbt8Q435uwL\n\tQmC2W35Jm0Aog6kRIjRA90BczzqDuK2SH0vnDU2/wo9bhex5q5psZbtgI5P+wovGDJ\n\txxv2Bwab0nNK5an60PnOPSftEnswPtTckfZwg0aeYguKD+5PxzRIt9+ARSp63zsQg/\n\tGaeTkWhJ1PiaT1IutZnbzl7g1MtlUTcaflqDLNC+44ZW/JOqsXSKUAtGCa1WhlJpI6\n\tlTxih/ANawq61RG3HOZvYvd+1+67nv+/51mdEWD9DJhUl57h3csFR5VHQyjgYQsdzu\n\ttrIZJLFDSTVpg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1685545193; x=1688137193;\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=Byc76HmqMuw6PSUDFD9Nvv/FBgLodhleOnCFvD0I4bw=;\n\tb=W+4OGsIxpqUhHNYgL1BkhbdycAmFnu2vRAHqpfsmMI7VswaNMXl7+5zGJT7yeZ3HIt\n\tJbM6QkGIPawmkuMe/LvJtZp32nErEb0ZfUWjZpRtNeNH0POlpRyQfDVuSz8inlDisVqE\n\tMsf4Bs5lP3HAj6MXFUya1OdFidn7pcBpmfgbIPmc2Ft4lxbxWRhzUjieyUcRQnARB4py\n\t96fPya5CxCWEwINMs1hXTw/gU0o7xNFuBIZ6ytb7vdKazppW8YYonRu4DAKA+GTS2sB7\n\tfKutWXJM18i5ONse7lM6v5IBY2MLfsJ53BPF1Lf01nWoJUeek+2Vv4mBs9vyDxPVvUTc\n\t8Xdg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"W+4OGsIx\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1685545193; x=1688137193;\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=Byc76HmqMuw6PSUDFD9Nvv/FBgLodhleOnCFvD0I4bw=;\n\tb=fj1cOlLlnfdyhRetnGW0griC6X1mfC8kAyIgGodlg3wMNbGuhtiVxsfFBKqlJY2pRZ\n\twFt+zJuD77IXpPg+vm0MfC7RW3uR774Ed51CHhGHHIsGVQDGI4Wi+XpFTqwGt8sLfN7Y\n\tw7YEPx/LV7T/gWj4f8ZhomJlED+bYnseRe/FurykXJB8aPimW3FBvv9KyxDt6eW84y/8\n\tznqt2+lfA0hBxL/kowAM7A8iYZiwwyhbP5lULhHiVfC6mD7f5xsoL/CWBaKMP/LLqjvw\n\tMgqjMGJqdaUPQLsc0KImSLmLWaAyRBQU5i2LbyF4AB0P62WiV6H6qwPWdzOutUjy3LXq\n\tOiow==","X-Gm-Message-State":"AC+VfDznZ8ar5XJQSCTBKFsDrramgURQpaTvH6fWW6ENYJ+hBB3HvkWW\n\tntq4BZJ4ozNUI8/FGrFCD6mj48DS0Uj3HZMKLhc9FnO6lSNmyirx","X-Google-Smtp-Source":"ACHHUZ4mMUaJVoK0Q+sa1tqL2l9aziS3MrTovc3xn1S3t54p6lyZYNJm7lduRxgcPXAYUf5f8qbYTCikz5IkClsdNBc=","X-Received":"by 2002:a05:6870:52d6:b0:19a:aea3:c96 with SMTP id\n\tp22-20020a05687052d600b0019aaea30c96mr2733558oak.55.1685545193623;\n\tWed, 31 May 2023 07:59:53 -0700 (PDT)","MIME-Version":"1.0","References":"<20230531143946.23571-1-naush@raspberrypi.com>\n\t<20230531143946.23571-2-naush@raspberrypi.com>","In-Reply-To":"<20230531143946.23571-2-naush@raspberrypi.com>","Date":"Wed, 31 May 2023 15:59:42 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] pipeline: ipa: rpi: Flag if\n\tthe sensor is a mono variant","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","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27218,"web_url":"https://patchwork.libcamera.org/comment/27218/","msgid":"<20230602074655.GA19463@pendragon.ideasonboard.com>","date":"2023-06-02T07:46:55","subject":"Re: [libcamera-devel] [PATCH v1 1/2] pipeline: ipa: rpi: Flag if\n\tthe sensor is a mono variant","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, May 31, 2023 at 03:39:45PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Add a flag to the ipa->init() interface to indicate a mono sensor\n> variant. This flag will be used in a future commit to handle controls\n> that are invalid for mono sensors.\n\nThe patch itself seems to do the job, so\n\nReviewed-by: Laurent Pinchart \n\nI'm however wondering if we could do a bit better. The need to identify\nthe sensor type isn't specific to Raspberry Pi, so a solution that\ncovers the other IPA modules would be nice. The IPU3 and RkISP1 IPA\ninterfaces both pass an IPACameraSensorInfo structure to the init()\nfunction. Would it make sense to extend that with a mono flag, and use\nit for the Raspberry Pi IPA module too ?\n\nAnother improvement would be to pass a CFA type instead of a mono flag,\nthat would pave the way to support other CFA patterns (I'm thinking\nabour RGB+IR for instance). It may be too early to do so though.\n\nFinally, we already pass the sensor model in the IPASettings structure,\nwhich could be used on the IPA side to check what CFA pattern the sensor\nuses by adding that information to the camera sensor helpers. Any\nopinion about this ?\n\n> Signed-off-by: Naushir Patuck \n> ---\n> include/libcamera/ipa/raspberrypi.mojom | 1 +\n> src/ipa/rpi/common/ipa_base.cpp | 1 +\n> src/ipa/rpi/common/ipa_base.h | 1 +\n> src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 +++-\n> 4 files changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index ba786e647ca1..d35289ee6229 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -21,6 +21,7 @@ struct SensorConfig {\n> \n> struct InitParams {\n> \tbool lensPresent;\n> +\tbool monoSensor;\n> };\n> \n> struct InitResult {\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index db7a0eb3a1ca..150fe433d0df 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -139,6 +139,7 @@ int32_t IpaBase::init(const IPASettings &settings, const InitParams ¶ms, Ini\n> \t}\n> \n> \tlensPresent_ = params.lensPresent;\n> +\tmonoSensor_ = params.monoSensor;\n> \n> \tcontroller_.initialise();\n> \n> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> index 6f9c46bb16b1..39d00760d012 100644\n> --- a/src/ipa/rpi/common/ipa_base.h\n> +++ b/src/ipa/rpi/common/ipa_base.h\n> @@ -87,6 +87,7 @@ private:\n> \tstd::map buffers_;\n> \n> \tbool lensPresent_;\n> +\tbool monoSensor_;\n> \tControlList libcameraMetadata_;\n> \n> \tstd::array rpiMetadata_;\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 3bb5ec531e4f..12698056cda1 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1131,6 +1131,7 @@ int CameraData::loadPipelineConfiguration()\n> int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> {\n> \tipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> +\tbool monoSensor = isMonoSensor(sensor_);\n> \n> \tif (!ipa_)\n> \t\treturn -ENOENT;\n> @@ -1143,7 +1144,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> \tchar const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_TUNING_FILE\");\n> \tif (!configFromEnv || *configFromEnv == '\\0') {\n> \t\tstd::string model = sensor_->model();\n> -\t\tif (isMonoSensor(sensor_))\n> +\t\tif (monoSensor)\n> \t\t\tmodel += \"_mono\";\n> \t\tconfigurationFile = ipa_->configurationFile(model + \".json\");\n> \t} else {\n> @@ -1154,6 +1155,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> \tipa::RPi::InitParams params;\n> \n> \tparams.lensPresent = !!sensor_->focusLens();\n> +\tparams.monoSensor = monoSensor;\n> \tint ret = platformInitIpa(params);\n> \tif (ret)\n> \t\treturn ret;","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 F18C9C3200\n\tfor ;\n\tFri, 2 Jun 2023 07:46:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5A2E362728;\n\tFri, 2 Jun 2023 09:46:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A405D626F5\n\tfor ;\n\tFri, 2 Jun 2023 09:46:57 +0200 (CEST)","from pendragon.ideasonboard.com (om126156168104.26.openmobile.ne.jp\n\t[126.156.168.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 24592B2A;\n\tFri, 2 Jun 2023 09:46:33 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685692019;\n\tbh=KEC9fX9Zo37GWnzyKNW/6rtSFGlajvqTTZAV9/QQnBI=;\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=Lk3Q8IuPwWUsJm9lNH63Ikd+g+ND7nA3g7n2IESKEwFxTZdugVo4llKfzlzh/Eny/\n\tJ+tWHevNAQUSHdzr/mWTKagwTVaUL+VjGZNBb/FAslBiTJT3FcRjIVsYXj3X6XO1EH\n\tkSE8I72bghqAEPCy2HoFsQEOd5r73gSVlCWsR8V8XctNJHEDT9e37pmGiwpDramIdW\n\t/4HrimYT8ddYnn69J8yjiDOhUsNfxpeIRfN6SHYrMlQAZKCtYLyoFAq2f1iFzc9vSC\n\tqXqCdPKT3pn3hWRK0wEymdbvS/7OBqchAE/1u67orw/65QtYCBXxt1mXZlXWbnGa8s\n\tnWtJY/grnzF1Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685691995;\n\tbh=KEC9fX9Zo37GWnzyKNW/6rtSFGlajvqTTZAV9/QQnBI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sKiFTJV0peUxfaWM08uHSLfEsjzKzeuRZLahl4QQmjCrrPKgQccLZO/HtV7+2oYvy\n\tjDcZFJDWg/CdBgwtVBPE3u4LiVEhNKIJcWL42h6m0pmRW5EgydJEjrSi3HVAqzlqpt\n\tJpUcJaJ2NjV93E4tkQxO4Rr3ugFDrsDHioCOcQIc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sKiFTJV0\"; dkim-atps=neutral","Date":"Fri, 2 Jun 2023 10:46:55 +0300","To":"Naushir Patuck ","Message-ID":"<20230602074655.GA19463@pendragon.ideasonboard.com>","References":"<20230531143946.23571-1-naush@raspberrypi.com>\n\t<20230531143946.23571-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230531143946.23571-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] pipeline: ipa: rpi: Flag if\n\tthe sensor is a mono variant","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27224,"web_url":"https://patchwork.libcamera.org/comment/27224/","msgid":"","date":"2023-06-02T08:55:03","subject":"Re: [libcamera-devel] [PATCH v1 1/2] pipeline: ipa: rpi: Flag if\n\tthe sensor is a mono variant","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Fri, 2 Jun 2023 at 08:46, Laurent Pinchart\n wrote:\n>\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Wed, May 31, 2023 at 03:39:45PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > Add a flag to the ipa->init() interface to indicate a mono sensor\n> > variant. This flag will be used in a future commit to handle controls\n> > that are invalid for mono sensors.\n>\n> The patch itself seems to do the job, so\n>\n> Reviewed-by: Laurent Pinchart \n>\n> I'm however wondering if we could do a bit better. The need to identify\n> the sensor type isn't specific to Raspberry Pi, so a solution that\n> covers the other IPA modules would be nice. The IPU3 and RkISP1 IPA\n> interfaces both pass an IPACameraSensorInfo structure to the init()\n> function. Would it make sense to extend that with a mono flag, and use\n> it for the Raspberry Pi IPA module too ?\n\nSounds good, this would allow any IPA to get this flag.\n\nIPACameraSensorInfo is a good choice, rpi/rkisp/ipu3 all take this in during the\nconfigure() call. Rpi does not have IPACameraSensorInfo passed in on init, and\nthe monoSensor_ flag is passed in init(). But really we don't need to know the\nstate of the flag until configure() so it's not a problem.\n\n>\n> Another improvement would be to pass a CFA type instead of a mono flag,\n> that would pave the way to support other CFA patterns (I'm thinking\n> abour RGB+IR for instance). It may be too early to do so though.\n>\n\nI think I actually prefer this approach over a flag! We have the CFA in\nCameraSensor::properties (properties::draft::ColorFilterArrangement) which we\ncan put in IPACameraSensorInfo. The CFA pattern might change when configuring\n(e.g. if a flip/mirror occurs) so this feels quite neat.\n\nWhat do you think? Let me know and I can rework this series.\n\n> Finally, we already pass the sensor model in the IPASettings structure,\n> which could be used on the IPA side to check what CFA pattern the sensor\n> uses by adding that information to the camera sensor helpers. Any\n> opinion about this ?\n\nThis might not work for the IMX296 as we have both mono and colour variants.\nThis is presuming that we only stick with one camera helper for IMX296 and the\nmodel string does not distinguish between the two?\n\nRegards,\nNaush\n\n>\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > include/libcamera/ipa/raspberrypi.mojom | 1 +\n> > src/ipa/rpi/common/ipa_base.cpp | 1 +\n> > src/ipa/rpi/common/ipa_base.h | 1 +\n> > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 +++-\n> > 4 files changed, 6 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> > index ba786e647ca1..d35289ee6229 100644\n> > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > @@ -21,6 +21,7 @@ struct SensorConfig {\n> >\n> > struct InitParams {\n> > bool lensPresent;\n> > + bool monoSensor;\n> > };\n> >\n> > struct InitResult {\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > index db7a0eb3a1ca..150fe433d0df 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -139,6 +139,7 @@ int32_t IpaBase::init(const IPASettings &settings, const InitParams ¶ms, Ini\n> > }\n> >\n> > lensPresent_ = params.lensPresent;\n> > + monoSensor_ = params.monoSensor;\n> >\n> > controller_.initialise();\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> > index 6f9c46bb16b1..39d00760d012 100644\n> > --- a/src/ipa/rpi/common/ipa_base.h\n> > +++ b/src/ipa/rpi/common/ipa_base.h\n> > @@ -87,6 +87,7 @@ private:\n> > std::map buffers_;\n> >\n> > bool lensPresent_;\n> > + bool monoSensor_;\n> > ControlList libcameraMetadata_;\n> >\n> > std::array rpiMetadata_;\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 3bb5ec531e4f..12698056cda1 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -1131,6 +1131,7 @@ int CameraData::loadPipelineConfiguration()\n> > int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> > {\n> > ipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> > + bool monoSensor = isMonoSensor(sensor_);\n> >\n> > if (!ipa_)\n> > return -ENOENT;\n> > @@ -1143,7 +1144,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> > char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_TUNING_FILE\");\n> > if (!configFromEnv || *configFromEnv == '\\0') {\n> > std::string model = sensor_->model();\n> > - if (isMonoSensor(sensor_))\n> > + if (monoSensor)\n> > model += \"_mono\";\n> > configurationFile = ipa_->configurationFile(model + \".json\");\n> > } else {\n> > @@ -1154,6 +1155,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> > ipa::RPi::InitParams params;\n> >\n> > params.lensPresent = !!sensor_->focusLens();\n> > + params.monoSensor = monoSensor;\n> > int ret = platformInitIpa(params);\n> > if (ret)\n> > return ret;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 CCA80C31E9\n\tfor ;\n\tFri, 2 Jun 2023 08:55:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 25AB1626F8;\n\tFri, 2 Jun 2023 10:55:10 +0200 (CEST)","from mail-yw1-x1134.google.com (mail-yw1-x1134.google.com\n\t[IPv6:2607:f8b0:4864:20::1134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 07B3A626F5\n\tfor ;\n\tFri, 2 Jun 2023 10:55:08 +0200 (CEST)","by mail-yw1-x1134.google.com with SMTP id\n\t00721157ae682-565d354b59fso19447877b3.0\n\tfor ;\n\tFri, 02 Jun 2023 01:55:07 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685696110;\n\tbh=4jrwQcpvuqzn/QiqawzbRGZG3p5RlxhOECXZ5eJY3X0=;\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=ndHClcbBaPINBJze6UVlICM4rmfYw9V4podmk1wI4H8voza/81URaMBL5hYKQvl71\n\t/lmZbFQqqKGb7CfJPNEedfnGva/b3WIufBxyFuC/l5lM+Ncpdn56A3tsC2slW0wpAr\n\tBLZILxijxHTzuN9hmGKySfn2np9z2eHPPMcSXz0KRW5blFScmpWs5Nj0JFyBJj3G1a\n\thxc/B0n7pNKpr9xwrZezCPvvi5t88m/OhOpBbMkv5a9vUG22ftFUD4PnkbrSIXhp0V\n\tlxm81r9UcpgcJ7ZP5Gn8XGsAnZ1l84CZdlyaxJZPQ1h3p5niP+JZM01cGbM/YYU45R\n\tXHdDGNpx3tUQA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1685696107; x=1688288107;\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=w8Sj1LT5uUS+hYXu+6l3KF4AP+Nkzkk9M2XLUajeeN0=;\n\tb=rRThqT+L0nhQKGNtBrf6BdTkMx2B3mFJEw1QFZtYoqP6b9BunNHH2izbVsxsPseqFt\n\tn1e/QE25L/6JeKq2MUN9GUXwYPrVA8dFyakdNpqnbXd6mBe1FA40PUL8kKnhi9qGrQpP\n\ts/czCsXUDGpH4gJqLpiPn6XJDpI4vCtYwCbtxgsApaPSdyJ7YpLpN4m9XrvMzKcGeBsh\n\t22a/ZZGaLY41299JvF5pBNGdUnJtVIIJfY5ruOuA/r2A5jLo+Acpw7T3aZ6/Vm54uidY\n\tt9qdjRDm1Gj6r2lbCFqqsoGnSZRQBlOV+kbmIkKgWsqe6XDoYOZo6zTpOs6bKEMWcKTx\n\tacXg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"rRThqT+L\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1685696107; x=1688288107;\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=w8Sj1LT5uUS+hYXu+6l3KF4AP+Nkzkk9M2XLUajeeN0=;\n\tb=DVhfil2+SKx+AHyqvMGYp83fGs6LrUKBT3EQohB8lrgS48CKhFKNEcbdW5pz9EGLzE\n\tlfqsG6uyTAJZ0tiEKWkhoGN4kADjiQzEGRKca/hjVoIjAqiQhjKNoaTWeNZXi+BrcmWr\n\tQCgtC6slmcSp8YHp3IpunZmma/AQAobGklIoQfuuULMchmTolIyXBWObfnG+9SezmkIC\n\tfNrnHOPaavv6u4ym1Qlz91AE5c4hh0h91Wwquqdi/l2pvTM7nyX9Z8fGFpby3RwaS5dI\n\t5tsR+QSstKjFnfPCNd9wlvGFSV8y876zdduJH02tt7uRyAdLSNTiTClluC9jNrEIbFx2\n\t+6kg==","X-Gm-Message-State":"AC+VfDwGqyozHHhIXHmW3cEdpnDiDkYgNj3cNmhVErX3PYvKGymB/B6X\n\trfQSv0qJjcNQ4C+ls39Ifkuz1RR9gKfBmPZIV5ySFigTOzuPxDIEu7IytA==","X-Google-Smtp-Source":"ACHHUZ4KDvbCYCRr9KddSW5ckFXJez5a/0FXdSms6KL4YfMfXlQXITDv9wZ5xHJSpUuIiD2LcSBreUxx8DfBFQ5b0lg=","X-Received":"by 2002:a0d:edc1:0:b0:552:a63e:18d1 with SMTP id\n\tw184-20020a0dedc1000000b00552a63e18d1mr10120686ywe.52.1685696106874;\n\tFri, 02 Jun 2023 01:55:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20230531143946.23571-1-naush@raspberrypi.com>\n\t<20230531143946.23571-2-naush@raspberrypi.com>\n\t<20230602074655.GA19463@pendragon.ideasonboard.com>","In-Reply-To":"<20230602074655.GA19463@pendragon.ideasonboard.com>","Date":"Fri, 2 Jun 2023 09:55:03 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] pipeline: ipa: rpi: Flag if\n\tthe sensor is a mono variant","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27230,"web_url":"https://patchwork.libcamera.org/comment/27230/","msgid":"<20230602155511.GN26944@pendragon.ideasonboard.com>","date":"2023-06-02T15:55:11","subject":"Re: [libcamera-devel] [PATCH v1 1/2] pipeline: ipa: rpi: Flag if\n\tthe sensor is a mono variant","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Fri, Jun 02, 2023 at 09:55:03AM +0100, Naushir Patuck wrote:\n> On Fri, 2 Jun 2023 at 08:46, Laurent Pinchart wrote:\n> > On Wed, May 31, 2023 at 03:39:45PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > Add a flag to the ipa->init() interface to indicate a mono sensor\n> > > variant. This flag will be used in a future commit to handle controls\n> > > that are invalid for mono sensors.\n> >\n> > The patch itself seems to do the job, so\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > I'm however wondering if we could do a bit better. The need to identify\n> > the sensor type isn't specific to Raspberry Pi, so a solution that\n> > covers the other IPA modules would be nice. The IPU3 and RkISP1 IPA\n> > interfaces both pass an IPACameraSensorInfo structure to the init()\n> > function. Would it make sense to extend that with a mono flag, and use\n> > it for the Raspberry Pi IPA module too ?\n> \n> Sounds good, this would allow any IPA to get this flag.\n> \n> IPACameraSensorInfo is a good choice, rpi/rkisp/ipu3 all take this in during the\n> configure() call. Rpi does not have IPACameraSensorInfo passed in on init, and\n> the monoSensor_ flag is passed in init(). But really we don't need to know the\n> state of the flag until configure() so it's not a problem.\n> \n> > Another improvement would be to pass a CFA type instead of a mono flag,\n> > that would pave the way to support other CFA patterns (I'm thinking\n> > abour RGB+IR for instance). It may be too early to do so though.\n> \n> I think I actually prefer this approach over a flag! We have the CFA in\n> CameraSensor::properties (properties::draft::ColorFilterArrangement) which we\n> can put in IPACameraSensorInfo. The CFA pattern might change when configuring\n> (e.g. if a flip/mirror occurs) so this feels quite neat.\n> \n> What do you think? Let me know and I can rework this series.\n\nI see you've sent a v2 already, let's continue the discussion there.\n\n> > Finally, we already pass the sensor model in the IPASettings structure,\n> > which could be used on the IPA side to check what CFA pattern the sensor\n> > uses by adding that information to the camera sensor helpers. Any\n> > opinion about this ?\n> \n> This might not work for the IMX296 as we have both mono and colour variants.\n> This is presuming that we only stick with one camera helper for IMX296 and the\n> model string does not distinguish between the two?\n\nGood point. I think we can ignore this question for now, as adding the\nCFA pattern in the IPACameraSensorInfo works nicely.\n\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > include/libcamera/ipa/raspberrypi.mojom | 1 +\n> > > src/ipa/rpi/common/ipa_base.cpp | 1 +\n> > > src/ipa/rpi/common/ipa_base.h | 1 +\n> > > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 +++-\n> > > 4 files changed, 6 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> > > index ba786e647ca1..d35289ee6229 100644\n> > > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > > @@ -21,6 +21,7 @@ struct SensorConfig {\n> > >\n> > > struct InitParams {\n> > > bool lensPresent;\n> > > + bool monoSensor;\n> > > };\n> > >\n> > > struct InitResult {\n> > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > > index db7a0eb3a1ca..150fe433d0df 100644\n> > > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > > @@ -139,6 +139,7 @@ int32_t IpaBase::init(const IPASettings &settings, const InitParams ¶ms, Ini\n> > > }\n> > >\n> > > lensPresent_ = params.lensPresent;\n> > > + monoSensor_ = params.monoSensor;\n> > >\n> > > controller_.initialise();\n> > >\n> > > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> > > index 6f9c46bb16b1..39d00760d012 100644\n> > > --- a/src/ipa/rpi/common/ipa_base.h\n> > > +++ b/src/ipa/rpi/common/ipa_base.h\n> > > @@ -87,6 +87,7 @@ private:\n> > > std::map buffers_;\n> > >\n> > > bool lensPresent_;\n> > > + bool monoSensor_;\n> > > ControlList libcameraMetadata_;\n> > >\n> > > std::array rpiMetadata_;\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index 3bb5ec531e4f..12698056cda1 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -1131,6 +1131,7 @@ int CameraData::loadPipelineConfiguration()\n> > > int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> > > {\n> > > ipa_ = IPAManager::createIPA(pipe(), 1, 1);\n> > > + bool monoSensor = isMonoSensor(sensor_);\n> > >\n> > > if (!ipa_)\n> > > return -ENOENT;\n> > > @@ -1143,7 +1144,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> > > char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_TUNING_FILE\");\n> > > if (!configFromEnv || *configFromEnv == '\\0') {\n> > > std::string model = sensor_->model();\n> > > - if (isMonoSensor(sensor_))\n> > > + if (monoSensor)\n> > > model += \"_mono\";\n> > > configurationFile = ipa_->configurationFile(model + \".json\");\n> > > } else {\n> > > @@ -1154,6 +1155,7 @@ int CameraData::loadIPA(ipa::RPi::InitResult *result)\n> > > ipa::RPi::InitParams params;\n> > >\n> > > params.lensPresent = !!sensor_->focusLens();\n> > > + params.monoSensor = monoSensor;\n> > > int ret = platformInitIpa(params);\n> > > if (ret)\n> > > return ret;","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 D7A9DC3200\n\tfor ;\n\tFri, 2 Jun 2023 15:55:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 646AA62709;\n\tFri, 2 Jun 2023 17:55:15 +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 0297B626FA\n\tfor ;\n\tFri, 2 Jun 2023 17:55:13 +0200 (CEST)","from pendragon.ideasonboard.com (om126156168104.26.openmobile.ne.jp\n\t[126.156.168.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 20EBB844;\n\tFri, 2 Jun 2023 17:54:49 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685721315;\n\tbh=qnDDMqUhSaB/Dw07pDugR9kSCpM62dWNNQHATXIN4RY=;\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=mkiEgE+Y6nEPdeNixYpjx5ESB9iJOr2RQaEeV5r2Zg7y6/ujbPkZAnzu/8Af2tAcX\n\tkQ2xTmMBidtnOIuGHalwjpKbzSIw+iTV/akokEtkDT/fBQmou3BHonu5cqQ+/7HPkE\n\tC0Fgr+5IdH2AHnJuSKZaojPvxMAQgQEV2geBAtwWg7ivupR5mDVLucCblZTdLEX14O\n\t1av0orLBdNqyQb3L/kITKF1Q+p9ZvDYRWmuhXoTsM58eDFoxiG7HhUJYl2Djj7LC8a\n\tbS0FYq9MGyBooqpLEJcGu8yedcjvJzDGEhZAnFHJeDXUOSs0vMU+HuuU1WWyXxzO70\n\tZiuHBPlK6Gedw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685721291;\n\tbh=qnDDMqUhSaB/Dw07pDugR9kSCpM62dWNNQHATXIN4RY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lKUtKYniLeLq9gZ0EAPTilEuf1Tn9C4y7wrMAnXjvKPONDwEeGCYXGBk2lTzidTU9\n\tVdDP/Mjf/6yVd47lndtbNVYmwJovgkKIQCWwR9YA1caKKRi7K8SMIQjM3MEJ7bmehS\n\tPnlK8UhA5ONFQggoPfnhaEkXw4IjkhWW6ZGfR8cM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"lKUtKYni\"; dkim-atps=neutral","Date":"Fri, 2 Jun 2023 18:55:11 +0300","To":"Naushir Patuck ","Message-ID":"<20230602155511.GN26944@pendragon.ideasonboard.com>","References":"<20230531143946.23571-1-naush@raspberrypi.com>\n\t<20230531143946.23571-2-naush@raspberrypi.com>\n\t<20230602074655.GA19463@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] pipeline: ipa: rpi: Flag if\n\tthe sensor is a mono variant","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]