[Mesa-dev] [PATCH 00/30] i965: Overhaul resolves
Jason Ekstrand
jason at jlekstrand.net
Sun May 28 01:44:18 UTC 2017
On Fri, May 26, 2017 at 5:34 PM, Jason Ekstrand <jason at jlekstrand.net>
wrote:
> On Fri, May 26, 2017 at 4:30 PM, Jason Ekstrand <jason at jlekstrand.net>
> 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.
>>
>
> One more little tidbit. For those of you who want some sort of proof that
> the new system is easier to reason about, consider this: While debugging
> patch 27, I only had a single rendering corruption (the Sandy Bridge bug
> mentioned below); the rest of the bugs were all segfaults or assertion
> failures. There were quite a few of those but they're way easier to find
> than GPU hangs and corruption.
>
>
>> 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.
>>
>
I did a bit of digging and it urns out the Sandy Bridge bug isn't my fault.
:-) It turns out we've been doing sandy bridge HiZ and stencil allocation
wrong ever since we enabled layered rendering. I did a bit of digging this
morning and I think I now understand gen6 HiZ well enough to fix it but my
initial attempt to fix it didn't quite work. Hopefully, I'll have a fix
early next week.
> 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/i
>> 965-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
>> 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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170527/1bceeb22/attachment.html>
More information about the mesa-dev
mailing list