[Mesa-dev] [PATCH 2/2] i965: Use intel_upload_map/unmap for pull constant uploads.

Kenneth Graunke kenneth at whitecape.org
Tue Mar 25 14:05:56 PDT 2014


On 03/25/2014 11:27 AM, Eric Anholt wrote:
> This also happens to fix a leak of the current GS pull constant BO on
> context destroy, by just not holding on to the pull const bos after the
> surface state is generated.

We should fix that separately so we have a patch which can be
cherry-picked to 10.1 or 10.0.

Otherwise, this looks good to me.  Even though it apparently didn't help
performance, I'm still fine with doing it.  If anything, it decreases
code complexity a little bit.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> No statistically significant performance difference on GLB2.7 on HSW at
> 1024x768 (n=40) or 320x240 (n=44), or on BYT at 320x240 (n=47).
> 
> v2: Rebase on intel_upload simplification.
> ---
>  src/mesa/drivers/dri/i965/brw_context.c          |  2 --
>  src/mesa/drivers/dri/i965/brw_context.h          |  3 ---
>  src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 26 +++++++++---------------
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 19 +++++++----------
>  4 files changed, 17 insertions(+), 33 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 9a76ee1..e113654 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -821,8 +821,6 @@ intelDestroyContext(__DRIcontext * driContextPriv)
>     brw_draw_destroy(brw);
>  
>     drm_intel_bo_unreference(brw->curbe.curbe_bo);
> -   drm_intel_bo_unreference(brw->vs.base.const_bo);
> -   drm_intel_bo_unreference(brw->wm.base.const_bo);
>  
>     free(brw->curbe.last_buf);
>     free(brw->curbe.next_buf);
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index e119682..dbe504a 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -933,9 +933,6 @@ struct brw_stage_state
>      */
>     drm_intel_bo *scratch_bo;
>  
> -   /** Pull constant buffer */
> -   drm_intel_bo *const_bo;
> -
>     /** Offset in the program cache to the program */
>     uint32_t prog_offset;
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> index bd97a22..cbe88ea 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> @@ -34,7 +34,7 @@
>  
>  #include "brw_context.h"
>  #include "brw_state.h"
> -
> +#include "intel_buffer_objects.h"
>  
>  void
>  brw_upload_vec4_pull_constants(struct brw_context *brw,
> @@ -52,9 +52,7 @@ brw_upload_vec4_pull_constants(struct brw_context *brw,
>     _mesa_load_state_parameters(&brw->ctx, prog->Parameters);
>  
>     if (!prog_data->base.nr_pull_params) {
> -      if (stage_state->const_bo) {
> -	 drm_intel_bo_unreference(stage_state->const_bo);
> -	 stage_state->const_bo = NULL;
> +      if (stage_state->surf_offset[surf_index]) {
>  	 stage_state->surf_offset[surf_index] = 0;
>  	 brw->state.dirty.brw |= brw_new_constbuf;
>        }
> @@ -62,32 +60,28 @@ brw_upload_vec4_pull_constants(struct brw_context *brw,
>     }
>  
>     /* _NEW_PROGRAM_CONSTANTS */
> -   drm_intel_bo_unreference(stage_state->const_bo);
>     uint32_t size = prog_data->base.nr_pull_params * 4;
> -   stage_state->const_bo = drm_intel_bo_alloc(brw->bufmgr, "vec4_const_buffer",
> -                                           size, 64);
> -
> -   drm_intel_gem_bo_map_gtt(stage_state->const_bo);
> +   drm_intel_bo *const_bo = NULL;
> +   uint32_t const_offset;
> +   float *constants = intel_upload_space(brw, size, 64,
> +                                         &const_bo, &const_offset);
>  
>     for (i = 0; i < prog_data->base.nr_pull_params; i++) {
> -      memcpy(stage_state->const_bo->virtual + i * 4,
> -	     prog_data->base.pull_param[i],
> -	     4);
> +      constants[i] = *prog_data->base.pull_param[i];
>     }
>  
>     if (0) {
>        for (i = 0; i < ALIGN(prog_data->base.nr_pull_params, 4) / 4; i++) {
> -	 float *row = (float *)stage_state->const_bo->virtual + i * 4;
> +	 float *row = &constants[i * 4];
>  	 fprintf(stderr, "const surface %3d: %4.3f %4.3f %4.3f %4.3f\n",
>                   i, row[0], row[1], row[2], row[3]);
>        }
>     }
>  
> -   drm_intel_gem_bo_unmap_gtt(stage_state->const_bo);
> -
> -   brw_create_constant_surface(brw, stage_state->const_bo, 0, size,
> +   brw_create_constant_surface(brw, const_bo, const_offset, size,
>                                 &stage_state->surf_offset[surf_index],
>                                 false);
> +   drm_intel_bo_unreference(const_bo);
>  
>     brw->state.dirty.brw |= brw_new_constbuf;
>  }
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 61f7ba5..443820b 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -467,37 +467,32 @@ brw_upload_wm_pull_constants(struct brw_context *brw)
>     const int size = brw->wm.prog_data->base.nr_pull_params * sizeof(float);
>     const int surf_index =
>        brw->wm.prog_data->base.binding_table.pull_constants_start;
> -   float *constants;
>     unsigned int i;
>  
>     _mesa_load_state_parameters(ctx, params);
>  
>     /* CACHE_NEW_WM_PROG */
>     if (brw->wm.prog_data->base.nr_pull_params == 0) {
> -      if (brw->wm.base.const_bo) {
> -	 drm_intel_bo_unreference(brw->wm.base.const_bo);
> -	 brw->wm.base.const_bo = NULL;
> +      if (brw->wm.base.surf_offset[surf_index]) {
>  	 brw->wm.base.surf_offset[surf_index] = 0;
>  	 brw->state.dirty.brw |= BRW_NEW_SURFACES;
>        }
>        return;
>     }
>  
> -   drm_intel_bo_unreference(brw->wm.base.const_bo);
> -   brw->wm.base.const_bo = drm_intel_bo_alloc(brw->bufmgr, "WM const bo",
> -					 size, 64);
> -
>     /* _NEW_PROGRAM_CONSTANTS */
> -   drm_intel_gem_bo_map_gtt(brw->wm.base.const_bo);
> -   constants = brw->wm.base.const_bo->virtual;
> +   drm_intel_bo *const_bo = NULL;
> +   uint32_t const_offset;
> +   float *constants = intel_upload_space(brw, size, 64,
> +                                         &const_bo, &const_offset);
>     for (i = 0; i < brw->wm.prog_data->base.nr_pull_params; i++) {
>        constants[i] = *brw->wm.prog_data->base.pull_param[i];
>     }
> -   drm_intel_gem_bo_unmap_gtt(brw->wm.base.const_bo);
>  
> -   brw_create_constant_surface(brw, brw->wm.base.const_bo, 0, size,
> +   brw_create_constant_surface(brw, const_bo, const_offset, size,
>                                 &brw->wm.base.surf_offset[surf_index],
>                                 true);
> +   drm_intel_bo_unreference(const_bo);
>  
>     brw->state.dirty.brw |= BRW_NEW_SURFACES;
>  }
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140325/8d8e25b6/attachment.sig>


More information about the mesa-dev mailing list