[Mesa-dev] [PATCH] loader: ifdef libdrm specific code and include

[Kenneth Graunke]

On 01/18/2014 07:23 PM, Emil Velikov wrote:
> Mesa provides the flexibility of building without the
> need to have libdrm present on the system. The situation
> has regressed with the recent commit
> 
> This is a regression caused by commit
> commit 8c2e7fd8460750543367053b1be9368cc38e1d6a
> Author: Emil Velikov <emil.l.velikov at gmail.com>
> Date:   Fri Jan 10 23:36:16 2014 +0000
> 
>     loader: introduce the loader util lib
> 
> By isolating libdrm code by #ifndef __NOT_HAVE_DRM_H we
> can have libdrm-less builds on across all build systems.
> 
> This patch converts Android's _EGL_NO_DRM to __NOT_HAVE_DRM_H
> to provide consistency with the other cases within mesa, allows
> compilation of libloader on scons and conditionally links
> against libdrm if present under automake.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73776
> BUgzilla: https://bugs.freedesktop.org/show_bug.cgi?id=73777
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>  configure.ac           |  1 +
>  src/loader/Android.mk  |  2 +-
>  src/loader/Makefile.am | 13 +++++++++++--
>  src/loader/SConscript  |  5 +----
>  src/loader/loader.c    |  7 ++++++-
>  5 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 9ca6704..d9e1896 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1935,6 +1935,7 @@ AM_CONDITIONAL(HAVE_MESA_LLVM, test x$MESA_LLVM = x1)
>  AC_SUBST([ELF_LIB])
>  
>  AM_CONDITIONAL(DRICOMMON_NEED_LIBDRM, test "x$DRICOMMON_NEED_LIBDRM" = xyes)
> +AM_CONDITIONAL(HAVE_LIBDRM, test "x$have_libdrm" = xyes)
>  AM_CONDITIONAL(HAVE_X11_DRIVER, test "x$enable_xlib_glx" = xyes)
>  AM_CONDITIONAL(HAVE_OSMESA, test "x$enable_osmesa" = xyes)
>  AM_CONDITIONAL(HAVE_GALLIUM_OSMESA, test "x$enable_gallium_osmesa" = xyes)
> diff --git a/src/loader/Android.mk b/src/loader/Android.mk
> index 01699dc..6890af4 100644
> --- a/src/loader/Android.mk
> +++ b/src/loader/Android.mk
> @@ -35,7 +35,7 @@ LOCAL_SRC_FILES := \
>  
>  # swrast only
>  ifeq ($(MESA_GPU_DRIVERS),swrast)
> -	LOCAL_CFLAGS += -D_EGL_NO_DRM
> +	LOCAL_CFLAGS += -D__NOT_HAVE_DRM_H
>  else
>  LOCAL_C_INCLUDES += \
>  	$(DRM_TOP)/include/drm \
> diff --git a/src/loader/Makefile.am b/src/loader/Makefile.am
> index a118fef..371dd57 100644
> --- a/src/loader/Makefile.am
> +++ b/src/loader/Makefile.am
> @@ -27,11 +27,20 @@ libloader_la_CPPFLAGS = \
>  	$(DEFINES) \
>  	-I$(top_srcdir)/include \
>  	$(VISIBILITY_CFLAGS) \
> -	$(LIBDRM_CFLAGS) \
>  	$(LIBUDEV_CFLAGS)
>  
>  libloader_la_LIBADD = \
> -	$(LIBDRM_LIBS) \
>  	$(LIBUDEV_LIBS)
>  
> +if !HAVE_LIBDRM
> +libloader_la_CPPFLAGS += \
> +	-D__NOT_HAVE_DRM_H
> +else
> +libloader_la_CPPFLAGS += \
> +	$(LIBDRM_CFLAGS)
> +
> +libloader_la_LIBADD += \
> +	$(LIBDRM_LIBS)
> +endif
> +
>  libloader_la_SOURCES = $(LOADER_C_FILES)
> diff --git a/src/loader/SConscript b/src/loader/SConscript
> index 76b0871..359fc18 100644
> --- a/src/loader/SConscript
> +++ b/src/loader/SConscript
> @@ -1,15 +1,12 @@
>  Import('*')
>  
> -if not env['drm']:
> -    Return()
> -
>  env = env.Clone()
>  
>  env.Prepend(CPPPATH = [
>      '#include'
>  ])
>  
> -env.PkgUseModules('DRM')
> +env.Append(CPPDEFINES = ['__NOT_HAVE_DRM_H'])
>  
>  if env['udev']:
>      env.PkgUseModules('UDEV')
> diff --git a/src/loader/loader.c b/src/loader/loader.c
> index 108e444..6fe8c8f 100644
> --- a/src/loader/loader.c
> +++ b/src/loader/loader.c
> @@ -69,7 +69,9 @@
>  #include <string.h>
>  #include "loader.h"
>  
> +#ifndef __NOT_HAVE_DRM_H
>  #include <xf86drm.h>
> +#endif
>  
>  #define __IS_LOADER
>  #include "pci_ids/pci_id_driver_map.h"
> @@ -149,7 +151,7 @@ out:
>     return (*chip_id >= 0);
>  }
>  
> -#elif defined(ANDROID) && !defined(_EGL_NO_DRM)
> +#elif defined(ANDROID) && !defined(__NOT_HAVE_DRM_H)
>  
>  /* for i915 */
>  #include <i915_drm.h>
> @@ -267,6 +269,8 @@ loader_get_driver_for_fd(int fd, unsigned driver_types)
>        driver_types = _LOADER_GALLIUM | _LOADER_DRI;
>  
>     if (!loader_get_pci_id_for_fd(fd, &vendor_id, &chip_id)) {
> +
> +#ifndef __NOT_HAVE_DRM_H
>        /* fallback to drmGetVersion(): */
>        drmVersionPtr version = drmGetVersion(fd);
>  
> @@ -279,6 +283,7 @@ loader_get_driver_for_fd(int fd, unsigned driver_types)
>        log(_LOADER_INFO, "using driver %s for %d", driver, fd);
>  
>        drmFreeVersion(version);
> +#endif
>  
>        return driver;
>     }
> 

Looks reasonable to me.  Thanks for cleaning up the loader stuff, Emil!

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>