Re: Obtaining Xorg DDX commit privilege

2018-06-05 Thread Kevin Brace
Hi Connor,

Since the development of r128 DDX has stalled for about two years, I was 
looking to obtain xf86-video-* DDX commit privilege so that I can add KMS 
support to r128 DDX.
Although some additional work remain (function renaming and removal of unused 
code), I have gotten to the point where unaccelerated OpenChrome DDX with KMS 
via OpenChrome DRM I feel is good enough for Linux kernel mainline tree 
inclusion even without living in the staging tree for a while.
I have managed 3 releases of OpenChrome DDX in the past 2 years with its 
reliability progressively improving with each new release.
Hence, I now have a little more time to invest in underserved graphics device 
drivers other than OpenChrome.
I did ask freedesktop.org to grant me commit privilege to xf86-video-* DDX 
repositories, but I have yet to hear from them.

https://bugs.freedesktop.org/show_bug.cgi?id=106605

Regarding patch #5, it is based on the observation that starting X Server with 
probing option does not really do anything inside PreInit recall back, hence, 
there is no need to probe the monitor via VBE.
Furthermore, it is intentionally made to not do anything if the user is using 
PowerPC (i.e., PowerMac), so I figured, why not do anything even if we are 
using an x86 processor?
In general, I am looking to reduce the calls to VBE, and I think r128 should 
ultimately only handle DDX UMS and DRM KMS at most.
In other words, VBE and FBDEV should not be used.

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Tuesday, June 05, 2018 at 8:16 PM
> From: "Connor Behan" 
> To: "Michel Dänzer" , "Kevin Brace" 
> Cc: xorg-devel@lists.x.org
> Subject: Re: Obtaining Xorg DDX commit privilege
>
> On 2018-06-05 05:33 AM, Michel Dänzer wrote:
> > On 2018-06-05 05:00 AM, Kevin Brace wrote:
> >> I do have OpenChrome related commit privilege and I wanted to extend it to 
> >> other xf86-video-* DDXs.
> >> I did post several patches over at xorg-driver-ati mailing list if that is 
> >> what is needed to earn this privilege.
> > Connor Behan has been looking after xf86-video-r128.
> 
> Thanks. I've been meaning to test your patches Kevin. The only one I'm
> nervous about so far is #5 which removes DDC.
> 
>
___
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 00/10] Re: Refactor egl_backends for wayland registry

2018-06-05 Thread Lyude Paul
Sweet! Everything looks good to me

For the whole series:

Reviewed-by: Lyude Paul 

On Tue, 2018-06-05 at 19:38 +0200, Olivier Fourdan wrote:
> Hi,
> 
> This is a follow-up on https://patchwork.freedesktop.org/series/44095/
> after Lyude and Emil reviews.
> 
> Cheers,
> Olivier
> 
> Olivier Fourdan (10):
>   xwayland: move glamor specific routines
>   xwayland: swap "name" and "id" in init_wl_registry()
>   xwayland: GBM should fail w/out "GL_OES_EGL_image"
>   xwayland: skip drm authentication with render node
>   xwayland: move egl_backend to its own struct
>   xwayland: Add Wayland interfaces check
>   xwayland: move EGL backend init to glamor
>   xwayland: refactor EGL backends for wayland registry
>   xwayland: check for EGLStream backend explicitly
>   xwayland: EGL_IMG_context_priority required by EGLStream
> 
>  hw/xwayland/xwayland-glamor-eglstream.c | 152 +++
>  hw/xwayland/xwayland-glamor-gbm.c   |  69 +++--
>  hw/xwayland/xwayland-glamor.c   | 186 
>  hw/xwayland/xwayland-present.c  |   5 +-
>  hw/xwayland/xwayland.c  |  28 ++--
>  hw/xwayland/xwayland.h  | 151 ++-
>  6 files changed, 369 insertions(+), 222 deletions(-)
> 
-- 
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

Re: [PATCH xserver 10/10] xwayland: EGL_IMG_context_priority required by EGLStream

2018-06-05 Thread Emil Velikov
On 5 June 2018 at 18:38, Olivier Fourdan  wrote:
> xwl_glamor_eglstream_init_egl() uses "EGL_IMG_context_priority"
> extension, make sure it's actually available before using it.
>
> Suggested-by: Emil Velikov 
> Signed-off-by: Olivier Fourdan 
> ---
>  hw/xwayland/xwayland-glamor-eglstream.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/xwayland/xwayland-glamor-eglstream.c 
> b/hw/xwayland/xwayland-glamor-eglstream.c
> index c226c0089..43f34eed1 100644
> --- a/hw/xwayland/xwayland-glamor-eglstream.c
> +++ b/hw/xwayland/xwayland-glamor-eglstream.c
> @@ -794,6 +794,12 @@ xwl_glamor_eglstream_init_egl(struct xwl_screen 
> *xwl_screen)
>  goto error;
>  }
>
> +if (!epoxy_has_egl_extension(xwl_screen->egl_display,
> + "EGL_IMG_context_priority")) {
> +ErrorF("EGL_IMG_context_priority not available\n");
> +goto error;
> +}
> +
Another approach is to adjust the array to omit the attributes.
Either way, the series is:

Reviewed-by: Emil Velikov 

Thanks for going through my nit-picky suggestions.
Emil
___
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 5/5] xwayland: refactor egl_backends for wayland registry

2018-06-05 Thread Emil Velikov
On 5 June 2018 at 18:28, Olivier Fourdan  wrote:
> Hi Emil,
>
> Many thanks for your detailed review!
>
> On Tue, Jun 5, 2018 at 12:37 PM, Emil Velikov  
> wrote:
>> Hi Olivier,
>>
>> There's a handful of mostly trivial suggestions below. The idea itself seems
>> reasonable IMHO. One gripe is that we're 'leaking' twice as much as before.
>>
>> Namely: even if the current backend cleans-up after itself (it some cases it
>> does not), the other backend 'leaks'. Not sure if/how much we should care in
>> that case.
>>
>> Was skimming if we cannot move more init_egl/init_screen tripping points
>> and I've noticed a few gotchas/bugs. None of which are requirement for the
>> series, although great to have.
>>
>>
>> xwl_glamor_gbm_init_egl
>>  - if !render_node error path is leaking
>
> humm, not sure, leaking what? we haven't allocated anything yet, have we?
> But anyhow, it's unralted to this refactoring I think.
>
You are right, this and the other bits listed just below are preexisting.
Thanks for tackling them.

The following are set during init_registry (wl_drm path) and
effectively leaked. Then again, there's plenty of small bits like that
throughout the code base.
I would _not_ bother with them bth.

::device_name -> strdup(device/node name)
::drm_fd -> open(^^)
::drm -> wl_registry_bind


>>> @@ -119,8 +134,8 @@ xwl_glamor_allow_commits(struct xwl_window *xwl_window)
>>>  {
>>>  struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
>>>
>>> -if (xwl_screen->egl_backend.allow_commits)
>>> -return xwl_screen->egl_backend.allow_commits(xwl_window);
>>> +if (xwl_screen->egl_backend && xwl_screen->egl_backend->allow_commits)
>> Why the NULL check? Unless I'm missing something that will never happen.
>
> We can have no “egl_backend” set at all if “-shm” was specified.
>
> Either we keep that test here or we need to check for
> "xwl_screen->glamor" in xwl_screen_post_damage(), I'll change for the
> later for clarity.
>
Ah, yes. Thanks.

-Emil
___
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: Obtaining Xorg DDX commit privilege

2018-06-05 Thread Connor Behan
On 2018-06-05 05:33 AM, Michel Dänzer wrote:
> On 2018-06-05 05:00 AM, Kevin Brace wrote:
>> I do have OpenChrome related commit privilege and I wanted to extend it to 
>> other xf86-video-* DDXs.
>> I did post several patches over at xorg-driver-ati mailing list if that is 
>> what is needed to earn this privilege.
> Connor Behan has been looking after xf86-video-r128.

Thanks. I've been meaning to test your patches Kevin. The only one I'm
nervous about so far is #5 which removes DDC.



signature.asc
Description: OpenPGP digital signature
___
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 09/10] xwayland: check for EGLStream backend explicitly

2018-06-05 Thread Olivier Fourdan
Now that we have separate backends for EGLStream and GBM, we can
explicitly check for the EGLStream backend to disable present support
in that case.

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-present.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index 9e9e78917..4db0d1efc 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -547,10 +547,9 @@ xwl_present_init(ScreenPtr screen)
 struct xwl_screen *xwl_screen = xwl_screen_get(screen);
 
 /*
- * doesn't work with the streams backend. we don't have an explicit
- * boolean for that, but we do know gbm doesn't fill in this hook...
+ * doesn't work with the EGLStream backend.
  */
-if (xwl_screen->egl_backend->post_damage != NULL)
+if (xwl_screen->egl_backend == _screen->eglstream_backend)
 return FALSE;
 
 if (!dixRegisterPrivateKey(_present_window_private_key, 
PRIVATE_WINDOW, 0))
-- 
2.17.1

___
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 10/10] xwayland: EGL_IMG_context_priority required by EGLStream

2018-06-05 Thread Olivier Fourdan
xwl_glamor_eglstream_init_egl() uses "EGL_IMG_context_priority"
extension, make sure it's actually available before using it.

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

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c 
b/hw/xwayland/xwayland-glamor-eglstream.c
index c226c0089..43f34eed1 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -794,6 +794,12 @@ xwl_glamor_eglstream_init_egl(struct xwl_screen 
*xwl_screen)
 goto error;
 }
 
+if (!epoxy_has_egl_extension(xwl_screen->egl_display,
+ "EGL_IMG_context_priority")) {
+ErrorF("EGL_IMG_context_priority not available\n");
+goto error;
+}
+
 eglChooseConfig(xwl_screen->egl_display, config_attribs, , 1, );
 if (!n) {
 ErrorF("No acceptable EGL configs found\n");
-- 
2.17.1

___
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 06/10] xwayland: Add Wayland interfaces check

2018-06-05 Thread Olivier Fourdan
Introduces a new egl_backend function to let the EGL backend check for
the presence of the required Wayland interfaces.

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor-eglstream.c | 28 ++---
 hw/xwayland/xwayland-glamor-gbm.c   | 14 +
 hw/xwayland/xwayland-glamor.c   | 11 ++
 hw/xwayland/xwayland.h  |  7 +++
 4 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c 
b/hw/xwayland/xwayland-glamor-eglstream.c
index f3fd97e6e..bf74ae329 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -660,6 +660,25 @@ xwl_glamor_eglstream_init_wl_registry(struct xwl_screen 
*xwl_screen,
 }
 }
 
+static Bool
+xwl_glamor_eglstream_has_wl_interfaces(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,14 +838,6 @@ xwl_glamor_eglstream_init_screen(struct xwl_screen 
*xwl_screen)
 xwl_eglstream_get(xwl_screen);
 ScreenPtr screen = xwl_screen->screen;
 
-if (!xwl_eglstream->controller) {
-ErrorF("No eglstream controller was exposed in the wayland registry. "
-   "This means your version of nvidia's EGL wayland libraries "
-   "are too old, as we require support for this.\n");
-xwl_eglstream_cleanup(xwl_screen);
-return FALSE;
-}
-
 /* We can just let glamor handle CreatePixmap */
 screen->DestroyPixmap = xwl_glamor_eglstream_destroy_pixmap;
 
@@ -898,6 +909,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_interfaces = 
xwl_glamor_eglstream_has_wl_interfaces;
 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;
diff --git a/hw/xwayland/xwayland-glamor-gbm.c 
b/hw/xwayland/xwayland-glamor-gbm.c
index b03e7ee17..33a576da2 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_interfaces(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)
 {
@@ -887,6 +900,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_interfaces = 
xwl_glamor_gbm_has_wl_interfaces;
 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;
diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index 14706f6e8..72995de00 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -76,6 +76,17 @@ xwl_glamor_init_wl_registry(struct xwl_screen *xwl_screen,
  id, interface, version);
 }
 
+Bool
+xwl_glamor_has_wl_interfaces(struct xwl_screen *xwl_screen,
+struct xwl_egl_backend *xwl_egl_backend)
+{
+if (xwl_egl_backend->has_wl_interfaces)
+return xwl_egl_backend->has_wl_interfaces(xwl_screen);
+
+/* If the backend has no requirement wrt WL interfaces, we're fine */
+return TRUE;
+}
+
 struct wl_buffer *
 xwl_glamor_pixmap_get_wl_buffer(PixmapPtr pixmap,
 unsigned short width,
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index 6bbe72e46..191f1b297 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -69,6 +69,11 @@ struct xwl_egl_backend {
  uint32_t id, const char *name,
  uint32_t version);
 
+/* Check that the required Wayland interfaces are available. This
+ * callback 

[PATCH xserver 08/10] xwayland: refactor EGL backends for wayland registry

2018-06-05 Thread Olivier Fourdan
To be able to check for availability of the Wayland interfaces required
to run a given EGL backend (either GBM or EGLStream for now), we need
to have each backend structures and vfuncs in place before we enter the
Wayland registry dance.

That basically means that we should init all backends at first, connect
to the Wayland compositor and query the available interfaces and then
decide which backend is available and should be used (or none if either
the Wayland interfaces or the EGL extensions are not available).

For this purpose, hold an egl_backend struct for each backend we are to
consider prior to connect to the Wayland display so that, when we get to
query the Wayland interfaces, everything is in place for each backend to
handle the various Wayland interfaces.

Eventually, when we need to chose which EGL backend to use for glamor,
the available Wayland interfaces and EGL extensions available are all
known to Xwayland.

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor-eglstream.c | 37 +-
 hw/xwayland/xwayland-glamor-gbm.c   | 43 +++
 hw/xwayland/xwayland-glamor.c   | 94 -
 hw/xwayland/xwayland-present.c  |  2 +-
 hw/xwayland/xwayland.c  | 12 ++--
 hw/xwayland/xwayland.h  | 23 +++---
 6 files changed, 148 insertions(+), 63 deletions(-)

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c 
b/hw/xwayland/xwayland-glamor-eglstream.c
index bf74ae329..c226c0089 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -638,7 +638,7 @@ const struct wl_eglstream_display_listener 
eglstream_display_listener = {
 .swapinterval_override = 
xwl_eglstream_display_handle_swapinterval_override,
 };
 
-static void
+static Bool
 xwl_glamor_eglstream_init_wl_registry(struct xwl_screen *xwl_screen,
   struct wl_registry *wl_registry,
   uint32_t id, const char *name,
@@ -654,10 +654,15 @@ xwl_glamor_eglstream_init_wl_registry(struct xwl_screen 
*xwl_screen,
 wl_eglstream_display_add_listener(xwl_eglstream->display,
   _display_listener,
   xwl_screen);
+return TRUE;
 } else if (strcmp(name, "wl_eglstream_controller") == 0) {
 xwl_eglstream->controller = wl_registry_bind(
 wl_registry, id, _eglstream_controller_interface, version);
+return TRUE;
 }
+
+/* no match */
+return FALSE;
 }
 
 static Bool
@@ -882,23 +887,24 @@ out:
 return device;
 }
 
