From bf05af3f0e8769f417bbd995470dc1b8083a0df9 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Wed, 28 Oct 2015 00:53:20 -0700 Subject: [PATCH] i965: Fix missing BRW_NEW_*_PROG_DATA flagging caused by cache reuse. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Consider the case of two nearly identical GLSL fragment shaders: out vec4 color; void main() { color = vec4(1); } and layout(early_fragment_tests) in; out vec4 color; void main() { color = vec4(1); } These shaders compile to the exact same assembly, but have distinct values for brw_wm_prog_data::early_fragment_tests. Since these are two independent GLSL shaders, they have different program keys - notably, brw_wm_prog_key::program_string_id differs. When uploading the second, brw_upload_cache will find an existing copy of the assembly in the cache BO, which means matching_data will be non-NULL. Although we create a second cache item (with the new key and prog_data), we set item->offset to the existing copy and avoid re-uploading duplicate assembly. However, brw_search_cache() would only flag BRW_NEW_*_PROG_DATA if item->offset differed from the supplied offset. With reuse, both programs have the same offset, but prog_data changed. We have to flag it, but failed to. To fix this, we simply need to check if the aux (prog_data) pointer changed. If either the assembly or the prog_data differs, flag it. This fixes a regression since 1bba29ed403e735ba0bf04ed8aa2e571884f, where Topi fixed brw_upload_cache() to actually reuse identical assembly. Prior to that, reuse basically never happened due to bugs. Unfortunately, this code apparently wasn't prepared to handle reuse! Fixes GPU hangs in Dolphin on Broadwell. Huge thanks to Pierre Bourdon and Ilia Mirkin for debugging this and helping track down the real issue. Cc: "11.0" Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92623 Signed-off-by: Kenneth Graunke Reviewed-by: Topi Pohjolainen Reviewed-by: Kristian Høgsberg Tested-by: Pierre Bourdon --- src/mesa/drivers/dri/i965/brw_state.h | 2 +- src/mesa/drivers/dri/i965/brw_state_cache.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h index dc2b9415673..6fc9c14f6b4 100644 --- a/src/mesa/drivers/dri/i965/brw_state.h +++ b/src/mesa/drivers/dri/i965/brw_state.h @@ -220,7 +220,7 @@ bool brw_search_cache(struct brw_cache *cache, enum brw_cache_id cache_id, const void *key, GLuint key_size, - uint32_t *inout_offset, void *out_aux); + uint32_t *inout_offset, void *inout_aux); void brw_state_cache_check_size( struct brw_context *brw ); void brw_init_caches( struct brw_context *brw ); diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c b/src/mesa/drivers/dri/i965/brw_state_cache.c index 7f07bef253a..f7c0a2037d9 100644 --- a/src/mesa/drivers/dri/i965/brw_state_cache.c +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c @@ -137,7 +137,7 @@ bool brw_search_cache(struct brw_cache *cache, enum brw_cache_id cache_id, const void *key, GLuint key_size, - uint32_t *inout_offset, void *out_aux) + uint32_t *inout_offset, void *inout_aux) { struct brw_context *brw = cache->brw; struct brw_cache_item *item; @@ -155,11 +155,12 @@ brw_search_cache(struct brw_cache *cache, if (item == NULL) return false; - *(void **)out_aux = ((char *)item->key + item->key_size); + void *aux = ((char *) item->key) + item->key_size; - if (item->offset != *inout_offset) { + if (item->offset != *inout_offset || aux != *((void **) inout_aux)) { brw->ctx.NewDriverState |= (1 << cache_id); *inout_offset = item->offset; + *((void **) inout_aux) = aux; } return true; -- 2.30.2