On January 3, 2018 03:21:51 Iago Toral Quiroga <ito...@igalia.com> wrote:
Although in theory this should not be necessary it seems that doing this
fixes a spurious low rate failure of ~1% in a CTS test that seems to happen
in some gen9+ platforms only.
Fixes flakyness in:
KHR-GL45.vertex_attrib_64bit.limits_test
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104335
---
I am not sure why this fixes the spurious fails, but it clearly does, at
least for
me. I have not seen any workarounds related to this in the PRMs so maybe
this is just papering over some other problem in the end. It would be great if
someone else could test this and verify that it fixes the spurious fails
for them
as well and I'd also appreciate any thoughts on what could be causing this
problem
and why this seems to fix it. There is more detail in the bug report.
src/mesa/drivers/dri/i965/genX_state_upload.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
b/src/mesa/drivers/dri/i965/genX_state_upload.c
index 50ac5bc59f..523e9688a6 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -494,12 +494,25 @@ genX(emit_vertices)(struct brw_context *brw)
* values. 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.
+ *
+ * In gen9+, the CTS test KHR-GL45.vertex_attrib_64bit.limits_test fails
+ * with a fail rate of ~1% for the cases where it reads gl_InstanceID
but no
+ * SGVS element is programmed. Programming the SGVS element in this case
+ * seems to make the problem go away.
*/
+#if GEN_GEN >= 9
+ const bool needs_sgvs_element = (vs_prog_data->uses_basevertex ||
+ vs_prog_data->uses_baseinstance ||
+ vs_prog_data->uses_instanceid ||
+ vs_prog_data->uses_vertexid);
+#else
const bool needs_sgvs_element = (vs_prog_data->uses_basevertex ||
vs_prog_data->uses_baseinstance ||
((vs_prog_data->uses_instanceid ||
vs_prog_data->uses_vertexid)
&& uses_edge_flag));
Something smells fishy here. Why are the last to && with uses_edge_flag in
the first place? (That's a rhetorical question; I went and read the code.)
It seems to me as if we want the element even if we're just going to put
instance or vertex id in it. In particular, I'm not sure that emitting
3DSTATE_VERTEX_ELEMENTS actually clears out the elements not specified so
we may have some random vertex element from a previous draw that we're
overwriting. When you combine that with the following two restrictions,
things could get very weird:
It is INVALID to use this command to overwrite any portion of a 64-bit
vertex element component.
It is INVALID to use this command to overwrite a EdgeFlag vertex element
component or any vertex
element beyond it.
I think this patch is probably more-or-less doing the right thing though I
would argue we should probably do it on all gen8+ and we could pull
needs_sgvs_element out of the #if entirely. Good work hunting this down.
--Jason
+#endif
+
#else
const bool needs_sgvs_element = (vs_prog_data->uses_basevertex ||
vs_prog_data->uses_baseinstance ||
--
2.11.0
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
