Re: [PATCH v3 1/5] drm: Add config for detecting libdrm

["Dmitry V. Levin" <ldv@altlinux.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2965 bytes --]

On Fri, Jul 31, 2015 at 11:09:11AM +0200, Patrik Jakobsson wrote:
> On Thu, Jul 30, 2015 at 10:04:49AM -0400, Mike Frysinger wrote:
> > On 30 Jul 2015 15:30, Patrik Jakobsson wrote:
> > > On Thu, Jul 23, 2015 at 05:48:21AM -0400, Mike Frysinger wrote:
> > > > On 01 Jul 2015 14:52, Patrik Jakobsson wrote:
> > > > > Use pkg-config to try to find libdrm. If that fails use the standard
> > > > > include directory for kernel drm headers in /usr/include/drm.
> > > > > 
> > > > > * configure.ac: Use pkg-config to find libdrm
> > > > > 
> > > > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > > > > ---
> > > > >  configure.ac | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > > 
> > > > > diff --git a/configure.ac b/configure.ac
> > > > > index bb8bf46..aa63af7 100644
> > > > > --- a/configure.ac
> > > > > +++ b/configure.ac
> > > > > @@ -844,6 +844,10 @@ fi
> > > > >  AM_CONDITIONAL([USE_LIBUNWIND], [test "x$use_libunwind" = xyes])
> > > > >  AC_MSG_RESULT([$use_libunwind])
> > > > >  
> > > > > +PKG_CHECK_MODULES([libdrm], [libdrm],
> > > > > +	[CPPFLAGS="$CPPFLAGS $libdrm_CFLAGS"],
> > > > > +	[CPPFLAGS="$CPPFLAGS -I/usr/include/drm"])
> > > > 
> > > > yikes, no, this is a really really bad idea.  it should read:
> > > > PKG_CHECK_MODULES([LIBDRM], [libdrm],
> > > > 	[CPPFLAGS="$CPPFLAGS $LIBDRM_CFLAGS"], [:])
> > > 
> > > I take it you don't want me to fallback on kernel headers and skip
> > > compiling with drm support if libdrm is not available?
> > 
> > you cannot hardcode any path at all.  if the kernel headers provide all
> > of the defines/structs that you need, then just include them directly
> > via #include <drm/xxx.h>.
> > -mike
> 
> The prefered "drm way" is to always use the libdrm headers and never the kernel
> headers. I know this is breaking the rules but it's what we got to work with.
> Some distros give you the kernel version and others the libdrm version. The
> kernel version is wrong and libdrm patches this up since we're not allowed to
> break the userspace interface. I think the safest way would be to only compile
> drm support for strace if libdrm is present and ignore the kernel headers.

Unlike most of userspace, strace attempts to show the picture as it's seen
from the kernel perspective.  Sometimes it forces us to use kernel headers
instead of headers provided by libc and other libraries.

If kernel itself uses uapi/drm, it should be safe for strace to use it
as well.  I suppose the check could be written this way:

PKG_CHECK_MODULES([LIBDRM], [libdrm],
		  [CPPFLAGS="$CPPFLAGS $LIBDRM_CFLAGS"
		   AC_CHECK_HEADERS([drm.h i915_drm.h])],
		  [AC_CHECK_HEADERS([drm/drm.h drm/i915_drm.h])])
...

#if defined HAVE_DRM_H || defined HAVE_DRM_DRM_H

# ifdef HAVE_DRM_H
#  include <drm.h>
# else
#  include <drm/drm.h>
# endif

[rest of drm.c]

#endif /* HAVE_DRM_H || HAVE_DRM_DRM_H */


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx