[Mesa-dev] [PATCH] egl/x11_dri3: stop restricting support to render nodes unnecessarily

[Emil Velikov]

On 10 June 2016 at 12:25, Martin Peres <martin.peres at linux.intel.com> wrote:
> On 10/06/16 13:30, Frank Binns wrote:
>>
>> In order to support the WL_bind_wayland_display extension it's necessary
>> to
>> make use of render nodes as DRI3 doesn't provide a mechanism for
>> authenticating
>> client opened fds. However, this does not apply when Wayland support isn't
>> enabled.
>
>
> Out of curiosity, why don't you want to use render nodes? Are you using a
> very old kernel?
>
>>
>> Signed-off-by: Frank Binns <frank.binns at imgtec.com>
>> ---
>>  src/egl/drivers/dri2/platform_x11_dri3.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c
>> b/src/egl/drivers/dri2/platform_x11_dri3.c
>> index 9363a8a..80aac01 100644
>> --- a/src/egl/drivers/dri2/platform_x11_dri3.c
>> +++ b/src/egl/drivers/dri2/platform_x11_dri3.c
>> @@ -442,6 +442,7 @@ dri3_get_device_name(int fd)
>>  {
>>     char *ret = NULL;
>>
>> +#ifdef HAVE_WAYLAND_PLATFORM
>>     ret = drmGetRenderDeviceNameFromFd(fd);
>>     if (ret)
>>        return ret;
>
>
> I would rather move the #ifdef HAVE_WAYLAND_PLATFORM to after the comment
> about WL_bind_wayland_display. This way, we keep the same logic of first
> asking libdrm, and then asking our internal function.
>
>> @@ -455,6 +456,7 @@ dri3_get_device_name(int fd)
>>        _eglLog(_EGL_WARNING, "Render node support not available, falling
>> back to dri2");
>>        _eglLog(_EGL_WARNING, "If you want to force dri3, set
>> EGL_FORCE_DRI3 environment variable");
>
>
> Maybe reworking this error message to add that if, as a user, you do not
> care about wayland, you can compile it out and the error will go away.
>
>>     } else
>> +#endif
>>        ret = loader_get_device_name_for_fd(fd);
>>
>>     return ret;
>>
>
> Now, did you actually test this patch? If you were using normal nodes and
> not render nodes, I wonder how you managed to get any context since no
> authentication is performed :s
>
Initially, that's what I was thinking as well. Then again things seems
more convoluted than needed imho. device_name is used/needed:
 - wayland-drm.la static module (protocol) used by EGL, GBM and
Vulkan. Passed into wayland_drm_init(), it's used solely for
wl_resource_post_event(resource, WL_DRM_DEVICE, drm->device_name)
 - internally in platform_wayland/platform_x11 (dri2) to open the
device and get its FD.

Assuming there are no dependencies the wayland-{client,server}.so
side, one could would just {re}move all device_name retrieval/handling
where needed. Although I'm think that one might need to pimp the
wayland-drm protocol first.

That's my humble take on it. I take it Frank might have move insight
on the topic.

-Emil