[{"id":11009,"web_url":"https://patchwork.libcamera.org/comment/11009/","msgid":"","date":"2020-06-30T10:35:52","subject":"Re: [libcamera-devel] [PATCH v1 7/9] ipa: raspberrypi: Pass sensor\n\tconfig back from configure()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your patch.\n\nOn Mon, 29 Jun 2020 at 00:19, Laurent Pinchart\n wrote:\n>\n> The Raspberry Pi IPA uses a custom frame action,\n> RPI_IPA_ACTION_SET_SENSOR_CONFIG, to send initial sensor configuration\n> to the pipeline handler when the IPA is configured. Replace this ad-hoc\n> mechanism by passing the corresponding data back from the IPA to the\n> pipeline handler through the configure() response. This allows\n> synchronous handling of the response on the pipeline handler side.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/ipa/raspberrypi.h | 3 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 44 ++++++++------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 60 +++++++++++--------\n> 3 files changed, 61 insertions(+), 46 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index c335d0259549..77a7e6d40a2f 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -12,6 +12,8 @@\n>\n> enum RPiConfigParameters {\n> RPI_IPA_CONFIG_LSTABLE = (1 << 0),\n> + RPI_IPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n> + RPI_IPA_CONFIG_SENSOR = (1 << 2),\n> };\n>\n> enum RPiOperations {\n> @@ -20,7 +22,6 @@ enum RPiOperations {\n> RPI_IPA_ACTION_STATS_METADATA_COMPLETE,\n> RPI_IPA_ACTION_RUN_ISP,\n> RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,\n> - RPI_IPA_ACTION_SET_SENSOR_CONFIG,\n> RPI_IPA_ACTION_EMBEDDED_COMPLETE,\n> RPI_IPA_EVENT_SIGNAL_STAT_READY,\n> RPI_IPA_EVENT_SIGNAL_ISP_PREPARE,\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index c9f7dea375a5..7da2ffdf84a1 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -93,7 +93,7 @@ private:\n> void reportMetadata();\n> bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);\n> void processStats(unsigned int bufferId);\n> - void applyAGC(const struct AgcStatus *agcStatus);\n> + void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n> void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);\n> void applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls);\n> void applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls);\n> @@ -195,6 +195,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> if (entityControls.empty())\n> return;\n>\n> + result->operation = 0;\n> +\n> unicam_ctrls_ = entityControls.at(0);\n> isp_ctrls_ = entityControls.at(1);\n> /* Setup a metadata ControlList to output metadata. */\n> @@ -217,18 +219,16 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> sensorMetadata = helper_->SensorEmbeddedDataPresent();\n> RPi::CamTransform orientation = helper_->GetOrientation();\n>\n> - IPAOperationData op;\n> - op.operation = RPI_IPA_ACTION_SET_SENSOR_CONFIG;\n> - op.data.push_back(gainDelay);\n> - op.data.push_back(exposureDelay);\n> - op.data.push_back(sensorMetadata);\n> + result->data.push_back(gainDelay);\n> + result->data.push_back(exposureDelay);\n> + result->data.push_back(sensorMetadata);\n>\n> ControlList ctrls(unicam_ctrls_);\n> ctrls.set(V4L2_CID_HFLIP, (int32_t) !!(orientation & RPi::CamTransform_HFLIP));\n> ctrls.set(V4L2_CID_VFLIP, (int32_t) !!(orientation & RPi::CamTransform_VFLIP));\n> - op.controls.push_back(ctrls);\n> + result->controls.push_back(ctrls);\n>\n> - queueFrameAction.emit(0, op);\n> + result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE;\n\nThe operation is flagged RPI_IPA_CONFIG_STAGGERED_WRITE, but it sets\nthe staggered delays as well as sensor orientation. This operation\nwas previously called RPI_IPA_ACTION_SET_SENSOR_CONFIG as a generic\ntag (perhaps a bit naughty) to capture this :) Perhaps a new tag\nRPI_IPA_CONFIG_SENSOR_ORIENTATION to reflect this?\n\n> }\n>\n> /* Re-assemble camera mode using the sensor info. */\n> @@ -273,8 +273,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n>\n> /* SwitchMode may supply updated exposure/gain values to use. */\n> metadata.Get(\"agc.status\", agcStatus);\n> - if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0)\n> - applyAGC(&agcStatus);\n> + if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {\n> + ControlList ctrls(unicam_ctrls_);\n> + applyAGC(&agcStatus, ctrls);\n> + result->controls.push_back(ctrls);\n> +\n> + result->operation |= RPI_IPA_CONFIG_SENSOR;\n> + }\n>\n> lastMode_ = mode_;\n>\n> @@ -781,8 +786,15 @@ void IPARPi::processStats(unsigned int bufferId)\n> controller_.Process(statistics, &rpiMetadata_);\n>\n> struct AgcStatus agcStatus;\n> - if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0)\n> - applyAGC(&agcStatus);\n> + if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0) {\n> + ControlList ctrls(unicam_ctrls_);\n> + applyAGC(&agcStatus, ctrls);\n> +\n> + IPAOperationData op;\n> + op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;\n> + op.controls.push_back(ctrls);\n> + queueFrameAction.emit(0, op);\n> + }\n> }\n>\n> void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> @@ -808,11 +820,8 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> static_cast(awbStatus->gain_b * 1000));\n> }\n>\n> -void IPARPi::applyAGC(const struct AgcStatus *agcStatus)\n> +void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n> {\n> - IPAOperationData op;\n> - op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;\n> -\n> int32_t gain_code = helper_->GainCode(agcStatus->analogue_gain);\n> int32_t exposure_lines = helper_->ExposureLines(agcStatus->shutter_time);\n>\n> @@ -831,11 +840,8 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus)\n> << agcStatus->analogue_gain << \" (Gain Code: \"\n> << gain_code << \")\";\n>\n> - ControlList ctrls(unicam_ctrls_);\n> ctrls.set(V4L2_CID_ANALOGUE_GAIN, gain_code);\n> ctrls.set(V4L2_CID_EXPOSURE, exposure_lines);\n> - op.controls.push_back(ctrls);\n> - queueFrameAction.emit(0, op);\n> }\n>\n> void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 903796790f44..60b01484b329 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -806,7 +806,7 @@ int PipelineHandlerRPi::start(Camera *camera)\n> /*\n> * Write the last set of gain and exposure values to the camera before\n> * starting. First check that the staggered ctrl has been initialised\n> - * by the IPA action.\n> + * by configure().\n> */\n> ASSERT(data->staggeredCtrl_);\n> data->staggeredCtrl_.reset();\n> @@ -1152,44 +1152,45 @@ int RPiCameraData::configureIPA()\n> }\n>\n> /* Ready the IPA - it must know about the sensor resolution. */\n> + IPAOperationData result;\n> +\n> ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> - nullptr);\n> + &result);\n>\n> - return 0;\n> -}\n> -\n> -void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action)\n> -{\n> - /*\n> - * The following actions can be handled when the pipeline handler is in\n> - * a stopped state.\n> - */\n> - switch (action.operation) {\n> - case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {\n> - ControlList controls = action.controls[0];\n> - if (!staggeredCtrl_.set(controls))\n> - LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> - goto done;\n> - }\n> -\n> - case RPI_IPA_ACTION_SET_SENSOR_CONFIG: {\n> + if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) {\n> /*\n> * Setup our staggered control writer with the sensor default\n> * gain and exposure delays.\n> */\n> if (!staggeredCtrl_) {\n> staggeredCtrl_.init(unicam_[Unicam::Image].dev(),\n> - { { V4L2_CID_ANALOGUE_GAIN, action.data[0] },\n> - { V4L2_CID_EXPOSURE, action.data[1] } });\n> - sensorMetadata_ = action.data[2];\n> + { { V4L2_CID_ANALOGUE_GAIN, result.data[0] },\n> + { V4L2_CID_EXPOSURE, result.data[1] } });\n> + sensorMetadata_ = result.data[2];\n> }\n>\n> /* Set the sensor orientation here as well. */\n> - ControlList controls = action.controls[0];\n> - unicam_[Unicam::Image].dev()->setControls(&controls);\n> - goto done;\n> + unicam_[Unicam::Image].dev()->setControls(&result.controls[0]);\n> }\n>\n> + if (result.operation & RPI_IPA_CONFIG_SENSOR) {\n> + unsigned int idx = result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE\n> + ? 1 : 0;\n> + const ControlList &ctrls = result.controls[idx];\n> + if (!staggeredCtrl_.set(ctrls))\n> + LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action)\n> +{\n> + /*\n> + * The following actions can be handled when the pipeline handler is in\n> + * a stopped state.\n> + */\n> + switch (action.operation) {\n> case RPI_IPA_ACTION_V4L2_SET_ISP: {\n> ControlList controls = action.controls[0];\n> isp_[Isp::Input].dev()->setControls(&controls);\n> @@ -1205,6 +1206,13 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n> * is in a stopped state.\n> */\n> switch (action.operation) {\n> + case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {\n> + const ControlList &controls = action.controls[0];\n> + if (!staggeredCtrl_.set(controls))\n> + LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> + break;\n> + }\n> +\n\nAs per the other discussion, perhaps this should be reverted to the\noriginal code to allow it to be set in a stopped state?\n\n> case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: {\n> unsigned int bufferId = action.data[0];\n> FrameBuffer *buffer = isp_[Isp::Stats].getBuffers()->at(bufferId).get();\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n\nRegards,\nNaush","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 11C5ABF415\n\tfor ;\n\tTue, 30 Jun 2020 10:36:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D210060C57;\n\tTue, 30 Jun 2020 12:36:10 +0200 (CEST)","from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 67C4A609C7\n\tfor ;\n\tTue, 30 Jun 2020 12:36:09 +0200 (CEST)","by mail-lj1-x234.google.com with SMTP id f5so5969485ljj.10\n\tfor ;\n\tTue, 30 Jun 2020 03:36:09 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"jKeY0oPC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=VZMLyh+cjrCMdh1kKVLaOWXWuR6lt8m3xs71ZJjETyo=;\n\tb=jKeY0oPClb0slyxPA3yEYbFoXIaUKg6UcP1ZvkEG6qog68+vv+/kmExt0NPk4IffRT\n\tST+8gaIGdoxDsT/X5ZWbg/GE4XUUjyN6Y8hea9YVO+x3YaXP6+mUu+bJCmevKRuaFUCM\n\tm2oQB2MsR4yP3EluWZMAlXP/H8uIwVYy0arowON+cu1Iv/T94G9ief9+ZoTcNTyDEdMp\n\tKUdyot2V+oRNSx+MfulcGDieMrH7TJf7yv6g6pLIiv63iMDXBCduUaj4yRh6ircNj+Qh\n\tTVgNVON1+X1lPc/9lxu90odCferxArOAakj4PwWVKgMLUt3Yz5aI3luQwTbO3GH9nvIF\n\t3Keg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=VZMLyh+cjrCMdh1kKVLaOWXWuR6lt8m3xs71ZJjETyo=;\n\tb=Y7WtQoanvDyt2W6vBG1zxuy0tp5N7TrY1xF6fapFwa1VpJH+2otP+yScOVPhZsX1gk\n\tlBqEwX4ZxZlUkXkLFb9tDDmyjHZf2JRsjmZGg//qD8inJ6Hnd5tnepW1bi6Bki5zaHH9\n\t7PoQT476mnhmaeBYFLRWEA8AGKNbIlGXJJtCw9nlH+C/ShYr/MNTLxdkfhanZunix5Hr\n\toE3Ohx12cNVryuFwZBscnfh9/xnRvD9+HmB7uh9WhUjcltoZIQHEfLLzomKsVpIHmlfp\n\teTMWgrmpNf9H8SGZGzKvWIhViycjh8P603AdAWRTEQgukH8zlqq9qcZUgkfkMPq3ctzL\n\t03LQ==","X-Gm-Message-State":"AOAM533pyh3uXvAv/nysTKAi6uAK+R7jjqSrFmJcvvUu8PWwwqzn0Mse\n\tEmGI+YuzNVeB+63WBDBk5aD+7AaLd2n3paKmJ7Z5mw==","X-Google-Smtp-Source":"ABdhPJxwSKpxXBwe5a3eK1UdNy0r5PE9kGOP62Ixnpa74a9Ruyd97uzv6JBXhVgXXZyhvxdYDHxjdEwZAw+bAnwihB0=","X-Received":"by 2002:a2e:3a04:: with SMTP id\n\th4mr10584538lja.103.1593513368669; \n\tTue, 30 Jun 2020 03:36:08 -0700 (PDT)","MIME-Version":"1.0","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-8-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20200628231934.29025-8-laurent.pinchart@ideasonboard.com>","From":"Naushir Patuck ","Date":"Tue, 30 Jun 2020 11:35:52 +0100","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v1 7/9] ipa: raspberrypi: Pass sensor\n\tconfig back from configure()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11165,"web_url":"https://patchwork.libcamera.org/comment/11165/","msgid":"<20200703193225.GB26361@pendragon.ideasonboard.com>","date":"2020-07-03T19:32:25","subject":"Re: [libcamera-devel] [PATCH v1 7/9] ipa: raspberrypi: Pass sensor\n\tconfig back from configure()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Tue, Jun 30, 2020 at 11:35:52AM +0100, Naushir Patuck wrote:\n> On Mon, 29 Jun 2020 at 00:19, Laurent Pinchart wrote:\n> >\n> > The Raspberry Pi IPA uses a custom frame action,\n> > RPI_IPA_ACTION_SET_SENSOR_CONFIG, to send initial sensor configuration\n> > to the pipeline handler when the IPA is configured. Replace this ad-hoc\n> > mechanism by passing the corresponding data back from the IPA to the\n> > pipeline handler through the configure() response. This allows\n> > synchronous handling of the response on the pipeline handler side.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/ipa/raspberrypi.h | 3 +-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 44 ++++++++------\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 60 +++++++++++--------\n> > 3 files changed, 61 insertions(+), 46 deletions(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> > index c335d0259549..77a7e6d40a2f 100644\n> > --- a/include/libcamera/ipa/raspberrypi.h\n> > +++ b/include/libcamera/ipa/raspberrypi.h\n> > @@ -12,6 +12,8 @@\n> >\n> > enum RPiConfigParameters {\n> > RPI_IPA_CONFIG_LSTABLE = (1 << 0),\n> > + RPI_IPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n> > + RPI_IPA_CONFIG_SENSOR = (1 << 2),\n> > };\n> >\n> > enum RPiOperations {\n> > @@ -20,7 +22,6 @@ enum RPiOperations {\n> > RPI_IPA_ACTION_STATS_METADATA_COMPLETE,\n> > RPI_IPA_ACTION_RUN_ISP,\n> > RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,\n> > - RPI_IPA_ACTION_SET_SENSOR_CONFIG,\n> > RPI_IPA_ACTION_EMBEDDED_COMPLETE,\n> > RPI_IPA_EVENT_SIGNAL_STAT_READY,\n> > RPI_IPA_EVENT_SIGNAL_ISP_PREPARE,\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index c9f7dea375a5..7da2ffdf84a1 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -93,7 +93,7 @@ private:\n> > void reportMetadata();\n> > bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);\n> > void processStats(unsigned int bufferId);\n> > - void applyAGC(const struct AgcStatus *agcStatus);\n> > + void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n> > void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);\n> > void applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls);\n> > void applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls);\n> > @@ -195,6 +195,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > if (entityControls.empty())\n> > return;\n> >\n> > + result->operation = 0;\n> > +\n> > unicam_ctrls_ = entityControls.at(0);\n> > isp_ctrls_ = entityControls.at(1);\n> > /* Setup a metadata ControlList to output metadata. */\n> > @@ -217,18 +219,16 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > sensorMetadata = helper_->SensorEmbeddedDataPresent();\n> > RPi::CamTransform orientation = helper_->GetOrientation();\n> >\n> > - IPAOperationData op;\n> > - op.operation = RPI_IPA_ACTION_SET_SENSOR_CONFIG;\n> > - op.data.push_back(gainDelay);\n> > - op.data.push_back(exposureDelay);\n> > - op.data.push_back(sensorMetadata);\n> > + result->data.push_back(gainDelay);\n> > + result->data.push_back(exposureDelay);\n> > + result->data.push_back(sensorMetadata);\n> >\n> > ControlList ctrls(unicam_ctrls_);\n> > ctrls.set(V4L2_CID_HFLIP, (int32_t) !!(orientation & RPi::CamTransform_HFLIP));\n> > ctrls.set(V4L2_CID_VFLIP, (int32_t) !!(orientation & RPi::CamTransform_VFLIP));\n> > - op.controls.push_back(ctrls);\n> > + result->controls.push_back(ctrls);\n> >\n> > - queueFrameAction.emit(0, op);\n> > + result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE;\n> \n> The operation is flagged RPI_IPA_CONFIG_STAGGERED_WRITE, but it sets\n> the staggered delays as well as sensor orientation. This operation\n> was previously called RPI_IPA_ACTION_SET_SENSOR_CONFIG as a generic\n> tag (perhaps a bit naughty) to capture this :) Perhaps a new tag\n> RPI_IPA_CONFIG_SENSOR_ORIENTATION to reflect this?\n\nActually I think I found a simpler option. I'll post a v2.\n\n> > }\n> >\n> > /* Re-assemble camera mode using the sensor info. */\n> > @@ -273,8 +273,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> >\n> > /* SwitchMode may supply updated exposure/gain values to use. */\n> > metadata.Get(\"agc.status\", agcStatus);\n> > - if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0)\n> > - applyAGC(&agcStatus);\n> > + if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {\n> > + ControlList ctrls(unicam_ctrls_);\n> > + applyAGC(&agcStatus, ctrls);\n> > + result->controls.push_back(ctrls);\n> > +\n> > + result->operation |= RPI_IPA_CONFIG_SENSOR;\n> > + }\n> >\n> > lastMode_ = mode_;\n> >\n> > @@ -781,8 +786,15 @@ void IPARPi::processStats(unsigned int bufferId)\n> > controller_.Process(statistics, &rpiMetadata_);\n> >\n> > struct AgcStatus agcStatus;\n> > - if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0)\n> > - applyAGC(&agcStatus);\n> > + if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0) {\n> > + ControlList ctrls(unicam_ctrls_);\n> > + applyAGC(&agcStatus, ctrls);\n> > +\n> > + IPAOperationData op;\n> > + op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;\n> > + op.controls.push_back(ctrls);\n> > + queueFrameAction.emit(0, op);\n> > + }\n> > }\n> >\n> > void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> > @@ -808,11 +820,8 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> > static_cast(awbStatus->gain_b * 1000));\n> > }\n> >\n> > -void IPARPi::applyAGC(const struct AgcStatus *agcStatus)\n> > +void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n> > {\n> > - IPAOperationData op;\n> > - op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;\n> > -\n> > int32_t gain_code = helper_->GainCode(agcStatus->analogue_gain);\n> > int32_t exposure_lines = helper_->ExposureLines(agcStatus->shutter_time);\n> >\n> > @@ -831,11 +840,8 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus)\n> > << agcStatus->analogue_gain << \" (Gain Code: \"\n> > << gain_code << \")\";\n> >\n> > - ControlList ctrls(unicam_ctrls_);\n> > ctrls.set(V4L2_CID_ANALOGUE_GAIN, gain_code);\n> > ctrls.set(V4L2_CID_EXPOSURE, exposure_lines);\n> > - op.controls.push_back(ctrls);\n> > - queueFrameAction.emit(0, op);\n> > }\n> >\n> > void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 903796790f44..60b01484b329 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -806,7 +806,7 @@ int PipelineHandlerRPi::start(Camera *camera)\n> > /*\n> > * Write the last set of gain and exposure values to the camera before\n> > * starting. First check that the staggered ctrl has been initialised\n> > - * by the IPA action.\n> > + * by configure().\n> > */\n> > ASSERT(data->staggeredCtrl_);\n> > data->staggeredCtrl_.reset();\n> > @@ -1152,44 +1152,45 @@ int RPiCameraData::configureIPA()\n> > }\n> >\n> > /* Ready the IPA - it must know about the sensor resolution. */\n> > + IPAOperationData result;\n> > +\n> > ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> > - nullptr);\n> > + &result);\n> >\n> > - return 0;\n> > -}\n> > -\n> > -void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action)\n> > -{\n> > - /*\n> > - * The following actions can be handled when the pipeline handler is in\n> > - * a stopped state.\n> > - */\n> > - switch (action.operation) {\n> > - case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {\n> > - ControlList controls = action.controls[0];\n> > - if (!staggeredCtrl_.set(controls))\n> > - LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> > - goto done;\n> > - }\n> > -\n> > - case RPI_IPA_ACTION_SET_SENSOR_CONFIG: {\n> > + if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) {\n> > /*\n> > * Setup our staggered control writer with the sensor default\n> > * gain and exposure delays.\n> > */\n> > if (!staggeredCtrl_) {\n> > staggeredCtrl_.init(unicam_[Unicam::Image].dev(),\n> > - { { V4L2_CID_ANALOGUE_GAIN, action.data[0] },\n> > - { V4L2_CID_EXPOSURE, action.data[1] } });\n> > - sensorMetadata_ = action.data[2];\n> > + { { V4L2_CID_ANALOGUE_GAIN, result.data[0] },\n> > + { V4L2_CID_EXPOSURE, result.data[1] } });\n> > + sensorMetadata_ = result.data[2];\n> > }\n> >\n> > /* Set the sensor orientation here as well. */\n> > - ControlList controls = action.controls[0];\n> > - unicam_[Unicam::Image].dev()->setControls(&controls);\n> > - goto done;\n> > + unicam_[Unicam::Image].dev()->setControls(&result.controls[0]);\n> > }\n> >\n> > + if (result.operation & RPI_IPA_CONFIG_SENSOR) {\n> > + unsigned int idx = result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE\n> > + ? 1 : 0;\n> > + const ControlList &ctrls = result.controls[idx];\n> > + if (!staggeredCtrl_.set(ctrls))\n> > + LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action)\n> > +{\n> > + /*\n> > + * The following actions can be handled when the pipeline handler is in\n> > + * a stopped state.\n> > + */\n> > + switch (action.operation) {\n> > case RPI_IPA_ACTION_V4L2_SET_ISP: {\n> > ControlList controls = action.controls[0];\n> > isp_[Isp::Input].dev()->setControls(&controls);\n> > @@ -1205,6 +1206,13 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n> > * is in a stopped state.\n> > */\n> > switch (action.operation) {\n> > + case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {\n> > + const ControlList &controls = action.controls[0];\n> > + if (!staggeredCtrl_.set(controls))\n> > + LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> > + break;\n> > + }\n> > +\n> \n> As per the other discussion, perhaps this should be reverted to the\n> original code to allow it to be set in a stopped state?\n\nYes, I'll do so, and we can decide later if we want to change it.\n\n> > case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: {\n> > unsigned int bufferId = action.data[0];\n> > FrameBuffer *buffer = isp_[Isp::Stats].getBuffers()->at(bufferId).get();","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 B63DABFFE2\n\tfor ;\n\tFri, 3 Jul 2020 19:32:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E2CC60CDE;\n\tFri, 3 Jul 2020 21:32:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 393F5609A9\n\tfor ;\n\tFri, 3 Jul 2020 21:32:30 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B44CD29E;\n\tFri, 3 Jul 2020 21:32:29 +0200 (CEST)"],"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=\"h6Z8mXlY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593804749;\n\tbh=mQC//ii3aMYK2sHZxdHsq8zz0OsnzzFJT2DZOXf63Sc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=h6Z8mXlYj65JRvcDatedDxlqVyiFAYlcssdhDC4WVr+a24tm2F/rcSdyorC+ugqXE\n\tUkbFfKT3gEheeH+B28ciA+np6LrT98JPEIvELyYorTP4tv2xAwJg3pQUshaha99bpO\n\t2bumW2VLyx16gRUd12nH+1cwBIkDSRzp79AEPOqM=","Date":"Fri, 3 Jul 2020 22:32:25 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"<20200703193225.GB26361@pendragon.ideasonboard.com>","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-8-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1 7/9] ipa: raspberrypi: Pass sensor\n\tconfig back from configure()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11166,"web_url":"https://patchwork.libcamera.org/comment/11166/","msgid":"<20200703193415.GG14282@pendragon.ideasonboard.com>","date":"2020-07-03T19:34:15","subject":"Re: [libcamera-devel] [PATCH v1 7/9] ipa: raspberrypi: Pass sensor\n\tconfig back from configure()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Jul 03, 2020 at 10:32:26PM +0300, Laurent Pinchart wrote:\n> On Tue, Jun 30, 2020 at 11:35:52AM +0100, Naushir Patuck wrote:\n> > On Mon, 29 Jun 2020 at 00:19, Laurent Pinchart wrote:\n> > >\n> > > The Raspberry Pi IPA uses a custom frame action,\n> > > RPI_IPA_ACTION_SET_SENSOR_CONFIG, to send initial sensor configuration\n> > > to the pipeline handler when the IPA is configured. Replace this ad-hoc\n> > > mechanism by passing the corresponding data back from the IPA to the\n> > > pipeline handler through the configure() response. This allows\n> > > synchronous handling of the response on the pipeline handler side.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > include/libcamera/ipa/raspberrypi.h | 3 +-\n> > > src/ipa/raspberrypi/raspberrypi.cpp | 44 ++++++++------\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 60 +++++++++++--------\n> > > 3 files changed, 61 insertions(+), 46 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> > > index c335d0259549..77a7e6d40a2f 100644\n> > > --- a/include/libcamera/ipa/raspberrypi.h\n> > > +++ b/include/libcamera/ipa/raspberrypi.h\n> > > @@ -12,6 +12,8 @@\n> > >\n> > > enum RPiConfigParameters {\n> > > RPI_IPA_CONFIG_LSTABLE = (1 << 0),\n> > > + RPI_IPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n> > > + RPI_IPA_CONFIG_SENSOR = (1 << 2),\n> > > };\n> > >\n> > > enum RPiOperations {\n> > > @@ -20,7 +22,6 @@ enum RPiOperations {\n> > > RPI_IPA_ACTION_STATS_METADATA_COMPLETE,\n> > > RPI_IPA_ACTION_RUN_ISP,\n> > > RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,\n> > > - RPI_IPA_ACTION_SET_SENSOR_CONFIG,\n> > > RPI_IPA_ACTION_EMBEDDED_COMPLETE,\n> > > RPI_IPA_EVENT_SIGNAL_STAT_READY,\n> > > RPI_IPA_EVENT_SIGNAL_ISP_PREPARE,\n> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > index c9f7dea375a5..7da2ffdf84a1 100644\n> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > @@ -93,7 +93,7 @@ private:\n> > > void reportMetadata();\n> > > bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);\n> > > void processStats(unsigned int bufferId);\n> > > - void applyAGC(const struct AgcStatus *agcStatus);\n> > > + void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n> > > void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);\n> > > void applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls);\n> > > void applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls);\n> > > @@ -195,6 +195,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > > if (entityControls.empty())\n> > > return;\n> > >\n> > > + result->operation = 0;\n> > > +\n> > > unicam_ctrls_ = entityControls.at(0);\n> > > isp_ctrls_ = entityControls.at(1);\n> > > /* Setup a metadata ControlList to output metadata. */\n> > > @@ -217,18 +219,16 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > > sensorMetadata = helper_->SensorEmbeddedDataPresent();\n> > > RPi::CamTransform orientation = helper_->GetOrientation();\n> > >\n> > > - IPAOperationData op;\n> > > - op.operation = RPI_IPA_ACTION_SET_SENSOR_CONFIG;\n> > > - op.data.push_back(gainDelay);\n> > > - op.data.push_back(exposureDelay);\n> > > - op.data.push_back(sensorMetadata);\n> > > + result->data.push_back(gainDelay);\n> > > + result->data.push_back(exposureDelay);\n> > > + result->data.push_back(sensorMetadata);\n> > >\n> > > ControlList ctrls(unicam_ctrls_);\n> > > ctrls.set(V4L2_CID_HFLIP, (int32_t) !!(orientation & RPi::CamTransform_HFLIP));\n> > > ctrls.set(V4L2_CID_VFLIP, (int32_t) !!(orientation & RPi::CamTransform_VFLIP));\n> > > - op.controls.push_back(ctrls);\n> > > + result->controls.push_back(ctrls);\n> > >\n> > > - queueFrameAction.emit(0, op);\n> > > + result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE;\n> > \n> > The operation is flagged RPI_IPA_CONFIG_STAGGERED_WRITE, but it sets\n> > the staggered delays as well as sensor orientation. This operation\n> > was previously called RPI_IPA_ACTION_SET_SENSOR_CONFIG as a generic\n> > tag (perhaps a bit naughty) to capture this :) Perhaps a new tag\n> > RPI_IPA_CONFIG_SENSOR_ORIENTATION to reflect this?\n> \n> Actually I think I found a simpler option. I'll post a v2.\n\nI spoke a bit too fast. I'll still send a v2, but my clever solution\nfailed miserably :-) I'll still address this.\n\n> > > }\n> > >\n> > > /* Re-assemble camera mode using the sensor info. */\n> > > @@ -273,8 +273,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > >\n> > > /* SwitchMode may supply updated exposure/gain values to use. */\n> > > metadata.Get(\"agc.status\", agcStatus);\n> > > - if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0)\n> > > - applyAGC(&agcStatus);\n> > > + if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {\n> > > + ControlList ctrls(unicam_ctrls_);\n> > > + applyAGC(&agcStatus, ctrls);\n> > > + result->controls.push_back(ctrls);\n> > > +\n> > > + result->operation |= RPI_IPA_CONFIG_SENSOR;\n> > > + }\n> > >\n> > > lastMode_ = mode_;\n> > >\n> > > @@ -781,8 +786,15 @@ void IPARPi::processStats(unsigned int bufferId)\n> > > controller_.Process(statistics, &rpiMetadata_);\n> > >\n> > > struct AgcStatus agcStatus;\n> > > - if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0)\n> > > - applyAGC(&agcStatus);\n> > > + if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0) {\n> > > + ControlList ctrls(unicam_ctrls_);\n> > > + applyAGC(&agcStatus, ctrls);\n> > > +\n> > > + IPAOperationData op;\n> > > + op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;\n> > > + op.controls.push_back(ctrls);\n> > > + queueFrameAction.emit(0, op);\n> > > + }\n> > > }\n> > >\n> > > void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> > > @@ -808,11 +820,8 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> > > static_cast(awbStatus->gain_b * 1000));\n> > > }\n> > >\n> > > -void IPARPi::applyAGC(const struct AgcStatus *agcStatus)\n> > > +void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n> > > {\n> > > - IPAOperationData op;\n> > > - op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;\n> > > -\n> > > int32_t gain_code = helper_->GainCode(agcStatus->analogue_gain);\n> > > int32_t exposure_lines = helper_->ExposureLines(agcStatus->shutter_time);\n> > >\n> > > @@ -831,11 +840,8 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus)\n> > > << agcStatus->analogue_gain << \" (Gain Code: \"\n> > > << gain_code << \")\";\n> > >\n> > > - ControlList ctrls(unicam_ctrls_);\n> > > ctrls.set(V4L2_CID_ANALOGUE_GAIN, gain_code);\n> > > ctrls.set(V4L2_CID_EXPOSURE, exposure_lines);\n> > > - op.controls.push_back(ctrls);\n> > > - queueFrameAction.emit(0, op);\n> > > }\n> > >\n> > > void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 903796790f44..60b01484b329 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -806,7 +806,7 @@ int PipelineHandlerRPi::start(Camera *camera)\n> > > /*\n> > > * Write the last set of gain and exposure values to the camera before\n> > > * starting. First check that the staggered ctrl has been initialised\n> > > - * by the IPA action.\n> > > + * by configure().\n> > > */\n> > > ASSERT(data->staggeredCtrl_);\n> > > data->staggeredCtrl_.reset();\n> > > @@ -1152,44 +1152,45 @@ int RPiCameraData::configureIPA()\n> > > }\n> > >\n> > > /* Ready the IPA - it must know about the sensor resolution. */\n> > > + IPAOperationData result;\n> > > +\n> > > ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> > > - nullptr);\n> > > + &result);\n> > >\n> > > - return 0;\n> > > -}\n> > > -\n> > > -void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action)\n> > > -{\n> > > - /*\n> > > - * The following actions can be handled when the pipeline handler is in\n> > > - * a stopped state.\n> > > - */\n> > > - switch (action.operation) {\n> > > - case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {\n> > > - ControlList controls = action.controls[0];\n> > > - if (!staggeredCtrl_.set(controls))\n> > > - LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> > > - goto done;\n> > > - }\n> > > -\n> > > - case RPI_IPA_ACTION_SET_SENSOR_CONFIG: {\n> > > + if (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) {\n> > > /*\n> > > * Setup our staggered control writer with the sensor default\n> > > * gain and exposure delays.\n> > > */\n> > > if (!staggeredCtrl_) {\n> > > staggeredCtrl_.init(unicam_[Unicam::Image].dev(),\n> > > - { { V4L2_CID_ANALOGUE_GAIN, action.data[0] },\n> > > - { V4L2_CID_EXPOSURE, action.data[1] } });\n> > > - sensorMetadata_ = action.data[2];\n> > > + { { V4L2_CID_ANALOGUE_GAIN, result.data[0] },\n> > > + { V4L2_CID_EXPOSURE, result.data[1] } });\n> > > + sensorMetadata_ = result.data[2];\n> > > }\n> > >\n> > > /* Set the sensor orientation here as well. */\n> > > - ControlList controls = action.controls[0];\n> > > - unicam_[Unicam::Image].dev()->setControls(&controls);\n> > > - goto done;\n> > > + unicam_[Unicam::Image].dev()->setControls(&result.controls[0]);\n> > > }\n> > >\n> > > + if (result.operation & RPI_IPA_CONFIG_SENSOR) {\n> > > + unsigned int idx = result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE\n> > > + ? 1 : 0;\n> > > + const ControlList &ctrls = result.controls[idx];\n> > > + if (!staggeredCtrl_.set(ctrls))\n> > > + LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> > > + }\n> > > +\n> > > + return 0;\n> > > +}\n> > > +\n> > > +void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action)\n> > > +{\n> > > + /*\n> > > + * The following actions can be handled when the pipeline handler is in\n> > > + * a stopped state.\n> > > + */\n> > > + switch (action.operation) {\n> > > case RPI_IPA_ACTION_V4L2_SET_ISP: {\n> > > ControlList controls = action.controls[0];\n> > > isp_[Isp::Input].dev()->setControls(&controls);\n> > > @@ -1205,6 +1206,13 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n> > > * is in a stopped state.\n> > > */\n> > > switch (action.operation) {\n> > > + case RPI_IPA_ACTION_V4L2_SET_STAGGERED: {\n> > > + const ControlList &controls = action.controls[0];\n> > > + if (!staggeredCtrl_.set(controls))\n> > > + LOG(RPI, Error) << \"V4L2 staggered set failed\";\n> > > + break;\n> > > + }\n> > > +\n> > \n> > As per the other discussion, perhaps this should be reverted to the\n> > original code to allow it to be set in a stopped state?\n> \n> Yes, I'll do so, and we can decide later if we want to change it.\n> \n> > > case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: {\n> > > unsigned int bufferId = action.data[0];\n> > > FrameBuffer *buffer = isp_[Isp::Stats].getBuffers()->at(bufferId).get();","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 66BE7BE905\n\tfor ;\n\tFri, 3 Jul 2020 19:34:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB55C609C5;\n\tFri, 3 Jul 2020 21:34:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 08804609A9\n\tfor ;\n\tFri, 3 Jul 2020 21:34:21 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 76D4C29E;\n\tFri, 3 Jul 2020 21:34:20 +0200 (CEST)"],"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=\"nhaVQ/9T\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593804860;\n\tbh=rr0Uq2BMcCLsDxJDwQp/znEA4ETdzpQae2p30zWeSpY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nhaVQ/9TdCvXNvRLzok9COFVJgP/MOx9VLQ+QwbgcLr/TJhu6MXAvcV9L/o32omlW\n\tdeTJZWlyywgc4UcgDe0oHeK5cacO+enUvUSkWoILm3Ef9oeV22Xrt9rePsZKuBu8Rw\n\tfuk6BLxAL1Zvy4d0FUi6zhUmXvqSp402LSE9924s=","Date":"Fri, 3 Jul 2020 22:34:15 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"<20200703193415.GG14282@pendragon.ideasonboard.com>","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-8-laurent.pinchart@ideasonboard.com>\n\t\n\t<20200703193225.GB26361@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200703193225.GB26361@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 7/9] ipa: raspberrypi: Pass sensor\n\tconfig back from configure()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]