[{"id":15801,"web_url":"https://patchwork.libcamera.org/comment/15801/","msgid":"","date":"2021-03-22T21:06:32","subject":"Re: [libcamera-devel] [PATCH 2/7] pipeline: ipa: raspberrypi: Open\n\tthe CamHelper on ipa::init()","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, Mar 17, 2021 at 10:02:06AM +0000, Naushir Patuck wrote:\n> Move the opening of the CamHelper from ipa::configure() to ipa::init().\n> This allows the pipeline handler to get the sensor specific parameters\n> in in pipeline_handler::match() where the ipa is initialised.\n\ns/in in/in/\n\n> \n> Having the sensor parameters available earlier will allow selective\n> use of the embedded data node in a future change.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> include/libcamera/ipa/raspberrypi.mojom | 7 +--\n> src/ipa/raspberrypi/raspberrypi.cpp | 60 +++++++++----------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 36 ++++++-----\n> 3 files changed, 46 insertions(+), 57 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index eb427697d349..a713c88eee19 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -15,10 +15,6 @@ enum BufferMask {\n> /* Size of the LS grid allocation. */\n> const uint32 MaxLsGridSize = 0x8000;\n> \n> -enum ConfigOutputParameters {\n> -\tConfigSensorParams = 0x01,\n> -};\n> -\n> struct SensorConfig {\n> \tuint32 gainDelay;\n> \tuint32 exposureDelay;\n> @@ -41,7 +37,6 @@ struct ConfigInput {\n> \n> struct ConfigOutput {\n> \tuint32 params;\n\nUnless I'm mistaken, the params field isn't used anymore. Unless it's\nused in a subsequent patch in the series, you can drop it.\n\n> -\tSensorConfig sensorConfig;\n> \tControlList controls;\n> };\n> \n> @@ -51,7 +46,7 @@ struct StartControls {\n> };\n> \n> interface IPARPiInterface {\n> -\tinit(IPASettings settings) => (int32 ret);\n> +\tinit(IPASettings settings) => (int32 ret, SensorConfig sensorConfig);\n> \tstart(StartControls controls) => (StartControls result);\n> \tstop();\n> \n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 7904225a29fa..8a9eb92b39ec 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -79,7 +79,7 @@ public:\n> \t\t\tmunmap(lsTable_, ipa::RPi::MaxLsGridSize);\n> \t}\n> \n> -\tint init(const IPASettings &settings) override;\n> +\tint init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig) override;\n> \tvoid start(const ipa::RPi::StartControls &data,\n> \t\t ipa::RPi::StartControls *result) override;\n> \tvoid stop() override {}\n> @@ -164,9 +164,35 @@ private:\n> \tdouble maxFrameDuration_;\n> };\n> \n> -int IPARPi::init(const IPASettings &settings)\n> +int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig)\n> {\n> \ttuningFile_ = settings.configurationFile;\n> +\n> +\t/*\n> +\t * Load the \"helper\" for this sensor. This tells us all the device specific stuff\n> +\t * that the kernel driver doesn't. We only do this the first time; we don't need\n> +\t * to re-parse the metadata after a simple mode-switch for no reason.\n> +\t */\n> +\thelper_ = std::unique_ptr(RPiController::CamHelper::Create(settings.sensorModel));\n\nNot something that needs to be done as a prerequisite, but would it make\nsense to change the return type of RPiController::CamHelper::Create() to\nstd::unique_ptr ?\n\n> +\tif (!helper_) {\n> +\t\tLOG(IPARPI, Error) << \"Could not create camera helper for \"\n> +\t\t\t\t << settings.sensorModel;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\t/*\n> +\t * Pass out the sensor config to the pipeline handler in order\n> +\t * to setup the staggered writer class.\n> +\t */\n> +\tint gainDelay, exposureDelay, vblankDelay, sensorMetadata;\n> +\thelper_->GetDelays(exposureDelay, gainDelay, vblankDelay);\n> +\tsensorMetadata = helper_->SensorEmbeddedDataPresent();\n> +\n> +\tsensorConfig->gainDelay = gainDelay;\n> +\tsensorConfig->exposureDelay = exposureDelay;\n> +\tsensorConfig->vblankDelay = vblankDelay;\n> +\tsensorConfig->sensorMetadata = sensorMetadata;\n> +\n> \treturn 0;\n> }\n> \n> @@ -319,36 +345,6 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t/* Setup a metadata ControlList to output metadata. */\n> \tlibcameraMetadata_ = ControlList(controls::controls);\n> \n> -\t/*\n> -\t * Load the \"helper\" for this sensor. This tells us all the device specific stuff\n> -\t * that the kernel driver doesn't. We only do this the first time; we don't need\n> -\t * to re-parse the metadata after a simple mode-switch for no reason.\n> -\t */\n> -\tstd::string cameraName(sensorInfo.model);\n> -\tif (!helper_) {\n> -\t\thelper_ = std::unique_ptr(RPiController::CamHelper::Create(cameraName));\n> -\n> -\t\tif (!helper_) {\n> -\t\t\tLOG(IPARPI, Error) << \"Could not create camera helper for \"\n> -\t\t\t\t\t << cameraName;\n> -\t\t\treturn -1;\n> -\t\t}\n> -\n> -\t\t/*\n> -\t\t * Pass out the sensor config to the pipeline handler in order\n> -\t\t * to setup the staggered writer class.\n> -\t\t */\n> -\t\tint gainDelay, exposureDelay, vblankDelay, sensorMetadata;\n> -\t\thelper_->GetDelays(exposureDelay, gainDelay, vblankDelay);\n> -\t\tsensorMetadata = helper_->SensorEmbeddedDataPresent();\n> -\n> -\t\tresult->params |= ipa::RPi::ConfigSensorParams;\n> -\t\tresult->sensorConfig.gainDelay = gainDelay;\n> -\t\tresult->sensorConfig.exposureDelay = exposureDelay;\n> -\t\tresult->sensorConfig.vblankDelay = vblankDelay;\n> -\t\tresult->sensorConfig.sensorMetadata = sensorMetadata;\n> -\t}\n> -\n> \t/* Re-assemble camera mode using the sensor info. */\n> \tsetMode(sensorInfo);\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 2c8ae31a28ad..ce1994186d66 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -146,7 +146,7 @@ public:\n> \n> \tvoid frameStarted(uint32_t sequence);\n> \n> -\tint loadIPA();\n> +\tint loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> \tint configureIPA(const CameraConfiguration *config);\n> \n> \tvoid statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n> @@ -1030,11 +1030,24 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> \tif (data->sensor_->init())\n> \t\treturn false;\n> \n> -\tif (data->loadIPA()) {\n> +\tipa::RPi::SensorConfig sensorConfig;\n> +\tif (data->loadIPA(&sensorConfig)) {\n> \t\tLOG(RPI, Error) << \"Failed to load a suitable IPA library\";\n> \t\treturn false;\n> \t}\n> \n> +\t/*\n> +\t * Setup our delayed control writer with the sensor default\n> +\t * gain and exposure delays. Mark VBLANK for priority write.\n> +\t */\n> +\tstd::unordered_map params = {\n> +\t\t{ V4L2_CID_ANALOGUE_GAIN, { sensorConfig.gainDelay, false } },\n> +\t\t{ V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false } },\n> +\t\t{ V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }\n> +\t};\n> +\tdata->delayedCtrls_ = std::make_unique(data->unicam_[Unicam::Image].dev(), params);\n> +\tdata->sensorMetadata_ = sensorConfig.sensorMetadata;\n> +\n\nAs this code deals with RPiCameraData only, could you move it to\nRPiCameraData::loadIPA() ? That way you won't have to prefix everything\nwith data->, and you'll avoid passing sensorConfig to loadIPA().\n\nWith these small changes,\n\nReviewed-by: Laurent Pinchart \n\n> \t/* Register the controls that the Raspberry Pi IPA can handle. */\n> \tdata->controlInfo_ = RPi::Controls;\n> \t/* Initialize the camera properties. */\n> @@ -1214,7 +1227,7 @@ void RPiCameraData::frameStarted(uint32_t sequence)\n> \tdelayedCtrls_->applyControls(sequence);\n> }\n> \n> -int RPiCameraData::loadIPA()\n> +int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n> {\n> \tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> \n> @@ -1230,7 +1243,7 @@ int RPiCameraData::loadIPA()\n> \tIPASettings settings(ipa_->configurationFile(sensor_->model() + \".json\"),\n> \t\t\t sensor_->model());\n> \n> -\treturn ipa_->init(settings);\n> +\treturn ipa_->init(settings, sensorConfig);\n> }\n> \n> int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> @@ -1293,21 +1306,6 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \t\treturn -EPIPE;\n> \t}\n> \n> -\tif (result.params & ipa::RPi::ConfigSensorParams) {\n> -\t\t/*\n> -\t\t * Setup our delayed control writer with the sensor default\n> -\t\t * gain and exposure delays. Mark VBLANK for priority write.\n> -\t\t */\n> -\t\tstd::unordered_map params = {\n> -\t\t\t{ V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },\n> -\t\t\t{ V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },\n> -\t\t\t{ V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }\n> -\t\t};\n> -\n> -\t\tdelayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), params);\n> -\t\tsensorMetadata_ = result.sensorConfig.sensorMetadata;\n> -\t}\n> -\n> \tif (!result.controls.empty()) {\n> \t\tControlList &ctrls = result.controls;\n> \t\tunicam_[Unicam::Image].dev()->setControls(&ctrls);","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 634B0C32E1\n\tfor ;\n\tMon, 22 Mar 2021 21:07:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A060568D62;\n\tMon, 22 Mar 2021 22:07:15 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 224F968D50\n\tfor ;\n\tMon, 22 Mar 2021 22:07:14 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 80D2EED;\n\tMon, 22 Mar 2021 22:07:13 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"E3lU5yBd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616447233;\n\tbh=kUCtUQs15fMaM7LNcY8NzMhDjuA4y5RkRJ5rxrP2bCc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=E3lU5yBd2RnxRT2UjnpSfYR7EjLPVB2y4HIvX6MtSlsI9BiLCx5S7Wpk7HxtxObZr\n\t+y/QxfAT3WFPzRElaUjJfXiK3l6zDm9C8VJfHfTkBCNLPrgiT1p7ej44AEO7t9UkDg\n\t/tV4gWXsCNQlRKN7xZlDxHab4REkwfCC22OdlbLs=","Date":"Mon, 22 Mar 2021 23:06:32 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210317100211.1067585-1-naush@raspberrypi.com>\n\t<20210317100211.1067585-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210317100211.1067585-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 2/7] pipeline: ipa: raspberrypi: Open\n\tthe CamHelper on ipa::init()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15805,"web_url":"https://patchwork.libcamera.org/comment/15805/","msgid":"","date":"2021-03-22T21:22:04","subject":"Re: [libcamera-devel] [PATCH 2/7] pipeline: ipa: raspberrypi: Open\n\tthe CamHelper on ipa::init()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your review feedback.\n\nOn Mon, 22 Mar 2021 at 21:07, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Wed, Mar 17, 2021 at 10:02:06AM +0000, Naushir Patuck wrote:\n> > Move the opening of the CamHelper from ipa::configure() to ipa::init().\n> > This allows the pipeline handler to get the sensor specific parameters\n> > in in pipeline_handler::match() where the ipa is initialised.\n>\n> s/in in/in/\n>\n> >\n> > Having the sensor parameters available earlier will allow selective\n> > use of the embedded data node in a future change.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > include/libcamera/ipa/raspberrypi.mojom | 7 +--\n> > src/ipa/raspberrypi/raspberrypi.cpp | 60 +++++++++----------\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 36 ++++++-----\n> > 3 files changed, 46 insertions(+), 57 deletions(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.mojom\n> b/include/libcamera/ipa/raspberrypi.mojom\n> > index eb427697d349..a713c88eee19 100644\n> > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > @@ -15,10 +15,6 @@ enum BufferMask {\n> > /* Size of the LS grid allocation. */\n> > const uint32 MaxLsGridSize = 0x8000;\n> >\n> > -enum ConfigOutputParameters {\n> > - ConfigSensorParams = 0x01,\n> > -};\n> > -\n> > struct SensorConfig {\n> > uint32 gainDelay;\n> > uint32 exposureDelay;\n> > @@ -41,7 +37,6 @@ struct ConfigInput {\n> >\n> > struct ConfigOutput {\n> > uint32 params;\n>\n> Unless I'm mistaken, the params field isn't used anymore. Unless it's\n> used in a subsequent patch in the series, you can drop it.\n>\n\nThis (and other bits) does get tidied up in the later commits for this\nseries.\n\n\n> > - SensorConfig sensorConfig;\n> > ControlList controls;\n> > };\n> >\n> > @@ -51,7 +46,7 @@ struct StartControls {\n> > };\n> >\n> > interface IPARPiInterface {\n> > - init(IPASettings settings) => (int32 ret);\n> > + init(IPASettings settings) => (int32 ret, SensorConfig\n> sensorConfig);\n> > start(StartControls controls) => (StartControls result);\n> > stop();\n> >\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 7904225a29fa..8a9eb92b39ec 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -79,7 +79,7 @@ public:\n> > munmap(lsTable_, ipa::RPi::MaxLsGridSize);\n> > }\n> >\n> > - int init(const IPASettings &settings) override;\n> > + int init(const IPASettings &settings, ipa::RPi::SensorConfig\n> *sensorConfig) override;\n> > void start(const ipa::RPi::StartControls &data,\n> > ipa::RPi::StartControls *result) override;\n> > void stop() override {}\n> > @@ -164,9 +164,35 @@ private:\n> > double maxFrameDuration_;\n> > };\n> >\n> > -int IPARPi::init(const IPASettings &settings)\n> > +int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig\n> *sensorConfig)\n> > {\n> > tuningFile_ = settings.configurationFile;\n> > +\n> > + /*\n> > + * Load the \"helper\" for this sensor. This tells us all the device\n> specific stuff\n> > + * that the kernel driver doesn't. We only do this the first time;\n> we don't need\n> > + * to re-parse the metadata after a simple mode-switch for no\n> reason.\n> > + */\n> > + helper_ =\n> std::unique_ptr(RPiController::CamHelper::Create(settings.sensorModel));\n>\n> Not something that needs to be done as a prerequisite, but would it make\n> sense to change the return type of RPiController::CamHelper::Create() to\n> std::unique_ptr ?\n>\n\nYes, that seems reasonable. I will make the change separately to this\nseries.\n\n\n>\n> > + if (!helper_) {\n> > + LOG(IPARPI, Error) << \"Could not create camera helper for \"\n> > + << settings.sensorModel;\n> > + return -EINVAL;\n> > + }\n> > +\n> > + /*\n> > + * Pass out the sensor config to the pipeline handler in order\n> > + * to setup the staggered writer class.\n> > + */\n> > + int gainDelay, exposureDelay, vblankDelay, sensorMetadata;\n> > + helper_->GetDelays(exposureDelay, gainDelay, vblankDelay);\n> > + sensorMetadata = helper_->SensorEmbeddedDataPresent();\n> > +\n> > + sensorConfig->gainDelay = gainDelay;\n> > + sensorConfig->exposureDelay = exposureDelay;\n> > + sensorConfig->vblankDelay = vblankDelay;\n> > + sensorConfig->sensorMetadata = sensorMetadata;\n> > +\n> > return 0;\n> > }\n> >\n> > @@ -319,36 +345,6 @@ int IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> > /* Setup a metadata ControlList to output metadata. */\n> > libcameraMetadata_ = ControlList(controls::controls);\n> >\n> > - /*\n> > - * Load the \"helper\" for this sensor. This tells us all the device\n> specific stuff\n> > - * that the kernel driver doesn't. We only do this the first time;\n> we don't need\n> > - * to re-parse the metadata after a simple mode-switch for no\n> reason.\n> > - */\n> > - std::string cameraName(sensorInfo.model);\n> > - if (!helper_) {\n> > - helper_ =\n> std::unique_ptr(RPiController::CamHelper::Create(cameraName));\n> > -\n> > - if (!helper_) {\n> > - LOG(IPARPI, Error) << \"Could not create camera\n> helper for \"\n> > - << cameraName;\n> > - return -1;\n> > - }\n> > -\n> > - /*\n> > - * Pass out the sensor config to the pipeline handler in\n> order\n> > - * to setup the staggered writer class.\n> > - */\n> > - int gainDelay, exposureDelay, vblankDelay, sensorMetadata;\n> > - helper_->GetDelays(exposureDelay, gainDelay, vblankDelay);\n> > - sensorMetadata = helper_->SensorEmbeddedDataPresent();\n> > -\n> > - result->params |= ipa::RPi::ConfigSensorParams;\n> > - result->sensorConfig.gainDelay = gainDelay;\n> > - result->sensorConfig.exposureDelay = exposureDelay;\n> > - result->sensorConfig.vblankDelay = vblankDelay;\n> > - result->sensorConfig.sensorMetadata = sensorMetadata;\n> > - }\n> > -\n> > /* Re-assemble camera mode using the sensor info. */\n> > setMode(sensorInfo);\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 2c8ae31a28ad..ce1994186d66 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -146,7 +146,7 @@ public:\n> >\n> > void frameStarted(uint32_t sequence);\n> >\n> > - int loadIPA();\n> > + int loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> > int configureIPA(const CameraConfiguration *config);\n> >\n> > void statsMetadataComplete(uint32_t bufferId, const ControlList\n> &controls);\n> > @@ -1030,11 +1030,24 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> > if (data->sensor_->init())\n> > return false;\n> >\n> > - if (data->loadIPA()) {\n> > + ipa::RPi::SensorConfig sensorConfig;\n> > + if (data->loadIPA(&sensorConfig)) {\n> > LOG(RPI, Error) << \"Failed to load a suitable IPA library\";\n> > return false;\n> > }\n> >\n> > + /*\n> > + * Setup our delayed control writer with the sensor default\n> > + * gain and exposure delays. Mark VBLANK for priority write.\n> > + */\n> > + std::unordered_map\n> params = {\n> > + { V4L2_CID_ANALOGUE_GAIN, { sensorConfig.gainDelay, false\n> } },\n> > + { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false }\n> },\n> > + { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }\n> > + };\n> > + data->delayedCtrls_ =\n> std::make_unique(data->unicam_[Unicam::Image].dev(),\n> params);\n> > + data->sensorMetadata_ = sensorConfig.sensorMetadata;\n> > +\n>\n> As this code deals with RPiCameraData only, could you move it to\n> RPiCameraData::loadIPA() ? That way you won't have to prefix everything\n> with data->, and you'll avoid passing sensorConfig to loadIPA().\n>\n\nMy original code did have this block in loadIPA(), but I ran into a small\nproblem.\nDelayedControls() needs to have the device (unicam_[Unicam::Image] in this\ncase)\nopened in order to validate the controls that are passed in via params.\nloadIPA()\ngets called before the devices are opened, so I had to move this code block\nhere.\n\nI could have moved it into loadIPA() with unicam_[Unicam::Image] already\nopened,\nbut then I would have to split the opening of unicam_[Unicam::Image] and\nunicam_[Unicam::Embedded] into two separate locations, which I thought\nmight be\nundesirable. I am not completely against it, what are your thoughts?\n\nRegards,\nNaush\n\n\n>\n> With these small changes,\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > /* Register the controls that the Raspberry Pi IPA can handle. */\n> > data->controlInfo_ = RPi::Controls;\n> > /* Initialize the camera properties. */\n> > @@ -1214,7 +1227,7 @@ void RPiCameraData::frameStarted(uint32_t sequence)\n> > delayedCtrls_->applyControls(sequence);\n> > }\n> >\n> > -int RPiCameraData::loadIPA()\n> > +int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n> > {\n> > ipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> >\n> > @@ -1230,7 +1243,7 @@ int RPiCameraData::loadIPA()\n> > IPASettings settings(ipa_->configurationFile(sensor_->model() +\n> \".json\"),\n> > sensor_->model());\n> >\n> > - return ipa_->init(settings);\n> > + return ipa_->init(settings, sensorConfig);\n> > }\n> >\n> > int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> > @@ -1293,21 +1306,6 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> > return -EPIPE;\n> > }\n> >\n> > - if (result.params & ipa::RPi::ConfigSensorParams) {\n> > - /*\n> > - * Setup our delayed control writer with the sensor default\n> > - * gain and exposure delays. Mark VBLANK for priority\n> write.\n> > - */\n> > - std::unordered_map DelayedControls::ControlParams> params = {\n> > - { V4L2_CID_ANALOGUE_GAIN, {\n> result.sensorConfig.gainDelay, false } },\n> > - { V4L2_CID_EXPOSURE, {\n> result.sensorConfig.exposureDelay, false } },\n> > - { V4L2_CID_VBLANK, {\n> result.sensorConfig.vblankDelay, true } }\n> > - };\n> > -\n> > - delayedCtrls_ =\n> std::make_unique(unicam_[Unicam::Image].dev(), params);\n> > - sensorMetadata_ = result.sensorConfig.sensorMetadata;\n> > - }\n> > -\n> > if (!result.controls.empty()) {\n> > ControlList &ctrls = result.controls;\n> > unicam_[Unicam::Image].dev()->setControls(&ctrls);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3C70BC32E1\n\tfor ;\n\tMon, 22 Mar 2021 21:22:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF56268D63;\n\tMon, 22 Mar 2021 22:22:22 +0100 (CET)","from mail-lj1-x231.google.com (mail-lj1-x231.google.com\n\t[IPv6:2a00:1450:4864:20::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CB7568D50\n\tfor ;\n\tMon, 22 Mar 2021 22:22:21 +0100 (CET)","by mail-lj1-x231.google.com with SMTP id s17so22914455ljc.5\n\tfor ;\n\tMon, 22 Mar 2021 14:22:21 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"im7Q7x4+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=EyDnjJ0BSQOAPnIHsgvjDjMChSjYDjhAbqNnqoNu/dA=;\n\tb=im7Q7x4+EoQo4ydatmClIbI31Nl4gCrv7wtlFpxVgfuvoTSr2gl1lz2sURf/cdAQYP\n\tUKq2tjPeM+lawA3d/65BRhliBlP7Ba/5aNuFvDSWtJR7J9buhMVdT8uQf3NUoyuRX9ua\n\tnHCqvfqHCMHHhZ4U98QcJFLR+RVpY4qRLgA00sR1nSzqd4chMMU4tWq2ZsqBDHzgYvU5\n\t2Bmj1aEU8HAKQNTajyjTIZZXvshrKDn/fytsKOsykyoKDk6cCCt/SRU7EZI099ZAxODD\n\tybU2zsWewqdeZZyxrjHnmQZdUmEu2xzXEO7dbA1LN+cDJ8KNvugTfdYmORclw5RNNGS+\n\tYj2w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=EyDnjJ0BSQOAPnIHsgvjDjMChSjYDjhAbqNnqoNu/dA=;\n\tb=FtmCg2y8Umvpuk8ycp5SnUg5XK5rkKCg8t4XmoOWdR5EW0UwXz1FqnL4sNAeGD/F79\n\tJJ0RXEMYoJLutLsQdGGB6KCZH9oLzJsCzG3HzWpSHCg1F0nbWh9UU+38uryXBNTE79rH\n\tWvgGn1DzTSb2uRbCP0zhZ5V6kXyyliqGLkYn4DUMJUtiX51GAc8Wi/ZaV/sMMjI9Sjun\n\tj+n3Xvm4uFNv9EfKHhsfHnliwvE0h4Xo00PonagZEcYrfCx9oq0IH3JJt8UTmGszjdDR\n\thFHgr8lPvxVptiwroTLiCKOSyDb/zTvw/JdRZJU8PatucusjqLCc06AzvvR6VPwhk/d3\n\tt5yg==","X-Gm-Message-State":"AOAM533bAMUUjva2bW+dmLSq3T05Ktbc25x0QcYFVvx/XClEgfvbiePc\n\to+C9E3sjA+uUc0iEyI9p2B+UrrwWNAvaa81OK/D3AcGvtUZCCw==","X-Google-Smtp-Source":"ABdhPJy004YIUwPE1ewMjNFxs81LBJz4QudaJv1IPScXpckHWCSzR0xSVWOAcyyWl5QsPm1BsAetujxRjzvuwlB0TMs=","X-Received":"by 2002:a2e:9b10:: with SMTP id u16mr815524lji.253.1616448140678;\n\tMon, 22 Mar 2021 14:22:20 -0700 (PDT)","MIME-Version":"1.0","References":"<20210317100211.1067585-1-naush@raspberrypi.com>\n\t<20210317100211.1067585-3-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Mon, 22 Mar 2021 21:22:04 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH 2/7] pipeline: ipa: raspberrypi: Open\n\tthe CamHelper on ipa::init()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"multipart/mixed;\n\tboundary=\"===============3749619607009559817==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15830,"web_url":"https://patchwork.libcamera.org/comment/15830/","msgid":"","date":"2021-03-23T13:02:38","subject":"Re: [libcamera-devel] [PATCH 2/7] pipeline: ipa: raspberrypi: Open\n\tthe CamHelper on ipa::init()","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, Mar 22, 2021 at 09:22:04PM +0000, Naushir Patuck wrote:\n> On Mon, 22 Mar 2021 at 21:07, Laurent Pinchart wrote:\n> > On Wed, Mar 17, 2021 at 10:02:06AM +0000, Naushir Patuck wrote:\n> > > Move the opening of the CamHelper from ipa::configure() to ipa::init().\n> > > This allows the pipeline handler to get the sensor specific parameters\n> > > in in pipeline_handler::match() where the ipa is initialised.\n> >\n> > s/in in/in/\n> >\n> > > Having the sensor parameters available earlier will allow selective\n> > > use of the embedded data node in a future change.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > include/libcamera/ipa/raspberrypi.mojom | 7 +--\n> > > src/ipa/raspberrypi/raspberrypi.cpp | 60 +++++++++----------\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 36 ++++++-----\n> > > 3 files changed, 46 insertions(+), 57 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> > > index eb427697d349..a713c88eee19 100644\n> > > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > > @@ -15,10 +15,6 @@ enum BufferMask {\n> > > /* Size of the LS grid allocation. */\n> > > const uint32 MaxLsGridSize = 0x8000;\n> > >\n> > > -enum ConfigOutputParameters {\n> > > - ConfigSensorParams = 0x01,\n> > > -};\n> > > -\n> > > struct SensorConfig {\n> > > uint32 gainDelay;\n> > > uint32 exposureDelay;\n> > > @@ -41,7 +37,6 @@ struct ConfigInput {\n> > >\n> > > struct ConfigOutput {\n> > > uint32 params;\n> >\n> > Unless I'm mistaken, the params field isn't used anymore. Unless it's\n> > used in a subsequent patch in the series, you can drop it.\n> \n> This (and other bits) does get tidied up in the later commits for this\n> series.\n> \n> > > - SensorConfig sensorConfig;\n> > > ControlList controls;\n> > > };\n> > >\n> > > @@ -51,7 +46,7 @@ struct StartControls {\n> > > };\n> > >\n> > > interface IPARPiInterface {\n> > > - init(IPASettings settings) => (int32 ret);\n> > > + init(IPASettings settings) => (int32 ret, SensorConfig sensorConfig);\n> > > start(StartControls controls) => (StartControls result);\n> > > stop();\n> > >\n> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > index 7904225a29fa..8a9eb92b39ec 100644\n> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > @@ -79,7 +79,7 @@ public:\n> > > munmap(lsTable_, ipa::RPi::MaxLsGridSize);\n> > > }\n> > >\n> > > - int init(const IPASettings &settings) override;\n> > > + int init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig) override;\n> > > void start(const ipa::RPi::StartControls &data,\n> > > ipa::RPi::StartControls *result) override;\n> > > void stop() override {}\n> > > @@ -164,9 +164,35 @@ private:\n> > > double maxFrameDuration_;\n> > > };\n> > >\n> > > -int IPARPi::init(const IPASettings &settings)\n> > > +int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig)\n> > > {\n> > > tuningFile_ = settings.configurationFile;\n> > > +\n> > > + /*\n> > > + * Load the \"helper\" for this sensor. This tells us all the device specific stuff\n> > > + * that the kernel driver doesn't. We only do this the first time; we don't need\n> > > + * to re-parse the metadata after a simple mode-switch for no reason.\n> > > + */\n> > > + helper_ = std::unique_ptr(RPiController::CamHelper::Create(settings.sensorModel));\n> >\n> > Not something that needs to be done as a prerequisite, but would it make\n> > sense to change the return type of RPiController::CamHelper::Create() to\n> > std::unique_ptr ?\n> \n> Yes, that seems reasonable. I will make the change separately to this\n> series.\n> \n> > > + if (!helper_) {\n> > > + LOG(IPARPI, Error) << \"Could not create camera helper for \"\n> > > + << settings.sensorModel;\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + /*\n> > > + * Pass out the sensor config to the pipeline handler in order\n> > > + * to setup the staggered writer class.\n> > > + */\n> > > + int gainDelay, exposureDelay, vblankDelay, sensorMetadata;\n> > > + helper_->GetDelays(exposureDelay, gainDelay, vblankDelay);\n> > > + sensorMetadata = helper_->SensorEmbeddedDataPresent();\n> > > +\n> > > + sensorConfig->gainDelay = gainDelay;\n> > > + sensorConfig->exposureDelay = exposureDelay;\n> > > + sensorConfig->vblankDelay = vblankDelay;\n> > > + sensorConfig->sensorMetadata = sensorMetadata;\n> > > +\n> > > return 0;\n> > > }\n> > >\n> > > @@ -319,36 +345,6 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > > /* Setup a metadata ControlList to output metadata. */\n> > > libcameraMetadata_ = ControlList(controls::controls);\n> > >\n> > > - /*\n> > > - * Load the \"helper\" for this sensor. This tells us all the device specific stuff\n> > > - * that the kernel driver doesn't. We only do this the first time; we don't need\n> > > - * to re-parse the metadata after a simple mode-switch for no reason.\n> > > - */\n> > > - std::string cameraName(sensorInfo.model);\n> > > - if (!helper_) {\n> > > - helper_ = std::unique_ptr(RPiController::CamHelper::Create(cameraName));\n> > > -\n> > > - if (!helper_) {\n> > > - LOG(IPARPI, Error) << \"Could not create camera helper for \"\n> > > - << cameraName;\n> > > - return -1;\n> > > - }\n> > > -\n> > > - /*\n> > > - * Pass out the sensor config to the pipeline handler in order\n> > > - * to setup the staggered writer class.\n> > > - */\n> > > - int gainDelay, exposureDelay, vblankDelay, sensorMetadata;\n> > > - helper_->GetDelays(exposureDelay, gainDelay, vblankDelay);\n> > > - sensorMetadata = helper_->SensorEmbeddedDataPresent();\n> > > -\n> > > - result->params |= ipa::RPi::ConfigSensorParams;\n> > > - result->sensorConfig.gainDelay = gainDelay;\n> > > - result->sensorConfig.exposureDelay = exposureDelay;\n> > > - result->sensorConfig.vblankDelay = vblankDelay;\n> > > - result->sensorConfig.sensorMetadata = sensorMetadata;\n> > > - }\n> > > -\n> > > /* Re-assemble camera mode using the sensor info. */\n> > > setMode(sensorInfo);\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 2c8ae31a28ad..ce1994186d66 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -146,7 +146,7 @@ public:\n> > >\n> > > void frameStarted(uint32_t sequence);\n> > >\n> > > - int loadIPA();\n> > > + int loadIPA(ipa::RPi::SensorConfig *sensorConfig);\n> > > int configureIPA(const CameraConfiguration *config);\n> > >\n> > > void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n> > > @@ -1030,11 +1030,24 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> > > if (data->sensor_->init())\n> > > return false;\n> > >\n> > > - if (data->loadIPA()) {\n> > > + ipa::RPi::SensorConfig sensorConfig;\n> > > + if (data->loadIPA(&sensorConfig)) {\n> > > LOG(RPI, Error) << \"Failed to load a suitable IPA library\";\n> > > return false;\n> > > }\n> > >\n> > > + /*\n> > > + * Setup our delayed control writer with the sensor default\n> > > + * gain and exposure delays. Mark VBLANK for priority write.\n> > > + */\n> > > + std::unordered_map params = {\n> > > + { V4L2_CID_ANALOGUE_GAIN, { sensorConfig.gainDelay, false } },\n> > > + { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false } },\n> > > + { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }\n> > > + };\n> > > + data->delayedCtrls_ = std::make_unique(data->unicam_[Unicam::Image].dev(), params);\n> > > + data->sensorMetadata_ = sensorConfig.sensorMetadata;\n> > > +\n> >\n> > As this code deals with RPiCameraData only, could you move it to\n> > RPiCameraData::loadIPA() ? That way you won't have to prefix everything\n> > with data->, and you'll avoid passing sensorConfig to loadIPA().\n> \n> My original code did have this block in loadIPA(), but I ran into a small problem.\n> DelayedControls() needs to have the device (unicam_[Unicam::Image] in this case)\n> opened in order to validate the controls that are passed in via params. loadIPA()\n> gets called before the devices are opened, so I had to move this code block\n> here.\n> \n> I could have moved it into loadIPA() with unicam_[Unicam::Image] already opened,\n> but then I would have to split the opening of unicam_[Unicam::Image] and\n> unicam_[Unicam::Embedded] into two separate locations, which I thought might be\n> undesirable. I am not completely against it, what are your thoughts?\n\nIt's a good point. Let's leave it as-is for now.\n\n> > With these small changes,\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > /* Register the controls that the Raspberry Pi IPA can handle. */\n> > > data->controlInfo_ = RPi::Controls;\n> > > /* Initialize the camera properties. */\n> > > @@ -1214,7 +1227,7 @@ void RPiCameraData::frameStarted(uint32_t sequence)\n> > > delayedCtrls_->applyControls(sequence);\n> > > }\n> > >\n> > > -int RPiCameraData::loadIPA()\n> > > +int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n> > > {\n> > > ipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> > >\n> > > @@ -1230,7 +1243,7 @@ int RPiCameraData::loadIPA()\n> > > IPASettings settings(ipa_->configurationFile(sensor_->model() + \".json\"),\n> > > sensor_->model());\n> > >\n> > > - return ipa_->init(settings);\n> > > + return ipa_->init(settings, sensorConfig);\n> > > }\n> > >\n> > > int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> > > @@ -1293,21 +1306,6 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> > > return -EPIPE;\n> > > }\n> > >\n> > > - if (result.params & ipa::RPi::ConfigSensorParams) {\n> > > - /*\n> > > - * Setup our delayed control writer with the sensor default\n> > > - * gain and exposure delays. Mark VBLANK for priority write.\n> > > - */\n> > > - std::unordered_map params = {\n> > > - { V4L2_CID_ANALOGUE_GAIN, { result.sensorConfig.gainDelay, false } },\n> > > - { V4L2_CID_EXPOSURE, { result.sensorConfig.exposureDelay, false } },\n> > > - { V4L2_CID_VBLANK, { result.sensorConfig.vblankDelay, true } }\n> > > - };\n> > > -\n> > > - delayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), params);\n> > > - sensorMetadata_ = result.sensorConfig.sensorMetadata;\n> > > - }\n> > > -\n> > > if (!result.controls.empty()) {\n> > > ControlList &ctrls = result.controls;\n> > > unicam_[Unicam::Image].dev()->setControls(&ctrls);","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 7253EC32E5\n\tfor ;\n\tTue, 23 Mar 2021 13:03:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6D1568D63;\n\tTue, 23 Mar 2021 14:03:21 +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 93902602D7\n\tfor ;\n\tTue, 23 Mar 2021 14:03:20 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E884885;\n\tTue, 23 Mar 2021 14:03:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fG69nGAZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616504600;\n\tbh=AKKefPGX9ZFV5hOTc7xrphXkacAEQwjYUgP9BS0WdyQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fG69nGAZ7xfN0+10jH/qnHLFmJMIok5CCRz529g9kOAJMnNZD6Yal7Q+/OUIFyH+0\n\twkQKsGf18a47XloKxk5cFqLak70SX/AzDJHeeiKV7frwWvd6y23KmH38wz7TETe0ty\n\tm8+cR1ipystMFT8uWnaSsV7TqvZd0EwWWLvYe7jw=","Date":"Tue, 23 Mar 2021 15:02:38 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210317100211.1067585-1-naush@raspberrypi.com>\n\t<20210317100211.1067585-3-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/7] pipeline: ipa: raspberrypi: Open\n\tthe CamHelper on ipa::init()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]