<html><head></head><body><div>Ken, do you have any comments about this patch? I'd like to push it otherwise.</div><div><br></div><div>Iago</div><div><br></div><div>On Thu, 2018-01-04 at 14:24 -0800, Jason Ekstrand wrote:</div><blockquote type="cite"><div dir="ltr"><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br><br></div>Ken?<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 3, 2018 at 6:55 PM, Iago Toral Quiroga <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote type="cite">Although on gen8+ platforms we can in theory use 3DSTATE_VF_SGVS<br>
to put these beyond the last vertex element it seems that we still<br>
need to allocate the SVGS element, otherwise we have observed cases<br>
where we end up reading garbage. Specifically, the CTS test mentioned<br>
below was flaky with a fail rate of ~1% on some gen9+ platforms caused<br>
by reading garbage for the gl_InstanceID value. The flakyness goes<br>
away as soon as we start allocating the SVGS element.<br>
<br>
v2:<br>
  - Do this for gen8+, not just gen9+, and pull the boolean<br>
    outside the #if block (Jason)<br>
<br>
Fixes flaky test:<br>
KHR-GL45.vertex_attrib_64bit.<wbr>limits_test<br>
<br>
Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=104335" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/<wbr>show_bug.cgi?id=104335</a><br>
---<br>
 src/mesa/drivers/dri/i965/<wbr>genX_state_upload.c | 17 ++---------------<br>
 1 file changed, 2 insertions(+), 15 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/<wbr>genX_state_upload.c b/src/mesa/drivers/dri/i965/<wbr>genX_state_upload.c<br>
index 50ac5bc59f..d0a980f973 100644<br>
--- a/src/mesa/drivers/dri/i965/<wbr>genX_state_upload.c<br>
+++ b/src/mesa/drivers/dri/i965/<wbr>genX_state_upload.c<br>
@@ -486,26 +486,13 @@ genX(emit_vertices)(struct brw_context *brw)<br>
    } else {<br>
       brw_batch_emit(brw, GENX(3DSTATE_VF_SGVS), vfs);<br>
    }<br>
+#endif<br>
<br>
-   /* Normally we don't need an element for the SGVS attribute because the<br>
-    * 3DSTATE_VF_SGVS instruction lets you store the generated attribute in an<br>
-    * element that is past the list in 3DSTATE_VERTEX_ELEMENTS. However if<br>
-    * we're using draw parameters then we need an element for the those<br>
-    * values.  Additionally if there is an edge flag element then the SGVS<br>
-    * can't be inserted past that so we need a dummy element to ensure that<br>
-    * the edge flag is the last one.<br>
-    */<br>
-   const bool needs_sgvs_element = (vs_prog_data->uses_basevertex ||<br>
-                                    vs_prog_data->uses_<wbr>baseinstance ||<br>
-                                    ((vs_prog_data->uses_<wbr>instanceid ||<br>
-                                      vs_prog_data->uses_vertexid)<br>
-                                     && uses_edge_flag));<br>
-#else<br>
    const bool needs_sgvs_element = (vs_prog_data->uses_basevertex ||<br>
                                     vs_prog_data->uses_<wbr>baseinstance ||<br>
                                     vs_prog_data->uses_instanceid ||<br>
                                     vs_prog_data->uses_vertexid);<br>
-#endif<br>
+<br>
    unsigned nr_elements =<br>
       brw->vb.nr_enabled + needs_sgvs_element + vs_prog_data->uses_drawid;<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
2.11.0<br>
<br>
</font></span><br></blockquote></div><br></div>
</blockquote></body></html>