Re: [RFC] PVFB: Add refresh period to XenStore parameters?

[Samuel Thibault <samuel.thibault@eu.citrix.com>]

Samuel Thibault, le Thu 01 May 2008 18:55:36 +0100, a écrit :
> Getting back to that old issue.
> 
> Samuel Thibault, le Tue 04 Mar 2008 16:12:20 +0000, a écrit :
> > > * If the frontend can track changes efficiently, it sends update
> > >   messages to the backend, which can use them to only redisplay
> > >   changed areas.
> > 
> > Well, that was actually my next target :)
> > (but for that we badly need the resize/redepth support).
> > 
> > However, that doesn't solve something that still bugs me, which is
> > that the VGA emulation layer wakes up every 30ms to just notice that
> > the width/height/depth registers didn't change etc. even if the actual
> > window is not shown.
> 
> What could be done is to make the backend set request-update to 0 when
> the window is minimized, which makes the frontend understand that there
> is temporarily no need to send updates any more, and set it back to 1
> when the window is restored.  Would that be OK?

Here is how it would be implemented:


ioemu: transmit idleness information to avoid useless polls

Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

diff -r 32342f2b5742 extras/mini-os/fbfront.c
--- a/extras/mini-os/fbfront.c	Fri May 02 15:12:43 2008 +0100
+++ b/extras/mini-os/fbfront.c	Fri May 02 17:05:17 2008 +0100
@@ -243,6 +243,8 @@ struct fbfront_dev {
     char *backend;
     int request_update;
 
+    struct xenbus_event *volatile events;
+
     int width;
     int height;
     int depth;
@@ -377,6 +379,8 @@ done:
         xenbus_unwatch_path(XBT_NIL, path);
 
         snprintf(path, sizeof(path), "%s/request-update", dev->backend);
+        dev->events = NULL;
+        xenbus_watch_path_token(XBT_NIL, path, path, &dev->events);
         dev->request_update = xenbus_read_integer(path);
 
         err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 4); /* connected */
@@ -407,12 +411,20 @@ static void fbfront_out_event(struct fbf
     notify_remote_via_evtchn(dev->evtchn);
 }
 
