From fb02b4ec482762ccf2a9fedf24fe6f50787932a9 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Mon, 13 Jul 2015 18:01:14 +0100 Subject: [PATCH] i965: Swap the order of the vertex ID and edge flag attributes MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The edge flag data on Gen6+ is passed through the fixed function hardware as an extra attribute. According to the PRM it must be the last valid VERTEX_ELEMENT structure. However if the vertex ID is also used then another extra element is added to source the VID. This made it so the vertex ID is in the wrong register in the vertex shader and the edge attribute is no longer in the last element. v2: Also implement for BDW+ v3 [by Ben]: Remove 10.5 tag. Too late. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84677 Cc: Reviewed-by: Kristian Høgsberg Signed-off-by: Ben Widawsky Tested-by: Ben Widawsky Tested-by: Mark Janes --- src/mesa/drivers/dri/i965/brw_draw_upload.c | 30 +++++------ src/mesa/drivers/dri/i965/gen8_draw_upload.c | 56 +++++++++++++++----- 2 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c b/src/mesa/drivers/dri/i965/brw_draw_upload.c index 33b1e41b1a5..21d8f1e6994 100644 --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c @@ -793,21 +793,6 @@ brw_emit_vertices(struct brw_context *brw) ((i * 4) << BRW_VE1_DST_OFFSET_SHIFT)); } - if (brw->gen >= 6 && gen6_edgeflag_input) { - uint32_t format = - brw_get_vertex_surface_type(brw, gen6_edgeflag_input->glarray); - - OUT_BATCH((gen6_edgeflag_input->buffer << GEN6_VE0_INDEX_SHIFT) | - GEN6_VE0_VALID | - GEN6_VE0_EDGE_FLAG_ENABLE | - (format << BRW_VE0_FORMAT_SHIFT) | - (gen6_edgeflag_input->offset << BRW_VE0_SRC_OFFSET_SHIFT)); - OUT_BATCH((BRW_VE1_COMPONENT_STORE_SRC << BRW_VE1_COMPONENT_0_SHIFT) | - (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_1_SHIFT) | - (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_2_SHIFT) | - (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_3_SHIFT)); - } - if (brw->vs.prog_data->uses_vertexid || brw->vs.prog_data->uses_instanceid) { uint32_t dw0 = 0, dw1 = 0; uint32_t comp0 = BRW_VE1_COMPONENT_STORE_0; @@ -848,6 +833,21 @@ brw_emit_vertices(struct brw_context *brw) OUT_BATCH(dw1); } + if (brw->gen >= 6 && gen6_edgeflag_input) { + uint32_t format = + brw_get_vertex_surface_type(brw, gen6_edgeflag_input->glarray); + + OUT_BATCH((gen6_edgeflag_input->buffer << GEN6_VE0_INDEX_SHIFT) | + GEN6_VE0_VALID | + GEN6_VE0_EDGE_FLAG_ENABLE | + (format << BRW_VE0_FORMAT_SHIFT) | + (gen6_edgeflag_input->offset << BRW_VE0_SRC_OFFSET_SHIFT)); + OUT_BATCH((BRW_VE1_COMPONENT_STORE_SRC << BRW_VE1_COMPONENT_0_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_1_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_2_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_3_SHIFT)); + } + ADVANCE_BATCH(); } diff --git a/src/mesa/drivers/dri/i965/gen8_draw_upload.c b/src/mesa/drivers/dri/i965/gen8_draw_upload.c index f7d9952aeec..2bac5ffb058 100644 --- a/src/mesa/drivers/dri/i965/gen8_draw_upload.c +++ b/src/mesa/drivers/dri/i965/gen8_draw_upload.c @@ -40,16 +40,25 @@ gen8_emit_vertices(struct brw_context *brw) { struct gl_context *ctx = &brw->ctx; uint32_t mocs_wb = brw->gen >= 9 ? SKL_MOCS_WB : BDW_MOCS_WB; + bool uses_edge_flag; brw_prepare_vertices(brw); brw_prepare_shader_draw_parameters(brw); + uses_edge_flag = (ctx->Polygon.FrontMode != GL_FILL || + ctx->Polygon.BackMode != GL_FILL); + if (brw->vs.prog_data->uses_vertexid || brw->vs.prog_data->uses_instanceid) { unsigned vue = brw->vb.nr_enabled; - WARN_ONCE(brw->vs.prog_data->inputs_read & VERT_BIT_EDGEFLAG, - "Using VID/IID with edgeflags, need to reorder the " - "vertex attributes"); + /* The element for the edge flags must always be last, so we have to + * insert the SGVS before it in that case. + */ + if (uses_edge_flag) { + assert(vue > 0); + vue--; + } + WARN_ONCE(vue >= 33, "Trying to insert VID/IID past 33rd vertex element, " "need to reorder the vertex attrbutes."); @@ -138,7 +147,18 @@ gen8_emit_vertices(struct brw_context *brw) ADVANCE_BATCH(); } - unsigned nr_elements = brw->vb.nr_enabled + brw->vs.prog_data->uses_vertexid; + /* Normally we don't need an element for the SGVS attribute because the + * 3DSTATE_VF_SGVS instruction lets you store the generated attribute in an + * element that is past the list in 3DSTATE_VERTEX_ELEMENTS. However if the + * vertex ID is used then it needs an element for the base vertex buffer. + * Additionally if there is an edge flag element then the SGVS can't be + * inserted past that so we need a dummy element to ensure that the edge + * flag is the last one. + */ + bool needs_sgvs_element = (brw->vs.prog_data->uses_vertexid || + (brw->vs.prog_data->uses_instanceid && + uses_edge_flag)); + unsigned nr_elements = brw->vb.nr_enabled + needs_sgvs_element; /* The hardware allows one more VERTEX_ELEMENTS than VERTEX_BUFFERS, * presumably for VertexID/InstanceID. @@ -192,6 +212,24 @@ gen8_emit_vertices(struct brw_context *brw) (comp3 << BRW_VE1_COMPONENT_3_SHIFT)); } + if (needs_sgvs_element) { + if (brw->vs.prog_data->uses_vertexid) { + OUT_BATCH(GEN6_VE0_VALID | + brw->vb.nr_buffers << GEN6_VE0_INDEX_SHIFT | + BRW_SURFACEFORMAT_R32_UINT << BRW_VE0_FORMAT_SHIFT); + OUT_BATCH((BRW_VE1_COMPONENT_STORE_SRC << BRW_VE1_COMPONENT_0_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_1_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_2_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_3_SHIFT)); + } else { + OUT_BATCH(GEN6_VE0_VALID); + OUT_BATCH((BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_0_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_1_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_2_SHIFT) | + (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_3_SHIFT)); + } + } + if (gen6_edgeflag_input) { uint32_t format = brw_get_vertex_surface_type(brw, gen6_edgeflag_input->glarray); @@ -206,16 +244,6 @@ gen8_emit_vertices(struct brw_context *brw) (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_2_SHIFT) | (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_3_SHIFT)); } - - if (brw->vs.prog_data->uses_vertexid) { - OUT_BATCH(GEN6_VE0_VALID | - brw->vb.nr_buffers << GEN6_VE0_INDEX_SHIFT | - BRW_SURFACEFORMAT_R32_UINT << BRW_VE0_FORMAT_SHIFT); - OUT_BATCH((BRW_VE1_COMPONENT_STORE_SRC << BRW_VE1_COMPONENT_0_SHIFT) | - (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_1_SHIFT) | - (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_2_SHIFT) | - (BRW_VE1_COMPONENT_STORE_0 << BRW_VE1_COMPONENT_3_SHIFT)); - } ADVANCE_BATCH(); for (unsigned i = 0; i < brw->vb.nr_enabled; i++) { -- 2.30.2