On 2021-09-29 15:06, Simon Ser wrote:
> The current logic checks whether the cursor plane blending
> properties match the primary plane's. However that's wrong,
> because the cursor is painted on all planes underneath. If
> the cursor is over the primary plane and the cursor plane,
Do you mean "and the underlay plane" here, instead of "and
the cursor plane"?
Harry
> it's painted on both pipes.
>
> Iterate over the CRTC planes and check their scaling match
> the cursor's.
>
> Signed-off-by: Simon Ser <contact@xxxxxxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: Harry Wentland <hwentlan@xxxxxxx>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>
> Cc: Bas Nieuwenhuizen <bas@xxxxxxxxxxxxxxxxxxx>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 49 +++++++++++++------
> 1 file changed, 34 insertions(+), 15 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 3c7a8f869b40..6472c0032b54 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -10505,18 +10505,18 @@ static int dm_check_crtc_cursor(struct drm_atomic_state *state,
> struct drm_crtc *crtc,
> struct drm_crtc_state *new_crtc_state)
> {
> - struct drm_plane_state *new_cursor_state, *new_primary_state;
> - int cursor_scale_w, cursor_scale_h, primary_scale_w, primary_scale_h;
> + struct drm_plane *cursor = crtc->cursor, *underlying;
> + struct drm_plane_state *new_cursor_state, *new_underlying_state;
> + int i;
> + int cursor_scale_w, cursor_scale_h, underlying_scale_w, underlying_scale_h;
>
> /* On DCE and DCN there is no dedicated hardware cursor plane. We get a
> * cursor per pipe but it's going to inherit the scaling and
> * positioning from the underlying pipe. Check the cursor plane's
> - * blending properties match the primary plane's. */
> + * blending properties match the underlying planes'. */
>
> - new_cursor_state = drm_atomic_get_new_plane_state(state, crtc->cursor);
> - new_primary_state = drm_atomic_get_new_plane_state(state, crtc->primary);
> - if (!new_cursor_state || !new_primary_state ||
> - !new_cursor_state->fb || !new_primary_state->fb) {
> + new_cursor_state = drm_atomic_get_new_plane_state(state, cursor);
> + if (!new_cursor_state || !new_cursor_state->fb) {
> return 0;
> }
>
> @@ -10525,15 +10525,34 @@ static int dm_check_crtc_cursor(struct drm_atomic_state *state,
> cursor_scale_h = new_cursor_state->crtc_h * 1000 /
> (new_cursor_state->src_h >> 16);
>
> - primary_scale_w = new_primary_state->crtc_w * 1000 /
> - (new_primary_state->src_w >> 16);
> - primary_scale_h = new_primary_state->crtc_h * 1000 /
> - (new_primary_state->src_h >> 16);
> + for_each_new_plane_in_state_reverse(state, underlying, new_underlying_state, i) {
> + /* Narrow down to non-cursor planes on the same CRTC as the cursor */
> + if (new_underlying_state->crtc != crtc || underlying == crtc->cursor)
> + continue;
>
> - if (cursor_scale_w != primary_scale_w ||
> - cursor_scale_h != primary_scale_h) {
> - drm_dbg_atomic(crtc->dev, "Cursor plane scaling doesn't match primary plane\n");
> - return -EINVAL;
> + /* Ignore disabled planes */
> + if (!new_underlying_state->fb)
> + continue;
> +
> + underlying_scale_w = new_underlying_state->crtc_w * 1000 /
> + (new_underlying_state->src_w >> 16);
> + underlying_scale_h = new_underlying_state->crtc_h * 1000 /
> + (new_underlying_state->src_h >> 16);
> +
> + if (cursor_scale_w != underlying_scale_w ||
> + cursor_scale_h != underlying_scale_h) {
> + drm_dbg_atomic(crtc->dev,
> + "Cursor [PLANE:%d:%s] scaling doesn't match underlying [PLANE:%d:%s]\n",
> + cursor->base.id, cursor->name, underlying->base.id, underlying->name);
> + return -EINVAL;
> + }
> +
> + /* If this plane covers the whole CRTC, no need to check planes underneath */
> + if (new_underlying_state->crtc_x <= 0 &&
> + new_underlying_state->crtc_y <= 0 &&
> + new_underlying_state->crtc_x + new_underlying_state->crtc_w >= new_crtc_state->mode.hdisplay &&
> + new_underlying_state->crtc_y + new_underlying_state->crtc_h >= new_crtc_state->mode.vdisplay)
> + break;
> }
>
> return 0;
>
