[libcamera-devel] [PATCH v3 2/2] libcamera: pipeline: ipu3: Apply the requested test pattern mode

[Kieran Bingham]

Quoting Hirokazu Honda (2021-11-02 02:55:22)
> This enables ipu3 pipeline handler to apply the test pattern mode
> per frame.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 41 ++++++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index eb714aa6..e81ba63c 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -59,6 +59,7 @@ public:
>         void statBufferReady(FrameBuffer *buffer);
>         void queuePendingRequests();
>         void cancelPendingRequests();
> +       void frameStart(uint32_t sequence);
>  
>         CIO2Device cio2_;
>         ImgUDevice *imgu_;
> @@ -78,6 +79,7 @@ public:
>         std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
>  
>         std::queue<Request *> pendingRequests_;
> +       std::queue<Request *> processingRequests_;
>  
>         ControlInfoMap ipaControls_;
>  
> @@ -569,6 +571,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>         cio2->sensor()->sensorInfo(&sensorInfo);
>         data->cropRegion_ = sensorInfo.analogCrop;
>  
> +       cio2->sensor()->setTestPatternMode(controls::draft::TestPatternModeOff);
> +
>         /*
>          * Configure the H/V flip controls based on the combination of
>          * the sensor and user transform.
> @@ -797,11 +801,13 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>         int ret = 0;
>  
>         data->cancelPendingRequests();
> +       data->processingRequests_ = {};
>  
>         data->ipa_->stop();
>  
>         ret |= data->imgu_->stop();
>         ret |= data->cio2_.stop();
> +
>         if (ret)
>                 LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
>  
> @@ -852,6 +858,8 @@ void IPU3CameraData::queuePendingRequests()
>  
>                 info->rawBuffer = rawBuffer;
>  
> +               processingRequests_.push(request);
> +
>                 ipa::ipu3::IPU3Event ev;
>                 ev.op = ipa::ipu3::EventProcessControls;
>                 ev.frame = info->id;
> @@ -1130,8 +1138,8 @@ int PipelineHandlerIPU3::registerCameras()
>                 data->delayedCtrls_ =
>                         std::make_unique<DelayedControls>(cio2->sensor()->device(),
>                                                           params);
> -               data->cio2_.frameStart().connect(data->delayedCtrls_.get(),
> -                                                &DelayedControls::applyControls);
> +               data->cio2_.frameStart().connect(data.get(),
> +                                                &IPU3CameraData::frameStart);
>  
>                 /* Convert the sensor rotation to a transformation */
>                 int32_t rotation = 0;
> @@ -1422,6 +1430,35 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>         ipa_->processEvent(ev);
>  }
>  
> +void IPU3CameraData::frameStart(uint32_t sequence)
> +{
> +       if (!processingRequests_.empty()) {
> +               /* Assumes applying the test pattern mode affects immediately. */
> +               Request *request = processingRequests_.front();
> +               processingRequests_.pop();
> +
> +               if (request->controls().contains(controls::draft::TestPatternMode)) {
> +                       const int32_t testPatternMode = request->controls().get(
> +                               controls::draft::TestPatternMode);
> +
> +                       LOG(IPU3, Debug) << "Apply test pattern mode: "
> +                                        << testPatternMode;
> +
> +                       int ret = cio2_.sensor()->setTestPatternMode(testPatternMode);
> +                       if (ret) {
> +                               LOG(IPU3, Error) << "Failed to set test pattern mode: "
> +                                                << ret;
> +                       } else {
> +                               request->metadata().set(controls::draft::TestPatternMode,
> +                                                       testPatternMode);
> +                       }

Seeing all this make me a bit concerned that we're making
TestPatternMode a 'special case', when it's just a control on the
CameraSensor class. This code here would have to be duplicated for every
pipeline handler.

Can this be handled inside the CameraSensor class itself, and simply
pass the request (and the cnotrol list, or a filtered control list) into
the CameraSensor?



> +               }
> +       }
> +
> +       /* Controls that don't affect immediately are applied in delayedCtrls. */
> +       delayedCtrls_->applyControls(sequence);
> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
>  
>  } /* namespace libcamera */
> -- 
> 2.33.1.1089.g2158813163f-goog
>