1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
|
From 5abc129c70cac08104fb7f8c757bc30afe8e8803 Mon Sep 17 00:00:00 2001
Message-Id: <5abc129c70cac08104fb7f8c757bc30afe8e8803.1526409254.git.jan.steffens@gmail.com>
In-Reply-To: <a9554de2a6028540fa12a7ebdab4daf2b46010fa.1526409254.git.jan.steffens@gmail.com>
References: <a9554de2a6028540fa12a7ebdab4daf2b46010fa.1526409254.git.jan.steffens@gmail.com>
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= <michel@daenzer.net>
Date: Tue, 8 May 2018 11:42:05 +0200
Subject: [PATCH 4/4] loader_dri3: Variant 2: Wait for pending swaps to
complete before drawable_fini.
On 2018-05-05 06:25 AM, Mario Kleiner wrote:
> On Sat, May 5, 2018 at 4:08 AM, Mike Lothian <mike@fireburn.co.uk> wrote:
>> I definately saw the steam bug with patch 1 but not with plasmashell,
>> I started seeing it with patch 2 but it seemed to fix itself
>
> I had two hangs of kwin_x11 within the last 6 hours when alt-tabbing
> between windows, where it got stuck in the
> loader_dri3_swapbuffer_barrier() from patch 1/2. Not sure how that is
> possible, or if the stacktrace was misleading, because i had to VT
> switch to a text console to attach the debugger and this might be just
> a side effect of that. But if it is true, then patch 1/2 would not be
> it. Also 1/2 has a potential performance impact, whereas 2/2 doesn't.
> However 2/2 would also need more work, as i can think of more complex
> scenarios where it would filter the wrong events, although not in the
> case of plasmashell or steam. Probably we'd need to sacrifice a few
> sbc bits in the Present events serial field to transport a unique tag
> for each incarnation of the loader_dri3_drawable, like a mini-hash of
> the draw->eid. Ugly ugly...
How about the below?
Idle notify events shouldn't need special treatment, since the pixmap
XIDs of the buffers will be different between loader_dri3_drawable
incarnations, aren't they?
This still leaves the issue that the SBC moves backwards, which could
theoretically result in hangs with apps using glXWaitForSbcOML. Fixing
that would probably require changing the loader_dri3_drawable lifetime
cycle, which would probably be very invasive, if feasible at all. Maybe
we don't need to care about that for the time being, until there's a
real world app running into it.
---
src/loader/loader_dri3_helper.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index 2e3b6c619e..e1eecb70a9 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -365,9 +365,18 @@ dri3_handle_present_event(struct loader_dri3_drawable *draw,
* checking for wrap.
*/
if (ce->kind == XCB_PRESENT_COMPLETE_KIND_PIXMAP) {
- draw->recv_sbc = (draw->send_sbc & 0xffffffff00000000LL) | ce->serial;
- if (draw->recv_sbc > draw->send_sbc)
- draw->recv_sbc -= 0x100000000;
+ uint64_t recv_sbc = (draw->send_sbc & 0xffffffff00000000LL) | ce->serial;
+
+ /* Only assume wraparound if that results in exactly the previous
+ * SBC + 1, otherwise ignore received SBC > sent SBC (those are
+ * probably from a previous loader_dri3_drawable instance) to avoid
+ * calculating bogus target MSC values in loader_dri3_swap_buffers_msc
+ */
+ if (recv_sbc <= draw->send_sbc)
+ draw->recv_sbc = recv_sbc;
+ else if (recv_sbc == (draw->recv_sbc + 0x100000001ULL))
+ draw->recv_sbc = recv_sbc - 0x100000000ULL;
+
switch (ce->mode) {
case XCB_PRESENT_COMPLETE_MODE_FLIP:
draw->flipping = true;
--
2.17.0
|
