qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie
@ 2012-02-16 23:11 Alon Levy
  2012-02-16 23:11 ` [Qemu-devel] [PATCH 2/2] qxl: make qxl_render_update async Alon Levy
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Alon Levy @ 2012-02-16 23:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, elmarco

Will be used in the next patch.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl-render.c    |    2 +-
 hw/qxl.c           |   63 +++++++++++++++++++++++++++++++++++++++------------
 hw/qxl.h           |    2 +-
 ui/spice-display.c |   24 +++++++++++++++++--
 ui/spice-display.h |   10 ++++++++
 5 files changed, 81 insertions(+), 20 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 133d093..b238b96 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -133,7 +133,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
 
     memset(dirty, 0, sizeof(dirty));
     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));
         dirty[0] = update;
diff --git a/hw/qxl.c b/hw/qxl.c
index ac69125..59d2b13 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -143,15 +143,21 @@ 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)
 {
     if (async == QXL_SYNC) {
         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
+        if (cookie == NULL) {
+            cookie = g_malloc0(sizeof(*cookie));
+            cookie->type = QXL_COOKIE_TYPE_IO;
+            cookie->io = QXL_IO_UPDATE_AREA_ASYNC;
+            cookie->data = 0;
+        }
         spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
-                                    clear_dirty_region, 0);
+                                    clear_dirty_region, (uint64_t)cookie);
 #else
         abort();
 #endif
@@ -170,12 +176,17 @@ 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) {
-#if SPICE_INTERFACE_QXL_MINOR < 1
-        abort();
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        cookie = g_malloc0(sizeof(*cookie));
+        cookie->type = QXL_COOKIE_TYPE_IO;
+        cookie->io = QXL_IO_DESTROY_SURFACE_ASYNC;
+        cookie->data = id;
+        spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id, (uint64_t)cookie);
 #else
-        spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id,
-                                        (uint64_t)id);
+        abort();
 #endif
     } else {
         qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
@@ -216,11 +227,17 @@ static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl)
 
 static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
 {
+    QXLCookie *cookie;
+
     if (async) {
 #if SPICE_INTERFACE_QXL_MINOR < 1
         abort();
 #else
-        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0);
+        cookie = g_malloc0(sizeof(*cookie));
+        cookie->type = QXL_COOKIE_TYPE_IO;
+        cookie->io = QXL_IO_DESTROY_ALL_SURFACES;
+        cookie->data = 0;
+        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, (uint64_t)cookie);
 #endif
     } else {
         qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
@@ -737,18 +754,20 @@ 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)
+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);
     current_async = qxl->current_async;
     qxl->current_async = QXL_UNDEFINED_IO;
     qemu_mutex_unlock(&qxl->async_lock);
+    dprint(qxl, 2, "async_complete: %d (%p) done\n", current_async, cookie);
+    if (current_async != cookie->io) {
+        fprintf(stderr, "qxl: %s: error: current_async = %d != %ld = cookie->io\n",
+                __func__, current_async, cookie->io);
+    }
 
-    dprint(qxl, 2, "async_complete: %d (%ld) done\n", current_async, cookie);
     switch (current_async) {
     case QXL_IO_CREATE_PRIMARY_ASYNC:
         qxl_create_guest_primary_complete(qxl);
@@ -757,12 +776,28 @@ 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, (uint32_t)cookie->data);
         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);
+}
+
 #endif
 
 static const QXLInterface qxl_interface = {
@@ -1077,9 +1112,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);
@@ -1215,7 +1248,7 @@ 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, NULL);
         break;
     }
     case QXL_IO_NOTIFY_CMD:
diff --git a/hw/qxl.h b/hw/qxl.h
index 766aa6d..666dd78 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 6c302a3..7f3f629 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -63,9 +63,15 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
 void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
                             qxl_async_io async)
 {
+    QXLCookie *cookie = g_malloc0(sizeof(*cookie));
+
     if (async != QXL_SYNC) {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-        spice_qxl_add_memslot_async(&ssd->qxl, memslot, 0);
+        cookie = g_malloc0(sizeof(*cookie));
+        cookie->type = QXL_COOKIE_TYPE_IO;
+        cookie->io = QXL_IO_MEMSLOT_ADD_ASYNC;
+        cookie->data = 0;
+        spice_qxl_add_memslot_async(&ssd->qxl, memslot, (uint64_t)cookie);
 #else
         abort();
 #endif
@@ -83,9 +89,15 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
                                        QXLDevSurfaceCreate *surface,
                                        qxl_async_io async)
 {
+    QXLCookie *cookie;
+
     if (async != QXL_SYNC) {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 0);
+        cookie = g_malloc0(sizeof(*cookie));
+        cookie->type = QXL_COOKIE_TYPE_IO;
+        cookie->io = QXL_IO_CREATE_PRIMARY_ASYNC;
+        cookie->data = 0;
+        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, (uint64_t)cookie);
 #else
         abort();
 #endif
@@ -98,9 +110,15 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
 void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
                                         uint32_t id, qxl_async_io async)
 {
+    QXLCookie *cookie;
+
     if (async != QXL_SYNC) {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 0);
+        cookie = g_malloc0(sizeof(*cookie));
+        cookie->type = QXL_COOKIE_TYPE_IO;
+        cookie->io = QXL_IO_DESTROY_PRIMARY_ASYNC;
+        cookie->data = 0;
+        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, (uint64_t)cookie);
 #else
         abort();
 #endif
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 5e52df9..63cb009 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -48,6 +48,16 @@ typedef enum qxl_async_io {
     QXL_ASYNC,
 } qxl_async_io;
 
+enum {
+    QXL_COOKIE_TYPE_IO,
+};
+
+typedef struct QXLCookie {
+    int      type;
+    uint64_t io;
+    uint64_t data;
+} QXLCookie;
+
 typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
 typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
 
-- 
1.7.9

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

* [Qemu-devel] [PATCH 2/2] qxl: make qxl_render_update async
  2012-02-16 23:11 [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie Alon Levy
@ 2012-02-16 23:11 ` Alon Levy
  2012-02-17  8:39 ` [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie Alon Levy
  2012-02-17  8:44 ` [Qemu-devel] [PATCH v2 " Alon Levy
  2 siblings, 0 replies; 12+ messages in thread
From: Alon Levy @ 2012-02-16 23:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, elmarco

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.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl-render.c    |   44 +++++++++++++++++++++++++++++++++-----------
 hw/qxl.c           |   19 ++++++++++++++++++-
 hw/qxl.h           |    2 ++
 ui/spice-display.h |    1 +
 4 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index b238b96..980f8e4 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -71,12 +71,20 @@ void qxl_render_resize(PCIQXLDevice *qxl)
     }
 }
 
