<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 16, 2017 at 3:41 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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: <a href="https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/i965-ccs-mod" rel="noreferrer" target="_blank">https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=review/i965-ccs-mod</a> </blockquote><div> </div><div>I've rebased on top of Topi's ISL reworks and force-pushed that branch. You can see the result of the rebase there. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Cc: Ben Widawsky <<a href="mailto:ben@bwidawsk.net">ben@bwidawsk.net</a>> Cc: Daniel Stone <<a href="mailto:daniels@collabora.com">daniels@collabora.com</a>> Cc: Varad Gautam <<a href="mailto:varad.gautam@collabora.com">varad.gautam@collabora.com</a>> Cc: Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>> Cc: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> 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 </blockquote></div> </div></div>