[Mesa-dev] [PATCH] st/va: Default to YUV420 RT format when creating a config

[Mark Thompson]

On 17/10/16 16:13, Julien Isorce wrote:
> Hi Mark,
> 
> Yes I actually saw that too in the intel driver though I think it does not add
> VA_RT_FORMAT_RGB32 ? Or I missed something ?
> Maybe this is a bug. In any case yes as said before " the intel va driver always
> return the full list for vpp." from vaQuerySurfaceAttributes
> no matter the format selected when creating the config.

Looks like the lack of RGB32 has already been noted and fixed:
<https://cgit.freedesktop.org/vaapi/intel-driver/commit/?id=e748bc7f0565d59a7ec2ba038e76a0a1de19c15c>.

> So combining all it should probably be something like this:
> 
> diff --git a/src/gallium/state_trackers/va/config.c
> b/src/gallium/state_trackers/va/config.c
> index 2f96eb6..11afc81 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -185,6 +185,8 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile profile,
> VAEntrypoint entrypoin
>     if (profile == VAProfileNone && entrypoint == VAEntrypointVideoProc) {
>        config->entrypoint = VAEntrypointVideoProc;
>        config->profile = PIPE_VIDEO_PROFILE_UNKNOWN;
> +      config->rt_format = VA_RT_FORMAT_YUV420 | VA_RT_FORMAT_RGB32;
> +
>        for (int i = 0; i < num_attribs; i++) {
>           if (attrib_list[i].type == VAConfigAttribRTFormat) {
>              if (attrib_list[i].value & (VA_RT_FORMAT_YUV420 |
> VA_RT_FORMAT_RGB32)) {

Please set a default for the non-VideoProc case too, so that codecs have the
same behaviour.

> diff --git a/src/gallium/state_trackers/va/surface.c
> b/src/gallium/state_trackers/va/surface.c
> index 5e92980..f8513d9 100644
> --- a/src/gallium/state_trackers/va/surface.c
> +++ b/src/gallium/state_trackers/va/surface.c
> @@ -419,7 +419,7 @@ vlVaQuerySurfaceAttributes(VADriverContextP ctx, VAConfigID
> config_id,
>     /* vlVaCreateConfig returns PIPE_VIDEO_PROFILE_UNKNOWN
>      * only for VAEntrypointVideoProc. */
>     if (config->profile == PIPE_VIDEO_PROFILE_UNKNOWN) {
> -      if (config->rt_format == VA_RT_FORMAT_RGB32) {
> +      if (config->rt_format & VA_RT_FORMAT_RGB32) {
>           for (j = 0; j < ARRAY_SIZE(vpp_surface_formats); ++j) {
>              attribs[i].type = VASurfaceAttribPixelFormat;
>              attribs[i].value.type = VAGenericValueTypeInteger;
> @@ -427,7 +427,8 @@ vlVaQuerySurfaceAttributes(VADriverContextP ctx, VAConfigID
> config_id,
>              attribs[i].value.value.i =
> PipeFormatToVaFourcc(vpp_surface_formats[j]);
>              i++;
>           }
> -      } else if (config->rt_format == VA_RT_FORMAT_YUV420) {
> +      }
> +      if (config->rt_format & VA_RT_FORMAT_YUV420) {
>           attribs[i].type = VASurfaceAttribPixelFormat;
>           attribs[i].value.type = VAGenericValueTypeInteger;
>           attribs[i].flags = VA_SURFACE_ATTRIB_GETTABLE |
> VA_SURFACE_ATTRIB_SETTABLE;
> 
> Will it be ok for your case ?
> 
> Cheers
> Julien

Yeah, that seems right to me (with the small change above).

Thanks,

- Mark