[prev in list] [next in list] [prev in thread] [next in thread]
List: mesa3d-dev
Subject: Re: [Mesa-dev] [PATCH v2] anv/blorp: Do more flushing around HiZ clears
From: Jason Ekstrand <jason () jlekstrand ! net>
Date: 2018-08-31 22:40:25
Message-ID: CAOFGe948+0L-gvEN=Ww=vBiQjvhMVR1mguihOj_whrPTYM7i+w () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Fri, Aug 31, 2018 at 5:38 PM Nanley Chery <nanleychery@gmail.com> wrote:
> On Fri, Aug 31, 2018 at 05:15:53PM -0500, Jason Ekstrand wrote:
> > We make the flush after a HiZ clear unconditional and add a flush/stall
> > before the clear as well.
> >
> > Cc: mesa-stable@lists.freedesktop.org
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107760
> > ---
> > src/intel/vulkan/anv_blorp.c | 44 +++++++++++++++++++++++++++---------
> > 1 file changed, 33 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > index 35b304f92b3..04bca4d261f 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -1604,6 +1604,24 @@ anv_image_hiz_clear(struct anv_cmd_buffer
> *cmd_buffer,
> > ISL_AUX_USAGE_NONE, &stencil);
> > }
> >
> > + /* From the Sky Lake PRM Volume 7, "Depth Buffer Clear":
> > + *
> > + * "The following is required when performing a depth buffer
> clear with
> > + * using the WM_STATE or 3DSTATE_WM:
> > + *
> > + * * If other rendering operations have preceded this clear, a
> > + * PIPE_CONTROL with depth cache flush enabled, Depth Stall
> bit
> > + * enabled must be issued before the rectangle primitive
> used for
> > + * the depth buffer clear operation.
> > + * * [...]"
> > + *
> > + * Even though the PRM only says that this is required if using
> 3DSTATE_WM
> > + * and a 3DPRIMITIVE, it appears to also sometimes hang when doing a
> clear
> > + * with WM_HZ_OP.
> ^
> This part was a little hard to parse because the PRM hasn't mentioned
> the hardware hanging and the subject's changed from the pipecontrol to
> the GPU. Maybe replace it with something like the following?
>
> the GPU appears to also need this to avoid
> occasional hangs when doing a clear with
> WM_HZ_OP.
>
Works for me.
> We could discuss it more on IRC if you prefer.
>
> With that changed, this patch is
> Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
>
Thanks!
> > + */
> > + cmd_buffer->state.pending_pipe_bits |=
> > + ANV_PIPE_DEPTH_CACHE_FLUSH_BIT | ANV_PIPE_DEPTH_STALL_BIT;
> > +
> > blorp_hiz_clear_depth_stencil(&batch, &depth, &stencil,
> > level, base_layer, layer_count,
> > area.offset.x, area.offset.y,
> > @@ -1618,18 +1636,22 @@ anv_image_hiz_clear(struct anv_cmd_buffer
> *cmd_buffer,
> >
> > /* From the SKL PRM, Depth Buffer Clear:
> > *
> > - * Depth Buffer Clear Workaround
> > - * Depth buffer clear pass using any of the methods (WM_STATE,
> 3DSTATE_WM
> > - * or 3DSTATE_WM_HZ_OP) must be followed by a PIPE_CONTROL command
> with
> > - * DEPTH_STALL bit and Depth FLUSH bits "set" before starting to
> render.
> > - * DepthStall and DepthFlush are not needed between consecutive
> depth clear
> > - * passes nor is it required if the depth-clear pass was done with
> > - * "full_surf_clear" bit set in the 3DSTATE_WM_HZ_OP.
> > + * "Depth Buffer Clear Workaround
> > + *
> > + * Depth buffer clear pass using any of the methods (WM_STATE,
> > + * 3DSTATE_WM or 3DSTATE_WM_HZ_OP) must be followed by a
> PIPE_CONTROL
> > + * command with DEPTH_STALL bit and Depth FLUSH bits "set" before
> > + * starting to render. DepthStall and DepthFlush are not needed
> between
> > + * consecutive depth clear passes nor is it required if the
> depth-clear
> > + * pass was done with "full_surf_clear" bit set in the
> > + * 3DSTATE_WM_HZ_OP."
> > + *
> > + * Even though the PRM provides a bunch of conditions under which
> this is
> > + * supposedly unnecessary, we choose to perform the flush
> unconditionally
> > + * just to be safe.
> > */
> > - if (aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {
> > - cmd_buffer->state.pending_pipe_bits |=
> > - ANV_PIPE_DEPTH_CACHE_FLUSH_BIT | ANV_PIPE_DEPTH_STALL_BIT;
> > - }
> > + cmd_buffer->state.pending_pipe_bits |=
> > + ANV_PIPE_DEPTH_CACHE_FLUSH_BIT | ANV_PIPE_DEPTH_STALL_BIT;
> > }
> >
> > void
> > --
> > 2.17.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
[Attachment #5 (text/html)]
<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, Aug 31, 2018 at 5:38 \
PM Nanley Chery <<a \
href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Aug 31, 2018 at 05:15:53PM \
-0500, Jason Ekstrand wrote:<br> > We make the flush after a HiZ clear \
unconditional and add a flush/stall<br> > before the clear as well.<br>
> <br>
> Cc: <a href="mailto:mesa-stable@lists.freedesktop.org" \
target="_blank">mesa-stable@lists.freedesktop.org</a><br> > Bugzilla: <a \
href="https://bugs.freedesktop.org/show_bug.cgi?id=107760" rel="noreferrer" \
target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=107760</a><br> > \
---<br> > src/intel/vulkan/anv_blorp.c | 44 \
+++++++++++++++++++++++++++---------<br> > 1 file changed, 33 insertions(+), 11 \
deletions(-)<br> > <br>
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c<br>
> index 35b304f92b3..04bca4d261f 100644<br>
> --- a/src/intel/vulkan/anv_blorp.c<br>
> +++ b/src/intel/vulkan/anv_blorp.c<br>
> @@ -1604,6 +1604,24 @@ anv_image_hiz_clear(struct anv_cmd_buffer \
*cmd_buffer,<br> > \
ISL_AUX_USAGE_NONE, &stencil);<br> > }<br>
> <br>
> + /* From the Sky Lake PRM Volume 7, "Depth Buffer Clear":<br>
> + *<br>
> + * "The following is required when performing a depth buffer \
clear with<br> > + * using the WM_STATE or 3DSTATE_WM:<br>
> + *<br>
> + * * If other rendering operations have preceded this clear, \
a<br> > + * PIPE_CONTROL with depth cache flush enabled, Depth \
Stall bit<br> > + * enabled must be issued before the rectangle \
primitive used for<br> > + * the depth buffer clear \
operation.<br> > + * * [...]"<br>
> + *<br>
> + * Even though the PRM only says that this is required if using \
3DSTATE_WM<br> > + * and a 3DPRIMITIVE, it appears to also sometimes hang \
when doing a clear<br> > + * with WM_HZ_OP.<br>
^<br>
This part was a little hard to parse because the PRM hasn't mentioned<br>
the hardware hanging and the subject's changed from the pipecontrol to<br>
the GPU. Maybe replace it with something like the following?<br>
<br>
the GPU appears to also need this to \
avoid<br>
occasional hangs when doing a clear with<br>
\
WM_HZ_OP.<br></blockquote><div><br></div><div>Works for me.<br></div><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> We could discuss it more on IRC if you prefer.<br>
<br>
With that changed, this patch is<br>
Reviewed-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com" \
target="_blank">nanley.g.chery@intel.com</a>><br></blockquote><div><br></div><div>Thanks!<br></div><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> > + */<br>
> + cmd_buffer->state.pending_pipe_bits |=<br>
> + ANV_PIPE_DEPTH_CACHE_FLUSH_BIT | ANV_PIPE_DEPTH_STALL_BIT;<br>
> +<br>
> blorp_hiz_clear_depth_stencil(&batch, &depth, &stencil,<br>
> level, base_layer, \
layer_count,<br> > \
area.offset.x, area.offset.y,<br> > @@ -1618,18 +1636,22 @@ \
anv_image_hiz_clear(struct anv_cmd_buffer *cmd_buffer,<br> > <br>
> /* From the SKL PRM, Depth Buffer Clear:<br>
> *<br>
> - * Depth Buffer Clear Workaround<br>
> - * Depth buffer clear pass using any of the methods (WM_STATE, \
3DSTATE_WM<br> > - * or 3DSTATE_WM_HZ_OP) must be followed by a PIPE_CONTROL \
command with<br> > - * DEPTH_STALL bit and Depth FLUSH bits "set" before \
starting to render.<br> > - * DepthStall and DepthFlush are not needed \
between consecutive depth clear<br> > - * passes nor is it required if the \
depth-clear pass was done with<br> > - * "full_surf_clear" bit set in the \
3DSTATE_WM_HZ_OP.<br> > + * "Depth Buffer Clear Workaround<br>
> + *<br>
> + * Depth buffer clear pass using any of the methods (WM_STATE,<br>
> + * 3DSTATE_WM or 3DSTATE_WM_HZ_OP) must be followed by a \
PIPE_CONTROL<br> > + * command with DEPTH_STALL bit and Depth FLUSH bits \
"set" before<br> > + * starting to render. DepthStall and DepthFlush \
are not needed between<br> > + * consecutive depth clear passes nor is \
it required if the depth-clear<br> > + * pass was done with \
"full_surf_clear" bit set in the<br> > + * 3DSTATE_WM_HZ_OP."<br>
> + *<br>
> + * Even though the PRM provides a bunch of conditions under which this \
is<br> > + * supposedly unnecessary, we choose to perform the flush \
unconditionally<br> > + * just to be safe.<br>
> */<br>
> - if (aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {<br>
> - cmd_buffer->state.pending_pipe_bits |=<br>
> - ANV_PIPE_DEPTH_CACHE_FLUSH_BIT | ANV_PIPE_DEPTH_STALL_BIT;<br>
> - }<br>
> + cmd_buffer->state.pending_pipe_bits |=<br>
> + ANV_PIPE_DEPTH_CACHE_FLUSH_BIT | ANV_PIPE_DEPTH_STALL_BIT;<br>
> }<br>
> <br>
> void<br>
> -- <br>
> 2.17.1<br>
> <br>
> _______________________________________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org" \
target="_blank">mesa-dev@lists.freedesktop.org</a><br> > <a \
href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" \
target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br> \
</blockquote></div></div>
[Attachment #6 (text/plain)]
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic