<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Feb 13, 2017 at 5:00 PM, Nanley Chery <span dir="ltr"><<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Feb 10, 2017 at 11:02:21AM -0800, Jason Ekstrand wrote:<br>
> This improves the performance of Dota 2 on my Sky Lake Skull Canyon<br>
> machine by about 2-3%.<br>
><br>
<br>
</span>This patch looks good. Could we compare our setup and testing in the<br>
office? I can't reproduce these numbers.<br></blockquote><div><br></div><div>I'll have to do more runs. At the moment, I'm not seeing a large boost but I'll get a larger sample size and see if it's visible.<br><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-Nanley<br>
<div><div class="h5"><br>
> Reviewed-by: Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a><wbr>><br>
> ---<br>
> src/intel/vulkan/anv_private.h | 1 +<br>
> src/intel/vulkan/gen8_cmd_<wbr>buffer.c | 157 ++++++++++++++++++++++++++++++<wbr>++++++-<br>
> src/intel/vulkan/genX_<wbr>pipeline.c | 6 +-<br>
> 3 files changed, 158 insertions(+), 6 deletions(-)<br>
><br>
> diff --git a/src/intel/vulkan/anv_<wbr>private.h b/src/intel/vulkan/anv_<wbr>private.h<br>
> index fa6032e..25c9c93 100644<br>
> --- a/src/intel/vulkan/anv_<wbr>private.h<br>
> +++ b/src/intel/vulkan/anv_<wbr>private.h<br>
> @@ -1482,6 +1482,7 @@ struct anv_pipeline {<br>
> bool writes_depth;<br>
> bool depth_test_enable;<br>
> bool writes_stencil;<br>
> + bool stencil_test_enable;<br>
> bool depth_clamp_enable;<br>
> bool kill_pixel;<br>
><br>
> diff --git a/src/intel/vulkan/gen8_cmd_<wbr>buffer.c b/src/intel/vulkan/gen8_cmd_<wbr>buffer.c<br>
> index 0628f3a..59fd537 100644<br>
> --- a/src/intel/vulkan/gen8_cmd_<wbr>buffer.c<br>
> +++ b/src/intel/vulkan/gen8_cmd_<wbr>buffer.c<br>
> @@ -157,16 +157,39 @@ __emit_sf_state(struct anv_cmd_buffer *cmd_buffer)<br>
> void<br>
> genX(cmd_buffer_enable_pma_<wbr>fix)(struct anv_cmd_buffer *cmd_buffer, bool enable)<br>
> {<br>
> -#if GEN_GEN == 8<br>
> if (cmd_buffer->state.pma_fix_<wbr>enabled == enable)<br>
> return;<br>
><br>
> + cmd_buffer->state.pma_fix_<wbr>enabled = enable;<br>
> +<br>
> + /* According to the Broadwell PIPE_CONTROL documentation, software should<br>
> + * emit a PIPE_CONTROL with the CS Stall and Depth Cache Flush bits set<br>
> + * prior to the LRI. If stencil buffer writes are enabled, then a Render<br>
> + * Cache Flush is also necessary.<br>
> + *<br>
> + * The Skylake docs say to use a depth stall rather than a command<br>
> + * streamer stall. However, the hardware seems to violently disagree.<br>
> + * A full command streamer stall seems to be needed in both cases.<br>
> + */<br>
> anv_batch_emit(&cmd_buffer-><wbr>batch, GENX(PIPE_CONTROL), pc) {<br>
> pc.DepthCacheFlushEnable = true;<br>
> pc.CommandStreamerStallEnable = true;<br>
> pc.<wbr>RenderTargetCacheFlushEnable = true;<br>
> }<br>
><br>
> +#if GEN_GEN == 9<br>
> +<br>
> + uint32_t cache_mode;<br>
> + anv_pack_struct(&cache_mode, GENX(CACHE_MODE_0),<br>
> + .STCPMAOptimizationEnable = enable,<br>
> + .STCPMAOptimizationEnableMask = true);<br>
> + anv_batch_emit(&cmd_buffer-><wbr>batch, GENX(MI_LOAD_REGISTER_IMM), lri) {<br>
> + lri.RegisterOffset = GENX(CACHE_MODE_0_num);<br>
> + lri.DataDWord = cache_mode;<br>
> + }<br>
> +<br>
> +#elif GEN_GEN == 8<br>
> +<br>
> uint32_t cache_mode;<br>
> anv_pack_struct(&cache_mode, GENX(CACHE_MODE_1),<br>
> .NPPMAFixEnable = enable,<br>
> @@ -178,18 +201,20 @@ genX(cmd_buffer_enable_pma_<wbr>fix)(struct anv_cmd_buffer *cmd_buffer, bool enable)<br>
> lri.DataDWord = cache_mode;<br>
> }<br>
><br>
> +#endif /* GEN_GEN == 8 */<br>
> +<br>
> /* After the LRI, a PIPE_CONTROL with both the Depth Stall and Depth Cache<br>
> * Flush bits is often necessary. We do it regardless because it's easier.<br>
> * The render cache flush is also necessary if stencil writes are enabled.<br>
> + *<br>
> + * Again, the Skylake docs give a different set of flushes but the BDW<br>
> + * flushes seem to work just as well.<br>
> */<br>
> anv_batch_emit(&cmd_buffer-><wbr>batch, GENX(PIPE_CONTROL), pc) {<br>
> pc.DepthStallEnable = true;<br>
> pc.DepthCacheFlushEnable = true;<br>
> pc.<wbr>RenderTargetCacheFlushEnable = true;<br>
> }<br>
> -<br>
> - cmd_buffer->state.pma_fix_<wbr>enabled = enable;<br>
> -#endif /* GEN_GEN == 8 */<br>
> }<br>
><br>
> static inline bool<br>
> @@ -281,6 +306,126 @@ want_depth_pma_fix(struct anv_cmd_buffer *cmd_buffer)<br>
> wm_prog_data->computed_depth_<wbr>mode != PSCDEPTH_OFF;<br>
> }<br>
><br>
> +static inline bool<br>
> +want_stencil_pma_fix(struct anv_cmd_buffer *cmd_buffer)<br>
> +{<br>
> + assert(GEN_GEN == 9);<br>
> +<br>
> + /* From the Skylake PRM Vol. 2c CACHE_MODE_1::STC PMA Optimization Enable:<br>
> + *<br>
> + * Clearing this bit will force the STC cache to wait for pending<br>
> + * retirement of pixels at the HZ-read stage and do the STC-test for<br>
> + * Non-promoted, R-computed and Computed depth modes instead of<br>
> + * postponing the STC-test to RCPFE.<br>
> + *<br>
> + * STC_TEST_EN = 3DSTATE_STENCIL_BUFFER::<wbr>STENCIL_BUFFER_ENABLE &&<br>
> + * 3DSTATE_WM_DEPTH_STENCIL::<wbr>StencilTestEnable<br>
> + *<br>
> + * STC_WRITE_EN = 3DSTATE_STENCIL_BUFFER::<wbr>STENCIL_BUFFER_ENABLE &&<br>
> + * (3DSTATE_WM_DEPTH_STENCIL::<wbr>Stencil Buffer Write Enable &&<br>
> + * 3DSTATE_DEPTH_BUFFER::STENCIL_<wbr>WRITE_ENABLE)<br>
> + *<br>
> + * COMP_STC_EN = STC_TEST_EN &&<br>
> + * 3DSTATE_PS_EXTRA::<wbr>PixelShaderComputesStencil<br>
> + *<br>
> + * SW parses the pipeline states to generate the following logical<br>
> + * signal indicating if PMA FIX can be enabled.<br>
> + *<br>
> + * STC_PMA_OPT =<br>
> + * 3DSTATE_WM::<wbr>ForceThreadDispatch != 1 &&<br>
> + * !(3DSTATE_RASTER::<wbr>ForceSampleCount != NUMRASTSAMPLES_0) &&<br>
> + * 3DSTATE_DEPTH_BUFFER::SURFACE_<wbr>TYPE != NULL &&<br>
> + * 3DSTATE_DEPTH_BUFFER::HIZ Enable &&<br>
> + * !(3DSTATE_WM::EDSC_Mode == 2) &&<br>
> + * 3DSTATE_PS_EXTRA::<wbr>PixelShaderValid &&<br>
> + * !(3DSTATE_WM_HZ_OP::<wbr>DepthBufferClear ||<br>
> + * 3DSTATE_WM_HZ_OP::<wbr>DepthBufferResolve ||<br>
> + * 3DSTATE_WM_HZ_OP::Hierarchical Depth Buffer Resolve Enable ||<br>
> + * 3DSTATE_WM_HZ_OP::<wbr>StencilBufferClear) &&<br>
> + * (COMP_STC_EN || STC_WRITE_EN) &&<br>
> + * ((3DSTATE_PS_EXTRA::<wbr>PixelShaderKillsPixels ||<br>
> + * 3DSTATE_WM::ForceKillPix == ON ||<br>
> + * 3DSTATE_PS_EXTRA::oMask Present to RenderTarget ||<br>
> + * 3DSTATE_PS_BLEND::<wbr>AlphaToCoverageEnable ||<br>
> + * 3DSTATE_PS_BLEND::<wbr>AlphaTestEnable ||<br>
> + * 3DSTATE_WM_CHROMAKEY::<wbr>ChromaKeyKillEnable) ||<br>
> + * (3DSTATE_PS_EXTRA::Pixel Shader Computed Depth mode != PSCDEPTH_OFF))<br>
> + */<br>
> +<br>
> + /* These are always true:<br>
> + * 3DSTATE_WM::<wbr>ForceThreadDispatch != 1 &&<br>
> + * !(3DSTATE_RASTER::<wbr>ForceSampleCount != NUMRASTSAMPLES_0)<br>
> + */<br>
> +<br>
> + /* We only enable the PMA fix if we know for certain that HiZ is enabled.<br>
> + * If we don't know whether HiZ is enabled or not, we disable the PMA fix<br>
> + * and there is no harm.<br>
> + *<br>
> + * (3DSTATE_DEPTH_BUFFER::<wbr>SURFACE_TYPE != NULL) &&<br>
> + * 3DSTATE_DEPTH_BUFFER::HIZ Enable<br>
> + */<br>
> + if (!cmd_buffer->state.hiz_<wbr>enabled)<br>
> + return false;<br>
> +<br>
> + /* We can't possibly know if HiZ is enabled without the framebuffer */<br>
> + assert(cmd_buffer->state.<wbr>framebuffer);<br>
> +<br>
> + /* HiZ is enabled so we had better have a depth buffer with HiZ */<br>
> + const struct anv_image_view *ds_iview =<br>
> + anv_cmd_buffer_get_depth_<wbr>stencil_view(cmd_buffer);<br>
> + assert(ds_iview && ds_iview->image->aux_usage == ISL_AUX_USAGE_HIZ);<br>
> +<br>
> + /* 3DSTATE_PS_EXTRA::<wbr>PixelShaderValid */<br>
> + struct anv_pipeline *pipeline = cmd_buffer->state.pipeline;<br>
> + if (!anv_pipeline_has_stage(<wbr>pipeline, MESA_SHADER_FRAGMENT))<br>
> + return false;<br>
> +<br>
> + /* !(3DSTATE_WM::EDSC_Mode == 2) */<br>
> + const struct brw_wm_prog_data *wm_prog_data = get_wm_prog_data(pipeline);<br>
> + if (wm_prog_data->early_fragment_<wbr>tests)<br>
> + return false;<br>
> +<br>
> + /* We never use anv_pipeline for HiZ ops so this is trivially true:<br>
> + * !(3DSTATE_WM_HZ_OP::<wbr>DepthBufferClear ||<br>
> + * 3DSTATE_WM_HZ_OP::<wbr>DepthBufferResolve ||<br>
> + * 3DSTATE_WM_HZ_OP::Hierarchical Depth Buffer Resolve Enable ||<br>
> + * 3DSTATE_WM_HZ_OP::<wbr>StencilBufferClear)<br>
> + */<br>
> +<br>
> + /* 3DSTATE_STENCIL_BUFFER::<wbr>STENCIL_BUFFER_ENABLE &&<br>
> + * 3DSTATE_WM_DEPTH_STENCIL::<wbr>StencilTestEnable<br>
> + */<br>
> + const bool stc_test_en =<br>
> + (ds_iview->image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT) &&<br>
> + pipeline->stencil_test_enable;<br>
> +<br>
> + /* 3DSTATE_STENCIL_BUFFER::<wbr>STENCIL_BUFFER_ENABLE &&<br>
> + * (3DSTATE_WM_DEPTH_STENCIL::<wbr>Stencil Buffer Write Enable &&<br>
> + * 3DSTATE_DEPTH_BUFFER::STENCIL_<wbr>WRITE_ENABLE)<br>
> + */<br>
> + const bool stc_write_en =<br>
> + (ds_iview->image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT) &&<br>
> + pipeline->writes_stencil;<br>
> +<br>
> + /* STC_TEST_EN && 3DSTATE_PS_EXTRA::<wbr>PixelShaderComputesStencil */<br>
> + const bool comp_stc_en = stc_test_en && wm_prog_data->computed_<wbr>stencil;<br>
> +<br>
> + /* COMP_STC_EN || STC_WRITE_EN */<br>
> + if (!(comp_stc_en || stc_write_en))<br>
> + return false;<br>
> +<br>
> + /* (3DSTATE_PS_EXTRA::<wbr>PixelShaderKillsPixels ||<br>
> + * 3DSTATE_WM::ForceKillPix == ON ||<br>
> + * 3DSTATE_PS_EXTRA::oMask Present to RenderTarget ||<br>
> + * 3DSTATE_PS_BLEND::<wbr>AlphaToCoverageEnable ||<br>
> + * 3DSTATE_PS_BLEND::<wbr>AlphaTestEnable ||<br>
> + * 3DSTATE_WM_CHROMAKEY::<wbr>ChromaKeyKillEnable) ||<br>
> + * (3DSTATE_PS_EXTRA::Pixel Shader Computed Depth mode != PSCDEPTH_OFF)<br>
> + */<br>
> + return pipeline->kill_pixel ||<br>
> + wm_prog_data->computed_depth_<wbr>mode != PSCDEPTH_OFF;<br>
> +}<br>
> +<br>
> void<br>
> genX(cmd_buffer_flush_dynamic_<wbr>state)(struct anv_cmd_buffer *cmd_buffer)<br>
> {<br>
> @@ -390,6 +535,7 @@ genX(cmd_buffer_flush_dynamic_<wbr>state)(struct anv_cmd_buffer *cmd_buffer)<br>
> }<br>
><br>
> if (cmd_buffer->state.dirty & (ANV_CMD_DIRTY_PIPELINE |<br>
> + ANV_CMD_DIRTY_RENDER_TARGETS |<br>
> ANV_CMD_DIRTY_DYNAMIC_STENCIL_<wbr>COMPARE_MASK |<br>
> ANV_CMD_DIRTY_DYNAMIC_STENCIL_<wbr>WRITE_MASK |<br>
> ANV_CMD_DIRTY_DYNAMIC_STENCIL_<wbr>REFERENCE)) {<br>
> @@ -415,6 +561,9 @@ genX(cmd_buffer_flush_dynamic_<wbr>state)(struct anv_cmd_buffer *cmd_buffer)<br>
><br>
> anv_batch_emit_merge(&cmd_<wbr>buffer->batch, dwords,<br>
> pipeline->gen9.wm_depth_<wbr>stencil);<br>
> +<br>
> + genX(cmd_buffer_enable_pma_<wbr>fix)(cmd_buffer,<br>
> + want_stencil_pma_fix(cmd_<wbr>buffer));<br>
> }<br>
> #endif<br>
><br>
> diff --git a/src/intel/vulkan/genX_<wbr>pipeline.c b/src/intel/vulkan/genX_<wbr>pipeline.c<br>
> index d2af8b9..3b0ae08 100644<br>
> --- a/src/intel/vulkan/genX_<wbr>pipeline.c<br>
> +++ b/src/intel/vulkan/genX_<wbr>pipeline.c<br>
> @@ -784,6 +784,7 @@ emit_ds_state(struct anv_pipeline *pipeline,<br>
> * to make sure it's initialized to something useful.<br>
> */<br>
> pipeline->writes_stencil = false;<br>
> + pipeline->stencil_test_enable = false;<br>
> pipeline->writes_depth = false;<br>
> pipeline->depth_test_enable = false;<br>
> memset(depth_stencil_dw, 0, sizeof(depth_stencil_dw));<br>
> @@ -799,6 +800,7 @@ emit_ds_state(struct anv_pipeline *pipeline,<br>
><br>
> VkPipelineDepthStencilStateCre<wbr>ateInfo info = *pCreateInfo;<br>
> sanitize_ds_state(&info, &pipeline->writes_stencil, ds_aspects);<br>
> + pipeline->stencil_test_enable = info.stencilTestEnable;<br>
> pipeline->writes_depth = info.depthWriteEnable;<br>
> pipeline->depth_test_enable = info.depthTestEnable;<br>
><br>
> @@ -1556,8 +1558,8 @@ compute_kill_pixel(struct anv_pipeline *pipeline,<br>
> const struct brw_wm_prog_data *wm_prog_data = get_wm_prog_data(pipeline);<br>
><br>
> /* This computes the KillPixel portion of the computation for whether or<br>
> - * not we want to enable the PMA fix on gen8. It's given by this chunk of<br>
> - * the giant formula:<br>
> + * not we want to enable the PMA fix on gen8 or gen9. It's given by this<br>
> + * chunk of the giant formula:<br>
> *<br>
> * (3DSTATE_PS_EXTRA::<wbr>PixelShaderKillsPixels ||<br>
> * 3DSTATE_PS_EXTRA::oMask Present to RenderTarget ||<br>
> --<br>
> 2.5.0.400.gff86faf<br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</blockquote></div><br></div></div>