[Qemu-devel] [PATCH] console: remove refresh timer when using spice & QXL mode

[Qemu-devel] [PATCH] console: remove refresh timer when using spice & QXL mode

[Marc-André Lureau]

There is no need for the display refresh timer when using Spice with QXL
driver. The refresh is restored when entering VGA mode, or if any other
display listener requires refresh. This saves a few wakeups per seconds.

Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=912763

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
---
 hw/display/qxl.c     | 14 ++++++++++++++
 include/ui/console.h |  3 +++
 ui/console.c         | 23 ++++++++++++++++++++---
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index b540dd6..34a6334 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1096,6 +1096,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d)
     d->mode = QXL_MODE_VGA;
     vga_dirty_log_start(&d->vga);
     graphic_hw_update(d->vga.con);
+    update_displaystate(d->ssd.dcl.ds);
 }
 
 static void qxl_exit_vga_mode(PCIQXLDevice *d)
@@ -1107,6 +1108,7 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
     graphic_console_set_hwops(d->ssd.dcl.con, &qxl_ops, d);
     vga_dirty_log_stop(&d->vga);
     qxl_destroy_primary(d, QXL_SYNC);
+    update_displaystate(d->ssd.dcl.ds);
 }
 
 static void qxl_update_irq(PCIQXLDevice *d)
@@ -1868,11 +1870,23 @@ static void display_refresh(DisplayChangeListener *dcl)
     }
 }
 
+static bool display_need_refresh(DisplayChangeListener *dcl)
+{
+    PCIQXLDevice *qxl = container_of(dcl, PCIQXLDevice, ssd.dcl);
+
+    if (qxl->mode == QXL_MODE_VGA) {
+        return true;
+    }
+
+    return false;
+}
+
 static DisplayChangeListenerOps display_listener_ops = {
     .dpy_name        = "spice/qxl",
     .dpy_gfx_update  = display_update,
     .dpy_gfx_switch  = display_switch,
     .dpy_refresh     = display_refresh,
+    .dpy_need_refresh = display_need_refresh,
 };
 
 static void qxl_init_ramsize(PCIQXLDevice *qxl)
diff --git a/include/ui/console.h b/include/ui/console.h
index 22ef8ca..cd0afc5 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -152,6 +152,8 @@ void cursor_get_mono_mask(QEMUCursor *c, int transparent, uint8_t *mask);
 typedef struct DisplayChangeListenerOps {
     const char *dpy_name;
 
+    bool (*dpy_need_refresh)(DisplayChangeListener *dcl);
+
     void (*dpy_refresh)(DisplayChangeListener *dcl);
 
     void (*dpy_gfx_update)(DisplayChangeListener *dcl,
@@ -185,6 +187,7 @@ struct DisplayChangeListener {
 };
 
 DisplayState *init_displaystate(void);
+void update_displaystate(DisplayState *ds);
 DisplaySurface *qemu_create_displaysurface_from(int width, int height,
                                                 pixman_format_code_t format,
                                                 int linesize, uint8_t *data);
diff --git a/ui/console.c b/ui/console.c
index 258af5d..9815667 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -180,6 +180,7 @@ static QEMUTimer *cursor_timer;
 
 static void text_console_do_init(CharDriverState *chr, DisplayState *ds);
 static void dpy_refresh(DisplayState *s);
+static bool dpy_need_refresh(DisplayState *s);
 static DisplayState *get_alloc_displaystate(void);
 static void text_console_update_cursor_timer(void);
 static void text_console_update_cursor(void *opaque);
@@ -216,7 +217,7 @@ static void gui_update(void *opaque)
     timer_mod(ds->gui_timer, ds->last_update + interval);
 }
 
-static void gui_setup_refresh(DisplayState *ds)
+void update_displaystate(DisplayState *ds)
 {
     DisplayChangeListener *dcl;
     bool need_timer = false;
@@ -235,6 +236,8 @@ static void gui_setup_refresh(DisplayState *ds)
         }
     }
 
+    need_timer &= dpy_need_refresh(ds);
+
     if (need_timer && ds->gui_timer == NULL) {
         ds->gui_timer = timer_new_ms(QEMU_CLOCK_REALTIME, gui_update, ds);
         timer_mod(ds->gui_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
@@ -1341,7 +1344,7 @@ void register_displaychangelistener(DisplayChangeListener *dcl)
     trace_displaychangelistener_register(dcl, dcl->ops->dpy_name);
     dcl->ds = get_alloc_displaystate();
     QLIST_INSERT_HEAD(&dcl->ds->listeners, dcl, next);
-    gui_setup_refresh(dcl->ds);
+    update_displaystate(dcl->ds);
     if (dcl->con) {
         dcl->con->dcls++;
         con = dcl->con;
@@ -1380,7 +1383,7 @@ void unregister_displaychangelistener(DisplayChangeListener *dcl)
         dcl->con->dcls--;
     }
     QLIST_REMOVE(dcl, next);
-    gui_setup_refresh(ds);
+    update_displaystate(ds);
 }
 
 int dpy_set_ui_info(QemuConsole *con, QemuUIInfo *info)
@@ -1450,6 +1453,20 @@ static void dpy_refresh(DisplayState *s)
     }
 }
 
+static bool dpy_need_refresh(DisplayState *s)
+{
+    DisplayChangeListener *dcl;
+
+    QLIST_FOREACH(dcl, &s->listeners, next) {
+        if (!dcl->ops->dpy_need_refresh ||
+            dcl->ops->dpy_need_refresh(dcl)) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y,
                   int dst_x, int dst_y, int w, int h)
 {
-- 
1.9.3

Re: [Qemu-devel] [PATCH] console: remove refresh timer when using spice & QXL mode

[Gerd Hoffmann]

On Di, 2014-11-04 at 12:23 +0100, Marc-André Lureau wrote:
> There is no need for the display refresh timer when using Spice with QXL
> driver.

No, there is a need.  Cursor updates (when running qxl with other ui
than spice) are done from the refresh timer.

Also we can already adjust the wakeup rate, we only need to use it,
there is no need for a new hook.

Alternative patches will come in a moment, please test.  I still a high
number of wakeups but most likely due to my spice-server being
unpatched.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] console: remove refresh timer when using spice & QXL mode

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

On Tue, Nov 4, 2014 at 3:08 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:

> No, there is a need.  Cursor updates (when running qxl with other ui
> than spice) are done from the refresh timer.
>
> Right, that's why my patch checks if there are "any other
display listener requires refresh"

Also we can already adjust the wakeup rate, we only need to use it,
> there is no need for a new hook.
>
> I think it is cleaner to have different path for refresh rate and
disabling refresh,
following the current code.

Alternative patches will come in a moment, please test.  I still a high
> number of wakeups but most likely due to my spice-server being
> unpatched.
>

Yes, please try with the RFC patch "remove guest-side timestamping" I sent
earlier (your review there would be useful to)
http://lists.freedesktop.org/archives/spice-devel/2014-November/017802.html

thanks



-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2004 bytes --]