[Mesa-dev] [PATCH v2 09/24] anv/cmd_buffer: Generalize transition_color_buffer

[Jason Ekstrand]

On Mon, Jan 22, 2018 at 12:17 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Fri, Jan 19, 2018 at 03:47:26PM -0800, Jason Ekstrand wrote:
> > This moves it to being based on layout_to_aux_usage instead of being
> > hard-coded based on bits of a priori knowledge of how transitions
> > interact with layouts.  This conceptually simplifies things because
> > we're now using layout_to_aux_usage and layout_supports_fast_clear to
> > make resolve decisions so changes to those functions will do what one
> > expects.
> >
> > This fixes a potential bug with window system integration on gen9+ where
> > we wouldn't do a resolve when transitioning to the PRESENT_SRC layout
> > because we just assume that everything that handles CCS_E can handle it
> > all the time.  When handing a CCS_E image off to the window system, we
> > may need to do a full resolve if the window system does not support the
> > CCS_E modifier.  The only reason why this hasn't been a problem yet is
> > because we don't support modifiers in Vulkan WSI and so we always get X
> > tiling which implies no CCS on gen9+.
> >
> > v2 (Jason Ekstrand):
> >  - Make a few more things const
> >  - Use the anv_fast_clear_support enum
> >
> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> >  src/intel/vulkan/genX_cmd_buffer.c | 56 ++++++++++++++++++++++++++++++
> --------
> >  1 file changed, 44 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index 6a6d8b2..fd27463 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -593,6 +593,7 @@ transition_color_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> >                          VkImageLayout initial_layout,
> >                          VkImageLayout final_layout)
> >  {
> > +   const struct gen_device_info *devinfo = &cmd_buffer->device->info;
> >     /* Validate the inputs. */
> >     assert(cmd_buffer);
> >     assert(image && image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV);
> > @@ -733,17 +734,51 @@ transition_color_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> >                                   VK_IMAGE_LAYOUT_COLOR_
> ATTACHMENT_OPTIMAL,
> >                                   final_layout);
> >        }
> > -   } else if (initial_layout != VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL)
> {
> > -      /* Resolves are only necessary if the subresource may contain
> blocks
> > -       * fast-cleared to values unsupported in other layouts. This only
> occurs
> > -       * if the initial layout is COLOR_ATTACHMENT_OPTIMAL.
> > -       */
> > -      return;
> > -   } else if (image->samples > 1) {
> > -      /* MCS buffers don't need resolving. */
> >        return;
> >     }
> >
> > +   /* If initial aux usage is NONE, there is nothing to resolve */
> > +   const enum isl_aux_usage initial_aux_usage =
> > +      anv_layout_to_aux_usage(devinfo, image, aspect, initial_layout);
> > +   if (initial_aux_usage == ISL_AUX_USAGE_NONE)
> > +      return;
> > +
> > +   enum isl_aux_op resolve_op = ISL_AUX_OP_NONE;
> > +
> > +   /* If the initial layout supports more fast clear than the final
> layout
> > +    * then we need at least a partial resolve.
> > +    */
> > +   const enum anv_fast_clear_type initial_fast_clear =
> > +      anv_layout_to_fast_clear_type(devinfo, image, aspect,
> initial_layout);
> > +   const enum anv_fast_clear_type final_fast_clear =
> > +      anv_layout_to_fast_clear_type(devinfo, image, aspect,
> final_layout);
> > +   if (final_fast_clear < initial_fast_clear)
> > +      resolve_op = ISL_AUX_OP_PARTIAL_RESOLVE;
> > +
> > +   const enum isl_aux_usage final_aux_usage =
> > +      anv_layout_to_aux_usage(devinfo, image, aspect, final_layout);
> > +   if (initial_aux_usage == ISL_AUX_USAGE_CCS_E &&
> > +       final_aux_usage != ISL_AUX_USAGE_CCS_E)
> > +      resolve_op = ISL_AUX_OP_FULL_RESOLVE;
> > +
> > +   /* CCS_D only supports full resolves and BLORP will assert on us if
> we try
> > +    * to do a partial resolve on a CCS_D surface.
> > +    */
> > +   if (resolve_op == ISL_AUX_OP_PARTIAL_RESOLVE &&
> > +       initial_aux_usage == ISL_AUX_USAGE_CCS_D)
> > +      resolve_op = ISL_AUX_OP_FULL_RESOLVE;
> > +
> > +   if (resolve_op == ISL_AUX_OP_NONE)
> > +      return;
> > +
> > +   /* Even though the above code can theoretically handle multiple
> resolve
> > +    * types such as CCS_D -> CCS_E, the predication code below can't.
> We only
> > +    * really handle a couple of cases.
> > +    */
> > +   assert(initial_aux_usage == ISL_AUX_USAGE_NONE ||
>
> Earlier there is explicit early return for
> "initial_aux_usage == ISL_AUX_USAGE_NONE". Just checking if you really
> meant
> to assert it here.
>

I don't know.  I'm trying to make the point with the assert of what the
code can handle.  I can't do this assert any earlier because we don't have
final_aux_usage until we get here.


> > +          final_aux_usage == ISL_AUX_USAGE_NONE ||
> > +          initial_aux_usage == final_aux_usage);
> > +
> >     /* Perform a resolve to synchronize data between the main and aux
> buffer.
> >      * Before we begin, we must satisfy the cache flushing requirement
> specified
> >      * in the Sky Lake PRM Vol. 7, "MCS Buffer for Render Target(s)":
> > @@ -774,10 +809,7 @@ transition_color_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> >        genX(load_needs_resolve_predicate)(cmd_buffer, image, aspect,
> level);
> >
> >        anv_image_ccs_op(cmd_buffer, image, aspect, level,
> > -                       base_layer, layer_count,
> > -                       image->planes[plane].aux_usage ==
> ISL_AUX_USAGE_CCS_E ?
> > -                       ISL_AUX_OP_PARTIAL_RESOLVE :
> ISL_AUX_OP_FULL_RESOLVE,
> > -                       true);
> > +                       base_layer, layer_count, resolve_op, true);
> >
> >        genX(set_image_needs_resolve)(cmd_buffer, image, aspect, level,
> false);
> >     }
> > --
> > 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/20180122/c3538b49/attachment-0001.html>