[Spice-devel] [PATCH 13/18] worker: move destroy_surface() familly

Frediano Ziglio fziglio at redhat.com
Wed Nov 25 01:52:34 PST 2015


> 
> 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


More information about the Spice-devel mailing list