[{"id":38458,"web_url":"https://patchwork.libcamera.org/comment/38458/","msgid":"<0ea76bbe-06e9-4213-9acd-4e7ef72ed8f2@ideasonboard.com>","date":"2026-03-31T08:47:54","subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta:\n> From: Daniel Scally <dan.scally@ideasonboard.com>\n> \n> Plumb in the MaliC55 pipeline handler support for capturing frames\n> from memory using the CRU.\n> \n> Introduce a data flow which uses the CRU to feed the ISP through\n> the IVC.\n> \n> In detail:\n> \n> - push incoming request to a pending queue until a buffer from the CRU\n>    is available\n> - delay the call to ipa_->fillParams() to the CRU buffer ready even\n\nevent ?\n\n\n> - once the IPA has computed parameters feed the ISP through the IVC\n>    with buffers from the CRU, params and statistics.\n> - Handle completion of in-flight requests: in m2m mode buffers are\n>    queued earlier to the ISP capture devices. If the camera is stopped\n>    these buffers are returned earlier in error state: complete the\n>    Request bypassing the IPA operations.\n> - As cancelled Requests might now be completed earlier then IPA events,\n>    modify the IPA event handler to ignore Requests that have been\n>    completed already\n> \n> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++----\n>   1 file changed, 207 insertions(+), 37 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index be3e38da95ab..10848c412849 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -21,6 +21,7 @@\n>   #include <libcamera/base/utils.h>\n>   \n>   #include <libcamera/camera.h>\n> +#include <libcamera/controls.h>\n>   #include <libcamera/formats.h>\n>   #include <libcamera/geometry.h>\n>   #include <libcamera/property_ids.h>\n> @@ -88,6 +89,7 @@ struct MaliC55FrameInfo {\n>   \n>   \tFrameBuffer *paramBuffer;\n>   \tFrameBuffer *statBuffer;\n> +\tFrameBuffer *rawBuffer;\n>   \n>   \tbool paramsDone;\n>   \tbool statsDone;\n> @@ -691,6 +693,7 @@ public:\n>   \tvoid imageBufferReady(FrameBuffer *buffer);\n>   \tvoid paramsBufferReady(FrameBuffer *buffer);\n>   \tvoid statsBufferReady(FrameBuffer *buffer);\n> +\tvoid cruBufferReady(FrameBuffer *buffer);\n>   \tvoid paramsComputed(unsigned int requestId, uint32_t bytesused);\n>   \tvoid statsProcessed(unsigned int requestId, const ControlList &metadata);\n>   \n> @@ -739,7 +742,7 @@ private:\n>   \n>   \tMaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer);\n>   \tMaliC55FrameInfo *findFrameInfo(Request *request);\n> -\tvoid tryComplete(MaliC55FrameInfo *info);\n> +\tvoid tryComplete(MaliC55FrameInfo *info, bool cancelled = false);\n>   \n>   \tint configureRawStream(MaliC55CameraData *data,\n>   \t\t\t       const StreamConfiguration &config,\n> @@ -747,6 +750,11 @@ private:\n>   \tint configureProcessedStream(MaliC55CameraData *data,\n>   \t\t\t\t     const StreamConfiguration &config,\n>   \t\t\t\t     V4L2SubdeviceFormat &subdevFormat);\n> +\tvoid cancelPendingRequests();\n> +\n> +\tMaliC55FrameInfo *\n> +\tprepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr);\n> +\tint queuePendingRequests(MaliC55CameraData *data);\n>   \n>   \tvoid applyScalerCrop(Camera *camera, const ControlList &controls);\n>   \n> @@ -772,6 +780,9 @@ private:\n>   \n>   \tstd::map<unsigned int, MaliC55FrameInfo> frameInfoMap_;\n>   \n> +\t/* Requests for which no buffer has been queued to the CRU device yet. */\n> +\tstd::queue<Request *> pendingRequests_;\n> +\n>   \tstd::array<MaliC55Pipe, MaliC55NumPipes> pipes_;\n>   \n>   \tbool dsFitted_;\n> @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera)\n>   \tif (params_->releaseBuffers())\n>   \t\tLOG(MaliC55, Error) << \"Failed to release params buffers\";\n>   \n> +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> +\t\tif (ivc_->releaseBuffers())\n> +\t\t\tLOG(MaliC55, Error) << \"Failed to release input buffers\";\n> +\t\tif (mem->cru_->freeBuffers())\n> +\t\t\tLOG(MaliC55, Error) << \"Failed to release CRU buffers\";\n> +\t}\n> +\n>   \treturn;\n>   }\n>   \n> @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n>   \t\t}\n>   \t};\n>   \n> +\tif (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {\n> +\t\tret = ivc_->importBuffers(RZG2LCRU::kBufferCount);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\t}\n> +\n>   \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n>   \tif (ret < 0)\n>   \t\treturn ret;\n> @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control\n>   \tif (ret)\n>   \t\treturn ret;\n>   \n> +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> +\t\tret = mem->cru_->start();\n> +\t\tif (ret) {\n> +\t\t\tLOG(MaliC55, Error)\n> +\t\t\t\t<< \"Failed to start CRU \" << camera->id();\n> +\t\t\tfreeBuffers(camera);\n\nMinor thing, but I think reworking these with `utils::scope_exit` is a worthwhile change\nif you plan to send a new version.\n\n\n> +\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tret = ivc_->streamOn();\n> +\t\tif (ret) {\n> +\t\t\tLOG(MaliC55, Error)\n> +\t\t\t\t<< \"Failed to start IVC\" << camera->id();\n> +\t\t\tfreeBuffers(camera);\n> +\t\t\treturn ret;\n> +\t\t}\n> +\t}\n> +\n>   \tif (data->ipa_) {\n>   \t\tret = data->ipa_->start();\n>   \t\tif (ret) {\n> @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control\n>   \treturn 0;\n>   }\n>   \n> +void PipelineHandlerMaliC55::cancelPendingRequests()\n> +{\n> +\twhile (!pendingRequests_.empty()) {\n> +\t\tcancelRequest(pendingRequests_.front());\n> +\t\tpendingRequests_.pop();\n> +\t}\n> +}\n> +\n>   void PipelineHandlerMaliC55::stopDevice(Camera *camera)\n>   {\n>   \tMaliC55CameraData *data = cameraData(camera);\n>   \n>   \tisp_->setFrameStartEnabled(false);\n>   \n> +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> +\t\tivc_->streamOff();\n> +\t\tmem->cru_->stop();\n> +\t}\n> +\n>   \tfor (MaliC55Pipe &pipe : pipes_) {\n>   \t\tif (!pipe.stream)\n>   \t\t\tcontinue;\n> @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera)\n>   \tif (data->ipa_)\n>   \t\tdata->ipa_->stop();\n>   \tfreeBuffers(camera);\n> +\n> +\tcancelPendingRequests();\n>   }\n>   \n>   void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n> @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n>   \t}\n>   }\n>   \n> +MaliC55FrameInfo *\n> +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru)\n> +{\n> +\tif (availableStatsBuffers_.empty()) {\n> +\t\tLOG(MaliC55, Error) << \"Stats buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tif (availableParamsBuffers_.empty()) {\n> +\t\tLOG(MaliC55, Error) << \"Params buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tMaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()];\n\nI think it would be better to do it last, not to leave an \"empty\" frame info\nobject in the map.\n\n\n> +\tframeInfo.request = request;\n> +\n> +\tif (cru) {\n> +\t\tframeInfo.rawBuffer = cru->queueBuffer(request);\n> +\t\tif (!frameInfo.rawBuffer)\n\nI'm wondering, can it not be guaranteed that there is always an available buffer?\nWouldn't it be sufficient to ensure that at most 4 requests are queued to the\npipeline handler? In that case I believe there should always be an available cru buffer.\n\n\n> +\t\t\treturn nullptr;\n> +\t}\n> +\n> +\tframeInfo.statBuffer = availableStatsBuffers_.front();\n> +\tavailableStatsBuffers_.pop();\n> +\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n> +\tavailableParamsBuffers_.pop();\n> +\n> +\tframeInfo.paramsDone = false;\n> +\tframeInfo.statsDone = false;\n> +\n> +\treturn &frameInfo;\n> +}\n> +\n> +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data)\n\nShould this not be called when a CRU buffer becomes available? If the queue is\nonly drained if a new request is queued, then I feel like things can get stuck\nif the application is stopping and is waiting for the pending requests to complete.\nI suppose it's fine now because everything uses hard-coded 4 buffers / requests.\n\n\n> +{\n> +\tauto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_);\n> +\tASSERT(mem);\n> +\n> +\twhile (!pendingRequests_.empty()) {\n> +\t\tRequest *request = pendingRequests_.front();\n> +\n> +\t\tif (!prepareFrameInfo(request, mem->cru_.get()))\n> +\t\t\treturn -ENOENT;\n> +\n> +\t\tfor (auto &[stream, buffer] : request->buffers()) {\n> +\t\t\tMaliC55Pipe *pipe = pipeFromStream(data, stream);\n> +\n> +\t\t\tpipe->cap->queueBuffer(buffer);\n> +\t\t}\n> +\n> +\t\tdata->ipa_->queueRequest(request->sequence(), request->controls());\n> +\n> +\t\tpendingRequests_.pop();\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n>   int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)\n>   {\n>   \tMaliC55CameraData *data = cameraData(camera);\n>   \n> +\t/*\n> +\t * If we're in memory input mode, we need to pop the requests onto the\n\npush ?\n\n\n> +\t * pending list until a CRU buffer is ready...otherwise we can just do\n> +\t * everything immediately.\n> +\t */\n\nOr maybe I was wrong above. But this says \"pending list until a CRU buffer is ready\",\nso if no CRU buffer is available, shouldn't it just push to `pendingRequests_` and\nreturn 0? Because this now returns non-zero, and that will cancel the request.\n\n\n> +\tif (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {\n> +\t\tpendingRequests_.push(request);\n> +\n> +\t\tint ret = queuePendingRequests(data);\n> +\t\tif (ret) {\n> +\t\t\tpendingRequests_.pop();\n> +\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n>   \t/* Do not run the IPA if the TPG is in use. */\n>   \tif (!data->ipa_) {\n>   \t\tMaliC55FrameInfo frameInfo;\n> @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)\n>   \t\treturn 0;\n>   \t}\n>   \n> -\tif (availableStatsBuffers_.empty()) {\n> -\t\tLOG(MaliC55, Error) << \"Stats buffer underrun\";\n> -\t\treturn -ENOENT;\n> -\t}\n> -\n> -\tif (availableParamsBuffers_.empty()) {\n> -\t\tLOG(MaliC55, Error) << \"Params buffer underrun\";\n> +\tauto frameInfo = prepareFrameInfo(request);\n> +\tif (!frameInfo)\n>   \t\treturn -ENOENT;\n> -\t}\n> -\n> -\tMaliC55FrameInfo frameInfo;\n> -\tframeInfo.request = request;\n> -\n> -\tframeInfo.statBuffer = availableStatsBuffers_.front();\n> -\tavailableStatsBuffers_.pop();\n> -\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n> -\tavailableParamsBuffers_.pop();\n> -\n> -\tframeInfo.paramsDone = false;\n> -\tframeInfo.statsDone = false;\n> -\n> -\tframeInfoMap_[request->sequence()] = frameInfo;\n>   \n>   \tdata->ipa_->queueRequest(request->sequence(), request->controls());\n>   \tdata->ipa_->fillParams(request->sequence(),\n> -\t\t\t       frameInfo.paramBuffer->cookie());\n> +\t\t\t       frameInfo->paramBuffer->cookie());\n>   \n>   \treturn 0;\n>   }\n> [...]","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A8F09BDCBD\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 31 Mar 2026 08:47:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC3D162D0E;\n\tTue, 31 Mar 2026 10:47:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C188E62CFD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 31 Mar 2026 10:47:57 +0200 (CEST)","from [192.168.33.35] (185.221.143.129.nat.pool.zt.hu\n\t[185.221.143.129])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 57256103F;\n\tTue, 31 Mar 2026 10:46:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sUOumHx8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1774946795;\n\tbh=QYLt0HyspiIvPxBH6eDCUMj6AT2fZ7YTaL32w9kJTmI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=sUOumHx8eM4kBplm8unmslwSTDf7XAwK+ZgOLGcn6iKajoJnTYWVDlX/xpoF6pzwg\n\tkjjt+R9lWoS7Afaq6wGBnOFBVZLoqWLHHKejJYyYQtOFEn24HQmtSeIf8lxG1KmWsI\n\tBJSChN6gS6wk+hul0Hf3OgtNcCD/tY2Rac1zaGa4=","Message-ID":"<0ea76bbe-06e9-4213-9acd-4e7ef72ed8f2@ideasonboard.com>","Date":"Tue, 31 Mar 2026 10:47:54 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tDaniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260325-mali-cru-v6-0-b16b0c49819a@ideasonboard.com>\n\t<20260325-mali-cru-v6-6-b16b0c49819a@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260325-mali-cru-v6-6-b16b0c49819a@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38461,"web_url":"https://patchwork.libcamera.org/comment/38461/","msgid":"<acvFNhxivkgXIMs_@zed>","date":"2026-03-31T13:12:08","subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote:\n> Hi\n>\n> 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta:\n> > From: Daniel Scally <dan.scally@ideasonboard.com>\n> >\n> > Plumb in the MaliC55 pipeline handler support for capturing frames\n> > from memory using the CRU.\n> >\n> > Introduce a data flow which uses the CRU to feed the ISP through\n> > the IVC.\n> >\n> > In detail:\n> >\n> > - push incoming request to a pending queue until a buffer from the CRU\n> >    is available\n> > - delay the call to ipa_->fillParams() to the CRU buffer ready even\n>\n> event ?\n>\n\nAh yes\n\n>\n> > - once the IPA has computed parameters feed the ISP through the IVC\n> >    with buffers from the CRU, params and statistics.\n> > - Handle completion of in-flight requests: in m2m mode buffers are\n> >    queued earlier to the ISP capture devices. If the camera is stopped\n> >    these buffers are returned earlier in error state: complete the\n> >    Request bypassing the IPA operations.\n> > - As cancelled Requests might now be completed earlier then IPA events,\n> >    modify the IPA event handler to ignore Requests that have been\n> >    completed already\n> >\n> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++----\n> >   1 file changed, 207 insertions(+), 37 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > index be3e38da95ab..10848c412849 100644\n> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > @@ -21,6 +21,7 @@\n> >   #include <libcamera/base/utils.h>\n> >   #include <libcamera/camera.h>\n> > +#include <libcamera/controls.h>\n> >   #include <libcamera/formats.h>\n> >   #include <libcamera/geometry.h>\n> >   #include <libcamera/property_ids.h>\n> > @@ -88,6 +89,7 @@ struct MaliC55FrameInfo {\n> >   \tFrameBuffer *paramBuffer;\n> >   \tFrameBuffer *statBuffer;\n> > +\tFrameBuffer *rawBuffer;\n> >   \tbool paramsDone;\n> >   \tbool statsDone;\n> > @@ -691,6 +693,7 @@ public:\n> >   \tvoid imageBufferReady(FrameBuffer *buffer);\n> >   \tvoid paramsBufferReady(FrameBuffer *buffer);\n> >   \tvoid statsBufferReady(FrameBuffer *buffer);\n> > +\tvoid cruBufferReady(FrameBuffer *buffer);\n> >   \tvoid paramsComputed(unsigned int requestId, uint32_t bytesused);\n> >   \tvoid statsProcessed(unsigned int requestId, const ControlList &metadata);\n> > @@ -739,7 +742,7 @@ private:\n> >   \tMaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer);\n> >   \tMaliC55FrameInfo *findFrameInfo(Request *request);\n> > -\tvoid tryComplete(MaliC55FrameInfo *info);\n> > +\tvoid tryComplete(MaliC55FrameInfo *info, bool cancelled = false);\n> >   \tint configureRawStream(MaliC55CameraData *data,\n> >   \t\t\t       const StreamConfiguration &config,\n> > @@ -747,6 +750,11 @@ private:\n> >   \tint configureProcessedStream(MaliC55CameraData *data,\n> >   \t\t\t\t     const StreamConfiguration &config,\n> >   \t\t\t\t     V4L2SubdeviceFormat &subdevFormat);\n> > +\tvoid cancelPendingRequests();\n> > +\n> > +\tMaliC55FrameInfo *\n> > +\tprepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr);\n> > +\tint queuePendingRequests(MaliC55CameraData *data);\n> >   \tvoid applyScalerCrop(Camera *camera, const ControlList &controls);\n> > @@ -772,6 +780,9 @@ private:\n> >   \tstd::map<unsigned int, MaliC55FrameInfo> frameInfoMap_;\n> > +\t/* Requests for which no buffer has been queued to the CRU device yet. */\n> > +\tstd::queue<Request *> pendingRequests_;\n> > +\n> >   \tstd::array<MaliC55Pipe, MaliC55NumPipes> pipes_;\n> >   \tbool dsFitted_;\n> > @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera)\n> >   \tif (params_->releaseBuffers())\n> >   \t\tLOG(MaliC55, Error) << \"Failed to release params buffers\";\n> > +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> > +\t\tif (ivc_->releaseBuffers())\n> > +\t\t\tLOG(MaliC55, Error) << \"Failed to release input buffers\";\n> > +\t\tif (mem->cru_->freeBuffers())\n> > +\t\t\tLOG(MaliC55, Error) << \"Failed to release CRU buffers\";\n> > +\t}\n> > +\n> >   \treturn;\n> >   }\n> > @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n> >   \t\t}\n> >   \t};\n> > +\tif (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {\n> > +\t\tret = ivc_->importBuffers(RZG2LCRU::kBufferCount);\n> > +\t\tif (ret < 0)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> >   \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n> >   \tif (ret < 0)\n> >   \t\treturn ret;\n> > @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control\n> >   \tif (ret)\n> >   \t\treturn ret;\n> > +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> > +\t\tret = mem->cru_->start();\n> > +\t\tif (ret) {\n> > +\t\t\tLOG(MaliC55, Error)\n> > +\t\t\t\t<< \"Failed to start CRU \" << camera->id();\n> > +\t\t\tfreeBuffers(camera);\n>\n> Minor thing, but I think reworking these with `utils::scope_exit` is a worthwhile change\n> if you plan to send a new version.\n>\n\nCan we consider doing this on top as the existing error paths would\nneed to be reworked otherwise ?\n\n>\n> > +\t\t\treturn ret;\n> > +\t\t}\n> > +\n> > +\t\tret = ivc_->streamOn();\n> > +\t\tif (ret) {\n> > +\t\t\tLOG(MaliC55, Error)\n> > +\t\t\t\t<< \"Failed to start IVC\" << camera->id();\n> > +\t\t\tfreeBuffers(camera);\n> > +\t\t\treturn ret;\n> > +\t\t}\n> > +\t}\n> > +\n> >   \tif (data->ipa_) {\n> >   \t\tret = data->ipa_->start();\n> >   \t\tif (ret) {\n> > @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control\n> >   \treturn 0;\n> >   }\n> > +void PipelineHandlerMaliC55::cancelPendingRequests()\n> > +{\n> > +\twhile (!pendingRequests_.empty()) {\n> > +\t\tcancelRequest(pendingRequests_.front());\n> > +\t\tpendingRequests_.pop();\n> > +\t}\n> > +}\n> > +\n> >   void PipelineHandlerMaliC55::stopDevice(Camera *camera)\n> >   {\n> >   \tMaliC55CameraData *data = cameraData(camera);\n> >   \tisp_->setFrameStartEnabled(false);\n> > +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> > +\t\tivc_->streamOff();\n> > +\t\tmem->cru_->stop();\n> > +\t}\n> > +\n> >   \tfor (MaliC55Pipe &pipe : pipes_) {\n> >   \t\tif (!pipe.stream)\n> >   \t\t\tcontinue;\n> > @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera)\n> >   \tif (data->ipa_)\n> >   \t\tdata->ipa_->stop();\n> >   \tfreeBuffers(camera);\n> > +\n> > +\tcancelPendingRequests();\n> >   }\n> >   void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n> > @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n> >   \t}\n> >   }\n> > +MaliC55FrameInfo *\n> > +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru)\n> > +{\n> > +\tif (availableStatsBuffers_.empty()) {\n> > +\t\tLOG(MaliC55, Error) << \"Stats buffer underrun\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tif (availableParamsBuffers_.empty()) {\n> > +\t\tLOG(MaliC55, Error) << \"Params buffer underrun\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tMaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()];\n>\n> I think it would be better to do it last, not to leave an \"empty\" frame info\n> object in the map.\n\nAs the only failure point is the below if (!frameInfo.rawBuffer) can I\ndo after that ?\n\n>\n>\n> > +\tframeInfo.request = request;\n> > +\n> > +\tif (cru) {\n> > +\t\tframeInfo.rawBuffer = cru->queueBuffer(request);\n> > +\t\tif (!frameInfo.rawBuffer)\n>\n> I'm wondering, can it not be guaranteed that there is always an available buffer?\n> Wouldn't it be sufficient to ensure that at most 4 requests are queued to the\n> pipeline handler? In that case I believe there should always be an available cru buffer.\n\nShould I initialize PipelineHandler() with maxQueuedRequestsDevice ==\nkBufferCount ?\n\n>\n>\n> > +\t\t\treturn nullptr;\n> > +\t}\n> > +\n> > +\tframeInfo.statBuffer = availableStatsBuffers_.front();\n> > +\tavailableStatsBuffers_.pop();\n> > +\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n> > +\tavailableParamsBuffers_.pop();\n> > +\n> > +\tframeInfo.paramsDone = false;\n> > +\tframeInfo.statsDone = false;\n> > +\n> > +\treturn &frameInfo;\n> > +}\n> > +\n> > +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data)\n>\n> Should this not be called when a CRU buffer becomes available? If the queue is\n> only drained if a new request is queued, then I feel like things can get stuck\n> if the application is stopping and is waiting for the pending requests to complete.\n> I suppose it's fine now because everything uses hard-coded 4 buffers / requests.\n>\n\nmmm, you're right. We only try to run when the application queue a new\nRequest. Should I create an handler for ivc_->bufferReady (which now\nsimply returns the buffer to the cru) and call\nPipelineHandlerMaliC55::queuePendingRequests() from there ?\n\n>\n> > +{\n> > +\tauto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_);\n> > +\tASSERT(mem);\n> > +\n> > +\twhile (!pendingRequests_.empty()) {\n> > +\t\tRequest *request = pendingRequests_.front();\n> > +\n> > +\t\tif (!prepareFrameInfo(request, mem->cru_.get()))\n> > +\t\t\treturn -ENOENT;\n> > +\n> > +\t\tfor (auto &[stream, buffer] : request->buffers()) {\n> > +\t\t\tMaliC55Pipe *pipe = pipeFromStream(data, stream);\n> > +\n> > +\t\t\tpipe->cap->queueBuffer(buffer);\n> > +\t\t}\n> > +\n> > +\t\tdata->ipa_->queueRequest(request->sequence(), request->controls());\n> > +\n> > +\t\tpendingRequests_.pop();\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> >   int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)\n> >   {\n> >   \tMaliC55CameraData *data = cameraData(camera);\n> > +\t/*\n> > +\t * If we're in memory input mode, we need to pop the requests onto the\n>\n> push ?\n\nups\n\n>\n>\n> > +\t * pending list until a CRU buffer is ready...otherwise we can just do\n> > +\t * everything immediately.\n> > +\t */\n>\n> Or maybe I was wrong above. But this says \"pending list until a CRU buffer is ready\",\n> so if no CRU buffer is available, shouldn't it just push to `pendingRequests_` and\n> return 0? Because this now returns non-zero, and that will cancel the request.\n>\n\nI actually think the below error path handling is wrong now that I\nlook at it more closely\n\n>\n> > +\tif (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {\n> > +\t\tpendingRequests_.push(request);\n> > +\n> > +\t\tint ret = queuePendingRequests(data);\n> > +\t\tif (ret) {\n> > +\t\t\tpendingRequests_.pop();\n\nRequests are popped from the queue in queuePendingRequests() only at\nthe end of the loop, so if we error out, there isn't any need to\nactually pop them here\n\n> > +\t\t\treturn ret;\n\nAnd, as you suggested, if we return an error the request is cancelled,\nso we should probably return 0 here, leave the Request in the queue\nand let the next queuePendingRequests() consume it ?\n\n> > +\t\t}\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> >   \t/* Do not run the IPA if the TPG is in use. */\n> >   \tif (!data->ipa_) {\n> >   \t\tMaliC55FrameInfo frameInfo;\n> > @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)\n> >   \t\treturn 0;\n> >   \t}\n> > -\tif (availableStatsBuffers_.empty()) {\n> > -\t\tLOG(MaliC55, Error) << \"Stats buffer underrun\";\n> > -\t\treturn -ENOENT;\n> > -\t}\n> > -\n> > -\tif (availableParamsBuffers_.empty()) {\n> > -\t\tLOG(MaliC55, Error) << \"Params buffer underrun\";\n> > +\tauto frameInfo = prepareFrameInfo(request);\n> > +\tif (!frameInfo)\n> >   \t\treturn -ENOENT;\n> > -\t}\n> > -\n> > -\tMaliC55FrameInfo frameInfo;\n> > -\tframeInfo.request = request;\n> > -\n> > -\tframeInfo.statBuffer = availableStatsBuffers_.front();\n> > -\tavailableStatsBuffers_.pop();\n> > -\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n> > -\tavailableParamsBuffers_.pop();\n> > -\n> > -\tframeInfo.paramsDone = false;\n> > -\tframeInfo.statsDone = false;\n> > -\n> > -\tframeInfoMap_[request->sequence()] = frameInfo;\n> >   \tdata->ipa_->queueRequest(request->sequence(), request->controls());\n> >   \tdata->ipa_->fillParams(request->sequence(),\n> > -\t\t\t       frameInfo.paramBuffer->cookie());\n> > +\t\t\t       frameInfo->paramBuffer->cookie());\n> >   \treturn 0;\n> >   }\n> > [...]","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 61A48BDCBD\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 31 Mar 2026 13:12:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 89F4862D0C;\n\tTue, 31 Mar 2026 15:12:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BA266274D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 31 Mar 2026 15:12:11 +0200 (CEST)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D64F2CE7;\n\tTue, 31 Mar 2026 15:10:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MBtxFXGJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1774962648;\n\tbh=N2LkgZh+pA89LiunutJIpcY1NIiS0b5rXczR1ohiNM0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MBtxFXGJN1FQfelWLT77x3+A/j8Mffx3yGI8E9W0jfP2/lh5QQvNfhcHVVX7k/hPl\n\twXlaoYErRysrx5MLMBLgXHaOjo+wf8cQdoge5rZaeVzOq/I2R9jM4uCIEOYMVoEKTe\n\toCuBqFgyVStcJRu9EqlosDrVVDb+N8tyrgTMx6w8=","Date":"Tue, 31 Mar 2026 15:12:08 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tDaniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","Message-ID":"<acvFNhxivkgXIMs_@zed>","References":"<20260325-mali-cru-v6-0-b16b0c49819a@ideasonboard.com>\n\t<20260325-mali-cru-v6-6-b16b0c49819a@ideasonboard.com>\n\t<0ea76bbe-06e9-4213-9acd-4e7ef72ed8f2@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<0ea76bbe-06e9-4213-9acd-4e7ef72ed8f2@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38464,"web_url":"https://patchwork.libcamera.org/comment/38464/","msgid":"<9859dd2c-2a51-4db9-9304-e41e99e031ad@ideasonboard.com>","date":"2026-03-31T14:10:53","subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 03. 31. 15:12 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote:\n>> Hi\n>>\n>> 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta:\n>>> From: Daniel Scally <dan.scally@ideasonboard.com>\n>>>\n>>> Plumb in the MaliC55 pipeline handler support for capturing frames\n>>> from memory using the CRU.\n>>>\n>>> Introduce a data flow which uses the CRU to feed the ISP through\n>>> the IVC.\n>>>\n>>> In detail:\n>>>\n>>> - push incoming request to a pending queue until a buffer from the CRU\n>>>     is available\n>>> - delay the call to ipa_->fillParams() to the CRU buffer ready even\n>>\n>> event ?\n>>\n> \n> Ah yes\n> \n>>\n>>> - once the IPA has computed parameters feed the ISP through the IVC\n>>>     with buffers from the CRU, params and statistics.\n>>> - Handle completion of in-flight requests: in m2m mode buffers are\n>>>     queued earlier to the ISP capture devices. If the camera is stopped\n>>>     these buffers are returned earlier in error state: complete the\n>>>     Request bypassing the IPA operations.\n>>> - As cancelled Requests might now be completed earlier then IPA events,\n>>>     modify the IPA event handler to ignore Requests that have been\n>>>     completed already\n>>>\n>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>>> ---\n>>>    src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++----\n>>>    1 file changed, 207 insertions(+), 37 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> index be3e38da95ab..10848c412849 100644\n>>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> @@ -21,6 +21,7 @@\n>>>    #include <libcamera/base/utils.h>\n>>>    #include <libcamera/camera.h>\n>>> +#include <libcamera/controls.h>\n>>>    #include <libcamera/formats.h>\n>>>    #include <libcamera/geometry.h>\n>>>    #include <libcamera/property_ids.h>\n>>> @@ -88,6 +89,7 @@ struct MaliC55FrameInfo {\n>>>    \tFrameBuffer *paramBuffer;\n>>>    \tFrameBuffer *statBuffer;\n>>> +\tFrameBuffer *rawBuffer;\n>>>    \tbool paramsDone;\n>>>    \tbool statsDone;\n>>> @@ -691,6 +693,7 @@ public:\n>>>    \tvoid imageBufferReady(FrameBuffer *buffer);\n>>>    \tvoid paramsBufferReady(FrameBuffer *buffer);\n>>>    \tvoid statsBufferReady(FrameBuffer *buffer);\n>>> +\tvoid cruBufferReady(FrameBuffer *buffer);\n>>>    \tvoid paramsComputed(unsigned int requestId, uint32_t bytesused);\n>>>    \tvoid statsProcessed(unsigned int requestId, const ControlList &metadata);\n>>> @@ -739,7 +742,7 @@ private:\n>>>    \tMaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer);\n>>>    \tMaliC55FrameInfo *findFrameInfo(Request *request);\n>>> -\tvoid tryComplete(MaliC55FrameInfo *info);\n>>> +\tvoid tryComplete(MaliC55FrameInfo *info, bool cancelled = false);\n>>>    \tint configureRawStream(MaliC55CameraData *data,\n>>>    \t\t\t       const StreamConfiguration &config,\n>>> @@ -747,6 +750,11 @@ private:\n>>>    \tint configureProcessedStream(MaliC55CameraData *data,\n>>>    \t\t\t\t     const StreamConfiguration &config,\n>>>    \t\t\t\t     V4L2SubdeviceFormat &subdevFormat);\n>>> +\tvoid cancelPendingRequests();\n>>> +\n>>> +\tMaliC55FrameInfo *\n>>> +\tprepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr);\n>>> +\tint queuePendingRequests(MaliC55CameraData *data);\n>>>    \tvoid applyScalerCrop(Camera *camera, const ControlList &controls);\n>>> @@ -772,6 +780,9 @@ private:\n>>>    \tstd::map<unsigned int, MaliC55FrameInfo> frameInfoMap_;\n>>> +\t/* Requests for which no buffer has been queued to the CRU device yet. */\n>>> +\tstd::queue<Request *> pendingRequests_;\n>>> +\n>>>    \tstd::array<MaliC55Pipe, MaliC55NumPipes> pipes_;\n>>>    \tbool dsFitted_;\n>>> @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera)\n>>>    \tif (params_->releaseBuffers())\n>>>    \t\tLOG(MaliC55, Error) << \"Failed to release params buffers\";\n>>> +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n>>> +\t\tif (ivc_->releaseBuffers())\n>>> +\t\t\tLOG(MaliC55, Error) << \"Failed to release input buffers\";\n>>> +\t\tif (mem->cru_->freeBuffers())\n>>> +\t\t\tLOG(MaliC55, Error) << \"Failed to release CRU buffers\";\n>>> +\t}\n>>> +\n>>>    \treturn;\n>>>    }\n>>> @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n>>>    \t\t}\n>>>    \t};\n>>> +\tif (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {\n>>> +\t\tret = ivc_->importBuffers(RZG2LCRU::kBufferCount);\n>>> +\t\tif (ret < 0)\n>>> +\t\t\treturn ret;\n>>> +\t}\n>>> +\n>>>    \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n>>>    \tif (ret < 0)\n>>>    \t\treturn ret;\n>>> @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control\n>>>    \tif (ret)\n>>>    \t\treturn ret;\n>>> +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n>>> +\t\tret = mem->cru_->start();\n>>> +\t\tif (ret) {\n>>> +\t\t\tLOG(MaliC55, Error)\n>>> +\t\t\t\t<< \"Failed to start CRU \" << camera->id();\n>>> +\t\t\tfreeBuffers(camera);\n>>\n>> Minor thing, but I think reworking these with `utils::scope_exit` is a worthwhile change\n>> if you plan to send a new version.\n>>\n> \n> Can we consider doing this on top as the existing error paths would\n> need to be reworked otherwise ?\n\nYes, certainly.\n\n\n> \n>>\n>>> +\t\t\treturn ret;\n>>> +\t\t}\n>>> +\n>>> +\t\tret = ivc_->streamOn();\n>>> +\t\tif (ret) {\n>>> +\t\t\tLOG(MaliC55, Error)\n>>> +\t\t\t\t<< \"Failed to start IVC\" << camera->id();\n>>> +\t\t\tfreeBuffers(camera);\n>>> +\t\t\treturn ret;\n>>> +\t\t}\n>>> +\t}\n>>> +\n>>>    \tif (data->ipa_) {\n>>>    \t\tret = data->ipa_->start();\n>>>    \t\tif (ret) {\n>>> @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control\n>>>    \treturn 0;\n>>>    }\n>>> +void PipelineHandlerMaliC55::cancelPendingRequests()\n>>> +{\n>>> +\twhile (!pendingRequests_.empty()) {\n>>> +\t\tcancelRequest(pendingRequests_.front());\n>>> +\t\tpendingRequests_.pop();\n>>> +\t}\n>>> +}\n>>> +\n>>>    void PipelineHandlerMaliC55::stopDevice(Camera *camera)\n>>>    {\n>>>    \tMaliC55CameraData *data = cameraData(camera);\n>>>    \tisp_->setFrameStartEnabled(false);\n>>> +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n>>> +\t\tivc_->streamOff();\n>>> +\t\tmem->cru_->stop();\n>>> +\t}\n>>> +\n>>>    \tfor (MaliC55Pipe &pipe : pipes_) {\n>>>    \t\tif (!pipe.stream)\n>>>    \t\t\tcontinue;\n>>> @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera)\n>>>    \tif (data->ipa_)\n>>>    \t\tdata->ipa_->stop();\n>>>    \tfreeBuffers(camera);\n>>> +\n>>> +\tcancelPendingRequests();\n>>>    }\n>>>    void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n>>> @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n>>>    \t}\n>>>    }\n>>> +MaliC55FrameInfo *\n>>> +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru)\n>>> +{\n>>> +\tif (availableStatsBuffers_.empty()) {\n>>> +\t\tLOG(MaliC55, Error) << \"Stats buffer underrun\";\n>>> +\t\treturn nullptr;\n>>> +\t}\n>>> +\n>>> +\tif (availableParamsBuffers_.empty()) {\n>>> +\t\tLOG(MaliC55, Error) << \"Params buffer underrun\";\n>>> +\t\treturn nullptr;\n>>> +\t}\n>>> +\n>>> +\tMaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()];\n>>\n>> I think it would be better to do it last, not to leave an \"empty\" frame info\n>> object in the map.\n> \n> As the only failure point is the below if (!frameInfo.rawBuffer) can I\n> do after that ?\n\nYes, or alternatively a temporary `MaliC55FrameInfo` could be filled, which is added\nto the map last. Like it was done previously.\n\n\n> \n>>\n>>\n>>> +\tframeInfo.request = request;\n>>> +\n>>> +\tif (cru) {\n>>> +\t\tframeInfo.rawBuffer = cru->queueBuffer(request);\n>>> +\t\tif (!frameInfo.rawBuffer)\n>>\n>> I'm wondering, can it not be guaranteed that there is always an available buffer?\n>> Wouldn't it be sufficient to ensure that at most 4 requests are queued to the\n>> pipeline handler? In that case I believe there should always be an available cru buffer.\n> \n> Should I initialize PipelineHandler() with maxQueuedRequestsDevice ==\n> kBufferCount ?\n> \n>>\n>>\n>>> +\t\t\treturn nullptr;\n>>> +\t}\n>>> +\n>>> +\tframeInfo.statBuffer = availableStatsBuffers_.front();\n>>> +\tavailableStatsBuffers_.pop();\n>>> +\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n>>> +\tavailableParamsBuffers_.pop();\n>>> +\n>>> +\tframeInfo.paramsDone = false;\n>>> +\tframeInfo.statsDone = false;\n>>> +\n>>> +\treturn &frameInfo;\n>>> +}\n>>> +\n>>> +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data)\n>>\n>> Should this not be called when a CRU buffer becomes available? If the queue is\n>> only drained if a new request is queued, then I feel like things can get stuck\n>> if the application is stopping and is waiting for the pending requests to complete.\n>> I suppose it's fine now because everything uses hard-coded 4 buffers / requests.\n>>\n> \n> mmm, you're right. We only try to run when the application queue a new\n> Request. Should I create an handler for ivc_->bufferReady (which now\n> simply returns the buffer to the cru) and call\n> PipelineHandlerMaliC55::queuePendingRequests() from there ?\n\nI'm looking at 5fb28bfe7438c8ed1029501c3c050e667eb2f507 and 1ed284ba487b7e0903e031c4e6604b601ff040a8.\nI think a similar change could be applied here as well: always allocate a constant number of cru, param,\nor stat buffers, and then set `maxQueuedRequestsDevice` to the minimum of those. If I'm not mistaken,\nthat should guarantee that at least one of all three types of buffers is available in `queueRequestDevice()`.\nThen `pendingRequests_` wouldn't even be needed as far as I can see. Maybe that's the best approach.\n\n> \n>>\n>>> +{\n>>> +\tauto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_);\n>>> +\tASSERT(mem);\n>>> +\n>>> +\twhile (!pendingRequests_.empty()) {\n>>> +\t\tRequest *request = pendingRequests_.front();\n>>> +\n>>> +\t\tif (!prepareFrameInfo(request, mem->cru_.get()))\n>>> +\t\t\treturn -ENOENT;\n>>> +\n>>> +\t\tfor (auto &[stream, buffer] : request->buffers()) {\n>>> +\t\t\tMaliC55Pipe *pipe = pipeFromStream(data, stream);\n>>> +\n>>> +\t\t\tpipe->cap->queueBuffer(buffer);\n>>> +\t\t}\n>>> +\n>>> +\t\tdata->ipa_->queueRequest(request->sequence(), request->controls());\n>>> +\n>>> +\t\tpendingRequests_.pop();\n>>> +\t}\n>>> +\n>>> +\treturn 0;\n>>> +}\n>>> +\n>>>    int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)\n>>>    {\n>>>    \tMaliC55CameraData *data = cameraData(camera);\n>>> +\t/*\n>>> +\t * If we're in memory input mode, we need to pop the requests onto the\n>>\n>> push ?\n> \n> ups\n> \n>>\n>>\n>>> +\t * pending list until a CRU buffer is ready...otherwise we can just do\n>>> +\t * everything immediately.\n>>> +\t */\n>>\n>> Or maybe I was wrong above. But this says \"pending list until a CRU buffer is ready\",\n>> so if no CRU buffer is available, shouldn't it just push to `pendingRequests_` and\n>> return 0? Because this now returns non-zero, and that will cancel the request.\n>>\n> \n> I actually think the below error path handling is wrong now that I\n> look at it more closely\n\nAhh, right. push() appends to the end, but pop() consumes from the front.\n\n\n> \n>>\n>>> +\tif (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {\n>>> +\t\tpendingRequests_.push(request);\n>>> +\n>>> +\t\tint ret = queuePendingRequests(data);\n>>> +\t\tif (ret) {\n>>> +\t\t\tpendingRequests_.pop();\n> \n> Requests are popped from the queue in queuePendingRequests() only at\n> the end of the loop, so if we error out, there isn't any need to\n> actually pop them here\n> \n>>> +\t\t\treturn ret;\n> \n> And, as you suggested, if we return an error the request is cancelled,\n> so we should probably return 0 here, leave the Request in the queue\n> and let the next queuePendingRequests() consume it ?\n> \n>>> +\t\t}\n>>> +\n>>> +\t\treturn 0;\n>>> +\t}\n>>> +\n>>>    \t/* Do not run the IPA if the TPG is in use. */\n>>>    \tif (!data->ipa_) {\n>>>    \t\tMaliC55FrameInfo frameInfo;\n>>> @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)\n>>>    \t\treturn 0;\n>>>    \t}\n>>> -\tif (availableStatsBuffers_.empty()) {\n>>> -\t\tLOG(MaliC55, Error) << \"Stats buffer underrun\";\n>>> -\t\treturn -ENOENT;\n>>> -\t}\n>>> -\n>>> -\tif (availableParamsBuffers_.empty()) {\n>>> -\t\tLOG(MaliC55, Error) << \"Params buffer underrun\";\n>>> +\tauto frameInfo = prepareFrameInfo(request);\n>>> +\tif (!frameInfo)\n>>>    \t\treturn -ENOENT;\n>>> -\t}\n>>> -\n>>> -\tMaliC55FrameInfo frameInfo;\n>>> -\tframeInfo.request = request;\n>>> -\n>>> -\tframeInfo.statBuffer = availableStatsBuffers_.front();\n>>> -\tavailableStatsBuffers_.pop();\n>>> -\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n>>> -\tavailableParamsBuffers_.pop();\n>>> -\n>>> -\tframeInfo.paramsDone = false;\n>>> -\tframeInfo.statsDone = false;\n>>> -\n>>> -\tframeInfoMap_[request->sequence()] = frameInfo;\n>>>    \tdata->ipa_->queueRequest(request->sequence(), request->controls());\n>>>    \tdata->ipa_->fillParams(request->sequence(),\n>>> -\t\t\t       frameInfo.paramBuffer->cookie());\n>>> +\t\t\t       frameInfo->paramBuffer->cookie());\n>>>    \treturn 0;\n>>>    }\n>>> [...]","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2E25ABDCBD\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 31 Mar 2026 14:11:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69D0862D0F;\n\tTue, 31 Mar 2026 16:10:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB9B26274D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 31 Mar 2026 16:10:57 +0200 (CEST)","from [192.168.33.35] (185.221.143.129.nat.pool.zt.hu\n\t[185.221.143.129])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 33D3720FF;\n\tTue, 31 Mar 2026 16:09:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mmumNk8w\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1774966175;\n\tbh=W/9KZhmdq7Vmw6ETOjPKJKVQxjAza8lBZ8Rcs8AJAE4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=mmumNk8wzlWHMoFzelo5l53NZvuFEIBRwdD/Ak1GHy/RNB9Mee9mZPgcsnirZlPMt\n\tFLPaolgg1JaRNSnqSIKnxHkyLFqAsx2+pb8HKE7YNJzO1G5U+7Q/+J4AF204ktYrG0\n\tZ/v48tflePaAe0nN3WdBAFf/ulUvl57mPoNqc800=","Message-ID":"<9859dd2c-2a51-4db9-9304-e41e99e031ad@ideasonboard.com>","Date":"Tue, 31 Mar 2026 16:10:53 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Daniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260325-mali-cru-v6-0-b16b0c49819a@ideasonboard.com>\n\t<20260325-mali-cru-v6-6-b16b0c49819a@ideasonboard.com>\n\t<0ea76bbe-06e9-4213-9acd-4e7ef72ed8f2@ideasonboard.com>\n\t<acvFNhxivkgXIMs_@zed>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<acvFNhxivkgXIMs_@zed>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38465,"web_url":"https://patchwork.libcamera.org/comment/38465/","msgid":"<acva5yaFiD5k5I4r@zed>","date":"2026-03-31T14:36:24","subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Tue, Mar 31, 2026 at 04:10:53PM +0200, Barnabás Pőcze wrote:\n> 2026. 03. 31. 15:12 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote:\n> > > Hi\n> > >\n> > > 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta:\n> > > > From: Daniel Scally <dan.scally@ideasonboard.com>\n> > > >\n> > > > Plumb in the MaliC55 pipeline handler support for capturing frames\n> > > > from memory using the CRU.\n> > > >\n> > > > Introduce a data flow which uses the CRU to feed the ISP through\n> > > > the IVC.\n> > > >\n> > > > In detail:\n> > > >\n> > > > - push incoming request to a pending queue until a buffer from the CRU\n> > > >     is available\n> > > > - delay the call to ipa_->fillParams() to the CRU buffer ready even\n> > >\n> > > event ?\n> > >\n> >\n> > Ah yes\n> >\n> > >\n> > > > - once the IPA has computed parameters feed the ISP through the IVC\n> > > >     with buffers from the CRU, params and statistics.\n> > > > - Handle completion of in-flight requests: in m2m mode buffers are\n> > > >     queued earlier to the ISP capture devices. If the camera is stopped\n> > > >     these buffers are returned earlier in error state: complete the\n> > > >     Request bypassing the IPA operations.\n> > > > - As cancelled Requests might now be completed earlier then IPA events,\n> > > >     modify the IPA event handler to ignore Requests that have been\n> > > >     completed already\n> > > >\n> > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > > ---\n> > > >    src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++----\n> > > >    1 file changed, 207 insertions(+), 37 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > index be3e38da95ab..10848c412849 100644\n> > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > @@ -21,6 +21,7 @@\n> > > >    #include <libcamera/base/utils.h>\n> > > >    #include <libcamera/camera.h>\n> > > > +#include <libcamera/controls.h>\n> > > >    #include <libcamera/formats.h>\n> > > >    #include <libcamera/geometry.h>\n> > > >    #include <libcamera/property_ids.h>\n> > > > @@ -88,6 +89,7 @@ struct MaliC55FrameInfo {\n> > > >    \tFrameBuffer *paramBuffer;\n> > > >    \tFrameBuffer *statBuffer;\n> > > > +\tFrameBuffer *rawBuffer;\n> > > >    \tbool paramsDone;\n> > > >    \tbool statsDone;\n> > > > @@ -691,6 +693,7 @@ public:\n> > > >    \tvoid imageBufferReady(FrameBuffer *buffer);\n> > > >    \tvoid paramsBufferReady(FrameBuffer *buffer);\n> > > >    \tvoid statsBufferReady(FrameBuffer *buffer);\n> > > > +\tvoid cruBufferReady(FrameBuffer *buffer);\n> > > >    \tvoid paramsComputed(unsigned int requestId, uint32_t bytesused);\n> > > >    \tvoid statsProcessed(unsigned int requestId, const ControlList &metadata);\n> > > > @@ -739,7 +742,7 @@ private:\n> > > >    \tMaliC55FrameInfo *findFrameInfo(FrameBuffer *buffer);\n> > > >    \tMaliC55FrameInfo *findFrameInfo(Request *request);\n> > > > -\tvoid tryComplete(MaliC55FrameInfo *info);\n> > > > +\tvoid tryComplete(MaliC55FrameInfo *info, bool cancelled = false);\n> > > >    \tint configureRawStream(MaliC55CameraData *data,\n> > > >    \t\t\t       const StreamConfiguration &config,\n> > > > @@ -747,6 +750,11 @@ private:\n> > > >    \tint configureProcessedStream(MaliC55CameraData *data,\n> > > >    \t\t\t\t     const StreamConfiguration &config,\n> > > >    \t\t\t\t     V4L2SubdeviceFormat &subdevFormat);\n> > > > +\tvoid cancelPendingRequests();\n> > > > +\n> > > > +\tMaliC55FrameInfo *\n> > > > +\tprepareFrameInfo(Request *request, RZG2LCRU *cru = nullptr);\n> > > > +\tint queuePendingRequests(MaliC55CameraData *data);\n> > > >    \tvoid applyScalerCrop(Camera *camera, const ControlList &controls);\n> > > > @@ -772,6 +780,9 @@ private:\n> > > >    \tstd::map<unsigned int, MaliC55FrameInfo> frameInfoMap_;\n> > > > +\t/* Requests for which no buffer has been queued to the CRU device yet. */\n> > > > +\tstd::queue<Request *> pendingRequests_;\n> > > > +\n> > > >    \tstd::array<MaliC55Pipe, MaliC55NumPipes> pipes_;\n> > > >    \tbool dsFitted_;\n> > > > @@ -1220,6 +1231,13 @@ void PipelineHandlerMaliC55::freeBuffers(Camera *camera)\n> > > >    \tif (params_->releaseBuffers())\n> > > >    \t\tLOG(MaliC55, Error) << \"Failed to release params buffers\";\n> > > > +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> > > > +\t\tif (ivc_->releaseBuffers())\n> > > > +\t\t\tLOG(MaliC55, Error) << \"Failed to release input buffers\";\n> > > > +\t\tif (mem->cru_->freeBuffers())\n> > > > +\t\t\tLOG(MaliC55, Error) << \"Failed to release CRU buffers\";\n> > > > +\t}\n> > > > +\n> > > >    \treturn;\n> > > >    }\n> > > > @@ -1249,6 +1267,12 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n> > > >    \t\t}\n> > > >    \t};\n> > > > +\tif (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {\n> > > > +\t\tret = ivc_->importBuffers(RZG2LCRU::kBufferCount);\n> > > > +\t\tif (ret < 0)\n> > > > +\t\t\treturn ret;\n> > > > +\t}\n> > > > +\n> > > >    \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n> > > >    \tif (ret < 0)\n> > > >    \t\treturn ret;\n> > > > @@ -1280,6 +1304,24 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control\n> > > >    \tif (ret)\n> > > >    \t\treturn ret;\n> > > > +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> > > > +\t\tret = mem->cru_->start();\n> > > > +\t\tif (ret) {\n> > > > +\t\t\tLOG(MaliC55, Error)\n> > > > +\t\t\t\t<< \"Failed to start CRU \" << camera->id();\n> > > > +\t\t\tfreeBuffers(camera);\n> > >\n> > > Minor thing, but I think reworking these with `utils::scope_exit` is a worthwhile change\n> > > if you plan to send a new version.\n> > >\n> >\n> > Can we consider doing this on top as the existing error paths would\n> > need to be reworked otherwise ?\n>\n> Yes, certainly.\n>\n>\n> >\n> > >\n> > > > +\t\t\treturn ret;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tret = ivc_->streamOn();\n> > > > +\t\tif (ret) {\n> > > > +\t\t\tLOG(MaliC55, Error)\n> > > > +\t\t\t\t<< \"Failed to start IVC\" << camera->id();\n> > > > +\t\t\tfreeBuffers(camera);\n> > > > +\t\t\treturn ret;\n> > > > +\t\t}\n> > > > +\t}\n> > > > +\n> > > >    \tif (data->ipa_) {\n> > > >    \t\tret = data->ipa_->start();\n> > > >    \t\tif (ret) {\n> > > > @@ -1355,12 +1397,25 @@ int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control\n> > > >    \treturn 0;\n> > > >    }\n> > > > +void PipelineHandlerMaliC55::cancelPendingRequests()\n> > > > +{\n> > > > +\twhile (!pendingRequests_.empty()) {\n> > > > +\t\tcancelRequest(pendingRequests_.front());\n> > > > +\t\tpendingRequests_.pop();\n> > > > +\t}\n> > > > +}\n> > > > +\n> > > >    void PipelineHandlerMaliC55::stopDevice(Camera *camera)\n> > > >    {\n> > > >    \tMaliC55CameraData *data = cameraData(camera);\n> > > >    \tisp_->setFrameStartEnabled(false);\n> > > > +\tif (auto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_)) {\n> > > > +\t\tivc_->streamOff();\n> > > > +\t\tmem->cru_->stop();\n> > > > +\t}\n> > > > +\n> > > >    \tfor (MaliC55Pipe &pipe : pipes_) {\n> > > >    \t\tif (!pipe.stream)\n> > > >    \t\t\tcontinue;\n> > > > @@ -1374,6 +1429,8 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera)\n> > > >    \tif (data->ipa_)\n> > > >    \t\tdata->ipa_->stop();\n> > > >    \tfreeBuffers(camera);\n> > > > +\n> > > > +\tcancelPendingRequests();\n> > > >    }\n> > > >    void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n> > > > @@ -1472,10 +1529,85 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n> > > >    \t}\n> > > >    }\n> > > > +MaliC55FrameInfo *\n> > > > +PipelineHandlerMaliC55::prepareFrameInfo(Request *request, RZG2LCRU *cru)\n> > > > +{\n> > > > +\tif (availableStatsBuffers_.empty()) {\n> > > > +\t\tLOG(MaliC55, Error) << \"Stats buffer underrun\";\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\tif (availableParamsBuffers_.empty()) {\n> > > > +\t\tLOG(MaliC55, Error) << \"Params buffer underrun\";\n> > > > +\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\tMaliC55FrameInfo &frameInfo = frameInfoMap_[request->sequence()];\n> > >\n> > > I think it would be better to do it last, not to leave an \"empty\" frame info\n> > > object in the map.\n> >\n> > As the only failure point is the below if (!frameInfo.rawBuffer) can I\n> > do after that ?\n>\n> Yes, or alternatively a temporary `MaliC55FrameInfo` could be filled, which is added\n> to the map last. Like it was done previously.\n\nOh well, if we can avoid a copy, that's probably better\n\n>\n>\n> >\n> > >\n> > >\n> > > > +\tframeInfo.request = request;\n> > > > +\n> > > > +\tif (cru) {\n> > > > +\t\tframeInfo.rawBuffer = cru->queueBuffer(request);\n> > > > +\t\tif (!frameInfo.rawBuffer)\n> > >\n> > > I'm wondering, can it not be guaranteed that there is always an available buffer?\n> > > Wouldn't it be sufficient to ensure that at most 4 requests are queued to the\n> > > pipeline handler? In that case I believe there should always be an available cru buffer.\n> >\n> > Should I initialize PipelineHandler() with maxQueuedRequestsDevice ==\n> > kBufferCount ?\n> >\n> > >\n> > >\n> > > > +\t\t\treturn nullptr;\n> > > > +\t}\n> > > > +\n> > > > +\tframeInfo.statBuffer = availableStatsBuffers_.front();\n> > > > +\tavailableStatsBuffers_.pop();\n> > > > +\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n> > > > +\tavailableParamsBuffers_.pop();\n> > > > +\n> > > > +\tframeInfo.paramsDone = false;\n> > > > +\tframeInfo.statsDone = false;\n> > > > +\n> > > > +\treturn &frameInfo;\n> > > > +}\n> > > > +\n> > > > +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data)\n> > >\n> > > Should this not be called when a CRU buffer becomes available? If the queue is\n> > > only drained if a new request is queued, then I feel like things can get stuck\n> > > if the application is stopping and is waiting for the pending requests to complete.\n> > > I suppose it's fine now because everything uses hard-coded 4 buffers / requests.\n> > >\n> >\n> > mmm, you're right. We only try to run when the application queue a new\n> > Request. Should I create an handler for ivc_->bufferReady (which now\n> > simply returns the buffer to the cru) and call\n> > PipelineHandlerMaliC55::queuePendingRequests() from there ?\n>\n> I'm looking at 5fb28bfe7438c8ed1029501c3c050e667eb2f507 and 1ed284ba487b7e0903e031c4e6604b601ff040a8.\n> I think a similar change could be applied here as well: always allocate a constant number of cru, param,\n> or stat buffers, and then set `maxQueuedRequestsDevice` to the minimum of those. If I'm not mistaken,\n> that should guarantee that at least one of all three types of buffers is available in `queueRequestDevice()`.\n> Then `pendingRequests_` wouldn't even be needed as far as I can see. Maybe that's the best approach.\n\nThing is that with Mali, we have one more actor: the CRU.\n\nRight now the CRU allocates a number of buffers defined by\nRZG2LCRU::kBufferCount.\n\nI'm tempted to make RZG2LCRU::start() accept a number of buffers to\nallocate and move the buffer number definition in the MaliC55 file so\nthat we can use to allocate buffers for the cru, params and stats.\n\nI would however keep pendingRequests_ for the time being ?\n\n>\n> >\n> > >\n> > > > +{\n> > > > +\tauto *mem = std::get_if<MaliC55CameraData::Memory>(&data->input_);\n> > > > +\tASSERT(mem);\n> > > > +\n> > > > +\twhile (!pendingRequests_.empty()) {\n> > > > +\t\tRequest *request = pendingRequests_.front();\n> > > > +\n> > > > +\t\tif (!prepareFrameInfo(request, mem->cru_.get()))\n> > > > +\t\t\treturn -ENOENT;\n> > > > +\n> > > > +\t\tfor (auto &[stream, buffer] : request->buffers()) {\n> > > > +\t\t\tMaliC55Pipe *pipe = pipeFromStream(data, stream);\n> > > > +\n> > > > +\t\t\tpipe->cap->queueBuffer(buffer);\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tdata->ipa_->queueRequest(request->sequence(), request->controls());\n> > > > +\n> > > > +\t\tpendingRequests_.pop();\n> > > > +\t}\n> > > > +\n> > > > +\treturn 0;\n> > > > +}\n> > > > +\n> > > >    int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)\n> > > >    {\n> > > >    \tMaliC55CameraData *data = cameraData(camera);\n> > > > +\t/*\n> > > > +\t * If we're in memory input mode, we need to pop the requests onto the\n> > >\n> > > push ?\n> >\n> > ups\n> >\n> > >\n> > >\n> > > > +\t * pending list until a CRU buffer is ready...otherwise we can just do\n> > > > +\t * everything immediately.\n> > > > +\t */\n> > >\n> > > Or maybe I was wrong above. But this says \"pending list until a CRU buffer is ready\",\n> > > so if no CRU buffer is available, shouldn't it just push to `pendingRequests_` and\n> > > return 0? Because this now returns non-zero, and that will cancel the request.\n> > >\n> >\n> > I actually think the below error path handling is wrong now that I\n> > look at it more closely\n>\n> Ahh, right. push() appends to the end, but pop() consumes from the front.\n>\n>\n> >\n> > >\n> > > > +\tif (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {\n> > > > +\t\tpendingRequests_.push(request);\n> > > > +\n> > > > +\t\tint ret = queuePendingRequests(data);\n> > > > +\t\tif (ret) {\n> > > > +\t\t\tpendingRequests_.pop();\n> >\n> > Requests are popped from the queue in queuePendingRequests() only at\n> > the end of the loop, so if we error out, there isn't any need to\n> > actually pop them here\n> >\n> > > > +\t\t\treturn ret;\n> >\n> > And, as you suggested, if we return an error the request is cancelled,\n> > so we should probably return 0 here, leave the Request in the queue\n> > and let the next queuePendingRequests() consume it ?\n> >\n\nI'm now returning 0 unconditionally from here.\n\nThanks\n  j\n\n> > > > +\t\t}\n> > > > +\n> > > > +\t\treturn 0;\n> > > > +\t}\n> > > > +\n> > > >    \t/* Do not run the IPA if the TPG is in use. */\n> > > >    \tif (!data->ipa_) {\n> > > >    \t\tMaliC55FrameInfo frameInfo;\n> > > > @@ -1496,32 +1628,13 @@ int PipelineHandlerMaliC55::queueRequestDevice(Camera *camera, Request *request)\n> > > >    \t\treturn 0;\n> > > >    \t}\n> > > > -\tif (availableStatsBuffers_.empty()) {\n> > > > -\t\tLOG(MaliC55, Error) << \"Stats buffer underrun\";\n> > > > -\t\treturn -ENOENT;\n> > > > -\t}\n> > > > -\n> > > > -\tif (availableParamsBuffers_.empty()) {\n> > > > -\t\tLOG(MaliC55, Error) << \"Params buffer underrun\";\n> > > > +\tauto frameInfo = prepareFrameInfo(request);\n> > > > +\tif (!frameInfo)\n> > > >    \t\treturn -ENOENT;\n> > > > -\t}\n> > > > -\n> > > > -\tMaliC55FrameInfo frameInfo;\n> > > > -\tframeInfo.request = request;\n> > > > -\n> > > > -\tframeInfo.statBuffer = availableStatsBuffers_.front();\n> > > > -\tavailableStatsBuffers_.pop();\n> > > > -\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n> > > > -\tavailableParamsBuffers_.pop();\n> > > > -\n> > > > -\tframeInfo.paramsDone = false;\n> > > > -\tframeInfo.statsDone = false;\n> > > > -\n> > > > -\tframeInfoMap_[request->sequence()] = frameInfo;\n> > > >    \tdata->ipa_->queueRequest(request->sequence(), request->controls());\n> > > >    \tdata->ipa_->fillParams(request->sequence(),\n> > > > -\t\t\t       frameInfo.paramBuffer->cookie());\n> > > > +\t\t\t       frameInfo->paramBuffer->cookie());\n> > > >    \treturn 0;\n> > > >    }\n> > > > [...]\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 91725BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 31 Mar 2026 14:36:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C6DF062D10;\n\tTue, 31 Mar 2026 16:36:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9A5416274D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 31 Mar 2026 16:36:27 +0200 (CEST)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1542B143C;\n\tTue, 31 Mar 2026 16:35:04 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"a/teEt6T\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1774967705;\n\tbh=97T1zNd3lFDkTTeiKGPneJ58oZZmHSxXunhI8qBXw54=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=a/teEt6TTLWgytvrzCpOsNJaF1Z1dC8Vw4JCE22o8i3UgncdVuKBBw4TArwVdc18M\n\tqAl7JT/rQ1h5kUxBhlQJ0Lk9oP6dH+loqrz1gXjV/0j3DJKc80ld6cZqHNxK7cU2cF\n\tnt2lG0SAVaVteQmF9UGZOcIKlXv2sQlFxJMIxNRw=","Date":"Tue, 31 Mar 2026 16:36:24 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tDaniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","Message-ID":"<acva5yaFiD5k5I4r@zed>","References":"<20260325-mali-cru-v6-0-b16b0c49819a@ideasonboard.com>\n\t<20260325-mali-cru-v6-6-b16b0c49819a@ideasonboard.com>\n\t<0ea76bbe-06e9-4213-9acd-4e7ef72ed8f2@ideasonboard.com>\n\t<acvFNhxivkgXIMs_@zed>\n\t<9859dd2c-2a51-4db9-9304-e41e99e031ad@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<9859dd2c-2a51-4db9-9304-e41e99e031ad@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38467,"web_url":"https://patchwork.libcamera.org/comment/38467/","msgid":"<4826fb43-94c7-45c3-89cb-743f815239da@ideasonboard.com>","date":"2026-03-31T15:49:02","subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 03. 31. 16:36 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Tue, Mar 31, 2026 at 04:10:53PM +0200, Barnabás Pőcze wrote:\n>> 2026. 03. 31. 15:12 keltezéssel, Jacopo Mondi írta:\n>>> Hi Barnabás\n>>>\n>>> On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote:\n>>>> Hi\n>>>>\n>>>> 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta:\n>>>>> From: Daniel Scally <dan.scally@ideasonboard.com>\n>>>>>\n>>>>> Plumb in the MaliC55 pipeline handler support for capturing frames\n>>>>> from memory using the CRU.\n>>>>>\n>>>>> Introduce a data flow which uses the CRU to feed the ISP through\n>>>>> the IVC.\n>>>>>\n>>>>> In detail:\n>>>>>\n>>>>> - push incoming request to a pending queue until a buffer from the CRU\n>>>>>      is available\n>>>>> - delay the call to ipa_->fillParams() to the CRU buffer ready even\n>>>>\n>>>> event ?\n>>>>\n>>>\n>>> Ah yes\n>>>\n>>>>\n>>>>> - once the IPA has computed parameters feed the ISP through the IVC\n>>>>>      with buffers from the CRU, params and statistics.\n>>>>> - Handle completion of in-flight requests: in m2m mode buffers are\n>>>>>      queued earlier to the ISP capture devices. If the camera is stopped\n>>>>>      these buffers are returned earlier in error state: complete the\n>>>>>      Request bypassing the IPA operations.\n>>>>> - As cancelled Requests might now be completed earlier then IPA events,\n>>>>>      modify the IPA event handler to ignore Requests that have been\n>>>>>      completed already\n>>>>>\n>>>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n>>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>>>>> ---\n>>>>>     src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++----\n>>>>>     1 file changed, 207 insertions(+), 37 deletions(-)\n>>>>>\n>>>>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>>> index be3e38da95ab..10848c412849 100644\n>>>>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>>>> @@ -21,6 +21,7 @@\n> [...]\n>>>>> +\tframeInfo.request = request;\n>>>>> +\n>>>>> +\tif (cru) {\n>>>>> +\t\tframeInfo.rawBuffer = cru->queueBuffer(request);\n>>>>> +\t\tif (!frameInfo.rawBuffer)\n>>>>\n>>>> I'm wondering, can it not be guaranteed that there is always an available buffer?\n>>>> Wouldn't it be sufficient to ensure that at most 4 requests are queued to the\n>>>> pipeline handler? In that case I believe there should always be an available cru buffer.\n>>>\n>>> Should I initialize PipelineHandler() with maxQueuedRequestsDevice ==\n>>> kBufferCount ?\n>>>\n>>>>\n>>>>\n>>>>> +\t\t\treturn nullptr;\n>>>>> +\t}\n>>>>> +\n>>>>> +\tframeInfo.statBuffer = availableStatsBuffers_.front();\n>>>>> +\tavailableStatsBuffers_.pop();\n>>>>> +\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n>>>>> +\tavailableParamsBuffers_.pop();\n>>>>> +\n>>>>> +\tframeInfo.paramsDone = false;\n>>>>> +\tframeInfo.statsDone = false;\n>>>>> +\n>>>>> +\treturn &frameInfo;\n>>>>> +}\n>>>>> +\n>>>>> +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data)\n>>>>\n>>>> Should this not be called when a CRU buffer becomes available? If the queue is\n>>>> only drained if a new request is queued, then I feel like things can get stuck\n>>>> if the application is stopping and is waiting for the pending requests to complete.\n>>>> I suppose it's fine now because everything uses hard-coded 4 buffers / requests.\n>>>>\n>>>\n>>> mmm, you're right. We only try to run when the application queue a new\n>>> Request. Should I create an handler for ivc_->bufferReady (which now\n>>> simply returns the buffer to the cru) and call\n>>> PipelineHandlerMaliC55::queuePendingRequests() from there ?\n>>\n>> I'm looking at 5fb28bfe7438c8ed1029501c3c050e667eb2f507 and 1ed284ba487b7e0903e031c4e6604b601ff040a8.\n>> I think a similar change could be applied here as well: always allocate a constant number of cru, param,\n>> or stat buffers, and then set `maxQueuedRequestsDevice` to the minimum of those. If I'm not mistaken,\n>> that should guarantee that at least one of all three types of buffers is available in `queueRequestDevice()`.\n>> Then `pendingRequests_` wouldn't even be needed as far as I can see. Maybe that's the best approach.\n> \n> Thing is that with Mali, we have one more actor: the CRU.\n\nThat's true, but I don't think it is fundamentally different.\nIf `maxQueuedRequestsDevice` is set to min(cruBuffers, paramBuffers, statBuffers),\nthen then that should work the same.\n\n\n> \n> Right now the CRU allocates a number of buffers defined by\n> RZG2LCRU::kBufferCount.\n> \n> I'm tempted to make RZG2LCRU::start() accept a number of buffers to\n> allocate and move the buffer number definition in the MaliC55 file so\n> that we can use to allocate buffers for the cru, params and stats.\n\nThat sounds reasonable.\n\n\n> \n> I would however keep pendingRequests_ for the time being ?\n\nI think if `maxQueuedRequestsDevice` is chosen well, then it will\njust be empty all the time. Unless I'm missing something, removing\nit seems like a great simplification?\n\n\n> [...]","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 700E3BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 31 Mar 2026 15:49:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9EB562D11;\n\tTue, 31 Mar 2026 17:49:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 98CCE6274D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 31 Mar 2026 17:49:05 +0200 (CEST)","from [192.168.33.35] (185.221.143.151.nat.pool.zt.hu\n\t[185.221.143.151])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1D364244D;\n\tTue, 31 Mar 2026 17:47:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sNTuVd5D\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1774972063;\n\tbh=rwDJz4qmz5QyNEfsGs1OAOVf5wdoPbWPIgul4BYeLXE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=sNTuVd5DHilR+266mHK/9OLScK/3VEIYTtp0hJdMuXnhERhLFQDXp7gyqjl3/5BPC\n\tb/9rcHGqIhFVXpNRtJEiT9NJC2jbZVQSX/ipjLZaURuqryBe1wATVkZyJUP3bN4sGU\n\t8latgEIIbh1BOSBuZItRAVSYdopLelvSivPGMU2s=","Message-ID":"<4826fb43-94c7-45c3-89cb-743f815239da@ideasonboard.com>","Date":"Tue, 31 Mar 2026 17:49:02 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Daniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260325-mali-cru-v6-0-b16b0c49819a@ideasonboard.com>\n\t<20260325-mali-cru-v6-6-b16b0c49819a@ideasonboard.com>\n\t<0ea76bbe-06e9-4213-9acd-4e7ef72ed8f2@ideasonboard.com>\n\t<acvFNhxivkgXIMs_@zed>\n\t<9859dd2c-2a51-4db9-9304-e41e99e031ad@ideasonboard.com>\n\t<acva5yaFiD5k5I4r@zed>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<acva5yaFiD5k5I4r@zed>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38469,"web_url":"https://patchwork.libcamera.org/comment/38469/","msgid":"<acvvd1IRhpuWkq6L@zed>","date":"2026-03-31T16:01:58","subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Tue, Mar 31, 2026 at 05:49:02PM +0200, Barnabás Pőcze wrote:\n> 2026. 03. 31. 16:36 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Tue, Mar 31, 2026 at 04:10:53PM +0200, Barnabás Pőcze wrote:\n> > > 2026. 03. 31. 15:12 keltezéssel, Jacopo Mondi írta:\n> > > > Hi Barnabás\n> > > >\n> > > > On Tue, Mar 31, 2026 at 10:47:54AM +0200, Barnabás Pőcze wrote:\n> > > > > Hi\n> > > > >\n> > > > > 2026. 03. 25. 15:44 keltezéssel, Jacopo Mondi írta:\n> > > > > > From: Daniel Scally <dan.scally@ideasonboard.com>\n> > > > > >\n> > > > > > Plumb in the MaliC55 pipeline handler support for capturing frames\n> > > > > > from memory using the CRU.\n> > > > > >\n> > > > > > Introduce a data flow which uses the CRU to feed the ISP through\n> > > > > > the IVC.\n> > > > > >\n> > > > > > In detail:\n> > > > > >\n> > > > > > - push incoming request to a pending queue until a buffer from the CRU\n> > > > > >      is available\n> > > > > > - delay the call to ipa_->fillParams() to the CRU buffer ready even\n> > > > >\n> > > > > event ?\n> > > > >\n> > > >\n> > > > Ah yes\n> > > >\n> > > > >\n> > > > > > - once the IPA has computed parameters feed the ISP through the IVC\n> > > > > >      with buffers from the CRU, params and statistics.\n> > > > > > - Handle completion of in-flight requests: in m2m mode buffers are\n> > > > > >      queued earlier to the ISP capture devices. If the camera is stopped\n> > > > > >      these buffers are returned earlier in error state: complete the\n> > > > > >      Request bypassing the IPA operations.\n> > > > > > - As cancelled Requests might now be completed earlier then IPA events,\n> > > > > >      modify the IPA event handler to ignore Requests that have been\n> > > > > >      completed already\n> > > > > >\n> > > > > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > > > > ---\n> > > > > >     src/libcamera/pipeline/mali-c55/mali-c55.cpp | 244 +++++++++++++++++++++++----\n> > > > > >     1 file changed, 207 insertions(+), 37 deletions(-)\n> > > > > >\n> > > > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > > index be3e38da95ab..10848c412849 100644\n> > > > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > > @@ -21,6 +21,7 @@\n> > [...]\n> > > > > > +\tframeInfo.request = request;\n> > > > > > +\n> > > > > > +\tif (cru) {\n> > > > > > +\t\tframeInfo.rawBuffer = cru->queueBuffer(request);\n> > > > > > +\t\tif (!frameInfo.rawBuffer)\n> > > > >\n> > > > > I'm wondering, can it not be guaranteed that there is always an available buffer?\n> > > > > Wouldn't it be sufficient to ensure that at most 4 requests are queued to the\n> > > > > pipeline handler? In that case I believe there should always be an available cru buffer.\n> > > >\n> > > > Should I initialize PipelineHandler() with maxQueuedRequestsDevice ==\n> > > > kBufferCount ?\n> > > >\n> > > > >\n> > > > >\n> > > > > > +\t\t\treturn nullptr;\n> > > > > > +\t}\n> > > > > > +\n> > > > > > +\tframeInfo.statBuffer = availableStatsBuffers_.front();\n> > > > > > +\tavailableStatsBuffers_.pop();\n> > > > > > +\tframeInfo.paramBuffer = availableParamsBuffers_.front();\n> > > > > > +\tavailableParamsBuffers_.pop();\n> > > > > > +\n> > > > > > +\tframeInfo.paramsDone = false;\n> > > > > > +\tframeInfo.statsDone = false;\n> > > > > > +\n> > > > > > +\treturn &frameInfo;\n> > > > > > +}\n> > > > > > +\n> > > > > > +int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data)\n> > > > >\n> > > > > Should this not be called when a CRU buffer becomes available? If the queue is\n> > > > > only drained if a new request is queued, then I feel like things can get stuck\n> > > > > if the application is stopping and is waiting for the pending requests to complete.\n> > > > > I suppose it's fine now because everything uses hard-coded 4 buffers / requests.\n> > > > >\n> > > >\n> > > > mmm, you're right. We only try to run when the application queue a new\n> > > > Request. Should I create an handler for ivc_->bufferReady (which now\n> > > > simply returns the buffer to the cru) and call\n> > > > PipelineHandlerMaliC55::queuePendingRequests() from there ?\n> > >\n> > > I'm looking at 5fb28bfe7438c8ed1029501c3c050e667eb2f507 and 1ed284ba487b7e0903e031c4e6604b601ff040a8.\n> > > I think a similar change could be applied here as well: always allocate a constant number of cru, param,\n> > > or stat buffers, and then set `maxQueuedRequestsDevice` to the minimum of those. If I'm not mistaken,\n> > > that should guarantee that at least one of all three types of buffers is available in `queueRequestDevice()`.\n> > > Then `pendingRequests_` wouldn't even be needed as far as I can see. Maybe that's the best approach.\n> >\n> > Thing is that with Mali, we have one more actor: the CRU.\n>\n> That's true, but I don't think it is fundamentally different.\n> If `maxQueuedRequestsDevice` is set to min(cruBuffers, paramBuffers, statBuffers),\n> then then that should work the same.\n>\n>\n> >\n> > Right now the CRU allocates a number of buffers defined by\n> > RZG2LCRU::kBufferCount.\n> >\n> > I'm tempted to make RZG2LCRU::start() accept a number of buffers to\n> > allocate and move the buffer number definition in the MaliC55 file so\n> > that we can use to allocate buffers for the cru, params and stats.\n>\n> That sounds reasonable.\n>\n>\n> >\n> > I would however keep pendingRequests_ for the time being ?\n>\n> I think if `maxQueuedRequestsDevice` is chosen well, then it will\n> just be empty all the time. Unless I'm missing something, removing\n> it seems like a great simplification?\n\nYou're right. If we limit the number of requests in-flight to the\nnumber of available CRU buffers, the PipelineHandler base class\nbasically does the queueing for us.\n\nProbably this patch pre-dates the introduction of\nPipelineHandler::maxQueuedRequestsDevice_\n\nI'll run a few more tests with\n\n        ASSERT(pendingRequests_.size() < 2);\n\nand if it's all good I'll remove the queue\n>\n>\n> > [...]","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AC086BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 31 Mar 2026 16:02:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF25162D17;\n\tTue, 31 Mar 2026 18:02:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 47F556274D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 31 Mar 2026 18:02:01 +0200 (CEST)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 95B82244D;\n\tTue, 31 Mar 2026 18:00:38 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kIcNpOpk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1774972838;\n\tbh=5/p1Q708HkTLOdc1q5PUcp5eDu2EWlab1AIUAbegY40=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kIcNpOpkDMF3nrAEJSLo5amVvyp2BcsFeKYUwROFqCYfKVOFQ0/ov6t7s9u74Q/U+\n\tDwr1QPIRezf0A2x7/IzJt0T8ShgvTb/4Ah2eQmtY5FO9soyCMinJUpXpxvrWlgpfsW\n\txVSWMlmcgUhM0kJ5lmHzkRO5o7LBwuhSkoOF002o=","Date":"Tue, 31 Mar 2026 18:01:58 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tDaniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v6 6/7] libcamera: mali-c55: Implement capture for\n\tmemory-to-memory","Message-ID":"<acvvd1IRhpuWkq6L@zed>","References":"<20260325-mali-cru-v6-0-b16b0c49819a@ideasonboard.com>\n\t<20260325-mali-cru-v6-6-b16b0c49819a@ideasonboard.com>\n\t<0ea76bbe-06e9-4213-9acd-4e7ef72ed8f2@ideasonboard.com>\n\t<acvFNhxivkgXIMs_@zed>\n\t<9859dd2c-2a51-4db9-9304-e41e99e031ad@ideasonboard.com>\n\t<acva5yaFiD5k5I4r@zed>\n\t<4826fb43-94c7-45c3-89cb-743f815239da@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<4826fb43-94c7-45c3-89cb-743f815239da@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]