[PATCH xserver 1/2] glamor: Unbreak glamor_fd_from_pixmap()

Lyude Paul lyude at redhat.com
Wed Jun 20 22:33:40 UTC 2018


When support for allocating GBM BOs with modifiers was added,
glamor_fd_from_pixmap() was changed so that it would return an error if
it got a bo with modifiers set from glamor_fds_from_pixmap(). The
problem is that on systems that support BOs with modifiers,
glamor_fds_from_pixmap() will always return BOs with modifiers.

This means that glamor_fd_from_pixmap() was broken entirely, which broke
a number of other things including glamor_shareable_fd_from_pixmap(),
which meant that modesetting using multiple GPUs with the modesetting
DDX was also broken. Easy reproducer:

- Find a laptop with DRI prime that has outputs connected to the
  dedicated GPU and integrated GPU
- Try to enable one display on each using the modesetting DDX
- Fail

Since there isn't a way to ask for no modifiers from
glamor_fds_from_pixmap, we create a shared _glamor_fds_from_pixmap()
function used by both glamor_fds_from_pixmap() and
glamor_fd_from_pixmap() that calls down to the appropriate
glamor_egl_fd*_from_pixmap() function.

Signed-off-by: Lyude Paul <lyude at redhat.com>
Cc: Louis-Francis Ratté-Boulianne <lfrb at collabora.com>
Fixes: c8c276c956 ("glamor: Implement PixmapFromBuffers and BuffersFromPixmap")
---
 glamor/glamor.c                   | 52 +++++++++++++++++--------------
 glamor/glamor.h                   |  3 +-
 glamor/glamor_egl.c               |  2 +-
 glamor/glamor_egl_stubs.c         |  7 +++++
 hw/xwayland/xwayland-glamor-gbm.c | 10 ++++++
 5 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 54ca0db35..aaa528545 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -826,10 +826,10 @@ glamor_get_drawable_modifiers(DrawablePtr draw, uint32_t format,
     return TRUE;
 }
 
-_X_EXPORT int
-glamor_fds_from_pixmap(ScreenPtr screen, PixmapPtr pixmap, int *fds,
-                       uint32_t *strides, uint32_t *offsets,
-                       uint64_t *modifier)
+static int
+_glamor_fds_from_pixmap(ScreenPtr screen, PixmapPtr pixmap, int *fds,
+                        void *strides, uint32_t *offsets,
+                        CARD32 *size, uint64_t *modifier)
 {
     glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap);
     glamor_screen_private *glamor_priv =
@@ -843,39 +843,43 @@ glamor_fds_from_pixmap(ScreenPtr screen, PixmapPtr pixmap, int *fds,
         if (!glamor_pixmap_ensure_fbo(pixmap, pixmap->drawable.depth == 30 ?
                                       GL_RGB10_A2 : GL_RGBA, 0))
             return 0;
-        return glamor_egl_fds_from_pixmap(screen, pixmap, fds,
-                                          strides, offsets,
-                                          modifier);
+
+        if (modifier) {
+            return glamor_egl_fds_from_pixmap(screen, pixmap, fds,
+                                              strides, offsets,
+                                              modifier);
+        } else {
+            fds[0] = glamor_egl_fd_from_pixmap(screen, pixmap, strides, size);
+            return fds[0] >= 0;
+        }
     default:
         break;
     }
     return 0;
 }
 
+_X_EXPORT int
+glamor_fds_from_pixmap(ScreenPtr screen, PixmapPtr pixmap, int *fds,
+                       uint32_t *strides, uint32_t *offsets,
+                       uint64_t *modifier)
+{
+    return _glamor_fds_from_pixmap(screen, pixmap, fds, strides, offsets,
+                                   NULL, modifier);
+}
+
 _X_EXPORT int
 glamor_fd_from_pixmap(ScreenPtr screen,
                       PixmapPtr pixmap, CARD16 *stride, CARD32 *size)
 {
+    int fd;
     int ret;
-    int fds[4];
-    uint32_t strides[4], offsets[4];
-    uint64_t modifier;
-
-    ret = glamor_fds_from_pixmap(screen, pixmap, fds, strides, offsets,
-                                 &modifier);
 
-    /* Pixmaps with multi-planes/modifier are not supported in this interface */
-    if (ret != 1 || offsets[0] != 0) {
-        while (ret > 0)
-            close(fds[--ret]);
+    ret = _glamor_fds_from_pixmap(screen, pixmap, &fd, stride, NULL, size,
+                                  NULL);
+    if (ret == 1)
+        return fd;
+    else
         return -1;
-    }
-
-    ret = fds[0];
-    *stride = strides[0];
-    *size = pixmap->drawable.height * *stride;
-
-    return ret;
 }
 
 _X_EXPORT int
