[{"id":13632,"web_url":"https://patchwork.libcamera.org/comment/13632/","msgid":"<20201109043024.GB1791@pyrite.rasen.tech>","date":"2020-11-09T04:30:24","subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA and allow it to set sensor controls","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Nov 05, 2020 at 01:15:43AM +0100, Niklas Söderlund wrote:\n> Attache to an 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> src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++-\n> 1 file changed, 82 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 62e99a308a6136a7..d161c2e68c0db0f2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -14,11 +14,13 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> #include \"libcamera/internal/camera_sensor.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> @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData\n> {\n> public:\n> \tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe)\n> +\t\t: CameraData(pipe), delayedCtrls_(nullptr)\n> \t{\n> \t}\n> \n> +\tint loadIPA();\n> +\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n> \n> @@ -62,6 +66,11 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tDelayedControls *delayedCtrls_;\n> +\n> +private:\n> +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera)\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> +\n> \tint ret;\n> \n> \t/* Allocate buffers for internal pipeline usage. */\n> @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tret = data->ipa_->start();\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 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera)\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[0] = {\n> +\t\t.pixelFormat = data->vfStream_.configuration().pixelFormat,\n> +\t\t.size = data->vfStream_.configuration().size,\n> +\t};\n\nWhy are you overwriting streamConfig[0] here?\n\n\nPaul\n\n> +\n> +\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> +\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, nullptr);\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 +674,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 +807,20 @@ 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\tdata->delayedCtrls_ = cio2->sensor()->delayedContols();\n> +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_,\n> +\t\t\t\t\t\t &DelayedControls::frameStart);\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 +864,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> -- \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 1A297BDB89\n\tfor ;\n\tMon, 9 Nov 2020 04:30:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91A6A63067;\n\tMon, 9 Nov 2020 05:30:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB7DD6033F\n\tfor ;\n\tMon, 9 Nov 2020 05:30:32 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4C5E2FE;\n\tMon, 9 Nov 2020 05:30:30 +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=\"InjeUYuf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604896232;\n\tbh=Bk2EfsQQ6WMy7UvSywQB5RNuRmrsQenBTGy0CjTmmDE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=InjeUYuf8awpqSWMmryK1vheNvcbkJwKXxdYMgfPX/tFjs3ZDi1D+m2o/yDcF3KSl\n\tD3+1WPHeb0s79KO/zwaNUIOV1waOV3QusIGOMZtBCZz7Km/fDGkmyIifQTU15UGC2Z\n\tiWS3uCJjTwEL4dqEUcDe/eEALPqRPwxbCm37I/Eo=","Date":"Mon, 9 Nov 2020 13:30:24 +0900","From":"paul.elder@ideasonboard.com","To":"Niklas =?iso-8859-1?q?S=F6derlund?= ","Message-ID":"<20201109043024.GB1791@pyrite.rasen.tech>","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA 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\" "}},{"id":13670,"web_url":"https://patchwork.libcamera.org/comment/13670/","msgid":"<02ae5be7-16c1-4d22-7e2b-acaabf5452ce@gmail.com>","date":"2020-11-10T14:22:27","subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA and allow it to set sensor controls","submitter":{"id":74,"url":"https://patchwork.libcamera.org/api/people/74/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@gmail.com"},"content":"Hi Niklas,\n\nOn 05/11/2020 01:15, Niklas Söderlund wrote:\n> Attache to an 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> src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++-\n> 1 file changed, 82 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 62e99a308a6136a7..d161c2e68c0db0f2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -14,11 +14,13 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> #include \"libcamera/internal/camera_sensor.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> @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData\n> {\n> public:\n> \tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe)\n> +\t\t: CameraData(pipe), delayedCtrls_(nullptr)\n> \t{\n> \t}\n> \n> +\tint loadIPA();\n> +\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n> \n> @@ -62,6 +66,11 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tDelayedControls *delayedCtrls_;\n> +\n> +private:\n> +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera)\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> +\n> \tint ret;\n> \n> \t/* Allocate buffers for internal pipeline usage. */\n> @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tret = data->ipa_->start();\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 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera)\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\nI tried this patch series on my Surface Go 2 and I have some issues with\nit. The most critical is that I get a core dump :-) :\n\n[0:08:57.210851579] [4959] WARN IPU3 ipu3.cpp:646 Camera sensor\ninformation not available\n[0:08:57.211150487] [4959] ERROR IPAIPU3 ipu3.cpp:88 Can't find exposure\ncontrol\nterminate called after throwing an instance of 'std::out_of_range'\n what(): _Map_base::at\nAborted (core dumped)\n\nThanks,\nJM","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 B4916BDB89\n\tfor ;\n\tTue, 10 Nov 2020 14:22:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 39A86630BA;\n\tTue, 10 Nov 2020 15:22:32 +0100 (CET)","from mail-wr1-x443.google.com (mail-wr1-x443.google.com\n\t[IPv6:2a00:1450:4864:20::443])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 097BB60342\n\tfor ;\n\tTue, 10 Nov 2020 15:22:30 +0100 (CET)","by mail-wr1-x443.google.com with SMTP id d12so11336789wrr.13\n\tfor ;\n\tTue, 10 Nov 2020 06:22:30 -0800 (PST)","from ?IPv6:2a01:e0a:169:7140:e74e:2016:2a5f:f5ba?\n\t([2a01:e0a:169:7140:e74e:2016:2a5f:f5ba])\n\tby smtp.gmail.com with ESMTPSA id\n\tc17sm9307772wro.19.2020.11.10.06.22.28\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 10 Nov 2020 06:22:28 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"AIbMlPaR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=subject:to:references:from:message-id:date:user-agent:mime-version\n\t:in-reply-to:content-language:content-transfer-encoding;\n\tbh=IoQvJnrylS6rStZjrixtKaSyfe+ggHmIy1WMgtxP4gk=;\n\tb=AIbMlPaRkoNdWet8wJ8eAHgwtxKtJXLrtuGXDKGTdNyIiWykRi0B14g1KcvMOMdSub\n\tXDrNhiIYy7FfYomf9xG/pn+rBkrodn7t8pNtq3NVrNIhHEHD2E/wPpWNpqswNq3PcZmf\n\tHaH4XUuxgtu+NmMJc9OxuMyNslVSwcO/Ylmh0fnE/FBFypL0jKauW1WlkVdqroU8GHvp\n\tc8sblFnjgZjSZXK7kdUJMjQJUKmvCV+iBQ65YTA3Ic2kq2Q4oHuz/AIVkm+YgVkV+az2\n\t4MB6SrGdERc/DCxpvMIipbJpFbGRopkYpsmCp000xM7dwO6bR6m/jmaSTWx0oj6Lsch8\n\tDwLg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=IoQvJnrylS6rStZjrixtKaSyfe+ggHmIy1WMgtxP4gk=;\n\tb=Wi3ZOa86UsGoF2dtUEw1LZ+vhu+VFniZNXeYWF1i045hiB/Fe6bmm67Snp4/xTG0l7\n\tjI/GjnTq3ZeTjTsNdRz5qNaj172LgiyNFshOmd8Udimg7h9Rcr1rSWNawriPmxAEeHaP\n\ty0zO3gGbtVzqnm5JAN26o7Gd3n/ywUO8Sj6cU+39YwWx4mNd8XHkDSLbIi2OcUO8ioKC\n\t7VflJpBlnGgV2UhNJh1tfAF3EQozxU5I7nDelGfZIhp9U2wy6TfRBVK0prYgPwk/rAd3\n\tTFfEPzNdMsTaLUW9Qh4E0UxjJXqsxol47sywxFfMuG6JFjWm2Lguj+3v+7VOISsZg8i1\n\tz8JA==","X-Gm-Message-State":"AOAM532dl0HAp5ywzekbxQ4kkdphwt5Bc5BOlVLqtGcOBEoLY7MVEVBZ\n\tlmA/8bq+VDjdy2f99fTiR5G7jkRWtB7tlw==","X-Google-Smtp-Source":"ABdhPJyT29lEU4Q/jz2SmGCgKr/gEU/pXASe7dqbvkxa3SpW1iPx1zDK5LMEurJk/vwj0TTSisC9QA==","X-Received":"by 2002:adf:de85:: with SMTP id w5mr9316563wrl.90.1605018149092; \n\tTue, 10 Nov 2020 06:22:29 -0800 (PST)","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>","From":"Jean-Michel Hautbois ","Message-ID":"<02ae5be7-16c1-4d22-7e2b-acaabf5452ce@gmail.com>","Date":"Tue, 10 Nov 2020 15:22:27 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.3.2","MIME-Version":"1.0","In-Reply-To":"<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA 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":13745,"web_url":"https://patchwork.libcamera.org/comment/13745/","msgid":"","date":"2020-11-17T11:45:03","subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA 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\nOn 05/11/2020 01:15, Niklas Söderlund wrote:\n> Attache to an 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> src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++-\n> 1 file changed, 82 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 62e99a308a6136a7..d161c2e68c0db0f2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -14,11 +14,13 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> \n> #include \"libcamera/internal/camera_sensor.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> @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData\n> {\n> public:\n> \tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe)\n> +\t\t: CameraData(pipe), delayedCtrls_(nullptr)\n> \t{\n> \t}\n> \n> +\tint loadIPA();\n> +\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n> \n> @@ -62,6 +66,11 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tDelayedControls *delayedCtrls_;\n> +\n> +private:\n> +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> };\n> \n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera)\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> +\n> \tint ret;\n> \n> \t/* Allocate buffers for internal pipeline usage. */\n> @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tret = data->ipa_->start();\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 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera)\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[0] = {\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, nullptr);\n> +\n> \treturn 0;\n\nThere is an issue there, if no controls are found (not implemented) then\nyou will go on without really nowing...\n\n> \n> error:\n> +\tdata->ipa_->stop();\n> \tfreeBuffers(camera);\n> \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> \n> @@ -631,6 +674,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 +807,20 @@ 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\tdata->delayedCtrls_ = cio2->sensor()->delayedContols();\n> +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_,\n> +\t\t\t\t\t\t &DelayedControls::frameStart);\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 +864,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\nAnd you will push an empty list...\nwhich leads to \"terminate called after throwing an instance of\n'std::out_of_range'\"\n\nIt should not abort but deal with it ?\n\nThanks,\nJM","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 72FFDBE082\n\tfor ;\n\tTue, 17 Nov 2020 11:45:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E82E263329;\n\tTue, 17 Nov 2020 12:45:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B8D456331F\n\tfor ;\n\tTue, 17 Nov 2020 12:45:04 +0100 (CET)","from [IPv6:2a01:e0a:169:7140:d5de:1f8c:872a:c380] (unknown\n\t[IPv6:2a01:e0a:169:7140:d5de:1f8c:872a:c380])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 751F22A3;\n\tTue, 17 Nov 2020 12:45:03 +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=\"iWx+Su9b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1605613503;\n\tbh=fHGShZtpwv+9I1Fxoq8439eatYz8bDMZZCVDWIJXSVo=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=iWx+Su9bQ+Pgur5o3UGXUfwL9BYHNoWBRSIHcG2dtHeSz23cAShD+7ICiuG6Yci9y\n\tio91VyL4QYhcMjHVHIVtZidXsWGcRPWM8DBLXONsSxN3w6iUJNBZGqmK6lC7ueTRGa\n\tb3tBXQg+KaXlNMnZVLzdGpC3h8cO8Yrqy7nrOmzM=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Tue, 17 Nov 2020 12:45:03 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.3.2","MIME-Version":"1.0","In-Reply-To":"<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA 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":13789,"web_url":"https://patchwork.libcamera.org/comment/13789/","msgid":"<20201118154828.7md33lawnmz2wbbw@uno.localdomain>","date":"2020-11-18T15:48:28","subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA 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 Thu, Nov 05, 2020 at 01:15:43AM +0100, Niklas Söderlund wrote:\n> Attache to an IPA and allow it to push V4L2 controls that applies on the\n\nAttach\nto the IPA\n\nThe question on which control identifiers to use is the same as in the\nprevious patches. Do we want the IPA and pipeline to speak V4L2_CID_\nor should we use libcamera controls where possible and let the\nCameraSensor do the translation ?\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> src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++-\n> 1 file changed, 82 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 62e99a308a6136a7..d161c2e68c0db0f2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -14,11 +14,13 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n>\n> #include \"libcamera/internal/camera_sensor.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> @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData\n> {\n> public:\n> \tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe)\n> +\t\t: CameraData(pipe), delayedCtrls_(nullptr)\n> \t{\n> \t}\n>\n> +\tint loadIPA();\n> +\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n>\n> @@ -62,6 +66,11 @@ public:\n> \tStream outStream_;\n> \tStream vfStream_;\n> \tStream rawStream_;\n> +\n> +\tDelayedControls *delayedCtrls_;\n> +\n> +private:\n> +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> };\n>\n> class IPU3CameraConfiguration : public CameraConfiguration\n> @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera)\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> +\n> \tint ret;\n>\n> \t/* Allocate buffers for internal pipeline usage. */\n> @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera)\n> \tif (ret)\n> \t\treturn ret;\n>\n> +\tret = data->ipa_->start();\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 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera)\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[0] = {\n\nOverwriting streamConfig[0] ?\n\nIt's anyway so far ignored in the IPA\n\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, nullptr);\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 +674,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 +807,20 @@ 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\tdata->delayedCtrls_ = cio2->sensor()->delayedContols();\n> +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_,\n> +\t\t\t\t\t\t &DelayedControls::frameStart);\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 +864,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\nDo we want to have a CameraSensor::setDelayedControls() ?\nThis direct handling of delayed controls bypasses the CameraSensor\ncompletely pushing to each pipeline handler the controls translation\nburden\n\n\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> --\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 11FCEBE08A\n\tfor ;\n\tWed, 18 Nov 2020 15:48:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 67B667E9B0;\n\tWed, 18 Nov 2020 16:48:26 +0100 (CET)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 52A6F632AF\n\tfor ;\n\tWed, 18 Nov 2020 16:48:25 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id D349E100006;\n\tWed, 18 Nov 2020 15:48:24 +0000 (UTC)"],"Date":"Wed, 18 Nov 2020 16:48:28 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20201118154828.7md33lawnmz2wbbw@uno.localdomain>","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA 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":14119,"web_url":"https://patchwork.libcamera.org/comment/14119/","msgid":"","date":"2020-12-08T02:46:15","subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA 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 Jacopo,\n\nOn Wed, Nov 18, 2020 at 04:48:28PM +0100, Jacopo Mondi wrote:\n> Hi Niklas\n> \n> On Thu, Nov 05, 2020 at 01:15:43AM +0100, Niklas Söderlund wrote:\n> > Attache to an IPA and allow it to push V4L2 controls that applies on the\n> \n> Attach\n> to the IPA\n> \n> The question on which control identifiers to use is the same as in the\n> previous patches. Do we want the IPA and pipeline to speak V4L2_CID_\n> or should we use libcamera controls where possible and let the\n> CameraSensor do the translation ?\n\nThe latter, possible with CameraSensor controls instead of libcamera\ncontrols (at least a separate ControlInfoMap, to report the right\nlimits). That requires work on the CameraSensor side and I don't think\nthis series needs to depend on that, as the RPi IPA already uses V4L2\ncontrols.\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> > src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++-\n> > 1 file changed, 82 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 62e99a308a6136a7..d161c2e68c0db0f2 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -14,11 +14,13 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> >\n> > #include \"libcamera/internal/camera_sensor.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> > @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData\n> > {\n> > public:\n> > \tIPU3CameraData(PipelineHandler *pipe)\n> > -\t\t: CameraData(pipe)\n> > +\t\t: CameraData(pipe), delayedCtrls_(nullptr)\n> > \t{\n> > \t}\n> >\n> > +\tint loadIPA();\n> > +\n> > \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> > \tvoid cio2BufferReady(FrameBuffer *buffer);\n> >\n> > @@ -62,6 +66,11 @@ public:\n> > \tStream outStream_;\n> > \tStream vfStream_;\n> > \tStream rawStream_;\n> > +\n> > +\tDelayedControls *delayedCtrls_;\n> > +\n> > +private:\n> > +\tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> > };\n> >\n> > class IPU3CameraConfiguration : public CameraConfiguration\n> > @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera)\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> > +\n> > \tint ret;\n> >\n> > \t/* Allocate buffers for internal pipeline usage. */\n> > @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera)\n> > \tif (ret)\n> > \t\treturn ret;\n> >\n> > +\tret = data->ipa_->start();\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 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera)\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[0] = {\n> \n> Overwriting streamConfig[0] ?\n> \n> It's anyway so far ignored in the IPA\n> \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, nullptr);\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 +674,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 +807,20 @@ 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\tdata->delayedCtrls_ = cio2->sensor()->delayedContols();\n> > +\t\tdata->cio2_.frameStart().connect(data->delayedCtrls_,\n> > +\t\t\t\t\t\t &DelayedControls::frameStart);\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 +864,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> \n> Do we want to have a CameraSensor::setDelayedControls() ?\n> This direct handling of delayed controls bypasses the CameraSensor\n> completely pushing to each pipeline handler the controls translation\n> burden\n\nI think we want to embed an instance of DelayedControls in CameraSensor,\nyes, and create a nice API to interface with it. I think this is also\nnot a blocker for this series, but it should be done sooner than later.\n\nOverall I think this patch is a good step forward. It would be nice to\naddress the crash that Jean-Michel experienced though :-)\n\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 7FDDFBDB20\n\tfor ;\n\tTue, 8 Dec 2020 02:46:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0EA6367E6F;\n\tTue, 8 Dec 2020 03:46:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2343167E6C\n\tfor ;\n\tTue, 8 Dec 2020 03:46:19 +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 9FFADDD;\n\tTue, 8 Dec 2020 03:46:18 +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=\"WOxRU7Ch\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1607395578;\n\tbh=7z4MjWUBtnXWcEU3BMf4bht1OwRYiA2mS/Nw56JPjss=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WOxRU7ChRiNJqk1LGm2ZAF9ptw53X8NhjnZkMHfkcv4Y2ExLfHUMFl+Kxe9JiMLpX\n\tS4ewNTAAq7FK1mVhj+HN5uTxV/vio/8IHvTZm2rMGhUeSscWhkBIUe1lNYRbclVuei\n\tmnONdnvhPGYeNQ6kNWFNQQgYq1lcYx1Lpc90pH2I=","Date":"Tue, 8 Dec 2020 04:46:15 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-9-niklas.soderlund@ragnatech.se>\n\t<20201118154828.7md33lawnmz2wbbw@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201118154828.7md33lawnmz2wbbw@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 08/11] libcamera: ipu3: Attach to an\n\tIPA 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\" "}}]