date:20180530

Re: [PATCH xserver 1/3] xwayland: Add hook to check wl interface for glamor

2018-05-30 Thread Lyude Paul

On Wed, 2018-05-30 at 11:30 +0200, Olivier Fourdan wrote:
> Add an optional egl_backend callback to check that the required Wayland
> interfaces are available.
> 
> Signed-off-by: Olivier Fourdan 
> ---
>  hw/xwayland/xwayland-glamor.c | 9 +
>  hw/xwayland/xwayland.c| 4 
>  hw/xwayland/xwayland.h| 7 +++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
> index 6ae274c08..aeb0b27b9 100644
> --- a/hw/xwayland/xwayland-glamor.c
> +++ b/hw/xwayland/xwayland-glamor.c
> @@ -162,6 +162,15 @@ xwl_glamor_init_wl_registry(struct xwl_screen
> *xwl_screen,
>   interface, id, version);
>  }
>  
> +Bool
> +xwl_glamor_has_wl_interface(struct xwl_screen *xwl_screen)
> +{
> +if (xwl_screen->egl_backend.has_wl_interface)
> +return xwl_screen->egl_backend.has_wl_interface(xwl_screen);
> +
> +return TRUE;
> +}
> +
>  struct wl_buffer *
>  xwl_glamor_pixmap_get_wl_buffer(PixmapPtr pixmap,
>  unsigned short width,
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index 4dd8f3810..d831291ca 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -1091,6 +1091,10 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char
> **argv)
>  return FALSE;
>  
>  #ifdef XWL_HAS_GLAMOR
> +if (xwl_screen->glamor && !xwl_glamor_has_wl_interface(xwl_screen)) {
> +ErrorF("Missing Wayland interfaces required for glamor, falling
> back to sw\n");
> +xwl_screen->glamor = 0;
> +}
NAK. I'm starting to see the problems with this approach I originally hit now.

So: there's some unexpected catches when it comes to EGL client extensions. On
a system using glvnd with the nvidia driver, the EGL client extension list
will have /both/ the client extensions for the nvidia EGL driver and the
client extensions for the Mesa driver. An example from my test machine:

EGL client extensions string:
EGL_EXT_platform_base EGL_EXT_device_base # ← nvidia!
EGL_KHR_client_get_all_proc_addresses EGL_EXT_client_extensions
EGL_KHR_debug EGL_EXT_platform_x11 EGL_EXT_platform_device
EGL_EXT_platform_wayland EGL_MESA_platform_gbm # ← mesa!
EGL_MESA_platform_surfaceless

This is fine, but what it means is that we're going to have to take the
presence of either of these extensions as a sign that their respective EGL
backends -might- be there. e.g. we can't try to make a choice on avoiding
trying EGLStreams simply because GBM might be there, which is currently what
this patch does:

 * During init, xwl_glamor_should_use_gbm() tells us we should prefer GBM over
   EGLStreams
 * We select the gbm egl backend's hook for checking for the wl interfaces
 * In xwl_screen_init(), we check for wl_drm and don't see it
 * xwl_glamor_has_wl_interface() returns FALSE, which makes us disable glamor
   and fall back to software rendering

I think the approach we're going to have to take instead is to use the EGL
extensions to determine which EGL backends we should try probing the wl
interfaces for. If -eglstream gets specified, we only try probing for EGL
streams, etc. etc.

Afterwards, if we have gbm we try probing for it's wl interfaces. If that
works, select gbm as the egl backend. If that doesn't work and we have
eglstreams, try probing for it's wl interfaces. If that works, use eglstreams
as the EGL backend. And finally, if all else fails default to sw.

iirc, this is basically what I did very early on when working on adding
eglstreams.
>  if (xwl_screen->glamor && !xwl_glamor_init(xwl_screen)) {
>  ErrorF("Failed to initialize glamor, falling back to sw\n");
>  xwl_screen->glamor = 0;
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index ff746114c..62cb40f08 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -120,6 +120,12 @@ struct xwl_screen {
>   const char *name, uint32_t id,
>   uint32_t version);
>  
> +/* Check that the required Wayland interfaces are available.
> + * This callback is optional.
> + */
> +Bool (*has_wl_interface)(struct xwl_screen *xwl_screen);
> +
> +
>  /* Called before glamor has been initialized. Backends should setup
> a
>   * valid, glamor compatible EGL context in this hook.
>   */
> @@ -429,6 +435,7 @@ void xwl_glamor_init_wl_registry(struct xwl_screen
> *xwl_screen,
>   struct wl_registry *registry,
>   uint32_t id, const char *interface,
>   uint32_t version);
> +Bool xwl_glamor_has_wl_interface(struct xwl_screen *xwl_screen);
>  void xwl_glamor_post_damage(struct xwl_window *xwl_window,
>  PixmapPtr pixmap, RegionPtr region);
>  Bool xwl_glamor_allow_commits(struct xwl_window *xwl_window

Re: [PATCH v2 xserver] Xwayland: Enable EGL backend automatically

2018-05-30 Thread Lyude Paul

Reviewed-by: Lyude Paul 

On Wed, 2018-05-30 at 11:19 +0200, Olivier Fourdan wrote:
> Check for "EGL_MESA_platform_gbm" in the avaiable EGL extensions, and
> try automatically EGL stream if not present.
> 
> The command line options “-eglstream” is kept for compatibility to force
> the use of the EGL streams backend.
> 
> Suggested-by: Ray Strode 
> Signed-off-by: Olivier Fourdan 
> ---
>  v2: Use epoxy_has_egl_extension() instead of strstr() which is error prone
>  as poitned out by Pekka.
> 
>  hw/xwayland/xwayland-glamor.c | 6 ++
>  hw/xwayland/xwayland.c| 2 +-
>  hw/xwayland/xwayland.h| 1 +
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
> index f543f321d..6ae274c08 100644
> --- a/hw/xwayland/xwayland-glamor.c
> +++ b/hw/xwayland/xwayland-glamor.c
> @@ -58,6 +58,12 @@ xwl_glamor_egl_supports_device_probing(void)
>  return epoxy_has_egl_extension(NULL, "EGL_EXT_device_base");
>  }
>  
> +Bool
> +xwl_glamor_should_use_gbm(void)
> +{
> +return epoxy_has_egl_extension(NULL, "EGL_MESA_platform_gbm");
> +}
> +
>  void **
>  xwl_glamor_egl_get_devices(int *num_devices)
>  {
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index a08d58451..4dd8f3810 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -939,7 +939,7 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char
> **argv)
>  struct xwl_screen *xwl_screen;
>  Pixel red_mask, blue_mask, green_mask;
>  int ret, bpc, green_bpc, i;
> -Bool use_eglstreams = FALSE;
> +Bool use_eglstreams = !xwl_glamor_should_use_gbm();
>  
>  xwl_screen = calloc(1, sizeof *xwl_screen);
>  if (xwl_screen == NULL)
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index 39bc20a7e..ff746114c 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -444,6 +444,7 @@ void xwl_screen_init_xdg_output(struct xwl_screen
> *xwl_screen);
>  
>  void xwl_glamor_egl_make_current(struct xwl_screen *xwl_screen);
>  Bool xwl_glamor_egl_supports_device_probing(void);
> +Bool xwl_glamor_should_use_gbm(void);
>  void **xwl_glamor_egl_get_devices(int *num_devices);
>  Bool xwl_glamor_egl_device_has_egl_extensions(void *device,
>const char **ext_list,
-- 
Cheers,
Lyude Paul
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 3/3] xwayland: Check required interfaces for EGLstreams

2018-05-30 Thread Olivier Fourdan

Use the newly added “has_wl_interface” hook to check availability of
“wl_eglstream_display” and “wl_eglstream_controller” interfaces prior to
enable glamor with EGL Streams backend.

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor-eglstream.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c 
b/hw/xwayland/xwayland-glamor-eglstream.c
index 8dd1cc304..fcf7a7149 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -581,6 +581,26 @@ xwl_glamor_eglstream_init_wl_registry(struct xwl_screen 
*xwl_screen,
 }
 }
 
+static Bool
+xwl_glamor_eglstream_has_wl_interface(struct xwl_screen *xwl_screen)
+{
+struct xwl_eglstream_private *xwl_eglstream =
+xwl_eglstream_get(xwl_screen);
+
+if (xwl_eglstream->display == NULL) {
+ErrorF("glamor: 'wl_eglstream_display' not supported\n");
+return FALSE;
+}
+
+if (xwl_eglstream->controller == NULL) {
+ErrorF("glamor: 'wl_eglstream_controller' not supported\n");
+return FALSE;
+}
+
+return TRUE;
+}
+
+
 static inline void
 xwl_eglstream_init_shaders(struct xwl_screen *xwl_screen)
 {
@@ -819,6 +839,7 @@ xwl_glamor_init_eglstream(struct xwl_screen *xwl_screen)
 
 xwl_screen->egl_backend.init_egl = xwl_glamor_eglstream_init_egl;
 xwl_screen->egl_backend.init_wl_registry = 
xwl_glamor_eglstream_init_wl_registry;
+xwl_screen->egl_backend.has_wl_interface = 
xwl_glamor_eglstream_has_wl_interface;
 xwl_screen->egl_backend.init_screen = xwl_glamor_eglstream_init_screen;
 xwl_screen->egl_backend.get_wl_buffer_for_pixmap = 
xwl_glamor_eglstream_get_wl_buffer_for_pixmap;
 xwl_screen->egl_backend.post_damage = xwl_glamor_eglstream_post_damage;
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/3] xwayland: Add hook to check wl interface for glamor

2018-05-30 Thread Olivier Fourdan

Add an optional egl_backend callback to check that the required Wayland
interfaces are available.

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor.c | 9 +
 hw/xwayland/xwayland.c| 4 
 hw/xwayland/xwayland.h| 7 +++
 3 files changed, 20 insertions(+)

diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index 6ae274c08..aeb0b27b9 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -162,6 +162,15 @@ xwl_glamor_init_wl_registry(struct xwl_screen *xwl_screen,
  interface, id, version);
 }
 
+Bool
+xwl_glamor_has_wl_interface(struct xwl_screen *xwl_screen)
+{
+if (xwl_screen->egl_backend.has_wl_interface)
+return xwl_screen->egl_backend.has_wl_interface(xwl_screen);
+
+return TRUE;
+}
+
 struct wl_buffer *
 xwl_glamor_pixmap_get_wl_buffer(PixmapPtr pixmap,
 unsigned short width,
diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 4dd8f3810..d831291ca 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -1091,6 +1091,10 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 return FALSE;
 
 #ifdef XWL_HAS_GLAMOR
+if (xwl_screen->glamor && !xwl_glamor_has_wl_interface(xwl_screen)) {
+ErrorF("Missing Wayland interfaces required for glamor, falling back 
to sw\n");
+xwl_screen->glamor = 0;
+}
 if (xwl_screen->glamor && !xwl_glamor_init(xwl_screen)) {
 ErrorF("Failed to initialize glamor, falling back to sw\n");
 xwl_screen->glamor = 0;
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index ff746114c..62cb40f08 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -120,6 +120,12 @@ struct xwl_screen {
  const char *name, uint32_t id,
  uint32_t version);
 
+/* Check that the required Wayland interfaces are available.
+ * This callback is optional.
+ */
+Bool (*has_wl_interface)(struct xwl_screen *xwl_screen);
+
+
 /* Called before glamor has been initialized. Backends should setup a
  * valid, glamor compatible EGL context in this hook.
  */
@@ -429,6 +435,7 @@ void xwl_glamor_init_wl_registry(struct xwl_screen 
*xwl_screen,
  struct wl_registry *registry,
  uint32_t id, const char *interface,
  uint32_t version);
+Bool xwl_glamor_has_wl_interface(struct xwl_screen *xwl_screen);
 void xwl_glamor_post_damage(struct xwl_window *xwl_window,
 PixmapPtr pixmap, RegionPtr region);
 Bool xwl_glamor_allow_commits(struct xwl_window *xwl_window);
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 2/3] xwayland: Check required interfaces for GBM backend

2018-05-30 Thread Olivier Fourdan

Use the newly added “has_wl_interface” hook to check availability of
“wl_drm” interface prior to enable glamor with GBM backend.

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor-gbm.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/hw/xwayland/xwayland-glamor-gbm.c 
b/hw/xwayland/xwayland-glamor-gbm.c
index 29325adac..0df24be20 100644
--- a/hw/xwayland/xwayland-glamor-gbm.c
+++ b/hw/xwayland/xwayland-glamor-gbm.c
@@ -746,6 +746,19 @@ xwl_glamor_gbm_init_wl_registry(struct xwl_screen 
*xwl_screen,
 xwl_screen_set_dmabuf_interface(xwl_screen, id, version);
 }
 
+static Bool
+xwl_glamor_gbm_has_wl_interface(struct xwl_screen *xwl_screen)
+{
+struct xwl_gbm_private *xwl_gbm = xwl_gbm_get(xwl_screen);
+
+if (xwl_gbm->drm == NULL) {
+ErrorF("glamor: 'wl_drm' not supported\n");
+return FALSE;
+}
+
+return TRUE;
+}
+
 static Bool
 xwl_glamor_gbm_init_egl(struct xwl_screen *xwl_screen)
 {
@@ -879,6 +892,7 @@ xwl_glamor_init_gbm(struct xwl_screen *xwl_screen)
   xwl_gbm);
 
 xwl_screen->egl_backend.init_wl_registry = xwl_glamor_gbm_init_wl_registry;
+xwl_screen->egl_backend.has_wl_interface = xwl_glamor_gbm_has_wl_interface;
 xwl_screen->egl_backend.init_egl = xwl_glamor_gbm_init_egl;
 xwl_screen->egl_backend.init_screen = xwl_glamor_gbm_init_screen;
 xwl_screen->egl_backend.get_wl_buffer_for_pixmap = 
xwl_glamor_gbm_get_wl_buffer_for_pixmap;
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] Xwayland: Enable EGL backend automatically

2018-05-30 Thread Olivier Fourdan

Hi Lyude,

On Tue, May 29, 2018 at 7:54 PM, Lyude Paul  wrote:

> On Mon, 2018-05-28 at 09:11 +0200, Olivier Fourdan wrote:
>
> [...]
> I agree that we need to check for the protocol availability of course, yet
> it doesn't mean we should ditch this patch.
>
> Imagine (I say imagine, I do not know if that's plausible, nor if it's
> even possible), some vendor pushing for EGL streams realizing they could as
> well support GBM in the future, we would end up with both being supported,
> in which case we should probably still prefer GBM if we were to chose
> automatically.
>
> So, in this scenario, checking for GBM availability to decide whether or
> not we should enable one or the other backend is still a good thing,
> independently of the protocol availability.
>
> Oh! I should have looked closer at the patch, sorry about that-I had
> assumed it was checking for the EGLStream interfaces in the way that
> halfline had mentioned.
>
> Anyway:
> Reviewed-by: Lyude Paul 
>
>
Thanks!

This patch *is* the way Halfline had mentioned, it's useful in the
(hypothetical) case where we would have *both* GBM and EGLstreams
available, we should prefer GBM if not requested explicitly EGL Streams
(from the command line).

Also, following Pekka (very good) point that we should not use strstr(), I
shall post an updated version of this patch (v3) using
epoxy_has_egl_extension() which does exactly what we want, the way we want
it. I am therefore not adding your R-b to that new iteration of that patch
since it's different.

But it's not sufficient, we also need to check for Wayland interfaces
availability which is another series that I shall post shortly.

Cheers,
Olivier
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH v2 xserver] Xwayland: Enable EGL backend automatically

