<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 3, 2017 at 2:22 AM, Topi Pohjolainen <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Patches 1-17 are revision that - rework hiz on gen6 to use on-demand offset calculator allowing one to drop dependency to miptree structure and - rework all auxiliary surfaces to be created against isl directly. Patches 18 and 19 introduce new surface layout in ISL. This is called back-to-back and similar to layout ALL_SLICES_AT_EACH_LOD found in i965 for gen6 hiz and stencil. This layout stacks slices for each level after one and other, or back to back. All slices ate each lod is almost the same except that it places levels one and two side-by-side trying to preserve space. Back-to-back wastes a little more memory but aligns each level on page boundary simplifying driver logic. </blockquote><div> </div><div>My primary gripe here is that you seem to have half-added back-to-back to ISL. If this layout is a long-term thing, then we should add a new ISL_DIM_LAYOUT_GEN6_BACK_TO_BACK layout and plumb your offset function through isl_surf_get_image_offset_sa. Is this intended to be a permanent solution? I think eventually, I'd like us to go with one surf per miplevel (which is almost the same) but I can see how this is easier at the moment. However, I think this works sufficiently well that I'm ok with doing the back-to-back thing for a while. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Patch 20 switches gen6 hiz to use back-to-back. Patches 22-37 prepare i965 driver to work with miptrees based on isl. Patches 38 and 39 start to use isl for stencil surfaces and effectively switches to back-to-back stencil layout on gen6. Patch 25 is mostly unneeded but it doesn't hurt and it provides me the tiling converter I need in patch 36. There are two uglies, patches 21 and 37. Perhaps Nanley, Jason or Chad can help me with 21... </blockquote><div> </div><div>Other than making it "nicer" I don't think there's much you can do about 21. Arrayed HiZ is completely undocumented. Let's just do what works. For 37, that one strikes me as very odd as it seems to go against the hardware docs and you didn't give much of an explanation as to why it's needed. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Jason: You have reviewed most of 1-17, and I don't think they have changed that much. Rafael: I have conflicting patches with your series addressing depth and stencil state emission. We should try to land your patches first and then I'll rebase this on top. If we agree on the approach here, I'll continue with gen4/5 depth surface alignment workaround aiming to base depth surfaces also on isl. That should allow me to start using isl state emitter for depth-hiz-stencil. </blockquote><div> </div><div>I've looked through the whole series and I think it seems reasonable. It lets us switch stuff over one bit at a time and then we can have one big patch at the end which deletes all of the "if (mt->surf.size > 0)" cases. </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> CC: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> CC: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> CC: Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>> CC: Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> Topi Pohjolainen (39): i965/dbg: Add means for forcing stencil sampling using y-tiled copy i965/gen6: Remove dead code in hiz surface setup i965/blorp/gen6: Drop unnecessary stencil/hiz surf dimension adjust i965/gen6: Calculate stencil offset on demand i965/gen6: Calculate hiz offset on demand i965/blorp/gen6: Use on-demand stencil/hiz offset resolvers i965/gen6: Drop miptrees in depth/stencil offset resolvers i965/blorp/gen6: Set aux pitch directly i965/gen6/hiz: Add direct buffer size resolver i965/gen6: Allocate hiz directly without miptree i965/miptree: Refactor aux surface allocation i965/miptree: Refactor ISL aux usage resolver i965/miptree: Use ISL for MCS layouts i965/miptree: Drop MIPTREE_LAYOUT_ACCELERATED_UPLOAD in mcs init i965/miptree/gen7+: Use ISL for HIZ layouts i965/blorp: Use hiz surface instead of creating copy i965: Use stored hiz surface instead of creating copy intel/isl/gen6: Add offsetting support for back-to-back layouts intel/isl/gen6: Add size calculator for back-to-back layouts i965/hiz/gen6: Use isl back-to-back layout intel/isl/gen6/hack: Use hiz vertical alignment of 16 instead of 8 i965/miptree: Add support for resolving offsets using isl i965/blorp: Add support for isl based miptrees i965: Prepare up/downsampling for isl based miptrees i965: Prepare blit engine for isl based miptrees i965: Prepare image validation for isl based miptrees i965: Refactor miptree to isl converter and adjustment i965: Prepare tex, img and rt state emission for isl based miptrees i965: Prepare slice validator for isl based miptrees i965: Prepare framebuffer validator for isl based miptrees i965/tex: Prepare image update for isl based miptrees i965: Prepare texture validator for isl based miptrees i965: Prepare slice copy for isl based miptrees i965/gen7: Prepare depth state emission for isl based miptrees i965/gen8+: Prepare depth state emission for isl based miptrees i965: Add isl based miptree creator intel/isl/gen7/hack: Use stencil vertical alignment of 8 instead of 4 i965/miptree: Represent w-tiled stencil surfaces with isl i965/miptree: Represent y-tiled stencil copies with isl src/intel/blorp/blorp.c | 4 +- src/intel/blorp/blorp_blit.c | 11 +- src/intel/common/gen_debug.c | 1 + src/intel/common/gen_debug.h | 1 + src/intel/isl/isl.c | 55 +- src/intel/isl/isl.h | 20 +- src/intel/isl/isl_gen6.c | 109 +++ src/intel/isl/isl_gen7.c | 6 +- src/intel/isl/isl_storage_image.c | 3 +- src/mesa/drivers/dri/i965/brw_blorp.c | 124 ++-- src/mesa/drivers/dri/i965/brw_misc_state.c | 16 +- src/mesa/drivers/dri/i965/brw_tex_layout.c | 16 + src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 114 ++-- src/mesa/drivers/dri/i965/gen6_depth_state.c | 50 +- src/mesa/drivers/dri/i965/gen7_misc_state.c | 31 +- src/mesa/drivers/dri/i965/gen8_depth_state.c | 47 +- src/mesa/drivers/dri/i965/intel_blit.c | 45 +- src/mesa/drivers/dri/i965/intel_blit.h | 13 + src/mesa/drivers/dri/i965/intel_fbo.c | 42 +- src/mesa/drivers/dri/i965/intel_fbo.h | 9 +- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 822 ++++++++++------------- src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 58 +- src/mesa/drivers/dri/i965/intel_screen.c | 3 +- src/mesa/drivers/dri/i965/intel_tex_image.c | 21 +- src/mesa/drivers/dri/i965/intel_tex_validate.c | 8 +- 25 files changed, 935 insertions(+), 694 deletions(-) -- 2.9.3 </blockquote></div> </div></div>