[Libva] [Libva-intel-driver][PATCH 02/17] Move all curbe related settings to the inner structure in i965_gpe_context

Xiang, Haihao haihao.xiang at intel.com
Thu Nov 17 16:58:04 UTC 2016



>-----Original Message-----
>From: Zhao, Yakui
>Sent: Thursday, November 17, 2016 8:42 PM
>To: Xiang, Haihao <haihao.xiang at intel.com>
>Cc: libva at lists.freedesktop.org
>Subject: Re: [Libva] [Libva-intel-driver][PATCH 02/17] Move all curbe related
>settings to the inner structure in i965_gpe_context
>
>On 11/17/2016 04:35 PM, Xiang, Haihao wrote:
>> To avoid confusion between curbe.length and curbe_size, this patch
>> uses curbe.length only. curbe.bo is always set even if curbe is a part
>> of the dynamic state buffer, hence we can use curbe related settings
>> no matter it is a part of the dynamic state buffer or not.
>
>The curbe.bo in *_gpe_context is defined/used for the old platform.
>In fact for the platform from Sandybridge, it can reside in the
>dynamic_state.bo.

>
>If the curbe.bo/curbe.offset is used directly,

previously curbe_offset is also used directly.

> maybe it brings the confusion
>that one dedicated bo is declared/defined for curbe.

curbe.bo/curbe.offset is set in gpe functions, gpe user just uses it and needn't care about 
it is a dedicated bo or not.

>
>So I think that the curbe_offset can follow the HW spec.

curbe.offset still follows the HW spec.  The problem for the old structure is that we have an inner curbe structure 
and curbe_offset/ curbe_size in the same structure.  It is easy to confuse a new developer.  

>Maybe we can add the wrapper function that can map/unmap the virtual
>address of curbe_buffer. In such case it can also simplify the mapping related
>with curbe_buffer.

We have already such functions, but we don't use them for some old platforms.  We can add similar functions for idrt and sampler. 

