From 124062686e7268f7b44157269e037e09f0faba4f Mon Sep 17 00:00:00 2001
From: Yaniv Kamay <ykamay@redhat.com>
Date: Mon, 11 Jan 2010 12:57:23 -0200
Subject: [PATCH] qxl: defer vga updates in case commands ring is full #544785

RH-Author: Yaniv Kamay <ykamay@redhat.com>
Message-id: <1263214643-28734-1-git-send-email-ykamay@redhat.com>
Patchwork-id: 6144
O-Subject: [PATCH] qxl: defer vga updates in case commands ring is full #544785
Bugzilla: 544785
RH-Acked-by: Markus Armbruster <armbru@redhat.com>
RH-Acked-by: Izik Eidus <ieidus@redhat.com>
RH-Acked-by: Arnon Gilboa <agilboa@redhat.com>

From: Yaniv Kamay <ykamay@redhat.com>

If there's no space in the QXL VGA ring buffer, QEMU blocks until
there is.  While it's blocked, it is unresponsive.  Especially bad
when the (remote) client goes away.

Instead of blocking in qxl_display_update(), accumulate a dirty
rectangle, and attempt to flush it in new qxl_display_update().
---
 qemu/hw/qxl.c |  195 +++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 126 insertions(+), 69 deletions(-)

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu/hw/qxl.c |  195 +++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 126 insertions(+), 69 deletions(-)

diff --git a/qemu/hw/qxl.c b/qemu/hw/qxl.c
index 7090939..8778a7d 100644
--- a/qemu/hw/qxl.c
+++ b/qemu/hw/qxl.c
@@ -36,10 +36,6 @@
 #define TRUE 1
 #define FALSE 0
 
-#define WAIT_CMD_DELAY_MS 10 
-#define WAIT_CMD_MESSAGE_THRESHOLD 2
-#define WAIT_CMD_MESSAGE_INTERVAL_MS 5000
-
 #define QXL_DEV_NAME "qxl"
 #define VDI_PORT_DEV_NAME "vdi_port"
 
@@ -134,6 +130,7 @@ struct PCIQXLDevice {
     QXLState state;
     int id;
     DisplayState ds;
+    Rect dirty_rect;
     struct PCIQXLDevice *dev_next;
     struct PCIQXLDevice *vga_next;
     int pipe_fd[2];
@@ -216,6 +213,7 @@ typedef struct QXLVga {
     PCIQXLDevice *clients;
     int active_clients;
     QEMUTimer *timer;
+    int need_update;
 } QXLVga;
 
 static void qxl_exit_vga_mode(PCIQXLDevice *d);
@@ -631,6 +629,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d)
     d->state.rom->mode = ~0;
     d->state.shadow_rom.mode = ~0;
     d->state.mode = QXL_MODE_VGA;
+    memset(&d->dirty_rect, 0, sizeof(d->dirty_rect));
     qxl_notify_mode_change(d);
     qxl_add_vga_client();
 }
@@ -1148,77 +1147,131 @@ inline uint32_t msb_mask(uint32_t val)
     return mask;
 }
 
