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

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

Hello,

Something like this then?



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	Mon May 05 17:44:16 2008 +0100
@@ -249,11 +249,41 @@ struct fbfront_dev {
     int stride;
     int mem_length;
     int offset;
+
+    int status;
 };
 
 void fbfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
 {
+    struct fbfront_dev *dev = data;
+    struct xenfb_page *page = dev->page;
+    uint32_t prod, cons;
+    int i;
+
     wake_up(&fbfront_queue);
+    prod = page->in_prod;
+
+    if (prod == page->in_cons)
+        return;
+
+    rmb();      /* ensure we see ring contents up to prod */
+
+    for (i = 0, cons = page->in_cons; cons != prod; i++, cons++) {
+        union xenfb_in_event *event = &XENFB_IN_RING_REF(page, cons);
+        switch (event->type) {
+        case XENFB_TYPE_BACKEND_STATUS:
+            dev->status = event->status.status;
+            printk("got status %d\n", dev->status);
+            break;
+        default:
+            /* ignore */
+            break;
+        }
+    }
+    mb();       /* ensure we got ring contents */
+    page->in_cons = cons;
+
+    notify_remote_via_evtchn(dev->evtchn);
 }
 
 struct fbfront_dev *init_fbfront(char *nodename, unsigned long *mfns, int width, int height, int depth, int stride, int n)
@@ -292,6 +322,7 @@ struct fbfront_dev *init_fbfront(char *n
     dev->stride = s->line_length = stride;
     dev->mem_length = s->mem_length = n * PAGE_SIZE;
     dev->offset = 0;
+    dev->status = XENFB_BACKEND_STATUS_ACTIVE;
 
     const int max_pd = sizeof(s->pd) / sizeof(s->pd[0]);
     unsigned long mapped = 0;
@@ -405,6 +436,11 @@ static void fbfront_out_event(struct fbf
     wmb(); /* ensure ring contents visible */
     page->out_prod = prod + 1;
     notify_remote_via_evtchn(dev->evtchn);
+}
+
+int fbfront_status(struct fbfront_dev *dev)
+{
+    return dev->status;
 }
 
 void fbfront_update(struct fbfront_dev *dev, int x, int y, int width, int height)
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	Mon May 05 17:44:16 2008 +0100
@@ -36,6 +36,7 @@ int fbfront_open(struct fbfront_dev *dev
 int fbfront_open(struct fbfront_dev *dev);
 #endif
 
+int fbfront_status(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	Mon May 05 17:44:16 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 notified_active;   /* Did we request update */
 	char protocol[64];	/* frontend protocol */
 };
 
@@ -536,6 +537,40 @@ static void xenfb_on_fb_event(struct xen
 	xc_evtchn_notify(xenfb->evt_xch, xenfb->fb.port);
 }
 
+static int xenfb_queue_full(struct xenfb *xenfb)
+{
+        struct xenfb_page *page = xenfb->fb.page;
+        uint32_t cons, prod;
+
+        prod = page->in_prod;
+        cons = page->in_cons;
+        return prod - cons == XENFB_IN_RING_LEN;
+}
+
+static void xenfb_send_event(struct xenfb *xenfb, union xenfb_in_event *event)
+{
+        uint32_t prod;
+        struct xenfb_page *page = xenfb->fb.page;
+
+        prod = page->in_prod;
+        /* caller ensures !xenfb_queue_full() */
+        xen_mb();                   /* ensure ring space available */
+        XENFB_IN_RING_REF(page, prod) = *event;
+        xen_wmb();                  /* ensure ring contents visible */
+        page->in_prod = prod + 1;
+
+	xc_evtchn_notify(xenfb->evt_xch, xenfb->fb.port);
+}
+
+static void xenfb_send_status(struct xenfb *xenfb, int active)
+{
+        union xenfb_in_event event;
+        event.type = XENFB_TYPE_BACKEND_STATUS;
+        event.status.status = active ? XENFB_BACKEND_STATUS_ACTIVE
+                                     : XENFB_BACKEND_STATUS_IDLE ;
+        xenfb_send_event(xenfb, &event);
+}
+
 static void xenfb_on_kbd_event(struct xenfb *xenfb)
 {
 	struct xenkbd_page *page = xenfb->kbd.page;
@@ -707,6 +742,7 @@ static int xenfb_read_frontend_fb_config
                             xenfb->protocol) < 0)
                 xenfb->protocol[0] = '\0';
         xenfb_xs_printf(xenfb->xsh, xenfb->fb.nodename, "request-update", "1");
+        xenfb->notified_active = 1;
 
         /* TODO check for permitted ranges */
         fb_page = xenfb->fb.page;
@@ -1185,10 +1221,21 @@ static void xenfb_guest_copy(struct xenf
     dpy_update(xenfb->ds, x, y, w, h);
 }
 
-/* Periodic update of display, no need for any in our case */
+/* Periodic update of display, just announce idleness to the front end */
 static void xenfb_update(void *opaque)
 {
     struct xenfb *xenfb = opaque;
+    if (xenfb->ds->idle) {
+        if (xenfb->notified_active && !xenfb_queue_full(xenfb)) {
+            xenfb_send_status(xenfb, 0);
+            xenfb->notified_active = 0;
+        }
+    } else {
+        if (!xenfb->notified_active && !xenfb_queue_full(xenfb)) {
+            xenfb_send_status(xenfb, 1);
+            xenfb->notified_active = 1;
+        }
+    }
 }
 
 /* QEMU display state changed, so refresh the framebuffer copy */
@@ -1318,7 +1365,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_status(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 +1509,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	Mon May 05 17:44:16 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	Mon May 05 17:44:16 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	Mon May 05 17:44:16 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	Mon May 05 17:44:16 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;
diff -r 32342f2b5742 xen/include/public/io/fbif.h
--- a/xen/include/public/io/fbif.h	Fri May 02 15:12:43 2008 +0100
+++ b/xen/include/public/io/fbif.h	Mon May 05 17:44:16 2008 +0100
@@ -80,14 +80,30 @@ union xenfb_out_event
 
 /*
  * Frontends should ignore unknown in events.
- * No in events currently defined.
  */
+
+/*
+ * Backend idleness report
+ * Backend sends it when the output window is somehow non visible
+ * (minimized, no client, etc.)
+ */
+#define XENFB_TYPE_BACKEND_STATUS 1
+
+#define XENFB_BACKEND_STATUS_IDLE 0
+#define XENFB_BACKEND_STATUS_ACTIVE 1
+
+struct xenfb_backend_status
+{
+    uint8_t type;    /* XENFB_TYPE_BACKEND_STATUS */
+    uint8_t status;  /* XENFB_BACKEND_STATUS_* */
+};
 
 #define XENFB_IN_EVENT_SIZE 40
 
 union xenfb_in_event
 {
     uint8_t type;
+    struct xenfb_backend_status status;
     char pad[XENFB_IN_EVENT_SIZE];
 };