[{"id":35083,"web_url":"https://patchwork.libcamera.org/comment/35083/","msgid":"<175334657367.560048.16108750255691483760@ping.linuxembedded.co.uk>","date":"2025-07-24T08:42:53","subject":"Re: [PATCH 07/10] libcamera: mali-c55: Add RZG2LCRU class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Daniel Scally (2025-07-24 07:52:53)\n> Add a class allowing us to control the RZ/G2L Camera Receiver Unit\n> which is found on the KakiP board. This will allow us to capture\n> buffers from the sensor, which in this configuration is not directly\n> connected to the ISP.\n> \n> As this is not a guaranteed component of a video pipeline involving\n> the Mali-C55 ISP we would ideally use modular pipelines to fill this\n> role, but for now this will let us enable capture on the KakiP.\n> \n\nIs the Mali-C55 using the MediaPipeline component yet to support\narbitrary length media graphs ?\n\nI wonder if thats' where we could factor out the CSI2 receiver too\nsometime as this component looks like it could be quite generic too.\n\nBut to support getting the Kakip supported, I'd be fine merging this.\n\n> Signed-off-by: Daniel Scally \n> ---\n> .clang-format | 1 -\n> src/libcamera/pipeline/mali-c55/meson.build | 3 +-\n> src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp | 236 ++++++++++++++++++\n> src/libcamera/pipeline/mali-c55/rzg2l-cru.h | 66 +++++\n> 4 files changed, 304 insertions(+), 2 deletions(-)\n> create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp\n> create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.h\n> \n> diff --git a/.clang-format b/.clang-format\n> index 7fc30f61..c6b3dd24 100644\n> --- a/.clang-format\n> +++ b/.clang-format\n> @@ -75,7 +75,6 @@ IncludeCategories:\n> Priority: 9\n> # Qt includes (match before C++ standard library)\n> - Regex: ''\n> - CaseSensitive: true\n> Priority: 9\n\nI don't think this should be in here.\n\n\n> # Headers in <> with an extension. (+system libraries)\n> - Regex: '<([A-Za-z0-9\\-_])+\\.h>'\n> diff --git a/src/libcamera/pipeline/mali-c55/meson.build b/src/libcamera/pipeline/mali-c55/meson.build\n> index eba8e5a3..4e768242 100644\n> --- a/src/libcamera/pipeline/mali-c55/meson.build\n> +++ b/src/libcamera/pipeline/mali-c55/meson.build\n> @@ -1,5 +1,6 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_internal_sources += files([\n> - 'mali-c55.cpp'\n> + 'mali-c55.cpp',\n> + 'rzg2l-cru.cpp',\n> ])\n> diff --git a/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp b/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp\n> new file mode 100644\n> index 00000000..6b4e7b91\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp\n> @@ -0,0 +1,236 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas on Board Oy\n> + *\n> + * Pipine handler element for the Renesas RZ/G2L Camera Receiver Unit\n> + */\n> +\n> +#include \"rzg2l-cru.h\"\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/framebuffer.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> +#include \"libcamera/internal/request.h\"\n> +\n> +namespace libcamera {\n> +\n> +static const std::map bitDepthToFmt {\n> + { 10, formats::RAW10_CRU },\n> + { 12, formats::RAW12_CRU },\n> + { 14, formats::RAW14_CRU },\n> +};\n> +\n> +LOG_DEFINE_CATEGORY(RZG2LCRU)\n> +\n> +std::vector RZG2LCRU::sizes(unsigned int mbusCode) const\n> +{\n> + std::vector sensorSizes = sensor_->sizes(mbusCode);\n> + std::vector cruSizes = {};\n> +\n> + for (auto size : sensorSizes) {\n> + if (size > csi2Resolution_)\n> + continue;\n> +\n> + cruSizes.push_back(size);\n> + }\n> +\n> + return cruSizes;\n> +}\n> +\n> +const Size RZG2LCRU::resolution() const\n> +{\n> + return sensor_->resolution().boundedTo(csi2Resolution_);\n> +}\n> +\n> +void RZG2LCRU::setSensorAndCSI2Pointers(std::shared_ptr sensor,\n> + std::shared_ptr csi2)\n> +{\n\nSetters for pointers sounds a bit ... odd ... I guess I'll see how it\ndevelops.\n\n> + std::vector csi2Sizes;\n> +\n> + sensor_ = sensor;\n> + csi2_ = csi2;\n> +\n> + V4L2Subdevice::Formats formats = csi2_->formats(0);\n> + if (formats.empty())\n> + return;\n> +\n> + for (const auto &format : formats) {\n> + const std::vector &ranges = format.second;\n> + std::transform(ranges.begin(), ranges.end(), std::back_inserter(csi2Sizes),\n> + [](const SizeRange &range) { return range.max; });\n> + }\n> +\n> + csi2Resolution_ = csi2Sizes.back();\n> +}\n> +\n> +FrameBuffer *RZG2LCRU::queueBuffer(Request *request)\n> +{\n> + FrameBuffer *buffer;\n> +\n> + if (availableBuffers_.empty()) {\n> + LOG(RZG2LCRU, Debug) << \"CRU buffer underrun\";\n> + return nullptr;\n> + }\n> +\n> + buffer = availableBuffers_.front();\n> +\n> + int ret = output_->queueBuffer(buffer);\n> + if (ret) {\n> + LOG(RZG2LCRU, Error) << \"Failed to queue buffer to CRU\";\n> + return nullptr;\n> + }\n> +\n> + availableBuffers_.pop();\n> + buffer->_d()->setRequest(request);\n> +\n> + return buffer;\n> +}\n> +\n> +void RZG2LCRU::cruReturnBuffer(FrameBuffer *buffer)\n> +{\n> + for (const std::unique_ptr &buf : buffers_) {\n> + if (buf.get() == buffer) {\n> + availableBuffers_.push(buffer);\n> + break;\n> + }\n> + }\n> +}\n> +\n> +int RZG2LCRU::start()\n> +{\n> + int ret = output_->exportBuffers(kBufferCount, &buffers_);\n> + if (ret < 0)\n> + return ret;\n> +\n> + ret = output_->importBuffers(kBufferCount);\n> + if (ret)\n> + return ret;\n> +\n> + for (std::unique_ptr &buffer : buffers_)\n> + availableBuffers_.push(buffer.get());\n> +\n> + ret = output_->streamOn();\n> + if (ret) {\n> + freeBuffers();\n> + return ret;\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +int RZG2LCRU::stop()\n> +{\n> + int ret;\n> +\n> + csi2_->setFrameStartEnabled(false);\n> +\n> + ret = output_->streamOff();\n> +\n> + freeBuffers();\n> +\n> + return ret;\n> +}\n> +\n> +void RZG2LCRU::freeBuffers()\n> +{\n> + availableBuffers_ = {};\n> + buffers_.clear();\n> +\n> + if (output_->releaseBuffers())\n> + LOG(RZG2LCRU, Error) << \"Failed to release CRU buffers\";\n> +}\n> +\n> +int RZG2LCRU::configure(V4L2SubdeviceFormat *subdevFormat, V4L2DeviceFormat *inputFormat)\n> +{\n> + int ret;\n> +\n> + /*\n> + * The sensor and CSI-2 rx have already had their format set by the\n> + * CameraData class...all we need to do is propagate it to the remaining\n> + * elements of the CRU graph - the CRU subdevice and output video device\n> + */\n> +\n> + ret = cru_->setFormat(0, subdevFormat);\n> + if (ret)\n> + return ret;\n> +\n> + ret = cru_->getFormat(1, subdevFormat);\n> + if (ret)\n> + return ret;\n> +\n> + /*\n> + * The capture device needs to be set with a format that can be produced\n> + * from the mbus code of the subdevFormat. The CRU and IVC use bayer\n> + * order agnostic pixel formats, so all we need to do is find the right\n> + * bitdepth and select the appropriate format.\n> + */\n> + BayerFormat bayerFormat = BayerFormat::fromMbusCode(subdevFormat->code);\n> + if (!bayerFormat.isValid())\n> + return -EINVAL;\n> +\n> + PixelFormat pixelFormat = bitDepthToFmt.at(bayerFormat.bitDepth);\n> +\n> + V4L2DeviceFormat captureFormat;\n> + captureFormat.fourcc = output_->toV4L2PixelFormat(pixelFormat);\n> + captureFormat.size = subdevFormat->size;\n> +\n> + ret = output_->setFormat(&captureFormat);\n> + if (ret)\n> + return ret;\n> +\n> + /*\n> + * We return the format that we set against the output device, as the\n> + * same format will also need to be set against the Input Video Control\n> + * Block device.\n> + */\n> + *inputFormat = captureFormat;\n> +\n> + return 0;\n> +}\n> +\n> +int RZG2LCRU::init(const MediaDevice *media, MediaEntity **sensorEntity)\n> +{\n> + int ret;\n> +\n> + MediaEntity *csi2Entity = media->getEntityByName(std::regex(\"csi-[0-9a-f]{8}.csi2\"));\n> + if (!csi2Entity)\n> + return -ENODEV;\n> +\n> + const std::vector &pads = csi2Entity->pads();\n> + if (pads.empty())\n> + return -ENODEV;\n> +\n> + /* The receiver has a single sink pad at index 0 */\n> + MediaPad *sink = pads[0];\n> + const std::vector &links = sink->links();\n> + if (links.empty())\n> + return -ENODEV;\n> +\n> + MediaLink *link = links[0];\n> + *sensorEntity = link->source()->entity();\n> +\n> + /*\n> + * We don't handle the sensor and csi-2 rx subdevice here, as the\n> + * CameraData class does that already.\n> + *\n> + * \\todo lose this horrible hack by making modular pipelines.\n> + */\n> + cru_ = V4L2Subdevice::fromEntityName(media, std::regex(\"cru-ip-[0-9a-f]{8}.cru[0-9]\"));\n> + ret = cru_->open();\n> + if (ret)\n> + return ret;\n> +\n> + output_ = V4L2VideoDevice::fromEntityName(media, \"CRU output\");\n> + return output_->open();\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/mali-c55/rzg2l-cru.h b/src/libcamera/pipeline/mali-c55/rzg2l-cru.h\n> new file mode 100644\n> index 00000000..5beb3a6e\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/mali-c55/rzg2l-cru.h\n> @@ -0,0 +1,66 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas on Board Oy\n> + *\n> + * Pipine handler element for the Renesas RZ/G2L Camera Receiver Unit\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/v4l2_subdevice.h\"\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class CameraSensor;\n> +class FrameBuffer;\n> +class MediaDevice;\n> +class PixelFormat;\n> +class Request;\n> +class Size;\n> +class SizeRange;\n> +struct StreamConfiguration;\n> +enum class Transform;\n> +\n> +class RZG2LCRU\n> +{\n> +public:\n> + static constexpr unsigned int kBufferCount = 4;\n> +\n> + RZG2LCRU() = default;\n> +\n> + std::vector sizes(unsigned int mbusCode) const;\n> + int init(const MediaDevice *media, MediaEntity **sensorEntity);\n> + const Size resolution() const;\n> + void setSensorAndCSI2Pointers(std::shared_ptr sensor,\n> + std::shared_ptr csi2);\n> + int configure(V4L2SubdeviceFormat *subdevFormat, V4L2DeviceFormat *inputFormat);\n> + FrameBuffer *queueBuffer(Request *request);\n> + void cruReturnBuffer(FrameBuffer *buffer);\n> + V4L2VideoDevice *output() { return output_.get(); }\n\nI wonder if we can end up with something 'common' that is a simple CSI2\nreceiver with a V4L2VideoDevice - so we could use it for both Unicam\n\n\n\n> + int start();\n> + int stop();\n> +private:\n> + void freeBuffers();\n> +\n> + void cruBufferReady(FrameBuffer *buffer);\n> +\n> + std::shared_ptr sensor_;\n> + std::shared_ptr csi2_;\n> + std::unique_ptr cru_;\n> + std::unique_ptr output_;\n> +\n> + std::vector> buffers_;\n> + std::queue availableBuffers_;\n> +\n> + Size csi2Resolution_;\n> +};\n> +\n> +} /* namespace libcamera */\n> -- \n> 2.30.2\n>","headers":{"Return-Path":"","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 F2741BDCC1\n\tfor ;\n\tThu, 24 Jul 2025 08:42:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 24479690BD;\n\tThu, 24 Jul 2025 10:42: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 AF076690B4\n\tfor ;\n\tThu, 24 Jul 2025 10:42:56 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B07777E1;\n\tThu, 24 Jul 2025 10:42:17 +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=\"MrVe5hSi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1753346537;\n\tbh=tQRpTdvIt1JXKv0+pemXaS5dtbE2HFhtd70I0DyA4fY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=MrVe5hSiK/XN70LjnnabtYOwtpQpFPIHV/IthuaDfGReBpegi/BG9UiB+FPXbX++4\n\tFTtUavmIrEH70YNuO1ujlJgnzNi3zGnurLQi5PX0j3eg1RnaCuflld2aJiMJLLisWD\n\tl4K987Fnyou1vZpUnNgvlDtJaaeyk5DGh/SGFTls=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250724065256.75175-8-dan.scally@ideasonboard.com>","References":"<20250724065256.75175-1-dan.scally@ideasonboard.com>\n\t<20250724065256.75175-8-dan.scally@ideasonboard.com>","Subject":"Re: [PATCH 07/10] libcamera: mali-c55: Add RZG2LCRU class","From":"Kieran Bingham ","Cc":"Daniel Scally ","To":"Daniel Scally ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 24 Jul 2025 09:42:53 +0100","Message-ID":"<175334657367.560048.16108750255691483760@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36555,"web_url":"https://patchwork.libcamera.org/comment/36555/","msgid":"","date":"2025-10-30T13:46:16","subject":"Re: [PATCH 07/10] libcamera: mali-c55: Add RZG2LCRU class","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Thu, Jul 24, 2025 at 07:52:53AM +0100, Daniel Scally wrote:\n> Add a class allowing us to control the RZ/G2L Camera Receiver Unit\n> which is found on the KakiP board. This will allow us to capture\n\nI already expressed my preference for dropping G2L from everywhere,\nbut I would also drop KakiP (it's like advertising \"support for\nRockPi4\" when we added the rkisp1 support).\n\nI would simply mention CRU.\n\n> buffers from the sensor, which in this configuration is not directly\n> connected to the ISP.\n>\n> As this is not a guaranteed component of a video pipeline involving\n> the Mali-C55 ISP we would ideally use modular pipelines to fill this\n> role, but for now this will let us enable capture on the KakiP.\n\nI already suggested that that pipeline/mali-c55 should become\npipeline/renesas-rzv2h/ and the only thing mali is actually the IPA.\n\nI'm not asking to do it right away, and we should get at least one\nmore platform with a Mali ISP before deciding what to do here...\n\n>\n> Signed-off-by: Daniel Scally \n> ---\n> .clang-format | 1 -\n> src/libcamera/pipeline/mali-c55/meson.build | 3 +-\n> src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp | 236 ++++++++++++++++++\n> src/libcamera/pipeline/mali-c55/rzg2l-cru.h | 66 +++++\n> 4 files changed, 304 insertions(+), 2 deletions(-)\n> create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp\n> create mode 100644 src/libcamera/pipeline/mali-c55/rzg2l-cru.h\n>\n> diff --git a/.clang-format b/.clang-format\n> index 7fc30f61..c6b3dd24 100644\n> --- a/.clang-format\n> +++ b/.clang-format\n> @@ -75,7 +75,6 @@ IncludeCategories:\n> Priority: 9\n> # Qt includes (match before C++ standard library)\n> - Regex: ''\n> - CaseSensitive: true\n\nIs this intentional ?\n\n> Priority: 9\n> # Headers in <> with an extension. (+system libraries)\n> - Regex: '<([A-Za-z0-9\\-_])+\\.h>'\n> diff --git a/src/libcamera/pipeline/mali-c55/meson.build b/src/libcamera/pipeline/mali-c55/meson.build\n> index eba8e5a3..4e768242 100644\n> --- a/src/libcamera/pipeline/mali-c55/meson.build\n> +++ b/src/libcamera/pipeline/mali-c55/meson.build\n> @@ -1,5 +1,6 @@\n> # SPDX-License-Identifier: CC0-1.0\n>\n> libcamera_internal_sources += files([\n> - 'mali-c55.cpp'\n> + 'mali-c55.cpp',\n> + 'rzg2l-cru.cpp',\n> ])\n> diff --git a/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp b/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp\n> new file mode 100644\n> index 00000000..6b4e7b91\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/mali-c55/rzg2l-cru.cpp\n\nwhat about rz-cru ?\n\n> @@ -0,0 +1,236 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas on Board Oy\n> + *\n> + * Pipine handler element for the Renesas RZ/G2L Camera Receiver Unit\n> + */\n> +\n> +#include \"rzg2l-cru.h\"\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/framebuffer.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> +#include \"libcamera/internal/request.h\"\n> +\n> +namespace libcamera {\n> +\n> +static const std::map bitDepthToFmt {\n> +\t{ 10, formats::RAW10_CRU },\n> +\t{ 12, formats::RAW12_CRU },\n> +\t{ 14, formats::RAW14_CRU },\n> +};\n> +\n> +LOG_DEFINE_CATEGORY(RZG2LCRU)\n> +\n> +std::vector RZG2LCRU::sizes(unsigned int mbusCode) const\n> +{\n> +\tstd::vector sensorSizes = sensor_->sizes(mbusCode);\n> +\tstd::vector cruSizes = {};\n> +\n> +\tfor (auto size : sensorSizes) {\n> +\t\tif (size > csi2Resolution_)\n> +\t\t\tcontinue;\n> +\n> +\t\tcruSizes.push_back(size);\n> +\t}\n> +\n> +\treturn cruSizes;\n> +}\n> +\n> +const Size RZG2LCRU::resolution() const\n> +{\n> +\treturn sensor_->resolution().boundedTo(csi2Resolution_);\n> +}\n\nPlease have a look at the fixup I sent you, this won't work well if\nthe sensor sizes are larger than the CRU supported sizes.\n\n> +\n> +void RZG2LCRU::setSensorAndCSI2Pointers(std::shared_ptr sensor,\n> +\t\t\t\t\tstd::shared_ptr csi2)\n\nThe CRU should never outlive the pipeline handler, I wonder if passing\nshared_ptr<> by copy (increasing the reference counter) is necessary\n\nApart from that nit, I don't like the fact you have, in\nMaliC55::registerMemoryInputCamera() the following pattern\n\n\tdata->cru_ = std::make_unique();\n\tret = data->cru_->init(cruMedia_, &sensorEntity);\n\tif (ret)\n\t\treturn false;\n\n\tif (data->init(sensorEntity))\n\t\treturn false;\n\n\tdata->cru_->setSensorAndCSI2Pointers(data->sensor_, data->csi_);\n\nCRU::init() actually finds sensorEntity() but then it passes it\nback to the pipeline, that makes a CameraSensor out of it, then finds\nthe CSI-2 receiver entity connected to it, and then set it back to\nthe CRU.\n\nYes, re-using MaliC55CameraData::init() for the CRU as well might save\nsome lines of code that should be duplicated in the CRU class, but I\ndon't think this is worth the complexity.\n\nI think the CRU should be self-contained and handle the creation and\nconfiguration of CSI-2 and CameraSensor.\n\nIn this way, if the pipeline handler uses the CRU, it shouldn't access\nsensor_ or csi_ at all, but it will only handle them if the ISP is\nused in-line.\n\nUnless I'm missing something, this should be doable and will\n\n1) Make the pipeline handler code less convoluted\n2) Make it easier to split the inline mode from the offline mode\nwhenever we can make the pipeline modular.\n\nI can have a look at this and possibly send patches your way.\n\n> +{\n> +\tstd::vector csi2Sizes;\n> +\n> +\tsensor_ = sensor;\n> +\tcsi2_ = csi2;\n> +\n> +\tV4L2Subdevice::Formats formats = csi2_->formats(0);\n> +\tif (formats.empty())\n> +\t\treturn;\n> +\n> +\tfor (const auto &format : formats) {\n> +\t\tconst std::vector &ranges = format.second;\n> +\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(csi2Sizes),\n> +\t\t\t [](const SizeRange &range) { return range.max; });\n> +\t}\n> +\n> +\tcsi2Resolution_ = csi2Sizes.back();\n> +}\n> +\n> +FrameBuffer *RZG2LCRU::queueBuffer(Request *request)\n> +{\n> +\tFrameBuffer *buffer;\n> +\n> +\tif (availableBuffers_.empty()) {\n> +\t\tLOG(RZG2LCRU, Debug) << \"CRU buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tbuffer = availableBuffers_.front();\n> +\n> +\tint ret = output_->queueBuffer(buffer);\n> +\tif (ret) {\n> +\t\tLOG(RZG2LCRU, Error) << \"Failed to queue buffer to CRU\";\n> +\t\treturn nullptr;\n> +\t}\n> +\n> +\tavailableBuffers_.pop();\n> +\tbuffer->_d()->setRequest(request);\n> +\n> +\treturn buffer;\n> +}\n> +\n> +void RZG2LCRU::cruReturnBuffer(FrameBuffer *buffer)\n\ns/RZG2LCRU::cruReturnBuffer/RZG2LCRU::returnBuffer\n\n> +{\n> +\tfor (const std::unique_ptr &buf : buffers_) {\n> +\t\tif (buf.get() == buffer) {\n\nI'm not sure I get the purpose of this for() { if() }\n\nDo you want to make sure the buffer acutally belongs to the CRU ?\nThen we should assert somehow if that's not the case\n\nSomething like the below not tested nor compiled code should do\n\n\n auto it = std::find_if(buffers_.begin(), buffers.end(),\n [buffer](const auto &b) {\n return b.get() == buffer;\n });\n assert(it != buffers.end());\n\n availableBuffers_.push(buffer);\n\n> +\t\t\tavailableBuffers_.push(buffer);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +}\n> +\n> +int RZG2LCRU::start()\n> +{\n> +\tint ret = output_->exportBuffers(kBufferCount, &buffers_);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tret = output_->importBuffers(kBufferCount);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tfor (std::unique_ptr &buffer : buffers_)\n> +\t\tavailableBuffers_.push(buffer.get());\n> +\n> +\tret = output_->streamOn();\n> +\tif (ret) {\n> +\t\tfreeBuffers();\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int RZG2LCRU::stop()\n> +{\n> +\tint ret;\n> +\n> +\tcsi2_->setFrameStartEnabled(false);\n\nThis is never set to true though\n\n> +\n> +\tret = output_->streamOff();\n> +\n> +\tfreeBuffers();\n> +\n> +\treturn ret;\n> +}\n> +\n> +void RZG2LCRU::freeBuffers()\n> +{\n> +\tavailableBuffers_ = {};\n> +\tbuffers_.clear();\n> +\n> +\tif (output_->releaseBuffers())\n> +\t\tLOG(RZG2LCRU, Error) << \"Failed to release CRU buffers\";\n> +}\n> +\n> +int RZG2LCRU::configure(V4L2SubdeviceFormat *subdevFormat, V4L2DeviceFormat *inputFormat)\n> +{\n> +\tint ret;\n> +\n> +\t/*\n> +\t * The sensor and CSI-2 rx have already had their format set by the\n> +\t * CameraData class...all we need to do is propagate it to the remaining\n> +\t * elements of the CRU graph - the CRU subdevice and output video device\n\nNote: I wrote this before the above comments on making the CRU class\nself-contained, so I'm repeating myself a bit here.\n\nIsn't this something we might want to move here ? (setting formats on\nCSI-2 and sensor, I mean). The Mali pipeline has a bit of a complex\nhandling of the 3 possible inputs (TPG/inline CSI-2/m2m CRU) and maybe\nmoving things here could simpify things like:\n\n if (sensor_) {\n\n }\n\n if (csi_) {\n if (cru_) {\n\n } else {\n\n }\n }\n\nThat I see in MaliC55::configure(), down to\n\n if (cru_) {\n cru_->configure();\n } else if (csi_) {\n sensor_->configure();\n csi_->configure();\n } else {\n /* TPG */\n }\n\nIf I read the code in the mali-c55 pipeline right the\nif (sensor_) and if (csi_) conditions are actually the same because of\n\n\tsensor_ = CameraSensorFactoryBase::create(entity_);\n\tif (!sensor_)\n\t\treturn -ENODEV;\n\n\tconst MediaPad *sourcePad = entity_->getPadByIndex(0);\n\tMediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity();\n\n\tcsi_ = std::make_unique(csiEntity);\n\nIn other words, if you have sensor_ you will have csi_, isn't it ?\n\n> +\t */\n> +\n> +\tret = cru_->setFormat(0, subdevFormat);\n\n int ret\n\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = cru_->getFormat(1, subdevFormat);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/*\n> +\t * The capture device needs to be set with a format that can be produced\n> +\t * from the mbus code of the subdevFormat. The CRU and IVC use bayer\n> +\t * order agnostic pixel formats, so all we need to do is find the right\n> +\t * bitdepth and select the appropriate format.\n> +\t */\n> +\tBayerFormat bayerFormat = BayerFormat::fromMbusCode(subdevFormat->code);\n> +\tif (!bayerFormat.isValid())\n> +\t\treturn -EINVAL;\n> +\n> +\tPixelFormat pixelFormat = bitDepthToFmt.at(bayerFormat.bitDepth);\n\nis there any risk the desired bit depth is not in the map ?\n\n> +\n> +\tV4L2DeviceFormat captureFormat;\n> +\tcaptureFormat.fourcc = output_->toV4L2PixelFormat(pixelFormat);\n> +\tcaptureFormat.size = subdevFormat->size;\n> +\n> +\tret = output_->setFormat(&captureFormat);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/*\n> +\t * We return the format that we set against the output device, as the\n> +\t * same format will also need to be set against the Input Video Control\n> +\t * Block device.\n> +\t */\n> +\t*inputFormat = captureFormat;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int RZG2LCRU::init(const MediaDevice *media, MediaEntity **sensorEntity)\n> +{\n> +\tint ret;\n> +\n> +\tMediaEntity *csi2Entity = media->getEntityByName(std::regex(\"csi-[0-9a-f]{8}.csi2\"));\n> +\tif (!csi2Entity)\n> +\t\treturn -ENODEV;\n> +\n> +\tconst std::vector &pads = csi2Entity->pads();\n> +\tif (pads.empty())\n> +\t\treturn -ENODEV;\n> +\n> +\t/* The receiver has a single sink pad at index 0 */\n> +\tMediaPad *sink = pads[0];\n> +\tconst std::vector &links = sink->links();\n> +\tif (links.empty())\n> +\t\treturn -ENODEV;\n> +\n> +\tMediaLink *link = links[0];\n> +\t*sensorEntity = link->source()->entity();\n> +\n> +\t/*\n> +\t * We don't handle the sensor and csi-2 rx subdevice here, as the\n> +\t * CameraData class does that already.\n> +\t *\n\nAh, see... I think we should instead\n\n> +\t * \\todo lose this horrible hack by making modular pipelines.\n> +\t */\n> +\tcru_ = V4L2Subdevice::fromEntityName(media, std::regex(\"cru-ip-[0-9a-f]{8}.cru[0-9]\"));\n> +\tret = cru_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\toutput_ = V4L2VideoDevice::fromEntityName(media, \"CRU output\");\n> +\treturn output_->open();\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/mali-c55/rzg2l-cru.h b/src/libcamera/pipeline/mali-c55/rzg2l-cru.h\n> new file mode 100644\n> index 00000000..5beb3a6e\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/mali-c55/rzg2l-cru.h\n> @@ -0,0 +1,66 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas on Board Oy\n> + *\n> + * Pipine handler element for the Renesas RZ/G2L Camera Receiver Unit\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/v4l2_subdevice.h\"\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +#include \n\nMove it up with other includes from STL\n\n> +\n> +namespace libcamera {\n> +\n> +class CameraSensor;\n> +class FrameBuffer;\n> +class MediaDevice;\n> +class PixelFormat;\n> +class Request;\n> +class Size;\n> +class SizeRange;\n> +struct StreamConfiguration;\n> +enum class Transform;\n> +\n> +class RZG2LCRU\n> +{\n> +public:\n> +\tstatic constexpr unsigned int kBufferCount = 4;\n> +\n> +\tRZG2LCRU() = default;\n\nNo need to I guess\n\n> +\n> +\tstd::vector sizes(unsigned int mbusCode) const;\n> +\tint init(const MediaDevice *media, MediaEntity **sensorEntity);\n> +\tconst Size resolution() const;\n> +\tvoid setSensorAndCSI2Pointers(std::shared_ptr sensor,\n> +\t\t\t\t std::shared_ptr csi2);\n> +\tint configure(V4L2SubdeviceFormat *subdevFormat, V4L2DeviceFormat *inputFormat);\n> +\tFrameBuffer *queueBuffer(Request *request);\n> +\tvoid cruReturnBuffer(FrameBuffer *buffer);\n> +\tV4L2VideoDevice *output() { return output_.get(); }\n> +\tint start();\n> +\tint stop();\n\nEmpty line maybe\n\nThanks\n j\n\n> +private:\n> +\tvoid freeBuffers();\n> +\n> +\tvoid cruBufferReady(FrameBuffer *buffer);\n> +\n> +\tstd::shared_ptr sensor_;\n> +\tstd::shared_ptr csi2_;\n> +\tstd::unique_ptr cru_;\n> +\tstd::unique_ptr output_;\n> +\n> +\tstd::vector> buffers_;\n> +\tstd::queue availableBuffers_;\n> +\n> +\tSize csi2Resolution_;\n> +};\n> +\n> +} /* namespace libcamera */\n> --\n> 2.30.2\n>","headers":{"Return-Path":"","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 63976C3259\n\tfor ;\n\tThu, 30 Oct 2025 13:46:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DE44608D4;\n\tThu, 30 Oct 2025 14:46:22 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6CB2F603ED\n\tfor ;\n\tThu, 30 Oct 2025 14:46:20 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4F8EC1A8F;\n\tThu, 30 Oct 2025 14:44:30 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"i/GKWoXf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761831870;\n\tbh=vP3ZyDtk73xscnQQ2MNJ4NrwzOhPu09eY/3qz+26jhc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i/GKWoXfHpYTrvI3RvEw/P3NkATu2maNCb38NSJk4rMUAUuacI96kkzzgf0Ua7pGT\n\tlwq5RRm71Gh8Q2Kj9zYA+fuHLznPkNjslxxzK8iaovuES7/REiA3HZBSMb2dxjDkj6\n\tj/YZ9cPmTNYSJtoLfr0AdvM3+pk73pOZ2G7zYFMQ=","Date":"Thu, 30 Oct 2025 14:46:16 +0100","From":"Jacopo Mondi ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 07/10] libcamera: mali-c55: Add RZG2LCRU class","Message-ID":"","References":"<20250724065256.75175-1-dan.scally@ideasonboard.com>\n\t<20250724065256.75175-8-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250724065256.75175-8-dan.scally@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]