Patch Detail
Show a patch.
GET /api/1.1/patches/14811/?format=api
{ "id": 14811, "url": "https://patchwork.libcamera.org/api/1.1/patches/14811/?format=api", "web_url": "https://patchwork.libcamera.org/patch/14811/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20211126153538.2594702-2-naush@raspberrypi.com>", "date": "2021-11-26T15:35:38", "name": "[libcamera-devel,v4,2/2] pipeline: raspberrypi: Tidy the camera enumeration and registration logic", "commit_ref": "22574ff19545d96e8f873081d51fde2682a12293", "pull_url": null, "state": "accepted", "archived": false, "hash": "795fe4d448b26a50bae3fa4b55c4dc135250526a", "submitter": { "id": 34, "url": "https://patchwork.libcamera.org/api/1.1/people/34/?format=api", "name": "Naushir Patuck", "email": "naush@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/14811/mbox/", "series": [ { "id": 2764, "url": "https://patchwork.libcamera.org/api/1.1/series/2764/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2764", "date": "2021-11-26T15:35:37", "name": "[libcamera-devel,v4,1/2] pipeline: raspberrypi: Split out device enumeration and camera registration", "version": 4, "mbox": "https://patchwork.libcamera.org/series/2764/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/14811/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/14811/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 05923BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 26 Nov 2021 15:35:50 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B319760559;\n\tFri, 26 Nov 2021 16:35:49 +0100 (CET)", "from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com\n\t[IPv6:2a00:1450:4864:20::32d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C60286011E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Nov 2021 16:35:45 +0100 (CET)", "by mail-wm1-x32d.google.com with SMTP id\n\tk37-20020a05600c1ca500b00330cb84834fso11003862wms.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Nov 2021 07:35:45 -0800 (PST)", "from naush-laptop.pitowers.org\n\t([2a00:1098:3142:14:89da:d0c:9f56:bbfc])\n\tby smtp.gmail.com with ESMTPSA id\n\tp14sm10591272wms.29.2021.11.26.07.35.44\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 26 Nov 2021 07:35:44 -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=\"r7PFpsO0\"; 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=9UvufzJ/yJE+8hSE0cgIFM2u970E24snpZO91WK9W18=;\n\tb=r7PFpsO0yT5D4cekfMSGLJvj/GttQCdpDayiqA4wsFutQnTV3x6BjUY0CrokfVNrjC\n\t19VGZHFWMIGKx+ECbHi0JKfwJWUr/bpVhnYIyTVunw9DgtlRRRc1AMNm4LZsr5uVje5G\n\thRSmwCxDn6o+PT41ex9QkL36vsgUaPg2g9ZsVVwMUXaqMwJpxReDP0Trg+P1SGMdM63O\n\tAY260ZDQevsCCPcGaM8oFK8n6m7dmHpMXhTnAHk1fRNKtnaUBueWMZrZe2mCLwQn4SBy\n\taHo83bPML7giq2fnOWzGwjvdTLXOGGkXoEdMnDJCn1VtEZMJV7LK+f/+sIIgUEjsGDfp\n\t7NrQ==", "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=9UvufzJ/yJE+8hSE0cgIFM2u970E24snpZO91WK9W18=;\n\tb=Wmk/ckEMSHc3LyK4xMkurXQ6zNzcDdXdmoWaSj0XyIRnn82GUThwvr20n0wRYsrq9D\n\tYiRlsdgsxNBqRDV80IdGl//2ILc9xtk163VKoAN716hHvThg1mZpu2pC3xd1Y98HAGBl\n\tamj8i2T3zkO4QZHVDToLoB58lzsbeQ6pLkpUrayNTboGPwUdLwMl3rI1ErkuElPHz9aS\n\tox+haEgkID6gSegA9uZzGmxE9qyXjnrgoOIrnnH38WT7O0KrPk9SSwJcAu4kgDyvFFVe\n\tAcg08GIuQEb+HBXb24KPRYpaiPi69X2qcl4QnofXRFIQRt0EF5OUbDohQ5lAaXJEbf4x\n\tAENA==", "X-Gm-Message-State": "AOAM530f99aAK+tdVsFLlK2OzLEFE4w8Tq2Fs+46okLP0W43OjSdp5Pf\n\tKw1tFU10u/28GBG54Hj4t6cezYYutvC6sK5/", "X-Google-Smtp-Source": "ABdhPJwfT39Fv4wlyO1Vw7aPUj12B4NQ7OrzyaYRwTdkFDk+kvhGaaK9Yp0NEaI7DgzUl2LCk9b59w==", "X-Received": "by 2002:a1c:98ca:: with SMTP id\n\ta193mr16192526wme.162.1637940945149; \n\tFri, 26 Nov 2021 07:35:45 -0800 (PST)", "From": "Naushir Patuck <naush@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 26 Nov 2021 15:35:38 +0000", "Message-Id": "<20211126153538.2594702-2-naush@raspberrypi.com>", "X-Mailer": "git-send-email 2.25.1", "In-Reply-To": "<20211126153538.2594702-1-naush@raspberrypi.com>", "References": "<20211126153538.2594702-1-naush@raspberrypi.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v4 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 | 89 +++++++++++--------\n 1 file changed, 52 insertions(+), 37 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 5a6dfa4e8b2a..e31fa3b23859 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -314,14 +314,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@@ -512,7 +509,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@@ -997,48 +994,62 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)\n bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n {\n \tDeviceMatch unicam(\"unicam\");\n-\tDeviceMatch isp(\"bcm2835-isp\");\n+\tMediaDevice *unicamDevice = 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+\tMediaDevice *ispDevice = 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@@ -1049,7 +1060,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@@ -1057,23 +1068,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@@ -1093,13 +1104,14 @@ bool PipelineHandlerRPi::registerCamera()\n \t\tdata->streams_.push_back(&stream);\n \n \tfor (auto stream : data->streams_) {\n-\t\tif (stream->dev()->open())\n-\t\t\treturn false;\n+\t\tint ret = stream->dev()->open();\n+\t\tif (ret)\n+\t\t\treturn ret;\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@@ -1161,7 +1173,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@@ -1181,7 +1193,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", "v4", "2/2" ] }