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

[Frank Binns]

On 10/06/16 14:32, Emil Velikov wrote:
> 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
I think you've pretty much summed it all up :)

Thanks
Frank