[Mesa-dev] [RFC] pipe-loader: abstract GALLIUM_STATIC_TARGETS behind pipe_loader API

Rob Clark robdclark at gmail.com
Wed Sep 30 13:43:42 PDT 2015


From: Rob Clark <robclark at freedesktop.org>

Not actually working yet, ie. doesn't even compile yet, but an idea.

Initial motivation was for drm_gralloc/pipe, which is essentially a sort
of mini state-tracker, that needs to be able to share pipe_screen with
libGL linked into the same process (to ensure we didn't end up with
duplicate pipe_resource's, etc).  I think same situation happens with
vdpau+gl interop.

Currently drm_gralloc/pipe (and other state trackers??) statically link
the winsys code, which completely defeats the purpose magic in the
winsys code to detect when the same device is opened multiple times and
return the same pipe_screen (since you end up w/ multiple copies of the
hashtable in the same process).  See for example:

  5bb41d9094b3c9bdf0669fd55418981ed83347e3
  fee0686c21c631d96d6042741267a3c218c23ffc

Rough idea is that we should have something like libgallium.so which
contains the pipe-loader API, and then optionally (depending on
GALLIUM_STATIC_TARGETS) all of the gallium pipe drivers.  The various
different state trackers would link against (or dlopen()) libgallium.so
and use the pipe-loader API to get themselves a pipe_screen (and config
params, etc).  And then you end up with:

                +---+
                | l |
    clover  --> | i |
                | b |
    mesa-st --> | g |
                | a |--> pipe driver
    vdapau  --> | l |
                | l |
    gralloc --> | i |
                | u |
    xa      --> | m |
                |   |
                +---+
or:
                +---+-------------+
                | l |             |
    clover  --> | i |             |
                | b |             |
    mesa-st --> | g |             |
                | a | pipe driver |
    vdapau  --> | l |             |
                | l |             |
    gralloc --> | i |             |
                | u |             |
    xa      --> | m |             |
                |   |             |
                +---+-------------+

depending on GALLIUM_STATIC_TARGETS.  Either way, all the state trackers
in the same process share a single copy of the hashtable in the winsys
code which allows them to share the same pipe_screen.

I think that ends up being an extra level of library indirection vs
current state w/ pipe drivers, ie. with mesa dri loader stuff directly
loading gallium_dri.so which contains all the drivers plus mesa state
tracker.  If this was concerning to someone, what I'd suggest would be
to, for all the state trackers that already have some sort of loader
sitting between them and the user, just pull them directly into the
mega-mega libgallium.so, ie. something like:

      +---------+---+-------------+
      |         |   |             |
      | clover  | l |             |
      |         | i |             |
      | mesa-st | b |             |
      |         | g | pipe driver |
      | vdapau  | a |             |
      |         | l |             |
      +---------| l |             |
                | i |             |
    gralloc --> | u |             |
                | m |             |
    xa      --> |   |             |
                |   |             |
                +---+-------------+

Anyways, I'm far from an expert in the build architecture of things so
I might have some facts wrong or be a bit confused.  And I'll probably
regret bringing the subject up.  But somehow or another it seems like
it would be good to (a) clean up all the GALLIUM_STATIC_TARGETS
ifdeffery spread throughout all the different state trackers, and (b)
have the pipe driver[*] only exist once per process rather than once per
state-tracker.  Especially for android where each process using GL will
have both gralloc and mesa-st.. and perhaps even clover/omx/etc.

[*] in particular the winsys code that ensures there is but one screen,
but once you've done that you might as well go the whole way.
---
 configure.ac                                        |  6 ------
 src/gallium/auxiliary/pipe-loader/Makefile.am       | 10 ++--------
 src/gallium/auxiliary/pipe-loader/pipe_loader.c     |  2 --
 src/gallium/auxiliary/pipe-loader/pipe_loader.h     |  4 ----
 src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c | 12 ++++++++++++
 src/gallium/auxiliary/vl/vl_winsys_dri.c            |  8 --------
 src/gallium/state_trackers/dri/dri2.c               | 13 -------------
 src/gallium/state_trackers/dri/dri_screen.c         |  2 --
 src/gallium/state_trackers/xa/xa_tracker.c          | 10 +---------
 9 files changed, 15 insertions(+), 52 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1ef5fbc..7d0f9d9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2044,7 +2044,6 @@ gallium_require_drm_loader() {
         if test "x$need_pci_id$have_pci_id" = xyesno; then
             AC_MSG_ERROR([Gallium drm loader requires libudev >= $LIBUDEV_REQUIRED or sysfs])
         fi
-        enable_gallium_drm_loader=yes
     fi
     if test "x$enable_va" = xyes && test "x$7" != x; then
          GALLIUM_TARGET_DIRS="$GALLIUM_TARGET_DIRS $7"
@@ -2248,10 +2247,6 @@ if test "x$enable_gallium_loader" = xyes; then
         GALLIUM_PIPE_LOADER_DEFINES="$GALLIUM_PIPE_LOADER_DEFINES -DHAVE_PIPE_LOADER_DRI"
     fi
 
-    if test "x$enable_gallium_drm_loader" = xyes; then
-        GALLIUM_PIPE_LOADER_DEFINES="$GALLIUM_PIPE_LOADER_DEFINES -DHAVE_PIPE_LOADER_DRM"
-    fi
-
     AC_SUBST([GALLIUM_PIPE_LOADER_DEFINES])
 fi
 
@@ -2269,7 +2264,6 @@ AM_CONDITIONAL(NEED_WINSYS_XLIB, test "x$NEED_WINSYS_XLIB" = xyes)
 AM_CONDITIONAL(NEED_RADEON_LLVM, test x$NEED_RADEON_LLVM = xyes)
 AM_CONDITIONAL(USE_R600_LLVM_COMPILER, test x$USE_R600_LLVM_COMPILER = xyes)
 AM_CONDITIONAL(HAVE_LOADER_GALLIUM, test x$enable_gallium_loader = xyes)
-AM_CONDITIONAL(HAVE_DRM_LOADER_GALLIUM, test x$enable_gallium_drm_loader = xyes)
 AM_CONDITIONAL(HAVE_GALLIUM_COMPUTE, test x$enable_opencl = xyes)
 AM_CONDITIONAL(HAVE_MESA_LLVM, test x$MESA_LLVM = x1)
 AM_CONDITIONAL(USE_VC4_SIMULATOR, test x$USE_VC4_SIMULATOR = xyes)
diff --git a/src/gallium/auxiliary/pipe-loader/Makefile.am b/src/gallium/auxiliary/pipe-loader/Makefile.am
index 8c83799..616b785 100644
--- a/src/gallium/auxiliary/pipe-loader/Makefile.am
+++ b/src/gallium/auxiliary/pipe-loader/Makefile.am
@@ -5,6 +5,7 @@ include $(top_srcdir)/src/gallium/Automake.inc
 AM_CFLAGS = \
 	-I$(top_srcdir)/src/loader \
 	-I$(top_srcdir)/src/gallium/winsys \
+	$(LIBDRM_CFLAGS) \
 	$(GALLIUM_PIPE_LOADER_DEFINES) \
 	$(GALLIUM_CFLAGS) \
 	$(VISIBILITY_CFLAGS)
@@ -12,17 +13,10 @@ AM_CFLAGS = \
 noinst_LTLIBRARIES = libpipe_loader.la
 
 libpipe_loader_la_SOURCES = \
-	$(COMMON_SOURCES)
-
-if HAVE_DRM_LOADER_GALLIUM
-AM_CFLAGS += \
-	$(LIBDRM_CFLAGS)
-
-libpipe_loader_la_SOURCES += \
+	$(COMMON_SOURCES) \
 	$(DRM_SOURCES)
 
 libpipe_loader_la_LIBADD = \
 	$(top_builddir)/src/loader/libloader.la
 
-endif
 
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
index 8e79f85..535c2ce 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c
@@ -35,9 +35,7 @@
 #define MODULE_PREFIX "pipe_"
 
 static int (*backends[])(struct pipe_loader_device **, int) = {
-#ifdef HAVE_PIPE_LOADER_DRM
    &pipe_loader_drm_probe,
-#endif
    &pipe_loader_sw_probe
 };
 
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
index 9b87126..cf0c51b 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
@@ -158,8 +158,6 @@ boolean
 pipe_loader_sw_probe_wrapped(struct pipe_loader_device **dev,
                              struct pipe_screen *screen);
 
-#ifdef HAVE_PIPE_LOADER_DRM
-
 /**
  * Get a list of known DRM devices.
  *
@@ -180,8 +178,6 @@ pipe_loader_drm_probe(struct pipe_loader_device **devs, int ndev);
 bool
 pipe_loader_drm_probe_fd(struct pipe_loader_device **dev, int fd);
 
-#endif
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
index 1799df7..70e1dba 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_drm.c
@@ -50,7 +50,9 @@
 
 struct pipe_loader_drm_device {
    struct pipe_loader_device base;
+#if !GALLIUM_STATIC_TARGETS
    struct util_dl_library *lib;
+#endif
    int fd;
 };
 
@@ -132,8 +134,10 @@ pipe_loader_drm_release(struct pipe_loader_device **dev)
 {
    struct pipe_loader_drm_device *ddev = pipe_loader_drm_device(*dev);
 
+#if !GALLIUM_STATIC_TARGETS
    if (ddev->lib)
       util_dl_close(ddev->lib);
+#endif
 
    close(ddev->fd);
    FREE(ddev->base.driver_name);
@@ -145,6 +149,9 @@ static const struct drm_conf_ret *
 pipe_loader_drm_configuration(struct pipe_loader_device *dev,
                               enum drm_conf conf)
 {
+#if GALLIUM_STATIC_TARGETS
+   return dd_configuration(conf);
+#else
    struct pipe_loader_drm_device *ddev = pipe_loader_drm_device(dev);
    const struct drm_driver_descriptor *dd;
 
@@ -162,6 +169,7 @@ pipe_loader_drm_configuration(struct pipe_loader_device *dev,
       return NULL;
 
    return dd->configuration(conf);
+#endif
 }
 
 static struct pipe_screen *
@@ -169,6 +177,9 @@ pipe_loader_drm_create_screen(struct pipe_loader_device *dev,
                               const char *library_paths)
 {
    struct pipe_loader_drm_device *ddev = pipe_loader_drm_device(dev);
+#if GALLIUM_STATIC_TARGETS
+   return dd_create_screen(ddev->fd);
+#else
    const struct drm_driver_descriptor *dd;
 
    if (!ddev->lib)
@@ -184,6 +195,7 @@ pipe_loader_drm_create_screen(struct pipe_loader_device *dev,
       return NULL;
 
    return dd->create_screen(ddev->fd);
+#endif
 }
 
 static struct pipe_loader_ops pipe_loader_drm_ops = {
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri.c b/src/gallium/auxiliary/vl/vl_winsys_dri.c
index 3b1b87f..c5c87c8 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri.c
@@ -387,12 +387,8 @@ vl_screen_create(Display *display, int screen)
    if (authenticate == NULL || !authenticate->authenticated)
       goto free_authenticate;
 
-#if GALLIUM_STATIC_TARGETS
-   scrn->base.pscreen = dd_create_screen(fd);
-#else
    if (pipe_loader_drm_probe_fd(&scrn->base.dev, fd))
       scrn->base.pscreen = pipe_loader_create_screen(scrn->base.dev, PIPE_SEARCH_DIR);
-#endif // GALLIUM_STATIC_TARGETS
 
    if (!scrn->base.pscreen)
       goto release_pipe;
@@ -409,10 +405,8 @@ vl_screen_create(Display *display, int screen)
    return &scrn->base;
 
 release_pipe:
-#if !GALLIUM_STATIC_TARGETS
    if (scrn->base.dev)
       pipe_loader_release(&scrn->base.dev, 1);
-#endif // !GALLIUM_STATIC_TARGETS
 free_authenticate:
    free(authenticate);
 free_connect:
@@ -440,8 +434,6 @@ void vl_screen_destroy(struct vl_screen *vscreen)
 
    vl_dri2_destroy_drawable(scrn);
    scrn->base.pscreen->destroy(scrn->base.pscreen);
-#if !GALLIUM_STATIC_TARGETS
    pipe_loader_release(&scrn->base.dev, 1);
-#endif // !GALLIUM_STATIC_TARGETS
    FREE(scrn);
 }
diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c
index 91b4431..a24acdd 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -1454,19 +1454,12 @@ dri2_init_screen(__DRIscreen * sPriv)
 
    sPriv->driverPrivate = (void *)screen;
 
-#if GALLIUM_STATIC_TARGETS
-   pscreen = dd_create_screen(screen->fd);
-
-   throttle_ret = dd_configuration(DRM_CONF_THROTTLE);
-   dmabuf_ret = dd_configuration(DRM_CONF_SHARE_FD);
-#else
    if (pipe_loader_drm_probe_fd(&screen->dev, screen->fd)) {
       pscreen = pipe_loader_create_screen(screen->dev, PIPE_SEARCH_DIR);
 
       throttle_ret = pipe_loader_configuration(screen->dev, DRM_CONF_THROTTLE);
       dmabuf_ret = pipe_loader_configuration(screen->dev, DRM_CONF_SHARE_FD);
    }
-#endif // GALLIUM_STATIC_TARGETS
 
    if (throttle_ret && throttle_ret->val.val_int != -1) {
       screen->throttling_enabled = TRUE;
@@ -1492,11 +1485,7 @@ dri2_init_screen(__DRIscreen * sPriv)
 
    /* dri_init_screen_helper checks pscreen for us */
 
-#if GALLIUM_STATIC_TARGETS
-   configs = dri_init_screen_helper(screen, pscreen, dd_driver_name());
-#else
    configs = dri_init_screen_helper(screen, pscreen, screen->dev->driver_name);
-#endif // GALLIUM_STATIC_TARGETS
    if (!configs)
       goto fail;
 
@@ -1508,10 +1497,8 @@ dri2_init_screen(__DRIscreen * sPriv)
    return configs;
 fail:
    dri_destroy_screen_helper(screen);
-#if !GALLIUM_STATIC_TARGETS
    if (screen->dev)
       pipe_loader_release(&screen->dev, 1);
-#endif // !GALLIUM_STATIC_TARGETS
    FREE(screen);
    return NULL;
 }
diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c
index c4c2d9c..cf0f265 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -390,9 +390,7 @@ dri_destroy_screen(__DRIscreen * sPriv)
 
    dri_destroy_screen_helper(screen);
 
-#if !GALLIUM_STATIC_TARGETS
    pipe_loader_release(&screen->dev, 1);
-#endif // !GALLIUM_STATIC_TARGETS
 
    free(screen);
    sPriv->driverPrivate = NULL;
diff --git a/src/gallium/state_trackers/xa/xa_tracker.c b/src/gallium/state_trackers/xa/xa_tracker.c
index 4fdbdc9..598d540 100644
--- a/src/gallium/state_trackers/xa/xa_tracker.c
+++ b/src/gallium/state_trackers/xa/xa_tracker.c
@@ -157,16 +157,12 @@ xa_tracker_create(int drm_fd)
     if (!xa)
 	return NULL;
 
-#if GALLIUM_STATIC_TARGETS
-    xa->screen = dd_create_screen(drm_fd);
-    (void) loader_fd; /* silence unused var warning */
-#else
     loader_fd = dup(drm_fd);
     if (loader_fd == -1)
         return NULL;
     if (pipe_loader_drm_probe_fd(&xa->dev, loader_fd))
 	xa->screen = pipe_loader_create_screen(xa->dev, PIPE_SEARCH_DIR);
-#endif
+
     if (!xa->screen)
 	goto out_no_screen;
 
@@ -214,10 +210,8 @@ xa_tracker_create(int drm_fd)
  out_no_pipe:
     xa->screen->destroy(xa->screen);
  out_no_screen:
-#if !GALLIUM_STATIC_TARGETS
     if (xa->dev)
 	pipe_loader_release(&xa->dev, 1);
-#endif
     free(xa);
     return NULL;
 }
@@ -228,9 +222,7 @@ xa_tracker_destroy(struct xa_tracker *xa)
     free(xa->supported_formats);
     xa_context_destroy(xa->default_ctx);
     xa->screen->destroy(xa->screen);
-#if !GALLIUM_STATIC_TARGETS
     pipe_loader_release(&xa->dev, 1);
-#endif
     free(xa);
 }
 
-- 
2.4.3



More information about the mesa-dev mailing list