[Intel-gfx] [PATCH v3 1/5] drm: Add config for detecting libdrm

[Patrik Jakobsson]

On Sat, Aug 1, 2015 at 8:22 PM, Dmitry V. Levin <ldv at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>