| Message ID | 20260325-mali-cru-v6-6-b16b0c49819a@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta: > From: Daniel Scally <dan.scally@ideasonboard.com> > > Plumb in the MaliC55 pipeline handler support for capturing frames > from memory using the CRU. > > Introduce a data flow which uses the CRU to feed the ISP through > the IVC. > > In detail: > > - push incoming request to a pending queue until a buffer from the CRU > is available > - delay the call to ipa_->fillParams() to the CRU buffer ready even event ? > - once the IPA has computed parameters feed the ISP through the IVC > with buffers from the CRU, params and statistics. > - Handle completion of in-flight requests: in m2m mode buffers are > queued earlier to the ISP capture devices. If the camera is stopped > these buffers are returned earlier in error state: complete the > Request bypassing the IPA operations. > - As cancelled Requests might now be completed earlier then IPA events, > modify the IPA event handler to ignore Requests that have been > completed already > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++---- > 1 file changed, 207 insertions(+), 37 deletions(-) > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > index be3e38da95ab..10848c412849 100644 > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > @@ -21,6 +21,7 @@ > #include <libcamera/base/utils.h> > > #include <libcamera/camera.h> > +#include <libcamera/controls.h> > #include <libcamera/formats.h> > #include <libcamera/geometry.h> > #include <libcamera/property_ids.h> > @@ -88,6 +89,7 @@ struct MaliC55FrameInfo { > > FrameBuffer *paramBuffer; > FrameBuffer *statBuffer; > + FrameBuffer *rawBuffer; > > bool paramsDone; > bool statsDone; > @@ -691,6 +693,7 @@ public: > void imageBufferReady(FrameBuffer *buffer); > void paramsBufferReady(FrameBuffer *buffer); > void statsBufferReady(FrameBuffer *buffer); > + void cruBufferReady(FrameBuffer *buffer); > void paramsComputed(unsigned int requestId, uint32_t bytesused); > void statsProcessed(unsigned int requestId, const ControlList &metadata); > > @@ -739,7 +742,7 @@ private: > > MaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer); > MaliC55FrameInfo *findFrameInfo(Request *request); > - void tryComplete(MaliC55FrameInfo *info); > + void tryComplete(MaliC55FrameInfo *info, bool cancelled = false); > > int configureRawStream(MaliC55CameraData *data, > const StreamConfiguration &config, > @@ -747,6 +750,11 @@ private: > int configureProcessedStream(MaliC55CameraData *data, > const StreamConfiguration &config, > V4L2SubdeviceFormat &subdevFormat); > + void cancelPendingRequests(); > + > + MaliC55FrameInfo * > + prepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr); > + int queuePendingRequests(MaliC55CameraData *data); > > void applyScalerCrop(Camera *camera, const ControlList &controls); > > @@ -772,6 +780,9 @@ private: > > std::map<unsigned int, MaliC55FrameInfo> frameInfoMap_; > > + /* Requests for which no buffer has been queued to the CRU device yet. */ > + std::queue<Request *> pendingRequests_; > + > std::array<MaliC55Pipe, MaliC55NumPipes> pipes_; > > bool dsFitted_; > @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera) > if (params_->releaseBuffers()) > LOG(MaliC55, Error) << "Failed to release params buffers"; > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > + if (ivc_->releaseBuffers()) > + LOG(MaliC55, Error) << "Failed to release input buffers"; > + if (mem->cru_->freeBuffers()) > + LOG(MaliC55, Error) << "Failed to release CRU buffers"; > + } > + > return; > } > > @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) > } > }; > > + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { > + ret = ivc_->importBuffers(RZG2LCRU::kBufferCount); > + if (ret < 0) > + return ret; > + } > + > ret = stats_->allocateBuffers(bufferCount, &statsBuffers_); > if (ret < 0) > return ret; > @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control > if (ret) > return ret; > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > + ret = mem->cru_->start(); > + if (ret) { > + LOG(MaliC55, Error) > + << "Failed to start CRU " << camera->id(); > + freeBuffers(camera); Minor thing, but I think reworking these with `utils::scope_exit` is a worthwhile change if you plan to send a new version. > + return ret; > + } > + > + ret = ivc_->streamOn(); > + if (ret) { > + LOG(MaliC55, Error) > + << "Failed to start IVC" << camera->id(); > + freeBuffers(camera); > + return ret; > + } > + } > + > if (data->ipa_) { > ret = data->ipa_->start(); > if (ret) { > @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control > return 0; > } > > +void PipelineHandlerMaliC55::cancelPendingRequests() > +{ > + while (!pendingRequests_.empty()) { > + cancelRequest(pendingRequests_.front()); > + pendingRequests_.pop(); > + } > +} > + > void PipelineHandlerMaliC55::stopDevice(Camera *camera) > { > MaliC55CameraData *data = cameraData(camera); > > isp_->setFrameStartEnabled(false); > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > + ivc_->streamOff(); > + mem->cru_->stop(); > + } > + > for (MaliC55Pipe &pipe : pipes_) { > if (!pipe.stream) > continue; > @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera) > if (data->ipa_) > data->ipa_->stop(); > freeBuffers(camera); > + > + cancelPendingRequests(); > } > > void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, > @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, > } > } > > +MaliC55FrameInfo * > +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru) > +{ > + if (availableStatsBuffers_.empty()) { > + LOG(MaliC55, Error) << "Stats buffer underrun"; > + return nullptr; > + } > + > + if (availableParamsBuffers_.empty()) { > + LOG(MaliC55, Error) << "Params buffer underrun"; > + return nullptr; > + } > + > + MaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()]; I think it would be better to do it last, not to leave an "empty" frame info object in the map. > + frameInfo.request = request; > + > + if (cru) { > + frameInfo.rawBuffer = cru->queueBuffer(request); > + if (!frameInfo.rawBuffer) I'm wondering, can it not be guaranteed that there is always an available buffer? Wouldn't it be sufficient to ensure that at most 4 requests are queued to the pipeline handler? In that case I believe there should always be an available cru buffer. > + return nullptr; > + } > + > + frameInfo.statBuffer = availableStatsBuffers_.front(); > + availableStatsBuffers_.pop(); > + frameInfo.paramBuffer = availableParamsBuffers_.front(); > + availableParamsBuffers_.pop(); > + > + frameInfo.paramsDone = false; > + frameInfo.statsDone = false; > + > + return &frameInfo; > +} > + > +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data) Should this not be called when a CRU buffer becomes available? If the queue is only drained if a new request is queued, then I feel like things can get stuck if the application is stopping and is waiting for the pending requests to complete. I suppose it's fine now because everything uses hard-coded 4 buffers / requests. > +{ > + auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_); > + ASSERT(mem); > + > + while (!pendingRequests_.empty()) { > + Request *request = pendingRequests_.front(); > + > + if (!prepareFrameInfo(request, mem->cru_.get())) > + return -ENOENT; > + > + for (auto &[stream, buffer] : request->buffers()) { > + MaliC55Pipe *pipe = pipeFromStream(data, stream); > + > + pipe->cap->queueBuffer(buffer); > + } > + > + data->ipa_->queueRequest(request->sequence(), request->controls()); > + > + pendingRequests_.pop(); > + } > + > + return 0; > +} > + > int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) > { > MaliC55CameraData *data = cameraData(camera); > > + /* > + * If we're in memory input mode, we need to pop the requests onto the push ? > + * pending list until a CRU buffer is ready...otherwise we can just do > + * everything immediately. > + */ Or maybe I was wrong above. But this says "pending list until a CRU buffer is ready", so if no CRU buffer is available, shouldn't it just push to `pendingRequests_` and return 0? Because this now returns non-zero, and that will cancel the request. > + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { > + pendingRequests_.push(request); > + > + int ret = queuePendingRequests(data); > + if (ret) { > + pendingRequests_.pop(); > + return ret; > + } > + > + return 0; > + } > + > /* Do not run the IPA if the TPG is in use. */ > if (!data->ipa_) { > MaliC55FrameInfo frameInfo; > @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) > return 0; > } > > - if (availableStatsBuffers_.empty()) { > - LOG(MaliC55, Error) << "Stats buffer underrun"; > - return -ENOENT; > - } > - > - if (availableParamsBuffers_.empty()) { > - LOG(MaliC55, Error) << "Params buffer underrun"; > + auto frameInfo = prepareFrameInfo(request); > + if (!frameInfo) > return -ENOENT; > - } > - > - MaliC55FrameInfo frameInfo; > - frameInfo.request = request; > - > - frameInfo.statBuffer = availableStatsBuffers_.front(); > - availableStatsBuffers_.pop(); > - frameInfo.paramBuffer = availableParamsBuffers_.front(); > - availableParamsBuffers_.pop(); > - > - frameInfo.paramsDone = false; > - frameInfo.statsDone = false; > - > - frameInfoMap_[request->sequence()] = frameInfo; > > data->ipa_->queueRequest(request->sequence(), request->controls()); > data->ipa_->fillParams(request->sequence(), > - frameInfo.paramBuffer->cookie()); > + frameInfo->paramBuffer->cookie()); > > return 0; > } > [...]
Hi Barnabás On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote: > Hi > > 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta: > > From: Daniel Scally <dan.scally@ideasonboard.com> > > > > Plumb in the MaliC55 pipeline handler support for capturing frames > > from memory using the CRU. > > > > Introduce a data flow which uses the CRU to feed the ISP through > > the IVC. > > > > In detail: > > > > - push incoming request to a pending queue until a buffer from the CRU > > is available > > - delay the call to ipa_->fillParams() to the CRU buffer ready even > > event ? > Ah yes > > > - once the IPA has computed parameters feed the ISP through the IVC > > with buffers from the CRU, params and statistics. > > - Handle completion of in-flight requests: in m2m mode buffers are > > queued earlier to the ISP capture devices. If the camera is stopped > > these buffers are returned earlier in error state: complete the > > Request bypassing the IPA operations. > > - As cancelled Requests might now be completed earlier then IPA events, > > modify the IPA event handler to ignore Requests that have been > > completed already > > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++---- > > 1 file changed, 207 insertions(+), 37 deletions(-) > > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > index be3e38da95ab..10848c412849 100644 > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > @@ -21,6 +21,7 @@ > > #include <libcamera/base/utils.h> > > #include <libcamera/camera.h> > > +#include <libcamera/controls.h> > > #include <libcamera/formats.h> > > #include <libcamera/geometry.h> > > #include <libcamera/property_ids.h> > > @@ -88,6 +89,7 @@ struct MaliC55FrameInfo { > > FrameBuffer *paramBuffer; > > FrameBuffer *statBuffer; > > + FrameBuffer *rawBuffer; > > bool paramsDone; > > bool statsDone; > > @@ -691,6 +693,7 @@ public: > > void imageBufferReady(FrameBuffer *buffer); > > void paramsBufferReady(FrameBuffer *buffer); > > void statsBufferReady(FrameBuffer *buffer); > > + void cruBufferReady(FrameBuffer *buffer); > > void paramsComputed(unsigned int requestId, uint32_t bytesused); > > void statsProcessed(unsigned int requestId, const ControlList &metadata); > > @@ -739,7 +742,7 @@ private: > > MaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer); > > MaliC55FrameInfo *findFrameInfo(Request *request); > > - void tryComplete(MaliC55FrameInfo *info); > > + void tryComplete(MaliC55FrameInfo *info, bool cancelled = false); > > int configureRawStream(MaliC55CameraData *data, > > const StreamConfiguration &config, > > @@ -747,6 +750,11 @@ private: > > int configureProcessedStream(MaliC55CameraData *data, > > const StreamConfiguration &config, > > V4L2SubdeviceFormat &subdevFormat); > > + void cancelPendingRequests(); > > + > > + MaliC55FrameInfo * > > + prepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr); > > + int queuePendingRequests(MaliC55CameraData *data); > > void applyScalerCrop(Camera *camera, const ControlList &controls); > > @@ -772,6 +780,9 @@ private: > > std::map<unsigned int, MaliC55FrameInfo> frameInfoMap_; > > + /* Requests for which no buffer has been queued to the CRU device yet. */ > > + std::queue<Request *> pendingRequests_; > > + > > std::array<MaliC55Pipe, MaliC55NumPipes> pipes_; > > bool dsFitted_; > > @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera) > > if (params_->releaseBuffers()) > > LOG(MaliC55, Error) << "Failed to release params buffers"; > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > > + if (ivc_->releaseBuffers()) > > + LOG(MaliC55, Error) << "Failed to release input buffers"; > > + if (mem->cru_->freeBuffers()) > > + LOG(MaliC55, Error) << "Failed to release CRU buffers"; > > + } > > + > > return; > > } > > @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) > > } > > }; > > + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { > > + ret = ivc_->importBuffers(RZG2LCRU::kBufferCount); > > + if (ret < 0) > > + return ret; > > + } > > + > > ret = stats_->allocateBuffers(bufferCount, &statsBuffers_); > > if (ret < 0) > > return ret; > > @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control > > if (ret) > > return ret; > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > > + ret = mem->cru_->start(); > > + if (ret) { > > + LOG(MaliC55, Error) > > + << "Failed to start CRU " << camera->id(); > > + freeBuffers(camera); > > Minor thing, but I think reworking these with `utils::scope_exit` is a worthwhile change > if you plan to send a new version. > Can we consider doing this on top as the existing error paths would need to be reworked otherwise ? > > > + return ret; > > + } > > + > > + ret = ivc_->streamOn(); > > + if (ret) { > > + LOG(MaliC55, Error) > > + << "Failed to start IVC" << camera->id(); > > + freeBuffers(camera); > > + return ret; > > + } > > + } > > + > > if (data->ipa_) { > > ret = data->ipa_->start(); > > if (ret) { > > @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control > > return 0; > > } > > +void PipelineHandlerMaliC55::cancelPendingRequests() > > +{ > > + while (!pendingRequests_.empty()) { > > + cancelRequest(pendingRequests_.front()); > > + pendingRequests_.pop(); > > + } > > +} > > + > > void PipelineHandlerMaliC55::stopDevice(Camera *camera) > > { > > MaliC55CameraData *data = cameraData(camera); > > isp_->setFrameStartEnabled(false); > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > > + ivc_->streamOff(); > > + mem->cru_->stop(); > > + } > > + > > for (MaliC55Pipe &pipe : pipes_) { > > if (!pipe.stream) > > continue; > > @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera) > > if (data->ipa_) > > data->ipa_->stop(); > > freeBuffers(camera); > > + > > + cancelPendingRequests(); > > } > > void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, > > @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, > > } > > } > > +MaliC55FrameInfo * > > +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru) > > +{ > > + if (availableStatsBuffers_.empty()) { > > + LOG(MaliC55, Error) << "Stats buffer underrun"; > > + return nullptr; > > + } > > + > > + if (availableParamsBuffers_.empty()) { > > + LOG(MaliC55, Error) << "Params buffer underrun"; > > + return nullptr; > > + } > > + > > + MaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()]; > > I think it would be better to do it last, not to leave an "empty" frame info > object in the map. As the only failure point is the below if (!frameInfo.rawBuffer) can I do after that ? > > > > + frameInfo.request = request; > > + > > + if (cru) { > > + frameInfo.rawBuffer = cru->queueBuffer(request); > > + if (!frameInfo.rawBuffer) > > I'm wondering, can it not be guaranteed that there is always an available buffer? > Wouldn't it be sufficient to ensure that at most 4 requests are queued to the > pipeline handler? In that case I believe there should always be an available cru buffer. Should I initialize PipelineHandler() with maxQueuedRequestsDevice == kBufferCount ? > > > > + return nullptr; > > + } > > + > > + frameInfo.statBuffer = availableStatsBuffers_.front(); > > + availableStatsBuffers_.pop(); > > + frameInfo.paramBuffer = availableParamsBuffers_.front(); > > + availableParamsBuffers_.pop(); > > + > > + frameInfo.paramsDone = false; > > + frameInfo.statsDone = false; > > + > > + return &frameInfo; > > +} > > + > > +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data) > > Should this not be called when a CRU buffer becomes available? If the queue is > only drained if a new request is queued, then I feel like things can get stuck > if the application is stopping and is waiting for the pending requests to complete. > I suppose it's fine now because everything uses hard-coded 4 buffers / requests. > mmm, you're right. We only try to run when the application queue a new Request. Should I create an handler for ivc_->bufferReady (which now simply returns the buffer to the cru) and call PipelineHandlerMaliC55::queuePendingRequests() from there ? > > > +{ > > + auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_); > > + ASSERT(mem); > > + > > + while (!pendingRequests_.empty()) { > > + Request *request = pendingRequests_.front(); > > + > > + if (!prepareFrameInfo(request, mem->cru_.get())) > > + return -ENOENT; > > + > > + for (auto &[stream, buffer] : request->buffers()) { > > + MaliC55Pipe *pipe = pipeFromStream(data, stream); > > + > > + pipe->cap->queueBuffer(buffer); > > + } > > + > > + data->ipa_->queueRequest(request->sequence(), request->controls()); > > + > > + pendingRequests_.pop(); > > + } > > + > > + return 0; > > +} > > + > > int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) > > { > > MaliC55CameraData *data = cameraData(camera); > > + /* > > + * If we're in memory input mode, we need to pop the requests onto the > > push ? ups > > > > + * pending list until a CRU buffer is ready...otherwise we can just do > > + * everything immediately. > > + */ > > Or maybe I was wrong above. But this says "pending list until a CRU buffer is ready", > so if no CRU buffer is available, shouldn't it just push to `pendingRequests_` and > return 0? Because this now returns non-zero, and that will cancel the request. > I actually think the below error path handling is wrong now that I look at it more closely > > > + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { > > + pendingRequests_.push(request); > > + > > + int ret = queuePendingRequests(data); > > + if (ret) { > > + pendingRequests_.pop(); Requests are popped from the queue in queuePendingRequests() only at the end of the loop, so if we error out, there isn't any need to actually pop them here > > + return ret; And, as you suggested, if we return an error the request is cancelled, so we should probably return 0 here, leave the Request in the queue and let the next queuePendingRequests() consume it ? > > + } > > + > > + return 0; > > + } > > + > > /* Do not run the IPA if the TPG is in use. */ > > if (!data->ipa_) { > > MaliC55FrameInfo frameInfo; > > @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) > > return 0; > > } > > - if (availableStatsBuffers_.empty()) { > > - LOG(MaliC55, Error) << "Stats buffer underrun"; > > - return -ENOENT; > > - } > > - > > - if (availableParamsBuffers_.empty()) { > > - LOG(MaliC55, Error) << "Params buffer underrun"; > > + auto frameInfo = prepareFrameInfo(request); > > + if (!frameInfo) > > return -ENOENT; > > - } > > - > > - MaliC55FrameInfo frameInfo; > > - frameInfo.request = request; > > - > > - frameInfo.statBuffer = availableStatsBuffers_.front(); > > - availableStatsBuffers_.pop(); > > - frameInfo.paramBuffer = availableParamsBuffers_.front(); > > - availableParamsBuffers_.pop(); > > - > > - frameInfo.paramsDone = false; > > - frameInfo.statsDone = false; > > - > > - frameInfoMap_[request->sequence()] = frameInfo; > > data->ipa_->queueRequest(request->sequence(), request->controls()); > > data->ipa_->fillParams(request->sequence(), > > - frameInfo.paramBuffer->cookie()); > > + frameInfo->paramBuffer->cookie()); > > return 0; > > } > > [...]
2026. 03. 31. 15:12 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote: >> Hi >> >> 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta: >>> From: Daniel Scally <dan.scally@ideasonboard.com> >>> >>> Plumb in the MaliC55 pipeline handler support for capturing frames >>> from memory using the CRU. >>> >>> Introduce a data flow which uses the CRU to feed the ISP through >>> the IVC. >>> >>> In detail: >>> >>> - push incoming request to a pending queue until a buffer from the CRU >>> is available >>> - delay the call to ipa_->fillParams() to the CRU buffer ready even >> >> event ? >> > > Ah yes > >> >>> - once the IPA has computed parameters feed the ISP through the IVC >>> with buffers from the CRU, params and statistics. >>> - Handle completion of in-flight requests: in m2m mode buffers are >>> queued earlier to the ISP capture devices. If the camera is stopped >>> these buffers are returned earlier in error state: complete the >>> Request bypassing the IPA operations. >>> - As cancelled Requests might now be completed earlier then IPA events, >>> modify the IPA event handler to ignore Requests that have been >>> completed already >>> >>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>> --- >>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++---- >>> 1 file changed, 207 insertions(+), 37 deletions(-) >>> >>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>> index be3e38da95ab..10848c412849 100644 >>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>> @@ -21,6 +21,7 @@ >>> #include <libcamera/base/utils.h> >>> #include <libcamera/camera.h> >>> +#include <libcamera/controls.h> >>> #include <libcamera/formats.h> >>> #include <libcamera/geometry.h> >>> #include <libcamera/property_ids.h> >>> @@ -88,6 +89,7 @@ struct MaliC55FrameInfo { >>> FrameBuffer *paramBuffer; >>> FrameBuffer *statBuffer; >>> + FrameBuffer *rawBuffer; >>> bool paramsDone; >>> bool statsDone; >>> @@ -691,6 +693,7 @@ public: >>> void imageBufferReady(FrameBuffer *buffer); >>> void paramsBufferReady(FrameBuffer *buffer); >>> void statsBufferReady(FrameBuffer *buffer); >>> + void cruBufferReady(FrameBuffer *buffer); >>> void paramsComputed(unsigned int requestId, uint32_t bytesused); >>> void statsProcessed(unsigned int requestId, const ControlList &metadata); >>> @@ -739,7 +742,7 @@ private: >>> MaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer); >>> MaliC55FrameInfo *findFrameInfo(Request *request); >>> - void tryComplete(MaliC55FrameInfo *info); >>> + void tryComplete(MaliC55FrameInfo *info, bool cancelled = false); >>> int configureRawStream(MaliC55CameraData *data, >>> const StreamConfiguration &config, >>> @@ -747,6 +750,11 @@ private: >>> int configureProcessedStream(MaliC55CameraData *data, >>> const StreamConfiguration &config, >>> V4L2SubdeviceFormat &subdevFormat); >>> + void cancelPendingRequests(); >>> + >>> + MaliC55FrameInfo * >>> + prepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr); >>> + int queuePendingRequests(MaliC55CameraData *data); >>> void applyScalerCrop(Camera *camera, const ControlList &controls); >>> @@ -772,6 +780,9 @@ private: >>> std::map<unsigned int, MaliC55FrameInfo> frameInfoMap_; >>> + /* Requests for which no buffer has been queued to the CRU device yet. */ >>> + std::queue<Request *> pendingRequests_; >>> + >>> std::array<MaliC55Pipe, MaliC55NumPipes> pipes_; >>> bool dsFitted_; >>> @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera) >>> if (params_->releaseBuffers()) >>> LOG(MaliC55, Error) << "Failed to release params buffers"; >>> + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { >>> + if (ivc_->releaseBuffers()) >>> + LOG(MaliC55, Error) << "Failed to release input buffers"; >>> + if (mem->cru_->freeBuffers()) >>> + LOG(MaliC55, Error) << "Failed to release CRU buffers"; >>> + } >>> + >>> return; >>> } >>> @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) >>> } >>> }; >>> + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { >>> + ret = ivc_->importBuffers(RZG2LCRU::kBufferCount); >>> + if (ret < 0) >>> + return ret; >>> + } >>> + >>> ret = stats_->allocateBuffers(bufferCount, &statsBuffers_); >>> if (ret < 0) >>> return ret; >>> @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control >>> if (ret) >>> return ret; >>> + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { >>> + ret = mem->cru_->start(); >>> + if (ret) { >>> + LOG(MaliC55, Error) >>> + << "Failed to start CRU " << camera->id(); >>> + freeBuffers(camera); >> >> Minor thing, but I think reworking these with `utils::scope_exit` is a worthwhile change >> if you plan to send a new version. >> > > Can we consider doing this on top as the existing error paths would > need to be reworked otherwise ? Yes, certainly. > >> >>> + return ret; >>> + } >>> + >>> + ret = ivc_->streamOn(); >>> + if (ret) { >>> + LOG(MaliC55, Error) >>> + << "Failed to start IVC" << camera->id(); >>> + freeBuffers(camera); >>> + return ret; >>> + } >>> + } >>> + >>> if (data->ipa_) { >>> ret = data->ipa_->start(); >>> if (ret) { >>> @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control >>> return 0; >>> } >>> +void PipelineHandlerMaliC55::cancelPendingRequests() >>> +{ >>> + while (!pendingRequests_.empty()) { >>> + cancelRequest(pendingRequests_.front()); >>> + pendingRequests_.pop(); >>> + } >>> +} >>> + >>> void PipelineHandlerMaliC55::stopDevice(Camera *camera) >>> { >>> MaliC55CameraData *data = cameraData(camera); >>> isp_->setFrameStartEnabled(false); >>> + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { >>> + ivc_->streamOff(); >>> + mem->cru_->stop(); >>> + } >>> + >>> for (MaliC55Pipe &pipe : pipes_) { >>> if (!pipe.stream) >>> continue; >>> @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera) >>> if (data->ipa_) >>> data->ipa_->stop(); >>> freeBuffers(camera); >>> + >>> + cancelPendingRequests(); >>> } >>> void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, >>> @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, >>> } >>> } >>> +MaliC55FrameInfo * >>> +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru) >>> +{ >>> + if (availableStatsBuffers_.empty()) { >>> + LOG(MaliC55, Error) << "Stats buffer underrun"; >>> + return nullptr; >>> + } >>> + >>> + if (availableParamsBuffers_.empty()) { >>> + LOG(MaliC55, Error) << "Params buffer underrun"; >>> + return nullptr; >>> + } >>> + >>> + MaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()]; >> >> I think it would be better to do it last, not to leave an "empty" frame info >> object in the map. > > As the only failure point is the below if (!frameInfo.rawBuffer) can I > do after that ? Yes, or alternatively a temporary `MaliC55FrameInfo` could be filled, which is added to the map last. Like it was done previously. > >> >> >>> + frameInfo.request = request; >>> + >>> + if (cru) { >>> + frameInfo.rawBuffer = cru->queueBuffer(request); >>> + if (!frameInfo.rawBuffer) >> >> I'm wondering, can it not be guaranteed that there is always an available buffer? >> Wouldn't it be sufficient to ensure that at most 4 requests are queued to the >> pipeline handler? In that case I believe there should always be an available cru buffer. > > Should I initialize PipelineHandler() with maxQueuedRequestsDevice == > kBufferCount ? > >> >> >>> + return nullptr; >>> + } >>> + >>> + frameInfo.statBuffer = availableStatsBuffers_.front(); >>> + availableStatsBuffers_.pop(); >>> + frameInfo.paramBuffer = availableParamsBuffers_.front(); >>> + availableParamsBuffers_.pop(); >>> + >>> + frameInfo.paramsDone = false; >>> + frameInfo.statsDone = false; >>> + >>> + return &frameInfo; >>> +} >>> + >>> +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data) >> >> Should this not be called when a CRU buffer becomes available? If the queue is >> only drained if a new request is queued, then I feel like things can get stuck >> if the application is stopping and is waiting for the pending requests to complete. >> I suppose it's fine now because everything uses hard-coded 4 buffers / requests. >> > > mmm, you're right. We only try to run when the application queue a new > Request. Should I create an handler for ivc_->bufferReady (which now > simply returns the buffer to the cru) and call > PipelineHandlerMaliC55::queuePendingRequests() from there ? I'm looking at 5fb28bfe7438c8ed1029501c3c050e667eb2f507 and 1ed284ba487b7e0903e031c4e6604b601ff040a8. I think a similar change could be applied here as well: always allocate a constant number of cru, param, or stat buffers, and then set `maxQueuedRequestsDevice` to the minimum of those. If I'm not mistaken, that should guarantee that at least one of all three types of buffers is available in `queueRequestDevice()`. Then `pendingRequests_` wouldn't even be needed as far as I can see. Maybe that's the best approach. > >> >>> +{ >>> + auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_); >>> + ASSERT(mem); >>> + >>> + while (!pendingRequests_.empty()) { >>> + Request *request = pendingRequests_.front(); >>> + >>> + if (!prepareFrameInfo(request, mem->cru_.get())) >>> + return -ENOENT; >>> + >>> + for (auto &[stream, buffer] : request->buffers()) { >>> + MaliC55Pipe *pipe = pipeFromStream(data, stream); >>> + >>> + pipe->cap->queueBuffer(buffer); >>> + } >>> + >>> + data->ipa_->queueRequest(request->sequence(), request->controls()); >>> + >>> + pendingRequests_.pop(); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) >>> { >>> MaliC55CameraData *data = cameraData(camera); >>> + /* >>> + * If we're in memory input mode, we need to pop the requests onto the >> >> push ? > > ups > >> >> >>> + * pending list until a CRU buffer is ready...otherwise we can just do >>> + * everything immediately. >>> + */ >> >> Or maybe I was wrong above. But this says "pending list until a CRU buffer is ready", >> so if no CRU buffer is available, shouldn't it just push to `pendingRequests_` and >> return 0? Because this now returns non-zero, and that will cancel the request. >> > > I actually think the below error path handling is wrong now that I > look at it more closely Ahh, right. push() appends to the end, but pop() consumes from the front. > >> >>> + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { >>> + pendingRequests_.push(request); >>> + >>> + int ret = queuePendingRequests(data); >>> + if (ret) { >>> + pendingRequests_.pop(); > > Requests are popped from the queue in queuePendingRequests() only at > the end of the loop, so if we error out, there isn't any need to > actually pop them here > >>> + return ret; > > And, as you suggested, if we return an error the request is cancelled, > so we should probably return 0 here, leave the Request in the queue > and let the next queuePendingRequests() consume it ? > >>> + } >>> + >>> + return 0; >>> + } >>> + >>> /* Do not run the IPA if the TPG is in use. */ >>> if (!data->ipa_) { >>> MaliC55FrameInfo frameInfo; >>> @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) >>> return 0; >>> } >>> - if (availableStatsBuffers_.empty()) { >>> - LOG(MaliC55, Error) << "Stats buffer underrun"; >>> - return -ENOENT; >>> - } >>> - >>> - if (availableParamsBuffers_.empty()) { >>> - LOG(MaliC55, Error) << "Params buffer underrun"; >>> + auto frameInfo = prepareFrameInfo(request); >>> + if (!frameInfo) >>> return -ENOENT; >>> - } >>> - >>> - MaliC55FrameInfo frameInfo; >>> - frameInfo.request = request; >>> - >>> - frameInfo.statBuffer = availableStatsBuffers_.front(); >>> - availableStatsBuffers_.pop(); >>> - frameInfo.paramBuffer = availableParamsBuffers_.front(); >>> - availableParamsBuffers_.pop(); >>> - >>> - frameInfo.paramsDone = false; >>> - frameInfo.statsDone = false; >>> - >>> - frameInfoMap_[request->sequence()] = frameInfo; >>> data->ipa_->queueRequest(request->sequence(), request->controls()); >>> data->ipa_->fillParams(request->sequence(), >>> - frameInfo.paramBuffer->cookie()); >>> + frameInfo->paramBuffer->cookie()); >>> return 0; >>> } >>> [...]
Hi Barnabás On Tue, Mar 31, 2026 at 04:10:53PM +0200, Barnabás Pőcze wrote: > 2026. 03. 31. 15:12 keltezéssel, Jacopo Mondi írta: > > Hi Barnabás > > > > On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote: > > > Hi > > > > > > 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta: > > > > From: Daniel Scally <dan.scally@ideasonboard.com> > > > > > > > > Plumb in the MaliC55 pipeline handler support for capturing frames > > > > from memory using the CRU. > > > > > > > > Introduce a data flow which uses the CRU to feed the ISP through > > > > the IVC. > > > > > > > > In detail: > > > > > > > > - push incoming request to a pending queue until a buffer from the CRU > > > > is available > > > > - delay the call to ipa_->fillParams() to the CRU buffer ready even > > > > > > event ? > > > > > > > Ah yes > > > > > > > > > - once the IPA has computed parameters feed the ISP through the IVC > > > > with buffers from the CRU, params and statistics. > > > > - Handle completion of in-flight requests: in m2m mode buffers are > > > > queued earlier to the ISP capture devices. If the camera is stopped > > > > these buffers are returned earlier in error state: complete the > > > > Request bypassing the IPA operations. > > > > - As cancelled Requests might now be completed earlier then IPA events, > > > > modify the IPA event handler to ignore Requests that have been > > > > completed already > > > > > > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > --- > > > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++---- > > > > 1 file changed, 207 insertions(+), 37 deletions(-) > > > > > > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > index be3e38da95ab..10848c412849 100644 > > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > @@ -21,6 +21,7 @@ > > > > #include <libcamera/base/utils.h> > > > > #include <libcamera/camera.h> > > > > +#include <libcamera/controls.h> > > > > #include <libcamera/formats.h> > > > > #include <libcamera/geometry.h> > > > > #include <libcamera/property_ids.h> > > > > @@ -88,6 +89,7 @@ struct MaliC55FrameInfo { > > > > FrameBuffer *paramBuffer; > > > > FrameBuffer *statBuffer; > > > > + FrameBuffer *rawBuffer; > > > > bool paramsDone; > > > > bool statsDone; > > > > @@ -691,6 +693,7 @@ public: > > > > void imageBufferReady(FrameBuffer *buffer); > > > > void paramsBufferReady(FrameBuffer *buffer); > > > > void statsBufferReady(FrameBuffer *buffer); > > > > + void cruBufferReady(FrameBuffer *buffer); > > > > void paramsComputed(unsigned int requestId, uint32_t bytesused); > > > > void statsProcessed(unsigned int requestId, const ControlList &metadata); > > > > @@ -739,7 +742,7 @@ private: > > > > MaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer); > > > > MaliC55FrameInfo *findFrameInfo(Request *request); > > > > - void tryComplete(MaliC55FrameInfo *info); > > > > + void tryComplete(MaliC55FrameInfo *info, bool cancelled = false); > > > > int configureRawStream(MaliC55CameraData *data, > > > > const StreamConfiguration &config, > > > > @@ -747,6 +750,11 @@ private: > > > > int configureProcessedStream(MaliC55CameraData *data, > > > > const StreamConfiguration &config, > > > > V4L2SubdeviceFormat &subdevFormat); > > > > + void cancelPendingRequests(); > > > > + > > > > + MaliC55FrameInfo * > > > > + prepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr); > > > > + int queuePendingRequests(MaliC55CameraData *data); > > > > void applyScalerCrop(Camera *camera, const ControlList &controls); > > > > @@ -772,6 +780,9 @@ private: > > > > std::map<unsigned int, MaliC55FrameInfo> frameInfoMap_; > > > > + /* Requests for which no buffer has been queued to the CRU device yet. */ > > > > + std::queue<Request *> pendingRequests_; > > > > + > > > > std::array<MaliC55Pipe, MaliC55NumPipes> pipes_; > > > > bool dsFitted_; > > > > @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera) > > > > if (params_->releaseBuffers()) > > > > LOG(MaliC55, Error) << "Failed to release params buffers"; > > > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > > > > + if (ivc_->releaseBuffers()) > > > > + LOG(MaliC55, Error) << "Failed to release input buffers"; > > > > + if (mem->cru_->freeBuffers()) > > > > + LOG(MaliC55, Error) << "Failed to release CRU buffers"; > > > > + } > > > > + > > > > return; > > > > } > > > > @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) > > > > } > > > > }; > > > > + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { > > > > + ret = ivc_->importBuffers(RZG2LCRU::kBufferCount); > > > > + if (ret < 0) > > > > + return ret; > > > > + } > > > > + > > > > ret = stats_->allocateBuffers(bufferCount, &statsBuffers_); > > > > if (ret < 0) > > > > return ret; > > > > @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control > > > > if (ret) > > > > return ret; > > > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > > > > + ret = mem->cru_->start(); > > > > + if (ret) { > > > > + LOG(MaliC55, Error) > > > > + << "Failed to start CRU " << camera->id(); > > > > + freeBuffers(camera); > > > > > > Minor thing, but I think reworking these with `utils::scope_exit` is a worthwhile change > > > if you plan to send a new version. > > > > > > > Can we consider doing this on top as the existing error paths would > > need to be reworked otherwise ? > > Yes, certainly. > > > > > > > > > > > + return ret; > > > > + } > > > > + > > > > + ret = ivc_->streamOn(); > > > > + if (ret) { > > > > + LOG(MaliC55, Error) > > > > + << "Failed to start IVC" << camera->id(); > > > > + freeBuffers(camera); > > > > + return ret; > > > > + } > > > > + } > > > > + > > > > if (data->ipa_) { > > > > ret = data->ipa_->start(); > > > > if (ret) { > > > > @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control > > > > return 0; > > > > } > > > > +void PipelineHandlerMaliC55::cancelPendingRequests() > > > > +{ > > > > + while (!pendingRequests_.empty()) { > > > > + cancelRequest(pendingRequests_.front()); > > > > + pendingRequests_.pop(); > > > > + } > > > > +} > > > > + > > > > void PipelineHandlerMaliC55::stopDevice(Camera *camera) > > > > { > > > > MaliC55CameraData *data = cameraData(camera); > > > > isp_->setFrameStartEnabled(false); > > > > + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { > > > > + ivc_->streamOff(); > > > > + mem->cru_->stop(); > > > > + } > > > > + > > > > for (MaliC55Pipe &pipe : pipes_) { > > > > if (!pipe.stream) > > > > continue; > > > > @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera) > > > > if (data->ipa_) > > > > data->ipa_->stop(); > > > > freeBuffers(camera); > > > > + > > > > + cancelPendingRequests(); > > > > } > > > > void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, > > > > @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, > > > > } > > > > } > > > > +MaliC55FrameInfo * > > > > +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru) > > > > +{ > > > > + if (availableStatsBuffers_.empty()) { > > > > + LOG(MaliC55, Error) << "Stats buffer underrun"; > > > > + return nullptr; > > > > + } > > > > + > > > > + if (availableParamsBuffers_.empty()) { > > > > + LOG(MaliC55, Error) << "Params buffer underrun"; > > > > + return nullptr; > > > > + } > > > > + > > > > + MaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()]; > > > > > > I think it would be better to do it last, not to leave an "empty" frame info > > > object in the map. > > > > As the only failure point is the below if (!frameInfo.rawBuffer) can I > > do after that ? > > Yes, or alternatively a temporary `MaliC55FrameInfo` could be filled, which is added > to the map last. Like it was done previously. Oh well, if we can avoid a copy, that's probably better > > > > > > > > > > > > > > + frameInfo.request = request; > > > > + > > > > + if (cru) { > > > > + frameInfo.rawBuffer = cru->queueBuffer(request); > > > > + if (!frameInfo.rawBuffer) > > > > > > I'm wondering, can it not be guaranteed that there is always an available buffer? > > > Wouldn't it be sufficient to ensure that at most 4 requests are queued to the > > > pipeline handler? In that case I believe there should always be an available cru buffer. > > > > Should I initialize PipelineHandler() with maxQueuedRequestsDevice == > > kBufferCount ? > > > > > > > > > > > > + return nullptr; > > > > + } > > > > + > > > > + frameInfo.statBuffer = availableStatsBuffers_.front(); > > > > + availableStatsBuffers_.pop(); > > > > + frameInfo.paramBuffer = availableParamsBuffers_.front(); > > > > + availableParamsBuffers_.pop(); > > > > + > > > > + frameInfo.paramsDone = false; > > > > + frameInfo.statsDone = false; > > > > + > > > > + return &frameInfo; > > > > +} > > > > + > > > > +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data) > > > > > > Should this not be called when a CRU buffer becomes available? If the queue is > > > only drained if a new request is queued, then I feel like things can get stuck > > > if the application is stopping and is waiting for the pending requests to complete. > > > I suppose it's fine now because everything uses hard-coded 4 buffers / requests. > > > > > > > mmm, you're right. We only try to run when the application queue a new > > Request. Should I create an handler for ivc_->bufferReady (which now > > simply returns the buffer to the cru) and call > > PipelineHandlerMaliC55::queuePendingRequests() from there ? > > I'm looking at 5fb28bfe7438c8ed1029501c3c050e667eb2f507 and 1ed284ba487b7e0903e031c4e6604b601ff040a8. > I think a similar change could be applied here as well: always allocate a constant number of cru, param, > or stat buffers, and then set `maxQueuedRequestsDevice` to the minimum of those. If I'm not mistaken, > that should guarantee that at least one of all three types of buffers is available in `queueRequestDevice()`. > Then `pendingRequests_` wouldn't even be needed as far as I can see. Maybe that's the best approach. Thing is that with Mali, we have one more actor: the CRU. Right now the CRU allocates a number of buffers defined by RZG2LCRU::kBufferCount. I'm tempted to make RZG2LCRU::start() accept a number of buffers to allocate and move the buffer number definition in the MaliC55 file so that we can use to allocate buffers for the cru, params and stats. I would however keep pendingRequests_ for the time being ? > > > > > > > > > > +{ > > > > + auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_); > > > > + ASSERT(mem); > > > > + > > > > + while (!pendingRequests_.empty()) { > > > > + Request *request = pendingRequests_.front(); > > > > + > > > > + if (!prepareFrameInfo(request, mem->cru_.get())) > > > > + return -ENOENT; > > > > + > > > > + for (auto &[stream, buffer] : request->buffers()) { > > > > + MaliC55Pipe *pipe = pipeFromStream(data, stream); > > > > + > > > > + pipe->cap->queueBuffer(buffer); > > > > + } > > > > + > > > > + data->ipa_->queueRequest(request->sequence(), request->controls()); > > > > + > > > > + pendingRequests_.pop(); > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) > > > > { > > > > MaliC55CameraData *data = cameraData(camera); > > > > + /* > > > > + * If we're in memory input mode, we need to pop the requests onto the > > > > > > push ? > > > > ups > > > > > > > > > > > > + * pending list until a CRU buffer is ready...otherwise we can just do > > > > + * everything immediately. > > > > + */ > > > > > > Or maybe I was wrong above. But this says "pending list until a CRU buffer is ready", > > > so if no CRU buffer is available, shouldn't it just push to `pendingRequests_` and > > > return 0? Because this now returns non-zero, and that will cancel the request. > > > > > > > I actually think the below error path handling is wrong now that I > > look at it more closely > > Ahh, right. push() appends to the end, but pop() consumes from the front. > > > > > > > > > > > + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { > > > > + pendingRequests_.push(request); > > > > + > > > > + int ret = queuePendingRequests(data); > > > > + if (ret) { > > > > + pendingRequests_.pop(); > > > > Requests are popped from the queue in queuePendingRequests() only at > > the end of the loop, so if we error out, there isn't any need to > > actually pop them here > > > > > > + return ret; > > > > And, as you suggested, if we return an error the request is cancelled, > > so we should probably return 0 here, leave the Request in the queue > > and let the next queuePendingRequests() consume it ? > > I'm now returning 0 unconditionally from here. Thanks j > > > > + } > > > > + > > > > + return 0; > > > > + } > > > > + > > > > /* Do not run the IPA if the TPG is in use. */ > > > > if (!data->ipa_) { > > > > MaliC55FrameInfo frameInfo; > > > > @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) > > > > return 0; > > > > } > > > > - if (availableStatsBuffers_.empty()) { > > > > - LOG(MaliC55, Error) << "Stats buffer underrun"; > > > > - return -ENOENT; > > > > - } > > > > - > > > > - if (availableParamsBuffers_.empty()) { > > > > - LOG(MaliC55, Error) << "Params buffer underrun"; > > > > + auto frameInfo = prepareFrameInfo(request); > > > > + if (!frameInfo) > > > > return -ENOENT; > > > > - } > > > > - > > > > - MaliC55FrameInfo frameInfo; > > > > - frameInfo.request = request; > > > > - > > > > - frameInfo.statBuffer = availableStatsBuffers_.front(); > > > > - availableStatsBuffers_.pop(); > > > > - frameInfo.paramBuffer = availableParamsBuffers_.front(); > > > > - availableParamsBuffers_.pop(); > > > > - > > > > - frameInfo.paramsDone = false; > > > > - frameInfo.statsDone = false; > > > > - > > > > - frameInfoMap_[request->sequence()] = frameInfo; > > > > data->ipa_->queueRequest(request->sequence(), request->controls()); > > > > data->ipa_->fillParams(request->sequence(), > > > > - frameInfo.paramBuffer->cookie()); > > > > + frameInfo->paramBuffer->cookie()); > > > > return 0; > > > > } > > > > [...] >
2026. 03. 31. 16:36 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Tue, Mar 31, 2026 at 04:10:53PM +0200, Barnabás Pőcze wrote: >> 2026. 03. 31. 15:12 keltezéssel, Jacopo Mondi írta: >>> Hi Barnabás >>> >>> On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote: >>>> Hi >>>> >>>> 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta: >>>>> From: Daniel Scally <dan.scally@ideasonboard.com> >>>>> >>>>> Plumb in the MaliC55 pipeline handler support for capturing frames >>>>> from memory using the CRU. >>>>> >>>>> Introduce a data flow which uses the CRU to feed the ISP through >>>>> the IVC. >>>>> >>>>> In detail: >>>>> >>>>> - push incoming request to a pending queue until a buffer from the CRU >>>>> is available >>>>> - delay the call to ipa_->fillParams() to the CRU buffer ready even >>>> >>>> event ? >>>> >>> >>> Ah yes >>> >>>> >>>>> - once the IPA has computed parameters feed the ISP through the IVC >>>>> with buffers from the CRU, params and statistics. >>>>> - Handle completion of in-flight requests: in m2m mode buffers are >>>>> queued earlier to the ISP capture devices. If the camera is stopped >>>>> these buffers are returned earlier in error state: complete the >>>>> Request bypassing the IPA operations. >>>>> - As cancelled Requests might now be completed earlier then IPA events, >>>>> modify the IPA event handler to ignore Requests that have been >>>>> completed already >>>>> >>>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>>>> --- >>>>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++---- >>>>> 1 file changed, 207 insertions(+), 37 deletions(-) >>>>> >>>>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>>>> index be3e38da95ab..10848c412849 100644 >>>>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>>>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>>>> @@ -21,6 +21,7 @@ > [...] >>>>> + frameInfo.request = request; >>>>> + >>>>> + if (cru) { >>>>> + frameInfo.rawBuffer = cru->queueBuffer(request); >>>>> + if (!frameInfo.rawBuffer) >>>> >>>> I'm wondering, can it not be guaranteed that there is always an available buffer? >>>> Wouldn't it be sufficient to ensure that at most 4 requests are queued to the >>>> pipeline handler? In that case I believe there should always be an available cru buffer. >>> >>> Should I initialize PipelineHandler() with maxQueuedRequestsDevice == >>> kBufferCount ? >>> >>>> >>>> >>>>> + return nullptr; >>>>> + } >>>>> + >>>>> + frameInfo.statBuffer = availableStatsBuffers_.front(); >>>>> + availableStatsBuffers_.pop(); >>>>> + frameInfo.paramBuffer = availableParamsBuffers_.front(); >>>>> + availableParamsBuffers_.pop(); >>>>> + >>>>> + frameInfo.paramsDone = false; >>>>> + frameInfo.statsDone = false; >>>>> + >>>>> + return &frameInfo; >>>>> +} >>>>> + >>>>> +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data) >>>> >>>> Should this not be called when a CRU buffer becomes available? If the queue is >>>> only drained if a new request is queued, then I feel like things can get stuck >>>> if the application is stopping and is waiting for the pending requests to complete. >>>> I suppose it's fine now because everything uses hard-coded 4 buffers / requests. >>>> >>> >>> mmm, you're right. We only try to run when the application queue a new >>> Request. Should I create an handler for ivc_->bufferReady (which now >>> simply returns the buffer to the cru) and call >>> PipelineHandlerMaliC55::queuePendingRequests() from there ? >> >> I'm looking at 5fb28bfe7438c8ed1029501c3c050e667eb2f507 and 1ed284ba487b7e0903e031c4e6604b601ff040a8. >> I think a similar change could be applied here as well: always allocate a constant number of cru, param, >> or stat buffers, and then set `maxQueuedRequestsDevice` to the minimum of those. If I'm not mistaken, >> that should guarantee that at least one of all three types of buffers is available in `queueRequestDevice()`. >> Then `pendingRequests_` wouldn't even be needed as far as I can see. Maybe that's the best approach. > > Thing is that with Mali, we have one more actor: the CRU. That's true, but I don't think it is fundamentally different. If `maxQueuedRequestsDevice` is set to min(cruBuffers, paramBuffers, statBuffers), then then that should work the same. > > Right now the CRU allocates a number of buffers defined by > RZG2LCRU::kBufferCount. > > I'm tempted to make RZG2LCRU::start() accept a number of buffers to > allocate and move the buffer number definition in the MaliC55 file so > that we can use to allocate buffers for the cru, params and stats. That sounds reasonable. > > I would however keep pendingRequests_ for the time being ? I think if `maxQueuedRequestsDevice` is chosen well, then it will just be empty all the time. Unless I'm missing something, removing it seems like a great simplification? > [...]
Hi Barnabás On Tue, Mar 31, 2026 at 05:49:02PM +0200, Barnabás Pőcze wrote: > 2026. 03. 31. 16:36 keltezéssel, Jacopo Mondi írta: > > Hi Barnabás > > > > On Tue, Mar 31, 2026 at 04:10:53PM +0200, Barnabás Pőcze wrote: > > > 2026. 03. 31. 15:12 keltezéssel, Jacopo Mondi írta: > > > > Hi Barnabás > > > > > > > > On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote: > > > > > Hi > > > > > > > > > > 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta: > > > > > > From: Daniel Scally <dan.scally@ideasonboard.com> > > > > > > > > > > > > Plumb in the MaliC55 pipeline handler support for capturing frames > > > > > > from memory using the CRU. > > > > > > > > > > > > Introduce a data flow which uses the CRU to feed the ISP through > > > > > > the IVC. > > > > > > > > > > > > In detail: > > > > > > > > > > > > - push incoming request to a pending queue until a buffer from the CRU > > > > > > is available > > > > > > - delay the call to ipa_->fillParams() to the CRU buffer ready even > > > > > > > > > > event ? > > > > > > > > > > > > > Ah yes > > > > > > > > > > > > > > > - once the IPA has computed parameters feed the ISP through the IVC > > > > > > with buffers from the CRU, params and statistics. > > > > > > - Handle completion of in-flight requests: in m2m mode buffers are > > > > > > queued earlier to the ISP capture devices. If the camera is stopped > > > > > > these buffers are returned earlier in error state: complete the > > > > > > Request bypassing the IPA operations. > > > > > > - As cancelled Requests might now be completed earlier then IPA events, > > > > > > modify the IPA event handler to ignore Requests that have been > > > > > > completed already > > > > > > > > > > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > --- > > > > > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++---- > > > > > > 1 file changed, 207 insertions(+), 37 deletions(-) > > > > > > > > > > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > > > index be3e38da95ab..10848c412849 100644 > > > > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > > > @@ -21,6 +21,7 @@ > > [...] > > > > > > + frameInfo.request = request; > > > > > > + > > > > > > + if (cru) { > > > > > > + frameInfo.rawBuffer = cru->queueBuffer(request); > > > > > > + if (!frameInfo.rawBuffer) > > > > > > > > > > I'm wondering, can it not be guaranteed that there is always an available buffer? > > > > > Wouldn't it be sufficient to ensure that at most 4 requests are queued to the > > > > > pipeline handler? In that case I believe there should always be an available cru buffer. > > > > > > > > Should I initialize PipelineHandler() with maxQueuedRequestsDevice == > > > > kBufferCount ? > > > > > > > > > > > > > > > > > > > > + return nullptr; > > > > > > + } > > > > > > + > > > > > > + frameInfo.statBuffer = availableStatsBuffers_.front(); > > > > > > + availableStatsBuffers_.pop(); > > > > > > + frameInfo.paramBuffer = availableParamsBuffers_.front(); > > > > > > + availableParamsBuffers_.pop(); > > > > > > + > > > > > > + frameInfo.paramsDone = false; > > > > > > + frameInfo.statsDone = false; > > > > > > + > > > > > > + return &frameInfo; > > > > > > +} > > > > > > + > > > > > > +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data) > > > > > > > > > > Should this not be called when a CRU buffer becomes available? If the queue is > > > > > only drained if a new request is queued, then I feel like things can get stuck > > > > > if the application is stopping and is waiting for the pending requests to complete. > > > > > I suppose it's fine now because everything uses hard-coded 4 buffers / requests. > > > > > > > > > > > > > mmm, you're right. We only try to run when the application queue a new > > > > Request. Should I create an handler for ivc_->bufferReady (which now > > > > simply returns the buffer to the cru) and call > > > > PipelineHandlerMaliC55::queuePendingRequests() from there ? > > > > > > I'm looking at 5fb28bfe7438c8ed1029501c3c050e667eb2f507 and 1ed284ba487b7e0903e031c4e6604b601ff040a8. > > > I think a similar change could be applied here as well: always allocate a constant number of cru, param, > > > or stat buffers, and then set `maxQueuedRequestsDevice` to the minimum of those. If I'm not mistaken, > > > that should guarantee that at least one of all three types of buffers is available in `queueRequestDevice()`. > > > Then `pendingRequests_` wouldn't even be needed as far as I can see. Maybe that's the best approach. > > > > Thing is that with Mali, we have one more actor: the CRU. > > That's true, but I don't think it is fundamentally different. > If `maxQueuedRequestsDevice` is set to min(cruBuffers, paramBuffers, statBuffers), > then then that should work the same. > > > > > > Right now the CRU allocates a number of buffers defined by > > RZG2LCRU::kBufferCount. > > > > I'm tempted to make RZG2LCRU::start() accept a number of buffers to > > allocate and move the buffer number definition in the MaliC55 file so > > that we can use to allocate buffers for the cru, params and stats. > > That sounds reasonable. > > > > > > I would however keep pendingRequests_ for the time being ? > > I think if `maxQueuedRequestsDevice` is chosen well, then it will > just be empty all the time. Unless I'm missing something, removing > it seems like a great simplification? You're right. If we limit the number of requests in-flight to the number of available CRU buffers, the PipelineHandler base class basically does the queueing for us. Probably this patch pre-dates the introduction of PipelineHandler::maxQueuedRequestsDevice_ I'll run a few more tests with ASSERT(pendingRequests_.size() < 2); and if it's all good I'll remove the queue > > > > [...]
diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index be3e38da95ab..10848c412849 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -21,6 +21,7 @@ #include <libcamera/base/utils.h> #include <libcamera/camera.h> +#include <libcamera/controls.h> #include <libcamera/formats.h> #include <libcamera/geometry.h> #include <libcamera/property_ids.h> @@ -88,6 +89,7 @@ struct MaliC55FrameInfo { FrameBuffer *paramBuffer; FrameBuffer *statBuffer; + FrameBuffer *rawBuffer; bool paramsDone; bool statsDone; @@ -691,6 +693,7 @@ public: void imageBufferReady(FrameBuffer *buffer); void paramsBufferReady(FrameBuffer *buffer); void statsBufferReady(FrameBuffer *buffer); + void cruBufferReady(FrameBuffer *buffer); void paramsComputed(unsigned int requestId, uint32_t bytesused); void statsProcessed(unsigned int requestId, const ControlList &metadata); @@ -739,7 +742,7 @@ private: MaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer); MaliC55FrameInfo *findFrameInfo(Request *request); - void tryComplete(MaliC55FrameInfo *info); + void tryComplete(MaliC55FrameInfo *info, bool cancelled = false); int configureRawStream(MaliC55CameraData *data, const StreamConfiguration &config, @@ -747,6 +750,11 @@ private: int configureProcessedStream(MaliC55CameraData *data, const StreamConfiguration &config, V4L2SubdeviceFormat &subdevFormat); + void cancelPendingRequests(); + + MaliC55FrameInfo * + prepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr); + int queuePendingRequests(MaliC55CameraData *data); void applyScalerCrop(Camera *camera, const ControlList &controls); @@ -772,6 +780,9 @@ private: std::map<unsigned int, MaliC55FrameInfo> frameInfoMap_; + /* Requests for which no buffer has been queued to the CRU device yet. */ + std::queue<Request *> pendingRequests_; + std::array<MaliC55Pipe, MaliC55NumPipes> pipes_; bool dsFitted_; @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera) if (params_->releaseBuffers()) LOG(MaliC55, Error) << "Failed to release params buffers"; + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { + if (ivc_->releaseBuffers()) + LOG(MaliC55, Error) << "Failed to release input buffers"; + if (mem->cru_->freeBuffers()) + LOG(MaliC55, Error) << "Failed to release CRU buffers"; + } + return; } @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) } }; + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { + ret = ivc_->importBuffers(RZG2LCRU::kBufferCount); + if (ret < 0) + return ret; + } + ret = stats_->allocateBuffers(bufferCount, &statsBuffers_); if (ret < 0) return ret; @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control if (ret) return ret; + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { + ret = mem->cru_->start(); + if (ret) { + LOG(MaliC55, Error) + << "Failed to start CRU " << camera->id(); + freeBuffers(camera); + return ret; + } + + ret = ivc_->streamOn(); + if (ret) { + LOG(MaliC55, Error) + << "Failed to start IVC" << camera->id(); + freeBuffers(camera); + return ret; + } + } + if (data->ipa_) { ret = data->ipa_->start(); if (ret) { @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control return 0; } +void PipelineHandlerMaliC55::cancelPendingRequests() +{ + while (!pendingRequests_.empty()) { + cancelRequest(pendingRequests_.front()); + pendingRequests_.pop(); + } +} + void PipelineHandlerMaliC55::stopDevice(Camera *camera) { MaliC55CameraData *data = cameraData(camera); isp_->setFrameStartEnabled(false); + if (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) { + ivc_->streamOff(); + mem->cru_->stop(); + } + for (MaliC55Pipe &pipe : pipes_) { if (!pipe.stream) continue; @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera) if (data->ipa_) data->ipa_->stop(); freeBuffers(camera); + + cancelPendingRequests(); } void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera, } } +MaliC55FrameInfo * +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru) +{ + if (availableStatsBuffers_.empty()) { + LOG(MaliC55, Error) << "Stats buffer underrun"; + return nullptr; + } + + if (availableParamsBuffers_.empty()) { + LOG(MaliC55, Error) << "Params buffer underrun"; + return nullptr; + } + + MaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()]; + frameInfo.request = request; + + if (cru) { + frameInfo.rawBuffer = cru->queueBuffer(request); + if (!frameInfo.rawBuffer) + return nullptr; + } + + frameInfo.statBuffer = availableStatsBuffers_.front(); + availableStatsBuffers_.pop(); + frameInfo.paramBuffer = availableParamsBuffers_.front(); + availableParamsBuffers_.pop(); + + frameInfo.paramsDone = false; + frameInfo.statsDone = false; + + return &frameInfo; +} + +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data) +{ + auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_); + ASSERT(mem); + + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + + if (!prepareFrameInfo(request, mem->cru_.get())) + return -ENOENT; + + for (auto &[stream, buffer] : request->buffers()) { + MaliC55Pipe *pipe = pipeFromStream(data, stream); + + pipe->cap->queueBuffer(buffer); + } + + data->ipa_->queueRequest(request->sequence(), request->controls()); + + pendingRequests_.pop(); + } + + return 0; +} + int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) { MaliC55CameraData *data = cameraData(camera); + /* + * If we're in memory input mode, we need to pop the requests onto the + * pending list until a CRU buffer is ready...otherwise we can just do + * everything immediately. + */ + if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { + pendingRequests_.push(request); + + int ret = queuePendingRequests(data); + if (ret) { + pendingRequests_.pop(); + return ret; + } + + return 0; + } + /* Do not run the IPA if the TPG is in use. */ if (!data->ipa_) { MaliC55FrameInfo frameInfo; @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request) return 0; } - if (availableStatsBuffers_.empty()) { - LOG(MaliC55, Error) << "Stats buffer underrun"; - return -ENOENT; - } - - if (availableParamsBuffers_.empty()) { - LOG(MaliC55, Error) << "Params buffer underrun"; + auto frameInfo = prepareFrameInfo(request); + if (!frameInfo) return -ENOENT; - } - - MaliC55FrameInfo frameInfo; - frameInfo.request = request; - - frameInfo.statBuffer = availableStatsBuffers_.front(); - availableStatsBuffers_.pop(); - frameInfo.paramBuffer = availableParamsBuffers_.front(); - availableParamsBuffers_.pop(); - - frameInfo.paramsDone = false; - frameInfo.statsDone = false; - - frameInfoMap_[request->sequence()] = frameInfo; data->ipa_->queueRequest(request->sequence(), request->controls()); data->ipa_->fillParams(request->sequence(), - frameInfo.paramBuffer->cookie()); + frameInfo->paramBuffer->cookie()); return 0; } @@ -1540,18 +1653,21 @@ MaliC55FrameInfo *PipelineHandlerMaliC55::findFrameInfo(FrameBuffer *buffer) { for (auto &[sequence, info] : frameInfoMap_) { if (info.paramBuffer == buffer || - info.statBuffer == buffer) + info.statBuffer == buffer || + info.rawBuffer == buffer) return &info; } return nullptr; } -void PipelineHandlerMaliC55::tryComplete(MaliC55FrameInfo *info) +void PipelineHandlerMaliC55::tryComplete(MaliC55FrameInfo *info, bool cancelled) { - if (!info->paramsDone) - return; - if (!info->statsDone) + /* + * If the buffer has been cancelled, we complete the request without + * waiting for the IPA. + */ + if (!cancelled && (!info->paramsDone || !info->statsDone)) return; Request *request = info->request; @@ -1575,13 +1691,15 @@ void PipelineHandlerMaliC55::imageBufferReady(FrameBuffer *buffer) ASSERT(info); if (completeBuffer(request, buffer)) - tryComplete(info); + tryComplete(info, + buffer->metadata().status == FrameMetadata::FrameCancelled); } void PipelineHandlerMaliC55::paramsBufferReady(FrameBuffer *buffer) { MaliC55FrameInfo *info = findFrameInfo(buffer); - ASSERT(info); + if (!info) + return; info->paramsDone = true; @@ -1591,7 +1709,8 @@ void PipelineHandlerMaliC55::paramsBufferReady(FrameBuffer *buffer) void PipelineHandlerMaliC55::statsBufferReady(FrameBuffer *buffer) { MaliC55FrameInfo *info = findFrameInfo(buffer); - ASSERT(info); + if (!info) + return; Request *request = info->request; MaliC55CameraData *data = cameraData(request->_d()->camera()); @@ -1602,32 +1721,80 @@ void PipelineHandlerMaliC55::statsBufferReady(FrameBuffer *buffer) sensorControls); } +void PipelineHandlerMaliC55::cruBufferReady(FrameBuffer *buffer) +{ + /* + * If the buffer has been cancelled, do not ask the IPA to prepare + * parameters. + * + * The Request this cancelled buffer belongs to will be handled by + * imageBufferReady() as we have queued buffers to the ISP capture + * devices at the same time we have queued this buffer to the CRU + * when running in m2m mode. + */ + if (buffer->metadata().status == FrameMetadata::FrameCancelled) + return; + + MaliC55FrameInfo *info = findFrameInfo(buffer); + ASSERT(info); + + Request *request = info->request; + request->_d()->metadata().set(controls::SensorTimestamp, + buffer->metadata().timestamp); + + /* Ought we do something with the sensor's controls here...? */ + MaliC55CameraData *data = cameraData(request->_d()->camera()); + data->ipa_->fillParams(request->sequence(), info->paramBuffer->cookie()); +} + void PipelineHandlerMaliC55::paramsComputed(unsigned int requestId, uint32_t bytesused) { - MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId]; + auto it = frameInfoMap_.find(requestId); + if (it == frameInfoMap_.end()) + return; + + MaliC55FrameInfo &frameInfo = it->second; Request *request = frameInfo.request; + if (!request) + return; + MaliC55CameraData *data = cameraData(request->_d()->camera()); /* * Queue buffers for stats and params, then queue buffers to the capture - * video devices. + * video devices if we're running in Inline mode or with the TPG. + * + * If we're running in M2M buffers have been queued to the capture + * devices at queuePendingRequests() time and here we only have to queue + * buffers to the IVC input to start a transfer. */ frameInfo.paramBuffer->_d()->metadata().planes()[0].bytesused = bytesused; params_->queueBuffer(frameInfo.paramBuffer); stats_->queueBuffer(frameInfo.statBuffer); - for (auto &[stream, buffer] : request->buffers()) { - MaliC55Pipe *pipe = pipeFromStream(data, stream); + if (!std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) { + for (auto &[stream, buffer] : request->buffers()) { + MaliC55Pipe *pipe = pipeFromStream(data, stream); - pipe->cap->queueBuffer(buffer); + pipe->cap->queueBuffer(buffer); + } + } else { + ivc_->queueBuffer(frameInfo.rawBuffer); + frameInfo.rawBuffer = nullptr; } } void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId, const ControlList &metadata) { - MaliC55FrameInfo &frameInfo = frameInfoMap_[requestId]; + auto it = frameInfoMap_.find(requestId); + if (it == frameInfoMap_.end()) + return; + + MaliC55FrameInfo &frameInfo = it->second; + if (!frameInfo.request) + return; frameInfo.statsDone = true; frameInfo.request->_d()->metadata().merge(metadata); @@ -1757,6 +1924,9 @@ bool PipelineHandlerMaliC55::registerMemoryInputCamera() ivc_->bufferReady.connect(mem->cru_.get(), &RZG2LCRU::returnBuffer); + V4L2VideoDevice *cruOutput = mem->cru_->output(); + cruOutput->bufferReady.connect(this, &PipelineHandlerMaliC55::cruBufferReady); + return registerMaliCamera(std::move(data), sensor->device()->entity()->name()); }