This may be a more appropriate patch: https://lore.kernel.org/amd-gfx/20250723150413.18445-1-xaver.h...@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 <alexdeuc...@gmail.com> wrote: >> >> + Harry, Leo >> >> On Mon, Jun 23, 2025 at 9:38 AM Christopher Snowhill <ch...@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.