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

[Liu, Leo]

Hi Emil,

>-----Original Message-----
>From: Emil Velikov [mailto:emil.l.velikov at gmail.com]
>Sent: Sunday, September 28, 2014 4:43 PM
>To: Liu, Leo; mesa-dev at lists.freedesktop.org
>Cc: emil.l.velikov at gmail.com; Koenig, Christian
>Subject: Re: [Mesa-dev] [PATCH v3 2/6] st/va: skeleton VAAPI state tracker
>
>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
>
This's better.

>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.
>
This also makes sense.

>It seems that you've dropped the sym file. Is that intentional, can we get it back
>please ?
>
I will get it back.
What's the purpose of this sym file? Thanks.

>> +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 :)
>
So, Can I just keep it like this for now?

>> 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 ?
>
Sure.

Thanks,
Leo

>Thanks
>Emil