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

[Emil Velikov]

Hi Leo,

On 26/09/14 21:13, 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.
> 
> v2: fixes in configure.ac and va state_tracker Makefile.am
s/state_tracker Makefile.am/target Makefile.am/

> v3: configure.ac:
>        generate a marco for link to xcb
>        auto-dectecting VA version
>        rebase with upstream changes
>     state-trackers/va/Makefile.am:
>        pass symbol for auto-detecting VA version
>     targets/va/Makefile.am
>        rebase with omx/Makefile.am
>     use macro VA_DRIVER_INIT_FUNC for auto-detect
> 
The changelog is rather difficult to read. Can I suggest something like
 - Do not link against libva.
 - Detect libva version, and correctly set driver entrypoint name.
 - Rebase(cleanup) targets/va/Makefile.am

Rather than setting up the whole function name in configure.ac can't we
just pass the version number ? VDPAU is doing it and seems to be what
Gwenole was suggesting.

It seems that you've dropped the sym file. Is that intentional, can we
get it back please ?

> +dnl Directory for VA libs
> +
> +AC_ARG_WITH([va-libdir],
> +    [AS_HELP_STRING([--with-va-libdir=DIR],
> +        [directory for the VA libraries @<:@default=`pkg-config libva --variable=driverdir`@:>@])],
> +    [VA_LIB_INSTALL_DIR="$withval"],
> +    [VA_LIB_INSTALL_DIR=`pkg-config libva --variable=driverdir`])
> +AC_SUBST([VA_LIB_INSTALL_DIR])
> +
The above hunk has silently changed since v2 and I'm not sure this one
is better. Also there is a discussion about mesa's use of the above
pkg-config to set the destination/install dir. I'll amend mesa at a
later stage once a decision is made :)

> diff --git a/src/gallium/state_trackers/va/Makefile.am b/src/gallium/state_trackers/va/Makefile.am
> new file mode 100644
> index 0000000..b6d3f9a
> --- /dev/null
> +++ b/src/gallium/state_trackers/va/Makefile.am
[...]
> +AM_CFLAGS = \
> +	$(GALLIUM_CFLAGS) \
> +	$(VISIBILITY_CFLAGS) \
> +	$(VA_CFLAGS)
> +AM_CFLAGS += -DVA_DRIVER_INIT_FUNC=$(VA_DRIVER_INIT_FUNC)
> +
Just merge the two ?

Thanks
Emil