Re: [Mesa-dev] [PATCH] st/va: set default rt formats when calling vaCreateConfig

[Julien Isorce]

Hi Christian,

I pushed it before already.  I can try to add a git notes if you want.
In any case thx for the review.

Cheers
Julien


On 18 October 2016 at 10:09, Christian König 
wrote:

> Am 17.10.2016 um 21:40 schrieb Julien Isorce:
>
>
>
> On Monday, 17 October 2016, Mark Thompson  wrote:
>
>> On 17/10/16 17:33, Julien Isorce wrote:
>> > As specified in va.h, default value should be set on attributes
>> > not present in the input list.
>> >
>> > Signed-off-by: Julien Isorce 
>> > ---
>> >  src/gallium/state_trackers/va/config.c  | 9 +
>> >  src/gallium/state_trackers/va/surface.c | 5 +++--
>> >  2 files changed, 12 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/gallium/state_trackers/va/config.c
>> b/src/gallium/state_trackers/va/config.c
>> > index 2f96eb6..fb236f1 100644
>> > --- a/src/gallium/state_trackers/va/config.c
>> > +++ b/src/gallium/state_trackers/va/config.c
>> > @@ -195,6 +195,11 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile
>> profile, VAEntrypoint entrypoin
>> >  }
>> >   }
>> >}
>> > +
>> > +  /* Default value if not specified in the input attributes. */
>> > +  if (!config->rt_format)
>> > +config->rt_format = VA_RT_FORMAT_YUV420 | VA_RT_FORMAT_RGB32;
>>
>> Indent should be three spaces.
>>
>> > +
>> >pipe_mutex_lock(drv->mutex);
>> >*config_id = handle_table_add(drv->htab, config);
>> >pipe_mutex_unlock(drv->mutex);
>> > @@ -256,6 +261,10 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile
>> profile, VAEntrypoint entrypoin
>> >}
>> > }
>> >
>> > +   /* Default value if not specified in the input attributes. */
>> > +   if (!config->rt_format)
>> > + config->rt_format = VA_RT_FORMAT_YUV420;
>>
>> And here.
>
>
> Oh I forgot :) , cheers.
>
>
>> > +
>> > pipe_mutex_lock(drv->mutex);
>> > *config_id = handle_table_add(drv->htab, config);
>> > pipe_mutex_unlock(drv->mutex);
>> > 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_surfa
>> ce_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;
>> >
>>
>> Actual code LGTM, and tested.
>>
>> Reviewed-by: Mark Thompson 
>
>
> Thx, will push it soon.
>
>
> If you haven't already pushed it with the fixes Mark mentioned the patch
> is Reviewed-by: Christian König 
>  as well.
>
> Regards,
> Christian.
>
> Julien
>
>
>>
>> Thanks,
>>
>> - Mark
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: set default rt formats when calling vaCreateConfig

[Christian König]

Am 17.10.2016 um 21:40 schrieb Julien Isorce:



On Monday, 17 October 2016, Mark Thompson > wrote:


On 17/10/16 17:33, Julien Isorce wrote:
> As specified in va.h, default value should be set on attributes
> not present in the input list.
>
> Signed-off-by: Julien Isorce >
> ---
>  src/gallium/state_trackers/va/config.c  | 9 +
>  src/gallium/state_trackers/va/surface.c | 5 +++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/config.c
b/src/gallium/state_trackers/va/config.c
> index 2f96eb6..fb236f1 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -195,6 +195,11 @@ vlVaCreateConfig(VADriverContextP ctx,
VAProfile profile, VAEntrypoint entrypoin
>  }
>   }
>}
> +
> +  /* Default value if not specified in the input attributes. */
> +  if (!config->rt_format)
> +config->rt_format = VA_RT_FORMAT_YUV420 |
VA_RT_FORMAT_RGB32;

Indent should be three spaces.

> +
>pipe_mutex_lock(drv->mutex);
>*config_id = handle_table_add(drv->htab, config);
>pipe_mutex_unlock(drv->mutex);
> @@ -256,6 +261,10 @@ vlVaCreateConfig(VADriverContextP ctx,
VAProfile profile, VAEntrypoint entrypoin
>}
> }
>
> +   /* Default value if not specified in the input attributes. */
> +   if (!config->rt_format)
> + config->rt_format = VA_RT_FORMAT_YUV420;