2018-05-30 Thread Olivier Fourdan

Check for "EGL_MESA_platform_gbm" in the avaiable EGL extensions, and
try automatically EGL stream if not present.

The command line options “-eglstream” is kept for compatibility to force
the use of the EGL streams backend.

Suggested-by: Ray Strode 
Signed-off-by: Olivier Fourdan 
---
 v2: Use epoxy_has_egl_extension() instead of strstr() which is error prone
 as poitned out by Pekka.

 hw/xwayland/xwayland-glamor.c | 6 ++
 hw/xwayland/xwayland.c| 2 +-
 hw/xwayland/xwayland.h| 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index f543f321d..6ae274c08 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -58,6 +58,12 @@ xwl_glamor_egl_supports_device_probing(void)
 return epoxy_has_egl_extension(NULL, "EGL_EXT_device_base");
 }
 
+Bool
+xwl_glamor_should_use_gbm(void)
+{
+return epoxy_has_egl_extension(NULL, "EGL_MESA_platform_gbm");
+}
+
 void **
 xwl_glamor_egl_get_devices(int *num_devices)
 {
diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index a08d58451..4dd8f3810 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -939,7 +939,7 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 struct xwl_screen *xwl_screen;
 Pixel red_mask, blue_mask, green_mask;
 int ret, bpc, green_bpc, i;
-Bool use_eglstreams = FALSE;
+Bool use_eglstreams = !xwl_glamor_should_use_gbm();
 
 xwl_screen = calloc(1, sizeof *xwl_screen);
 if (xwl_screen == NULL)
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index 39bc20a7e..ff746114c 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -444,6 +444,7 @@ void xwl_screen_init_xdg_output(struct xwl_screen 
*xwl_screen);
 
 void xwl_glamor_egl_make_current(struct xwl_screen *xwl_screen);
 Bool xwl_glamor_egl_supports_device_probing(void);
+Bool xwl_glamor_should_use_gbm(void);
 void **xwl_glamor_egl_get_devices(int *num_devices);
 Bool xwl_glamor_egl_device_has_egl_extensions(void *device,
   const char **ext_list,
-- 
2.17.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/3] xwayland: Add hook to check wl interface for glamor

Re: [PATCH v2 xserver] Xwayland: Enable EGL backend automatically

[PATCH xserver 3/3] xwayland: Check required interfaces for EGLstreams

[PATCH xserver 1/3] xwayland: Add hook to check wl interface for glamor

[PATCH xserver 2/3] xwayland: Check required interfaces for GBM backend

Re: [PATCH xserver] Xwayland: Enable EGL backend automatically

[PATCH v2 xserver] Xwayland: Enable EGL backend automatically

7 matches

Site Navigation

Mail list logo

Footer information