[PATCH rdma-core v2 3/3] configure: Add check for the presence of DRM headers

[Xiong, Jianxin]

> -----Original Message-----
> From: Daniel Vetter <daniel at ffwll.ch>
> Sent: Friday, February 05, 2021 5:54 AM
> To: Jason Gunthorpe <jgg at ziepe.ca>
> Cc: Xiong, Jianxin <jianxin.xiong at intel.com>; Yishai Hadas <yishaih at nvidia.com>; Leon Romanovsky <leon at kernel.org>; linux-rdma <linux-
> rdma at vger.kernel.org>; John Hubbard <jhubbard at nvidia.com>; Edward Srouji <edwards at nvidia.com>; Emil Velikov
> <emil.l.velikov at gmail.com>; Gal Pressman <galpress at amazon.com>; dri-devel <dri-devel at lists.freedesktop.org>; Doug Ledford
> <dledford at redhat.com>; Ali Alnubani <alialnu at nvidia.com>; Vetter, Daniel <daniel.vetter at intel.com>; Christian Koenig
> <christian.koenig at amd.com>
> Subject: Re: [PATCH rdma-core v2 3/3] configure: Add check for the presence of DRM headers
> 
> On Fri, Feb 5, 2021 at 2:22 PM Jason Gunthorpe <jgg at ziepe.ca> wrote:
> >
> > On Thu, Feb 04, 2021 at 04:29:14PM -0800, Jianxin Xiong wrote:
> > > Compilation of pyverbs/dmabuf_alloc.c depends on a few DRM headers
> > > that are installed by either the kernel-header or the libdrm package.
> > > The installation is optional and the location is not unique.
> > >
> > > Check the presence of the headers at both the standard locations and
> > > any location defined by custom libdrm installation. If the headers
> > > are missing, the dmabuf allocation routines are replaced by stubs
> > > that return suitable error to allow the related tests to skip.
> > >
> > > Signed-off-by: Jianxin Xiong <jianxin.xiong at intel.com>
> > >  CMakeLists.txt              | 15 +++++++++++++++
> > >  pyverbs/CMakeLists.txt      | 14 ++++++++++++--
> > >  pyverbs/dmabuf_alloc.c      |  8 ++++----
> > >  pyverbs/dmabuf_alloc_stub.c | 39
> > > +++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 70 insertions(+), 6 deletions(-)  create mode
> > > 100644 pyverbs/dmabuf_alloc_stub.c
> > >
> > > diff --git a/CMakeLists.txt b/CMakeLists.txt index 4113423..95aec11
> > > 100644
> > > +++ b/CMakeLists.txt
> > > @@ -515,6 +515,21 @@ find_package(Systemd)
> > >  include_directories(${SYSTEMD_INCLUDE_DIRS})
> > >  RDMA_DoFixup("${SYSTEMD_FOUND}" "systemd/sd-daemon.h")
> > >
> > > +# drm headers
> > > +
> > > +# First check the standard locations. The headers could have been
> > > +installed # by either the kernle-headers package or the libdrm package.
> > > +find_path(DRM_INCLUDE_DIRS "drm.h" PATH_SUFFIXES "drm" "libdrm")
> >
> > Is there a reason not to just always call pkg_check_modules?
> 
> Note that the gpu-specific libraries are split out, so I'd also check for those just to be sure - I don't know whether all distros package the
> uapi headers consistently in libdrm or sometimes also in one of the libdrm-$vendor packages.

The headers come from the libdrm-devel package, which present itself as "libdrm" 
under pkg-config. The gpu-specific packages only include the libraries, not the headers.

The kernel-headers packages doesn't have pkg-config info and can't be checked via pkg_check_modules().

One change I can make here is to use find_path() only for the headers installed by the
kernel-headers package (the "drm" path). The "libdrm" path is covered by the pkg_check_modules() check anyway.

> -Daniel
> 
> >
> > > +# Then check custom installation of libdrm if (NOT
> > > +DRM_INCLUDE_DIRS)
> > > +  pkg_check_modules(DRM libdrm)
> > > +endif()
> > > +
> > > +if (DRM_INCLUDE_DIRS)
> > > +  include_directories(${DRM_INCLUDE_DIRS})
> > > +endif()
> >
> > This needs a hunk at the end:
> >
> > if (NOT DRM_INCLUDE_DIRS)
> >   message(STATUS " DMABUF NOT supported (disabling some tests)")
> > endif()

Thanks, missed that.

> >
> > >  #-------------------------
> > >  # Apply fixups
> > >
> > > diff --git a/pyverbs/CMakeLists.txt b/pyverbs/CMakeLists.txt index
> > > 6fd7625..922253f 100644
> > > +++ b/pyverbs/CMakeLists.txt
> > > @@ -13,8 +13,6 @@ rdma_cython_module(pyverbs ""
> > >    cmid.pyx
> > >    cq.pyx
> > >    device.pyx
> > > -  dmabuf.pyx
> > > -  dmabuf_alloc.c
> > >    enums.pyx
> > >    mem_alloc.pyx
> > >    mr.pyx
> > > @@ -25,6 +23,18 @@ rdma_cython_module(pyverbs ""
> > >    xrcd.pyx
> > >  )
> > >
> > > +if (DRM_INCLUDE_DIRS)
> > > +rdma_cython_module(pyverbs ""
> > > +  dmabuf.pyx
> > > +  dmabuf_alloc.c
> > > +)
> > > +else()
> > > +rdma_cython_module(pyverbs ""
> > > +  dmabuf.pyx
> > > +  dmabuf_alloc_stub.c
> > > +)
> > > +endif()
> >
> > Like this:
> >
> > if (DRM_INCLUDE_DIRS)
> >   set(DMABUF_ALLOC dmabuf_alloc.c)
> > else()
> >   set(DMABUF_ALLOC dmabuf_alloc_stbub.c)
> > endif()
> > rdma_cython_module(pyverbs ""
> >   dmabuf.pyx
> >   $(DMABUF_ALLOC)
> > )

Sure, will change.

> >
> > Jason
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch