All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update
@ 2012-02-24 21:19 Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 1/7] qxl: fix spice+sdl no cursor regression Alon Levy
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Alon Levy @ 2012-02-24 21:19 UTC (permalink / raw)
  To: qemu-devel, kraxel

v5->v6:
 rebased
 dropped vga/console patches
 addressed checkpatch complaints
 
Alon Levy (7):
  qxl: fix spice+sdl no cursor regression
  sdl: remove NULL check, g_malloc0 can't fail
  qxl: drop qxl_spice_update_area_async definition
  qxl: require spice >= 0.8.2
  qxl: remove flipped
  qxl: introduce QXLCookie
  qxl: make qxl_render_update async

 configure          |    2 +-
 hw/qxl-render.c    |  156 ++++++++++++++++++++++++++++++-------------------
 hw/qxl.c           |  164 +++++++++++++++++++++++++++++++++++-----------------
 hw/qxl.h           |   24 ++++----
 ui/sdl.c           |    4 -
 ui/spice-core.c    |   17 ------
 ui/spice-display.c |   57 +++++++++++--------
 ui/spice-display.h |   21 +++++++
 8 files changed, 274 insertions(+), 171 deletions(-)

-- 
1.7.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 1/7] qxl: fix spice+sdl no cursor regression
  2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
@ 2012-02-24 21:19 ` Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 2/7] sdl: remove NULL check, g_malloc0 can't fail Alon Levy
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alon Levy @ 2012-02-24 21:19 UTC (permalink / raw)
  To: qemu-devel, kraxel

regression introduced by 075360945860ad9bdd491921954b383bf762b0e5,

v2: lock around qemu_spice_cursor_refresh_unlocked

Reported-by: Fabiano Fidêncio <fabiano@fidencio.org>
Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c           |    4 ++++
 ui/spice-display.c |   23 ++++++++++++++---------
 ui/spice-display.h |    1 +
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index f643667..0bbf0e8 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1545,6 +1545,10 @@ static void display_refresh(struct DisplayState *ds)
 {
     if (qxl0->mode == QXL_MODE_VGA) {
         qemu_spice_display_refresh(&qxl0->ssd);
+    } else {
+        qemu_mutex_lock(&qxl0->ssd.lock);
+        qemu_spice_cursor_refresh_unlocked(&qxl0->ssd);
+        qemu_mutex_unlock(&qxl0->ssd.lock);
     }
 }
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 6c302a3..c6e61d8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -317,16 +317,8 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
     ssd->notify++;
 }
 
-void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
+void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
 {
-    dprint(3, "%s:\n", __FUNCTION__);
-    vga_hw_update();
-
-    qemu_mutex_lock(&ssd->lock);
-    if (ssd->update == NULL) {
-        ssd->update = qemu_spice_create_update(ssd);
-        ssd->notify++;
-    }
     if (ssd->cursor) {
         ssd->ds->cursor_define(ssd->cursor);
         cursor_put(ssd->cursor);
@@ -337,6 +329,19 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
         ssd->mouse_x = -1;
         ssd->mouse_y = -1;
     }
+}
+
+void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
+{
+    dprint(3, "%s:\n", __func__);
+    vga_hw_update();
+
+    qemu_mutex_lock(&ssd->lock);
+    if (ssd->update == NULL) {
+        ssd->update = qemu_spice_create_update(ssd);
+        ssd->notify++;
+    }
+    qemu_spice_cursor_refresh_unlocked(ssd);
     qemu_mutex_unlock(&ssd->lock);
 
     if (ssd->notify) {
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 5e52df9..a23bfc8 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -97,6 +97,7 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
                                int x, int y, int w, int h);
 void qemu_spice_display_resize(SimpleSpiceDisplay *ssd);
 void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
+void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd);
 
 void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
                             qxl_async_io async);
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 2/7] sdl: remove NULL check, g_malloc0 can't fail
  2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 1/7] qxl: fix spice+sdl no cursor regression Alon Levy
@ 2012-02-24 21:19 ` Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 3/7] qxl: drop qxl_spice_update_area_async definition Alon Levy
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alon Levy @ 2012-02-24 21:19 UTC (permalink / raw)
  To: qemu-devel, kraxel

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 ui/sdl.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/ui/sdl.c b/ui/sdl.c
index 6f8091c..f6f711c 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -167,10 +167,6 @@ static PixelFormat sdl_to_qemu_pixelformat(SDL_PixelFormat *sdl_pf)
 static DisplaySurface* sdl_create_displaysurface(int width, int height)
 {
     DisplaySurface *surface = (DisplaySurface*) g_malloc0(sizeof(DisplaySurface));
-    if (surface == NULL) {
-        fprintf(stderr, "sdl_create_displaysurface: malloc failed\n");
-        exit(1);
-    }
 
     surface->width = width;
     surface->height = height;
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 3/7] qxl: drop qxl_spice_update_area_async definition
  2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 1/7] qxl: fix spice+sdl no cursor regression Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 2/7] sdl: remove NULL check, g_malloc0 can't fail Alon Levy
