Re: [Mesa-dev] [PATCH 10/13] i965: Clean up code for VS pull constant surface creation.

[Eric Anholt]

On Tue,  8 Nov 2011 14:32:09 -0800, Kenneth Graunke  
wrote:
> Like for the WM pull constants, we can merge the former prepare/emit
> stages into one tracked state atom.  Furthermore, the code that used to
> handle the binding table was removed in the last commit, leaving some
> rather silly looking short functions that can easily be folded in.
> 
> Signed-off-by: Kenneth Graunke 

> -   &brw_vs_constants, /* Before vs_surfaces and constant_buffer */
> -   &brw_wm_pull_constants,  /* Before brw_wm_binding_table */
> +   /* Pull constants must be before brw_binding_table */
> +   &brw_vs_pull_constants,
> +   &brw_wm_pull_constants,
> +
> &gen6_vs_push_constants, /* Before vs_state */
> &gen6_wm_push_constants, /* Before wm_state */
>  
> -   &brw_vs_surfaces, /* must do before unit */
> &brw_renderbuffer_surfaces,  /* must do before unit */
> &brw_texture_surfaces,   /* must do before unit */
> &brw_binding_table,

I think the pull constants atoms should live down with the rest of the
surfaces.

> -/**
> - * Update the surface state for a VS constant buffer.
> - *
> - * Sets brw->vs.surf_bo[surf] and brw->vp->const_buffer.
> - */
> -static void
> -brw_update_vs_constant_surface( struct gl_context *ctx,
> -GLuint surf)
> -{
> -   struct brw_context *brw = brw_context(ctx);
> -   struct intel_context *intel = &brw->intel;
> -   struct brw_vertex_program *vp =
> -  (struct brw_vertex_program *) brw->vertex_program;
> -   const struct gl_program_parameter_list *params = 
> vp->program.Base.Parameters;
> -
> -   /* If there's no constant buffer, then no surface BO is needed to point at
> -* it.
> -*/
> -   if (brw->vs.const_bo == NULL) {
> -  brw->bind.surf_offset[surf] = 0;
> -  return;
> -   }
>  
> +   const int surf = SURF_INDEX_VERT_CONST_BUFFER;
> intel->vtbl.create_constant_surface(brw, brw->vs.const_bo,
>  params->NumParameters,
>  &brw->bind.surf_offset[surf]);

> -const struct brw_tracked_state brw_vs_surfaces = {
> +const struct brw_tracked_state brw_vs_pull_constants = {
> .dirty = {
> -  .mesa = 0,
> -  .brw = (BRW_NEW_VS_CONSTBUF |
> -   BRW_NEW_BATCH),
> -  .cache = 0
> +  .mesa = (_NEW_PROGRAM_CONSTANTS),
> +  .brw = (BRW_NEW_VERTEX_PROGRAM),
> +  .cache = CACHE_NEW_VS_PROG,
> },
> -   .emit = brw_upload_vs_surfaces,
> +   .emit = brw_upload_vs_pull_constants,
>  };

Looks like BRW_NEW_BATCH got misplaced, which is required since you're
uploading state batch data (create_constant_surface()).

Hmm.  You know, I ought to add some debug code that checks that
!bo_references(intel->batch.bo, old_batch_bo).


pgpm732kX1QIo.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 10/13] i965: Clean up code for VS pull constant surface creation.

[Kenneth Graunke]

Like for the WM pull constants, we can merge the former prepare/emit
stages into one tracked state atom.  Furthermore, the code that used to
handle the binding table was removed in the last commit, leaving some
rather silly looking short functions that can easily be folded in.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_state.h|3 +-
 src/mesa/drivers/dri/i965/brw_state_upload.c |   20 ---
 src/mesa/drivers/dri/i965/brw_vs_surface_state.c |   61 +++---
 3 files changed, 20 insertions(+), 64 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_state.h 
b/src/mesa/drivers/dri/i965/brw_state.h
index c273996..44f5fe1 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -41,7 +41,7 @@ extern const struct brw_tracked_state brw_cc_unit;
 extern const struct brw_tracked_state brw_check_fallback;
 extern const struct brw_tracked_state brw_clip_prog;
 extern const struct brw_tracked_state brw_clip_unit;
-extern const struct brw_tracked_state brw_vs_constants;
+extern const struct brw_tracked_state brw_vs_pull_constants;
 extern const struct brw_tracked_state brw_wm_pull_constants;
 extern const struct brw_tracked_state brw_constant_buffer;
 extern const struct brw_tracked_state brw_curbe_offsets;
@@ -63,7 +63,6 @@ extern const struct brw_tracked_state brw_sf_vp;
 extern const struct brw_tracked_state brw_state_base_address;
 extern const struct brw_tracked_state brw_urb_fence;
 extern const struct brw_tracked_state brw_vertex_state;
-extern const struct brw_tracked_state brw_vs_surfaces;
 extern const struct brw_tracked_state brw_vs_prog;
 extern const struct brw_tracked_state brw_vs_unit;
 extern const struct brw_tracked_state brw_wm_input_sizes;
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index c94b0eb..fe4aed0 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -63,10 +63,10 @@ static const struct brw_tracked_state *gen4_atoms[] =
&brw_cc_vp,
&brw_cc_unit,
 
-   &brw_vs_constants, /* Before vs_surfaces and constant_buffer */
-   &brw_wm_pull_constants, /* Before brw_wm_binding_table */
+   /* Must be before brw_binding_table */
+   &brw_vs_pull_constants,
+   &brw_wm_pull_constants,
 
-   &brw_vs_surfaces,   /* must do before unit */
&brw_renderbuffer_surfaces,  /* must do before unit */
&brw_texture_surfaces,   /* must do before unit */
&brw_binding_table,
@@ -133,12 +133,13 @@ static const struct brw_tracked_state *gen6_atoms[] =
&gen6_depth_stencil_state,  /* must do before cc unit */
&gen6_cc_state_pointers,
 
-   &brw_vs_constants, /* Before vs_surfaces and constant_buffer */
-   &brw_wm_pull_constants,  /* Before brw_wm_binding_table */
+   /* Pull constants must be before brw_binding_table */
+   &brw_vs_pull_constants,
+   &brw_wm_pull_constants,
+
&gen6_vs_push_constants, /* Before vs_state */
&gen6_wm_push_constants, /* Before wm_state */
 
-   &brw_vs_surfaces,   /* must do before unit */
&brw_renderbuffer_surfaces,  /* must do before unit */
&brw_texture_surfaces,   /* must do before unit */
&brw_binding_table,
@@ -198,12 +199,13 @@ const struct brw_tracked_state *gen7_atoms[] =
&gen7_cc_state_pointer,
&gen7_depth_stencil_state_pointer,
 
-   &brw_vs_constants, /* Before vs_surfaces and constant_buffer */
-   &brw_wm_pull_constants,  /* Before brw_wm_binding_table */
+   /* Pull constants must be before brw_binding_table */
+   &brw_vs_pull_constants,
+   &brw_wm_pull_constants,
+
&gen6_vs_push_constants, /* Before vs_state */
&gen6_wm_push_constants, /* Before wm_surfaces and constant_buffer */
 
-   &brw_vs_surfaces,   /* must do before unit */
&brw_renderbuffer_surfaces,  /* must do before unit */
&brw_texture_surfaces,   /* must do before unit */
&brw_binding_table,
diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
index 66d5545..77db28f 100644
--- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
@@ -65,6 +65,7 @@ brw_upload_vs_pull_constants(struct brw_context *brw)
   if (brw->vs.const_bo) {
 drm_intel_bo_unreference(brw->vs.const_bo);
 brw->vs.const_bo = NULL;
+brw->bind.surf_offset[SURF_INDEX_VERT_CONST_BUFFER] = 0;
 brw->state.dirty.brw |= BRW_NEW_VS_CONSTBUF;
   }
   return;
@@ -92,66 +93,20 @@ brw_upload_vs_pull_constants(struct brw_context *brw)
}
 
drm_intel_gem_bo_unmap_gtt(brw->vs.const_bo);
-   brw->state.dirty.brw |= BRW_NEW_VS_CONSTBUF;
-}
-
-const struct brw_tracked_state brw_vs_constants = {
-   .dirty = {
-  .mesa = (_NEW_PROGRAM_CONSTANTS),
-  .brw = (BRW_NEW_VERTEX_PROGRAM),
-  .cache = CACHE_NEW_VS_PROG,
-   },
-   .emit = brw_upload_vs_pull_constants,
-};