[PATCH 3/6] amd/display: fail on cursor plane without an underlying plane

[Michel Dänzer]

On 2021-02-19 7:58 p.m., Simon Ser wrote:
> Make sure there's an underlying pipe that can be used for the
> cursor.
> 
> Signed-off-by: Simon Ser <contact at emersion.fr>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: Harry Wentland <hwentlan at amd.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> 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 acbe1537e7cf..a5d6010405bf 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9226,9 +9226,14 @@ static int dm_check_crtc_cursor(struct drm_atomic_state *state,
>   	}
>   
>   	new_cursor_state = drm_atomic_get_new_plane_state(state, crtc->cursor);
> -	if (!new_cursor_state || !new_underlying_state || !new_cursor_state->fb)
> +	if (!new_cursor_state || !new_cursor_state->fb)
>   		return 0;
>   
> +	if (!new_underlying_state || !new_underlying_state->fb) {
> +		drm_dbg_atomic(crtc->dev, "Cursor plane can't be enabled without underlying plane\n");
> +		return -EINVAL;
> +	}
> +
>   	cursor_scale_w = new_cursor_state->crtc_w * 1000 /
>   			 (new_cursor_state->src_w >> 16);
>   	cursor_scale_h = new_cursor_state->crtc_h * 1000 /
> 

Houston, we have a problem I'm afraid. Adding Daniel.


If the primary plane is enabled with a format which isn't compatible with the HW cursor, and no overlay plane is enabled, the same issues as described in b836a274b797 "drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is" can again occur:


* The legacy cursor ioctl fails with EINVAL for a non-0 cursor FB ID

   (which enables the cursor plane).

* If the cursor plane is enabled (e.g. using the legacy cursor ioctl
   during DPMS off), changing the legacy DPMS property
  value from off to
   on fails with EINVAL.


Moreover, in the same scenario plus an overlay plane enabled with a HW cursor compatible format, if the FB bound to the overlay plane is destroyed, the common DRM code will attempt to disable the overlay plane, but dm_check_crtc_cursor will reject that now. I can't remember exactly what the result is, but AFAIR it's not pretty.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer