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

[Christopher Snowhill]

On Mon Jun 23, 2025 at 3:46 AM PDT, Christopher Snowhill wrote:
> 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.

Probably should have CC'd the drm misc maintainers when I started poking
drm misc instead of amdgpu. Pity there isn't a list for that...