[Mesa-dev] [PATCH] radv: ignore the loadOp if the first use of an attachment is a resolve

[Bas Nieuwenhuizen]

On Sun, May 26, 2019 at 2:50 PM Bas Nieuwenhuizen
<bas at basnieuwenhuizen.nl> wrote:
>
> On Wed, May 22, 2019 at 11:20 AM Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
> >
> > Based on ANV.
> >
> > Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> > ---
> >  src/amd/vulkan/radv_meta_resolve.c | 21 ++++++++++++---------
> >  1 file changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/amd/vulkan/radv_meta_resolve.c b/src/amd/vulkan/radv_meta_resolve.c
> > index ade5d438438..6a8abce1ddb 100644
> > --- a/src/amd/vulkan/radv_meta_resolve.c
> > +++ b/src/amd/vulkan/radv_meta_resolve.c
> > @@ -618,15 +618,6 @@ radv_cmd_buffer_resolve_subpass(struct radv_cmd_buffer *cmd_buffer)
> >         struct radv_meta_saved_state saved_state;
> >         enum radv_resolve_method resolve_method = RESOLVE_HW;
> >
> > -       /* FINISHME(perf): Skip clears for resolve attachments.
> > -        *
> > -        * From the Vulkan 1.0 spec:
> > -        *
> > -        *    If the first use of an attachment in a render pass is as a resolve
> > -        *    attachment, then the loadOp is effectively ignored as the resolve is
> > -        *    guaranteed to overwrite all pixels in the render area.
> > -        */
> > -
> >         if (!subpass->has_resolve)
> >                 return;
> >
> > @@ -637,6 +628,18 @@ radv_cmd_buffer_resolve_subpass(struct radv_cmd_buffer *cmd_buffer)
> >                 if (dest_att.attachment == VK_ATTACHMENT_UNUSED)
> >                         continue;
> >
> > +               if (cmd_buffer->state.attachments[dest_att.attachment].pending_clear_aspects) {
> > +                       /* From the Vulkan 1.0 spec:
> > +                        *
> > +                        *    If the first use of an attachment in a render
> > +                        *    pass is as a resolve attachment, then the loadOp
> > +                        *    is effectively ignored as the resolve is
> > +                        *    guaranteed to overwrite all pixels in the render
> > +                        *    area.
> > +                       */
> > +                       cmd_buffer->state.attachments[dest_att.attachment].pending_clear_aspects = 0;
> > +               }
> > +
>
> You can drop the if statement. Also this seems like a correctness
> issue, as we only cleared the first time it was used as a color
> attachment, which might happen after a resolve?

Forgot to say, r-b with the if removed, and maybe want to cc stable?
>
> >                 struct radv_image *dst_img = cmd_buffer->state.framebuffer->attachments[dest_att.attachment].attachment->image;
> >                 struct radv_image *src_img = cmd_buffer->state.framebuffer->attachments[src_att.attachment].attachment->image;
> >
> > --
> > 2.21.0
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev