<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>