+static int rect_is_empty(const Rect* r)
+{
+    return r->top == r->bottom || r->left == r->right;
+}
+
+static void rect_union(Rect *dest, const Rect *r)
+{
+    if (rect_is_empty(r)) {
+        return;
+    }
+
+    if (rect_is_empty(dest)) {
+        *dest = *r;
+        return;
+    }
+
+    dest->top = MIN(dest->top, r->top);
+    dest->left = MIN(dest->left, r->left);
+    dest->bottom = MAX(dest->bottom, r->bottom);
+    dest->right = MAX(dest->right, r->right);
+}
+
+static void qxl_vga_update_one(PCIQXLDevice *d)
+{
+    QXLDrawable *drawable;
+    QXLImage *image;
+    QXLCommandRing *ring;
+    QXLCommand *cmd;
+    int wait;
+    int notify;
+    Rect *dirty_rect;
+
+    dirty_rect = &d->dirty_rect;
+    ring = &d->state.vga_ring;
+    RING_PROD_WAIT(ring, wait);
+
+    if (wait) {
+        qxl_vga.need_update = TRUE;
+        return;
+    }
+
+    drawable = (QXLDrawable *)malloc(sizeof(*drawable) + sizeof(*image));
+    if (!drawable) {
+        printf("%s: alloc drawable failed\n", __FUNCTION__);
+        abort();
+    }
+
+    image = (QXLImage *)(drawable + 1);
+    drawable->bbox = *dirty_rect;
+    drawable->clip.type = CLIP_TYPE_NONE;
+    drawable->clip.data = 0;
+    drawable->effect = QXL_EFFECT_OPAQUE;
+    drawable->release_info.id = (UINT64)drawable;
+    drawable->bitmap_offset = 0;
+    drawable->type = QXL_DRAW_COPY;
+
+    drawable->u.copy.rop_decriptor =  ROPD_OP_PUT;
+    drawable->u.copy.src_bitmap = (PHYSICAL)image;
+    drawable->u.copy.src_area.left = drawable->u.copy.src_area.top = 0;
+    drawable->u.copy.src_area.right = dirty_rect->right - dirty_rect->left;
+    drawable->u.copy.src_area.bottom = dirty_rect->bottom - dirty_rect->top;
+    drawable->u.copy.scale_mode = 0;
+    memset(&drawable->u.copy.mask, 0, sizeof(QMask));
+
+    image->descriptor.type = IMAGE_TYPE_BITMAP;
+    image->descriptor.flags = 0;
+    QXL_SET_IMAGE_ID(image, QXL_IMAGE_GROUP_DEVICE, ++d->state.bits_unique);
+    image->bitmap.flags = QXL_BITMAP_DIRECT | QXL_BITMAP_TOP_DOWN | QXL_BITMAP_UNSTABLE;
+    image->bitmap.format = BITMAP_FMT_32BIT;
+    image->bitmap.stride = qxl_vga.ds->linesize;
+    image->descriptor.width = image->bitmap.x = drawable->u.copy.src_area.right;
+    image->descriptor.height = image->bitmap.y = drawable->u.copy.src_area.bottom;
+    image->bitmap.data = (PHYSICAL)(qxl_vga.ds->data +
+                                    dirty_rect->top * qxl_vga.ds->linesize +
+                                    dirty_rect->left * 4);
+    image->bitmap.palette = 0;
+
+    cmd = RING_PROD_ITEM(ring);
+    cmd->type = QXL_CMD_DRAW;
+    cmd->data = (PHYSICAL)drawable;
+    RING_PUSH(ring, notify);
+
+    if (notify) {
+        d->worker->wakeup(d->worker);
+    }
+
+    memset(dirty_rect, 0, sizeof(*dirty_rect));
+}
+
 static void qxl_display_update(struct DisplayState *ds, int x, int y, int w, int h)
 {
     PCIQXLDevice *client;
+    Rect update_area;
 
-    client = qxl_vga.clients;
-    while (client) {
+    update_area.left = x,
+    update_area.right = x + w;
+    update_area.top = y;
+    update_area.bottom = y + h;
+
+    if (rect_is_empty(&update_area)) {
+        return;
+    }
+
+    qxl_vga.need_update = FALSE;
+
+    for (client = qxl_vga.clients; client; client = client->vga_next) {
         if (client->state.mode == QXL_MODE_VGA && client->state.running) {
-            QXLDrawable *drawable;
-            QXLImage *image;
-            QXLCommandRing *ring;
-            QXLCommand *cmd;
-            int wait;
-            int notify;
-            int wait_count;
-
-            drawable = (QXLDrawable *)malloc(sizeof(*drawable) + sizeof(*image));
-            ASSERT(drawable);
-            image = (QXLImage *)(drawable + 1);
-            drawable->bbox.left = x;
-            drawable->bbox.right = x + w;
-            drawable->bbox.top = y;
-            drawable->bbox.bottom = y + h;
-            drawable->clip.type = CLIP_TYPE_NONE;
-            drawable->clip.data = 0;
-            drawable->effect = QXL_EFFECT_OPAQUE;
-            drawable->release_info.id = (UINT64)drawable;
-            drawable->bitmap_offset = 0;
-            drawable->type = QXL_DRAW_COPY;
-
-            drawable->u.copy.rop_decriptor =  ROPD_OP_PUT;
-            drawable->u.copy.src_bitmap = (PHYSICAL)image;
-            drawable->u.copy.src_area.left = drawable->u.copy.src_area.top = 0;
-            drawable->u.copy.src_area.right = w;
-            drawable->u.copy.src_area.bottom = h;
-            drawable->u.copy.scale_mode = 0;
-            memset(&drawable->u.copy.mask, 0, sizeof(QMask));
-
-            image->descriptor.type = IMAGE_TYPE_BITMAP;
-            image->descriptor.flags = 0;
-            QXL_SET_IMAGE_ID(image, QXL_IMAGE_GROUP_DEVICE, ++client->state.bits_unique);
-            image->bitmap.flags = QXL_BITMAP_DIRECT | QXL_BITMAP_TOP_DOWN | QXL_BITMAP_UNSTABLE;
-            image->bitmap.format = BITMAP_FMT_32BIT;
-            image->bitmap.stride = ds->linesize;
-            image->descriptor.width = image->bitmap.x = w;
-            image->descriptor.height = image->bitmap.y = h;
-            image->bitmap.data = (PHYSICAL)(ds->data + y * ds->linesize + x * 4);
-            image->bitmap.palette = 0;
-
-            ring = &client->state.vga_ring;
-            wait_count = -WAIT_CMD_MESSAGE_THRESHOLD;
-            for (;;) {    
-                RING_PROD_WAIT(ring, wait);
-                if (wait) {
-                    usleep(WAIT_CMD_DELAY_MS * 1000);
-                    if (!(wait_count++ % (WAIT_CMD_MESSAGE_INTERVAL_MS / WAIT_CMD_DELAY_MS))) {
-                        printf("%s: waiting for command\n", __FUNCTION__);
-                    }
-                    continue;
-                }
-                break;
-            }
-
-            cmd = RING_PROD_ITEM(ring);
-            cmd->type = QXL_CMD_DRAW;
-            cmd->data = (PHYSICAL)drawable;
-            RING_PUSH(ring, notify);
-            if (notify) {
-                client->worker->wakeup(client->worker);
-            }
+            rect_union(&client->dirty_rect, &update_area);
+            qxl_vga_update_one(client);
+        }
+    }
+}
+
+static void qxl_vga_update(void)
+{
+    PCIQXLDevice *client;
+
+    qxl_vga.need_update = FALSE;
+
+    for (client = qxl_vga.clients; client; client = client->vga_next) {
+        if (client->state.mode == QXL_MODE_VGA && client->state.running &&
+                !rect_is_empty(&client->dirty_rect)) {
+
+            qxl_vga_update_one(client);
         }
-        client = client->vga_next;
     }
 }
 
@@ -1251,6 +1304,9 @@ static void qxl_display_refresh(struct DisplayState *ds)
 {
     if (qxl_vga.active_clients) {
         vga_hw_update();
+        if (qxl_vga.need_update) {
+            qxl_vga_update();
+        }
     }
 }
 
@@ -1433,6 +1489,7 @@ static void qxl_vm_change_state_handler(void *opaque, int running)
     printf("QXL: %s: running=%d\n", __FUNCTION__, running);
 
     if (running) {
+        qxl_vga.need_update = TRUE;
         d->state.running = TRUE;
         qemu_set_fd_handler(d->pipe_fd[0], qxl_pipe_read, NULL, d);
         d->worker->start(d->worker);
-- 
1.6.6