[v5,6/7] libcamera: mali-c55: Implement capture for memory-to-memory
diff mbox series

Message ID 20260313-mali-cru-v5-6-48f93e431294@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: mali-c55: Add support for memory-to-memory
Related show

Commit Message

Jacopo Mondi March 13, 2026, 4:14 p.m. UTC 
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
- once the IPA has computed parameters feed the ISP through the IVC
  with buffers from the CRU, params and statistics.

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 | 166 ++++++++++++++++++++++++++-
 1 file changed, 161 insertions(+), 5 deletions(-)

Comments

Barnabás Pőcze March 18, 2026, 3:54 p.m. UTC | #1 
Hi


2026. 03. 13. 17:14 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

I think this sentence was cut of.


> - once the IPA has computed parameters feed the ISP through the IVC
>    with buffers from the CRU, params and statistics.
> 
> 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 | 166 ++++++++++++++++++++++++++-
>   1 file changed, 161 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> index 9b83927d8441..84c2a030b470 100644
> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> [...]
> @@ -1404,6 +1443,12 @@ void PipelineHandlerMaliC55::stopDevice(Camera *camera)
>   		pipe.cap->releaseBuffers();
>   	}
>   
> +	if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {
> +		cancelPendingRequests();
> +		ivc_->streamOff();
> +		data->cru()->stop();
> +	}

I think there is an issue here. Specifically, `RZG2LCRU::stop()` frees the buffers, but dangling
references are still in `ivc_->V4l2VideoDevice::cache_` until `ivc_->releaseBuffers()` is called
in `freeBuffers()`. When `ipa_->stop()` is called, it is after these cru buffers have been freed.
If there are pending `{params,stats}Completed` signals, those are synchronously dispatched. Those
will reference the already freed buffers, for example when `ivc_->queueBuffer()` is called,
leading to a use after free.

==1182==ERROR: AddressSanitizer: heap-use-after-free on address 0xfc1f8d207b30 at pc 0xffff9c2c5b3c bp 0xfbff8aff9610 sp 0xfbff8aff9628
READ of size 8 at 0xfc1f8d207b30 thread T1
     #0 0xffff9c2c5b38 in libcamera::V4L2BufferCache::Entry::operator==(libcamera::FrameBuffer const&) const ../src/libcamera/v4l2_videodevice.cpp:292
     #1 0xffff9c2d5968 in libcamera::V4L2BufferCache::get(libcamera::FrameBuffer const&) ../src/libcamera/v4l2_videodevice.cpp:243
     #2 0xffff9c2e1648 in libcamera::V4L2VideoDevice::queueBuffer(libcamera::FrameBuffer*, libcamera::V4L2Request const*) ../src/libcamera/v4l2_videodevice.cpp:1671
     #3 0xffff9c48e904 in libcamera::PipelineHandlerMaliC55::paramsComputed(unsigned int, unsigned int) ../src/libcamera/pipeline/mali-c55/mali-c55.cpp:1778
     #4 0xffff99242704 in libcamera::BoundMethodBase::activatePack(std::shared_ptr<libcamera::BoundMethodPackBase>, bool) ../src/libcamera/base/bound_method.cpp:85
     #5 0xffff9c51b0e4 in libcamera::BoundMethodMember<libcamera::PipelineHandlerMaliC55, void, unsigned int, unsigned int>::activate(unsigned int, unsigned int, bool) ../include/libcamera/base/bound_method.h:173
     #6 0xffff9bcc7b1c in libcamera::Signal<unsigned int, unsigned int>::emit(unsigned int, unsigned int) ../include/libcamera/base/signal.h:147
     #7 0xffff9bc8ae5c in libcamera::ipa::mali_c55::IPAProxyMaliC55Threaded::paramsComputedHandler(unsigned int, unsigned int) src/libcamera/proxy/mali-c55_ipa_proxy.cpp:166
     #8 0xffff9924bf54 in libcamera::Object::message(libcamera::Message*) ../src/libcamera/base/object.cpp:211
     #9 0xffff992dfaf0 in libcamera::Thread::dispatchMessages(libcamera::Message::Type, libcamera::Object*) ../src/libcamera/base/thread.cpp:662
     #10 0xffff9bc8f274 in libcamera::ipa::mali_c55::IPAProxyMaliC55Threaded::stop() src/libcamera/proxy/mali-c55_ipa_proxy.cpp:105
     #11 0xffff9c49599c in libcamera::PipelineHandlerMaliC55::stopDevice(libcamera::Camera*) ../src/libcamera/pipeline/mali-c55/mali-c55.cpp:1420
     [...]

and

