[Libva] [Libva-intel-driver][PATCH] Code cleanup for vme/mfc initializing on SKL
Zhao Yakui
yakui.zhao at intel.com
Wed Sep 7 05:49:55 UTC 2016
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.
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_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