Re: [Mesa-dev] [PATCH] anv: Do color resolve tracking one slice at a time for 3D images
I've pushed a new branch with just this series (not the GENERAL layout
stuff):
https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/anv-resolve-rework
On Fri, Feb 2, 2018 at 11:04 PM, Jason Ekstrand
wrote:
> On Fri, Feb 2, 2018 at 6:55 PM, Nanley Chery
> wrote:
>
>> On Thu, Feb 01, 2018 at 06:31:18PM -0800, Jason Ekstrand wrote:
>> > ---
>> > src/intel/vulkan/anv_image.c | 14 +-
>>
>> We should also update the comment in anv_image that describes 3D as
>> having one slice per LOD.
>>
>
> Yup. Fixed locally. I found a couple more bugs with my branch to do aux
> in GENERAL layout. That one suddenly forced more resolves and pointed out
> some 3D bugs the other didn't.
>
>
>> > src/intel/vulkan/anv_private.h | 9 -
>> > src/intel/vulkan/genX_cmd_buffer.c | 34 --
>>
>> > 3 files changed, 33 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
>> > index 6008e3c..a3e857c 100644
>> > --- a/src/intel/vulkan/anv_image.c
>> > +++ b/src/intel/vulkan/anv_image.c
>> > @@ -262,11 +262,15 @@ add_aux_state_tracking_buffer(struct anv_image
>> *image,
>> > /* Clear color and fast clear type */
>> > unsigned state_size = device->isl_dev.ss.clear_value_size + 4;
>> >
>> > - /* We only need to track compression on CCS_E surfaces. We don't
>> consider
>> > -* 3D images as actually having multiple array layers.
>> > -*/
>> > - if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E)
>> > - state_size += image->levels * image->array_size * 4;
>> > + /* We only need to track compression on CCS_E surfaces. */
>> > + if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E) {
>> > + if (image->type == VK_IMAGE_TYPE_3D) {
>> > + for (uint32_t l = 0; l < image->levels; l++)
>> > +state_size += anv_minify(image->extent.depth, l) * 4;
>> > + } else {
>> > + state_size += image->levels * image->array_size * 4;
>> > + }
>> > + }
>> >
>> > image->planes[plane].fast_clear_state_offset =
>> >image->planes[plane].offset + image->planes[plane].size;
>> > diff --git a/src/intel/vulkan/anv_private.h
>> b/src/intel/vulkan/anv_private.h
>> > index 0cd94bf..f208618 100644
>> > --- a/src/intel/vulkan/anv_private.h
>> > +++ b/src/intel/vulkan/anv_private.h
>> > @@ -2573,8 +2573,15 @@ anv_image_get_compression_state_addr(const
>> struct anv_device *device,
>> > struct anv_address addr =
>> >anv_image_get_fast_clear_type_addr(device, image, aspect);
>> > addr.offset += 4; /* Go past the fast clear type */
>> > - addr.offset += level * image->array_size * 4;
>> > +
>> > + if (image->type == VK_IMAGE_TYPE_3D) {
>> > + for (uint32_t l = 0; l < image->levels; l++)
>> > + addr.offset += anv_minify(image->extent.depth, l) * 4;
>> > + } else {
>> > + addr.offset += level * image->array_size * 4;
>> > + }
>> > addr.offset += array_layer * 4;
>> > +
>> > return addr;
>> > }
>> >
>> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
>> b/src/intel/vulkan/genX_cmd_buffer.c
>> > index e29228d..b4b6b7d 100644
>> > --- a/src/intel/vulkan/genX_cmd_buffer.c
>> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
>> > @@ -632,14 +632,8 @@ anv_cmd_predicated_ccs_resolve(struct
>> anv_cmd_buffer *cmd_buffer,
>> >mip.CompareOperation = COMPARE_SRCS_EQUAL;
>> > }
>> >
>> > - if (image->type == VK_IMAGE_TYPE_3D) {
>> > - anv_image_ccs_op(cmd_buffer, image, aspect, level,
>> > - 0, anv_minify(image->extent.depth, level),
>> > - resolve_op, true);
>> > - } else {
>> > - anv_image_ccs_op(cmd_buffer, image, aspect, level,
>> > - array_layer, 1, resolve_op, true);
>> > - }
>> > + anv_image_ccs_op(cmd_buffer, image, aspect, level,
>> > +array_layer, 1, resolve_op, true);
>> > }
>> >
>> > void
>> > @@ -836,9 +830,6 @@ transition_color_buffer(struct anv_cmd_buffer
>> *cmd_buffer,
>> > base_layer, layer_count);
>> > }
>> >
>> > - if (image->type == VK_IMAGE_TYPE_3D)
>> > - base_layer = 0;
>> > -
>> > if (base_layer >= anv_image_aux_layers(image, aspect, base_level))
>> >return;
>> >
>> > @@ -897,10 +888,6 @@ transition_color_buffer(struct anv_cmd_buffer
>> *cmd_buffer,
>> > uint32_t level_layer_count =
>> > MIN2(layer_count, anv_image_aux_layers(image, aspect,
>> level));
>> >
>> > -/* A transition of a 3D subresource works on all slices. */
>> > -if (image->type == VK_IMAGE_TYPE_3D)
>> > - level_layer_count = anv_minify(image->extent.depth,
>> level);
>> > -
>> > anv_image_ccs_op(cmd_buffer, image, aspect, level,
>> > base_layer, level_layer_count,
>> > ISL_AUX_OP_AMBIGUATE, false);
>> > @@ -994,7 +981,10 @@ tra
Re: [Mesa-dev] [PATCH] anv: Do color resolve tracking one slice at a time for 3D images
On Fri, Feb 2, 2018 at 6:55 PM, Nanley Chery wrote:
> On Thu, Feb 01, 2018 at 06:31:18PM -0800, Jason Ekstrand wrote:
> > ---
> > src/intel/vulkan/anv_image.c | 14 +-
>
> We should also update the comment in anv_image that describes 3D as
> having one slice per LOD.
>
Yup. Fixed locally. I found a couple more bugs with my branch to do aux
in GENERAL layout. That one suddenly forced more resolves and pointed out
some 3D bugs the other didn't.
> > src/intel/vulkan/anv_private.h | 9 -
> > src/intel/vulkan/genX_cmd_buffer.c | 34 --
>
> > 3 files changed, 33 insertions(+), 24 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> > index 6008e3c..a3e857c 100644
> > --- a/src/intel/vulkan/anv_image.c
> > +++ b/src/intel/vulkan/anv_image.c
> > @@ -262,11 +262,15 @@ add_aux_state_tracking_buffer(struct anv_image
> *image,
> > /* Clear color and fast clear type */
> > unsigned state_size = device->isl_dev.ss.clear_value_size + 4;
> >
> > - /* We only need to track compression on CCS_E surfaces. We don't
> consider
> > -* 3D images as actually having multiple array layers.
> > -*/
> > - if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E)
> > - state_size += image->levels * image->array_size * 4;
> > + /* We only need to track compression on CCS_E surfaces. */
> > + if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E) {
> > + if (image->type == VK_IMAGE_TYPE_3D) {
> > + for (uint32_t l = 0; l < image->levels; l++)
> > +state_size += anv_minify(image->extent.depth, l) * 4;
> > + } else {
> > + state_size += image->levels * image->array_size * 4;
> > + }
> > + }
> >
> > image->planes[plane].fast_clear_state_offset =
> >image->planes[plane].offset + image->planes[plane].size;
> > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> > index 0cd94bf..f208618 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -2573,8 +2573,15 @@ anv_image_get_compression_state_addr(const
> struct anv_device *device,
> > struct anv_address addr =
> >anv_image_get_fast_clear_type_addr(device, image, aspect);
> > addr.offset += 4; /* Go past the fast clear type */
> > - addr.offset += level * image->array_size * 4;
> > +
> > + if (image->type == VK_IMAGE_TYPE_3D) {
> > + for (uint32_t l = 0; l < image->levels; l++)
> > + addr.offset += anv_minify(image->extent.depth, l) * 4;
> > + } else {
> > + addr.offset += level * image->array_size * 4;
> > + }
> > addr.offset += array_layer * 4;
> > +
> > return addr;
> > }
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index e29228d..b4b6b7d 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -632,14 +632,8 @@ anv_cmd_predicated_ccs_resolve(struct
> anv_cmd_buffer *cmd_buffer,
> >mip.CompareOperation = COMPARE_SRCS_EQUAL;
> > }
> >
> > - if (image->type == VK_IMAGE_TYPE_3D) {
> > - anv_image_ccs_op(cmd_buffer, image, aspect, level,
> > - 0, anv_minify(image->extent.depth, level),
> > - resolve_op, true);
> > - } else {
> > - anv_image_ccs_op(cmd_buffer, image, aspect, level,
> > - array_layer, 1, resolve_op, true);
> > - }
> > + anv_image_ccs_op(cmd_buffer, image, aspect, level,
> > +array_layer, 1, resolve_op, true);
> > }
> >
> > void
> > @@ -836,9 +830,6 @@ transition_color_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> > base_layer, layer_count);
> > }
> >
> > - if (image->type == VK_IMAGE_TYPE_3D)
> > - base_layer = 0;
> > -
> > if (base_layer >= anv_image_aux_layers(image, aspect, base_level))
> >return;
> >
> > @@ -897,10 +888,6 @@ transition_color_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> > uint32_t level_layer_count =
> > MIN2(layer_count, anv_image_aux_layers(image, aspect,
> level));
> >
> > -/* A transition of a 3D subresource works on all slices. */
> > -if (image->type == VK_IMAGE_TYPE_3D)
> > - level_layer_count = anv_minify(image->extent.depth,
> level);
> > -
> > anv_image_ccs_op(cmd_buffer, image, aspect, level,
> > base_layer, level_layer_count,
> > ISL_AUX_OP_AMBIGUATE, false);
> > @@ -994,7 +981,10 @@ transition_color_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> >
> > for (uint32_t l = 0; l < level_count; l++) {
> >uint32_t level = base_level + l;
> > - for (uint32_t a = 0; a < layer_count; a++) {
> > + uint32_t level_layer_count =
> > + MIN2(layer_count, anv_image_aux_layers(image, aspect, level));
> > +
Re: [Mesa-dev] [PATCH] anv: Do color resolve tracking one slice at a time for 3D images
On Thu, Feb 01, 2018 at 06:31:18PM -0800, Jason Ekstrand wrote:
> ---
> src/intel/vulkan/anv_image.c | 14 +-
We should also update the comment in anv_image that describes 3D as
having one slice per LOD.
> src/intel/vulkan/anv_private.h | 9 -
> src/intel/vulkan/genX_cmd_buffer.c | 34 --
> 3 files changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index 6008e3c..a3e857c 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -262,11 +262,15 @@ add_aux_state_tracking_buffer(struct anv_image *image,
> /* Clear color and fast clear type */
> unsigned state_size = device->isl_dev.ss.clear_value_size + 4;
>
> - /* We only need to track compression on CCS_E surfaces. We don't consider
> -* 3D images as actually having multiple array layers.
> -*/
> - if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E)
> - state_size += image->levels * image->array_size * 4;
> + /* We only need to track compression on CCS_E surfaces. */
> + if (image->planes[plane].aux_usage == ISL_AUX_USAGE_CCS_E) {
> + if (image->type == VK_IMAGE_TYPE_3D) {
> + for (uint32_t l = 0; l < image->levels; l++)
> +state_size += anv_minify(image->extent.depth, l) * 4;
> + } else {
> + state_size += image->levels * image->array_size * 4;
> + }
> + }
>
> image->planes[plane].fast_clear_state_offset =
>image->planes[plane].offset + image->planes[plane].size;
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index 0cd94bf..f208618 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -2573,8 +2573,15 @@ anv_image_get_compression_state_addr(const struct
> anv_device *device,
> struct anv_address addr =
>anv_image_get_fast_clear_type_addr(device, image, aspect);
> addr.offset += 4; /* Go past the fast clear type */
> - addr.offset += level * image->array_size * 4;
> +
> + if (image->type == VK_IMAGE_TYPE_3D) {
> + for (uint32_t l = 0; l < image->levels; l++)
> + addr.offset += anv_minify(image->extent.depth, l) * 4;
> + } else {
> + addr.offset += level * image->array_size * 4;
> + }
> addr.offset += array_layer * 4;
> +
> return addr;
> }
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index e29228d..b4b6b7d 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -632,14 +632,8 @@ anv_cmd_predicated_ccs_resolve(struct anv_cmd_buffer
> *cmd_buffer,
>mip.CompareOperation = COMPARE_SRCS_EQUAL;
> }
>
> - if (image->type == VK_IMAGE_TYPE_3D) {
> - anv_image_ccs_op(cmd_buffer, image, aspect, level,
> - 0, anv_minify(image->extent.depth, level),
> - resolve_op, true);
> - } else {
> - anv_image_ccs_op(cmd_buffer, image, aspect, level,
> - array_layer, 1, resolve_op, true);
> - }
> + anv_image_ccs_op(cmd_buffer, image, aspect, level,
> +array_layer, 1, resolve_op, true);
> }
>
> void
> @@ -836,9 +830,6 @@ transition_color_buffer(struct anv_cmd_buffer *cmd_buffer,
> base_layer, layer_count);
> }
>
> - if (image->type == VK_IMAGE_TYPE_3D)
> - base_layer = 0;
> -
> if (base_layer >= anv_image_aux_layers(image, aspect, base_level))
>return;
>
> @@ -897,10 +888,6 @@ transition_color_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> uint32_t level_layer_count =
> MIN2(layer_count, anv_image_aux_layers(image, aspect, level));
>
> -/* A transition of a 3D subresource works on all slices. */
> -if (image->type == VK_IMAGE_TYPE_3D)
> - level_layer_count = anv_minify(image->extent.depth, level);
> -
> anv_image_ccs_op(cmd_buffer, image, aspect, level,
> base_layer, level_layer_count,
> ISL_AUX_OP_AMBIGUATE, false);
> @@ -994,7 +981,10 @@ transition_color_buffer(struct anv_cmd_buffer
> *cmd_buffer,
>
> for (uint32_t l = 0; l < level_count; l++) {
>uint32_t level = base_level + l;
> - for (uint32_t a = 0; a < layer_count; a++) {
> + uint32_t level_layer_count =
> + MIN2(layer_count, anv_image_aux_layers(image, aspect, level));
> +
> + for (uint32_t a = 0; a < level_layer_count; a++) {
> uint32_t array_layer = base_layer + a;
> anv_cmd_predicated_ccs_resolve(cmd_buffer, image, aspect,
> level, array_layer, resolve_op,
> @@ -1663,12 +1653,20 @@ void genX(CmdPipelineBarrier)(
> anv_image_expand_aspects(image, range->aspectMask);
> uint32_t aspect_bit;
>
> + uint32_t ba
