[Mesa-dev] [PATCH] radv: prevent dirtying of dynamic state when it does not change

Samuel Pitoiset samuel.pitoiset at gmail.com
Wed Jan 16 08:08:12 UTC 2019


I wrote a similar patch in the past but I didn't notice any improvements.

Though, as we already check state changes in some other places, I think 
it's better to be consistent.

With the TODO comment removed, patch is:

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

On 1/15/19 10:59 PM, Rhys Perry wrote:
> DXVK often sets dynamic state without actually changing it.
>
> Signed-off-by: Rhys Perry <pendingchaos02 at gmail.com>
> ---
>   src/amd/vulkan/radv_cmd_buffer.c | 92 ++++++++++++++++++++++++++------
>   1 file changed, 76 insertions(+), 16 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 59903ab64d8..56b3c934c2e 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -2965,6 +2965,11 @@ void radv_CmdSetViewport(
>   	assert(firstViewport < MAX_VIEWPORTS);
>   	assert(total_count >= 1 && total_count <= MAX_VIEWPORTS);
>   
> +	if (!memcmp(state->dynamic.viewport.viewports + firstViewport,
> +		    pViewports, viewportCount * sizeof(*pViewports))) {
> +		return;
> +	}
> +
>   	memcpy(state->dynamic.viewport.viewports + firstViewport, pViewports,
>   	       viewportCount * sizeof(*pViewports));
>   
> @@ -2984,6 +2989,11 @@ void radv_CmdSetScissor(
>   	assert(firstScissor < MAX_SCISSORS);
>   	assert(total_count >= 1 && total_count <= MAX_SCISSORS);
>   
> +	if (!memcmp(state->dynamic.scissor.scissors + firstScissor, pScissors,
> +		    scissorCount * sizeof(*pScissors))) {
> +		return;
> +	}
> +
>   	memcpy(state->dynamic.scissor.scissors + firstScissor, pScissors,
>   	       scissorCount * sizeof(*pScissors));
>   
> @@ -2995,6 +3005,10 @@ void radv_CmdSetLineWidth(
>   	float                                       lineWidth)
>   {
>   	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +
> +	if (cmd_buffer->state.dynamic.line_width == lineWidth)
> +		return;
> +
>   	cmd_buffer->state.dynamic.line_width = lineWidth;
>   	cmd_buffer->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_LINE_WIDTH;
>   }
> @@ -3006,12 +3020,19 @@ void radv_CmdSetDepthBias(
>   	float                                       depthBiasSlopeFactor)
>   {
>   	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +	struct radv_cmd_state *state = &cmd_buffer->state;
>   
> -	cmd_buffer->state.dynamic.depth_bias.bias = depthBiasConstantFactor;
> -	cmd_buffer->state.dynamic.depth_bias.clamp = depthBiasClamp;
> -	cmd_buffer->state.dynamic.depth_bias.slope = depthBiasSlopeFactor;
> +	if (state->dynamic.depth_bias.bias == depthBiasConstantFactor &&
> +	    state->dynamic.depth_bias.clamp == depthBiasClamp &&
> +	    state->dynamic.depth_bias.slope == depthBiasSlopeFactor) {
> +		return;
> +	}
>   
> -	cmd_buffer->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_DEPTH_BIAS;
> +	state->dynamic.depth_bias.bias = depthBiasConstantFactor;
> +	state->dynamic.depth_bias.clamp = depthBiasClamp;
> +	state->dynamic.depth_bias.slope = depthBiasSlopeFactor;
> +
> +	state->dirty |= RADV_CMD_DIRTY_DYNAMIC_DEPTH_BIAS;
>   }
>   
>   void radv_CmdSetBlendConstants(
> @@ -3019,11 +3040,14 @@ void radv_CmdSetBlendConstants(
>   	const float                                 blendConstants[4])
>   {
>   	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +	struct radv_cmd_state *state = &cmd_buffer->state;
>   
> -	memcpy(cmd_buffer->state.dynamic.blend_constants,
> -	       blendConstants, sizeof(float) * 4);
> +	if (!memcmp(state->dynamic.blend_constants, blendConstants, sizeof(float) * 4))
> +		return;
> +
> +	memcpy(state->dynamic.blend_constants, blendConstants, sizeof(float) * 4);
>   
> -	cmd_buffer->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_BLEND_CONSTANTS;
> +	state->dirty |= RADV_CMD_DIRTY_DYNAMIC_BLEND_CONSTANTS;
>   }
>   
>   void radv_CmdSetDepthBounds(
> @@ -3032,11 +3056,17 @@ void radv_CmdSetDepthBounds(
>   	float                                       maxDepthBounds)
>   {
>   	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +	struct radv_cmd_state *state = &cmd_buffer->state;
>   
> -	cmd_buffer->state.dynamic.depth_bounds.min = minDepthBounds;
> -	cmd_buffer->state.dynamic.depth_bounds.max = maxDepthBounds;
> +	if (state->dynamic.depth_bounds.min == minDepthBounds &&
> +	    state->dynamic.depth_bounds.max == maxDepthBounds) {
> +		return;
> +	}
> +
> +	state->dynamic.depth_bounds.min = minDepthBounds;
> +	state->dynamic.depth_bounds.max = maxDepthBounds;
>   
> -	cmd_buffer->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_DEPTH_BOUNDS;
> +	state->dirty |= RADV_CMD_DIRTY_DYNAMIC_DEPTH_BOUNDS;
>   }
>   
>   void radv_CmdSetStencilCompareMask(
> @@ -3045,13 +3075,21 @@ void radv_CmdSetStencilCompareMask(
>   	uint32_t                                    compareMask)
>   {
>   	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +	struct radv_cmd_state *state = &cmd_buffer->state;
> +	bool front_same = state->dynamic.stencil_compare_mask.front == compareMask;
> +	bool back_same = state->dynamic.stencil_compare_mask.back == compareMask;
> +
> +	if ((!(faceMask & VK_STENCIL_FACE_FRONT_BIT) || front_same) &&
> +	    (!(faceMask & VK_STENCIL_FACE_BACK_BIT) || back_same)) {
> +		return;
> +	}
>   
>   	if (faceMask & VK_STENCIL_FACE_FRONT_BIT)
> -		cmd_buffer->state.dynamic.stencil_compare_mask.front = compareMask;
> +		state->dynamic.stencil_compare_mask.front = compareMask;
>   	if (faceMask & VK_STENCIL_FACE_BACK_BIT)
> -		cmd_buffer->state.dynamic.stencil_compare_mask.back = compareMask;
> +		state->dynamic.stencil_compare_mask.back = compareMask;
>   
> -	cmd_buffer->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_STENCIL_COMPARE_MASK;
> +	state->dirty |= RADV_CMD_DIRTY_DYNAMIC_STENCIL_COMPARE_MASK;
>   }
>   
>   void radv_CmdSetStencilWriteMask(
> @@ -3060,13 +3098,21 @@ void radv_CmdSetStencilWriteMask(
>   	uint32_t                                    writeMask)
>   {
>   	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +	struct radv_cmd_state *state = &cmd_buffer->state;
> +	bool front_same = state->dynamic.stencil_write_mask.front == writeMask;
> +	bool back_same = state->dynamic.stencil_write_mask.back == writeMask;
> +
> +	if ((!(faceMask & VK_STENCIL_FACE_FRONT_BIT) || front_same) &&
> +	    (!(faceMask & VK_STENCIL_FACE_BACK_BIT) || back_same)) {
> +		return;
> +	}
>   
>   	if (faceMask & VK_STENCIL_FACE_FRONT_BIT)
> -		cmd_buffer->state.dynamic.stencil_write_mask.front = writeMask;
> +		state->dynamic.stencil_write_mask.front = writeMask;
>   	if (faceMask & VK_STENCIL_FACE_BACK_BIT)
> -		cmd_buffer->state.dynamic.stencil_write_mask.back = writeMask;
> +		state->dynamic.stencil_write_mask.back = writeMask;
>   
> -	cmd_buffer->state.dirty |= RADV_CMD_DIRTY_DYNAMIC_STENCIL_WRITE_MASK;
> +	state->dirty |= RADV_CMD_DIRTY_DYNAMIC_STENCIL_WRITE_MASK;
>   }
>   
>   void radv_CmdSetStencilReference(
> @@ -3075,6 +3121,14 @@ void radv_CmdSetStencilReference(
>   	uint32_t                                    reference)
>   {
>   	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
> +	struct radv_cmd_state *state = &cmd_buffer->state;
> +	bool front_same = state->dynamic.stencil_reference.front == reference;
> +	bool back_same = state->dynamic.stencil_reference.back == reference;
> +
> +	if ((!(faceMask & VK_STENCIL_FACE_FRONT_BIT) || front_same) &&
> +	    (!(faceMask & VK_STENCIL_FACE_BACK_BIT) || back_same)) {
> +		return;
> +	}
>   
>   	if (faceMask & VK_STENCIL_FACE_FRONT_BIT)
>   		cmd_buffer->state.dynamic.stencil_reference.front = reference;
> @@ -3097,6 +3151,11 @@ void radv_CmdSetDiscardRectangleEXT(
>   	assert(firstDiscardRectangle < MAX_DISCARD_RECTANGLES);
>   	assert(total_count >= 1 && total_count <= MAX_DISCARD_RECTANGLES);
>   
> +	if (!memcmp(state->dynamic.discard_rectangle.rectangles + firstDiscardRectangle,
> +		    pDiscardRectangles, discardRectangleCount * sizeof(*pDiscardRectangles))) {
> +		return;
> +	}
> +
>   	typed_memcpy(&state->dynamic.discard_rectangle.rectangles[firstDiscardRectangle],
>   	             pDiscardRectangles, discardRectangleCount);
>   
> @@ -3648,6 +3707,7 @@ static bool radv_need_late_scissor_emission(struct radv_cmd_buffer *cmd_buffer,
>   	if (cmd_buffer->state.dirty & used_states)
>   		return true;
>   
> +	//TODO(pendingchaos): this doesn't apply for GFX9?
>   	if (indexed_draw && state->pipeline->graphics.prim_restart_enable &&
>   	    (state->index_type ? 0xffffffffu : 0xffffu) != state->last_primitive_reset_index)
>   		return true;


More information about the mesa-dev mailing list