[Mesa-dev] [PATCH 3/3] i965: Fold ABO state upload code into the SSBO/UBO state upload code.

Jason Ekstrand jason at jlekstrand.net
Thu Nov 9 20:40:50 UTC 2017


On Thu, Nov 9, 2017 at 12:45 AM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> Having this separate could potentially make programs that rebind atomics
> but no other surfaces ever so slightly faster.  But it's a tiny amount
> of code to add to the existing UBO/SSBO atom, and very related.
>
> The extra atoms have a cost on every draw call, and so dropping some of
> them would be nice.  This also reclaims a dirty bit.
> ---
>  src/mesa/drivers/dri/i965/brw_context.h           |  6 --
>  src/mesa/drivers/dri/i965/brw_gs_surface_state.c  | 22 ------
>  src/mesa/drivers/dri/i965/brw_state.h             |  6 --
>  src/mesa/drivers/dri/i965/brw_state_upload.c      |  3 +-
>  src/mesa/drivers/dri/i965/brw_tcs_surface_state.c | 23 ------
>  src/mesa/drivers/dri/i965/brw_tes_surface_state.c | 23 ------
>  src/mesa/drivers/dri/i965/brw_vs_surface_state.c  | 22 ------
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 87
> ++++-------------------
>  src/mesa/drivers/dri/i965/genX_state_upload.c     | 11 ---
>  src/mesa/drivers/dri/i965/intel_buffer_objects.c  |  2 +-
>  10 files changed, 16 insertions(+), 189 deletions(-)
>
> Total diffstat for the series:
>
>  18 files changed, 54 insertions(+), 416 deletions(-)
>

