<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Feb 17, 2018 at 1:21 AM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Saturday, February 17, 2018 1:09:10 AM PST Jason Ekstrand wrote:<br>
> This fixes a pile of hangs caused by the recent shuffling of resolves<br>
> and transitions. The particularly problematic case is when you have at<br>
> least three attachments with load ops of CLEAR, LOAD, CLEAR. In this<br>
> case, we execute the first CLEAR followed by a MI memcpy to copy the<br>
> clear values over for the LOAD followed by a second CLEAR. The MI<br>
> commands cause the first CLEAR to hang which causes us to get stuck on<br>
> the 3DSTATE_MULTISAMPLE in the second CLEAR.<br>
><br>
> We also add guards for BLORP to fix the same issue. These shouldn't<br>
> actually do anything right now because the only use of indirect clears<br>
> in BLORP today is for resolves which are already guarded by a render<br>
> cache flush and CS stall. However, this will guard us against potential<br>
> issues in the future.<br>
><br>
> Cc: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
> Cc: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>><br>
> ---<br>
> src/intel/vulkan/genX_blorp_<wbr>exec.c | 10 ++++++++++<br>
> src/intel/vulkan/genX_gpu_<wbr>memcpy.c | 22 ++++++++++++++++++++++<br>
> 2 files changed, 32 insertions(+)<br>
><br>
> diff --git a/src/intel/vulkan/genX_blorp_<wbr>exec.c b/src/intel/vulkan/genX_blorp_<wbr>exec.c<br>
> index 04f7675..f956715 100644<br>
> --- a/src/intel/vulkan/genX_blorp_<wbr>exec.c<br>
> +++ b/src/intel/vulkan/genX_blorp_<wbr>exec.c<br>
> @@ -200,6 +200,16 @@ genX(blorp_exec)(struct blorp_batch *batch,<br>
> genX(cmd_buffer_config_l3)(<wbr>cmd_buffer, cfg);<br>
> }<br>
><br>
> +#if GEN_GEN == 7<br>
> + /* The MI_LOAD/STORE_REGISTER_MEM commands which BLORP uses to implement<br>
> + * indirect fast-clear colors can cause GPU hangs if we don't stall first.<br>
> + * See genX(cmd_buffer_mi_memcpy) for more details.<br>
> + */<br>
> + assert(params->src.clear_<wbr>color_addr.buffer == NULL);<br>
> + if (params->dst.clear_color_addr.<wbr>buffer)<br>
> + cmd_buffer->state.pending_<wbr>pipe_bits |= ANV_PIPE_CS_STALL_BIT;<br>
> +#endif<br>
> +<br>
> genX(cmd_buffer_apply_pipe_<wbr>flushes)(cmd_buffer);<br>
><br>
> genX(flush_pipeline_select_3d)<wbr>(cmd_buffer);<br>
> diff --git a/src/intel/vulkan/genX_gpu_<wbr>memcpy.c b/src/intel/vulkan/genX_gpu_<wbr>memcpy.c<br>
> index f3ada93..1d527fb 100644<br>
> --- a/src/intel/vulkan/genX_gpu_<wbr>memcpy.c<br>
> +++ b/src/intel/vulkan/genX_gpu_<wbr>memcpy.c<br>
> @@ -62,6 +62,28 @@ genX(cmd_buffer_mi_memcpy)(<wbr>struct anv_cmd_buffer *cmd_buffer,<br>
> assert(dst_offset % 4 == 0);<br>
> assert(src_offset % 4 == 0);<br>
><br>
> +#if GEN_GEN == 7<br>
> + /* On gen7, the combination of commands used here(MI_LOAD_REGISTER_MEM<br>
> + * and MI_STORE_REGISTER_MEM) can cause GPU hangs if any rendering is<br>
> + * in-flight when they are issued even if the memory touched is not<br>
> + * currently active for rendering. The weird bit is that it is not the<br>
> + * MI_LOAD/STORE_REGISTER_MEM commands which hang but rather the in-flight<br>
> + * rendering hangs such that the next stalling command after the<br>
> + * MI_LOAD/STORE_REGISTER_MEM commands will catch the hang.<br>
> + *<br>
> + * It is unclear exactly why this hang occurs. Both MI commands come with<br>
> + * warnings about the 3D pipeline but that doesn't seem to fully explain<br>
> + * it. My (Jason's) best theory is that it has something to do with the<br>
> + * fact that we're using a GPU state register as our temporary and that<br>
> + * something with reading/writing it is causing problems.<br>
> + *<br>
> + * In order to work around this issue, we emit a PIPE_CONTROL with the<br>
> + * command streamer stall bit set.<br>
> + */<br>
> + cmd_buffer->state.pending_<wbr>pipe_bits |= ANV_PIPE_CS_STALL_BIT;<br>
> + genX(cmd_buffer_apply_pipe_<wbr>flushes)(cmd_buffer);<br>
> +#endif<br>
> +<br>
> for (uint32_t i = 0; i < size; i += 4) {<br>
> const struct anv_address src_addr =<br>
> (struct anv_address) { src, src_offset + i};<br>
><br>
<br>
</div></div>This seems reasonable enough. I can't say I understand why this is<br>
necessary,</blockquote><div><br></div><div>That makes two of us and, when Nanley gets around to looking at it, I'm sure it'll make 3. :-)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">but if it fixes the problem...it's probably best to just<br>
do it. We can always change it later if we find new information.<br></blockquote><div><br>I'm fairly sure that this is more-or-less the correct fix and 10 Jenkins
runs along with 50 (soon to be 100) runs of
dEQP-VK.renderpass.dedicated_allocation.attachment.* agree.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Acked-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</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">
I'll let Nanley review as well.<br>
</blockquote></div><br></div><div class="gmail_extra">I can wait. I've only been fighting this for a week and a half...<br><br></div><div class="gmail_extra">--Jason<br></div></div>