[Mesa-stable] [PATCH v3 02/18] anv: Be more careful about fast-clear colors

[Juan A. Suarez Romero]

On Tue, 2018-03-06 at 08:39 -0800, Jason Ekstrand wrote:
> On Tue, Mar 6, 2018 at 8:30 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> > On Tue, Mar 6, 2018 at 7:49 AM, Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> > > On Wed, 2018-02-14 at 12:16 -0800, Jason Ekstrand wrote:
> > > > Previously, we just used all the channels regardless of the format.
> > > > This is less than ideal because some channels may have undefined values
> > > > and this should be ok from the client's perspective.  Even though the
> > > > driver should do the correct thing regardless of what is in the
> > > > undefined value, it makes things less deterministic.  In particular, the
> > > > driver may choose to fast-clear or not based on undefined values.  This
> > > > level of nondeterminism is bad.
> > > >
> > > > Cc: mesa-stable at lists.freedesktop.org
> > > 
> > > In order to be able to cherry-pick this commit in 17.3.x stable branch,
> > > I'm cherry-picking also:
> > > 
> > > 
> > > commit b1f11450126b0b19754c0cc08103ca98aa89658a
> > > Author: Jason Ekstrand <jason.ekstrand at intel.com>
> > > Date:   Wed Jan 17 21:31:09 2018 -0800
> > > 
> > >     anv: Only fast clear single-slice images
> > > 
> > > 
> > > Even when it was not nominated, I think it is a good idea to include as
> > > part of the stable release.
> > 
> > I don't think that's needed.  I'll try to make a version of the patch which applies to 17.3 and send it to stable.
> > 
> 
> I just sent two patches to stable.  This patch and the ISL patch it depends on.  I haven't actually run them through Jenkins yet but everything looks ok visually.
> 

That's nice. Thank you.


	J.A.

> --Jason
>  
> > --Jason
> >  
> > > Jason, let me know if you are against adding it into stable; if so,
> > > I'll drop both patches.
> > > 
> > > Thanks!
> > > 
> > > 
> > >         J.A.
> > > 
> > > > ---
> > > >  src/intel/vulkan/genX_cmd_buffer.c | 46 ++++++++++++++++----------------------
> > > >  1 file changed, 19 insertions(+), 27 deletions(-)
> > > >
> > > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c
> > > > index ce47b8a..8b1816a 100644
> > > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > > @@ -202,24 +202,6 @@ add_image_view_relocs(struct anv_cmd_buffer *cmd_buffer,
> > > >     }
> > > >  }
> > > >
> > > > -static bool
> > > > -color_is_zero_one(VkClearColorValue value, enum isl_format format)
> > > > -{
> > > > -   if (isl_format_has_int_channel(format)) {
> > > > -      for (unsigned i = 0; i < 4; i++) {
> > > > -         if (value.int32[i] != 0 && value.int32[i] != 1)
> > > > -            return false;
> > > > -      }
> > > > -   } else {
> > > > -      for (unsigned i = 0; i < 4; i++) {
> > > > -         if (value.float32[i] != 0.0f && value.float32[i] != 1.0f)
> > > > -            return false;
> > > > -      }
> > > > -   }
> > > > -
> > > > -   return true;
> > > > -}
> > > > -
> > > >  static void
> > > >  color_attachment_compute_aux_usage(struct anv_device * device,
> > > >                                     struct anv_cmd_state * cmd_state,
> > > > @@ -283,13 +265,25 @@ color_attachment_compute_aux_usage(struct anv_device * device,
> > > >
> > > >     assert(iview->image->planes[0].aux_surface.isl.usage & ISL_SURF_USAGE_CCS_BIT);
> > > >
> > > > +   const struct isl_format_layout *view_fmtl =
> > > > +      isl_format_get_layout(iview->planes[0].isl.format);
> > > > +   union isl_color_value clear_color = {};
> > > > +
> > > > +#define COPY_CLEAR_COLOR_CHANNEL(c, i) \
> > > > +   if (view_fmtl->channels.c.bits) \
> > > > +      clear_color.u32[i] = att_state->clear_value.color.uint32[i]
> > > > +
> > > > +   COPY_CLEAR_COLOR_CHANNEL(r, 0);
> > > > +   COPY_CLEAR_COLOR_CHANNEL(g, 1);
> > > > +   COPY_CLEAR_COLOR_CHANNEL(b, 2);
> > > > +   COPY_CLEAR_COLOR_CHANNEL(a, 3);
> > > > +
> > > > +#undef COPY_CLEAR_COLOR_CHANNEL
> > > > +
> > > >     att_state->clear_color_is_zero_one =
> > > > -      color_is_zero_one(att_state->clear_value.color, iview->planes[0].isl.format);
> > > > +      isl_color_value_is_zero_one(clear_color, iview->planes[0].isl.format);
> > > >     att_state->clear_color_is_zero =
> > > > -      att_state->clear_value.color.uint32[0] == 0 &&
> > > > -      att_state->clear_value.color.uint32[1] == 0 &&
> > > > -      att_state->clear_value.color.uint32[2] == 0 &&
> > > > -      att_state->clear_value.color.uint32[3] == 0;
> > > > +      isl_color_value_is_zero(clear_color, iview->planes[0].isl.format);
> > > >
> > > >     if (att_state->pending_clear_aspects == VK_IMAGE_ASPECT_COLOR_BIT) {
> > > >        /* Start off assuming fast clears are possible */
> > > > @@ -341,10 +335,8 @@ color_attachment_compute_aux_usage(struct anv_device * device,
> > > >                         "LOAD_OP_CLEAR.  Only fast-clearing the first slice");
> > > >        }
> > > >
> > > > -      if (att_state->fast_clear) {
> > > > -         memcpy(fast_clear_color->u32, att_state->clear_value.color.uint32,
> > > > -                sizeof(fast_clear_color->u32));
> > > > -      }
> > > > +      if (att_state->fast_clear)
> > > > +         *fast_clear_color = clear_color;
> > > >     } else {
> > > >        att_state->fast_clear = false;
> > > >     }
> 
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable