[Mesa-dev] [PATCH 1/3] i965: Make a better helper function for UBO/SSBO/ABO surface handling.
Jason Ekstrand
jason at jlekstrand.net
Thu Nov 9 20:31:18 UTC 2017
On Thu, Nov 9, 2017 at 12:45 AM, Kenneth Graunke <kenneth at whitecape.org>
wrote:
> This fixes the missing AutomaticSize handling in the ABO code, removes
> a bunch of duplicated code, and drops an extra layer of wrapping around
> brw_emit_buffer_surface_state().
> ---
> src/mesa/drivers/dri/i965/brw_context.h | 10 --
> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 113
> +++++++----------------
> src/mesa/drivers/dri/i965/gen6_constant_state.c | 7 +-
> 3 files changed, 36 insertions(+), 94 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 8aa0c5ff64c..5d19a6bfc9a 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1395,16 +1395,6 @@ brw_get_index_type(unsigned index_size)
> void brw_prepare_vertices(struct brw_context *brw);
>
> /* brw_wm_surface_state.c */
> -void brw_create_constant_surface(struct brw_context *brw,
> - struct brw_bo *bo,
> - uint32_t offset,
> - uint32_t size,
> - uint32_t *out_offset);
> -void brw_create_buffer_surface(struct brw_context *brw,
> - struct brw_bo *bo,
> - uint32_t offset,
> - uint32_t size,
> - uint32_t *out_offset);
> void brw_update_buffer_texture_surface(struct gl_context *ctx,
> unsigned unit,
> uint32_t *surf_offset);
> 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 27c241a87af..a483ba34151 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -672,44 +672,6 @@ brw_update_buffer_texture_surface(struct gl_context
> *ctx,
> 0);
> }
>
> -/**
> - * Create the constant buffer surface. Vertex/fragment shader constants
> will be
> - * read from this buffer with Data Port Read instructions/messages.
> - */
> -void
> -brw_create_constant_surface(struct brw_context *brw,
> - struct brw_bo *bo,
> - uint32_t offset,
> - uint32_t size,
> - uint32_t *out_offset)
> -{
> - brw_emit_buffer_surface_state(brw, out_offset, bo, offset,
> - ISL_FORMAT_R32G32B32A32_FLOAT,
> - size, 1, 0);
> -}
> -
> -/**
> - * Create the buffer surface. Shader buffer variables will be
> - * read from / write to this buffer with Data Port Read/Write
> - * instructions/messages.
> - */
> -void
> -brw_create_buffer_surface(struct brw_context *brw,
> - struct brw_bo *bo,
> - uint32_t offset,
> - uint32_t size,
> - uint32_t *out_offset)
> -{
> - /* Use a raw surface so we can reuse existing untyped read/write/atomic
> - * messages. We need these specifically for the fragment shader since
> they
> - * include a pixel mask header that we need to ensure correct behavior
> - * with helper invocations, which cannot write to the buffer.
> - */
> - brw_emit_buffer_surface_state(brw, out_offset, bo, offset,
> - ISL_FORMAT_RAW,
> - size, 1, RELOC_WRITE);
> -}
> -
> /**
> * Set up a binding table entry for use by stream output logic (transform
> * feedback).
> @@ -1271,6 +1233,31 @@ const struct brw_tracked_state
> brw_cs_texture_surfaces = {
> .emit = brw_update_cs_texture_surfaces,
> };
>
> +static void
> +upload_buffer_surface(struct brw_context *brw,
> + struct gl_buffer_binding *binding,
> + uint32_t *out_offset,
> + enum isl_format format,
> + unsigned reloc_flags)
> +{
> + struct gl_context *ctx = &brw->ctx;
> +
> + if (binding->BufferObject == ctx->Shared->NullBufferObj) {
> + emit_null_surface_state(brw, NULL, out_offset);
> + } else {
> + ptrdiff_t size = binding->BufferObject->Size - binding->Offset;
> + if (!binding->AutomaticSize)
> + size = MIN2(size, binding->Size);
> +
> + struct intel_buffer_object *iobj =
> + intel_buffer_object(binding->BufferObject);
> + struct brw_bo *bo =
> + intel_bufferobj_buffer(brw, iobj, binding->Offset, size, false);
>
You're using this for both reads and writes. I think you need another
boolean parameter instead of simply passing false all the time. Other than
that, looks good.
> +
> + brw_emit_buffer_surface_state(brw, out_offset, bo, binding->Offset,
> + format, size, 1, reloc_flags);
> + }
> +}
>
> void
> brw_upload_ubo_surfaces(struct brw_context *brw, struct gl_program *prog,
> @@ -1288,23 +1275,8 @@ brw_upload_ubo_surfaces(struct brw_context *brw,
> struct gl_program *prog,
> for (int i = 0; i < prog->info.num_ubos; i++) {
> struct gl_buffer_binding *binding =
> &ctx->UniformBufferBindings[prog->sh.UniformBlocks[i]->Binding];
> -
> - if (binding->BufferObject == ctx->Shared->NullBufferObj) {
> - emit_null_surface_state(brw, NULL, &ubo_surf_offsets[i]);
> - } else {
> - struct intel_buffer_object *intel_bo =
> - intel_buffer_object(binding->BufferObject);
> - GLsizeiptr size = binding->BufferObject->Size - binding->Offset;
> - if (!binding->AutomaticSize)
> - size = MIN2(size, binding->Size);
> - struct brw_bo *bo =
> - intel_bufferobj_buffer(brw, intel_bo,
> - binding->Offset,
> - size, false);
> - brw_create_constant_surface(brw, bo, binding->Offset,
> - size,
> - &ubo_surf_offsets[i]);
> - }
> + upload_buffer_surface(brw, binding, &ubo_surf_offsets[i],
> + ISL_FORMAT_R32G32B32A32_FLOAT, 0);
> }
>
> uint32_t *ssbo_surf_offsets =
> @@ -1314,22 +1286,8 @@ brw_upload_ubo_surfaces(struct brw_context *brw,
> struct gl_program *prog,
> struct gl_buffer_binding *binding =
> &ctx->ShaderStorageBufferBindings[prog->sh.ShaderStorageBlocks[
> i]->Binding];
>
> - if (binding->BufferObject == ctx->Shared->NullBufferObj) {
> - emit_null_surface_state(brw, NULL, &ssbo_surf_offsets[i]);
> - } else {
> - struct intel_buffer_object *intel_bo =
> - intel_buffer_object(binding->BufferObject);
> - GLsizeiptr size = binding->BufferObject->Size - binding->Offset;
> - if (!binding->AutomaticSize)
> - size = MIN2(size, binding->Size);
> - struct brw_bo *bo =
> - intel_bufferobj_buffer(brw, intel_bo,
> - binding->Offset,
> - size, true);
> - brw_create_buffer_surface(brw, bo, binding->Offset,
> - size,
> - &ssbo_surf_offsets[i]);
> - }
> + upload_buffer_surface(brw, binding, &ssbo_surf_offsets[i],
> + ISL_FORMAT_RAW, RELOC_WRITE);
> }
>
> stage_state->push_constants_dirty = true;
> @@ -1395,17 +1353,8 @@ brw_upload_abo_surfaces(struct brw_context *brw,
> for (unsigned i = 0; i < prog->info.num_abos; i++) {
> struct gl_buffer_binding *binding =
> &ctx->AtomicBufferBindings[prog->sh.AtomicBuffers[i]->
> Binding];
> - struct intel_buffer_object *intel_bo =
> - intel_buffer_object(binding->BufferObject);
> - struct brw_bo *bo =
> - intel_bufferobj_buffer(brw, intel_bo, binding->Offset,
> - intel_bo->Base.Size - binding->Offset,
> - true);
> -
> - brw_emit_buffer_surface_state(brw, &surf_offsets[i], bo,
> - binding->Offset, ISL_FORMAT_RAW,
> - bo->size - binding->Offset, 1,
> - RELOC_WRITE);
> + upload_buffer_surface(brw, binding, &surf_offsets[i],
> + ISL_FORMAT_RAW, RELOC_WRITE);
> }
>
> brw->ctx.NewDriverState |= BRW_NEW_SURFACES;
> diff --git a/src/mesa/drivers/dri/i965/gen6_constant_state.c
> b/src/mesa/drivers/dri/i965/gen6_constant_state.c
> index d89e7bde24b..89b1202dd65 100644
> --- a/src/mesa/drivers/dri/i965/gen6_constant_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_constant_state.c
> @@ -266,8 +266,11 @@ brw_upload_pull_constants(struct brw_context *brw,
> }
> }
>
> - brw_create_constant_surface(brw, const_bo, const_offset, size,
> - &stage_state->surf_offset[surf_index]);
> + brw_emit_buffer_surface_state(brw, &stage_state->surf_offset[
> surf_index],
> + const_bo, const_offset,
> + ISL_FORMAT_R32G32B32A32_FLOAT,
> + size, 1, 0);
> +
> brw_bo_unreference(const_bo);
>
> brw->ctx.NewDriverState |= brw_new_constbuf;
> --
> 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/86b93771/attachment-0001.html>
More information about the mesa-dev
mailing list