* Re: [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes
[not found] ` <DATV4CAOHVGV.1UJ803EX21II6@gmail.com>
@ 2025-06-23 13:38 ` Christopher Snowhill
2025-06-23 15:33 ` Alex Deucher
2025-08-15 16:11 ` Leo Li
0 siblings, 2 replies; 5+ messages in thread
From: Christopher Snowhill @ 2025-06-23 13:38 UTC (permalink / raw)
To: Christopher Snowhill, amd-gfx
Cc: Alex Deucher, Christian König, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, David Airlie,
dri-devel, linux-kernel
On Mon Jun 23, 2025 at 4:06 AM PDT, Christopher Snowhill wrote:
> 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...
I am a dumbass, I didn't notice get_maintainer.pl. Added more people,
and the correct list. Not sure if I should remove amd-gfx, since this
affects them, somewhat...
However, the intention of this thread was to seek commentary on the
situation as it is.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes
2025-06-23 13:38 ` [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes Christopher Snowhill
@ 2025-06-23 15:33 ` Alex Deucher
2025-08-15 13:17 ` Alex Deucher
2025-08-15 16:11 ` Leo Li
1 sibling, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2025-06-23 15:33 UTC (permalink / raw)
To: Christopher Snowhill, Wentland, Harry, Leo (Sunpeng) Li
Cc: Christopher Snowhill, amd-gfx, Alex Deucher, Christian König,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
dri-devel, linux-kernel
+ Harry, Leo
On Mon, Jun 23, 2025 at 9:38 AM Christopher Snowhill <chris@kode54.net> wrote:
>
> On Mon Jun 23, 2025 at 4:06 AM PDT, Christopher Snowhill wrote:
> > 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...
>
> I am a dumbass, I didn't notice get_maintainer.pl. Added more people,
> and the correct list. Not sure if I should remove amd-gfx, since this
> affects them, somewhat...
>
> However, the intention of this thread was to seek commentary on the
> situation as it is.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes
2025-06-23 15:33 ` Alex Deucher
@ 2025-08-15 13:17 ` Alex Deucher
2025-08-15 23:21 ` Christopher Snowhill
0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2025-08-15 13:17 UTC (permalink / raw)
To: Christopher Snowhill, Wentland, Harry, Leo (Sunpeng) Li
Cc: Christopher Snowhill, amd-gfx, Alex Deucher, Christian König,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
dri-devel, linux-kernel
@Wentland, Harry
, @Leo (Sunpeng) Li Can you guys take a look? This patch fixes a regression.
Thanks,
Alex
On Mon, Jun 23, 2025 at 11:33 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> + Harry, Leo
>
> On Mon, Jun 23, 2025 at 9:38 AM Christopher Snowhill <chris@kode54.net> wrote:
> >
> > On Mon Jun 23, 2025 at 4:06 AM PDT, Christopher Snowhill wrote:
> > > 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...
> >
> > I am a dumbass, I didn't notice get_maintainer.pl. Added more people,
> > and the correct list. Not sure if I should remove amd-gfx, since this
> > affects them, somewhat...
> >
> > However, the intention of this thread was to seek commentary on the
> > situation as it is.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes
2025-06-23 13:38 ` [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes Christopher Snowhill
2025-06-23 15:33 ` Alex Deucher
@ 2025-08-15 16:11 ` Leo Li
1 sibling, 0 replies; 5+ messages in thread
From: Leo Li @ 2025-08-15 16:11 UTC (permalink / raw)
To: Christopher Snowhill, Christopher Snowhill, amd-gfx
Cc: Alex Deucher, Christian König, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, dri-devel,
linux-kernel
On 2025-06-23 09:38, Christopher Snowhill wrote:
> On Mon Jun 23, 2025 at 4:06 AM PDT, Christopher Snowhill wrote:
>> 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,
Hi Christopher,
Adding some additional context lines here:
>>>> if (async_flip) {
>>>> /* check if the prop does a nop change */
>>>> if ((prop != config->prop_fb_id &&
>>>> prop != config->prop_in_fence_fd &&
>>>> prop != config->prop_fb_damage_clips)) {
>>>> ret = drm_atomic_plane_get_property(plane, plane_state,
>>>> prop, &old_val);
>>>> ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);## end of additional context ##>>>> }
>>>>
>>>> /* ask the driver if this non-primary plane is supported */
>>>> - if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
>>>> - ret = -EINVAL;
Firstly, apologies for the delay, and thanks for the patch.
The original code definitely looks fishy. The value of `ret =
drm_atomic_check_prop_changes` is not used afterwards and is ignored.
Looking at the history (v6.14), it seems the original intention was:
* For PRIMARY planes
* Allow modifying of properties fb_id, in_fence_fd, and fb_damage_clips
* Allow setting same value for all other properties
* For OVERLAY and CURSOR planes :
* Allow setting same value for all properties
(https://elixir.bootlin.com/linux/v6.14.11/source/drivers/gpu/drm/drm_atomic_uapi.c#L1078)
Then, the behavior changed in v6.15:
* For PRIMARY planes
* Allow modifying of (including setting same value for) **all** properties
* For OVERLAY and CURSOR planes
* **Allow modifying of all properties** if `atomic_async_check` callback
exists and passes
* **Setting same value can fail**, as behavior is now driver-specific.
(https://elixir.bootlin.com/linux/v6.15-rc1/source/drivers/gpu/drm/drm_atomic_uapi.c#L1081)
But IIUC, the desired behavior is actually:
* For PRIMARY planes
* Allow modifying of properties fb_id, in_fence_fd, and fb_damage_clips
* Allow setting same value for all other properties
* For OVERLAY and CURSOR planes :
* **Allow setting same value on all properties**
* Allow modifying of all properties if `atomic_async_check` callback exists
and passes
Is this the desired behavior? If so, I think it makes sense, but the existing
conditions need some cleaning up.
Thanks,
Leo
>>>> + 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...
>
> I am a dumbass, I didn't notice get_maintainer.pl. Added more people,
> and the correct list. Not sure if I should remove amd-gfx, since this
> affects them, somewhat...
>
> However, the intention of this thread was to seek commentary on the
> situation as it is.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes
2025-08-15 13:17 ` Alex Deucher
@ 2025-08-15 23:21 ` Christopher Snowhill
0 siblings, 0 replies; 5+ messages in thread
From: Christopher Snowhill @ 2025-08-15 23:21 UTC (permalink / raw)
To: Alex Deucher, Christopher Snowhill, Wentland, Harry,
Leo (Sunpeng) Li
Cc: amd-gfx, Alex Deucher, Christian König, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, dri-devel,
linux-kernel
This may be a more appropriate patch:
https://lore.kernel.org/amd-gfx/20250723150413.18445-1-xaver.hugl@kde.org/
On Fri Aug 15, 2025 at 6:17 AM PDT, Alex Deucher wrote:
> @Wentland, Harry
> , @Leo (Sunpeng) Li Can you guys take a look? This patch fixes a regression.
>
> Thanks,
>
> Alex
>
> On Mon, Jun 23, 2025 at 11:33 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>> + Harry, Leo
>>
>> On Mon, Jun 23, 2025 at 9:38 AM Christopher Snowhill <chris@kode54.net> wrote:
>> >
>> > On Mon Jun 23, 2025 at 4:06 AM PDT, Christopher Snowhill wrote:
>> > > 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...
>> >
>> > I am a dumbass, I didn't notice get_maintainer.pl. Added more people,
>> > and the correct list. Not sure if I should remove amd-gfx, since this
>> > affects them, somewhat...
>> >
>> > However, the intention of this thread was to seek commentary on the
>> > situation as it is.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-08-15 23:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250619125507.54384-1-kode54@gmail.com>
[not found] ` <DARA1U86AS72.QOIEVZWCFPYC@kode54.net>
[not found] ` <DATUOZZD8316.2INSL3KL5RA80@kode54.net>
[not found] ` <DATV4CAOHVGV.1UJ803EX21II6@gmail.com>
2025-06-23 13:38 ` [RFC PATCH] drm/amdgpu: Enable async flip for cursor planes Christopher Snowhill
2025-06-23 15:33 ` Alex Deucher
2025-08-15 13:17 ` Alex Deucher
2025-08-15 23:21 ` Christopher Snowhill
2025-08-15 16:11 ` Leo Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).