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

Mon Jun 19 16:42:38 UTC 2017

On Mon, Jun 19, 2017 at 5:49 AM, Eero Tamminen <eero.t.tamminen at intel.com>
wrote:

> Hi,
>
> I run this on few GEN9 machines and SynMark v7 DeferredAA test improved
> ~3%, so it seems multisample fast-clears do help. :-)
>

Thanks!  I'll add that to the commit message on patch 5.

--Jason

>
>         - Eero
>
>
> On 17.06.2017 01:41, Jason Ekstrand wrote:
>
>> 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 cec30a666930ddb8476a9452a89364
>> a24979ff62
>>     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
>>
>>
> _______________________________________________
> 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/20170619/e62367c7/attachment-0001.html>