[07/10] libcamera: mali-c55: Add RZG2LCRU class

23924 diff mbox series

Add a class allowing us to control the RZ/G2L Camera Receiver Unit
which is found on the KakiP board. This will allow us to capture
buffers from the sensor, which in this configuration is not directly
connected to the ISP.

As this is not a guaranteed component of a video pipeline involving
the Mali-C55 ISP we would ideally use modular pipelines to fill this
role, but for now this will let us enable capture on the KakiP.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 .clang-format                                 |   1 -
 src/libcamera/pipeline/mali-c55/meson.build   |   3 +-
 src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp | 236 ++++++++++++++++++
 src/libcamera/pipeline/mali-c55/rzg2l-cru.h   |  66 +++++
 4 files changed, 304 insertions(+), 2 deletions(-)
 create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp
 create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.h

Quoting Daniel Scally (2025-07-24 07:52:53)
> Add a class allowing us to control the RZ/G2L Camera Receiver Unit
> which is found on the KakiP board. This will allow us to capture
> buffers from the sensor, which in this configuration is not directly
> connected to the ISP.
> 
> As this is not a guaranteed component of a video pipeline involving
> the Mali-C55 ISP we would ideally use modular pipelines to fill this
> role, but for now this will let us enable capture on the KakiP.
> 

Is the Mali-C55 using the MediaPipeline component yet to support
arbitrary length media graphs ?

I wonder if thats' where we could factor out the CSI2 receiver too
sometime as this component looks like it could be quite generic too.

But to support getting the Kakip supported, I'd be fine merging this.

> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  .clang-format                                 |   1 -
>  src/libcamera/pipeline/mali-c55/meson.build   |   3 +-
>  src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp | 236 ++++++++++++++++++
>  src/libcamera/pipeline/mali-c55/rzg2l-cru.h   |  66 +++++
>  4 files changed, 304 insertions(+), 2 deletions(-)
>  create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp
>  create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.h
> 
> diff --git a/.clang-format b/.clang-format
> index 7fc30f61..c6b3dd24 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -75,7 +75,6 @@ IncludeCategories:
>      Priority:        9
>    # Qt includes (match before C++ standard library)
>    - Regex:           '<Q([A-Za-z0-9\-_])+>'
> -    CaseSensitive:   true
>      Priority:        9

I don't think this should be in here.


