[Mesa-dev] [PATCH 2/6] st/va: skeleton VAAPI state tracker

Emil Velikov emil.l.velikov at gmail.com
Tue Sep 23 12:49:00 PDT 2014


On 23/09/14 17:44, Leo Liu wrote:
> From: Christian König <christian.koenig at amd.com>
> 
> This patch adds a skeleton VA-API state tracker,
> which is filled with live in the subsequent patches.
> 
There are some pending patches (both from me and Matt) that will cause
some trivial clashes. I'm guessing that we'll get them in over the next
few days.

Other than that I have a couple of comments inline

> Signed-off-by: Christian König <christian.koenig at amd.com>
> Signed-off-by: Leo Liu <leo.liu at amd.com>
> ---
>  configure.ac                               |  35 ++++++-
>  src/gallium/Makefile.am                    |   4 +
>  src/gallium/state_trackers/va/Makefile.am  |  43 ++++++++
>  src/gallium/state_trackers/va/buffer.c     |  87 +++++++++++++++++
>  src/gallium/state_trackers/va/config.c     |  91 +++++++++++++++++
>  src/gallium/state_trackers/va/context.c    | 151 +++++++++++++++++++++++++++++
>  src/gallium/state_trackers/va/display.c    |  61 ++++++++++++
>  src/gallium/state_trackers/va/image.c      | 106 ++++++++++++++++++++
>  src/gallium/state_trackers/va/picture.c    |  56 +++++++++++
>  src/gallium/state_trackers/va/subpicture.c | 115 ++++++++++++++++++++++
>  src/gallium/state_trackers/va/surface.c    | 111 +++++++++++++++++++++
>  src/gallium/state_trackers/va/va_private.h | 116 ++++++++++++++++++++++
>  src/gallium/targets/va/Makefile.am         |  93 ++++++++++++++++++
>  src/gallium/targets/va/target.c            |   1 +
>  src/gallium/targets/va/va.sym              |   6 ++
>  15 files changed, 1074 insertions(+), 2 deletions(-)
>  create mode 100644 src/gallium/state_trackers/va/Makefile.am
>  create mode 100644 src/gallium/state_trackers/va/buffer.c
>  create mode 100644 src/gallium/state_trackers/va/config.c
>  create mode 100644 src/gallium/state_trackers/va/context.c
>  create mode 100644 src/gallium/state_trackers/va/display.c
>  create mode 100644 src/gallium/state_trackers/va/image.c
>  create mode 100644 src/gallium/state_trackers/va/picture.c
>  create mode 100644 src/gallium/state_trackers/va/subpicture.c
>  create mode 100644 src/gallium/state_trackers/va/surface.c
>  create mode 100644 src/gallium/state_trackers/va/va_private.h
>  create mode 100644 src/gallium/targets/va/Makefile.am
>  create mode 100644 src/gallium/targets/va/target.c
>  create mode 100644 src/gallium/targets/va/va.sym
> 
> diff --git a/configure.ac b/configure.ac
> index 12f914e..951de25 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -661,6 +661,11 @@ AC_ARG_ENABLE([omx],
>           [enable OpenMAX library @<:@default=no@:>@])],
>     [enable_omx="$enableval"],
>     [enable_omx=no])
> +AC_ARG_ENABLE([va],
> +   [AS_HELP_STRING([--enable-va],
> +         [enable va library @<:@default=auto@:>@])],
> +   [enable_va="$enableval"],
> +   [enable_va=auto])
>  AC_ARG_ENABLE([opencl],
>     [AS_HELP_STRING([--enable-opencl],
>           [enable OpenCL library @<:@default=no@:>@])],
> @@ -732,6 +737,7 @@ if test "x$enable_opengl" = xno -a \
>          "x$enable_xvmc" = xno -a \
>          "x$enable_vdpau" = xno -a \
>          "x$enable_omx" = xno -a \
> +        "x$enable_va" = xno -a \
>          "x$enable_opencl" = xno; then
>      AC_MSG_ERROR([at least one API should be enabled])
>  fi
> @@ -1413,6 +1419,12 @@ if test -n "$with_gallium_drivers" -a "x$with_gallium_drivers" != xswrast; then
>      if test "x$enable_omx" = xauto; then
>  	PKG_CHECK_EXISTS([libomxil-bellagio], [enable_omx=yes], [enable_omx=no])
>      fi
> +
> +    if test "x$enable_va" = xauto; then
> +        #don't enable vaapi state tracker even if package exists
Hmm the description says "auto" yet we explicitly disable it. Can we
update one or the other ?

> +        #PKG_CHECK_EXISTS([libva], [enable_va=yes], [enable_va=no])
> +        enable_va=no
> +    fi
>  fi
>  
>  if test "x$enable_xvmc" = xyes; then
> @@ -1437,6 +1449,12 @@ if test "x$enable_omx" = xyes; then
>  fi
>  AM_CONDITIONAL(HAVE_ST_OMX, test "x$enable_omx" = xyes)
>  
> +if test "x$enable_va" = xyes; then
> +    PKG_CHECK_MODULES([LIBVA], [libva >= 0.35.0 x11-xcb xcb-dri2 >= $XCBDRI2_REQUIRED])
> +    GALLIUM_STATE_TRACKERS_DIRS="$GALLIUM_STATE_TRACKERS_DIRS va"
> +fi
> +AM_CONDITIONAL(HAVE_ST_VA, test "x$enable_va" = xyes)
> +
>  dnl
>  dnl OpenCL configuration
>  dnl
> @@ -1816,6 +1834,14 @@ AC_ARG_WITH([omx-libdir],
>      [OMX_LIB_INSTALL_DIR="$OMX_LIB_INSTALL_DIR_DEFAULT"])
>  AC_SUBST([OMX_LIB_INSTALL_DIR])
>  
> +dnl Directory for VA libs
> +AC_ARG_WITH([va-libdir],
> +    [AS_HELP_STRING([--with-va-libdir=DIR],
> +        [directory for the VA libraries @<:@default=${libdir}/va@:>@])],
> +    [VA_LIB_INSTALL_DIR="$withval"],
> +    [VA_LIB_INSTALL_DIR='${libdir}/va'])
> +AC_SUBST([VA_LIB_INSTALL_DIR])
> +
Please use `$PKG_CONFIG libva --variable driverdir` as the default
similar to OMX above.

[...]
> diff --git a/src/gallium/state_trackers/va/Makefile.am b/src/gallium/state_trackers/va/Makefile.am
> new file mode 100644
> index 0000000..aef15e2
> --- /dev/null
> +++ b/src/gallium/state_trackers/va/Makefile.am
> @@ -0,0 +1,43 @@
> +# Copyright 2014 Advanced Micro Devices, Inc.
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +# NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> +# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> +# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +# DEALINGS IN THE SOFTWARE.
> +
> +include $(top_srcdir)/src/gallium/Automake.inc
> +
> +AM_CFLAGS = \
> +	$(GALLIUM_CFLAGS) \
> +	$(VISIBILITY_CFLAGS) \
> +	$(VA_CFLAGS)
> +
> +AM_CPPFLAGS = \
> +	-I$(top_srcdir)/include
> +
> +noinst_LTLIBRARIES = libvatracker.la
> +
> +libvatracker_la_SOURCES = \
> +	buffer.c \
> +	config.c \
> +	context.c \
> +	display.c \
> +	image.c \
> +	picture.c \
> +	subpicture.c \
> +	surface.c
Please put the sources list (incl headers) into Makefile.sources :)

[...]
> diff --git a/src/gallium/targets/va/Makefile.am b/src/gallium/targets/va/Makefile.am
> new file mode 100644
> index 0000000..1a4e9e2
> --- /dev/null
> +++ b/src/gallium/targets/va/Makefile.am
> @@ -0,0 +1,93 @@
> +include $(top_srcdir)/src/gallium/Automake.inc
> +
> +AM_CFLAGS = \
> +	$(GALLIUM_TARGET_CFLAGS)
> +
> +dridir = $(DRI_DRIVER_INSTALL_DIR)
> +dri_LTLIBRARIES = gallium_drv_video.la
> +
I belive the above two variables should be va* rather than dri* ?
The destination directory should be VA_LIB_INSTALL_DIR.

> +nodist_EXTRA_gallium_drv_video_la_SOURCES = dummy.cpp
> +gallium_drv_video_la_SOURCES = \
> +	$(top_srcdir)/src/gallium/auxiliary/vl/vl_winsys_dri.c
> +
> +gallium_drv_video_la_LDFLAGS = \
> +	-shared \
> +	-module \
> +	-no-undefined \
> +	-avoid-version \
> +	$(GC_SECTIONS) \
> +	$(LD_NO_UNDEFINED)
> +
> +if HAVE_LD_VERSION_SCRIPT
> +gallium_drv_video_la_LDFLAGS += \
> +	-Wl,--version-script=$(top_srcdir)/src/gallium/targets/va/va.sym
> +endif
> +
> +gallium_drv_video_la_LIBADD = \
> +	$(top_builddir)/src/gallium/state_trackers/va/libvatracker.la \
> +	$(top_builddir)/src/gallium/auxiliary/libgallium.la \
> +	$(top_builddir)/src/util/libmesautil.la \
> +	$(LIBVA_LIBS) \
Unless I've missed something this link should not be needed. If in doubt
just omit it, and let the _compiler_ shout about undefined symbols :)


Cheers,
Emil


More information about the mesa-dev mailing list