Re: [PATCH 13/18] worker: move destroy_surface() familly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]