From c66165ab2b15047792808433b788632a4b9df287 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Fri, 1 Aug 2014 10:35:20 +0200 Subject: [PATCH] i965/gen6/gs: Fix binding table clash between TF surfaces and textures. For gen6 geometry shaders we use the first BRW_MAX_SOL_BINDINGS entries of the binding table for transform feedback surfaces. However, vec4_visitor will setup the binding table so that textures use the same space in the binding table. This is done when calling assign_common_binding_table_offsets(0) as part if its run() method. To fix this clash we add a virtual method to the vec4_visitor hierarchy to assign the binding table offsets, so that we can change this behavior specifically for gen6 geometry shaders by mapping textures right after the first BRW_MAX_SOL_BINDINGS entries. Also, when there is no user-provided geometry shader, we only need to upload the binding table if we have transform feedback, however, in the case of a user-provided geometry shader, we can't only look into transform feedback to make that decision. This fixes multiple piglit tests for textureSize() and texelFetch() when these functions are called from a geometry shader in gen6, like these: bin/textureSize gs sampler2D -fbo -auto bin/texelFetch gs usampler2D -fbo -auto Acked-by: Kenneth Graunke Reviewed-by: Jordan Justen --- src/mesa/drivers/dri/i965/brw_context.h | 8 +- src/mesa/drivers/dri/i965/brw_vec4.cpp | 8 +- src/mesa/drivers/dri/i965/brw_vec4.h | 1 + src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp | 9 +++ src/mesa/drivers/dri/i965/gen6_gs_visitor.h | 1 + src/mesa/drivers/dri/i965/gen6_sol.c | 80 +++++++++++++------ 6 files changed, 78 insertions(+), 29 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index 3bdc4806163..377853ed96d 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -600,7 +600,6 @@ struct brw_vs_prog_data { 2 /* shader time, pull constants */) #define SURF_INDEX_GEN6_SOL_BINDING(t) (t) -#define BRW_MAX_GEN6_GS_SURFACES SURF_INDEX_GEN6_SOL_BINDING(BRW_MAX_SOL_BINDINGS) /* Note: brw_gs_prog_data_compare() must be updated when adding fields to * this struct! @@ -1260,7 +1259,12 @@ struct brw_context uint32_t state_offset; uint32_t bind_bo_offset; - uint32_t surf_offset[BRW_MAX_GEN6_GS_SURFACES]; + /** + * Surface offsets for the binding table. We only need surfaces to + * implement transform feedback so BRW_MAX_SOL_BINDINGS is all that we + * need in this case. + */ + uint32_t surf_offset[BRW_MAX_SOL_BINDINGS]; } ff_gs; struct { diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp index 4210ee00a54..d5180263010 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp @@ -1612,6 +1612,12 @@ vec4_vs_visitor::setup_payload(void) this->first_non_payload_grf = reg; } +void +vec4_visitor::assign_binding_table_offsets() +{ + assign_common_binding_table_offsets(0); +} + src_reg vec4_visitor::get_timestamp() { @@ -1711,7 +1717,7 @@ vec4_visitor::run() if (INTEL_DEBUG & DEBUG_SHADER_TIME) emit_shader_time_begin(); - assign_common_binding_table_offsets(0); + assign_binding_table_offsets(); emit_prolog(); diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h index 1909cbd75d6..69faa740cb2 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.h +++ b/src/mesa/drivers/dri/i965/brw_vec4.h @@ -589,6 +589,7 @@ protected: void setup_payload_interference(struct ra_graph *g, int first_payload_node, int reg_node_count); virtual dst_reg *make_reg_for_system_value(ir_variable *ir) = 0; + virtual void assign_binding_table_offsets(); virtual void setup_payload() = 0; virtual void emit_prolog() = 0; virtual void emit_program_code() = 0; diff --git a/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp b/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp index 704a02f9c3b..d16cc6ed8b7 100644 --- a/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp @@ -35,6 +35,15 @@ const unsigned MAX_GS_INPUT_VERTICES = 6; namespace brw { +void +gen6_gs_visitor::assign_binding_table_offsets() +{ + /* In gen6 we reserve the first BRW_MAX_SOL_BINDINGS entries for transform + * feedback surfaces. + */ + assign_common_binding_table_offsets(BRW_MAX_SOL_BINDINGS); +} + void gen6_gs_visitor::emit_prolog() { diff --git a/src/mesa/drivers/dri/i965/gen6_gs_visitor.h b/src/mesa/drivers/dri/i965/gen6_gs_visitor.h index d29afdb3f11..28f23c9e4f7 100644 --- a/src/mesa/drivers/dri/i965/gen6_gs_visitor.h +++ b/src/mesa/drivers/dri/i965/gen6_gs_visitor.h @@ -43,6 +43,7 @@ public: vec4_gs_visitor(brw, c, prog, mem_ctx, no_spills) {} protected: + virtual void assign_binding_table_offsets(); virtual void emit_prolog(); virtual void emit_thread_end(); virtual void visit(ir_emit_vertex *); diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c b/src/mesa/drivers/dri/i965/gen6_sol.c index d21a01037f1..997db1a891f 100644 --- a/src/mesa/drivers/dri/i965/gen6_sol.c +++ b/src/mesa/drivers/dri/i965/gen6_sol.c @@ -108,48 +108,76 @@ static void brw_gs_upload_binding_table(struct brw_context *brw) { uint32_t *bind; + struct gl_context *ctx = &brw->ctx; + const struct gl_shader_program *shaderprog; + bool need_binding_table = false; + + /* We have two scenarios here: + * 1) We are using a geometry shader only to implement transform feedback + * for a vertex shader (brw->geometry_program == NULL). In this case, we + * only need surfaces for transform feedback in the GS stage. + * 2) We have a user-provided geometry shader. In this case we may need + * surfaces for transform feedback and/or other stuff, like textures, + * in the GS stage. + */ if (!brw->geometry_program) { - struct gl_context *ctx = &brw->ctx; /* BRW_NEW_VERTEX_PROGRAM */ - const struct gl_shader_program *shaderprog = - ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX]; - bool has_surfaces = false; - + shaderprog = ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX]; if (shaderprog) { + /* Skip making a binding table if we don't have anything to put in it */ const struct gl_transform_feedback_info *linked_xfb_info = &shaderprog->LinkedTransformFeedback; - /* Currently we only ever upload surfaces for SOL. */ - has_surfaces = linked_xfb_info->NumOutputs != 0; - - /* Skip making a binding table if we don't have anything to put in it. */ - if (!has_surfaces) { - if (brw->ff_gs.bind_bo_offset != 0) { - brw->state.dirty.brw |= BRW_NEW_GS_BINDING_TABLE; - brw->ff_gs.bind_bo_offset = 0; - } - return; + need_binding_table = linked_xfb_info->NumOutputs > 0; + } + if (!need_binding_table) { + if (brw->ff_gs.bind_bo_offset != 0) { + brw->state.dirty.brw |= BRW_NEW_GS_BINDING_TABLE; + brw->ff_gs.bind_bo_offset = 0; } + return; } - } - /* Might want to calculate nr_surfaces first, to avoid taking up so much - * space for the binding table. - */ - if (brw->geometry_program) { + /* Might want to calculate nr_surfaces first, to avoid taking up so much + * space for the binding table. Anyway, in this case we know that we only + * use BRW_MAX_SOL_BINDINGS surfaces at most. + */ bind = brw_state_batch(brw, AUB_TRACE_BINDING_TABLE, - sizeof(uint32_t) * BRW_MAX_GEN6_GS_SURFACES, - 32, &brw->gs.base.bind_bo_offset); + sizeof(uint32_t) * BRW_MAX_SOL_BINDINGS, + 32, &brw->ff_gs.bind_bo_offset); /* BRW_NEW_SURFACES */ - memcpy(bind, brw->gs.base.surf_offset, BRW_MAX_GEN6_GS_SURFACES * sizeof(uint32_t)); + memcpy(bind, brw->ff_gs.surf_offset, + BRW_MAX_SOL_BINDINGS * sizeof(uint32_t)); } else { + /* BRW_NEW_GEOMETRY_PROGRAM */ + shaderprog = ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]; + if (shaderprog) { + /* Skip making a binding table if we don't have anything to put in it */ + struct brw_stage_prog_data *prog_data = brw->gs.base.prog_data; + const struct gl_transform_feedback_info *linked_xfb_info = + &shaderprog->LinkedTransformFeedback; + need_binding_table = linked_xfb_info->NumOutputs > 0 || + prog_data->binding_table.size_bytes > 0; + } + if (!need_binding_table) { + if (brw->gs.base.bind_bo_offset != 0) { + brw->gs.base.bind_bo_offset = 0; + brw->state.dirty.brw |= BRW_NEW_GS_BINDING_TABLE; + } + return; + } + + /* Might want to calculate nr_surfaces first, to avoid taking up so much + * space for the binding table. + */ bind = brw_state_batch(brw, AUB_TRACE_BINDING_TABLE, - sizeof(uint32_t) * BRW_MAX_GEN6_GS_SURFACES, - 32, &brw->ff_gs.bind_bo_offset); + sizeof(uint32_t) * BRW_MAX_SURFACES, + 32, &brw->gs.base.bind_bo_offset); /* BRW_NEW_SURFACES */ - memcpy(bind, brw->ff_gs.surf_offset, BRW_MAX_GEN6_GS_SURFACES * sizeof(uint32_t)); + memcpy(bind, brw->gs.base.surf_offset, + BRW_MAX_SURFACES * sizeof(uint32_t)); } brw->state.dirty.brw |= BRW_NEW_GS_BINDING_TABLE; -- 2.30.2