[{"id":37333,"web_url":"https://patchwork.libcamera.org/comment/37333/","msgid":"<c6e97e10-4d21-44de-8c7b-f1ddbea20056@ideasonboard.com>","date":"2025-12-12T14:31:49","subject":"Re: [PATCH v2 2/7] libcamera: mali-c55: Split TPG and Inline camera\n\thandling","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo,thanks for picking up this set!\n\nOn 10/12/2025 14:39, Jacopo Mondi wrote:\n> In order to prepare to support memory input cameras, split the handling of\n> the TPG and Inline camera cases.\n> \n> The Mali C55 pipeline handler uses the entity and subdevice stored in the\n> CameraData to support both the handling of the TPG and of the [CSI-2 +\n> sensor] use cases. Adding support for memory cameras by using the CRU unit\n> would add yet-another special case making the code harder to follow and\n> more prone to errors.\n> \n> Split the handling of the TPG and inline cameras by introducing the\n> MaliC55CameraData class hierarchy, to deflect functions called on the\n> camera data to the correct entities.\n> \n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 287 +++++++++++++++++----------\n>   1 file changed, 185 insertions(+), 102 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 cf0cb15f8bb39143eea38aa8acb8d2b1268f5530..552a258a6b849a2518fa6c83226cf9ab4e657717 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -92,17 +92,28 @@ struct MaliC55FrameInfo {\n>   class MaliC55CameraData : public Camera::Private\n>   {\n>   public:\n> -\tMaliC55CameraData(PipelineHandler *pipe, MediaEntity *entity)\n> -\t\t: Camera::Private(pipe), entity_(entity)\n> +\tenum CameraType {\n> +\t\tTpg,\n> +\t\tInline,\n> +\t};\n> +\n> +\tMaliC55CameraData(PipelineHandler *pipe)\n> +\t\t: Camera::Private(pipe)\n>   \t{\n>   \t}\n>   \n> -\tint init();\n>   \tint loadIPA();\n>   \n> +\tCameraType type() const { return type_; }\n> +\n>   \t/* Deflect these functionalities to either TPG or CameraSensor. */\n> -\tstd::vector<Size> sizes(unsigned int mbusCode) const;\n> -\tSize resolution() const;\n> +\tvirtual int init(MediaEntity *entity) = 0;\n> +\n> +\tvirtual std::vector<Size> sizes(unsigned int mbusCode) const = 0;\n> +\tvirtual V4L2Subdevice *subdev() const = 0;\n> +\tvirtual CameraSensor *sensor() const = 0;\n> +\tvirtual V4L2Subdevice *csi2() const = 0;\n> +\tvirtual Size resolution() const = 0;\n>   \n>   \tint pixfmtToMbusCode(const PixelFormat &pixFmt) const;\n>   \tconst PixelFormat &bestRawFormat() const;\n> @@ -112,11 +123,6 @@ public:\n>   \tPixelFormat adjustRawFormat(const PixelFormat &pixFmt) const;\n>   \tSize adjustRawSizes(const PixelFormat &pixFmt, const Size &rawSize) const;\n>   \n> -\tstd::unique_ptr<CameraSensor> sensor_;\n> -\n> -\tMediaEntity *entity_;\n> -\tstd::unique_ptr<V4L2Subdevice> csi_;\n> -\tstd::unique_ptr<V4L2Subdevice> sd_;\n>   \tStream frStream_;\n>   \tStream dsStream_;\n>   \n> @@ -126,58 +132,106 @@ public:\n>   \n>   \tstd::unique_ptr<DelayedControls> delayedCtrls_;\n>   \n> +protected:\n> +\tCameraType type_;\n> +\n>   private:\n> -\tvoid initTPGData();\n>   \tvoid setSensorControls(const ControlList &sensorControls);\n> -\n>   \tstd::string id_;\n> -\tSize tpgResolution_;\n>   };\n>   \n> -int MaliC55CameraData::init()\n> +class MaliC55TpgCameraData : public MaliC55CameraData\n>   {\n> -\tint ret;\n> +public:\n> +\tMaliC55TpgCameraData(PipelineHandler *pipe);\n>   \n> -\tsd_ = std::make_unique<V4L2Subdevice>(entity_);\n> -\tret = sd_->open();\n> -\tif (ret) {\n> -\t\tLOG(MaliC55, Error) << \"Failed to open sensor subdevice\";\n> -\t\treturn ret;\n> +\tint init(MediaEntity *entity) override;\n> +\n> +\tstd::vector<Size> sizes(unsigned int mbusCode) const override;\n> +\n> +\tSize resolution() const override\n> +\t{\n> +\t\treturn resolution_;\n>   \t}\n>   \n> -\t/* If this camera is created from TPG, we return here. */\n> -\tif (entity_->name() == \"mali-c55 tpg\") {\n> -\t\tinitTPGData();\n> -\t\treturn 0;\n> +\tV4L2Subdevice *subdev() const override\n> +\t{\n> +\t\treturn sd_.get();\n>   \t}\n>   \n> -\t/*\n> -\t * Register a CameraSensor if we connect to a sensor and create\n> -\t * an entity for the connected CSI-2 receiver.\n> -\t */\n> -\tsensor_ = CameraSensorFactoryBase::create(entity_);\n> -\tif (!sensor_)\n> -\t\treturn -ENODEV;\n> +\tCameraSensor *sensor() const override\n> +\t{\n> +\t\tASSERT(false);\n\nWhat's this for? just to throw an error if execution reaches this point?\n\nThe rest looks ok to me; it's a much cleaner way of separating them:\n\nReviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n\n> +\t\treturn nullptr;\n> +\t}\n>   \n> -\tconst MediaPad *sourcePad = entity_->getPadByIndex(0);\n> -\tMediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity();\n> +\tV4L2Subdevice *csi2() const override\n> +\t{\n> +\t\tASSERT(false);\n> +\t\treturn nullptr;\n> +\t}\n>   \n> -\tcsi_ = std::make_unique<V4L2Subdevice>(csiEntity);\n> -\tret = csi_->open();\n> -\tif (ret) {\n> -\t\tLOG(MaliC55, Error) << \"Failed to open CSI-2 subdevice\";\n> -\t\treturn ret;\n> +private:\n> +\tSize resolution_;\n> +\tstd::unique_ptr<V4L2Subdevice> sd_;\n> +};\n> +\n> +class MaliC55InlineCameraData : public MaliC55CameraData\n> +{\n> +public:\n> +\tMaliC55InlineCameraData(PipelineHandler *pipe);\n> +\n> +\tint init(MediaEntity *entity) override;\n> +\n> +\tstd::vector<Size> sizes(unsigned int mbusCode) const override\n> +\t{\n> +\t\treturn sensor_->sizes(mbusCode);\n>   \t}\n>   \n> -\treturn 0;\n> +\tSize resolution() const override\n> +\t{\n> +\t\treturn sensor_->resolution();\n> +\t}\n> +\n> +\tV4L2Subdevice *subdev() const override\n> +\t{\n> +\t\treturn sensor_->device();\n> +\t}\n> +\n> +\tCameraSensor *sensor() const override\n> +\t{\n> +\t\treturn sensor_.get();\n> +\t}\n> +\n> +\tV4L2Subdevice *csi2() const override\n> +\t{\n> +\t\treturn csi2_.get();\n> +\t}\n> +\n> +private:\n> +\tstd::unique_ptr<V4L2Subdevice> csi2_;\n> +\tstd::unique_ptr<CameraSensor> sensor_;\n> +};\n> +\n> +MaliC55TpgCameraData::MaliC55TpgCameraData(PipelineHandler *pipe)\n> +\t: MaliC55CameraData(pipe)\n> +{\n> +\ttype_ = CameraType::Tpg;\n>   }\n>   \n> -void MaliC55CameraData::initTPGData()\n> +int MaliC55TpgCameraData::init(MediaEntity *tpg)\n>   {\n> +\tsd_ = std::make_unique<V4L2Subdevice>(tpg);\n> +\tint ret = sd_->open();\n> +\tif (ret) {\n> +\t\tLOG(MaliC55, Error) << \"Failed to open TPG subdevice\";\n> +\t\treturn ret;\n> +\t}\n> +\n>   \t/* Replicate the CameraSensor implementation for TPG. */\n>   \tV4L2Subdevice::Formats formats = sd_->formats(0);\n>   \tif (formats.empty())\n> -\t\treturn;\n> +\t\treturn -EINVAL;\n>   \n>   \tstd::vector<Size> tpgSizes;\n>   \n> @@ -187,19 +241,13 @@ void MaliC55CameraData::initTPGData()\n>   \t\t\t       [](const SizeRange &range) { return range.max; });\n>   \t}\n>   \n> -\ttpgResolution_ = tpgSizes.back();\n> -}\n> +\tresolution_ = tpgSizes.back();\n>   \n> -void MaliC55CameraData::setSensorControls(const ControlList &sensorControls)\n> -{\n> -\tdelayedCtrls_->push(sensorControls);\n> +\treturn 0;\n>   }\n>   \n> -std::vector<Size> MaliC55CameraData::sizes(unsigned int mbusCode) const\n> +std::vector<Size> MaliC55TpgCameraData::sizes(unsigned int mbusCode) const\n>   {\n> -\tif (sensor_)\n> -\t\treturn sensor_->sizes(mbusCode);\n> -\n>   \tV4L2Subdevice::Formats formats = sd_->formats(0);\n>   \tif (formats.empty())\n>   \t\treturn {};\n> @@ -218,12 +266,35 @@ std::vector<Size> MaliC55CameraData::sizes(unsigned int mbusCode) const\n>   \treturn sizes;\n>   }\n>   \n> -Size MaliC55CameraData::resolution() const\n> +MaliC55InlineCameraData::MaliC55InlineCameraData(PipelineHandler *pipe)\n> +\t: MaliC55CameraData(pipe)\n>   {\n> -\tif (sensor_)\n> -\t\treturn sensor_->resolution();\n> +\ttype_ = CameraType::Inline;\n> +}\n>   \n> -\treturn tpgResolution_;\n> +int MaliC55InlineCameraData::init(MediaEntity *sensor)\n> +{\n> +\t/* Register a CameraSensor create an entity for the CSI-2 receiver. */\n> +\tsensor_ = CameraSensorFactoryBase::create(sensor);\n> +\tif (!sensor_)\n> +\t\treturn -EINVAL;\n> +\n> +\tconst MediaPad *sourcePad = sensor->getPadByIndex(0);\n> +\tMediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity();\n> +\n> +\tcsi2_ = std::make_unique<V4L2Subdevice>(csiEntity);\n> +\tint ret = csi2_->open();\n> +\tif (ret) {\n> +\t\tLOG(MaliC55, Error) << \"Failed to open CSI-2 subdevice\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn ret;\n> +}\n> +\n> +void MaliC55CameraData::setSensorControls(const ControlList &sensorControls)\n> +{\n> +\tdelayedCtrls_->push(sensorControls);\n>   }\n>   \n>   /*\n> @@ -242,7 +313,7 @@ int MaliC55CameraData::pixfmtToMbusCode(const PixelFormat &pixFmt) const\n>   \tif (!bayerFormat.isValid())\n>   \t\treturn -EINVAL;\n>   \n> -\tV4L2Subdevice::Formats formats = sd_->formats(0);\n> +\tV4L2Subdevice::Formats formats = subdev()->formats(0);\n>   \tunsigned int sensorMbusCode = 0;\n>   \tunsigned int bitDepth = 0;\n>   \n> @@ -280,7 +351,7 @@ const PixelFormat &MaliC55CameraData::bestRawFormat() const\n>   {\n>   \tstatic const PixelFormat invalidPixFmt = {};\n>   \n> -\tfor (const auto &fmt : sd_->formats(0)) {\n> +\tfor (const auto &fmt : subdev()->formats(0)) {\n>   \t\tBayerFormat sensorBayer = BayerFormat::fromMbusCode(fmt.first);\n>   \n>   \t\tif (!sensorBayer.isValid())\n> @@ -302,11 +373,11 @@ const PixelFormat &MaliC55CameraData::bestRawFormat() const\n>   \n>   void MaliC55CameraData::updateControls(const ControlInfoMap &ipaControls)\n>   {\n> -\tif (!sensor_)\n> +\tif (type_ == CameraType::Tpg)\n>   \t\treturn;\n>   \n>   \tIPACameraSensorInfo sensorInfo;\n> -\tint ret = sensor_->sensorInfo(&sensorInfo);\n> +\tint ret = sensor()->sensorInfo(&sensorInfo);\n>   \tif (ret) {\n>   \t\tLOG(MaliC55, Error) << \"Failed to retrieve sensor info\";\n>   \t\treturn;\n> @@ -379,7 +450,7 @@ int MaliC55CameraData::loadIPA()\n>   \tint ret;\n>   \n>   \t/* Do not initialize IPA for TPG. */\n> -\tif (!sensor_)\n> +\tif (type_ == CameraType::Tpg)\n>   \t\treturn 0;\n>   \n>   \tipa_ = IPAManager::createIPA<ipa::mali_c55::IPAProxyMaliC55>(pipe(), 1, 1);\n> @@ -388,20 +459,20 @@ int MaliC55CameraData::loadIPA()\n>   \n>   \tipa_->setSensorControls.connect(this, &MaliC55CameraData::setSensorControls);\n>   \n> -\tstd::string ipaTuningFile = ipa_->configurationFile(sensor_->model() + \".yaml\",\n> +\tstd::string ipaTuningFile = ipa_->configurationFile(sensor()->model() + \".yaml\",\n>   \t\t\t\t\t\t\t    \"uncalibrated.yaml\");\n>   \n>   \t/* We need to inform the IPA of the sensor configuration */\n>   \tipa::mali_c55::IPAConfigInfo ipaConfig{};\n>   \n> -\tret = sensor_->sensorInfo(&ipaConfig.sensorInfo);\n> +\tret = sensor()->sensorInfo(&ipaConfig.sensorInfo);\n>   \tif (ret)\n>   \t\treturn ret;\n>   \n> -\tipaConfig.sensorControls = sensor_->controls();\n> +\tipaConfig.sensorControls = sensor()->controls();\n>   \n>   \tControlInfoMap ipaControls;\n> -\tret = ipa_->init({ ipaTuningFile, sensor_->model() }, ipaConfig,\n> +\tret = ipa_->init({ ipaTuningFile, sensor()->model() }, ipaConfig,\n>   \t\t\t &ipaControls);\n>   \tif (ret) {\n>   \t\tLOG(MaliC55, Error) << \"Failed to initialise the Mali-C55 IPA\";\n> @@ -444,13 +515,13 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()\n>   \t * The TPG doesn't support flips, so we only need to calculate a\n>   \t * transform if we have a sensor.\n>   \t */\n> -\tif (data_->sensor_) {\n> +\tif (data_->type() == MaliC55CameraData::CameraType::Tpg) {\n> +\t\tcombinedTransform_ = Transform::Rot0;\n> +\t} else {\n>   \t\tOrientation requestedOrientation = orientation;\n> -\t\tcombinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> +\t\tcombinedTransform_ = data_->sensor()->computeTransform(&orientation);\n>   \t\tif (orientation != requestedOrientation)\n>   \t\t\tstatus = Adjusted;\n> -\t} else {\n> -\t\tcombinedTransform_ = Transform::Rot0;\n>   \t}\n>   \n>   \t/* Only 2 streams available. */\n> @@ -927,11 +998,17 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n>   \n>   \t/* Link the graph depending if we are operating the TPG or a sensor. */\n>   \tMaliC55CameraData *data = cameraData(camera);\n> -\tif (data->csi_) {\n> -\t\tconst MediaEntity *csiEntity = data->csi_->entity();\n> -\t\tret = csiEntity->getPadByIndex(1)->links()[0]->setEnabled(true);\n> -\t} else {\n> -\t\tret = data->entity_->getPadByIndex(0)->links()[0]->setEnabled(true);\n> +\tswitch (data->type()) {\n> +\tcase MaliC55CameraData::CameraType::Tpg: {\n> +\t\tconst MediaEntity *tpgEntity = data->subdev()->entity();\n> +\t\tret = tpgEntity->getPadByIndex(0)->links()[0]->setEnabled(true);\n> +\t\tbreak;\n> +\t}\n> +\tcase MaliC55CameraData::CameraType::Inline: {\n> +\t\tconst MediaEntity *csi2Entity = data->csi2()->entity();\n> +\t\tret = csi2Entity->getPadByIndex(1)->links()[0]->setEnabled(true);\n> +\t\tbreak;\n> +\t}\n>   \t}\n>   \tif (ret)\n>   \t\treturn ret;\n> @@ -939,26 +1016,30 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n>   \tMaliC55CameraConfiguration *maliConfig =\n>   \t\tstatic_cast<MaliC55CameraConfiguration *>(config);\n>   \tV4L2SubdeviceFormat subdevFormat = maliConfig->sensorFormat_;\n> -\tret = data->sd_->getFormat(0, &subdevFormat);\n> -\tif (ret)\n> -\t\treturn ret;\n>   \n> -\tif (data->sensor_) {\n> -\t\tret = data->sensor_->setFormat(&subdevFormat,\n> -\t\t\t\t\t       maliConfig->combinedTransform());\n> -\t\tif (ret)\n> -\t\t\treturn ret;\n> +\t/* Apply format to the origin of the pipeline and propagate it. */\n> +\tswitch (data->type()) {\n> +\tcase MaliC55CameraData::CameraType::Tpg: {\n> +\t\tret = data->subdev()->setFormat(0, &subdevFormat);\n> +\t\tbreak;\n>   \t}\n> -\n> -\tif (data->csi_) {\n> -\t\tret = data->csi_->setFormat(0, &subdevFormat);\n> +\tcase MaliC55CameraData::CameraType::Inline: {\n> +\t\tret = data->sensor()->setFormat(&subdevFormat,\n> +\t\t\t\t\t\tmaliConfig->combinedTransform());\n>   \t\tif (ret)\n>   \t\t\treturn ret;\n>   \n> -\t\tret = data->csi_->getFormat(1, &subdevFormat);\n> +\t\tret = data->csi2()->setFormat(0, &subdevFormat);\n>   \t\tif (ret)\n>   \t\t\treturn ret;\n> +\n> +\t\tret = data->csi2()->getFormat(1, &subdevFormat);\n> +\n> +\t\tbreak;\n> +\t}\n>   \t}\n> +\tif (ret)\n> +\t\treturn ret;\n>   \n>   \tV4L2DeviceFormat statsFormat;\n>   \tret = stats_->getFormat(&statsFormat);\n> @@ -973,8 +1054,6 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n>   \t/*\n>   \t * Propagate the format to the ISP sink pad and configure the input\n>   \t * crop rectangle (no crop at the moment).\n> -\t *\n> -\t * \\todo Configure the CSI-2 receiver.\n>   \t */\n>   \tret = isp_->setFormat(0, &subdevFormat);\n>   \tif (ret)\n> @@ -1058,18 +1137,18 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n>   \t/* We need to inform the IPA of the sensor configuration */\n>   \tipa::mali_c55::IPAConfigInfo ipaConfig{};\n>   \n> -\tret = data->sensor_->sensorInfo(&ipaConfig.sensorInfo);\n> +\tret = data->sensor()->sensorInfo(&ipaConfig.sensorInfo);\n>   \tif (ret)\n>   \t\treturn ret;\n>   \n> -\tipaConfig.sensorControls = data->sensor_->controls();\n> +\tipaConfig.sensorControls = data->sensor()->controls();\n>   \n>   \t/*\n>   \t * And we also need to tell the IPA the bayerOrder of the data (as\n>   \t * affected by any flips that we've configured)\n>   \t */\n>   \tconst Transform &combinedTransform = maliConfig->combinedTransform();\n> -\tBayerFormat::Order bayerOrder = data->sensor_->bayerOrder(combinedTransform);\n> +\tBayerFormat::Order bayerOrder = data->sensor()->bayerOrder(combinedTransform);\n>   \n>   \tControlInfoMap ipaControls;\n>   \tret = data->ipa_->configure(ipaConfig, utils::to_underlying(bayerOrder),\n> @@ -1283,7 +1362,7 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n>   \tif (!scalerCrop)\n>   \t\treturn;\n>   \n> -\tif (!data->sensor_) {\n> +\tif (data->type() == MaliC55CameraData::CameraType::Tpg) {\n>   \t\tLOG(MaliC55, Error) << \"ScalerCrop not supported for TPG\";\n>   \t\treturn;\n>   \t}\n> @@ -1291,7 +1370,7 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n>   \tRectangle nativeCrop = *scalerCrop;\n>   \n>   \tIPACameraSensorInfo sensorInfo;\n> -\tint ret = data->sensor_->sensorInfo(&sensorInfo);\n> +\tint ret = data->sensor()->sensorInfo(&sensorInfo);\n>   \tif (ret) {\n>   \t\tLOG(MaliC55, Error) << \"Failed to retrieve sensor info\";\n>   \t\treturn;\n> @@ -1573,10 +1652,11 @@ bool PipelineHandlerMaliC55::registerTPGCamera(MediaLink *link)\n>   \t}\n>   \n>   \tstd::unique_ptr<MaliC55CameraData> data =\n> -\t\tstd::make_unique<MaliC55CameraData>(this, link->source()->entity());\n> +\t\tstd::make_unique<MaliC55TpgCameraData>(this);\n>   \n> -\tif (data->init())\n> -\t\treturn false;\n> +\tint ret = data->init(link->source()->entity());\n> +\tif (ret)\n> +\t\treturn ret;\n>   \n>   \treturn registerMaliCamera(std::move(data), name);\n>   }\n> @@ -1600,21 +1680,24 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink)\n>   \t\t\tcontinue;\n>   \n>   \t\tstd::unique_ptr<MaliC55CameraData> data =\n> -\t\t\tstd::make_unique<MaliC55CameraData>(this, sensor);\n> -\t\tif (data->init())\n> -\t\t\treturn false;\n> +\t\t\tstd::make_unique<MaliC55InlineCameraData>(this);\n> +\n> +\t\tint ret = data->init(sensor);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n>   \n> -\t\tdata->properties_ = data->sensor_->properties();\n> +\t\tdata->properties_ = data->sensor()->properties();\n>   \n> -\t\tconst CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays();\n> +\t\tconst CameraSensorProperties::SensorDelays &delays =\n> +\t\t\tdata->sensor()->sensorDelays();\n>   \t\tstd::unordered_map<uint32_t, DelayedControls::ControlParams> params = {\n>   \t\t\t{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },\n>   \t\t\t{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },\n>   \t\t};\n>   \n> -\t\tdata->delayedCtrls_ =\n> -\t\t\tstd::make_unique<DelayedControls>(data->sensor_->device(),\n> -\t\t\t\t\t\t\t  params);\n> +\t\tV4L2Subdevice *sensorSubdev = data->sensor()->device();\n> +\t\tdata->delayedCtrls_ = std::make_unique<DelayedControls>(sensorSubdev,\n> +\t\t\t\t\t\t\t\t\tparams);\n>   \t\tisp_->frameStart.connect(data->delayedCtrls_.get(),\n>   \t\t\t\t\t &DelayedControls::applyControls);\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 28678BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Dec 2025 14:31:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C7D5A61698;\n\tFri, 12 Dec 2025 15:31:55 +0100 (CET)","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 47DC06142F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Dec 2025 15:31:54 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 060D7520;\n\tFri, 12 Dec 2025 15:31:50 +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=\"WGHlwdfu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765549911;\n\tbh=8tQlZ1htdGT13d3dOtaJjsZBP+fkuhMxOW4Rq4xWoIg=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=WGHlwdfuaGZGpd5ZQfGj0qf7owC61t2GKOq6ZVqvLI+HBM/Ph7fH5IXHwG34TraNa\n\t44RVdjkoJYiJeMJtKtp8u3b0ta9k63ixqemr2hvJvBywvh2RX0zNa6CnE/in/AB1q6\n\tkvVJTNPxOxe2DLmitvKMKLKD0wkvwDK4I8wVn+Us=","Message-ID":"<c6e97e10-4d21-44de-8c7b-f1ddbea20056@ideasonboard.com>","Date":"Fri, 12 Dec 2025 14:31:49 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 2/7] libcamera: mali-c55: Split TPG and Inline camera\n\thandling","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251210-mali-cru-v2-0-e26421de202b@ideasonboard.com>\n\t<20251210-mali-cru-v2-2-e26421de202b@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally <dan.scally@ideasonboard.com>","In-Reply-To":"<20251210-mali-cru-v2-2-e26421de202b@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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":37336,"web_url":"https://patchwork.libcamera.org/comment/37336/","msgid":"<ip6afee2fvho3qkcch7fi5kmetr4jnhbvd2xayq32o3ttyuho4@thi4gv5ktlne>","date":"2025-12-12T15:28:20","subject":"Re: [PATCH v2 2/7] libcamera: mali-c55: Split TPG and Inline camera\n\thandling","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Fri, Dec 12, 2025 at 02:31:49PM +0000, Dan Scally wrote:\n> Hi Jacopo,thanks for picking up this set!\n>\n> On 10/12/2025 14:39, Jacopo Mondi wrote:\n> > In order to prepare to support memory input cameras, split the handling of\n> > the TPG and Inline camera cases.\n> >\n> > The Mali C55 pipeline handler uses the entity and subdevice stored in the\n> > CameraData to support both the handling of the TPG and of the [CSI-2 +\n> > sensor] use cases. Adding support for memory cameras by using the CRU unit\n> > would add yet-another special case making the code harder to follow and\n> > more prone to errors.\n> >\n> > Split the handling of the TPG and inline cameras by introducing the\n> > MaliC55CameraData class hierarchy, to deflect functions called on the\n> > camera data to the correct entities.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 287 +++++++++++++++++----------\n> >   1 file changed, 185 insertions(+), 102 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 cf0cb15f8bb39143eea38aa8acb8d2b1268f5530..552a258a6b849a2518fa6c83226cf9ab4e657717 100644\n> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > @@ -92,17 +92,28 @@ struct MaliC55FrameInfo {\n> >   class MaliC55CameraData : public Camera::Private\n> >   {\n> >   public:\n> > -\tMaliC55CameraData(PipelineHandler *pipe, MediaEntity *entity)\n> > -\t\t: Camera::Private(pipe), entity_(entity)\n> > +\tenum CameraType {\n> > +\t\tTpg,\n> > +\t\tInline,\n> > +\t};\n> > +\n> > +\tMaliC55CameraData(PipelineHandler *pipe)\n> > +\t\t: Camera::Private(pipe)\n> >   \t{\n> >   \t}\n> > -\tint init();\n> >   \tint loadIPA();\n> > +\tCameraType type() const { return type_; }\n> > +\n> >   \t/* Deflect these functionalities to either TPG or CameraSensor. */\n> > -\tstd::vector<Size> sizes(unsigned int mbusCode) const;\n> > -\tSize resolution() const;\n> > +\tvirtual int init(MediaEntity *entity) = 0;\n> > +\n> > +\tvirtual std::vector<Size> sizes(unsigned int mbusCode) const = 0;\n> > +\tvirtual V4L2Subdevice *subdev() const = 0;\n> > +\tvirtual CameraSensor *sensor() const = 0;\n> > +\tvirtual V4L2Subdevice *csi2() const = 0;\n> > +\tvirtual Size resolution() const = 0;\n> >   \tint pixfmtToMbusCode(const PixelFormat &pixFmt) const;\n> >   \tconst PixelFormat &bestRawFormat() const;\n> > @@ -112,11 +123,6 @@ public:\n> >   \tPixelFormat adjustRawFormat(const PixelFormat &pixFmt) const;\n> >   \tSize adjustRawSizes(const PixelFormat &pixFmt, const Size &rawSize) const;\n> > -\tstd::unique_ptr<CameraSensor> sensor_;\n> > -\n> > -\tMediaEntity *entity_;\n> > -\tstd::unique_ptr<V4L2Subdevice> csi_;\n> > -\tstd::unique_ptr<V4L2Subdevice> sd_;\n> >   \tStream frStream_;\n> >   \tStream dsStream_;\n> > @@ -126,58 +132,106 @@ public:\n> >   \tstd::unique_ptr<DelayedControls> delayedCtrls_;\n> > +protected:\n> > +\tCameraType type_;\n> > +\n> >   private:\n> > -\tvoid initTPGData();\n> >   \tvoid setSensorControls(const ControlList &sensorControls);\n> > -\n> >   \tstd::string id_;\n> > -\tSize tpgResolution_;\n> >   };\n> > -int MaliC55CameraData::init()\n> > +class MaliC55TpgCameraData : public MaliC55CameraData\n> >   {\n> > -\tint ret;\n> > +public:\n> > +\tMaliC55TpgCameraData(PipelineHandler *pipe);\n> > -\tsd_ = std::make_unique<V4L2Subdevice>(entity_);\n> > -\tret = sd_->open();\n> > -\tif (ret) {\n> > -\t\tLOG(MaliC55, Error) << \"Failed to open sensor subdevice\";\n> > -\t\treturn ret;\n> > +\tint init(MediaEntity *entity) override;\n> > +\n> > +\tstd::vector<Size> sizes(unsigned int mbusCode) const override;\n> > +\n> > +\tSize resolution() const override\n> > +\t{\n> > +\t\treturn resolution_;\n> >   \t}\n> > -\t/* If this camera is created from TPG, we return here. */\n> > -\tif (entity_->name() == \"mali-c55 tpg\") {\n> > -\t\tinitTPGData();\n> > -\t\treturn 0;\n> > +\tV4L2Subdevice *subdev() const override\n> > +\t{\n> > +\t\treturn sd_.get();\n> >   \t}\n> > -\t/*\n> > -\t * Register a CameraSensor if we connect to a sensor and create\n> > -\t * an entity for the connected CSI-2 receiver.\n> > -\t */\n> > -\tsensor_ = CameraSensorFactoryBase::create(entity_);\n> > -\tif (!sensor_)\n> > -\t\treturn -ENODEV;\n> > +\tCameraSensor *sensor() const override\n> > +\t{\n> > +\t\tASSERT(false);\n>\n> What's this for? just to throw an error if execution reaches this point?\n\nTo make sure we never call this function on the TPG where it is\ninvalid.\n\n>\n> The rest looks ok to me; it's a much cleaner way of separating them:\n\nHowever, I think we could do better than this.\n\nThe presence of functions with ASSERT(false) and the fact that only\nthe Memory use case implements all of them, while on other some of\nthem doesn't even make sense (think of the cru() function, it doesn't\nmake sense on TPG or Inline) indicates that using dynamic polymorphism\nis probably not the best strategy here.\n\nAlso, I think I could get rid of the CRTP pattern in init(), which\nreally feels an hack\n\nBarnabas suggested to try with std::variants and overloaded visitors\nhttps://en.cppreference.com/w/cpp/utility/variant/visit2.html\nI might give it a try in the next days.\n\nHowever, I understand that from a process point of view it doesn't\nmake much sense, but I would like to do so on top as otherwise\nrebasing becomes really difficult.\n\nLet's see how it looks like.\n\n>\n> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n>\n> > +\t\treturn nullptr;\n> > +\t}\n> > -\tconst MediaPad *sourcePad = entity_->getPadByIndex(0);\n> > -\tMediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity();\n> > +\tV4L2Subdevice *csi2() const override\n> > +\t{\n> > +\t\tASSERT(false);\n> > +\t\treturn nullptr;\n> > +\t}\n> > -\tcsi_ = std::make_unique<V4L2Subdevice>(csiEntity);\n> > -\tret = csi_->open();\n> > -\tif (ret) {\n> > -\t\tLOG(MaliC55, Error) << \"Failed to open CSI-2 subdevice\";\n> > -\t\treturn ret;\n> > +private:\n> > +\tSize resolution_;\n> > +\tstd::unique_ptr<V4L2Subdevice> sd_;\n> > +};\n> > +\n> > +class MaliC55InlineCameraData : public MaliC55CameraData\n> > +{\n> > +public:\n> > +\tMaliC55InlineCameraData(PipelineHandler *pipe);\n> > +\n> > +\tint init(MediaEntity *entity) override;\n> > +\n> > +\tstd::vector<Size> sizes(unsigned int mbusCode) const override\n> > +\t{\n> > +\t\treturn sensor_->sizes(mbusCode);\n> >   \t}\n> > -\treturn 0;\n> > +\tSize resolution() const override\n> > +\t{\n> > +\t\treturn sensor_->resolution();\n> > +\t}\n> > +\n> > +\tV4L2Subdevice *subdev() const override\n> > +\t{\n> > +\t\treturn sensor_->device();\n> > +\t}\n> > +\n> > +\tCameraSensor *sensor() const override\n> > +\t{\n> > +\t\treturn sensor_.get();\n> > +\t}\n> > +\n> > +\tV4L2Subdevice *csi2() const override\n> > +\t{\n> > +\t\treturn csi2_.get();\n> > +\t}\n> > +\n> > +private:\n> > +\tstd::unique_ptr<V4L2Subdevice> csi2_;\n> > +\tstd::unique_ptr<CameraSensor> sensor_;\n> > +};\n> > +\n> > +MaliC55TpgCameraData::MaliC55TpgCameraData(PipelineHandler *pipe)\n> > +\t: MaliC55CameraData(pipe)\n> > +{\n> > +\ttype_ = CameraType::Tpg;\n> >   }\n> > -void MaliC55CameraData::initTPGData()\n> > +int MaliC55TpgCameraData::init(MediaEntity *tpg)\n> >   {\n> > +\tsd_ = std::make_unique<V4L2Subdevice>(tpg);\n> > +\tint ret = sd_->open();\n> > +\tif (ret) {\n> > +\t\tLOG(MaliC55, Error) << \"Failed to open TPG subdevice\";\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> >   \t/* Replicate the CameraSensor implementation for TPG. */\n> >   \tV4L2Subdevice::Formats formats = sd_->formats(0);\n> >   \tif (formats.empty())\n> > -\t\treturn;\n> > +\t\treturn -EINVAL;\n> >   \tstd::vector<Size> tpgSizes;\n> > @@ -187,19 +241,13 @@ void MaliC55CameraData::initTPGData()\n> >   \t\t\t       [](const SizeRange &range) { return range.max; });\n> >   \t}\n> > -\ttpgResolution_ = tpgSizes.back();\n> > -}\n> > +\tresolution_ = tpgSizes.back();\n> > -void MaliC55CameraData::setSensorControls(const ControlList &sensorControls)\n> > -{\n> > -\tdelayedCtrls_->push(sensorControls);\n> > +\treturn 0;\n> >   }\n> > -std::vector<Size> MaliC55CameraData::sizes(unsigned int mbusCode) const\n> > +std::vector<Size> MaliC55TpgCameraData::sizes(unsigned int mbusCode) const\n> >   {\n> > -\tif (sensor_)\n> > -\t\treturn sensor_->sizes(mbusCode);\n> > -\n> >   \tV4L2Subdevice::Formats formats = sd_->formats(0);\n> >   \tif (formats.empty())\n> >   \t\treturn {};\n> > @@ -218,12 +266,35 @@ std::vector<Size> MaliC55CameraData::sizes(unsigned int mbusCode) const\n> >   \treturn sizes;\n> >   }\n> > -Size MaliC55CameraData::resolution() const\n> > +MaliC55InlineCameraData::MaliC55InlineCameraData(PipelineHandler *pipe)\n> > +\t: MaliC55CameraData(pipe)\n> >   {\n> > -\tif (sensor_)\n> > -\t\treturn sensor_->resolution();\n> > +\ttype_ = CameraType::Inline;\n> > +}\n> > -\treturn tpgResolution_;\n> > +int MaliC55InlineCameraData::init(MediaEntity *sensor)\n> > +{\n> > +\t/* Register a CameraSensor create an entity for the CSI-2 receiver. */\n> > +\tsensor_ = CameraSensorFactoryBase::create(sensor);\n> > +\tif (!sensor_)\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tconst MediaPad *sourcePad = sensor->getPadByIndex(0);\n> > +\tMediaEntity *csiEntity = sourcePad->links()[0]->sink()->entity();\n> > +\n> > +\tcsi2_ = std::make_unique<V4L2Subdevice>(csiEntity);\n> > +\tint ret = csi2_->open();\n> > +\tif (ret) {\n> > +\t\tLOG(MaliC55, Error) << \"Failed to open CSI-2 subdevice\";\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\treturn ret;\n> > +}\n> > +\n> > +void MaliC55CameraData::setSensorControls(const ControlList &sensorControls)\n> > +{\n> > +\tdelayedCtrls_->push(sensorControls);\n> >   }\n> >   /*\n> > @@ -242,7 +313,7 @@ int MaliC55CameraData::pixfmtToMbusCode(const PixelFormat &pixFmt) const\n> >   \tif (!bayerFormat.isValid())\n> >   \t\treturn -EINVAL;\n> > -\tV4L2Subdevice::Formats formats = sd_->formats(0);\n> > +\tV4L2Subdevice::Formats formats = subdev()->formats(0);\n> >   \tunsigned int sensorMbusCode = 0;\n> >   \tunsigned int bitDepth = 0;\n> > @@ -280,7 +351,7 @@ const PixelFormat &MaliC55CameraData::bestRawFormat() const\n> >   {\n> >   \tstatic const PixelFormat invalidPixFmt = {};\n> > -\tfor (const auto &fmt : sd_->formats(0)) {\n> > +\tfor (const auto &fmt : subdev()->formats(0)) {\n> >   \t\tBayerFormat sensorBayer = BayerFormat::fromMbusCode(fmt.first);\n> >   \t\tif (!sensorBayer.isValid())\n> > @@ -302,11 +373,11 @@ const PixelFormat &MaliC55CameraData::bestRawFormat() const\n> >   void MaliC55CameraData::updateControls(const ControlInfoMap &ipaControls)\n> >   {\n> > -\tif (!sensor_)\n> > +\tif (type_ == CameraType::Tpg)\n> >   \t\treturn;\n> >   \tIPACameraSensorInfo sensorInfo;\n> > -\tint ret = sensor_->sensorInfo(&sensorInfo);\n> > +\tint ret = sensor()->sensorInfo(&sensorInfo);\n> >   \tif (ret) {\n> >   \t\tLOG(MaliC55, Error) << \"Failed to retrieve sensor info\";\n> >   \t\treturn;\n> > @@ -379,7 +450,7 @@ int MaliC55CameraData::loadIPA()\n> >   \tint ret;\n> >   \t/* Do not initialize IPA for TPG. */\n> > -\tif (!sensor_)\n> > +\tif (type_ == CameraType::Tpg)\n> >   \t\treturn 0;\n> >   \tipa_ = IPAManager::createIPA<ipa::mali_c55::IPAProxyMaliC55>(pipe(), 1, 1);\n> > @@ -388,20 +459,20 @@ int MaliC55CameraData::loadIPA()\n> >   \tipa_->setSensorControls.connect(this, &MaliC55CameraData::setSensorControls);\n> > -\tstd::string ipaTuningFile = ipa_->configurationFile(sensor_->model() + \".yaml\",\n> > +\tstd::string ipaTuningFile = ipa_->configurationFile(sensor()->model() + \".yaml\",\n> >   \t\t\t\t\t\t\t    \"uncalibrated.yaml\");\n> >   \t/* We need to inform the IPA of the sensor configuration */\n> >   \tipa::mali_c55::IPAConfigInfo ipaConfig{};\n> > -\tret = sensor_->sensorInfo(&ipaConfig.sensorInfo);\n> > +\tret = sensor()->sensorInfo(&ipaConfig.sensorInfo);\n> >   \tif (ret)\n> >   \t\treturn ret;\n> > -\tipaConfig.sensorControls = sensor_->controls();\n> > +\tipaConfig.sensorControls = sensor()->controls();\n> >   \tControlInfoMap ipaControls;\n> > -\tret = ipa_->init({ ipaTuningFile, sensor_->model() }, ipaConfig,\n> > +\tret = ipa_->init({ ipaTuningFile, sensor()->model() }, ipaConfig,\n> >   \t\t\t &ipaControls);\n> >   \tif (ret) {\n> >   \t\tLOG(MaliC55, Error) << \"Failed to initialise the Mali-C55 IPA\";\n> > @@ -444,13 +515,13 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()\n> >   \t * The TPG doesn't support flips, so we only need to calculate a\n> >   \t * transform if we have a sensor.\n> >   \t */\n> > -\tif (data_->sensor_) {\n> > +\tif (data_->type() == MaliC55CameraData::CameraType::Tpg) {\n> > +\t\tcombinedTransform_ = Transform::Rot0;\n> > +\t} else {\n> >   \t\tOrientation requestedOrientation = orientation;\n> > -\t\tcombinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> > +\t\tcombinedTransform_ = data_->sensor()->computeTransform(&orientation);\n> >   \t\tif (orientation != requestedOrientation)\n> >   \t\t\tstatus = Adjusted;\n> > -\t} else {\n> > -\t\tcombinedTransform_ = Transform::Rot0;\n> >   \t}\n> >   \t/* Only 2 streams available. */\n> > @@ -927,11 +998,17 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n> >   \t/* Link the graph depending if we are operating the TPG or a sensor. */\n> >   \tMaliC55CameraData *data = cameraData(camera);\n> > -\tif (data->csi_) {\n> > -\t\tconst MediaEntity *csiEntity = data->csi_->entity();\n> > -\t\tret = csiEntity->getPadByIndex(1)->links()[0]->setEnabled(true);\n> > -\t} else {\n> > -\t\tret = data->entity_->getPadByIndex(0)->links()[0]->setEnabled(true);\n> > +\tswitch (data->type()) {\n> > +\tcase MaliC55CameraData::CameraType::Tpg: {\n> > +\t\tconst MediaEntity *tpgEntity = data->subdev()->entity();\n> > +\t\tret = tpgEntity->getPadByIndex(0)->links()[0]->setEnabled(true);\n> > +\t\tbreak;\n> > +\t}\n> > +\tcase MaliC55CameraData::CameraType::Inline: {\n> > +\t\tconst MediaEntity *csi2Entity = data->csi2()->entity();\n> > +\t\tret = csi2Entity->getPadByIndex(1)->links()[0]->setEnabled(true);\n> > +\t\tbreak;\n> > +\t}\n> >   \t}\n> >   \tif (ret)\n> >   \t\treturn ret;\n> > @@ -939,26 +1016,30 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n> >   \tMaliC55CameraConfiguration *maliConfig =\n> >   \t\tstatic_cast<MaliC55CameraConfiguration *>(config);\n> >   \tV4L2SubdeviceFormat subdevFormat = maliConfig->sensorFormat_;\n> > -\tret = data->sd_->getFormat(0, &subdevFormat);\n> > -\tif (ret)\n> > -\t\treturn ret;\n> > -\tif (data->sensor_) {\n> > -\t\tret = data->sensor_->setFormat(&subdevFormat,\n> > -\t\t\t\t\t       maliConfig->combinedTransform());\n> > -\t\tif (ret)\n> > -\t\t\treturn ret;\n> > +\t/* Apply format to the origin of the pipeline and propagate it. */\n> > +\tswitch (data->type()) {\n> > +\tcase MaliC55CameraData::CameraType::Tpg: {\n> > +\t\tret = data->subdev()->setFormat(0, &subdevFormat);\n> > +\t\tbreak;\n> >   \t}\n> > -\n> > -\tif (data->csi_) {\n> > -\t\tret = data->csi_->setFormat(0, &subdevFormat);\n> > +\tcase MaliC55CameraData::CameraType::Inline: {\n> > +\t\tret = data->sensor()->setFormat(&subdevFormat,\n> > +\t\t\t\t\t\tmaliConfig->combinedTransform());\n> >   \t\tif (ret)\n> >   \t\t\treturn ret;\n> > -\t\tret = data->csi_->getFormat(1, &subdevFormat);\n> > +\t\tret = data->csi2()->setFormat(0, &subdevFormat);\n> >   \t\tif (ret)\n> >   \t\t\treturn ret;\n> > +\n> > +\t\tret = data->csi2()->getFormat(1, &subdevFormat);\n> > +\n> > +\t\tbreak;\n> > +\t}\n> >   \t}\n> > +\tif (ret)\n> > +\t\treturn ret;\n> >   \tV4L2DeviceFormat statsFormat;\n> >   \tret = stats_->getFormat(&statsFormat);\n> > @@ -973,8 +1054,6 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n> >   \t/*\n> >   \t * Propagate the format to the ISP sink pad and configure the input\n> >   \t * crop rectangle (no crop at the moment).\n> > -\t *\n> > -\t * \\todo Configure the CSI-2 receiver.\n> >   \t */\n> >   \tret = isp_->setFormat(0, &subdevFormat);\n> >   \tif (ret)\n> > @@ -1058,18 +1137,18 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n> >   \t/* We need to inform the IPA of the sensor configuration */\n> >   \tipa::mali_c55::IPAConfigInfo ipaConfig{};\n> > -\tret = data->sensor_->sensorInfo(&ipaConfig.sensorInfo);\n> > +\tret = data->sensor()->sensorInfo(&ipaConfig.sensorInfo);\n> >   \tif (ret)\n> >   \t\treturn ret;\n> > -\tipaConfig.sensorControls = data->sensor_->controls();\n> > +\tipaConfig.sensorControls = data->sensor()->controls();\n> >   \t/*\n> >   \t * And we also need to tell the IPA the bayerOrder of the data (as\n> >   \t * affected by any flips that we've configured)\n> >   \t */\n> >   \tconst Transform &combinedTransform = maliConfig->combinedTransform();\n> > -\tBayerFormat::Order bayerOrder = data->sensor_->bayerOrder(combinedTransform);\n> > +\tBayerFormat::Order bayerOrder = data->sensor()->bayerOrder(combinedTransform);\n> >   \tControlInfoMap ipaControls;\n> >   \tret = data->ipa_->configure(ipaConfig, utils::to_underlying(bayerOrder),\n> > @@ -1283,7 +1362,7 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n> >   \tif (!scalerCrop)\n> >   \t\treturn;\n> > -\tif (!data->sensor_) {\n> > +\tif (data->type() == MaliC55CameraData::CameraType::Tpg) {\n> >   \t\tLOG(MaliC55, Error) << \"ScalerCrop not supported for TPG\";\n> >   \t\treturn;\n> >   \t}\n> > @@ -1291,7 +1370,7 @@ void PipelineHandlerMaliC55::applyScalerCrop(Camera *camera,\n> >   \tRectangle nativeCrop = *scalerCrop;\n> >   \tIPACameraSensorInfo sensorInfo;\n> > -\tint ret = data->sensor_->sensorInfo(&sensorInfo);\n> > +\tint ret = data->sensor()->sensorInfo(&sensorInfo);\n> >   \tif (ret) {\n> >   \t\tLOG(MaliC55, Error) << \"Failed to retrieve sensor info\";\n> >   \t\treturn;\n> > @@ -1573,10 +1652,11 @@ bool PipelineHandlerMaliC55::registerTPGCamera(MediaLink *link)\n> >   \t}\n> >   \tstd::unique_ptr<MaliC55CameraData> data =\n> > -\t\tstd::make_unique<MaliC55CameraData>(this, link->source()->entity());\n> > +\t\tstd::make_unique<MaliC55TpgCameraData>(this);\n> > -\tif (data->init())\n> > -\t\treturn false;\n> > +\tint ret = data->init(link->source()->entity());\n> > +\tif (ret)\n> > +\t\treturn ret;\n> >   \treturn registerMaliCamera(std::move(data), name);\n> >   }\n> > @@ -1600,21 +1680,24 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink)\n> >   \t\t\tcontinue;\n> >   \t\tstd::unique_ptr<MaliC55CameraData> data =\n> > -\t\t\tstd::make_unique<MaliC55CameraData>(this, sensor);\n> > -\t\tif (data->init())\n> > -\t\t\treturn false;\n> > +\t\t\tstd::make_unique<MaliC55InlineCameraData>(this);\n> > +\n> > +\t\tint ret = data->init(sensor);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > -\t\tdata->properties_ = data->sensor_->properties();\n> > +\t\tdata->properties_ = data->sensor()->properties();\n> > -\t\tconst CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays();\n> > +\t\tconst CameraSensorProperties::SensorDelays &delays =\n> > +\t\t\tdata->sensor()->sensorDelays();\n> >   \t\tstd::unordered_map<uint32_t, DelayedControls::ControlParams> params = {\n> >   \t\t\t{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },\n> >   \t\t\t{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },\n> >   \t\t};\n> > -\t\tdata->delayedCtrls_ =\n> > -\t\t\tstd::make_unique<DelayedControls>(data->sensor_->device(),\n> > -\t\t\t\t\t\t\t  params);\n> > +\t\tV4L2Subdevice *sensorSubdev = data->sensor()->device();\n> > +\t\tdata->delayedCtrls_ = std::make_unique<DelayedControls>(sensorSubdev,\n> > +\t\t\t\t\t\t\t\t\tparams);\n> >   \t\tisp_->frameStart.connect(data->delayedCtrls_.get(),\n> >   \t\t\t\t\t &DelayedControls::applyControls);\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 4EDBBBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Dec 2025 15:28:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 35D00617F3;\n\tFri, 12 Dec 2025 16:28:25 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BAE776142F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Dec 2025 16:28:23 +0100 (CET)","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 C4FEF183;\n\tFri, 12 Dec 2025 16:28:20 +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=\"rumbSJHP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1765553300;\n\tbh=vHtV1xUpcrIILef/PRdJsN2Zcv4tWFzCjvEYOLnRx0w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rumbSJHPYSAVR3Ym+aoSTpKiKS3Xjum0hU1lchawfbFnkKAJAAPuNxI5xO+vrVrl1\n\tIIeXmCbA6Ww/aywohVPY8/CUCzXbYh2dfUgkkzEOt0VE/x0rKjvJ0KbsZi92uohSts\n\t08J+golcVa6H2ue3iX/E2zVw5a3x25rEQFjlrRes=","Date":"Fri, 12 Dec 2025 16:28:20 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Dan Scally <dan.scally@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 2/7] libcamera: mali-c55: Split TPG and Inline camera\n\thandling","Message-ID":"<ip6afee2fvho3qkcch7fi5kmetr4jnhbvd2xayq32o3ttyuho4@thi4gv5ktlne>","References":"<20251210-mali-cru-v2-0-e26421de202b@ideasonboard.com>\n\t<20251210-mali-cru-v2-2-e26421de202b@ideasonboard.com>\n\t<c6e97e10-4d21-44de-8c7b-f1ddbea20056@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<c6e97e10-4d21-44de-8c7b-f1ddbea20056@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>"}}]