From c26805e29312fee136008a57c70a2f5f140ba706 Mon Sep 17 00:00:00 2001

From: Alon Levy <alevy@redhat.com>

Date: Fri, 24 Feb 2012 23:19:31 +0200

Subject: [PATCH 418/434] qxl: make qxl_render_update async

RHBZ# 747011

Removes the last user of QXL_SYNC when using update drivers that use the

_ASYNC io ports.

The last user is qxl_render_update, it is called both by qxl_hw_update

which is the vga_hw_update_ptr passed to graphic_console_init, and by

qxl_hw_screen_dump.

At the same time the QXLRect area being passed to the red_worker thread

is passed as a copy, as part of the QXLCookie.

The implementation uses interface_update_area_complete with a bh to make

sure dpy_update and qxl_flip are called from the io thread, otherwise

the vga->ds->surface.data can change under our feet.

With this patch sdl+spice works fine. But spice by itself doesn't

produce the expected screendumps unless repeated a few times, due to

ppm_save being called before update_area (rendering done in spice server

thread) having a chance to complete. Fixed by next patch, but see commit

message for problem introduced by it.

Signed-off-by: Alon Levy <alevy@redhat.com>

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

---

 hw/qxl-render.c    |   96 +++++++++++++++++++++++++++++++++++++---------------

 hw/qxl.c           |   69 ++++++++++++++++++++++++++++++++++---

 hw/qxl.h           |   10 ++++++

 ui/spice-display.h |    6 ++++

 4 files changed, 150 insertions(+), 31 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c

index 4c22166..4857838 100644

--- a/hw/qxl-render.c

+++ b/hw/qxl-render.c

@@ -82,17 +82,25 @@ void qxl_render_resize(PCIQXLDevice *qxl)

     }

 }

-void qxl_render_update(PCIQXLDevice *qxl)

+static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)

+{

+    area->left   = 0;

+    area->right  = qxl->guest_primary.surface.width;

+    area->top    = 0;

+    area->bottom = qxl->guest_primary.surface.height;

+}

+

+static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)

 {

     VGACommonState *vga = &qxl->vga;

-    QXLRect dirty[32], update;

-    int i, redraw = 0;

+    int i;

     DisplaySurface *surface = vga->ds->surface;

     if (qxl->guest_primary.resized) {

         qxl->guest_primary.resized = 0;

-

         qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram);

+        qxl_set_rect_to_surface(qxl, &qxl->dirty[0]);

+        qxl->num_dirty_rects = 1;

         dprint(qxl, 1, "%s: %dx%d, stride %d, bpp %d, depth %d\n",

                __FUNCTION__,

                qxl->guest_primary.surface.width,

@@ -103,9 +111,9 @@ void qxl_render_update(PCIQXLDevice *qxl)

     }

     if (surface->width != qxl->guest_primary.surface.width ||

         surface->height != qxl->guest_primary.surface.height) {

-        dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n",

-               __func__);

         if (qxl->guest_primary.qxl_stride > 0) {

+            dprint(qxl, 1, "%s: using guest_primary for displaysurface\n",

+                   __func__);

             qemu_free_displaysurface(vga->ds);

             qemu_create_displaysurface_from(qxl->guest_primary.surface.width,

                                             qxl->guest_primary.surface.height,

@@ -113,36 +121,70 @@ void qxl_render_update(PCIQXLDevice *qxl)

                                             qxl->guest_primary.abs_stride,

                                             qxl->guest_primary.data);

         } else {

+            dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n",

+                   __func__);

             qemu_resize_displaysurface(vga->ds,

                     qxl->guest_primary.surface.width,

                     qxl->guest_primary.surface.height);

         }

     }

-    update.left   = 0;

-    update.right  = qxl->guest_primary.surface.width;

-    update.top    = 0;

-    update.bottom = qxl->guest_primary.surface.height;

-

-    memset(dirty, 0, sizeof(dirty));

-    if (runstate_is_running() && qxl->guest_primary.commands) {

-        qxl->guest_primary.commands = 0;

-        qxl_spice_update_area(qxl, 0, &update,

-                              dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL);

-    }

-    if (redraw) {

-        memset(dirty, 0, sizeof(dirty));

-        dirty[0] = update;

-    }

