radv: disable binning state in some situations

Submitted by Samuel Pitoiset on March 22, 2018, 5:40 p.m.

Details

Message ID 20180322174032.18056-1-samuel.pitoiset@gmail.com
State New
Headers show
Series "radv: disable binning state in some situations" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset March 22, 2018, 5:40 p.m.
This is imported from RadeonSI/ADMVLK. That might or might not
improve performance. Anyway, it's still disabled by default.

Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/vulkan/radv_pipeline.c | 56 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index dd5baec117..6c0d552c81 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -2134,6 +2134,59 @@  radv_compute_bin_size(struct radv_pipeline *pipeline, const VkGraphicsPipelineCr
 	return extent;
 }
 
+static bool
+radv_is_depth_write_enabled(const VkPipelineDepthStencilStateCreateInfo *pCreateInfo)
+{
+	assert(pCreateInfo);
+	return pCreateInfo->depthTestEnable &&
+	       pCreateInfo->depthWriteEnable &&
+	       pCreateInfo->depthCompareOp != VK_COMPARE_OP_NEVER;
+}
+
+static bool
+radv_is_stencil_write_enabled(const VkPipelineDepthStencilStateCreateInfo *pCreateInfo)
+{
+	assert(pCreateInfo);
+	return pCreateInfo->stencilTestEnable &&
+	       (pCreateInfo->front.failOp != VK_STENCIL_OP_KEEP ||
+		pCreateInfo->front.passOp != VK_STENCIL_OP_KEEP ||
+		pCreateInfo->front.depthFailOp != VK_STENCIL_OP_KEEP ||
+		pCreateInfo->back.failOp != VK_STENCIL_OP_KEEP ||
+		pCreateInfo->back.passOp != VK_STENCIL_OP_KEEP ||
+		pCreateInfo->back.depthFailOp != VK_STENCIL_OP_KEEP);
+}
+
+static bool
+radv_should_enable_pbb(struct radv_pipeline *pipeline,
+		       const VkGraphicsPipelineCreateInfo *pCreateInfo)
+{
+	RADV_FROM_HANDLE(radv_render_pass, pass, pCreateInfo->renderPass);
+	struct radv_subpass *subpass = pass->subpasses + pCreateInfo->subpass;
+	struct radv_shader_variant *ps = pipeline->shaders[MESA_SHADER_FRAGMENT];
+
+	if (!pipeline->device->pbb_allowed)
+		return false;
+
+	/* Whether we can disable binning state based on the pixel shader, MSAA
+	 * states and depth/stencil writes.
+	 */
+	bool ps_can_kill = ps->info.fs.can_discard ||
+			   ps->info.info.ps.writes_sample_mask ||
+			   pCreateInfo->pMultisampleState;
+	bool ps_can_reject_z_trivially = !ps->info.info.ps.writes_z;
+	bool ds_write_enabled =
+		pCreateInfo->pDepthStencilState &&
+		subpass->depth_stencil_attachment.attachment != VK_ATTACHMENT_UNUSED &&
+		(radv_is_depth_write_enabled(pCreateInfo->pDepthStencilState) ||
+		 radv_is_stencil_write_enabled(pCreateInfo->pDepthStencilState));
+
+	/* Disable binning state if PS can kill trivially with DB writes. */
+	if (ps_can_kill && ps_can_reject_z_trivially && ds_write_enabled)
+		return false;
+
+	return true;
+}
+
 static void
 radv_pipeline_generate_binning_state(struct radeon_winsys_cs *cs,
 				     struct radv_pipeline *pipeline,
@@ -2168,7 +2221,8 @@  radv_pipeline_generate_binning_state(struct radeon_winsys_cs *cs,
 		unreachable("unhandled family while determining binning state.");
 	}
 
-	if (pipeline->device->pbb_allowed && bin_size.width && bin_size.height) {
+	if (radv_should_enable_pbb(pipeline, pCreateInfo) &&
+	    bin_size.width && bin_size.height) {
 		pa_sc_binner_cntl_0 =
 	                S_028C44_BINNING_MODE(V_028C44_BINNING_ALLOWED) |
 	                S_028C44_BIN_SIZE_X(bin_size.width == 16) |

Comments

I'd rather we figure out a story of when it is faster. I tried a lot
of stuff with the currently available games, and getting it
consistently faster was difficult.

So if you have a raven ridge, feel free to try something, but I'm not
really a fan of copying it blindly without understanding it.

On Thu, Mar 22, 2018 at 6:40 PM, Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
> This is imported from RadeonSI/ADMVLK. That might or might not
> improve performance. Anyway, it's still disabled by default.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/radv_pipeline.c | 56 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
> index dd5baec117..6c0d552c81 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -2134,6 +2134,59 @@ radv_compute_bin_size(struct radv_pipeline *pipeline, const VkGraphicsPipelineCr
>         return extent;
>  }
>
> +static bool
> +radv_is_depth_write_enabled(const VkPipelineDepthStencilStateCreateInfo *pCreateInfo)
> +{
> +       assert(pCreateInfo);
> +       return pCreateInfo->depthTestEnable &&
> +              pCreateInfo->depthWriteEnable &&
> +              pCreateInfo->depthCompareOp != VK_COMPARE_OP_NEVER;
> +}
> +
> +static bool
> +radv_is_stencil_write_enabled(const VkPipelineDepthStencilStateCreateInfo *pCreateInfo)
> +{
> +       assert(pCreateInfo);
> +       return pCreateInfo->stencilTestEnable &&
> +              (pCreateInfo->front.failOp != VK_STENCIL_OP_KEEP ||
> +               pCreateInfo->front.passOp != VK_STENCIL_OP_KEEP ||
> +               pCreateInfo->front.depthFailOp != VK_STENCIL_OP_KEEP ||
> +               pCreateInfo->back.failOp != VK_STENCIL_OP_KEEP ||
> +               pCreateInfo->back.passOp != VK_STENCIL_OP_KEEP ||
> +               pCreateInfo->back.depthFailOp != VK_STENCIL_OP_KEEP);
> +}
> +
> +static bool
> +radv_should_enable_pbb(struct radv_pipeline *pipeline,
> +                      const VkGraphicsPipelineCreateInfo *pCreateInfo)
> +{
> +       RADV_FROM_HANDLE(radv_render_pass, pass, pCreateInfo->renderPass);
> +       struct radv_subpass *subpass = pass->subpasses + pCreateInfo->subpass;
> +       struct radv_shader_variant *ps = pipeline->shaders[MESA_SHADER_FRAGMENT];
> +
> +       if (!pipeline->device->pbb_allowed)
> +               return false;
> +
> +       /* Whether we can disable binning state based on the pixel shader, MSAA
> +        * states and depth/stencil writes.
> +        */
> +       bool ps_can_kill = ps->info.fs.can_discard ||
> +                          ps->info.info.ps.writes_sample_mask ||
> +                          pCreateInfo->pMultisampleState;
> +       bool ps_can_reject_z_trivially = !ps->info.info.ps.writes_z;
> +       bool ds_write_enabled =
> +               pCreateInfo->pDepthStencilState &&
> +               subpass->depth_stencil_attachment.attachment != VK_ATTACHMENT_UNUSED &&
> +               (radv_is_depth_write_enabled(pCreateInfo->pDepthStencilState) ||
> +                radv_is_stencil_write_enabled(pCreateInfo->pDepthStencilState));
> +
> +       /* Disable binning state if PS can kill trivially with DB writes. */
> +       if (ps_can_kill && ps_can_reject_z_trivially && ds_write_enabled)
> +               return false;
> +
> +       return true;
> +}
> +
>  static void
>  radv_pipeline_generate_binning_state(struct radeon_winsys_cs *cs,
>                                      struct radv_pipeline *pipeline,
> @@ -2168,7 +2221,8 @@ radv_pipeline_generate_binning_state(struct radeon_winsys_cs *cs,
>                 unreachable("unhandled family while determining binning state.");
>         }
>
> -       if (pipeline->device->pbb_allowed && bin_size.width && bin_size.height) {
> +       if (radv_should_enable_pbb(pipeline, pCreateInfo) &&
> +           bin_size.width && bin_size.height) {
>                 pa_sc_binner_cntl_0 =
>                         S_028C44_BINNING_MODE(V_028C44_BINNING_ALLOWED) |
>                         S_028C44_BIN_SIZE_X(bin_size.width == 16) |
> --
> 2.16.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev