[Mesa-dev] [PATCH 06/25] anv: Rework the way render target surfaces are allocated

Jason Ekstrand jason at jlekstrand.net
Wed Nov 9 23:57:56 UTC 2016


On Wed, Nov 9, 2016 at 3:22 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> On Tue, Nov 08, 2016 at 06:07:41PM -0800, Jason Ekstrand wrote:
> > On Tue, Nov 8, 2016 at 5:16 PM, Nanley Chery <nanleychery at gmail.com>
> wrote:
> >
> > > On Tue, Nov 08, 2016 at 05:02:29PM -0800, Jason Ekstrand wrote:
> > > > On Tue, Nov 8, 2016 at 5:00 PM, Nanley Chery <nanleychery at gmail.com>
> > > wrote:
> > > >
> > > > > On Tue, Nov 08, 2016 at 04:24:48PM -0800, Jason Ekstrand wrote:
> > > > > > On Tue, Nov 8, 2016 at 3:13 PM, Nanley Chery <
> nanleychery at gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > > On Sat, Oct 22, 2016 at 10:50:37AM -0700, Jason Ekstrand wrote:
> > > > > > > > This commit moves the allocation and filling out of surface
> state
> > > > > from
> > > > > > > > CreateImageView time to BeginRenderPass time.  Instead of
> > > allocating
> > > > > the
> > > > > > > > render target surface state as part of the image view, we
> > > allocate
> > > > > it in
> > > > > > > > the command buffer state at the same time that we set up
> > > clears.  For
> > > > > > > > secondary command buffers, we allocate memory for the surface
> > > states
> > > > > in
> > > > > > > > BeginCommandBuffer but don't fill them out; instead, we use
> our
> > > new
> > > > > > > > SOL-based memcpy function to copy the surface states from the
> > > primary
> > > > > > > > command buffer.  This allows us to handle secondary command
> > > buffers
> > > > > > > without
> > > > > > > > the user specifying the framebuffer ahead-of-time.
> > > > > > > > ---
> > > > > > > >  src/intel/vulkan/anv_cmd_buffer.c  |  56 ----------
> > > > > > > >  src/intel/vulkan/anv_image.c       |  22 ----
> > > > > > > >  src/intel/vulkan/anv_private.h     |  24 ++++-
> > > > > > > >  src/intel/vulkan/genX_cmd_buffer.c | 204
> > > > > +++++++++++++++++++++++++++++-
> > > > > > > -------
> > > > > > > >  4 files changed, 180 insertions(+), 126 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/src/intel/vulkan/anv_cmd_buffer.c
> > > > > > > b/src/intel/vulkan/anv_cmd_buffer.c
> > > > > > > > index a652f9a..372030c 100644
> > > > > > > > --- a/src/intel/vulkan/anv_cmd_buffer.c
> > > > > > > > +++ b/src/intel/vulkan/anv_cmd_buffer.c
> > > > > > > > @@ -144,62 +144,6 @@ anv_cmd_state_reset(struct
> anv_cmd_buffer
> > > > > > > *cmd_buffer)
> > > > > > > >     state->gen7.index_buffer = NULL;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > -/**
> > > > > > > > - * Setup anv_cmd_state::attachments for
> vkCmdBeginRenderPass.
> > > > > > > > - */
> > > > > > > > -void
> > > > > > > > -anv_cmd_state_setup_attachments(struct anv_cmd_buffer
> > > *cmd_buffer,
> > > > > > > > -                                const VkRenderPassBeginInfo
> > > *info)
> > > > > > > > -{
> > > > > > > > -   struct anv_cmd_state *state = &cmd_buffer->state;
> > > > > > > > -   ANV_FROM_HANDLE(anv_render_pass, pass,
> info->renderPass);
> > > > > > > > -
> > > > > > > > -   vk_free(&cmd_buffer->pool->alloc, state->attachments);
> > > > > > > > -
> > > > > > > > -   if (pass->attachment_count == 0) {
> > > > > > > > -      state->attachments = NULL;
> > > > > > > > -      return;
> > > > > > > > -   }
> > > > > > > > -
> > > > > > > > -   state->attachments = vk_alloc(&cmd_buffer->pool->alloc,
> > > > > > > > -                                  pass->attachment_count *
> > > > > > > > -
> > > > >  sizeof(state->attachments[0]),
> > > > > > > > -                                  8,
> VK_SYSTEM_ALLOCATION_SCOPE_
> > > > > > > OBJECT);
> > > > > > > > -   if (state->attachments == NULL) {
> > > > > > > > -      /* FIXME: Propagate VK_ERROR_OUT_OF_HOST_MEMORY to
> > > > > > > vkEndCommandBuffer */
> > > > > > > > -      abort();
> > > > > > > > -   }
> > > > > > > > -
> > > > > > > > -   for (uint32_t i = 0; i < pass->attachment_count; ++i) {
> > > > > > > > -      struct anv_render_pass_attachment *att =
> > > > > &pass->attachments[i];
> > > > > > > > -      VkImageAspectFlags att_aspects =
> > > > > vk_format_aspects(att->format);
> > > > > > > > -      VkImageAspectFlags clear_aspects = 0;
> > > > > > > > -
> > > > > > > > -      if (att_aspects == VK_IMAGE_ASPECT_COLOR_BIT) {
> > > > > > > > -         /* color attachment */
> > > > > > > > -         if (att->load_op == VK_ATTACHMENT_LOAD_OP_CLEAR) {
> > > > > > > > -            clear_aspects |= VK_IMAGE_ASPECT_COLOR_BIT;
> > > > > > > > -         }
> > > > > > > > -      } else {
> > > > > > > > -         /* depthstencil attachment */
> > > > > > > > -         if ((att_aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
> > > > > > > > -             att->load_op == VK_ATTACHMENT_LOAD_OP_CLEAR) {
> > > > > > > > -            clear_aspects |= VK_IMAGE_ASPECT_DEPTH_BIT;
> > > > > > > > -         }
> > > > > > > > -         if ((att_aspects & VK_IMAGE_ASPECT_STENCIL_BIT) &&
> > > > > > > > -             att->stencil_load_op ==
> > > VK_ATTACHMENT_LOAD_OP_CLEAR) {
> > > > > > > > -            clear_aspects |= VK_IMAGE_ASPECT_STENCIL_BIT;
> > > > > > > > -         }
> > > > > > > > -      }
> > > > > > > > -
> > > > > > > > -      state->attachments[i].pending_clear_aspects =
> > > clear_aspects;
> > > > > > > > -      if (clear_aspects) {
> > > > > > > > -         assert(info->clearValueCount > i);
> > > > > > > > -         state->attachments[i].clear_value =
> > > info->pClearValues[i];
> > > > > > > > -      }
> > > > > > > > -   }
> > > > > > > > -}
> > > > > > > > -
> > > > > > > >  VkResult
> > > > > > > >  anv_cmd_buffer_ensure_push_constants_size(struct
> anv_cmd_buffer
> > > > > > > *cmd_buffer,
> > > > > > > >                                            gl_shader_stage
> stage,
> > > > > > > uint32_t size)
> > > > > > > > diff --git a/src/intel/vulkan/anv_image.c
> > > > > b/src/intel/vulkan/anv_image.c
> > > > > > > > index b7c2e99..b014985 100644
> > > > > > > > --- a/src/intel/vulkan/anv_image.c
> > > > > > > > +++ b/src/intel/vulkan/anv_image.c
> > > > > > > > @@ -504,23 +504,6 @@ anv_CreateImageView(VkDevice _device,
> > > > > > > >        iview->sampler_surface_state.alloc_size = 0;
> > > > > > > >     }
> > > > > > > >
> > > > > > > > -   if (image->usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT)
> {
> > > > > > > > -      iview->color_rt_surface_state =
> > > alloc_surface_state(device);
> > > > > > > > -
> > > > > > > > -      struct isl_view view = iview->isl;
> > > > > > > > -      view.usage |= ISL_SURF_USAGE_RENDER_TARGET_BIT;
> > > > > > > > -      isl_surf_fill_state(&device->isl_dev,
> > > > > > > > -                          iview->color_rt_surface_state.
> map,
> > > > > > > > -                          .surf = &surface->isl,
> > > > > > > > -                          .view = &view,
> > > > > > > > -                          .mocs = device->default_mocs);
> > > > > > > > -
> > > > > > > > -      if (!device->info.has_llc)
> > > > > > > > -         anv_state_clflush(iview->color_rt_surface_state);
> > > > > > > > -   } else {
> > > > > > > > -      iview->color_rt_surface_state.alloc_size = 0;
> > > > > > > > -   }
> > > > > > > > -
> > > > > > > >     /* NOTE: This one needs to go last since it may stomp
> > > > > > > isl_view.format */
> > > > > > > >     if (image->usage & VK_IMAGE_USAGE_STORAGE_BIT) {
> > > > > > > >        iview->storage_surface_state =
> > > alloc_surface_state(device);
> > > > > > > > @@ -565,11 +548,6 @@ anv_DestroyImageView(VkDevice _device,
> > > > > VkImageView
> > > > > > > _iview,
> > > > > > > >     ANV_FROM_HANDLE(anv_device, device, _device);
> > > > > > > >     ANV_FROM_HANDLE(anv_image_view, iview, _iview);
> > > > > > > >
> > > > > > > > -   if (iview->color_rt_surface_state.alloc_size > 0) {
> > > > > > > > -      anv_state_pool_free(&device->surface_state_pool,
> > > > > > > > -                          iview->color_rt_surface_state);
> > > > > > > > -   }
> > > > > > > > -
> > > > > > > >     if (iview->sampler_surface_state.alloc_size > 0) {
> > > > > > > >        anv_state_pool_free(&device->surface_state_pool,
> > > > > > > >                            iview->sampler_surface_state);
> > > > > > > > diff --git a/src/intel/vulkan/anv_private.h
> > > b/src/intel/vulkan/anv_
> > > > > > > private.h
> > > > > > > > index a6611f1..2a98ea1 100644
> > > > > > > > --- a/src/intel/vulkan/anv_private.h
> > > > > > > > +++ b/src/intel/vulkan/anv_private.h
> > > > > > > > @@ -1059,6 +1059,8 @@ void anv_dynamic_state_copy(struct
> > > > > > > anv_dynamic_state *dest,
> > > > > > > >   * The clear value is valid only if there exists a pending
> > > clear.
> > > > > > > >   */
> > > > > > > >  struct anv_attachment_state {
> > > > > > > > +   struct anv_state
>  color_rt_state;
> > > > > > > > +
> > > > > > > >     VkImageAspectFlags
> > > > >  pending_clear_aspects;
> > > > > > > >     VkClearValue                                 clear_value;
> > > > > > > >  };
> > > > > > > > @@ -1099,6 +1101,19 @@ struct anv_cmd_state {
> > > > > > > >      */
> > > > > > > >     struct anv_attachment_state *                attachments;
> > > > > > > >
> > > > > > > > +   /**
> > > > > > > > +    * Surface states for color render targets.  These are
> > > stored in
> > > > > a
> > > > > > > single
> > > > > > > > +    * flat array.  For depth-stencil attachments, the
> surface
> > > state
> > > > > is
> > > > > > > simply
> > > > > > > > +    * left blank.
> > > > > > > > +    */
> > > > > > > > +   struct anv_state
> > >  render_pass_states;
> > > > > > > > +
> > > > > > > > +   /**
> > > > > > > > +    * A null surface state of the right size to match the
> > > > > framebuffer.
> > > > > > > This
> > > > > > > > +    * is one of the states in render_pass_states.
> > > > > > > > +    */
> > > > > > > > +   struct anv_state
> > >  null_surface_state;
> > > > > > > > +
> > > > > > > >     struct {
> > > > > > > >        struct anv_buffer *
>  index_buffer;
> > > > > > > >        uint32_t                                  index_type;
> /**<
> > > > > > > 3DSTATE_INDEX_BUFFER.IndexFormat */
> > > > > > > > @@ -1237,8 +1252,10 @@ void gen8_cmd_buffer_emit_depth_
> > > > > viewport(struct
> > > > > > > anv_cmd_buffer *cmd_buffer,
> > > > > > > >                                           bool
> > > depth_clamp_enable);
> > > > > > > >  void gen7_cmd_buffer_emit_scissor(struct anv_cmd_buffer
> > > > > *cmd_buffer);
> > > > > > > >
> > > > > > > > -void anv_cmd_state_setup_attachments(struct anv_cmd_buffer
> > > > > *cmd_buffer,
> > > > > > > > -                                     const
> VkRenderPassBeginInfo
> > > > > *info);
> > > > > > > > +void anv_cmd_buffer_setup_attachments(struct anv_cmd_buffer
> > > > > > > *cmd_buffer,
> > > > > > > > +                                      struct anv_render_pass
> > > *pass,
> > > > > > > > +                                      struct anv_framebuffer
> > > > > > > *framebuffer,
> > > > > > > > +                                      const VkClearValue
> > > > > *clear_values);
> > > > > > > >
> > > > > > > >  struct anv_state
> > > > > > > >  anv_cmd_buffer_push_constants(struct anv_cmd_buffer
> > > *cmd_buffer,
> > > > > > > > @@ -1549,9 +1566,6 @@ struct anv_image_view {
> > > > > > > >     VkFormat vk_format;
> > > > > > > >     VkExtent3D extent; /**< Extent of VkImageViewCreateInfo::
> > > > > baseMipLevel.
> > > > > > > */
> > > > > > > >
> > > > > > > > -   /** RENDER_SURFACE_STATE when using image as a color
> render
> > > > > target.
> > > > > > > */
> > > > > > > > -   struct anv_state color_rt_surface_state;
> > > > > > > > -
> > > > > > > >     /** RENDER_SURFACE_STATE when using image as a sampler
> > > surface.
> > > > > */
> > > > > > > >     struct anv_state sampler_surface_state;
> > > > > > > >
> > > > > > > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > > > > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > > > > > > index 8734389..78b9bcc 100644
> > > > > > > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > > > > > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > > > > > > @@ -25,6 +25,7 @@
> > > > > > > >  #include <stdbool.h>
> > > > > > > >
> > > > > > > >  #include "anv_private.h"
> > > > > > > > +#include "vk_format_info.h"
> > > > > > > >
> > > > > > > >  #include "common/gen_l3_config.h"
> > > > > > > >  #include "genxml/gen_macros.h"
> > > > > > > > @@ -150,6 +151,142 @@ genX(cmd_buffer_emit_state_
> > > > > base_address)(struct
> > > > > > > anv_cmd_buffer *cmd_buffer)
> > > > > > > >     }
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +/**
> > > > > > > > + * Setup anv_cmd_state::attachments for
> vkCmdBeginRenderPass.
> > > > > > > > + */
> > > > > > > > +static void
> > > > > > > > +genX(cmd_buffer_setup_attachments)(struct anv_cmd_buffer
> > > > > *cmd_buffer,
> > > > > > > > +                                   struct anv_render_pass
> *pass,
> > > > > > > > +                                   struct anv_framebuffer
> > > > > *framebuffer,
> > > > > > > > +                                   const VkClearValue
> > > *clear_values)
> > > > > > > > +{
> > > > > > > > +   const struct isl_device *isl_dev =
> > > &cmd_buffer->device->isl_dev;
> > > > > > > > +   struct anv_cmd_state *state = &cmd_buffer->state;
> > > > > > > > +
> > > > > > > > +   vk_free(&cmd_buffer->pool->alloc, state->attachments);
> > > > > > > > +
> > > > > > > > +   if (pass->attachment_count == 0) {
> > > > > > > > +      state->attachments = NULL;
> > > > > > > > +      return;
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   state->attachments = vk_alloc(&cmd_buffer->pool->alloc,
> > > > > > > > +                                 pass->attachment_count *
> > > > > > > > +
> > > sizeof(state->attachments[0]),
> > > > > > > > +                                 8,
> VK_SYSTEM_ALLOCATION_SCOPE_
> > > > > OBJECT);
> > > > > > > > +   if (state->attachments == NULL) {
> > > > > > > > +      /* FIXME: Propagate VK_ERROR_OUT_OF_HOST_MEMORY to
> > > > > > > vkEndCommandBuffer */
> > > > > > > > +      abort();
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   bool need_null_state = false;
> > > > > > > > +   for (uint32_t s = 0; s < pass->subpass_count; ++s) {
> > > > > > > > +      if (pass->subpasses[s].color_count == 0) {
> > > > > > > > +         need_null_state = true;
> > > > > > > > +         break;
> > > > > > > > +      }
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   unsigned num_states = need_null_state;
> > > > > > > > +   for (uint32_t i = 0; i < pass->attachment_count; ++i) {
> > > > > > > > +      if (vk_format_is_color(pass->attachments[i].format))
> > > > > > > > +         num_states++;
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   const uint32_t ss_stride = align_u32(isl_dev->ss.size,
> > > > > > > isl_dev->ss.align);
> > > > > > > > +   state->render_pass_states =
> > > > > > > > +      anv_state_stream_alloc(&cmd_bu
> ffer->surface_state_stream,
> > > > > > > > +                             num_states * ss_stride,
> > > > > isl_dev->ss.align);
> > > > > > > > +
> > > > > > > > +   struct anv_state next_state = state->render_pass_states;
> > > > > > > > +   next_state.alloc_size = isl_dev->ss.size;
> > > > > > > > +
> > > > > > > > +   if (need_null_state) {
> > > > > > > > +      state->null_surface_state = next_state;
> > > > > > > > +      next_state.offset += ss_stride;
> > > > > > > > +      next_state.map += ss_stride;
> > > > > > > > +   }
> > > > > > > > +
> > > > > > > > +   for (uint32_t i = 0; i < pass->attachment_count; ++i) {
> > > > > > > > +      if (vk_format_is_color(pass->attachments[i].format))
> {
> > > > > > > > +         state->attachments[i].color_rt_state = next_state;
> > > > > > > > +         next_state.offset += ss_stride;
> > > > > > > > +         next_state.map += ss_stride;
> > > > > > > > +      }
> > > > > > > > +   }
> > > > > > > > +   assert(next_state.offset ==
> state->render_pass_states.offset
> > > +
> > > > > > > > +                               state->render_pass_states.
> > > > > alloc_size);
> > > > > > > > +
> > > > > > > > +   if (framebuffer) {
> > > > > > > > +      assert(pass->attachment_count ==
> > > > > framebuffer->attachment_count);
> > > > > > > > +
> > > > > > > > +      if (need_null_state) {
> > > > > > > > +         struct GENX(RENDER_SURFACE_STATE) null_ss = {
> > > > > > > > +            .SurfaceType = SURFTYPE_NULL,
> > > > > > > > +            .SurfaceArray = framebuffer->layers > 0,
> > > > > > > > +            .SurfaceFormat = ISL_FORMAT_R8G8B8A8_UNORM,
> > > > > > > > +#if GEN_GEN >= 8
> > > > > > > > +            .TileMode = YMAJOR,
> > > > > > > > +#else
> > > > > > > > +            .TiledSurface = true,
> > > > > > > > +#endif
> > > > > > > > +            .Width = framebuffer->width - 1,
> > > > > > > > +            .Height = framebuffer->height - 1,
> > > > > > > > +            .Depth = framebuffer->layers - 1,
> > > > > > > > +            .RenderTargetViewExtent = framebuffer->layers -
> 1,
> > > > > > > > +         };
> > > > > > > > +         GENX(RENDER_SURFACE_STATE_pack)(NULL,
> > > > > > > state->null_surface_state.map,
> > > > > > > > +                                         &null_ss);
> > > > > > > > +      }
> > > > > > > > +
> > > > > > > > +      for (uint32_t i = 0; i < pass->attachment_count; ++i)
> {
> > > > > > > > +         struct anv_render_pass_attachment *att =
> > > > > &pass->attachments[i];
> > > > > > > > +         VkImageAspectFlags att_aspects =
> > > > > vk_format_aspects(att->format)
> > > > > > > ;
> > > > > > > > +         VkImageAspectFlags clear_aspects = 0;
> > > > > > > > +
> > > > > > > > +         if (att_aspects == VK_IMAGE_ASPECT_COLOR_BIT) {
> > > > > > > > +            /* color attachment */
> > > > > > > > +            if (att->load_op ==
> VK_ATTACHMENT_LOAD_OP_CLEAR) {
> > > > > > > > +               clear_aspects |= VK_IMAGE_ASPECT_COLOR_BIT;
> > > > > > > > +            }
> > > > > > > > +         } else {
> > > > > > > > +            /* depthstencil attachment */
> > > > > > > > +            if ((att_aspects & VK_IMAGE_ASPECT_DEPTH_BIT) &&
> > > > > > > > +                att->load_op ==
> VK_ATTACHMENT_LOAD_OP_CLEAR) {
> > > > > > > > +               clear_aspects |= VK_IMAGE_ASPECT_DEPTH_BIT;
> > > > > > > > +            }
> > > > > > > > +            if ((att_aspects & VK_IMAGE_ASPECT_STENCIL_BIT)
> &&
> > > > > > > > +                att->stencil_load_op ==
> > > > > VK_ATTACHMENT_LOAD_OP_CLEAR) {
> > > > > > > > +               clear_aspects |= VK_IMAGE_ASPECT_STENCIL_BIT;
> > > > > > > > +            }
> > > > > > > > +         }
> > > > > > > > +
> > > > > > > > +         state->attachments[i].pending_clear_aspects =
> > > > > clear_aspects;
> > > > > > > > +         if (clear_aspects)
> > > > > > > > +            state->attachments[i].clear_value =
> > > clear_values[i];
> > > > > > > > +
> > > > > > > > +         struct anv_image_view *iview =
> > > framebuffer->attachments[i];
> > > > > > > > +         assert(iview->image->vk_format == att->format);
> > > > > > >
> > > > > > > This assertion fails when running Dota 2 on your
> wip/anv-null-fb
> > > > > branch.
> > > > > > > I don't know if this branch is the most up-to-date version of
> this
> > > > > > > series, but it was the easiest way to test it (I couldn't get
> the
> > > > > mailing
> > > > > > > list patches to apply with git am). Are you able to reproduce
> this?
> > > > > > >
> > > > > >
> > > > > > That's odd... I haven't seen that and I have run dota2 with CCS
> on
> > > top of
> > > > > > this branch but may be not in a debug build.  What are the two
> > > formats
> > > > > when
> > > > > > it fails?  I suppose it's possible that it's ATTACHMENT_UNUSED or
> > > > > something.
> > > > > >
> > > > >
> > > > > (gdb) p iview->image->vk_format
> > > > > $1 = VK_FORMAT_B8G8R8A8_SRGB
> > > > > (gdb) p att->format
> > > > > $2 = VK_FORMAT_B8G8R8A8_UNORM
> > > > >
> > > >
> > > > Ugh... Looks like that assertion is invalid.  The format of the view
> > > needs
> > > > to match but not necesaraly the format of the image.  I'll change it
> to
> > > > "assert(iview->vk_format == att->format);"
> > > >
> > >
> > > I unfortunately still get a failure with the new assertion:
> > >
> > > (gdb) p iview->vk_format
> > > $1 = VK_FORMAT_D24_UNORM_S8_UINT
> > > (gdb) p att->format
> > > $2 = VK_FORMAT_D32_SFLOAT_S8_UINT
> > >
> >
> > That looks a lot like a real dota2 bug... Those formats aren't even the
> > same size.  If we can confirm that they're really giving us a mismatched
> > render pass and framebuffer, I'll talk to the guys at Valve.  We should
> > also file a bug against the validation layers.
> >
>
> Yes, this looks like a bug indeed. The validation layer seems to catch a
> number of these errors. Here's one for example:
>
> DS(ERROR): object: 0x4e97 type: 19 location: 3207 msgCode: 50: At Draw
> time the active render pass (0x4e7d) is incompatible w/ gfx pipeline
> (0x4e97) that was created w/ render pass (0x3e84) due to: color
> attachments at index 0 of subpass index 0 are not compatible.
>
> My Dota2 may not be up-to-date however, so this problem still needs to
> be confirmed on the latest version.
>

I just ran Dota2 as of some time last week against my wip/anv-null-fb (this
series) with the fixed assertion, and it triggered.  I'll get a hold of my
contact at Valve.  For some reason, the validation layer isn't catching it
for me.  Maybe I don't have it turned on properly.

--Jason


> - Nanley
>
> > --Jason
> >
> >
> > > > --Jason
> > > >
> > > >
> > > > > >
> > > > > > > - Nanley
> > > > > > >
> > > > > > > > +
> > > > > > > > +         if (att_aspects == VK_IMAGE_ASPECT_COLOR_BIT) {
> > > > > > > > +            struct isl_view view = iview->isl;
> > > > > > > > +            view.usage |= ISL_SURF_USAGE_RENDER_TARGET_BIT;
> > > > > > > > +            isl_surf_fill_state(isl_dev,
> > > > > > > > +                                state->attachments[i].color_
> > > > > > > rt_state.map,
> > > > > > > > +                                .surf =
> > > > > &iview->image->color_surface.
> > > > > > > isl,
> > > > > > > > +                                .view = &view,
> > > > > > > > +                                .mocs =
> > > cmd_buffer->device->default_
> > > > > > > mocs);
> > > > > > > > +
> > > > > > > > +            anv_cmd_buffer_add_surface_sta
> te_reloc(cmd_buffer,
> > > > > > > > +               state->attachments[i].color_rt_state,
> iview->bo,
> > > > > > > iview->offset);
> > > > > > > > +         }
> > > > > > > > +      }
> > > > > > > > +
> > > > > > > > +      if (!cmd_buffer->device->info.has_llc)
> > > > > > > > +         anv_state_clflush(state->render_pass_states);
> > > > > > > > +   }
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  VkResult
> > > > > > > >  genX(BeginCommandBuffer)(
> > > > > > > >      VkCommandBuffer
>  commandBuffer,
> > > > > > > > @@ -189,6 +326,9 @@ genX(BeginCommandBuffer)(
> > > > > > > >        cmd_buffer->state.subpass =
> > > > > > > >           &cmd_buffer->state.pass->subpasses[pBeginInfo->
> > > > > > > pInheritanceInfo->subpass];
> > > > > > > >
> > > > > > > > +      genX(cmd_buffer_setup_attachments)(cmd_buffer,
> > > > > > > cmd_buffer->state.pass,
> > > > > > > > +                                         NULL, NULL);
> > > > > > > > +
> > > > > > > >        cmd_buffer->state.dirty |=
> ANV_CMD_DIRTY_RENDER_TARGETS;
> > > > > > > >     }
> > > > > > > >
> > > > > > > > @@ -232,6 +372,22 @@ genX(CmdExecuteCommands)(
> > > > > > > >
> > > > > > > >        assert(secondary->level == VK_COMMAND_BUFFER_LEVEL_
> > > > > SECONDARY);
> > > > > > > >
> > > > > > > > +      if (secondary->usage_flags &
> > > > > > > > +          VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT)
> {
> > > > > > > > +         /* If we're continuing a render pass from the
> primary,
> > > we
> > > > > need
> > > > > > > to
> > > > > > > > +          * copy the surface states for the current subpass
> > > into the
> > > > > > > storage
> > > > > > > > +          * we allocated for them in BeginCommandBuffer.
> > > > > > > > +          */
> > > > > > > > +         struct anv_bo *ss_bo = &primary->device->surface_
> > > > > > > state_block_pool.bo;
> > > > > > > > +         struct anv_state src_state =
> > > primary->state.render_pass_
> > > > > > > states;
> > > > > > > > +         struct anv_state dst_state =
> > > secondary->state.render_pass_
> > > > > > > states;
> > > > > > > > +         assert(src_state.alloc_size ==
> dst_state.alloc_size);
> > > > > > > > +
> > > > > > > > +         genX(cmd_buffer_gpu_memcpy)(primary, ss_bo,
> > > > > dst_state.offset,
> > > > > > > > +                                     ss_bo,
> src_state.offset,
> > > > > > > > +                                     src_state.alloc_size);
> > > > > > > > +      }
> > > > > > > > +
> > > > > > > >        anv_cmd_buffer_add_secondary(primary, secondary);
> > > > > > > >     }
> > > > > > > >
> > > > > > > > @@ -628,43 +784,11 @@ cmd_buffer_alloc_push_constants(struct
> > > > > > > anv_cmd_buffer *cmd_buffer)
> > > > > > > >     cmd_buffer->state.push_constants_dirty |=
> > > > > > > VK_SHADER_STAGE_ALL_GRAPHICS;
> > > > > > > >  }
> > > > > > > >
> > > > > > > > -static struct anv_state
> > > > > > > > -alloc_null_surface_state(struct anv_cmd_buffer *cmd_buffer,
> > > > > > > > -                         struct anv_framebuffer *fb)
> > > > > > > > -{
> > > > > > > > -   struct anv_state state =
> > > > > > > > -      anv_cmd_buffer_alloc_surface_state(cmd_buffer);
> > > > > > > > -
> > > > > > > > -   struct GENX(RENDER_SURFACE_STATE) null_ss = {
> > > > > > > > -      .SurfaceType = SURFTYPE_NULL,
> > > > > > > > -      .SurfaceArray = fb->layers > 0,
> > > > > > > > -      .SurfaceFormat = ISL_FORMAT_R8G8B8A8_UNORM,
> > > > > > > > -#if GEN_GEN >= 8
> > > > > > > > -      .TileMode = YMAJOR,
> > > > > > > > -#else
> > > > > > > > -      .TiledSurface = true,
> > > > > > > > -#endif
> > > > > > > > -      .Width = fb->width - 1,
> > > > > > > > -      .Height = fb->height - 1,
> > > > > > > > -      .Depth = fb->layers - 1,
> > > > > > > > -      .RenderTargetViewExtent = fb->layers - 1,
> > > > > > > > -   };
> > > > > > > > -
> > > > > > > > -   GENX(RENDER_SURFACE_STATE_pack)(NULL, state.map,
> &null_ss);
> > > > > > > > -
> > > > > > > > -   if (!cmd_buffer->device->info.has_llc)
> > > > > > > > -      anv_state_clflush(state);
> > > > > > > > -
> > > > > > > > -   return state;
> > > > > > > > -}
> > > > > > > > -
> > > > > > > > -
> > > > > > > >  static VkResult
> > > > > > > >  emit_binding_table(struct anv_cmd_buffer *cmd_buffer,
> > > > > > > >                     gl_shader_stage stage,
> > > > > > > >                     struct anv_state *bt_state)
> > > > > > > >  {
> > > > > > > > -   struct anv_framebuffer *fb =
> cmd_buffer->state.framebuffer;
> > > > > > > >     struct anv_subpass *subpass = cmd_buffer->state.subpass;
> > > > > > > >     struct anv_pipeline *pipeline;
> > > > > > > >     uint32_t bias, state_offset;
> > > > > > > > @@ -743,17 +867,10 @@ emit_binding_table(struct
> anv_cmd_buffer
> > > > > > > *cmd_buffer,
> > > > > > > >           assert(stage == MESA_SHADER_FRAGMENT);
> > > > > > > >           assert(binding->binding == 0);
> > > > > > > >           if (binding->index < subpass->color_count) {
> > > > > > > > -            const struct anv_image_view *iview =
> > > > > > > > -               fb->attachments[subpass->
> > > color_attachments[binding->
> > > > > > > index]];
> > > > > > > > -
> > > > > > > > -            assert(iview->color_rt_surface
> _state.alloc_size);
> > > > > > > > -            surface_state = iview->color_rt_surface_state;
> > > > > > > > -            anv_cmd_buffer_add_surface_sta
> te_reloc(cmd_buffer,
> > > > > > > surface_state,
> > > > > > > > -
>  iview->bo,
> > > > > > > iview->offset);
> > > > > > > > +            const unsigned att = subpass->color_attachments[
> > > > > > > binding->index];
> > > > > > > > +            surface_state = cmd_buffer->state.attachments[
> > > > > > > att].color_rt_state;
> > > > > > > >           } else {
> > > > > > > > -            /* Null render target */
> > > > > > > > -            struct anv_framebuffer *fb =
> > > > > cmd_buffer->state.framebuffer;
> > > > > > > > -            surface_state = alloc_null_surface_state(cmd_
> > > buffer,
> > > > > fb);
> > > > > > > > +            surface_state = cmd_buffer->state.null_
> > > surface_state;
> > > > > > > >           }
> > > > > > > >
> > > > > > > >           bt_map[bias + s] = surface_state.offset +
> state_offset;
> > > > > > > > @@ -1837,7 +1954,8 @@ void genX(CmdBeginRenderPass)(
> > > > > > > >     cmd_buffer->state.framebuffer = framebuffer;
> > > > > > > >     cmd_buffer->state.pass = pass;
> > > > > > > >     cmd_buffer->state.render_area =
> pRenderPassBegin->renderArea;
> > > > > > > > -   anv_cmd_state_setup_attachments(cmd_buffer,
> > > pRenderPassBegin);
> > > > > > > > +   genX(cmd_buffer_setup_attachments)(cmd_buffer, pass,
> > > > > framebuffer,
> > > > > > > > +                                      pRenderPassBegin->
> > > > > pClearValues);
> > > > > > > >
> > > > > > > >     genX(flush_pipeline_select_3d)(cmd_buffer);
> > > > > > > >
> > > > > > > > --
> > > > > > > > 2.5.0.400.gff86faf
> > > > > > > >
> > > > > > > > _______________________________________________
> > > > > > > > mesa-dev mailing list
> > > > > > > > mesa-dev at lists.freedesktop.org
> > > > > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > > > > >
> > > > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161109/ca24b065/attachment-0001.html>


More information about the mesa-dev mailing list