On Mon, Feb 20, 2012 at 03:37:45PM +0200, Alon Levy wrote: > On Mon, Feb 20, 2012 at 02:52:51PM +0200, Yonit Halperin wrote: > > Hi, > > On 02/19/2012 02:53 PM, Alon Levy wrote: > > >On Sun, Feb 19, 2012 at 02:37:16PM +0200, Alon Levy wrote: > > >>Bumps spice to 0.9.2, since it adds a new symbol. This will be 0.8.4 > > >>in 0.8 branch - the syms file is updated to contain 0.8.3 and 0.8.4. > > > > > >Forgot to change this comment. I'll drop the 0.10.1->0.10.2 change since > > >Marc Andre sent a patch that does that already. > > > > > >> > > >>Add an asynchronous version of update_area that updates the dirty > > >>rectangles, similarily to the sync one. The existing async version > > >>doesn't pass the dirty list at all, which is good for > > >>QXL_IO_UPDATE_AREA, but bad for the monitor command screen_dump and > > >>vnc/sdl interoperability. > > >> > > (1) It would be nice to explain why the call to the sync version > > caused deadlock. > > Right. > > > > > (2) Is it necessary to add another async version of update_area? > > Can't we use the update_area_complete callback instead? > > Ok, I completely forgot about that. There is the downside that the way > it is currently written any implementation of update_area_complete will > be called always, and causes a little work in > handle_dev_update_async_helper. It also causes the need to note when we > start a qxl_render_update and complete it, to know when we want to act > on the update_area_complete call. Not sure it's better then introducing > a new API, but I agree it isn't really a must. >
Gerd, Can you contribute your opinion on this? Do you think it's better to: - add the new API spice_qxl_update_area_dirty_async - use update_area_complete always - set update_area_complete when it is required in qxl_render_update, and unset it in qxl_render_update_complete - something else ? Alon > > > > Regards, > > Yonit. > > >>RHBZ: 747011 > > >>FDBZ: 41622 > > >>--- > > >> configure.ac | 2 +- > > >> server/red_dispatcher.c | 42 +++++++++++++++++++++++++++---- > > >> server/red_dispatcher.h | 9 +++++++ > > >> server/red_worker.c | 60 > > >> +++++++++++++++++++++++++++++++++------------- > > >> server/red_worker.h | 3 ++ > > >> server/spice-server.syms | 5 ++++ > > >> server/spice.h | 6 ++++- > > >> 7 files changed, 102 insertions(+), 25 deletions(-) > > >> > > >>diff --git a/configure.ac b/configure.ac > > >>index 1c15e74..a617096 100644 > > >>--- a/configure.ac > > >>+++ b/configure.ac > > >>@@ -2,7 +2,7 @@ AC_PREREQ([2.57]) > > >> > > >> m4_define([SPICE_MAJOR], 0) > > >> m4_define([SPICE_MINOR], 10) > > >>-m4_define([SPICE_MICRO], 1) > > >>+m4_define([SPICE_MICRO], 2) > > >> > > >> AC_INIT(spice, [SPICE_MAJOR.SPICE_MINOR.SPICE_MICRO], [], spice) > > >> > > >>diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c > > >>index 321232b..5b8097c 100644 > > >>--- a/server/red_dispatcher.c > > >>+++ b/server/red_dispatcher.c > > >>@@ -334,6 +334,28 @@ static void > > >>red_dispatcher_update_area_async(RedDispatcher *dispatcher, > > >> &payload); > > >> } > > >> > > >>+static void red_dispatcher_update_area_dirty_async(RedDispatcher > > >>*dispatcher, > > >>+ uint32_t surface_id, > > >>+ struct QXLRect *qxl_area, > > >>+ struct QXLRect *qxl_dirty_rects, > > >>+ uint32_t num_dirty_rects, > > >>+ uint32_t clear_dirty_region, > > >>+ uint64_t cookie) > > >>+{ > > >>+ RedWorkerMessage message = RED_WORKER_MESSAGE_UPDATE_DIRTY_ASYNC; > > >>+ RedWorkerMessageUpdateDirtyAsync payload; > > >>+ > > >>+ payload.base.cmd = async_command_alloc(dispatcher, message, cookie); > > >>+ payload.surface_id = surface_id; > > >>+ payload.qxl_area = *qxl_area; > > >>+ payload.qxl_dirty_rects = qxl_dirty_rects; > > >>+ payload.num_dirty_rects = num_dirty_rects; > > >>+ payload.clear_dirty_region = clear_dirty_region; > > >>+ dispatcher_send_message(&dispatcher->dispatcher, > > >>+ message, > > >>+&payload); > > >>+} > > >>+ > > >> static void qxl_worker_update_area(QXLWorker *qxl_worker, uint32_t > > >> surface_id, > > >> QXLRect *qxl_area, QXLRect > > >> *qxl_dirty_rects, > > >> uint32_t num_dirty_rects, uint32_t > > >> clear_dirty_region) > > >>@@ -870,6 +892,17 @@ void spice_qxl_loadvm_commands(QXLInstance > > >>*instance, struct QXLCommandExt *ext, > > >> } > > >> > > >> SPICE_GNUC_VISIBLE > > >>+void spice_qxl_update_area_dirty_async(QXLInstance *instance, uint32_t > > >>surface_id, > > >>+ struct QXLRect *qxl_area, struct > > >>QXLRect *dirty_rects, > > >>+ uint32_t num_dirty_rects, > > >>uint32_t clear_dirty_region, > > >>+ uint64_t cookie) > > >>+{ > > >>+ red_dispatcher_update_area_dirty_async(instance->st->dispatcher, > > >>surface_id, qxl_area, > > >>+ dirty_rects, num_dirty_rects, > > >>clear_dirty_region, > > >>+ cookie); > > >>+} > > >>+ > > >>+SPICE_GNUC_VISIBLE > > >> void spice_qxl_update_area_async(QXLInstance *instance, uint32_t > > >> surface_id, QXLRect *qxl_area, > > >> uint32_t clear_dirty_region, uint64_t > > >> cookie) > > >> { > > >>@@ -926,10 +959,11 @@ void red_dispatcher_async_complete(struct > > >>RedDispatcher *dispatcher, > > >> pthread_mutex_unlock(&dispatcher->async_lock); > > >> switch (async_command->message) { > > >> case RED_WORKER_MESSAGE_UPDATE_ASYNC: > > >>- break; > > >>+ case RED_WORKER_MESSAGE_UPDATE_DIRTY_ASYNC: > > >> case RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC: > > >>- break; > > >> case RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC: > > >>+ case RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC: > > >>+ case RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC: > > >> break; > > >> case RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC: > > >> red_dispatcher_create_primary_surface_complete(dispatcher); > > >>@@ -937,10 +971,6 @@ void red_dispatcher_async_complete(struct > > >>RedDispatcher *dispatcher, > > >> case RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC: > > >> red_dispatcher_destroy_primary_surface_complete(dispatcher); > > >> break; > > >>- case RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC: > > >>- break; > > >>- case RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC: > > >>- break; > > >> default: > > >> WARN("unexpected message"); > > >> } > > >>diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h > > >>index 36db4e3..8bf7433 100644 > > >>--- a/server/red_dispatcher.h > > >>+++ b/server/red_dispatcher.h > > >>@@ -89,6 +89,15 @@ typedef struct RedWorkerMessageUpdateAsync { > > >> uint32_t clear_dirty_region; > > >> } RedWorkerMessageUpdateAsync; > > >> > > >>+typedef struct RedWorkerMessageUpdateDirtyAsync { > > >>+ RedWorkerMessageAsync base; > > >>+ uint32_t surface_id; > > >>+ QXLRect qxl_area; > > >>+ QXLRect *qxl_dirty_rects; > > >>+ uint32_t num_dirty_rects; > > >>+ uint32_t clear_dirty_region; > > >>+} RedWorkerMessageUpdateDirtyAsync; > > >>+ > > >> typedef struct RedWorkerMessageAddMemslot { > > >> QXLDevMemSlot mem_slot; > > >> } RedWorkerMessageAddMemslot; > > >>diff --git a/server/red_worker.c b/server/red_worker.c > > >>index 4c73952..19a0632 100644 > > >>--- a/server/red_worker.c > > >>+++ b/server/red_worker.c > > >>@@ -43,6 +43,7 @@ > > >> #include<netinet/tcp.h> > > >> #include<setjmp.h> > > >> #include<openssl/ssl.h> > > >>+#include<inttypes.h> > > >> > > >> #include<spice/qxl_dev.h> > > >> #include "spice.h" > > >>@@ -10237,17 +10238,12 @@ static void > > >>surface_dirty_region_to_rects(RedSurface *surface, > > >> free(dirty_rects); > > >> } > > >> > > >>-void handle_dev_update_async(void *opaque, void *payload) > > >>+void handle_dev_update_async_helper(RedWorker *worker, uint32_t > > >>surface_id, QXLRect qxl_area, > > >>+ QXLRect *qxl_dirty_rects, uint32_t num_dirty_rects, uint32_t > > >>clear_dirty_region) > > >> { > > >>- RedWorker *worker = opaque; > > >>- RedWorkerMessageUpdateAsync *msg = payload; > > >> SpiceRect rect; > > >>- QXLRect *qxl_dirty_rects; > > >>- uint32_t num_dirty_rects; > > >> RedSurface *surface; > > >>- uint32_t surface_id = msg->surface_id; > > >>- QXLRect qxl_area = msg->qxl_area; > > >>- uint32_t clear_dirty_region = msg->clear_dirty_region; > > >>+ int free_dirty = 0; > > >> > > >> red_get_rect_ptr(&rect,&qxl_area); > > >> flush_display_commands(worker); > > >>@@ -10256,20 +10252,45 @@ void handle_dev_update_async(void *opaque, void > > >>*payload) > > >> > > >> validate_surface(worker, surface_id); > > >> red_update_area(worker,&rect, surface_id); > > >>- if (!worker->qxl->st->qif->update_area_complete) { > > >>+ if (!worker->qxl->st->qif->update_area_complete&& !qxl_dirty_rects) > > >>{ > > >> return; > > >> } > > >>+ > > >> surface =&worker->surfaces[surface_id]; > > >>- num_dirty_rects = > > >>pixman_region32_n_rects(&surface->draw_dirty_region); > > >>- if (num_dirty_rects == 0) { > > >>- return; > > >>+ if (!qxl_dirty_rects) { > > >>+ num_dirty_rects = > > >>pixman_region32_n_rects(&surface->draw_dirty_region); > > >>+ if (num_dirty_rects != 0) { > > >>+ qxl_dirty_rects = spice_new0(QXLRect, num_dirty_rects); > > >>+ free_dirty = 1; > > >>+ } > > >> } > > >>- qxl_dirty_rects = spice_new0(QXLRect, num_dirty_rects); > > >>- surface_dirty_region_to_rects(surface, qxl_dirty_rects, > > >>num_dirty_rects, > > >>- clear_dirty_region); > > >>- worker->qxl->st->qif->update_area_complete(worker->qxl, surface_id, > > >>+ if (num_dirty_rects> 0) { > > >>+ surface_dirty_region_to_rects(surface, qxl_dirty_rects, > > >>num_dirty_rects, > > >>+ clear_dirty_region); > > >>+ } > > >>+ if (worker->qxl->st->qif->update_area_complete) { > > >>+ worker->qxl->st->qif->update_area_complete(worker->qxl, > > >>surface_id, > > >> qxl_dirty_rects, > > >> num_dirty_rects); > > >>- free(qxl_dirty_rects); > > >>+ } > > >>+ if (free_dirty) { > > >>+ free(qxl_dirty_rects); > > >>+ } > > >>+} > > >>+ > > >>+void handle_dev_update_dirty_async(void *opaque, void *payload) > > >>+{ > > >>+ RedWorkerMessageUpdateDirtyAsync *msg = payload; > > >>+ > > >>+ handle_dev_update_async_helper((RedWorker *)opaque, msg->surface_id, > > >>+ msg->qxl_area, msg->qxl_dirty_rects, msg->num_dirty_rects, > > >>msg->clear_dirty_region); > > >>+} > > >>+ > > >>+void handle_dev_update_async(void *opaque, void *payload) > > >>+{ > > >>+ RedWorkerMessageUpdateAsync *msg = payload; > > >>+ > > >>+ handle_dev_update_async_helper((RedWorker *)opaque, msg->surface_id, > > >>+ msg->qxl_area, NULL, 0, msg->clear_dirty_region); > > >> } > > >> > > >> void handle_dev_update(void *opaque, void *payload) > > >>@@ -10914,6 +10935,11 @@ static void register_callbacks(Dispatcher > > >>*dispatcher) > > >> sizeof(RedWorkerMessageUpdateAsync), > > >> DISPATCHER_ASYNC); > > >> dispatcher_register_handler(dispatcher, > > >>+ RED_WORKER_MESSAGE_UPDATE_DIRTY_ASYNC, > > >>+ handle_dev_update_dirty_async, > > >>+ sizeof(RedWorkerMessageUpdateDirtyAsync), > > >>+ DISPATCHER_ASYNC); > > >>+ dispatcher_register_handler(dispatcher, > > >> RED_WORKER_MESSAGE_ADD_MEMSLOT, > > >> handle_dev_add_memslot, > > >> sizeof(RedWorkerMessageAddMemslot), > > >>diff --git a/server/red_worker.h b/server/red_worker.h > > >>index 1f63d01..987563c 100644 > > >>--- a/server/red_worker.h > > >>+++ b/server/red_worker.h > > >>@@ -85,6 +85,9 @@ enum { > > >> RED_WORKER_MESSAGE_DISPLAY_CHANNEL_CREATE, > > >> RED_WORKER_MESSAGE_CURSOR_CHANNEL_CREATE, > > >> > > >>+ /* support monitor async screen dump */ > > >>+ RED_WORKER_MESSAGE_UPDATE_DIRTY_ASYNC, > > >>+ > > >> RED_WORKER_MESSAGE_COUNT // LAST > > >> }; > > >> > > >>diff --git a/server/spice-server.syms b/server/spice-server.syms > > >>index d9beec3..8b5a2ff 100644 > > >>--- a/server/spice-server.syms > > >>+++ b/server/spice-server.syms > > >>@@ -101,3 +101,8 @@ global: > > >> spice_server_add_client; > > >> spice_server_add_ssl_client; > > >> } SPICE_SERVER_0.10.0; > > >>+ > > >>+SPICE_SERVER_0.10.2 { > > >>+global: > > >>+ spice_qxl_update_area_dirty_async; > > >>+} SPICE_SERVER_0.10.1; > > >>diff --git a/server/spice.h b/server/spice.h > > >>index 7397655..a096fbc 100644 > > >>--- a/server/spice.h > > >>+++ b/server/spice.h > > >>@@ -22,7 +22,7 @@ > > >> #include<sys/socket.h> > > >> #include<spice/qxl_dev.h> > > >> > > >>-#define SPICE_SERVER_VERSION 0x000a01 /* release 0.10.1 */ > > >>+#define SPICE_SERVER_VERSION 0x000a02 /* release 0.10.2 */ > > >> > > >> /* interface base type */ > > >> > > >>@@ -153,6 +153,10 @@ void spice_qxl_loadvm_commands(QXLInstance > > >>*instance, struct QXLCommandExt *ext, > > >> /* async versions of commands. when complete spice calls async_complete > > >> */ > > >> void spice_qxl_update_area_async(QXLInstance *instance, uint32_t > > >> surface_id, QXLRect *qxl_area, > > >> uint32_t clear_dirty_region, uint64_t > > >> cookie); > > >>+void spice_qxl_update_area_dirty_async(QXLInstance *instance, uint32_t > > >>surface_id, > > >>+ struct QXLRect *qxl_area, struct > > >>QXLRect *dirty_rects, > > >>+ uint32_t num_dirty_rects, > > >>uint32_t clear_dirty_region, > > >>+ uint64_t cookie); > > >> void spice_qxl_add_memslot_async(QXLInstance *instance, QXLDevMemSlot > > >> *slot, uint64_t cookie); > > >> void spice_qxl_destroy_surfaces_async(QXLInstance *instance, uint64_t > > >> cookie); > > >> void spice_qxl_destroy_primary_surface_async(QXLInstance *instance, > > >> uint32_t surface_id, uint64_t cookie); > > >>-- > > >>1.7.9 > > >> > > >>_______________________________________________ > > >>Spice-devel mailing list > > >>Spice-devel@lists.freedesktop.org > > >>http://lists.freedesktop.org/mailman/listinfo/spice-devel > > >_______________________________________________ > > >Spice-devel mailing list > > >Spice-devel@lists.freedesktop.org > > >http://lists.freedesktop.org/mailman/listinfo/spice-devel > > > _______________________________________________ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel