Re: [PATCH] Revert "drm/amd/display: Optimize cursor position updates"

[Leo Li <sunpeng.li@xxxxxxx>]

On 2025-01-02 13:16, Aurabindo Pillai wrote:
> This reverts commit 742d670b416b272e42f6674e30e393bbb8ffa6d1.
>
> SW and HW state are not always matching in some cases causing cursor to
> be disabled.

With your SOB, this is

Reviewed-by: Leo Li <sunpeng.li@xxxxxxx>

> ---
>   drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp.c   |  7 +++----
>   .../gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c  |  6 ++----
>   drivers/gpu/drm/amd/display/dc/hubp/dcn20/dcn20_hubp.c |  8 +++-----
>   .../gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c   | 10 ++++------
>   4 files changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp.c b/drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp.c
> index 8f6529a98f31f..e1da48b05d009 100644
> --- a/drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp.c
> +++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp.c
> @@ -480,11 +480,10 @@ void dpp1_set_cursor_position(
>   	if (src_y_offset + cursor_height <= 0)
>   		cur_en = 0;  /* not visible beyond top edge*/
>   
> -	if (dpp_base->pos.cur0_ctl.bits.cur0_enable != cur_en) {
> -		REG_UPDATE(CURSOR0_CONTROL, CUR0_ENABLE, cur_en);
> +	REG_UPDATE(CURSOR0_CONTROL,
> +			CUR0_ENABLE, cur_en);
>   
> -		dpp_base->pos.cur0_ctl.bits.cur0_enable = cur_en;
> -	}
> +	dpp_base->pos.cur0_ctl.bits.cur0_enable = cur_en;
>   }
>   
>   void dpp1_cnv_set_optional_cursor_attributes(
> diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c
> index 1236e0f9a2560..3b6ca7974e188 100644
> --- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c
> +++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c
> @@ -154,11 +154,9 @@ void dpp401_set_cursor_position(
>   	struct dcn401_dpp *dpp = TO_DCN401_DPP(dpp_base);
>   	uint32_t cur_en = pos->enable ? 1 : 0;
>   
> -	if (dpp_base->pos.cur0_ctl.bits.cur0_enable != cur_en) {
> -		REG_UPDATE(CURSOR0_CONTROL, CUR0_ENABLE, cur_en);
> +	REG_UPDATE(CURSOR0_CONTROL, CUR0_ENABLE, cur_en);
>   
> -		dpp_base->pos.cur0_ctl.bits.cur0_enable = cur_en;
> -	}
> +	dpp_base->pos.cur0_ctl.bits.cur0_enable = cur_en;
>   }
>   
>   void dpp401_set_optional_cursor_attributes(
> diff --git a/drivers/gpu/drm/amd/display/dc/hubp/dcn20/dcn20_hubp.c b/drivers/gpu/drm/amd/display/dc/hubp/dcn20/dcn20_hubp.c
> index d537d0c53cf03..c74f6a3313a27 100644
> --- a/drivers/gpu/drm/amd/display/dc/hubp/dcn20/dcn20_hubp.c
> +++ b/drivers/gpu/drm/amd/display/dc/hubp/dcn20/dcn20_hubp.c
> @@ -1058,13 +1058,11 @@ void hubp2_cursor_set_position(
>   	if (src_y_offset + cursor_height <= 0)
>   		cur_en = 0;  /* not visible beyond top edge*/
>   
> -	if (hubp->pos.cur_ctl.bits.cur_enable != cur_en) {
> -		if (cur_en && REG_READ(CURSOR_SURFACE_ADDRESS) == 0)
> -			hubp->funcs->set_cursor_attributes(hubp, &hubp->curs_attr);
> +	if (cur_en && REG_READ(CURSOR_SURFACE_ADDRESS) == 0)
> +		hubp->funcs->set_cursor_attributes(hubp, &hubp->curs_attr);
>   
> -		REG_UPDATE(CURSOR_CONTROL,
> +	REG_UPDATE(CURSOR_CONTROL,
>   			CURSOR_ENABLE, cur_en);
> -	}
>   
>   	REG_SET_2(CURSOR_POSITION, 0,
>   			CURSOR_X_POSITION, pos->x,
> diff --git a/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c b/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c
> index 3595c74a3a2fb..d38e3f3a1107c 100644
> --- a/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c
> +++ b/drivers/gpu/drm/amd/display/dc/hubp/dcn401/dcn401_hubp.c
> @@ -730,13 +730,11 @@ void hubp401_cursor_set_position(
>   			dc_fixpt_from_int(dst_x_offset),
>   			param->h_scale_ratio));
>   
> -	if (hubp->pos.cur_ctl.bits.cur_enable != cur_en) {
> -		if (cur_en && REG_READ(CURSOR_SURFACE_ADDRESS) == 0)
> -			hubp->funcs->set_cursor_attributes(hubp, &hubp->curs_attr);
> +	if (cur_en && REG_READ(CURSOR_SURFACE_ADDRESS) == 0)
> +		hubp->funcs->set_cursor_attributes(hubp, &hubp->curs_attr);
>   
> -		REG_UPDATE(CURSOR_CONTROL,
> -			CURSOR_ENABLE, cur_en);
> -	}
> +	REG_UPDATE(CURSOR_CONTROL,
> +		CURSOR_ENABLE, cur_en);
>   
>   	REG_SET_2(CURSOR_POSITION, 0,
>   		CURSOR_X_POSITION, x_pos,