On Wed, Jan 24, 2018 at 4:39 PM, Nanley Chery <nanleych...@gmail.com> wrote:
> 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?
>
I think that would work, yes. I can make the change if you prefer. I
hightly doubt there are a lot of depth-1 3D images though. :) I don't
really care as it's going away later anyway.
> > > > + 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
