diff mbox series

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

Message ID 1612464651-54073-4-git-send-email-jianxin.xiong@intel.com (mailing list archive)
State Superseded
Headers show
Series Dma-buf related fixes | expand

Commit Message

Xiong, Jianxin Feb. 4, 2021, 6:50 p.m. UTC 
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@intel.com>
---
 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

Comments

Jason Gunthorpe Feb. 4, 2021, 9:12 p.m. UTC | #1 
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@intel.com>
>  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})

> +#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

Jaason
Xiong, Jianxin Feb. 4, 2021, 10:11 p.m. UTC | #2 
> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, February 04, 2021 1:12 PM
> To: Xiong, Jianxin <jianxin.xiong@intel.com>
> Cc: linux-rdma@vger.kernel.org; dri-devel@lists.freedesktop.org; Doug Ledford <dledford@redhat.com>; Leon Romanovsky
> <leon@kernel.org>; Sumit Semwal <sumit.semwal@linaro.org>; Christian Koenig <christian.koenig@amd.com>; Vetter, Daniel
> <daniel.vetter@intel.com>; Edward Srouji <edwards@nvidia.com>; Yishai Hadas <yishaih@nvidia.com>; John Hubbard
> <jhubbard@nvidia.com>; Ali Alnubani <alialnu@nvidia.com>; Gal Pressman <galpress@amazon.com>; Emil Velikov
> <emil.l.velikov@gmail.com>
> 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@intel.com>
> >  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
diff mbox series

Patch

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4113423..feaba3a 100644
--- a/CMakeLists.txt
+++ 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
--- /dev/null
+++ 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()
diff --git a/buildlib/config.h.in b/buildlib/config.h.in
index c5b0bf5..e8dff54 100644
--- a/buildlib/config.h.in
+++ b/buildlib/config.h.in
@@ -67,6 +67,8 @@ 
 # define VERBS_WRITE_ONLY 0
 #endif
 
+#define HAVE_DRM_H @HAVE_DRM_H@
+
 // Configuration defaults
 
 #define IBACM_SERVER_MODE_UNIX 0
diff --git a/pyverbs/dmabuf_alloc.c b/pyverbs/dmabuf_alloc.c
index 93267bf..22a8ab8 100644
--- a/pyverbs/dmabuf_alloc.c
+++ b/pyverbs/dmabuf_alloc.c
@@ -9,13 +9,18 @@ 
 #include <unistd.h>
 #include <string.h>
 #include <errno.h>
-#include <drm/drm.h>
-#include <drm/i915_drm.h>
-#include <drm/amdgpu_drm.h>
-#include <drm/radeon_drm.h>
+
+#include "config.h"
+#include "dmabuf_alloc.h"
+
+#if HAVE_DRM_H
+
+#include <drm.h>
+#include <i915_drm.h>
+#include <amdgpu_drm.h>
+#include <radeon_drm.h>
 #include <fcntl.h>
 #include <sys/ioctl.h>
-#include "dmabuf_alloc.h"
 
 /*
  * Abstraction of the buffer allocation mechanism using the DRM interface.
@@ -276,3 +281,35 @@  uint64_t dmabuf_get_offset(struct dmabuf *dmabuf)
 	return dmabuf->map_offset;
 }
 
+#else
+
+struct dmabuf *dmabuf_alloc(uint64_t size, int gpu, int gtt)
+{
+	errno = EOPNOTSUPP;
+	return NULL;
+}
+
+void dmabuf_free(struct dmabuf *dmabuf)
+{
+	errno = EOPNOTSUPP;
+}
+
+int dmabuf_get_drm_fd(struct dmabuf *dmabuf)
+{
+	errno = EOPNOTSUPP;
+	return -1;
+}
+
+int dmabuf_get_fd(struct dmabuf *dmabuf)
+{
+	errno = EOPNOTSUPP;
+	return -1;
+}
+
+uint64_t dmabuf_get_offset(struct dmabuf *dmabuf)
+{
+	errno = EOPNOTSUPP;
+	return -1;
+}
+
+#endif