-    for (i = 0; i < ARRAY_SIZE(dirty); i++) {

-        if (qemu_spice_rect_is_empty(dirty+i)) {

+    for (i = 0; i < qxl->num_dirty_rects; i++) {

+        if (qemu_spice_rect_is_empty(qxl->dirty+i)) {

             break;

         }

-        qxl_flip(qxl, dirty+i);

+        qxl_flip(qxl, qxl->dirty+i);

         dpy_update(vga->ds,

-                   dirty[i].left, dirty[i].top,

-                   dirty[i].right - dirty[i].left,

-                   dirty[i].bottom - dirty[i].top);

+                   qxl->dirty[i].left, qxl->dirty[i].top,

+                   qxl->dirty[i].right - qxl->dirty[i].left,

+                   qxl->dirty[i].bottom - qxl->dirty[i].top);

+    }

+    qxl->num_dirty_rects = 0;

+}

+

+/*

+ * use ssd.lock to protect render_update_cookie_num.

+ * qxl_render_update is called by io thread or vcpu thread, and the completion

+ * callbacks are called by spice_server thread, defering to bh called from the

+ * io thread.

+ */

+void qxl_render_update(PCIQXLDevice *qxl)

+{

+    QXLCookie *cookie;

+

+    qemu_mutex_lock(&qxl->ssd.lock);

+

+    if (!runstate_is_running() || !qxl->guest_primary.commands) {

+        qxl_render_update_area_unlocked(qxl);

+        qemu_mutex_unlock(&qxl->ssd.lock);

+        return;

     }

+

+    qxl->guest_primary.commands = 0;

+    qxl->render_update_cookie_num++;

+    qemu_mutex_unlock(&qxl->ssd.lock);

+    cookie = qxl_cookie_new(QXL_COOKIE_TYPE_RENDER_UPDATE_AREA,

+                            0);

+    qxl_set_rect_to_surface(qxl, &cookie->u.render.area);

+    qxl_spice_update_area(qxl, 0, &cookie->u.render.area, NULL,

+                          0, 1 /* clear_dirty_region */, QXL_ASYNC, cookie);

+}

+

+void qxl_render_update_area_bh(void *opaque)

+{

+    PCIQXLDevice *qxl = opaque;

+

+    qemu_mutex_lock(&qxl->ssd.lock);

+    qxl_render_update_area_unlocked(qxl);

+    qemu_mutex_unlock(&qxl->ssd.lock);

+}

+

+void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie)

+{

+    qemu_mutex_lock(&qxl->ssd.lock);

+    qemu_bh_schedule(qxl->update_area_bh);

+    qxl->render_update_cookie_num--;

+    qemu_mutex_unlock(&qxl->ssd.lock);

+    g_free(cookie);

 }

 static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)

diff --git a/hw/qxl.c b/hw/qxl.c

index e6e65d9..73be115 100644

--- a/hw/qxl.c

+++ b/hw/qxl.c

@@ -750,6 +750,11 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)

                 __func__, current_async, cookie->io);

     }

     switch (current_async) {

+    case QXL_IO_MEMSLOT_ADD_ASYNC:

+    case QXL_IO_DESTROY_PRIMARY_ASYNC:

+    case QXL_IO_UPDATE_AREA_ASYNC:

+    case QXL_IO_FLUSH_SURFACES_ASYNC:

+        break;

     case QXL_IO_CREATE_PRIMARY_ASYNC:

         qxl_create_guest_primary_complete(qxl);

         break;

@@ -759,11 +764,54 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)

     case QXL_IO_DESTROY_SURFACE_ASYNC:

         qxl_spice_destroy_surface_wait_complete(qxl, cookie->u.surface_id);

         break;

+    default:

+        fprintf(stderr, "qxl: %s: unexpected current_async %d\n", __func__,

+                current_async);

     }

     qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);

 }

 /* called from spice server thread context only */

+static void interface_update_area_complete(QXLInstance *sin,

+        uint32_t surface_id,

+        QXLRect *dirty, uint32_t num_updated_rects)

+{

+    PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);

+    int i;

+    int qxl_i;

+

+    qemu_mutex_lock(&qxl->ssd.lock);

+    if (surface_id != 0 || !qxl->render_update_cookie_num) {

+        qemu_mutex_unlock(&qxl->ssd.lock);

+        return;

+    }

+    if (qxl->num_dirty_rects + num_updated_rects > QXL_NUM_DIRTY_RECTS) {

+        /*

+         * overflow - treat this as a full update. Not expected to be common.

+         */

+        dprint(qxl, 1, "%s: overflow of dirty rects\n", __func__);

+        qxl->guest_primary.resized = 1;

+    }

+    if (qxl->guest_primary.resized) {

+        /*

+         * Don't bother copying or scheduling the bh since we will flip

+         * the whole area anyway on completion of the update_area async call

+         */

+        qemu_mutex_unlock(&qxl->ssd.lock);

+        return;

+    }

+    qxl_i = qxl->num_dirty_rects;

+    for (i = 0; i < num_updated_rects; i++) {

+        qxl->dirty[qxl_i++] = dirty[i];

+    }

+    qxl->num_dirty_rects += num_updated_rects;

+    dprint(qxl, 1, "%s: scheduling update_area_bh, #dirty %d\n",

+           __func__, qxl->num_dirty_rects);

+    qemu_bh_schedule(qxl->update_area_bh);

+    qemu_mutex_unlock(&qxl->ssd.lock);

+}

+

+/* called from spice server thread context only */

 static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)

 {

     PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);

@@ -772,12 +820,16 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)

     switch (cookie->type) {

     case QXL_COOKIE_TYPE_IO:

         interface_async_complete_io(qxl, cookie);

+        g_free(cookie);

+        break;

+    case QXL_COOKIE_TYPE_RENDER_UPDATE_AREA:

+        qxl_render_update_area_done(qxl, cookie);

         break;

     default:

         fprintf(stderr, "qxl: %s: unexpected cookie type %d\n",

                 __func__, cookie->type);

+        g_free(cookie);

     }

-    g_free(cookie);

 }

 static const QXLInterface qxl_interface = {

@@ -800,6 +852,7 @@ static const QXLInterface qxl_interface = {

     .notify_update           = interface_notify_update,

     .flush_resources         = interface_flush_resources,

     .async_complete          = interface_async_complete,

+    .update_area_complete    = interface_update_area_complete,

 };

 static void qxl_enter_vga_mode(PCIQXLDevice *d)

@@ -1216,11 +1269,17 @@ async_common:

     switch (io_port) {

     case QXL_IO_UPDATE_AREA:

     {

+        QXLCookie *cookie = NULL;

         QXLRect update = d->ram->update_area;

+

+        if (async == QXL_ASYNC) {

+            cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,

+                                    QXL_IO_UPDATE_AREA_ASYNC);

+            cookie->u.area = update;

+        }

         qxl_spice_update_area(d, d->ram->update_surface,

-                              &update, NULL, 0, 0, async,

-                              qxl_cookie_new(QXL_COOKIE_TYPE_IO,

-                                             QXL_IO_UPDATE_AREA_ASYNC));

+                              cookie ? &cookie->u.area : &update,

+                              NULL, 0, 0, async, cookie);

         break;

     }

     case QXL_IO_NOTIFY_CMD:

@@ -1652,6 +1711,8 @@ static int qxl_init_common(PCIQXLDevice *qxl)

     init_pipe_signaling(qxl);

     qxl_reset_state(qxl);

+    qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);

+

     return 0;

 }

diff --git a/hw/qxl.h b/hw/qxl.h

index 1443925..86e415b 100644

--- a/hw/qxl.h

+++ b/hw/qxl.h

@@ -18,6 +18,8 @@ enum qxl_mode {

 #define QXL_UNDEFINED_IO UINT32_MAX

+#define QXL_NUM_DIRTY_RECTS 64

+

 typedef struct PCIQXLDevice {

     PCIDevice          pci;

     SimpleSpiceDisplay ssd;

@@ -93,6 +95,12 @@ typedef struct PCIQXLDevice {

     /* user-friendly properties (in megabytes) */

     uint32_t          ram_size_mb;

     uint32_t          vram_size_mb;

+

+    /* qxl_render_update state */

+    int                render_update_cookie_num;

+    int                num_dirty_rects;

+    QXLRect            dirty[QXL_NUM_DIRTY_RECTS];

+    QEMUBH            *update_area_bh;

 } PCIQXLDevice;

 #define PANIC_ON(x) if ((x)) {                         \

@@ -134,3 +142,5 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);

 void qxl_render_resize(PCIQXLDevice *qxl);

 void qxl_render_update(PCIQXLDevice *qxl);

 void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);

+void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie);

+void qxl_render_update_area_bh(void *opaque);

diff --git a/ui/spice-display.h b/ui/spice-display.h

index 8a010cb..12e50b6 100644

--- a/ui/spice-display.h

+++ b/ui/spice-display.h

@@ -50,6 +50,7 @@ typedef enum qxl_async_io {

 enum {

     QXL_COOKIE_TYPE_IO,

+    QXL_COOKIE_TYPE_RENDER_UPDATE_AREA,

 };

 typedef struct QXLCookie {

@@ -57,6 +58,11 @@ typedef struct QXLCookie {

     uint64_t io;

     union {

         uint32_t surface_id;

+        QXLRect area;

+        struct {

+            QXLRect area;

+            int redraw;

+        } render;

     } u;

 } QXLCookie;

-- 

1.7.10