diff mbox series

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

Message ID 1612484954-75514-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. 5, 2021, 12:29 a.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.

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@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

Comments

Jason Gunthorpe Feb. 5, 2021, 1:22 p.m. UTC | #1 
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@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?

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

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

Jason
Daniel Vetter Feb. 5, 2021, 1:54 p.m. UTC | #2 
On Fri, Feb 5, 2021 at 2:22 PM Jason Gunthorpe <jgg@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@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.
-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()
>
> >  #-------------------------
> >  # 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)
> )
>
> Jason
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Xiong, Jianxin Feb. 5, 2021, 5:23 p.m. UTC | #3 
> -----Original Message-----
> From: Daniel Vetter <daniel@ffwll.ch>
> Sent: Friday, February 05, 2021 5:54 AM
> To: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Xiong, Jianxin <jianxin.xiong@intel.com>; Yishai Hadas <yishaih@nvidia.com>; Leon Romanovsky <leon@kernel.org>; linux-rdma <linux-
> rdma@vger.kernel.org>; John Hubbard <jhubbard@nvidia.com>; Edward Srouji <edwards@nvidia.com>; Emil Velikov
> <emil.l.velikov@gmail.com>; Gal Pressman <galpress@amazon.com>; dri-devel <dri-devel@lists.freedesktop.org>; Doug Ledford
> <dledford@redhat.com>; Ali Alnubani <alialnu@nvidia.com>; Vetter, Daniel <daniel.vetter@intel.com>; Christian Koenig
> <christian.koenig@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@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@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
diff mbox series

Patch

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4113423..95aec11 100644
--- a/CMakeLists.txt
+++ 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")
+
+# 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()
+
 #-------------------------
 # Apply fixups
 
diff --git a/pyverbs/CMakeLists.txt b/pyverbs/CMakeLists.txt
index 6fd7625..922253f 100644
--- a/pyverbs/CMakeLists.txt
+++ 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()
+
 rdma_python_module(pyverbs
   __init__.py
   pyverbs_error.py
diff --git a/pyverbs/dmabuf_alloc.c b/pyverbs/dmabuf_alloc.c
index 85ffb7a..9978a5b 100644
--- a/pyverbs/dmabuf_alloc.c
+++ b/pyverbs/dmabuf_alloc.c
@@ -9,12 +9,12 @@ 
 #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 <fcntl.h>
 #include <sys/ioctl.h>
+#include <drm.h>
+#include <i915_drm.h>
+#include <amdgpu_drm.h>
+#include <radeon_drm.h>
 #include "dmabuf_alloc.h"
 
 /*
diff --git a/pyverbs/dmabuf_alloc_stub.c b/pyverbs/dmabuf_alloc_stub.c
new file mode 100644
index 0000000..a73a5da
--- /dev/null
+++ b/pyverbs/dmabuf_alloc_stub.c
@@ -0,0 +1,39 @@ 
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/*
+ * Copyright 2021 Intel Corporation. All rights reserved. See COPYING file
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+#include <errno.h>
+#include "dmabuf_alloc.h"
+
+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;
+}
+