[xserver,v3,23/24] xwayland: Cut off upper 32bit of queued vblank delay

Submitted by Roman Gilg on March 13, 2018, 3 p.m.

Details

Message ID 1520953257-24768-24-git-send-email-subdiff@gmail.com
State New
Series "Per window flips in Present with support for them in Xwayland"
Headers show

Commit Message

Roman Gilg March 13, 2018, 3 p.m.
This ensures the same behavior as in Present's fake counter and xfree86.

At the moment clients might do put vblanks too far into the future, because
the fake vblank code in Present and the xfree86 driver tolerate cut off upper
32bit due to an 64 to 32bit conversion. Do this therefore here as well to not
suddenly regress on Xwayland only.

The sample client, that triggers this behavior, is the Steam client.

Signed-off-by: Roman Gilg <subdiff@gmail.com>
---
 hw/xwayland/xwayland-present.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index f403ff7..300d96f 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -299,6 +299,7 @@  xwl_present_queue_vblank(WindowPtr present_window,
 {
     struct xwl_window *xwl_window = xwl_window_of_top(present_window);
     struct xwl_present_event *event;
+    INT32 delay;
 
     if (!xwl_window)
         return BadMatch;
@@ -317,7 +318,10 @@  xwl_present_queue_vblank(WindowPtr present_window,
     event->event_id = event_id;
     event->present_window = present_window;
     event->xwl_window = xwl_window;
-    event->target_msc = msc;
+
+    /* Cut off upper 32bit, copies present_fake_queue_vblank. */
+    delay = (int64_t) (msc - xwl_window->present_msc);
+    event->target_msc = xwl_window->present_msc + delay;
 
     xorg_list_append(&event->list, &xwl_window->present_event_list);
 

Comments

Michel Dänzer March 14, 2018, 10:02 a.m.
On 2018-03-13 04:00 PM, Roman Gilg wrote:
> This ensures the same behavior as in Present's fake counter and xfree86.
> 
> At the moment clients might do put vblanks too far into the future, because
> the fake vblank code in Present and the xfree86 driver tolerate cut off upper
> 32bit due to an 64 to 32bit conversion. Do this therefore here as well to not
> suddenly regress on Xwayland only.
> 
> The sample client, that triggers this behavior, is the Steam client.
> 
> Signed-off-by: Roman Gilg <subdiff@gmail.com>
> ---
>  hw/xwayland/xwayland-present.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index f403ff7..300d96f 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -299,6 +299,7 @@ xwl_present_queue_vblank(WindowPtr present_window,
>  {
>      struct xwl_window *xwl_window = xwl_window_of_top(present_window);
>      struct xwl_present_event *event;
> +    INT32 delay;
>  
>      if (!xwl_window)
>          return BadMatch;
> @@ -317,7 +318,10 @@ xwl_present_queue_vblank(WindowPtr present_window,
>      event->event_id = event_id;
>      event->present_window = present_window;
>      event->xwl_window = xwl_window;
> -    event->target_msc = msc;
> +
> +    /* Cut off upper 32bit, copies present_fake_queue_vblank. */
> +    delay = (int64_t) (msc - xwl_window->present_msc);
> +    event->target_msc = xwl_window->present_msc + delay;

FWIW, I think it would be better to make delay unsigned, and calculate
it something like

    delay = max((int64_t) (msc - xwl_window->present_msc), 0);


However, since I've been unable to reproduce the issue you described
with Steam, despite kind of going out of my way to do so, I'm really
reluctant to add this workaround without getting more information about
the problem from someone who can reproduce it. Without understanding the
problem, the workaround might turn out to be too strict or too lenient,
and clients might accidentally become dependent on the workaround.
Adam Jackson March 28, 2018, 6:40 p.m.
On Wed, 2018-03-14 at 11:02 +0100, Michel Dänzer wrote:

> However, since I've been unable to reproduce the issue you described
> with Steam, despite kind of going out of my way to do so, I'm really
> reluctant to add this workaround without getting more information about
> the problem from someone who can reproduce it. Without understanding the
> problem, the workaround might turn out to be too strict or too lenient,
> and clients might accidentally become dependent on the workaround.

Yeah, I'm not thrilled with this workaround either, and I've not been
able to trigger it. We can revisit this later if necessary.

The rest of the series I've been running with for a while now with no
issues, so, merged:

remote: I: patch #210201 updated using rev dda7efec36b495e100e670e057d7ef5022ef3976.
remote: I: patch #210202 updated using rev c5c50c6db1e71e976596750277b1a618704c04aa.
remote: I: patch #210203 updated using rev 5365ece70a75a05df3d6351767d19c3edcf0305d.
remote: I: patch #201328 updated using rev 1e7d8902bfe7cfb79c41b14fc6b50bcbe4f7c800.
remote: I: patch #210205 updated using rev 679ffbf5f39822ea508e50f1b7c92a2a9e79f7bb.
remote: I: patch #210206 updated using rev 1db7cf0429eabf33f8e2b55a15db4d1f87e1fb95.
remote: I: patch #210208 updated using rev 84112a1d0b221c00d7d3c23fd5b97687e6e3749a.
remote: I: patch #210207 updated using rev 6a338b5959ca5a9e5260d71b6a739a5c672d77e7.
remote: I: patch #210210 updated using rev 6d813bbd5ea0fc38a8114c08368a7954eeb2ef37.
remote: I: patch #201332 updated using rev 3aaaac0be573fb09a206966075d81ebe0510ca23.
remote: I: patch #207397 updated using rev 84e47f3fe68f05f7b0b762e96acd4c95fa8000ca.
remote: I: patch #210212 updated using rev 92b91b8cf34a38de39281044d8441b6cabe87a85.
remote: I: patch #207396 updated using rev 7b071b4e440313254398f06eb59b1596a6d3e8fe.
remote: I: patch #210218 updated using rev 8d370fcdcaed210d9f4afc1650aa8b161c7fbb44.
remote: I: patch #210214 updated using rev 029608dd80204ac96423ef79ec46c1a18bbdd5ff.
remote: I: patch #207399 updated using rev 66a5c0bccb222ad8b9b57b10490c3041e1b3f05e.
remote: I: patch #201340 updated using rev a337949f99bc473ea0ae0af64736eae3d5b39399.
remote: I: patch #201341 updated using rev 902429f077325b98e30ede2710bd7a88440d2937.
remote: I: patch #210219 updated using rev 8fba2a03f1410f3bc7504e218ac1e5c964279ea2.
remote: I: patch #210220 updated using rev 0fb2cca193e60b731c8e75a2a7e795477fb5fd8f.
remote: I: patch #207404 updated using rev 86df366973de1c10da5fbdc57d1ff12b681c321f.
remote: I: patch #210221 updated using rev 07750ff3c084c6549a5612d1f935a9a3ab3df67c.
remote: I: patch #210224 updated using rev be087778a0eae3093ffdbba3ff7c9f3863d8e1d4.
remote: I: 23 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   4303deae78..be087778a0  master -> master

- ajax