On Tuesday, January 17, 2017 8:48:22 PM PST Topi Pohjolainen wrote:
> Current blorp logic issues unconditional "flush everything"
> (see brw_emit_mi_flush()) after each render. For example, all
> blits issue this unconditionally which shouldn't be needed if
> they set render cache properly os that subsequent renders do
> necessary flushing before drawing.
>
> In case of piglit:
>
> ext_framebuffer_multisample-accuracy all_samples depth_draw small
>
> intel_hiz_exec() is always preceded by blorb blit and the
> unconditional flush looks to hide the lack of stall and flushes
> in depth clears. By removing the brw_emit_mi_flush() I get gpu
> hangs.
>
> This patch adds the stalls and flushes mandated by the spec
> and gets rid of those hangs.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_clear.c | 38
> ++++++++++++++++++++++------
> src/mesa/drivers/dri/i965/gen8_depth_state.c | 16 ++++++++++++
> 2 files changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
> b/src/mesa/drivers/dri/i965/brw_clear.c
> index 488732c..b053a7d 100644
> --- a/src/mesa/drivers/dri/i965/brw_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> @@ -36,6 +36,7 @@
>
> #include "brw_context.h"
> #include "brw_blorp.h"
> +#include "brw_defines.h"
>
> #define FILE_DEBUG_FLAG DEBUG_BLIT
>
> @@ -174,14 +175,35 @@ brw_fast_clear_depth(struct gl_context *ctx)
> mt->depth_clear_value = depth_clear_value;
> }
>
> - /* 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_mi_flush(brw);
> + 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.
> + */
> + 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);
It sounds like the cited text is suggesting we do:
brw_emit_pipe_control_flush(brw, PIPE_CONTROL_DEPTH_STALL |
PIPE_CONTROL_DEPTH_CACHE_FLUSH);
which is illegal according to the PIPE_CONTROL docs in the IVB PRM, but
that text is gone from the latest internal docs. That said, the newer
internal docs provide /other/ contradictory data...so...who knows?
Assuming we resolve that somehow, the series is:
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
Mark says he's seeing far fewer hangs with this series, so I think it's
a lot better. But we still got a hang, on a PIPE_CONTROL after a
RECTLIST used for a fast color clear. So it may not be quite right
yet...
Curro, do you have time to take a look at these? See if you spot
anything?
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
