[Mesa-dev] [PATCH 14/33] i965/gen4: Fold WM surface state prepare()/emit() together.

[Paul Berry]

On 24 October 2011 14:17, Eric Anholt <eric at anholt.net> wrote:

> These produce BRW_NEW_SURFACES (used by binding table emit()) and
> BRW_NEW_NR_WM_SURFACES (used by WM unit emit()).  Fixes a bug where
> with no texturing and no color buffer, we wouldn't consider the null
> renderbuffer in nr_surfaces.  This was harmless because nr_surfaces is
> only used for the prefetch info in the unit state.
> ---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |   54
> +++++++--------------
>  1 files changed, 18 insertions(+), 36 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index bb7fd2e..782efd5 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -541,47 +541,16 @@ brw_update_renderbuffer_surface(struct brw_context
> *brw,
>                           I915_GEM_DOMAIN_RENDER);
>  }
>
> -static void
> -prepare_wm_surfaces(struct brw_context *brw)
> -{
> -   struct gl_context *ctx = &brw->intel.ctx;
> -   int i;
> -   int nr_surfaces = 0;
> -
> -   for (i = 0; i < ctx->DrawBuffer->_NumColorDrawBuffers; i++) {
> -      nr_surfaces = SURF_INDEX_DRAW(i) + 1;
> -   }
> -
> -   if (brw->wm.const_bo) {
> -      nr_surfaces = SURF_INDEX_FRAG_CONST_BUFFER + 1;
> -   }
> -
> -   for (i = 0; i < BRW_MAX_TEX_UNIT; i++) {
> -      const struct gl_texture_unit *texUnit = &ctx->Texture.Unit[i];
> -
> -      if (texUnit->_ReallyEnabled) {
> -        nr_surfaces = SURF_INDEX_TEXTURE(i) + 1;
> -      }
> -   }
> -
> -   /* Have to update this in our prepare, since the unit's prepare
> -    * relies on it.
> -    */
> -   if (brw->wm.nr_surfaces != nr_surfaces) {
> -      brw->wm.nr_surfaces = nr_surfaces;
> -      brw->state.dirty.brw |= BRW_NEW_NR_WM_SURFACES;
> -   }
> -}
> -
>  /**
>  * Constructs the set of surface state objects pointed to by the
>  * binding table.
>  */
>  static void
> -upload_wm_surfaces(struct brw_context *brw)
> +brw_upload_wm_surfaces(struct brw_context *brw)
>  {
>    struct gl_context *ctx = &brw->intel.ctx;
>    GLuint i;
> +   int nr_surfaces = 0;
>
>    /* _NEW_BUFFERS | _NEW_COLOR */
>    /* Update surfaces for drawing buffers */
> @@ -595,8 +564,15 @@ upload_wm_surfaces(struct brw_context *brw)
>            brw_update_null_renderbuffer_surface(brw, i);
>         }
>       }
> +      nr_surfaces =
> SURF_INDEX_DRAW(ctx->DrawBuffer->_NumColorDrawBuffers);
>    } else {
>       brw_update_null_renderbuffer_surface(brw, 0);
> +      nr_surfaces = SURF_INDEX_DRAW(0) + 1;
>

This looks like a behavioral change to me.  The old prepare_wm_surfaces()
code would have left nr_surfaces = 0 in this case.  Was this an intentional
change?  Was the old code buggy?

I have similar questions about patch 15/33.


> +   }
> +
> +   /* BRW_NEW_WM_CONSTBUF */
> +   if (brw->wm.const_bo) {
> +      nr_surfaces = SURF_INDEX_FRAG_CONST_BUFFER + 1;
>    }
>
>    /* Update surfaces for textures */
> @@ -607,11 +583,17 @@ upload_wm_surfaces(struct brw_context *brw)
>       /* _NEW_TEXTURE */
>       if (texUnit->_ReallyEnabled) {
>         brw_update_texture_surface(ctx, i);
> +        nr_surfaces = SURF_INDEX_TEXTURE(i) + 1;
>       } else {
>          brw->wm.surf_offset[surf] = 0;
>       }
>    }
>
> +   if (brw->wm.nr_surfaces != nr_surfaces) {
> +      brw->wm.nr_surfaces = nr_surfaces;
> +      brw->state.dirty.brw |= BRW_NEW_NR_WM_SURFACES;
> +   }
> +
>    brw->state.dirty.brw |= BRW_NEW_WM_SURFACES;
>  }
>
> @@ -620,11 +602,11 @@ const struct brw_tracked_state brw_wm_surfaces = {
>       .mesa = (_NEW_COLOR |
>                _NEW_TEXTURE |
>                _NEW_BUFFERS),
> -      .brw = (BRW_NEW_BATCH),
> +      .brw = (BRW_NEW_BATCH |
> +             BRW_NEW_WM_CONSTBUF),
>       .cache = 0
>    },
> -   .prepare = prepare_wm_surfaces,
> -   .emit = upload_wm_surfaces,
> +   .emit = brw_upload_wm_surfaces,
>  };
>
>  /**
> --
> 1.7.7
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111026/9cbe93d3/attachment.htm>