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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Thursday, February 04, 2021 1:12 PM
> To: Xiong, Jianxin <jianxin.xiong@xxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky
> <leon@xxxxxxxxxx>; Sumit Semwal <sumit.semwal@xxxxxxxxxx>; Christian Koenig <christian.koenig@xxxxxxx>; Vetter, Daniel
> <daniel.vetter@xxxxxxxxx>; Edward Srouji <edwards@xxxxxxxxxx>; Yishai Hadas <yishaih@xxxxxxxxxx>; John Hubbard
> <jhubbard@xxxxxxxxxx>; Ali Alnubani <alialnu@xxxxxxxxxx>; Gal Pressman <galpress@xxxxxxxxxx>; Emil Velikov
> <emil.l.velikov@xxxxxxxxx>
> Subject: Re: [PATCH rdma-core 3/3] configure: Add check for the presence of DRM headers
> 
> On Thu, Feb 04, 2021 at 10:50:51AM -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.
> >
> > The standard locations (such as /usr/include/drm, /usr/include/libdrm)
> > are checked first. If failed, pkg-config is tried to find the include
> > path of custom libdrm installation. The dmabuf allocation routines now
> > return suitable error when the headers are not available. The related
> > tests will recognize this error code and skip.
> >
> > Signed-off-by: Jianxin Xiong <jianxin.xiong@xxxxxxxxx>
> >  CMakeLists.txt         |  7 +++++++
> >  buildlib/Finddrm.cmake | 19 +++++++++++++++++++
> >  buildlib/config.h.in   |  2 ++
> >  pyverbs/dmabuf_alloc.c | 47
> > ++++++++++++++++++++++++++++++++++++++++++-----
> >  4 files changed, 70 insertions(+), 5 deletions(-)  create mode 100644
> > buildlib/Finddrm.cmake
> >
> > diff --git a/CMakeLists.txt b/CMakeLists.txt index 4113423..feaba3a
> > 100644
> > +++ b/CMakeLists.txt
> > @@ -515,6 +515,13 @@ find_package(Systemd)
> >  include_directories(${SYSTEMD_INCLUDE_DIRS})
> >  RDMA_DoFixup("${SYSTEMD_FOUND}" "systemd/sd-daemon.h")
> >
> > +# drm headers
> > +find_package(drm)
> > +if (DRM_INCLUDE_DIRS)
> > +  include_directories(${DRM_INCLUDE_DIRS})
> > +  set(HAVE_DRM_H 1)
> > +endif()
> > +
> >  #-------------------------
> >  # Apply fixups
> >
> > diff --git a/buildlib/Finddrm.cmake b/buildlib/Finddrm.cmake new file
> > mode 100644 index 0000000..6f8e5f2
> > +++ b/buildlib/Finddrm.cmake
> > @@ -0,0 +1,19 @@
> > +# COPYRIGHT (c) 2021 Intel Corporation.
> > +# Licensed under BSD (MIT variant) or GPLv2. See COPYING.
> > +
> > +# Check standard locations first
> > +find_path(DRM_INCLUDE_DIRS "drm.h" PATH_SUFFIXES "drm" "libdrm")
> > +
> > +# Check custom libdrm installation, if any if (NOT DRM_INCLUDE_DIRS)
> > +  execute_process(COMMAND pkg-config --cflags-only-I libdrm
> > +    OUTPUT_VARIABLE _LIBDRM
> > +    RESULT_VARIABLE _LIBDRM_RESULT
> > +    ERROR_QUIET)
> > +
> > +  if (NOT _LIBDRM_RESULT)
> > +    string(REGEX REPLACE "^-I" "" DRM_INCLUDE_DIRS "${_LIBDRM}")
> > +  endif()
> > +  unset(_LIBDRM)
> > +  unset(_LIBDRM_RESULT)
> > +endif()
> 
> I think this should be using pkg_check_modules() ??
> 
> Look at the NL stuff:
> 
>   pkg_check_modules(NL libnl-3.0 libnl-route-3.0 REQUIRED)
>   include_directories(${NL_INCLUDE_DIRS})
>   link_directories(${NL_LIBRARY_DIRS})
>

Yes, this is much simpler than the pkg-config method. 
 
> > +#if HAVE_DRM_H
> > +
> 
> Would rather you use cmake to conditionally compile a dmabuf_alloc.c or a dmabuf_alloc_stub.c than ifdef the entire file

Sure, will try that.

> 
> Jaason
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux