[libcamera-devel] [PATCH 4/4] libcamera: device_enumerator_udev: Support entities sharing device nodes

[Laurent Pinchart]

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 <laurent.pinchart at ideasonboard.com>
---
 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<MediaDevice> &media)
 {
-	unsigned int pendingNodes = 0;
-	int ret;
+	std::set<dev_t> 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<MediaDevice>
 		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<MediaDevice> 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 <list>
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <sys/types.h>
 
@@ -39,11 +40,27 @@ private:
 	struct udev_monitor *monitor_;
 	EventNotifier *notifier_;
 
-	std::map<std::shared_ptr<MediaDevice>, std::list<dev_t>> deps_;
-	std::map<dev_t, std::shared_ptr<MediaDevice>> devnumToDevice_;
-	std::map<dev_t, MediaEntity *> devnumToEntity_;
+	using DependencyMap = std::map<dev_t, std::list<MediaEntity *>>;
 
-	std::list<dev_t> orphans_;
+	struct MediaDeviceDeps {
+		MediaDeviceDeps(const std::shared_ptr<MediaDevice> &media,
+				const DependencyMap &deps)
+			: media_(media), deps_(deps)
+		{
+		}
+
+		bool operator==(const MediaDeviceDeps &other) const
+		{
+			return media_ == other.media_;
+		}
+
+		std::shared_ptr<MediaDevice> media_;
+		DependencyMap deps_;
+	};
+
+	std::set<dev_t> orphans_;
+	std::list<MediaDeviceDeps> pending_;
+	std::map<dev_t, MediaDeviceDeps *> devMap_;
 
 	int addUdevDevice(struct udev_device *dev);
 	int populateMediaDevice(const std::shared_ptr<MediaDevice> &media);
-- 
Regards,

Laurent Pinchart