<div dir="ltr"><div dir="ltr">Hi Niklas,<div><br></div><div>Thank you for your patch.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 15 Dec 2020 at 00:48, Niklas Söderlund <<a href="mailto:niklas.soderlund@ragnatech.se">niklas.soderlund@ragnatech.se</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Use the libcamera core helper DelayedControls instead of the local<br>
StaggeredCtrl. The new helper is modeled after the StaggeredCtrl<br>
implementation and behaves the same.<br>
<br>
Signed-off-by: Niklas Söderlund <<a href="mailto:niklas.soderlund@ragnatech.se" target="_blank">niklas.soderlund@ragnatech.se</a>><br>
---<br>
 .../pipeline/raspberrypi/raspberrypi.cpp      | 54 +++++++++----------<br>
 1 file changed, 24 insertions(+), 30 deletions(-)<br>
<br>
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
index 7a5f5881b9b30129..a0186bee9926f945 100644<br>
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
@@ -27,6 +27,7 @@<br>
 #include "libcamera/internal/bayer_format.h"<br>
 #include "libcamera/internal/buffer.h"<br>
 #include "libcamera/internal/camera_sensor.h"<br>
+#include "libcamera/internal/delayed_controls.h"<br>
 #include "libcamera/internal/device_enumerator.h"<br>
 #include "libcamera/internal/ipa_manager.h"<br>
 #include "libcamera/internal/media_device.h"<br>
@@ -37,7 +38,6 @@<br>
<br>
 #include "dma_heaps.h"<br>
 #include "rpi_stream.h"<br>
-#include "staggered_ctrl.h"<br>
<br>
 namespace libcamera {<br>
<br>
@@ -178,8 +178,7 @@ public:<br>
        RPi::DmaHeap dmaHeap_;<br>
        FileDescriptor lsTable_;<br>
<br>
-       RPi::StaggeredCtrl staggeredCtrl_;<br>
-       uint32_t expectedSequence_;<br>
+       std::unique_ptr<DelayedControls> delayedCtrls_;<br>
        bool sensorMetadata_;<br>
<br>
        /*<br>
@@ -766,11 +765,9 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont<br>
        }<br>
<br>
        /* Apply any gain/exposure settings that the IPA may have passed back. */<br>
-       ASSERT(data->staggeredCtrl_);<br>
        if (result.operation & RPi::IPA_CONFIG_SENSOR) {<br>
-               const ControlList &ctrls = result.controls[0];<br>
-               if (!data->staggeredCtrl_.set(ctrls))<br>
-                       LOG(RPI, Error) << "V4L2 staggered set failed";<br>
+               ControlList ctrls = result.controls[0];<br>
+               data->unicam_[Unicam::Image].dev()->setControls(&ctrls);<br></blockquote><div><br></div><div>Could we optimise the copy to ctrls out here?  Not sure if the compiler will do this for us or not...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
        }<br>
<br>
        if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) {<br>
@@ -802,13 +799,10 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont<br>
        data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true);<br>
<br>
        /*<br>
-        * Write the last set of gain and exposure values to the camera before<br>
-        * starting. First check that the staggered ctrl has been initialised<br>
-        * by configure().<br>
+        * Reset the delayed controls with the  gain and exposure values set by<br>
+        * the IPA.<br>
         */<br>
-       data->staggeredCtrl_.reset();<br>
-       data->staggeredCtrl_.write();<br>
-       data->expectedSequence_ = 0;<br>
+       data->delayedCtrls_->reset();<br>
<br>
        data->state_ = RPiCameraData::State::Idle;<br>
<br>
@@ -1147,7 +1141,7 @@ void RPiCameraData::frameStarted(uint32_t sequence)<br>
        LOG(RPI, Debug) << "frame start " << sequence;<br>
<br>
        /* Write any controls for the next frame as soon as we can. */<br>
-       staggeredCtrl_.write();<br>
+       delayedCtrls_->applyControls(sequence);<br>
 }<br>
<br>
 int RPiCameraData::loadIPA()<br>
@@ -1230,18 +1224,22 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)<br>
                 * Setup our staggered control writer with the sensor default<br>
                 * gain and exposure delays.<br>
                 */<br>
-               if (!staggeredCtrl_) {<br>
-                       staggeredCtrl_.init(unicam_[Unicam::Image].dev(),<br>
-                                           { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },<br>
-                                             { V4L2_CID_EXPOSURE, result.data[resultIdx++] } });<br>
+<br>
+               if (!delayedCtrls_) {<br>
+                       std::unordered_map<uint32_t, unsigned int> delays = {<br>
+                               { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },<br>
+                               { V4L2_CID_EXPOSURE, result.data[resultIdx++] }<br>
+                       };<br>
+<br>
+                       delayedCtrls_ = std::make_unique<DelayedControls>(unicam_[Unicam::Image].dev(), delays);<br>
+<br>
                        sensorMetadata_ = result.data[resultIdx++];<br>
                }<br>
        }<br>
<br>
        if (result.operation & RPi::IPA_CONFIG_SENSOR) {<br>
-               const ControlList &ctrls = result.controls[0];<br>
-               if (!staggeredCtrl_.set(ctrls))<br>
-                       LOG(RPI, Error) << "V4L2 staggered set failed";<br>
+               ControlList ctrls = result.controls[0];<br>
+               unicam_[Unicam::Image].dev()->setControls(&ctrls);<br>
        }<br>
<br>
        /*<br>
@@ -1270,8 +1268,8 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame,<br>
        switch (action.operation) {<br>
        case RPi::IPA_ACTION_V4L2_SET_STAGGERED: {<br></blockquote><div><br></div><div>This label probably wants to change to SET_DELAYED_CTRLS or similar to be consistent.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                const ControlList &controls = action.controls[0];<br>
-               if (!staggeredCtrl_.set(controls))<br>
-                       LOG(RPI, Error) << "V4L2 staggered set failed";<br>
+               if (!delayedCtrls_->push(controls))<br>
+                       LOG(RPI, Error) << "V4L2 delay set failed";<br>
                goto done;<br>
        }<br>
<br>
@@ -1375,11 +1373,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)<br>
        } else {<br>
                embeddedQueue_.push(buffer);<br>
<br>
-               std::unordered_map<uint32_t, int32_t> ctrl;<br>
-               int offset = buffer->metadata().sequence - expectedSequence_;<br>
-               staggeredCtrl_.get(ctrl, offset);<br>
-<br>
-               expectedSequence_ = buffer->metadata().sequence + 1;<br>
+               ControlList ctrl = delayedCtrls_->get(buffer->metadata().sequence);<br>
<br>
                /*<br>
                 * Sensor metadata is unavailable, so put the expected ctrl<br>
@@ -1391,8 +1385,8 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)<br>
                        auto it = mappedEmbeddedBuffers_.find(bufferId);<br>
                        if (it != mappedEmbeddedBuffers_.end()) {<br>
                                uint32_t *mem = reinterpret_cast<uint32_t *>(it->second.maps()[0].data());<br>
-                               mem[0] = ctrl[V4L2_CID_EXPOSURE];<br>
-                               mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];<br>
+                               mem[0] = ctrl.get(V4L2_CID_EXPOSURE).get<int32_t>();<br>
+                               mem[1] = ctrl.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();<br>
                        } else {<br>
                                LOG(RPI, Warning) << "Failed to find embedded buffer "<br>
                                                  << bufferId;<br></blockquote><div><br></div><div>Apart from the minors, </div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-- <br>
2.29.2<br>
<br>
</blockquote></div></div>