Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return unsupported profile

2016-10-18 Thread Eoff, Ullysses A
Please run the tests... I suspect this patch will cause some to fail.  Those 
tests will need updated too.

U. Artie

> -Original Message-
> From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Daniel 
> Charles
> Sent: Monday, October 17, 2016 5:45 PM
> To: libva@lists.freedesktop.org
> Subject: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return 
> unsupported profile
> 
> When all the profiles are not supported return
> VA_STATUS_ERROR_UNSUPPORTED_PROFILE instead of
> VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT.
> 
> Also change the style on the code modified to be common
> on all cases
> 
> Signed-off-by: Daniel Charles 
> ---
>  src/i965_drv_video.c | 49 -
>  1 file changed, 40 insertions(+), 9 deletions(-)
> 
> diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
> index fbf2cda..0830ae0 100644
> --- a/src/i965_drv_video.c
> +++ b/src/i965_drv_video.c
> @@ -711,6 +711,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> profile,
>  if ((HAS_MPEG2_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
>  (HAS_MPEG2_ENCODING(i965) && entrypoint == 
> VAEntrypointEncSlice)) {
>  va_status = VA_STATUS_SUCCESS;
> +} else if (!HAS_MPEG2_DECODING(i965) && !HAS_MPEG2_ENCODING(i965)){
> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>  } else {
>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>  }
> @@ -723,6 +725,9 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> profile,
>  (HAS_H264_ENCODING(i965) && entrypoint == VAEntrypointEncSlice) 
> ||
>  (HAS_LP_H264_ENCODING(i965) && entrypoint == 
> VAEntrypointEncSliceLP)) {
>  va_status = VA_STATUS_SUCCESS;
> +} else if (!HAS_H264_DECODING(i965) && !HAS_H264_ENCODING(i965) &&
> +   !HAS_LP_H264_ENCODING(i965)){
> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>  } else {
>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>  }
> @@ -733,6 +738,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> profile,
>  case VAProfileVC1Advanced:
>  if (HAS_VC1_DECODING(i965) && entrypoint == VAEntrypointVLD) {
>  va_status = VA_STATUS_SUCCESS;
> +} else if (!HAS_VC1_DECODING(i965)) {
> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>  } else {
>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>  }
> @@ -741,6 +748,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> profile,
>  case VAProfileNone:
>  if (HAS_VPP(i965) && VAEntrypointVideoProc == entrypoint) {
>  va_status = VA_STATUS_SUCCESS;
> +} else if (!HAS_VPP(i965)){
> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>  } else {
>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>  }
> @@ -750,6 +759,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> profile,
>  if ((HAS_JPEG_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
>  (HAS_JPEG_ENCODING(i965) && entrypoint == 
> VAEntrypointEncPicture)) {
>  va_status = VA_STATUS_SUCCESS;
> +} else if (!HAS_JPEG_DECODING(i965) && !HAS_JPEG_ENCODING(i965)){
> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>  } else {
>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>  }
> @@ -759,6 +770,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> profile,
>  if ((HAS_VP8_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
>  (HAS_VP8_ENCODING(i965) && entrypoint == VAEntrypointEncSlice)) {
>  va_status = VA_STATUS_SUCCESS;
> +} else if (!HAS_VP8_DECODING(i965) && !HAS_VP8_ENCODING(i965)){
> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>  } else {
>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>  }
> @@ -768,8 +781,12 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> profile,
>  case VAProfileH264StereoHigh:
>  if ((HAS_H264_MVC_DECODING_PROFILE(i965, profile) &&
>   entrypoint == VAEntrypointVLD) ||
> -(HAS_H264_MVC_ENCODING(i965) && entrypoint == 
> VAEntrypointEncSlice)) {
> +(HAS_H264_MVC_ENCODING(i965) &&
> + entrypoint == VAEntrypointEncSlice)) {
>  va_status = VA_STATUS_SUCCESS;
> +} else if(!HAS_H264_MVC_DECODING_PROFILE(i965, profile) &&
> +  !HAS_H264_MVC_ENCODING(i965)) {
> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>  } else {
>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>  }
> @@ -778,32 +795,46 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> profile,
> 
>  case VAProfileHEVCMain:
>  if 

Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return unsupported profile

2016-10-18 Thread Eoff, Ullysses A

> -Original Message-
> From: Charles, Daniel [mailto:daniel.char...@intel.com]
> Sent: Tuesday, October 18, 2016 11:09 AM
> To: Eoff, Ullysses A 
> Cc: libva@lists.freedesktop.org
> Subject: Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return 
> unsupported profile
> 
> On Tue, Oct 18, 2016 at 10:48 AM, Eoff, Ullysses A
>  wrote:
> >> -Original Message-
> >> From: Charles, Daniel [mailto:daniel.char...@intel.com]
> >> Sent: Tuesday, October 18, 2016 10:20 AM
> >> To: Eoff, Ullysses A 
> >> Cc: libva@lists.freedesktop.org
> >> Subject: Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: 
> >> return unsupported profile
> >>
> >> On Tue, Oct 18, 2016 at 8:25 AM, Eoff, Ullysses A
> >>  wrote:
> >> > Please run the tests... I suspect this patch will cause some to fail.  
> >> > Those tests will need updated too.
> >> >
> >>
> >> Ran the test suite on ToT, no regression found with this patch.
> >>
> >
> > Ah yes, I see why now.   The AVC and JPEG config tests still pass on most 
> > HW because UNSUPPORTED_PROFILE is only returned if
> *both* encode *and* decode are unsupported.  Thus, if there is such HW that 
> doesn't support one or the other then those tests will
> be regressed.
> 
> I am not sure I follow you here.
> 
> Unsupported Profile is sent when neither encoder and decoder are
> supported (VC1 is only decoder, VPP is a special case and VP9 decoder
> can also be hybrid on some h/w).  When h/w only supports one of them
> (regardless which it is), it should return UNSUPPORTED_ENTRYPOINT as
> profile is supported at least once.  This patch is not changing this
> last scenario. My testing was done on a h/w that has Jpeg Decoder but
> no Jpeg Encoder and everything is okay.
> 

Right now, there are tests for AVC and JPEG that create a config for encode and 
decode entrypoints.  Each test defines its own expected result of CreateConfig 
and compares with the actual result.  If there is a platform that doesn't 
support any of the entrypoints for AVC or JPEG, then the expected result will 
be calculated wrong (i.e. the test expectation is not taking into account your 
new changes).  (See i965_avce_config_test.cpp, i965_avcd_config_test.cpp, 
i965_jpege_config_test.cpp and i965_jpegd_config_test.cpp).

U. Artie

> --
> Daniel.
> >
> >> --
> >> Daniel.
> >> > U. Artie
> >> >
> >> >> -Original Message-
> >> >> From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of 
> >> >> Daniel Charles
> >> >> Sent: Monday, October 17, 2016 5:45 PM
> >> >> To: libva@lists.freedesktop.org
> >> >> Subject: [Libva] [PATCH][libva-intel-driver] i965_validate_config: 
> >> >> return unsupported profile
> >> >>
> >> >> When all the profiles are not supported return
> >> >> VA_STATUS_ERROR_UNSUPPORTED_PROFILE instead of
> >> >> VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT.
> >> >>
> >> >> Also change the style on the code modified to be common
> >> >> on all cases
> >> >>
> >> >> Signed-off-by: Daniel Charles 
> >> >> ---
> >> >>  src/i965_drv_video.c | 49 
> >> >> -
> >> >>  1 file changed, 40 insertions(+), 9 deletions(-)
> >> >>
> >> >> diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
> >> >> index fbf2cda..0830ae0 100644
> >> >> --- a/src/i965_drv_video.c
> >> >> +++ b/src/i965_drv_video.c
> >> >> @@ -711,6 +711,8 @@ i965_validate_config(VADriverContextP ctx, 
> >> >> VAProfile profile,
> >> >>  if ((HAS_MPEG2_DECODING(i965) && entrypoint == 
> >> >> VAEntrypointVLD) ||
> >> >>  (HAS_MPEG2_ENCODING(i965) && entrypoint == 
> >> >> VAEntrypointEncSlice)) {
> >> >>  va_status = VA_STATUS_SUCCESS;
> >> >> +} else if (!HAS_MPEG2_DECODING(i965) && 
> >> >> !HAS_MPEG2_ENCODING(i965)){
> >> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> >> >>  } else {
> >> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >> >>  }
> >> >> @@ -723,6 +725,9 @@ i965_validate_config(VADriverContextP ctx, 
> >> >> VAProfile profile,
> >> >>  (HAS_H264_ENCODING(i965) && entrypoint == 
> >> >> VAEntrypointEncSlice) ||
> >> >>  (HAS_LP_H264_ENCODING(i965) && entrypoint == 
> >> >> VAEntrypointEncSliceLP)) {
> >> >>  va_status = VA_STATUS_SUCCESS;
> >> >> +} else if (!HAS_H264_DECODING(i965) && 
> >> >> !HAS_H264_ENCODING(i965) &&
> >> >> +   !HAS_LP_H264_ENCODING(i965)){
> >> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> >> >>  } else {
> >> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >> >>  }
> >> >> @@ -733,6 +738,8 @@ i965_validate_config(VADriverContextP ctx, 
> >> >> VAProfile profile,
> >> >>  case VAProfileVC1Advanced:
> >> >>  if (HAS_VC1_DECODING(i965) && entrypoint == 

Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return unsupported profile

2016-10-18 Thread Eoff, Ullysses A
> -Original Message-
> From: Charles, Daniel [mailto:daniel.char...@intel.com]
> Sent: Tuesday, October 18, 2016 10:20 AM
> To: Eoff, Ullysses A 
> Cc: libva@lists.freedesktop.org
> Subject: Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return 
> unsupported profile
> 
> On Tue, Oct 18, 2016 at 8:25 AM, Eoff, Ullysses A
>  wrote:
> > Please run the tests... I suspect this patch will cause some to fail.  
> > Those tests will need updated too.
> >
> 
> Ran the test suite on ToT, no regression found with this patch.
> 

Ah yes, I see why now.   The AVC and JPEG config tests still pass on most HW 
because UNSUPPORTED_PROFILE is only returned if *both* encode *and* decode are 
unsupported.  Thus, if there is such HW that doesn't support one or the other 
then those tests will be regressed.

> --
> Daniel.
> > U. Artie
> >
> >> -Original Message-
> >> From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of 
> >> Daniel Charles
> >> Sent: Monday, October 17, 2016 5:45 PM
> >> To: libva@lists.freedesktop.org
> >> Subject: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return 
> >> unsupported profile
> >>
> >> When all the profiles are not supported return
> >> VA_STATUS_ERROR_UNSUPPORTED_PROFILE instead of
> >> VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT.
> >>
> >> Also change the style on the code modified to be common
> >> on all cases
> >>
> >> Signed-off-by: Daniel Charles 
> >> ---
> >>  src/i965_drv_video.c | 49 
> >> -
> >>  1 file changed, 40 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
> >> index fbf2cda..0830ae0 100644
> >> --- a/src/i965_drv_video.c
> >> +++ b/src/i965_drv_video.c
> >> @@ -711,6 +711,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> >> profile,
> >>  if ((HAS_MPEG2_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
> >>  (HAS_MPEG2_ENCODING(i965) && entrypoint == 
> >> VAEntrypointEncSlice)) {
> >>  va_status = VA_STATUS_SUCCESS;
> >> +} else if (!HAS_MPEG2_DECODING(i965) && 
> >> !HAS_MPEG2_ENCODING(i965)){
> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> >>  } else {
> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >>  }
> >> @@ -723,6 +725,9 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> >> profile,
> >>  (HAS_H264_ENCODING(i965) && entrypoint == 
> >> VAEntrypointEncSlice) ||
> >>  (HAS_LP_H264_ENCODING(i965) && entrypoint == 
> >> VAEntrypointEncSliceLP)) {
> >>  va_status = VA_STATUS_SUCCESS;
> >> +} else if (!HAS_H264_DECODING(i965) && !HAS_H264_ENCODING(i965) &&
> >> +   !HAS_LP_H264_ENCODING(i965)){
> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> >>  } else {
> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >>  }
> >> @@ -733,6 +738,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> >> profile,
> >>  case VAProfileVC1Advanced:
> >>  if (HAS_VC1_DECODING(i965) && entrypoint == VAEntrypointVLD) {
> >>  va_status = VA_STATUS_SUCCESS;
> >> +} else if (!HAS_VC1_DECODING(i965)) {
> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> >>  } else {
> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >>  }
> >> @@ -741,6 +748,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> >> profile,
> >>  case VAProfileNone:
> >>  if (HAS_VPP(i965) && VAEntrypointVideoProc == entrypoint) {
> >>  va_status = VA_STATUS_SUCCESS;
> >> +} else if (!HAS_VPP(i965)){
> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> >>  } else {
> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >>  }
> >> @@ -750,6 +759,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> >> profile,
> >>  if ((HAS_JPEG_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
> >>  (HAS_JPEG_ENCODING(i965) && entrypoint == 
> >> VAEntrypointEncPicture)) {
> >>  va_status = VA_STATUS_SUCCESS;
> >> +} else if (!HAS_JPEG_DECODING(i965) && !HAS_JPEG_ENCODING(i965)){
> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> >>  } else {
> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >>  }
> >> @@ -759,6 +770,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
> >> profile,
> >>  if ((HAS_VP8_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
> >>  (HAS_VP8_ENCODING(i965) && entrypoint == 
> >> VAEntrypointEncSlice)) {
> >>  va_status = VA_STATUS_SUCCESS;
> >> +} else if (!HAS_VP8_DECODING(i965) && !HAS_VP8_ENCODING(i965)){
> >> +

Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return unsupported profile

2016-10-18 Thread Charles, Daniel
On Tue, Oct 18, 2016 at 8:25 AM, Eoff, Ullysses A
 wrote:
> Please run the tests... I suspect this patch will cause some to fail.  Those 
> tests will need updated too.
>

Ran the test suite on ToT, no regression found with this patch.

-- 
Daniel.
> U. Artie
>
>> -Original Message-
>> From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Daniel 
>> Charles
>> Sent: Monday, October 17, 2016 5:45 PM
>> To: libva@lists.freedesktop.org
>> Subject: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return 
>> unsupported profile
>>
>> When all the profiles are not supported return
>> VA_STATUS_ERROR_UNSUPPORTED_PROFILE instead of
>> VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT.
>>
>> Also change the style on the code modified to be common
>> on all cases
>>
>> Signed-off-by: Daniel Charles 
>> ---
>>  src/i965_drv_video.c | 49 -
>>  1 file changed, 40 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
>> index fbf2cda..0830ae0 100644
>> --- a/src/i965_drv_video.c
>> +++ b/src/i965_drv_video.c
>> @@ -711,6 +711,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> profile,
>>  if ((HAS_MPEG2_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
>>  (HAS_MPEG2_ENCODING(i965) && entrypoint == 
>> VAEntrypointEncSlice)) {
>>  va_status = VA_STATUS_SUCCESS;
>> +} else if (!HAS_MPEG2_DECODING(i965) && !HAS_MPEG2_ENCODING(i965)){
>> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>>  } else {
>>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>  }
>> @@ -723,6 +725,9 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> profile,
>>  (HAS_H264_ENCODING(i965) && entrypoint == VAEntrypointEncSlice) 
>> ||
>>  (HAS_LP_H264_ENCODING(i965) && entrypoint == 
>> VAEntrypointEncSliceLP)) {
>>  va_status = VA_STATUS_SUCCESS;
>> +} else if (!HAS_H264_DECODING(i965) && !HAS_H264_ENCODING(i965) &&
>> +   !HAS_LP_H264_ENCODING(i965)){
>> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>>  } else {
>>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>  }
>> @@ -733,6 +738,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> profile,
>>  case VAProfileVC1Advanced:
>>  if (HAS_VC1_DECODING(i965) && entrypoint == VAEntrypointVLD) {
>>  va_status = VA_STATUS_SUCCESS;
>> +} else if (!HAS_VC1_DECODING(i965)) {
>> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>>  } else {
>>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>  }
>> @@ -741,6 +748,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> profile,
>>  case VAProfileNone:
>>  if (HAS_VPP(i965) && VAEntrypointVideoProc == entrypoint) {
>>  va_status = VA_STATUS_SUCCESS;
>> +} else if (!HAS_VPP(i965)){
>> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>>  } else {
>>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>  }
>> @@ -750,6 +759,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> profile,
>>  if ((HAS_JPEG_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
>>  (HAS_JPEG_ENCODING(i965) && entrypoint == 
>> VAEntrypointEncPicture)) {
>>  va_status = VA_STATUS_SUCCESS;
>> +} else if (!HAS_JPEG_DECODING(i965) && !HAS_JPEG_ENCODING(i965)){
>> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>>  } else {
>>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>  }
>> @@ -759,6 +770,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> profile,
>>  if ((HAS_VP8_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
>>  (HAS_VP8_ENCODING(i965) && entrypoint == VAEntrypointEncSlice)) 
>> {
>>  va_status = VA_STATUS_SUCCESS;
>> +} else if (!HAS_VP8_DECODING(i965) && !HAS_VP8_ENCODING(i965)){
>> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>>  } else {
>>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>  }
>> @@ -768,8 +781,12 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> profile,
>>  case VAProfileH264StereoHigh:
>>  if ((HAS_H264_MVC_DECODING_PROFILE(i965, profile) &&
>>   entrypoint == VAEntrypointVLD) ||
>> -(HAS_H264_MVC_ENCODING(i965) && entrypoint == 
>> VAEntrypointEncSlice)) {
>> +(HAS_H264_MVC_ENCODING(i965) &&
>> + entrypoint == VAEntrypointEncSlice)) {
>>  va_status = VA_STATUS_SUCCESS;
>> +} else if(!HAS_H264_MVC_DECODING_PROFILE(i965, profile) &&
>> +  !HAS_H264_MVC_ENCODING(i965)) {
>> +va_status = 

Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return unsupported profile

2016-10-18 Thread Charles, Daniel
On Tue, Oct 18, 2016 at 10:48 AM, Eoff, Ullysses A
 wrote:
>> -Original Message-
>> From: Charles, Daniel [mailto:daniel.char...@intel.com]
>> Sent: Tuesday, October 18, 2016 10:20 AM
>> To: Eoff, Ullysses A 
>> Cc: libva@lists.freedesktop.org
>> Subject: Re: [Libva] [PATCH][libva-intel-driver] i965_validate_config: 
>> return unsupported profile
>>
>> On Tue, Oct 18, 2016 at 8:25 AM, Eoff, Ullysses A
>>  wrote:
>> > Please run the tests... I suspect this patch will cause some to fail.  
>> > Those tests will need updated too.
>> >
>>
>> Ran the test suite on ToT, no regression found with this patch.
>>
>
> Ah yes, I see why now.   The AVC and JPEG config tests still pass on most HW 
> because UNSUPPORTED_PROFILE is only returned if *both* encode *and* decode 
> are unsupported.  Thus, if there is such HW that doesn't support one or the 
> other then those tests will be regressed.

I am not sure I follow you here.

Unsupported Profile is sent when neither encoder and decoder are
supported (VC1 is only decoder, VPP is a special case and VP9 decoder
can also be hybrid on some h/w).  When h/w only supports one of them
(regardless which it is), it should return UNSUPPORTED_ENTRYPOINT as
profile is supported at least once.  This patch is not changing this
last scenario. My testing was done on a h/w that has Jpeg Decoder but
no Jpeg Encoder and everything is okay.

-- 
Daniel.
>
>> --
>> Daniel.
>> > U. Artie
>> >
>> >> -Original Message-
>> >> From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of 
>> >> Daniel Charles
>> >> Sent: Monday, October 17, 2016 5:45 PM
>> >> To: libva@lists.freedesktop.org
>> >> Subject: [Libva] [PATCH][libva-intel-driver] i965_validate_config: return 
>> >> unsupported profile
>> >>
>> >> When all the profiles are not supported return
>> >> VA_STATUS_ERROR_UNSUPPORTED_PROFILE instead of
>> >> VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT.
>> >>
>> >> Also change the style on the code modified to be common
>> >> on all cases
>> >>
>> >> Signed-off-by: Daniel Charles 
>> >> ---
>> >>  src/i965_drv_video.c | 49 
>> >> -
>> >>  1 file changed, 40 insertions(+), 9 deletions(-)
>> >>
>> >> diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
>> >> index fbf2cda..0830ae0 100644
>> >> --- a/src/i965_drv_video.c
>> >> +++ b/src/i965_drv_video.c
>> >> @@ -711,6 +711,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> >> profile,
>> >>  if ((HAS_MPEG2_DECODING(i965) && entrypoint == VAEntrypointVLD) 
>> >> ||
>> >>  (HAS_MPEG2_ENCODING(i965) && entrypoint == 
>> >> VAEntrypointEncSlice)) {
>> >>  va_status = VA_STATUS_SUCCESS;
>> >> +} else if (!HAS_MPEG2_DECODING(i965) && 
>> >> !HAS_MPEG2_ENCODING(i965)){
>> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>> >>  } else {
>> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>> >>  }
>> >> @@ -723,6 +725,9 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> >> profile,
>> >>  (HAS_H264_ENCODING(i965) && entrypoint == 
>> >> VAEntrypointEncSlice) ||
>> >>  (HAS_LP_H264_ENCODING(i965) && entrypoint == 
>> >> VAEntrypointEncSliceLP)) {
>> >>  va_status = VA_STATUS_SUCCESS;
>> >> +} else if (!HAS_H264_DECODING(i965) && !HAS_H264_ENCODING(i965) 
>> >> &&
>> >> +   !HAS_LP_H264_ENCODING(i965)){
>> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>> >>  } else {
>> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>> >>  }
>> >> @@ -733,6 +738,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> >> profile,
>> >>  case VAProfileVC1Advanced:
>> >>  if (HAS_VC1_DECODING(i965) && entrypoint == VAEntrypointVLD) {
>> >>  va_status = VA_STATUS_SUCCESS;
>> >> +} else if (!HAS_VC1_DECODING(i965)) {
>> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>> >>  } else {
>> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>> >>  }
>> >> @@ -741,6 +748,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> >> profile,
>> >>  case VAProfileNone:
>> >>  if (HAS_VPP(i965) && VAEntrypointVideoProc == entrypoint) {
>> >>  va_status = VA_STATUS_SUCCESS;
>> >> +} else if (!HAS_VPP(i965)){
>> >> +va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
>> >>  } else {
>> >>  va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>> >>  }
>> >> @@ -750,6 +759,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile 
>> >> profile,
>> >>  if ((HAS_JPEG_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
>> >>  (HAS_JPEG_ENCODING(i965) && entrypoint == 
>> >> VAEntrypointEncPicture)) {
>> >> 

Re: [Libva] [PATCH v2] test: use valarray for raw image comparison

2016-10-18 Thread Eoff, Ullysses A
Ah, I overlooked the data types on my first review.  Indeed it's important to 
take it into account when diff'ing.  This is why they were being cast to "int" 
previously to calculate the diff.

On my system /dev/urandom seems to work (i.e. not all zeros).  But I agree, 
let's drop it since it does not seem to be very uniform.

This version looks much better to me.

Thanks,

U. Artie

> -Original Message-
> From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Scott D 
> Phillips
> Sent: Monday, October 17, 2016 1:01 PM
> To: libva@lists.freedesktop.org
> Subject: [Libva] [PATCH v2] test: use valarray for raw image comparison
> 
> std::valarray can fuse elementwise operations across arrays for
> more efficient comparison.
> 
> Signed-off-by: Scott D Phillips 
> ---
> Changes since v1:
> - s/width * height/sizes/
> - Added another array 'signs' which ensures that 0 and 255 never
>   compare as separated by one.
> 
> Also, the patch about /dev/urandom actually was accidentally
> generating all zeroes instead of random data.  Fixing the bug
> there eliminated the performance gain, so I'm dropping that patch
> from the series.
> 
>  test/i965_jpeg_encode_test.cpp | 29 +
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/test/i965_jpeg_encode_test.cpp b/test/i965_jpeg_encode_test.cpp
> index 29c14dc..173cd93 100644
> --- a/test/i965_jpeg_encode_test.cpp
> +++ b/test/i965_jpeg_encode_test.cpp
> @@ -30,6 +30,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  namespace JPEG {
>  namespace Encode {
> @@ -400,23 +401,19 @@ protected:
>  ASSERT_EQ(expect->height(), image.height);
>  ASSERT_NO_FAILURE(uint8_t *data = mapBuffer(image.buf));
> 
> -auto isClose = [](const uint8_t& a, const uint8_t& b) {
> -return std::abs(int(a)-int(b)) <= 2;
> -};
> -
>  for (size_t i(0); i < image.num_planes; ++i) {
> -size_t w = expect->widths[i];
> -size_t h = expect->heights[i];
> -
> -const ByteData::value_type *source = expect->plane(i);
> -const uint8_t *result = data + image.offsets[i];
> -ASSERT_GE(image.pitches[i], w);
> -for (size_t r(0); r < h; ++r) {
> -EXPECT_TRUE(std::equal(result, result + w, source, isClose))
> -<< "Byte(s) mismatch in plane " << i << " row " << r;
> -source += w;
> -result += image.pitches[i];
> -}
> +ASSERT_GE(image.pitches[i], expect->widths[i]);
> +std::valarray source(expect->plane(i), 
> expect->sizes[i]);
> +std::gslice result_slice(0, {expect->heights[i], 
> expect->widths[i]},
> +{image.pitches[i], 1});
> +std::valarray result = std::valarray(
> +data + image.offsets[i],
> +image.pitches[i] * expect->heights[i])[result_slice];
> +std::valarray signs(1, result.size());
> +signs[result > source] = -1;
> +ASSERT_EQ(source.size(), result.size());
> +EXPECT_TRUE((source * signs - result * signs).max() <= 2)
> +<< "Byte(s) mismatch in plane " << i;
>  }
> 
>  unmapBuffer(image.buf);
> --
> 2.7.4
> 
> ___
> Libva mailing list
> Libva@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/libva
___
Libva mailing list
Libva@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libva


Re: [Libva] [Libva-intel-driver][PATCH] Use Media Read message if possible on Gen8+

2016-10-18 Thread Eoff, Ullysses A
I've created a bugzilla for the issue... 
https://bugs.freedesktop.org/show_bug.cgi?id=98311

Let's continue discussion there.

U. Artie

> -Original Message-
> From: Sean V Kelley [mailto:sean.v.kel...@intel.com]
> Sent: Monday, October 17, 2016 10:35 AM
> To: Xiang, Haihao 
> Cc: Zhao, Yakui ; Eoff, Ullysses A 
> ; libva@lists.freedesktop.org
> Subject: Re: [Libva] [Libva-intel-driver][PATCH] Use Media Read message if 
> possible on Gen8+
> 
> Yes, I want to throttle the pace of the changes until we qualify and
> understand the performance and test failure issues. Then we can resume
> normal flow.
> 
> For these tests to be meaningful they have to be consistent across
> distributions and platforms.
> 
> Thanks,
> 
> Sean
> 
> 
> On Sun, Oct 16, 2016 at 8:11 PM, Xiang, Haihao  wrote:
> >
> > Hi Sean,
> >
> > I can't reproduce the issue mentioned by you and Artie on my SKL. I am
> > using Ubuntu 14.04 but built kernel and libdrm from source code:
> >
> > Linux kernel:
> > aab15c274da587bcab19376d2caa9d6626440335 (drm-intel-nightly: 2016y-09m-
> > 26d-12h-11m-33s)
> >
> > libdrm: 2.4.70
> >
> > I don't think the bus error is related to this patch, but I would hold
> > off on merging this patch if you reproduced the error with
> > Common/JPEGEncodeInputTest.Full/95 only.
> >
> > Thanks
> > Haihao
> >
> >
> >
> >> On 10/15/2016 03:58 AM, Eoff, Ullysses A wrote:
> >> >
> >> > > -Original Message-
> >> > > From: Libva [mailto:libva-boun...@lists.freedesktop.org] On
> >> > > Behalf Of Sean V Kelley
> >> > > Sent: Friday, October 14, 2016 11:44 AM
> >> > > To: Zhao, Yakui; Xiang, Haihao >> > > @intel.com>
> >> > > Cc: libva@lists.freedesktop.org
> >> > > Subject: Re: [Libva] [Libva-intel-driver][PATCH] Use Media Read
> >> > > message if possible on Gen8+
> >> > >
> >> > > On Fri, 2016-10-14 at 08:37 +0800, Zhao Yakui wrote:
> >> > > > On 10/13/2016 03:07 AM, Xiang, Haihao wrote:
> >> > > > > AVS can't gurantee bit-match for a large surface. This fixes
> >> > > > > the
> >> > > > > failure reported by gtest case
> >> > > > > Common/JPEGEncodeInputTest.Full/95.
> >> > > > >
> >> > > > > before:
> >> > > > > [  FAILED  ] Common/JPEGEncodeInputTest.Full/95, where
> >> > > > > GetParam() =
> >> > > > > (Fixed Size 7680x4320, 0x501176 pointing to "I420") (9239 ms)
> >> > > > > [--] 1 test from Common/JPEGEncodeInputTest (9239 ms
> >> > > > > total)
> >> > > > >
> >> > > > > [--] Global test environment tear-down
> >> > > > > [==] 1 test from 1 test case ran. (9361 ms total)
> >> > > > > [  PASSED  ] 0 tests.
> >> > > > > [  FAILED  ] 1 test, listed below:
> >> > > > > [  FAILED  ] Common/JPEGEncodeInputTest.Full/95, where
> >> > > > > GetParam() =
> >> > > > > (Fixed Size 7680x4320, 0x501176 pointing to "I420")
> >> > > > >
> >> > > > > after:
> >> > > > > [   OK ] Common/JPEGEncodeInputTest.Full/95 (15250 ms)
> >> > > > > [--] 1 test from Common/JPEGEncodeInputTest (15250 ms
> >> > > > > total)
> >> > > > >
> >> > > > > [--] Global test environment tear-down
> >> > > > > [==] 1 test from 1 test case ran. (15365 ms total)
> >> > > > > [  PASSED  ] 1 test.
> >> > > > >
> >> > > > > Signed-off-by: Xiang, Haihao
> >> > > >
> >> > > > This looks good to me.
> >> > > >
> >> > > > Add: Reviewed-by: Zhao Yakui
> >> > >
> >> > >
> >> > > Getting a core dump although the test is now passing.  We'll need
> >> > > to
> >> > > debug further.
> >> > >
> >> >
> >> > I don't get any core dump on my SKL...  And the test passes for me
> >> > with
> >> > this patch.  Are you running the test in isolation or with the
> >> > entire suite?
> >> > If with the entire suite, does the core dump occur during this test
> >> > case or
> >> > another?
> >> >
> >> > I've noticed that I occasionally get a "bus error" with the
> >> > Big/JPEGEncodeInputTest.* test cases (i.e. 8192x8192) even without
> >> > this patch.  Perhaps this is what you're encountering?
> >>
> >> Hi, Sean/Artie
> >>
> >>  Is it possible that you can send out the dmesg log when the
> >> bus_error is triggered?
> >>  I try the test several times on one KBL machine(similar to SKL)
> >> and
> >> unfortunately there is no "sig_bus error".
> >>
> >> Thanks
> >> Yakui
> >>
> >> >
> >> > U. Artie
> >> >
> >> > > Sean
> >> > >
> >> > > >
> >> > > > Thanks
> >> > > > Yakui
> >> > > >
> >> > > > > ---
> >> > > > >src/gen8_post_processing.c | 56
> >> > > > > +-
> >> > > > >src/shaders/post_processing/gen8/Makefile.am   |  2 +
> >> > > > >.../gen8/PL2_media_read_buf0123.g8a| 65
> >> > > > > +
> >> > > > >.../gen8/PL3_media_read_buf0123.g8a| 68
> >> > > > > ++
> >> > > > >