+typedef struct QXLRenderUpdateData {
+    int redraw;
+    QXLRect dirty[32];
+    QXLRect area;
+} QXLRenderUpdateData;
+
 void qxl_render_update(PCIQXLDevice *qxl)
 {
     VGACommonState *vga = &qxl->vga;
-    QXLRect dirty[32], update;
+    QXLRect dirty[32];
     void *ptr;
-    int i, redraw = 0;
+    int redraw = 0;
+    QXLCookie *cookie;
+    QXLRenderUpdateData *data;
 
     if (!is_buffer_shared(vga->ds->surface)) {
         dprint(qxl, 1, "%s: restoring shared displaysurface\n", __func__);
@@ -126,20 +134,33 @@ void qxl_render_update(PCIQXLDevice *qxl)
     }
     qxl->guest_primary.commands = 0;
 
-    update.left   = 0;
-    update.right  = qxl->guest_primary.surface.width;
-    update.top    = 0;
-    update.bottom = qxl->guest_primary.surface.height;
+    cookie = g_malloc0(sizeof(*cookie));
+    cookie->type = QXL_COOKIE_TYPE_RENDER_UPDATE_AREA;
+    data = g_malloc0(sizeof(*data));
+    cookie->data = (uint64_t)data;
+    data->redraw = redraw;
+    data->area.left   = 0;
+    data->area.right  = qxl->guest_primary.surface.width;
+    data->area.top    = 0;
+    data->area.bottom = qxl->guest_primary.surface.height;
+    qxl_spice_update_area(qxl, 0, &data->area,
+                          data->dirty, ARRAY_SIZE(dirty), 1, QXL_ASYNC, cookie);
+}
+
+void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie)
+{
+    QXLRenderUpdateData *data = (QXLRenderUpdateData *)cookie->data;
+    QXLRect update = data->area;
+    QXLRect *dirty = data->dirty;
+    VGACommonState *vga = &qxl->vga;
+    int i;
 
-    memset(dirty, 0, sizeof(dirty));
-    qxl_spice_update_area(qxl, 0, &update,
-                          dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL);
-    if (redraw) {
+    if (data->redraw) {
         memset(dirty, 0, sizeof(dirty));
         dirty[0] = update;
     }
 
-    for (i = 0; i < ARRAY_SIZE(dirty); i++) {
+    for (i = 0; i < ARRAY_SIZE(data->dirty); i++) {
         if (qemu_spice_rect_is_empty(dirty+i)) {
             break;
         }
@@ -151,6 +172,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
                    dirty[i].right - dirty[i].left,
                    dirty[i].bottom - dirty[i].top);
     }
+    g_free(data);
 }
 
 static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
diff --git a/hw/qxl.c b/hw/qxl.c
index 59d2b13..4c28fea 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -145,6 +145,7 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            uint32_t clear_dirty_region,
                            qxl_async_io async, QXLCookie *cookie)
 {
+    struct QXLRect *area_copy;
     if (async == QXL_SYNC) {
         qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area,
                         dirty_rects, num_dirty_rects, clear_dirty_region);
@@ -154,7 +155,10 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
             cookie = g_malloc0(sizeof(*cookie));
             cookie->type = QXL_COOKIE_TYPE_IO;
             cookie->io = QXL_IO_UPDATE_AREA_ASYNC;
-            cookie->data = 0;
+            area_copy = g_malloc0(sizeof(*area_copy));
+            memcpy(area_copy, area, sizeof(*area));
+            cookie->data = (uint64_t)area_copy;
+            area = area_copy;
         }
         spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
                                     clear_dirty_region, (uint64_t)cookie);
