Re: [PATCH] drm/amd/display: Reject cursor plane on DCE when scaled differently than primary

[Rodrigo Siqueira <siqueira@igalia.com>]

On 01/18, Timur Kristóf wrote:
> Currently DCE doesn't support the overlay cursor, so the
> dm_crtc_get_cursor_mode() function returns DM_CURSOR_NATIVE_MODE
> unconditionally. The outcome is that it doesn't check for the
> conditions that would necessitate the overlay cursor, meaning
> that it doesn't reject cases where the native cursor mode isn't
> supported on DCE.
> 
> Remove the early return from dm_crtc_get_cursor_mode() for
> DCE and instead let it perform the necessary checks and
> return DM_CURSOR_OVERLAY_MODE. Add a later check that rejects
> when DM_CURSOR_OVERLAY_MODE would be used with DCE.
> 
> Fixes: 1b04dcca4fb1 ("drm/amd/display: Introduce overlay cursor mode")
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4600
> Suggested-by: Leo Li <sunpeng.li@amd.com>
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 655c9fcb078a..3e1ba5521f2b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -12262,10 +12262,9 @@ static int dm_crtc_get_cursor_mode(struct amdgpu_device *adev,
>  
>  	/* Overlay cursor not supported on HW before DCN
>  	 * DCN401 does not have the cursor-on-scaled-plane or cursor-on-yuv-plane restrictions
> -	 * as previous DCN generations, so enable native mode on DCN401 in addition to DCE
> +	 * as previous DCN generations, so enable native mode on DCN401
>  	 */
> -	if (amdgpu_ip_version(adev, DCE_HWIP, 0) == 0 ||
> -	    amdgpu_ip_version(adev, DCE_HWIP, 0) == IP_VERSION(4, 0, 1)) {
> +	if (amdgpu_ip_version(adev, DCE_HWIP, 0) == IP_VERSION(4, 0, 1)) {
>  		*cursor_mode = DM_CURSOR_NATIVE_MODE;
>  		return 0;
>  	}
> @@ -12585,6 +12584,12 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>  		 * need to be added for DC to not disable a plane by mistake
>  		 */
>  		if (dm_new_crtc_state->cursor_mode == DM_CURSOR_OVERLAY_MODE) {
> +			if (amdgpu_ip_version(adev, DCE_HWIP, 0) == 0) {
> +				drm_dbg(dev, "Overlay cursor not supported on DCE\n");
> +				ret = -EINVAL;
> +				goto fail;
> +			}
> +
>  			ret = drm_atomic_add_affected_planes(state, crtc);
>  			if (ret)
>  				goto fail;
> -- 
> 2.52.0
>

Hi,

This change lgtm.

Alex, Dan,
Could you include this patch in the next week promotion? 

Reviewed-by: Rodrigo Siqueira <siqueira@igalia.com>

Thanks

-- 
Rodrigo Siqueira