| Message ID | 1435755168-16207-2-git-send-email-patrik.jakobsson@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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
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(+)