Re: [PATCH 12/18] worker: move display_channel_new

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

 



> 
> Reduced diff:
> 
> 
> --- before.c	2015-11-25 10:09:19.413943460 +0000
> +++ after.c	2015-11-25 09:56:05.709537277 +0000
> @@ -1700,20 +1700,21 @@
>                                                                        RedsStream
>                                                                        *stream,
>                                                                        int
>                                                                        mig_target,
>                                                                        uint32_t
>                                                                        *common_caps,
>                                                                        int
>                                                                        num_common_caps,
>                                                                        uint32_t
>                                                                        *caps,
>                                                                        int
>                                                                        num_caps,
>                                                                        SpiceImageCompression
>                                                                        image_compression,
>                                                                        spice_wan_compression_t
>                                                                        jpeg_state,
>                                                                        spice_wan_compression_t
>                                                                        zlib_glz_state);
>  void                       dcc_start
>  (DisplayChannelClient *dcc);
> +void                       dcc_stop
> (DisplayChannelClient *dcc);
>  int                        dcc_handle_message
>  (RedChannelClient *rcc,
>                                                                        uint32_t
>                                                                        size,
>                                                                        uint16_t
>                                                                        type,
>                                                                        void
>                                                                        *msg);
>  int                        dcc_handle_migrate_data
>  (DisplayChannelClient *dcc,
>                                                                        uint32_t
>                                                                        size,
>                                                                        void
>                                                                        *message);
>  void                       dcc_push_monitors_config
>  (DisplayChannelClient *dcc);
>  void                       dcc_destroy_surface
>  (DisplayChannelClient *dcc,
>                                                                        uint32_t
>                                                                        surface_id);
>  void                       dcc_stream_agent_clip
>  (DisplayChannelClient* dcc,
>                                                                        StreamAgent
>                                                                        *agent);
> @@ -1737,20 +1738,22 @@
>  void                       dcc_prepend_drawable
>  (DisplayChannelClient *dcc,
>                                                                        Drawable
>                                                                        *drawable);
>  void                       dcc_append_drawable
>  (DisplayChannelClient *dcc,
>                                                                        Drawable
>                                                                        *drawable);
>  void                       dcc_add_drawable_after
>  (DisplayChannelClient *dcc,
>                                                                        Drawable
>                                                                        *drawable,
>                                                                        PipeItem
>                                                                        *pos);
>  void                       dcc_release_item
>  (DisplayChannelClient *dcc,
>                                                                        PipeItem
>                                                                        *item,
>                                                                        int
>                                                                        item_pushed);
> +void                       dcc_send_item
> (DisplayChannelClient *dcc,
> +
> PipeItem
> *item);
>  
>  typedef struct compress_send_data_t {
>      void*    comp_buf;
>      uint32_t comp_buf_size;
>      SpicePalette *lzplt_palette;
>      int is_lossy;
>  } compress_send_data_t;
>  
>  int                        dcc_compress_image
>  (DisplayChannelClient *dcc,
>                                                                        SpiceImage
>                                                                        *dest,
>                                                                        SpiceBitmap
>                                                                        *src,
>                                                                        Drawable
>                                                                        *drawable,
> @@ -3175,58 +3178,62 @@
>          return;
>  
>      draw_until(display, surface, last);
>      surface_update_dest(surface, area);
>  }
>  
>  static void on_disconnect(RedChannelClient *rcc)
>  {
>      DisplayChannel *display;
>      DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> -    CommonChannel *common;
>      RedWorker *worker;
>  
> -    if (!rcc) {
> -        return;
> -    }
>      spice_info(NULL);
> -    common = SPICE_CONTAINEROF(rcc->channel, CommonChannel, base);
> -    worker = common->worker;
> -    display = (DisplayChannel *)rcc->channel;
> -    spice_assert(display == worker->display_channel);
> -    display_channel_compress_stats_print(display);
> +    spice_return_if_fail(rcc != NULL);

The spice_info(NULL) is moved before the check for rcc NULL.
Not change much as rcc should be not NULL here.

> +
> +    display = DCC_TO_DC(dcc);
> +    worker = COMMON_CHANNEL(display)->worker;
> +    spice_return_if_fail(rcc->channel ==
> red_worker_get_display_channel(worker));

the worker is used only for the check.
I suspect this was changed when we moved some fields from worker to display.
The two lines above (and worker variable declaration) can be removed.

> +
>      dcc_stop(dcc); // TODO: start/stop -> connect/disconnect?
> +    display_channel_compress_stats_print(display);
>  

Here compression statistics are printed after stopping the dcc.
However is not a problem, they just access data from display.

>      // this was the last channel client
>      spice_debug("#draw=%d, #red_draw=%d, #glz_draw=%d",
>                  display->drawable_count, display->red_drawable_count,
>                  display->glz_drawable_count);
>  }
>  
> +static void send_item(RedChannelClient *rcc, PipeItem *item)
> +{
> +    dcc_send_item(RCC_TO_DCC(rcc), item);
> +}
> +

This is required as send_item was renamed as dcc_send_item and first parameter
changed to DisplayChannelClient.
Perhaps adding an inline could help the compiler but I don't think that
much.

>  static void hold_item(RedChannelClient *rcc, PipeItem *item)
>  {
> -    spice_assert(item);
> +    spice_return_if_fail(item);
> +

Still prefer the spice_assert but in this case does not harm

>      switch (item->type) {
>      case PIPE_ITEM_TYPE_DRAW:
>          drawable_pipe_item_ref(SPICE_CONTAINEROF(item, DrawablePipeItem,
>          dpi_pipe_item));
>          break;
>      case PIPE_ITEM_TYPE_STREAM_CLIP:
>          ((StreamClipItem *)item)->refs++;
>          break;
>      case PIPE_ITEM_TYPE_UPGRADE:
>          ((UpgradeItem *)item)->refs++;
>          break;
>      case PIPE_ITEM_TYPE_IMAGE:
>          ((ImageItem *)item)->refs++;
>          break;
>      default:
> -        spice_critical("invalid item type");
> +        spice_warn_if_reached();
>      }
>  }
>  
>  static void release_item(RedChannelClient *rcc, PipeItem *item, int
>  item_pushed)
>  {
>      DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
>  
>      spice_return_if_fail(item != NULL);
>      dcc_release_item(dcc, item, item_pushed);
>  }
> @@ -3256,52 +3263,50 @@
>  
>  static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces,
>  uint32_t surface_id)
>  {
>      DisplayChannel *display = SPICE_CONTAINEROF(surfaces, DisplayChannel,
>      image_surfaces);
>  
>      spice_return_val_if_fail(validate_surface(display, surface_id), NULL);
>  
>      return display->surfaces[surface_id].context.canvas;
>  }
>  
> -static void display_channel_create(RedWorker *worker, int migrate, int
> stream_video,
> +DisplayChannel* display_channel_new(RedWorker *worker, int migrate, int
> stream_video,
>                                     uint32_t n_surfaces)
>  {
>      DisplayChannel *display;
>      ChannelCbs cbs = {
>          .on_disconnect = on_disconnect,
>          .send_item = send_item,
>          .hold_item = hold_item,
>          .release_item = release_item,
>          .handle_migrate_flush_mark = handle_migrate_flush_mark,
>          .handle_migrate_data = handle_migrate_data,
>          .handle_migrate_data_get_serial = handle_migrate_data_get_serial
>      };
>      static SpiceImageSurfacesOps image_surfaces_ops = {
>          image_surfaces_get,
>      };
>  
> -    spice_return_if_fail(num_renderers > 0);
> +    spice_return_val_if_fail(num_renderers > 0, NULL);
>  
>      spice_info("create display channel");
> -    if (!(display = (DisplayChannel *)red_worker_new_channel(
> +    display = (DisplayChannel *)red_worker_new_channel(
>              worker, sizeof(*display), "display_channel",
>              SPICE_CHANNEL_DISPLAY,
>              SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER,
> -            &cbs, dcc_handle_message))) {
> -        spice_warning("failed to create display channel");
> -        return;
> -    }
> -    worker->display_channel = display;
> -    stat_init(&display->add_stat, "add", worker->clockid);
> -    stat_init(&display->exclude_stat, "exclude", worker->clockid);
> -    stat_init(&display->__exclude_stat, "__exclude", worker->clockid);
> +        &cbs, dcc_handle_message);
> +    spice_return_val_if_fail(display, NULL);
> +
> +    stat_init(&display->add_stat, "add", red_worker_get_clockid(worker));
> +    stat_init(&display->exclude_stat, "exclude",
> red_worker_get_clockid(worker));
> +    stat_init(&display->__exclude_stat, "__exclude",
> red_worker_get_clockid(worker));
>  #ifdef RED_STATISTICS
>      RedChannel *channel = RED_CHANNEL(display);
>      display->cache_hits_counter = stat_add_counter(channel->stat,
>                                                             "cache_hits",
>                                                             TRUE);
>      display->add_to_cache_counter = stat_add_counter(channel->stat,
>                                                               "add_to_cache",
>                                                               TRUE);
>      display->non_cache_counter = stat_add_counter(channel->stat,
>                                                            "non_cache",
>                                                            TRUE);
>  #endif
>      stat_compress_init(&display->lz_stat, "lz");
> @@ -3316,22 +3321,23 @@
>      display->num_renderers = num_renderers;
>      memcpy(display->renderers, renderers, sizeof(display->renderers));
>      display->renderer = RED_RENDERER_INVALID;
>  
>      ring_init(&display->current_list);
>      display->image_surfaces.ops = &image_surfaces_ops;
>      drawables_init(display);
>      image_cache_init(&display->image_cache);
>      display->stream_video = stream_video;
>      display_channel_init_streams(display);
> -}
>  
> +    return display;
> +}
>  /* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
>  /*
>     Copyright (C) 2009-2015 Red Hat, Inc.
>  
>     This library is free software; you can redistribute it and/or
>     modify it under the terms of the GNU Lesser General Public
>     License as published by the Free Software Foundation; either
>     version 2.1 of the License, or (at your option) any later version.
>  
>     This library is distributed in the hope that it will be useful,
> @@ -3571,20 +3577,24 @@
>  } SurfaceDestroyItem;
>  
>  typedef struct UpgradeItem {
>      PipeItem base;
>      int refs;
>      Drawable *drawable;
>      SpiceClipRects *rects;
>  } UpgradeItem;
>  
>  
> +DisplayChannel*            display_channel_new
> (RedWorker *worker,
> +                                                                      int
> migrate,
> +                                                                      int
> stream_video,
> +
> uint32_t
> n_surfaces);
>  void                       display_channel_draw
>  (DisplayChannel *display,
>                                                                        const
>                                                                        SpiceRect
>                                                                        *area,
>                                                                        int
>                                                                        surface_id);
>  void                       display_channel_draw_until
>  (DisplayChannel *display,
>                                                                        const
>                                                                        SpiceRect
>                                                                        *area,
>                                                                        int
>                                                                        surface_id,
>                                                                        Drawable
>                                                                        *last);
>  void                       display_channel_free_some
>  (DisplayChannel *display);
>  void                       display_channel_set_stream_video
>  (DisplayChannel *display,
>                                                                        int
>                                                                        stream_video);
> @@ -3877,22 +3887,20 @@
>      BITMAP_DATA_TYPE_BITMAP,
>      BITMAP_DATA_TYPE_BITMAP_TO_CACHE,
>  } BitmapDataType;
>  
>  typedef struct BitmapData {
>      BitmapDataType type;
>      uint64_t id; // surface id or cache item id
>      SpiceRect lossy_rect;
>  } BitmapData;
>  
> -static inline int validate_surface(DisplayChannel *display, uint32_t
> surface_id);
> -
>  static inline void display_begin_send_message(RedChannelClient *rcc);
>  static void red_create_surface(DisplayChannel *display, uint32_t surface_id,
>  uint32_t width,
>                                 uint32_t height, int32_t stride, uint32_t
>                                 format,
>                                 void *line_0, int data_is_valid, int
>                                 send_client);
>  
>  QXLInstance* red_worker_get_qxl(RedWorker *worker)
>  {
>      spice_return_val_if_fail(worker != NULL, NULL);
>  
>      return worker->qxl;
> @@ -7133,33 +7141,33 @@
>      SpiceMsgDisplayStreamActivateReport msg;
>  
>      red_channel_client_init_send_data(rcc,
>      SPICE_MSG_DISPLAY_STREAM_ACTIVATE_REPORT, NULL);
>      msg.stream_id = stream_id;
>      msg.unique_id = agent->report_id;
>      msg.max_window_size = RED_STREAM_CLIENT_REPORT_WINDOW;
>      msg.timeout_ms = RED_STREAM_CLIENT_REPORT_TIMEOUT;
>      spice_marshall_msg_display_stream_activate_report(base_marshaller,
>      &msg);
>  }
>  
> -static inline void red_display_reset_send_data(DisplayChannelClient *dcc)
> +static void reset_send_data(DisplayChannelClient *dcc)
>  {
>      dcc->send_data.free_list.res->count = 0;
>      dcc->send_data.num_pixmap_cache_items = 0;
>      memset(dcc->send_data.free_list.sync, 0,
>      sizeof(dcc->send_data.free_list.sync));
>  }
>  
> -static void send_item(RedChannelClient *rcc, PipeItem *pipe_item)
> +void dcc_send_item(DisplayChannelClient *dcc, PipeItem *pipe_item)
>  {
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(dcc);
>      SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> -    DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
>  
> -    red_display_reset_send_data(dcc);
> +    reset_send_data(dcc);
>      switch (pipe_item->type) {
>      case PIPE_ITEM_TYPE_DRAW: {
>          DrawablePipeItem *dpi = SPICE_CONTAINEROF(pipe_item,
>          DrawablePipeItem, dpi_pipe_item);
>          marshall_qxl_drawable(rcc, m, dpi);
>          break;
>      }
>      case PIPE_ITEM_TYPE_INVAL_ONE:
>          red_marshall_inval_palette(rcc, m, (CacheItem *)pipe_item);
>          break;
>      case PIPE_ITEM_TYPE_STREAM_CREATE: {
> @@ -8724,21 +8732,22 @@
>                            init_info.memslot_gen_bits,
>                            init_info.memslot_id_bits,
>                            init_info.internal_groupslot_id);
>  
>      spice_warn_if(init_info.n_surfaces > NUM_SURFACES);
>  
>      worker->event_timeout = INF_EVENT_WAIT;
>  
>      worker->cursor_channel = cursor_channel_new(worker);
>      // TODO: handle seemless migration. Temp, setting migrate to FALSE
> -    display_channel_create(worker, FALSE, streaming_video,
> init_info.n_surfaces);
> +    worker->display_channel = display_channel_new(worker, FALSE,
> streaming_video,
> +                                                  init_info.n_surfaces);
>  
>      return worker;
>  }
>  
>  SPICE_GNUC_NORETURN static void *red_worker_main(void *arg)
>  {
>      RedWorker *worker = arg;
>  
>      spice_info("begin");
>      spice_assert(MAX_PIPE_SIZE > WIDE_CLIENT_ACK_WINDOW &&

I'll remove the 2 useless lines above.

Frediano
_______________________________________________
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]