From patchwork Thu Sep 12 20:03:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1960 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 09B53600E9 for ; Thu, 12 Sep 2019 22:03:45 +0200 (CEST) Received: from pendragon.lan (bl10-204-24.dsl.telepac.pt [85.243.204.24]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 78C0C57E; Thu, 12 Sep 2019 22:03:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1568318624; bh=bo7SorejL3DE7ScmvSpk3qcsNv9md85LasT1Dr71sbA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ofZ8MDegN+EL3kuLu6G8H6D38rIQ78ZFu54fHVXC7G6zXYvAvKy9Obgm/3CGF1t+w scWCi51dbh8d0yh9Usvgkf3iGfE036jqp3O+OaIuKvinVGaNI8WPlj3JtzHG9+7zTz G1wrAv9z5jQoNItrXpWP9u7q7FvzONrmkTx5mBGA= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 12 Sep 2019 23:03:27 +0300 Message-Id: <20190912200330.19004-2-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 Subject: [libcamera-devel] [PATCH 1/4] libcamera: device_enumerator: Move lookupDeviceNode() to child classes X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Sep 2019 20:03:45 -0000 The lookupDeviceNode() method is declared as pure virtual in the base DeviceEnumerator class, but is only called by derived classes. Move it to the DeviceEnumeratorSysfs and DeviceEnumeratorUdev. This allows changing the udev version to take a dev_t instead of separate major/minor, as that's what both the caller and the callee end up using. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- src/libcamera/device_enumerator.cpp | 13 ------------- src/libcamera/device_enumerator_sysfs.cpp | 11 +++++++++++ src/libcamera/device_enumerator_udev.cpp | 19 ++++++++++++------- src/libcamera/include/device_enumerator.h | 2 -- .../include/device_enumerator_sysfs.h | 2 +- .../include/device_enumerator_udev.h | 2 +- 6 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index e76438afdd65..0b596bcec363 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -306,17 +306,4 @@ std::shared_ptr DeviceEnumerator::search(const DeviceMatch &dm) return nullptr; } -/** - * \fn DeviceEnumerator::lookupDeviceNode(int major, int minor) - * \brief Lookup device node path from device number - * \param[in] major The device major number - * \param[in] minor The device minor number - * - * Translate a device number given as \a major and \a minor to a device node - * path. - * - * \return the device node path on success, or an empty string if the lookup - * fails - */ - } /* namespace libcamera */ diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp index 78a7da8d79e5..ad26affb38a3 100644 --- a/src/libcamera/device_enumerator_sysfs.cpp +++ b/src/libcamera/device_enumerator_sysfs.cpp @@ -112,6 +112,17 @@ int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr if (entity->deviceMajor() == 0 && entity->deviceMinor() == 0) continue; - std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), - entity->deviceMinor()); dev_t devnum = makedev(entity->deviceMajor(), entity->deviceMinor()); + std::string deviceNode = lookupDeviceNode(devnum); /* Take device from orphan list first, if it is in the list. */ if (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) { @@ -205,14 +204,21 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr return pendingNodes; } -std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor) +/** + * \brief Lookup device node path from device number + * \param[in] devnum The device number + * + * Translate a device number given as \a devnum to a device node path. + * + * \return The device node path on success, or an empty string if the lookup + * fails + */ +std::string DeviceEnumeratorUdev::lookupDeviceNode(dev_t devnum) { struct udev_device *device; const char *name; - dev_t devnum; std::string deviceNode = std::string(); - devnum = makedev(major, minor); device = udev_device_new_from_devnum(udev_, 'c', devnum); if (!device) return std::string(); @@ -246,8 +252,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum) return 0; } - std::string deviceNode = lookupDeviceNode(entity->deviceMajor(), - entity->deviceMinor()); + std::string deviceNode = lookupDeviceNode(devnum); if (deviceNode.empty()) return -EINVAL; diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h index c5d26f1a883e..770f42772270 100644 --- a/src/libcamera/include/device_enumerator.h +++ b/src/libcamera/include/device_enumerator.h @@ -50,8 +50,6 @@ protected: private: std::vector> devices_; - - virtual std::string lookupDeviceNode(int major, int minor) = 0; }; } /* namespace libcamera */ diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h index 242b22b289b0..9063f6a75e11 100644 --- a/src/libcamera/include/device_enumerator_sysfs.h +++ b/src/libcamera/include/device_enumerator_sysfs.h @@ -24,7 +24,7 @@ public: private: int populateMediaDevice(const std::shared_ptr &media); - std::string lookupDeviceNode(int major, int minor) final; + std::string lookupDeviceNode(int major, int minor); }; } /* namespace libcamera */ diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h index 5bdcdea65c35..fb7cac8011a1 100644 --- a/src/libcamera/include/device_enumerator_udev.h +++ b/src/libcamera/include/device_enumerator_udev.h @@ -47,7 +47,7 @@ private: int addUdevDevice(struct udev_device *dev); int populateMediaDevice(const std::shared_ptr &media); - std::string lookupDeviceNode(int major, int minor) final; + std::string lookupDeviceNode(dev_t devnum); int addV4L2Device(dev_t devnum); void udevNotify(EventNotifier *notifier); From patchwork Thu Sep 12 20:03:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1961 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 97FD260C1A for ; Thu, 12 Sep 2019 22:03:45 +0200 (CEST) Received: from pendragon.lan (bl10-204-24.dsl.telepac.pt [85.243.204.24]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0CEF633A; Thu, 12 Sep 2019 22:03:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1568318625; bh=A5c81SPgvpkLHXknSmlBCbcSsOwAvMPMabq1kb1p0WU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Sx0js3NTmZWjaFPmEDB/OuiX0urM9qOaNYTOriFNedLxt717NBhkhbA4Lj+rH8FrI oA1+8wqJCK8LlUA53kL39gzz91hs9CkYyl+GkbIWFr3GQIjw+wGmFarLeOO6MT5MZW OyrDDl4SIdKf3H6TQKDFYnenaZKC7750oNJcqfSQ= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 12 Sep 2019 23:03:28 +0300 Message-Id: <20190912200330.19004-3-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 Subject: [libcamera-devel] [PATCH 2/4] libcamera: device_enumerator_udev: Delay device node lookup X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Sep 2019 20:03:46 -0000 When populating media devices, we look up device nodes for every entity in the media device, regardless of if the entity is present in the orphans list. This causes unnecessary lookups (that may also fail as the device node may not be ready yet at that time). Move the lookup at a later time, when the device node is actually needed. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- src/libcamera/device_enumerator_udev.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp index e0c646c997de..210f5c1f2870 100644 --- a/src/libcamera/device_enumerator_udev.cpp +++ b/src/libcamera/device_enumerator_udev.cpp @@ -180,10 +180,10 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr dev_t devnum = makedev(entity->deviceMajor(), entity->deviceMinor()); - std::string deviceNode = lookupDeviceNode(devnum); /* Take device from orphan list first, if it is in the list. */ if (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) { + std::string deviceNode = lookupDeviceNode(devnum); if (deviceNode.empty()) return -EINVAL; From patchwork Thu Sep 12 20:03:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1962 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BCDB60BCF for ; Thu, 12 Sep 2019 22:03:46 +0200 (CEST) Received: from pendragon.lan (bl10-204-24.dsl.telepac.pt [85.243.204.24]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B0BED33A; Thu, 12 Sep 2019 22:03:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1568318626; bh=8WZ0j3NkepX1Vnf4dCcBzOIHMU6JJmZJLIqiZDXgw0U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CI68Kh1bloZuP1w1NxgREOUb8SXeHgiHG4Duze/hd+MS09NuJ+qnw40l83Q7H9+Jx Qw/Gqsl1Dhwe5GdLwPGxVnv5ZeMX5gBVv2EkOVzGZvSJ7LL4M77yAg+vICKOse02BI vEzpejq4IDJI47AP2XmeQZq71r4CAitsFnRSa4HE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 12 Sep 2019 23:03:29 +0300 Message-Id: <20190912200330.19004-4-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 Subject: [libcamera-devel] [PATCH 3/4] libcamera: device_enumerator_udev: Avoid double list lookup X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Sep 2019 20:03:46 -0000 DeviceEnumeratorUdev::populateMediaDevice() searches for orphan devices in an std::list, and if found removes them using std::list::remove(). This ends up looking up the entry twice. Replace the remove() call with erase() to fix it. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- src/libcamera/device_enumerator_udev.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp index 210f5c1f2870..c40770911d3d 100644 --- a/src/libcamera/device_enumerator_udev.cpp +++ b/src/libcamera/device_enumerator_udev.cpp @@ -182,7 +182,8 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr entity->deviceMinor()); /* Take device from orphan list first, if it is in the list. */ - if (std::find(orphans_.begin(), orphans_.end(), devnum) != orphans_.end()) { + auto orphan = std::find(orphans_.begin(), orphans_.end(), devnum); + if (orphan != orphans_.end()) { std::string deviceNode = lookupDeviceNode(devnum); if (deviceNode.empty()) return -EINVAL; @@ -191,7 +192,7 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr if (ret) return ret; - orphans_.remove(devnum); + orphans_.erase(orphan); continue; } From patchwork Thu Sep 12 20:03:30 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1963 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 54E7E6195F for ; Thu, 12 Sep 2019 22:03:47 +0200 (CEST) Received: from pendragon.lan (bl10-204-24.dsl.telepac.pt [85.243.204.24]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 9DA8133A; Thu, 12 Sep 2019 22:03:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1568318627; bh=4rTg6U9ANtXakvxxTZDu6ww9bU4KooB3Mp1nTy3auCA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tj2fEhYPnTl/lJ9y/SSZ62ozzCW4FxxMX9kf+dRkhRuFXG8vN8lk6Ns5L/ScXPRHW f5kxDlkIrG7V9Vcea+odQDgDR4iSqzX3Pq7tIa622KgPcS7LcGVnJ8yzPOXsJWigap oqjj166zHxZMZzXzGNR8AnXc9VVw4z6HGJe6cezc= 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 Subject: [libcamera-devel] [PATCH 4/4] libcamera: device_enumerator_udev: Support entities sharing device nodes X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Sep 2019 20:03:47 -0000 Some media devices, such as V4L2 M2M devices, share the same device node for multiple entities. The udev enumerator used to support this, but commit 6e620349009d ("libcamera: device_enumerator: fix udev media graph loading dependency") broke this. To fix the problem, rework the media device to V4L2 devices matching code. A new MediaDeviceDeps internal struct stores unmet device number dependencies for a media device, and is stored in a list of pending media devices. To avoid linear lookups, the dependencies are cached in a reverse map of device number to media device dependencies. Fixes: 6e620349009d ("libcamera: device_enumerator: fix udev media graph loading dependency") Signed-off-by: Laurent Pinchart Tested-by: Kieran Bingham Reviewed-by: Kieran Bingham --- src/libcamera/device_enumerator_udev.cpp | 100 ++++++++++++------ .../include/device_enumerator_udev.h | 25 ++++- 2 files changed, 89 insertions(+), 36 deletions(-) diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp index c40770911d3d..ddcd59ea52c1 100644 --- a/src/libcamera/device_enumerator_udev.cpp +++ b/src/libcamera/device_enumerator_udev.cpp @@ -170,8 +170,8 @@ done: int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr &media) { - unsigned int pendingNodes = 0; - int ret; + std::set children; + DependencyMap deps; /* Associate entities to device node paths. */ for (MediaEntity *entity : media->entities()) { @@ -181,28 +181,50 @@ int DeviceEnumeratorUdev::populateMediaDevice(const std::shared_ptr dev_t devnum = makedev(entity->deviceMajor(), entity->deviceMinor()); - /* Take device from orphan list first, if it is in the list. */ - auto orphan = std::find(orphans_.begin(), orphans_.end(), devnum); - if (orphan != orphans_.end()) { - std::string deviceNode = lookupDeviceNode(devnum); - if (deviceNode.empty()) - return -EINVAL; - - ret = entity->setDeviceNode(deviceNode); - if (ret) - return ret; - - orphans_.erase(orphan); + /* + * If the devnum isn't in the orphans list, add it to the unmet + * dependencies. + */ + if (orphans_.find(devnum) == orphans_.end()) { + deps[devnum].push_back(entity); continue; } - deps_[media].push_back(devnum); - devnumToDevice_[devnum] = media; - devnumToEntity_[devnum] = entity; - pendingNodes++; + /* + * Otherwise take it from the orphans list. Don't remove the + * entry from the list yet as other entities in this media + * device may need the same device. + */ + std::string deviceNode = lookupDeviceNode(devnum); + if (deviceNode.empty()) + return -EINVAL; + + int ret = entity->setDeviceNode(deviceNode); + if (ret) + return ret; + + children.insert(devnum); + } + + /* Remove all found children from the orphans list. */ + for (auto it = orphans_.begin(), last = orphans_.end(); it != last;) { + if (children.find(*it) != children.end()) + it = orphans_.erase(it); + else + ++it; + } + + /* + * If the media device has unmet dependencies, add it to the pending + * list and update the devnum map accordingly. + */ + if (!deps.empty()) { + pending_.emplace_back(media, deps); + for (const auto &dep : deps) + devMap_[dep.first] = &pending_.back(); } - return pendingNodes; + return deps.size(); } /** @@ -247,28 +269,42 @@ std::string DeviceEnumeratorUdev::lookupDeviceNode(dev_t devnum) */ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum) { - MediaEntity *entity = devnumToEntity_[devnum]; - if (!entity) { - orphans_.push_back(devnum); + /* + * If the devnum doesn't belong to any media device, add it to the + * orphans list. + */ + auto it = devMap_.find(devnum); + if (it == devMap_.end()) { + orphans_.insert(devnum); return 0; } + /* + * Set the device node for all entities matching the devnum. Multiple + * entities can share the same device node, for instance for V4L2 M2M + * devices. + */ std::string deviceNode = lookupDeviceNode(devnum); if (deviceNode.empty()) return -EINVAL; - int ret = entity->setDeviceNode(deviceNode); - if (ret) - return ret; + MediaDeviceDeps *deps = it->second; + for (MediaEntity *entity : deps->deps_[devnum]) { + int ret = entity->setDeviceNode(deviceNode); + if (ret) + return ret; + } - std::shared_ptr media = devnumToDevice_[devnum]; - deps_[media].remove(devnum); - devnumToDevice_.erase(devnum); - devnumToEntity_.erase(devnum); + /* + * Remove the devnum from the unmet dependencies for this media device. + * If no more dependency is unmet, add the media device to the + * enumerator. + */ + deps->deps_.erase(devnum); - if (deps_[media].empty()) { - addDevice(media); - deps_.erase(media); + if (deps->deps_.empty()) { + addDevice(deps->media_); + pending_.remove(*deps); } return 0; diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h index fb7cac8011a1..6d8268620185 100644 --- a/src/libcamera/include/device_enumerator_udev.h +++ b/src/libcamera/include/device_enumerator_udev.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -39,11 +40,27 @@ private: struct udev_monitor *monitor_; EventNotifier *notifier_; - std::map, std::list> deps_; - std::map> devnumToDevice_; - std::map devnumToEntity_; + using DependencyMap = std::map>; - std::list orphans_; + struct MediaDeviceDeps { + MediaDeviceDeps(const std::shared_ptr &media, + const DependencyMap &deps) + : media_(media), deps_(deps) + { + } + + bool operator==(const MediaDeviceDeps &other) const + { + return media_ == other.media_; + } + + std::shared_ptr media_; + DependencyMap deps_; + }; + + std::set orphans_; + std::list pending_; + std::map devMap_; int addUdevDevice(struct udev_device *dev); int populateMediaDevice(const std::shared_ptr &media);