[{"id":14420,"web_url":"https://patchwork.libcamera.org/comment/14420/","msgid":"<8aa4189e-6bef-b412-319f-6d5c91c02dcd@ideasonboard.com>","date":"2020-12-30T17:00:51","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Niklas,\n\nThanks for the patch.\n\nOn 29/12/2020 17:03, Niklas Söderlund wrote:\n> Attach to the IPA and allow it to push V4L2 controls that applies on the\n> camera sensor. The IPA is not fully integrated in the pipeline as\n> statistics and parameters buffers are not yet allocated, processed by\n> the IPA nor queued to the hardware.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> * Changes since v1\n> - Rewrite to not use CameraSensor.\n> - Fix mistake where configuration sen to IPA was overwritten.\n> - Check that IPA configuration was successful before starting.\n> - Update commit message.\n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 103 +++++++++++++++++++++++++++\n> 1 file changed, 103 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index a87ce8f3378ba2fe..95f1b75dc8be5d40 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -14,11 +14,14 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> #include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -53,6 +56,8 @@ public:\n> \t{\n> \t}\n> \n> +\tint loadIPA();\n> +\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n> \n> @@ -62,6 +67,11 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tstd::unique_ptr delayedCtrls_;\n> +\n> +private:\n> +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -590,6 +600,13 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tIPU3CameraData *data = cameraData(camera);\n> \tCIO2Device *cio2 = &data->cio2_;\n> \tImgUDevice *imgu = data->imgu_;\n> +\n> +\tCameraSensorInfo sensorInfo = {};\n> +\tstd::map streamConfig;\n> +\tstd::map entityControls;\n> +\tIPAOperationData ipaConfig;\n> +\tIPAOperationData result = {};\n> +\n> \tint ret;\n> \n> \t/* Allocate buffers for internal pipeline usage. */\n> @@ -597,6 +614,11 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tIPAOperationData ipaData = {};\n> +\tret = data->ipa_->start(ipaData, nullptr);\n> +\tif (ret)\n> +\t\tgoto error;\n> +\n> \t/*\n> \t * Start the ImgU video devices, buffers will be queued to the\n> \t * ImgU output and viewfinder when requests will be queued.\n> @@ -612,9 +634,40 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \t\tgoto error;\n> \t}\n> \n> +\t/* Inform IPA of stream configuration and sensor controls. */\n> +\tret = data->cio2_.sensor()->sensorInfo(&sensorInfo);\n> +\tif (ret) {\n> +\t\t/* \\todo Turn to hard failure once sensors info is mandatory. */\n> +\t\tLOG(IPU3, Warning) << \"Camera sensor information not available\";\n> +\t\tsensorInfo = {};\n> +\t\tret = 0;\n> +\t}\n> +\n> +\tstreamConfig[0] = {\n> +\t\t.pixelFormat = data->outStream_.configuration().pixelFormat,\n> +\t\t.size = data->outStream_.configuration().size,\n> +\t};\n> +\tstreamConfig[1] = {\n> +\t\t.pixelFormat = data->vfStream_.configuration().pixelFormat,\n> +\t\t.size = data->vfStream_.configuration().size,\n> +\t};\n> +\n> +\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> +\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, &result);\n> +\n> +\tif ((result.operation != IPU3_IPA_STATUS_CONFIGURATION) ||\n> +\t (result.data.size() != 1) || (result.data.at(0) != 1)) {\n> +\t\tLOG(IPU3, Warning) << \"Failed to configure IPA\";\n> +\t\tret = -EINVAL;\n> +\t\tgoto error;\n> +\t}\n> +\n> \treturn 0;\n> \n> error:\n> +\tdata->ipa_->stop();\n> \tfreeBuffers(camera);\n> \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> \n> @@ -631,6 +684,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> \tif (ret)\n> \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> \n> +\tdata->ipa_->stop();\n> +\n> \tfreeBuffers(camera);\n> }\n> \n> @@ -762,12 +817,32 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> +\t\tret = data->loadIPA();\n> +\t\tif (ret)\n> +\t\t\tcontinue;\n> +\n> \t\t/* Initialize the camera properties. */\n> \t\tdata->properties_ = cio2->sensor()->properties();\n> \n> \t\t/* Initialze the camera controls. */\n> \t\tdata->controlInfo_ = IPU3Controls;\n> \n> +\t\t/*\n> +\t\t * \\todo Read dealy values from the sensor itself or from a\n> +\t\t * a sensor database. For now use generic values taken from\n> +\t\t * the Raspberry Pi and listed as generic values.\n> +\t\t */\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> +\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> +\t\t};\n\nI agree, the controls we want to have need to be parsed in some way.\nI am wondering what makes a control beeing a \"delayedControl\" and what\nis not ?\n\nLooking at the RPi pipeline and IPA, I can see the\nV4L2_CID_ANALOGUE_GAIN and V4L2_CID_EXPOSURE controls beeing delayed\ncontrols, but in the RPi IPA there is for example a call to the\nV4L2_CID_DIGITAL_GAIN control.\n\nWould we need a database to know which controls are effectively to be\ndelayed ones, and when the sensor is configured, parse the controls\navailable and add the corresponding ones to the map ?\nThis is an open question :-).\n\n> +\t\tdata->delayedCtrls_ =\n> +\t\t\tstd::make_unique(cio2->sensor()->device(),\n> +\t\t\t\t\t\t\t delays);\n> +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t &DelayedControls::applyControls);\n> +\n> \t\t/**\n> \t\t * \\todo Dynamically assign ImgU and output devices to each\n> \t\t * stream and camera; as of now, limit support to two cameras\n> @@ -811,6 +886,34 @@ int PipelineHandlerIPU3::registerCameras()\n> \treturn numCameras ? 0 : -ENODEV;\n> }\n> \n> +int IPU3CameraData::loadIPA()\n> +{\n> +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> +\tif (!ipa_)\n> +\t\treturn -ENOENT;\n> +\n> +\tipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa);\n> +\n> +\tipa_->init(IPASettings{});\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> +\t\t\t const IPAOperationData &action)\n> +{\n> +\tswitch (action.operation) {\n> +\tcase IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {\n> +\t\tconst ControlList &controls = action.controls[0];\n> +\t\tdelayedCtrls_->push(controls);\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> /* -----------------------------------------------------------------------------\n> * Buffer Ready slots\n> */\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 77222C0F1A\n\tfor ;\n\tWed, 30 Dec 2020 17:00:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E04AC615B4;\n\tWed, 30 Dec 2020 18:00:53 +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 87F5860526\n\tfor ;\n\tWed, 30 Dec 2020 18:00:52 +0100 (CET)","from [IPv6:2a01:e0a:169:7140:a3f5:d5c:b0c8:4cf1] (unknown\n\t[IPv6:2a01:e0a:169:7140:a3f5:d5c:b0c8:4cf1])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1C7822A3;\n\tWed, 30 Dec 2020 18:00:52 +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=\"Up4Zneuw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609347652;\n\tbh=p9R6qcrBQPs1SJM9qEZnwqu2b7l5n+0i18gpoaQwUrU=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=Up4ZneuwEsTXlsT9lSLXafOA7SurWtg3SfyCdjOBxjV1aH8XuMHF7RkuLKU7o6yrL\n\tc2oFeNdGEo4WcZ0ET54Sj+LpkQYKLaESTTIvvmZwF2epDaoc+hzoHqxJFiF+8YFVve\n\tFUYrd8WDCC3tMHwfgFRtEaxc7rsSYTc07EkCEPRE=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-8-niklas.soderlund@ragnatech.se>","From":"Jean-Michel Hautbois ","Message-ID":"<8aa4189e-6bef-b412-319f-6d5c91c02dcd@ideasonboard.com>","Date":"Wed, 30 Dec 2020 18:00:51 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.5.0","MIME-Version":"1.0","In-Reply-To":"<20201229160318.77536-8-niklas.soderlund@ragnatech.se>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14467,"web_url":"https://patchwork.libcamera.org/comment/14467/","msgid":"","date":"2021-01-07T12:00:00","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Niklas,\n\nOn 12/29/20 9:33 PM, Niklas Söderlund wrote:\n> Attach to the IPA and allow it to push V4L2 controls that applies on the\n> camera sensor. The IPA is not fully integrated in the pipeline as\n> statistics and parameters buffers are not yet allocated, processed by\n> the IPA nor queued to the hardware.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> * Changes since v1\n> - Rewrite to not use CameraSensor.\n> - Fix mistake where configuration sen to IPA was overwritten.\n> - Check that IPA configuration was successful before starting.\n> - Update commit message.\n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 103 +++++++++++++++++++++++++++\n> 1 file changed, 103 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index a87ce8f3378ba2fe..95f1b75dc8be5d40 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -14,11 +14,14 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> #include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -53,6 +56,8 @@ public:\n> \t{\n> \t}\n> \n> +\tint loadIPA();\n> +\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n> \n> @@ -62,6 +67,11 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tstd::unique_ptr delayedCtrls_;\n> +\n> +private:\n> +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -590,6 +600,13 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tIPU3CameraData *data = cameraData(camera);\n> \tCIO2Device *cio2 = &data->cio2_;\n> \tImgUDevice *imgu = data->imgu_;\n> +\n> +\tCameraSensorInfo sensorInfo = {};\n> +\tstd::map streamConfig;\n> +\tstd::map entityControls;\n> +\tIPAOperationData ipaConfig;\n> +\tIPAOperationData result = {};\n> +\n> \tint ret;\n> \n> \t/* Allocate buffers for internal pipeline usage. */\n> @@ -597,6 +614,11 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tIPAOperationData ipaData = {};\n> +\tret = data->ipa_->start(ipaData, nullptr);\n> +\tif (ret)\n> +\t\tgoto error;\n> +\n> \t/*\n> \t * Start the ImgU video devices, buffers will be queued to the\n> \t * ImgU output and viewfinder when requests will be queued.\n> @@ -612,9 +634,40 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \t\tgoto error;\n> \t}\n> \n> +\t/* Inform IPA of stream configuration and sensor controls. */\n> +\tret = data->cio2_.sensor()->sensorInfo(&sensorInfo);\n> +\tif (ret) {\n> +\t\t/* \\todo Turn to hard failure once sensors info is mandatory. */\n> +\t\tLOG(IPU3, Warning) << \"Camera sensor information not available\";\n> +\t\tsensorInfo = {};\n> +\t\tret = 0;\n> +\t}\n> +\n> +\tstreamConfig[0] = {\n> +\t\t.pixelFormat = data->outStream_.configuration().pixelFormat,\n> +\t\t.size = data->outStream_.configuration().size,\n> +\t};\n> +\tstreamConfig[1] = {\n> +\t\t.pixelFormat = data->vfStream_.configuration().pixelFormat,\n> +\t\t.size = data->vfStream_.configuration().size,\n> +\t};\n> +\n> +\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> +\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, &result);\n> +\n> +\tif ((result.operation != IPU3_IPA_STATUS_CONFIGURATION) ||\n> +\t (result.data.size() != 1) || (result.data.at(0) != 1)) {\n> +\t\tLOG(IPU3, Warning) << \"Failed to configure IPA\";\n> +\t\tret = -EINVAL;\n> +\t\tgoto error;\n> +\t}\n> +\n> \treturn 0;\n> \n> error:\n> +\tdata->ipa_->stop();\n> \tfreeBuffers(camera);\n> \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> \n> @@ -631,6 +684,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> \tif (ret)\n> \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> \n> +\tdata->ipa_->stop();\n> +\n> \tfreeBuffers(camera);\n> }\n> \n> @@ -762,12 +817,32 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> +\t\tret = data->loadIPA();\n> +\t\tif (ret)\n> +\t\t\tcontinue;\n> +\n> \t\t/* Initialize the camera properties. */\n> \t\tdata->properties_ = cio2->sensor()->properties();\n> \n> \t\t/* Initialze the camera controls. */\n> \t\tdata->controlInfo_ = IPU3Controls;\n> \n> +\t\t/*\n> +\t\t * \\todo Read dealy values from the sensor itself or from a\ns/dealy/delay/\n> +\t\t * a sensor database. For now use generic values taken from\n> +\t\t * the Raspberry Pi and listed as generic values.\nmay be you meant : s/generic values/'generic values'  ?\n> +\t\t */\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> +\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> +\t\t};\n> +\n> +\t\tdata->delayedCtrls_ =\n> +\t\t\tstd::make_unique(cio2->sensor()->device(),\n> +\t\t\t\t\t\t\t delays);\n> +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t &DelayedControls::applyControls);\n> +\n> \t\t/**\n> \t\t * \\todo Dynamically assign ImgU and output devices to each\n> \t\t * stream and camera; as of now, limit support to two cameras\n> @@ -811,6 +886,34 @@ int PipelineHandlerIPU3::registerCameras()\n> \treturn numCameras ? 0 : -ENODEV;\n> }\n> \n> +int IPU3CameraData::loadIPA()\n> +{\n> +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> +\tif (!ipa_)\n> +\t\treturn -ENOENT;\n> +\n> +\tipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa);\n> +\n> +\tipa_->init(IPASettings{});\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> +\t\t\t const IPAOperationData &action)\n> +{\n> +\tswitch (action.operation) {\n> +\tcase IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {\n> +\t\tconst ControlList &controls = action.controls[0];\n> +\t\tdelayedCtrls_->push(controls);\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> /* -----------------------------------------------------------------------------\n> * Buffer Ready slots\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 2B55BC3256\n\tfor ;\n\tThu, 7 Jan 2021 12:00:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8FDC763411;\n\tThu, 7 Jan 2021 13:00:08 +0100 (CET)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8013062013\n\tfor ;\n\tThu, 7 Jan 2021 13:00:06 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"QrK5P+5V\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1610020805; bh=x1O3bYI0UCbeKhO1wK3+0ugsqCvRMb/1o8huisTabok=;\n\th=Subject:To:References:From:In-Reply-To;\n\tb=QrK5P+5VSoa6e1B0YkxJuIVwNVKBUn/zTezmCFP3pT0PbjkV5FL6JsMsYw9OPyV77\n\tA8TSes0Jj6KnPhZ4sNAA9NGebgUuRr1JwBHDwNXvKhV+Ns9RKUq/12FrhEnvJMEvr8\n\tSshFg3EgmCkowaFjUQ5o4MB3KMiQ+cZhniJdkbQ0s1zE11LzEAKv0VpvmOc4ix3dB3\n\tZx+PCpUitQPrk3AskUe0xASnsqJzpqNCp9p39qlptRFGIiCSheI9d/r+91RyKaJm5S\n\trutvOglrBQjdeQyi2P3ZhoDzHt6KuQ67N+Ao3eE/uIO2zaIsLbM5I8/rIprXeEAF/z\n\tSVsuEKVxx/fyQ==","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-8-niklas.soderlund@ragnatech.se>","From":"Umang Jain ","Message-ID":"","Date":"Thu, 7 Jan 2021 17:30:00 +0530","Mime-Version":"1.0","In-Reply-To":"<20201229160318.77536-8-niklas.soderlund@ragnatech.se>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","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","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14469,"web_url":"https://patchwork.libcamera.org/comment/14469/","msgid":"<20210107124050.udy2du6bbqq7zxpe@uno.localdomain>","date":"2021-01-07T12:40:50","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Dec 29, 2020 at 05:03:14PM +0100, Niklas Söderlund wrote:\n> Attach to the IPA and allow it to push V4L2 controls that applies on the\n> camera sensor. The IPA is not fully integrated in the pipeline as\n> statistics and parameters buffers are not yet allocated, processed by\n> the IPA nor queued to the hardware.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> * Changes since v1\n> - Rewrite to not use CameraSensor.\n> - Fix mistake where configuration sen to IPA was overwritten.\n> - Check that IPA configuration was successful before starting.\n> - Update commit message.\n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 103 +++++++++++++++++++++++++++\n> 1 file changed, 103 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index a87ce8f3378ba2fe..95f1b75dc8be5d40 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -14,11 +14,14 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n>\n> #include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -53,6 +56,8 @@ public:\n> \t{\n> \t}\n>\n> +\tint loadIPA();\n> +\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n>\n> @@ -62,6 +67,11 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tstd::unique_ptr delayedCtrls_;\n> +\n> +private:\n> +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> };\n>\n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -590,6 +600,13 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tIPU3CameraData *data = cameraData(camera);\n> \tCIO2Device *cio2 = &data->cio2_;\n> \tImgUDevice *imgu = data->imgu_;\n> +\n> +\tCameraSensorInfo sensorInfo = {};\n> +\tstd::map streamConfig;\n> +\tstd::map entityControls;\n> +\tIPAOperationData ipaConfig;\n> +\tIPAOperationData result = {};\n\nYou could s/= {}/{}\nbut it's minor\n\n> +\n> \tint ret;\n>\n> \t/* Allocate buffers for internal pipeline usage. */\n> @@ -597,6 +614,11 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tif (ret)\n> \t\treturn ret;\n>\n> +\tIPAOperationData ipaData = {};\n> +\tret = data->ipa_->start(ipaData, nullptr);\n> +\tif (ret)\n> +\t\tgoto error;\n> +\n> \t/*\n> \t * Start the ImgU video devices, buffers will be queued to the\n> \t * ImgU output and viewfinder when requests will be queued.\n> @@ -612,9 +634,40 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \t\tgoto error;\n> \t}\n>\n> +\t/* Inform IPA of stream configuration and sensor controls. */\n> +\tret = data->cio2_.sensor()->sensorInfo(&sensorInfo);\n> +\tif (ret) {\n> +\t\t/* \\todo Turn to hard failure once sensors info is mandatory. */\n\nwe're rather working in the direction of making SensorInfo never fail.\nI think you can drop this todo and hard fail here as if you have a\nfailure, it's because something bad happened when reading controls or\nselection targets.\n\n> +\t\tLOG(IPU3, Warning) << \"Camera sensor information not available\";\n> +\t\tsensorInfo = {};\n> +\t\tret = 0;\n> +\t}\n> +\n> +\tstreamConfig[0] = {\n> +\t\t.pixelFormat = data->outStream_.configuration().pixelFormat,\n> +\t\t.size = data->outStream_.configuration().size,\n> +\t};\n> +\tstreamConfig[1] = {\n> +\t\t.pixelFormat = data->vfStream_.configuration().pixelFormat,\n> +\t\t.size = data->vfStream_.configuration().size,\n> +\t};\n\nIs this ok if one of the two streams (likely vfStream_) is not\nassigned ?\n\n> +\n> +\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> +\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, &result);\n> +\n> +\tif ((result.operation != IPU3_IPA_STATUS_CONFIGURATION) ||\n\nThis seems like it can't happen at the moment, but it's ok\n\n> +\t (result.data.size() != 1) || (result.data.at(0) != 1)) {\n\nI think the fact that returning 1 in data[0] means success should be\ndocumented in the IPA protocol. Ah wait, we don't have one.\n\n> +\t\tLOG(IPU3, Warning) << \"Failed to configure IPA\";\n> +\t\tret = -EINVAL;\n> +\t\tgoto error;\n> +\t}\n> +\n> \treturn 0;\n>\n> error:\n> +\tdata->ipa_->stop();\n> \tfreeBuffers(camera);\n> \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n>\n> @@ -631,6 +684,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> \tif (ret)\n> \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n>\n> +\tdata->ipa_->stop();\n> +\n> \tfreeBuffers(camera);\n> }\n>\n> @@ -762,12 +817,32 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\tif (ret)\n> \t\t\tcontinue;\n>\n> +\t\tret = data->loadIPA();\n> +\t\tif (ret)\n> +\t\t\tcontinue;\n> +\n> \t\t/* Initialize the camera properties. */\n> \t\tdata->properties_ = cio2->sensor()->properties();\n>\n> \t\t/* Initialze the camera controls. */\n> \t\tdata->controlInfo_ = IPU3Controls;\n>\n> +\t\t/*\n> +\t\t * \\todo Read dealy values from the sensor itself or from a\n> +\t\t * a sensor database. For now use generic values taken from\n> +\t\t * the Raspberry Pi and listed as generic values.\n> +\t\t */\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> +\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> +\t\t};\n> +\n> +\t\tdata->delayedCtrls_ =\n> +\t\t\tstd::make_unique(cio2->sensor()->device(),\n> +\t\t\t\t\t\t\t delays);\n\nsigh\n\nwe should get the sensor database in sooner than later and move this\nto the camera sensor before the same patter spread in multiple\npipelines\n\n> +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t &DelayedControls::applyControls);\n> +\n> \t\t/**\n> \t\t * \\todo Dynamically assign ImgU and output devices to each\n> \t\t * stream and camera; as of now, limit support to two cameras\n> @@ -811,6 +886,34 @@ int PipelineHandlerIPU3::registerCameras()\n> \treturn numCameras ? 0 : -ENODEV;\n> }\n>\n> +int IPU3CameraData::loadIPA()\n> +{\n> +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> +\tif (!ipa_)\n> +\t\treturn -ENOENT;\n> +\n> +\tipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa);\n> +\n> +\tipa_->init(IPASettings{});\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> +\t\t\t const IPAOperationData &action)\n> +{\n> +\tswitch (action.operation) {\n> +\tcase IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {\n> +\t\tconst ControlList &controls = action.controls[0];\n> +\t\tdelayedCtrls_->push(controls);\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n\nOverall, that's a good starting point\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> /* -----------------------------------------------------------------------------\n> * Buffer Ready slots\n> */\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 B0D7DC3256\n\tfor ;\n\tThu, 7 Jan 2021 12:40:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1CF376347A;\n\tThu, 7 Jan 2021 13:40:37 +0100 (CET)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6AA5462013\n\tfor ;\n\tThu, 7 Jan 2021 13:40:35 +0100 (CET)","from uno.localdomain (unknown [93.56.74.111])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 8ED6EFF806;\n\tThu, 7 Jan 2021 12:40:34 +0000 (UTC)"],"X-Originating-IP":"93.56.74.111","Date":"Thu, 7 Jan 2021 13:40:50 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210107124050.udy2du6bbqq7zxpe@uno.localdomain>","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-8-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201229160318.77536-8-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14496,"web_url":"https://patchwork.libcamera.org/comment/14496/","msgid":"","date":"2021-01-10T16:10:27","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Wed, Dec 30, 2020 at 06:00:51PM +0100, Jean-Michel Hautbois wrote:\n> On 29/12/2020 17:03, Niklas Söderlund wrote:\n> > Attach to the IPA and allow it to push V4L2 controls that applies on the\n> > camera sensor. The IPA is not fully integrated in the pipeline as\n> > statistics and parameters buffers are not yet allocated, processed by\n> > the IPA nor queued to the hardware.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > * Changes since v1\n> > - Rewrite to not use CameraSensor.\n> > - Fix mistake where configuration sen to IPA was overwritten.\n> > - Check that IPA configuration was successful before starting.\n> > - Update commit message.\n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 103 +++++++++++++++++++++++++++\n> > 1 file changed, 103 insertions(+)\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index a87ce8f3378ba2fe..95f1b75dc8be5d40 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -14,11 +14,14 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> > \n> > #include \"libcamera/internal/camera_sensor.h\"\n> > +#include \"libcamera/internal/delayed_controls.h\"\n> > #include \"libcamera/internal/device_enumerator.h\"\n> > +#include \"libcamera/internal/ipa_manager.h\"\n> > #include \"libcamera/internal/log.h\"\n> > #include \"libcamera/internal/media_device.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > @@ -53,6 +56,8 @@ public:\n> > \t{\n> > \t}\n> > \n> > +\tint loadIPA();\n> > +\n> > \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> > \tvoid cio2BufferReady(FrameBuffer *buffer);\n> > \n> > @@ -62,6 +67,11 @@ public:\n> > \tStream outStream_;\n> > \tStream vfStream_;\n> > \tStream rawStream_;\n> > +\n> > +\tstd::unique_ptr delayedCtrls_;\n> > +\n> > +private:\n> > +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> > };\n> > \n> > class IPU3CameraConfiguration : public CameraConfiguration\n> > @@ -590,6 +600,13 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > \tIPU3CameraData *data = cameraData(camera);\n> > \tCIO2Device *cio2 = &data->cio2_;\n> > \tImgUDevice *imgu = data->imgu_;\n> > +\n> > +\tCameraSensorInfo sensorInfo = {};\n> > +\tstd::map streamConfig;\n> > +\tstd::map entityControls;\n> > +\tIPAOperationData ipaConfig;\n> > +\tIPAOperationData result = {};\n> > +\n> > \tint ret;\n> > \n> > \t/* Allocate buffers for internal pipeline usage. */\n> > @@ -597,6 +614,11 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > +\tIPAOperationData ipaData = {};\n> > +\tret = data->ipa_->start(ipaData, nullptr);\n> > +\tif (ret)\n> > +\t\tgoto error;\n> > +\n> > \t/*\n> > \t * Start the ImgU video devices, buffers will be queued to the\n> > \t * ImgU output and viewfinder when requests will be queued.\n> > @@ -612,9 +634,40 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > \t\tgoto error;\n> > \t}\n> > \n> > +\t/* Inform IPA of stream configuration and sensor controls. */\n> > +\tret = data->cio2_.sensor()->sensorInfo(&sensorInfo);\n> > +\tif (ret) {\n> > +\t\t/* \\todo Turn to hard failure once sensors info is mandatory. */\n> > +\t\tLOG(IPU3, Warning) << \"Camera sensor information not available\";\n> > +\t\tsensorInfo = {};\n> > +\t\tret = 0;\n> > +\t}\n> > +\n> > +\tstreamConfig[0] = {\n> > +\t\t.pixelFormat = data->outStream_.configuration().pixelFormat,\n> > +\t\t.size = data->outStream_.configuration().size,\n> > +\t};\n> > +\tstreamConfig[1] = {\n> > +\t\t.pixelFormat = data->vfStream_.configuration().pixelFormat,\n> > +\t\t.size = data->vfStream_.configuration().size,\n> > +\t};\n> > +\n> > +\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> > +\n> > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > +\t\t\t ipaConfig, &result);\n> > +\n> > +\tif ((result.operation != IPU3_IPA_STATUS_CONFIGURATION) ||\n> > +\t (result.data.size() != 1) || (result.data.at(0) != 1)) {\n> > +\t\tLOG(IPU3, Warning) << \"Failed to configure IPA\";\n> > +\t\tret = -EINVAL;\n> > +\t\tgoto error;\n> > +\t}\n> > +\n> > \treturn 0;\n> > \n> > error:\n> > +\tdata->ipa_->stop();\n> > \tfreeBuffers(camera);\n> > \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> > \n> > @@ -631,6 +684,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> > \tif (ret)\n> > \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> > \n> > +\tdata->ipa_->stop();\n> > +\n> > \tfreeBuffers(camera);\n> > }\n> > \n> > @@ -762,12 +817,32 @@ int PipelineHandlerIPU3::registerCameras()\n> > \t\tif (ret)\n> > \t\t\tcontinue;\n> > \n> > +\t\tret = data->loadIPA();\n> > +\t\tif (ret)\n> > +\t\t\tcontinue;\n> > +\n> > \t\t/* Initialize the camera properties. */\n> > \t\tdata->properties_ = cio2->sensor()->properties();\n> > \n> > \t\t/* Initialze the camera controls. */\n> > \t\tdata->controlInfo_ = IPU3Controls;\n> > \n> > +\t\t/*\n> > +\t\t * \\todo Read dealy values from the sensor itself or from a\n> > +\t\t * a sensor database. For now use generic values taken from\n> > +\t\t * the Raspberry Pi and listed as generic values.\n> > +\t\t */\n> > +\t\tstd::unordered_map delays = {\n> > +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> > +\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> > +\t\t};\n> \n> I agree, the controls we want to have need to be parsed in some way.\n> I am wondering what makes a control beeing a \"delayedControl\" and what\n> is not ?\n> \n> Looking at the RPi pipeline and IPA, I can see the\n> V4L2_CID_ANALOGUE_GAIN and V4L2_CID_EXPOSURE controls beeing delayed\n> controls, but in the RPi IPA there is for example a call to the\n> V4L2_CID_DIGITAL_GAIN control.\n> \n> Would we need a database to know which controls are effectively to be\n> delayed ones, and when the sensor is configured, parse the controls\n> available and add the corresponding ones to the map ?\n> This is an open question :-).\n\nYes, we do, and the DelayedControl instance should be moved to the\nCameraSensor class, with control handling becoming more transparent for\nthe pipeline handlers.\n\n> > +\t\tdata->delayedCtrls_ =\n> > +\t\t\tstd::make_unique(cio2->sensor()->device(),\n> > +\t\t\t\t\t\t\t delays);\n> > +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> > +\t\t\t\t\t\t &DelayedControls::applyControls);\n> > +\n> > \t\t/**\n> > \t\t * \\todo Dynamically assign ImgU and output devices to each\n> > \t\t * stream and camera; as of now, limit support to two cameras\n> > @@ -811,6 +886,34 @@ int PipelineHandlerIPU3::registerCameras()\n> > \treturn numCameras ? 0 : -ENODEV;\n> > }\n> > \n> > +int IPU3CameraData::loadIPA()\n> > +{\n> > +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> > +\tif (!ipa_)\n> > +\t\treturn -ENOENT;\n> > +\n> > +\tipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa);\n> > +\n> > +\tipa_->init(IPASettings{});\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> > +\t\t\t const IPAOperationData &action)\n> > +{\n> > +\tswitch (action.operation) {\n> > +\tcase IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {\n> > +\t\tconst ControlList &controls = action.controls[0];\n> > +\t\tdelayedCtrls_->push(controls);\n> > +\t\tbreak;\n> > +\t}\n> > +\tdefault:\n> > +\t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> > +\t\tbreak;\n> > +\t}\n> > +}\n> > +\n> > /* -----------------------------------------------------------------------------\n> > * Buffer Ready slots\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 13940BD808\n\tfor ;\n\tSun, 10 Jan 2021 16:10:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8558768069;\n\tSun, 10 Jan 2021 17:10:42 +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 A353360523\n\tfor ;\n\tSun, 10 Jan 2021 17:10:41 +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 1BEABDA;\n\tSun, 10 Jan 2021 17:10:41 +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=\"KKN2YdOx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610295041;\n\tbh=pzKhiXuczZYBgTifmFOItCrIkAWQdQbsrE1ze92+6VU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KKN2YdOxSbsGDRYfWoCeHb9hgtO8lNhSkrNEHX0Ijuy7uQ9NT0hXtsIoZ3hbbsc7O\n\tMIm1ZIMRHK8SDUPEPyPtM6dPqw+JOMdHIjQwF7J7x4Nj4GrvDsqq1v8wg3+fitZCj+\n\tN8TF6hWlwGoLib5/y5LCrI8Y1fMNHoX08dGvYBHo=","Date":"Sun, 10 Jan 2021 18:10:27 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-8-niklas.soderlund@ragnatech.se>\n\t<8aa4189e-6bef-b412-319f-6d5c91c02dcd@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<8aa4189e-6bef-b412-319f-6d5c91c02dcd@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14497,"web_url":"https://patchwork.libcamera.org/comment/14497/","msgid":"","date":"2021-01-10T16:46:46","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Thu, Jan 07, 2021 at 01:40:50PM +0100, Jacopo Mondi wrote:\n> On Tue, Dec 29, 2020 at 05:03:14PM +0100, Niklas Söderlund wrote:\n> > Attach to the IPA and allow it to push V4L2 controls that applies on the\n\ns/applies on/apply to/\n\n> > camera sensor. The IPA is not fully integrated in the pipeline as\n> > statistics and parameters buffers are not yet allocated, processed by\n> > the IPA nor queued to the hardware.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > * Changes since v1\n> > - Rewrite to not use CameraSensor.\n> > - Fix mistake where configuration sen to IPA was overwritten.\n> > - Check that IPA configuration was successful before starting.\n> > - Update commit message.\n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 103 +++++++++++++++++++++++++++\n> > 1 file changed, 103 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index a87ce8f3378ba2fe..95f1b75dc8be5d40 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -14,11 +14,14 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> >\n> > #include \"libcamera/internal/camera_sensor.h\"\n> > +#include \"libcamera/internal/delayed_controls.h\"\n> > #include \"libcamera/internal/device_enumerator.h\"\n> > +#include \"libcamera/internal/ipa_manager.h\"\n> > #include \"libcamera/internal/log.h\"\n> > #include \"libcamera/internal/media_device.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > @@ -53,6 +56,8 @@ public:\n> > \t{\n> > \t}\n> >\n> > +\tint loadIPA();\n> > +\n> > \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> > \tvoid cio2BufferReady(FrameBuffer *buffer);\n> >\n> > @@ -62,6 +67,11 @@ public:\n> > \tStream outStream_;\n> > \tStream vfStream_;\n> > \tStream rawStream_;\n> > +\n> > +\tstd::unique_ptr delayedCtrls_;\n> > +\n> > +private:\n> > +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> > };\n> >\n> > class IPU3CameraConfiguration : public CameraConfiguration\n> > @@ -590,6 +600,13 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > \tIPU3CameraData *data = cameraData(camera);\n> > \tCIO2Device *cio2 = &data->cio2_;\n> > \tImgUDevice *imgu = data->imgu_;\n> > +\n> > +\tCameraSensorInfo sensorInfo = {};\n> > +\tstd::map streamConfig;\n> > +\tstd::map entityControls;\n> > +\tIPAOperationData ipaConfig;\n> > +\tIPAOperationData result = {};\n> \n> You could s/= {}/{}\n> but it's minor\n> \n> > +\n> > \tint ret;\n> >\n> > \t/* Allocate buffers for internal pipeline usage. */\n> > @@ -597,6 +614,11 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n> > +\tIPAOperationData ipaData = {};\n> > +\tret = data->ipa_->start(ipaData, nullptr);\n> > +\tif (ret)\n> > +\t\tgoto error;\n> > +\n> > \t/*\n> > \t * Start the ImgU video devices, buffers will be queued to the\n> > \t * ImgU output and viewfinder when requests will be queued.\n> > @@ -612,9 +634,40 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > \t\tgoto error;\n> > \t}\n> >\n> > +\t/* Inform IPA of stream configuration and sensor controls. */\n> > +\tret = data->cio2_.sensor()->sensorInfo(&sensorInfo);\n> > +\tif (ret) {\n> > +\t\t/* \\todo Turn to hard failure once sensors info is mandatory. */\n> \n> we're rather working in the direction of making SensorInfo never fail.\n> I think you can drop this todo and hard fail here as if you have a\n> failure, it's because something bad happened when reading controls or\n> selection targets.\n\nAgreed.\n\n> > +\t\tLOG(IPU3, Warning) << \"Camera sensor information not available\";\n> > +\t\tsensorInfo = {};\n> > +\t\tret = 0;\n> > +\t}\n> > +\n> > +\tstreamConfig[0] = {\n> > +\t\t.pixelFormat = data->outStream_.configuration().pixelFormat,\n> > +\t\t.size = data->outStream_.configuration().size,\n> > +\t};\n> > +\tstreamConfig[1] = {\n> > +\t\t.pixelFormat = data->vfStream_.configuration().pixelFormat,\n> > +\t\t.size = data->vfStream_.configuration().size,\n> > +\t};\n> \n> Is this ok if one of the two streams (likely vfStream_) is not\n> assigned ?\n\nBased on our current IPA interface, I'd say only active streams should\nbe reported here. We're however moving to a per-pipeline handler IPA\nprotocol, and it would be fine having a hardcoded array of two streams,\nas long, of course, as there's enough information for the IPA to tell\nwhich streams are active (invalid pixel format for instance). Obviously\na hardcoded array isn't required with the custom IPA protocol, we can\nstill device to pass the active streams only in that case.\n\nThe StreamConfiguration() stored in a Stream is initialized with\nappropriate defaults, but if we configure the camera with two streams,\nand then again with a single stream, I think we'll have stale values.\n\n> > +\n> > +\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> > +\n> > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > +\t\t\t ipaConfig, &result);\n> > +\n> > +\tif ((result.operation != IPU3_IPA_STATUS_CONFIGURATION) ||\n> \n> This seems like it can't happen at the moment, but it's ok\n\nYes, I'm not sure what the idea behind the check is.\n\n> > +\t (result.data.size() != 1) || (result.data.at(0) != 1)) {\n> \n> I think the fact that returning 1 in data[0] means success should be\n> documented in the IPA protocol. Ah wait, we don't have one.\n\nRPi checks\n\n\tif (result.operation & RPi::IPA_CONFIG_FAILED)\n\nand we could do something similar for now.\n\n> > +\t\tLOG(IPU3, Warning) << \"Failed to configure IPA\";\n> > +\t\tret = -EINVAL;\n> > +\t\tgoto error;\n> > +\t}\n> > +\n> > \treturn 0;\n> >\n> > error:\n> > +\tdata->ipa_->stop();\n> > \tfreeBuffers(camera);\n> > \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> >\n> > @@ -631,6 +684,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> > \tif (ret)\n> > \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> >\n> > +\tdata->ipa_->stop();\n> > +\n> > \tfreeBuffers(camera);\n> > }\n> >\n> > @@ -762,12 +817,32 @@ int PipelineHandlerIPU3::registerCameras()\n> > \t\tif (ret)\n> > \t\t\tcontinue;\n> >\n> > +\t\tret = data->loadIPA();\n> > +\t\tif (ret)\n> > +\t\t\tcontinue;\n> > +\n> > \t\t/* Initialize the camera properties. */\n> > \t\tdata->properties_ = cio2->sensor()->properties();\n> >\n> > \t\t/* Initialze the camera controls. */\n> > \t\tdata->controlInfo_ = IPU3Controls;\n> >\n> > +\t\t/*\n> > +\t\t * \\todo Read dealy values from the sensor itself or from a\n> > +\t\t * a sensor database. For now use generic values taken from\n> > +\t\t * the Raspberry Pi and listed as generic values.\n> > +\t\t */\n> > +\t\tstd::unordered_map delays = {\n> > +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> > +\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> > +\t\t};\n> > +\n> > +\t\tdata->delayedCtrls_ =\n> > +\t\t\tstd::make_unique(cio2->sensor()->device(),\n> > +\t\t\t\t\t\t\t delays);\n> \n> sigh\n> \n> we should get the sensor database in sooner than later and move this\n> to the camera sensor before the same patter spread in multiple\n> pipelines\n\nI can't disagree on this :-) (moving on the review of the corresponding\npatches next)\n\n> > +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> > +\t\t\t\t\t\t &DelayedControls::applyControls);\n> > +\n> > \t\t/**\n> > \t\t * \\todo Dynamically assign ImgU and output devices to each\n> > \t\t * stream and camera; as of now, limit support to two cameras\n> > @@ -811,6 +886,34 @@ int PipelineHandlerIPU3::registerCameras()\n> > \treturn numCameras ? 0 : -ENODEV;\n> > }\n> >\n> > +int IPU3CameraData::loadIPA()\n> > +{\n> > +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> > +\tif (!ipa_)\n> > +\t\treturn -ENOENT;\n> > +\n> > +\tipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa);\n> > +\n> > +\tipa_->init(IPASettings{});\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> > +\t\t\t const IPAOperationData &action)\n\nMaybe s/actOnIpa/queueFrameAction/ to match the other pipeline handlers\n?\n\n> > +{\n> > +\tswitch (action.operation) {\n> > +\tcase IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {\n> > +\t\tconst ControlList &controls = action.controls[0];\n> > +\t\tdelayedCtrls_->push(controls);\n> > +\t\tbreak;\n> > +\t}\n> > +\tdefault:\n> > +\t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> > +\t\tbreak;\n> > +\t}\n> > +}\n> > +\n> \n> Overall, that's a good starting point\n> Reviewed-by: Jacopo Mondi \n\nSame,\n\nReviewed-by: Laurent Pinchart \n\n> > /* -----------------------------------------------------------------------------\n> > * Buffer Ready slots\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 49C59BD80C\n\tfor ;\n\tSun, 10 Jan 2021 16:47:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6D3F6806C;\n\tSun, 10 Jan 2021 17:47:02 +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 63BA160523\n\tfor ;\n\tSun, 10 Jan 2021 17:47:01 +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 C6C67DA;\n\tSun, 10 Jan 2021 17:47:00 +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=\"OkLPIHcJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610297221;\n\tbh=loQjkzF0HP1j06qJ+rVNXR0jCJG57wURVZOYbV0Y64E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OkLPIHcJ46E/dx9nROfgtihH7HN5wfoGY4W9gL1+dQ8mcQbRKOyoacJ7njWkJGbid\n\tN2fCiT71qKaL426ukJ3ag6YzHIv1GO69BRGD+9/8mbopQEN6NUpwDMFo3bglcps+p5\n\tbL9RVUN4IpL3mJL22KC/M89m2JOJQNd8rL1oRYJ8=","Date":"Sun, 10 Jan 2021 18:46:46 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-8-niklas.soderlund@ragnatech.se>\n\t<20210107124050.udy2du6bbqq7zxpe@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210107124050.udy2du6bbqq7zxpe@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14527,"web_url":"https://patchwork.libcamera.org/comment/14527/","msgid":"<5cbc2995-1ea3-8837-c851-a54c75001b1e@uajain.com>","date":"2021-01-11T16:40:56","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Niklas,\n\nQuick comment on my findings last friday,\n\nOn 12/29/20 9:33 PM, Niklas Söderlund wrote:\n> Attach to the IPA and allow it to push V4L2 controls that applies on the\n> camera sensor. The IPA is not fully integrated in the pipeline as\n> statistics and parameters buffers are not yet allocated, processed by\n> the IPA nor queued to the hardware.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> * Changes since v1\n> - Rewrite to not use CameraSensor.\n> - Fix mistake where configuration sen to IPA was overwritten.\n> - Check that IPA configuration was successful before starting.\n> - Update commit message.\n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 103 +++++++++++++++++++++++++++\n> 1 file changed, 103 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index a87ce8f3378ba2fe..95f1b75dc8be5d40 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -14,11 +14,14 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> #include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/ipa_manager.h\"\n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -53,6 +56,8 @@ public:\n> \t{\n> \t}\n> \n> +\tint loadIPA();\n> +\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n> \n> @@ -62,6 +67,11 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tstd::unique_ptr delayedCtrls_;\n> +\n> +private:\n> +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -590,6 +600,13 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tIPU3CameraData *data = cameraData(camera);\n> \tCIO2Device *cio2 = &data->cio2_;\n> \tImgUDevice *imgu = data->imgu_;\n> +\n> +\tCameraSensorInfo sensorInfo = {};\n> +\tstd::map streamConfig;\n> +\tstd::map entityControls;\n> +\tIPAOperationData ipaConfig;\n> +\tIPAOperationData result = {};\n> +\n> \tint ret;\n> \n> \t/* Allocate buffers for internal pipeline usage. */\n> @@ -597,6 +614,11 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tIPAOperationData ipaData = {};\n> +\tret = data->ipa_->start(ipaData, nullptr);\n> +\tif (ret)\n> +\t\tgoto error;\n> +\n> \t/*\n> \t * Start the ImgU video devices, buffers will be queued to the\n> \t * ImgU output and viewfinder when requests will be queued.\n> @@ -612,9 +634,40 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> \t\tgoto error;\n> \t}\n> \n> +\t/* Inform IPA of stream configuration and sensor controls. */\n> +\tret = data->cio2_.sensor()->sensorInfo(&sensorInfo);\n> +\tif (ret) {\n> +\t\t/* \\todo Turn to hard failure once sensors info is mandatory. */\n> +\t\tLOG(IPU3, Warning) << \"Camera sensor information not available\";\n> +\t\tsensorInfo = {};\n> +\t\tret = 0;\n> +\t}\n> +\n> +\tstreamConfig[0] = {\n> +\t\t.pixelFormat = data->outStream_.configuration().pixelFormat,\n> +\t\t.size = data->outStream_.configuration().size,\n> +\t};\n> +\tstreamConfig[1] = {\n> +\t\t.pixelFormat = data->vfStream_.configuration().pixelFormat,\n> +\t\t.size = data->vfStream_.configuration().size,\n> +\t};\n> +\n> +\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> +\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, &result);\n> +\n> +\tif ((result.operation != IPU3_IPA_STATUS_CONFIGURATION) ||\n> +\t (result.data.size() != 1) || (result.data.at(0) != 1)) {\n> +\t\tLOG(IPU3, Warning) << \"Failed to configure IPA\";\n> +\t\tret = -EINVAL;\n> +\t\tgoto error;\nRunning your branch as is, `ipu3/ipa` triggered this path and spit out \nEBUSY with freeBuffers() below - so one of the quick things Jacopo \nnoticed that you need to stop the ImgU and CIO2 here first. I had \napplied the change locally and it fixed it for me.\n\nI also wrote a associated patch(nothing major) for simplifying error \npaths with subject \"ipu3: Simplify error path\" - maybe it's interesting \nto you.\n> +\t}\n> +\n> \treturn 0;\n> \n> error:\n> +\tdata->ipa_->stop();\n> \tfreeBuffers(camera);\n> \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> \n> @@ -631,6 +684,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> \tif (ret)\n> \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> \n> +\tdata->ipa_->stop();\n> +\n> \tfreeBuffers(camera);\n> }\n> \n> @@ -762,12 +817,32 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> +\t\tret = data->loadIPA();\n> +\t\tif (ret)\n> +\t\t\tcontinue;\n> +\n> \t\t/* Initialize the camera properties. */\n> \t\tdata->properties_ = cio2->sensor()->properties();\n> \n> \t\t/* Initialze the camera controls. */\n> \t\tdata->controlInfo_ = IPU3Controls;\n> \n> +\t\t/*\n> +\t\t * \\todo Read dealy values from the sensor itself or from a\n> +\t\t * a sensor database. For now use generic values taken from\n> +\t\t * the Raspberry Pi and listed as generic values.\n> +\t\t */\n> +\t\tstd::unordered_map delays = {\n> +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> +\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> +\t\t};\n> +\n> +\t\tdata->delayedCtrls_ =\n> +\t\t\tstd::make_unique(cio2->sensor()->device(),\n> +\t\t\t\t\t\t\t delays);\n> +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> +\t\t\t\t\t\t &DelayedControls::applyControls);\n> +\n> \t\t/**\n> \t\t * \\todo Dynamically assign ImgU and output devices to each\n> \t\t * stream and camera; as of now, limit support to two cameras\n> @@ -811,6 +886,34 @@ int PipelineHandlerIPU3::registerCameras()\n> \treturn numCameras ? 0 : -ENODEV;\n> }\n> \n> +int IPU3CameraData::loadIPA()\n> +{\n> +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> +\tif (!ipa_)\n> +\t\treturn -ENOENT;\n> +\n> +\tipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa);\n> +\n> +\tipa_->init(IPASettings{});\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> +\t\t\t const IPAOperationData &action)\n> +{\n> +\tswitch (action.operation) {\n> +\tcase IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {\n> +\t\tconst ControlList &controls = action.controls[0];\n> +\t\tdelayedCtrls_->push(controls);\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> /* -----------------------------------------------------------------------------\n> * Buffer Ready slots\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 30E89BD80C\n\tfor ;\n\tMon, 11 Jan 2021 16:41:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B5609680BD;\n\tMon, 11 Jan 2021 17:41:01 +0100 (CET)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6513F6010E\n\tfor ;\n\tMon, 11 Jan 2021 17:41:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"AJl8NZdB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1610383259; bh=gGLm+JNOF3yyYwnSB9UwkmfTxnbm9Zvn0phCrUhs5Rw=;\n\th=Subject:To:References:From:In-Reply-To;\n\tb=AJl8NZdBhoA7/szqeVg938KQ5NrpPKB1rm/H7zPbRB0SyR7HM9dFc3ggxYz49puuq\n\tMZt6vQALFhQGNE/cO+cwFAuLsDpa2/xPaO3Oryqev/fnmKO9qZq4E/EQ4/h4eAM8Xj\n\tg/OjqeQJFy5Db1QP372ltOnQeQr7cQneZWU1ZI4BCEb/RRMYl/WEE48I9quAKGGG7f\n\tQEQK/5aTiaB26q75bJA3FXeOIC3bUptwY0K94spie5kcbFIWeezeceTZfrJqRYH3Hf\n\tx5YRQBklyAUJpHeigARlK/+El/qDOnjphleXgWILqNPznSu2DUOJuYT557svAmiBUG\n\tHmbq1ygQ7Sy8w==","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-8-niklas.soderlund@ragnatech.se>","From":"Umang Jain ","Message-ID":"<5cbc2995-1ea3-8837-c851-a54c75001b1e@uajain.com>","Date":"Mon, 11 Jan 2021 22:10:56 +0530","Mime-Version":"1.0","In-Reply-To":"<20201229160318.77536-8-niklas.soderlund@ragnatech.se>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","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","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14926,"web_url":"https://patchwork.libcamera.org/comment/14926/","msgid":"","date":"2021-02-03T14:35:44","subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo and Laurent,\n\nThanks for your feedback.\n\nOn 2021-01-10 18:46:46 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Thu, Jan 07, 2021 at 01:40:50PM +0100, Jacopo Mondi wrote:\n> > On Tue, Dec 29, 2020 at 05:03:14PM +0100, Niklas Söderlund wrote:\n> > > Attach to the IPA and allow it to push V4L2 controls that applies on the\n> \n> s/applies on/apply to/\n> \n> > > camera sensor. The IPA is not fully integrated in the pipeline as\n> > > statistics and parameters buffers are not yet allocated, processed by\n> > > the IPA nor queued to the hardware.\n> > >\n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > * Changes since v1\n> > > - Rewrite to not use CameraSensor.\n> > > - Fix mistake where configuration sen to IPA was overwritten.\n> > > - Check that IPA configuration was successful before starting.\n> > > - Update commit message.\n> > > ---\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 103 +++++++++++++++++++++++++++\n> > > 1 file changed, 103 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index a87ce8f3378ba2fe..95f1b75dc8be5d40 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -14,11 +14,14 @@\n> > > #include \n> > > #include \n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > >\n> > > #include \"libcamera/internal/camera_sensor.h\"\n> > > +#include \"libcamera/internal/delayed_controls.h\"\n> > > #include \"libcamera/internal/device_enumerator.h\"\n> > > +#include \"libcamera/internal/ipa_manager.h\"\n> > > #include \"libcamera/internal/log.h\"\n> > > #include \"libcamera/internal/media_device.h\"\n> > > #include \"libcamera/internal/pipeline_handler.h\"\n> > > @@ -53,6 +56,8 @@ public:\n> > > \t{\n> > > \t}\n> > >\n> > > +\tint loadIPA();\n> > > +\n> > > \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> > > \tvoid cio2BufferReady(FrameBuffer *buffer);\n> > >\n> > > @@ -62,6 +67,11 @@ public:\n> > > \tStream outStream_;\n> > > \tStream vfStream_;\n> > > \tStream rawStream_;\n> > > +\n> > > +\tstd::unique_ptr delayedCtrls_;\n> > > +\n> > > +private:\n> > > +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> > > };\n> > >\n> > > class IPU3CameraConfiguration : public CameraConfiguration\n> > > @@ -590,6 +600,13 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > > \tIPU3CameraData *data = cameraData(camera);\n> > > \tCIO2Device *cio2 = &data->cio2_;\n> > > \tImgUDevice *imgu = data->imgu_;\n> > > +\n> > > +\tCameraSensorInfo sensorInfo = {};\n> > > +\tstd::map streamConfig;\n> > > +\tstd::map entityControls;\n> > > +\tIPAOperationData ipaConfig;\n> > > +\tIPAOperationData result = {};\n> > \n> > You could s/= {}/{}\n> > but it's minor\n> > \n\nI could but I find it harder to read.\n\n> > > +\n> > > \tint ret;\n> > >\n> > > \t/* Allocate buffers for internal pipeline usage. */\n> > > @@ -597,6 +614,11 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > > \tif (ret)\n> > > \t\treturn ret;\n> > >\n> > > +\tIPAOperationData ipaData = {};\n> > > +\tret = data->ipa_->start(ipaData, nullptr);\n> > > +\tif (ret)\n> > > +\t\tgoto error;\n> > > +\n> > > \t/*\n> > > \t * Start the ImgU video devices, buffers will be queued to the\n> > > \t * ImgU output and viewfinder when requests will be queued.\n> > > @@ -612,9 +634,40 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] ControlList *con\n> > > \t\tgoto error;\n> > > \t}\n> > >\n> > > +\t/* Inform IPA of stream configuration and sensor controls. */\n> > > +\tret = data->cio2_.sensor()->sensorInfo(&sensorInfo);\n> > > +\tif (ret) {\n> > > +\t\t/* \\todo Turn to hard failure once sensors info is mandatory. */\n> > \n> > we're rather working in the direction of making SensorInfo never fail.\n> > I think you can drop this todo and hard fail here as if you have a\n> > failure, it's because something bad happened when reading controls or\n> > selection targets.\n> \n> Agreed.\n\nI thinks this shows the age of this patch :-)\n\nThis TODO was recorded as the call failed at the time of patch creation. \nNow the calls succeeds and this should indeed be turned into a hard \nfail. Will do so in next version.\n\n> \n> > > +\t\tLOG(IPU3, Warning) << \"Camera sensor information not available\";\n> > > +\t\tsensorInfo = {};\n> > > +\t\tret = 0;\n> > > +\t}\n> > > +\n> > > +\tstreamConfig[0] = {\n> > > +\t\t.pixelFormat = data->outStream_.configuration().pixelFormat,\n> > > +\t\t.size = data->outStream_.configuration().size,\n> > > +\t};\n> > > +\tstreamConfig[1] = {\n> > > +\t\t.pixelFormat = data->vfStream_.configuration().pixelFormat,\n> > > +\t\t.size = data->vfStream_.configuration().size,\n> > > +\t};\n> > \n> > Is this ok if one of the two streams (likely vfStream_) is not\n> > assigned ?\n> \n> Based on our current IPA interface, I'd say only active streams should\n> be reported here. We're however moving to a per-pipeline handler IPA\n> protocol, and it would be fine having a hardcoded array of two streams,\n> as long, of course, as there's enough information for the IPA to tell\n> which streams are active (invalid pixel format for instance). Obviously\n> a hardcoded array isn't required with the custom IPA protocol, we can\n> still device to pass the active streams only in that case.\n> \n> The StreamConfiguration() stored in a Stream is initialized with\n> appropriate defaults, but if we configure the camera with two streams,\n> and then again with a single stream, I think we'll have stale values.\n\nThe viewfinder is always configured in configue(), even if it's not \nneeded by the configuration we are asked to do, so it will never be \nstale. It's also always started in start(). The only different between \nif the configuration contains viewifinder or not is that we won't allow \napplications to queue buffers to it if its not.\n\nI'm open to any solution here that moves us forward as quickly as \npossible. But given that the stream configuration is not consumed by the \nIPA and is only expressed here to satisfy our current IPA interface that \nthat we know will be replaced soon I would prefers to either keep this \nas-is or remove this and simply pass an empty map to ipa \nconfiguration(). Creating something complex here to only report active \nstreams seems a bit pointless at this point.\n\n> \n> > > +\n> > > +\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> > > +\n> > > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > > +\t\t\t ipaConfig, &result);\n> > > +\n> > > +\tif ((result.operation != IPU3_IPA_STATUS_CONFIGURATION) ||\n> > \n> > This seems like it can't happen at the moment, but it's ok\n> \n> Yes, I'm not sure what the idea behind the check is.\n> \n> > > +\t (result.data.size() != 1) || (result.data.at(0) != 1)) {\n> > \n> > I think the fact that returning 1 in data[0] means success should be\n> > documented in the IPA protocol. Ah wait, we don't have one.\n> \n> RPi checks\n> \n> \tif (result.operation & RPi::IPA_CONFIG_FAILED)\n> \n> and we could do something similar for now.\n\nWops, All of this result check is left from a experiment I did. This \nshould indeed be removed. Thanks for spotting it.\n\n> \n> > > +\t\tLOG(IPU3, Warning) << \"Failed to configure IPA\";\n> > > +\t\tret = -EINVAL;\n> > > +\t\tgoto error;\n> > > +\t}\n> > > +\n> > > \treturn 0;\n> > >\n> > > error:\n> > > +\tdata->ipa_->stop();\n> > > \tfreeBuffers(camera);\n> > > \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> > >\n> > > @@ -631,6 +684,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> > > \tif (ret)\n> > > \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> > >\n> > > +\tdata->ipa_->stop();\n> > > +\n> > > \tfreeBuffers(camera);\n> > > }\n> > >\n> > > @@ -762,12 +817,32 @@ int PipelineHandlerIPU3::registerCameras()\n> > > \t\tif (ret)\n> > > \t\t\tcontinue;\n> > >\n> > > +\t\tret = data->loadIPA();\n> > > +\t\tif (ret)\n> > > +\t\t\tcontinue;\n> > > +\n> > > \t\t/* Initialize the camera properties. */\n> > > \t\tdata->properties_ = cio2->sensor()->properties();\n> > >\n> > > \t\t/* Initialze the camera controls. */\n> > > \t\tdata->controlInfo_ = IPU3Controls;\n> > >\n> > > +\t\t/*\n> > > +\t\t * \\todo Read dealy values from the sensor itself or from a\n> > > +\t\t * a sensor database. For now use generic values taken from\n> > > +\t\t * the Raspberry Pi and listed as generic values.\n> > > +\t\t */\n> > > +\t\tstd::unordered_map delays = {\n> > > +\t\t\t{ V4L2_CID_ANALOGUE_GAIN, 1 },\n> > > +\t\t\t{ V4L2_CID_EXPOSURE, 2 },\n> > > +\t\t};\n> > > +\n> > > +\t\tdata->delayedCtrls_ =\n> > > +\t\t\tstd::make_unique(cio2->sensor()->device(),\n> > > +\t\t\t\t\t\t\t delays);\n> > \n> > sigh\n> > \n> > we should get the sensor database in sooner than later and move this\n> > to the camera sensor before the same patter spread in multiple\n> > pipelines\n> \n> I can't disagree on this :-) (moving on the review of the corresponding\n> patches next)\n\nI too would like to get the database in place as soon as possible.\n\n> \n> > > +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> > > +\t\t\t\t\t\t &DelayedControls::applyControls);\n> > > +\n> > > \t\t/**\n> > > \t\t * \\todo Dynamically assign ImgU and output devices to each\n> > > \t\t * stream and camera; as of now, limit support to two cameras\n> > > @@ -811,6 +886,34 @@ int PipelineHandlerIPU3::registerCameras()\n> > > \treturn numCameras ? 0 : -ENODEV;\n> > > }\n> > >\n> > > +int IPU3CameraData::loadIPA()\n> > > +{\n> > > +\tipa_ = IPAManager::createIPA(pipe_, 1, 1);\n> > > +\tif (!ipa_)\n> > > +\t\treturn -ENOENT;\n> > > +\n> > > +\tipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa);\n> > > +\n> > > +\tipa_->init(IPASettings{});\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> > > +\t\t\t const IPAOperationData &action)\n> \n> Maybe s/actOnIpa/queueFrameAction/ to match the other pipeline handlers\n> ?\n\nSure.\n\n> \n> > > +{\n> > > +\tswitch (action.operation) {\n> > > +\tcase IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: {\n> > > +\t\tconst ControlList &controls = action.controls[0];\n> > > +\t\tdelayedCtrls_->push(controls);\n> > > +\t\tbreak;\n> > > +\t}\n> > > +\tdefault:\n> > > +\t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> > > +\t\tbreak;\n> > > +\t}\n> > > +}\n> > > +\n> > \n> > Overall, that's a good starting point\n> > Reviewed-by: Jacopo Mondi \n> \n> Same,\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > > /* -----------------------------------------------------------------------------\n> > > * Buffer Ready slots\n> > > */\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 B2FCCBD161\n\tfor ;\n\tWed, 3 Feb 2021 14:35:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 37C0068447;\n\tWed, 3 Feb 2021 15:35:48 +0100 (CET)","from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com\n\t[IPv6:2a00:1450:4864:20::52d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4B041683FE\n\tfor ;\n\tWed, 3 Feb 2021 15:35:46 +0100 (CET)","by mail-ed1-x52d.google.com with SMTP id q2so12400146edi.4\n\tfor ;\n\tWed, 03 Feb 2021 06:35:46 -0800 (PST)","from localhost (p4fca2458.dip0.t-ipconnect.de. [79.202.36.88])\n\tby smtp.gmail.com with ESMTPSA id\n\to60sm1005081edd.50.2021.02.03.06.35.44\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 03 Feb 2021 06:35:44 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"UJzpm06M\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=yc5i4n+cZzhbZhZI/tkur+pZ2BNs/GsVWE0GMYxkbMw=;\n\tb=UJzpm06MfeHDpzh6T+oGDnuJ/xVakfnjv4SCBLzYh6ws7flbbEYzt02bEETp/eluQH\n\t1bghrQAMYcqP3O+cpeMJypVlp9bmceUQzd4Oyt9c0bwi2O22Goh27Px5bkp8k/T6Gqdu\n\t+Lj4pBLDrv1H8dIgm0rmirW2PAkHdZqmHD2dOhF56m4iKquSEZNFD6753xPL9UpcFijB\n\tuR2BYtBr7x5Cp8P3YctEWbkC9ZmeCk7t5+iQ18fZtdSIQ7hp8uTk9yGT4DIgaW83y46a\n\twm6II2PUy5hlcGpDJ3yCCVjF0N72z+4CcPss1OA6eaMECdirozQ5PnJkSbqW0LnhpgR4\n\tT9Dw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=yc5i4n+cZzhbZhZI/tkur+pZ2BNs/GsVWE0GMYxkbMw=;\n\tb=eweXqbQ/3dONAqTzq55wD/JV0d4vH8iJn1V2XsQY1Kmr0dcV6NkBRe6bTPB6MGewnA\n\tkYOt3F78mbcfECdVXG8cXB/k4DOQBUPVHaAxvvm/e6QqbrICYjr9DnGJW06MPYbpVr0G\n\tibOqq9THnW0xncF75qkpXcr2UeWdwPZupgltXoM8hPydWNNyFFWw70mPspm+A46ay/Jz\n\t+mkp0rof+t9hrGHMabAxoVNr+vOd5qlwnrarctKK67WBpyZch2dIjOtWz9mSTu9M6B1Q\n\tGpYrWEnXxUt+KleuYE0k9s+E/4mTupj2NRTBi8fybPWjPSssgON2FL16RygBNTxf1m48\n\tkvyQ==","X-Gm-Message-State":"AOAM531TQOj9QaVZTstzzkIQ6xGwhEOS9r+OFc4scd3qrTiFrSfZ6ITP\n\t+eLxkzjrhN8zDIR6yqxFEC0IHDZw0cJX0eIP","X-Google-Smtp-Source":"ABdhPJwY0yWIw8GPa/wDEwTC+bEvK07zLIA1opz9g9mORVrUeh3Zq5rdWOkec9W0vqfQcvHve0TN+w==","X-Received":"by 2002:a05:6402:1ad1:: with SMTP id\n\tba17mr3284761edb.243.1612362945766; \n\tWed, 03 Feb 2021 06:35:45 -0800 (PST)","Date":"Wed, 3 Feb 2021 15:35:44 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ,\n\tJacopo Mondi ","Message-ID":"","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-8-niklas.soderlund@ragnatech.se>\n\t<20210107124050.udy2du6bbqq7zxpe@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 07/11] libcamera: ipu3: Attach to\n\tan IPA and allow it to set sensor controls","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]