On Thu, Nov 07, 2013 at 05:13:36PM +0100, Axel Davy wrote:
> This patch moves the code to open the graphic device in the Wayland backend,
> removes the authentication request when we are on a render-node,
> and has a few fixes.
>
> Signed-off-by: Axel Davy <axel.d...@ens.fr>
> ---
> src/egl/drivers/dri2/egl_dri2.h | 1 +
> src/egl/drivers/dri2/platform_wayland.c | 93
> ++++++++++++++++++++++-----------
> 2 files changed, 63 insertions(+), 31 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index c7d6484..350a626 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -133,6 +133,7 @@ struct dri2_egl_display
> int authenticated;
> int formats;
> uint32_t capabilities;
> + int enable_tiling;
> #endif
>
> int (*authenticate) (_EGLDisplay *disp, uint32_t id);
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index c0de16b..709df36 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -622,8 +622,8 @@ dri2_terminate(_EGLDriver *drv, _EGLDisplay *disp)
> _eglCleanupDisplay(disp);
>
> dri2_dpy->core->destroyScreen(dri2_dpy->dri_screen);
> - close(dri2_dpy->fd);
> dlclose(dri2_dpy->driver);
> + close(dri2_dpy->fd);
Why is this necessary? If it's a fix not a typo, it should be in its
own patch.
> free(dri2_dpy->driver_name);
> free(dri2_dpy->device_name);
> wl_drm_destroy(dri2_dpy->wl_drm);
> @@ -635,34 +635,28 @@ dri2_terminate(_EGLDriver *drv, _EGLDisplay *disp)
> return EGL_TRUE;
> }
>
> +static char
> +is_fd_render_node(int fd)
> +{
> + struct stat render;
> +
> + if (fstat(fd, &render))
> + return 0;
> +
> + if (!S_ISCHR(render.st_mode))
> + return 0;
> +
> + if (render.st_rdev & 0x80)
> + return 1;
> + return 0;
> +}
mesa (and in particular this file) uses 3 space indents, please follow
the convention.
> static void
> drm_handle_device(void *data, struct wl_drm *drm, const char *device)
> {
> struct dri2_egl_display *dri2_dpy = data;
> - drm_magic_t magic;
>
> dri2_dpy->device_name = strdup(device);
> - if (!dri2_dpy->device_name)
> - return;
> -
> -#ifdef O_CLOEXEC
> - dri2_dpy->fd = open(dri2_dpy->device_name, O_RDWR | O_CLOEXEC);
> - if (dri2_dpy->fd == -1 && errno == EINVAL)
> -#endif
> - {
> - dri2_dpy->fd = open(dri2_dpy->device_name, O_RDWR);
> - if (dri2_dpy->fd != -1)
> - fcntl(dri2_dpy->fd, F_SETFD, fcntl(dri2_dpy->fd, F_GETFD) |
> - FD_CLOEXEC);
> - }
> - if (dri2_dpy->fd == -1) {
> - _eglLog(_EGL_WARNING, "wayland-egl: could not open %s (%s)",
> - dri2_dpy->device_name, strerror(errno));
> - return;
> - }
> -
> - drmGetMagic(dri2_dpy->fd, &magic);
> - wl_drm_authenticate(dri2_dpy->wl_drm, magic);
> }
>
> static void
> @@ -738,7 +732,8 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay
> *disp)
> struct dri2_egl_display *dri2_dpy;
> const __DRIconfig *config;
> uint32_t types;
> - int i;
> + int i, is_render_node;
> + drm_magic_t magic;
> static const unsigned int argb_masks[4] =
> { 0xff0000, 0xff00, 0xff, 0xff000000 };
> static const unsigned int rgb_masks[4] = { 0xff0000, 0xff00, 0xff, 0 };
> @@ -778,9 +773,39 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay
> *disp)
> if (roundtrip(dri2_dpy) < 0 || dri2_dpy->wl_drm == NULL)
> goto cleanup_dpy;
>
> - if (roundtrip(dri2_dpy) < 0 || dri2_dpy->fd == -1)
> + if (roundtrip(dri2_dpy) < 0 || dri2_dpy->device_name == NULL)
> goto cleanup_drm;
>
> +#ifdef O_CLOEXEC
> + dri2_dpy->fd = open(dri2_dpy->device_name, O_RDWR | O_CLOEXEC);
> + if (dri2_dpy->fd == -1 && errno == EINVAL)
> +#endif
> + {
> + dri2_dpy->fd = open(dri2_dpy->device_name, O_RDWR);
> + if (dri2_dpy->fd != -1)
> + fcntl(dri2_dpy->fd, F_SETFD, fcntl(dri2_dpy->fd, F_GETFD) |
> + FD_CLOEXEC);
> + }
Let's start out with a patch to split this O_CLOEXEC helper out into
its own function, open_cloexec() or something. Having this compat
logic here makes the interesting code harder to follow and your 3/3
patch duplicates it.
> + if (dri2_dpy->fd == -1) {
> + _eglLog(_EGL_WARNING, "wayland-egl: could not open %s (%s)",
> + dri2_dpy->device_name, strerror(errno));
> + goto cleanup_drm;
> + }
> +
> + if (is_fd_render_node(dri2_dpy->fd))
I would like the compositor to still send the classic drm device in
the wl_drm.device event. The client can then use stat(2) to stat it
and defer the corresponding render node from that by adding 128 to the
minor. This way we don't break older mesa versions by sending them a
render node that they'll then fail to authenticate.
> + {
The open brace '{' goes on the same line as the if statement, like
everywhere else in this file.
> + _eglLog(_EGL_DEBUG, "wayland-egl: card is render-node");
> + dri2_dpy->authenticated = 1;
> + is_render_node = 1;
> + }
> + else
> + {
The closing brace '}', else and the open brace '{' all goes on the same line:
} else {
> + drmGetMagic(dri2_dpy->fd, &magic);
> + wl_drm_authenticate(dri2_dpy->wl_drm, magic);
> + is_render_node = 0;
> + }
> + dri2_dpy->enable_tiling = 1;
> +
> if (roundtrip(dri2_dpy) < 0 || !dri2_dpy->authenticated)
> goto cleanup_fd;
>
> @@ -799,7 +824,7 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay
> *disp)
> dri2_dpy->dri2_loader_extension.flushFrontBuffer =
> dri2_flush_front_buffer;
> dri2_dpy->dri2_loader_extension.getBuffersWithFormat =
> dri2_get_buffers_with_format;
> -
> +
> dri2_dpy->extensions[0] = &dri2_dpy->dri2_loader_extension.base;
> dri2_dpy->extensions[1] = &image_lookup_extension.base;
> dri2_dpy->extensions[2] = &use_invalidate.base;
> @@ -808,14 +833,16 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay
> *disp)
> if (!dri2_create_screen(disp))
> goto cleanup_driver;
>
> - /* The server shouldn't advertise WL_DRM_CAPABILITY_PRIME if the driver
> - * doesn't have createImageFromFds, since we're using the same driver on
> - * both sides. We don't want crash if that happens anyway, so fall back
> to
> - * gem names if we don't have prime support. */
This comment still holds, no need to delete it.
> + /* Render-nodes need to be able to export prime fd,
> + * since they are not allowed to use GEM names.*/
>
> if (dri2_dpy->image->base.version < 7 ||
> dri2_dpy->image->createImageFromFds == NULL)
> - dri2_dpy->capabilities &= WL_DRM_CAPABILITY_PRIME;
> + {
(brace on the same line as if again)
> + dri2_dpy->capabilities &= ~WL_DRM_CAPABILITY_PRIME;
> + if (is_render_node)
> + goto cleanup_screen;
> + }
We need to move this check up before we try to get a render node from
/dev/dri/card0 above, and just not do that if we don't have version 7.
>
> types = EGL_WINDOW_BIT;
> for (i = 0; dri2_dpy->driver_configs[i]; i++) {
> @@ -840,6 +867,8 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay
> *disp)
>
> return EGL_TRUE;
>
> + cleanup_screen:
> + dri2_dpy->core->destroyScreen(dri2_dpy->dri_screen);
> cleanup_driver:
> dlclose(dri2_dpy->driver);
> cleanup_driver_name:
> @@ -849,6 +878,8 @@ dri2_initialize_wayland(_EGLDriver *drv, _EGLDisplay
> *disp)
> cleanup_drm:
> free(dri2_dpy->device_name);
> wl_drm_destroy(dri2_dpy->wl_drm);
> + if (dri2_dpy->own_device)
> + wl_display_disconnect(dri2_dpy->wl_dpy);
> cleanup_dpy:
> free(dri2_dpy);
>
> --
> 1.8.1.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
