On Wed, Nov 25, 2015 at 10:52 AM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: >> >> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> >> >> --- >> server/dcc.c | 80 ++++++++++++++++++++++ >> server/dcc.h | 2 + >> server/display-channel.c | 69 ++++++++++++++++++- >> server/display-channel.h | 20 ++++++ >> server/red_worker.c | 173 >> ++--------------------------------------------- >> 5 files changed, 175 insertions(+), 169 deletions(-) >> >> diff --git a/server/dcc.c b/server/dcc.c >> index e3b0c55..6c089da 100644 >> --- a/server/dcc.c >> +++ b/server/dcc.c >> @@ -22,6 +22,8 @@ >> #include "dcc.h" >> #include "display-channel.h" >> >> +#define DISPLAY_CLIENT_SHORT_TIMEOUT 15000000000ULL //nano >> + >> static SurfaceCreateItem *surface_create_item_new(RedChannel* channel, >> uint32_t surface_id, >> uint32_t width, >> uint32_t height, uint32_t >> format, uint32_t flags) >> @@ -41,6 +43,84 @@ static SurfaceCreateItem >> *surface_create_item_new(RedChannel* channel, >> return create; >> } >> >> +/* >> + * Return: TRUE if wait_if_used == FALSE, or otherwise, if all of the pipe >> items that >> + * are related to the surface have been cleared (or sent) from the pipe. >> + */ >> +int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int >> surface_id, >> + int wait_if_used) >> +{ >> + Ring *ring; >> + PipeItem *item; >> + int x; >> + RedChannelClient *rcc; >> + >> + spice_return_val_if_fail(dcc != NULL, TRUE); >> + /* removing the newest drawables that their destination is surface_id >> and >> + no other drawable depends on them */ >> + >> + rcc = RED_CHANNEL_CLIENT(dcc); >> + ring = &rcc->pipe; >> + item = (PipeItem *) ring; >> + while ((item = (PipeItem *)ring_next(ring, (RingItem *)item))) { >> + Drawable *drawable; >> + DrawablePipeItem *dpi = NULL; >> + int depend_found = FALSE; >> + >> + if (item->type == PIPE_ITEM_TYPE_DRAW) { >> + dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item); >> + drawable = dpi->drawable; >> + } else if (item->type == PIPE_ITEM_TYPE_UPGRADE) { >> + drawable = ((UpgradeItem *)item)->drawable; >> + } else { >> + continue; >> + } >> + >> + if (drawable->surface_id == surface_id) { >> + PipeItem *tmp_item = item; >> + item = (PipeItem *)ring_prev(ring, (RingItem *)item); >> + red_channel_client_pipe_remove_and_release(rcc, tmp_item); >> + if (!item) { >> + item = (PipeItem *)ring; >> + } >> + continue; >> + } >> + >> + for (x = 0; x < 3; ++x) { >> + if (drawable->surface_deps[x] == surface_id) { >> + depend_found = TRUE; >> + break; >> + } >> + } >> + >> + if (depend_found) { >> + spice_debug("surface %d dependent item found %p, %p", >> surface_id, drawable, item); >> + if (wait_if_used) { >> + break; >> + } else { >> + return TRUE; >> + } >> + } >> + } >> + >> + if (!wait_if_used) { >> + return TRUE; >> + } >> + >> + if (item) { >> + return >> red_channel_client_wait_pipe_item_sent(RED_CHANNEL_CLIENT(dcc), item, >> + >> DISPLAY_CLIENT_TIMEOUT); >> + } else { >> + /* >> + * in case that the pipe didn't contain any item that is dependent >> on the surface, but >> + * there is one during sending. Use a shorter timeout, since it is >> just one item >> + */ >> + return >> red_channel_client_wait_outgoing_item(RED_CHANNEL_CLIENT(dcc), >> + >> DISPLAY_CLIENT_SHORT_TIMEOUT); >> + } >> + return TRUE; >> +} >> + >> void dcc_create_surface(DisplayChannelClient *dcc, int surface_id) >> { >> DisplayChannel *display; >> diff --git a/server/dcc.h b/server/dcc.h >> index dc6f1e7..c5767c9 100644 >> --- a/server/dcc.h >> +++ b/server/dcc.h >> @@ -201,6 +201,8 @@ void dcc_release_item >> (DisplayCha >> int >> item_pushed); >> void dcc_send_item >> (DisplayChannelClient *dcc, >> PipeItem >> *item); >> +int dcc_clear_surface_drawables_from_pipe >> (DisplayChannelClient *dcc, >> + int >> surface_id, int force); >> >> typedef struct compress_send_data_t { >> void* comp_buf; >> diff --git a/server/display-channel.c b/server/display-channel.c >> index 66b661e..ec076ce 100644 >> --- a/server/display-channel.c >> +++ b/server/display-channel.c >> @@ -1374,7 +1374,7 @@ void display_channel_draw_until(DisplayChannel >> *display, const SpiceRect *area, >> * newer item then 'last' in one of the surfaces that >> * display_channel_draw is called for, Otherwise, 'now' would have >> * already been rendered. See the call for >> - * red_handle_depends_on_target_surface in red_process_draw >> + * draw_depend_on_me in red_process_draw >> */ >> draw_until(display, surface, last); >> surface_update_dest(surface, area); >> @@ -1403,6 +1403,73 @@ void display_channel_draw(DisplayChannel *display, >> const SpiceRect *area, int su >> surface_update_dest(surface, area); >> } >> >> +static void clear_surface_drawables_from_pipes(DisplayChannel *display, int >> surface_id, >> + int wait_if_used) >> +{ >> + RingItem *item, *next; >> + DisplayChannelClient *dcc; >> + >> + FOREACH_DCC(display, item, next, dcc) { >> + if (!dcc_clear_surface_drawables_from_pipe(dcc, surface_id, >> wait_if_used)) { >> + red_channel_client_disconnect(RED_CHANNEL_CLIENT(dcc)); >> + } >> + } >> +} >> + >> +/* TODO: cleanup/refactor destroy functions */ >> +void display_channel_destroy_surface(DisplayChannel *display, uint32_t >> surface_id) >> +{ >> + draw_depend_on_me(display, surface_id); >> + /* note that draw_depend_on_me must be called before current_remove_all. >> + otherwise "current" will hold items that other drawables may depend >> on, and then >> + current_remove_all will remove them from the pipe. */ >> + current_remove_all(display, surface_id); >> + clear_surface_drawables_from_pipes(display, surface_id, FALSE); >> + display_channel_surface_unref(display, surface_id); >> +} >> + >> +void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t >> surface_id) >> +{ >> + if (!validate_surface(display, surface_id)) >> + return; >> + if (!display->surfaces[surface_id].context.canvas) >> + return; >> + >> + draw_depend_on_me(display, surface_id); >> + /* note that draw_depend_on_me must be called before current_remove_all. >> + otherwise "current" will hold items that other drawables may depend >> on, and then >> + current_remove_all will remove them from the pipe. */ >> + current_remove_all(display, surface_id); >> + clear_surface_drawables_from_pipes(display, surface_id, TRUE); >> +} >> + >> +/* called upon device reset */ >> +/* TODO: split me*/ >> +void display_channel_destroy_surfaces(DisplayChannel *display) >> +{ >> + int i; >> + >> + spice_debug(NULL); >> + //to handle better >> + for (i = 0; i < NUM_SURFACES; ++i) { >> + if (display->surfaces[i].context.canvas) { >> + display_channel_destroy_surface_wait(display, i); >> + if (display->surfaces[i].context.canvas) { >> + display_channel_surface_unref(display, i); >> + } >> + spice_assert(!display->surfaces[i].context.canvas); >> + } >> + } >> + spice_warn_if_fail(ring_is_empty(&display->streams)); >> + >> + if (red_channel_is_connected(RED_CHANNEL(display))) { >> + red_channel_pipes_add_type(RED_CHANNEL(display), >> PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE); >> + red_pipes_add_verb(RED_CHANNEL(display), >> SPICE_MSG_DISPLAY_STREAM_DESTROY_ALL); >> + } >> + >> + display_channel_free_glz_drawables(display); >> +} >> + >> static void on_disconnect(RedChannelClient *rcc) >> { >> DisplayChannel *display; >> diff --git a/server/display-channel.h b/server/display-channel.h >> index 42b3850..34caafe 100644 >> --- a/server/display-channel.h >> +++ b/server/display-channel.h >> @@ -284,6 +284,11 @@ int >> display_channel_wait_for_migrate_data (DisplayCha >> void display_channel_flush_all_surfaces >> (DisplayChannel *display); >> void >> display_channel_free_glz_drawables_to_free(DisplayChannel >> *display); >> void display_channel_free_glz_drawables >> (DisplayChannel *display); >> +void display_channel_destroy_surface_wait >> (DisplayChannel *display, >> + >> uint32_t >> surface_id); >> +void display_channel_destroy_surfaces >> (DisplayChannel *display); >> +void display_channel_destroy_surface >> (DisplayChannel *display, >> + >> uint32_t >> surface_id); >> >> static inline int validate_surface(DisplayChannel *display, uint32_t >> surface_id) >> { >> @@ -415,6 +420,21 @@ static inline void region_add_clip_rects(QRegion *rgn, >> SpiceClipRects *data) >> } >> } >> >> +static inline void draw_depend_on_me(DisplayChannel *display, uint32_t >> surface_id) >> +{ >> + RedSurface *surface; >> + RingItem *ring_item; >> + >> + surface = &display->surfaces[surface_id]; >> + >> + while ((ring_item = ring_get_tail(&surface->depend_on_me))) { >> + Drawable *drawable; >> + DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, >> ring_item); >> + drawable = depended_item->drawable; >> + display_channel_draw(display, &drawable->red_drawable->bbox, >> drawable->surface_id); >> + } >> +} >> + >> void current_remove_drawable(DisplayChannel *display, Drawable *item); >> void red_pipes_remove_drawable(Drawable *drawable); >> void current_remove(DisplayChannel *display, TreeItem *item); >> diff --git a/server/red_worker.c b/server/red_worker.c >> index cef546e..7af9c9b 100644 >> --- a/server/red_worker.c >> +++ b/server/red_worker.c >> @@ -72,8 +72,6 @@ >> #define CMD_RING_POLL_TIMEOUT 10 //milli >> #define CMD_RING_POLL_RETRIES 200 >> >> -#define DISPLAY_CLIENT_SHORT_TIMEOUT 15000000000ULL //nano >> - >> #define MAX_EVENT_SOURCES 20 >> #define INF_EVENT_WAIT ~0 >> >> @@ -334,101 +332,6 @@ void current_remove_all(DisplayChannel *display, int >> surface_id) >> } >> } >> >> -/* >> - * Return: TRUE if wait_if_used == FALSE, or otherwise, if all of the pipe >> items that >> - * are related to the surface have been cleared (or sent) from the pipe. >> - */ >> -static int red_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, >> int surface_id, >> - int wait_if_used) >> -{ >> - Ring *ring; >> - PipeItem *item; >> - int x; >> - RedChannelClient *rcc; >> - >> - if (!dcc) { >> - return TRUE; >> - } >> - >> - /* removing the newest drawables that their destination is surface_id >> and >> - no other drawable depends on them */ >> - >> - rcc = RED_CHANNEL_CLIENT(dcc); >> - ring = &rcc->pipe; >> - item = (PipeItem *) ring; >> - while ((item = (PipeItem *)ring_next(ring, (RingItem *)item))) { >> - Drawable *drawable; >> - DrawablePipeItem *dpi = NULL; >> - int depend_found = FALSE; >> - >> - if (item->type == PIPE_ITEM_TYPE_DRAW) { >> - dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item); >> - drawable = dpi->drawable; >> - } else if (item->type == PIPE_ITEM_TYPE_UPGRADE) { >> - drawable = ((UpgradeItem *)item)->drawable; >> - } else { >> - continue; >> - } >> - >> - if (drawable->surface_id == surface_id) { >> - PipeItem *tmp_item = item; >> - item = (PipeItem *)ring_prev(ring, (RingItem *)item); >> - red_channel_client_pipe_remove_and_release(rcc, tmp_item); >> - if (!item) { >> - item = (PipeItem *)ring; >> - } >> - continue; >> - } >> - >> - for (x = 0; x < 3; ++x) { >> - if (drawable->surface_deps[x] == surface_id) { >> - depend_found = TRUE; >> - break; >> - } >> - } >> - >> - if (depend_found) { >> - spice_debug("surface %d dependent item found %p, %p", >> surface_id, drawable, item); >> - if (wait_if_used) { >> - break; >> - } else { >> - return TRUE; >> - } >> - } >> - } >> - >> - if (!wait_if_used) { >> - return TRUE; >> - } >> - >> - if (item) { >> - return >> red_channel_client_wait_pipe_item_sent(RED_CHANNEL_CLIENT(dcc), item, >> - >> DISPLAY_CLIENT_TIMEOUT); >> - } else { >> - /* >> - * in case that the pipe didn't contain any item that is dependent >> on the surface, but >> - * there is one during sending. Use a shorter timeout, since it is >> just one item >> - */ >> - return >> red_channel_client_wait_outgoing_item(RED_CHANNEL_CLIENT(dcc), >> - >> DISPLAY_CLIENT_SHORT_TIMEOUT); >> - } >> - return TRUE; >> -} >> - >> -static void red_clear_surface_drawables_from_pipes(DisplayChannel *display, >> - int surface_id, >> - int wait_if_used) >> -{ >> - RingItem *item, *next; >> - DisplayChannelClient *dcc; >> - >> - FOREACH_DCC(display, item, next, dcc) { >> - if (!red_clear_surface_drawables_from_pipe(dcc, surface_id, >> wait_if_used)) { >> - red_channel_client_disconnect(RED_CHANNEL_CLIENT(dcc)); >> - } >> - } >> -} >> - >> static int red_display_drawable_is_in_pipe(DisplayChannelClient *dcc, >> Drawable *drawable) >> { >> DrawablePipeItem *dpi; >> @@ -697,23 +600,6 @@ static Drawable *get_drawable(RedWorker *worker, uint8_t >> effect, RedDrawable *re >> return drawable; >> } >> >> -static inline int red_handle_depends_on_target_surface(DisplayChannel >> *display, uint32_t surface_id) >> -{ >> - RedSurface *surface; >> - RingItem *ring_item; >> - >> - surface = &display->surfaces[surface_id]; >> - >> - while ((ring_item = ring_get_tail(&surface->depend_on_me))) { >> - Drawable *drawable; >> - DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, >> ring_item); >> - drawable = depended_item->drawable; >> - display_channel_draw(display, &drawable->red_drawable->bbox, >> drawable->surface_id); >> - } >> - >> - return TRUE; >> -} >> - >> static inline void add_to_surface_dependency(DisplayChannel *display, int >> depend_on_surface_id, >> DependItem *depend_item, >> Drawable *drawable) >> { >> @@ -812,9 +698,7 @@ static inline void red_process_draw(RedWorker *worker, >> RedDrawable *red_drawable >> goto cleanup; >> } >> >> - if (!red_handle_depends_on_target_surface(worker->display_channel, >> surface_id)) { >> - goto cleanup; >> - } >> + draw_depend_on_me(display, surface_id); >> >> if (!red_handle_surfaces_dependencies(worker->display_channel, >> drawable)) { >> goto cleanup; >> @@ -870,16 +754,10 @@ static inline void red_process_surface(RedWorker >> *worker, RedSurfaceCmd *surface >> break; >> } >> set_surface_release_info(&red_surface->destroy, >> surface->release_info, group_id); >> - red_handle_depends_on_target_surface(display, surface_id); >> - /* note that red_handle_depends_on_target_surface must be called >> before current_remove_all. >> - otherwise "current" will hold items that other drawables may >> depend on, and then >> - current_remove_all will remove them from the pipe. */ >> - current_remove_all(display, surface_id); >> - red_clear_surface_drawables_from_pipes(display, surface_id, FALSE); >> - display_channel_surface_unref(display, surface_id); >> + display_channel_destroy_surface(display, surface_id); >> break; >> default: >> - spice_error("unknown surface command"); >> + spice_warn_if_reached(); >> }; >> exit: >> red_put_surface_cmd(surface); >> @@ -4118,21 +3996,6 @@ static void handle_dev_del_memslot(void *opaque, void >> *payload) >> red_memslot_info_del_slot(&worker->mem_slots, slot_group_id, slot_id); >> } >> >> -void display_channel_destroy_surface_wait(DisplayChannel *display, int >> surface_id) >> -{ >> - if (!validate_surface(display, surface_id)) >> - return; >> - if (!display->surfaces[surface_id].context.canvas) >> - return; >> - >> - red_handle_depends_on_target_surface(display, surface_id); >> - /* note that red_handle_depends_on_target_surface must be called before >> current_remove_all. >> - otherwise "current" will hold items that other drawables may depend >> on, and then >> - current_remove_all will remove them from the pipe. */ >> - current_remove_all(display, surface_id); >> - red_clear_surface_drawables_from_pipes(display, surface_id, TRUE); >> -} >> - >> static void handle_dev_destroy_surface_wait(void *opaque, void *payload) >> { >> RedWorkerMessageDestroySurfaceWait *msg = payload; >> @@ -4144,34 +4007,6 @@ static void handle_dev_destroy_surface_wait(void >> *opaque, void *payload) >> display_channel_destroy_surface_wait(worker->display_channel, >> msg->surface_id); >> } >> >> -/* called upon device reset */ >> - >> -/* TODO: split me*/ >> -void display_channel_destroy_surfaces(DisplayChannel *display) >> -{ >> - int i; >> - >> - spice_debug(NULL); >> - //to handle better >> - for (i = 0; i < NUM_SURFACES; ++i) { >> - if (display->surfaces[i].context.canvas) { >> - display_channel_destroy_surface_wait(display, i); >> - if (display->surfaces[i].context.canvas) { >> - display_channel_surface_unref(display, i); >> - } >> - spice_assert(!display->surfaces[i].context.canvas); >> - } >> - } >> - spice_warn_if_fail(ring_is_empty(&display->streams)); >> - >> - if (red_channel_is_connected(RED_CHANNEL(display))) { >> - red_channel_pipes_add_type(RED_CHANNEL(display), >> PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE); >> - red_pipes_add_verb(RED_CHANNEL(display), >> SPICE_MSG_DISPLAY_STREAM_DESTROY_ALL); >> - } >> - >> - display_channel_free_glz_drawables(display); >> -} >> - >> static void handle_dev_destroy_surfaces(void *opaque, void *payload) >> { >> RedWorker *worker = opaque; >> @@ -4739,6 +4574,8 @@ static void register_callbacks(Dispatcher *dispatcher) >> dispatcher_register_async_done_callback( >> dispatcher, >> worker_handle_dispatcher_async_done); >> + >> + /* TODO: register cursor & display specific msg in respective channel >> files */ >> dispatcher_register_handler(dispatcher, >> RED_WORKER_MESSAGE_DISPLAY_CONNECT, >> handle_dev_display_connect, > > Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > Frediano > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel Acked-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx> _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel