{"id":14712,"url":"https://patchwork.libcamera.org/api/patches/14712/?format=json","web_url":"https://patchwork.libcamera.org/patch/14712/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20211123131040.1542581-3-naush@raspberrypi.com>","date":"2021-11-23T13:10:40","name":"[libcamera-devel,v3,2/2] pipeline: raspberrypi: Tidy the camera enumeration and registration logic","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"186c10ed0a88f66d5f77a90526f64c4856027ca0","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/?format=json","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/14712/mbox/","series":[{"id":2745,"url":"https://patchwork.libcamera.org/api/series/2745/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2745","date":"2021-11-23T13:10:38","name":"Raspberry Pi: Multi-camera changes","version":3,"mbox":"https://patchwork.libcamera.org/series/2745/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/14712/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/14712/checks/","tags":{},"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 2AB07C324F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Nov 2021 13:11:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 373D1603C2;\n\tTue, 23 Nov 2021 14:11:20 +0100 (CET)","from mail-wr1-x429.google.com (mail-wr1-x429.google.com\n\t[IPv6:2a00:1450:4864:20::429])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9792E60230\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Nov 2021 14:11:16 +0100 (CET)","by mail-wr1-x429.google.com with SMTP id s13so38937879wrb.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Nov 2021 05:11:16 -0800 (PST)","from naush-laptop.pitowers.org\n\t([2a00:1098:3142:14:34c6:a31e:c2e1:260d])\n\tby smtp.gmail.com with ESMTPSA id\n\tb197sm1106142wmb.24.2021.11.23.05.11.14\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 23 Nov 2021 05:11:15 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"QkUycv9P\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=ggF4Mfm9Kuz2SdxEbLUoqjJl/yr8IZtfRCVoHgVM3To=;\n\tb=QkUycv9PkprUJNoP4kCl1nyAWcZpUfxZW4So8pE5a6ZoY+o36rlqN4rp1YRXavBItc\n\tihg6hF2J3tDW6jhzaWEhayB3ykhGerSV2e8Mj3OPTkF8It7OMNZ1PXiSrL2iFMLJKZcl\n\tAIR/IiCdAvev9AFSEEfyjwbGpHfe9CQ58KTZb4/117ZX0QPUbHSpPGH0tt2l+U8P/BE7\n\tSzGiOrKKiqtN8dLlpAjVNkddpcsvghkZ8jT/vML6NIUHL2VQ64fhbAeCTxqIYIwdLitK\n\ta1I+VSNoghMtBPt08LqM6kLg4pMmFc7aNRHcMmy4ip11T275LKeP4v//pd0ujWCWjK+V\n\t/18A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=ggF4Mfm9Kuz2SdxEbLUoqjJl/yr8IZtfRCVoHgVM3To=;\n\tb=bqckVjaNCTLiILxDDJrAMZzfP4PxZSJdWUKBGMc18tA7DiUHfMMD0tTcpaxFNntO6A\n\tqN+1W7CdM41w9XAjOIC8a2UfYgOgbe5mrDOWGE+VrVwEPuVtoDMQP9eTA41L9D4b9GvL\n\tRjc+vG/Ay/lvydm7dbHtomEm39HuW9GQDi4cO5nU892WY//hrFszDTK371TViKjlXw22\n\tKqcqWuvA5ghLsZ7Hp8TcjtSxpXwQun1dCjBDSCrLmNatnTvTQD6j1qW6/ChB0LVUr2bJ\n\tRPX7yOFOCZUX9yx6kZl5P6WVCTkyd1MpcIEA+cAb0aoQxrJc1Uvz+mkrt251vue3MezG\n\tG+nQ==","X-Gm-Message-State":"AOAM533oQjkQ6XDYDDWjFRduwVDD8s1SkszONahrptv3d0ZPv79F9pRC\n\tSbHCSSpFCY31ZO06Qf9uQ/DXHOCOR7cJ0qnE","X-Google-Smtp-Source":"ABdhPJxfzbPvjkAR7GBRnvHB7ibDTwRMUWLsYJOjif0YNMaNe50b6B98VMkSgozarMwZs1sfz+7Vwg==","X-Received":"by 2002:a05:6000:1acd:: with SMTP id\n\ti13mr7330504wry.398.1637673076038; \n\tTue, 23 Nov 2021 05:11:16 -0800 (PST)","From":"Naushir Patuck <naush@raspberrypi.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 23 Nov 2021 13:10:40 +0000","Message-Id":"<20211123131040.1542581-3-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20211123131040.1542581-1-naush@raspberrypi.com>","References":"<20211123131040.1542581-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v3 2/2] pipeline: raspberrypi: Tidy the\n\tcamera enumeration and registration logic","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>"},"content":"When acquiring the media device, it is not necessary to match all entity names,\nso remove it. Aditionally, we do not need to keep the MediaEntity pointers for\nthe Unicam and ISP devices stored within the PipelineHandlerRPi class. Instead\nthese can be stored locally in PipelineHandlerRPi::match().\n\nPipelineHandlerRPi::registerCamera() now returns an int error code instead of a\nboolean for pass/fail.\n\nSigned-off-by: Naushir Patuck <naush@raspberrypi.com>\n---\n .../pipeline/raspberrypi/raspberrypi.cpp      | 88 +++++++++++--------\n 1 file changed, 52 insertions(+), 36 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 12fd38061241..c5034480820e 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -311,14 +311,11 @@ private:\n \t\treturn static_cast<RPiCameraData *>(camera->_d());\n \t}\n \n-\tbool registerCamera();\n+\tint registerCamera(MediaDevice *unicam, MediaDevice *isp);\n \tint queueAllBuffers(Camera *camera);\n \tint prepareBuffers(Camera *camera);\n \tvoid freeBuffers(Camera *camera);\n \tvoid mapBuffers(Camera *camera, const RPi::BufferMap &buffers, unsigned int mask);\n-\n-\tMediaDevice *unicam_;\n-\tMediaDevice *isp_;\n };\n \n RPiCameraConfiguration::RPiCameraConfiguration(const RPiCameraData *data)\n@@ -509,7 +506,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n }\n \n PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)\n-\t: PipelineHandler(manager), unicam_(nullptr), isp_(nullptr)\n+\t: PipelineHandler(manager)\n {\n }\n \n@@ -993,49 +990,65 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)\n \n bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n {\n+\tMediaDevice *unicamDevice, *ispDevice;\n+\n \tDeviceMatch unicam(\"unicam\");\n-\tDeviceMatch isp(\"bcm2835-isp\");\n+\tunicamDevice = acquireMediaDevice(enumerator, unicam);\n \n-\tunicam.add(\"unicam-image\");\n+\tif (!unicamDevice) {\n+\t\tLOG(RPI, Debug) << \"Unable to acquire a Unicam instance\";\n+\t\treturn false;\n+\t}\n \n-\tisp.add(\"bcm2835-isp0-output0\"); /* Input */\n-\tisp.add(\"bcm2835-isp0-capture1\"); /* Output 0 */\n-\tisp.add(\"bcm2835-isp0-capture2\"); /* Output 1 */\n-\tisp.add(\"bcm2835-isp0-capture3\"); /* Stats */\n+\tDeviceMatch isp(\"bcm2835-isp\");\n+\tispDevice = acquireMediaDevice(enumerator, isp);\n \n-\tunicam_ = acquireMediaDevice(enumerator, unicam);\n-\tif (!unicam_)\n+\tif (!ispDevice) {\n+\t\tLOG(RPI, Debug) << \"Unable to acquire ISP instance\";\n \t\treturn false;\n+\t}\n \n-\tisp_ = acquireMediaDevice(enumerator, isp);\n-\tif (!isp_)\n+\tint ret = registerCamera(unicamDevice, ispDevice);\n+\tif (ret) {\n+\t\tLOG(RPI, Error) << \"Failed to register camera: \" << ret;\n \t\treturn false;\n+\t}\n \n-\treturn registerCamera();\n+\treturn true;\n }\n \n-bool PipelineHandlerRPi::registerCamera()\n+int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp)\n {\n \tstd::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);\n+\n \tif (!data->dmaHeap_.isValid())\n-\t\treturn false;\n+\t\treturn -ENOMEM;\n+\n+\tMediaEntity *unicamImage = unicam->getEntityByName(\"unicam-image\");\n+\tMediaEntity *ispOutput0 = isp->getEntityByName(\"bcm2835-isp0-output0\");\n+\tMediaEntity *ispCapture1 = isp->getEntityByName(\"bcm2835-isp0-capture1\");\n+\tMediaEntity *ispCapture2 = isp->getEntityByName(\"bcm2835-isp0-capture2\");\n+\tMediaEntity *ispCapture3 = isp->getEntityByName(\"bcm2835-isp0-capture3\");\n+\n+\tif (!unicamImage || !ispOutput0 || !ispCapture1 || !ispCapture2 || !ispCapture3)\n+\t\treturn -ENOENT;\n \n \t/* Locate and open the unicam video streams. */\n-\tdata->unicam_[Unicam::Image] = RPi::Stream(\"Unicam Image\", unicam_->getEntityByName(\"unicam-image\"));\n+\tdata->unicam_[Unicam::Image] = RPi::Stream(\"Unicam Image\", unicamImage);\n \n \t/* An embedded data node will not be present if the sensor does not support it. */\n-\tMediaEntity *embeddedEntity = unicam_->getEntityByName(\"unicam-embedded\");\n-\tif (embeddedEntity) {\n-\t\tdata->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\", embeddedEntity);\n+\tMediaEntity *unicamEmbedded = unicam->getEntityByName(\"unicam-embedded\");\n+\tif (unicamEmbedded) {\n+\t\tdata->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\", unicamEmbedded);\n \t\tdata->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(),\n \t\t\t\t\t\t\t\t\t   &RPiCameraData::unicamBufferDequeue);\n \t}\n \n \t/* Tag the ISP input stream as an import stream. */\n-\tdata->isp_[Isp::Input] = RPi::Stream(\"ISP Input\", isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n-\tdata->isp_[Isp::Output0] = RPi::Stream(\"ISP Output0\", isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n-\tdata->isp_[Isp::Output1] = RPi::Stream(\"ISP Output1\", isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n-\tdata->isp_[Isp::Stats] = RPi::Stream(\"ISP Stats\", isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n+\tdata->isp_[Isp::Input] = RPi::Stream(\"ISP Input\", ispOutput0, true);\n+\tdata->isp_[Isp::Output0] = RPi::Stream(\"ISP Output0\", ispCapture1);\n+\tdata->isp_[Isp::Output1] = RPi::Stream(\"ISP Output1\", ispCapture2);\n+\tdata->isp_[Isp::Stats] = RPi::Stream(\"ISP Stats\", ispCapture3);\n \n \t/* Wire up all the buffer connections. */\n \tdata->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(), &RPiCameraData::frameStarted);\n@@ -1046,7 +1059,7 @@ bool PipelineHandlerRPi::registerCamera()\n \tdata->isp_[Isp::Stats].dev()->bufferReady.connect(data.get(), &RPiCameraData::ispOutputDequeue);\n \n \t/* Identify the sensor. */\n-\tfor (MediaEntity *entity : unicam_->entities()) {\n+\tfor (MediaEntity *entity : unicam->entities()) {\n \t\tif (entity->function() == MEDIA_ENT_F_CAM_SENSOR) {\n \t\t\tdata->sensor_ = std::make_unique<CameraSensor>(entity);\n \t\t\tbreak;\n@@ -1054,23 +1067,23 @@ bool PipelineHandlerRPi::registerCamera()\n \t}\n \n \tif (!data->sensor_)\n-\t\treturn false;\n+\t\treturn -EINVAL;\n \n \tif (data->sensor_->init())\n-\t\treturn false;\n+\t\treturn -EINVAL;\n \n \tdata->sensorFormats_ = populateSensorFormats(data->sensor_);\n \n \tipa::RPi::SensorConfig sensorConfig;\n \tif (data->loadIPA(&sensorConfig)) {\n \t\tLOG(RPI, Error) << \"Failed to load a suitable IPA library\";\n-\t\treturn false;\n+\t\treturn -EINVAL;\n \t}\n \n-\tif (sensorConfig.sensorMetadata ^ !!embeddedEntity) {\n+\tif (sensorConfig.sensorMetadata ^ !!unicamEmbedded) {\n \t\tLOG(RPI, Warning) << \"Mismatch between Unicam and CamHelper for embedded data usage!\";\n \t\tsensorConfig.sensorMetadata = false;\n-\t\tif (embeddedEntity)\n+\t\tif (unicamEmbedded)\n \t\t\tdata->unicam_[Unicam::Embedded].dev()->bufferReady.disconnect();\n \t}\n \n@@ -1091,12 +1104,12 @@ bool PipelineHandlerRPi::registerCamera()\n \n \tfor (auto stream : data->streams_) {\n \t\tif (stream->dev()->open())\n-\t\t\treturn false;\n+\t\t\tcontinue;\n \t}\n \n \tif (!data->unicam_[Unicam::Image].dev()->caps().hasMediaController()) {\n \t\tLOG(RPI, Error) << \"Unicam driver does not use the MediaController, please update your kernel!\";\n-\t\treturn false;\n+\t\treturn -EINVAL;\n \t}\n \n \t/*\n@@ -1158,7 +1171,7 @@ bool PipelineHandlerRPi::registerCamera()\n \n \tif (!bayerFormat.isValid()) {\n \t\tLOG(RPI, Error) << \"No Bayer format found\";\n-\t\treturn false;\n+\t\treturn -EINVAL;\n \t}\n \tdata->nativeBayerOrder_ = bayerFormat.order;\n \n@@ -1178,7 +1191,10 @@ bool PipelineHandlerRPi::registerCamera()\n \t\tCamera::create(std::move(data), id, streams);\n \tPipelineHandler::registerCamera(std::move(camera));\n \n-\treturn true;\n+\tLOG(RPI, Info) << \"Registered camera \" << id\n+\t\t       << \" to Unicam device \" << unicam->deviceNode()\n+\t\t       << \" and ISP device \" << isp->deviceNode();\n+\treturn 0;\n }\n \n int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n","prefixes":["libcamera-devel","v3","2/2"]}