Mesa (master): i965: Fix missing BRW_NEW_*_PROG_DATA flagging caused by cache reuse.

Kenneth Graunke kwg at kemper.freedesktop.org
Thu Oct 29 04:20:32 UTC 2015


Module: Mesa
Branch: master
Commit: bf05af3f0e8769f417bbd995470dc1b8083a0df9
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=bf05af3f0e8769f417bbd995470dc1b8083a0df9

Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Wed Oct 28 00:53:20 2015 -0700

i965: Fix missing BRW_NEW_*_PROG_DATA flagging caused by cache reuse.

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" <mesa-stable at lists.freedesktop.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92623
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
Reviewed-by: Kristian Høgsberg <krh at bitplanet.net>
Tested-by: Pierre Bourdon <delroth at gmail.com>

---

 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 dc2b941..6fc9c14 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 7f07bef..f7c0a20 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;




More information about the mesa-commit mailing list