[Mesa-dev] [PATCH 00/30] i965: Overhaul resolves

Tue May 30 08:48:15 UTC 2017

On Fri, May 26, 2017 at 04:30:04PM -0700, Jason Ekstrand wrote:
> This patch series does a complete overhaul of the current resolve handling
> framework inside the i916 OpenGL driver.  For HiZ and MCS, the current
> resolve code is ok but not optimal.  For CCS, however, it's pretty bad.
> I've been looking at the code for a week now and I still don't know how Ben
> ever got it to do a partial resolve for his CCS modifiers series.  So far
> as I can tell, it's not capable of doing so.  The new resolve system is
> hopefully much easier to reason about.  For users of the system, there are
> fewer entry-points and depth and color are no longer separate.  The guts of
> the system are much more explicit and, thanks to the new isl_aux_state
> enum, should be easier to understand.
> 
> As of my last Jenkins run, the series is still failing 2 piglit tests on
> Sandy Bridge and I have yet to do any benchmarking.  However, I wanted to
> send it out early so that I could get feedback on the structure of the
> system as quickly as possible.  Discussion of the structure can happen in
> parallel with final tweaking.  Personally, I'm fairly happy with it and I
> think this looks like a good way to go but I'd like more eyes.
> 
> The patch series itself is organized as follows
> 
>  * The first 13 patches are various cleanups which make later patches
>    simpler.  They should be fairly benign.  These can easily land on their
>    own as I think most of them are good clean-ups anyway.
> 
>  * Patch 14 adds the new isl_aux_state enum and the accompanying comment
> 
>  * Patch 15 adds the new interface for doing resolves.  All of the
>    functions are just dummies which call the already existing functions.
> 
>  * Patches 16-26 convert everything over to using the new resolve
>    interface.  I tried as hard as I could to not make any functional
>    changes while doing so.  If you see any, they are probably bugs!
> 
>  * Patch 27 wholesale replaces the current color resolve scheme with a new
>    one based on isl_aux_state.  It's a bit unfortunate that it all had to
>    happen in one go but it's not easy to switch resolve schemes slowly.
> 
>  * Patch 28 replaces the HiZ resolve framework.  This one is not nearly as
>    drastic as patch 27 because the current HiZ framework is already pretty
>    good.
> 
>  * Patch 29 deletes the now unused intel_resolve_map struct
> 
>  * Patch 30 enables fast-clears for non-CCS_E capable surfaces.  In
>    particular, this gives us fast-clears for sRGB.
> 
> I'd appreciate it if the initial review focussed on patches 14, 15, and 27.
> Those are where you see the new resolve system in action.
> 
> This series is available here:
> 
> https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/i965-resolve-rework
> 
> Happy Reviewing!
> 
> --Jason Ekstrand
> 
> Cc: Chad Versace <chadversary at chromium.org>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Nanley Chery <nanley.g.chery at intel.com>
> Cc: Topi Pohjolainen <topi.pohjolainen at intel.com>
> 
> Jason Ekstrand (30):
>   i965: Mark depth surfaces as needing a HiZ resolve after blitting
>   i965/surface_state: Images can't handle CCS at all
>   intel/isl: Add a helper for determining if a color is 0/1
>   i965/miptree: Store fast clear colors in an isl_color_value
>   i965/miptree: Clean up the depth resolve helpers a little
>   i965/miptree: Refactor intel_miptree_resolve_color
>   i965: Get rid of intel_renderbuffer_resolve_*
>   i965: Inline renderbuffer_att_set_needs_depth_resolve
>   i965/miptree: Move color resolve on map to intel_miptree_map
>   i965/blorp: Take an explicit fast clear op in resolve_color
>   i965/blorp: Refactor do_single_blorp_clear
>   i965/blorp: Move MCS allocation earlier for clears
>   i965: Combine render target resolve code

Patches 3-13:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

>   intel/isl: Add an enum for describing auxiliary compression state
>   i965/miptree: Add new entrypoints for resolve management
>   i965: Use the new resolve function for several simple cases
>   i965: Finalize miptrees before prepare_texture
>   i965: Move texturing to the new resolve functions
>   i965: Move color rendering to the new resolve functions
>   i965: Remove an unneeded render_cache_set_check_flush
>   i965: Move framebuffer fetch to the new resolve functions
>   i965: Move images to the new resolve functions
>   i965: Move depth to the new resolve functions
>   i965: Move blorp to the new resolve functions
>   i965: Use the new get/set_aux_state functions for color clears
>   i965: Delete most of the old resolve interface
>   i965: Wholesale replace the color resolve tracking code
>   i965: Use the new tracking mechanism for HiZ
>   i965: Delete intel_resolve_map
>   i965: Enable non-CCS_E fast-clears on gen9+
> 
>  src/intel/blorp/blorp_genX_exec.h                |   2 +-
>  src/intel/isl/isl.c                              |  19 +
>  src/intel/isl/isl.h                              | 146 ++++
>  src/intel/isl/isl_emit_depth_stencil.c           |  19 +
>  src/mesa/drivers/dri/i965/Makefile.sources       |   2 -
>  src/mesa/drivers/dri/i965/brw_blorp.c            | 143 ++--
>  src/mesa/drivers/dri/i965/brw_blorp.h            |   3 +-
>  src/mesa/drivers/dri/i965/brw_clear.c            |  30 +-
>  src/mesa/drivers/dri/i965/brw_context.c          | 123 +--
>  src/mesa/drivers/dri/i965/brw_context.h          |   1 -
>  src/mesa/drivers/dri/i965/brw_draw.c             |  51 +-
>  src/mesa/drivers/dri/i965/brw_meta_util.c        |  56 +-
>  src/mesa/drivers/dri/i965/brw_meta_util.h        |   7 +-
>  src/mesa/drivers/dri/i965/brw_misc_state.c       |  23 +-
>  src/mesa/drivers/dri/i965/brw_state.h            |   3 +
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c |  14 +-
>  src/mesa/drivers/dri/i965/gen6_depth_state.c     |   7 +-
>  src/mesa/drivers/dri/i965/gen7_misc_state.c      |   7 +-
>  src/mesa/drivers/dri/i965/gen8_depth_state.c     |   3 +-
>  src/mesa/drivers/dri/i965/intel_blit.c           |  12 +-
>  src/mesa/drivers/dri/i965/intel_fbo.c            |  41 -
>  src/mesa/drivers/dri/i965/intel_fbo.h            |  27 -
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c    | 912 ++++++++++++++++-------
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h    | 247 +++---
>  src/mesa/drivers/dri/i965/intel_pixel_bitmap.c   |   2 +-
>  src/mesa/drivers/dri/i965/intel_pixel_read.c     |   2 +-
>  src/mesa/drivers/dri/i965/intel_resolve_map.c    |  97 ---
>  src/mesa/drivers/dri/i965/intel_resolve_map.h    | 168 -----
>  src/mesa/drivers/dri/i965/intel_tex_image.c      |   7 +-
>  src/mesa/drivers/dri/i965/intel_tex_subimage.c   |   7 +-
>  30 files changed, 1139 insertions(+), 1042 deletions(-)
>  delete mode 100644 src/mesa/drivers/dri/i965/intel_resolve_map.c
>  delete mode 100644 src/mesa/drivers/dri/i965/intel_resolve_map.h
> 
> Fun fact:  While this series is technically +103 lines, the comment on
> isl_aux_state by itself is 125 lines.
> 
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev