[Mesa-dev] [PATCH 00/30] i965: Add support for I915_FORMAT_MOD_Y_TILED_CCS

Fri Jun 16 22:41:22 UTC 2017

This series is a rework of Ben's series to enable the CCS format modifier.
It started as an attempt to rebase his original patches on top of my
resolve reworks inside the miptree code.  However, as I started to dive
deeper, I found a number of subtle issues:

 1) Thanks to the terrible set of INTEL_AUX_DISABLE_* flags that we use to
    choose what aux buffers to use, we were set up to never use CCS_E for
    any external buffers.  Even when Y-tiled on gen9, the most they would
    ever get was CCS_D.

 2) Whether to do a full or partial resolve or not was based on is_scanout
    and not on the actual modifier.  If we use I915_FORMAT_MOD_Y_TILED (not
    the CCS modifier) and choose to use CCS_E with it, it would only get
    partial resolves and not full resolves.  Of course, this wasn't
    actually a problem thanks to problem 1 above.

 3) If a user ever imported an image with I915_FORMAT_MOD_Y_TILED_CCS
    through EGL or GBM and did glClear on it, they would get a fast clear
    with no way to force a resolve before handing it off to the other
    process.  Since the other process doesn't know the clear color, this
    means that any blocks in the clear state in the surface will get
    whatever random clear color process B thinks it has.

 4) There were three different places where we computed the pitch of the
    CCS and they all did so differently.  When we go to create the image,
    we would allocate the CCS with the same pitch as the main surface.  We
    would then calculate the CCS pitch with ISL when we created mt->mcs_buf.
    Finally, we had a different mechanism to compute the pitch when we pass
    it back to the user.  Fortunately, the first only caused us to over-
    allocate and I think the last two were equivalent (at least for the
    simple case) so nothing exploded.

 5) Thanks again to our confusing aux enable/disable, we haven't been doing
    multisample fast-clears since cec30a666930ddb8476a9452a89364a24979ff62
    around a year ago.

This series takes a bit more round-about approach to enabling the CCS
modifier that should fix these issues:

 * Patches 1-5 do a bit of refactoring and then rework the way we choose
   the type of aux compression to use.  They move us away from the crazy
   enable/disable system to a simple choice system.  This fixes (1) and (5)
   above.

 * Patches 6-15 refactor things so that we have only one path for going
   from a __DRIimage to an intel_mipmap_tree.  This was rather painful
   because we have to be careful to take into account the differences
   between window system images regular images.

 * Patches 16-22 rework image creation and import to use ISL to do their
   surface layout calculations.  Previously, all of the surface layout
   calculations were simply hand-rolled here.  In the particular case of
   images, the hand-rolling was fairly safe because they were only ever
   simple 2D non-array images.  However, with the addition of CCS, things
   were going to get a bit tricky.

 * Patches 23-30 add support for I915_FORMAT_MOD_Y_TILED.

I've tested this series on our Jenkins system which runs piglit as well as
the OpenGL and OpenGL ES test suites.  Both piglit and the OpenGL ES suite
have some number of EGL tests which I hope have tested some of this.  I've
also tested with kmscube and have verified that I get basically the same
bandwidth numbers as Ben got on his original series, so I think CCS is
working properly.

This series can be found here:

https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/i965-ccs-mod

Cc: Ben Widawsky <ben at bwidawsk.net>
Cc: Daniel Stone <daniels at collabora.com>
Cc: Varad Gautam <varad.gautam at collabora.com>
Cc: Chad Versace <chadversary at chromium.org>
Cc: Topi Pohjolainen <topi.pohjolainen at intel.com>

Ben Widawsky (6):
  i965/miptree: Add a return for updating of winsys
  i965/miptree: Allocate mt earlier in update winsys
  i965: Support images with aux buffers
  i965/miptree: Allocate mcs_buf for an image's CCS
  i965: Pretend that CCS modified images are two planes
  i965: Advertise the CCS modifier

Jason Ekstrand (24):
  i965/miptree: Delete the layered rendering resolve
  i965/miptree: Rename the non_msrt_mcs functions to _ccs
  i965: Don't bother with HiZ in renderbuffer_move_to_temp
  i965: Clamp clear colors to the representable range
  i965/miptree: Rework aux enabling
  i965: Move the DRIimage -> miptree code to intel_mipmap_tree.c
  i965/miptree: Pass the offset into create_for_bo in
    create_for_dri_image
  i965/miptree: Add tile_x/y to total_width/height
  i965/miptree: Set level_x/h in create_for_dri_image
  i965: Use miptree_create_for_dri_image in
    image_target_renderbuffer_storage
  i965/miptree: Add an explicit format parameter to create_for_dri_image
  i965/miptree: Add support for window system images to
    create_for_dri_image
  i965: Use create_for_dri_image in intel_update_image_buffer
  i965/miptree: Move CCS allocation into create_for_dri_image
  i965: Add an isl_device to intel_screen
  intel/isl: Add basic modifier introspection
  intel/isl: Add a helper to convert tilings fro ISL to i915
  i965/screen: Use ISL for allocating image BOs
  i965/screen: Use ISL for doing image import checks
  i965/screen: Drop get_tiled_height
  intel/isl: Add support for I915_FORMAT_MOD_Y_TILED_CCS
  intel/isl: Add a row_pitch parameter to surf_get_ccs_surf
  i965/screen: Support import and export of surfaces with CCS
  i965/miptree: More conservatively resolve external images

 src/intel/Makefile.am                         |   1 +
 src/intel/Makefile.sources                    |   1 +
 src/intel/isl/isl.c                           |   4 +-
 src/intel/isl/isl.h                           |  28 +-
 src/intel/isl/isl_drm.c                       |  93 +++++
 src/intel/vulkan/anv_image.c                  |   2 +-
 src/mesa/drivers/dri/i965/brw_blorp.c         |   4 +-
 src/mesa/drivers/dri/i965/brw_context.c       |  44 ++-
 src/mesa/drivers/dri/i965/brw_meta_util.c     |  40 +++
 src/mesa/drivers/dri/i965/intel_fbo.c         |  30 +-
 src/mesa/drivers/dri/i965/intel_image.h       |   6 +
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 496 ++++++++++++++++++--------
 src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  77 ++--
 src/mesa/drivers/dri/i965/intel_screen.c      | 216 +++++++----
 src/mesa/drivers/dri/i965/intel_screen.h      |   4 +
 src/mesa/drivers/dri/i965/intel_tex_image.c   |  98 +----
 16 files changed, 767 insertions(+), 377 deletions(-)
 create mode 100644 src/intel/isl/isl_drm.c

-- 
2.5.0.400.gff86faf