[Mesa-dev,v2] i965: Perform program state upload outside of atom handling

Submitted by Carl Worth on Feb. 13, 2015, 1:38 a.m.

Details

Message ID 1423791489-1387-1-git-send-email-cworth@cworth.org
State New
Headers show

Not browsing as part of any series.

Commit Message

Carl Worth Feb. 13, 2015, 1:38 a.m.
Across the board of the various generations, the intial few atoms in
all of the atom lists are basically the same, (performing uploads for
the various programs). The only difference is that prior to gen6
there's an ff_gs upload in place of the later gs upload.

In this commit, instead of using the atom lists for this program state
upload, we add a new function brw_upload_programs that performs the
same state checks and calls the various upload functions as necessary.

This commit is intended to have no functional change. The motivation
is that future code, (such as the shader cache), wants to have a
single function within which to perform various operations before and
after program upload, (with some local variables holding state across
the upload).
---

In this revision of the patch, I've changed things so that the lists
of flags remain in the same files they were in before, (near the code
performing the operations that the flags control). So I think this
aspect is a bit cleaner.

I'm still not a huge fan of the resulting conditional expressions,
(and the number of parentheses involved in them). It would not be too
hard to imagine one of those getting edited incorrectly.

So feedback would be most welcome on improving those.

Maybe:

    int dirty = 0;

    dirty |= (brw->state.dirty.mesa & (FLAG | FLAG2 | FLAG3);
    dirty |= (brw->state.dirty.brw & (BRW_FLAG | BRW_FLAG2);

    if (dirty == 0)
       return;

?

 src/mesa/drivers/dri/i965/brw_ff_gs.c        | 21 +++++++------
 src/mesa/drivers/dri/i965/brw_ff_gs.h        |  3 ++
 src/mesa/drivers/dri/i965/brw_gs.c           | 23 ++++++--------
 src/mesa/drivers/dri/i965/brw_gs.h           |  5 ++++
 src/mesa/drivers/dri/i965/brw_state_upload.c | 35 ++++++++++++----------
 src/mesa/drivers/dri/i965/brw_vs.c           | 32 +++++++++-----------
 src/mesa/drivers/dri/i965/brw_vs.h           |  3 ++
 src/mesa/drivers/dri/i965/brw_wm.c           | 45 +++++++++++++---------------
 src/mesa/drivers/dri/i965/brw_wm.h           |  3 ++
 9 files changed, 86 insertions(+), 84 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c b/src/mesa/drivers/dri/i965/brw_ff_gs.c
index 653c4b6..fd68aa3 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
@@ -221,10 +221,19 @@  static void populate_key(struct brw_context *brw,
 
 /* Calculate interpolants for triangle and line rasterization.
  */
-static void
+void
 brw_upload_ff_gs_prog(struct brw_context *brw)
 {
    struct brw_ff_gs_prog_key key;
+
+   if (((brw->state.dirty.mesa & _NEW_LIGHT) |
+	(brw->state.dirty.brw & (BRW_NEW_PRIMITIVE |
+				 BRW_NEW_TRANSFORM_FEEDBACK |
+				 BRW_NEW_VS_PROG_DATA))) == 0)
+   {
+      return;
+   }
+
    /* Populate the key:
     */
    populate_key(brw, &key);
@@ -247,13 +256,3 @@  void gen6_brw_upload_ff_gs_prog(struct brw_context *brw)
 {
    brw_upload_ff_gs_prog(brw);
 }
-
-const struct brw_tracked_state brw_ff_gs_prog = {
-   .dirty = {
-      .mesa  = _NEW_LIGHT,
-      .brw   = BRW_NEW_PRIMITIVE |
-               BRW_NEW_TRANSFORM_FEEDBACK |
-               BRW_NEW_VS_PROG_DATA,
-   },
-   .emit = brw_upload_ff_gs_prog
-};
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.h b/src/mesa/drivers/dri/i965/brw_ff_gs.h
index a538948..e4afdab 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.h
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.h
@@ -112,4 +112,7 @@  void gen6_sol_program(struct brw_ff_gs_compile *c,
                       unsigned num_verts, bool check_edge_flag);
 void gen6_brw_upload_ff_gs_prog(struct brw_context *brw);
 
+void
+brw_upload_ff_gs_prog(struct brw_context *brw);
+
 #endif
diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
index c7ebe5f..0fa06bb 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -292,8 +292,7 @@  do_gs_prog(struct brw_context *brw,
    return true;
 }
 
-
-static void
+void
 brw_upload_gs_prog(struct brw_context *brw)
 {
    struct gl_context *ctx = &brw->ctx;
@@ -303,6 +302,14 @@  brw_upload_gs_prog(struct brw_context *brw)
    struct brw_geometry_program *gp =
       (struct brw_geometry_program *) brw->geometry_program;
 
+   if (((brw->state.dirty.mesa & _NEW_TEXTURE) |
+	(brw->state.dirty.brw & (BRW_NEW_GEOMETRY_PROGRAM |
+				 BRW_NEW_TRANSFORM_FEEDBACK |
+				 BRW_NEW_VUE_MAP_VS))) == 0)
+   {
+      return;
+   }
+
    if (gp == NULL) {
       /* No geometry shader.  Vertex data just passes straight through. */
       if (brw->state.dirty.brw & BRW_NEW_VUE_MAP_VS) {
@@ -358,18 +365,6 @@  brw_upload_gs_prog(struct brw_context *brw)
    }
 }
 
-
-const struct brw_tracked_state brw_gs_prog = {
-   .dirty = {
-      .mesa  = _NEW_TEXTURE,
-      .brw   = BRW_NEW_GEOMETRY_PROGRAM |
-               BRW_NEW_TRANSFORM_FEEDBACK |
-               BRW_NEW_VUE_MAP_VS,
-   },
-   .emit = brw_upload_gs_prog
-};
-
-
 bool
 brw_gs_precompile(struct gl_context *ctx,
                   struct gl_shader_program *shader_prog,
diff --git a/src/mesa/drivers/dri/i965/brw_gs.h b/src/mesa/drivers/dri/i965/brw_gs.h
index 5a15fa9..5f7c437 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.h
+++ b/src/mesa/drivers/dri/i965/brw_gs.h
@@ -26,6 +26,8 @@ 
 
 #include <stdbool.h>
 
+#include "brw_context.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -36,6 +38,9 @@  struct gl_program;
 
 bool brw_gs_prog_data_compare(const void *a, const void *b);
 
+void
+brw_upload_gs_prog(struct brw_context *brw);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 52d96f4..049bdbee 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -36,17 +36,17 @@ 
 #include "drivers/common/meta.h"
 #include "intel_batchbuffer.h"
 #include "intel_buffers.h"
+#include "brw_vs.h"
+#include "brw_ff_gs.h"
+#include "brw_gs.h"
+#include "brw_wm.h"
 
 static const struct brw_tracked_state *gen4_atoms[] =
 {
-   &brw_vs_prog, /* must do before GS prog, state base address. */
-   &brw_ff_gs_prog, /* must do before state base address */
-
    &brw_interpolation_map,
 
    &brw_clip_prog, /* must do before state base address */
    &brw_sf_prog, /* must do before state base address */
-   &brw_wm_prog, /* must do before state base address */
 
    /* Once all the programs are done, we know how large urb entry
     * sizes need to be and can decide if we need to change the urb
@@ -107,10 +107,6 @@  static const struct brw_tracked_state *gen4_atoms[] =
 
 static const struct brw_tracked_state *gen6_atoms[] =
 {
-   &brw_vs_prog, /* must do before state base address */
-   &brw_gs_prog, /* must do before state base address */
-   &brw_wm_prog, /* must do before state base address */
-
    &gen6_clip_vp,
    &gen6_sf_vp,
 
@@ -180,10 +176,6 @@  static const struct brw_tracked_state *gen6_atoms[] =
 
 static const struct brw_tracked_state *gen7_atoms[] =
 {
-   &brw_vs_prog,
-   &brw_gs_prog,
-   &brw_wm_prog,
-
    /* Command packets: */
 
    /* must do before binding table pointers, cc state ptrs */
@@ -256,10 +248,6 @@  static const struct brw_tracked_state *gen7_atoms[] =
 
 static const struct brw_tracked_state *gen8_atoms[] =
 {
-   &brw_vs_prog,
-   &brw_gs_prog,
-   &brw_wm_prog,
-
    /* Command packets: */
    &gen8_state_base_address,
 
@@ -562,6 +550,19 @@  brw_print_dirty_count(struct dirty_bit_map *bit_map)
    }
 }
 
+static void
+brw_upload_programs(struct brw_context *brw)
+{
+   brw_upload_vs_prog(brw);
+
+   if (brw->gen < 6)
+      brw_upload_ff_gs_prog(brw);
+   else
+      brw_upload_gs_prog(brw);
+
+   brw_upload_wm_prog(brw);
+}
+
 /***********************************************************************
  * Emit all state:
  */
@@ -612,6 +613,8 @@  void brw_upload_state(struct brw_context *brw)
    if ((state->mesa | state->brw) == 0)
       return;
 
+   brw_upload_programs(brw);
+
    if (unlikely(INTEL_DEBUG)) {
       /* Debug version which enforces various sanity checks on the
        * state flags which are generated and checked to help ensure
diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
index 2d56b74..a7d2988 100644
--- a/src/mesa/drivers/dri/i965/brw_vs.c
+++ b/src/mesa/drivers/dri/i965/brw_vs.c
@@ -409,8 +409,8 @@  brw_setup_vue_key_clip_info(struct brw_context *brw,
    }
 }
 
-
-static void brw_upload_vs_prog(struct brw_context *brw)
+void
+brw_upload_vs_prog(struct brw_context *brw)
 {
    struct gl_context *ctx = &brw->ctx;
    struct brw_vs_prog_key key;
@@ -420,6 +420,18 @@  static void brw_upload_vs_prog(struct brw_context *brw)
    struct gl_program *prog = (struct gl_program *) brw->vertex_program;
    int i;
 
+   if (((brw->state.dirty.mesa & (_NEW_BUFFERS |
+				 _NEW_LIGHT |
+				 _NEW_POINT |
+				 _NEW_POLYGON |
+				 _NEW_TEXTURE |
+				 _NEW_TRANSFORM)) |
+	(brw->state.dirty.brw & (BRW_NEW_VERTEX_PROGRAM |
+				 BRW_NEW_VS_ATTRIB_WORKAROUNDS))) == 0)
+   {
+      return;
+   }
+
    memset(&key, 0, sizeof(key));
 
    /* Just upload the program verbatim for now.  Always send it all
@@ -482,22 +494,6 @@  static void brw_upload_vs_prog(struct brw_context *brw)
    }
 }
 
-/* See brw_vs.c:
- */
-const struct brw_tracked_state brw_vs_prog = {
-   .dirty = {
-      .mesa  = _NEW_BUFFERS |
-               _NEW_LIGHT |
-               _NEW_POINT |
-               _NEW_POLYGON |
-               _NEW_TEXTURE |
-               _NEW_TRANSFORM,
-      .brw   = BRW_NEW_VERTEX_PROGRAM |
-               BRW_NEW_VS_ATTRIB_WORKAROUNDS,
-   },
-   .emit = brw_upload_vs_prog
-};
-
 bool
 brw_vs_precompile(struct gl_context *ctx,
                   struct gl_shader_program *shader_prog,
diff --git a/src/mesa/drivers/dri/i965/brw_vs.h b/src/mesa/drivers/dri/i965/brw_vs.h
index 93c5389..bad0f07 100644
--- a/src/mesa/drivers/dri/i965/brw_vs.h
+++ b/src/mesa/drivers/dri/i965/brw_vs.h
@@ -72,6 +72,9 @@  void brw_vs_debug_recompile(struct brw_context *brw,
                             const struct brw_vs_prog_key *key);
 bool brw_vs_prog_data_compare(const void *a, const void *b);
 
+void
+brw_upload_vs_prog(struct brw_context *brw);
+
 #ifdef __cplusplus
 } /* extern "C" */
 
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
index e7939f0..eace7f9 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -582,8 +582,7 @@  static void brw_wm_populate_key( struct brw_context *brw,
    key->program_string_id = fp->id;
 }
 
-
-static void
+void
 brw_upload_wm_prog(struct brw_context *brw)
 {
    struct gl_context *ctx = &brw->ctx;
@@ -591,6 +590,25 @@  brw_upload_wm_prog(struct brw_context *brw)
    struct brw_fragment_program *fp = (struct brw_fragment_program *)
       brw->fragment_program;
 
+   if (((brw->state.dirty.mesa & (_NEW_BUFFERS |
+				  _NEW_COLOR |
+				  _NEW_DEPTH |
+				  _NEW_FRAG_CLAMP |
+				  _NEW_HINT |
+				  _NEW_LIGHT |
+				  _NEW_LINE |
+				  _NEW_MULTISAMPLE |
+				  _NEW_POLYGON |
+				  _NEW_STENCIL |
+				  _NEW_TEXTURE)) |
+	(brw->state.dirty.brw & (BRW_NEW_FRAGMENT_PROGRAM |
+				 BRW_NEW_REDUCED_PRIMITIVE |
+				 BRW_NEW_STATS_WM |
+				 BRW_NEW_VUE_MAP_GEOM_OUT))) == 0)
+   {
+      return;
+   }
+
    brw_wm_populate_key(brw, &key);
 
    if (!brw_search_cache(&brw->cache, BRW_CACHE_FS_PROG,
@@ -603,26 +621,3 @@  brw_upload_wm_prog(struct brw_context *brw)
    }
    brw->wm.base.prog_data = &brw->wm.prog_data->base;
 }
-
-
-const struct brw_tracked_state brw_wm_prog = {
-   .dirty = {
-      .mesa  = _NEW_BUFFERS |
-               _NEW_COLOR |
-               _NEW_DEPTH |
-               _NEW_FRAG_CLAMP |
-               _NEW_HINT |
-               _NEW_LIGHT |
-               _NEW_LINE |
-               _NEW_MULTISAMPLE |
-               _NEW_POLYGON |
-               _NEW_STENCIL |
-               _NEW_TEXTURE,
-      .brw   = BRW_NEW_FRAGMENT_PROGRAM |
-               BRW_NEW_REDUCED_PRIMITIVE |
-               BRW_NEW_STATS_WM |
-               BRW_NEW_VUE_MAP_GEOM_OUT,
-   },
-   .emit = brw_upload_wm_prog
-};
-
diff --git a/src/mesa/drivers/dri/i965/brw_wm.h b/src/mesa/drivers/dri/i965/brw_wm.h
index a12c7d4..f54530f 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.h
+++ b/src/mesa/drivers/dri/i965/brw_wm.h
@@ -83,4 +83,7 @@  void brw_wm_debug_recompile(struct brw_context *brw,
                             const struct brw_wm_prog_key *key);
 bool brw_wm_prog_data_compare(const void *a, const void *b);
 
+void
+brw_upload_wm_prog(struct brw_context *brw);
+
 #endif

Comments

On Thursday, February 12, 2015 05:38:09 PM Carl Worth wrote:
> Across the board of the various generations, the intial few atoms in
> all of the atom lists are basically the same, (performing uploads for
> the various programs). The only difference is that prior to gen6
> there's an ff_gs upload in place of the later gs upload.
> 
> In this commit, instead of using the atom lists for this program state
> upload, we add a new function brw_upload_programs that performs the
> same state checks and calls the various upload functions as necessary.
> 
> This commit is intended to have no functional change. The motivation
> is that future code, (such as the shader cache), wants to have a
> single function within which to perform various operations before and
> after program upload, (with some local variables holding state across
> the upload).
> ---
> 
> In this revision of the patch, I've changed things so that the lists
> of flags remain in the same files they were in before, (near the code
> performing the operations that the flags control). So I think this
> aspect is a bit cleaner.
> 
> I'm still not a huge fan of the resulting conditional expressions,
> (and the number of parentheses involved in them). It would not be too
> hard to imagine one of those getting edited incorrectly.
> 
> So feedback would be most welcome on improving those.
> 
> Maybe:
> 
>     int dirty = 0;
> 
>     dirty |= (brw->state.dirty.mesa & (FLAG | FLAG2 | FLAG3);
>     dirty |= (brw->state.dirty.brw & (BRW_FLAG | BRW_FLAG2);
> 
>     if (dirty == 0)
>        return;
> 
> ?
> 
>  src/mesa/drivers/dri/i965/brw_ff_gs.c        | 21 +++++++------
>  src/mesa/drivers/dri/i965/brw_ff_gs.h        |  3 ++
>  src/mesa/drivers/dri/i965/brw_gs.c           | 23 ++++++--------
>  src/mesa/drivers/dri/i965/brw_gs.h           |  5 ++++
>  src/mesa/drivers/dri/i965/brw_state_upload.c | 35 ++++++++++++----------
>  src/mesa/drivers/dri/i965/brw_vs.c           | 32 +++++++++-----------
>  src/mesa/drivers/dri/i965/brw_vs.h           |  3 ++
>  src/mesa/drivers/dri/i965/brw_wm.c           | 45 +++++++++++++---------------
>  src/mesa/drivers/dri/i965/brw_wm.h           |  3 ++
>  9 files changed, 86 insertions(+), 84 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c b/src/mesa/drivers/dri/i965/brw_ff_gs.c
> index 653c4b6..fd68aa3 100644
> --- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
> @@ -221,10 +221,19 @@ static void populate_key(struct brw_context *brw,
>  
>  /* Calculate interpolants for triangle and line rasterization.
>   */
> -static void
> +void
>  brw_upload_ff_gs_prog(struct brw_context *brw)
>  {
>     struct brw_ff_gs_prog_key key;
> +
> +   if (((brw->state.dirty.mesa & _NEW_LIGHT) |
> +        (brw->state.dirty.brw & (BRW_NEW_PRIMITIVE |
> +                                 BRW_NEW_TRANSFORM_FEEDBACK |
> +                                 BRW_NEW_VS_PROG_DATA))) == 0)
> +   {
> +      return;
> +   }
> +
>     /* Populate the key:
>      */
>     populate_key(brw, &key);
> @@ -247,13 +256,3 @@ void gen6_brw_upload_ff_gs_prog(struct brw_context *brw)
>  {
>     brw_upload_ff_gs_prog(brw);
>  }
> -
> -const struct brw_tracked_state brw_ff_gs_prog = {
> -   .dirty = {
> -      .mesa  = _NEW_LIGHT,
> -      .brw   = BRW_NEW_PRIMITIVE |
> -               BRW_NEW_TRANSFORM_FEEDBACK |
> -               BRW_NEW_VS_PROG_DATA,
> -   },
> -   .emit = brw_upload_ff_gs_prog
> -};
> diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.h b/src/mesa/drivers/dri/i965/brw_ff_gs.h
> index a538948..e4afdab 100644
> --- a/src/mesa/drivers/dri/i965/brw_ff_gs.h
> +++ b/src/mesa/drivers/dri/i965/brw_ff_gs.h
> @@ -112,4 +112,7 @@ void gen6_sol_program(struct brw_ff_gs_compile *c,
>                        unsigned num_verts, bool check_edge_flag);
>  void gen6_brw_upload_ff_gs_prog(struct brw_context *brw);
>  
> +void
> +brw_upload_ff_gs_prog(struct brw_context *brw);
> +
>  #endif
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
> index c7ebe5f..0fa06bb 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -292,8 +292,7 @@ do_gs_prog(struct brw_context *brw,
>     return true;
>  }
>  
> -
> -static void
> +void
>  brw_upload_gs_prog(struct brw_context *brw)
>  {
>     struct gl_context *ctx = &brw->ctx;
> @@ -303,6 +302,14 @@ brw_upload_gs_prog(struct brw_context *brw)
>     struct brw_geometry_program *gp =
>        (struct brw_geometry_program *) brw->geometry_program;
>  
> +   if (((brw->state.dirty.mesa & _NEW_TEXTURE) |
> +        (brw->state.dirty.brw & (BRW_NEW_GEOMETRY_PROGRAM |
> +                                 BRW_NEW_TRANSFORM_FEEDBACK |
> +                                 BRW_NEW_VUE_MAP_VS))) == 0)
> +   {
> +      return;
> +   }
> +
>     if (gp == NULL) {
>        /* No geometry shader.  Vertex data just passes straight through. */
>        if (brw->state.dirty.brw & BRW_NEW_VUE_MAP_VS) {
> @@ -358,18 +365,6 @@ brw_upload_gs_prog(struct brw_context *brw)
>     }
>  }
>  
> -
> -const struct brw_tracked_state brw_gs_prog = {
> -   .dirty = {
> -      .mesa  = _NEW_TEXTURE,
> -      .brw   = BRW_NEW_GEOMETRY_PROGRAM |
> -               BRW_NEW_TRANSFORM_FEEDBACK |
> -               BRW_NEW_VUE_MAP_VS,
> -   },
> -   .emit = brw_upload_gs_prog
> -};
> -
> -
>  bool
>  brw_gs_precompile(struct gl_context *ctx,
>                    struct gl_shader_program *shader_prog,
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.h b/src/mesa/drivers/dri/i965/brw_gs.h
> index 5a15fa9..5f7c437 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.h
> +++ b/src/mesa/drivers/dri/i965/brw_gs.h
> @@ -26,6 +26,8 @@
>  
>  #include <stdbool.h>
>  
> +#include "brw_context.h"
> +
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
> @@ -36,6 +38,9 @@ struct gl_program;
>  
>  bool brw_gs_prog_data_compare(const void *a, const void *b);
>  
> +void
> +brw_upload_gs_prog(struct brw_context *brw);
> +
>  #ifdef __cplusplus
>  } /* extern "C" */
>  #endif
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index 52d96f4..049bdbee 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -36,17 +36,17 @@
>  #include "drivers/common/meta.h"
>  #include "intel_batchbuffer.h"
>  #include "intel_buffers.h"
> +#include "brw_vs.h"
> +#include "brw_ff_gs.h"
> +#include "brw_gs.h"
> +#include "brw_wm.h"
>  
>  static const struct brw_tracked_state *gen4_atoms[] =
>  {
> -   &brw_vs_prog, /* must do before GS prog, state base address. */
> -   &brw_ff_gs_prog, /* must do before state base address */
> -
>     &brw_interpolation_map,
>  
>     &brw_clip_prog, /* must do before state base address */
>     &brw_sf_prog, /* must do before state base address */
> -   &brw_wm_prog, /* must do before state base address */
>  
>     /* Once all the programs are done, we know how large urb entry
>      * sizes need to be and can decide if we need to change the urb
> @@ -107,10 +107,6 @@ static const struct brw_tracked_state *gen4_atoms[] =
>  
>  static const struct brw_tracked_state *gen6_atoms[] =
>  {
> -   &brw_vs_prog, /* must do before state base address */
> -   &brw_gs_prog, /* must do before state base address */
> -   &brw_wm_prog, /* must do before state base address */
> -
>     &gen6_clip_vp,
>     &gen6_sf_vp,
>  
> @@ -180,10 +176,6 @@ static const struct brw_tracked_state *gen6_atoms[] =
>  
>  static const struct brw_tracked_state *gen7_atoms[] =
>  {
> -   &brw_vs_prog,
> -   &brw_gs_prog,
> -   &brw_wm_prog,
> -
>     /* Command packets: */
>  
>     /* must do before binding table pointers, cc state ptrs */
> @@ -256,10 +248,6 @@ static const struct brw_tracked_state *gen7_atoms[] =
>  
>  static const struct brw_tracked_state *gen8_atoms[] =
>  {
> -   &brw_vs_prog,
> -   &brw_gs_prog,
> -   &brw_wm_prog,
> -
>     /* Command packets: */
>     &gen8_state_base_address,
>  
> @@ -562,6 +550,19 @@ brw_print_dirty_count(struct dirty_bit_map *bit_map)
>     }
>  }
>  
> +static void
> +brw_upload_programs(struct brw_context *brw)
> +{
> +   brw_upload_vs_prog(brw);
> +
> +   if (brw->gen < 6)
> +      brw_upload_ff_gs_prog(brw);
> +   else
> +      brw_upload_gs_prog(brw);
> +
> +   brw_upload_wm_prog(brw);
> +}
> +
>  /***********************************************************************
>   * Emit all state:
>   */
> @@ -612,6 +613,8 @@ void brw_upload_state(struct brw_context *brw)
>     if ((state->mesa | state->brw) == 0)
>        return;
>  
> +   brw_upload_programs(brw);
> +
>     if (unlikely(INTEL_DEBUG)) {
>        /* Debug version which enforces various sanity checks on the
>         * state flags which are generated and checked to help ensure
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index 2d56b74..a7d2988 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -409,8 +409,8 @@ brw_setup_vue_key_clip_info(struct brw_context *brw,
>     }
>  }
>  
> -
> -static void brw_upload_vs_prog(struct brw_context *brw)
> +void
> +brw_upload_vs_prog(struct brw_context *brw)
>  {
>     struct gl_context *ctx = &brw->ctx;
>     struct brw_vs_prog_key key;
> @@ -420,6 +420,18 @@ static void brw_upload_vs_prog(struct brw_context *brw)
>     struct gl_program *prog = (struct gl_program *) brw->vertex_program;
>     int i;
>  
> +   if (((brw->state.dirty.mesa & (_NEW_BUFFERS |
> +                                 _NEW_LIGHT |
> +                                 _NEW_POINT |
> +                                 _NEW_POLYGON |
> +                                 _NEW_TEXTURE |
> +                                 _NEW_TRANSFORM)) |
> +        (brw->state.dirty.brw & (BRW_NEW_VERTEX_PROGRAM |
> +                                 BRW_NEW_VS_ATTRIB_WORKAROUNDS))) == 0)
> +   {
> +      return;
> +   }
> +
>     memset(&key, 0, sizeof(key));
>  
>     /* Just upload the program verbatim for now.  Always send it all
> @@ -482,22 +494,6 @@ static void brw_upload_vs_prog(struct brw_context *brw)
>     }
>  }
>  
> -/* See brw_vs.c:
> - */
> -const struct brw_tracked_state brw_vs_prog = {
> -   .dirty = {
> -      .mesa  = _NEW_BUFFERS |
> -               _NEW_LIGHT |
> -               _NEW_POINT |
> -               _NEW_POLYGON |
> -               _NEW_TEXTURE |
> -               _NEW_TRANSFORM,
> -      .brw   = BRW_NEW_VERTEX_PROGRAM |
> -               BRW_NEW_VS_ATTRIB_WORKAROUNDS,
> -   },
> -   .emit = brw_upload_vs_prog
> -};
> -
>  bool
>  brw_vs_precompile(struct gl_context *ctx,
>                    struct gl_shader_program *shader_prog,
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.h b/src/mesa/drivers/dri/i965/brw_vs.h
> index 93c5389..bad0f07 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.h
> +++ b/src/mesa/drivers/dri/i965/brw_vs.h
> @@ -72,6 +72,9 @@ void brw_vs_debug_recompile(struct brw_context *brw,
>                              const struct brw_vs_prog_key *key);
>  bool brw_vs_prog_data_compare(const void *a, const void *b);
>  
> +void
> +brw_upload_vs_prog(struct brw_context *brw);
> +
>  #ifdef __cplusplus
>  } /* extern "C" */
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index e7939f0..eace7f9 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -582,8 +582,7 @@ static void brw_wm_populate_key( struct brw_context *brw,
>     key->program_string_id = fp->id;
>  }
>  
> -
> -static void
> +void
>  brw_upload_wm_prog(struct brw_context *brw)
>  {
>     struct gl_context *ctx = &brw->ctx;
> @@ -591,6 +590,25 @@ brw_upload_wm_prog(struct brw_context *brw)
>     struct brw_fragment_program *fp = (struct brw_fragment_program *)
>        brw->fragment_program;
>  

What about putting this in brw_state.h:

static inline
state_dirty(struct brw_context *brw, uint32_t mesa_bits, uint64_t brw_bits)
{
   return (brw->state.dirty.mesa & mesa_bits) |
          (brw->state.dirty.brw & brw_bits);
}

and then writing this as:

   if (!state_dirty(brw,
                    _NEW_COLOR |
                    _NEW_DEPTH |
                    _NEW_FRAG_CLAMP |
                    _NEW_HINT |
                    _NEW_LIGHT |
                    _NEW_LINE |
                    _NEW_MULTISAMPLE |
                    _NEW_POLYGON |
                    _NEW_STENCIL |
                    _NEW_TEXTURE,
                    BRW_NEW_FRAGMENT_PROGRAM |
                    BRW_NEW_REDUCED_PRIMITIVE |
                    BRW_NEW_STATS_WM |
                    BRW_NEW_VUE_MAP_GEOM_OUT))
      return;

I definitely like how the dirty flag list remains close to the code that
relies on them - in fact, it's even closer now.

With that suggested change, this patch is:
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

I could potentially see us moving to this approach in general - instead
of a gen7_atoms list, we could just have a gen7_upload_state() function
that calls everything directly, rather than via a function pointer list.
This could be more efficient, and writing a function directly might give
us a bit more flexibility to pass data between pieces without having to
store everything in brw_context awkwardly.

I'm not suggesting you do extra work - just saying, I like this :)

Jordan may have thoughts as well.

> +   if (((brw->state.dirty.mesa & (_NEW_BUFFERS |
> +                                  _NEW_COLOR |
> +                                  _NEW_DEPTH |
> +                                  _NEW_FRAG_CLAMP |
> +                                  _NEW_HINT |
> +                                  _NEW_LIGHT |
> +                                  _NEW_LINE |
> +                                  _NEW_MULTISAMPLE |
> +                                  _NEW_POLYGON |
> +                                  _NEW_STENCIL |
> +                                  _NEW_TEXTURE)) |
> +        (brw->state.dirty.brw & (BRW_NEW_FRAGMENT_PROGRAM |
> +                                 BRW_NEW_REDUCED_PRIMITIVE |
> +                                 BRW_NEW_STATS_WM |
> +                                 BRW_NEW_VUE_MAP_GEOM_OUT))) == 0)
> +   {
> +      return;
> +   }
> +
>     brw_wm_populate_key(brw, &key);
>  
>     if (!brw_search_cache(&brw->cache, BRW_CACHE_FS_PROG,
> @@ -603,26 +621,3 @@ brw_upload_wm_prog(struct brw_context *brw)
>     }
>     brw->wm.base.prog_data = &brw->wm.prog_data->base;
>  }
> -
> -
> -const struct brw_tracked_state brw_wm_prog = {
> -   .dirty = {
> -      .mesa  = _NEW_BUFFERS |
> -               _NEW_COLOR |
> -               _NEW_DEPTH |
> -               _NEW_FRAG_CLAMP |
> -               _NEW_HINT |
> -               _NEW_LIGHT |
> -               _NEW_LINE |
> -               _NEW_MULTISAMPLE |
> -               _NEW_POLYGON |
> -               _NEW_STENCIL |
> -               _NEW_TEXTURE,
> -      .brw   = BRW_NEW_FRAGMENT_PROGRAM |
> -               BRW_NEW_REDUCED_PRIMITIVE |
> -               BRW_NEW_STATS_WM |
> -               BRW_NEW_VUE_MAP_GEOM_OUT,
> -   },
> -   .emit = brw_upload_wm_prog
> -};
> -
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.h b/src/mesa/drivers/dri/i965/brw_wm.h
> index a12c7d4..f54530f 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.h
> +++ b/src/mesa/drivers/dri/i965/brw_wm.h
> @@ -83,4 +83,7 @@ void brw_wm_debug_recompile(struct brw_context *brw,
>                              const struct brw_wm_prog_key *key);
>  bool brw_wm_prog_data_compare(const void *a, const void *b);
>  
> +void
> +brw_upload_wm_prog(struct brw_context *brw);
> +
>  #endif
>