[Mesa-dev] [v2 3/7] i965/gen4: Remove redundant check for depth when rebasing stencil

[Jason Ekstrand]

This patch makes the assumption that, if stencil_irb != NULL then depth_irb
!= NULL.  I did a bunch of digging and I am now convinced that this is a
fine assumption.  Specifically, in intel_renderbuffer_format() in
intel_fbo.c, we map all STENCIL_INDEX internal formats to Z24_UNORM_S8_UINT
on gen4-5.

On Mon, May 22, 2017 at 12:12 PM, Topi Pohjolainen <
topi.pohjolainen at gmail.com> wrote:

> In case of gen < 6 stencil (if present) is always combined with
> depth. Both stencil and depth attachments point to the same
> physical surface.
> Alignment workaround starts by considering depth and updates
> stencil accordingly. Current logic continues with stencil and
> in vain considers the case where depth would refer to different
> surface than stencil.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_misc_state.c | 63
> ++++++------------------------
>  1 file changed, 12 insertions(+), 51 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
> b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index 6ea1fb0..9fe3655 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -295,28 +295,14 @@ brw_workaround_depthstencil_alignment(struct
> brw_context *brw,
>        }
>
>        if (stencil_irb) {
> -         stencil_mt = get_stencil_miptree(stencil_irb);
> -         intel_miptree_get_image_offset(stencil_mt,
> -                                        stencil_irb->mt_level,
> -                                        stencil_irb->mt_layer,
> -                                        &stencil_draw_x, &stencil_draw_y);
> -         int stencil_tile_x = stencil_draw_x & tile_mask_x;
> -         int stencil_tile_y = stencil_draw_y & tile_mask_y;
> -
> -         /* If stencil doesn't match depth, then we'll need to rebase
> stencil
> -          * as well.  (if we hadn't decided to rebase stencil before, the
> -          * post-stencil depth test will also rebase depth to try to
> match it
> -          * up).
> -          */
> -         if (tile_x != stencil_tile_x ||
> -             tile_y != stencil_tile_y) {
> -            rebase_stencil = true;
> -         }
> +         assert(stencil_irb->mt == depth_irb->mt);
> +         assert(stencil_irb->mt_level == depth_irb->mt_level);
> +         assert(stencil_irb->mt_layer == depth_irb->mt_layer);
>        }
>     }
>
>     /* If we have (just) stencil, check it for ignored low bits as well */
> -   if (stencil_irb) {
> +   if (!depth_irb && stencil_irb) {
>        intel_miptree_get_image_offset(stencil_mt,
>                                       stencil_irb->mt_level,
>                                       stencil_irb->mt_layer,
> @@ -334,6 +320,14 @@ brw_workaround_depthstencil_alignment(struct
> brw_context *brw,
>     }
>
>     if (rebase_stencil) {
> +      /* If stencil needs rebase, there isn't a depth attachment and the
> +       * combined depth-stencil is used for stencil only. Otherwise in
> case
> +       * depth attachment is present both stencil and depth point to the
> same
> +       * miptree. Rebase of depth is considered first updating stencil
> +       * attachment accordingly - hence stencil is rebased only if there
> is no
> +       * depth attachment.
> +       */
> +      assert(!depth_irb);
>        perf_debug("HW workaround: blitting stencil level %d to a temporary
> "
>                   "to fix alignment (stencil tile offset %d,%d)\n",
>                   stencil_irb->mt_level, stencil_tile_x, stencil_tile_y);
> @@ -347,39 +341,6 @@ brw_workaround_depthstencil_alignment(struct
> brw_context *brw,
>                                       &stencil_draw_x, &stencil_draw_y);
>        stencil_tile_x = stencil_draw_x & tile_mask_x;
>        stencil_tile_y = stencil_draw_y & tile_mask_y;
> -
> -      if (depth_irb && depth_irb->mt == stencil_irb->mt) {
> -         intel_miptree_reference(&depth_irb->mt, stencil_irb->mt);
> -         intel_renderbuffer_set_draw_offset(depth_irb);
> -      } else if (depth_irb && !rebase_depth) {
> -         if (tile_x != stencil_tile_x ||
> -             tile_y != stencil_tile_y) {
> -            perf_debug("HW workaround: blitting depth level %d to a
> temporary "
> -                       "to match stencil level %d alignment (depth tile
> offset "
> -                       "%d,%d, stencil offset %d,%d)\n",
> -                       depth_irb->mt_level,
> -                       stencil_irb->mt_level,
> -                       tile_x, tile_y,
> -                       stencil_tile_x, stencil_tile_y);
> -
> -            intel_renderbuffer_move_to_temp(brw, depth_irb,
> invalidate_depth);
> -
> -            tile_x = depth_irb->draw_x & tile_mask_x;
> -            tile_y = depth_irb->draw_y & tile_mask_y;
> -
> -            if (stencil_irb && stencil_irb->mt == depth_mt) {
> -               intel_miptree_reference(&stencil_irb->mt, depth_irb->mt);
> -               intel_renderbuffer_set_draw_offset(stencil_irb);
> -            }
> -
> -            WARN_ONCE(stencil_tile_x != tile_x ||
> -                      stencil_tile_y != tile_y,
> -                      "Rebased stencil tile offset (%d,%d) doesn't match
> depth "
> -                      "tile offset (%d,%d).\n",
> -                      stencil_tile_x, stencil_tile_y,
> -                      tile_x, tile_y);
> -         }
> -      }
>     }
>
>     if (!depth_irb) {
> --
> 2.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170615/3ffde6bd/attachment-0001.html>