[Spice-devel] [PATCH 13/18] worker: move destroy_surface() familly
Fabiano Fidêncio
fidencio at redhat.com
Wed Dec 2 08:04:53 PST 2015
On Wed, Nov 25, 2015 at 10:52 AM, Frediano Ziglio <fziglio at redhat.com> wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>>
>> ---
>> 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 at redhat.com>
>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
Acked-by: Fabiano Fidêncio <fidencio at redhat.com>
More information about the Spice-devel
mailing list