On 04/11/2015 04:17, Michel Dänzer wrote:
On 03.11.2015 17:14, Axel Davy wrote:
According to the spec, PresentOptionAsync should only
trigger a different behaviour when the target msc has been reached.
In this case if the driver is able to do async swaps, we use
them to avoid a screen copy.
When the target msc hasn't been reached yet, we want to use sync swaps.
Signed-off-by: Axel Davy <[email protected]>
---
I'm not sure for indentation, I tried to do the same than previous check
present/present.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/present/present.c b/present/present.c
index beb4ff0..3b8679c 100644
--- a/present/present.c
+++ b/present/present.c
@@ -836,19 +836,22 @@ present_pixmap(WindowPtr window,
vblank->notifies = notifies;
vblank->num_notifies = num_notifies;
- if (!(options & PresentOptionAsync))
- vblank->sync_flip = TRUE;
-
- if (!(options & PresentOptionCopy) &&
- !((options & PresentOptionAsync) &&
- (!screen_priv->info ||
- !(screen_priv->info->capabilities & PresentCapabilityAsync))) &&
- pixmap != NULL &&
- present_check_flip (target_crtc, window, pixmap, vblank->sync_flip,
valid, x_off, y_off))
- {
- vblank->flip = TRUE;
- if (vblank->sync_flip)
- target_msc--;
+ if (pixmap != NULL &&
+ !(options & PresentOptionCopy) &&
+ screen_priv->info) {
+ if (target_msc > crtc_msc &&
+ present_check_flip (target_crtc, window, pixmap, TRUE, valid,
x_off, y_off))
+ {
+ vblank->flip = TRUE;
+ vblank->sync_flip = TRUE;
+ target_msc--;
These three statements should be indented by two more spaces.
Good catch
Also, shouldn't we take this case if !(options & PresentOptionAsync)
even if target_msc <= crtc_msc?
At this point of the code, we have !(option & PresentOptionAsync) implies
target_msc > crtc_msc. See the part "Adjust target_msc to match modulus"
+ } else if (target_msc == crtc_msc &&
+ (options & PresentOptionAsync) &&
+ (screen_priv->info->capabilities & PresentCapabilityAsync) &&
+ present_check_flip (target_crtc, window, pixmap, FALSE, valid,
x_off, y_off))
+ {
+ vblank->flip = TRUE;
+ }
Why only take this case if target_msc == crtc_msc?
At this point of the code, when we have (option & PresentOptionAsync),
target_msc >= crtc_msc
If target_msc > crtc_msc, then we want to use a sync flip (An async flip
can cause tears),
and we use the previous if statement.
But in the case target_msc == crtc_msc, we want to copy immediately (via
an async flip, or a copy).
I realize that the "(option & PresentOptionAsync)" part of the check is
unneccessary.
Yours,
Axel
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
