On Wed, Jan 24, 2018 at 4:04 PM, Nanley Chery <nanleych...@gmail.com> wrote:
> On Fri, Jan 19, 2018 at 03:47:30PM -0800, Jason Ekstrand wrote:
> > The current strategy we use for managing resolves has an issues where we
> > track clear colors and the need for resolves per-LOD but we still allow
> > resolves of only a subset of the slices in any given LOD and doing so
> > sets the "needs resolve" flag for that LOD to false while leaving the
> > remaining layers unresolved. This patch is only the first step and does
> > not, by itself fix anything. However, it's fairly self-contained and
> > splitting it out means any performance regressions should bisect to this
> > nice obvious commit rather than to the giant "rework aux tracking"
> > commit.
> >
> > Nanley and I did some testing and none of the applications we tested
> > even tried to fast-clear anything other than the first slice of an
> > image. The applications tested were:
> >
>
> Thanks for documenting the tested apps! We could be a tad more precise
> by noting that we didn't check if an app cleared a subset of its
> layers.. though even if they did, we wouldn't try to optimize for it so
> it doesn't really matter.
>
Sure, I can make the comment a bit more descriptive:
Nanley and I did some testing and none of the applications we tested
even tried to fast-clear anything other than the first slice of an
image. The test was done by adding a printf right before we call
blorp_fast_clear if we were every going to touch any slice other than
the first with a fast-clear. Due to the way the original code was
structured, this would not have included applications which only cleared
a subset of layers. The applications tested were:
> > * All Sascha Willems demos
> > * Aztec Ruins
> > * Dota 2
> > * The Talos Principle
> > * Mad Max
> >
>
> I'm also seeing the same behavior in the following apps:
> * Warhammer 40,000: Dawn of War III
> * Serious Sam Fusion 2017: BFE
>
> Feel free to add them on.
>
Will do. Thanks!
> > While not the full list of shipping applications, it's a pretty good
> > spread and covers most of the engines we've seen running on our driver.
> > If this is ever shown to be a performance problem in the future, we can
> > reconsider our strategy.
> > ---
> > src/intel/vulkan/genX_cmd_buffer.c | 34 +++++++++++++++++-------------
> ----
> > 1 file changed, 17 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index ad7b9fc..0f56719 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -309,23 +309,6 @@ color_attachment_compute_aux_usage(struct
> anv_device * device,
> > if (GEN_GEN <= 8 && !att_state->clear_color_is_zero_one)
> > att_state->fast_clear = false;
> >
> > - /* We allow fast clears when all aux layers of the miplevel are
> targeted.
> > - * See add_fast_clear_state_buffer() for more information. Also,
> because
> > - * we only either do a fast clear or a normal clear and not both,
> this
> > - * complies with the gen7 restriction of not fast-clearing
> multiple
> > - * layers.
> > - */
> > - if (cmd_state->framebuffer->layers !=
> > - anv_image_aux_layers(iview->image, VK_IMAGE_ASPECT_COLOR_BIT,
> > - iview->planes[0].isl.base_level)) {
> > - att_state->fast_clear = false;
> > - if (GEN_GEN == 7) {
> > - anv_perf_warn(device->instance, iview->image,
> > - "Not fast-clearing the first layer in "
> > - "a multi-layer fast clear.");
> > - }
> > - }
> > -
> > /* We only allow fast clears in the GENERAL layout if the
> auxiliary
> > * buffer is always enabled and the fast-clear value is all 0's.
> See
> > * add_fast_clear_state_buffer() for more information.
> > @@ -337,6 +320,23 @@ color_attachment_compute_aux_usage(struct
> anv_device * device,
> > att_state->fast_clear = false;
> > }
> >
> > + /* We only allow fast clears to the first slice of an image
> (level 0,
> > + * layer 0) and only for the entire slice. This guarantees us
> that, at
> > + * any given time, there is only one clear color on any given
> image at
> > + * any given time. At the time of our testing (Jan 17, 2018),
> there
> > + * were known applications which would benefit from fast-clearing
> more
> ^
> no
>
Yup. Fixed locally.
> > + * than just the first slice.
> > + */
> > + if (att_state->fast_clear &&
> > + (iview->planes[0].isl.base_level > 0 ||
> > + iview->image->type == VK_IMAGE_TYPE_3D ||
> > + iview->image->array_size > 0)) {
>
> These conditions seem too restrictive. Instead of restricting
> fast-clears to the first slice of an image, we're restricting
> fast-clears to images with only one layer. Also, the imageview could
> point to the first slice even if the base image is 3D.
>
Yes, it is. We need to restrict it this far due to issues with resolve
tracking. The last patch in the series lifts the restriction back to the
point we originally discussed of clearing the first slice of 3D and array
textures.
> > + anv_perf_warn(device->instance, iview->image,
> > + "Rendering to a multi-LOD or multi-layer
> framebuffer "
> > + "with LOAD_OP_CLEAR. Not fast-clearing");
> > + att_state->fast_clear = false;
> > + }
> > +
> > if (att_state->fast_clear) {
> > memcpy(fast_clear_color->u32, att_state->clear_value.color.
> uint32,
> > sizeof(fast_clear_color->u32));
> > --
> > 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
