-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EXT_device_enumeration: unclear how hot unplug works #120
Comments
@jjulianoatnv may be interested here as well, as similar questions have been raised regarding VkPhysicalDevice objects in Vulkan, and we should probably resolve both issues in a compatible way. |
Another way of looking at the problem is:
You could invalidate an EGLDeviceEXT handle, as long as you never re-use that handle for a different device. If you allow re-using a handle, then applications would have to deal with a valid EGLDeviceEXT handle suddenly pointing to a different device, possibly between successive EGL calls. For functions which take an EGLDeviceEXT handle, just returning an error code (probably EGL_BAD_DEVICE_EXT) could make sense. But, I don't know what should happen with an EGLDisplay that uses that device, especially if that EGLDisplay owns the current context. |
After enumerating the list of devices, if an old device isn't advertised anymore, the old device is no longer valid.
Out-of-scope. On Linux, udev can be used to monitor new devices appearing/disappearing.
It doesn't seem like this is related to EGLDevice. These are relevant I think: |
We'd still need to define what happens with EGL functions if a device was removed after the last call to eglQueryDevicesEXT. We also don't want applications to be constantly polling eglQueryDevicesEXT, and it wouldn't be sufficient even if they did, since an unplug is still asynchronous. So, there needs to be some way for an application to know that it needs to call eglQueryDevicesEXT.
That's fair. Adding a new device doesn't affect what an application is doing with an existing device, so some asynchronous notification should be fine, and that notification is necessarily OS-specific.
It's part of the same problem -- a EGLDisplay is still a top-level EGL object associated with a device. If that device becomes unusable, then the EGLDisplay also does, just like an EGLDeviceEXT. |
Yeah, but I don't think EGL should be involved. Clients will likely want to integrate hotplug detection into their event loop, and that's too system-specific for EGL to handle. Just ask users to use platform-specific APIs such as udev. If you really think EGL should be involved, I'd suggest working on a separate extension, and not block this issue because of it.
Yes, but it's not specific to the device platform. It may happen when the EGLDisplay was created for another platform, like X11/GBM/Wayland/surfaceless. The driver will pick a physical device under-the-hood, which may disappear without the client noticing. |
For adding a new device, we don't need EGL to notify the application. An application can carry on using whatever device it was using and still work just fine. If an application cares about new devices, then it can implement whatever OS-specific hotplug detection it needs, and then call eglQueryDevicesEXT again as necessary. I'll need to update libglvnd to deal with added devices, of course, but that'll be easy enough. To deal with removing devices, I think we are going to want a new extension. If nothing else, an extension would be a good way to define the behavior that an application should expect. To provide a clean way for applications to cope with device removal, an extension could also define a new error code and/or a new query to distinguish between "that device handle is invalid" and "that device handle is valid, but the device disappeared when you weren't looking." Anyway, this is what I was thinking for removal behavior. It's a rough sketch right now, but I can write this up a more formal spec if it sounds reasonable. For EGLDeviceEXT handles:
For EGLDisplays:
That last point is important for libglvnd. When an application calls eglGetPlatformDisplay, more than one vendor might be able to work with the native display, and libglvnd will just pick the first vendor that returns a non-NULL EGLDisplay handle. If the device behind that display disappears, then that first vendor might not have another device it can use, in which case eglInitialize will fail. But, when the application calls eglGetPlatformDisplay, then a different vendor could pick up the display instead, in which case it would return a new EGLDisplay handle. |
Looks like a good idea.
Hm. Something like an
What if the same device is re-connected but on a different port? Re-using handles between hotplugs sounds like a footgun TBH. What's the motivation? |
Something like that, yeah. It wouldn't strictly be needed, since you could just call eglQueryDevicesEXT and look for the handle. Using a specific device query might be easier or faster, though.
Emphasis on "can" -- a driver would be allowed, not required, to re-use a handle for the same device. The inverse is the critical part: A driver must not re-use the same handle for a different device, because if it did, then an application would suddenly be working with a different device between two EGL calls without any way to realize that anything changed. If a driver re-uses a handle for the same device, then you'ref fine: An application using that handle would continue to use the same device just like it expects. |
How should a driver handle device unplug when it supports EXT_device_enumeration?
This issue stems from https://gitlab.freedesktop.org/glvnd/libglvnd/-/issues/215. We need to be careful to pick behavior that is compatible with a vendor-neutral EGL loader such as glvnd.
For instance, it's not safe for a driver to invalidate an unplugged EGLDevice after the EGL client calls
eglQueryDevicesEXT
, because that would allow another driver to re-use the exact same handle and prevent the EGL client from figuring out the EGLDevice is gone or has changed. Or maybe glvnd should wrap vendor EGLDevices to prevent this?cc @cubanismo @kbrenneman
The text was updated successfully, but these errors were encountered: