i965: Move program_id to intel_screen instead of brw_context.

According to bug #54524, I regressed oglconform's multicontext test
when I reenabled the fragment shader precompile.

However, these test cases only passed by miraculous coincedence.  We
assign each fragment program a unique ID (brw_fragment_program::id which
becomes brw_wm_prog_key::program_string_id) which we obtain by storing a
per-context counter.

The test case uses GLX context sharing to access the same fragment
program from two different contexts.  This means that we share a program
cache.  Before the precompile, if both contexts happened to use the same
shaders in the same order, we'd obtain the same program_string_ids (by
virtue of doing the same computation twice).  However, the more likely
scenario is that they completely disagree on program_string_id.

This meant that we'd have two completely different fragment shaders in
the cache with the same ID, tricking us to think they were the same
(aside from NOS), so we'd render using the wrong program.

This patch implements a simple fix suggested by Eric: it moves the
global counter out of brw_context and into intel_screen, which is shared
across all contexts.  A mutex protects it from concurrent access.

This is also the first direct usage of pthreads in the i965 driver.

Fixes 10 subcases of oglconform's multicontext test.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54524
Reviewed-by: Eric Anholt <eric@anholt.net>

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index c7fc5864d80ff34d0459f1ee1fa6c665972a2fe0..f3a3efed0dd89ed5c37994cf04364d0ff29ef904 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1053,9 +1053,6 @@ struct brw_context
       int index;
       bool begin_emitted;
    } query;
-   /* Used to give every program string a unique id
-    */
-   GLuint program_id;
 
    int num_atoms;
    const struct brw_tracked_state **atoms;

diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
index 5bfdcca216b5c975b40d2b58bb227e38ccc981ab..75eb6bc66899056e59e93ffc134ea587cbda2a0d 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -29,6 +29,7 @@
   *   Keith Whitwell <keith@tungstengraphics.com>
   */
   
+#include <pthread.h>
 #include "main/imports.h"
 #include "main/enums.h"
 #include "main/shaderobj.h"
@@ -41,6 +42,16 @@
 #include "brw_context.h"
 #include "brw_wm.h"
 
+static unsigned
+get_new_program_id(struct intel_screen *screen)
+{
+   static pthread_mutex_t m = PTHREAD_MUTEX_INITIALIZER;
+   pthread_mutex_lock(&m);
+   unsigned id = screen->program_id++;
+   pthread_mutex_unlock(&m);
+   return id;
+}
+
 static void brwBindProgram( struct gl_context *ctx,
                            GLenum target, 
                            struct gl_program *prog )
@@ -67,7 +78,7 @@ static struct gl_program *brwNewProgram( struct gl_context *ctx,
    case GL_VERTEX_PROGRAM_ARB: {
       struct brw_vertex_program *prog = CALLOC_STRUCT(brw_vertex_program);
       if (prog) {
-        prog->id = brw->program_id++;
+        prog->id = get_new_program_id(brw->intel.intelScreen);
 
         return _mesa_init_vertex_program( ctx, &prog->program,
                                             target, id );
@@ -79,7 +90,7 @@ static struct gl_program *brwNewProgram( struct gl_context *ctx,
    case GL_FRAGMENT_PROGRAM_ARB: {
       struct brw_fragment_program *prog = CALLOC_STRUCT(brw_fragment_program);
       if (prog) {
-        prog->id = brw->program_id++;
+        prog->id = get_new_program_id(brw->intel.intelScreen);
 
         return _mesa_init_fragment_program( ctx, &prog->program,
                                             target, id );
@@ -123,7 +134,7 @@ brwProgramStringNotify(struct gl_context *ctx,
 
       if (newFP == curFP)
         brw->state.dirty.brw |= BRW_NEW_FRAGMENT_PROGRAM;
-      newFP->id = brw->program_id++;      
+      newFP->id = get_new_program_id(brw->intel.intelScreen);
    }
    else if (target == GL_VERTEX_PROGRAM_ARB) {
       struct gl_vertex_program *vprog = (struct gl_vertex_program *) prog;
@@ -136,7 +147,7 @@ brwProgramStringNotify(struct gl_context *ctx,
       if (newVP->program.IsPositionInvariant) {
         _mesa_insert_mvp_code(ctx, &newVP->program);
       }
-      newVP->id = brw->program_id++;      
+      newVP->id = get_new_program_id(brw->intel.intelScreen);
 
       /* Also tell tnl about it:
        */

diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c
index 798fc6ae884b5922d6e5a78b54a0ec3ff99930f6..37a8e585c6d308029cfcf3581cd561c41915e125 100644 (file)
--- a/src/mesa/drivers/dri/intel/intel_context.c
+++ b/src/mesa/drivers/dri/intel/intel_context.c
@@ -610,6 +610,8 @@ intelInitContext(struct intel_context *intel,
       mesaVis = &visual;
    }
 
+   intel->intelScreen = intelScreen;
+
    if (!_mesa_initialize_context(&intel->ctx, api, mesaVis, shareCtx,
                                  functions)) {
       printf("%s: failed to init mesa context\n", __FUNCTION__);
@@ -617,7 +619,6 @@ intelInitContext(struct intel_context *intel,
    }
 
    driContextPriv->driverPrivate = intel;
-   intel->intelScreen = intelScreen;
    intel->driContext = driContextPriv;
    intel->driFd = sPriv->fd;
 

diff --git a/src/mesa/drivers/dri/intel/intel_screen.h b/src/mesa/drivers/dri/intel/intel_screen.h
index f5a374d2293fcc684fa8a50bf32fc9d82f8172a3..8a4a0a2d98c03064617f2a9434774d71b4381640 100644 (file)
--- a/src/mesa/drivers/dri/intel/intel_screen.h
+++ b/src/mesa/drivers/dri/intel/intel_screen.h
@@ -64,6 +64,11 @@ struct intel_screen
    dri_bufmgr *bufmgr;
    struct _mesa_HashTable *named_regions;
 
+   /**
+    * A unique ID for shader programs.
+    */
+   unsigned program_id;
+
    /**
    * Configuration cache with default values for all contexts
    */