[Mesa-dev] [PATCH 09/15] i965: Build the driver into a shared mesa_dri_drivers.so .

Chad Versace chad.versace at linux.intel.com
Thu Oct 17 01:56:06 CEST 2013


The bits about globalDriverAPI and megadriver_stub.c feel strongly
logically disjoint from the rest of the patch. And, they really aren't
related to i965. It looks like they should get separated out into a
separate patch the immediately precedes this one.

Also, as I said in my first reply, this patch breaks my build.

That's all.

On 10/11/2013 06:03 PM, Eric Anholt wrote:
> Previously, we've split things such that mesa core is in libdricore,
> exposing the whole Mesa core interface in the global namespace, and the
> i965_dri.so code all links against that.  Along with polluting application
> namespace terribly, it requires extra PLT indirections and prevents LTO.
>
> Instead, we can build all of the driver contents into the same .so with
> just a few symbols exposed to be referenced from the actual driver .so
> file, allowing LTO and reducing our exposed symbol count massively.
>
> FPS improvement on GLB2.7 with INTEL_NO_HW=1: 2.61061% +/- 1.16957% (n=50)
> (without LTO, just the PLT reductions from this commit)
>
> v2: Set a global driverAPI variable so loaders don't have to update to
>      createNewScreen2() (though they may want to for thread safety).
>
> Reviewed-by: Matt Turner <mattst88 at gmail.com> (v1)
> ---
>   configure.ac                                  | 29 +++++++++++---
>   src/mesa/drivers/dri/Makefile.am              | 54 ++++++++++++++++++++++++++-
>   src/mesa/drivers/dri/common/Makefile.am       |  3 ++
>   src/mesa/drivers/dri/common/dri_util.c        | 19 +++++++++-
>   src/mesa/drivers/dri/common/dri_util.h        |  2 +-
>   src/mesa/drivers/dri/common/megadriver_stub.c | 41 ++++++++++++++++++++
>   src/mesa/drivers/dri/i965/Makefile.am         | 27 +++-----------
>   src/mesa/drivers/dri/i965/intel_screen.c      | 18 +++++++--
>   src/mesa/drivers/dri/i965/intel_screen.h      |  2 +
>   9 files changed, 162 insertions(+), 33 deletions(-)
>   create mode 100644 src/mesa/drivers/dri/common/megadriver_stub.c
>
> diff --git a/configure.ac b/configure.ac
> index 0d082d2..f7f165f 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -705,8 +705,6 @@ fi
>   AM_CONDITIONAL(HAVE_DRI_GLX, test "x$enable_glx" = xyes -a \
>                                     "x$enable_dri" = xyes)
>   AM_CONDITIONAL(HAVE_DRI, test "x$enable_dri" = xyes)
> -AM_CONDITIONAL(NEED_LIBMESA, test "x$enable_xlib_glx" = xyes -o \
> -                                  "x$enable_osmesa" = xyes)
>
>   AC_ARG_ENABLE([shared-glapi],
>       [AS_HELP_STRING([--enable-shared-glapi],
> @@ -858,8 +856,6 @@ AC_SUBST([GLESv1_CM_PC_LIB_PRIV])
>   AC_SUBST([GLESv2_LIB_DEPS])
>   AC_SUBST([GLESv2_PC_LIB_PRIV])
>
> -DRI_LIB_DEPS="\$(top_builddir)/src/mesa/libdricore/libdricore${VERSION}.la"
> -
>   AC_SUBST([HAVE_XF86VIDMODE])
>
>   dnl
> @@ -1035,10 +1031,32 @@ if test "x$enable_dri" = xyes; then
>
>       DRI_DRIVER_LDFLAGS="-module -avoid-version -shared -Wl,-Bsymbolic"
>   fi
> -AM_CONDITIONAL(NEED_LIBDRICORE, test -n "$DRI_DIRS")
> +
> +enable_dricore=no
> +enable_megadriver=no
> +for driver in $DRI_DIRS; do
> +    if test $driver != "i965"; then
> +        enable_dricore=yes
> +    else
> +        enable_megadriver=yes
> +    fi
> +done
> +
> +# megadriver wants to use libmesa.la, while non-megadrivers want to
> +# automatically get libdricore.  Some day hopefully we'll transition
> +# everything to megadriver.
> +MEGADRIVER_DRI_LIB_DEPS=$DRI_LIB_DEPS
> +DRI_LIB_DEPS="\$(top_builddir)/src/mesa/libdricore/libdricore${VERSION}.la $DRI_LIB_DEPS"
> +
> +AM_CONDITIONAL(NEED_LIBDRICORE, test "x$enable_dricore" = xyes)
> +AM_CONDITIONAL(NEED_MEGADRIVER, test "x$enable_megadriver" = xyes)
> +AM_CONDITIONAL(NEED_LIBMESA, test "x$enable_xlib_glx" = xyes -o \
> +                                  "x$enable_osmesa" = xyes -o \
> +                                  "x$enable_megadriver" = xyes)
>   AC_SUBST([EXPAT_INCLUDES])
>   AC_SUBST([DRI_LIB_DEPS])
>   AC_SUBST([DRI_DRIVER_LDFLAGS])
> +AC_SUBST([MEGADRIVER_DRI_LIB_DEPS])
>   AC_SUBST([GALLIUM_DRI_LIB_DEPS])
>
>   case $DRI_DIRS in
> @@ -1950,6 +1968,7 @@ AC_SUBST([ELF_LIB])
>   AM_CONDITIONAL(NEED_LIBPROGRAM, test "x$with_gallium_drivers" != x -o \
>                                        "x$enable_xlib_glx" = xyes -o \
>                                        "x$enable_osmesa" = xyes -o \
> +                                     "x$enable_megadriver" = xyes -o \
>                                        "x$enable_gallium_osmesa" = xyes)
>   AM_CONDITIONAL(HAVE_X11_DRIVER, test "x$enable_xlib_glx" = xyes)
>   AM_CONDITIONAL(HAVE_OSMESA, test "x$enable_osmesa" = xyes)
> diff --git a/src/mesa/drivers/dri/Makefile.am b/src/mesa/drivers/dri/Makefile.am
> index 48d3685..9d15c43 100644
> --- a/src/mesa/drivers/dri/Makefile.am
> +++ b/src/mesa/drivers/dri/Makefile.am
> @@ -1,4 +1,15 @@
> +dridir = $(DRI_DRIVER_INSTALL_DIR)
> +
> +AM_CPPFLAGS = \
> +	-I$(top_srcdir)/src/mesa/ \
> +	-I$(top_srcdir)/src/mapi/ \
> +        -I$(top_srcdir)/src/mesa/drivers/dri/common \
> +        $(LIBDRM_CFLAGS) \
> +        $()
> +
>   SUBDIRS =
> +MEGADRIVERS =
> +MEGADRIVERS_DEPS =
>
>   if HAVE_COMMON_DRI
>   SUBDIRS+=common
> @@ -9,7 +20,9 @@ SUBDIRS+=i915
>   endif
>
>   if HAVE_I965_DRI
> -SUBDIRS+=i965
> +SUBDIRS += i965
> +MEGADRIVERS_DEPS += i965/libi965_dri.la
> +MEGADRIVERS += i965_dri.so
>   endif
>
>   if HAVE_NOUVEAU_DRI
> @@ -33,3 +46,42 @@ pkgconfig_DATA = dri.pc
>
>   driincludedir = $(includedir)/GL/internal
>   driinclude_HEADERS = $(top_srcdir)/include/GL/internal/dri_interface.h
> +
> +nodist_EXTRA_mesa_dri_drivers_la_SOURCES = dummy.cpp
> +mesa_dri_drivers_la_SOURCES =
> +mesa_dri_drivers_la_LDFLAGS = \
> +        -module -avoid-version -shared \
> +        -Wl,-Bsymbolic \
> +        $()
> +mesa_dri_drivers_la_LIBADD = \
> +        ../../libmesa.la \
> +        common/libmegadriver_stub.la \
> +        common/libdricommon.la \
> +        $(MEGADRIVERS_DEPS) \
> +        $(MEGADRIVER_DRI_LIB_DEPS) \
> +        $()
> +
> +if NEED_MEGADRIVER
> +dri_LTLIBRARIES = mesa_dri_drivers.la
> +
> +# Add a link to allow setting LD_LIBRARY_PATH/LIBGL_DRIVERS_PATH to /lib of the build tree.
> +all-local: mesa_dri_drivers.la
> +	$(MKDIR_P) $(top_builddir)/$(LIB_DIR);
> +	$(AM_V_GEN)ln -f .libs/mesa_dri_drivers.so \
> +			 $(top_builddir)/$(LIB_DIR)/mesa_dri_drivers.so;
> +	$(AM_V_GEN)for i in $(MEGADRIVERS); do \
> +		ln -f $(top_builddir)/$(LIB_DIR)/mesa_dri_drivers.so \
> +		      $(top_builddir)/$(LIB_DIR)/$$i; \
> +	done;
> +
> +# hardlink each megadriver instance, but don't actually have
> +# mesa_dri_drivers.so in the set of final installed files.
> +install-data-hook:
> +	for i in $(MEGADRIVERS); do \
> +		ln -f $(dridir)/mesa_dri_drivers.so \
> +		      $(dridir)/$$i; \
> +	done;
> +	$(RM) -f $(dridir)/mesa_dri_drivers.so
> +	$(RM) -f $(dridir)/mesa_dri_drivers.la
> +
> +endif
> diff --git a/src/mesa/drivers/dri/common/Makefile.am b/src/mesa/drivers/dri/common/Makefile.am
> index ce4119d..e9c4aca 100644
> --- a/src/mesa/drivers/dri/common/Makefile.am
> +++ b/src/mesa/drivers/dri/common/Makefile.am
> @@ -32,6 +32,7 @@ AM_CFLAGS = \
>
>   noinst_LTLIBRARIES = \
>   	libdricommon.la \
> +	libmegadriver_stub.la \
>   	libdri_test_stubs.la
>
>   libdricommon_la_SOURCES = \
> @@ -43,4 +44,6 @@ libdri_test_stubs_la_SOURCES = \
>   	dri_test.c
>   libdri_test_stubs_la_CFLAGS = $(AM_CFLAGS) -DNO_MAIN
>
> +libmegadriver_stub_la_SOURCES = megadriver_stub.c
> +
>   sysconf_DATA = drirc
> diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c
> index c9110d6..3b360fd 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -81,6 +81,23 @@ setupLoaderExtensions(__DRIscreen *psp,
>   }
>
>   /**
> + * This pointer determines which driver API we'll use in the case of the
> + * loader not passing us an explicit driver extensions list (that would,
> + * itself, contain a pointer to a driver API.)
> + *
> + * A driver's driDriverGetExtensions_drivername() can update this pointer to
> + * what it's returning, and a loader that is ignorant of createNewScreen2()
> + * will get the correct driver screen created, as long as no other
> + * driDriverGetExtensions() happened in between the first one and the
> + * createNewScreen().
> + *
> + * This allows the X Server to not require the significant dri_interface.h
> + * updates for doing createNewScreen2(), which would discourage backporting of
> + * the X Server patches to support the new loader interface.
> + */
> +const struct __DriverAPIRec *globalDriverAPI = &driDriverAPI;
> +
> +/**
>    * This is the first entrypoint in the driver called by the DRI driver loader
>    * after dlopen()ing it.
>    *
> @@ -101,7 +118,7 @@ dri2CreateNewScreen2(int scrn, int fd,
>   	return NULL;
>
>       /* By default, use the global driDriverAPI symbol (non-megadrivers). */
> -    psp->driver = &driDriverAPI;
> +    psp->driver = globalDriverAPI;
>
>       /* If the driver exposes its vtable through its extensions list
>        * (megadrivers), use that instead.
> diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h
> index 61c80bc..5b56061 100644
> --- a/src/mesa/drivers/dri/common/dri_util.h
> +++ b/src/mesa/drivers/dri/common/dri_util.h
> @@ -116,7 +116,7 @@ struct __DriverAPIRec {
>   };
>
>   extern const struct __DriverAPIRec driDriverAPI;
> -
> +extern const struct __DriverAPIRec *globalDriverAPI;
>
>   /**
>    * Per-screen private driver information.
> diff --git a/src/mesa/drivers/dri/common/megadriver_stub.c b/src/mesa/drivers/dri/common/megadriver_stub.c
> new file mode 100644
> index 0000000..6bf5d73
> --- /dev/null
> +++ b/src/mesa/drivers/dri/common/megadriver_stub.c
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include <stdio.h>
> +#include "dri_util.h"
> +
> +static const
> +__DRIconfig **stub_error_init_screen(__DRIscreen *psp)
> +{
> +   fprintf(stderr, "An updated DRI driver loader (libGL.so or X Server) is "
> +           "required for this Mesa driver.\n");
> +   return NULL;
> +}
> +
> +/**
> + * This is a stub driDriverAPI that is referenced by dri_util.c but should
> + * never be used.
> + */
> +const struct __DriverAPIRec driDriverAPI = {
> +   .InitScreen = stub_error_init_screen,
> +};
> diff --git a/src/mesa/drivers/dri/i965/Makefile.am b/src/mesa/drivers/dri/i965/Makefile.am
> index 71442dd..589a074 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.am
> +++ b/src/mesa/drivers/dri/i965/Makefile.am
> @@ -38,30 +38,19 @@ AM_CFLAGS = \
>
>   AM_CXXFLAGS = $(AM_CFLAGS)
>
> -dridir = $(DRI_DRIVER_INSTALL_DIR)
> -
>   noinst_LTLIBRARIES = libi965_dri.la
> -dri_LTLIBRARIES = i965_dri.la
> -
>   libi965_dri_la_SOURCES = $(i965_FILES)
> +libi965_dri_la_LIBADD = $(INTEL_LIBS)
>
> -# list of libs to be linked against by i965_dri.so and i965 test programs.
> -COMMON_LIBS = \
> +TEST_LIBS = \
>   	libi965_dri.la \
>   	../common/libdricommon.la \
> -	$(DRI_LIB_DEPS) \
> -	$(INTEL_LIBS)
> -
> -TEST_LIBS = \
> -	$(COMMON_LIBS) \
> +	../common/libmegadriver_stub.la \
> +	$(MEGADRIVER_DRI_LIB_DEPS) \
> +        ../../../libmesa.la \
>           -lrt \
>   	../common/libdri_test_stubs.la
>
> -i965_dri_la_SOURCES =
> -nodist_EXTRA_i965_dri_la_SOURCES = dummy2.cpp
> -i965_dri_la_LIBADD = $(COMMON_LIBS)
> -i965_dri_la_LDFLAGS = $(DRI_DRIVER_LDFLAGS)
> -
>   TESTS = \
>           test_eu_compact \
>           test_vec4_register_coalesce
> @@ -77,9 +66,3 @@ test_eu_compact_SOURCES = \
>   	test_eu_compact.c
>   nodist_EXTRA_test_eu_compact_SOURCES = dummy.cpp
>   test_eu_compact_LDADD = $(TEST_LIBS)
> -
> -# Provide compatibility with scripts for the old Mesa build system for
> -# a while by putting a link to the driver into /lib of the build tree.
> -all-local: i965_dri.la
> -	$(MKDIR_P) $(top_builddir)/$(LIB_DIR);
> -	ln -f .libs/i965_dri.so $(top_builddir)/$(LIB_DIR)/i965_dri.so;
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 94093ea..cc27b93 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1345,7 +1345,7 @@ intelReleaseBuffer(__DRIscreen *screen, __DRIbuffer *buffer)
>   }
>
>
> -const struct __DriverAPIRec driDriverAPI = {
> +static const struct __DriverAPIRec brw_driver_api = {
>      .InitScreen		 = intelInitScreen2,
>      .DestroyScreen	 = intelDestroyScreen,
>      .CreateContext	 = brwCreateContext,
> @@ -1358,10 +1358,22 @@ const struct __DriverAPIRec driDriverAPI = {
>      .ReleaseBuffer        = intelReleaseBuffer
>   };
>
> -/* This is the table of extensions that the loader will dlsym() for. */
> -PUBLIC const __DRIextension *__driDriverExtensions[] = {
> +static const struct __DRIDriverVtableExtensionRec brw_vtable = {
> +   .base = { __DRI_DRIVER_VTABLE, 1 },
> +   .vtable = &brw_driver_api,
> +};
> +
> +static const __DRIextension *brw_driver_extensions[] = {
>       &driCoreExtension.base,
>       &driDRI2Extension.base,
> +    &brw_vtable.base,
>       &brw_config_options.base,
>       NULL
>   };
> +
> +PUBLIC const __DRIextension **__driDriverGetExtensions_i965(void)
> +{
> +   globalDriverAPI = &brw_driver_api;
> +
> +   return brw_driver_extensions;
> +}
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index fef17bc..b9cbb94 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -72,6 +72,8 @@ extern void intelDestroyContext(__DRIcontext * driContextPriv);
>
>   extern GLboolean intelUnbindContext(__DRIcontext * driContextPriv);
>
> +PUBLIC const __DRIextension **__driDriverGetExtensions_i965(void);
> +
>   extern GLboolean
>   intelMakeCurrent(__DRIcontext * driContextPriv,
>                    __DRIdrawable * driDrawPriv,
>



More information about the mesa-dev mailing list