@ 2012-02-24 21:19 ` Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 4/7] qxl: require spice >= 0.8.2 Alon Levy
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alon Levy @ 2012-02-24 21:19 UTC (permalink / raw)
  To: qemu-devel, kraxel

It was never used. Introduced in
5ff4e36c804157bd84af43c139f8cd3a59722db9
qxl: async io support using new spice api

But not used even then.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.h |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/hw/qxl.h b/hw/qxl.h
index d062991..a615eca 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -138,9 +138,3 @@ 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);
-#if SPICE_INTERFACE_QXL_MINOR >= 1
-void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
-                                 struct QXLRect *area,
-                                 uint32_t clear_dirty_region,
-                                 int is_vga);
-#endif
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 4/7] qxl: require spice >= 0.8.2
  2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
                   ` (2 preceding siblings ...)
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 3/7] qxl: drop qxl_spice_update_area_async definition Alon Levy
@ 2012-02-24 21:19 ` Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 5/7] qxl: remove flipped Alon Levy
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alon Levy @ 2012-02-24 21:19 UTC (permalink / raw)
  To: qemu-devel, kraxel

drop all ifdefs on SPICE_INTERFACE_QXL_MINOR >= 1 as a result,
any check for SPICE_SERVER_VERSION that is now always satisfied,
and SPICE_INTERFACE_CORE_MINOR >= 3 tests, because
0.8.2 has SPICE_INTERFACE_QXL_MINOR == 1 and
SPICE_INTERFACE_CORE_MINOR == 3.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 configure          |    2 +-
 hw/qxl.c           |   40 ----------------------------------------
 hw/qxl.h           |    4 ----
 ui/spice-core.c    |   17 -----------------
 ui/spice-display.c |   12 ------------
 5 files changed, 1 insertions(+), 74 deletions(-)

diff --git a/configure b/configure
index f9d5330..9535f66 100755
--- a/configure
+++ b/configure
@@ -2547,7 +2547,7 @@ int main(void) { spice_server_new(); return 0; }
 EOF
   spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
   spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
-  if $pkg_config --atleast-version=0.6.0 spice-server >/dev/null 2>&1 && \
+  if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
      compile_prog "$spice_cflags" "$spice_libs" ; then
     spice="yes"
     libs_softmmu="$libs_softmmu $spice_libs"
diff --git a/hw/qxl.c b/hw/qxl.c
index 0bbf0e8..52df978 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -125,9 +125,7 @@ static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
 
 void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
 {
-#if SPICE_INTERFACE_QXL_MINOR >= 1
     qxl_send_events(qxl, QXL_INTERRUPT_ERROR);
-#endif
     if (qxl->guestdebug) {
         va_list ap;
         va_start(ap, msg);
@@ -149,12 +147,8 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
         qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area,
                         dirty_rects, num_dirty_rects, clear_dirty_region);
     } else {
-#if SPICE_INTERFACE_QXL_MINOR >= 1
         spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
                                     clear_dirty_region, 0);
-#else
-        abort();
-#endif
     }
 }
 
@@ -171,24 +165,18 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
                                            qxl_async_io async)
 {
     if (async) {
-#if SPICE_INTERFACE_QXL_MINOR < 1
-        abort();
-#else
         spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id,
                                         (uint64_t)id);
-#endif
     } else {
         qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
         qxl_spice_destroy_surface_wait_complete(qxl, id);
     }
 }
 
-#if SPICE_INTERFACE_QXL_MINOR >= 1
 static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
 {
     spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, 0);
 }
-#endif
 
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
                                uint32_t count)
@@ -217,11 +205,7 @@ static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl)
 static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
 {
     if (async) {
-#if SPICE_INTERFACE_QXL_MINOR < 1
-        abort();
-#else
         spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0);
-#endif
     } else {
         qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
         qxl_spice_destroy_surfaces_complete(qxl);
@@ -490,7 +474,6 @@ static const char *io_port_to_string(uint32_t io_port)
         [QXL_IO_DESTROY_PRIMARY]        = "QXL_IO_DESTROY_PRIMARY",
         [QXL_IO_DESTROY_SURFACE_WAIT]   = "QXL_IO_DESTROY_SURFACE_WAIT",
         [QXL_IO_DESTROY_ALL_SURFACES]   = "QXL_IO_DESTROY_ALL_SURFACES",
-#if SPICE_INTERFACE_QXL_MINOR >= 1
         [QXL_IO_UPDATE_AREA_ASYNC]      = "QXL_IO_UPDATE_AREA_ASYNC",
         [QXL_IO_MEMSLOT_ADD_ASYNC]      = "QXL_IO_MEMSLOT_ADD_ASYNC",
         [QXL_IO_CREATE_PRIMARY_ASYNC]   = "QXL_IO_CREATE_PRIMARY_ASYNC",
@@ -500,7 +483,6 @@ static const char *io_port_to_string(uint32_t io_port)
                                         = "QXL_IO_DESTROY_ALL_SURFACES_ASYNC",
         [QXL_IO_FLUSH_SURFACES_ASYNC]   = "QXL_IO_FLUSH_SURFACES_ASYNC",
         [QXL_IO_FLUSH_RELEASE]          = "QXL_IO_FLUSH_RELEASE",
-#endif
     };
     return io_port_to_string[io_port];
 }
@@ -735,8 +717,6 @@ static int interface_flush_resources(QXLInstance *sin)
 
 static void qxl_create_guest_primary_complete(PCIQXLDevice *d);
 
-#if SPICE_INTERFACE_QXL_MINOR >= 1
-
 /* called from spice server thread context only */
 static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
 {
@@ -764,8 +744,6 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
     qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
 }
 
-#endif
-
 static const QXLInterface qxl_interface = {
     .base.type               = SPICE_INTERFACE_QXL,
     .base.description        = "qxl gpu",
@@ -785,9 +763,7 @@ static const QXLInterface qxl_interface = {
     .req_cursor_notification = interface_req_cursor_notification,
     .notify_update           = interface_notify_update,
     .flush_resources         = interface_flush_resources,
-#if SPICE_INTERFACE_QXL_MINOR >= 1
     .async_complete          = interface_async_complete,
-#endif
 };
 
 static void qxl_enter_vga_mode(PCIQXLDevice *d)
@@ -1137,9 +1113,7 @@ static void ioport_write(void *opaque, target_phys_addr_t addr,
     PCIQXLDevice *d = opaque;
     uint32_t io_port = addr;
     qxl_async_io async = QXL_SYNC;
-#if SPICE_INTERFACE_QXL_MINOR >= 1
     uint32_t orig_io_port = io_port;
-#endif
 
     switch (io_port) {
     case QXL_IO_RESET:
@@ -1149,10 +1123,8 @@ static void ioport_write(void *opaque, target_phys_addr_t addr,
     case QXL_IO_CREATE_PRIMARY:
     case QXL_IO_UPDATE_IRQ:
     case QXL_IO_LOG:
-#if SPICE_INTERFACE_QXL_MINOR >= 1
     case QXL_IO_MEMSLOT_ADD_ASYNC:
     case QXL_IO_CREATE_PRIMARY_ASYNC:
-#endif
         break;
     default:
         if (d->mode != QXL_MODE_VGA) {
@@ -1160,17 +1132,14 @@ static void ioport_write(void *opaque, target_phys_addr_t addr,
         }
         dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
             __func__, io_port, io_port_to_string(io_port));
-#if SPICE_INTERFACE_QXL_MINOR >= 1
         /* be nice to buggy guest drivers */
         if (io_port >= QXL_IO_UPDATE_AREA_ASYNC &&
             io_port <= QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
             qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
         }
-#endif
         return;
     }
 
-#if SPICE_INTERFACE_QXL_MINOR >= 1
     /* we change the io_port to avoid ifdeffery in the main switch */
     orig_io_port = io_port;
     switch (io_port) {
@@ -1209,7 +1178,6 @@ async_common:
     default:
         break;
     }
-#endif
 
     switch (io_port) {
     case QXL_IO_UPDATE_AREA:
@@ -1301,7 +1269,6 @@ async_common:
         }
         qxl_spice_destroy_surface_wait(d, val, async);
         break;
-#if SPICE_INTERFACE_QXL_MINOR >= 1
     case QXL_IO_FLUSH_RELEASE: {
         QXLReleaseRing *ring = &d->ram->release_ring;
         if (ring->prod - ring->cons + 1 == ring->num_items) {
@@ -1322,7 +1289,6 @@ async_common:
                d->num_free_res);
         qxl_spice_flush_surfaces_async(d);
         break;
-#endif
     case QXL_IO_DESTROY_ALL_SURFACES:
         d->mode = QXL_MODE_UNDEFINED;
         qxl_spice_destroy_surfaces(d, async);
@@ -1333,16 +1299,12 @@ async_common:
     }
     return;
 cancel_async:
-#if SPICE_INTERFACE_QXL_MINOR >= 1
     if (async) {
         qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
         qemu_mutex_lock(&d->async_lock);
         d->current_async = QXL_UNDEFINED_IO;
         qemu_mutex_unlock(&d->async_lock);
     }
-#else
-    return;
-#endif
 }
 
 static uint64_t ioport_read(void *opaque, target_phys_addr_t addr,
@@ -1604,9 +1566,7 @@ static int qxl_init_common(PCIQXLDevice *qxl)
     case 2: /* spice 0.6 -- qxl-2 */
         pci_device_rev = QXL_REVISION_STABLE_V06;
         break;
-#if SPICE_INTERFACE_QXL_MINOR >= 1
     case 3: /* qxl-3 */
-#endif
     default:
         pci_device_rev = QXL_DEFAULT_REVISION;
         break;
diff --git a/hw/qxl.h b/hw/qxl.h
index a615eca..9288e46 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -108,11 +108,7 @@ typedef struct PCIQXLDevice {
         }                                                               \
     } while (0)
 
-#if SPICE_INTERFACE_QXL_MINOR >= 1
 #define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10
-#else
-#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V06
-#endif
 
 /* qxl.c */
 void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 1308a3d..30d2c62 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -139,8 +139,6 @@ static void watch_remove(SpiceWatch *watch)
     g_free(watch);
 }
 
-#if SPICE_INTERFACE_CORE_MINOR >= 3
-
 typedef struct ChannelList ChannelList;
 struct ChannelList {
     SpiceChannelEventInfo *info;
@@ -257,15 +255,6 @@ static void channel_event(int event, SpiceChannelEventInfo *info)
     }
 }
 
-#else /* SPICE_INTERFACE_CORE_MINOR >= 3 */
-
-static QList *channel_list_get(void)
-{
-    return NULL;
-}
-
-#endif /* SPICE_INTERFACE_CORE_MINOR >= 3 */
-
 static SpiceCoreInterface core_interface = {
     .base.type          = SPICE_INTERFACE_CORE,
     .base.description   = "qemu core services",
@@ -281,9 +270,7 @@ static SpiceCoreInterface core_interface = {
     .watch_update_mask  = watch_update_mask,
     .watch_remove       = watch_remove,
 
-#if SPICE_INTERFACE_CORE_MINOR >= 3
     .channel_event      = channel_event,
-#endif
 };
 
 #ifdef SPICE_INTERFACE_MIGRATION
@@ -490,7 +477,6 @@ static void migration_state_notifier(Notifier *notifier, void *data)
         spice_server_migrate_start(spice_server);
 #endif
     } else if (migration_has_finished(s)) {
-#if SPICE_SERVER_VERSION >= 0x000701 /* 0.7.1 */
 #ifndef SPICE_INTERFACE_MIGRATION
         spice_server_migrate_switch(spice_server);
 #else
@@ -498,7 +484,6 @@ static void migration_state_notifier(Notifier *notifier, void *data)
     } else if (migration_has_failed(s)) {
         spice_server_migrate_end(spice_server, false);
 #endif
-#endif
     }
 }
 
@@ -659,11 +644,9 @@ void qemu_spice_init(void)
         spice_server_set_noauth(spice_server);
     }
 
-#if SPICE_SERVER_VERSION >= 0x000801
     if (qemu_opt_get_bool(opts, "disable-copy-paste", 0)) {
         spice_server_set_agent_copypaste(spice_server, false);
     }
-#endif
 
     compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
     str = qemu_opt_get(opts, "image-compression");
diff --git a/ui/spice-display.c b/ui/spice-display.c
index c6e61d8..ad76bae 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -64,11 +64,7 @@ void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
                             qxl_async_io async)
 {
     if (async != QXL_SYNC) {
-#if SPICE_INTERFACE_QXL_MINOR >= 1
         spice_qxl_add_memslot_async(&ssd->qxl, memslot, 0);
-#else
-        abort();
-#endif
     } else {
         ssd->worker->add_memslot(ssd->worker, memslot);
     }
@@ -84,11 +80,7 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
                                        qxl_async_io async)
 {
     if (async != QXL_SYNC) {
-#if SPICE_INTERFACE_QXL_MINOR >= 1
         spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 0);
-#else
-        abort();
-#endif
     } else {
         ssd->worker->create_primary_surface(ssd->worker, id, surface);
     }
@@ -99,11 +91,7 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
                                         uint32_t id, qxl_async_io async)
 {
     if (async != QXL_SYNC) {
-#if SPICE_INTERFACE_QXL_MINOR >= 1
         spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 0);
-#else
-        abort();
-#endif
     } else {
         ssd->worker->destroy_primary_surface(ssd->worker, id);
     }
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 5/7] qxl: remove flipped
  2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
                   ` (3 preceding siblings ...)
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 4/7] qxl: require spice >= 0.8.2 Alon Levy
@ 2012-02-24 21:19 ` Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 6/7] qxl: introduce QXLCookie Alon Levy
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Alon Levy @ 2012-02-24 21:19 UTC (permalink / raw)
  To: qemu-devel, kraxel

Tested on linux and windows guests. For negative stride, qxl_flip copies
directly to vga->ds->surface->data, for positive it's reallocated to
share qxl->guest_primary.data

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl-render.c |   66 +++++++++++++++++++++++++------------------------------
 hw/qxl.h        |    2 +-
 2 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 7084143..c32d2fd 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -23,10 +23,21 @@
 
 static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect)
 {
-    uint8_t *src = qxl->guest_primary.data;
-    uint8_t *dst = qxl->guest_primary.flipped;
+    uint8_t *src;
+    uint8_t *dst = qxl->vga.ds->surface->data;
     int len, i;
 
+    if (qxl->guest_primary.qxl_stride > 0) {
+        return;
+    }
+    if (!qxl->guest_primary.data) {
+        dprint(qxl, 1, "%s: initializing guest_primary.data\n", __func__);
+        qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram);
+    }
+    dprint(qxl, 1, "%s: stride %d, [%d, %d, %d, %d]\n", __func__,
+            qxl->guest_primary.qxl_stride,
+            rect->left, rect->right, rect->top, rect->bottom);
+    src = qxl->guest_primary.data;
     src += (qxl->guest_primary.surface.height - rect->top - 1) *
         qxl->guest_primary.abs_stride;
     dst += rect->top  * qxl->guest_primary.abs_stride;
@@ -75,52 +86,38 @@ void qxl_render_update(PCIQXLDevice *qxl)
 {
     VGACommonState *vga = &qxl->vga;
     QXLRect dirty[32], update;
-    void *ptr;
     int i, redraw = 0;
-
-    if (!is_buffer_shared(vga->ds->surface)) {
-        dprint(qxl, 1, "%s: restoring shared displaysurface\n", __func__);
-        qxl->guest_primary.resized++;
-        qxl->guest_primary.commands++;
-        redraw = 1;
-    }
+    DisplaySurface *surface = vga->ds->surface;
 
     if (qxl->guest_primary.resized) {
         qxl->guest_primary.resized = 0;
 
-        if (qxl->guest_primary.flipped) {
-            g_free(qxl->guest_primary.flipped);
-            qxl->guest_primary.flipped = NULL;
-        }
-        qemu_free_displaysurface(vga->ds);
-
         qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram);
-        if (qxl->guest_primary.qxl_stride < 0) {
-            /* spice surface is upside down -> need extra buffer to flip */
-            qxl->guest_primary.flipped =
-                g_malloc(qxl->guest_primary.surface.width *
-                         qxl->guest_primary.abs_stride);
-            ptr = qxl->guest_primary.flipped;
-        } else {
-            ptr = qxl->guest_primary.data;
-        }
-        dprint(qxl, 1, "%s: %dx%d, stride %d, bpp %d, depth %d, flip %s\n",
+        dprint(qxl, 1, "%s: %dx%d, stride %d, bpp %d, depth %d\n",
                __FUNCTION__,
                qxl->guest_primary.surface.width,
                qxl->guest_primary.surface.height,
                qxl->guest_primary.qxl_stride,
                qxl->guest_primary.bytes_pp,
-               qxl->guest_primary.bits_pp,
-               qxl->guest_primary.flipped ? "yes" : "no");
-        vga->ds->surface =
+               qxl->guest_primary.bits_pp);
+    }
+    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) {
+            qemu_free_displaysurface(vga->ds);
             qemu_create_displaysurface_from(qxl->guest_primary.surface.width,
                                             qxl->guest_primary.surface.height,
                                             qxl->guest_primary.bits_pp,
                                             qxl->guest_primary.abs_stride,
-                                            ptr);
-        dpy_resize(vga->ds);
+                                            qxl->guest_primary.data);
+        } else {
+            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;
@@ -136,14 +133,11 @@ void qxl_render_update(PCIQXLDevice *qxl)
         memset(dirty, 0, sizeof(dirty));
         dirty[0] = update;
     }
-
     for (i = 0; i < ARRAY_SIZE(dirty); i++) {
         if (qemu_spice_rect_is_empty(dirty+i)) {
             break;
         }
-        if (qxl->guest_primary.flipped) {
-            qxl_flip(qxl, dirty+i);
-        }
+        qxl_flip(qxl, dirty+i);
         dpy_update(vga->ds,
                    dirty[i].left, dirty[i].top,
                    dirty[i].right - dirty[i].left,
diff --git a/hw/qxl.h b/hw/qxl.h
index 9288e46..53a3ace 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -52,7 +52,7 @@ typedef struct PCIQXLDevice {
         uint32_t       abs_stride;
         uint32_t       bits_pp;
         uint32_t       bytes_pp;
-        uint8_t        *data, *flipped;
+        uint8_t        *data;
     } guest_primary;
 
     struct surfaces {
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 6/7] qxl: introduce QXLCookie
  2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
                   ` (4 preceding siblings ...)
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 5/7] qxl: remove flipped Alon Levy
@ 2012-02-24 21:19 ` Alon Levy
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 7/7] qxl: make qxl_render_update async Alon Levy
  2012-02-27  9:14 ` [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Gerd Hoffmann
  7 siblings, 0 replies; 9+ messages in thread
From: Alon Levy @ 2012-02-24 21:19 UTC (permalink / raw)
  To: qemu-devel, kraxel

Will be used in the next patch.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl-render.c    |    2 +-
 hw/qxl.c           |   61 ++++++++++++++++++++++++++++++++++++++++------------
 hw/qxl.h           |    2 +-
 ui/spice-display.c |   22 ++++++++++++++++--
 ui/spice-display.h |   14 ++++++++++++
 5 files changed, 82 insertions(+), 19 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index c32d2fd..c45dc30 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -127,7 +127,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
     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);
