On Tue, Nov 08, 2016 at 03:09:39PM -0800, Jason Ekstrand wrote: > On Tue, Nov 8, 2016 at 2:26 PM, Nanley Chery <nanleych...@gmail.com> wrote: > > > On Tue, Nov 08, 2016 at 01:52:15PM -0800, Jason Ekstrand wrote: > > > On Tue, Nov 8, 2016 at 1:36 PM, Nanley Chery <nanleych...@gmail.com> > > 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. > > > > > > > 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. > > > > 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. > >
Yes, I would think AUX_USAGE_NONE would take care of that. Good to know that we'll likely have this option if this path ever needs optimizing. - Nanley > > > > > > > - 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 <k...@bitplanet.net> > > > > > Cc: Chad Versace <chadvers...@chromium.org> > > > > > Cc: Nanley Chery <nanley.g.ch...@intel.com> > > > > > Cc: Topi Pohjolainen <topi.pohjolai...@intel.com> > > > > > > > > > > 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 > > > > > 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
