Re: [Mesa-dev] [PATCH 12/14] anv/cmd_buffer: Mark depth/stencil surfaces written in begin_subpass
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
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
[Mesa-dev] [PATCH 12/14] anv/cmd_buffer: Mark depth/stencil surfaces written in begin_subpass
--- 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) { +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) { +/* 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