Re: [Libva] [PATCH 2/4] Set the pipeline to use the new VP8 encoding shaders on BSW

2017-01-11 Thread Xiang, Haihao 

Hi Mark,

Can you reproduce the issue you mentioned below? If yes, I would like to fix it
in the new version of the patch series. 

Thanks
Haihao

> > 
> > On Tue, Jan 10, 2017 at 4:21 PM, Mark Thompson  wrote:
> > > On 10/01/17 22:02, Sean V Kelley wrote:
> > > > From: "Xiang, Haihao" 
> > > > 
> > > > Currently only one temporal layer is supported
> > > > 
> > > > Signed-off-by: Xiang, Haihao 
> > > > Reviewed-by: Sean V Kelley 
> > > > ---
> > > >   src/Makefile.am        |    3 +
> > > >   src/gen8_encoder_vp8.c |  140 +
> > > >   src/gen8_mfc.c         |    8 +-
> > > >   src/gen8_vme.c         |    5 +
> > > >   src/i965_defines.h     |   10 +
> > > >   src/i965_encoder.c     |    2 +
> > > >   src/i965_encoder_vp8.c | 6697
> > > 
> > > >   src/i965_encoder_vp8.h | 2643 +++
> > > >   8 files changed, 9507 insertions(+), 1 deletion(-)
> > > 
> > > I had a go with this on Kaby Lake.  In general, big win - looks like it
> > > can
> > > be under half the bitrate at comparable quality (though it was pretty
> > > terrible before...).
> > > 
> > > However, the rate control seems to do odd things at low bitrate relative
> > > to
> > > the frame size?  I can get GPU hangs and wildly varying output bitrate
> > > with
> > > it, though it seems ok at high bitrate.
> > That's a concern.  Please report the If it really is a GPU hang, I need
> > the error report for the DRM card0 log.
> > 
> > cat /sys/class/drm/card0/error
> > 
> > Please rerun and capture the DRM (i915) card0 error log.
> >  
> > >  
> > > I had a look around the rate control and found two minor issues in the RC
> > > configuration, though I don't think either of them are relevant to my
> > > problem (see below).  I can try to make a reproducer if this is not
> > > already
> > > known?
> > > 
> > Please do attempt to reproduce.  That's why I've put the patches out here to
> > test.
> 
> Thanks for testing the patch, could you detail the steps to reproduce this
> issue?
> 
> 
> > 
> > Thanks,
> > 
> > Sean
> >  
> > >  Thanks,
> > > 
> > > - Mark
> > > 
> > > 
> > > > ...
> > > > +
> > > > +static void
> > > > +i965_encoder_vp8_get_misc_parameters(VADriverContextP ctx,
> > > > +                                     struct encode_state *encode_state,
> > > > +                                     struct intel_encoder_context
> > > *encoder_context)
> > > > +{
> > > > +    struct i965_encoder_vp8_context *vp8_context = encoder_context-
> > > > vme_context;
> > > > +
> > > > +    if (vp8_context->internal_rate_mode == I965_BRC_CQP) {
> > > > +        vp8_context->init_vbv_buffer_fullness_in_bit = 0;
> > > > +        vp8_context->vbv_buffer_size_in_bit = 0;
> > > > +        vp8_context->target_bit_rate = 0;
> > > > +        vp8_context->max_bit_rate = 0;
> > > > +        vp8_context->min_bit_rate = 0;
> > > > +        vp8_context->brc_need_reset = 0;
> > > > +    } else {
> > > > +        vp8_context->gop_size = encoder_context->brc.gop_size;
> > > > +
> > > > +        if (encoder_context->brc.need_reset) {
> > > > +            vp8_context->framerate = encoder_context->brc.framerate[0];
> > > > +            vp8_context->vbv_buffer_size_in_bit = encoder_context-
> > > > brc.hrd_buffer_size;
> > > > +            vp8_context->init_vbv_buffer_fullness_in_bit =
> > > encoder_context->brc.hrd_initial_buffer_fullness;
> > > > +            vp8_context->max_bit_rate = encoder_context-
> > > > brc.bits_per_second[0]; // currently only one layer is supported
> > > > +            vp8_context->brc_need_reset = (vp8_context->brc_initted &&
> > > encoder_context->brc.need_reset);
> > > > +
> > > > +            if (vp8_context->internal_rate_mode == I965_BRC_CBR) {
> > > > +                vp8_context->min_bit_rate = vp8_context->max_bit_rate;
> > > > +                vp8_context->target_bit_rate = vp8_context-
> > > > >max_bit_rate;
> > > > +            } else {
> > > > +                assert(vp8_context->internal_rate_mode ==
> > > > I965_BRC_VBR);
> > > > +                vp8_context->min_bit_rate = vp8_context->max_bit_rate *
> > > (2 * encoder_context->brc.target_percentage[0] - 100) / 100;
> > > 
> > > If target percentage is < 50 then (2 * encoder_context-
> > > > brc.target_percentage[0] - 100) is negative.  Since it's unsigned, you
> > > > end
> > > up with a garbage number in min_bit_rate.
> > That's a concern, also we may need to reconcile this with our handling for
> > VP9
> > encode.
> >  
> > >  
> > > > +                vp8_context->target_bit_rate = vp8_context-
> > > > >max_bit_rate
> > > * encoder_context->brc.target_percentage[0] / 100;
> > > > +            }
> > > > +        }
> > > > +    }
> > > > +
> > > > +    if (encoder_context->quality_level == ENCODER_LOW_QUALITY)
> > > > +        vp8_context->hme_16x_supported = 0;
> > > > +}
> > > > +
> > > > ...
> > > > +
> > > > +static void
> > > >

Re: [Libva] [PATCH 31/31] ENC:support more quality level and switch to new AVC encoder solution on SKL

2017-01-11 Thread Chen, Peng C 


-Original Message-
From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Sean V 
Kelley
Sent: Wednesday, January 11, 2017 7:38 AM
To: libva@lists.freedesktop.org
Subject: [Libva] [PATCH 31/31] ENC:support more quality level and switch to new 
AVC encoder solution on SKL

From: Pengfei Qu 

Signed-off-by: Pengfei Qu 
Signed-off-by: Sean V Kelley 
---
 src/Makefile.am  | 11 +++
 src/i965_drv_video.c |  8 ++--
 src/i965_drv_video.h |  2 ++
 src/i965_encoder.c   | 39 +++
 4 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am index 424812b3..9a5e44bc 100755
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -101,6 +101,11 @@ source_c = \
gen9_vp9_encoder_kernels.c  \
gen9_vp9_const_def.c  \
gen9_vp9_encoder.c  \
+   i965_avc_encoder_common.c  \
+   i965_encoder_common.c  \
+   gen9_avc_encoder_kernels.c  \
+   gen9_avc_const_def.c  \
+   gen9_avc_encoder.c  \
intel_common_vpp_internal.c   \
$(NULL)
 
@@ -154,6 +159,12 @@ source_h = \
gen9_vp9_encapi.h   \
gen9_vp9_const_def.h  \
gen9_vp9_encoder_kernels.h   \
+   i965_encoder_api.h   \
+   i965_avc_encoder_common.h   \
+   i965_encoder_common.h   \
+   gen9_avc_encoder.h   \
+   gen9_avc_const_def.h  \
+   gen9_avc_encoder_kernels.h   \
intel_gen_vppapi.h   \
intel_common_vpp_internal.h   \
$(NULL)
diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c index 
cc371905..64cc0e20 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -911,6 +911,7 @@ i965_GetConfigAttributes(VADriverContextP ctx,
  VAConfigAttrib *attrib_list,  /* in/out */
  int num_attribs)  {
+struct i965_driver_data * const i965 = i965_driver_data(ctx);
 VAStatus va_status;
 int i;
 
@@ -1003,8 +1004,11 @@ i965_GetConfigAttributes(VADriverContextP ctx,
 attrib_list[i].value = 1;
 if (profile == VAProfileH264ConstrainedBaseline ||
 profile == VAProfileH264Main ||
-profile == VAProfileH264High )
-attrib_list[i].value = ENCODER_QUALITY_RANGE;
+profile == VAProfileH264High ){
+attrib_list[i].value = ENCODER_QUALITY_RANGE;
+if(IS_GEN9(i965->intel.device_info))
+attrib_list[i].value = ENCODER_QUALITY_RANGE_AVC;
+}
 break;
 }
 break;
diff --git a/src/i965_drv_video.h b/src/i965_drv_video.h index 
7cba3a37..334b7882 100644
--- a/src/i965_drv_video.h
+++ b/src/i965_drv_video.h
@@ -69,7 +69,9 @@
 #define DEFAULT_SATURATION  50
 
 #define ENCODER_QUALITY_RANGE 2
+#define ENCODER_QUALITY_RANGE_AVC 8
 #define ENCODER_DEFAULT_QUALITY   1
+#define ENCODER_DEFAULT_QUALITY_AVC   4
 #define ENCODER_HIGH_QUALITY  ENCODER_DEFAULT_QUALITY
 #define ENCODER_LOW_QUALITY   2
 
diff --git a/src/i965_encoder.c b/src/i965_encoder.c index 0a648d4d..beac9115 
100644
--- a/src/i965_encoder.c
+++ b/src/i965_encoder.c
@@ -41,6 +41,7 @@
 #include "gen6_mfc.h"
 
 #include "i965_post_processing.h"
+#include "i965_encoder_api.h"
 
 static struct intel_fraction
 reduce_fraction(struct intel_fraction f) @@ -789,6 +790,7 @@ 
intel_encoder_check_temporal_layer_structure(VADriverContextP ctx,
 
 static VAStatus
 intel_encoder_check_misc_parameter(VADriverContextP ctx,
+  VAProfile profile,
   struct encode_state *encode_state,
   struct intel_encoder_context 
*encoder_context)  { @@ -800,12 +802,23 @@ 
intel_encoder_check_misc_parameter(VADriverContextP ctx,
 VAEncMiscParameterBufferQualityLevel* param_quality_level = 
(VAEncMiscParameterBufferQualityLevel*)pMiscParam->data;
 encoder_context->quality_level = param_quality_level->quality_level;
 
-if (encoder_context->quality_level == 0)
-encoder_context->quality_level = ENCODER_DEFAULT_QUALITY;
-else if (encoder_context->quality_level > 
encoder_context->quality_range) {
-ret = VA_STATUS_ERROR_INVALID_PARAMETER;
-goto out;
+switch (profile) {
+case VAProfileH264ConstrainedBaseline:
+case VAProfileH264Main:
+case VAProfileH264High:
+if (encoder_context->quality_level == 0)
+encoder_context->quality_level = ENCODER_DEFAULT_QUALITY_AVC;
+break;
+default:
+if (encoder_context->quality_level == 0)
+encoder_context->quality_level = ENCODER_DEFAULT_QUALITY;
+

Re: [Libva] [PATCH 01/31] ENC: move gpe related function into src/i965_gpe_utils.h/c

2017-01-11 Thread Qu, Pengfei 


-Original Message-
From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Zhao Yakui
Sent: Wednesday, January 11, 2017 10:26 PM
To: libva@lists.freedesktop.org
Subject: Re: [Libva] [PATCH 01/31] ENC: move gpe related function into 
src/i965_gpe_utils.h/c

On 01/11/2017 07:37 AM, Sean V Kelley wrote:
> From: Pengfei Qu
>
> Signed-off-by: Pengfei Qu
> Reviewed-by: Sean V Kelley
> ---
>   src/gen9_vp9_encoder.c | 154 ++--
>   src/gen9_vp9_encoder.h |  10 --
>   src/i965_gpe_utils.c   | 265 
> -
>   src/i965_gpe_utils.h   |  87 
>   4 files changed, 356 insertions(+), 160 deletions(-)
>
> diff --git a/src/gen9_vp9_encoder.c b/src/gen9_vp9_encoder.c index 
> 05d86dae..32ed729c 100644
> --- a/src/gen9_vp9_encoder.c
> +++ b/src/gen9_vp9_encoder.c
> @@ -58,7 +58,6 @@
>   #define BRC_KERNEL_AVBR 0x0040
>   #define BRC_KERNEL_CQL  0x0080
>
> -#define DEFAULT_MOCS  0x02
>   #define VP9_PIC_STATE_BUFFER_SIZE 192
>
>   typedef struct _intel_kernel_header_ @@ -842,7 +841,7 @@ 
> gen9_vp9_free_resources(struct gen9_encoder_context_vp9 *vme_context)
>
>   static void
>   gen9_init_media_object_walker_parameter(struct intel_encoder_context 
> *encoder_context,
> -struct 
> vp9_encoder_kernel_walker_parameter *kernel_walker_param,
> +struct 
> + gpe_encoder_kernel_walker_parameter *kernel_walker_param,
>   struct 
> gpe_media_object_walker_parameter *walker_param)
>   {
>   memset(walker_param, 0, sizeof(*walker_param)); @@ -924,147 
> +923,6 @@ gen9_init_media_object_walker_parameter(struct 
> intel_encoder_context *encoder_co
>   }
>
>   static void
> -gen9_add_2d_gpe_surface(VADriverContextP ctx,
> -struct i965_gpe_context *gpe_context,
> -struct object_surface *obj_surface,
> -int is_uv_surface,
> -int is_media_block_rw,
> -unsigned int format,
> -int index)
> -{
> -struct i965_gpe_resource gpe_resource;
> -struct i965_gpe_surface gpe_surface;
> -
> -memset(_surface, 0, sizeof(gpe_surface));
> -
> -i965_object_surface_to_2d_gpe_resource(_resource, obj_surface);
> -gpe_surface.gpe_resource =_resource;
> -gpe_surface.is_2d_surface = 1;
> -gpe_surface.is_uv_surface = !!is_uv_surface;
> -gpe_surface.is_media_block_rw = !!is_media_block_rw;
> -
> -gpe_surface.cacheability_control = DEFAULT_MOCS;
> -gpe_surface.format = format;
> -
> -gen9_gpe_context_add_surface(gpe_context,_surface, index);
> -i965_free_gpe_resource(_resource);
> -}
> -
> -static void
> -gen9_add_adv_gpe_surface(VADriverContextP ctx,
> - struct i965_gpe_context *gpe_context,
> - struct object_surface *obj_surface,
> - int index)
> -{
> -struct i965_gpe_resource gpe_resource;
> -struct i965_gpe_surface gpe_surface;
> -
> -memset(_surface, 0, sizeof(gpe_surface));
> -
> -i965_object_surface_to_2d_gpe_resource(_resource, obj_surface);
> -gpe_surface.gpe_resource =_resource;
> -gpe_surface.is_adv_surface = 1;
> -gpe_surface.cacheability_control = DEFAULT_MOCS;
> -gpe_surface.v_direction = 2;
> -
> -gen9_gpe_context_add_surface(gpe_context,_surface, index);
> -i965_free_gpe_resource(_resource);
> -}
> -
> -static void
> -gen9_add_buffer_gpe_surface(VADriverContextP ctx,
> -struct i965_gpe_context *gpe_context,
> -struct i965_gpe_resource *gpe_buffer,
> -int is_raw_buffer,
> -unsigned int size,
> -unsigned int offset,
> -int index)
> -{
> -struct i965_gpe_surface gpe_surface;
> -
> -memset(_surface, 0, sizeof(gpe_surface));
> -
> -gpe_surface.gpe_resource = gpe_buffer;
> -gpe_surface.is_buffer = 1;
> -gpe_surface.is_raw_buffer = !!is_raw_buffer;
> -gpe_surface.cacheability_control = DEFAULT_MOCS;
> -gpe_surface.size = size;
> -gpe_surface.offset = offset;
> -
> -gen9_gpe_context_add_surface(gpe_context,_surface, index);
> -}
> -
> -static void
> -gen9_add_buffer_2d_gpe_surface(VADriverContextP ctx,
> -   struct i965_gpe_context *gpe_context,
> -   struct i965_gpe_resource *gpe_buffer,
> -   int is_media_block_rw,
> -   unsigned int format,
> -   int index)
> -{
> -struct i965_gpe_surface gpe_surface;
> -
> -memset(_surface, 0, sizeof(gpe_surface));
> -
> -

Re: [Libva] [PATCH 03/31] ENC:add context init function for AVC/HEVC encoder

2017-01-11 Thread Qu, Pengfei 


-Original Message-
From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Zhao Yakui
Sent: Wednesday, January 11, 2017 9:37 AM
To: libva@lists.freedesktop.org
Subject: Re: [Libva] [PATCH 03/31] ENC:add context init function for AVC/HEVC 
encoder

On 01/11/2017 07:37 AM, Sean V Kelley wrote:
> From: Pengfei Qu
>
> Signed-off-by: Pengfei Qu
> Reviewed-by: Sean V Kelley
> ---
>   src/i965_encoder_api.h | 59 
> ++
>   1 file changed, 59 insertions(+)
>   create mode 100755 src/i965_encoder_api.h
>
> diff --git a/src/i965_encoder_api.h b/src/i965_encoder_api.h new file 
> mode 100755 index ..0f1ab6cd
> --- /dev/null
> +++ b/src/i965_encoder_api.h
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, 
> +including
> + * without limitation the rights to use, copy, modify, merge, 
> +publish,
> + * distribute, sub license, and/or sell copies of the Software, and 
> +to
> + * permit persons to whom the Software is furnished to do so, subject 
> +to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including 
> +the
> + * next paragraph) shall be included in all copies or substantial 
> +portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE 
> +FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF 
> +CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Authors:
> + * Pengfei Qu
> + *
> + */
> +
> +#ifndef _I965_ENCODER_API_H_
> +#define _I965_ENCODER_API_H_
> +
> +#include
> +
> +struct intel_encoder_context;
> +struct hw_context;
> +
> +/* H264/AVC */
> +extern Bool
> +gen9_avc_vme_context_init(VADriverContextP ctx, struct 
> +intel_encoder_context *encoder_context);
> +
> +extern Bool
> +gen9_avc_pak_context_init(VADriverContextP ctx, struct 
> +intel_encoder_context *encoder_context);
> +
> +extern VAStatus
> +gen9_avc_coded_status(VADriverContextP ctx, char *buffer, struct 
> +hw_context *hw_context);
> +
> +
> +/* H265/HEVC */
> +extern Bool
> +gen9_hevc_vme_context_init(VADriverContextP ctx, struct 
> +intel_encoder_context *encoder_context);
> +
> +extern Bool
> +gen9_hevc_pak_context_init(VADriverContextP ctx, struct 
> +intel_encoder_context *encoder_context);
> +
> +extern VAStatus
> +gen9_hevc_coded_status(VADriverContextP ctx, char *buffer, struct 
> +hw_context *hw_context);
> +

It seems that the function API is defined. But there is no implementation.
In such case the compiler will complain the warning message.

So it will be better to declare them after the implementation is added.

At the same time this patch set is mainly added for H264.
It will be better to focus the stuff related with H264.

Thanks
[Pengfei] I will only keep H264 part. Here this file is not in the Makefile.am 
and we will not see the warning.
This file is added into Makefile.am when it really need compling.

> +
> +#endif  // _I965_ENCODER_API_H_

___
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] [PATCH 08/31] ENC: add misc parameter check for AVC encoder

2017-01-11 Thread Qu, Pengfei 

-Original Message-
From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Zhao Yakui
Sent: Wednesday, January 11, 2017 11:13 AM
To: Sean V Kelley 
Cc: libva@lists.freedesktop.org
Subject: Re: [Libva] [PATCH 08/31] ENC: add misc parameter check for AVC encoder

On 01/11/2017 07:37 AM, Sean V Kelley wrote:
> From: Pengfei Qu
>
> Signed-off-by: Pengfei Qu
> Reviewed-by: Sean V Kelley
> ---
>   src/gen9_avc_encoder.c | 548 
> +
>   1 file changed, 548 insertions(+)
>   create mode 100755 src/gen9_avc_encoder.c
>
> diff --git a/src/gen9_avc_encoder.c b/src/gen9_avc_encoder.c new file 
> mode 100755 index ..8f1ad79f
> --- /dev/null
> +++ b/src/gen9_avc_encoder.c
> @@ -0,0 +1,548 @@
> +/*
> + * Copyright ? 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, 
> +including
> + * without limitation the rights to use, copy, modify, merge, 
> +publish,
> + * distribute, sub license, and/or sell copies of the Software, and 
> +to
> + * permit persons to whom the Software is furnished to do so, subject 
> +to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including 
> +the
> + * next paragraph) shall be included in all copies or substantial 
> +portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE 
> +FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF 
> +CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWAR
> + *
> + * Authors:
> + *Pengfei Qu
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "intel_batchbuffer.h"
> +#include "intel_driver.h"
> +
> +#include "i965_defines.h"
> +#include "i965_structs.h"
> +#include "i965_drv_video.h"
> +#include "i965_encoder.h"
> +#include "i965_encoder_utils.h"
> +#include "intel_media.h"
> +
> +#include "i965_gpe_utils.h"
> +#include "i965_encoder_common.h"
> +#include "i965_avc_encoder_common.h"
> +#include "gen9_avc_encoder_kernels.h"
> +#include "gen9_avc_encoder.h"
> +#include "gen9_avc_const_def.h"
> +
> +#define MAX_URB_SIZE4096 /* In register */
> +#define NUM_KERNELS_PER_GPE_CONTEXT 1
> +#define MBENC_KERNEL_BASE GEN9_AVC_KERNEL_MBENC_QUALITY_I
> +

Please use the OUT_BCS_RELOC64 instead of OUT_BCS_RELOC so that it can work for 
48-bit GPU address.
[Pengfei] Good comments. It will be modified.

> +#define OUT_BUFFER_2DW(batch, bo, is_target, delta)  do {   \
> +if (bo) {   \
> +OUT_BCS_RELOC(batch,\
> +bo, \
> +I915_GEM_DOMAIN_INSTRUCTION,\
> +is_target ? I915_GEM_DOMAIN_INSTRUCTION : 0, 
> \
> +delta); \
> +OUT_BCS_BATCH(batch, 0);\
> +} else {\
> +OUT_BCS_BATCH(batch, 0);\
> +OUT_BCS_BATCH(batch, 0);\
> +}   \
> +} while (0)
> +
> +#define OUT_BUFFER_3DW(batch, bo, is_target, delta, attr)  do { \
> +OUT_BUFFER_2DW(batch, bo, is_target, delta);\
> +OUT_BCS_BATCH(batch, attr); \
> +} while (0)
> +
> +
> +static const uint32_t qm_flat[16] = {
> +0x10101010, 0x10101010, 0x10101010, 0x10101010,
> +0x10101010, 0x10101010, 0x10101010, 0x10101010,
> +0x10101010, 0x10101010, 0x10101010, 0x10101010,
> +0x10101010, 0x10101010, 0x10101010, 0x10101010 };
> +
> +static const uint32_t fqm_flat[32] = {
> +0x10001000, 0x10001000, 0x10001000, 0x10001000,
> +0x10001000, 0x10001000, 0x10001000, 0x10001000,
> +0x10001000, 0x10001000, 0x10001000, 0x10001000,
> +0x10001000, 0x10001000, 0x10001000, 0x10001000,
> +0x10001000, 0x10001000, 0x10001000, 0x10001000,
> +0x10001000, 0x10001000, 0x10001000, 0x10001000,
> +0x10001000, 0x10001000, 0x10001000, 0x10001000,
> +0x10001000, 0x10001000, 0x10001000, 0x10001000 };
> +
> +static unsigned int

Re: [Libva] [PATCH 00/31] Encoder Architecture Changes (Primarily AVC)

2017-01-11 Thread Xiang, Haihao 
On Thu, 2017-01-12 at 09:40 +0800, Zhao Yakui wrote:
> On 01/11/2017 07:37 AM, Sean V Kelley wrote:
> > Encoder architecture restructuring for H.264 (with some impact to HEVC now)
> > on HSW+
> > * Improvements to the shaders
> > * Improvements to the B frame efficiency
> > * Improvements to the low bit rate mode
> > * Improved features in two stage VME/PAK pipeline
> > 
> 
> After checking the code, it seems that assert is used widely.
> But the assert is only for the debug purpose, which causes that the 
> program will abort. This doesn't make sense. It will be better that the 
> failure status is returned instead of crash program.
> 
> At the same time if the NDEBUG definition is added, the check is 
> disabled. In such case the driver should handle the failure scenario and 
> return the failure status to upper middleware.
> 
> That is to say: The assert had better be avoided and add more check to 
> check the failure status.

We can add some assert() for internal logic check in the driver, it would be
better not use assert() for input validation.

Thanks
Haihao


> Thanks.
> 
> > 
> > Pengfei Qu (31):
> >    ENC: move gpe related function into src/i965_gpe_utils.h/c
> >    ENC: add common structure for AVC/HEVC encoder
> >    ENC:add context init function for AVC/HEVC encoder
> >    ENC: add const data/table for AVC encoder
> >    ENC: add AVC kernel binary on SKL
> >    ENC: add AVC common structure and functions
> >    ENC: add kernel related structure and define for AVC
> >    ENC: add misc parameter check for AVC encoder
> >    ENC: add resource and surface allocation and free function for AVC
> >  encoder
> >    ENC: add init table for frame/mb brc update
> >    ENC: add resource/surface allocation/free function for AVC encoder
> >    ENC: add kernel media object related functions for AVC encoder
> >    ENC: add scaling kernel for AVC encoder
> >    ENC: add const data/table init function for AVC RC logic
> >    ENC: add BRC init/reset kernel for AVC RC logic
> >    ENC: add BRC frame update kernel for AVC RC logic
> >    ENC: add BRC MB level update kernel for AVC RC logic
> >    ENC: add REF frame QA caculation and MB level const data init for AVC
> >  MBenc stage
> >    ENC: MBENC kernel for AVC encoder
> >    ENC: ME kernel for AVC encoder
> >    ENC: WP/SFD kernel for AVC encoder
> >    ENC: kernel init/destroy function for AVC encoder
> >    ENC: kernel related parameter check function for AVC encoder
> >    ENC: VME pipeline init/prepare/run function for AVC encoder
> >    ENC: add MFX command for AVC encoder
> >    ENC: add MFX command for AVC encoder
> >    ENC: add MFX Picture/slice level command init for AVC encoder
> >    ENC: add MFX pipeline init/prepare/run for AVC encoder
> >    ENC: add VME/MFX context init for AVC encoder
> >    ENC: add Misc parameter check for AVC encoder
> >    ENC:support more quality level and switch to new AVC encoder solution
> >  on SKL
> > 
> >   src/Makefile.am|11 +
> >   src/gen9_avc_const_def.c   |  1090 
> >   src/gen9_avc_const_def.h   |   115 +
> >   src/gen9_avc_encoder.c |  7613 
> >   src/gen9_avc_encoder.h |  2345 
> >   src/gen9_avc_encoder_kernels.c | 12081
> > +++
> >   src/gen9_avc_encoder_kernels.h |36 +
> >   src/gen9_vp9_encoder.c |   154 +-
> >   src/gen9_vp9_encoder.h |10 -
> >   src/i965_avc_encoder_common.c  |   319 ++
> >   src/i965_avc_encoder_common.h  |   305 +
> >   src/i965_drv_video.c   | 8 +-
> >   src/i965_drv_video.h   | 2 +
> >   src/i965_encoder.c |39 +-
> >   src/i965_encoder_api.h |59 +
> >   src/i965_encoder_common.c  |   124 +
> >   src/i965_encoder_common.h  |   533 ++
> >   src/i965_gpe_utils.c   |   265 +-
> >   src/i965_gpe_utils.h   |87 +
> >   19 files changed, 25026 insertions(+), 170 deletions(-)
> >   create mode 100755 src/gen9_avc_const_def.c
> >   create mode 100755 src/gen9_avc_const_def.h
> >   create mode 100755 src/gen9_avc_encoder.c
> >   create mode 100755 src/gen9_avc_encoder.h
> >   create mode 100755 src/gen9_avc_encoder_kernels.c
> >   create mode 100755 src/gen9_avc_encoder_kernels.h
> >   create mode 100755 src/i965_avc_encoder_common.c
> >   create mode 100755 src/i965_avc_encoder_common.h
> >   create mode 100755 src/i965_encoder_api.h
> >   create mode 100755 src/i965_encoder_common.c
> >   create mode 100755 src/i965_encoder_common.h
> > 
> 
> ___
> 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] [PATCH 00/31] Encoder Architecture Changes (Primarily AVC)

2017-01-11 Thread Zhao Yakui 

On 01/11/2017 07:37 AM, Sean V Kelley wrote:

Encoder architecture restructuring for H.264 (with some impact to HEVC now) on 
HSW+
* Improvements to the shaders
* Improvements to the B frame efficiency
* Improvements to the low bit rate mode
* Improved features in two stage VME/PAK pipeline



After checking the code, it seems that assert is used widely.
But the assert is only for the debug purpose, which causes that the 
program will abort. This doesn't make sense. It will be better that the 
failure status is returned instead of crash program.


At the same time if the NDEBUG definition is added, the check is 
disabled. In such case the driver should handle the failure scenario and 
return the failure status to upper middleware.


That is to say: The assert had better be avoided and add more check to 
check the failure status.


Thanks.



Pengfei Qu (31):
   ENC: move gpe related function into src/i965_gpe_utils.h/c
   ENC: add common structure for AVC/HEVC encoder
   ENC:add context init function for AVC/HEVC encoder
   ENC: add const data/table for AVC encoder
   ENC: add AVC kernel binary on SKL
   ENC: add AVC common structure and functions
   ENC: add kernel related structure and define for AVC
   ENC: add misc parameter check for AVC encoder
   ENC: add resource and surface allocation and free function for AVC
 encoder
   ENC: add init table for frame/mb brc update
   ENC: add resource/surface allocation/free function for AVC encoder
   ENC: add kernel media object related functions for AVC encoder
   ENC: add scaling kernel for AVC encoder
   ENC: add const data/table init function for AVC RC logic
   ENC: add BRC init/reset kernel for AVC RC logic
   ENC: add BRC frame update kernel for AVC RC logic
   ENC: add BRC MB level update kernel for AVC RC logic
   ENC: add REF frame QA caculation and MB level const data init for AVC
 MBenc stage
   ENC: MBENC kernel for AVC encoder
   ENC: ME kernel for AVC encoder
   ENC: WP/SFD kernel for AVC encoder
   ENC: kernel init/destroy function for AVC encoder
   ENC: kernel related parameter check function for AVC encoder
   ENC: VME pipeline init/prepare/run function for AVC encoder
   ENC: add MFX command for AVC encoder
   ENC: add MFX command for AVC encoder
   ENC: add MFX Picture/slice level command init for AVC encoder
   ENC: add MFX pipeline init/prepare/run for AVC encoder
   ENC: add VME/MFX context init for AVC encoder
   ENC: add Misc parameter check for AVC encoder
   ENC:support more quality level and switch to new AVC encoder solution
 on SKL

  src/Makefile.am|11 +
  src/gen9_avc_const_def.c   |  1090 
  src/gen9_avc_const_def.h   |   115 +
  src/gen9_avc_encoder.c |  7613 
  src/gen9_avc_encoder.h |  2345 
  src/gen9_avc_encoder_kernels.c | 12081 +++
  src/gen9_avc_encoder_kernels.h |36 +
  src/gen9_vp9_encoder.c |   154 +-
  src/gen9_vp9_encoder.h |10 -
  src/i965_avc_encoder_common.c  |   319 ++
  src/i965_avc_encoder_common.h  |   305 +
  src/i965_drv_video.c   | 8 +-
  src/i965_drv_video.h   | 2 +
  src/i965_encoder.c |39 +-
  src/i965_encoder_api.h |59 +
  src/i965_encoder_common.c  |   124 +
  src/i965_encoder_common.h  |   533 ++
  src/i965_gpe_utils.c   |   265 +-
  src/i965_gpe_utils.h   |87 +
  19 files changed, 25026 insertions(+), 170 deletions(-)
  create mode 100755 src/gen9_avc_const_def.c
  create mode 100755 src/gen9_avc_const_def.h
  create mode 100755 src/gen9_avc_encoder.c
  create mode 100755 src/gen9_avc_encoder.h
  create mode 100755 src/gen9_avc_encoder_kernels.c
  create mode 100755 src/gen9_avc_encoder_kernels.h
  create mode 100755 src/i965_avc_encoder_common.c
  create mode 100755 src/i965_avc_encoder_common.h
  create mode 100755 src/i965_encoder_api.h
  create mode 100755 src/i965_encoder_common.c
  create mode 100755 src/i965_encoder_common.h



___
Libva mailing list
Libva@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libva

Re: [Libva] [PATCH 00/31] Encoder Architecture Changes (Primarily AVC)

2017-01-11 Thread Sean V Kelley 
On Wed, Jan 11, 2017 at 11:15 AM, Mark Thompson  wrote:

> On 10/01/17 23:37, Sean V Kelley wrote:
> > Encoder architecture restructuring for H.264 (with some impact to HEVC
> now) on HSW+
> > * Improvements to the shaders
> > * Improvements to the B frame efficiency
> > * Improvements to the low bit rate mode
> > * Improved features in two stage VME/PAK pipeline
> >
> >
> > Pengfei Qu (31):
> >   ENC: move gpe related function into src/i965_gpe_utils.h/c
> >   ENC: add common structure for AVC/HEVC encoder
> >   ENC:add context init function for AVC/HEVC encoder
> >   ENC: add const data/table for AVC encoder
> >   ENC: add AVC kernel binary on SKL
>
> This patch (5/31) is missing?  (Not in the archive either: <
> https://lists.freedesktop.org/archives/libva/2017-January/thread.html>.)
>
> Looks like it is this part:
>
> >  src/gen9_avc_encoder.h |  2345 
> >  src/gen9_avc_encoder_kernels.c | 12081 ++
> +
>
> so maybe it was rejected by the ML for being too big?
>


Yes, I need to just approve it in the mailing list moderation.

Sean



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



-- 
Sean V. Kelley 
Open Source Technology Center / SSG
Intel Corp.
___
Libva mailing list
Libva@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libva

Re: [Libva] [PATCH 00/31] Encoder Architecture Changes (Primarily AVC)

2017-01-11 Thread Mark Thompson 
On 10/01/17 23:37, Sean V Kelley wrote:
> Encoder architecture restructuring for H.264 (with some impact to HEVC now) 
> on HSW+
> * Improvements to the shaders
> * Improvements to the B frame efficiency
> * Improvements to the low bit rate mode
> * Improved features in two stage VME/PAK pipeline
> 
> 
> Pengfei Qu (31):
>   ENC: move gpe related function into src/i965_gpe_utils.h/c
>   ENC: add common structure for AVC/HEVC encoder
>   ENC:add context init function for AVC/HEVC encoder
>   ENC: add const data/table for AVC encoder
>   ENC: add AVC kernel binary on SKL

This patch (5/31) is missing?  (Not in the archive either: 
.)

Looks like it is this part:

>  src/gen9_avc_encoder.h |  2345 
>  src/gen9_avc_encoder_kernels.c | 12081 
> +++

so maybe it was rejected by the ML for being too big?

Thanks,

- Mark
___
Libva mailing list
Libva@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libva

Re: [Libva] [PATCH 02/31] ENC: add common structure for AVC/HEVC encoder

2017-01-11 Thread Qu, Pengfei 


-Original Message-
From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Zhao Yakui
Sent: Wednesday, January 11, 2017 10:33 PM
To: Sean V Kelley 
Cc: libva@lists.freedesktop.org
Subject: Re: [Libva] [PATCH 02/31] ENC: add common structure for AVC/HEVC 
encoder

On 01/11/2017 07:37 AM, Sean V Kelley wrote:
> From: Pengfei Qu
>
> Signed-off-by: Pengfei Qu
> Reviewed-by: Sean V Kelley
> ---
>   src/i965_encoder_common.c | 124 +++
>   src/i965_encoder_common.h | 533 
> ++
>   2 files changed, 657 insertions(+)
>   create mode 100755 src/i965_encoder_common.c
>   create mode 100755 src/i965_encoder_common.h
>
> diff --git a/src/i965_encoder_common.c b/src/i965_encoder_common.c new 
> file mode 100755 index ..930aba99
> --- /dev/null
> +++ b/src/i965_encoder_common.c
> @@ -0,0 +1,124 @@
> +/*
> + * Copyright ? 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, 
> +including
> + * without limitation the rights to use, copy, modify, merge, 
> +publish,
> + * distribute, sub license, and/or sell copies of the Software, and 
> +to
> + * permit persons to whom the Software is furnished to do so, subject 
> +to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including 
> +the
> + * next paragraph) shall be included in all copies or substantial 
> +portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE 
> +FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF 
> +CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWAR
> + *
> + * Authors:
> + * Pengfei Qu
> + *
> + */
> +#include
> +#include
> +#include "intel_batchbuffer.h"
> +#include "intel_driver.h"
> +#include "i965_encoder_common.h"
> +#include "i965_gpe_utils.h"
> +
> +
> +const unsigned int table_enc_search_path[2][8][16] = {
> +// I-Frame&  P-Frame
> +{
> +// MEMethod: 0
> +{
> +0x120FF10F, 0x1E22E20D, 0x20E2FF10, 0x2EDD06FC, 0x11D33FF1, 
> 0xEB1FF33D, 0x4EF1F1F1, 0xF1F21211,
> +0x0DE0, 0x11201F1F, 0x1105F1CF, 0x, 0x, 
> 0x, 0x, 0x
> +},
> +// MEMethod: 1
> +{
> +0x120FF10F, 0x1E22E20D, 0x20E2FF10, 0x2EDD06FC, 0x11D33FF1, 
> 0xEB1FF33D, 0x4EF1F1F1, 0xF1F21211,
> +0x0DE0, 0x11201F1F, 0x1105F1CF, 0x, 0x, 
> 0x, 0x, 0x
> +},
> +// MEMethod: 2
> +{
> +0x, 0x, 0x, 0x, 0x, 
> 0x, 0x, 0x,
> +0x, 0x, 0x, 0x, 0x, 
> 0x, 0x, 0x
> +},
> +// MEMethod: 3
> +{
> +0x01010101, 0x11010101, 0x01010101, 0x11010101, 0x01010101, 
> 0x11010101, 0x01010101, 0x11010101,
> +0x01010101, 0x11010101, 0x01010101, 0x00010101, 0x, 
> 0x, 0x, 0x
> +},
> +// MEMethod: 4
> +{
> +0x0101F00F, 0x0F0F1010, 0xF0F0F00F, 0x01010101, 0x10101010, 
> 0x0F0F0F0F, 0xF0F0F00F, 0x0101F0F0,
> +0x01010101, 0x10101010, 0x0F0F1010, 0x0F0F0F0F, 0xF0F0F00F, 
> 0xF0F0F0F0, 0x, 0x
> +},
> +// MEMethod: 5
> +{
> +0x0101F00F, 0x0F0F1010, 0xF0F0F00F, 0x01010101, 0x10101010, 
> 0x0F0F0F0F, 0xF0F0F00F, 0x0101F0F0,
> +0x01010101, 0x10101010, 0x0F0F1010, 0x0F0F0F0F, 0xF0F0F00F, 
> 0xF0F0F0F0, 0x, 0x
> +},
> +// MEMethod: 6
> +{
> +0x120FF10F, 0x1E22E20D, 0x20E2FF10, 0x2EDD06FC, 0x11D33FF1, 
> 0xEB1FF33D, 0x4EF1F1F1, 0xF1F21211,
> +0x0DE0, 0x11201F1F, 0x1105F1CF, 0x, 0x, 
> 0x, 0x, 0x
> +},
> +// MEMethod: 7 used for mpeg2 encoding P frames
> +{
> +0x1F11F10F, 0x2E22E2FE, 0x20E220DF, 0x2EDD06FC, 0x11D33FF1, 
> 0xEB1FF33D, 0x02F1F1F1, 0x1F20,
> +0xF1EFFF0C, 0xF01104F1, 0x10FF0A50, 0x000FF1C0, 0x, 
> 0x, 0x, 0x
> +}
> +},
> +// B-Frame
> +{
> +// MEMethod: 0
> +{
> +0x0101F00F, 0x0F0F1010, 0xF0F0F00F, 0x01010101, 0x10101010, 
> 0x0F0F0F0F, 0xF0F0F00F, 0x0101F0F0,
> +