[Mesa-dev] [PATCH] i965/gen6: Fix HiZ hang in WebGL Google maps

[Paul Berry]

On 20 December 2013 04:47, Chad Versace <chad.versace at linux.intel.com>wrote:

> We need to emit depth stall flushes before depth and hiz resolves.
> Placing them at the top of blorp's state emission fixes the hang.
>
> Fixes HiZ hang in the new WebGL Google maps on Sandybridge Chrome OS.
> Tested by zooming in and out continuously for 2 hours.
>
> This patch is based on
>
> https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/8bc07bb70163c3706fb4ba5f980e57dc942f56dd
>
> CC: mesa-stable at lists.freedesktop.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70740
> Signed-off-by: Stéphane Marchesin <marcheu at chromium.org>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/gen6_blorp.cpp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>

Are you aware of any text in the bspec saying that these flushes are
necessary?  If so it would be nice to quote it in a comment.  I searched
for a while and wasn't able to find anything.


>
> diff --git a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> index 6a5841f..3a0e7ec 100644
> --- a/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen6_blorp.cpp
> @@ -1012,6 +1012,16 @@ gen6_blorp_emit_primitive(struct brw_context *brw,
>     ADVANCE_BATCH();
>  }
>
> +static void
> +gen6_emit_hiz_workaround(struct brw_context *brw, enum gen6_hiz_op hiz_op)
> +{
> +   if (hiz_op == GEN6_HIZ_OP_DEPTH_RESOLVE ||
> +       hiz_op == GEN6_HIZ_OP_HIZ_RESOLVE) {
>

Should we also include GEN6_HIZ_OP_DEPTH_CLEAR?  I found this text in the
bspec that suggests maybe we should (Graphics BSpec: 3D-Media-GPGPU Engine
> 3D Pipeline Stages > Pixel > Depth and Stencil > Hierarchical Depth
Buffer > 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.


And later on the same page:


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.


(Note, however that these depth clear flushes apply to all versions of the
hardware, so perhaps we should handle them in a different patch and a
different place in the code).


Regardless of whether you make any changes due to my comments above, the
patch is:


Reviewed-by: Paul Berry <stereotype441 at gmail.com>


> +      brw->batch.need_workaround_flush = true;
> +      intel_emit_post_sync_nonzero_flush(brw);
> +      intel_emit_depth_stall_flushes(brw);
> +   }
> +}
>
>  /**
>   * \brief Execute a blit or render pass operation.
> @@ -1034,6 +1044,8 @@ gen6_blorp_exec(struct brw_context *brw,
>     uint32_t wm_bind_bo_offset = 0;
>
>     uint32_t prog_offset = params->get_wm_prog(brw, &prog_data);
> +
> +   gen6_emit_hiz_workaround(brw, params->hiz_op);
>     gen6_emit_3dstate_multisample(brw, params->dst.num_samples);
>     gen6_emit_3dstate_sample_mask(brw,
>                                   params->dst.num_samples > 1 ?
> --
> 1.8.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131220/036dc198/attachment.html>