[libcamera-devel] [PATCH 11/11] libcamera: ipu3: Share parameter and statistic buffers with IPA
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Dec 29 15:53:09 CET 2020
Hi Jacopo,
Thanks for your comments.
On 2020-11-18 18:17:24 +0100, Jacopo Mondi wrote:
> Hi Niklas,
>
> On Thu, Nov 05, 2020 at 01:15:46AM +0100, Niklas Söderlund wrote:
> > Use the IPU3Frames helper to share parameter and statistic buffers with
> > the IPA. Track which parameter and statistic buffer is used for witch
>
> parameters and statistics ?
> s/witch/which/
>
> > request and make sure the parameter buffers is filled in by the IPA
> > before it's needed and that the statistic buffer is consumed and meta
> > data generated before completing the request.
> >
> > With this change the IPU3 pipeline is prepared to fully operate with an
> > IPA component.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 106 +++++++++++++++++++++++++--
> > 1 file changed, 100 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index d161c2e68c0db0f2..58f8feaba4e87c54 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -28,6 +28,7 @@
> > #include "libcamera/internal/v4l2_controls.h"
> >
> > #include "cio2.h"
> > +#include "frames.h"
> > #include "imgu.h"
> >
> > namespace libcamera {
> > @@ -59,6 +60,8 @@ public:
> >
> > void imguOutputBufferReady(FrameBuffer *buffer);
> > void cio2BufferReady(FrameBuffer *buffer);
> > + void paramBufferReady(FrameBuffer *buffer);
> > + void statBufferReady(FrameBuffer *buffer);
> >
> > CIO2Device cio2_;
> > ImgUDevice *imgu_;
> > @@ -68,6 +71,7 @@ public:
> > Stream rawStream_;
> >
> > DelayedControls *delayedCtrls_;
> > + std::unique_ptr<IPU3Frames> frameInfo_;
> >
> > private:
> > void actOnIpa(unsigned int id, const IPAOperationData &op);
> > @@ -582,6 +586,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
> > if (ret < 0)
> > return ret;
> >
> > + data->frameInfo_->mapBuffers(imgu->paramBuffers_, imgu->statBuffers_);
> > +
> > return 0;
> > }
> >
> > @@ -589,6 +595,8 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)
> > {
> > IPU3CameraData *data = cameraData(camera);
> >
> > + data->frameInfo_->unmapBuffers();
> > +
> > data->imgu_->freeBuffers();
> >
> > return 0;
> > @@ -684,6 +692,10 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
> > IPU3CameraData *data = cameraData(camera);
> > int error = 0;
> >
> > + IPU3Frames::Info *info = data->frameInfo_->create(request);
> > + if (!info)
> > + return -ENOENT;
> > +
> > /*
> > * Queue a buffer on the CIO2, using the raw stream buffer provided in
> > * the request, if any, or a CIO2 internal buffer otherwise.
> > @@ -693,7 +705,10 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
> > if (!rawBuffer)
> > return -ENOMEM;
> >
> > + info->rawBuffer = rawBuffer;
> > +
> > /* Queue all buffers from the request aimed for the ImgU. */
> > + bool onlyRaw = true;
> > for (auto it : request->buffers()) {
> > const Stream *stream = it.first;
> > FrameBuffer *buffer = it.second;
> > @@ -708,6 +723,23 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
> >
> > if (ret < 0)
> > error = ret;
> > +
> > + onlyRaw = false;
> > + }
> > +
> > + /* If request only contains a raw buffer do not involve IPA. */
> > + if (onlyRaw) {
> > + info->paramDequeued = true;
> > + info->metadataProcessed = true;
> > + } else {
> > + IPAOperationData op;
> > + op.operation = IPU3_IPA_EVENT_FILL_PARAMS;
> > + op.data = { info->id, info->paramBuffer->cookie() };
> > + op.controls = { request->controls() };
> > + data->ipa_->processEvent(op);
> > +
> > + data->imgu_->param_->queueBuffer(info->paramBuffer);
> > + data->imgu_->stat_->queueBuffer(info->statBuffer);
> > }
> >
> > return error;
> > @@ -845,6 +877,10 @@ int PipelineHandlerIPU3::registerCameras()
> > &IPU3CameraData::imguOutputBufferReady);
> > data->imgu_->viewfinder_->bufferReady.connect(data.get(),
> > &IPU3CameraData::imguOutputBufferReady);
> > + data->imgu_->param_->bufferReady.connect(data.get(),
> > + &IPU3CameraData::paramBufferReady);
> > + data->imgu_->stat_->bufferReady.connect(data.get(),
> > + &IPU3CameraData::statBufferReady);
> >
> > /* Create and register the Camera instance. */
> > std::string cameraId = cio2->sensor()->id();
> > @@ -874,10 +910,12 @@ int IPU3CameraData::loadIPA()
> >
> > ipa_->init(IPASettings{});
> >
> > + frameInfo_ = std::make_unique<IPU3Frames>(pipe_, ipa_.get());
>
> nit: this a a collection of frame info, I would use a plural name like
> framesInfo_
>
> > +
> > return 0;
> > }
> >
> > -void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,
> > +void IPU3CameraData::actOnIpa(unsigned int id,
> > const IPAOperationData &action)
> > {
> > switch (action.operation) {
> > @@ -886,6 +924,24 @@ void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,
> > delayedCtrls_->push(controls);
> > break;
> > }
> > + case IPU3_IPA_ACTION_PARAM_FILLED: {
> > + IPU3Frames::Info *info = frameInfo_->find(id);
> > + if (!info)
> > + break;
> > +
> > + info->paramFilled = true;
> > + break;
> > + }
> > + case IPU3_IPA_ACTION_METADATA_READY: {
> > + IPU3Frames::Info *info = frameInfo_->find(id);
> > + if (!info)
> > + break;
> > +
> > + info->request->metadata() = action.controls[0];
> > + info->metadataProcessed = true;
> > + frameInfo_->tryComplete(info);
> > + break;
> > + }
> > default:
> > LOG(IPU3, Error) << "Unknown action " << action.operation;
> > break;
> > @@ -906,13 +962,15 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
> > {
> > Request *request = buffer->request();
> >
> > - if (!pipe_->completeBuffer(request, buffer))
> > - /* Request not completed yet, return here. */
>
> I would not drop the early return, if the request has pending buffers
> the below frameInfo_->tryComplete() will fail too.
If we do that the pipeline depth won't be set.
>
> > + pipe_->completeBuffer(request, buffer);
> > +
> > + IPU3Frames::Info *info = frameInfo_->find(buffer);
> > + if (!info)
> > return;
> >
> > - /* Mark the request as complete. */
> > request->metadata().set(controls::draft::PipelineDepth, 3);
>
> If we complete in a different call to frameInfo_->tryCompelte() this
> metadata won't be set I guess.
Correct, it's set to 2 when the CIO2 buffer completes and is then
updated to 3 when the IMGU buffer completes. This way we deal correctly
with both Requests that contains only RAW buffers and once that are
processed by the IMGU.
>
> > - pipe_->completeRequest(request);
> > +
> > + frameInfo_->tryComplete(info);
> > }
> >
> > /**
> > @@ -928,6 +986,10 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> > if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> > return;
> >
> > + IPU3Frames::Info *info = frameInfo_->find(buffer);
> > + if (!info)
> > + return;
> > +
> > Request *request = buffer->request();
> >
> > /*
> > @@ -938,14 +1000,46 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> > bool isComplete = pipe_->completeBuffer(request, buffer);
> > if (isComplete) {
>
> This might be a good occasion to drop 'isComplete' and replace it with
> if (pipe_->completeBuffer(request, buffer)
Good point.
>
> > request->metadata().set(controls::draft::PipelineDepth, 2);
> > - pipe_->completeRequest(request);
> > + frameInfo_->tryComplete(info);
> > return;
> > }
> > }
> >
> > + if (!info->paramFilled)
> > + LOG(IPU3, Info)
> > + << "Parameters not ready on time for id " << info->id;
> > +
>
> Oh, now I see what paramFilled is for, but if we queue a onlyRaw
> request, we never trigger the IPA's FILL_PARAMS operation and will
> never receive a PARAMS_FILLED back, so this will always be false ?
Correct. But in the onlyRaw case isComplete will be true and we will
never reach this check.
>
> Recording if the request was onlyRaw might help ? In this case you can
> check both here and in the frame helper
> if (!onlyRaw && !info->paramFilled)
> dropping paramDequeued completely ?
We could turn paramFilled flag into a onlyRaw flag but I don't see how
that is much different.
>
>
>
> > imgu_->input_->queueBuffer(buffer);
> > }
> >
> > +void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
> > +{
> > + if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> > + return;
> > +
> > + IPU3Frames::Info *info = frameInfo_->find(buffer);
> > + if (!info)
> > + return;
> > +
> > + info->paramDequeued = true;
> > + frameInfo_->tryComplete(info);
> > +}
> > +
> > +void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
> > +{
> > + if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> > + return;
> > +
> > + IPU3Frames::Info *info = frameInfo_->find(buffer);
> > + if (!info)
> > + return;
> > +
> > + IPAOperationData op;
> > + op.operation = IPU3_IPA_EVENT_PARSE_STAT;
> > + op.data = { info->id, info->statBuffer->cookie() };
> > + ipa_->processEvent(op);
> > +}
> > +
> > REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
> >
> > } /* namespace libcamera */
> > --
> > 2.29.2
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list