+int fbfront_update_requested(struct fbfront_dev *dev)
+{
+    struct xenbus_event *event;
+    while ((event = dev->events)) {
+        dev->events = event->next;
+        dev->request_update = xenbus_read_integer(event->path);
+        xfree(event);
+    }
+    return dev->request_update;
+}
+
 void fbfront_update(struct fbfront_dev *dev, int x, int y, int width, int height)
 {
     struct xenfb_update update;
-
-    if (dev->request_update <= 0)
-        return;
 
     if (x < 0) {
         width += x;
@@ -461,6 +473,7 @@ void shutdown_fbfront(struct fbfront_dev
 
     printk("close fb: backend at %s\n",dev->backend);
 
+    xenbus_unwatch_path_token(XBT_NIL, path, path);
     snprintf(path, sizeof(path), "%s/state", dev->backend);
     err = xenbus_printf(XBT_NIL, nodename, "state", "%u", 5); /* closing */
     xenbus_wait_for_value(path,"5");
diff -r 32342f2b5742 extras/mini-os/include/fbfront.h
--- a/extras/mini-os/include/fbfront.h	Fri May 02 15:12:43 2008 +0100
+++ b/extras/mini-os/include/fbfront.h	Fri May 02 17:05:17 2008 +0100
@@ -36,6 +36,7 @@ int fbfront_open(struct fbfront_dev *dev
 int fbfront_open(struct fbfront_dev *dev);
 #endif
 
+int fbfront_update_requested(struct fbfront_dev *dev);
 void fbfront_update(struct fbfront_dev *dev, int x, int y, int width, int height);
 void fbfront_resize(struct fbfront_dev *dev, int width, int height, int stride, int depth, int offset);
 
diff -r 32342f2b5742 tools/ioemu/hw/xenfb.c
--- a/tools/ioemu/hw/xenfb.c	Fri May 02 15:12:43 2008 +0100
+++ b/tools/ioemu/hw/xenfb.c	Fri May 02 17:05:17 2008 +0100
@@ -59,6 +59,7 @@ struct xenfb {
 	int offset;             /* offset of the framebuffer */
 	int abs_pointer_wanted; /* Whether guest supports absolute pointer */
 	int button_state;       /* Last seen pointer button state */
+        int requested_update;   /* Did we request update */
 	char protocol[64];	/* frontend protocol */
 };
 
@@ -706,7 +707,8 @@ static int xenfb_read_frontend_fb_config
         if (xenfb_xs_scanf1(xenfb->xsh, xenfb->fb.otherend, "protocol", "%63s",
                             xenfb->protocol) < 0)
                 xenfb->protocol[0] = '\0';
-        xenfb_xs_printf(xenfb->xsh, xenfb->fb.nodename, "request-update", "1");
+        xenfb_xs_printf(xenfb->xsh, xenfb->fb.nodename, "request-update", xenfb->ds->idle ? "0" : "1");
+        xenfb->requested_update = !xenfb->ds->idle;
 
         /* TODO check for permitted ranges */
         fb_page = xenfb->fb.page;
@@ -1189,6 +1191,17 @@ static void xenfb_update(void *opaque)
 static void xenfb_update(void *opaque)
 {
     struct xenfb *xenfb = opaque;
+    if (xenfb->ds->idle) {
+        if (xenfb->requested_update) {
+            xenfb_xs_printf(xenfb->xsh, xenfb->fb.nodename, "request-update", "0");
+            xenfb->requested_update = 0;
+        }
+    } else {
+        if (!xenfb->requested_update) {
+            xenfb_xs_printf(xenfb->xsh, xenfb->fb.nodename, "request-update", "1");
+            xenfb->requested_update = 1;
+        }
+    }
 }
 
 /* QEMU display state changed, so refresh the framebuffer copy */
@@ -1318,7 +1331,22 @@ static void xenfb_pv_setdata(DisplayStat
 
 static void xenfb_pv_refresh(DisplayState *ds)
 {
+    struct fbfront_dev *fb_dev = ds->opaque;
+
     vga_hw_update();
+
+    if (!fb_dev)
+        return;
+
+    if (fbfront_update_requested(fb_dev)) {
+        /* Back to default interval */
+        ds->gui_timer_interval = 0;
+        ds->idle = 0;
+    } else {
+        /* Sleeping interval */
+        ds->gui_timer_interval = 500;
+        ds->idle = 1;
+    }
 }
 
 static void xenfb_kbd_handler(void *opaque)
@@ -1447,6 +1475,7 @@ int xenfb_pv_display_init(DisplayState *
     ds->dpy_colourdepth = xenfb_pv_colourdepth;
     ds->dpy_setdata = xenfb_pv_setdata;
     ds->dpy_refresh = xenfb_pv_refresh;
+    ds->opaque = NULL;
     return 0;
 }
 
diff -r 32342f2b5742 tools/ioemu/sdl.c
--- a/tools/ioemu/sdl.c	Fri May 02 15:12:43 2008 +0100
+++ b/tools/ioemu/sdl.c	Fri May 02 17:05:17 2008 +0100
@@ -696,9 +696,11 @@ static void sdl_refresh(DisplayState *ds
 		if (ev->active.gain) {
 		    /* Back to default interval */
 		    ds->gui_timer_interval = 0;
+		    ds->idle = 0;
 		} else {
 		    /* Sleeping interval */
 		    ds->gui_timer_interval = 500;
+		    ds->idle = 1;
 		}
 	    }
             break;
diff -r 32342f2b5742 tools/ioemu/vl.c
--- a/tools/ioemu/vl.c	Fri May 02 15:12:43 2008 +0100
+++ b/tools/ioemu/vl.c	Fri May 02 17:05:17 2008 +0100
@@ -4467,6 +4467,8 @@ void dumb_display_init(DisplayState *ds)
     ds->dpy_resize = dumb_resize;
     ds->dpy_colourdepth = NULL;
     ds->dpy_refresh = dumb_refresh;
+    ds->gui_timer_interval = 500;
+    ds->idle = 1;
 }
 
 /***********************************************************/
diff -r 32342f2b5742 tools/ioemu/vl.h
--- a/tools/ioemu/vl.h	Fri May 02 15:12:43 2008 +0100
+++ b/tools/ioemu/vl.h	Fri May 02 17:05:17 2008 +0100
@@ -939,6 +939,7 @@ struct DisplayState {
     void *opaque;
     uint32_t *palette;
     uint64_t gui_timer_interval;
+    int idle;
 
     int shared_buf;
     
diff -r 32342f2b5742 tools/ioemu/vnc.c
--- a/tools/ioemu/vnc.c	Fri May 02 15:12:43 2008 +0100
+++ b/tools/ioemu/vnc.c	Fri May 02 17:05:17 2008 +0100
@@ -778,6 +778,7 @@ static void _vnc_update_client(void *opa
     vs->has_update = 0;
     vnc_flush(vs);
     vs->last_update_time = now;
+    vs->ds->idle = 0;
 
     vs->timer_interval /= 2;
     if (vs->timer_interval < VNC_REFRESH_INTERVAL_BASE)
@@ -790,26 +791,29 @@ static void _vnc_update_client(void *opa
     vs->timer_interval += VNC_REFRESH_INTERVAL_INC;
     if (vs->timer_interval > VNC_REFRESH_INTERVAL_MAX) {
 	vs->timer_interval = VNC_REFRESH_INTERVAL_MAX;
-	if (now - vs->last_update_time >= VNC_MAX_UPDATE_INTERVAL &&
-            vs->update_requested) {
-	    /* Send a null update.  If the client is no longer
-	       interested (e.g. minimised) it'll ignore this, and we
-	       can stop scanning the buffer until it sends another
-	       update request. */
-	    /* It turns out that there's a bug in realvncviewer 4.1.2
-	       which means that if you send a proper null update (with
-	       no update rectangles), it gets a bit out of sync and
-	       never sends any further requests, regardless of whether
-	       it needs one or not.  Fix this by sending a single 1x1
-	       update rectangle instead. */
-	    vnc_write_u8(vs, 0);
-	    vnc_write_u8(vs, 0);
-	    vnc_write_u16(vs, 1);
-	    send_framebuffer_update(vs, 0, 0, 1, 1);
-	    vnc_flush(vs);
-	    vs->last_update_time = now;
-            vs->update_requested--;
-	    return;
+	if (now - vs->last_update_time >= VNC_MAX_UPDATE_INTERVAL) {
+            if (!vs->update_requested) {
+                vs->ds->idle = 1;
+            } else {
+                /* Send a null update.  If the client is no longer
+                   interested (e.g. minimised) it'll ignore this, and we
+                   can stop scanning the buffer until it sends another
+                   update request. */
+                /* It turns out that there's a bug in realvncviewer 4.1.2
+                   which means that if you send a proper null update (with
+                   no update rectangles), it gets a bit out of sync and
+                   never sends any further requests, regardless of whether
+                   it needs one or not.  Fix this by sending a single 1x1
+                   update rectangle instead. */
+                vnc_write_u8(vs, 0);
+                vnc_write_u8(vs, 0);
+                vnc_write_u16(vs, 1);
+                send_framebuffer_update(vs, 0, 0, 1, 1);
+                vnc_flush(vs);
+                vs->last_update_time = now;
+                vs->update_requested--;
+                return;
+            }
 	}
     }
     qemu_mod_timer(vs->timer, now + vs->timer_interval);
@@ -970,6 +974,7 @@ static int vnc_client_io_error(VncState 
 	qemu_set_fd_handler2(vs->csock, NULL, NULL, NULL, NULL);
 	closesocket(vs->csock);
 	vs->csock = -1;
+        vs->ds->idle = 1;
 	buffer_reset(&vs->input);
 	buffer_reset(&vs->output);
         free_queue(vs);
@@ -2443,6 +2448,7 @@ static void vnc_listen_read(void *opaque
     vs->csock = accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);
     if (vs->csock != -1) {
 	VNC_DEBUG("New client on socket %d\n", vs->csock);
+        vs->ds->idle = 0;
         socket_set_nonblock(vs->csock);
 	qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, opaque);
 	vnc_write(vs, "RFB 003.008\n", 12);
@@ -2468,6 +2474,7 @@ void vnc_display_init(DisplayState *ds)
 	exit(1);
 
     ds->opaque = vs;
+    ds->idle = 1;
     vnc_state = vs;
     vs->display = NULL;
     vs->password = NULL;