Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-18 Thread Nanley Chery 
On Sat, Jan 13, 2018 at 10:12:53AM -0800, Jason Ekstrand wrote:
> On Sat, Jan 13, 2018 at 9:55 AM, Jason Ekstrand 
> wrote:
> 
> > On Wed, Dec 13, 2017 at 4:46 PM, Nanley Chery 
> > wrote:
> >
> >> On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> >> > Currently, this helper does nothing but we call it every place where an
> >> > image is written through the render pipeline.  This will allow us to
> >> > properly mark the aux state so that we can handle resolves correctly.
> >> > ---
> >> >  src/intel/vulkan/anv_blorp.c   | 36 ++
> >> ++
> >> >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
> >> >  src/intel/vulkan/anv_genX.h|  6 ++
> >> >  src/intel/vulkan/anv_private.h |  7 +++
> >> >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
> >> >  5 files changed, 78 insertions(+)
> >> >
> >> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> >> > index da273d6..46e2eb0 100644
> >> > --- a/src/intel/vulkan/anv_blorp.c
> >> > +++ b/src/intel/vulkan/anv_blorp.c
> >> > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
> >> >
> >> >assert(anv_image_aspects_compatible(src_mask, dst_mask));
> >> >
> >> > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
> >> >if (_mesa_bitcount(src_mask) > 1) {
> >> >   uint32_t aspect_bit;
> >> >   anv_foreach_image_aspect_bit(aspect_bit, src_image,
> >> src_mask) {
> >> > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
> >> >  get_blorp_surf_for_anv_image(cmd_buffer->device,
> >> >   dst_image, 1UL << aspect_bit,
> >> >   ANV_AUX_USAGE_DEFAULT,
> >> _surf);
> >> > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> >> > +  1UL << aspect_bit,
> >> > +  dst_surf.aux_usage,
> >> > +  dst_level);
> >> >
> >> >  for (unsigned i = 0; i < layer_count; i++) {
> >> > blorp_copy(, _surf,
> >> pRegions[r].srcSubresource.mipLevel,
> >> > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
> >> >ANV_AUX_USAGE_DEFAULT,
> >> _surf);
> >> >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image,
> >> dst_mask,
> >> >ANV_AUX_USAGE_DEFAULT,
> >> _surf);
> >> > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> >> dst_mask,
> >> > +   dst_surf.aux_usage,
> >> dst_level);
> >> >
> >> >   for (unsigned i = 0; i < layer_count; i++) {
> >> >  blorp_copy(, _surf,
> >> pRegions[r].srcSubresource.mipLevel,
> >> > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer
> >> *cmd_buffer,
> >> >  extent.width, extent.height,
> >> >  buffer_row_pitch, buffer_format,
> >> >  , _isl_surf);
> >> > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> >> > + assert(dst == );
> >> > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> >> > +   aspect, dst->surf.aux_usage,
> >> > +   dst->level);
> >> > +  }
> >> >
> >> >for (unsigned z = 0; z < extent.depth; z++) {
> >> >   blorp_copy(, >surf, src->level, src->offset.z,
> >> > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
> >> >get_blorp_surf_for_anv_image(cmd_buffer->device,
> >> > dst_image, dst_res->aspectMask,
> >> > ANV_AUX_USAGE_DEFAULT, );
> >> > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> >> > +dst_res->aspectMask,
> >> > +dst.aux_usage,
> >> > +dst_res->mipLevel);
> >> >
> >> >struct anv_format_plane src_format =
> >> >   anv_get_format_plane(_buffer->device->info,
> >> src_image->vk_format,
> >> > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
> >> >  layer_count = anv_minify(image->extent.depth, level);
> >> >   }
> >> >
> >> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> >> > +   pRanges[r].aspectMask,
> >> > +   surf.aux_usage, level);
> >> > +
> >> >   blorp_clear(, ,
> >> >   src_format.isl_format, src_format.swizzle,
> >> >   level, base_layer, layer_count,
> >> > @@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct
> >> anv_cmd_buffer *cmd_buffer)
> >> >

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-13 Thread Jason Ekstrand 
On Sat, Jan 13, 2018 at 9:55 AM, Jason Ekstrand 
wrote:

> On Wed, Dec 13, 2017 at 4:46 PM, Nanley Chery 
> wrote:
>
>> On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
>> > Currently, this helper does nothing but we call it every place where an
>> > image is written through the render pipeline.  This will allow us to
>> > properly mark the aux state so that we can handle resolves correctly.
>> > ---
>> >  src/intel/vulkan/anv_blorp.c   | 36 ++
>> ++
>> >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
>> >  src/intel/vulkan/anv_genX.h|  6 ++
>> >  src/intel/vulkan/anv_private.h |  7 +++
>> >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
>> >  5 files changed, 78 insertions(+)
>> >
>> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
>> > index da273d6..46e2eb0 100644
>> > --- a/src/intel/vulkan/anv_blorp.c
>> > +++ b/src/intel/vulkan/anv_blorp.c
>> > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
>> >
>> >assert(anv_image_aspects_compatible(src_mask, dst_mask));
>> >
>> > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
>> >if (_mesa_bitcount(src_mask) > 1) {
>> >   uint32_t aspect_bit;
>> >   anv_foreach_image_aspect_bit(aspect_bit, src_image,
>> src_mask) {
>> > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
>> >  get_blorp_surf_for_anv_image(cmd_buffer->device,
>> >   dst_image, 1UL << aspect_bit,
>> >   ANV_AUX_USAGE_DEFAULT,
>> _surf);
>> > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
>> > +  1UL << aspect_bit,
>> > +  dst_surf.aux_usage,
>> > +  dst_level);
>> >
>> >  for (unsigned i = 0; i < layer_count; i++) {
>> > blorp_copy(, _surf,
>> pRegions[r].srcSubresource.mipLevel,
>> > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
>> >ANV_AUX_USAGE_DEFAULT,
>> _surf);
>> >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image,
>> dst_mask,
>> >ANV_AUX_USAGE_DEFAULT,
>> _surf);
>> > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
>> dst_mask,
>> > +   dst_surf.aux_usage,
>> dst_level);
>> >
>> >   for (unsigned i = 0; i < layer_count; i++) {
>> >  blorp_copy(, _surf,
>> pRegions[r].srcSubresource.mipLevel,
>> > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer
>> *cmd_buffer,
>> >  extent.width, extent.height,
>> >  buffer_row_pitch, buffer_format,
>> >  , _isl_surf);
>> > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
>> > + assert(dst == );
>> > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
>> > +   aspect, dst->surf.aux_usage,
>> > +   dst->level);
>> > +  }
>> >
>> >for (unsigned z = 0; z < extent.depth; z++) {
>> >   blorp_copy(, >surf, src->level, src->offset.z,
>> > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
>> >get_blorp_surf_for_anv_image(cmd_buffer->device,
>> > dst_image, dst_res->aspectMask,
>> > ANV_AUX_USAGE_DEFAULT, );
>> > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
>> > +dst_res->aspectMask,
>> > +dst.aux_usage,
>> > +dst_res->mipLevel);
>> >
>> >struct anv_format_plane src_format =
>> >   anv_get_format_plane(_buffer->device->info,
>> src_image->vk_format,
>> > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
>> >  layer_count = anv_minify(image->extent.depth, level);
>> >   }
>> >
>> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
>> > +   pRanges[r].aspectMask,
>> > +   surf.aux_usage, level);
>> > +
>> >   blorp_clear(, ,
>> >   src_format.isl_format, src_format.swizzle,
>> >   level, base_layer, layer_count,
>> > @@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct
>> anv_cmd_buffer *cmd_buffer)
>> >  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT |
>> ANV_PIPE_CS_STALL_BIT;
>> >} else {
>> >   assert(image->n_planes == 1);
>> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
>> > +   VK_IMAGE_ASPECT_COLOR_BIT,

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-13 Thread Jason Ekstrand 
On Thu, Jan 11, 2018 at 5:14 PM, Nanley Chery  wrote:

> On Thu, Nov 30, 2017 at 06:20:51PM +0200, Pohjolainen, Topi wrote:
> > On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> > > Currently, this helper does nothing but we call it every place where an
> > > image is written through the render pipeline.  This will allow us to
> > > properly mark the aux state so that we can handle resolves correctly.
> > > ---
> > >  src/intel/vulkan/anv_blorp.c   | 36 ++
> ++
> > >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
> > >  src/intel/vulkan/anv_genX.h|  6 ++
> > >  src/intel/vulkan/anv_private.h |  7 +++
> > >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
> > >  5 files changed, 78 insertions(+)
> > >
> > > diff --git a/src/intel/vulkan/anv_blorp.c
> b/src/intel/vulkan/anv_blorp.c
> > > index da273d6..46e2eb0 100644
> > > --- a/src/intel/vulkan/anv_blorp.c
> > > +++ b/src/intel/vulkan/anv_blorp.c
> > > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
> > >
> > >assert(anv_image_aspects_compatible(src_mask, dst_mask));
> > >
> > > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
> > >if (_mesa_bitcount(src_mask) > 1) {
> > >   uint32_t aspect_bit;
> > >   anv_foreach_image_aspect_bit(aspect_bit, src_image,
> src_mask) {
> > > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
> > >  get_blorp_surf_for_anv_image(cmd_buffer->device,
> > >   dst_image, 1UL << aspect_bit,
> > >   ANV_AUX_USAGE_DEFAULT,
> _surf);
> > > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > > +  1UL << aspect_bit,
> > > +  dst_surf.aux_usage,
> > > +  dst_level);
> > >
> > >  for (unsigned i = 0; i < layer_count; i++) {
> > > blorp_copy(, _surf,
> pRegions[r].srcSubresource.mipLevel,
> > > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
> > >ANV_AUX_USAGE_DEFAULT,
> _surf);
> > >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image,
> dst_mask,
> > >ANV_AUX_USAGE_DEFAULT,
> _surf);
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> dst_mask,
> > > +   dst_surf.aux_usage,
> dst_level);
> > >
> > >   for (unsigned i = 0; i < layer_count; i++) {
> > >  blorp_copy(, _surf, pRegions[r].srcSubresource.
> mipLevel,
> > > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer
> *cmd_buffer,
> > >  extent.width, extent.height,
> > >  buffer_row_pitch, buffer_format,
> > >  , _isl_surf);
> > > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> >
> > In all the other call sites you call anv_cmd_buffer_mark_image_written()
> > regardless if aux usage is none. Is there something special here?
> >
>
> Here we need to call anv_cmd_buffer_mark_image_written() when
> buffer_to_image == true, so there must be an if condition. I think this
> if condition is more direct compared to the alternatives of:
>* if (buffer_to_image)
>* if (dst == image)
>

I know I had some reason but I don't remember exactly what it was.  I think
it was probably an artifact of history combined with me being overly
clever.  I've switched it to the second of Nanley's suggestions.


> -Nanley
>
> > > + assert(dst == );
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> > > +   aspect,
> dst->surf.aux_usage,
> > > +   dst->level);
> > > +  }
> > >
> > >for (unsigned z = 0; z < extent.depth; z++) {
> > >   blorp_copy(, >surf, src->level, src->offset.z,
> > > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
> > >get_blorp_surf_for_anv_image(cmd_buffer->device,
> > > dst_image, dst_res->aspectMask,
> > > ANV_AUX_USAGE_DEFAULT, );
> > > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > > +dst_res->aspectMask,
> > > +dst.aux_usage,
> > > +dst_res->mipLevel);
> > >
> > >struct anv_format_plane src_format =
> > >   anv_get_format_plane(_buffer->device->info,
> src_image->vk_format,
> > > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
> > >  layer_count = anv_minify(image->extent.depth, level);
> > >   }
> > >
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > > +

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-13 Thread Jason Ekstrand 
On Wed, Dec 13, 2017 at 4:46 PM, Nanley Chery  wrote:

> On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> > Currently, this helper does nothing but we call it every place where an
> > image is written through the render pipeline.  This will allow us to
> > properly mark the aux state so that we can handle resolves correctly.
> > ---
> >  src/intel/vulkan/anv_blorp.c   | 36 ++
> ++
> >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
> >  src/intel/vulkan/anv_genX.h|  6 ++
> >  src/intel/vulkan/anv_private.h |  7 +++
> >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
> >  5 files changed, 78 insertions(+)
> >
> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > index da273d6..46e2eb0 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
> >
> >assert(anv_image_aspects_compatible(src_mask, dst_mask));
> >
> > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
> >if (_mesa_bitcount(src_mask) > 1) {
> >   uint32_t aspect_bit;
> >   anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask)
> {
> > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
> >  get_blorp_surf_for_anv_image(cmd_buffer->device,
> >   dst_image, 1UL << aspect_bit,
> >   ANV_AUX_USAGE_DEFAULT,
> _surf);
> > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > +  1UL << aspect_bit,
> > +  dst_surf.aux_usage,
> > +  dst_level);
> >
> >  for (unsigned i = 0; i < layer_count; i++) {
> > blorp_copy(, _surf, pRegions[r].srcSubresource.
> mipLevel,
> > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
> >ANV_AUX_USAGE_DEFAULT, _surf);
> >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image,
> dst_mask,
> >ANV_AUX_USAGE_DEFAULT, _surf);
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> dst_mask,
> > +   dst_surf.aux_usage,
> dst_level);
> >
> >   for (unsigned i = 0; i < layer_count; i++) {
> >  blorp_copy(, _surf, pRegions[r].srcSubresource.
> mipLevel,
> > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer
> *cmd_buffer,
> >  extent.width, extent.height,
> >  buffer_row_pitch, buffer_format,
> >  , _isl_surf);
> > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> > + assert(dst == );
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> > +   aspect, dst->surf.aux_usage,
> > +   dst->level);
> > +  }
> >
> >for (unsigned z = 0; z < extent.depth; z++) {
> >   blorp_copy(, >surf, src->level, src->offset.z,
> > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
> >get_blorp_surf_for_anv_image(cmd_buffer->device,
> > dst_image, dst_res->aspectMask,
> > ANV_AUX_USAGE_DEFAULT, );
> > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > +dst_res->aspectMask,
> > +dst.aux_usage,
> > +dst_res->mipLevel);
> >
> >struct anv_format_plane src_format =
> >   anv_get_format_plane(_buffer->device->info,
> src_image->vk_format,
> > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
> >  layer_count = anv_minify(image->extent.depth, level);
> >   }
> >
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > +   pRanges[r].aspectMask,
> > +   surf.aux_usage, level);
> > +
> >   blorp_clear(, ,
> >   src_format.isl_format, src_format.swizzle,
> >   level, base_layer, layer_count,
> > @@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct
> anv_cmd_buffer *cmd_buffer)
> >  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT |
> ANV_PIPE_CS_STALL_BIT;
> >} else {
> >   assert(image->n_planes == 1);
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > +   VK_IMAGE_ASPECT_COLOR_BIT,
> > +   att_state->aux_usage,
> > +   iview->planes[0].isl.base_
> level);
> > +
> >

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-13 Thread Pohjolainen, Topi 
On Thu, Jan 11, 2018 at 05:14:57PM -0800, Nanley Chery wrote:
> On Thu, Nov 30, 2017 at 06:20:51PM +0200, Pohjolainen, Topi wrote:
> > On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> > > Currently, this helper does nothing but we call it every place where an
> > > image is written through the render pipeline.  This will allow us to
> > > properly mark the aux state so that we can handle resolves correctly.
> > > ---
> > >  src/intel/vulkan/anv_blorp.c   | 36 
> > > 
> > >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
> > >  src/intel/vulkan/anv_genX.h|  6 ++
> > >  src/intel/vulkan/anv_private.h |  7 +++
> > >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
> > >  5 files changed, 78 insertions(+)
> > > 
> > > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > > index da273d6..46e2eb0 100644
> > > --- a/src/intel/vulkan/anv_blorp.c
> > > +++ b/src/intel/vulkan/anv_blorp.c
> > > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
> > >  
> > >assert(anv_image_aspects_compatible(src_mask, dst_mask));
> > >  
> > > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
> > >if (_mesa_bitcount(src_mask) > 1) {
> > >   uint32_t aspect_bit;
> > >   anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask) {
> > > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
> > >  get_blorp_surf_for_anv_image(cmd_buffer->device,
> > >   dst_image, 1UL << aspect_bit,
> > >   ANV_AUX_USAGE_DEFAULT, 
> > > _surf);
> > > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > > +  1UL << aspect_bit,
> > > +  dst_surf.aux_usage,
> > > +  dst_level);
> > >  
> > >  for (unsigned i = 0; i < layer_count; i++) {
> > > blorp_copy(, _surf, 
> > > pRegions[r].srcSubresource.mipLevel,
> > > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
> > >ANV_AUX_USAGE_DEFAULT, _surf);
> > >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image, 
> > > dst_mask,
> > >ANV_AUX_USAGE_DEFAULT, _surf);
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, 
> > > dst_mask,
> > > +   dst_surf.aux_usage, 
> > > dst_level);
> > >  
> > >   for (unsigned i = 0; i < layer_count; i++) {
> > >  blorp_copy(, _surf, 
> > > pRegions[r].srcSubresource.mipLevel,
> > > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer 
> > > *cmd_buffer,
> > >  extent.width, extent.height,
> > >  buffer_row_pitch, buffer_format,
> > >  , _isl_surf);
> > > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> > 
> > In all the other call sites you call anv_cmd_buffer_mark_image_written()
> > regardless if aux usage is none. Is there something special here?
> > 
> 
> Here we need to call anv_cmd_buffer_mark_image_written() when
> buffer_to_image == true, so there must be an if condition. I think this
> if condition is more direct compared to the alternatives of:
>* if (buffer_to_image)
>* if (dst == image)

Right, thanks, I got it now. This also became clearer to me when looking in
a tree.

> 
> -Nanley
> 
> > > + assert(dst == );
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> > > +   aspect, dst->surf.aux_usage,
> > > +   dst->level);
> > > +  }
> > >  
> > >for (unsigned z = 0; z < extent.depth; z++) {
> > >   blorp_copy(, >surf, src->level, src->offset.z,
> > > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
> > >get_blorp_surf_for_anv_image(cmd_buffer->device,
> > > dst_image, dst_res->aspectMask,
> > > ANV_AUX_USAGE_DEFAULT, );
> > > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > > +dst_res->aspectMask,
> > > +dst.aux_usage,
> > > +dst_res->mipLevel);
> > >  
> > >struct anv_format_plane src_format =
> > >   anv_get_format_plane(_buffer->device->info, 
> > > src_image->vk_format,
> > > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
> > >  layer_count = anv_minify(image->extent.depth, level);
> > >   }
> > >  
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > > +   pRanges[r].aspectMask,
> > > +

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-11 Thread Nanley Chery 
On Thu, Nov 30, 2017 at 06:20:51PM +0200, Pohjolainen, Topi wrote:
> On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> > Currently, this helper does nothing but we call it every place where an
> > image is written through the render pipeline.  This will allow us to
> > properly mark the aux state so that we can handle resolves correctly.
> > ---
> >  src/intel/vulkan/anv_blorp.c   | 36 
> > 
> >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
> >  src/intel/vulkan/anv_genX.h|  6 ++
> >  src/intel/vulkan/anv_private.h |  7 +++
> >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
> >  5 files changed, 78 insertions(+)
> > 
> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > index da273d6..46e2eb0 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
> >  
> >assert(anv_image_aspects_compatible(src_mask, dst_mask));
> >  
> > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
> >if (_mesa_bitcount(src_mask) > 1) {
> >   uint32_t aspect_bit;
> >   anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask) {
> > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
> >  get_blorp_surf_for_anv_image(cmd_buffer->device,
> >   dst_image, 1UL << aspect_bit,
> >   ANV_AUX_USAGE_DEFAULT, _surf);
> > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > +  1UL << aspect_bit,
> > +  dst_surf.aux_usage,
> > +  dst_level);
> >  
> >  for (unsigned i = 0; i < layer_count; i++) {
> > blorp_copy(, _surf, 
> > pRegions[r].srcSubresource.mipLevel,
> > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
> >ANV_AUX_USAGE_DEFAULT, _surf);
> >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image, 
> > dst_mask,
> >ANV_AUX_USAGE_DEFAULT, _surf);
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, dst_mask,
> > +   dst_surf.aux_usage, dst_level);
> >  
> >   for (unsigned i = 0; i < layer_count; i++) {
> >  blorp_copy(, _surf, 
> > pRegions[r].srcSubresource.mipLevel,
> > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer *cmd_buffer,
> >  extent.width, extent.height,
> >  buffer_row_pitch, buffer_format,
> >  , _isl_surf);
> > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> 
> In all the other call sites you call anv_cmd_buffer_mark_image_written()
> regardless if aux usage is none. Is there something special here?
> 

Here we need to call anv_cmd_buffer_mark_image_written() when
buffer_to_image == true, so there must be an if condition. I think this
if condition is more direct compared to the alternatives of:
   * if (buffer_to_image)
   * if (dst == image)

-Nanley

> > + assert(dst == );
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> > +   aspect, dst->surf.aux_usage,
> > +   dst->level);
> > +  }
> >  
> >for (unsigned z = 0; z < extent.depth; z++) {
> >   blorp_copy(, >surf, src->level, src->offset.z,
> > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
> >get_blorp_surf_for_anv_image(cmd_buffer->device,
> > dst_image, dst_res->aspectMask,
> > ANV_AUX_USAGE_DEFAULT, );
> > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > +dst_res->aspectMask,
> > +dst.aux_usage,
> > +dst_res->mipLevel);
> >  
> >struct anv_format_plane src_format =
> >   anv_get_format_plane(_buffer->device->info, 
> > src_image->vk_format,
> > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
> >  layer_count = anv_minify(image->extent.depth, level);
> >   }
> >  
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > +   pRanges[r].aspectMask,
> > +   surf.aux_usage, level);
> > +
> >   blorp_clear(, ,
> >   src_format.isl_format, src_format.swizzle,
> >   level, base_layer, layer_count,
> > @@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer 
> > *cmd_buffer)
> >  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT |

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2017-12-13 Thread Nanley Chery 
On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> Currently, this helper does nothing but we call it every place where an
> image is written through the render pipeline.  This will allow us to
> properly mark the aux state so that we can handle resolves correctly.
> ---
>  src/intel/vulkan/anv_blorp.c   | 36 
>  src/intel/vulkan/anv_cmd_buffer.c  | 12 
>  src/intel/vulkan/anv_genX.h|  6 ++
>  src/intel/vulkan/anv_private.h |  7 +++
>  src/intel/vulkan/genX_cmd_buffer.c | 17 +
>  5 files changed, 78 insertions(+)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index da273d6..46e2eb0 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
>  
>assert(anv_image_aspects_compatible(src_mask, dst_mask));
>  
> +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
>if (_mesa_bitcount(src_mask) > 1) {
>   uint32_t aspect_bit;
>   anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask) {
> @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
>  get_blorp_surf_for_anv_image(cmd_buffer->device,
>   dst_image, 1UL << aspect_bit,
>   ANV_AUX_USAGE_DEFAULT, _surf);
> +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> +  1UL << aspect_bit,
> +  dst_surf.aux_usage,
> +  dst_level);
>  
>  for (unsigned i = 0; i < layer_count; i++) {
> blorp_copy(, _surf, 
> pRegions[r].srcSubresource.mipLevel,
> @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
>ANV_AUX_USAGE_DEFAULT, _surf);
>   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image, 
> dst_mask,
>ANV_AUX_USAGE_DEFAULT, _surf);
> + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, dst_mask,
> +   dst_surf.aux_usage, dst_level);
>  
>   for (unsigned i = 0; i < layer_count; i++) {
>  blorp_copy(, _surf, 
> pRegions[r].srcSubresource.mipLevel,
> @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer *cmd_buffer,
>  extent.width, extent.height,
>  buffer_row_pitch, buffer_format,
>  , _isl_surf);
> +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> + assert(dst == );
> + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> +   aspect, dst->surf.aux_usage,
> +   dst->level);
> +  }
>  
>for (unsigned z = 0; z < extent.depth; z++) {
>   blorp_copy(, >surf, src->level, src->offset.z,
> @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
>get_blorp_surf_for_anv_image(cmd_buffer->device,
> dst_image, dst_res->aspectMask,
> ANV_AUX_USAGE_DEFAULT, );
> +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> +dst_res->aspectMask,
> +dst.aux_usage,
> +dst_res->mipLevel);
>  
>struct anv_format_plane src_format =
>   anv_get_format_plane(_buffer->device->info, 
> src_image->vk_format,
> @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
>  layer_count = anv_minify(image->extent.depth, level);
>   }
>  
> + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> +   pRanges[r].aspectMask,
> +   surf.aux_usage, level);
> +
>   blorp_clear(, ,
>   src_format.isl_format, src_format.swizzle,
>   level, base_layer, layer_count,
> @@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer 
> *cmd_buffer)
>  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT | ANV_PIPE_CS_STALL_BIT;
>} else {
>   assert(image->n_planes == 1);
> + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> +   VK_IMAGE_ASPECT_COLOR_BIT,
> +   att_state->aux_usage,
> +   iview->planes[0].isl.base_level);
> +
>   blorp_clear(, , iview->planes[0].isl.format,
>   anv_swizzle_for_render(iview->planes[0].isl.swizzle),
>   iview->planes[0].isl.base_level,
> @@ -1355,6 +1381,8 @@ resolve_image(struct anv_device *device,
>

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2017-11-30 Thread Pohjolainen, Topi 
On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> Currently, this helper does nothing but we call it every place where an
> image is written through the render pipeline.  This will allow us to
> properly mark the aux state so that we can handle resolves correctly.
> ---
>  src/intel/vulkan/anv_blorp.c   | 36 
>  src/intel/vulkan/anv_cmd_buffer.c  | 12 
>  src/intel/vulkan/anv_genX.h|  6 ++
>  src/intel/vulkan/anv_private.h |  7 +++
>  src/intel/vulkan/genX_cmd_buffer.c | 17 +
>  5 files changed, 78 insertions(+)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index da273d6..46e2eb0 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
>  
>assert(anv_image_aspects_compatible(src_mask, dst_mask));
>  
> +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
>if (_mesa_bitcount(src_mask) > 1) {
>   uint32_t aspect_bit;
>   anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask) {
> @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
>  get_blorp_surf_for_anv_image(cmd_buffer->device,
>   dst_image, 1UL << aspect_bit,
>   ANV_AUX_USAGE_DEFAULT, _surf);
> +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> +  1UL << aspect_bit,
> +  dst_surf.aux_usage,
> +  dst_level);
>  
>  for (unsigned i = 0; i < layer_count; i++) {
> blorp_copy(, _surf, 
> pRegions[r].srcSubresource.mipLevel,
> @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
>ANV_AUX_USAGE_DEFAULT, _surf);
>   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image, 
> dst_mask,
>ANV_AUX_USAGE_DEFAULT, _surf);
> + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, dst_mask,
> +   dst_surf.aux_usage, dst_level);
>  
>   for (unsigned i = 0; i < layer_count; i++) {
>  blorp_copy(, _surf, 
> pRegions[r].srcSubresource.mipLevel,
> @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer *cmd_buffer,
>  extent.width, extent.height,
>  buffer_row_pitch, buffer_format,
>  , _isl_surf);
> +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {

In all the other call sites you call anv_cmd_buffer_mark_image_written()
regardless if aux usage is none. Is there something special here?

> + assert(dst == );
> + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> +   aspect, dst->surf.aux_usage,
> +   dst->level);
> +  }
>  
>for (unsigned z = 0; z < extent.depth; z++) {
>   blorp_copy(, >surf, src->level, src->offset.z,
> @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
>get_blorp_surf_for_anv_image(cmd_buffer->device,
> dst_image, dst_res->aspectMask,
> ANV_AUX_USAGE_DEFAULT, );
> +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> +dst_res->aspectMask,
> +dst.aux_usage,
> +dst_res->mipLevel);
>  
>struct anv_format_plane src_format =
>   anv_get_format_plane(_buffer->device->info, 
> src_image->vk_format,
> @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
>  layer_count = anv_minify(image->extent.depth, level);
>   }
>  
> + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> +   pRanges[r].aspectMask,
> +   surf.aux_usage, level);
> +
>   blorp_clear(, ,
>   src_format.isl_format, src_format.swizzle,
>   level, base_layer, layer_count,
> @@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer 
> *cmd_buffer)
>  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT | ANV_PIPE_CS_STALL_BIT;
>} else {
>   assert(image->n_planes == 1);
> + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> +   VK_IMAGE_ASPECT_COLOR_BIT,
> +   att_state->aux_usage,
> +   iview->planes[0].isl.base_level);
> +
>   blorp_clear(, , iview->planes[0].isl.format,
>   anv_swizzle_for_render(iview->planes[0].isl.swizzle),
>

[Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2017-11-27 Thread Jason Ekstrand 
Currently, this helper does nothing but we call it every place where an
image is written through the render pipeline.  This will allow us to
properly mark the aux state so that we can handle resolves correctly.
---
 src/intel/vulkan/anv_blorp.c   | 36 
 src/intel/vulkan/anv_cmd_buffer.c  | 12 
 src/intel/vulkan/anv_genX.h|  6 ++
 src/intel/vulkan/anv_private.h |  7 +++
 src/intel/vulkan/genX_cmd_buffer.c | 17 +
 5 files changed, 78 insertions(+)

diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index da273d6..46e2eb0 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -271,6 +271,7 @@ void anv_CmdCopyImage(
 
   assert(anv_image_aspects_compatible(src_mask, dst_mask));
 
+  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
   if (_mesa_bitcount(src_mask) > 1) {
  uint32_t aspect_bit;
  anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask) {
@@ -281,6 +282,10 @@ void anv_CmdCopyImage(
 get_blorp_surf_for_anv_image(cmd_buffer->device,
  dst_image, 1UL << aspect_bit,
  ANV_AUX_USAGE_DEFAULT, _surf);
+anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
+  1UL << aspect_bit,
+  dst_surf.aux_usage,
+  dst_level);
 
 for (unsigned i = 0; i < layer_count; i++) {
blorp_copy(, _surf, 
pRegions[r].srcSubresource.mipLevel,
@@ -298,6 +303,8 @@ void anv_CmdCopyImage(
   ANV_AUX_USAGE_DEFAULT, _surf);
  get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image, dst_mask,
   ANV_AUX_USAGE_DEFAULT, _surf);
+ anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, dst_mask,
+   dst_surf.aux_usage, dst_level);
 
  for (unsigned i = 0; i < layer_count; i++) {
 blorp_copy(, _surf, pRegions[r].srcSubresource.mipLevel,
@@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer *cmd_buffer,
 extent.width, extent.height,
 buffer_row_pitch, buffer_format,
 , _isl_surf);
+  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
+ assert(dst == );
+ anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
+   aspect, dst->surf.aux_usage,
+   dst->level);
+  }
 
   for (unsigned z = 0; z < extent.depth; z++) {
  blorp_copy(, >surf, src->level, src->offset.z,
@@ -497,6 +510,10 @@ void anv_CmdBlitImage(
   get_blorp_surf_for_anv_image(cmd_buffer->device,
dst_image, dst_res->aspectMask,
ANV_AUX_USAGE_DEFAULT, );
+  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
+dst_res->aspectMask,
+dst.aux_usage,
+dst_res->mipLevel);
 
   struct anv_format_plane src_format =
  anv_get_format_plane(_buffer->device->info, src_image->vk_format,
@@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
 layer_count = anv_minify(image->extent.depth, level);
  }
 
+ anv_cmd_buffer_mark_image_written(cmd_buffer, image,
+   pRanges[r].aspectMask,
+   surf.aux_usage, level);
+
  blorp_clear(, ,
  src_format.isl_format, src_format.swizzle,
  level, base_layer, layer_count,
@@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer 
*cmd_buffer)
 ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT | ANV_PIPE_CS_STALL_BIT;
   } else {
  assert(image->n_planes == 1);
+ anv_cmd_buffer_mark_image_written(cmd_buffer, image,
+   VK_IMAGE_ASPECT_COLOR_BIT,
+   att_state->aux_usage,
+   iview->planes[0].isl.base_level);
+
  blorp_clear(, , iview->planes[0].isl.format,
  anv_swizzle_for_render(iview->planes[0].isl.swizzle),
  iview->planes[0].isl.base_level,
@@ -1355,6 +1381,8 @@ resolve_image(struct anv_device *device,
   uint32_t src_x, uint32_t src_y, uint32_t dst_x, uint32_t dst_y,
   uint32_t width, uint32_t height)
 {
+   struct anv_cmd_buffer *cmd_buffer = batch->driver_batch;
+
assert(src_image->type == VK_IMAGE_TYPE_2D);
assert(src_image->samples > 1);