KhronosGroup / EGL-Registry

EGL API and Extension Registry
115 stars 105 forks source link

extensions: Add EGL_EXT_device_drm_render_node #127

Closed cubanismo closed 3 years ago

cubanismo commented 3 years ago

From the spec overview:

The EGL_EXT_device_drm extension provided a method for applications to query the primary DRM device node file associated with a given EGLDeviceEXT object, but many cards also or exclusively have an associated render node DRM device node. This extension enables querying the render node file path of an EGLDeviceEXT object.

emersion commented 3 years ago

Note that EGL_DRM_DEVICE_FILE_EXT doesn't specify whether the returned node should be primary or render. When only a render node is available, Mesa will return it for EGL_DRM_DEVICE_FILE_EXT.

We were contemplating the idea of always returning the render node for EGL_DRM_DEVICE_FILE_EXT if we have one, and only fallback to primary nodes if we don't have one (there's always the special case of split display/render SoCs that Mesa doesn't handle well). It seems that would be incompatible with this spec? I don't mind updating Mesa. cc @fooishbar

Maybe the overview could mention that the previous extension was too unclear?

In any case, I think it's always good to have a more well-defined attribute!

emersion commented 3 years ago

BTW, is it clear enough with this proposed wording that it's invalid to return a primary node for EGL_DRM_RENDER_NODE_FILE_EXT, and it's invalid to return a render node for EGL_DRM_DEVICE_FILE_EXT? ie, implementations MUST return NULL if the correct node type is unavailable.

cubanismo commented 3 years ago

I was afraid of that, but had a backup plan. In that case, would it make sense to update this spec to say that when this extension is present EGL_DRM_DEVICE_FILE_EXT returns a primary node, but when it is not present, EGL_DRM_DEVICE_FILE_EXT returns either a primary or render node, but which is undefined?

cubanismo commented 3 years ago

And yes, I could update it to say when this extension and EGL_EXT_device_drm are present, EGL_DRM_DEVICE_FILE_EXT returns NULL if no primary node is available, same for EGL_DRM_RENDER_NODE_FILE_EXT regardless of whether EGL_EXT_device_drm is present.

emersion commented 3 years ago

Looks like a good idea to me!

emersion commented 3 years ago

Hm, the only tricky case is that implementing this will regress EGL clients that aren't aware of this extension for devices that don't have a primary node. They previously got a render node via EGL_DRM_DEVICE_FILE_EXT, but with this ext would get NULL. I'm not sure we should care.

stonesthrow commented 3 years ago

If you have predictable and backward supportable behavior for none, one or both extensions - that is best, of course.

fooishbar commented 3 years ago

I was afraid of that, but had a backup plan. In that case, would it make sense to update this spec to say that when this extension is present EGL_DRM_DEVICE_FILE_EXT returns a primary node, but when it is not present, EGL_DRM_DEVICE_FILE_EXT returns either a primary or render node, but which is undefined?

A primary node is always present. It may not be accessible by the client, but stat() will at least reveal something about it.

I think these are good semantics though. FILE_EXT being primary is compatible with all the shipping implementations I know today, RENDER_NODE_EXT allows userspace to be confident in the correct result for heterogeneous hardware (assuming that @emersion's text specifies that the node returned is the actual GPU used for acceleration, I haven't checked because I'm in the middle of making dinner), and llvmpipe+KMS can return a positive result for FILE_EXT (the KMS node you're targeting) and nothing for RENDER_NODE_EXT (there is none).

cubanismo commented 3 years ago

Yes, it's not quite ideal, but if Mesa devs are OK with it, I wouldn't lose sleep over apps out there in the wild relying on EGL_DRM_DEVICE_FILE_EXT returning a render node in some case where a primary isn't available for a particular device. On the NV driver, these apps would fail already, so they're relying on poorly-defined spec language (my bad) & vendor-specific implementation details.

fooishbar commented 3 years ago

The spec isn't perfect, and neither is Mesa's implementation. We're all adults, we can deal with it. :)

emersion commented 3 years ago

Yeah, the Mesa implementation can be… surprising. On a split render/display SoC, listing devices will only return the render-only device, but if you create an EGLDisplay for KMS purposes and then get the device for this display, a new EGLDevice will magically appear, will advertise the display-only DRM device via FILE_EXT, but will actually use the render-only device under the hood.

Anyways, your proposal still sounds good to me.

cubanismo commented 3 years ago

Thanks. Wording updated.

cubanismo commented 3 years ago

@stonesthrow, I think this is ready to merge now, absent any further feedback. Please let me know if anything else is needed, or pass it along to Jon if not.

stonesthrow commented 3 years ago

@oddhack - Please add this new EXT to the EGL repo