[Libva] [Libva-intel-driver][PATCH] Code cleanup for vme/mfc initializing on SKL
Sean V Kelley
seanvk at posteo.de
Wed Sep 7 16:58:33 UTC 2016
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<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_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
> Libva at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/libva
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/libva/attachments/20160907/4582e8ec/attachment.sig>
More information about the Libva
mailing list