Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xserver: Present extension backport from X.org #423

Open
wants to merge 76 commits into
base: 3.6.x
Choose a base branch
from

Conversation

sunweaver
Copy link
Member

this is currently w-i-p...

@sunweaver sunweaver added this to the 3.6.1.0 milestone Apr 11, 2017
@sunweaver sunweaver self-assigned this Apr 11, 2017
keith-packard and others added 26 commits April 12, 2017 19:20
Provides both a software implementation using timers and driver hooks
to base everything on vblank intervals.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
A client destroying objects in the middle of an unflip can end up
having the screen flip window or fence set to NULL in the unflip
notify path. Check for these and don't try to use those objects.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
…gnals

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
If the timer fired too early, we'd sometimes mis-compute the MSC for
fake vblanks. Rounding the computation to the nearest MSC fixes this nicely.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
We use event_id 0 to mean 'no such event'; if a driver sends us that
event_id, make sure we don't accidentally match it.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This eliminates dereferencing freed window pointers when there is a
flip for that window in progress. The flip will complete, and then
immediately get undone (as we can't stop an in-progress flip).

Remove the vblank->window_destroyed field as we can signal this with
vblank->window == NULL instead.

Change check to vblank->window == NULL in:

	present_flip_notify

Add check for vblank->window == NULL in:
	present_vblank_notify
	present_execute

present_flip_notify was also using vblank->window->drawable.pScreen,
so stop doing that and use vblank->screen instead.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Pend presentation until wait_fence is also triggered by having the
SyncFence trigger invoke present_execute once triggered.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
unflip happens after the clip lists have been updated, so instead of
smashing the whole screen and drawing over other windows, just draw to
the original flip window; it'll have the right clip list and so the
copy will work just fine.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
When an application provides two pixmaps for the same MSC, the
previous one is skipped. This just dumps out some information at that point

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
If the window is destroyed, then we've got cleanup work to do, even if
the vblank has already been executed -- we need to clear the window
pointer so that we don't try to deliver events to it.

Leaving it on the window list meant that when walking that list, we
need to know whether the vblank is waiting to be executed or waiting
for the flip to complete, so a new 'queued' flag was added to the
vblank to distinguish between the two states.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This makes other drawing to the window appear on the screen.

Note that no child windows can be affected because only full-screen
windows are eligible for flipping, and so we only need to set pixmap
for the window itself.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This allows GL to support the GLX_INTEL_swap_event extension.

v2: Return GLX_BLIT_COMPLETE_INTEL for unknown swap types

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
If the driver doesn't have the necessary hooks for Present, then the
target_crtc needs to be set to NULL to make sure the extension uses
the present_fake code.

Signed-off-by: Keith Packard <keithp@keithp.com>
Tested-by: Fredrik Höglund <fredrik@kde.org>
Reviewed-by: Adam Jackson <ajax@redhat.com>
This makes sure that things like software cursors continue to work
while the screen is flipped.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Limit damage to the 'update' region.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Among much else Present depends on RANDR types, and RANDR isn't properly
Xinerama-aware yet anyway.

Reviewed-by: Julien Cristau <jcristau@debian.org>
Signed-off-by: Adam Jackson <ajax@redhat.com>
Newly created windows inherit the pixmap of their parent, similarly,
reparenting a tree inherits the pixmap of the destination tree.

Making present preserve the invariant that unredirected windows always
have the same pixmap as their parent ensures that the above cases work
correctly.

v2: name the recursive function to 'set_tree_pixmap' instead of 'set_window_pixmap'

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
We want to advertise the version we implement, not the version the
protocol headers happen to describe.

Reviewed-by: Jasper St. Pierre <<jstpierre@mecheye.net>
Signed-off-by: Adam Jackson <ajax@redhat.com>
Check for Async flag and execute immediately if set, otherwise wait
for the next appropriate vblank before copying.

Signed-off-by: Keith Packard <keithp@keithp.com>
Presents which are not marked 'queued' and are in the window present
list are waiting for the flip event; discarding those won't work very
well (it'll end up trashing displayed content for the next frame), so
skip over those when looking for duplicate frame presents

Signed-off-by: Keith Packard <keithp@keithp.com>
Skipped present pixmap calls were not setting the mode to
PresentCompleteModeSkip for skipped operations.

Signed-off-by: Keith Packard <keithp@keithp.com>
This lets us stop using the 'pointer' typedef in Xdefs.h as 'pointer'
is used throughout the X server for other things, and having duplicate
names generates compiler warnings.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Eric Anholt <eric@anholt.net>
When a flip (or unflip) is pending and a flip request comes in, leave
it queued until the pending flip completes and then execute it.

This fixes a bug where an application submitting back-to-back
present_pixmap requests for sequential frames would alternate between
flipping and copying as the pending flip would cause the new
present_pixmap request to not use a flip.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Frank Binns <frank.binns@imgtec.com>
Once the vblank is actually getting executed, it's lifetime is no
longer tied to the window, and so it shouldn't be controlled by window
destruction. In particular, if the vblank is queued for flip, it will
get stored in the flip_pending field, and will be correctly destroyed
when the flip completes.

Signed-off-by: Keith Packard <keithp@keithp.com>
If a 2D application is started on top of a fullscreen 3D application, which
is flipping, then we need to stop flipping and restore the root window, and
possibly the flip window, to using the screen pixmap. Normally this would
be done as part of an unflip. However, in the case that there is a pending
flip there is no mechanism to abort so the unflip is deferred until the
pending flip completes. This provides a window of opportunity for the 2D
application to draw to the wrong pixmap.

Restore the screen pixmap at the point a pending flip is marked as aborted,
thus avoiding this issue.

Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Frank Binns <frank.binns@imgtec.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
If we present several pixmaps in advance for different msc, the later one
shouldn't cancel the previous ones.

This reverts a change made by commit
e6f5d9d7b7efdacea0f22f1808efca849bcede4c

Without this fix, vblank_mode=0 glxgears doesn't update
with the present fallback.

Signed-off-by: Axel Davy <axel.davy@ens.fr>
Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Michel Dänzer and others added 29 commits April 12, 2017 19:20
When a window moves from one CRTC to another, present_window_to_crtc_msc
updates window_priv->msc_offset according to the delta between the
current MSC values of the old and new CRTC:

            window_priv->msc_offset += new_msc - old_msc;

window_priv->msc_offset is initially 0, so if new_msc < old_msc,
window_priv->msc_offset wraps around and becomes a large number. If the
window_msc parameter passed in is small (in particular if it's 0, such as
is the case when the client just wants to know the current window MSC
value), the returned CRTC MSC value may still be a large number. In that
case, the existing MSC comparisons in pixmap_present weren't working as
intended, resulting in scheduling a wait far into the future when the
target MSC had actually already passed. This would result in the client
(e.g. the Chromium browser) hanging when moving its window between CRTCs.

In order to fix this, introduce msc_is_(equal_or_)after helper functions
which take the wraparound into account for comparing two MSC values.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
To make them usable from any other function in the file. No functional
change.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
For flipping, we wait for the MSC before the target MSC and then call
the driver flip hook. If the latter fails, we have to wait for the
target MSC before falling back to a copy, or else it's executed too
early.

Fixes glxgears running at unbounded framerate (not synchronized to the
refresh rate) in fullscreen if the driver flip hook fails.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
While present_pixmap decrements target_msc by 1 for present_queue_vblank,
it leaves the original vblank->target_msc intact. So incrementing the
latter for requeueing resulted in the requeued presentation being
executed too late.

Also, no need to requeue if the target MSC is already reached.

This further reduces stutter when a popup menu appears on top of a
flipping fullscreen window.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
present_unflip may be called several times from present_check_flip_window
during the same unflip. We can only copy to the screen pixmap the first
time, otherwise we may scribble over other windows. The flip pixmap
contents don't get updated after the first time anyway.

Fixes at least the following problems, which were introduced by commit
806470b9 ("present: Copy unflip contents back to the Screen Pixmap"):

On xfwm4 without compositing, run glxgears and put its window into
fullscreen mode to start flipping. While in fullscreen, open the xfwm4
window menu by pressing Alt-Space. The window menu was invisible most
of the time because it was getting scribbled over by a repeated unflip
copy.

When switching a flipping window out of fullscreen, a repeated unflip
copy could leave artifacts of the flip pixmap on the desktop.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94325

Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
…p (v2)

The following fix will use the refactored function.

The logic in the refactored function is slightly simplified, exploiting
the fact that this function is only ever called with a valid flip
pixmap.

v2: Assert that flip_pixmap is non-NULL instead of testing and bailing
    on NULL, preserve test for flip_window being non-NULL before calling
    present_set_tree_pixmap for it (Keith Packard)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Reviewed-by: Keith Packard <keithp@keithp.com>
After present_set_abort_flip, the screen pixmap will be used for all
screen drawing, so we need to restore the current flip pixmap contents
to the screen pixmap here as well.

Improves flashing / stutter e.g. when something like a popup menu appears
on top of a flipping fullscreen window or when switching out of
fullscreen.

Note that this means present_set_abort_flip now relies on screen->root
being non-NULL, but that's already the case in other present code.

Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Due to the way present currently works, we don't ever check with the
secondary adapters if we can flip at all.

We shouldn't flip if the secondary adapters are attached to the pixmap
currently, however using the current check_flip callback isn't possible
as it passes the Window to the driver (something we shouldn't be doing),
so the slave driver can never get it's own screen ptr back.

For now to fix the problem just block flips if we have any slaves
configured. We can fix the ABI up later, but this fix can be backported
to stable.

Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
This code was added to deal with the driver present hook failing, in
which case we need to wait for the next MSC before executing the
presentation.

However, it could also take effect in cases where the driver incorrectly
thinks the current MSC matches the target one (e.g. due to the kernel
interface only supporting 32-bit MSC values), in which case it could
result in the presentation getting requeued over and over.

To prevent such issues, check specifically for the target MSC
immediately following the current MSC.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94596
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
With large numbers of queued vblank, the list iteration on every
interupt dominates processing time. If we reorder the list to be in
ascending event order, then not only is also likely to be in order for
notification queries (i.e. the notification will be near the start of
the list), we can also stop iterating when past the target event_id.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
The flip queue currently only holds events submitted to the driver for
flipping, awaiting the completion notifier. It is short. We therefore
can speed up interrupt processing by keeping the small number of events
ready to be flipped on the end of the flip queue. By appending the
events to the flip_queue in the order that they become ready, we also
resolve one issue causing Present to display frames out of order.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
…oad slaves

A single provider can be both a offload and source slave at the same time,
the use of seperate lists breaks in this case e.g. :

xrandr --listproviders
Providers: number : 2
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 3 outputs: 2 associated providers: 0 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 2 outputs: 5 associated providers: 0 name:modesetting

xrandr --setprovideroutputsource 1 0x7b
xrandr --listproviders
Providers: number : 2
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 3 outputs: 2 associated providers: 1 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 2 outputs: 5 associated providers: 1 name:modesetting

xrandr --setprovideroffloadsink 1 0x7b
xrandr --listproviders
Providers: number : 3
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 3 outputs: 2 associated providers: 2 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 2 outputs: 5 associated providers: 2 name:modesetting
Provider 2: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink Offload crtcs: 2 outputs: 5 associated providers: 2 name:modesetting

Not good. The problem is that the provider with id 0x46 now is on both
the output_slave_list and the offload_slave_list of the master screen.

This commit fixes this by unifying all 3 lists into a single slaves list.

Note that this does change the struct _Screen definition, so this is an ABI
break. I do not expect any of the drivers to actually use the removed / changed
fields so a recompile should suffice.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>
present_restore_screen_pixmap's work doesn't need to be done several
times for the same pending flip.

Fixes a crash if the X server quits while a flip is pending, in which
case present_set_abort_flip may be called several times, including when
screen->root is already cleared to NULL.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
From the Present extension specification:

 An event context is associated with a specific window; using
 an existing event context with a different window generates
 a Match error.

 If eventContext specifies an existing event context, then if
 eventMask is empty, PresentSelectInput deletes the specified
 context, otherwise the specified event context is changed to
 select a different set of events.

 If eventContext is an unused XID, then if eventMask is empty
 no operation is performed. Otherwise, a new event context is
 created selecting the specified events.

Without this change, there's no way for a client to explicitly change
or destroy an existing event mask entry. Trying to do so as specified
above would just result in a protocol error.

v2: (Keith Packard)
* Use dixLookupResourceByType instead of walking window_priv->events
* Return BadMatch if the existing event context is associated with a
  different window or client
* Call LEGAL_NEW_RESOURCE again when creating a new event context
* Drop invalid "leak fix"

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Easier than dealing with it in all paths that can end up here during
server shutdown.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
…flip

We were asserting that these were called before from other places, but
that isn't always the case, e.g. during server shutdown.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96951
Reported-and-Tested-by: Tod Jackson <tod.jackson@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Plug a leak in present_fake_queue_vblank() where the OsTimer would not
be freed.

 492,608 (482,816 direct, 9,792 indirect) bytes in 15,088 blocks
are definitely lost in loss record 3,954 of 3,954
    at 0x4C2ABDE: malloc (in vgpreload_memcheck-amd64-linux.so)
    by 0x586B19: TimerSet (WaitFor.c:433)
    by 0x4F1AA9: present_fake_queue_vblank (present_fake.c:108)
    by 0x4F15E0: present_pixmap (present.c:954)
    by 0x4F23B4: proc_present_pixmap (present_request.c:138)
    by 0x552BCE: Dispatch (dispatch.c:430)
    by 0x556C22: dix_main (main.c:300)
    by 0x6F0D290: (below main) (in /usr/lib/libc-2.24.so)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97065
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
We are no longer using the present_flip_queue list only for presents
which have already been submitted to the driver for page flipping, but
also for those which we are queueing up to be flipped later, marked
with vblank->queued == TRUE. We were incorrectly calling
present_flip_notify for such entries, failing the assertion in
present_flip_notify (or presumably resulting in other undesirable
behaviour with assertions disabled).

Reproduction recipe: Run the JavaFX test case referenced by
https://bugs.freedesktop.org/show_bug.cgi?id=98831#c6 and alt-tab out
of it while it's fullscreen. May take a few attempts to hit the
assertion failure.

Fixes: bab0f450a719 ("present: Fix presentation of flips out of order")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98854
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
This prevents the tearing of moving window in a composite WM
desktop when output slave is attached but none of its crtc is
really active.

Signed-off-by: Qiang Yu <Qiang.Yu@amd.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Works fine now.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
SBC is entire a client-side notion, so remove it from the protocol.
No need to have two events with the same content, but we do need to
tell which request generated the event so stick a new field in some
spare bytes

Signed-off-by: Keith Packard <keithp@keithp.com>
In PresentRegion, add the PresentNotify list, add explicit CRTC.
In PresentRedirectNotify, add CRTC and update_window values

Signed-off-by: Keith Packard <keithp@keithp.com>
Changes the name of the PresentPixmap request from PresentRegion in
anticipation of future additions of non-pixmap sourced updates (YUV
images in particular).

Adds definitions for all of the new PresentPixmap options.

Adds PresentQueryCapabilities to provide applications the ability to
learn what the underlying hardware can support. One requirement for
any capability is that the X server must do something sensible even if
the client behaves as if a capability is supported when it is not.

Adds IdleNotify events. As pixmaps can go idle in any order, it's
important for applications to know which pixmap to use next. We cannot
use fences as the fence itself may not be signaled for some time after
the X server has figured out which pixmap to idle.

Note that the encoding and header files are not entirely up to date now.

Signed-off-by: Keith Packard <keithp@keithp.com>
Add presentproto.h updates for current protocol.
Finish encoding specification.

Signed-off-by: Keith Packard <keithp@keithp.com>
If you don't do this then the client libs on 64-bit machines see them as
XIDs, which are 64-bit wide (sigh), which ruins the wire encoding and
nothing works.

Reviewed-by: Jasper St. Pierre <jstpierre@mecheye.net>
Signed-off-by: Adam Jackson <ajax@redhat.com>
Leave it all under #if PRESENT_FUTURE_VERSION for documentation

Signed-off-by: Keith Packard <keithp@keithp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet