On Wed, Jun 21, 2017 at 05:51:00PM -0700, Jason Ekstrand wrote: > On Fri, Jun 16, 2017 at 3:41 PM, Jason Ekstrand <ja...@jlekstrand.net> > 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 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.
Suggestion to revise the commit message in patch 12. Typo in the subject in patch 19. In 25 there is a question about incoming aux data - it looks that were are taking it as containing meaningful data in some cases. Question in patch 27. Typo in patch 28. Given we have those sorted, patches 12-29 are: Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > > > 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 > > > > I've rebased on top of Topi's ISL reworks and force-pushed that branch. > You can see the result of the rebase there. > > > > 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 > > > > -- > > 2.5.0.400.gff86faf > > > > > _______________________________________________ > 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