diff --git a/glamor/glamor.h b/glamor/glamor.h
index 06e11506f..09e9c895c 100644
--- a/glamor/glamor.h
+++ b/glamor/glamor.h
@@ -141,7 +141,7 @@ extern _X_EXPORT void glamor_egl_exchange_buffers(PixmapPtr front,
 extern _X_EXPORT void glamor_pixmap_exchange_fbos(PixmapPtr front,
                                                   PixmapPtr back);
 
-/* The DDX is not supposed to call these three functions */
+/* The DDX is not supposed to call these four functions */
 extern _X_EXPORT void glamor_enable_dri3(ScreenPtr screen);
 extern _X_EXPORT int glamor_egl_fds_from_pixmap(ScreenPtr, PixmapPtr, int *,
                                                 uint32_t *, uint32_t *,
@@ -150,6 +150,7 @@ extern _X_EXPORT int glamor_egl_fd_name_from_pixmap(ScreenPtr, PixmapPtr,
                                                     CARD16 *, CARD32 *);
 
 extern _X_EXPORT struct gbm_device *glamor_egl_get_gbm_device(ScreenPtr screen);
+extern _X_EXPORT int glamor_egl_fd_from_pixmap(ScreenPtr, PixmapPtr, CARD16 *, CARD32 *);
 
 /* @glamor_supports_pixmap_import_export: Returns whether
  * glamor_fds_from_pixmap(), glamor_name_from_pixmap(), and
diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c
index 4a4ca4bd8..5a3b7f193 100644
--- a/glamor/glamor_egl.c
+++ b/glamor/glamor_egl.c
@@ -402,7 +402,7 @@ glamor_egl_fds_from_pixmap(ScreenPtr screen, PixmapPtr pixmap, int *fds,
 #endif
 }
 
-static int
+_X_EXPORT int
 glamor_egl_fd_from_pixmap(ScreenPtr screen, PixmapPtr pixmap,
                           CARD16 *stride, CARD32 *size)
 {
diff --git a/glamor/glamor_egl_stubs.c b/glamor/glamor_egl_stubs.c
index aae909e9f..91ab9a7ae 100644
--- a/glamor/glamor_egl_stubs.c
+++ b/glamor/glamor_egl_stubs.c
@@ -51,3 +51,10 @@ glamor_egl_fds_from_pixmap(ScreenPtr screen, PixmapPtr pixmap, int *fds,
 {
     return 0;
 }
+
+int
+glamor_egl_fd_from_pixmap(ScreenPtr screen, PixmapPtr pixmap,
+                          CARD16 *stride, CARD32 *size)
+{
+    return -1;
+}
diff --git a/hw/xwayland/xwayland-glamor-gbm.c b/hw/xwayland/xwayland-glamor-gbm.c
index 68c2cc32e..ab067a740 100644
--- a/hw/xwayland/xwayland-glamor-gbm.c
+++ b/hw/xwayland/xwayland-glamor-gbm.c
@@ -517,6 +517,16 @@ glamor_egl_fds_from_pixmap(ScreenPtr screen, PixmapPtr pixmap, int *fds,
 #endif
 }
 
+/* Not actually used, just defined here so there's something for
+ * _glamor_egl_fds_from_pixmap() to link against
+ */
+_X_EXPORT int
+glamor_egl_fd_from_pixmap(ScreenPtr screen, PixmapPtr pixmap,
+                          CARD16 *stride, CARD32 *size)
+{
+    return -1;
+}
+
 _X_EXPORT Bool
 glamor_get_formats(ScreenPtr screen,
                    CARD32 *num_formats, CARD32 **formats)
-- 
2.17.1



More information about the xorg-devel mailing list