>-----Original Message----- >From: Zhao, Yakui >Sent: Thursday, November 17, 2016 8:42 PM >To: Xiang, Haihao <[email protected]> >Cc: [email protected] >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<[email protected]> >> --- >> 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 _______________________________________________ Libva mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libva
