On Mon, Jun 19, 2017 at 5:49 AM, Eero Tamminen <eero.t.tammi...@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 <b...@bwidawsk.net> >> Cc: Daniel Stone <dani...@collabora.com> >> Cc: Varad Gautam <varad.gau...@collabora.com> >> Cc: Chad Versace <chadvers...@chromium.org> >> Cc: Topi Pohjolainen <topi.pohjolai...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev