[Mesa-dev] i965: Use isl for hiz and stencil

Mon May 8 23:51:35 UTC 2017

On Wed, May 3, 2017 at 2:22 AM, Topi Pohjolainen <topi.pohjolainen at gmail.com
> wrote:

> Patches 1-17 are revision that
>
>   - rework hiz on gen6 to use on-demand offset calculator allowing
>     one to drop dependency to miptree structure and
>   - rework all auxiliary surfaces to be created against isl directly.
>
> Patches 18 and 19 introduce new surface layout in ISL. This is called
> back-to-back and similar to layout ALL_SLICES_AT_EACH_LOD found in
> i965 for gen6 hiz and stencil. This layout stacks slices for each level
> after one and other, or back to back. All slices ate each lod is almost
> the same except that it places levels one and two side-by-side trying
> to preserve space. Back-to-back wastes a little more memory but aligns
> each level on page boundary simplifying driver logic.
>

My primary gripe here is that you seem to have half-added back-to-back to
ISL.  If this layout is a long-term thing, then we should add a new
ISL_DIM_LAYOUT_GEN6_BACK_TO_BACK layout and plumb your offset function
through isl_surf_get_image_offset_sa.  Is this intended to be a permanent
solution?  I think eventually, I'd like us to go with one surf per miplevel
(which is almost the same) but I can see how this is easier at the moment.
However, I think this works sufficiently well that I'm ok with doing the
back-to-back thing for a while.

> Patch 20 switches gen6 hiz to use back-to-back.
>
> Patches 22-37 prepare i965 driver to work with miptrees based on isl.
> Patches 38 and 39 start to use isl for stencil surfaces and effectively
> switches to back-to-back stencil layout on gen6.
> Patch 25 is mostly unneeded but it doesn't hurt and it provides me the
> tiling converter I need in patch 36.
>
> There are two uglies, patches 21 and 37. Perhaps Nanley, Jason or Chad
> can help me with 21...
>

Other than making it "nicer" I don't think there's much you can do about
21.  Arrayed HiZ is completely undocumented.  Let's just do what works.
For 37, that one strikes me as very odd as it seems to go against the
hardware docs and you didn't give much of an explanation as to why it's
needed.

> Jason: You have reviewed most of 1-17, and I don't think they have
>        changed that much.
>
> Rafael: I have conflicting patches with your series addressing depth
>         and stencil state emission. We should try to land your patches
>         first and then I'll rebase this on top.
>
>
> If we agree on the approach here, I'll continue with gen4/5 depth
> surface alignment workaround aiming to base depth surfaces also on
> isl. That should allow me to start using isl state emitter for
> depth-hiz-stencil.
>

I've looked through the whole series and I think it seems reasonable.  It
lets us switch stuff over one bit at a time and then we can have one big
patch at the end which deletes all of the "if (mt->surf.size > 0)" cases.

--Jason

>
> CC: Jason Ekstrand <jason at jlekstrand.net>
> CC: Nanley Chery <nanley.g.chery at intel.com>
> CC: Chad Versace <chadversary at chromium.org>
> CC: Rafael Antognolli <rafael.antognolli at intel.com>
>
> Topi Pohjolainen (39):
>   i965/dbg: Add means for forcing stencil sampling using y-tiled copy
>   i965/gen6: Remove dead code in hiz surface setup
>   i965/blorp/gen6: Drop unnecessary stencil/hiz surf dimension adjust
>   i965/gen6: Calculate stencil offset on demand
>   i965/gen6: Calculate hiz offset on demand
>   i965/blorp/gen6: Use on-demand stencil/hiz offset resolvers
>   i965/gen6: Drop miptrees in depth/stencil offset resolvers
>   i965/blorp/gen6: Set aux pitch directly
>   i965/gen6/hiz: Add direct buffer size resolver
>   i965/gen6: Allocate hiz directly without miptree
>   i965/miptree: Refactor aux surface allocation
>   i965/miptree: Refactor ISL aux usage resolver
>   i965/miptree: Use ISL for MCS layouts
>   i965/miptree: Drop MIPTREE_LAYOUT_ACCELERATED_UPLOAD in mcs init
>   i965/miptree/gen7+: Use ISL for HIZ layouts
>   i965/blorp: Use hiz surface instead of creating copy
>   i965: Use stored hiz surface instead of creating copy
>   intel/isl/gen6: Add offsetting support for back-to-back layouts
>   intel/isl/gen6: Add size calculator for back-to-back layouts
>   i965/hiz/gen6: Use isl back-to-back layout
>   intel/isl/gen6/hack: Use hiz vertical alignment of 16 instead of 8
>   i965/miptree: Add support for resolving offsets using isl
>   i965/blorp: Add support for isl based miptrees
>   i965: Prepare up/downsampling for isl based miptrees
>   i965: Prepare blit engine for isl based miptrees
>   i965: Prepare image validation for isl based miptrees
>   i965: Refactor miptree to isl converter and adjustment
>   i965: Prepare tex, img and rt state emission for isl based miptrees
>   i965: Prepare slice validator for isl based miptrees
>   i965: Prepare framebuffer validator for isl based miptrees
>   i965/tex: Prepare image update for isl based miptrees
>   i965: Prepare texture validator for isl based miptrees
>   i965: Prepare slice copy for isl based miptrees
>   i965/gen7: Prepare depth state emission for isl based miptrees
>   i965/gen8+: Prepare depth state emission for isl based miptrees
>   i965: Add isl based miptree creator
>   intel/isl/gen7/hack: Use stencil vertical alignment of 8 instead of 4
>   i965/miptree: Represent w-tiled stencil surfaces with isl
>   i965/miptree: Represent y-tiled stencil copies with isl
>
>  src/intel/blorp/blorp.c                          |   4 +-
>  src/intel/blorp/blorp_blit.c                     |  11 +-
>  src/intel/common/gen_debug.c                     |   1 +
>  src/intel/common/gen_debug.h                     |   1 +
>  src/intel/isl/isl.c                              |  55 +-
>  src/intel/isl/isl.h                              |  20 +-
>  src/intel/isl/isl_gen6.c                         | 109 +++
>  src/intel/isl/isl_gen7.c                         |   6 +-
>  src/intel/isl/isl_storage_image.c                |   3 +-
>  src/mesa/drivers/dri/i965/brw_blorp.c            | 124 ++--
>  src/mesa/drivers/dri/i965/brw_misc_state.c       |  16 +-
>  src/mesa/drivers/dri/i965/brw_tex_layout.c       |  16 +
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 114 ++--
>  src/mesa/drivers/dri/i965/gen6_depth_state.c     |  50 +-
>  src/mesa/drivers/dri/i965/gen7_misc_state.c      |  31 +-
>  src/mesa/drivers/dri/i965/gen8_depth_state.c     |  47 +-
>  src/mesa/drivers/dri/i965/intel_blit.c           |  45 +-
>  src/mesa/drivers/dri/i965/intel_blit.h           |  13 +
>  src/mesa/drivers/dri/i965/intel_fbo.c            |  42 +-
>  src/mesa/drivers/dri/i965/intel_fbo.h            |   9 +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c    | 822
> ++++++++++-------------
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h    |  58 +-
>  src/mesa/drivers/dri/i965/intel_screen.c         |   3 +-
>  src/mesa/drivers/dri/i965/intel_tex_image.c      |  21 +-
>  src/mesa/drivers/dri/i965/intel_tex_validate.c   |   8 +-
>  25 files changed, 935 insertions(+), 694 deletions(-)
>
> --
> 2.9.3
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170508/51c14473/attachment.html>