| Message ID | 20210421064847.324118-2-hiroh@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, sorry took me a while to re-read the whole discussion on the previous version and put me in the same perspective as you and Laurent had On Wed, Apr 21, 2021 at 03:48:46PM +0900, Hirokazu Honda wrote: > PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a > request when there are not sufficient buffers for the request. > Since the request will be successful if it is queued later when > enough buffers are available. The requests failed due to a buffer > shortage should be stored and retried later in the FIFO order. > This introduces the queue in IPU3CameraData to do that. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > --- > src/libcamera/pipeline/ipu3/cio2.cpp | 2 +- > src/libcamera/pipeline/ipu3/frames.cpp | 4 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++------- > 3 files changed, 56 insertions(+), 23 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 3cd777d1..8bbef174 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > @@ -272,7 +272,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer) > /* If no buffer is provided in the request, use an internal one. */ > if (!buffer) { > if (availableBuffers_.empty()) { > - LOG(IPU3, Error) << "CIO2 buffer underrun"; > + LOG(IPU3, Debug) << "CIO2 buffer underrun"; > return nullptr; > } > > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp > index a1b014ee..2c4fe508 100644 > --- a/src/libcamera/pipeline/ipu3/frames.cpp > +++ b/src/libcamera/pipeline/ipu3/frames.cpp > @@ -44,12 +44,12 @@ IPU3Frames::Info *IPU3Frames::create(Request *request) > unsigned int id = request->sequence(); > > if (availableParamBuffers_.empty()) { > - LOG(IPU3, Error) << "Parameters buffer underrun"; > + LOG(IPU3, Debug) << "Parameters buffer underrun"; > return nullptr; > } > > if (availableStatBuffers_.empty()) { > - LOG(IPU3, Error) << "Statistics buffer underrun"; > + LOG(IPU3, Debug) << "Statistics buffer underrun"; > return nullptr; > } > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 73306cea..3f311e58 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -66,6 +66,8 @@ public: > void cio2BufferReady(FrameBuffer *buffer); > void paramBufferReady(FrameBuffer *buffer); > void statBufferReady(FrameBuffer *buffer); > + void queuePendingRequests(); > + void cancelPendingRequests(); > > CIO2Device cio2_; > ImgUDevice *imgu_; > @@ -84,6 +86,8 @@ public: > > std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_; > > + std::queue<Request *> pendingRequests_; > + > private: > void queueFrameAction(unsigned int id, > const ipa::ipu3::IPU3Action &action); > @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > IPU3CameraData *data = cameraData(camera); > int ret = 0; > > + data->cancelPendingRequests(); > + > data->ipa_->stop(); > > ret |= data->imgu_->stop(); > @@ -774,33 +780,60 @@ void PipelineHandlerIPU3::stop(Camera *camera) > freeBuffers(camera); > } > > -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) > +void IPU3CameraData::cancelPendingRequests() > { > - IPU3CameraData *data = cameraData(camera); > + while (!pendingRequests_.empty()) { > + Request *request = pendingRequests_.front(); > > - IPU3Frames::Info *info = data->frameInfos_.create(request); > - if (!info) > - return -ENOBUFS; > + for (auto it : request->buffers()) { > + FrameBuffer *buffer = it.second; > + buffer->cancel(); > + pipe_->completeBuffer(request, buffer); > + } > > - /* > - * Queue a buffer on the CIO2, using the raw stream buffer provided in > - * the request, if any, or a CIO2 internal buffer otherwise. > - */ > - FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_); > - FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer); > - if (!rawBuffer) { > - data->frameInfos_.remove(info); > - return -ENOMEM; > + pipe_->completeRequest(request); > + pendingRequests_.pop(); > } > +} > > - info->rawBuffer = rawBuffer; > +void IPU3CameraData::queuePendingRequests() > +{ > + while (!pendingRequests_.empty()) { > + Request *request = pendingRequests_.front(); > > - ipa::ipu3::IPU3Event ev; > - ev.op = ipa::ipu3::EventProcessControls; > - ev.frame = info->id; > - ev.controls = request->controls(); > - data->ipa_->processEvent(ev); > + IPU3Frames::Info *info = frameInfos_.create(request); > + if (!info) > + break; > + > + /* > + * Queue a buffer on the CIO2, using the raw stream buffer > + * provided in the request, if any, or a CIO2 internal buffer > + * otherwise. > + */ > + FrameBuffer *reqRawBuffer = request->findBuffer(&rawStream_); > + FrameBuffer *rawBuffer = cio2_.queueBuffer(request, reqRawBuffer); > + if (!rawBuffer) { > + frameInfos_.remove(info); > + break; > + } > + > + info->rawBuffer = rawBuffer; > > + ipa::ipu3::IPU3Event ev; > + ev.op = ipa::ipu3::EventProcessControls; > + ev.frame = info->id; > + ev.controls = request->controls(); > + ipa_->processEvent(ev); > + > + pendingRequests_.pop(); > + } > +} > + > +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) > +{ > + IPU3CameraData *data = cameraData(camera); > + data->pendingRequests_.push(request); > + data->queuePendingRequests(); I originally wrote this comment: ---------------------------------------------------------- This makes it impossible to return errors to the pipeline handler base class, even if those errors are not due to missing buffers but are reported by the video device, in example at cio2_.queueBuffer() time. I don't think that's desired, as errors would go unnonticed. I would int queueRequestDevice(Request *req) { pending.push(req); while(!pending.empty()) { r = pending.front() info = frameInfo.create(); if (!info) break; pending.pop(); rawB = request->findBuffer(req); raw = cio2.queue(rawB); if (!raw) { frameInfo.remove(info); return -ENOMEM; } info->rawBuffer = raw; ... ipa_->processEvent(ev); } } To maintain the ability to return errors from the video device ------------------------------------------------------------- But then I noticed CIO2::queueBuffer() is pretty idiotic, it behaves differently if the raw buffer is nullptr or not, and to work around this it cannot return an error code to signal an actual queueing error, but it just return nullptr which can mean either "I don't have memory available" or "there's been an error in queueing the buffer to the device". It's impossible to decouple the two error conditions and too much magic is happening in that function which makes it not easy predictable from the caller. So, I would start by splitting the buffer retrieval from the CIO2 from the buffer queueing as in: raw = request->findBuffer(req); if (!raw) raw = cio2->getRawBuffer(); if (!raw) break; ret = cio2->queueBuffer(raw); if (ret) return ret; But as you don't have to solve all the issues you encounter, the patch is fine as it is with a little nit that I find void funcA() { while(something) { code; } } void funcB() { funcA(); } worse to read compared to unfolding the while loop in the caller. > return 0; > } > > -- > 2.31.1.368.gbe11c130af-goog > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, thank you for reviewing. On Tue, May 11, 2021 at 12:57 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > Hi Hiro, > sorry took me a while to re-read the whole discussion on the > previous version and put me in the same perspective as you and > Laurent had > > On Wed, Apr 21, 2021 at 03:48:46PM +0900, Hirokazu Honda wrote: > > PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a > > request when there are not sufficient buffers for the request. > > Since the request will be successful if it is queued later when > > enough buffers are available. The requests failed due to a buffer > > shortage should be stored and retried later in the FIFO order. > > This introduces the queue in IPU3CameraData to do that. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > --- > > src/libcamera/pipeline/ipu3/cio2.cpp | 2 +- > > src/libcamera/pipeline/ipu3/frames.cpp | 4 +- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++------- > > 3 files changed, 56 insertions(+), 23 deletions(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp > b/src/libcamera/pipeline/ipu3/cio2.cpp > > index 3cd777d1..8bbef174 100644 > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > > @@ -272,7 +272,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request > *request, FrameBuffer *rawBuffer) > > /* If no buffer is provided in the request, use an internal one. */ > > if (!buffer) { > > if (availableBuffers_.empty()) { > > - LOG(IPU3, Error) << "CIO2 buffer underrun"; > > + LOG(IPU3, Debug) << "CIO2 buffer underrun"; > > return nullptr; > > } > > > > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp > b/src/libcamera/pipeline/ipu3/frames.cpp > > index a1b014ee..2c4fe508 100644 > > --- a/src/libcamera/pipeline/ipu3/frames.cpp > > +++ b/src/libcamera/pipeline/ipu3/frames.cpp > > @@ -44,12 +44,12 @@ IPU3Frames::Info *IPU3Frames::create(Request > *request) > > unsigned int id = request->sequence(); > > > > if (availableParamBuffers_.empty()) { > > - LOG(IPU3, Error) << "Parameters buffer underrun"; > > + LOG(IPU3, Debug) << "Parameters buffer underrun"; > > return nullptr; > > } > > > > if (availableStatBuffers_.empty()) { > > - LOG(IPU3, Error) << "Statistics buffer underrun"; > > + LOG(IPU3, Debug) << "Statistics buffer underrun"; > > return nullptr; > > } > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp > b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 73306cea..3f311e58 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -66,6 +66,8 @@ public: > > void cio2BufferReady(FrameBuffer *buffer); > > void paramBufferReady(FrameBuffer *buffer); > > void statBufferReady(FrameBuffer *buffer); > > + void queuePendingRequests(); > > + void cancelPendingRequests(); > > > > CIO2Device cio2_; > > ImgUDevice *imgu_; > > @@ -84,6 +86,8 @@ public: > > > > std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_; > > > > + std::queue<Request *> pendingRequests_; > > + > > private: > > void queueFrameAction(unsigned int id, > > const ipa::ipu3::IPU3Action &action); > > @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > IPU3CameraData *data = cameraData(camera); > > int ret = 0; > > > > + data->cancelPendingRequests(); > > + > > data->ipa_->stop(); > > > > ret |= data->imgu_->stop(); > > @@ -774,33 +780,60 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > freeBuffers(camera); > > } > > > > -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request > *request) > > +void IPU3CameraData::cancelPendingRequests() > > { > > - IPU3CameraData *data = cameraData(camera); > > + while (!pendingRequests_.empty()) { > > + Request *request = pendingRequests_.front(); > > > > - IPU3Frames::Info *info = data->frameInfos_.create(request); > > - if (!info) > > - return -ENOBUFS; > > + for (auto it : request->buffers()) { > > + FrameBuffer *buffer = it.second; > > + buffer->cancel(); > > + pipe_->completeBuffer(request, buffer); > > + } > > > > - /* > > - * Queue a buffer on the CIO2, using the raw stream buffer > provided in > > - * the request, if any, or a CIO2 internal buffer otherwise. > > - */ > > - FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_); > > - FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, > reqRawBuffer); > > - if (!rawBuffer) { > > - data->frameInfos_.remove(info); > > - return -ENOMEM; > > + pipe_->completeRequest(request); > > + pendingRequests_.pop(); > > } > > +} > > > > - info->rawBuffer = rawBuffer; > > +void IPU3CameraData::queuePendingRequests() > > +{ > > + while (!pendingRequests_.empty()) { > > + Request *request = pendingRequests_.front(); > > > > - ipa::ipu3::IPU3Event ev; > > - ev.op = ipa::ipu3::EventProcessControls; > > - ev.frame = info->id; > > - ev.controls = request->controls(); > > - data->ipa_->processEvent(ev); > > + IPU3Frames::Info *info = frameInfos_.create(request); > > + if (!info) > > + break; > > + > > + /* > > + * Queue a buffer on the CIO2, using the raw stream buffer > > + * provided in the request, if any, or a CIO2 internal > buffer > > + * otherwise. > > + */ > > + FrameBuffer *reqRawBuffer = > request->findBuffer(&rawStream_); > > + FrameBuffer *rawBuffer = cio2_.queueBuffer(request, > reqRawBuffer); > > + if (!rawBuffer) { > > + frameInfos_.remove(info); > > + break; > > + } > > + > > + info->rawBuffer = rawBuffer; > > > > + ipa::ipu3::IPU3Event ev; > > + ev.op = ipa::ipu3::EventProcessControls; > > + ev.frame = info->id; > > + ev.controls = request->controls(); > > + ipa_->processEvent(ev); > > + > > + pendingRequests_.pop(); > > + } > > +} > > + > > +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request > *request) > > +{ > > + IPU3CameraData *data = cameraData(camera); > > + data->pendingRequests_.push(request); > > + data->queuePendingRequests(); > > I originally wrote this comment: > > ---------------------------------------------------------- > This makes it impossible to return errors to the pipeline handler base > class, even if those errors are not due to missing buffers but are > reported by the video device, in example at cio2_.queueBuffer() time. > I don't think that's desired, as errors would go unnonticed. > > I would > > int queueRequestDevice(Request *req) > { > pending.push(req); > while(!pending.empty()) { > > r = pending.front() > > info = frameInfo.create(); > if (!info) > break; > > pending.pop(); > rawB = request->findBuffer(req); > raw = cio2.queue(rawB); > if (!raw) { > frameInfo.remove(info); > return -ENOMEM; > } > > info->rawBuffer = raw; > > ... > ipa_->processEvent(ev); > } > > } > > > To maintain the ability to return errors from the video device > ------------------------------------------------------------- > > But then I noticed CIO2::queueBuffer() is pretty idiotic, it behaves > differently if the raw buffer is nullptr or not, and to work around > this it cannot return an error code to signal an actual queueing > error, but it just return nullptr which can mean either "I don't have > memory available" or "there's been an error in queueing the buffer to > the device". It's impossible to decouple the two error conditions and > too much magic is happening in that function which makes it not easy > predictable from the caller. > > So, I would start by splitting the buffer retrieval from the CIO2 from > the buffer queueing as in: > > raw = request->findBuffer(req); > if (!raw) > raw = cio2->getRawBuffer(); > > if (!raw) > break; > > ret = cio2->queueBuffer(raw); > if (ret) > return ret; > > But as you don't have to solve all the issues you encounter, the patch > is fine as it is with a little nit that I find > > Thanks for finding. I am also thinking of how to report the error. Since we delay queueing a request, it may be difficult to return the error properly with an associated request with the current solution. I honestly have no idea. > void funcA() > { > while(something) { > code; > } > } > > void funcB() > { > funcA(); > } > > worse to read compared to unfolding the while loop in the caller. > > I think you are saying about queuePendingRequests(). queuePendingRequests() is called in queueRequestDevice() in this patch. But it will be called from other places in 2/2. So the current style is fine for you? -Hiro > > > return 0; > > } > > > > -- > > 2.31.1.368.gbe11c130af-goog > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel >
Hi Hiro, On Tue, May 11, 2021 at 01:18:53PM +0900, Hirokazu Honda wrote: > Hi Jacopo, thank you for reviewing. > > On Tue, May 11, 2021 at 12:57 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > > > Hi Hiro, > > sorry took me a while to re-read the whole discussion on the > > previous version and put me in the same perspective as you and > > Laurent had > > > > On Wed, Apr 21, 2021 at 03:48:46PM +0900, Hirokazu Honda wrote: > > > PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a > > > request when there are not sufficient buffers for the request. > > > Since the request will be successful if it is queued later when > > > enough buffers are available. The requests failed due to a buffer > > > shortage should be stored and retried later in the FIFO order. > > > This introduces the queue in IPU3CameraData to do that. > > > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > --- > > > src/libcamera/pipeline/ipu3/cio2.cpp | 2 +- > > > src/libcamera/pipeline/ipu3/frames.cpp | 4 +- > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++------- > > > 3 files changed, 56 insertions(+), 23 deletions(-) > > > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp > > b/src/libcamera/pipeline/ipu3/cio2.cpp > > > index 3cd777d1..8bbef174 100644 > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > > > @@ -272,7 +272,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request > > *request, FrameBuffer *rawBuffer) > > > /* If no buffer is provided in the request, use an internal one. */ > > > if (!buffer) { > > > if (availableBuffers_.empty()) { > > > - LOG(IPU3, Error) << "CIO2 buffer underrun"; > > > + LOG(IPU3, Debug) << "CIO2 buffer underrun"; > > > return nullptr; > > > } > > > > > > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp > > b/src/libcamera/pipeline/ipu3/frames.cpp > > > index a1b014ee..2c4fe508 100644 > > > --- a/src/libcamera/pipeline/ipu3/frames.cpp > > > +++ b/src/libcamera/pipeline/ipu3/frames.cpp > > > @@ -44,12 +44,12 @@ IPU3Frames::Info *IPU3Frames::create(Request > > *request) > > > unsigned int id = request->sequence(); > > > > > > if (availableParamBuffers_.empty()) { > > > - LOG(IPU3, Error) << "Parameters buffer underrun"; > > > + LOG(IPU3, Debug) << "Parameters buffer underrun"; > > > return nullptr; > > > } > > > > > > if (availableStatBuffers_.empty()) { > > > - LOG(IPU3, Error) << "Statistics buffer underrun"; > > > + LOG(IPU3, Debug) << "Statistics buffer underrun"; > > > return nullptr; > > > } > > > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp > > b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > index 73306cea..3f311e58 100644 > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > @@ -66,6 +66,8 @@ public: > > > void cio2BufferReady(FrameBuffer *buffer); > > > void paramBufferReady(FrameBuffer *buffer); > > > void statBufferReady(FrameBuffer *buffer); > > > + void queuePendingRequests(); > > > + void cancelPendingRequests(); > > > > > > CIO2Device cio2_; > > > ImgUDevice *imgu_; > > > @@ -84,6 +86,8 @@ public: > > > > > > std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_; > > > > > > + std::queue<Request *> pendingRequests_; > > > + > > > private: > > > void queueFrameAction(unsigned int id, > > > const ipa::ipu3::IPU3Action &action); > > > @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > > IPU3CameraData *data = cameraData(camera); > > > int ret = 0; > > > > > > + data->cancelPendingRequests(); > > > + > > > data->ipa_->stop(); > > > > > > ret |= data->imgu_->stop(); > > > @@ -774,33 +780,60 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > > freeBuffers(camera); > > > } > > > > > > -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request > > *request) > > > +void IPU3CameraData::cancelPendingRequests() > > > { > > > - IPU3CameraData *data = cameraData(camera); > > > + while (!pendingRequests_.empty()) { > > > + Request *request = pendingRequests_.front(); > > > > > > - IPU3Frames::Info *info = data->frameInfos_.create(request); > > > - if (!info) > > > - return -ENOBUFS; > > > + for (auto it : request->buffers()) { > > > + FrameBuffer *buffer = it.second; > > > + buffer->cancel(); > > > + pipe_->completeBuffer(request, buffer); > > > + } > > > > > > - /* > > > - * Queue a buffer on the CIO2, using the raw stream buffer > > provided in > > > - * the request, if any, or a CIO2 internal buffer otherwise. > > > - */ > > > - FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_); > > > - FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, > > reqRawBuffer); > > > - if (!rawBuffer) { > > > - data->frameInfos_.remove(info); > > > - return -ENOMEM; > > > + pipe_->completeRequest(request); > > > + pendingRequests_.pop(); > > > } > > > +} > > > > > > - info->rawBuffer = rawBuffer; > > > +void IPU3CameraData::queuePendingRequests() > > > +{ > > > + while (!pendingRequests_.empty()) { > > > + Request *request = pendingRequests_.front(); > > > > > > - ipa::ipu3::IPU3Event ev; > > > - ev.op = ipa::ipu3::EventProcessControls; > > > - ev.frame = info->id; > > > - ev.controls = request->controls(); > > > - data->ipa_->processEvent(ev); > > > + IPU3Frames::Info *info = frameInfos_.create(request); > > > + if (!info) > > > + break; > > > + > > > + /* > > > + * Queue a buffer on the CIO2, using the raw stream buffer > > > + * provided in the request, if any, or a CIO2 internal > > buffer > > > + * otherwise. > > > + */ > > > + FrameBuffer *reqRawBuffer = > > request->findBuffer(&rawStream_); > > > + FrameBuffer *rawBuffer = cio2_.queueBuffer(request, > > reqRawBuffer); > > > + if (!rawBuffer) { > > > + frameInfos_.remove(info); > > > + break; > > > + } > > > + > > > + info->rawBuffer = rawBuffer; > > > > > > + ipa::ipu3::IPU3Event ev; > > > + ev.op = ipa::ipu3::EventProcessControls; > > > + ev.frame = info->id; > > > + ev.controls = request->controls(); > > > + ipa_->processEvent(ev); > > > + > > > + pendingRequests_.pop(); > > > + } > > > +} > > > + > > > +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request > > *request) > > > +{ > > > + IPU3CameraData *data = cameraData(camera); > > > + data->pendingRequests_.push(request); > > > + data->queuePendingRequests(); > > > > I originally wrote this comment: > > > > ---------------------------------------------------------- > > This makes it impossible to return errors to the pipeline handler base > > class, even if those errors are not due to missing buffers but are > > reported by the video device, in example at cio2_.queueBuffer() time. > > I don't think that's desired, as errors would go unnonticed. > > > > I would > > > > int queueRequestDevice(Request *req) > > { > > pending.push(req); > > while(!pending.empty()) { > > > > r = pending.front() > > > > info = frameInfo.create(); > > if (!info) > > break; > > > > pending.pop(); > > rawB = request->findBuffer(req); > > raw = cio2.queue(rawB); > > if (!raw) { > > frameInfo.remove(info); > > return -ENOMEM; > > } > > > > info->rawBuffer = raw; > > > > ... > > ipa_->processEvent(ev); > > } > > > > } > > > > > > To maintain the ability to return errors from the video device > > ------------------------------------------------------------- > > > > But then I noticed CIO2::queueBuffer() is pretty idiotic, it behaves > > differently if the raw buffer is nullptr or not, and to work around > > this it cannot return an error code to signal an actual queueing > > error, but it just return nullptr which can mean either "I don't have > > memory available" or "there's been an error in queueing the buffer to > > the device". It's impossible to decouple the two error conditions and > > too much magic is happening in that function which makes it not easy > > predictable from the caller. > > > > So, I would start by splitting the buffer retrieval from the CIO2 from > > the buffer queueing as in: > > > > raw = request->findBuffer(req); > > if (!raw) > > raw = cio2->getRawBuffer(); > > > > if (!raw) > > break; > > > > ret = cio2->queueBuffer(raw); > > if (ret) > > return ret; > > > > But as you don't have to solve all the issues you encounter, the patch > > is fine as it is with a little nit that I find > > > > > Thanks for finding. > I am also thinking of how to report the error. > Since we delay queueing a request, it may be difficult to return the error > properly with an associated request with the current solution. > I honestly have no idea. Don't you have a Request * to return an error on if queuing failed ? > > > > void funcA() > > { > > while(something) { > > code; > > } > > } > > > > void funcB() > > { > > funcA(); > > } > > > > worse to read compared to unfolding the while loop in the caller. > > > > > I think you are saying about queuePendingRequests(). Yes, I meant that function. > queuePendingRequests() is called in queueRequestDevice() in this patch. > But it will be called from other places in 2/2. Oh sorry, I missed that ! > So the current style is fine for you? Considering 2/2 yes. Please add Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > -Hiro > > > > > > return 0; > > > } > > > > > > -- > > > 2.31.1.368.gbe11c130af-goog > > > > > > _______________________________________________ > > > libcamera-devel mailing list > > > libcamera-devel@lists.libcamera.org > > > https://lists.libcamera.org/listinfo/libcamera-devel > >
Hi again, sorry I rushed a bit in the previous reply On Wed, Apr 21, 2021 at 03:48:46PM +0900, Hirokazu Honda wrote: > PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a > request when there are not sufficient buffers for the request. > Since the request will be successful if it is queued later when > enough buffers are available. The requests failed due to a buffer > shortage should be stored and retried later in the FIFO order. > This introduces the queue in IPU3CameraData to do that. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > --- > src/libcamera/pipeline/ipu3/cio2.cpp | 2 +- > src/libcamera/pipeline/ipu3/frames.cpp | 4 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++------- > 3 files changed, 56 insertions(+), 23 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 3cd777d1..8bbef174 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > @@ -272,7 +272,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer) > /* If no buffer is provided in the request, use an internal one. */ > if (!buffer) { > if (availableBuffers_.empty()) { > - LOG(IPU3, Error) << "CIO2 buffer underrun"; > + LOG(IPU3, Debug) << "CIO2 buffer underrun"; > return nullptr; > } > > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp > index a1b014ee..2c4fe508 100644 > --- a/src/libcamera/pipeline/ipu3/frames.cpp > +++ b/src/libcamera/pipeline/ipu3/frames.cpp > @@ -44,12 +44,12 @@ IPU3Frames::Info *IPU3Frames::create(Request *request) > unsigned int id = request->sequence(); > > if (availableParamBuffers_.empty()) { > - LOG(IPU3, Error) << "Parameters buffer underrun"; > + LOG(IPU3, Debug) << "Parameters buffer underrun"; > return nullptr; > } > > if (availableStatBuffers_.empty()) { > - LOG(IPU3, Error) << "Statistics buffer underrun"; > + LOG(IPU3, Debug) << "Statistics buffer underrun"; > return nullptr; > } > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 73306cea..3f311e58 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -66,6 +66,8 @@ public: > void cio2BufferReady(FrameBuffer *buffer); > void paramBufferReady(FrameBuffer *buffer); > void statBufferReady(FrameBuffer *buffer); > + void queuePendingRequests(); > + void cancelPendingRequests(); > > CIO2Device cio2_; > ImgUDevice *imgu_; > @@ -84,6 +86,8 @@ public: > > std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_; > > + std::queue<Request *> pendingRequests_; > + > private: > void queueFrameAction(unsigned int id, > const ipa::ipu3::IPU3Action &action); > @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > IPU3CameraData *data = cameraData(camera); > int ret = 0; > > + data->cancelPendingRequests(); > + > data->ipa_->stop(); > > ret |= data->imgu_->stop(); > @@ -774,33 +780,60 @@ void PipelineHandlerIPU3::stop(Camera *camera) > freeBuffers(camera); > } > > -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) > +void IPU3CameraData::cancelPendingRequests() > { > - IPU3CameraData *data = cameraData(camera); > + while (!pendingRequests_.empty()) { > + Request *request = pendingRequests_.front(); > > - IPU3Frames::Info *info = data->frameInfos_.create(request); > - if (!info) > - return -ENOBUFS; > + for (auto it : request->buffers()) { > + FrameBuffer *buffer = it.second; > + buffer->cancel(); > + pipe_->completeBuffer(request, buffer); > + } > > - /* > - * Queue a buffer on the CIO2, using the raw stream buffer provided in > - * the request, if any, or a CIO2 internal buffer otherwise. > - */ > - FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_); > - FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer); > - if (!rawBuffer) { > - data->frameInfos_.remove(info); > - return -ENOMEM; > + pipe_->completeRequest(request); > + pendingRequests_.pop(); > } > +} > > - info->rawBuffer = rawBuffer; > +void IPU3CameraData::queuePendingRequests() > +{ > + while (!pendingRequests_.empty()) { > + Request *request = pendingRequests_.front(); > > - ipa::ipu3::IPU3Event ev; > - ev.op = ipa::ipu3::EventProcessControls; > - ev.frame = info->id; > - ev.controls = request->controls(); > - data->ipa_->processEvent(ev); > + IPU3Frames::Info *info = frameInfos_.create(request); > + if (!info) > + break; > + > + /* > + * Queue a buffer on the CIO2, using the raw stream buffer > + * provided in the request, if any, or a CIO2 internal buffer > + * otherwise. > + */ > + FrameBuffer *reqRawBuffer = request->findBuffer(&rawStream_); > + FrameBuffer *rawBuffer = cio2_.queueBuffer(request, reqRawBuffer); > + if (!rawBuffer) { > + frameInfos_.remove(info); > + break; Replying here to the previous question about error signaling. Before this change if the cio2 had no buffer available or had errors when queuing to the device, it reported ENOMEM and the request failed at queueRequestDevice() time. Applications would detect the failure and likely stop. Now if we have no buffer or there's an error on the device, the requests remains in the queue, and it is re-tried later, something that might lead to try over and over the request even after an unrecoverable device error. I'm afraid this requires de-coupling the "no buffer available on CIO2" error from "device queue error on CIO2" one. If there are no buffers, then we can break and try later. If there's an actual device error the request should be completed with errors like you do in cancelPendingRequests() to signal that to applications. However this won't immediately notify to applications that queueRequest() has failed. I think that's fine, however now applications will start receiving sequences of failed requests if the error is not not recoverable and should be prepared to handle the situation. Ideally, we could possibily augment the error states with a "Fatal error on the device" state, that applications can detect and stop immediately from queuing any additional request. As we don't have that, I think for the moment is fine completing the request(s) in error state, but that at least should be signaled to applications... > + } > + > + info->rawBuffer = rawBuffer; > > + ipa::ipu3::IPU3Event ev; > + ev.op = ipa::ipu3::EventProcessControls; > + ev.frame = info->id; > + ev.controls = request->controls(); > + ipa_->processEvent(ev); > + > + pendingRequests_.pop(); > + } > +} > + > +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) > +{ > + IPU3CameraData *data = cameraData(camera); can we add on empty line here > + data->pendingRequests_.push(request); > + data->queuePendingRequests(); and here to give the code some space ? Very minor, can be changed when applying > return 0; > } > > -- > 2.31.1.368.gbe11c130af-goog > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Tue, May 11, 2021 at 7:34 PM Jacopo Mondi <jacopo@jmondi.org> wrote: > Hi again, > sorry I rushed a bit in the previous reply > > On Wed, Apr 21, 2021 at 03:48:46PM +0900, Hirokazu Honda wrote: > > PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a > > request when there are not sufficient buffers for the request. > > Since the request will be successful if it is queued later when > > enough buffers are available. The requests failed due to a buffer > > shortage should be stored and retried later in the FIFO order. > > This introduces the queue in IPU3CameraData to do that. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > --- > > src/libcamera/pipeline/ipu3/cio2.cpp | 2 +- > > src/libcamera/pipeline/ipu3/frames.cpp | 4 +- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++------- > > 3 files changed, 56 insertions(+), 23 deletions(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp > b/src/libcamera/pipeline/ipu3/cio2.cpp > > index 3cd777d1..8bbef174 100644 > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > > @@ -272,7 +272,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request > *request, FrameBuffer *rawBuffer) > > /* If no buffer is provided in the request, use an internal one. */ > > if (!buffer) { > > if (availableBuffers_.empty()) { > > - LOG(IPU3, Error) << "CIO2 buffer underrun"; > > + LOG(IPU3, Debug) << "CIO2 buffer underrun"; > > return nullptr; > > } > > > > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp > b/src/libcamera/pipeline/ipu3/frames.cpp > > index a1b014ee..2c4fe508 100644 > > --- a/src/libcamera/pipeline/ipu3/frames.cpp > > +++ b/src/libcamera/pipeline/ipu3/frames.cpp > > @@ -44,12 +44,12 @@ IPU3Frames::Info *IPU3Frames::create(Request > *request) > > unsigned int id = request->sequence(); > > > > if (availableParamBuffers_.empty()) { > > - LOG(IPU3, Error) << "Parameters buffer underrun"; > > + LOG(IPU3, Debug) << "Parameters buffer underrun"; > > return nullptr; > > } > > > > if (availableStatBuffers_.empty()) { > > - LOG(IPU3, Error) << "Statistics buffer underrun"; > > + LOG(IPU3, Debug) << "Statistics buffer underrun"; > > return nullptr; > > } > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp > b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 73306cea..3f311e58 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -66,6 +66,8 @@ public: > > void cio2BufferReady(FrameBuffer *buffer); > > void paramBufferReady(FrameBuffer *buffer); > > void statBufferReady(FrameBuffer *buffer); > > + void queuePendingRequests(); > > + void cancelPendingRequests(); > > > > CIO2Device cio2_; > > ImgUDevice *imgu_; > > @@ -84,6 +86,8 @@ public: > > > > std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_; > > > > + std::queue<Request *> pendingRequests_; > > + > > private: > > void queueFrameAction(unsigned int id, > > const ipa::ipu3::IPU3Action &action); > > @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > IPU3CameraData *data = cameraData(camera); > > int ret = 0; > > > > + data->cancelPendingRequests(); > > + > > data->ipa_->stop(); > > > > ret |= data->imgu_->stop(); > > @@ -774,33 +780,60 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > freeBuffers(camera); > > } > > > > -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request > *request) > > +void IPU3CameraData::cancelPendingRequests() > > { > > - IPU3CameraData *data = cameraData(camera); > > + while (!pendingRequests_.empty()) { > > + Request *request = pendingRequests_.front(); > > > > - IPU3Frames::Info *info = data->frameInfos_.create(request); > > - if (!info) > > - return -ENOBUFS; > > + for (auto it : request->buffers()) { > > + FrameBuffer *buffer = it.second; > > + buffer->cancel(); > > + pipe_->completeBuffer(request, buffer); > > + } > > > > - /* > > - * Queue a buffer on the CIO2, using the raw stream buffer > provided in > > - * the request, if any, or a CIO2 internal buffer otherwise. > > - */ > > - FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_); > > - FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, > reqRawBuffer); > > - if (!rawBuffer) { > > - data->frameInfos_.remove(info); > > - return -ENOMEM; > > + pipe_->completeRequest(request); > > + pendingRequests_.pop(); > > } > > +} > > > > - info->rawBuffer = rawBuffer; > > +void IPU3CameraData::queuePendingRequests() > > +{ > > + while (!pendingRequests_.empty()) { > > + Request *request = pendingRequests_.front(); > > > > - ipa::ipu3::IPU3Event ev; > > - ev.op = ipa::ipu3::EventProcessControls; > > - ev.frame = info->id; > > - ev.controls = request->controls(); > > - data->ipa_->processEvent(ev); > > + IPU3Frames::Info *info = frameInfos_.create(request); > > + if (!info) > > + break; > > + > > + /* > > + * Queue a buffer on the CIO2, using the raw stream buffer > > + * provided in the request, if any, or a CIO2 internal > buffer > > + * otherwise. > > + */ > > + FrameBuffer *reqRawBuffer = > request->findBuffer(&rawStream_); > > + FrameBuffer *rawBuffer = cio2_.queueBuffer(request, > reqRawBuffer); > > + if (!rawBuffer) { > > + frameInfos_.remove(info); > > + break; > > Replying here to the previous question about error signaling. > > Before this change if the cio2 had no buffer available or had errors > when queuing to the device, it reported ENOMEM and the request failed at > queueRequestDevice() time. Applications would detect the failure and > likely stop. > > Now if we have no buffer or there's an error on the device, the > requests remains in the queue, and it is re-tried later, something > that might lead to try over and over the request even after an > unrecoverable device error. > > I'm afraid this requires de-coupling the "no buffer available on CIO2" > error from "device queue error on CIO2" one. > > If there are no buffers, then we can break and try later. > > If there's an actual device error the request should be completed with > errors > like you do in cancelPendingRequests() to signal that to applications. > However this won't immediately notify to applications that > queueRequest() has failed. I think that's fine, however now > applications will start receiving sequences of failed requests if the > error is not not recoverable and should be prepared to handle the > situation. Ideally, we could possibily augment the error states with a > "Fatal error on the device" state, that applications can detect and > stop immediately from queuing any additional request. As we don't have > that, I think for the moment is fine completing the request(s) in > error state, but that at least should be signaled to applications... > > I agree. Yeah, as you said, CIO2Device::queueBuffer() doesn't return a error code. :( We need to change the function, I am going to do it in a follow-up patch with leaving todo comment in this patch series. Is it okay for you? -Hiro > > + } > > + > > + info->rawBuffer = rawBuffer; > > > > + ipa::ipu3::IPU3Event ev; > > + ev.op = ipa::ipu3::EventProcessControls; > > + ev.frame = info->id; > > + ev.controls = request->controls(); > > + ipa_->processEvent(ev); > > + > > + pendingRequests_.pop(); > > + } > > +} > > + > > +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request > *request) > > +{ > > + IPU3CameraData *data = cameraData(camera); > > can we add on empty line here > > > + data->pendingRequests_.push(request); > > + data->queuePendingRequests(); > > and here to give the code some space ? > > Very minor, can be changed when applying > > > > > return 0; > > } > > > > -- > > 2.31.1.368.gbe11c130af-goog > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel >
diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 3cd777d1..8bbef174 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -272,7 +272,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer) /* If no buffer is provided in the request, use an internal one. */ if (!buffer) { if (availableBuffers_.empty()) { - LOG(IPU3, Error) << "CIO2 buffer underrun"; + LOG(IPU3, Debug) << "CIO2 buffer underrun"; return nullptr; } diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp index a1b014ee..2c4fe508 100644 --- a/src/libcamera/pipeline/ipu3/frames.cpp +++ b/src/libcamera/pipeline/ipu3/frames.cpp @@ -44,12 +44,12 @@ IPU3Frames::Info *IPU3Frames::create(Request *request) unsigned int id = request->sequence(); if (availableParamBuffers_.empty()) { - LOG(IPU3, Error) << "Parameters buffer underrun"; + LOG(IPU3, Debug) << "Parameters buffer underrun"; return nullptr; } if (availableStatBuffers_.empty()) { - LOG(IPU3, Error) << "Statistics buffer underrun"; + LOG(IPU3, Debug) << "Statistics buffer underrun"; return nullptr; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 73306cea..3f311e58 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -66,6 +66,8 @@ public: void cio2BufferReady(FrameBuffer *buffer); void paramBufferReady(FrameBuffer *buffer); void statBufferReady(FrameBuffer *buffer); + void queuePendingRequests(); + void cancelPendingRequests(); CIO2Device cio2_; ImgUDevice *imgu_; @@ -84,6 +86,8 @@ public: std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_; + std::queue<Request *> pendingRequests_; + private: void queueFrameAction(unsigned int id, const ipa::ipu3::IPU3Action &action); @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) IPU3CameraData *data = cameraData(camera); int ret = 0; + data->cancelPendingRequests(); + data->ipa_->stop(); ret |= data->imgu_->stop(); @@ -774,33 +780,60 @@ void PipelineHandlerIPU3::stop(Camera *camera) freeBuffers(camera); } -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) +void IPU3CameraData::cancelPendingRequests() { - IPU3CameraData *data = cameraData(camera); + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); - IPU3Frames::Info *info = data->frameInfos_.create(request); - if (!info) - return -ENOBUFS; + for (auto it : request->buffers()) { + FrameBuffer *buffer = it.second; + buffer->cancel(); + pipe_->completeBuffer(request, buffer); + } - /* - * Queue a buffer on the CIO2, using the raw stream buffer provided in - * the request, if any, or a CIO2 internal buffer otherwise. - */ - FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_); - FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer); - if (!rawBuffer) { - data->frameInfos_.remove(info); - return -ENOMEM; + pipe_->completeRequest(request); + pendingRequests_.pop(); } +} - info->rawBuffer = rawBuffer; +void IPU3CameraData::queuePendingRequests() +{ + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); - ipa::ipu3::IPU3Event ev; - ev.op = ipa::ipu3::EventProcessControls; - ev.frame = info->id; - ev.controls = request->controls(); - data->ipa_->processEvent(ev); + IPU3Frames::Info *info = frameInfos_.create(request); + if (!info) + break; + + /* + * Queue a buffer on the CIO2, using the raw stream buffer + * provided in the request, if any, or a CIO2 internal buffer + * otherwise. + */ + FrameBuffer *reqRawBuffer = request->findBuffer(&rawStream_); + FrameBuffer *rawBuffer = cio2_.queueBuffer(request, reqRawBuffer); + if (!rawBuffer) { + frameInfos_.remove(info); + break; + } + + info->rawBuffer = rawBuffer; + ipa::ipu3::IPU3Event ev; + ev.op = ipa::ipu3::EventProcessControls; + ev.frame = info->id; + ev.controls = request->controls(); + ipa_->processEvent(ev); + + pendingRequests_.pop(); + } +} + +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) +{ + IPU3CameraData *data = cameraData(camera); + data->pendingRequests_.push(request); + data->queuePendingRequests(); return 0; }
PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a request when there are not sufficient buffers for the request. Since the request will be successful if it is queued later when enough buffers are available. The requests failed due to a buffer shortage should be stored and retried later in the FIFO order. This introduces the queue in IPU3CameraData to do that. Signed-off-by: Hirokazu Honda <hiroh@chromium.org> --- src/libcamera/pipeline/ipu3/cio2.cpp | 2 +- src/libcamera/pipeline/ipu3/frames.cpp | 4 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++------- 3 files changed, 56 insertions(+), 23 deletions(-)