[PATCH] drm/amd/display: Only enable cursor on pipes that need it

[PATCH] drm/amd/display: Only enable cursor on pipes that need it

[Nicholas Kazlauskas]

[Why]
In current code we're essentially drawing the cursor on every pipe
that contains it. This only works when the planes have the same
scaling for src to dest rect, otherwise we'll get "double cursor" where
one cursor is incorrectly filtered and offset from the real position.

[How]
Without dedicated cursor planes on DCN we require at least one pipe
that matches the scaling of the current timing.

This is an optimization and workaround for the most common case where
the top-most plane is not scaled but the bottom-most plane is scaled.

Whenever a pipe has a parent pipe in the blending tree whose recout
fully contains the current pipe we can disable the pipe.

This only applies when the pipe is actually visible of course.

Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
index f2127afb37b2..1008ac8a0f2a 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
@@ -2911,6 +2911,33 @@ void dcn10_update_dchub(struct dce_hwseq *hws, struct dchub_init_data *dh_data)
 	hubbub->funcs->update_dchub(hubbub, dh_data);
 }
 
+static bool dcn10_can_pipe_disable_cursor(struct pipe_ctx *pipe_ctx)
+{
+	struct pipe_ctx *test_pipe;
+	const struct rect *r1 = &pipe_ctx->plane_res.scl_data.recout, *r2;
+	int r1_r = r1->x + r1->width, r1_b = r1->y + r1->height, r2_r, r2_b;
+
+	/**
+	 * Disable the cursor if there's another pipe above this with a
+	 * plane that contains this pipe's viewport to prevent double cursor
+	 * and incorrect scaling artifacts.
+	 */
+	for (test_pipe = pipe_ctx->top_pipe; test_pipe;
+	     test_pipe = test_pipe->top_pipe) {
+		if (!test_pipe->plane_state->visible)
+			continue;
+
+		r2 = &test_pipe->plane_res.scl_data.recout;
+		r2_r = r2->x + r2->width;
+		r2_b = r2->y + r2->height;
+
+		if (r1->x >= r2->x && r1->y >= r2->y && r1_r <= r2_r && r1_b <= r2_b)
+			return true;
+	}
+
+	return false;
+}
+
 void dcn10_set_cursor_position(struct pipe_ctx *pipe_ctx)
 {
 	struct dc_cursor_position pos_cpy = pipe_ctx->stream->cursor_position;
@@ -2956,6 +2983,9 @@ void dcn10_set_cursor_position(struct pipe_ctx *pipe_ctx)
 			== PLN_ADDR_TYPE_VIDEO_PROGRESSIVE)
 		pos_cpy.enable = false;
 
+	if (pos_cpy.enable && dcn10_can_pipe_disable_cursor(pipe_ctx))
+		pos_cpy.enable = false;
+
 	// Swap axis and mirror horizontally
 	if (param.rotation == ROTATION_ANGLE_90) {
 		uint32_t temp_x = pos_cpy.x;
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: Only enable cursor on pipes that need it

[Harry Wentland]

On 2020-01-30 1:29 p.m., Nicholas Kazlauskas wrote:
> [Why]
> In current code we're essentially drawing the cursor on every pipe
> that contains it. This only works when the planes have the same
> scaling for src to dest rect, otherwise we'll get "double cursor" where
> one cursor is incorrectly filtered and offset from the real position.
> 
> [How]
> Without dedicated cursor planes on DCN we require at least one pipe
> that matches the scaling of the current timing.
> 
> This is an optimization and workaround for the most common case where
> the top-most plane is not scaled but the bottom-most plane is scaled.
> 
> Whenever a pipe has a parent pipe in the blending tree whose recout
> fully contains the current pipe we can disable the pipe.
> 
> This only applies when the pipe is actually visible of course.
> 
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index f2127afb37b2..1008ac8a0f2a 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -2911,6 +2911,33 @@ void dcn10_update_dchub(struct dce_hwseq *hws, struct dchub_init_data *dh_data)
>  	hubbub->funcs->update_dchub(hubbub, dh_data);
>  }
>  
> +static bool dcn10_can_pipe_disable_cursor(struct pipe_ctx *pipe_ctx)
> +{
> +	struct pipe_ctx *test_pipe;
> +	const struct rect *r1 = &pipe_ctx->plane_res.scl_data.recout, *r2;
> +	int r1_r = r1->x + r1->width, r1_b = r1->y + r1->height, r2_r, r2_b;
> +
> +	/**
> +	 * Disable the cursor if there's another pipe above this with a
> +	 * plane that contains this pipe's viewport to prevent double cursor
> +	 * and incorrect scaling artifacts.
> +	 */
> +	for (test_pipe = pipe_ctx->top_pipe; test_pipe;
> +	     test_pipe = test_pipe->top_pipe) {
> +		if (!test_pipe->plane_state->visible)
> +			continue;
> +
> +		r2 = &test_pipe->plane_res.scl_data.recout;
> +		r2_r = r2->x + r2->width;
> +		r2_b = r2->y + r2->height;
> +
> +		if (r1->x >= r2->x && r1->y >= r2->y && r1_r <= r2_r && r1_b <= r2_b)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  void dcn10_set_cursor_position(struct pipe_ctx *pipe_ctx)
>  {
>  	struct dc_cursor_position pos_cpy = pipe_ctx->stream->cursor_position;
> @@ -2956,6 +2983,9 @@ void dcn10_set_cursor_position(struct pipe_ctx *pipe_ctx)
>  			== PLN_ADDR_TYPE_VIDEO_PROGRESSIVE)
>  		pos_cpy.enable = false;
>  
> +	if (pos_cpy.enable && dcn10_can_pipe_disable_cursor(pipe_ctx))
> +		pos_cpy.enable = false;
> +
>  	// Swap axis and mirror horizontally
>  	if (param.rotation == ROTATION_ANGLE_90) {
>  		uint32_t temp_x = pos_cpy.x;
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx