From 62497d48609e6ce3d06bf0ca6b6d5ea25939d06f Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Mon, 13 Mar 2023 23:16:43 -0400 Subject: [PATCH] util/prim_convert: Don't set index_bounds_valid draw->index_bounds_valid tells drivers that the values of min_index/max_index are set correctly and can be used e.g. to allocate memory for varyings. If set incorrectly, the GL promises badness. But, with primconvert, we go mucking with index buffers and then never update the bounds. So it doesn't matter if the original index bounds were valid, we can't promise the original bounds are *still* valid. If we were trying to optimize CPU overhead, we could try to preserve the new min/max index but seeing as only older Mali cares about this flag, and if you're using primconvert you're already screwed, I'm not too inclined to go rework primconvert. Fixes* page faults in primitive-restart-draw-mode on Mali-G52 for GL_QUAD_STRIPS and GL_POLYGON, which hit the primconvert path. The full dmesg splat looks like: [ 5438.811727] panfrost ffe40000.gpu: Unhandled Page fault in AS0 at VA 0x000000100A16BAC0 Reason: TODO raw fault status: 0x25002C1 decoded fault status: SLAVE FAULT exception type 0xC1: TRANSLATION_FAULT_1 access type 0x2: READ source id 0x250 Notice that a high bit is randomly set in the address, this is trying to read a varying from the actual varying buffer in the vicinity of 0xa16bac0. What's actually happening is that we're trying to read index #0 despite promising the driver a minimum index of 2, causing an integer underflow as we try to read index -2, or as the hardware sees, 4294967294. As long as we stop lying to panfrost about the bounds being correct, panfrost is able to calculate the real (post-primconverted) bounds on its own, fixing the test. * Alternatively, maybe Panfrost should just ignore this bit, in which I don't know why we have it in Gallium, since it's probably not conformant to fault on out-of-range glDrawRangeElements. Fixes: 72ff53098c6 ("gallium: add pipe_draw_info::index_bounds_valid") Signed-off-by: Alyssa Rosenzweig Reviewed-by: Mike Blumenkrantz Reviewed-by: Emma Anholt Part-of: --- src/gallium/auxiliary/indices/u_primconvert.c | 12 +++++++++--- src/panfrost/ci/panfrost-g52-fails.txt | 2 -- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/gallium/auxiliary/indices/u_primconvert.c b/src/gallium/auxiliary/indices/u_primconvert.c index dcbc905..526e7b6 100644 --- a/src/gallium/auxiliary/indices/u_primconvert.c +++ b/src/gallium/auxiliary/indices/u_primconvert.c @@ -130,9 +130,15 @@ primconvert_init_draw(struct primconvert_context *pc, return false; util_draw_init_info(new_info); - new_info->index_bounds_valid = info->index_bounds_valid; - new_info->min_index = info->min_index; - new_info->max_index = info->max_index; + + /* Because we've changed the index buffer, the original min_index/max_index + * for the draw are no longer valid. That's ok, but we need to tell drivers + * so they don't optimize incorrectly. + */ + new_info->index_bounds_valid = false; + new_info->min_index = 0; + new_info->max_index = ~0; + new_info->start_instance = info->start_instance; new_info->instance_count = info->instance_count; new_info->primitive_restart = info->primitive_restart; diff --git a/src/panfrost/ci/panfrost-g52-fails.txt b/src/panfrost/ci/panfrost-g52-fails.txt index 9bf8ccc..c4144a2 100644 --- a/src/panfrost/ci/panfrost-g52-fails.txt +++ b/src/panfrost/ci/panfrost-g52-fails.txt @@ -495,8 +495,6 @@ spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA32I/Destinat spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA32UI/Destination: GL_RGBA32UI,Fail spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA8/Destination: GL_RGBA8,Fail spec@nv_copy_image@nv_copy_image-formats --samples=4@Source: GL_RGBA8_SNORM/Destination: GL_RGBA8_SNORM,Fail -spec@nv_primitive_restart@primitive-restart-draw-mode-polygon,Crash -spec@nv_primitive_restart@primitive-restart-draw-mode-quad_strip,Crash spec@oes_texture_view@rendering-formats,Crash spec@oes_texture_view@sampling-2d-array-as-cubemap,Crash spec@!opengl 1.0@gl-1.0-edgeflag-const,Fail -- 2.7.4