[Spice-devel] [PATCH] server: spice_qxl_update_area_dirty_async
Yonit Halperin
yhalperi at redhat.com
Mon Feb 20 04:52:51 PST 2012
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.
(2) Is it necessary to add another async version of update_area? Can't
we use the update_area_complete callback instead?
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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list