[Spice-commits] 3 commits - server/red_worker.c
Yonit Halperin
yhalperi at kemper.freedesktop.org
Mon Aug 30 00:17:38 PDT 2010
server/red_worker.c | 159 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 110 insertions(+), 49 deletions(-)
New commits:
commit 494f5d4e2c46c6f4970c7bf2e052aaaf961667e1
Author: Yonit Halperin <yhalperi at redhat.com>
Date: Thu Aug 26 11:51:23 2010 +0300
server: cleanups in destorying surfaces code
diff --git a/server/red_worker.c b/server/red_worker.c
index 64611f0..7576087 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1493,20 +1493,20 @@ static inline void red_destroy_surface(RedWorker *worker, uint32_t surface_id)
if (surface_id == 0) {
red_reset_stream_trace(worker);
}
- if (surface->context.canvas) {
- surface->context.canvas->ops->destroy(surface->context.canvas);
- if (surface->create.info) {
- worker->qxl->st->qif->release_resource(worker->qxl, surface->create);
- }
- if (surface->destroy.info) {
- worker->qxl->st->qif->release_resource(worker->qxl, surface->destroy);
- }
+ ASSERT(surface->context.canvas);
- region_destroy(&surface->draw_dirty_region);
- surface->context.canvas = NULL;
- red_destroy_surface_item(worker, surface_id);
+ surface->context.canvas->ops->destroy(surface->context.canvas);
+ if (surface->create.info) {
+ worker->qxl->st->qif->release_resource(worker->qxl, surface->create);
+ }
+ if (surface->destroy.info) {
+ worker->qxl->st->qif->release_resource(worker->qxl, surface->destroy);
}
+ region_destroy(&surface->draw_dirty_region);
+ surface->context.canvas = NULL;
+ red_destroy_surface_item(worker, surface_id);
+
PANIC_ON(!ring_is_empty(&surface->depend_on_me));
}
}
@@ -3527,6 +3527,9 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
PANIC_ON(!red_surface->context.canvas);
set_surface_release_info(worker, surface_id, 0, surface->release_info, group_id);
red_handle_depends_on_target_surface(worker, surface_id);
+ /* note that red_handle_depends_on_target_surface must be called before red_current_clear.
+ otherwise "current" will hold items that other drawables may depend on, and then
+ red_current_clear will remove them from the pipe. */
red_current_clear(worker, surface_id);
red_clear_surface_drawables_from_pipe(worker, surface_id, FALSE);
red_destroy_surface(worker, surface_id);
@@ -8824,13 +8827,6 @@ static inline void flush_all_qxl_commands(RedWorker *worker)
flush_cursor_commands(worker);
}
-static inline void red_flush_surface_pipe(RedWorker *worker)
-{
- if (worker->display_channel) {
- display_channel_push(worker);
- }
-}
-
static void push_new_primary_surface(RedWorker *worker)
{
DisplayChannel *display_channel;
@@ -9759,10 +9755,14 @@ static inline void handle_dev_del_memslot(RedWorker *worker)
static inline void destroy_surface_wait(RedWorker *worker, int surface_id)
{
- if (worker->surfaces[surface_id].context.canvas) {
- red_handle_depends_on_target_surface(worker, surface_id);
+ if (!worker->surfaces[surface_id].context.canvas) {
+ return;
}
- red_flush_surface_pipe(worker);
+
+ red_handle_depends_on_target_surface(worker, surface_id);
+ /* note that red_handle_depends_on_target_surface must be called before red_current_clear.
+ otherwise "current" will hold items that other drawables may depend on, and then
+ red_current_clear will remove them from the pipe. */
red_current_clear(worker, surface_id);
red_clear_surface_drawables_from_pipe(worker, surface_id, TRUE);
// in case that the pipe didn't contain any item that is dependent on the surface, but
@@ -9800,15 +9800,13 @@ static inline void handle_dev_destroy_surfaces(RedWorker *worker)
red_printf("");
flush_all_qxl_commands(worker);
//to handle better
- if (worker->surfaces[0].context.canvas) {
- destroy_surface_wait(worker, 0);
- }
for (i = 0; i < NUM_SURFACES; ++i) {
if (worker->surfaces[i].context.canvas) {
destroy_surface_wait(worker, i);
if (worker->surfaces[i].context.canvas) {
red_destroy_surface(worker, i);
}
+ ASSERT(!worker->surfaces[i].context.canvas);
}
}
ASSERT(ring_is_empty(&worker->streams));
@@ -9829,11 +9827,6 @@ static inline void handle_dev_destroy_surfaces(RedWorker *worker)
red_display_clear_glz_drawables(worker->display_channel);
- //to handle better
- for (i = 0; i < NUM_SURFACES; ++i) {
- ASSERT(!worker->surfaces[i].context.canvas);
- }
-
worker->cursor_visible = TRUE;
worker->cursor_position.x = worker->cursor_position.y = 0;
worker->cursor_trail_length = worker->cursor_trail_frequency = 0;
commit 27f18287e899ff90fc0ee5ab697a8697d85953b2
Author: Yonit Halperin <yhalperi at redhat.com>
Date: Thu Aug 26 11:42:43 2010 +0300
server: really wait for a surface to be destroyed, when calling destroy_surface_wait
Waiting till all the pipe items that are dependent on the surface will be sent.
This was probably the cause for freedesktop bug #29750.
diff --git a/server/red_worker.c b/server/red_worker.c
index e6a6c14..64611f0 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -74,6 +74,9 @@
#define DETACH_TIMEOUT 15000000000ULL //nano
#define DETACH_SLEEP_DURATION 10000 //micro
+#define CHANNEL_PUSH_TIMEOUT 30000000000ULL //nano
+#define CHANNEL_PUSH_SLEEP_DURATION 10000 //micro
+
#define DISPLAY_CLIENT_TIMEOUT 15000000000ULL //nano
#define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro
@@ -979,6 +982,7 @@ static void reset_rate(StreamAgent *stream_agent);
static BitmapGradualType _get_bitmap_graduality_level(RedWorker *worker, SpiceBitmap *bitmap, uint32_t group_id);
static inline int _stride_is_extra(SpiceBitmap *bitmap);
static void red_disconnect_cursor(RedChannel *channel);
+static void red_wait_pipe_item_sent(RedChannel *channel, PipeItem *item);
#ifdef DUMP_BITMAP
static void dump_bitmap(RedWorker *worker, SpiceBitmap *bitmap, uint32_t group_id);
@@ -1780,7 +1784,7 @@ static void red_current_clear(RedWorker *worker, int surface_id)
}
}
-static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface_id)
+static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface_id, int force)
{
Ring *ring;
PipeItem *item;
@@ -1790,10 +1794,14 @@ static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface
return;
}
+ /* removing the newest drawables that their destination is surface_id and
+ no other drawable depends on them */
+
ring = &worker->display_channel->base.pipe;
item = (PipeItem *) ring;
while ((item = (PipeItem *)ring_next(ring, (RingItem *)item))) {
Drawable *drawable;
+ int depend_found = FALSE;
if (item->type == PIPE_ITEM_TYPE_DRAW) {
drawable = SPICE_CONTAINEROF(item, Drawable, pipe_item);
} else if (item->type == PIPE_ITEM_TYPE_UPGRADE) {
@@ -1802,12 +1810,6 @@ static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface
continue;
}
- for (x = 0; x < 3; ++x) {
- if (drawable->surfaces_dest[x] == surface_id) {
- return;
- }
- }
-
if (drawable->surface_id == surface_id) {
PipeItem *tmp_item = item;
item = (PipeItem *)ring_prev(ring, (RingItem *)item);
@@ -1818,7 +1820,27 @@ static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface
if (!item) {
item = (PipeItem *)ring;
}
+ continue;
+ }
+
+ for (x = 0; x < 3; ++x) {
+ if (drawable->surfaces_dest[x] == surface_id) {
+ depend_found = TRUE;
+ break;
+ }
}
+
+ if (depend_found) {
+ if (force) {
+ break;
+ } else {
+ return;
+ }
+ }
+ }
+
+ if (item) {
+ red_wait_pipe_item_sent(&worker->display_channel->base, item);
}
}
@@ -3506,7 +3528,7 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
set_surface_release_info(worker, surface_id, 0, surface->release_info, group_id);
red_handle_depends_on_target_surface(worker, surface_id);
red_current_clear(worker, surface_id);
- red_clear_surface_drawables_from_pipe(worker, surface_id);
+ red_clear_surface_drawables_from_pipe(worker, surface_id, FALSE);
red_destroy_surface(worker, surface_id);
break;
default:
@@ -9632,6 +9654,48 @@ static void red_wait_outgoing_item(RedChannel *channel)
red_unref_channel(channel);
}
+static void red_wait_pipe_item_sent(RedChannel *channel, PipeItem *item)
+{
+ uint64_t end_time;
+ int item_in_pipe;
+
+ if (!channel) {
+ return;
+ }
+
+ red_printf("");
+ red_ref_channel(channel);
+ channel->hold_item(item);
+
+ end_time = red_now() + CHANNEL_PUSH_TIMEOUT;
+
+ if (channel->send_data.blocked) {
+ red_receive(channel);
+ red_send_data(channel, NULL);
+ }
+ // todo: different push for each channel
+ red_push(channel->worker);
+
+ while((item_in_pipe = ring_item_is_linked(&item->link)) && (red_now() < end_time)) {
+ usleep(CHANNEL_PUSH_SLEEP_DURATION);
+ red_receive(channel);
+ red_send_data(channel, NULL);
+ red_push(channel->worker);
+ }
+
+ if (item_in_pipe) {
+ red_printf("timeout");
+ channel->disconnect(channel);
+ } else {
+ if (channel->send_data.item == item) {
+ red_wait_outgoing_item(channel);
+ }
+ }
+
+ channel->release_item(channel, item);
+ red_unref_channel(channel);
+}
+
static inline void handle_dev_update(RedWorker *worker)
{
RedWorkerMessage message;
@@ -9700,7 +9764,9 @@ static inline void destroy_surface_wait(RedWorker *worker, int surface_id)
}
red_flush_surface_pipe(worker);
red_current_clear(worker, surface_id);
- red_clear_surface_drawables_from_pipe(worker, surface_id);
+ red_clear_surface_drawables_from_pipe(worker, surface_id, TRUE);
+ // in case that the pipe didn't contain any item that is dependent on the surface, but
+ // there is one during sending.
red_wait_outgoing_item((RedChannel *)worker->display_channel);
if (worker->display_channel) {
ASSERT(!worker->display_channel->base.send_data.item);
commit 1c8ec8f1cf956745ff8b33b67bc12e408ac7e075
Author: Yonit Halperin <yhalperi at redhat.com>
Date: Thu Aug 26 11:29:18 2010 +0300
server: consider also PIPE_ITEM_UPGRADE when searching for drawables in red_clear_surface_drawables_from_pipe
diff --git a/server/red_worker.c b/server/red_worker.c
index 173ee65..e6a6c14 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1793,30 +1793,32 @@ static void red_clear_surface_drawables_from_pipe(RedWorker *worker, int surface
ring = &worker->display_channel->base.pipe;
item = (PipeItem *) ring;
while ((item = (PipeItem *)ring_next(ring, (RingItem *)item))) {
+ Drawable *drawable;
if (item->type == PIPE_ITEM_TYPE_DRAW) {
- PipeItem *tmp_item;
- Drawable *drawable;
-
drawable = SPICE_CONTAINEROF(item, Drawable, pipe_item);
+ } else if (item->type == PIPE_ITEM_TYPE_UPGRADE) {
+ drawable = ((UpgradeItem *)item)->drawable;
+ } else {
+ continue;
+ }
- for (x = 0; x < 3; ++x) {
- if (drawable->surfaces_dest[x] == surface_id) {
- return;
- }
+ for (x = 0; x < 3; ++x) {
+ if (drawable->surfaces_dest[x] == surface_id) {
+ return;
}
+ }
- if (drawable->surface_id == surface_id) {
- tmp_item = item;
- item = (PipeItem *)ring_prev(ring, (RingItem *)item);
- ring_remove(&tmp_item->link);
- release_drawable(worker, drawable);
- worker->display_channel->base.pipe_size--;
+ if (drawable->surface_id == surface_id) {
+ PipeItem *tmp_item = item;
+ item = (PipeItem *)ring_prev(ring, (RingItem *)item);
+ ring_remove(&tmp_item->link);
+ worker->display_channel->base.release_item(&worker->display_channel->base, tmp_item);
+ worker->display_channel->base.pipe_size--;
- if (!item) {
- item = (PipeItem *)ring;
- }
+ if (!item) {
+ item = (PipeItem *)ring;
}
- }
+ }
}
}
More information about the Spice-commits
mailing list