>
>Similar considerations for Interface_descriptor_data, sampler_buffer.
>
>
>>
>> Signed-off-by: Xiang, Haihao<haihao.xiang at intel.com>
>> ---
>>   src/gen75_vpp_gpe.c        |  2 +-
>>   src/gen8_mfc.c             |  2 +-
>>   src/gen8_vme.c             | 12 ++++++------
>>   src/gen9_post_processing.c |  3 +--
>>   src/gen9_vme.c             | 12 ++++++------
>>   src/gen9_vp9_encoder.c     | 22 ++++++++++------------
>>   src/i965_gpe_utils.c       | 28 +++++++++++++++++++---------
>>   src/i965_gpe_utils.h       |  3 +--
>>   8 files changed, 45 insertions(+), 39 deletions(-)
>>
>> diff --git a/src/gen75_vpp_gpe.c b/src/gen75_vpp_gpe.c index
>> 9850c1c..2cddb5a 100644
>> --- a/src/gen75_vpp_gpe.c
>> +++ b/src/gen75_vpp_gpe.c
>> @@ -890,7 +890,7 @@ vpp_gpe_context_init(VADriverContextP ctx)
>>           gpe_ctx->surface_state_binding_table.length =
>>                  (SURFACE_STATE_PADDED_SIZE_GEN8 + sizeof(unsigned
>> int)) * MAX_MEDIA_SURFACES_GEN6;
>>
>> -        gpe_ctx->curbe_size = CURBE_TOTAL_DATA_LENGTH;
>> +        gpe_ctx->curbe.length = CURBE_TOTAL_DATA_LENGTH;
>>           gpe_ctx->idrt_size  = sizeof(struct
>> gen8_interface_descriptor_data) * MAX_INTERFACE_DESC_GEN6;
>>
>>       }
>> diff --git a/src/gen8_mfc.c b/src/gen8_mfc.c index 63ffea5..3ed9e84
>> 100644
>> --- a/src/gen8_mfc.c
>> +++ b/src/gen8_mfc.c
>> @@ -4609,7 +4609,7 @@ Bool gen8_mfc_context_init(VADriverContextP
>ctx, struct intel_encoder_context *e
>>       mfc_context->gpe_context.surface_state_binding_table.length =
>> (SURFACE_STATE_PADDED_SIZE + sizeof(unsigned int)) *
>> MAX_MEDIA_SURFACES_GEN6;
>>
>>       mfc_context->gpe_context.idrt_size = sizeof(struct
>gen8_interface_descriptor_data) * MAX_INTERFACE_DESC_GEN6;
>> -    mfc_context->gpe_context.curbe_size = 32 * 4;
>> +    mfc_context->gpe_context.curbe.length = 32 * 4;
>>       mfc_context->gpe_context.sampler_size = 0;
>>
>>       mfc_context->gpe_context.vfe_state.max_num_threads = 60 - 1;
>> diff --git a/src/gen8_vme.c b/src/gen8_vme.c index c79c62b..96835bf
>> 100644
>> --- a/src/gen8_vme.c
>> +++ b/src/gen8_vme.c
>> @@ -389,10 +389,10 @@ static VAStatus
>> gen8_vme_constant_setup(VADriverContextP ctx,
>>
>>       vme_state_message[31] = mv_num;
>>
>> -    dri_bo_map(vme_context->gpe_context.dynamic_state.bo, 1);
>> -    assert(vme_context->gpe_context.dynamic_state.bo->virtual);
>> -    constant_buffer = (unsigned char *)vme_context-
>>gpe_context.dynamic_state.bo->virtual +
>> -                                         vme_context->gpe_context.curbe_offset;
>> +    dri_bo_map(vme_context->gpe_context.curbe.bo, 1);
>> +    assert(vme_context->gpe_context.curbe.bo->virtual);
>> +    constant_buffer = (unsigned char *)vme_context-
>>gpe_context.curbe.bo->virtual +
>> +
>> + vme_context->gpe_context.curbe.offset;
>>
>>       /* VME MV/Mb cost table is passed by using const buffer */
>>       /* Now it uses the fixed search path. So it is constructed
>> directly @@ -400,7 +400,7 @@ static VAStatus
>gen8_vme_constant_setup(VADriverContextP ctx,
>>        */
>>       memcpy(constant_buffer, (char *)vme_context->vme_state_message,
>> 128);
>>
>> -    dri_bo_unmap(vme_context->gpe_context.dynamic_state.bo);
>> +    dri_bo_unmap(vme_context->gpe_context.curbe.bo);
>>
>>       return VA_STATUS_SUCCESS;
>>   }
>> @@ -1379,7 +1379,7 @@ Bool gen8_vme_context_init(VADriverContextP
>ctx, struct intel_encoder_context *e
>>           vme_context->gpe_context.surface_state_binding_table.length
>> = (SURFACE_STATE_PADDED_SIZE + sizeof(unsigned int)) *
>> MAX_MEDIA_SURFACES_GEN6;
>>
>>           vme_context->gpe_context.idrt_size = sizeof(struct
>gen8_interface_descriptor_data) * MAX_INTERFACE_DESC_GEN6;
>> -        vme_context->gpe_context.curbe_size =
>CURBE_TOTAL_DATA_LENGTH;
>> +        vme_context->gpe_context.curbe.length =
>> + CURBE_TOTAL_DATA_LENGTH;
>>           vme_context->gpe_context.sampler_size = 0;
>>
>>
>> diff --git a/src/gen9_post_processing.c b/src/gen9_post_processing.c
>> index a5d345c..71da501 100644
>> --- a/src/gen9_post_processing.c
>> +++ b/src/gen9_post_processing.c
>> @@ -538,8 +538,7 @@
>gen9_post_processing_context_init(VADriverContextP ctx,
>>       gen8_gpe_load_kernels(ctx, gpe_context,&scaling_kernel, 1);
>>       gpe_context->idrt_size = ALIGN(sizeof(struct
>gen8_interface_descriptor_data), 64);
>>       gpe_context->sampler_size = ALIGN(sizeof(struct gen8_sampler_state),
>64);
>> -    gpe_context->curbe_size = ALIGN(sizeof(struct
>scaling_input_parameter), 64);
>> -    gpe_context->curbe.length = gpe_context->curbe_size;
>> +    gpe_context->curbe.length = ALIGN(sizeof(struct
>> + scaling_input_parameter), 64);
>>
>>       gpe_context->surface_state_binding_table.max_entries =
>MAX_SCALING_SURFACES;
>>       gpe_context->surface_state_binding_table.binding_table_offset =
>> 0; diff --git a/src/gen9_vme.c b/src/gen9_vme.c index 6ad8fff..a59fe2a
>> 100644
>> --- a/src/gen9_vme.c
>> +++ b/src/gen9_vme.c
>> @@ -438,10 +438,10 @@ static VAStatus
>> gen9_vme_constant_setup(VADriverContextP ctx,
>>
>>       vme_state_message[31] = mv_num;
>>
>> -    dri_bo_map(vme_context->gpe_context.dynamic_state.bo, 1);
>> -    assert(vme_context->gpe_context.dynamic_state.bo->virtual);
>> -    constant_buffer = (unsigned char *)vme_context-
>>gpe_context.dynamic_state.bo->virtual +
>> -                                         vme_context->gpe_context.curbe_offset;
>> +    dri_bo_map(vme_context->gpe_context.curbe.bo, 1);
>> +    assert(vme_context->gpe_context.curbe.bo->virtual);
>> +    constant_buffer = (unsigned char *)vme_context-
>>gpe_context.curbe.bo->virtual +
>> +
>> + vme_context->gpe_context.curbe.offset;
>>
>>       /* VME MV/Mb cost table is passed by using const buffer */
>>       /* Now it uses the fixed search path. So it is constructed
>> directly @@ -449,7 +449,7 @@ static VAStatus
>gen9_vme_constant_setup(VADriverContextP ctx,
>>        */
>>       memcpy(constant_buffer, (char *)vme_context->vme_state_message,
>> 128);
>>
>> -    dri_bo_unmap(vme_context->gpe_context.dynamic_state.bo);
>> +    dri_bo_unmap(vme_context->gpe_context.curbe.bo);
>>
>>       return VA_STATUS_SUCCESS;
>>   }
>> @@ -2032,7 +2032,7 @@ Bool gen9_vme_context_init(VADriverContextP
>ctx, struct intel_encoder_context *e
>>       vme_context->gpe_context.surface_state_binding_table.length =
>> (SURFACE_STATE_PADDED_SIZE + sizeof(unsigned int)) *
>> MAX_MEDIA_SURFACES_GEN6;
>>
>>       vme_context->gpe_context.idrt_size = sizeof(struct
>gen8_interface_descriptor_data) * MAX_INTERFACE_DESC_GEN6;
>> -    vme_context->gpe_context.curbe_size = CURBE_TOTAL_DATA_LENGTH;
>> +    vme_context->gpe_context.curbe.length =
>CURBE_TOTAL_DATA_LENGTH;
>>       vme_context->gpe_context.sampler_size = 0;
>>
>>
>> diff --git a/src/gen9_vp9_encoder.c b/src/gen9_vp9_encoder.c index
>> f39d6d0..5ad7b26 100644
>> --- a/src/gen9_vp9_encoder.c
>> +++ b/src/gen9_vp9_encoder.c
>> @@ -1820,18 +1820,18 @@
>gen9_brc_update_add_surfaces_vp9(VADriverContextP ctx,
>>       /* 4. Mbenc curbe input buffer */
>>       gen9_add_dri_buffer_gpe_surface(ctx,
>>                                       brc_gpe_context,
>> -                                    mbenc_gpe_context->dynamic_state.bo,
>> +                                    mbenc_gpe_context->curbe.bo,
>>                                       0,
>> -                                    ALIGN(mbenc_gpe_context->curbe_size, 64),
>> -                                    mbenc_gpe_context->curbe_offset,
>> +                                    ALIGN(mbenc_gpe_context->curbe.length, 64),
>> +                                    mbenc_gpe_context->curbe.offset,
>>                                       VP9_BTI_BRC_MBENC_CURBE_INPUT_G9);
>>       /* 5. Mbenc curbe output buffer */
>>       gen9_add_dri_buffer_gpe_surface(ctx,
>>                                       brc_gpe_context,
>> -                                    mbenc_gpe_context->dynamic_state.bo,
>> +                                    mbenc_gpe_context->curbe.bo,
>>                                       0,
>> -                                    ALIGN(mbenc_gpe_context->curbe_size, 64),
>> -                                    mbenc_gpe_context->curbe_offset,
>> +                                    ALIGN(mbenc_gpe_context->curbe.length, 64),
>> +                                    mbenc_gpe_context->curbe.offset,
>>
>> VP9_BTI_BRC_MBENC_CURBE_OUTPUT_G9);
>>
>>       /* 6. BRC_PIC_STATE read buffer */ @@ -3289,10 +3289,10 @@
>> gen9_vp9_send_mbenc_surface(VADriverContextP ctx,
>>
>>           gen9_add_dri_buffer_gpe_surface(ctx,
>>                                           gpe_context,
>> -                                        mbenc_param->gpe_context_tx->dynamic_state.bo,
>> +
>> + mbenc_param->gpe_context_tx->curbe.bo,
>>                                           0,
>>                                           ALIGN(res_size, 64),
>> -                                        mbenc_param->gpe_context_tx->curbe_offset,
>> +
>> + mbenc_param->gpe_context_tx->curbe.offset,
>>                                           VP9_BTI_MBENC_TX_CURBE_G9);
>>
>>           break;
>> @@ -3441,10 +3441,10 @@
>gen9_vp9_send_mbenc_surface(VADriverContextP
>> ctx,
>>
>>           gen9_add_dri_buffer_gpe_surface(ctx,
>>                                           gpe_context,
>> -                                        mbenc_param->gpe_context_tx->dynamic_state.bo,
>> +
>> + mbenc_param->gpe_context_tx->curbe.bo,
>>                                           0,
>>                                           ALIGN(res_size, 64),
>> -                                        mbenc_param->gpe_context_tx->curbe_offset,
>> +
>> + mbenc_param->gpe_context_tx->curbe.offset,
>>                                           VP9_BTI_MBENC_TX_CURBE_G9);
>>
>>
>> @@ -3684,8 +3684,6 @@ gen9_init_gpe_context_vp9(struct
>i965_gpe_context *gpe_context,
>>   {
>>       gpe_context->curbe.length = kernel_param->curbe_size; // in
>> bytes
>>
>> -    gpe_context->curbe_size = ALIGN(kernel_param->curbe_size, 64);
>> -
>>       gpe_context->sampler_size = 0;
>>       if (kernel_param->sampler_size) {
>>           gpe_context->sampler_size =
>> ALIGN(kernel_param->sampler_size, 64); diff --git
>> a/src/i965_gpe_utils.c b/src/i965_gpe_utils.c index c5a8935..3739a88
>> 100644
>> --- a/src/i965_gpe_utils.c
>> +++ b/src/i965_gpe_utils.c
>> @@ -1066,8 +1066,8 @@ gen8_gpe_curbe_load(VADriverContextP ctx,
>>
>>       OUT_BATCH(batch, CMD_MEDIA_CURBE_LOAD | (4 - 2));
>>       OUT_BATCH(batch, 0);
>> -    OUT_BATCH(batch, gpe_context->curbe_size);
>> -    OUT_BATCH(batch, gpe_context->curbe_offset);
>> +    OUT_BATCH(batch, gpe_context->curbe.length);
>> +    OUT_BATCH(batch, gpe_context->curbe.offset);
>>
>>       ADVANCE_BATCH(batch);
>>   }
>> @@ -1122,7 +1122,7 @@ gen8_gpe_context_init(VADriverContextP ctx,
>>       assert(bo);
>>       gpe_context->surface_state_binding_table.bo = bo;
>>
>> -    bo_size = gpe_context->idrt_size + gpe_context->curbe_size +
>gpe_context->sampler_size + 192;
>> +    bo_size = gpe_context->idrt_size + gpe_context->curbe.length +
>> + gpe_context->sampler_size + 192;
>>       dri_bo_unreference(gpe_context->dynamic_state.bo);
>>       bo = dri_bo_alloc(i965->intel.bufmgr,
>>                         "surface state&  binding table", @@ -1137,8
>> +1137,11 @@ gen8_gpe_context_init(VADriverContextP ctx,
>>
>>       /* Constant buffer offset */
>>       start_offset = ALIGN(end_offset, 64);
>> -    gpe_context->curbe_offset = start_offset;
>> -    end_offset = start_offset + gpe_context->curbe_size;
>> +    dri_bo_unreference(gpe_context->curbe.bo);
>> +    gpe_context->curbe.bo = bo;
>> +    dri_bo_reference(gpe_context->curbe.bo);
>> +    gpe_context->curbe.offset = start_offset;
>> +    end_offset = start_offset + gpe_context->curbe.length;
>>
>>       /* Interface descriptor offset */
>>       start_offset = ALIGN(end_offset, 64); @@ -1170,6 +1173,8 @@
>> gen8_gpe_context_destroy(struct i965_gpe_context *gpe_context)
>>       dri_bo_unreference(gpe_context->indirect_state.bo);
>>       gpe_context->indirect_state.bo = NULL;
>>
>> +    dri_bo_unreference(gpe_context->curbe.bo);
>> +    gpe_context->curbe.bo = NULL;
>>   }
>>
>>
>> @@ -1619,7 +1624,12 @@
>gen8_gpe_context_set_dynamic_buffer(VADriverContextP ctx,
>>       dri_bo_reference(gpe_context->dynamic_state.bo);
>>       gpe_context->dynamic_state.bo_size = ds->bo_size;
>>
>> -    gpe_context->curbe_offset = ds->curbe_offset;
>> +    /* curbe buffer is a part of the dynamic buffer */
>> +    dri_bo_unreference(gpe_context->curbe.bo);
>> +    gpe_context->curbe.bo = ds->bo;
>> +    dri_bo_reference(gpe_context->curbe.bo);
>> +    gpe_context->curbe.offset = ds->curbe_offset;
>> +
>>       gpe_context->idrt_offset = ds->idrt_offset;
>>       gpe_context->sampler_offset = ds->sampler_offset;
>>
>> @@ -1629,15 +1639,15 @@
>gen8_gpe_context_set_dynamic_buffer(VADriverContextP ctx,
>>   void *
>>   gen8p_gpe_context_map_curbe(struct i965_gpe_context *gpe_context)
>>   {
>> -    dri_bo_map(gpe_context->dynamic_state.bo, 1);
>> +    dri_bo_map(gpe_context->curbe.bo, 1);
>>
>> -    return (char *)gpe_context->dynamic_state.bo->virtual + gpe_context-
>>curbe_offset;
>> +    return (char *)gpe_context->curbe.bo->virtual +
>> + gpe_context->curbe.offset;
>>   }
>>
>>   void
>>   gen8p_gpe_context_unmap_curbe(struct i965_gpe_context
>*gpe_context)
>>   {
>> -    dri_bo_unmap(gpe_context->dynamic_state.bo);
>> +    dri_bo_unmap(gpe_context->curbe.bo);
>>   }
>>
>>   void
>> diff --git a/src/i965_gpe_utils.h b/src/i965_gpe_utils.h index
>> 0cbef43..92123fe 100644
>> --- a/src/i965_gpe_utils.h
>> +++ b/src/i965_gpe_utils.h
>> @@ -92,6 +92,7 @@ struct i965_gpe_context
>>       struct {
>>           dri_bo *bo;
>>           unsigned int length;            /* in bytes */
>> +        unsigned int offset;
>>       } curbe;
>>
>>       struct {
>> @@ -168,8 +169,6 @@ struct i965_gpe_context
>>       int sampler_size;
>>       unsigned int idrt_offset;
>>       int idrt_size;
>> -    unsigned int curbe_offset;
>> -    int curbe_size;
>>   };
>>
>>   struct gpe_mi_flush_dw_parameter



More information about the Libva mailing list