* 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-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
* 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
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
all inboxes | Powered by JetHome®