[Mesa-dev] [PATCH 16/19] i965: Manipulate state batches for HiZ operation meta-ops

[Eric Anholt]

On Fri, 23 Sep 2011 17:37:46 -0700, Chad Versace <chad at chad-versace.us> wrote:

That's a pretty terse commit message :)

> Signed-off-by: Chad Versace <chad at chad-versace.us>
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c          |    3 ++
>  src/mesa/drivers/dri/i965/gen6_clip_state.c   |   17 +++++++++++++++
>  src/mesa/drivers/dri/i965/gen6_depthstencil.c |   22 +++++++++++++++++-
>  src/mesa/drivers/dri/i965/gen6_sf_state.c     |   16 ++++++++++++-
>  src/mesa/drivers/dri/i965/gen6_wm_state.c     |   28 +++++++++++++++++++++---
>  5 files changed, 78 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen6_depthstencil.c b/src/mesa/drivers/dri/i965/gen6_depthstencil.c
> index 5d14147..59f3fe4 100644
> --- a/src/mesa/drivers/dri/i965/gen6_depthstencil.c
> +++ b/src/mesa/drivers/dri/i965/gen6_depthstencil.c
> @@ -77,10 +77,28 @@ gen6_prepare_depth_stencil_state(struct brw_context *brw)
>     }
>  
>     /* _NEW_DEPTH */
> -   if (ctx->Depth.Test) {
> -      ds->ds2.depth_test_enable = 1;
> +   if (ctx->Depth.Test || brw->hiz.op != 0) {
>        ds->ds2.depth_test_func = intel_translate_compare_func(ctx->Depth.Func);
>        ds->ds2.depth_write_enable = ctx->Depth.Mask;
> +
> +      /* See the following sections of the Sandy Bridge PRM, Volume 1, Part2:
> +       *   - 7.5.3.1 Depth Buffer Clear
> +       *   - 7.5.3.2 Depth Buffer Resolve
> +       *   - 7.5.3.3 Hierarchical Depth Buffer Resolve
> +       */
> +      switch (brw->hiz.op) {
> +      case BRW_HIZ_OP_NONE:
> +      case BRW_HIZ_OP_DEPTH_RESOLVE:
> +         ds->ds2.depth_test_enable = 1;
> +         break;
> +      case BRW_HIZ_OP_DEPTH_CLEAR:
> +      case BRW_HIZ_OP_HIZ_RESOLVE:
> +         ds->ds2.depth_test_enable = 0;
> +         break;
> +      default:
> +         assert(0);
> +         break;
> +      }

Isn't the metaop already ensuring that the depth test is
enabled/disabled as appropriate?

> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index 5cbfe78..bdd02ed 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> @@ -149,8 +149,20 @@ upload_sf_state(struct brw_context *brw)
>        num_outputs << GEN6_SF_NUM_OUTPUTS_SHIFT |
>        urb_entry_read_length << GEN6_SF_URB_ENTRY_READ_LENGTH_SHIFT |
>        urb_entry_read_offset << GEN6_SF_URB_ENTRY_READ_OFFSET_SHIFT;
> -   dw2 = GEN6_SF_VIEWPORT_TRANSFORM_ENABLE |
> -      GEN6_SF_STATISTICS_ENABLE;
> +
> +   dw2 = GEN6_SF_STATISTICS_ENABLE;
> +
> +   /* Enable viewport transform only if no HiZ operation is progress
> +    *
> +    * From page 11 of the SandyBridge PRM, Volume 2, Part 1, Section 1.3, "3D
> +    * Primitives Overview":
> +    *     RECTLIST: Viewport Mapping must be DISABLED (as is typical with the
> +    *     use of screen- space coordinates).
> +    */
> +   if (brw->hiz.op == 0) {
> +      dw2 |= GEN6_SF_VIEWPORT_TRANSFORM_ENABLE;
> +   }

> diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> index 07e9995..bbc421f 100644
> --- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> @@ -144,6 +144,23 @@ upload_wm_state(struct brw_context *brw)
>     dw4 |= (brw->wm.prog_data->first_curbe_grf_16 <<
>  	   GEN6_WM_DISPATCH_START_GRF_SHIFT_2);
>  
> +   switch (brw->hiz.op) {
> +   case BRW_HIZ_OP_NONE:
> +      break;
> +   case BRW_HIZ_OP_DEPTH_CLEAR:
> +      dw4 |= GEN6_WM_DEPTH_CLEAR;
> +      break;
> +   case BRW_HIZ_OP_DEPTH_RESOLVE:
> +      dw4 |= GEN6_WM_DEPTH_RESOLVE;
> +      break;
> +   case BRW_HIZ_OP_HIZ_RESOLVE:
> +      dw4 |= GEN6_WM_HIERARCHICAL_DEPTH_RESOLVE;
> +      break;
> +   default:
> +      assert(0);
> +      break;
> +   }
> +
>     dw5 |= (brw->wm_max_threads - 1) << GEN6_WM_MAX_THREADS_SHIFT;
>  
>     /* CACHE_NEW_WM_PROG */
> @@ -166,15 +183,18 @@ upload_wm_state(struct brw_context *brw)
>     /* BRW_NEW_FRAGMENT_PROGRAM */
>     if (fp->program.Base.InputsRead & (1 << FRAG_ATTRIB_WPOS))
>        dw5 |= GEN6_WM_USES_SOURCE_DEPTH | GEN6_WM_USES_SOURCE_W;
> -   if (fp->program.Base.OutputsWritten & BITFIELD64_BIT(FRAG_RESULT_DEPTH))
> +   if ((fp->program.Base.OutputsWritten & BITFIELD64_BIT(FRAG_RESULT_DEPTH)) &&
> +       !brw->hiz.op)
>        dw5 |= GEN6_WM_COMPUTED_DEPTH;
>  
>     /* _NEW_COLOR */
> -   if (fp->program.UsesKill || ctx->Color.AlphaEnabled)
> +   if ((fp->program.UsesKill || ctx->Color.AlphaEnabled) &&
> +       !brw->hiz.op)
>        dw5 |= GEN6_WM_KILL_ENABLE;
>  
> -   if (brw_color_buffer_write_enabled(brw) ||
> -       dw5 & (GEN6_WM_KILL_ENABLE | GEN6_WM_COMPUTED_DEPTH)) {
> +   if ((brw_color_buffer_write_enabled(brw) ||
> +        (dw5 & (GEN6_WM_KILL_ENABLE | GEN6_WM_COMPUTED_DEPTH))) &&
> +       !brw->hiz.op) {
>        dw5 |= GEN6_WM_DISPATCH_ENABLE;

It looks to me like if the meta code just set up an empty fragment
shader with its VS, none of the hiz.op checks here would be needed.  I
think that would be cleaner.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110926/881eccd6/attachment.pgp>