Re: [Mesa-dev] [PATCH 12/14] anv/cmd_buffer: Mark depth/stencil surfaces written in begin_subpass

2018-02-13 Thread Jason Ekstrand
On Tue, Feb 13, 2018 at 4:47 PM, Nanley Chery  wrote:

> On Mon, Feb 05, 2018 at 02:35:01PM -0800, Jason Ekstrand wrote:
> > ---
> >  src/intel/vulkan/genX_cmd_buffer.c | 50 ++
> 
> >  1 file changed, 29 insertions(+), 21 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index 4eee85a..2d17c28 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -3255,27 +3255,6 @@ cmd_buffer_emit_depth_stencil(struct
> anv_cmd_buffer *cmd_buffer)
> > isl_emit_depth_stencil_hiz_s(&device->isl_dev, dw, &info);
> >
> > cmd_buffer->state.hiz_enabled = info.hiz_usage == ISL_AUX_USAGE_HIZ;
> > -
> > -   /* We may be writing depth or stencil so we need to mark the surface.
> > -* Unfortunately, there's no way to know at this point whether the
> depth or
> > -* stencil tests used will actually write to the surface.
> > -*/
> > -   if (image && (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT)) {
> > -  genX(cmd_buffer_mark_image_written)(cmd_buffer, image,
> > -  VK_IMAGE_ASPECT_DEPTH_BIT,
> > -  info.hiz_usage,
> > -  info.view->base_level,
> > -  info.view->base_array_layer,
> > -  info.view->array_len);
> > -   }
> > -   if (image && (image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT)) {
> > -  genX(cmd_buffer_mark_image_written)(cmd_buffer, image,
> > -  VK_IMAGE_ASPECT_STENCIL_BIT,
> > -  ISL_AUX_USAGE_NONE,
> > -  info.view->base_level,
> > -  info.view->base_array_layer,
> > -  info.view->array_len);
> > -   }
> >  }
> >
> >
> > @@ -3550,6 +3529,35 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer
> *cmd_buffer,
> >   iview->planes[0].isl.base_
> level,
> >   iview->planes[0].isl.base_
> array_layer,
> >   fb->layers);
> > +  } else if (subpass->attachments[i].usage ==
> > + VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
> > + /* We may be writing depth or stencil so we need to mark the
> surface.
> > +  * Unfortunately, there's no way to know at this point whether
> the
> > +  * depth or stencil tests used will actually write to the
> surface.
> > +  *
> > +  * Even though stencil may be plane 1, it always shares a
> base_level
> > +  * with depth.
> > +  */
> > + const struct isl_view *ds_view = &iview->planes[0].isl;
> > + if (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {
>
> I think this should be iview->aspect_mask.
>

Done.


> > +genX(cmd_buffer_mark_image_written)(cmd_buffer, image,
> > +
> VK_IMAGE_ASPECT_DEPTH_BIT,
> > +att_state->aux_usage,
> > +ds_view->base_level,
> > +
> ds_view->base_array_layer,
> > +fb->layers);
> > + }
> > + if (image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT) {
>
> Same comment as above.
>

Done.


> With those two issues fixed, this patch is
> Reviewed-by: Nanley Chery 
>

Thanks!


>
> > +/* Even though stencil may be plane 1, it always shares a
> > + * base_level with depth.
> > + */
> > +genX(cmd_buffer_mark_image_written)(cmd_buffer, image,
> > +
> VK_IMAGE_ASPECT_STENCIL_BIT,
> > +ISL_AUX_USAGE_NONE,
> > +ds_view->base_level,
> > +
> ds_view->base_array_layer,
> > +fb->layers);
> > + }
> >}
> >
> >att_state->pending_clear_aspects = 0;
> > --
> > 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


Re: [Mesa-dev] [PATCH 12/14] anv/cmd_buffer: Mark depth/stencil surfaces written in begin_subpass

2018-02-13 Thread Nanley Chery
On Mon, Feb 05, 2018 at 02:35:01PM -0800, Jason Ekstrand wrote:
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 50 
> ++
>  1 file changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 4eee85a..2d17c28 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -3255,27 +3255,6 @@ cmd_buffer_emit_depth_stencil(struct anv_cmd_buffer 
> *cmd_buffer)
> isl_emit_depth_stencil_hiz_s(&device->isl_dev, dw, &info);
>  
> cmd_buffer->state.hiz_enabled = info.hiz_usage == ISL_AUX_USAGE_HIZ;
> -
> -   /* We may be writing depth or stencil so we need to mark the surface.
> -* Unfortunately, there's no way to know at this point whether the depth 
> or
> -* stencil tests used will actually write to the surface.
> -*/
> -   if (image && (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT)) {
> -  genX(cmd_buffer_mark_image_written)(cmd_buffer, image,
> -  VK_IMAGE_ASPECT_DEPTH_BIT,
> -  info.hiz_usage,
> -  info.view->base_level,
> -  info.view->base_array_layer,
> -  info.view->array_len);
> -   }
> -   if (image && (image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT)) {
> -  genX(cmd_buffer_mark_image_written)(cmd_buffer, image,
> -  VK_IMAGE_ASPECT_STENCIL_BIT,
> -  ISL_AUX_USAGE_NONE,
> -  info.view->base_level,
> -  info.view->base_array_layer,
> -  info.view->array_len);
> -   }
>  }
>  
>  
> @@ -3550,6 +3529,35 @@ cmd_buffer_begin_subpass(struct anv_cmd_buffer 
> *cmd_buffer,
>   iview->planes[0].isl.base_level,
>   
> iview->planes[0].isl.base_array_layer,
>   fb->layers);
> +  } else if (subpass->attachments[i].usage ==
> + VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
> + /* We may be writing depth or stencil so we need to mark the 
> surface.
> +  * Unfortunately, there's no way to know at this point whether the
> +  * depth or stencil tests used will actually write to the surface.
> +  *
> +  * Even though stencil may be plane 1, it always shares a base_level
> +  * with depth.
> +  */
> + const struct isl_view *ds_view = &iview->planes[0].isl;
> + if (image->aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {

I think this should be iview->aspect_mask.

> +genX(cmd_buffer_mark_image_written)(cmd_buffer, image,
> +VK_IMAGE_ASPECT_DEPTH_BIT,
> +att_state->aux_usage,
> +ds_view->base_level,
> +ds_view->base_array_layer,
> +fb->layers);
> + }
> + if (image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT) {

Same comment as above.

With those two issues fixed, this patch is
Reviewed-by: Nanley Chery 


> +/* Even though stencil may be plane 1, it always shares a
> + * base_level with depth.
> + */
> +genX(cmd_buffer_mark_image_written)(cmd_buffer, image,
> +VK_IMAGE_ASPECT_STENCIL_BIT,
> +ISL_AUX_USAGE_NONE,
> +ds_view->base_level,
> +ds_view->base_array_layer,
> +fb->layers);
> + }
>}
>  
>att_state->pending_clear_aspects = 0;
> -- 
> 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