[cc'ed Trvtko since he is looking into the libdrm API for userptr and
associated issues.]
On Thu, Apr 02, 2015 at 04:30:24PM +0200, Gwenole Beauchesne wrote:
> Allow creating VA surfaces with userptr allocated buffers. This requires
> a recent enough version of libdrm (>= 2.4.57), but also a kernel (>= 3.16)
> which contains appropriate fixes for userptr.
>
> v2: only request synchronized mappings (Chris Wilson).
>
> Signed-off-by: Gwenole Beauchesne <gwenole.beauche...@intel.com>
> ---
> configure.ac | 19 ++++++++++++
> src/i965_drv_video.c | 81
> +++++++++++++++++++++++++++++++++++++++++-----------
> src/i965_drv_video.h | 1 +
> src/intel_driver.h | 1 +
> src/intel_memman.c | 64 ++++++++++++++++++++++++++++++++++++++++-
> src/intel_memman.h | 7 +++++
> 6 files changed, 156 insertions(+), 17 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index d71a3cc..3c19cd2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -76,6 +76,25 @@ LIBDRM_VERSION=libdrm_version
> PKG_CHECK_MODULES([DRM], [libdrm >= $LIBDRM_VERSION])
> AC_SUBST(LIBDRM_VERSION)
>
> +AC_CACHE_CHECK([whether libdrm supports userptr allocations],
> + [ac_cv_libdrm_has_userptr],
> + [saved_CPPFLAGS="$CPPFLAGS"
> + saved_CFLAGS="$CFLAGS"
> + CPPFLAGS="$CPPFLAGS $DRM_CFLAGS"
> + LIBS="$LIBS $DRM_LIBS -ldrm_intel"
> + AC_TRY_LINK(
> + [#include <intel_bufmgr.h>],
> + [drm_intel_bo_alloc_userptr(NULL, NULL, NULL, 0, 0, 0, 0)],
> + [ac_cv_libdrm_has_userptr="yes"],
> + [ac_cv_libdrm_has_userptr="no"])
> + CPPFLAGS="$saved_CPPFLAGS"
> + LIBS="$saved_LIBS"]
> +)
> +if test "$ac_cv_libdrm_has_userptr" = "yes"; then
> + AC_DEFINE([HAVE_DRM_INTEL_USERPTR], [1],
> + [Defined to 1 if libdrm supports userptr allocations])
> +fi
Did you consider
if pkg-config --exists 'libdrm_intel >= 2.4.57'; then
AC_DEFINE([HAVE_DRM_INTEL_USERPTR], [1],
[Defined to 1 if libdrm supports userptr allocations])
fi
? The full check for the function in the library should be foolproof,
but overkill?
> +drm_intel_bo *
> +do_import_userptr(struct intel_driver_data *intel, const char *name,
> + void *data, size_t data_size, uint32_t va_flags)
> +{
> +#ifdef HAVE_DRM_INTEL_USERPTR
> + uint32_t page_size, tiling_mode, flags = 0;
> + drm_intel_bo *bo;
> +
> + /* XXX: userptr is only supported for page-aligned allocations */
> + page_size = getpagesize();
> + if ((uintptr_t)data & (page_size - 1))
> + return NULL;
I would pass through the pointer alignment checks to the kernel. Then if
they are relaxed or tightened, the library doesn't need to be adapted.
> + tiling_mode = (va_flags & VA_SURFACE_EXTBUF_DESC_ENABLE_TILING) ?
> + I915_TILING_Y : I915_TILING_NONE;
Hmm, if you going to allow I915_TILING_Y, you are going to need to
sanity check the allocation size. There is a kernel/libdrm patch coming
to allow you to specify a padding for underallocated user buffers. Does
the client ever specify width,height,pitch for their userptr surface?
> + bo = drm_intel_bo_alloc_userptr(intel->bufmgr, name, data, tiling_mode,
> 0,
> + data_size, flags);
> + if (bo)
> + return bo;
pad_to_size = 0;
if (tiling_mode != I915_TILING_MODE) {
aligned_y = ALIGN(surface_height, 16);
pitch = ALIGN(surface_width * surface_cpp, 128);
pad_to_size = aligned_y * pitch;
}
drm_intel_bo_pad_to_size(bo, pad_to_size);
Actually, that is probably best done inside libdrm
drm_intel_bo_pad_for_tiling(bo, width, height, cpp, I915_TILING_Y);
since libdrm already has the information about required padding for
different tile modes.
> +#endif
> + return NULL;
> +}
> +
> +drm_intel_bo *
> +intel_memman_import_userptr(struct intel_driver_data *intel, const char
> *name,
> + void *data, size_t data_size, uint32_t va_flags)
> +{
> + if (!intel_memman_has_userptr(intel))
> + return NULL;
> + return do_import_userptr(intel, name, data, data_size, va_flags);
> +}
> +
> +bool
> +intel_memman_has_userptr(struct intel_driver_data *intel)
> +{
> + drm_intel_bo *bo;
> + size_t page_size;
> + void *page;
> +
> + if (intel->userptr_disabled > 1) {
> + intel->userptr_disabled = 1;
> +
> + page_size = getpagesize();
> + if (posix_memalign(&page, page_size, page_size) == 0) {
> + bo = do_import_userptr(intel, "userptr test buffer",
> + page, page_size, 0);
> + if (bo) {
> + drm_intel_bo_unreference(bo);
> + intel->userptr_disabled = 0;
> + }
> + free(page);
> + }
> + }
> + return !intel->userptr_disabled;
This doesn't actually serve any purpose. It is redundant as libdrm_intel
does the very same check itself, and in both cases (if libdrm_intel
userptr sanity check fails, or if !intel_memman_has_userptr fails) you
just return NULL.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Libva mailing list
Libva@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libva
