[Libva] [Libva-intel-driver][PATCH] Code cleanup for vme/mfc initializing on SKL
Xiang, Haihao
haihao.xiang at intel.com
Wed Sep 7 03:01:43 UTC 2016
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.
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<haihao.xiang at intel.com>
> > ---
> > 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_context_
> > init,
> > - gen9_vp9_pak_context_
> > 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);
> > }
>
More information about the Libva
mailing list