>    # Headers in <> with an extension. (+system libraries)
>    - Regex:           '<([A-Za-z0-9\-_])+\.h>'
> diff --git a/src/libcamera/pipeline/mali-c55/meson.build b/src/libcamera/pipeline/mali-c55/meson.build
> index eba8e5a3..4e768242 100644
> --- a/src/libcamera/pipeline/mali-c55/meson.build
> +++ b/src/libcamera/pipeline/mali-c55/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  libcamera_internal_sources += files([
> -    'mali-c55.cpp'
> +    'mali-c55.cpp',
> +    'rzg2l-cru.cpp',
>  ])
> diff --git a/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp b/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp
> new file mode 100644
> index 00000000..6b4e7b91
> --- /dev/null
> +++ b/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp
> @@ -0,0 +1,236 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2025, Ideas on Board Oy
> + *
> + * Pipine handler element for the Renesas RZ/G2L Camera Receiver Unit
> + */
> +
> +#include "rzg2l-cru.h"
> +
> +#include <map>
> +
> +#include <libcamera/base/log.h>
> +#include <libcamera/base/regex.h>
> +
> +#include <libcamera/formats.h>
> +#include <libcamera/pixel_format.h>
> +
> +#include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/camera_sensor.h"
> +#include "libcamera/internal/framebuffer.h"
> +#include "libcamera/internal/media_device.h"
> +#include "libcamera/internal/request.h"
> +
> +namespace libcamera {
> +
> +static const std::map<uint8_t, PixelFormat> bitDepthToFmt {
> +       { 10, formats::RAW10_CRU },
> +       { 12, formats::RAW12_CRU },
> +       { 14, formats::RAW14_CRU },
> +};
> +
> +LOG_DEFINE_CATEGORY(RZG2LCRU)
> +
> +std::vector<Size> RZG2LCRU::sizes(unsigned int mbusCode) const
> +{
> +       std::vector<Size> sensorSizes = sensor_->sizes(mbusCode);
> +       std::vector<Size> cruSizes = {};
> +
> +       for (auto size : sensorSizes) {
> +               if (size > csi2Resolution_)
> +                       continue;
> +
> +               cruSizes.push_back(size);
> +       }
> +
> +       return cruSizes;
> +}
> +
> +const Size RZG2LCRU::resolution() const
> +{
> +       return sensor_->resolution().boundedTo(csi2Resolution_);
> +}
> +
> +void RZG2LCRU::setSensorAndCSI2Pointers(std::shared_ptr<CameraSensor> sensor,
> +                                       std::shared_ptr<V4L2Subdevice> csi2)
> +{

Setters for pointers sounds a bit ... odd ... I guess I'll see how it
develops.

> +       std::vector<Size> csi2Sizes;
> +
> +       sensor_ = sensor;
> +       csi2_ = csi2;
> +
> +       V4L2Subdevice::Formats formats = csi2_->formats(0);
> +       if (formats.empty())
> +               return;
> +
> +       for (const auto &format : formats) {
> +               const std::vector<SizeRange> &ranges = format.second;
> +               std::transform(ranges.begin(), ranges.end(), std::back_inserter(csi2Sizes),
> +                              [](const SizeRange &range) { return range.max; });
> +       }
> +
> +       csi2Resolution_ = csi2Sizes.back();
> +}
> +
> +FrameBuffer *RZG2LCRU::queueBuffer(Request *request)
> +{
> +       FrameBuffer *buffer;
> +
> +       if (availableBuffers_.empty()) {
> +               LOG(RZG2LCRU, Debug) << "CRU buffer underrun";
> +               return nullptr;
> +       }
> +
> +       buffer = availableBuffers_.front();
> +
> +       int ret = output_->queueBuffer(buffer);
> +       if (ret) {
> +               LOG(RZG2LCRU, Error) << "Failed to queue buffer to CRU";
> +               return nullptr;
> +       }
> +
> +       availableBuffers_.pop();
> +       buffer->_d()->setRequest(request);
> +
> +       return buffer;
> +}
> +
> +void RZG2LCRU::cruReturnBuffer(FrameBuffer *buffer)
> +{
> +       for (const std::unique_ptr<FrameBuffer> &buf : buffers_) {
> +               if (buf.get() == buffer) {
> +                       availableBuffers_.push(buffer);
> +                       break;
> +               }
> +       }
> +}
> +
> +int RZG2LCRU::start()
> +{
> +       int ret = output_->exportBuffers(kBufferCount, &buffers_);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = output_->importBuffers(kBufferCount);
> +       if (ret)
> +               return ret;
> +
> +       for (std::unique_ptr<FrameBuffer> &buffer : buffers_)
> +               availableBuffers_.push(buffer.get());
> +
> +       ret = output_->streamOn();
> +       if (ret) {
> +               freeBuffers();
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +int RZG2LCRU::stop()
> +{
> +       int ret;
> +
> +       csi2_->setFrameStartEnabled(false);
> +
> +       ret = output_->streamOff();
> +
> +       freeBuffers();
> +
> +       return ret;
> +}
> +
> +void RZG2LCRU::freeBuffers()
> +{
> +       availableBuffers_ = {};
> +       buffers_.clear();
> +
> +       if (output_->releaseBuffers())
> +               LOG(RZG2LCRU, Error) << "Failed to release CRU buffers";
> +}
> +
> +int RZG2LCRU::configure(V4L2SubdeviceFormat *subdevFormat, V4L2DeviceFormat *inputFormat)
> +{
> +       int ret;
> +
> +       /*
> +        * The sensor and CSI-2 rx have already had their format set by the
> +        * CameraData class...all we need to do is propagate it to the remaining
> +        * elements of the CRU graph - the CRU subdevice and output video device
> +        */
> +
> +       ret = cru_->setFormat(0, subdevFormat);
> +       if (ret)
> +               return ret;
> +
> +       ret = cru_->getFormat(1, subdevFormat);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * The capture device needs to be set with a format that can be produced
> +        * from the mbus code of the subdevFormat. The CRU and IVC use bayer
> +        * order agnostic pixel formats, so all we need to do is find the right
> +        * bitdepth and select the appropriate format.
> +        */
> +       BayerFormat bayerFormat = BayerFormat::fromMbusCode(subdevFormat->code);
> +       if (!bayerFormat.isValid())
> +               return -EINVAL;
> +
> +       PixelFormat pixelFormat = bitDepthToFmt.at(bayerFormat.bitDepth);
> +
> +       V4L2DeviceFormat captureFormat;
> +       captureFormat.fourcc = output_->toV4L2PixelFormat(pixelFormat);
> +       captureFormat.size = subdevFormat->size;
> +
> +       ret = output_->setFormat(&captureFormat);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * We return the format that we set against the output device, as the
> +        * same format will also need to be set against the Input Video Control
> +        * Block device.
> +        */
> +       *inputFormat = captureFormat;
> +
> +       return 0;
> +}
> +
> +int RZG2LCRU::init(const MediaDevice *media, MediaEntity **sensorEntity)
> +{
> +       int ret;
> +
> +       MediaEntity *csi2Entity = media->getEntityByName(std::regex("csi-[0-9a-f]{8}.csi2"));
> +       if (!csi2Entity)
> +               return -ENODEV;
> +
> +       const std::vector<MediaPad *> &pads = csi2Entity->pads();
> +       if (pads.empty())
> +               return -ENODEV;
> +
> +       /* The receiver has a single sink pad at index 0 */
> +       MediaPad *sink = pads[0];
> +       const std::vector<MediaLink *> &links = sink->links();
> +       if (links.empty())
> +               return -ENODEV;
> +
> +       MediaLink *link = links[0];
> +       *sensorEntity = link->source()->entity();
> +
> +       /*
> +        * We don't handle the sensor and csi-2 rx subdevice here, as the
> +        * CameraData class does that already.
> +        *
> +        * \todo lose this horrible hack by making modular pipelines.
> +        */
> +       cru_ = V4L2Subdevice::fromEntityName(media, std::regex("cru-ip-[0-9a-f]{8}.cru[0-9]"));
> +       ret = cru_->open();
> +       if (ret)
> +               return ret;
> +
> +       output_ = V4L2VideoDevice::fromEntityName(media, "CRU output");
> +       return output_->open();
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/mali-c55/rzg2l-cru.h b/src/libcamera/pipeline/mali-c55/rzg2l-cru.h
> new file mode 100644
> index 00000000..5beb3a6e
> --- /dev/null
> +++ b/src/libcamera/pipeline/mali-c55/rzg2l-cru.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2025, Ideas on Board Oy
> + *
> + * Pipine handler element for the Renesas RZ/G2L Camera Receiver Unit
> + */
> +
> +#pragma once
> +
> +#include <memory>
> +#include <vector>
> +
> +#include <libcamera/base/signal.h>
> +
> +#include "libcamera/internal/v4l2_subdevice.h"
> +#include "libcamera/internal/v4l2_videodevice.h"
> +
> +#include <queue>
> +
> +namespace libcamera {
> +
> +class CameraSensor;
> +class FrameBuffer;
> +class MediaDevice;
> +class PixelFormat;
> +class Request;
> +class Size;
> +class SizeRange;
> +struct StreamConfiguration;
> +enum class Transform;
> +
> +class RZG2LCRU
> +{
> +public:
> +       static constexpr unsigned int kBufferCount = 4;
> +
> +       RZG2LCRU() = default;
> +
> +       std::vector<Size> sizes(unsigned int mbusCode) const;
> +       int init(const MediaDevice *media, MediaEntity **sensorEntity);
> +       const Size resolution() const;
> +       void setSensorAndCSI2Pointers(std::shared_ptr<CameraSensor> sensor,
> +                                     std::shared_ptr<V4L2Subdevice> csi2);
> +       int configure(V4L2SubdeviceFormat *subdevFormat, V4L2DeviceFormat *inputFormat);
> +       FrameBuffer *queueBuffer(Request *request);
> +       void cruReturnBuffer(FrameBuffer *buffer);
> +       V4L2VideoDevice *output() { return output_.get(); }

I wonder if we can end up with something 'common' that is a simple CSI2
receiver with a V4L2VideoDevice - so we could use it for both Unicam



> +       int start();
> +       int stop();
> +private:
> +       void freeBuffers();
> +
> +       void cruBufferReady(FrameBuffer *buffer);
> +
> +       std::shared_ptr<CameraSensor> sensor_;
> +       std::shared_ptr<V4L2Subdevice> csi2_;
> +       std::unique_ptr<V4L2Subdevice> cru_;
> +       std::unique_ptr<V4L2VideoDevice> output_;
> +
> +       std::vector<std::unique_ptr<FrameBuffer>> buffers_;
> +       std::queue<FrameBuffer *> availableBuffers_;
> +
> +       Size csi2Resolution_;
> +};
> +
> +} /* namespace libcamera */
> -- 
> 2.30.2
>

Hi Dan

On Thu, Jul 24, 2025 at 07:52:53AM +0100, Daniel Scally wrote:
> Add a class allowing us to control the RZ/G2L Camera Receiver Unit
> which is found on the KakiP board. This will allow us to capture

I already expressed my preference for dropping G2L from everywhere,
but I would also drop KakiP (it's like advertising "support for
RockPi4" when we added the rkisp1 support).

I would simply mention CRU.

> buffers from the sensor, which in this configuration is not directly
> connected to the ISP.
>
> As this is not a guaranteed component of a video pipeline involving
> the Mali-C55 ISP we would ideally use modular pipelines to fill this
> role, but for now this will let us enable capture on the KakiP.

I already suggested that that pipeline/mali-c55 should become
pipeline/renesas-rzv2h/ and the only thing mali is actually the IPA.

I'm not asking to do it right away, and we should get at least one
more platform with a Mali ISP before deciding what to do here...

>
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  .clang-format                                 |   1 -
>  src/libcamera/pipeline/mali-c55/meson.build   |   3 +-
>  src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp | 236 ++++++++++++++++++
>  src/libcamera/pipeline/mali-c55/rzg2l-cru.h   |  66 +++++
>  4 files changed, 304 insertions(+), 2 deletions(-)
>  create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp
>  create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.h
>
> diff --git a/.clang-format b/.clang-format
> index 7fc30f61..c6b3dd24 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -75,7 +75,6 @@ IncludeCategories:
>      Priority:        9
>    # Qt includes (match before C++ standard library)
>    - Regex:           '<Q([A-Za-z0-9\-_])+>'
> -    CaseSensitive:   true

Is this intentional ?

>      Priority:        9
>    # Headers in <> with an extension. (+system libraries)
>    - Regex:           '<([A-Za-z0-9\-_])+\.h>'
> diff --git a/src/libcamera/pipeline/mali-c55/meson.build b/src/libcamera/pipeline/mali-c55/meson.build
> index eba8e5a3..4e768242 100644
> --- a/src/libcamera/pipeline/mali-c55/meson.build
> +++ b/src/libcamera/pipeline/mali-c55/meson.build
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>
>  libcamera_internal_sources += files([
> -    'mali-c55.cpp'
> +    'mali-c55.cpp',
> +    'rzg2l-cru.cpp',
>  ])
> diff --git a/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp b/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp
> new file mode 100644
> index 00000000..6b4e7b91
> --- /dev/null
> +++ b/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp

what about rz-cru ?

> @@ -0,0 +1,236 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2025, Ideas on Board Oy
> + *
> + * Pipine handler element for the Renesas RZ/G2L Camera Receiver Unit
> + */
> +
> +#include "rzg2l-cru.h"
> +
> +#include <map>
> +
> +#include <libcamera/base/log.h>
> +#include <libcamera/base/regex.h>
> +
> +#include <libcamera/formats.h>
> +#include <libcamera/pixel_format.h>
> +
> +#include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/camera_sensor.h"
> +#include "libcamera/internal/framebuffer.h"
> +#include "libcamera/internal/media_device.h"
> +#include "libcamera/internal/request.h"
> +
> +namespace libcamera {
> +
> +static const std::map<uint8_t, PixelFormat> bitDepthToFmt {
> +	{ 10, formats::RAW10_CRU },
> +	{ 12, formats::RAW12_CRU },
> +	{ 14, formats::RAW14_CRU },
> +};
> +
> +LOG_DEFINE_CATEGORY(RZG2LCRU)
> +
> +std::vector<Size> RZG2LCRU::sizes(unsigned int mbusCode) const
> +{
> +	std::vector<Size> sensorSizes = sensor_->sizes(mbusCode);
> +	std::vector<Size> cruSizes = {};
> +
> +	for (auto size : sensorSizes) {
> +		if (size > csi2Resolution_)
> +			continue;
> +
> +		cruSizes.push_back(size);
> +	}
> +
> +	return cruSizes;
> +}
> +
> +const Size RZG2LCRU::resolution() const
> +{
> +	return sensor_->resolution().boundedTo(csi2Resolution_);
> +}

Please have a look at the fixup I sent you, this won't work well if
the sensor sizes are larger than the CRU supported sizes.

> +
> +void RZG2LCRU::setSensorAndCSI2Pointers(std::shared_ptr<CameraSensor> sensor,
> +					std::shared_ptr<V4L2Subdevice> csi2)

The CRU should never outlive the pipeline handler, I wonder if passing
shared_ptr<> by copy (increasing the reference counter) is necessary

Apart from that nit, I don't like the fact you have, in
MaliC55::registerMemoryInputCamera() the following pattern

	data->cru_ = std::make_unique<RZG2LCRU>();
	ret = data->cru_->init(cruMedia_, &sensorEntity);
	if (ret)
		return false;

	if (data->init(sensorEntity))
		return false;

	data->cru_->setSensorAndCSI2Pointers(data->sensor_, data->csi_);

CRU::init() actually finds sensorEntity() but then it passes it
back to the pipeline, that makes a CameraSensor out of it, then finds
the CSI-2 receiver entity connected to it, and then set it back to
the CRU.

Yes, re-using MaliC55CameraData::init() for the CRU as well might save
some lines of code that should be duplicated in the CRU class, but I
don't think this is worth the complexity.

I think the CRU should be self-contained and handle the creation and
configuration of CSI-2 and CameraSensor.

In this way, if the pipeline handler uses the CRU, it shouldn't access
sensor_ or csi_ at all, but it will only handle them if the ISP is
used in-line.

Unless I'm missing something, this should be doable and will

1) Make the pipeline handler code less convoluted
2) Make it easier to split the inline mode from the offline mode
whenever we can make the pipeline modular.

I can have a look at this and possibly send patches your way.

> +{
> +	std::vector<Size> csi2Sizes;
> +
> +	sensor_ = sensor;
> +	csi2_ = csi2;
> +
> +	V4L2Subdevice::Formats formats = csi2_->formats(0);
> +	if (formats.empty())
> +		return;
> +
> +	for (const auto &format : formats) {
> +		const std::vector<SizeRange> &ranges = format.second;
> +		std::transform(ranges.begin(), ranges.end(), std::back_inserter(csi2Sizes),
> +			       [](const SizeRange &range) { return range.max; });
> +	}
> +
> +	csi2Resolution_ = csi2Sizes.back();
> +}
> +
> +FrameBuffer *RZG2LCRU::queueBuffer(Request *request)
> +{
> +	FrameBuffer *buffer;
> +
> +	if (availableBuffers_.empty()) {
> +		LOG(RZG2LCRU, Debug) << "CRU buffer underrun";
> +		return nullptr;
> +	}
> +
> +	buffer = availableBuffers_.front();
> +
> +	int ret = output_->queueBuffer(buffer);
> +	if (ret) {
> +		LOG(RZG2LCRU, Error) << "Failed to queue buffer to CRU";
> +		return nullptr;
> +	}
> +
> +	availableBuffers_.pop();
> +	buffer->_d()->setRequest(request);
> +
> +	return buffer;
> +}
> +
> +void RZG2LCRU::cruReturnBuffer(FrameBuffer *buffer)

s/RZG2LCRU::cruReturnBuffer/RZG2LCRU::returnBuffer

> +{
> +	for (const std::unique_ptr<FrameBuffer> &buf : buffers_) {
> +		if (buf.get() == buffer) {

I'm not sure I get the purpose of this for() { if() }

Do you want to make sure the buffer acutally belongs to the CRU ?
Then we should assert somehow if that's not the case

Something like the below not tested nor compiled code should do


        auto it = std::find_if(buffers_.begin(), buffers.end(),
                               [buffer](const auto &b) {
                                        return b.get() == buffer;
                               });
        assert(it != buffers.end());

        availableBuffers_.push(buffer);

> +			availableBuffers_.push(buffer);
> +			break;
> +		}
> +	}
> +}
> +
> +int RZG2LCRU::start()
> +{
> +	int ret = output_->exportBuffers(kBufferCount, &buffers_);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = output_->importBuffers(kBufferCount);
> +	if (ret)
> +		return ret;
> +
> +	for (std::unique_ptr<FrameBuffer> &buffer : buffers_)
> +		availableBuffers_.push(buffer.get());
> +
> +	ret = output_->streamOn();
> +	if (ret) {
> +		freeBuffers();
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int RZG2LCRU::stop()
> +{
> +	int ret;
> +
> +	csi2_->setFrameStartEnabled(false);

This is never set to true though

> +
> +	ret = output_->streamOff();
> +
> +	freeBuffers();
> +
> +	return ret;
> +}
> +
> +void RZG2LCRU::freeBuffers()
> +{
> +	availableBuffers_ = {};
> +	buffers_.clear();
> +
> +	if (output_->releaseBuffers())
> +		LOG(RZG2LCRU, Error) << "Failed to release CRU buffers";
> +}
> +
> +int RZG2LCRU::configure(V4L2SubdeviceFormat *subdevFormat, V4L2DeviceFormat *inputFormat)
> +{
> +	int ret;
> +
> +	/*
> +	 * The sensor and CSI-2 rx have already had their format set by the
> +	 * CameraData class...all we need to do is propagate it to the remaining
> +	 * elements of the CRU graph - the CRU subdevice and output video device

Note: I wrote this before the above comments on making the CRU class
self-contained, so I'm repeating myself a bit here.

Isn't this something we might want to move here ? (setting formats on
CSI-2 and sensor, I mean). The Mali pipeline has a bit of a complex
handling of the 3 possible inputs (TPG/inline CSI-2/m2m CRU) and maybe
moving things here could simpify things like:

        if (sensor_) {

        }

        if (csi_) {
                if (cru_) {

                } else {

                }
        }

That I see in MaliC55::configure(), down to

        if (cru_) {
                cru_->configure();
        } else if (csi_) {
                sensor_->configure();
                csi_->configure();
        } else {
                /* TPG */
        }

If I read the code in the mali-c55 pipeline right the
if (sensor_) and if (csi_) conditions are actually the same because of

	sensor_ = CameraSensorFactoryBase::create(entity_);
	if (!sensor_)
		return -ENODEV;

	const MediaPad *sourcePad = entity_->getPadByIndex(0);
	MediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity();

	csi_ = std::make_unique<V4L2Subdevice>(csiEntity);

In other words, if you have sensor_ you will have csi_, isn't it ?

> +	 */
> +
> +	ret = cru_->setFormat(0, subdevFormat);

        int ret

> +	if (ret)
> +		return ret;
> +
> +	ret = cru_->getFormat(1, subdevFormat);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The capture device needs to be set with a format that can be produced
> +	 * from the mbus code of the subdevFormat. The CRU and IVC use bayer
> +	 * order agnostic pixel formats, so all we need to do is find the right
> +	 * bitdepth and select the appropriate format.
> +	 */
> +	BayerFormat bayerFormat = BayerFormat::fromMbusCode(subdevFormat->code);
> +	if (!bayerFormat.isValid())
> +		return -EINVAL;
> +
> +	PixelFormat pixelFormat = bitDepthToFmt.at(bayerFormat.bitDepth);

is there any risk the desired bit depth is not in the map ?

> +
> +	V4L2DeviceFormat captureFormat;
> +	captureFormat.fourcc = output_->toV4L2PixelFormat(pixelFormat);
> +	captureFormat.size = subdevFormat->size;
> +
> +	ret = output_->setFormat(&captureFormat);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * We return the format that we set against the output device, as the
> +	 * same format will also need to be set against the Input Video Control
> +	 * Block device.
> +	 */
> +	*inputFormat = captureFormat;
> +
> +	return 0;
> +}
> +
> +int RZG2LCRU::init(const MediaDevice *media, MediaEntity **sensorEntity)
> +{
> +	int ret;
> +
> +	MediaEntity *csi2Entity = media->getEntityByName(std::regex("csi-[0-9a-f]{8}.csi2"));
> +	if (!csi2Entity)
> +		return -ENODEV;
> +
> +	const std::vector<MediaPad *> &pads = csi2Entity->pads();
> +	if (pads.empty())
> +		return -ENODEV;
> +
> +	/* The receiver has a single sink pad at index 0 */
> +	MediaPad *sink = pads[0];
> +	const std::vector<MediaLink *> &links = sink->links();
> +	if (links.empty())
> +		return -ENODEV;
> +
> +	MediaLink *link = links[0];
> +	*sensorEntity = link->source()->entity();
> +
> +	/*
> +	 * We don't handle the sensor and csi-2 rx subdevice here, as the
> +	 * CameraData class does that already.
> +	 *

Ah, see... I think we should instead

> +	 * \todo lose this horrible hack by making modular pipelines.
> +	 */
> +	cru_ = V4L2Subdevice::fromEntityName(media, std::regex("cru-ip-[0-9a-f]{8}.cru[0-9]"));
> +	ret = cru_->open();
> +	if (ret)
> +		return ret;
> +
> +	output_ = V4L2VideoDevice::fromEntityName(media, "CRU output");
> +	return output_->open();
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/mali-c55/rzg2l-cru.h b/src/libcamera/pipeline/mali-c55/rzg2l-cru.h
> new file mode 100644
> index 00000000..5beb3a6e
> --- /dev/null
> +++ b/src/libcamera/pipeline/mali-c55/rzg2l-cru.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2025, Ideas on Board Oy
> + *
> + * Pipine handler element for the Renesas RZ/G2L Camera Receiver Unit
> + */
> +
> +#pragma once
> +
> +#include <memory>
> +#include <vector>
> +
> +#include <libcamera/base/signal.h>
> +
> +#include "libcamera/internal/v4l2_subdevice.h"
> +#include "libcamera/internal/v4l2_videodevice.h"
> +
> +#include <queue>

Move it up with other includes from STL

> +
> +namespace libcamera {
> +
> +class CameraSensor;
> +class FrameBuffer;
> +class MediaDevice;
> +class PixelFormat;
> +class Request;
> +class Size;
> +class SizeRange;
> +struct StreamConfiguration;
> +enum class Transform;
> +
> +class RZG2LCRU
> +{
> +public:
> +	static constexpr unsigned int kBufferCount = 4;
> +
> +	RZG2LCRU() = default;

No need to I guess

> +
> +	std::vector<Size> sizes(unsigned int mbusCode) const;
> +	int init(const MediaDevice *media, MediaEntity **sensorEntity);
> +	const Size resolution() const;
> +	void setSensorAndCSI2Pointers(std::shared_ptr<CameraSensor> sensor,
> +				      std::shared_ptr<V4L2Subdevice> csi2);
> +	int configure(V4L2SubdeviceFormat *subdevFormat, V4L2DeviceFormat *inputFormat);
> +	FrameBuffer *queueBuffer(Request *request);
> +	void cruReturnBuffer(FrameBuffer *buffer);
> +	V4L2VideoDevice *output() { return output_.get(); }
> +	int start();
> +	int stop();

Empty line maybe

Thanks
  j

> +private:
> +	void freeBuffers();
> +
> +	void cruBufferReady(FrameBuffer *buffer);
> +
> +	std::shared_ptr<CameraSensor> sensor_;
> +	std::shared_ptr<V4L2Subdevice> csi2_;
> +	std::unique_ptr<V4L2Subdevice> cru_;
> +	std::unique_ptr<V4L2VideoDevice> output_;
> +
> +	std::vector<std::unique_ptr<FrameBuffer>> buffers_;
> +	std::queue<FrameBuffer *> availableBuffers_;
> +
> +	Size csi2Resolution_;
> +};
> +
> +} /* namespace libcamera */
> --
> 2.30.2
>