-Bool
+void
 xwl_glamor_init_eglstream(struct xwl_screen *xwl_screen)
 {
 struct xwl_eglstream_private *xwl_eglstream;
 EGLDeviceEXT egl_device;
 
+xwl_screen->eglstream_backend.is_available = FALSE;
 egl_device = xwl_eglstream_get_device(xwl_screen);
 if (egl_device == EGL_NO_DEVICE_EXT)
-return FALSE;
+return;
 
 if (!dixRegisterPrivateKey(_eglstream_private_key, PRIVATE_SCREEN, 0))
-return FALSE;
+return;
 
 xwl_eglstream = calloc(sizeof(*xwl_eglstream), 1);
 if (!xwl_eglstream) {
-ErrorF("Failed to allocate memory required to init eglstream 
support\n");
-return FALSE;
+ErrorF("Failed to allocate memory required to init EGLStream 
support\n");
+return;
 }
 
 dixSetPrivate(_screen->screen->devPrivates,
@@ -907,15 +913,12 @@ xwl_glamor_init_eglstream(struct xwl_screen *xwl_screen)
 xwl_eglstream->egl_device = egl_device;
 xorg_list_init(_eglstream->pending_streams);
 
-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_interfaces = 
xwl_glamor_eglstream_has_wl_interfaces;
-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;
-xwl_screen->egl_backend.allow_commits = xwl_glamor_eglstream_allow_commits;
-
-ErrorF("glamor: Using nvidia's eglstream interface, direct rendering 
impossible.\n");
-ErrorF("glamor: Performance may be affected. Ask your vendor to support 
GBM!\n");
-return TRUE;
+xwl_screen->eglstream_backend.init_egl = xwl_glamor_eglstream_init_egl;
+xwl_screen->eglstream_backend.init_wl_registry = 
xwl_glamor_eglstream_init_wl_registry;
+xwl_screen->eglstream_backend.has_wl_interfaces = 
xwl_glamor_eglstream_has_wl_interfaces;
+xwl_screen->eglstream_backend.init_screen = 
xwl_glamor_eglstream_init_screen;
+xwl_screen->eglstream_backend.get_wl_buffer_for_pixmap = 
xwl_glamor_eglstream_get_wl_buffer_for_pixmap;
+xwl_screen->eglstream_backend.post_damage = 
xwl_glamor_eglstream_post_damage;
+

[PATCH xserver 03/10] xwayland: GBM should fail w/out "GL_OES_EGL_image"

2018-06-05 Thread Olivier Fourdan
Surely, we should fail to init GBM backend if "GL_OES_EGL_image" is
missing.

This seems to have been lost with commit 1545e2dba ("xwayland: Decouple
GBM from glamor").

Suggested-by: Emil Velikov 
Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor-gbm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/xwayland/xwayland-glamor-gbm.c 
b/hw/xwayland/xwayland-glamor-gbm.c
index d6b979561..367e65a9a 100644
--- a/hw/xwayland/xwayland-glamor-gbm.c
+++ b/hw/xwayland/xwayland-glamor-gbm.c
@@ -807,8 +807,10 @@ xwl_glamor_gbm_init_egl(struct xwl_screen *xwl_screen)
 goto error;
 }
 
-if (!epoxy_has_gl_extension("GL_OES_EGL_image"))
+if (!epoxy_has_gl_extension("GL_OES_EGL_image")) {
 ErrorF("GL_OES_EGL_image not available\n");
+goto error;
+}
 
 if (epoxy_has_egl_extension(xwl_screen->egl_display,
 "EXT_image_dma_buf_import") &&
-- 
2.17.1

___
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 07/10] xwayland: move EGL backend init to glamor

2018-06-05 Thread Olivier Fourdan
Move EGL backends initialization to its own function in
xwayland-glamor.c

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor.c | 17 +
 hw/xwayland/xwayland.c| 16 ++--
 hw/xwayland/xwayland.h|  2 ++
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index 72995de00..3792dfa8c 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -162,6 +162,23 @@ glamor_egl_fd_name_from_pixmap(ScreenPtr screen,
 return 0;
 }
 
+void
+xwl_glamor_init_backends(struct xwl_screen *xwl_screen, Bool use_eglstream)
+{
+#ifdef XWL_HAS_EGLSTREAM
+if (use_eglstream) {
+if (!xwl_glamor_init_eglstream(xwl_screen)) {
+ErrorF("xwayland glamor: failed to setup EGLStream backend\n");
+use_eglstream = FALSE;
+}
+}
+#endif
+if (!use_eglstream && !xwl_glamor_init_gbm(xwl_screen)) {
+ErrorF("xwayland glamor: failed to setup GBM backend, falling back to 
sw accel\n");
+xwl_screen->glamor = 0;
+}
+}
+
 Bool
 xwl_glamor_init(struct xwl_screen *xwl_screen)
 {
diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 806d45675..8c02c02f8 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -992,20 +992,8 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 }
 
 #ifdef XWL_HAS_GLAMOR
-if (xwl_screen->glamor) {
-#ifdef XWL_HAS_EGLSTREAM
-if (use_eglstreams) {
-if (!xwl_glamor_init_eglstream(xwl_screen)) {
-ErrorF("xwayland glamor: failed to setup EGLStream backend\n");
-use_eglstreams = FALSE;
-}
-}
-#endif
-if (!use_eglstreams && !xwl_glamor_init_gbm(xwl_screen)) {
-ErrorF("xwayland glamor: failed to setup GBM backend, falling back 
to sw accel\n");
-xwl_screen->glamor = 0;
-}
-}
+if (xwl_screen->glamor)
+xwl_glamor_init_backends(xwl_screen, use_eglstreams);
 #endif
 
 /* In rootless mode, we don't have any screen storage, and the only
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index 191f1b297..304484ccc 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -423,6 +423,8 @@ Bool xwl_shm_destroy_pixmap(PixmapPtr pixmap);
 struct wl_buffer *xwl_shm_pixmap_get_wl_buffer(PixmapPtr pixmap);
 
 #ifdef XWL_HAS_GLAMOR
+void xwl_glamor_init_backends(struct xwl_screen *xwl_screen,
+  Bool use_eglstream);
 Bool xwl_glamor_init(struct xwl_screen *xwl_screen);
 
 Bool xwl_screen_set_drm_interface(struct xwl_screen *xwl_screen,
-- 
2.17.1

___
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 02/10] xwayland: swap "name" and "id" in init_wl_registry()

2018-06-05 Thread Olivier Fourdan
Both xwl_glamor_init_wl_registry() and the Wayland global registry
handler use the interface id/name in that order, using name/id in the
egl_backend vfunc makes things confusing and error prone.

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor-eglstream.c | 4 ++--
 hw/xwayland/xwayland-glamor-gbm.c   | 4 ++--
 hw/xwayland/xwayland-glamor.c   | 2 +-
 hw/xwayland/xwayland.h  | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c 
b/hw/xwayland/xwayland-glamor-eglstream.c
index 89c531f4a..f3fd97e6e 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -641,8 +641,8 @@ const struct wl_eglstream_display_listener 
eglstream_display_listener = {
 static void
 xwl_glamor_eglstream_init_wl_registry(struct xwl_screen *xwl_screen,
   struct wl_registry *wl_registry,
-  const char *name,
-  uint32_t id, uint32_t version)
+  uint32_t id, const char *name,
+  uint32_t version)
 {
 struct xwl_eglstream_private *xwl_eglstream =
 xwl_eglstream_get(xwl_screen);
diff --git a/hw/xwayland/xwayland-glamor-gbm.c 
b/hw/xwayland/xwayland-glamor-gbm.c
index 29325adac..d6b979561 100644
--- a/hw/xwayland/xwayland-glamor-gbm.c
+++ b/hw/xwayland/xwayland-glamor-gbm.c
@@ -737,8 +737,8 @@ xwl_screen_set_dmabuf_interface(struct xwl_screen 
*xwl_screen,
 static void
 xwl_glamor_gbm_init_wl_registry(struct xwl_screen *xwl_screen,
 struct wl_registry *wl_registry,
-const char *name,
-uint32_t id, uint32_t version)
+uint32_t id, const char *name,
+uint32_t version)
 {
 if (strcmp(name, "wl_drm") == 0)
 xwl_screen_set_drm_interface(xwl_screen, id, version);
diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index c7ae51336..14706f6e8 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -73,7 +73,7 @@ xwl_glamor_init_wl_registry(struct xwl_screen *xwl_screen,
 {
 if (xwl_screen->egl_backend.init_wl_registry)
 xwl_screen->egl_backend.init_wl_registry(xwl_screen, registry,
- interface, id, version);
+ id, interface, version);
 }
 
 struct wl_buffer *
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index 0d4afdf8a..a32fcf6a5 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -117,7 +117,7 @@ struct xwl_screen {
  */
 void (*init_wl_registry)(struct xwl_screen *xwl_screen,
  struct wl_registry *wl_registry,
- const char *name, uint32_t id,
+ uint32_t id, const char *name,
  uint32_t version);
 
 /* Called before glamor has been initialized. Backends should setup a
-- 
2.17.1

___
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 05/10] xwayland: move egl_backend to its own struct

2018-06-05 Thread Olivier Fourdan
EGL backend availability requires both EGL extensions and Wayland
interfaces to be present, so we will need to consider multiple backends
during initialization.

As a preliminary work, move the egl_backend to its own struct so that we
can have more than one backend at any given time.

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland.h | 99 ++
 1 file changed, 51 insertions(+), 48 deletions(-)

diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index a32fcf6a5..6bbe72e46 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -57,6 +57,56 @@ struct xwl_format {
 
 struct xwl_pixmap;
 struct xwl_window;
+struct xwl_screen;
+
+struct xwl_egl_backend {
+/* Called once for each interface in the global registry. Backends
+ * should use this to bind to any wayland interfaces they need. This
+ * callback is optional.
+ */
+void (*init_wl_registry)(struct xwl_screen *xwl_screen,
+ struct wl_registry *wl_registry,
+ uint32_t id, const char *name,
+ uint32_t version);
+
+/* Called before glamor has been initialized. Backends should setup a
+ * valid, glamor compatible EGL context in this hook.
+ */
+Bool (*init_egl)(struct xwl_screen *xwl_screen);
+
+/* Called after glamor has been initialized, and after all of the
+ * common Xwayland DDX hooks have been connected. Backends should use
+ * this to setup any required wraps around X server callbacks like
+ * CreatePixmap.
+ */
+ Bool (*init_screen)(struct xwl_screen *xwl_screen);
+
+ /* Called by Xwayland to retrieve a pointer to a valid wl_buffer for
+  * the given window/pixmap combo so that damage to the pixmap may be
+  * displayed on-screen. Backends should use this to create a new
+  * wl_buffer for a currently buffer-less pixmap, or simply return the
+  * pixmap they've prepared beforehand.
+  */
+ struct wl_buffer *(*get_wl_buffer_for_pixmap)(PixmapPtr pixmap,
+   unsigned short width,
+   unsigned short height,
+   Bool *created);
+
+ /* Called by Xwayland to perform any pre-wl_surface damage routines
+  * that are required by the backend. If your backend is poorly
+  * designed and lacks the ability to render directly to a surface,
+  * you should implement blitting from the glamor pixmap to the wayland
+  * pixmap here. Otherwise, this callback is optional.
+  */
+ void (*post_damage)(struct xwl_window *xwl_window,
+ PixmapPtr pixmap, RegionPtr region);
+
+ /* Called by Xwayland to confirm with the egl backend that the given
+  * pixmap is completely setup and ready for display on-screen. This
+  * callback is optional.
+  */
+ Bool (*allow_commits)(struct xwl_window *xwl_window);
+};
 
 struct xwl_screen {
 int width;
@@ -110,54 +160,7 @@ struct xwl_screen {
 void *egl_display, *egl_context;
 
 /* the current backend for creating pixmaps on wayland */
-struct {
-/* Called once for each interface in the global registry. Backends
- * should use this to bind to any wayland interfaces they need. This
- * callback is optional.
- */
-void (*init_wl_registry)(struct xwl_screen *xwl_screen,
- struct wl_registry *wl_registry,
- uint32_t id, const char *name,
- uint32_t version);
-
-/* Called before glamor has been initialized. Backends should setup a
- * valid, glamor compatible EGL context in this hook.
- */
-Bool (*init_egl)(struct xwl_screen *xwl_screen);
-
-/* Called after glamor has been initialized, and after all of the
- * common Xwayland DDX hooks have been connected. Backends should use
- * this to setup any required wraps around X server callbacks like
- * CreatePixmap.
- */
-Bool (*init_screen)(struct xwl_screen *xwl_screen);
-
-/* Called by Xwayland to retrieve a pointer to a valid wl_buffer for
- * the given window/pixmap combo so that damage to the pixmap may be
- * displayed on-screen. Backends should use this to create a new
- * wl_buffer for a currently buffer-less pixmap, or simply return the
- * pixmap they've prepared beforehand.
- */
-struct wl_buffer *(*get_wl_buffer_for_pixmap)(PixmapPtr pixmap,
-  unsigned short width,
-  unsigned short height,
-  Bool *created);
-
-/* Called by Xwayland to perform any pre-wl_surface damage routines
- * that 

[PATCH xserver 01/10] xwayland: move glamor specific routines

2018-06-05 Thread Olivier Fourdan
Functions such as:

  xwl_glamor_egl_supports_device_probing()
  xwl_glamor_egl_get_devices()
  xwl_glamor_egl_device_has_egl_extensions()

Are of no use outside of EGLStream support, move them to the relevant
source file.

Similarly, the other glamor functions such as:

  xwl_glamor_init()
  xwl_screen_set_drm_interface()
  xwl_screen_set_dmabuf_interface()
  xwl_glamor_pixmap_get_wl_buffer()
  xwl_glamor_init_wl_registry()
  xwl_glamor_post_damage()
  xwl_glamor_allow_commits()
  xwl_glamor_egl_make_current()

Are useless without glamor support enabled, move those within a
a "#ifdef XWL_HAS_GLAMOR" in xwayland.h

Signed-off-by: Olivier Fourdan 
---
 hw/xwayland/xwayland-glamor-eglstream.c | 79 
 hw/xwayland/xwayland-glamor.c   | 80 -
 hw/xwayland/xwayland.h  | 24 
 3 files changed, 89 insertions(+), 94 deletions(-)

diff --git a/hw/xwayland/xwayland-glamor-eglstream.c 
b/hw/xwayland/xwayland-glamor-eglstream.c
index 8dd1cc304..89c531f4a 100644
--- a/hw/xwayland/xwayland-glamor-eglstream.c
+++ b/hw/xwayland/xwayland-glamor-eglstream.c
@@ -187,6 +187,85 @@ xwl_eglstream_cleanup(struct xwl_screen *xwl_screen)
 free(xwl_eglstream);
 }
 
+static Bool
+xwl_glamor_egl_supports_device_probing(void)
+{
+return epoxy_has_egl_extension(NULL, "EGL_EXT_device_base");
+}
+
+static void **
+xwl_glamor_egl_get_devices(int *num_devices)
+{
+EGLDeviceEXT *devices;
+Bool ret;
+int drm_dev_count = 0;
+int i;
+
+if (!xwl_glamor_egl_supports_device_probing())
+return NULL;
+
+/* Get the number of devices */
+ret = eglQueryDevicesEXT(0, NULL, num_devices);
+if (!ret || *num_devices < 1)
+return NULL;
+
+devices = calloc(*num_devices, sizeof(EGLDeviceEXT));
+if (!devices)
+return NULL;
+
+ret = eglQueryDevicesEXT(*num_devices, devices, num_devices);
+if (!ret)
+goto error;
+
+/* We're only ever going to care about devices that support
+ * EGL_EXT_device_drm, so filter out the ones that don't
+ */
+for (i = 0; i < *num_devices; i++) {
+const char *extension_str =
+eglQueryDeviceStringEXT(devices[i], EGL_EXTENSIONS);
+
+if (!epoxy_extension_in_string(extension_str, "EGL_EXT_device_drm"))
+continue;
+
+devices[drm_dev_count++] = devices[i];
+}
+if (!drm_dev_count)
+goto error;
+
+*num_devices = drm_dev_count;
+devices = realloc(devices, sizeof(EGLDeviceEXT) * drm_dev_count);
+
+return devices;
+
+error:
+free(devices);
+
+return NULL;
+}
+
+static Bool
+xwl_glamor_egl_device_has_egl_extensions(void *device,
+ const char **ext_list, size_t size)
+{
+EGLDisplay egl_display;
+int i;
+Bool has_exts = TRUE;
+
+egl_display = glamor_egl_get_display(EGL_PLATFORM_DEVICE_EXT, device);
+if (!egl_display || !eglInitialize(egl_display, NULL, NULL))
+return FALSE;
+
+for (i = 0; i < size; i++) {
+if (!epoxy_has_egl_extension(egl_display, ext_list[i])) {
+has_exts = FALSE;
+break;
+}
+}
+
+eglTerminate(egl_display);
+return has_exts;
+}
+
 static void
 xwl_eglstream_unref_pixmap_stream(struct xwl_pixmap *xwl_pixmap)
 {
diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index f543f321d..c7ae51336 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -52,86 +52,6 @@ xwl_glamor_egl_make_current(struct xwl_screen *xwl_screen)
 xwl_screen->glamor_ctx->make_current(xwl_screen->glamor_ctx);
 }
 
-Bool
-xwl_glamor_egl_supports_device_probing(void)
-{
-return epoxy_has_egl_extension(NULL, "EGL_EXT_device_base");
-}
-
-void **
-xwl_glamor_egl_get_devices(int *num_devices)
-{
-#ifdef XWL_HAS_EGLSTREAM
-EGLDeviceEXT *devices;
-Bool ret;
-int drm_dev_count = 0;
-int i;
-
-if (!xwl_glamor_egl_supports_device_probing())
-return NULL;
-
-/* Get the number of devices */
-ret = eglQueryDevicesEXT(0, NULL, num_devices);
-if (!ret || *num_devices < 1)
-return NULL;
-
-devices = calloc(*num_devices, sizeof(EGLDeviceEXT));
-if (!devices)
-return NULL;
-
-ret = eglQueryDevicesEXT(*num_devices, devices, num_devices);
-if (!ret)
-goto error;
-
-/* We're only ever going to care about devices that support
- * EGL_EXT_device_drm, so filter out the ones that don't
- */
-for (i = 0; i < *num_devices; i++) {
-const char *extension_str =
-eglQueryDeviceStringEXT(devices[i], EGL_EXTENSIONS);
-
-if (!epoxy_extension_in_string(extension_str, "EGL_EXT_device_drm"))
-continue;
-
-devices[drm_dev_count++] = devices[i];
-}
-if (!drm_dev_count)
-goto error;
-
-*num_devices = drm_dev_count;
-devices = realloc(devices, sizeof(EGLDeviceEXT) * drm_dev_count);
-
-return 

[PATCH xserver 04/10] xwayland: skip drm authentication with render node

2018-06-05 Thread Olivier Fourdan
If using a render node, we can skip DRM authentication.

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

diff --git a/hw/xwayland/xwayland-glamor-gbm.c 
b/hw/xwayland/xwayland-glamor-gbm.c
index 367e65a9a..b03e7ee17 100644
--- a/hw/xwayland/xwayland-glamor-gbm.c
+++ b/hw/xwayland/xwayland-glamor-gbm.c
@@ -837,11 +837,16 @@ error:
 static Bool
 xwl_glamor_gbm_init_screen(struct xwl_screen *xwl_screen)
 {
+struct xwl_gbm_private *xwl_gbm = xwl_gbm_get(xwl_screen);
+
 if (!dri3_screen_init(xwl_screen->screen, _dri3_info)) {
 ErrorF("Failed to initialize dri3\n");
 goto error;
 }
 
+if (xwl_gbm->fd_render_node)
+goto skip_drm_auth;
+
 if (!dixRegisterPrivateKey(_auth_state_private_key, PRIVATE_CLIENT,
0)) {
 ErrorF("Failed to register private key\n");
@@ -854,6 +859,7 @@ xwl_glamor_gbm_init_screen(struct xwl_screen *xwl_screen)
 goto error;
 }
 
+skip_drm_auth:
 xwl_screen->screen->CreatePixmap = xwl_glamor_gbm_create_pixmap;
 xwl_screen->screen->DestroyPixmap = xwl_glamor_gbm_destroy_pixmap;
 
-- 
2.17.1

___
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 5/5] xwayland: make xwl_output_get_xdg_output() static

2018-06-05 Thread Olivier Fourdan
Make xwl_output_get_xdg_output() private, it doesn't need to be
available elsewhere.

Signed-off-by: Olivier Fourdan 
Reviewed-by: Lyude Paul 
Reviewed-by: Emil Velikov 
---
 hw/xwayland/xwayland-output.c | 4 +++-
 hw/xwayland/xwayland.h| 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
index 48faeb142..379062549 100644
--- a/hw/xwayland/xwayland-output.c
+++ b/hw/xwayland/xwayland-output.c
@@ -38,6 +38,8 @@
RR_Reflect_X  | \
RR_Reflect_Y)
 
+static void xwl_output_get_xdg_output(struct xwl_output *xwl_output);
+
 static Rotation
 wl_transform_to_xrandr(enum wl_output_transform transform)
 {
@@ -435,7 +437,7 @@ xwl_screen_init_output(struct xwl_screen *xwl_screen)
 return TRUE;
 }
 
-void
+static void
 xwl_output_get_xdg_output(struct xwl_output *xwl_output)
 {
 struct xwl_screen *xwl_screen = xwl_output->xwl_screen;
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index 25112e2cb..39bc20a7e 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -440,7 +440,6 @@ void xwl_present_cleanup(WindowPtr window);
 
 void xwl_screen_release_tablet_manager(struct xwl_screen *xwl_screen);
 
-void xwl_output_get_xdg_output(struct xwl_output *xwl_output);
 void xwl_screen_init_xdg_output(struct xwl_screen *xwl_screen);
 
 void xwl_glamor_egl_make_current(struct xwl_screen *xwl_screen);
-- 
2.17.1

___
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 4/5] xwayland: do not disable glamor if EGLStream failed

2018-06-05 Thread Olivier Fourdan
EGLStream requires glamor, but the opposite is not true. So if someone
passes "-eglstream" with a GPU which does not support EGLStream, we
could maybe still try GBM and be lucky.

That allows Wayland compositors to pass "-eglstream" regardless of the
actual hardware, if they want to enable EGLStream on GPU which support
it.

Signed-off-by: Olivier Fourdan 
Reviewed-by: Lyude Paul 
Reviewed-by: Emil Velikov 
---
 hw/xwayland/xwayland.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 9121ef666..806d45675 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -939,9 +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;
-#ifdef XWL_HAS_EGLSTREAM
 Bool use_eglstreams = FALSE;
-#endif
 
 xwl_screen = calloc(1, sizeof *xwl_screen);
 if (xwl_screen == NULL)
@@ -998,12 +996,12 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 #ifdef XWL_HAS_EGLSTREAM
 if (use_eglstreams) {
 if (!xwl_glamor_init_eglstream(xwl_screen)) {
-ErrorF("xwayland glamor: failed to setup eglstream backend, 
falling back to swaccel\n");
-xwl_screen->glamor = 0;
+ErrorF("xwayland glamor: failed to setup EGLStream backend\n");
+use_eglstreams = FALSE;
 }
-} else
+}
 #endif
-if (!xwl_glamor_init_gbm(xwl_screen)) {
+if (!use_eglstreams && !xwl_glamor_init_gbm(xwl_screen)) {
 ErrorF("xwayland glamor: failed to setup GBM backend, falling back 
to sw accel\n");
 xwl_screen->glamor = 0;
 }
-- 
2.17.1

___
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 00/10] Re: Refactor egl_backends for wayland registry

2018-06-05 Thread Olivier Fourdan
Hi,

This is a follow-up on https://patchwork.freedesktop.org/series/44095/
after Lyude and Emil reviews.

Cheers,
Olivier

Olivier Fourdan (10):
  xwayland: move glamor specific routines
  xwayland: swap "name" and "id" in init_wl_registry()
  xwayland: GBM should fail w/out "GL_OES_EGL_image"
  xwayland: skip drm authentication with render node
  xwayland: move egl_backend to its own struct
  xwayland: Add Wayland interfaces check
  xwayland: move EGL backend init to glamor
  xwayland: refactor EGL backends for wayland registry
  xwayland: check for EGLStream backend explicitly
  xwayland: EGL_IMG_context_priority required by EGLStream

 hw/xwayland/xwayland-glamor-eglstream.c | 152 +++
 hw/xwayland/xwayland-glamor-gbm.c   |  69 +++--
 hw/xwayland/xwayland-glamor.c   | 186 
 hw/xwayland/xwayland-present.c  |   5 +-
 hw/xwayland/xwayland.c  |  28 ++--
 hw/xwayland/xwayland.h  | 151 ++-
 6 files changed, 369 insertions(+), 222 deletions(-)

-- 
2.17.1

___
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/5] xwayland: process Wayland events after adding screen

2018-06-05 Thread Olivier Fourdan
When we're done adding a new screen, we need to process any pending
Wayland events again.

Hence we don't end up processing xdg_output events unexpectedly when
glamor is disabled. Be that because "-shm" was passed or "-eglstream"
has failed.

Failing to do that could lead to a crash at startup:

Xwayland: dixGetPrivateAddr: Assertion `key->initialized' failed.
(EE)
(EE) Backtrace:
(EE) 0: Xwayland (OsSigHandler)
(EE) 1: libpthread.so.0 (funlockfile)
(EE) 2: libc.so.6 (gsignal)
(EE) 3: libc.so.6 (abort)
(EE) 4: libc.so.6 (?+0x0)
(EE) 5: libc.so.6 (__assert_fail)
(EE) 6: Xwayland (dixGetPrivateAddr)
(EE) 7: Xwayland (_fbGetWindowPixmap)
(EE) 8: Xwayland (getDrawableDamageRef)
(EE) 9: Xwayland (damageRegionProcessPending)
(EE) 10: Xwayland (damagePolyFillRect)
(EE) 11: Xwayland (miPaintWindow)
(EE) 12: Xwayland (miWindowExposures)
(EE) 13: Xwayland (miHandleValidateExposures)
(EE) 14: Xwayland (SetRootClip)
(EE) 15: Xwayland (update_screen_size)
(EE) 16: Xwayland (apply_output_change)
(EE) 17: libffi.so.6 (ffi_call_unix64)
(EE) 18: libffi.so.6 (ffi_call)
(EE) 19: libwayland-client.so.0 (wl_log_set_handler_client)
(EE) 20: libwayland-client.so.0 (_init)
(EE) 21: libwayland-client.so.0 (wl_display_dispatch_queue_pending)
(EE) 22: libwayland-client.so.0 (wl_display_roundtrip_queue)
(EE) 23: Xwayland (InitInput)
(EE) 24: Xwayland (dix_main)
(EE) 25: libc.so.6 (__libc_start_main)
(EE) 26: Xwayland (_start)
(EE)
(EE)
Fatal server error:
(EE) Caught signal 6 (Aborted). Server aborting
(EE)
Aborted (core dumped)

Signed-off-by: Olivier Fourdan 
Reviewed-by: Lyude Paul 
Reviewed-by: Emil Velikov 
---
 hw/xwayland/xwayland.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index d9548a874..9121ef666 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -1132,6 +1132,10 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 
 AddCallback(, xwl_property_callback, pScreen);
 
+wl_display_roundtrip(xwl_screen->display);
+while (xwl_screen->expecting_event)
+wl_display_roundtrip(xwl_screen->display);
+
 return ret;
 }
 
-- 
2.17.1

___
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/5] xwayland: "EGL_EXT_device_base" required for EGLStream

2018-06-05 Thread Olivier Fourdan
eglQueryDevicesEXT() would abort if the required extensions are not
available, meaning that enabling “-eglstream” on a non-EGLStream
capable hardware would lead to an abort().

Check that "EGL_EXT_device_base" extension is available and bail out
early if not, so we don't abort() later in eglQueryDevicesEXT().

Signed-off-by: Olivier Fourdan 
Reviewed-by: Lyude Paul 
Reviewed-by: Emil Velikov 
---
 hw/xwayland/xwayland-glamor.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index cdca072ed..f543f321d 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -67,6 +67,9 @@ xwl_glamor_egl_get_devices(int *num_devices)
 int drm_dev_count = 0;
 int i;
 
+if (!xwl_glamor_egl_supports_device_probing())
+return NULL;
+
 /* Get the number of devices */
 ret = eglQueryDevicesEXT(0, NULL, num_devices);
 if (!ret || *num_devices < 1)
-- 
2.17.1

___
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/5] xwayland: allow "-eglstream" option

2018-06-05 Thread Olivier Fourdan
The command line option "-eglstream" used to enable EGLStream support
for NVidia GPU was made available only when Xwayland was built with
EGLStream support enabled.

Wayland compositors who spawn Xwayland have no easy way to tell whether
or not Xwayland was built with EGLStream support enabled, and adding
"-eglstream" command line option to Xwayland when it wasn't built with
EGLStream support would prevent Xwayland from starting (“Unrecognized
option” error).

Make sure we support the command line option "-eglstream" regardless of
EGLStream support in Xwayland. Obviously, if Xwayland was built without
EGLStream support, this has no effect.

Signed-off-by: Olivier Fourdan 
Reviewed-by: Lyude Paul 
Reviewed-by: Emil Velikov 
---
 hw/xwayland/xwayland.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 1d6b49979..d9548a874 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -96,9 +96,7 @@ ddxUseMsg(void)
 ErrorF("-rootless  run rootless, requires wm support\n");
 ErrorF("-wm fd create X client for wm on given fd\n");
 ErrorF("-listen fd add give fd as a listen socket\n");
-#ifdef XWL_HAS_EGLSTREAM
 ErrorF("-eglstream use eglstream backend for nvidia GPUs\n");
-#endif
 }
 
 int
@@ -117,11 +115,9 @@ ddxProcessArgument(int argc, char *argv[], int i)
 else if (strcmp(argv[i], "-shm") == 0) {
 return 1;
 }
-#ifdef XWL_HAS_EGLSTREAM
 else if (strcmp(argv[i], "-eglstream") == 0) {
 return 1;
 }
-#endif
 
 return 0;
 }
@@ -988,11 +984,13 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 else if (strcmp(argv[i], "-shm") == 0) {
 xwl_screen->glamor = 0;
 }
-#ifdef XWL_HAS_EGLSTREAM
 else if (strcmp(argv[i], "-eglstream") == 0) {
+#ifdef XWL_HAS_EGLSTREAM
 use_eglstreams = TRUE;
-}
+#else
+ErrorF("xwayland glamor: this build does not have EGLStream 
support\n");
 #endif
+}
 }
 
 #ifdef XWL_HAS_GLAMOR
-- 
2.17.1

___
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 0/5] Re: Xwayland fixes wrt eglstream support

2018-06-05 Thread Olivier Fourdan
Hi,

This is a follow-up on https://patchwork.freedesktop.org/series/43704/
after Lyude and Emil reviews.

Emil, for 1/5, I changed the error message if eglstream wasn't enabled at
build time to:

  "xwayland glamor: this build does not have eglstream support"

which sounds to me like the most accurate description :)

I rephrased the commit messages in 1/3 and in 5/5 as per your wording.

I also fixed a few typos in the commit messages and added the R-b from
both Lyude and yourself.

Cheers,
Olivier

Olivier Fourdan (5):
  xwayland: allow "-eglstream" option
  xwayland: "EGL_EXT_device_base" required for EGLStream
  xwayland: process Wayland events after adding screen
  xwayland: do not disable glamor if EGLStream failed
  xwayland: make xwl_output_get_xdg_output() static

 hw/xwayland/xwayland-glamor.c |  3 +++
 hw/xwayland/xwayland-output.c |  4 +++-
 hw/xwayland/xwayland.c| 24 
 hw/xwayland/xwayland.h|  1 -
 4 files changed, 18 insertions(+), 14 deletions(-)

-- 
2.17.1

___
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 5/5] xwayland: refactor egl_backends for wayland registry

2018-06-05 Thread Olivier Fourdan
Hi Emil,

Many thanks for your detailed review!

On Tue, Jun 5, 2018 at 12:37 PM, Emil Velikov  wrote:
> Hi Olivier,
>
> There's a handful of mostly trivial suggestions below. The idea itself seems
> reasonable IMHO. One gripe is that we're 'leaking' twice as much as before.
>
> Namely: even if the current backend cleans-up after itself (it some cases it
> does not), the other backend 'leaks'. Not sure if/how much we should care in
> that case.
>
> Was skimming if we cannot move more init_egl/init_screen tripping points
> and I've noticed a few gotchas/bugs. None of which are requirement for the
> series, although great to have.
>
>
> xwl_glamor_gbm_init_egl
>  - if !render_node error path is leaking

humm, not sure, leaking what? we haven't allocated anything yet, have we?
But anyhow, it's unralted to this refactoring I think.

>  - surely we'd want to error out when GL_OES_EGL_image is missing
> xwl_glamor_gbm_init_screen

Sure, will add separate patch for that, seems it got lost with commit
1545e2dba ("xwayland: Decouple GBM from glamor").

>  - no need to RegisterPrivateKey/AddCallback if a rendernode is opened

Yeap, will add a separate patch for this.

> xwl_glamor_eglstream_init_egl
>  - using EGL_IMG_context_priority w/o checking for it
> xwl_glamor_eglstream_init_screen

Ditto.

>  - the ->controller check is no longer needed

Ditto.

>> +static Bool
>> +xwl_glamor_gbm_has_egl_extension(void)
>> +{
>> +return epoxy_has_egl_extension(NULL, "EGL_MESA_platform_gbm");
> I'd also check for EGL_KHR_platform_gbm - it's trivial enough ;-)

Sure!

>> @@ -71,9 +71,24 @@ xwl_glamor_init_wl_registry(struct xwl_screen *xwl_screen,
>>  uint32_t id, const char *interface,
>>  uint32_t version)
>>  {
>> -if (xwl_screen->egl_backend.init_wl_registry)
>> -xwl_screen->egl_backend.init_wl_registry(xwl_screen, registry,
>> - interface, id, version);
>> +#ifdef GLAMOR_HAS_GBM
>> +if (xwl_screen->gbm_backend.is_available &&
>> +xwl_screen->gbm_backend.init_wl_registry &&
>> +xwl_screen->gbm_backend.init_wl_registry(xwl_screen,
>> + registry,
>> + id,
>> + interface,
>> + version)); /* no-op */
>> +#endif
>> +#ifdef XWL_HAS_EGLSTREAM
>> +else if (xwl_screen->eglstreams_backend.is_available &&
>> + xwl_screen->eglstreams_backend.init_wl_registry &&
>> + xwl_screen->eglstreams_backend.init_wl_registry(xwl_screen,
>> + registry,
>> + id,
>> + interface,
>> + version)); /* 
>> no-op */
>> +#endif
> Both ifdef guards can go.

Well, the idea was to save a few conditionals if the support was not
enabled at build time, but I can certainly remove those ifdefs.

>> @@ -119,8 +134,8 @@ xwl_glamor_allow_commits(struct xwl_window *xwl_window)
>>  {
>>  struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
>>
>> -if (xwl_screen->egl_backend.allow_commits)
>> -return xwl_screen->egl_backend.allow_commits(xwl_window);
>> +if (xwl_screen->egl_backend && xwl_screen->egl_backend->allow_commits)
> Why the NULL check? Unless I'm missing something that will never happen.

We can have no “egl_backend” set at all if “-shm” was specified.

Either we keep that test here or we need to check for
"xwl_screen->glamor" in xwl_screen_post_damage(), I'll change for the
later for clarity.

>> @@ -165,17 +180,35 @@ glamor_egl_fd_name_from_pixmap(ScreenPtr screen,
>>  void
>>  xwl_glamor_init_backends(struct xwl_screen *xwl_screen, Bool use_eglstreams)
>>  {
>
> General nit:
> Drop the return type of xwl_glamor_init_$backend now that we check
> ::is_available.

Sure, it's unused anyway.

> Instead of relying on the coded probe order (alongside the
> is_available = false workaround),
> we could use -eglstream to control which backend is checked/probed first.
>
> If that fails we fall-back to the other.
>
> If that sounds reasonable, then the following can be reworked as follows:

Sounds reasonable.

>> +#ifdef GLAMOR_HAS_GBM
>> +/* Init GBM even if "-eglstream" is requested, as EGL streams may fail 
>> */
>> +xwl_glamor_init_gbm(xwl_screen);
> Here we add:
>
>   if (!xwl_screen->gbm_backend.is_available && !use_eglstreams)
> ErrorF(xwayland glamor: GBM (default) is not available);
>
>> +#endif
>>  #ifdef XWL_HAS_EGLSTREAM
>> +xwl_glamor_init_eglstream(xwl_screen);
>
>>  if (use_eglstreams) {
>> -if (!xwl_glamor_init_eglstream(xwl_screen)) {
>> -ErrorF("xwayland glamor: failed to setup eglstream backend\n");

Re: [PATCH app/xauth 2/2] Sort entries from most specific to most generic.

2018-06-05 Thread Walter Harms


> Michal Srb  hat am 31. Mai 2018 um 15:12 geschrieben:
> 
> 
> There is no point in adding entry or merging lists if a FamilyWild entry would
> end in front of any entry, or entry without display number would end in front
> of entry with number.
> 
> This sorts all entries in order:
>   * FamilyWild without display number
>   * FamilyWild with display number
>   * Other family without display number
>   * Other family with display number
> 
> The order of the entries in each category is kept.
> ---
>  process.c | 41 +
>  1 file changed, 41 insertions(+)
> 
> diff --git a/process.c b/process.c
> index d3ea435..f3d6ca4 100644
> --- a/process.c
> +++ b/process.c
> @@ -1170,6 +1170,45 @@ merge_entries(AuthList **firstp, AuthList *second, int
> *nnewp, int *nreplp)
>  
>  }
>  
> +static void
> +sort_entries(AuthList **firstp)
> +{
> +/* Insert sort, in each pass it removes auth records of certain */
> +/* cathegory from the given list and inserts them into the sorted list.
> */
> +
> +AuthList *sorted = NULL, *sorted_tail = NULL;
> +AuthList *prev, *iter, *next;
> +
> +#define SORT_OUT(EXPRESSION) { \
> + prev = NULL; \
> + for (iter = *firstp; iter; iter = next) { \
> + next = iter->next; \
> + if (EXPRESSION) { \
> + if (prev) \
> + prev->next = next; \
> + else \
> + *firstp = next; \
> + if (sorted_tail == NULL) { \
> + sorted = sorted_tail = iter; \
> + } else { \
> + sorted_tail->next = iter; \
> + sorted_tail = iter; \
> + } \
> + iter->next = NULL; \
> + } else { \
> + prev = iter; \
> + } \
> + } \
> +}
> +
> +SORT_OUT(iter->auth->family != FamilyWild && iter->auth->number_length !=
> 0);
> +SORT_OUT(iter->auth->family != FamilyWild && iter->auth->number_length ==
> 0);
> +SORT_OUT(iter->auth->family == FamilyWild && iter->auth->number_length !=
> 0);
> +SORT_OUT(iter->auth->family == FamilyWild && iter->auth->number_length ==
> 0);
> +
> +*firstp = sorted;
> +}
> +
>  static Xauth *
>  copyAuth(Xauth *auth)
>  {
> @@ -1508,6 +1547,7 @@ do_merge(const char *inputfilename, int lineno, int
> argc, const char **argv)
> printf ("%d entries read in:  %d new, %d replacement%s\n",
> nentries, nnew, nrepl, nrepl != 1 ? "s" : "");
>   if (nentries > 0) xauth_modified = True;
> + sort_entries(_head);
>  }
>  
>  return 0;
> @@ -1656,6 +1696,7 @@ do_add(const char *inputfilename, int lineno, int argc,
> const char **argv)
>   fprintf (stderr, "unable to merge in added record\n");
>   return 1;
>  }
> +sort_entries(_head);
>  
>  xauth_modified = True;
>  return 0;


MMh, so far i understand you do an add and then sort the whole thing.
perhaps it would be more easy to start with a sorted field and insert (add)
a key on the correct location ?

re,
 wh

> -- 
> 2.13.6
> 
> ___
> 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
___
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/5] xwayland: Allow "-eglstream" option

2018-06-05 Thread Olivier Fourdan
Hi Emil,

On 5 June 2018 at 12:41, Emil Velikov  wrote:

> [...]
>
>> > +#else
> >> > +ErrorF("xwayland glamor: eglstream backend support not
> >> > enabled\n");
> >> Something is really weird here:
> >>
> >> On one hand '-eglstream' is recognised and used (by potential user) on
> >> the other "... support is not _enabled_" is printed.
> >> Surely you meant "not built", right? After all explicitly passing the
> >> enable (runtime) flag should be enough to enable it ;-)
> >
> >
> > Yes, I literally mean "enabled at build time".
> >
> This wording is even better.
>

Eventually, in the updated series I am about to post in a minute, I opted
for the following wording:

   "xwayland glamor: this build does not have eglstream support"

which, I reckon, seems like the most accurate description :)

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

Re: [PATCH xserver 1/5] xwayland: Allow "-eglstream" option

2018-06-05 Thread Emil Velikov
On 4 June 2018 at 15:37, Olivier Fourdan  wrote:
> Hi
>
> On 4 June 2018 at 16:24, Emil Velikov  wrote:
>>
>> On 24 May 2018 at 15:10, Olivier Fourdan  wrote:
>> > The command line option "-eglstream" used to enable EGLi stream support
>> > for NVidia GPU was made available only when Xwayland was built with EGL
>> > stream support enabled.
>> >
>> > Wayland compositors who spawn Xwayland have no easy way to tell whether
>> > or not Xwayland was built with EGL stream support enabled, and adding
>> > "-eglstream" command line option to Xwayland when it wasn't built with
>> > EGL support would prevent Xwayland from starting (“Unrecognized option”
>> > error).
>> >
>> > Make sure we support the command line option "-eglstream" regardless of
>> > EGL stream support in Xwayland, obviously without EGL stream support
>> > this has no effect.
>> >
>> > Signed-off-by: Olivier Fourdan 
>> > ---
>> >  hw/xwayland/xwayland.c | 10 --
>> >  1 file changed, 4 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
>> > index 1d6b49979..b4049d2cc 100644
>> > --- a/hw/xwayland/xwayland.c
>> > +++ b/hw/xwayland/xwayland.c
>> > @@ -96,9 +96,7 @@ ddxUseMsg(void)
>> >  ErrorF("-rootless  run rootless, requires wm
>> > support\n");
>> >  ErrorF("-wm fd create X client for wm on given
>> > fd\n");
>> >  ErrorF("-listen fd add give fd as a listen socket\n");
>> > -#ifdef XWL_HAS_EGLSTREAM
>> >  ErrorF("-eglstream use eglstream backend for nvidia
>> > GPUs\n");
>> > -#endif
>> >  }
>> >
>> >  int
>> > @@ -117,11 +115,9 @@ ddxProcessArgument(int argc, char *argv[], int i)
>> >  else if (strcmp(argv[i], "-shm") == 0) {
>> >  return 1;
>> >  }
>> > -#ifdef XWL_HAS_EGLSTREAM
>> >  else if (strcmp(argv[i], "-eglstream") == 0) {
>> >  return 1;
>> >  }
>> > -#endif
>> >
>> >  return 0;
>> >  }
>> > @@ -988,11 +984,13 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char
>> > **argv)
>> >  else if (strcmp(argv[i], "-shm") == 0) {
>> >  xwl_screen->glamor = 0;
>> >  }
>> > -#ifdef XWL_HAS_EGLSTREAM
>> >  else if (strcmp(argv[i], "-eglstream") == 0) {
>> > +#ifdef XWL_HAS_EGLSTREAM
>> >  use_eglstreams = TRUE;
>> > -}
>> > +#else
>> > +ErrorF("xwayland glamor: eglstream backend support not
>> > enabled\n");
>> Something is really weird here:
>>
>> On one hand '-eglstream' is recognised and used (by potential user) on
>> the other "... support is not _enabled_" is printed.
>> Surely you meant "not built", right? After all explicitly passing the
>> enable (runtime) flag should be enough to enable it ;-)
>
>
> Yes, I literally mean "enabled at build time".
>
This wording is even better.

Thanks
Emil
___
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 5/5] xwayland: refactor egl_backends for wayland registry

2018-06-05 Thread Emil Velikov
Hi Olivier,

There's a handful of mostly trivial suggestions below. The idea itself seems
reasonable IMHO. One gripe is that we're 'leaking' twice as much as before.

Namely: even if the current backend cleans-up after itself (it some cases it
does not), the other backend 'leaks'. Not sure if/how much we should care in
that case.

Was skimming if we cannot move more init_egl/init_screen tripping points
and I've noticed a few gotchas/bugs. None of which are requirement for the
series, although great to have.


xwl_glamor_gbm_init_egl
 - if !render_node error path is leaking
 - surely we'd want to error out when GL_OES_EGL_image is missing
xwl_glamor_gbm_init_screen
 - no need to RegisterPrivateKey/AddCallback if a rendernode is opened

xwl_glamor_eglstream_init_egl
 - using EGL_IMG_context_priority w/o checking for it
xwl_glamor_eglstream_init_screen
 - the ->controller check is no longer needed

On 1 June 2018 at 15:31, Olivier Fourdan  wrote:

> +static Bool
> +xwl_glamor_gbm_has_egl_extension(void)
> +{
> +return epoxy_has_egl_extension(NULL, "EGL_MESA_platform_gbm");
I'd also check for EGL_KHR_platform_gbm - it's trivial enough ;-)


> @@ -71,9 +71,24 @@ xwl_glamor_init_wl_registry(struct xwl_screen *xwl_screen,
>  uint32_t id, const char *interface,
>  uint32_t version)
>  {
> -if (xwl_screen->egl_backend.init_wl_registry)
> -xwl_screen->egl_backend.init_wl_registry(xwl_screen, registry,
> - interface, id, version);
> +#ifdef GLAMOR_HAS_GBM
> +if (xwl_screen->gbm_backend.is_available &&
> +xwl_screen->gbm_backend.init_wl_registry &&
> +xwl_screen->gbm_backend.init_wl_registry(xwl_screen,
> + registry,
> + id,
> + interface,
> + version)); /* no-op */
> +#endif
> +#ifdef XWL_HAS_EGLSTREAM
> +else if (xwl_screen->eglstreams_backend.is_available &&
> + xwl_screen->eglstreams_backend.init_wl_registry &&
> + xwl_screen->eglstreams_backend.init_wl_registry(xwl_screen,
> + registry,
> + id,
> + interface,
> + version)); /* 
> no-op */
> +#endif
Both ifdef guards can go.


> @@ -119,8 +134,8 @@ xwl_glamor_allow_commits(struct xwl_window *xwl_window)
>  {
>  struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
>
> -if (xwl_screen->egl_backend.allow_commits)
> -return xwl_screen->egl_backend.allow_commits(xwl_window);
> +if (xwl_screen->egl_backend && xwl_screen->egl_backend->allow_commits)
Why the NULL check? Unless I'm missing something that will never happen.


> @@ -165,17 +180,35 @@ glamor_egl_fd_name_from_pixmap(ScreenPtr screen,
>  void
>  xwl_glamor_init_backends(struct xwl_screen *xwl_screen, Bool use_eglstreams)
>  {

General nit:
Drop the return type of xwl_glamor_init_$backend now that we check
::is_available.

Instead of relying on the coded probe order (alongside the
is_available = false workaround),
we could use -eglstream to control which backend is checked/probed first.

If that fails we fall-back to the other.

If that sounds reasonable, then the following can be reworked as follows:

> +#ifdef GLAMOR_HAS_GBM
> +/* Init GBM even if "-eglstream" is requested, as EGL streams may fail */
> +xwl_glamor_init_gbm(xwl_screen);
Here we add:

  if (!xwl_screen->gbm_backend.is_available && !use_eglstreams)
ErrorF(xwayland glamor: GBM (default) is not available);

> +#endif
>  #ifdef XWL_HAS_EGLSTREAM
> +xwl_glamor_init_eglstream(xwl_screen);

>  if (use_eglstreams) {
> -if (!xwl_glamor_init_eglstream(xwl_screen)) {
> -ErrorF("xwayland glamor: failed to setup eglstream backend\n");
> -use_eglstreams = FALSE;
> -}
> +/* Force GBM backend off */
> +xwl_screen->gbm_backend.is_available = FALSE;
This is no longer needed, and we can now fold the two conditionals -
just like the GBM codepath.

> +if (!xwl_screen->eglstreams_backend.is_available)
> +ErrorF("xwayland glamor: EGLstreams requested but not 
> available\n");
>  }
>  #endif
> -if (!use_eglstreams && !xwl_glamor_init_gbm(xwl_screen)) {
> -ErrorF("xwayland glamor: failed to setup GBM backend, falling back 
> to sw accel\n");
> -xwl_screen->glamor = 0;
> +}
> +
> +void
> +xwl_glamor_select_backend(struct xwl_screen *xwl_screen, Bool use_eglstreams)
> +{
> +if (xwl_screen->egl_backend == NULL && 
> xwl_screen->gbm_backend.is_available) {
> +if (xwl_glamor_has_wl_interfaces(xwl_screen, 
> _screen->gbm_backend))
> + 

Re: [PATCH xserver 3/5] xwayland: Add Wayland interfaces check

2018-06-05 Thread Emil Velikov
On 1 June 2018 at 15:31, Olivier Fourdan  wrote:

> +static Bool
> +xwl_glamor_gbm_has_wl_interfaces(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;
> +}
> +
Please add a comment about dmabuf. I know it's optional, but in the
long run we'd want it instead of wl_drm ;-)


> @@ -69,6 +69,11 @@ struct xwl_egl_backend {
>   const char *name, uint32_t id,
>   uint32_t version);
>
> +/* Check that the required Wayland interfaces are available. This
> + * callback is optional.
Since it's implemented by both backends, so I'd call it mandatory and
drop the NULL check in xwl_glamor_has_wl_interfaces.

-Emil
___
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 2/5] xwayland: move egl_backend to its own struct

2018-06-05 Thread Emil Velikov
On 1 June 2018 at 15:31, Olivier Fourdan  wrote:
> EGL backend availability requires both EGL extensions and Wayland
> interfaces to be present, so we will need to consider multiple backends
> during initialization.
>
> As a preliminary work, move the egl_backend to its own struct so that we
> can have more than one backend at any given time.
>
> Signed-off-by: Olivier Fourdan 
> ---
>  hw/xwayland/xwayland.h | 99 ++
>  1 file changed, 51 insertions(+), 48 deletions(-)
>
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index 0d4afdf8a..0d6a4c246 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -57,6 +57,56 @@ struct xwl_format {
>
>  struct xwl_pixmap;
>  struct xwl_window;
> +struct xwl_screen;
> +
> +struct xwl_egl_backend {
> +/* Called once for each interface in the global registry. Backends
> + * should use this to bind to any wayland interfaces they need. This
> + * callback is optional.
> + */
Since both backends implement this, I'd drop the "optional" statement
alongside the NULL check in the code.
Might be better to keep as separate patch though.

-Emil
___
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] Xext/shm: Refuse to work for remote clients

2018-06-05 Thread Alexander Volkov
Avoid access to System V shared memory segment on the X server side
for clients forwarded via SSH. Also prevent them from hanging while
waiting for the reply from the ShmCreateSegment request.

v2: Allow ShmQueryVersion request even for remote clients

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=11080
Signed-off-by: Alexander Volkov 
---
 Xext/shm.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/Xext/shm.c b/Xext/shm.c
index fc8441c43..896a966e3 100644
--- a/Xext/shm.c
+++ b/Xext/shm.c
@@ -1302,9 +1302,14 @@ static int
 ProcShmDispatch(ClientPtr client)
 {
 REQUEST(xReq);
-switch (stuff->data) {
-case X_ShmQueryVersion:
+
+if (stuff->data == X_ShmQueryVersion)
 return ProcShmQueryVersion(client);
+
+if (!client->local)
+return BadRequest;
+
+switch (stuff->data) {
 case X_ShmAttach:
 return ProcShmAttach(client);
 case X_ShmDetach:
@@ -1461,9 +1466,14 @@ static int _X_COLD
 SProcShmDispatch(ClientPtr client)
 {
 REQUEST(xReq);
-switch (stuff->data) {
-case X_ShmQueryVersion:
+
+if (stuff->data == X_ShmQueryVersion)
 return SProcShmQueryVersion(client);
+
+if (!client->local)
+return BadRequest;
+
+switch (stuff->data) {
 case X_ShmAttach:
 return SProcShmAttach(client);
 case X_ShmDetach:
-- 
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: Obtaining Xorg DDX commit privilege

2018-06-05 Thread Michel Dänzer
On 2018-06-05 05:00 AM, Kevin Brace wrote:
> 
> I do have OpenChrome related commit privilege and I wanted to extend it to 
> other xf86-video-* DDXs.
> I did post several patches over at xorg-driver-ati mailing list if that is 
> what is needed to earn this privilege.

Connor Behan has been looking after xf86-video-r128.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
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