On Thu, Nov 9, 2017 at 2:42 PM, Kenneth Graunke <[email protected]> wrote:
> On Thursday, November 9, 2017 12:31:18 PM PST Jason Ekstrand wrote: > > On Thu, Nov 9, 2017 at 12:45 AM, Kenneth Graunke <[email protected]> > > 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. > > Whoops, thanks for catching that! I've changed it from 'false' to > (reloc_flags & RELOC_WRITE) != 0. > > With that change, should I add your R-b? > Yeah, that's probably ok. I don't know how I feel about keying it off of the reloc flag as opposed to the other way around but I don't care too much. With that, it would be Reviewed-by: Jason Ekstrand <[email protected]>
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
