On 12/20/2013 07:56 AM, Paul Berry wrote: > On 20 December 2013 04:47, Chad Versace <chad.vers...@linux.intel.com > <mailto:chad.vers...@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-sta...@lists.freedesktop.org > <mailto:mesa-sta...@lists.freedesktop.org> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70740 > Signed-off-by <https://bugs.freedesktop.org/show_bug.cgi?id=70740 > Signed-off-by>: Stéphane Marchesin <marc...@chromium.org > <mailto:marc...@chromium.org>> > Signed-off-by: Chad Versace <chad.vers...@linux.intel.com > <mailto:chad.vers...@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.
We already at least attempt to do this: before emitting 3DSTATE_DEPTH_BUFFER, we call intel_emit_depth_stall_flushes(), both in the main rendering code and from BLORP. Since all depth clears go through one of those two methods, the flushes do happen before rendering. Maybe the timing is slightly off. Chad's patch does add an additional set of these flushes. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
