[Mesa-dev] [PATCH v2 7/7] anv: Implement the Skylake stencil PMA optimization

Nanley Chery nanleychery at gmail.com
Tue Feb 14 18:35:13 UTC 2017


On Mon, Feb 13, 2017 at 08:45:47PM -0800, Jason Ekstrand wrote:
> On Mon, Feb 13, 2017 at 6:13 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> 
> > On Mon, Feb 13, 2017 at 5:00 PM, Nanley Chery <nanleychery at gmail.com>
> > wrote:
> >
> >> On Fri, Feb 10, 2017 at 11:02:21AM -0800, Jason Ekstrand wrote:
> >> > This improves the performance of Dota 2 on my Sky Lake Skull Canyon
> >> > machine by about 2-3%.
> >> >
> >>
> >> This patch looks good. Could we compare our setup and testing in the
> >> office? I can't reproduce these numbers.
> >>
> >
> > 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.
> >
> 
> I did a bunch more runs.  It looks like it helps somewhere between 0 and
> 1%.  Not as much as I saw earlier, but it's definitely non-negative.
> 

Although I've confirmed that the fix is enabled in Dota 2, I'm also not
seeing any major changes on my SKL GT4. I get positive and negative
changes in FPS, but it's all < 1% and seemingly noise. 

With the commit message updated with your new findings, this patch is
Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>


> --Jason
> 
> 
> > --Jason
> >
> >
> >> -Nanley
> >>
> >> > Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> >> > ---
> >> >  src/intel/vulkan/anv_private.h     |   1 +
> >> >  src/intel/vulkan/gen8_cmd_buffer.c | 157
> >> ++++++++++++++++++++++++++++++++++++-
> >> >  src/intel/vulkan/genX_pipeline.c   |   6 +-
> >> >  3 files changed, 158 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/src/intel/vulkan/anv_private.h
> >> b/src/intel/vulkan/anv_private.h
> >> > index fa6032e..25c9c93 100644
> >> > --- a/src/intel/vulkan/anv_private.h
> >> > +++ b/src/intel/vulkan/anv_private.h
> >> > @@ -1482,6 +1482,7 @@ struct anv_pipeline {
> >> >     bool                                         writes_depth;
> >> >     bool                                         depth_test_enable;
> >> >     bool                                         writes_stencil;
> >> > +   bool                                         stencil_test_enable;
> >> >     bool                                         depth_clamp_enable;
> >> >     bool                                         kill_pixel;
> >> >
> >> > diff --git a/src/intel/vulkan/gen8_cmd_buffer.c
> >> b/src/intel/vulkan/gen8_cmd_buffer.c
> >> > index 0628f3a..59fd537 100644
> >> > --- a/src/intel/vulkan/gen8_cmd_buffer.c
> >> > +++ b/src/intel/vulkan/gen8_cmd_buffer.c
> >> > @@ -157,16 +157,39 @@ __emit_sf_state(struct anv_cmd_buffer *cmd_buffer)
> >> >  void
> >> >  genX(cmd_buffer_enable_pma_fix)(struct anv_cmd_buffer *cmd_buffer,
> >> bool enable)
> >> >  {
> >> > -#if GEN_GEN == 8
> >> >     if (cmd_buffer->state.pma_fix_enabled == enable)
> >> >        return;
> >> >
> >> > +   cmd_buffer->state.pma_fix_enabled = enable;
> >> > +
> >> > +   /* According to the Broadwell PIPE_CONTROL documentation, software
> >> should
> >> > +    * emit a PIPE_CONTROL with the CS Stall and Depth Cache Flush bits
> >> set
> >> > +    * prior to the LRI.  If stencil buffer writes are enabled, then a
> >> Render
> >> > +    * Cache Flush is also necessary.
> >> > +    *
> >> > +    * The Skylake docs say to use a depth stall rather than a command
> >> > +    * streamer stall.  However, the hardware seems to violently
> >> disagree.
> >> > +    * A full command streamer stall seems to be needed in both cases.
> >> > +    */
> >> >     anv_batch_emit(&cmd_buffer->batch, GENX(PIPE_CONTROL), pc) {
> >> >        pc.DepthCacheFlushEnable = true;
> >> >        pc.CommandStreamerStallEnable = true;
> >> >        pc.RenderTargetCacheFlushEnable = true;
> >> >     }
> >> >
> >> > +#if GEN_GEN == 9
> >> > +
> >> > +   uint32_t cache_mode;
> >> > +   anv_pack_struct(&cache_mode, GENX(CACHE_MODE_0),
> >> > +                   .STCPMAOptimizationEnable = enable,
> >> > +                   .STCPMAOptimizationEnableMask = true);
> >> > +   anv_batch_emit(&cmd_buffer->batch, GENX(MI_LOAD_REGISTER_IMM),
> >> lri) {
> >> > +      lri.RegisterOffset   = GENX(CACHE_MODE_0_num);
> >> > +      lri.DataDWord        = cache_mode;
> >> > +   }
> >> > +
> >> > +#elif GEN_GEN == 8
> >> > +
> >> >     uint32_t cache_mode;
> >> >     anv_pack_struct(&cache_mode, GENX(CACHE_MODE_1),
> >> >                     .NPPMAFixEnable = enable,
> >> > @@ -178,18 +201,20 @@ genX(cmd_buffer_enable_pma_fix)(struct
> >> anv_cmd_buffer *cmd_buffer, bool enable)
> >> >        lri.DataDWord        = cache_mode;
> >> >     }
> >> >
> >> > +#endif /* GEN_GEN == 8 */
> >> > +
> >> >     /* After the LRI, a PIPE_CONTROL with both the Depth Stall and
> >> Depth Cache
> >> >      * Flush bits is often necessary.  We do it regardless because it's
> >> easier.
> >> >      * The render cache flush is also necessary if stencil writes are
> >> enabled.
> >> > +    *
> >> > +    * Again, the Skylake docs give a different set of flushes but the
> >> BDW
> >> > +    * flushes seem to work just as well.
> >> >      */
> >> >     anv_batch_emit(&cmd_buffer->batch, GENX(PIPE_CONTROL), pc) {
> >> >        pc.DepthStallEnable = true;
> >> >        pc.DepthCacheFlushEnable = true;
> >> >        pc.RenderTargetCacheFlushEnable = true;
> >> >     }
> >> > -
> >> > -   cmd_buffer->state.pma_fix_enabled = enable;
> >> > -#endif /* GEN_GEN == 8 */
> >> >  }
> >> >
> >> >  static inline bool
> >> > @@ -281,6 +306,126 @@ want_depth_pma_fix(struct anv_cmd_buffer
> >> *cmd_buffer)
> >> >            wm_prog_data->computed_depth_mode != PSCDEPTH_OFF;
> >> >  }
> >> >
> >> > +static inline bool
> >> > +want_stencil_pma_fix(struct anv_cmd_buffer *cmd_buffer)
> >> > +{
> >> > +   assert(GEN_GEN == 9);
> >> > +
> >> > +   /* From the Skylake PRM Vol. 2c CACHE_MODE_1::STC PMA Optimization
> >> Enable:
> >> > +    *
> >> > +    *    Clearing this bit will force the STC cache to wait for pending
> >> > +    *    retirement of pixels at the HZ-read stage and do the STC-test
> >> for
> >> > +    *    Non-promoted, R-computed and Computed depth modes instead of
> >> > +    *    postponing the STC-test to RCPFE.
> >> > +    *
> >> > +    *    STC_TEST_EN = 3DSTATE_STENCIL_BUFFER::STENCIL_BUFFER_ENABLE
> >> &&
> >> > +    *                  3DSTATE_WM_DEPTH_STENCIL::StencilTestEnable
> >> > +    *
> >> > +    *    STC_WRITE_EN = 3DSTATE_STENCIL_BUFFER::STENCIL_BUFFER_ENABLE
> >> &&
> >> > +    *                   (3DSTATE_WM_DEPTH_STENCIL::Stencil Buffer
> >> Write Enable &&
> >> > +    *                    3DSTATE_DEPTH_BUFFER::STENCIL_WRITE_ENABLE)
> >> > +    *
> >> > +    *    COMP_STC_EN = STC_TEST_EN &&
> >> > +    *                  3DSTATE_PS_EXTRA::PixelShaderComputesStencil
> >> > +    *
> >> > +    *    SW parses the pipeline states to generate the following
> >> logical
> >> > +    *    signal indicating if PMA FIX can be enabled.
> >> > +    *
> >> > +    *    STC_PMA_OPT =
> >> > +    *       3DSTATE_WM::ForceThreadDispatch != 1 &&
> >> > +    *       !(3DSTATE_RASTER::ForceSampleCount != NUMRASTSAMPLES_0) &&
> >> > +    *       3DSTATE_DEPTH_BUFFER::SURFACE_TYPE != NULL &&
> >> > +    *       3DSTATE_DEPTH_BUFFER::HIZ Enable &&
> >> > +    *       !(3DSTATE_WM::EDSC_Mode == 2) &&
> >> > +    *       3DSTATE_PS_EXTRA::PixelShaderValid &&
> >> > +    *       !(3DSTATE_WM_HZ_OP::DepthBufferClear ||
> >> > +    *         3DSTATE_WM_HZ_OP::DepthBufferResolve ||
> >> > +    *         3DSTATE_WM_HZ_OP::Hierarchical Depth Buffer Resolve
> >> Enable ||
> >> > +    *         3DSTATE_WM_HZ_OP::StencilBufferClear) &&
> >> > +    *       (COMP_STC_EN || STC_WRITE_EN) &&
> >> > +    *       ((3DSTATE_PS_EXTRA::PixelShaderKillsPixels ||
> >> > +    *         3DSTATE_WM::ForceKillPix == ON ||
> >> > +    *         3DSTATE_PS_EXTRA::oMask Present to RenderTarget ||
> >> > +    *         3DSTATE_PS_BLEND::AlphaToCoverageEnable ||
> >> > +    *         3DSTATE_PS_BLEND::AlphaTestEnable ||
> >> > +    *         3DSTATE_WM_CHROMAKEY::ChromaKeyKillEnable) ||
> >> > +    *        (3DSTATE_PS_EXTRA::Pixel Shader Computed Depth mode !=
> >> PSCDEPTH_OFF))
> >> > +    */
> >> > +
> >> > +   /* These are always true:
> >> > +    *    3DSTATE_WM::ForceThreadDispatch != 1 &&
> >> > +    *    !(3DSTATE_RASTER::ForceSampleCount != NUMRASTSAMPLES_0)
> >> > +    */
> >> > +
> >> > +   /* We only enable the PMA fix if we know for certain that HiZ is
> >> enabled.
> >> > +    * If we don't know whether HiZ is enabled or not, we disable the
> >> PMA fix
> >> > +    * and there is no harm.
> >> > +    *
> >> > +    * (3DSTATE_DEPTH_BUFFER::SURFACE_TYPE != NULL) &&
> >> > +    * 3DSTATE_DEPTH_BUFFER::HIZ Enable
> >> > +    */
> >> > +   if (!cmd_buffer->state.hiz_enabled)
> >> > +      return false;
> >> > +
> >> > +   /* We can't possibly know if HiZ is enabled without the framebuffer
> >> */
> >> > +   assert(cmd_buffer->state.framebuffer);
> >> > +
> >> > +   /* HiZ is enabled so we had better have a depth buffer with HiZ */
> >> > +   const struct anv_image_view *ds_iview =
> >> > +      anv_cmd_buffer_get_depth_stencil_view(cmd_buffer);
> >> > +   assert(ds_iview && ds_iview->image->aux_usage == ISL_AUX_USAGE_HIZ);
> >> > +
> >> > +   /* 3DSTATE_PS_EXTRA::PixelShaderValid */
> >> > +   struct anv_pipeline *pipeline = cmd_buffer->state.pipeline;
> >> > +   if (!anv_pipeline_has_stage(pipeline, MESA_SHADER_FRAGMENT))
> >> > +      return false;
> >> > +
> >> > +   /* !(3DSTATE_WM::EDSC_Mode == 2) */
> >> > +   const struct brw_wm_prog_data *wm_prog_data =
> >> get_wm_prog_data(pipeline);
> >> > +   if (wm_prog_data->early_fragment_tests)
> >> > +      return false;
> >> > +
> >> > +   /* We never use anv_pipeline for HiZ ops so this is trivially true:
> >> > +   *    !(3DSTATE_WM_HZ_OP::DepthBufferClear ||
> >> > +    *      3DSTATE_WM_HZ_OP::DepthBufferResolve ||
> >> > +    *      3DSTATE_WM_HZ_OP::Hierarchical Depth Buffer Resolve Enable
> >> ||
> >> > +    *      3DSTATE_WM_HZ_OP::StencilBufferClear)
> >> > +    */
> >> > +
> >> > +   /* 3DSTATE_STENCIL_BUFFER::STENCIL_BUFFER_ENABLE &&
> >> > +    * 3DSTATE_WM_DEPTH_STENCIL::StencilTestEnable
> >> > +    */
> >> > +   const bool stc_test_en =
> >> > +      (ds_iview->image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT) &&
> >> > +      pipeline->stencil_test_enable;
> >> > +
> >> > +   /* 3DSTATE_STENCIL_BUFFER::STENCIL_BUFFER_ENABLE &&
> >> > +    * (3DSTATE_WM_DEPTH_STENCIL::Stencil Buffer Write Enable &&
> >> > +    *  3DSTATE_DEPTH_BUFFER::STENCIL_WRITE_ENABLE)
> >> > +    */
> >> > +   const bool stc_write_en =
> >> > +      (ds_iview->image->aspects & VK_IMAGE_ASPECT_STENCIL_BIT) &&
> >> > +      pipeline->writes_stencil;
> >> > +
> >> > +   /* STC_TEST_EN && 3DSTATE_PS_EXTRA::PixelShaderComputesStencil */
> >> > +   const bool comp_stc_en = stc_test_en &&
> >> wm_prog_data->computed_stencil;
> >> > +
> >> > +   /* COMP_STC_EN || STC_WRITE_EN */
> >> > +   if (!(comp_stc_en || stc_write_en))
> >> > +      return false;
> >> > +
> >> > +   /* (3DSTATE_PS_EXTRA::PixelShaderKillsPixels ||
> >> > +    *  3DSTATE_WM::ForceKillPix == ON ||
> >> > +    *  3DSTATE_PS_EXTRA::oMask Present to RenderTarget ||
> >> > +    *  3DSTATE_PS_BLEND::AlphaToCoverageEnable ||
> >> > +    *  3DSTATE_PS_BLEND::AlphaTestEnable ||
> >> > +    *  3DSTATE_WM_CHROMAKEY::ChromaKeyKillEnable) ||
> >> > +    * (3DSTATE_PS_EXTRA::Pixel Shader Computed Depth mode !=
> >> PSCDEPTH_OFF)
> >> > +    */
> >> > +   return pipeline->kill_pixel ||
> >> > +          wm_prog_data->computed_depth_mode != PSCDEPTH_OFF;
> >> > +}
> >> > +
> >> >  void
> >> >  genX(cmd_buffer_flush_dynamic_state)(struct anv_cmd_buffer
> >> *cmd_buffer)
> >> >  {
> >> > @@ -390,6 +535,7 @@ genX(cmd_buffer_flush_dynamic_state)(struct
> >> anv_cmd_buffer *cmd_buffer)
> >> >     }
> >> >
> >> >     if (cmd_buffer->state.dirty & (ANV_CMD_DIRTY_PIPELINE |
> >> > +                                  ANV_CMD_DIRTY_RENDER_TARGETS |
> >> >                                    ANV_CMD_DIRTY_DYNAMIC_STENCIL_COMPARE_MASK
> >> |
> >> >                                    ANV_CMD_DIRTY_DYNAMIC_STENCIL_WRITE_MASK
> >> |
> >> >                                    ANV_CMD_DIRTY_DYNAMIC_STENCIL_REFERENCE))
> >> {
> >> > @@ -415,6 +561,9 @@ genX(cmd_buffer_flush_dynamic_state)(struct
> >> anv_cmd_buffer *cmd_buffer)
> >> >
> >> >        anv_batch_emit_merge(&cmd_buffer->batch, dwords,
> >> >                             pipeline->gen9.wm_depth_stencil);
> >> > +
> >> > +      genX(cmd_buffer_enable_pma_fix)(cmd_buffer,
> >> > +                                      want_stencil_pma_fix(cmd_buffe
> >> r));
> >> >     }
> >> >  #endif
> >> >
> >> > diff --git a/src/intel/vulkan/genX_pipeline.c
> >> b/src/intel/vulkan/genX_pipeline.c
> >> > index d2af8b9..3b0ae08 100644
> >> > --- a/src/intel/vulkan/genX_pipeline.c
> >> > +++ b/src/intel/vulkan/genX_pipeline.c
> >> > @@ -784,6 +784,7 @@ emit_ds_state(struct anv_pipeline *pipeline,
> >> >         * to make sure it's initialized to something useful.
> >> >         */
> >> >        pipeline->writes_stencil = false;
> >> > +      pipeline->stencil_test_enable = false;
> >> >        pipeline->writes_depth = false;
> >> >        pipeline->depth_test_enable = false;
> >> >        memset(depth_stencil_dw, 0, sizeof(depth_stencil_dw));
> >> > @@ -799,6 +800,7 @@ emit_ds_state(struct anv_pipeline *pipeline,
> >> >
> >> >     VkPipelineDepthStencilStateCreateInfo info = *pCreateInfo;
> >> >     sanitize_ds_state(&info, &pipeline->writes_stencil, ds_aspects);
> >> > +   pipeline->stencil_test_enable = info.stencilTestEnable;
> >> >     pipeline->writes_depth = info.depthWriteEnable;
> >> >     pipeline->depth_test_enable = info.depthTestEnable;
> >> >
> >> > @@ -1556,8 +1558,8 @@ compute_kill_pixel(struct anv_pipeline *pipeline,
> >> >     const struct brw_wm_prog_data *wm_prog_data =
> >> get_wm_prog_data(pipeline);
> >> >
> >> >     /* This computes the KillPixel portion of the computation for
> >> whether or
> >> > -    * not we want to enable the PMA fix on gen8.  It's given by this
> >> chunk of
> >> > -    * the giant formula:
> >> > +    * not we want to enable the PMA fix on gen8 or gen9.  It's given
> >> by this
> >> > +    * chunk of the giant formula:
> >> >      *
> >> >      *    (3DSTATE_PS_EXTRA::PixelShaderKillsPixels ||
> >> >      *     3DSTATE_PS_EXTRA::oMask Present to RenderTarget ||
> >> > --
> >> > 2.5.0.400.gff86faf
> >> >
> >> > _______________________________________________
> >> > mesa-dev mailing list
> >> > mesa-dev at lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >>
> >
> >


More information about the mesa-dev mailing list