And here.


Oh I forgot :) , cheers.


> +
> pipe_mutex_lock(drv->mutex);
> *config_id = handle_table_add(drv->htab, config);
> pipe_mutex_unlock(drv->mutex);
> 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;
>

Actual code LGTM, and tested.

Reviewed-by: Mark Thompson >


Thx, will push it soon.


If you haven't already pushed it with the fixes Mark mentioned the patch 
is Reviewed-by: Christian König  as well.


Regards,
Christian.


Julien


Thanks,

- Mark

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org 
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: set default rt formats when calling vaCreateConfig

[Julien Isorce]

On Monday, 17 October 2016, Mark Thompson  wrote:

> On 17/10/16 17:33, Julien Isorce wrote:
> > As specified in va.h, default value should be set on attributes
> > not present in the input list.
> >
> > Signed-off-by: Julien Isorce >
> > ---
> >  src/gallium/state_trackers/va/config.c  | 9 +
> >  src/gallium/state_trackers/va/surface.c | 5 +++--
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/va/config.c
> b/src/gallium/state_trackers/va/config.c
> > index 2f96eb6..fb236f1 100644
> > --- a/src/gallium/state_trackers/va/config.c
> > +++ b/src/gallium/state_trackers/va/config.c
> > @@ -195,6 +195,11 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile
> profile, VAEntrypoint entrypoin
> >  }
> >   }
> >}
> > +
> > +  /* Default value if not specified in the input attributes. */
> > +  if (!config->rt_format)
> > +config->rt_format = VA_RT_FORMAT_YUV420 | VA_RT_FORMAT_RGB32;
>
> Indent should be three spaces.
>
> > +
> >pipe_mutex_lock(drv->mutex);
> >*config_id = handle_table_add(drv->htab, config);
> >pipe_mutex_unlock(drv->mutex);
> > @@ -256,6 +261,10 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile
> profile, VAEntrypoint entrypoin
> >}
> > }
> >
> > +   /* Default value if not specified in the input attributes. */
> > +   if (!config->rt_format)
> > + config->rt_format = VA_RT_FORMAT_YUV420;
>
> And here.


Oh I forgot :) , cheers.


> > +
> > pipe_mutex_lock(drv->mutex);
> > *config_id = handle_table_add(drv->htab, config);
> > pipe_mutex_unlock(drv->mutex);
> > 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;
> >
>
> Actual code LGTM, and tested.
>
> Reviewed-by: Mark Thompson >


Thx, will push it soon.
Julien


>
> Thanks,
>
> - Mark
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org 
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: set default rt formats when calling vaCreateConfig

[Mark Thompson]

On 17/10/16 17:33, Julien Isorce wrote:
> As specified in va.h, default value should be set on attributes
> not present in the input list.
> 
> Signed-off-by: Julien Isorce 
> ---
>  src/gallium/state_trackers/va/config.c  | 9 +
>  src/gallium/state_trackers/va/surface.c | 5 +++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/state_trackers/va/config.c 
> b/src/gallium/state_trackers/va/config.c
> index 2f96eb6..fb236f1 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -195,6 +195,11 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
> profile, VAEntrypoint entrypoin
>  }
>   }
>}
> +
> +  /* Default value if not specified in the input attributes. */
> +  if (!config->rt_format)
> +config->rt_format = VA_RT_FORMAT_YUV420 | VA_RT_FORMAT_RGB32;

Indent should be three spaces.

> +
>pipe_mutex_lock(drv->mutex);
>*config_id = handle_table_add(drv->htab, config);
>pipe_mutex_unlock(drv->mutex);
> @@ -256,6 +261,10 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
> profile, VAEntrypoint entrypoin
>}
> }
>  
> +   /* Default value if not specified in the input attributes. */
> +   if (!config->rt_format)
> + config->rt_format = VA_RT_FORMAT_YUV420;

And here.

> +
> pipe_mutex_lock(drv->mutex);
> *config_id = handle_table_add(drv->htab, config);
> pipe_mutex_unlock(drv->mutex);
> 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;
> 

Actual code LGTM, and tested.

Reviewed-by: Mark Thompson 

Thanks,

- Mark

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev