Re: [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes

["Christopher Snowhill" <chris@kode54.net>]

On Fri Jun 20, 2025 at 3:10 AM PDT, Christopher Snowhill wrote:
> Here's another alternative change, which may be more thorough. It does
> seem to fix the issue, at least. The issue does indeed appear to be
> no-op plane changes sent to the cursor plane.
>
> If anyone wants to propose style changes, and suggest a proper commit
> message, if this is indeed a welcome fix for the problem, please let me
> know.
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index c2726af6698e..b741939698e8 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1087,17 +1087,22 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
>  			}
>
>  			/* ask the driver if this non-primary plane is supported */
> -			if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
> -				ret = -EINVAL;
> +			else if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
> +				ret = drm_atomic_plane_get_property(plane, plane_state,
> +								    prop, &old_val);
> +
> +				if (ret || old_val != prop_value) {
> +					ret = -EINVAL;
>
> -				if (plane_funcs && plane_funcs->atomic_async_check)
> -					ret = plane_funcs->atomic_async_check(plane, state, true);
> +					if (plane_funcs && plane_funcs->atomic_async_check)
> +						ret = plane_funcs->atomic_async_check(plane, state, true);
>
> -				if (ret) {
> -					drm_dbg_atomic(prop->dev,
> -						       "[PLANE:%d:%s] does not support async flips\n",
> -						       obj->id, plane->name);
> -					break;
> +					if (ret) {
> +						drm_dbg_atomic(prop->dev,
> +							       "[PLANE:%d:%s] does not support async flips\n",
> +							       obj->id, plane->name);
> +						break;
> +					}
>  				}
>  			}
>  		}

Upon further testing and reflection, I have come to the conclusion that
this is indeed best handled by a kernel fix, rather than breaking user
space.

I attempted to work around this in wlroots, adjusting 0.18, 0.19, and
0.20 git with similar patches. First I attempted to stash all the
written properties for the atomic code, storing an initial value of all
0xFE so it was always likely to write the first time, and only setting a
property if it changed from the last commit.

This resulted in whole commits breaking for one or both framebuffers
until I ctrl-alt-fx switched to a tty and back again, and this would
work again temporarily.

So I went back to the drawing board and only withheld seemingly
duplicate plane properties. This "worked", until I attempted to play a
game, and then it started glitching spectacularly, and not updating at
all if the game was doing direct scanout and vrr.

Clearly this is wrong.

The wlroots library queues up properties for each commit. On every
commit where the cursor is disabled, it queues up both fb_id=0 and
crtc_id=0. Every commit. Is this wrong? Should it only be queueing up
the disablement properties once? It also queues up the full plane and
hotspot properties when enabled, even if the cursor doesn't change
position or appearance.