| Message ID | 20250820132316.1033443-1-dan.scally@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
On Wed, Aug 20, 2025 at 02:23:13PM +0100, Daniel Scally wrote: > Hello all > > At present, a CameraManager that is created before the device nodes for subdev > entities in a media graph have been created (for example, because some subdev > drivers have not yet probed and so the v4l2-async framework hasn't called for > them to be built yet) will never match any pipeline handlers for that media > device. This is because the MediaDevice populates itself with entities, > interfaces, pads and links but ignores the fact that the interfaces for the > subdev entities are missing. When DeviceEnumeratorUdev:: populateMediaDevice() > is called, the entity's major and minor numbers are checked, seen to be zero (as > they come from the missing interfaces) and then the entity is simply ignored > rather than treated as dependencies that need to be watched for. I may be missing something, but aren't the entity and interface registered together on the kernel side ? Even if the device node hasn't been created by udev yet, the interface should be available, with a major and minor, in the media graph. > The MediaDevice > is added through DeviceEnumerator::addDevice() to be checked against the > pipeline handler's match() functions. When the device nodes for the subdevices > are eventually detected as v4l2-async completes they are simply placed onto a > list of orphaned nodes and ignored. > > To remedy the problem this set checks for missing interfaces during > MediaDevice::populate() and if it sees some are missing, allows the MediaDevice > to be created successfully but leaves the isValid() flag false. > DeviceEnumeratorUdev is updated to check isValid() and when it is false, defers > the creation of MediaEntity / MediaPad / MediaLink objects. Each time > udevNotify() is triggered by a new device MediaDevice::populate() is checked > again for each deferred MediaDevice, and once the missing interfaces are found > in the topology the other objects are created, dependencies resolved and the > MediaDevice added as normal. > > There may be a better way of receiving notice from the kernel that the media > graph is fully complete and ready to be parsed; at the moment we rely on the > reported topology version number becoming stable across two iterations of a loop > that checks MEDIA_IOC_G_TOPOLOGY, but that's not sufficient to avoid this issue. > > Thanks > Dan > > Daniel Scally (3): > libcamera: media_device: Expand return values for populateEntities() > libcamera: device_enumerator_udev: Add initMediaDevice() > libcamera: device_enumerator_udev: Defer invalid media devices > > .../internal/device_enumerator_udev.h | 3 + > include/libcamera/internal/media_device.h | 2 +- > src/libcamera/device_enumerator.cpp | 2 +- > src/libcamera/device_enumerator_udev.cpp | 85 ++++++++++++++----- > src/libcamera/media_device.cpp | 27 +++--- > 5 files changed, 85 insertions(+), 34 deletions(-)
Hi Laurent On 20/08/2025 20:44, Laurent Pinchart wrote: > On Wed, Aug 20, 2025 at 02:23:13PM +0100, Daniel Scally wrote: >> Hello all >> >> At present, a CameraManager that is created before the device nodes for subdev >> entities in a media graph have been created (for example, because some subdev >> drivers have not yet probed and so the v4l2-async framework hasn't called for >> them to be built yet) will never match any pipeline handlers for that media >> device. This is because the MediaDevice populates itself with entities, >> interfaces, pads and links but ignores the fact that the interfaces for the >> subdev entities are missing. When DeviceEnumeratorUdev:: populateMediaDevice() >> is called, the entity's major and minor numbers are checked, seen to be zero (as >> they come from the missing interfaces) and then the entity is simply ignored >> rather than treated as dependencies that need to be watched for. > > I may be missing something, but aren't the entity and interface > registered together on the kernel side ? Even if the device node hasn't > been created by udev yet, the interface should be available, with a > major and minor, in the media graph. I don't think so...I think that the subdev's entity gets created by media_device_register_entity() from __v4l2_device_register_subdev() when that subdev is matched to a waiting notifier in v4l2_async_match_notify() but the interfaces not until media_devnode_create() from video_register_media_controller(), from __video_register_device() which is called for each subdevice in __v4l2_device_register_subdev_nodes() - which typically is delayed until a notifier's .complete() callback. Certainly MediaDevice::findInterface() doesn't find them, so the other option would be that the topology is missing the interface links that connect the entity and the interface but I think that that happens immediately after the above anyway. > >> The MediaDevice >> is added through DeviceEnumerator::addDevice() to be checked against the >> pipeline handler's match() functions. When the device nodes for the subdevices >> are eventually detected as v4l2-async completes they are simply placed onto a >> list of orphaned nodes and ignored. >> >> To remedy the problem this set checks for missing interfaces during >> MediaDevice::populate() and if it sees some are missing, allows the MediaDevice >> to be created successfully but leaves the isValid() flag false. >> DeviceEnumeratorUdev is updated to check isValid() and when it is false, defers >> the creation of MediaEntity / MediaPad / MediaLink objects. Each time >> udevNotify() is triggered by a new device MediaDevice::populate() is checked >> again for each deferred MediaDevice, and once the missing interfaces are found >> in the topology the other objects are created, dependencies resolved and the >> MediaDevice added as normal. >> >> There may be a better way of receiving notice from the kernel that the media >> graph is fully complete and ready to be parsed; at the moment we rely on the >> reported topology version number becoming stable across two iterations of a loop >> that checks MEDIA_IOC_G_TOPOLOGY, but that's not sufficient to avoid this issue. >> >> Thanks >> Dan >> >> Daniel Scally (3): >> libcamera: media_device: Expand return values for populateEntities() >> libcamera: device_enumerator_udev: Add initMediaDevice() >> libcamera: device_enumerator_udev: Defer invalid media devices >> >> .../internal/device_enumerator_udev.h | 3 + >> include/libcamera/internal/media_device.h | 2 +- >> src/libcamera/device_enumerator.cpp | 2 +- >> src/libcamera/device_enumerator_udev.cpp | 85 ++++++++++++++----- >> src/libcamera/media_device.cpp | 27 +++--- >> 5 files changed, 85 insertions(+), 34 deletions(-) >
On Wed, Aug 20, 2025 at 10:30:19PM +0100, Daniel Scally wrote: > On 20/08/2025 20:44, Laurent Pinchart wrote: > > On Wed, Aug 20, 2025 at 02:23:13PM +0100, Daniel Scally wrote: > >> Hello all > >> > >> At present, a CameraManager that is created before the device nodes for subdev > >> entities in a media graph have been created (for example, because some subdev > >> drivers have not yet probed and so the v4l2-async framework hasn't called for > >> them to be built yet) will never match any pipeline handlers for that media > >> device. This is because the MediaDevice populates itself with entities, > >> interfaces, pads and links but ignores the fact that the interfaces for the > >> subdev entities are missing. When DeviceEnumeratorUdev:: populateMediaDevice() > >> is called, the entity's major and minor numbers are checked, seen to be zero (as > >> they come from the missing interfaces) and then the entity is simply ignored > >> rather than treated as dependencies that need to be watched for. > > > > I may be missing something, but aren't the entity and interface > > registered together on the kernel side ? Even if the device node hasn't > > been created by udev yet, the interface should be available, with a > > major and minor, in the media graph. > > I don't think so...I think that the subdev's entity gets created by > media_device_register_entity() from __v4l2_device_register_subdev() > when that subdev is matched to a waiting notifier in > v4l2_async_match_notify() but the interfaces not until > media_devnode_create() from video_register_media_controller(), from > __video_register_device() which is called for each subdevice in > __v4l2_device_register_subdev_nodes() - which typically is delayed > until a notifier's .complete() callback. You are right, thanks for the explanation. > Certainly MediaDevice::findInterface() doesn't find them, so the other > option would be that the topology is missing the interface links that > connect the entity and the interface but I think that that happens > immediately after the above anyway. Let's discuss this in the context of patch 1/3 to avoid splitting the discussion. > >> The MediaDevice > >> is added through DeviceEnumerator::addDevice() to be checked against the > >> pipeline handler's match() functions. When the device nodes for the subdevices > >> are eventually detected as v4l2-async completes they are simply placed onto a > >> list of orphaned nodes and ignored. > >> > >> To remedy the problem this set checks for missing interfaces during > >> MediaDevice::populate() and if it sees some are missing, allows the MediaDevice > >> to be created successfully but leaves the isValid() flag false. > >> DeviceEnumeratorUdev is updated to check isValid() and when it is false, defers > >> the creation of MediaEntity / MediaPad / MediaLink objects. Each time > >> udevNotify() is triggered by a new device MediaDevice::populate() is checked > >> again for each deferred MediaDevice, and once the missing interfaces are found > >> in the topology the other objects are created, dependencies resolved and the > >> MediaDevice added as normal. > >> > >> There may be a better way of receiving notice from the kernel that the media > >> graph is fully complete and ready to be parsed; at the moment we rely on the > >> reported topology version number becoming stable across two iterations of a loop > >> that checks MEDIA_IOC_G_TOPOLOGY, but that's not sufficient to avoid this issue. > >> > >> Thanks > >> Dan > >> > >> Daniel Scally (3): > >> libcamera: media_device: Expand return values for populateEntities() > >> libcamera: device_enumerator_udev: Add initMediaDevice() > >> libcamera: device_enumerator_udev: Defer invalid media devices > >> > >> .../internal/device_enumerator_udev.h | 3 + > >> include/libcamera/internal/media_device.h | 2 +- > >> src/libcamera/device_enumerator.cpp | 2 +- > >> src/libcamera/device_enumerator_udev.cpp | 85 ++++++++++++++----- > >> src/libcamera/media_device.cpp | 27 +++--- > >> 5 files changed, 85 insertions(+), 34 deletions(-)