[Mesa-dev] [PATCH 00/25] anv: A major rework of color attachment surface states

Nanley Chery nanleychery at gmail.com
Wed Nov 9 23:27:37 UTC 2016


On Tue, Nov 08, 2016 at 03:09:39PM -0800, Jason Ekstrand wrote:
> On Tue, Nov 8, 2016 at 2:26 PM, Nanley Chery <nanleychery at 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 <nanleychery at 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 <krh at bitplanet.net>
> > > > > Cc: Chad Versace <chadversary at chromium.org>
> > > > > Cc: Nanley Chery <nanley.g.chery at intel.com>
> > > > > Cc: Topi Pohjolainen <topi.pohjolainen at 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 at lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > >
> >


More information about the mesa-dev mailing list