<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 8, 2016 at 2:26 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Nov 08, 2016 at 01:52:15PM -0800, Jason Ekstrand wrote: > On Tue, Nov 8, 2016 at 1:36 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> wrote: > > > On Sat, Oct 22, 2016 at 10:50:31AM -0700, Jason Ekstrand wrote: > > > This series does some fairly major surgery on color attachment surface > > > state allocation and fill-out in the Intel Vulkan driver. This is in > > > preparation for doing color compression, fast-clears, and HiZ-capable > > input > > > attachments. Naturally, as with everything else I've done in the last 2 > > > months, it also involves some non-trivial blorp work. > > > > > > Let's start off at the beginning... For a variety of reasons, we can't > > > really know 100% of the details of an attachment's surface state at any > > > other places than vkCmdBeginRenderPass and vkCmdNextSubpss. The same > > > applies for depth buffers if you consider 3DSTATE_DEPTH_BUFFER and > > friends > > > to be the depth and stencil buffer's "surface state". That's a fairly > > > strong statement, but there are a couple of reasons for this: > > > > > > 1) In order for fast-clears to work, the surface state has to contain > > the > > > clear color. (This is it's own packet for HiZ but not for color.) > > We > > > don't know the clear value until BeginRenderPass. This means we > > can't > > > fully fill out the surface state in vkCmdCreateImageView. > > > > > > > We could alternatively merge the view's surface state packet into > > another that only contains the clear color(s) right? > > > > Potentially, yes. However that adds a good bit of complication because we > now have to emit render target surfaces on-the-fly because you may be > building two different batches simultaneously that use the same VkImageView > as a render target with two different clear colors. It also doesn't solve > the null framebuffer problem. > </div></div>I'm not suggesting that this optimization solves the null framebuffer problem, nor that we could add the clear color to the VkImageView's surface state. I'm trying to confirm that we could allocate the block of states (as is done in this series), then assign a block entry the VkImageView's surface state + a surface state struct that only contains the clear colors. </blockquote><div> </div><div>Yes, that might work and would let us keep the isl_surf_fill_state call in anv_image.c. We would also have to deal with at least the AuxUsage field in the OR-in as well. I think we could set up the other aux buffer information in isl_surf_fill_state and the hardware *should* ignore if we set AuxUsage to AUX_USAGE_NONE. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > > > - Nanley > > > > > 2) The Vulkan spec requires that you be able to call > > vkBeginCommandBuffer > > > on a secondary command buffer with USAGE_RENDER_PASS_CONTINUE_BIT set > > > but with a null framebuffer. In this case, the secondary is supposed > > > to inherit the framebuffer from the primary. (This is not something > > we > > > have properly implemented until now.) This means that anything that > > is > > > callable from a render-pass-continuing secondary command buffer has > > to > > > be able to operate without knowing any surface details that aren't > > part > > > of the VkRenderPass object. Basically, all you know is the Vulkan > > > format (not the isl format) and the sample count. > > > > > > Between the two of those, about the only two entrypoints left at which we > > > actually know surface details are vkCmdBeginRenderPass and > > vkCmdNextSubpass > > > so we have to figure out how to do everything there. As it turns out, > > this > > > works out surprisingly well. The format and the sample count turn out to > > > be exactly the data we actually need in order to do all of our pipeline > > > programming. The only hard part is refactoring things so that it pulls > > the > > > data from the render pass instead of the framebuffer. There are a number > > > of places where we were grabbing the image view for an attachment because > > > we either wanted to shove something into blorp or because we wanted the > > > format and we were lazy. > > > > > > The approach taken in this patch series is the following: > > > > > > 1) Instead of allocating render target surface states in > > vkCreateImageView, > > > we allocate them as part of render pass setup in > > vkCmdBeginRenderPass. > > > All of the surface states we will ever need (including a null surface > > > state) are allocated up-front out of a single contiguous block. > > > > > > 2) For secondary command buffers with USAGE_RENDER_PASS_CONTINUE_BIT > > set, > > > we allocate storage for all of the surface states but don't actually > > > fill them out. In the secondary command buffer, all binding tables > > > refer to these surface states rather than the ones in the primary. > > > > > > 3) A blorp entrypoint is added that performs a clear operation without > > > touching the depth/stencil buffer state and with a color attachment > > > binding table explicitly provided by the caller. This means that > > even > > > our blorp clears are using the surface states allocated in > > > vkCmdBeginRenderPass. Unfortunately, this turned out to be more work > > > than expected because I had to add vertex shader support to blorp > > along > > > the way. > > > > > > 4) Here's the tricky bit. When vkCmdExecuteCommands is called during a > > > render pass, we use transform feedback (yeah, crazy) to copy the > > block > > > of surface states from the primary into the secondary right before > > > executing the secondary. > > > > > > It's kind of a crazy scheme but I like the end result quite a bit. > > > > > > Cc: Kristian Høgsberg Kristensen <<a href="mailto:krh@bitplanet.net">krh@bitplanet.net</a>> > > > Cc: Chad Versace <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>> > > > Cc: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> > > > Cc: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > > > > > > Jason Ekstrand (25): > > > intel/isl: Add some basic info about RENDER_SURFACE_STATE to > > > isl_device > > > intel/genxml: Add SO_WRITE_OFFSET registers for gen7-9 > > > anv: Add a helper for doing buffer copies with nothing but VF and SOL. > > > anv/cmd_buffer: Use the surface state alloc helper in > > > null_surface_state > > > anv/cmd_buffer: Expose add_surface_state_reloc as an inline helper > > > anv: Rework the way render target surfaces are allocated > > > anv/cmd_buffer: Stop relying on the framebuffer for 3DSTATE_SF on gen7 > > > intel/genxml: Make some VS/GS fields consistent across gens > > > intel/blorp: Make the number of samples an explicit parameter > > > intel/blorp: Add a shader type to make keys more unique > > > intel/blorp: Remove NIR support for uniforms > > > intel/blorp: Rename compile_nir_shader to compile_fs > > > intel/blorp: Rework our usage of ralloc when compiling shaders > > > intel/blorp: Handle NIR clear inputs the same way as blit inputs > > > blorp/exec: Use uint32_t for copying varying data > > > intel/blorp: Use an actual chunk of vertex buffer for the VUE header > > > intel/blorp: Add support for vertex shaders > > > intel/blorp: Add capability to use pre-baked binding tables > > > intel/blorp: Add a clear_attachments entrypoint > > > anv: Bring back anv_cmd_buffer_emit_state_base_address > > > anv/blorp: Break the guts of alloc_binding_table into a shared helper > > > anv/blorp: Use the new clear_attachments entrypoint for attachment > > > clears > > > anv: Set framebuffer to NULL in secondary command buffers > > > Allocate a null state whenever there is depth/stencil > > > anv/blorp: Handle VK_ATTACHMENT_UNUSED in CmdClearAttachments > > > > > > src/intel/blorp/blorp.c | 74 ++++---- > > > src/intel/blorp/blorp.h | 11 ++ > > > src/intel/blorp/blorp_blit.c | 34 ++-- > > > src/intel/blorp/blorp_clear.c | 189 > > +++++++++++++++++-- > > > src/intel/blorp/blorp_genX_exec.h | 196 > > ++++++++++--------- > > > src/intel/blorp/blorp_priv.h | 52 ++++- > > > src/intel/genxml/gen6.xml | 6 +- > > > src/intel/genxml/gen7.xml | 22 ++- > > > src/intel/genxml/gen75.xml | 22 ++- > > > src/intel/genxml/gen8.xml | 16 ++ > > > src/intel/genxml/gen9.xml | 16 ++ > > > src/intel/isl/isl.c | 19 ++ > > > src/intel/isl/isl.h | 11 ++ > > > src/intel/vulkan/Makefile.sources | 4 + > > > src/intel/vulkan/anv_batch_chain.c | 4 +- > > > src/intel/vulkan/anv_blorp.c | 125 +++++++++---- > > > src/intel/vulkan/anv_cmd_buffer.c | 74 ++------ > > > src/intel/vulkan/anv_genX.h | 5 + > > > src/intel/vulkan/anv_image.c | 22 --- > > > src/intel/vulkan/anv_private.h | 42 ++++- > > > src/intel/vulkan/gen7_cmd_buffer.c | 43 +++-- > > > src/intel/vulkan/gen7_pipeline.c | 6 +- > > > src/intel/vulkan/genX_blorp_exec.c | 18 +- > > > src/intel/vulkan/genX_cmd_buffer.c | 229 > > +++++++++++++++++------ > > > src/intel/vulkan/genX_gpu_memcpy.c | 223 > > ++++++++++++++++++++++ > > > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 56 +++--- > > > 26 files changed, 1105 insertions(+), 414 deletions(-) > > > create mode 100644 src/intel/vulkan/genX_gpu_memcpy.c > > > > > > -- > > > 2.5.0.400.gff86faf > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > </div></div></blockquote></div> </div></div>