[Mesa-dev] [PATCH 03/32] i965/blorp: Do flushes around depth resolves
Jason Ekstrand
jason at jlekstrand.net
Thu Jul 20 17:25:04 UTC 2017
On Thu, Jul 20, 2017 at 2:38 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:
> On Wed, Jul 19, 2017 at 02:01:29PM -0700, Jason Ekstrand wrote:
> > It turns out that if you have rendering in-flight with CCS_E enabled and
> > you go to do a depth resolve without flushing, the CCS data may never
> > hit the memory.
> > ---
> > src/mesa/drivers/dri/i965/brw_blorp.c | 150
> ++++++++++++++++------------------
> > 1 file changed, 72 insertions(+), 78 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> > index 5335fae..efa3b39 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> > @@ -1079,51 +1079,48 @@ intel_hiz_exec(struct brw_context *brw, struct
> intel_mipmap_tree *mt,
> > __func__, opname, mt, level, start_layer, start_layer +
> num_layers - 1);
> >
> > /* The following stalls and flushes are only documented to be
> required for
> > - * HiZ clear operations. However, they also seem to be required for
> the
> > - * HiZ resolve operation which is basically the same as a fast clear
> only a
> > - * different value is written into the HiZ surface.
> > + * HiZ clear operations. However, they also seem to be required for
> > + * resolve operations.
>
> How would feel putting some of the rational in the commit message here?
> Sounds
> valuable.
>
Hrm... I can, but I think the problem is most likely more general than
CCS_E. The fact that CCS_E happens to show it off doesn't mean that's why
we need to do it.
> > */
> > - if (op == BLORP_HIZ_OP_DEPTH_CLEAR || op ==
> BLORP_HIZ_OP_HIZ_RESOLVE) {
> > - if (brw->gen == 6) {
> > - /* From the Sandy Bridge PRM, volume 2 part 1, page 313:
> > - *
> > - * "If other rendering operations have preceded this clear, a
> > - * PIPE_CONTROL with write cache flush enabled and Z-inhibit
> > - * disabled must be issued before the rectangle primitive
> used for
> > - * the depth buffer clear operation.
> > - */
> > - brw_emit_pipe_control_flush(brw,
> > - PIPE_CONTROL_RENDER_TARGET_FLUSH
> |
> > - PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> > - PIPE_CONTROL_CS_STALL);
> > - } else if (brw->gen >= 7) {
> > - /*
> > - * From the Ivybridge PRM, volume 2, "Depth Buffer Clear":
> > - *
> > - * 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.
> > - *
> > - * Same applies for Gen8 and Gen9.
> > - *
> > - * In addition, from the Ivybridge PRM, volume 2, 1.10.4.1
> > - * PIPE_CONTROL, Depth Cache Flush Enable:
> > - *
> > - * This bit must not be set when Depth Stall Enable bit is
> set in
> > - * this packet.
> > - *
> > - * This is confirmed to hold for real, HSW gets immediate gpu
> hangs.
> > - *
> > - * Therefore issue two pipe control flushes, one for cache
> flush and
> > - * another for depth stall.
> > - */
> > - brw_emit_pipe_control_flush(brw,
> > - PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> > - PIPE_CONTROL_CS_STALL);
> > + if (brw->gen == 6) {
> > + /* From the Sandy Bridge PRM, volume 2 part 1, page 313:
> > + *
> > + * "If other rendering operations have preceded this clear, a
> > + * PIPE_CONTROL with write cache flush enabled and Z-inhibit
> > + * disabled must be issued before the rectangle primitive used
> for
> > + * the depth buffer clear operation.
> > + */
> > + brw_emit_pipe_control_flush(brw,
> > + PIPE_CONTROL_RENDER_TARGET_FLUSH |
> > + PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> > + PIPE_CONTROL_CS_STALL);
> > + } else if (brw->gen >= 7) {
> > + /*
> > + * From the Ivybridge PRM, volume 2, "Depth Buffer Clear":
> > + *
> > + * 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.
> > + *
> > + * Same applies for Gen8 and Gen9.
> > + *
> > + * In addition, from the Ivybridge PRM, volume 2, 1.10.4.1
> > + * PIPE_CONTROL, Depth Cache Flush Enable:
> > + *
> > + * This bit must not be set when Depth Stall Enable bit is set
> in
> > + * this packet.
> > + *
> > + * This is confirmed to hold for real, HSW gets immediate gpu
> hangs.
> > + *
> > + * Therefore issue two pipe control flushes, one for cache flush
> and
> > + * another for depth stall.
> > + */
> > + brw_emit_pipe_control_flush(brw,
> > + PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> > + PIPE_CONTROL_CS_STALL);
> >
> > - brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_STALL);
> > - }
> > + brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_STALL);
> > }
> >
> >
> > @@ -1140,42 +1137,39 @@ intel_hiz_exec(struct brw_context *brw, struct
> intel_mipmap_tree *mt,
> > blorp_batch_finish(&batch);
> >
> > /* The following stalls and flushes are only documented to be
> required for
> > - * HiZ clear operations. However, they also seem to be required for
> the
> > - * HiZ resolve operation which is basically the same as a fast clear
> only a
> > - * different value is written into the HiZ surface.
> > + * HiZ clear operations. However, they also seem to be required for
> > + * resolve operations.
> > */
> > - if (op == BLORP_HIZ_OP_DEPTH_CLEAR || op ==
> BLORP_HIZ_OP_HIZ_RESOLVE) {
> > - if (brw->gen == 6) {
> > - /* From the Sandy Bridge PRM, volume 2 part 1, page 314:
> > - *
> > - * "DevSNB, DevSNB-B{W/A}]: Depth buffer clear pass must be
> > - * followed by a PIPE_CONTROL command with DEPTH_STALL bit
> set
> > - * and Then followed by Depth FLUSH'
> > - */
> > - brw_emit_pipe_control_flush(brw,
> > - PIPE_CONTROL_DEPTH_STALL);
> > -
> > - brw_emit_pipe_control_flush(brw,
> > - PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> > - PIPE_CONTROL_CS_STALL);
> > - } else if (brw->gen >= 8) {
> > - /*
> > - * From the Broadwell PRM, volume 7, "Depth Buffer Clear":
> > - *
> > - * "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."
> > - *
> > - * TODO: Such as the spec says, this could be conditional.
> > - */
> > - brw_emit_pipe_control_flush(brw,
> > - PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> > - PIPE_CONTROL_DEPTH_STALL);
> > + if (brw->gen == 6) {
> > + /* From the Sandy Bridge PRM, volume 2 part 1, page 314:
> > + *
> > + * "DevSNB, DevSNB-B{W/A}]: Depth buffer clear pass must be
> > + * followed by a PIPE_CONTROL command with DEPTH_STALL bit set
> > + * and Then followed by Depth FLUSH'
> > + */
> > + brw_emit_pipe_control_flush(brw,
> > + PIPE_CONTROL_DEPTH_STALL);
> > +
> > + brw_emit_pipe_control_flush(brw,
> > + PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> > + PIPE_CONTROL_CS_STALL);
> > + } else if (brw->gen >= 8) {
> > + /*
> > + * From the Broadwell PRM, volume 7, "Depth Buffer Clear":
> > + *
> > + * "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."
> > + *
> > + * TODO: Such as the spec says, this could be conditional.
> > + */
> > + brw_emit_pipe_control_flush(brw,
> > + PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> > + PIPE_CONTROL_DEPTH_STALL);
> >
> > - }
> > }
> > }
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170720/accdfc52/attachment-0001.html>
More information about the mesa-dev
mailing list