Re: [Qemu-devel] [PATCH] gtk: sync guest display updates to host display refresh

[Chen Zhang]

Update refresh interval upon frame tick;
Add callback of window state events to adjust refresh rate during iconified.
The callback handle is saved in struct VirtualConsole for its removal when the 
window is iconified.

Signed-off-by: Chen Zhang <address@hidden>
---
 include/ui/gtk.h |  1 +
 ui/gtk.c         | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 99edd3c..c1f7655 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -76,6 +76,7 @@ typedef struct VirtualConsole {
     GtkWidget *tab_item;
     GtkWidget *focus;
     VirtualConsoleType type;
+    guint frame_tick_cb;
     union {
         VirtualGfxConsole gfx;
 #if defined(CONFIG_VTE)
diff --git a/ui/gtk.c b/ui/gtk.c
index 579990b..9bc5812 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1661,6 +1661,43 @@ static gboolean gd_configure(GtkWidget *widget,
     return FALSE;
 }
 
+static gboolean gd_frame_tick(GtkWidget *widget,
+                              GdkFrameClock *frame_clock,
+                              gpointer opaque)
+{
+    VirtualConsole *vc = opaque;
+    gint64 interval;
+    gdk_frame_clock_get_refresh_info(frame_clock,
+            0,
+            &interval,
+            NULL);
+    interval = interval / 1000;
+    if (vc->gfx.dcl.update_interval != interval) {
+        update_displaychangelistener(&vc->gfx.dcl, interval);
+    }
+    return G_SOURCE_CONTINUE;
+}
+
+static void gd_window_state_event(GtkWidget *widget,
+                           GdkEventWindowState *event,
+                           GtkDisplayState *s)
+{
+    if (event->changed_mask & GDK_WINDOW_STATE_ICONIFIED) {
+        bool iconified = (event->new_window_state & 
GDK_WINDOW_STATE_ICONIFIED) != 0;
+        VirtualConsole *vc = gd_vc_find_current(s);
+        if (iconified) {
+            gtk_widget_remove_tick_callback(vc->gfx.drawing_area,
+                                            vc->frame_tick_cb);
+            update_displaychangelistener(&vc->gfx.dcl, 
GUI_REFRESH_INTERVAL_IDLE);
+        } else {
+            vc->frame_tick_cb =
+                gtk_widget_add_tick_callback(vc->gfx.drawing_area,
+                                             gd_frame_tick, vc, NULL);
+            update_displaychangelistener(&vc->gfx.dcl, 
GUI_REFRESH_INTERVAL_DEFAULT);
+        }
+    }
+}
+
 /** Virtual Console Callbacks **/
 
 static GSList *gd_vc_menu_init(GtkDisplayState *s, VirtualConsole *vc,
@@ -1911,6 +1948,17 @@ static void gd_connect_vc_gfx_signals(VirtualConsole *vc)
                          G_CALLBACK(gd_focus_out_event), vc);
         g_signal_connect(vc->gfx.drawing_area, "configure-event",
                          G_CALLBACK(gd_configure), vc);
+        if (1 /* make that a config option ??? */) {
+            update_displaychangelistener(&vc->gfx.dcl,
+                                         GUI_REFRESH_INTERVAL_IDLE);
+            vc->frame_tick_cb =
+                    gtk_widget_add_tick_callback(vc->gfx.drawing_area,
+                                                 gd_frame_tick, vc, NULL);
+
+            gtk_widget_add_events(vc->s->window, GDK_STRUCTURE_MASK);
+            g_signal_connect(vc->s->window, "window-state-event",
+                             G_CALLBACK(gd_window_state_event), vc->s);
+        }
     } else {
         g_signal_connect(vc->gfx.drawing_area, "key-press-event",
                          G_CALLBACK(gd_text_key_down), vc);
-- 
2.7.4


> On Nov 14, 2018, at 5:40 PM, Chen Zhang <address@hidden> wrote:
> 
> Additionally, the aforementioned patch performs OK for non-vfio OpenGL 
> enabled GTK displays, and fails DMA Buf iGVT-g display only, so far as I 
> could tell.
> 
> Perhaps a change like the following is better?
> 
>  static gboolean gd_frame_tick(GtkWidget *widget,
>                                GdkFrameClock *frame_clock,
>                                gpointer opaque)
>  {
>      VirtualConsole *vc = opaque;
> -
> -    vc->gfx.dcl.ops->dpy_refresh(&vc->gfx.dcl);
> +    gint64 interval;
> +    gdk_frame_clock_get_refresh_info(frame_clock,
> +            0,
> +            &interval,
> +            NULL);
> +    interval /= 1000;
> +    if (vc->gfx.dcl.update_interval != interval) {
> +        update_displaychangelistener(&vc->gfx.dcl, interval);
> +    }
>      return G_SOURCE_CONTINUE;
>  }
> 
> 
>> On Nov 14, 2018, at 11:10 AM, Chen Zhang <address@hidden 
>> <mailto:address@hidden>> wrote:
>> 
>> Hi,
>> 
>> I have briefly tested this patch. Unfortunately, it apparently caused 
>> deteriorated performance on a previously working Windows 10 guest with DMA 
>> Buf.
>> 
>> The patched qemu not only clogged up graphics drawing, but also obstructed 
>> the guest OS. The boot time and latency for guest operations (e.g. clicking 
>> Start menu in guest) dramatically increased. My best guess would be that GTK 
>> update events messed up with some polling in main_loop_wait(false), leading 
>> to blocked IO, or conversely the GUI being blocked by IO.
>> 
>> Best regards,
>> 
>> --
>> P.S:
>> 
>> The arguments for qemu:
>> 
>> /tmp/qemu-system-x86_64 -nodefaults -machine 
>> pc,accel=kvm,usb=off,kernel_irqchip=on \
>>  -vga none -display gtk,gl=on  -rtc base=localtime \
>>  -device 
>> vfio-pci,sysfsdev=/sys/bus/pci/devices/0000:00:02.0/{UUID-FOR-MDEV},x-igd-opregion=on,display=on,rombar=0
>>  \
>>  -m 3000 -realtime mlock=off -smp 3,sockets=1,cores=1 \
>>  -drive file=/home/user/disk.img,format=qcow2,media=disk,if=ide \
>>  -usb -device qemu-xhci,id=xhci -device usb-tablet,bus=xhci.0 \
>>  -cpu host -monitor stdio
>> 
>> The host runs a Ubuntu 18.04 desktop.
>> 
>>> On Nov 13, 2018, at 5:02 PM, Gerd Hoffmann <address@hidden 
>>> <mailto:address@hidden>> wrote:
>>> 
>>> Reduce console refresh timer to idle refresh rate.  Register a frame
>>> tick callback (called by gtk on each display frame) and use that to kick
>>> display updates instead.
>>> 
>>> That should sync qemu refresh rate to display refresh rate.  It'll also
>>> stop updating the qemu display in case the qemu is not visible (gtk
>>> stops calling the frame tick callback then).
>>> 
>>> Buglink: https://bugs.launchpad.net/bugs/1802915 
>>> <https://bugs.launchpad.net/bugs/1802915>
>>> Suggested-by: Chen Zhang <address@hidden <mailto:address@hidden>>
>>> Signed-off-by: Gerd Hoffmann <address@hidden <mailto:address@hidden>>
>>> ---
>>> ui/gtk.c | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>> 
>>> diff --git a/ui/gtk.c b/ui/gtk.c
>>> index 579990b865..8f79dfe42c 100644
>>> --- a/ui/gtk.c
>>> +++ b/ui/gtk.c
>>> @@ -1661,6 +1661,16 @@ static gboolean gd_configure(GtkWidget *widget,
>>>    return FALSE;
>>> }
>>> 
>>> +static gboolean gd_frame_tick(GtkWidget *widget,
>>> +                              GdkFrameClock *frame_clock,
>>> +                              gpointer opaque)
>>> +{
>>> +    VirtualConsole *vc = opaque;
>>> +
>>> +    vc->gfx.dcl.ops->dpy_refresh(&vc->gfx.dcl);
>>> +    return G_SOURCE_CONTINUE;
>>> +}
>>> +
>>> /** Virtual Console Callbacks **/
>>> 
>>> static GSList *gd_vc_menu_init(GtkDisplayState *s, VirtualConsole *vc,
>>> @@ -1911,6 +1921,12 @@ static void gd_connect_vc_gfx_signals(VirtualConsole 
>>> *vc)
>>>                         G_CALLBACK(gd_focus_out_event), vc);
>>>        g_signal_connect(vc->gfx.drawing_area, "configure-event",
>>>                         G_CALLBACK(gd_configure), vc);
>>> +        if (1 /* make that a config option ??? */) {
>>> +            update_displaychangelistener(&vc->gfx.dcl,
>>> +                                         GUI_REFRESH_INTERVAL_IDLE);
>>> +            gtk_widget_add_tick_callback(vc->gfx.drawing_area,
>>> +                                         gd_frame_tick, vc, NULL);
>>> +        }
>>>    } else {
>>>        g_signal_connect(vc->gfx.drawing_area, "key-press-event",
>>>                         G_CALLBACK(gd_text_key_down), vc);
>>> -- 
>>> 2.9.3
>>> 
>> 
>