@@ -769,6 +773,10 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
     }
 
     switch (current_async) {
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
+    case QXL_IO_DESTROY_PRIMARY_ASYNC:
+    case QXL_IO_FLUSH_SURFACES_ASYNC:
+        break;
     case QXL_IO_CREATE_PRIMARY_ASYNC:
         qxl_create_guest_primary_complete(qxl);
         break;
@@ -778,6 +786,12 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
     case QXL_IO_DESTROY_SURFACE_ASYNC:
         qxl_spice_destroy_surface_wait_complete(qxl, (uint32_t)cookie->data);
         break;
+    case QXL_IO_UPDATE_AREA_ASYNC:
+        g_free((void *)cookie->data);
+        break;
+    default:
+        fprintf(stderr, "qxl: %s: unexpected current_async %d\n", __func__,
+                current_async);
     }
     qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
 }
@@ -792,6 +806,9 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)
     case QXL_COOKIE_TYPE_IO:
         interface_async_complete_io(qxl, 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);
     }
diff --git a/hw/qxl.h b/hw/qxl.h
index 666dd78..1977ff3 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -140,3 +140,5 @@ void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
                                  uint32_t clear_dirty_region,
                                  int is_vga);
 #endif
+
+void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie);
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 63cb009..263c3e4 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 {
-- 
1.7.9

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

* Re: [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie
  2012-02-16 23:11 [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie Alon Levy
  2012-02-16 23:11 ` [Qemu-devel] [PATCH 2/2] qxl: make qxl_render_update async Alon Levy
@ 2012-02-17  8:39 ` Alon Levy
  2012-02-17  8:44 ` [Qemu-devel] [PATCH v2 " Alon Levy
  2 siblings, 0 replies; 12+ messages in thread
From: Alon Levy @ 2012-02-17  8:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, elmarco

On Fri, Feb 17, 2012 at 01:11:27AM +0200, Alon Levy wrote:
> Will be used in the next patch.

Self NACK, some errors in this patchset, sending v2 with cleanup
(qxl_cookie_new to reduce the duplicate code).

> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  hw/qxl-render.c    |    2 +-
>  hw/qxl.c           |   63 +++++++++++++++++++++++++++++++++++++++------------
>  hw/qxl.h           |    2 +-
>  ui/spice-display.c |   24 +++++++++++++++++--
>  ui/spice-display.h |   10 ++++++++
>  5 files changed, 81 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/qxl-render.c b/hw/qxl-render.c
> index 133d093..b238b96 100644
> --- a/hw/qxl-render.c
> +++ b/hw/qxl-render.c
> @@ -133,7 +133,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
>  
>      memset(dirty, 0, sizeof(dirty));
>      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));
>          dirty[0] = update;
> diff --git a/hw/qxl.c b/hw/qxl.c
> index ac69125..59d2b13 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -143,15 +143,21 @@ 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)
>  {
>      if (async == QXL_SYNC) {
>          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
> +        if (cookie == NULL) {
> +            cookie = g_malloc0(sizeof(*cookie));
> +            cookie->type = QXL_COOKIE_TYPE_IO;
> +            cookie->io = QXL_IO_UPDATE_AREA_ASYNC;
> +            cookie->data = 0;
> +        }
>          spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
> -                                    clear_dirty_region, 0);
> +                                    clear_dirty_region, (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> @@ -170,12 +176,17 @@ 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) {
> -#if SPICE_INTERFACE_QXL_MINOR < 1
> -        abort();
> +#if SPICE_INTERFACE_QXL_MINOR >= 1
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_DESTROY_SURFACE_ASYNC;
> +        cookie->data = id;
> +        spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id, (uint64_t)cookie);
>  #else
> -        spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id,
> -                                        (uint64_t)id);
> +        abort();
>  #endif
>      } else {
>          qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
> @@ -216,11 +227,17 @@ static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl)
>  
>  static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
>  {
> +    QXLCookie *cookie;
> +
>      if (async) {
>  #if SPICE_INTERFACE_QXL_MINOR < 1
>          abort();
>  #else
> -        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0);
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_DESTROY_ALL_SURFACES;
> +        cookie->data = 0;
> +        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, (uint64_t)cookie);
>  #endif
>      } else {
>          qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
> @@ -737,18 +754,20 @@ 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)
> +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);
>      current_async = qxl->current_async;
>      qxl->current_async = QXL_UNDEFINED_IO;
>      qemu_mutex_unlock(&qxl->async_lock);
> +    dprint(qxl, 2, "async_complete: %d (%p) done\n", current_async, cookie);
> +    if (current_async != cookie->io) {
> +        fprintf(stderr, "qxl: %s: error: current_async = %d != %ld = cookie->io\n",
> +                __func__, current_async, cookie->io);
> +    }
>  
> -    dprint(qxl, 2, "async_complete: %d (%ld) done\n", current_async, cookie);
>      switch (current_async) {
>      case QXL_IO_CREATE_PRIMARY_ASYNC:
>          qxl_create_guest_primary_complete(qxl);
> @@ -757,12 +776,28 @@ 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, (uint32_t)cookie->data);
>          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);
> +}
> +
>  #endif
>  
>  static const QXLInterface qxl_interface = {
> @@ -1077,9 +1112,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);
> @@ -1215,7 +1248,7 @@ 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, NULL);
>          break;
>      }
>      case QXL_IO_NOTIFY_CMD:
> diff --git a/hw/qxl.h b/hw/qxl.h
> index 766aa6d..666dd78 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 6c302a3..7f3f629 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -63,9 +63,15 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
>  void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
>                              qxl_async_io async)
>  {
> +    QXLCookie *cookie = g_malloc0(sizeof(*cookie));
> +
>      if (async != QXL_SYNC) {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> -        spice_qxl_add_memslot_async(&ssd->qxl, memslot, 0);
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_MEMSLOT_ADD_ASYNC;
> +        cookie->data = 0;
> +        spice_qxl_add_memslot_async(&ssd->qxl, memslot, (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> @@ -83,9 +89,15 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
>                                         QXLDevSurfaceCreate *surface,
>                                         qxl_async_io async)
>  {
> +    QXLCookie *cookie;
> +
>      if (async != QXL_SYNC) {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> -        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 0);
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_CREATE_PRIMARY_ASYNC;
> +        cookie->data = 0;
> +        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> @@ -98,9 +110,15 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
>  void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
>                                          uint32_t id, qxl_async_io async)
>  {
> +    QXLCookie *cookie;
> +
>      if (async != QXL_SYNC) {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> -        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 0);
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_DESTROY_PRIMARY_ASYNC;
> +        cookie->data = 0;
> +        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> diff --git a/ui/spice-display.h b/ui/spice-display.h
> index 5e52df9..63cb009 100644
> --- a/ui/spice-display.h
> +++ b/ui/spice-display.h
> @@ -48,6 +48,16 @@ typedef enum qxl_async_io {
>      QXL_ASYNC,
>  } qxl_async_io;
>  
> +enum {
> +    QXL_COOKIE_TYPE_IO,
> +};
> +
> +typedef struct QXLCookie {
> +    int      type;
> +    uint64_t io;
> +    uint64_t data;
> +} QXLCookie;
> +
>  typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
>  typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
>  
> -- 
> 1.7.9
> 
> 

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

* [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie
  2012-02-16 23:11 [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie Alon Levy
  2012-02-16 23:11 ` [Qemu-devel] [PATCH 2/2] qxl: make qxl_render_update async Alon Levy
  2012-02-17  8:39 ` [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie Alon Levy
@ 2012-02-17  8:44 ` Alon Levy
  2012-02-17  8:45   ` [Qemu-devel] [PATCH v2 2/2] qxl: make qxl_render_update async Alon Levy
                     ` (4 more replies)
  2 siblings, 5 replies; 12+ messages in thread
From: Alon Levy @ 2012-02-17  8:44 UTC (permalink / raw)
  To: qemu-devel, kraxel, elmarco

Will be used in the next patch.

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

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 133d093..b238b96 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -133,7 +133,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
 
     memset(dirty, 0, sizeof(dirty));
     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));
         dirty[0] = update;
diff --git a/hw/qxl.c b/hw/qxl.c
index ac69125..02708e3 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -143,15 +143,20 @@ 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)
 {
     if (async == QXL_SYNC) {
         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
+        if (cookie == NULL) {
+            cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                    QXL_IO_UPDATE_AREA_ASYNC,
+                                    0);
+        }
         spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
-                                    clear_dirty_region, 0);
+                                    clear_dirty_region, (uint64_t)cookie);
 #else
         abort();
 #endif
@@ -171,11 +176,13 @@ 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
+#if SPICE_INTERFACE_QXL_MINOR >= 1
         spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id,
-                                        (uint64_t)id);
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_DESTROY_SURFACE_ASYNC,
+                                         id));
+#else
+        abort();
 #endif
     } else {
         qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
@@ -217,10 +224,13 @@ 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();
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_DESTROY_ALL_SURFACES,
+                                         0));
 #else
-        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0);
+        abort();
 #endif
     } else {
         qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
@@ -737,18 +747,20 @@ 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)
+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);
     current_async = qxl->current_async;
     qxl->current_async = QXL_UNDEFINED_IO;
     qemu_mutex_unlock(&qxl->async_lock);
+    dprint(qxl, 2, "async_complete: %d (%p) done\n", current_async, cookie);
+    if (current_async != cookie->io) {
+        fprintf(stderr, "qxl: %s: error: current_async = %d != %ld = cookie->io\n",
+                __func__, current_async, cookie->io);
+    }
 
-    dprint(qxl, 2, "async_complete: %d (%ld) done\n", current_async, cookie);
     switch (current_async) {
     case QXL_IO_CREATE_PRIMARY_ASYNC:
         qxl_create_guest_primary_complete(qxl);
@@ -757,12 +769,28 @@ 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, (uint32_t)cookie->data);
         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);
+}
+
 #endif
 
 static const QXLInterface qxl_interface = {
@@ -1077,9 +1105,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);
@@ -1215,7 +1241,7 @@ 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, NULL);
         break;
     }
     case QXL_IO_NOTIFY_CMD:
diff --git a/hw/qxl.h b/hw/qxl.h
index 766aa6d..666dd78 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 6c302a3..680e6f4 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -60,12 +60,26 @@ 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, uint64_t data)
+{
+    QXLCookie *cookie;
+
+    cookie = g_malloc0(sizeof(*cookie));
+    cookie->type = type;
+    cookie->io = io;
+    cookie->data = data;
+    return cookie;
+}
+
 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);
+        spice_qxl_add_memslot_async(&ssd->qxl, memslot,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_MEMSLOT_ADD_ASYNC,
+                                         0));
 #else
         abort();
 #endif
@@ -85,7 +99,10 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
 {
     if (async != QXL_SYNC) {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-        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,
+                                         0));
 #else
         abort();
 #endif
