{"id":1963,"url":"https://patchwork.libcamera.org/api/patches/1963/?format=json","web_url":"https://patchwork.libcamera.org/patch/1963/","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":"<20190912200330.19004-5-laurent.pinchart@ideasonboard.com>","date":"2019-09-12T20:03:30","name":"[libcamera-devel,4/4] libcamera: device_enumerator_udev: Support entities sharing device nodes","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"7995d2d64d36d93a9cf3a2528a285a85d06133cd","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/1963/mbox/","series":[{"id":487,"url":"https://patchwork.libcamera.org/api/series/487/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=487","date":"2019-09-12T20:03:26","name":"Fix udev device enumerator with V4L2 M2M devices","version":1,"mbox":"https://patchwork.libcamera.org/series/487/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/1963/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/1963/checks/","tags":{},"headers":{"Return-Path":"","Received":["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 54E7E6195F\n\tfor ;\n\tThu, 12 Sep 2019 22:03:47 +0200 (CEST)","from pendragon.lan (bl10-204-24.dsl.telepac.pt [85.243.204.24])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9DA8133A;\n\tThu, 12 Sep 2019 22:03:46 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1568318627;\n\tbh=4rTg6U9ANtXakvxxTZDu6ww9bU4KooB3Mp1nTy3auCA=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=tj2fEhYPnTl/lJ9y/SSZ62ozzCW4FxxMX9kf+dRkhRuFXG8vN8lk6Ns5L/ScXPRHW\n\tf5kxDlkIrG7V9Vcea+odQDgDR4iSqzX3Pq7tIa622KgPcS7LcGVnJ8yzPOXsJWigap\n\toqjj166zHxZMZzXzGNR8AnXc9VVw4z6HGJe6cezc=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Thu, 12 Sep 2019 23:03:30 +0300","Message-Id":"<20190912200330.19004-5-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.21.0","In-Reply-To":"<20190912200330.19004-1-laurent.pinchart@ideasonboard.com>","References":"<20190912200330.19004-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 4/4] libcamera: device_enumerator_udev:\n\tSupport entities sharing device nodes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 12 Sep 2019 20:03:47 -0000"},"content":"Some media devices, such as V4L2 M2M devices, share the same device node\nfor multiple entities. The udev enumerator used to support this, but\ncommit 6e620349009d (\"libcamera: device_enumerator: fix udev media graph\nloading dependency\") broke this.\n\nTo fix the problem, rework the media device to V4L2 devices matching\ncode. A new MediaDeviceDeps internal struct stores unmet device number\ndependencies for a media device, and is stored in a list of pending\nmedia devices. To avoid linear lookups, the dependencies are cached in a\nreverse map of device number to media device dependencies.\n\nFixes: 6e620349009d (\"libcamera: device_enumerator: fix udev media graph loading dependency\")\nSigned-off-by: Laurent Pinchart \n---\n src/libcamera/device_enumerator_udev.cpp | 100 ++++++++++++------\n .../include/device_enumerator_udev.h | 25 ++++-\n 2 files changed, 89 insertions(+), 36 deletions(-)","diff":"diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\nindex c40770911d3d..ddcd59ea52c1 100644\n--- a/src/libcamera/device_enumerator_udev.cpp\n+++ b/src/libcamera/device_enumerator_udev.cpp\n@@ -170,8 +170,8 @@ done:\n \n int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr &media)\n {\n-\tunsigned int pendingNodes = 0;\n-\tint ret;\n+\tstd::set children;\n+\tDependencyMap deps;\n \n \t/* Associate entities to device node paths. */\n \tfor (MediaEntity *entity : media->entities()) {\n@@ -181,28 +181,50 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr\n \t\tdev_t devnum = makedev(entity->deviceMajor(),\n \t\t\t\t entity->deviceMinor());\n \n-\t\t/* Take device from orphan list first, if it is in the list. */\n-\t\tauto orphan = std::find(orphans_.begin(), orphans_.end(), devnum);\n-\t\tif (orphan != orphans_.end()) {\n-\t\t\tstd::string deviceNode = lookupDeviceNode(devnum);\n-\t\t\tif (deviceNode.empty())\n-\t\t\t\treturn -EINVAL;\n-\n-\t\t\tret = entity->setDeviceNode(deviceNode);\n-\t\t\tif (ret)\n-\t\t\t\treturn ret;\n-\n-\t\t\torphans_.erase(orphan);\n+\t\t/*\n+\t\t * If the devnum isn't in the orphans list, add it to the unmet\n+\t\t * dependencies.\n+\t\t */\n+\t\tif (orphans_.find(devnum) == orphans_.end()) {\n+\t\t\tdeps[devnum].push_back(entity);\n \t\t\tcontinue;\n \t\t}\n \n-\t\tdeps_[media].push_back(devnum);\n-\t\tdevnumToDevice_[devnum] = media;\n-\t\tdevnumToEntity_[devnum] = entity;\n-\t\tpendingNodes++;\n+\t\t/*\n+\t\t * Otherwise take it from the orphans list. Don't remove the\n+\t\t * entry from the list yet as other entities in this media\n+\t\t * device may need the same device.\n+\t\t */\n+\t\tstd::string deviceNode = lookupDeviceNode(devnum);\n+\t\tif (deviceNode.empty())\n+\t\t\treturn -EINVAL;\n+\n+\t\tint ret = entity->setDeviceNode(deviceNode);\n+\t\tif (ret)\n+\t\t\treturn ret;\n+\n+\t\tchildren.insert(devnum);\n+\t}\n+\n+\t/* Remove all found children from the orphans list. */\n+\tfor (auto it = orphans_.begin(), last = orphans_.end(); it != last;) {\n+\t\tif (children.find(*it) != children.end())\n+\t\t\tit = orphans_.erase(it);\n+\t\telse\n+\t\t\t++it;\n+\t}\n+\n+\t/*\n+\t * If the media device has unmet dependencies, add it to the pending\n+\t * list and update the devnum map accordingly.\n+\t */\n+\tif (!deps.empty()) {\n+\t\tpending_.emplace_back(media, deps);\n+\t\tfor (const auto &dep : deps)\n+\t\t\tdevMap_[dep.first] = &pending_.back();\n \t}\n \n-\treturn pendingNodes;\n+\treturn deps.size();\n }\n \n /**\n@@ -247,28 +269,42 @@ std::string DeviceEnumeratorUdev::lookupDeviceNode(dev_t devnum)\n */\n int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)\n {\n-\tMediaEntity *entity = devnumToEntity_[devnum];\n-\tif (!entity) {\n-\t\torphans_.push_back(devnum);\n+\t/*\n+\t * If the devnum doesn't belong to any media device, add it to the\n+\t * orphans list.\n+\t */\n+\tauto it = devMap_.find(devnum);\n+\tif (it == devMap_.end()) {\n+\t\torphans_.insert(devnum);\n \t\treturn 0;\n \t}\n \n+\t/*\n+\t * Set the device node for all entities matching the devnum. Multiple\n+\t * entities can share the same device node, for instance for V4L2 M2M\n+\t * devices.\n+\t */\n \tstd::string deviceNode = lookupDeviceNode(devnum);\n \tif (deviceNode.empty())\n \t\treturn -EINVAL;\n \n-\tint ret = entity->setDeviceNode(deviceNode);\n-\tif (ret)\n-\t\treturn ret;\n+\tMediaDeviceDeps *deps = it->second;\n+\tfor (MediaEntity *entity : deps->deps_[devnum]) {\n+\t\tint ret = entity->setDeviceNode(deviceNode);\n+\t\tif (ret)\n+\t\t\treturn ret;\n+\t}\n \n-\tstd::shared_ptr media = devnumToDevice_[devnum];\n-\tdeps_[media].remove(devnum);\n-\tdevnumToDevice_.erase(devnum);\n-\tdevnumToEntity_.erase(devnum);\n+\t/*\n+\t * Remove the devnum from the unmet dependencies for this media device.\n+\t * If no more dependency is unmet, add the media device to the\n+\t * enumerator.\n+\t */\n+\tdeps->deps_.erase(devnum);\n \n-\tif (deps_[media].empty()) {\n-\t\taddDevice(media);\n-\t\tdeps_.erase(media);\n+\tif (deps->deps_.empty()) {\n+\t\taddDevice(deps->media_);\n+\t\tpending_.remove(*deps);\n \t}\n \n \treturn 0;\ndiff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h\nindex fb7cac8011a1..6d8268620185 100644\n--- a/src/libcamera/include/device_enumerator_udev.h\n+++ b/src/libcamera/include/device_enumerator_udev.h\n@@ -10,6 +10,7 @@\n #include \n #include \n #include \n+#include \n #include \n #include \n \n@@ -39,11 +40,27 @@ private:\n \tstruct udev_monitor *monitor_;\n \tEventNotifier *notifier_;\n \n-\tstd::map, std::list> deps_;\n-\tstd::map> devnumToDevice_;\n-\tstd::map devnumToEntity_;\n+\tusing DependencyMap = std::map>;\n \n-\tstd::list orphans_;\n+\tstruct MediaDeviceDeps {\n+\t\tMediaDeviceDeps(const std::shared_ptr &media,\n+\t\t\t\tconst DependencyMap &deps)\n+\t\t\t: media_(media), deps_(deps)\n+\t\t{\n+\t\t}\n+\n+\t\tbool operator==(const MediaDeviceDeps &other) const\n+\t\t{\n+\t\t\treturn media_ == other.media_;\n+\t\t}\n+\n+\t\tstd::shared_ptr media_;\n+\t\tDependencyMap deps_;\n+\t};\n+\n+\tstd::set orphans_;\n+\tstd::list pending_;\n+\tstd::map devMap_;\n \n \tint addUdevDevice(struct udev_device *dev);\n \tint populateMediaDevice(const std::shared_ptr &media);\n","prefixes":["libcamera-devel","4/4"]}