On Wed, 2016-09-07 at 13:49 +0800, Zhao Yakui wrote: > On 09/07/2016 11:01 AM, Xiang, Haihao wrote: > > > > > > i965_encoder.c is a general file, it would be better not to include > > more HW/implementation related code in this file. > > > > Actually it is more clear if you look into the new > > gen9_vme_context_init() and gen9_mfc_context_init(). Previous it > > selects different path both in gen9_enc_hw_context_init(), > > gen9_enc_hw_context_init(), gen9_mfc_context_init() per codec. > > I see it. > > In fact there is no much difference. > > Of course it is still OK to me that the callback function is selected > in > gen9_vme_context_init/gen9_mfc_context_init.
I do prefer keeping i965_encoder.c more generic, otherwise we risk introducing a blurring of the lines between GEN specific changes. Sean > > Thanks > Yakui > > > > > > Thanks > > Haihao > > > > > > > > > > > > > > On 09/06/2016 11:39 PM, Xiang, Haihao wrote: > > > > > > > > It keeps i965_encoder.c simple > > > > > > > > > > Thanks for the patch. > > > But I don't think that this patch is necessary. The code looks > > > more > > > clear if it can select the different initialization callback > > > function > > > earlier based on the corresponding profile/entrypoint . At the > > > same > > > time > > > it can also avoid that the gen9_vme.c/gen9_mfc.c calls the other > > > API > > > implementation. > > > > > > > > > > > Signed-off-by: Xiang, Haihao<[email protected]> > > > > --- > > > > src/gen6_mfc.h | 6 ++++++ > > > > src/gen6_vme.h | 2 +- > > > > src/gen9_mfc.c | 25 +++++++++++++++++++------ > > > > src/gen9_vme.c | 15 ++++++++++++++- > > > > src/i965_encoder.c | 37 ++++++---------------------------- > > > > --- > > > > 5 files changed, 46 insertions(+), 39 deletions(-) > > > > > > > > diff --git a/src/gen6_mfc.h b/src/gen6_mfc.h > > > > index 4561d43..702596b 100644 > > > > --- a/src/gen6_mfc.h > > > > +++ b/src/gen6_mfc.h > > > > @@ -346,6 +346,12 @@ VAStatus > > > > gen6_mfc_pipeline(VADriverContextP > > > > ctx, > > > > void gen6_mfc_context_destroy(void *context); > > > > > > > > extern > > > > +Bool gen6_mfc_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context); > > > > + > > > > +extern > > > > +Bool gen7_mfc_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context); > > > > + > > > > +extern > > > > Bool gen75_mfc_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context); > > > > > > > > > > > > diff --git a/src/gen6_vme.h b/src/gen6_vme.h > > > > index e8f4742..f94085f 100644 > > > > --- a/src/gen6_vme.h > > > > +++ b/src/gen6_vme.h > > > > @@ -116,7 +116,7 @@ struct gen6_vme_context > > > > #define MPEG2_LEVEL_MAIN 0x08 > > > > #define MPEG2_LEVEL_HIGH 0x04 > > > > > > > > - > > > > +Bool gen6_vme_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context); > > > > Bool gen75_vme_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context); > > > > > > > > extern void intel_vme_update_mbmv_cost(VADriverContextP ctx, > > > > diff --git a/src/gen9_mfc.c b/src/gen9_mfc.c > > > > index b3d6e78..ce038b1 100644 > > > > --- a/src/gen9_mfc.c > > > > +++ b/src/gen9_mfc.c > > > > @@ -39,18 +39,31 @@ > > > > #include "i965_drv_video.h" > > > > #include "i965_encoder.h" > > > > #include "gen6_mfc.h" > > > > +#include "gen9_mfc.h" > > > > +#include "gen9_vdenc.h" > > > > +#include "gen9_vp9_encapi.h" > > > > > > > > Bool gen9_mfc_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context) > > > > { > > > > - if ((encoder_context->codec == CODEC_H264) || > > > > - (encoder_context->codec == CODEC_H264_MVC)) { > > > > + switch (encoder_context->codec) { > > > > + case CODEC_VP8: > > > > + case CODEC_MPEG2: > > > > + case CODEC_JPEG: > > > > + return gen8_mfc_context_init(ctx, encoder_context); > > > > + > > > > + case CODEC_H264: > > > > + case CODEC_H264_MVC: > > > > + if (encoder_context->low_power_mode) > > > > + return gen9_vdenc_context_init(ctx, > > > > encoder_context); > > > > + else > > > > return gen8_mfc_context_init(ctx, > > > > encoder_context); > > > > - } > > > > > > > > + case CODEC_HEVC: > > > > + return gen9_hcpe_context_init(ctx, encoder_context); > > > > > > > > - if ((encoder_context->codec == CODEC_VP8) || > > > > - (encoder_context->codec == CODEC_MPEG2)) > > > > - return gen8_mfc_context_init(ctx, encoder_context); > > > > + case CODEC_VP9: > > > > + return gen9_vp9_pak_context_init(ctx, > > > > encoder_context); > > > > + } > > > > > > > > /* Other profile/entrypoint pairs never get here, see > > > > gen9_enc_hw_context_init() */ > > > > assert(0); > > > > diff --git a/src/gen9_vme.c b/src/gen9_vme.c > > > > index 1625c2b..4f19409 100644 > > > > --- a/src/gen9_vme.c > > > > +++ b/src/gen9_vme.c > > > > @@ -40,6 +40,7 @@ > > > > #include "i965_encoder.h" > > > > #include "gen6_vme.h" > > > > #include "gen6_mfc.h" > > > > +#include "gen9_vp9_encapi.h" > > > > > > > > #ifdef SURFACE_STATE_PADDED_SIZE > > > > #undef SURFACE_STATE_PADDED_SIZE > > > > @@ -1817,10 +1818,22 @@ gen9_vme_context_destroy(void *context) > > > > > > > > Bool gen9_vme_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context) > > > > { > > > > - struct gen6_vme_context *vme_context = calloc(1, > > > > sizeof(struct > > > > gen6_vme_context)); > > > > + struct gen6_vme_context *vme_context; > > > > struct i965_kernel *vme_kernel_list = NULL; > > > > int i965_kernel_num; > > > > > > > > + if (encoder_context->low_power_mode || encoder_context- > > > > >codec > > > > == CODEC_JPEG) { > > > > + encoder_context->vme_context = NULL; > > > > + encoder_context->vme_pipeline = NULL; > > > > + encoder_context->vme_context_destroy = NULL; > > > > + > > > > + return True; > > > > + } else if (encoder_context->codec == CODEC_VP9) { > > > > + return gen9_vp9_vme_context_init(ctx, > > > > encoder_context); > > > > + } > > > > + > > > > + vme_context = calloc(1, sizeof(struct gen6_vme_context)); > > > > + > > > > switch (encoder_context->codec) { > > > > case CODEC_H264: > > > > case CODEC_H264_MVC: > > > > diff --git a/src/i965_encoder.c b/src/i965_encoder.c > > > > index 47368fb..be01e83 100644 > > > > --- a/src/i965_encoder.c > > > > +++ b/src/i965_encoder.c > > > > @@ -39,15 +39,6 @@ > > > > #include "i965_encoder.h" > > > > #include "gen6_vme.h" > > > > #include "gen6_mfc.h" > > > > -#include "gen9_mfc.h" > > > > -#include "gen9_vdenc.h" > > > > - > > > > -#include "gen9_vp9_encapi.h" > > > > - > > > > -extern Bool gen6_mfc_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context); > > > > -extern Bool gen6_vme_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context); > > > > -extern Bool gen7_mfc_context_init(VADriverContextP ctx, struct > > > > intel_encoder_context *encoder_context); > > > > -extern Bool gen9_hcpe_context_init(VADriverContextP ctx, > > > > struct > > > > intel_encoder_context *encoder_context); > > > > > > > > static VAStatus > > > > clear_border(struct object_surface *obj_surface) > > > > @@ -836,6 +827,9 @@ intel_enc_hw_context_init(VADriverContextP > > > > ctx, > > > > encoder_context->quality_level = > > > > ENCODER_DEFAULT_QUALITY; > > > > encoder_context->quality_range = 1; > > > > > > > > + if (obj_config->entrypoint == VAEntrypointEncSliceLP) > > > > + encoder_context->low_power_mode = 1; > > > > + > > > > switch (obj_config->profile) { > > > > case VAProfileMPEG2Simple: > > > > case VAProfileMPEG2Main: > > > > @@ -898,14 +892,8 @@ intel_enc_hw_context_init(VADriverContextP > > > > ctx, > > > > > > > > if (vme_context_init) { > > > > vme_context_init(ctx, encoder_context); > > > > - > > > > - if (obj_config->profile != VAProfileJPEGBaseline) { > > > > - assert(encoder_context->vme_context); > > > > - assert(encoder_context->vme_context_destroy); > > > > - assert(encoder_context->vme_pipeline); > > > > - } > > > > - } else { > > > > - encoder_context->low_power_mode = 1; > > > > + assert(!encoder_context->vme_context || > > > > + (encoder_context- > > > > > > > > > > vme_context_destroy&& encoder_context->vme_pipeline)); > > > > } > > > > > > > > mfc_context_init(ctx, encoder_context); > > > > @@ -944,18 +932,5 @@ gen8_enc_hw_context_init(VADriverContextP > > > > ctx, > > > > struct object_config *obj_config) > > > > struct hw_context * > > > > gen9_enc_hw_context_init(VADriverContextP ctx, struct > > > > object_config *obj_config) > > > > { > > > > - if (obj_config->entrypoint == VAEntrypointEncSliceLP) { > > > > - return intel_enc_hw_context_init(ctx, obj_config, > > > > NULL, > > > > gen9_vdenc_context_init); > > > > - } else { > > > > - if (obj_config->profile == VAProfileHEVCMain) { > > > > - return intel_enc_hw_context_init(ctx, obj_config, > > > > gen9_vme_context_init, gen9_hcpe_context_init); > > > > - } else if (obj_config->profile == > > > > VAProfileJPEGBaseline) > > > > - return intel_enc_hw_context_init(ctx, obj_config, > > > > gen8_vme_context_init, gen8_mfc_context_init); > > > > - else if (obj_config->profile == VAProfileVP9Profile0) > > > > - return intel_enc_hw_context_init(ctx, obj_config, > > > > - gen9_vp9_vme_cont > > > > ext_ > > > > init, > > > > - gen9_vp9_pak_cont > > > > ext_ > > > > init); > > > > - else > > > > - return intel_enc_hw_context_init(ctx, obj_config, > > > > gen9_vme_context_init, gen9_mfc_context_init); > > > > - } > > > > + return intel_enc_hw_context_init(ctx, obj_config, > > > > gen9_vme_context_init, gen9_mfc_context_init); > > > > } > > _______________________________________________ > Libva mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/libva
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Libva mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libva
