On Wed, Jan 24, 2018 at 04:15:33PM -0800, Jason Ekstrand wrote:
> 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:
>
>
>
That works.
> > > * 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.
>
>
Right, I must've skimmed the commit title earlier.
Changing: iview->image->type == VK_IMAGE_TYPE_3D
to: iview->image->extent.depth > 1
is more what we want right?
> > > + 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
