The value is not the current msc of the window, but the target value
the client sets independently of the window speicific msc offset. Make
this clearer.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Make the code more readable by going through some logical abort
conditions. Also make the function only about updating the crtc
msc value and not about also returning the next target msc.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Unfold and extensively annotate the target-msc adjustment function, to make
it easier to understand what's happening and why.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
We can use value arguments instead of pointers when adjusting the timings
by returning the adjusted value. This improves the readability.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Currently, the function `present_wnmd_abort_vblank()` would fail if the
given `crtc` is NULL.
However, `xwl_present_get_crtc()` can return `NULL` under some
circumstances, which would cause an unexpected termination of Xwayland
in such a case, caused by the assertion failure being triggered.
Remove the assertion, considering that the `crtc` isn't actually used in
neither `present_wnmd_abort_vblank()` nor `xwl_present_abort_vblank()`.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
Closes: https://gitlab.freedesktop.org/xorg/xserver/issues/937
Instead of testing window->redirectDraw.
With Xwayland, the toplevel window is always redirected, so this would
unnecessarily preclude flipping there in some cases, e.g. with wlroots
based Wayland compositors or with fullscreen X11 windows in weston.
Fixes issue #631.
Xwayland will add and remove CRTCs as Wayland outputs are added or
removed.
If there is a pending flip when this occurs, the
`xwl_present_sync_callback()` will be triggered after the Xwayland
output's RRCtrcPtr has been destroyed, hence causing a crash in Xwayland
while trying to use freed memory:
#1 abort ()
#2 OsAbort () at utils.c:1350
#3 AbortServer () at log.c:877
#4 FatalError () at log.c:1015
#5 OsSigHandler () at osinit.c:156
#6 <signal handler called>
#7 dixGetPrivate () at ../include/privates.h:122
#8 dixLookupPrivate () at ../include/privates.h:166
#9 present_screen_priv () at present_priv.h:198
#10 present_wnmd_flip () at present_wnmd.c:358
#11 present_wnmd_execute () at present_wnmd.c:466
#12 present_wnmd_re_execute () at present_wnmd.c:80
#13 xwl_present_sync_callback () at xwayland-present.c:287
#14 ffi_call_unix64 () from /lib64/libffi.so.6
#15 ffi_call () from /lib64/libffi.so.6
#16 wl_closure_invoke () at src/connection.c:1006
#17 dispatch_event () at src/wayland-client.c:1427
#18 dispatch_queue () at src/wayland-client.c:1573
#19 wl_display_dispatch_queue_pending () at src/wayland-client.c:1815
#20 wl_display_dispatch_pending () at src/wayland-client.c:1878
#21 xwl_read_events () at xwayland.c:814
#22 ospoll_wait () at ospoll.c:651
#23 WaitForSomething () at WaitFor.c:208
#24 Dispatch () at ../include/list.h:220
#25 dix_main () at main.c:276
To avoid the issue, get the `ScreenPtr` from the window instead of the
CRTC that might have been just freed, `xwl_present_flip()` has no use
for the CRTC anyway.
Bugzilla: https://bugs.freedesktop.org/108249
Suggested-by: Michel Daenzer <michel.daenzer@amd.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Daenzer <michel.daenzer@amd.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
When a vblank has been marked as aborted, it's going to be free in the
flip_notify function when stopped. We can't notify it after it's
stopped because the pointer is invalid.
Valgrind backtrace:
==5331== Invalid read of size 8
==5331== at 0x212B4D: present_vblank_notify (present_vblank.c:34)
==5331== by 0x21439B: present_wnmd_flip_notify (present_wnmd.c:194)
==5331== by 0x21439B: present_wnmd_event_notify (present_wnmd.c:228)
==5331== by 0x156216: xwl_present_sync_callback (xwayland-present.c:282)
==5331== by 0x6570FCD: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==5331== by 0x657093E: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==5331== by 0x4DDB183: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x4DD79D8: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x4DD8EA3: wl_display_dispatch_queue_pending (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x14BCCA: xwl_read_events (xwayland.c:814)
==5331== by 0x2AC0D0: ospoll_wait (ospoll.c:651)
==5331== by 0x2A5322: WaitForSomething (WaitFor.c:208)
==5331== by 0x27574B: Dispatch (dispatch.c:421)
==5331== Address 0x1b44dc98 is 40 bytes inside a block of size 184 free'd
==5331== at 0x48369EB: free (vg_replace_malloc.c:530)
==5331== by 0x213B0A: present_wnmd_free_idle_vblanks (present_wnmd.c:118)
==5331== by 0x213B0A: present_wnmd_flips_stop (present_wnmd.c:161)
==5331== by 0x2143EF: present_wnmd_flip_notify (present_wnmd.c:192)
==5331== by 0x2143EF: present_wnmd_event_notify (present_wnmd.c:228)
==5331== by 0x156216: xwl_present_sync_callback (xwayland-present.c:282)
==5331== by 0x6570FCD: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==5331== by 0x657093E: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==5331== by 0x4DDB183: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x4DD79D8: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x4DD8EA3: wl_display_dispatch_queue_pending (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0)
==5331== by 0x14BCCA: xwl_read_events (xwayland.c:814)
==5331== by 0x2AC0D0: ospoll_wait (ospoll.c:651)
==5331== by 0x2A5322: WaitForSomething (WaitFor.c:208)
==5331== Block was alloc'd at
==5331== at 0x48377D5: calloc (vg_replace_malloc.c:711)
==5331== by 0x212D9F: present_vblank_create (present_vblank.c:69)
==5331== by 0x214014: present_wnmd_pixmap (present_wnmd.c:610)
==5331== by 0x21576C: proc_present_pixmap (present_request.c:150)
==5331== by 0x27599D: Dispatch (dispatch.c:479)
==5331== by 0x279945: dix_main (main.c:276)
==5331== by 0x633AB16: (below main) (libc-start.c:310)
v2: Still notify aborted flips (Roman)
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107314
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Tested-by: Roman Gilg <subdiff@gmail.com>
The incorrect values could result in the new pixmap's contents
getting corrupted down the line.
v2:
* Guard screen_x/y lines by #ifdef COMPOSITE
Bugzilla: https://bugs.freedesktop.org/106841
Fixes: 029608dd80 "present: Add window flip mode"
Reviewed-by: Adam Jackson <ajax@redhat.com> # v1
Reviewed-by: Keith Packard <keithp@keithp.com> # v1
Reviewed-by: Roman Gilg <subdiff@gmail.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com> # v1
Instead of getting the current msc value from the window, which might be
different to old one directly take the last saved msc value saved in
the window_priv struct.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Calculate damage before trying to flip and report it to the driver.
This allows drivers to optimize their rendering.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
In contrast to screen flip mode this mode:
* supports flips per windows (these windows currently need to have the same
size as their parent windows with the same pixmap),
* sends pixmap idle signals to the client only after the driver has given
an additional event notify.
This patch only introduces the new mode as a stub. It additionally needs a
driver hook, such that it can get initialized and appropriate cleanup
functions.
Signed-off-by: Roman Gilg <subdiff@gmail.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
