[0/3] Fix fast-booting device enumeration problems
mbox series

Message ID 20250820132316.1033443-1-dan.scally@ideasonboard.com
Headers show
Series
  • Fix fast-booting device enumeration problems
Related show

Message

Dan Scally Aug. 20, 2025, 1:23 p.m. UTC
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. 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(-)

Comments

Laurent Pinchart Aug. 20, 2025, 7:44 p.m. UTC | #1
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(-)
Dan Scally Aug. 20, 2025, 9:30 p.m. UTC | #2
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(-)
>
Laurent Pinchart Aug. 21, 2025, 10:27 a.m. UTC | #3
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(-)