+                              dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL);
     }
     if (redraw) {
         memset(dirty, 0, sizeof(dirty));
diff --git a/hw/qxl.c b/hw/qxl.c
index 52df978..3bc0e5c 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -141,14 +141,15 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            struct QXLRect *area, struct QXLRect *dirty_rects,
                            uint32_t num_dirty_rects,
                            uint32_t clear_dirty_region,
-                           qxl_async_io async)
+                           qxl_async_io async, struct QXLCookie *cookie)
 {
     if (async == QXL_SYNC) {
         qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area,
                         dirty_rects, num_dirty_rects, clear_dirty_region);
     } else {
+        assert(cookie != NULL);
         spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
-                                    clear_dirty_region, 0);
+                                    clear_dirty_region, (uint64_t)cookie);
     }
 }
 
@@ -164,9 +165,13 @@ static void qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl,
 static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
                                            qxl_async_io async)
 {
+    QXLCookie *cookie;
+
     if (async) {
-        spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id,
-                                        (uint64_t)id);
+        cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                QXL_IO_DESTROY_SURFACE_ASYNC);
+        cookie->u.surface_id = id;
+        spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id, (uint64_t)cookie);
     } else {
         qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
         qxl_spice_destroy_surface_wait_complete(qxl, id);
@@ -175,7 +180,9 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
 
 static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
 {
-    spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, 0);
+    spice_qxl_flush_surfaces_async(&qxl->ssd.qxl,
+        (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                 QXL_IO_FLUSH_SURFACES_ASYNC));
 }
 
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
@@ -205,7 +212,9 @@ static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl)
 static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
 {
     if (async) {
-        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0);
+        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_DESTROY_ALL_SURFACES_ASYNC));
     } else {
         qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
         qxl_spice_destroy_surfaces_complete(qxl);
@@ -718,9 +727,8 @@ static int interface_flush_resources(QXLInstance *sin)
 static void qxl_create_guest_primary_complete(PCIQXLDevice *d);
 
 /* called from spice server thread context only */
-static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
+static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
 {
-    PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
     uint32_t current_async;
 
     qemu_mutex_lock(&qxl->async_lock);
@@ -728,8 +736,16 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
     qxl->current_async = QXL_UNDEFINED_IO;
     qemu_mutex_unlock(&qxl->async_lock);
 
-    dprint(qxl, 2, "async_complete: %d (%" PRId64 ") done\n",
-           current_async, cookie);
+    dprint(qxl, 2, "async_complete: %d (%p) done\n", current_async, cookie);
+    if (!cookie) {
+        fprintf(stderr, "qxl: %s: error, cookie is NULL\n", __func__);
+        return;
+    }
+    if (cookie && current_async != cookie->io) {
+        fprintf(stderr,
+                "qxl: %s: error: current_async = %d != %ld = cookie->io\n",
+                __func__, current_async, cookie->io);
+    }
     switch (current_async) {
     case QXL_IO_CREATE_PRIMARY_ASYNC:
         qxl_create_guest_primary_complete(qxl);
@@ -738,12 +754,29 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
         qxl_spice_destroy_surfaces_complete(qxl);
         break;
     case QXL_IO_DESTROY_SURFACE_ASYNC:
-        qxl_spice_destroy_surface_wait_complete(qxl, (uint32_t)cookie);
+        qxl_spice_destroy_surface_wait_complete(qxl, cookie->u.surface_id);
         break;
     }
     qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
 }
 
+/* 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);
+    QXLCookie *cookie = (QXLCookie *)cookie_token;
+
+    switch (cookie->type) {
+    case QXL_COOKIE_TYPE_IO:
+        interface_async_complete_io(qxl, cookie);
+        break;
+    default:
+        fprintf(stderr, "qxl: %s: unexpected cookie type %d\n",
+                __func__, cookie->type);
+    }
+    g_free(cookie);
+}
+
 static const QXLInterface qxl_interface = {
     .base.type               = SPICE_INTERFACE_QXL,
     .base.description        = "qxl gpu",
@@ -1054,9 +1087,7 @@ static int qxl_destroy_primary(PCIQXLDevice *d, qxl_async_io async)
     if (d->mode == QXL_MODE_UNDEFINED) {
         return 0;
     }
-
     dprint(d, 1, "%s\n", __FUNCTION__);
-
     d->mode = QXL_MODE_UNDEFINED;
     qemu_spice_destroy_primary_surface(&d->ssd, 0, async);
     qxl_spice_reset_cursor(d);
@@ -1184,7 +1215,9 @@ async_common:
     {
         QXLRect update = d->ram->update_area;
         qxl_spice_update_area(d, d->ram->update_surface,
-                              &update, NULL, 0, 0, async);
+                              &update, NULL, 0, 0, async,
+                              qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                             QXL_IO_UPDATE_AREA_ASYNC));
         break;
     }
     case QXL_IO_NOTIFY_CMD:
diff --git a/hw/qxl.h b/hw/qxl.h
index 53a3ace..1443925 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -118,7 +118,7 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            struct QXLRect *area, struct QXLRect *dirty_rects,
                            uint32_t num_dirty_rects,
                            uint32_t clear_dirty_region,
-                           qxl_async_io async);
+                           qxl_async_io async, QXLCookie *cookie);
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
                                uint32_t count);
 void qxl_spice_oom(PCIQXLDevice *qxl);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index ad76bae..ab266ae 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -60,11 +60,23 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
     dest->right = MAX(dest->right, r->right);
 }
 
+QXLCookie *qxl_cookie_new(int type, uint64_t io)
+{
+    QXLCookie *cookie;
+
+    cookie = g_malloc0(sizeof(*cookie));
+    cookie->type = type;
+    cookie->io = io;
+    return cookie;
+}
+
 void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
                             qxl_async_io async)
 {
     if (async != QXL_SYNC) {
-        spice_qxl_add_memslot_async(&ssd->qxl, memslot, 0);
+        spice_qxl_add_memslot_async(&ssd->qxl, memslot,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_MEMSLOT_ADD_ASYNC));
     } else {
         ssd->worker->add_memslot(ssd->worker, memslot);
     }
@@ -80,7 +92,9 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
                                        qxl_async_io async)
 {
     if (async != QXL_SYNC) {
-        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 0);
+        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_CREATE_PRIMARY_ASYNC));
     } else {
         ssd->worker->create_primary_surface(ssd->worker, id, surface);
     }
@@ -91,7 +105,9 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
                                         uint32_t id, qxl_async_io async)
 {
     if (async != QXL_SYNC) {
-        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 0);
+        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_DESTROY_PRIMARY_ASYNC));
     } else {
         ssd->worker->destroy_primary_surface(ssd->worker, id);
     }
diff --git a/ui/spice-display.h b/ui/spice-display.h
index a23bfc8..8a010cb 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -48,6 +48,20 @@ typedef enum qxl_async_io {
     QXL_ASYNC,
 } qxl_async_io;
 
+enum {
+    QXL_COOKIE_TYPE_IO,
+};
+
+typedef struct QXLCookie {
+    int      type;
+    uint64_t io;
+    union {
+        uint32_t surface_id;
+    } u;
+} QXLCookie;
+
+QXLCookie *qxl_cookie_new(int type, uint64_t io);
+
 typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
 typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
 
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 7/7] qxl: make qxl_render_update async
  2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
                   ` (5 preceding siblings ...)
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 6/7] qxl: introduce QXLCookie Alon Levy
@ 2012-02-24 21:19 ` Alon Levy
  2012-02-27  9:14 ` [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Gerd Hoffmann
  7 siblings, 0 replies; 9+ messages in thread
From: Alon Levy @ 2012-02-24 21:19 UTC (permalink / raw)
  To: qemu-devel, kraxel

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>
---
 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 c45dc30..f323a4d 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 3bc0e5c..d83d245 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -747,6 +747,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;
@@ -756,11 +761,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);
@@ -769,12 +817,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 = {
@@ -797,6 +849,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)
@@ -1213,11 +1266,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:
@@ -1649,6 +1708,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.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update
  2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
                   ` (6 preceding siblings ...)
  2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 7/7] qxl: make qxl_render_update async Alon Levy
@ 2012-02-27  9:14 ` Gerd Hoffmann
  7 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2012-02-27  9:14 UTC (permalink / raw)
  To: Alon Levy; +Cc: qemu-devel

On 02/24/12 22:19, Alon Levy wrote:
> v5->v6:
>  rebased
>  dropped vga/console patches
>  addressed checkpatch complaints
>  
> Alon Levy (7):
>   qxl: fix spice+sdl no cursor regression
>   sdl: remove NULL check, g_malloc0 can't fail
>   qxl: drop qxl_spice_update_area_async definition
>   qxl: require spice >= 0.8.2
>   qxl: remove flipped
>   qxl: introduce QXLCookie
>   qxl: make qxl_render_update async

Added to spice patch queue.

thanks,
  Gerd

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-02-27  9:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-24 21:19 [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Alon Levy
2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 1/7] qxl: fix spice+sdl no cursor regression Alon Levy
2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 2/7] sdl: remove NULL check, g_malloc0 can't fail Alon Levy
2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 3/7] qxl: drop qxl_spice_update_area_async definition Alon Levy
2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 4/7] qxl: require spice >= 0.8.2 Alon Levy
2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 5/7] qxl: remove flipped Alon Levy
2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 6/7] qxl: introduce QXLCookie Alon Levy
2012-02-24 21:19 ` [Qemu-devel] [PATCH v6 7/7] qxl: make qxl_render_update async Alon Levy
2012-02-27  9:14 ` [Qemu-devel] [PATCH v6 0/7] qxl: fix hangs caused by qxl_render_update Gerd Hoffmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.