Nice!  Patches 2 and 3 are

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 5d19a6bfc9a..60279dbde46 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -195,7 +195,6 @@ enum brw_state_id {
>     BRW_STATE_RASTERIZER_DISCARD,
>     BRW_STATE_STATS_WM,
>     BRW_STATE_UNIFORM_BUFFER,
> -   BRW_STATE_ATOMIC_BUFFER,
>     BRW_STATE_IMAGE_UNITS,
>     BRW_STATE_META_IN_PROGRESS,
>     BRW_STATE_PUSH_CONSTANT_ALLOCATION,
> @@ -288,7 +287,6 @@ enum brw_state_id {
>  #define BRW_NEW_RASTERIZER_DISCARD      (1ull <<
> BRW_STATE_RASTERIZER_DISCARD)
>  #define BRW_NEW_STATS_WM                (1ull << BRW_STATE_STATS_WM)
>  #define BRW_NEW_UNIFORM_BUFFER          (1ull << BRW_STATE_UNIFORM_BUFFER)
> -#define BRW_NEW_ATOMIC_BUFFER           (1ull << BRW_STATE_ATOMIC_BUFFER)
>  #define BRW_NEW_IMAGE_UNITS             (1ull << BRW_STATE_IMAGE_UNITS)
>  #define BRW_NEW_META_IN_PROGRESS        (1ull <<
> BRW_STATE_META_IN_PROGRESS)
>  #define BRW_NEW_PUSH_CONSTANT_ALLOCATION (1ull <<
> BRW_STATE_PUSH_CONSTANT_ALLOCATION)
> @@ -1406,10 +1404,6 @@ brw_update_sol_surface(struct brw_context *brw,
>  void brw_upload_ubo_surfaces(struct brw_context *brw, struct gl_program
> *prog,
>                               struct brw_stage_state *stage_state,
>                               struct brw_stage_prog_data *prog_data);
> -void brw_upload_abo_surfaces(struct brw_context *brw,
> -                             const struct gl_program *prog,
> -                             struct brw_stage_state *stage_state,
> -                             struct brw_stage_prog_data *prog_data);
>  void brw_upload_image_surfaces(struct brw_context *brw,
>                                 const struct gl_program *prog,
>                                 struct brw_stage_state *stage_state,
> diff --git a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> index 570f3fb4dd2..6f2629eb29d 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> @@ -91,28 +91,6 @@ const struct brw_tracked_state brw_gs_ubo_surfaces = {
>     .emit = brw_upload_gs_ubo_surfaces,
>  };
>
> -static void
> -brw_upload_gs_abo_surfaces(struct brw_context *brw)
> -{
> -   /* _NEW_PROGRAM */
> -   const struct gl_program *gp = brw->programs[MESA_SHADER_GEOMETRY];
> -
> -   if (gp) {
> -      /* BRW_NEW_GS_PROG_DATA */
> -      brw_upload_abo_surfaces(brw, gp, &brw->gs.base,
> brw->gs.base.prog_data);
> -   }
> -}
> -
> -const struct brw_tracked_state brw_gs_abo_surfaces = {
> -   .dirty = {
> -      .mesa = _NEW_PROGRAM,
> -      .brw = BRW_NEW_ATOMIC_BUFFER |
> -             BRW_NEW_BATCH |
> -             BRW_NEW_GS_PROG_DATA,
> -   },
> -   .emit = brw_upload_gs_abo_surfaces,
> -};
> -
>  static void
>  brw_upload_gs_image_surfaces(struct brw_context *brw)
>  {
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> b/src/mesa/drivers/dri/i965/brw_state.h
> index 927e77920ef..f668b807fc7 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -58,16 +58,12 @@ extern const struct brw_tracked_state
> brw_recalculate_urb_fence;
>  extern const struct brw_tracked_state brw_sf_vp;
>  extern const struct brw_tracked_state brw_cs_texture_surfaces;
>  extern const struct brw_tracked_state brw_vs_ubo_surfaces;
> -extern const struct brw_tracked_state brw_vs_abo_surfaces;
>  extern const struct brw_tracked_state brw_vs_image_surfaces;
>  extern const struct brw_tracked_state brw_tcs_ubo_surfaces;
> -extern const struct brw_tracked_state brw_tcs_abo_surfaces;
>  extern const struct brw_tracked_state brw_tcs_image_surfaces;
>  extern const struct brw_tracked_state brw_tes_ubo_surfaces;
> -extern const struct brw_tracked_state brw_tes_abo_surfaces;
>  extern const struct brw_tracked_state brw_tes_image_surfaces;
>  extern const struct brw_tracked_state brw_gs_ubo_surfaces;
> -extern const struct brw_tracked_state brw_gs_abo_surfaces;
>  extern const struct brw_tracked_state brw_gs_image_surfaces;
>  extern const struct brw_tracked_state brw_renderbuffer_surfaces;
>  extern const struct brw_tracked_state brw_renderbuffer_read_surfaces;
> @@ -78,10 +74,8 @@ extern const struct brw_tracked_state
> brw_tes_binding_table;
>  extern const struct brw_tracked_state brw_tcs_binding_table;
>  extern const struct brw_tracked_state brw_vs_binding_table;
>  extern const struct brw_tracked_state brw_wm_ubo_surfaces;
> -extern const struct brw_tracked_state brw_wm_abo_surfaces;
>  extern const struct brw_tracked_state brw_wm_image_surfaces;
>  extern const struct brw_tracked_state brw_cs_ubo_surfaces;
> -extern const struct brw_tracked_state brw_cs_abo_surfaces;
>  extern const struct brw_tracked_state brw_cs_image_surfaces;
>
>  extern const struct brw_tracked_state brw_psp_urb_cbs;
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c
> b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index f54e15e92bf..4867d8fba70 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -210,7 +210,7 @@ void brw_init_state( struct brw_context *brw )
>     ctx->DriverFlags.NewUniformBuffer = BRW_NEW_UNIFORM_BUFFER;
>     ctx->DriverFlags.NewShaderStorageBuffer = BRW_NEW_UNIFORM_BUFFER;
>     ctx->DriverFlags.NewTextureBuffer = BRW_NEW_TEXTURE_BUFFER;
> -   ctx->DriverFlags.NewAtomicBuffer = BRW_NEW_ATOMIC_BUFFER;
> +   ctx->DriverFlags.NewAtomicBuffer = BRW_NEW_UNIFORM_BUFFER;
>     ctx->DriverFlags.NewImageUnits = BRW_NEW_IMAGE_UNITS;
>     ctx->DriverFlags.NewDefaultTessLevels = BRW_NEW_DEFAULT_TESS_LEVELS;
>     ctx->DriverFlags.NewIntelConservativeRasterization =
> BRW_NEW_CONSERVATIVE_RASTERIZATION;
> @@ -329,7 +329,6 @@ static struct dirty_bit_map brw_bits[] = {
>     DEFINE_BIT(BRW_NEW_RASTERIZER_DISCARD),
>     DEFINE_BIT(BRW_NEW_STATS_WM),
>     DEFINE_BIT(BRW_NEW_UNIFORM_BUFFER),
> -   DEFINE_BIT(BRW_NEW_ATOMIC_BUFFER),
>     DEFINE_BIT(BRW_NEW_IMAGE_UNITS),
>     DEFINE_BIT(BRW_NEW_META_IN_PROGRESS),
>     DEFINE_BIT(BRW_NEW_PUSH_CONSTANT_ALLOCATION),
> diff --git a/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c
> index f4f9abc0620..73179c006a0 100644
> --- a/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c
> @@ -91,29 +91,6 @@ const struct brw_tracked_state brw_tcs_ubo_surfaces = {
>     .emit = brw_upload_tcs_ubo_surfaces,
>  };
>
> -static void
> -brw_upload_tcs_abo_surfaces(struct brw_context *brw)
> -{
> -   /* _NEW_PROGRAM */
> -   const struct gl_program *tcp = brw->programs[MESA_SHADER_TESS_CTRL];
> -
> -   if (tcp) {
> -      /* BRW_NEW_TCS_PROG_DATA */
> -      brw_upload_abo_surfaces(brw, tcp, &brw->tcs.base,
> -                              brw->tcs.base.prog_data);
> -   }
> -}
> -
> -const struct brw_tracked_state brw_tcs_abo_surfaces = {
> -   .dirty = {
> -      .mesa = _NEW_PROGRAM,
> -      .brw = BRW_NEW_ATOMIC_BUFFER |
> -             BRW_NEW_BATCH |
> -             BRW_NEW_TCS_PROG_DATA,
> -   },
> -   .emit = brw_upload_tcs_abo_surfaces,
> -};
> -
>  static void
>  brw_upload_tcs_image_surfaces(struct brw_context *brw)
>  {
> diff --git a/src/mesa/drivers/dri/i965/brw_tes_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_tes_surface_state.c
> index 85b285d8688..6e9e58a4f17 100644
> --- a/src/mesa/drivers/dri/i965/brw_tes_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_tes_surface_state.c
> @@ -91,29 +91,6 @@ const struct brw_tracked_state brw_tes_ubo_surfaces = {
>     .emit = brw_upload_tes_ubo_surfaces,
>  };
>
> -static void
> -brw_upload_tes_abo_surfaces(struct brw_context *brw)
> -{
> -   /* _NEW_PROGRAM */
> -   const struct gl_program *tep = brw->programs[MESA_SHADER_TESS_EVAL];
> -
> -   if (tep) {
> -      /* BRW_NEW_TES_PROG_DATA */
> -      brw_upload_abo_surfaces(brw, tep, &brw->tes.base,
> -                              brw->tes.base.prog_data);
> -   }
> -}
> -
> -const struct brw_tracked_state brw_tes_abo_surfaces = {
> -   .dirty = {
> -      .mesa = _NEW_PROGRAM,
> -      .brw = BRW_NEW_ATOMIC_BUFFER |
> -             BRW_NEW_BATCH |
> -             BRW_NEW_TES_PROG_DATA,
> -   },
> -   .emit = brw_upload_tes_abo_surfaces,
> -};
> -
>  static void
>  brw_upload_tes_image_surfaces(struct brw_context *brw)
>  {
> 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 24c22e087d9..289c791b872 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> @@ -93,28 +93,6 @@ const struct brw_tracked_state brw_vs_ubo_surfaces = {
>     .emit = brw_upload_vs_ubo_surfaces,
>  };
>
> -static void
> -brw_upload_vs_abo_surfaces(struct brw_context *brw)
> -{
> -   /* _NEW_PROGRAM */
> -   const struct gl_program *vp = brw->programs[MESA_SHADER_VERTEX];
> -
> -   if (vp) {
> -      /* BRW_NEW_VS_PROG_DATA */
> -      brw_upload_abo_surfaces(brw, vp, &brw->vs.base,
> brw->vs.base.prog_data);
> -   }
> -}
> -
> -const struct brw_tracked_state brw_vs_abo_surfaces = {
> -   .dirty = {
> -      .mesa = _NEW_PROGRAM,
> -      .brw = BRW_NEW_ATOMIC_BUFFER |
> -             BRW_NEW_BATCH |
> -             BRW_NEW_VS_PROG_DATA,
> -   },
> -   .emit = brw_upload_vs_abo_surfaces,
> -};
> -
>  static void
>  brw_upload_vs_image_surfaces(struct brw_context *brw)
>  {
> 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 0653fbfe798..0183da45bf1 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -1266,7 +1266,9 @@ brw_upload_ubo_surfaces(struct brw_context *brw,
> struct gl_program *prog,
>  {
>     struct gl_context *ctx = &brw->ctx;
>
> -   if (!prog)
> +   if (!prog || (prog->info.num_ubos == 0 &&
> +                 prog->info.num_ssbos == 0 &&
> +                 prog->info.num_abos == 0))
>        return;
>
>     uint32_t *ubo_surf_offsets =
> @@ -1279,9 +1281,16 @@ brw_upload_ubo_surfaces(struct brw_context *brw,
> struct gl_program *prog,
>                              ISL_FORMAT_R32G32B32A32_FLOAT, 0);
>     }
>
> -   uint32_t *ssbo_surf_offsets =
> -      &stage_state->surf_offset[prog_data->binding_table.ssbo_start] +
> -      prog->info.num_abos;
> +   uint32_t *abo_surf_offsets =
> +      &stage_state->surf_offset[prog_data->binding_table.ssbo_start];
> +   uint32_t *ssbo_surf_offsets = abo_surf_offsets + prog->info.num_abos;
> +
> +   for (int i = 0; i < prog->info.num_abos; i++) {
> +      struct gl_buffer_binding *binding =
> +         &ctx->AtomicBufferBindings[prog->sh.AtomicBuffers[i]->Binding];
> +      upload_buffer_surface(brw, binding, &abo_surf_offsets[i],
> +                            ISL_FORMAT_RAW, RELOC_WRITE);
> +   }
>
>     for (int i = 0; i < prog->info.num_ssbos; i++) {
>        struct gl_buffer_binding *binding =
> @@ -1292,9 +1301,7 @@ brw_upload_ubo_surfaces(struct brw_context *brw,
> struct gl_program *prog,
>     }
>
>     stage_state->push_constants_dirty = true;
> -
> -   if (prog->info.num_ubos || prog->info.num_ssbos)
> -      brw->ctx.NewDriverState |= BRW_NEW_SURFACES;
> +   brw->ctx.NewDriverState |= BRW_NEW_SURFACES;
>  }
>
>  static void
> @@ -1340,72 +1347,6 @@ const struct brw_tracked_state brw_cs_ubo_surfaces
> = {
>     .emit = brw_upload_cs_ubo_surfaces,
>  };
>
> -void
> -brw_upload_abo_surfaces(struct brw_context *brw,
> -                        const struct gl_program *prog,
> -                        struct brw_stage_state *stage_state,
> -                        struct brw_stage_prog_data *prog_data)
> -{
> -   struct gl_context *ctx = &brw->ctx;
> -   uint32_t *surf_offsets =
> -      &stage_state->surf_offset[prog_data->binding_table.ssbo_start];
> -
> -   if (prog->info.num_abos) {
> -      for (unsigned i = 0; i < prog->info.num_abos; i++) {
> -         struct gl_buffer_binding *binding =
> -            &ctx->AtomicBufferBindings[prog->sh.AtomicBuffers[i]->
> Binding];
> -         upload_buffer_surface(brw, binding, &surf_offsets[i],
> -                               ISL_FORMAT_RAW, RELOC_WRITE);
> -      }
> -
> -      brw->ctx.NewDriverState |= BRW_NEW_SURFACES;
> -   }
> -}
> -
> -static void
> -brw_upload_wm_abo_surfaces(struct brw_context *brw)
> -{
> -   /* _NEW_PROGRAM */
> -   const struct gl_program *wm = brw->programs[MESA_SHADER_FRAGMENT];
> -
> -   if (wm) {
> -      /* BRW_NEW_FS_PROG_DATA */
> -      brw_upload_abo_surfaces(brw, wm, &brw->wm.base,
> brw->wm.base.prog_data);
> -   }
> -}
> -
> -const struct brw_tracked_state brw_wm_abo_surfaces = {
> -   .dirty = {
> -      .mesa = _NEW_PROGRAM,
> -      .brw = BRW_NEW_ATOMIC_BUFFER |
> -             BRW_NEW_BATCH |
> -             BRW_NEW_FS_PROG_DATA,
> -   },
> -   .emit = brw_upload_wm_abo_surfaces,
> -};
> -
> -static void
> -brw_upload_cs_abo_surfaces(struct brw_context *brw)
> -{
> -   /* _NEW_PROGRAM */
> -   const struct gl_program *cp = brw->programs[MESA_SHADER_COMPUTE];
> -
> -   if (cp) {
> -      /* BRW_NEW_CS_PROG_DATA */
> -      brw_upload_abo_surfaces(brw, cp, &brw->cs.base,
> brw->cs.base.prog_data);
> -   }
> -}
> -
> -const struct brw_tracked_state brw_cs_abo_surfaces = {
> -   .dirty = {
> -      .mesa = _NEW_PROGRAM,
> -      .brw = BRW_NEW_ATOMIC_BUFFER |
> -             BRW_NEW_BATCH |
> -             BRW_NEW_CS_PROG_DATA,
> -   },
> -   .emit = brw_upload_cs_abo_surfaces,
> -};
> -
>  static void
>  brw_upload_cs_image_surfaces(struct brw_context *brw)
>  {
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
> b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index 453b8e4adda..d4b0de850c9 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -5483,19 +5483,14 @@ genX(init_atoms)(struct brw_context *brw)
>         */
>        &brw_vs_pull_constants,
>        &brw_vs_ubo_surfaces,
> -      &brw_vs_abo_surfaces,
>        &brw_tcs_pull_constants,
>        &brw_tcs_ubo_surfaces,
> -      &brw_tcs_abo_surfaces,
>        &brw_tes_pull_constants,
>        &brw_tes_ubo_surfaces,
> -      &brw_tes_abo_surfaces,
>        &brw_gs_pull_constants,
>        &brw_gs_ubo_surfaces,
> -      &brw_gs_abo_surfaces,
>        &brw_wm_pull_constants,
>        &brw_wm_ubo_surfaces,
> -      &brw_wm_abo_surfaces,
>        &gen6_renderbuffer_surfaces,
>        &brw_renderbuffer_read_surfaces,
>        &brw_texture_surfaces,
> @@ -5575,19 +5570,14 @@ genX(init_atoms)(struct brw_context *brw)
>         */
>        &brw_vs_pull_constants,
>        &brw_vs_ubo_surfaces,
> -      &brw_vs_abo_surfaces,
>        &brw_tcs_pull_constants,
>        &brw_tcs_ubo_surfaces,
> -      &brw_tcs_abo_surfaces,
>        &brw_tes_pull_constants,
>        &brw_tes_ubo_surfaces,
> -      &brw_tes_abo_surfaces,
>        &brw_gs_pull_constants,
>        &brw_gs_ubo_surfaces,
> -      &brw_gs_abo_surfaces,
>        &brw_wm_pull_constants,
>        &brw_wm_ubo_surfaces,
> -      &brw_wm_abo_surfaces,
>        &gen6_renderbuffer_surfaces,
>        &brw_renderbuffer_read_surfaces,
>        &brw_texture_surfaces,
> @@ -5657,7 +5647,6 @@ genX(init_atoms)(struct brw_context *brw)
>        &genX(cs_push_constants),
>        &genX(cs_pull_constants),
>        &brw_cs_ubo_surfaces,
> -      &brw_cs_abo_surfaces,
>        &brw_cs_texture_surfaces,
>        &brw_cs_work_groups_surface,
>        &genX(cs_samplers),
> diff --git a/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> b/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> index 49e68bd7392..8268040911a 100644
> --- a/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> +++ b/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> @@ -107,7 +107,7 @@ alloc_buffer_object(struct brw_context *brw,
>     if (intel_obj->Base.UsageHistory & USAGE_TEXTURE_BUFFER)
>        brw->ctx.NewDriverState |= BRW_NEW_TEXTURE_BUFFER;
>     if (intel_obj->Base.UsageHistory & USAGE_ATOMIC_COUNTER_BUFFER)
> -      brw->ctx.NewDriverState |= BRW_NEW_ATOMIC_BUFFER;
> +      brw->ctx.NewDriverState |= BRW_NEW_UNIFORM_BUFFER;
>
>     mark_buffer_inactive(intel_obj);
>     mark_buffer_invalid(intel_obj);
> --
> 2.15.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171109/e3fc1058/attachment-0001.html>


More information about the mesa-dev mailing list