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 <kenn...@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;
> }
>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