0xfc1f8d207b30 is located 0 bytes inside of 16-byte region [0xfc1f8d207b30,0xfc1f8d207b40)
freed by thread T1 here:
     #0 0xffff9f8815b8 in operator delete(void*, unsigned long) (/lib64/libasan.so.8+0xd95b8)
     #1 0xffff9c522238 in libcamera::FrameBuffer::~FrameBuffer() ../include/libcamera/framebuffer.h:63
     #2 0xffff9c522238 in std::default_delete<libcamera::FrameBuffer>::operator()(libcamera::FrameBuffer*) const /aarch64-buildroot-linux-gnu/include/c++/15.2.0/bits/unique_ptr.h:93
     #3 0xffff9c522238 in std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >::~unique_ptr() /aarch64-buildroot-linux-gnu/include/c++/15.2.0/bits/unique_ptr.h:399
     #4 0xffff9c522238 in void std::_Destroy<std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> > >(std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >*) /aarch64-buildroot-linux-gnu/include/c++/15.2.0/bits/stl_construct.h:166
     #5 0xffff9c522238 in void std::_Destroy<std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >*>(std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >*, std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >*) /aarch64-buildroot-linux-gnu/include/c++/15.2.0/bits/stl_construct.h:212
     #6 0xffff9c522238 in void std::_Destroy<std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >*, std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> > >(std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >*, std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >*, std::allocator<std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> > >&) /aarch64-buildroot-linux-gnu/include/c++/15.2.0/bits/alloc_traits.h:1045
     #7 0xffff9c522238 in std::vector<std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >, std::allocator<std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> > > >::_M_erase_at_end(std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >*) /aarch64-buildroot-linux-gnu/include/c++/15.2.0/bits/stl_vector.h:2238
     #8 0xffff9c522238 in std::vector<std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> >, std::allocator<std::unique_ptr<libcamera::FrameBuffer, std::default_delete<libcamera::FrameBuffer> > > >::clear() /aarch64-buildroot-linux-gnu/include/c++/15.2.0/bits/stl_vector.h:1864
     #9 0xffff9c522238 in libcamera::RZG2LCRU::freeBuffers() ../src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp:108
     #10 0xffff9c52314c in libcamera::RZG2LCRU::stop() ../src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp:100
     #11 0xffff9c495758 in libcamera::PipelineHandlerMaliC55::stopDevice(libcamera::Camera*) ../src/libcamera/pipeline/mali-c55/mali-c55.cpp:1414
     [...]


> +
>   	stats_->streamOff();
>   	params_->streamOff();
>   	if (data->ipa_)
> [...]
> @@ -1645,18 +1789,27 @@ void PipelineHandlerMaliC55::paramsComputed(unsigned int requestId, uint32_t byt
>   
>   	/*
>   	 * 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);
> +		}
>   	}
> +
> +	if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_))
> +		ivc_->queueBuffer(frameInfo.rawBuffer);

I think `frameInfo.rawBuffer = nullptr` is missing here. If not done, then
`findFrameInfo()` might find an "old" entry if the buffer is reused.


>   }
>   
>   void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,
> @@ -1793,6 +1946,9 @@ bool PipelineHandlerMaliC55::registerMemoryInputCamera()
>   
>   	ivc_->bufferReady.connect(data->cru(), &RZG2LCRU::cruReturnBuffer);
>   
> +	V4L2VideoDevice *cruOutput = data->cru()->output();
> +	cruOutput->bufferReady.connect(this, &PipelineHandlerMaliC55::cruBufferReady);
> +
>   	return registerMaliCamera(std::move(data), sensor->device()->entity()->name());
>   }
>   
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
index 9b83927d8441..84c2a030b470 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;
@@ -721,11 +723,14 @@  public:
 	int start(Camera *camera, const ControlList *controls) override;
 	void stopDevice(Camera *camera) override;
 
+	int queuePendingRequests(MaliC55CameraData *data);
+	void cancelPendingRequests();
 	int queueRequestDevice(Camera *camera, Request *request) override;
 
 	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);
 
@@ -807,6 +812,11 @@  private:
 
 	std::map<unsigned int, MaliC55FrameInfo> frameInfoMap_;
 
+	/* Requests for which no buffer has been queued to the CRU device yet. */
+	std::queue<Request *> pendingRequests_;
+	/* Requests queued to the CRU device but not yet processed by the ISP. */
+	std::queue<Request *> processingRequests_;
+
 	std::array<MaliC55Pipe, MaliC55NumPipes> pipes_;
 
 	bool dsFitted_;
@@ -1255,6 +1265,11 @@  void PipelineHandlerMaliC55::freeBuffers(Camera *camera)
 	if (params_->releaseBuffers())
 		LOG(MaliC55, Error) << "Failed to release params buffers";
 
+	if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {
+		if (ivc_->releaseBuffers())
+			LOG(MaliC55, Error) << "Failed to release input buffers";
+	}
+
 	return;
 }
 
@@ -1284,6 +1299,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;
@@ -1315,6 +1336,24 @@  int PipelineHandlerMaliC55::start(Camera *camera, [[maybe_unused]] const Control
 	if (ret)
 		return ret;
 
+	if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {
+		ret = data->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) {
@@ -1404,6 +1443,12 @@  void PipelineHandlerMaliC55::stopDevice(Camera *camera)
 		pipe.cap->releaseBuffers();
 	}
 
+	if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_)) {
+		cancelPendingRequests();
+		ivc_->streamOff();
+		data->cru()->stop();
+	}
+
 	stats_->streamOff();
 	params_->streamOff();
 	if (data->ipa_)
@@ -1507,10 +1552,88 @@  void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,
 	}
 }
 
+void PipelineHandlerMaliC55::cancelPendingRequests()
+{
+	processingRequests_ = {};
+
+	while (!pendingRequests_.empty()) {
+		Request *request = pendingRequests_.front();
+
+		completeRequest(request);
+		pendingRequests_.pop();
+	}
+}
+
+int PipelineHandlerMaliC55::queuePendingRequests(MaliC55CameraData *data)
+{
+	ASSERT(std::holds_alternative<MaliC55CameraData::Memory>(data->input_));
+
+	while (!pendingRequests_.empty()) {
+		Request *request = pendingRequests_.front();
+
+		if (availableStatsBuffers_.empty()) {
+			LOG(MaliC55, Error) << "Stats buffer underrun";
+			return -ENOENT;
+		}
+
+		if (availableParamsBuffers_.empty()) {
+			LOG(MaliC55, Error) << "Params buffer underrun";
+			return -ENOENT;
+		}
+
+		MaliC55FrameInfo frameInfo;
+		frameInfo.request = request;
+
+		frameInfo.rawBuffer = data->cru()->queueBuffer(request);
+		if (!frameInfo.rawBuffer)
+			return -ENOENT;
+
+		frameInfo.statBuffer = availableStatsBuffers_.front();
+		availableStatsBuffers_.pop();
+		frameInfo.paramBuffer = availableParamsBuffers_.front();
+		availableParamsBuffers_.pop();
+
+		frameInfo.paramsDone = false;
+		frameInfo.statsDone = false;
+
+		frameInfoMap_[request->sequence()] = frameInfo;
+
+		for (auto &[stream, buffer] : request->buffers()) {
+			MaliC55Pipe *pipe = pipeFromStream(data, stream);
+
+			pipe->cap->queueBuffer(buffer);
+		}
+
+		data->ipa_->queueRequest(request->sequence(), request->controls());
+
+		pendingRequests_.pop();
+		processingRequests_.push(request);
+	}
+
+	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;
@@ -1575,7 +1698,8 @@  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;
 	}
 
@@ -1637,6 +1761,26 @@  void PipelineHandlerMaliC55::statsBufferReady(FrameBuffer *buffer)
 				 sensorControls);
 }
 
+void PipelineHandlerMaliC55::cruBufferReady(FrameBuffer *buffer)
+{
+	MaliC55FrameInfo *info = findFrameInfo(buffer);
+	Request *request = info->request;
+	ASSERT(info);
+
+	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
+		frameInfoMap_.erase(request->sequence());
+		completeRequest(request);
+		return;
+	}
+
+	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];
@@ -1645,18 +1789,27 @@  void PipelineHandlerMaliC55::paramsComputed(unsigned int requestId, uint32_t byt
 
 	/*
 	 * 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);
+		}
 	}
+
+	if (std::holds_alternative<MaliC55CameraData::Memory>(data->input_))
+		ivc_->queueBuffer(frameInfo.rawBuffer);
 }
 
 void PipelineHandlerMaliC55::statsProcessed(unsigned int requestId,
@@ -1793,6 +1946,9 @@  bool PipelineHandlerMaliC55::registerMemoryInputCamera()
 
 	ivc_->bufferReady.connect(data->cru(), &RZG2LCRU::cruReturnBuffer);
 
+	V4L2VideoDevice *cruOutput = data->cru()->output();
+	cruOutput->bufferReady.connect(this, &PipelineHandlerMaliC55::cruBufferReady);
+
 	return registerMaliCamera(std::move(data), sensor->device()->entity()->name());
 }