@@ -100,7 +117,10 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
 {
     if (async != QXL_SYNC) {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-        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,
+                                         0));
 #else
         abort();
 #endif
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 5e52df9..c8747ab 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -48,6 +48,18 @@ typedef enum qxl_async_io {
     QXL_ASYNC,
 } qxl_async_io;
 
+enum {
+    QXL_COOKIE_TYPE_IO,
+};
+
+typedef struct QXLCookie {
+    int      type;
+    uint64_t io;
+    uint64_t data;
+} QXLCookie;
+
+QXLCookie *qxl_cookie_new(int type, uint64_t io, uint64_t data);
+
 typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
 typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
 
-- 
1.7.9

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

* [Qemu-devel] [PATCH v2 2/2] qxl: make qxl_render_update async
  2012-02-17  8:44 ` [Qemu-devel] [PATCH v2 " Alon Levy
@ 2012-02-17  8:45   ` Alon Levy
  2012-02-17 12:49   ` [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie Marc-André Lureau
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Alon Levy @ 2012-02-17  8:45 UTC (permalink / raw)
  To: qemu-devel, kraxel, elmarco

RHBZ# 794658

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, allocated and copied before passing, deallocated on
the interface_async_complete callback.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl-render.c    |   43 ++++++++++++++++++++++++++++++++-----------
 hw/qxl.c           |   19 ++++++++++++++++++-
 hw/qxl.h           |    2 ++
 ui/spice-display.h |    1 +
 4 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index b238b96..7f9fbca 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -71,12 +71,19 @@ void qxl_render_resize(PCIQXLDevice *qxl)
     }
 }
 
+typedef struct QXLRenderUpdateData {
+    int redraw;
+    QXLRect dirty[32];
+    QXLRect area;
+} QXLRenderUpdateData;
+
 void qxl_render_update(PCIQXLDevice *qxl)
 {
     VGACommonState *vga = &qxl->vga;
-    QXLRect dirty[32], update;
+    QXLRect dirty[32];
     void *ptr;
-    int i, redraw = 0;
+    int redraw = 0;
+    QXLRenderUpdateData *data;
 
     if (!is_buffer_shared(vga->ds->surface)) {
         dprint(qxl, 1, "%s: restoring shared displaysurface\n", __func__);
@@ -126,20 +133,33 @@ void qxl_render_update(PCIQXLDevice *qxl)
     }
     qxl->guest_primary.commands = 0;
 
-    update.left   = 0;
-    update.right  = qxl->guest_primary.surface.width;
-    update.top    = 0;
-    update.bottom = qxl->guest_primary.surface.height;
+    data = g_malloc0(sizeof(*data));
+    data->redraw = redraw;
+    data->area.left   = 0;
+    data->area.right  = qxl->guest_primary.surface.width;
+    data->area.top    = 0;
+    data->area.bottom = qxl->guest_primary.surface.height;
+    qxl_spice_update_area(qxl, 0, &data->area,
+                          data->dirty, ARRAY_SIZE(dirty), 1, QXL_ASYNC,
+                          qxl_cookie_new(QXL_COOKIE_TYPE_RENDER_UPDATE_AREA,
+                                         0,
+                                         (uint64_t)data));
+}
+
+void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie)
+{
+    QXLRenderUpdateData *data = (QXLRenderUpdateData *)cookie->data;
+    QXLRect update = data->area;
+    QXLRect *dirty = data->dirty;
+    VGACommonState *vga = &qxl->vga;
+    int i;
 
-    memset(dirty, 0, sizeof(dirty));
-    qxl_spice_update_area(qxl, 0, &update,
-                          dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL);
-    if (redraw) {
+    if (data->redraw) {
         memset(dirty, 0, sizeof(dirty));
         dirty[0] = update;
     }
 
-    for (i = 0; i < ARRAY_SIZE(dirty); i++) {
+    for (i = 0; i < ARRAY_SIZE(data->dirty); i++) {
         if (qemu_spice_rect_is_empty(dirty+i)) {
             break;
         }
@@ -151,6 +171,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
                    dirty[i].right - dirty[i].left,
                    dirty[i].bottom - dirty[i].top);
     }
+    g_free(data);
 }
 
 static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
diff --git a/hw/qxl.c b/hw/qxl.c
index 02708e3..3eaf2d6 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -145,15 +145,19 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            uint32_t clear_dirty_region,
                            qxl_async_io async, QXLCookie *cookie)
 {
+    struct QXLRect *area_copy;
     if (async == QXL_SYNC) {
         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
         if (cookie == NULL) {
+            area_copy = g_malloc0(sizeof(*area_copy));
+            memcpy(area_copy, area, sizeof(*area));
+            area = area_copy;
             cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
                                     QXL_IO_UPDATE_AREA_ASYNC,
-                                    0);
+                                    (uint64_t)area_copy);
         }
         spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
                                     clear_dirty_region, (uint64_t)cookie);
@@ -762,6 +766,10 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
     }
 
     switch (current_async) {
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
+    case QXL_IO_DESTROY_PRIMARY_ASYNC:
+    case QXL_IO_FLUSH_SURFACES_ASYNC:
+        break;
     case QXL_IO_CREATE_PRIMARY_ASYNC:
         qxl_create_guest_primary_complete(qxl);
         break;
@@ -771,6 +779,12 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
     case QXL_IO_DESTROY_SURFACE_ASYNC:
         qxl_spice_destroy_surface_wait_complete(qxl, (uint32_t)cookie->data);
         break;
+    case QXL_IO_UPDATE_AREA_ASYNC:
+        g_free((void *)cookie->data);
+        break;
+    default:
+        fprintf(stderr, "qxl: %s: unexpected current_async %d\n", __func__,
+                current_async);
     }
     qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
 }
@@ -785,6 +799,9 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)
     case QXL_COOKIE_TYPE_IO:
         interface_async_complete_io(qxl, 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);
     }
diff --git a/hw/qxl.h b/hw/qxl.h
index 666dd78..1977ff3 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -140,3 +140,5 @@ void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
                                  uint32_t clear_dirty_region,
                                  int is_vga);
 #endif
+
+void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie);
diff --git a/ui/spice-display.h b/ui/spice-display.h
index c8747ab..8f286f8 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 {
-- 
1.7.9

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

* Re: [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie
  2012-02-17  8:44 ` [Qemu-devel] [PATCH v2 " Alon Levy
  2012-02-17  8:45   ` [Qemu-devel] [PATCH v2 2/2] qxl: make qxl_render_update async Alon Levy
@ 2012-02-17 12:49   ` Marc-André Lureau
  2012-02-17 19:09     ` Alon Levy
  2012-02-17 12:54   ` Marc-André Lureau
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Marc-André Lureau @ 2012-02-17 12:49 UTC (permalink / raw)
  To: Alon Levy; +Cc: qemu-devel, elmarco, kraxel

Hi

On Fri, Feb 17, 2012 at 9:44 AM, Alon Levy <alevy@redhat.com> wrote:
> +/* 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) {

We still have spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, 0);

So cookie might be NULL in this case.

-- 
Marc-André Lureau

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

* Re: [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie
  2012-02-17  8:44 ` [Qemu-devel] [PATCH v2 " Alon Levy
  2012-02-17  8:45   ` [Qemu-devel] [PATCH v2 2/2] qxl: make qxl_render_update async Alon Levy
  2012-02-17 12:49   ` [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie Marc-André Lureau
@ 2012-02-17 12:54   ` Marc-André Lureau
  2012-02-17 13:11   ` Gerd Hoffmann
  2012-02-17 13:20   ` Alon Levy
  4 siblings, 0 replies; 12+ messages in thread
From: Marc-André Lureau @ 2012-02-17 12:54 UTC (permalink / raw)
  To: Alon Levy; +Cc: qemu-devel, elmarco, kraxel

Hi

On Fri, Feb 17, 2012 at 9:44 AM, Alon Levy <alevy@redhat.com> wrote:
> +QXLCookie *qxl_cookie_new(int type, uint64_t io, uint64_t data)
> +{
> +    QXLCookie *cookie;
> +
> +    cookie = g_malloc0(sizeof(*cookie));
> +    cookie->type = type;
> +    cookie->io = io;
> +    cookie->data = data;
> +    return cookie;
> +}

I don't know if it's prohibited in qemu code, but g_slice would be a
good fit here: frequently allocated bits of same size.

-- 
Marc-André Lureau

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

* Re: [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie
  2012-02-17  8:44 ` [Qemu-devel] [PATCH v2 " Alon Levy
                     ` (2 preceding siblings ...)
  2012-02-17 12:54   ` Marc-André Lureau
@ 2012-02-17 13:11   ` Gerd Hoffmann
  2012-02-17 15:16     ` Alon Levy
  2012-02-17 13:20   ` Alon Levy
  4 siblings, 1 reply; 12+ messages in thread
From: Gerd Hoffmann @ 2012-02-17 13:11 UTC (permalink / raw)
  To: Alon Levy; +Cc: qemu-devel, elmarco

On 02/17/12 09:44, Alon Levy wrote:
> Will be used in the next patch.
> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  hw/qxl-render.c    |    2 +-
>  hw/qxl.c           |   60 +++++++++++++++++++++++++++++++++++++--------------
>  hw/qxl.h           |    2 +-
>  ui/spice-display.c |   26 ++++++++++++++++++++--
>  ui/spice-display.h |   12 ++++++++++
>  5 files changed, 80 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/qxl-render.c b/hw/qxl-render.c
> index 133d093..b238b96 100644
> --- a/hw/qxl-render.c
> +++ b/hw/qxl-render.c
> @@ -133,7 +133,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
>  
>      memset(dirty, 0, sizeof(dirty));
>      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));
>          dirty[0] = update;
> diff --git a/hw/qxl.c b/hw/qxl.c
> index ac69125..02708e3 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -143,15 +143,20 @@ 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)
>  {
>      if (async == QXL_SYNC) {
>          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
> +        if (cookie == NULL) {
> +            cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
> +                                    QXL_IO_UPDATE_AREA_ASYNC,
> +                                    0);
> +        }

Hmm, why that?  If the callers want a cookie, then they should
explicitly pass in what they want instead of expecting the function to
magically do the correct thing.  If the callers don't need a cookie,
then use something like QXL_COOKIE_TYPE_UNUSED or simply pass on the
NULL pointer.

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie
  2012-02-17  8:44 ` [Qemu-devel] [PATCH v2 " Alon Levy
                     ` (3 preceding siblings ...)
  2012-02-17 13:11   ` Gerd Hoffmann
@ 2012-02-17 13:20   ` Alon Levy
  4 siblings, 0 replies; 12+ messages in thread
From: Alon Levy @ 2012-02-17 13:20 UTC (permalink / raw)
  To: qemu-devel, kraxel, elmarco

On Fri, Feb 17, 2012 at 10:44:59AM +0200, Alon Levy wrote:
> Will be used in the next patch.

NACK these two, Marc Andre found a cookie I missed, and the bz in the
commit message is a dup of an existing bug (747011).

> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  hw/qxl-render.c    |    2 +-
>  hw/qxl.c           |   60 +++++++++++++++++++++++++++++++++++++--------------
>  hw/qxl.h           |    2 +-
>  ui/spice-display.c |   26 ++++++++++++++++++++--
>  ui/spice-display.h |   12 ++++++++++
>  5 files changed, 80 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/qxl-render.c b/hw/qxl-render.c
> index 133d093..b238b96 100644
> --- a/hw/qxl-render.c
> +++ b/hw/qxl-render.c
> @@ -133,7 +133,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
>  
>      memset(dirty, 0, sizeof(dirty));
>      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));
>          dirty[0] = update;
> diff --git a/hw/qxl.c b/hw/qxl.c
> index ac69125..02708e3 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -143,15 +143,20 @@ 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)
>  {
>      if (async == QXL_SYNC) {
>          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
> +        if (cookie == NULL) {
> +            cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
> +                                    QXL_IO_UPDATE_AREA_ASYNC,
> +                                    0);
> +        }
>          spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
> -                                    clear_dirty_region, 0);
> +                                    clear_dirty_region, (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> @@ -171,11 +176,13 @@ 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
> +#if SPICE_INTERFACE_QXL_MINOR >= 1
>          spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id,
> -                                        (uint64_t)id);
> +                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
> +                                         QXL_IO_DESTROY_SURFACE_ASYNC,
> +                                         id));
> +#else
> +        abort();
>  #endif
>      } else {
>          qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
> @@ -217,10 +224,13 @@ 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();
> +#if SPICE_INTERFACE_QXL_MINOR >= 1
> +        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl,
> +                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
> +                                         QXL_IO_DESTROY_ALL_SURFACES,
> +                                         0));
>  #else
> -        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0);
> +        abort();
>  #endif
>      } else {
>          qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
> @@ -737,18 +747,20 @@ 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)
> +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);
>      current_async = qxl->current_async;
>      qxl->current_async = QXL_UNDEFINED_IO;
>      qemu_mutex_unlock(&qxl->async_lock);
> +    dprint(qxl, 2, "async_complete: %d (%p) done\n", current_async, cookie);
> +    if (current_async != cookie->io) {
> +        fprintf(stderr, "qxl: %s: error: current_async = %d != %ld = cookie->io\n",
> +                __func__, current_async, cookie->io);
> +    }
>  
> -    dprint(qxl, 2, "async_complete: %d (%ld) done\n", current_async, cookie);
>      switch (current_async) {
>      case QXL_IO_CREATE_PRIMARY_ASYNC:
>          qxl_create_guest_primary_complete(qxl);
> @@ -757,12 +769,28 @@ 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, (uint32_t)cookie->data);
>          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);
> +}
> +
>  #endif
>  
>  static const QXLInterface qxl_interface = {
> @@ -1077,9 +1105,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);
> @@ -1215,7 +1241,7 @@ 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, NULL);
>          break;
>      }
>      case QXL_IO_NOTIFY_CMD:
> diff --git a/hw/qxl.h b/hw/qxl.h
> index 766aa6d..666dd78 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 6c302a3..680e6f4 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -60,12 +60,26 @@ 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, uint64_t data)
> +{
> +    QXLCookie *cookie;
> +
> +    cookie = g_malloc0(sizeof(*cookie));
> +    cookie->type = type;
> +    cookie->io = io;
> +    cookie->data = data;
> +    return cookie;
> +}
> +
>  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);
> +        spice_qxl_add_memslot_async(&ssd->qxl, memslot,
> +                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
> +                                         QXL_IO_MEMSLOT_ADD_ASYNC,
> +                                         0));
>  #else
>          abort();
>  #endif
> @@ -85,7 +99,10 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
>  {
>      if (async != QXL_SYNC) {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> -        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,
> +                                         0));
>  #else
>          abort();
>  #endif
> @@ -100,7 +117,10 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
>  {
>      if (async != QXL_SYNC) {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> -        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,
> +                                         0));
>  #else
>          abort();
>  #endif
> diff --git a/ui/spice-display.h b/ui/spice-display.h
> index 5e52df9..c8747ab 100644
> --- a/ui/spice-display.h
> +++ b/ui/spice-display.h
> @@ -48,6 +48,18 @@ typedef enum qxl_async_io {
>      QXL_ASYNC,
>  } qxl_async_io;
>  
> +enum {
> +    QXL_COOKIE_TYPE_IO,
> +};
> +
> +typedef struct QXLCookie {
> +    int      type;
> +    uint64_t io;
> +    uint64_t data;
> +} QXLCookie;
> +
> +QXLCookie *qxl_cookie_new(int type, uint64_t io, uint64_t data);
> +
>  typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
>  typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
>  
> -- 
> 1.7.9
> 
> 

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

* Re: [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie
  2012-02-17 13:11   ` Gerd Hoffmann
@ 2012-02-17 15:16     ` Alon Levy
  0 siblings, 0 replies; 12+ messages in thread
From: Alon Levy @ 2012-02-17 15:16 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel, elmarco

On Fri, Feb 17, 2012 at 02:11:40PM +0100, Gerd Hoffmann wrote:
> On 02/17/12 09:44, Alon Levy wrote:
> > Will be used in the next patch.
> > 
> > Signed-off-by: Alon Levy <alevy@redhat.com>
> > ---
> >  hw/qxl-render.c    |    2 +-
> >  hw/qxl.c           |   60 +++++++++++++++++++++++++++++++++++++--------------
> >  hw/qxl.h           |    2 +-
> >  ui/spice-display.c |   26 ++++++++++++++++++++--
> >  ui/spice-display.h |   12 ++++++++++
> >  5 files changed, 80 insertions(+), 22 deletions(-)
> > 
> > diff --git a/hw/qxl-render.c b/hw/qxl-render.c
> > index 133d093..b238b96 100644
> > --- a/hw/qxl-render.c
> > +++ b/hw/qxl-render.c
> > @@ -133,7 +133,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
> >  
> >      memset(dirty, 0, sizeof(dirty));
> >      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));
> >          dirty[0] = update;
> > diff --git a/hw/qxl.c b/hw/qxl.c
> > index ac69125..02708e3 100644
> > --- a/hw/qxl.c
> > +++ b/hw/qxl.c
> > @@ -143,15 +143,20 @@ 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)
> >  {
> >      if (async == QXL_SYNC) {
> >          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
> > +        if (cookie == NULL) {
> > +            cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
> > +                                    QXL_IO_UPDATE_AREA_ASYNC,
> > +                                    0);
> > +        }
> 
> Hmm, why that?  If the callers want a cookie, then they should
> explicitly pass in what they want instead of expecting the function to
> magically do the correct thing.  If the callers don't need a cookie,
> then use something like QXL_COOKIE_TYPE_UNUSED or simply pass on the
> NULL pointer.

I can move it to the users if it makes it a little less magic.

Note that I will be sending a revized version, since:
1) I forgot about my previous attempt (october 2010,
http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg02758.html)
2) there are a few bugs with this approach, that I fixed correctly back
then: one is the async monitor command, which I will not send this time
since I don't want to get into the same argument.
3) the second is the dirty rectangles, for which I have to raise
SPICE_INTERFACE_QXL_MINOR.

> 
> cheers,
>   Gerd
> 

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

* Re: [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie
  2012-02-17 12:49   ` [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie Marc-André Lureau
@ 2012-02-17 19:09     ` Alon Levy
  2012-02-17 19:21       ` Alon Levy
  0 siblings, 1 reply; 12+ messages in thread
From: Alon Levy @ 2012-02-17 19:09 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: qemu-devel, elmarco, kraxel

On Fri, Feb 17, 2012 at 01:49:20PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Feb 17, 2012 at 9:44 AM, Alon Levy <alevy@redhat.com> wrote:
> > +/* 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) {
> 
> We still have spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, 0);
> 
> So cookie might be NULL in this case.

Yeah, Marc Andre also noticed (exprimentally - you take the cake on the
review :).

> 
> -- 
> Marc-André Lureau

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

* Re: [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie
  2012-02-17 19:09     ` Alon Levy
@ 2012-02-17 19:21       ` Alon Levy
  0 siblings, 0 replies; 12+ messages in thread
From: Alon Levy @ 2012-02-17 19:21 UTC (permalink / raw)
  To: Marc-André Lureau, qemu-devel, kraxel, elmarco

On Fri, Feb 17, 2012 at 09:09:11PM +0200, Alon Levy wrote:
> On Fri, Feb 17, 2012 at 01:49:20PM +0100, Marc-André Lureau wrote:
> > Hi
> > 
> > On Fri, Feb 17, 2012 at 9:44 AM, Alon Levy <alevy@redhat.com> wrote:
> > > +/* 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) {
> > 
> > We still have spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, 0);
> > 
> > So cookie might be NULL in this case.
> 
> Yeah, Marc Andre also noticed (exprimentally - you take the cake on the
> review :).

Please ignore.

> 
> > 
> > -- 
> > Marc-André Lureau
> 

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

end of thread, other threads:[~2012-02-17 19:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-16 23:11 [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie Alon Levy
2012-02-16 23:11 ` [Qemu-devel] [PATCH 2/2] qxl: make qxl_render_update async Alon Levy
2012-02-17  8:39 ` [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie Alon Levy
2012-02-17  8:44 ` [Qemu-devel] [PATCH v2 " Alon Levy
2012-02-17  8:45   ` [Qemu-devel] [PATCH v2 2/2] qxl: make qxl_render_update async Alon Levy
2012-02-17 12:49   ` [Qemu-devel] [PATCH v2 1/2] qxl: introduce QXLCookie Marc-André Lureau
2012-02-17 19:09     ` Alon Levy
2012-02-17 19:21       ` Alon Levy
2012-02-17 12:54   ` Marc-André Lureau
2012-02-17 13:11   ` Gerd Hoffmann
2012-02-17 15:16     ` Alon Levy
2012-02-17 13:20   ` Alon Levy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).