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

Submitted by Patrik Jakobsson on July 1, 2015, 12:52 p.m.

Details

Message ID 1435755168-16207-2-git-send-email-patrik.jakobsson@linux.intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Patrik Jakobsson July 1, 2015, 12:52 p.m.
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(+)

Patch hide | download patch | download mbox

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"])
+
 if test "$arch" = mips && test "$no_create" != yes; then
 	mkdir -p linux/mips
 	if $srcdir/linux/mips/genstub.sh linux/mips; then

Comments

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"], [:])
-mike
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"], [:])

Why [:] ?  Wouldn't [] suffice?
On 23 Jul 2015 13:44, Dmitry V. Levin 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"], [:])
> 
> Why [:] ?  Wouldn't [] suffice?

probably ... force of habit after being bitten by m4 macros that did not expect
to expand empty code and thus lead to shell errors (include macros by autotools
projects).  i.e. if the m4 looked something like:
	if ...check if pkg is available...; then
		$3
	else
		$4
	fi
then the generated configure script would have syntax errors.
-mike
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"], [:])
> -mike

Hi Mike

Please include me in CC.

I take it you don't want me to fallback on kernel headers and skip
compiling with drm support if libdrm is not available?

Cheers
Patrik
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
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.

What do you think?

Cheers
Patrik
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 */
On Sat, Aug 1, 2015 at 8:22 PM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> 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 */

That would be even better. We'll get some warnings when including the kernel
version of drm.h but I guess we can live with that.

Will soon send out a new version of the patches.

Thanks
Patrik

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