On 2017-12-14 05:40 AM, Pekka Paalanen wrote:
From: Pekka Paalanen <pekka.paala...@collabora.co.uk>

The intention is that in the future backends will dynamically allocate
weston_heads based on the resources they have. The lifetime of a
weston_head will be independent of the lifetime of a weston_output it
may be attached to. Backends allocate objects derived from weston_head,
like they currently do for weston_output. Backend will choose when to
destroy a weston_head.

For clone mode, struct weston_output gains head_list member, which is
the list of attached heads that will all show the same framebuffer.
Since heads are growing out of weston_output, management functions are
added.

Detaching a head from an enabled output is allowed to accommodate
disappearing heads. Attaching a head to an enabled output is disallowed
because it may need hardware reconfiguration and testing, and so
requires a weston_output_enable() call.

As a temporary measure, we have one weston_head embedded in
weston_output, so that backends can be migrated individually to the new
allocation scheme.

Signed-off-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
---
  libweston/compositor.c | 216 +++++++++++++++++++++++++++++++++++++++----------
  libweston/compositor.h |   7 +-
  2 files changed, 181 insertions(+), 42 deletions(-)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index c668aa28..efa961dc 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -160,13 +160,12 @@ weston_mode_switch_finish(struct weston_output *output,
        if (!mode_changed && !scale_changed)
                return;
- head = &output->head;
-
        /* notify clients of the changes */
-       weston_mode_switch_send_events(head, mode_changed, scale_changed);
+       wl_list_for_each(head, &output->head_list, output_link)
+               weston_mode_switch_send_events(head,
+                                              mode_changed, scale_changed);
  }
-
  static void
  weston_compositor_reflow_outputs(struct weston_compositor *compositor,
                                struct weston_output *resized_output, int 
delta_width);
@@ -362,12 +361,13 @@ weston_presentation_feedback_present(
        struct wl_resource *o;
        uint64_t secs;
- head = &output->head;
-       wl_resource_for_each(o, &head->resource_list) {
-               if (wl_resource_get_client(o) != client)
-                       continue;
+       wl_list_for_each(head, &output->head_list, output_link) {
+               wl_resource_for_each(o, &head->resource_list) {
+                       if (wl_resource_get_client(o) != client)
+                               continue;
- wp_presentation_feedback_send_sync_output(feedback->resource, o);
+                       
wp_presentation_feedback_send_sync_output(feedback->resource, o);
+               }
        }
secs = ts->tv_sec;
@@ -988,6 +988,7 @@ weston_surface_update_output_mask(struct weston_surface 
*es, uint32_t mask)
        uint32_t left = es->output_mask & different;
        uint32_t output_bit;
        struct weston_output *output;
+       struct weston_head *head;
es->output_mask = mask;
        if (es->resource == NULL)
@@ -1000,9 +1001,11 @@ weston_surface_update_output_mask(struct weston_surface 
*es, uint32_t mask)
                if (!(output_bit & different))
                        continue;
- weston_surface_send_enter_leave(es, &output->head,
-                                               output_bit & entered,
-                                               output_bit & left);
+               wl_list_for_each(head, &output->head_list, output_link) {
+                       weston_surface_send_enter_leave(es, head,
+                                                       output_bit & entered,
+                                                       output_bit & left);
+               }
        }
  }
@@ -4386,6 +4389,98 @@ weston_head_from_resource(struct wl_resource *resource)
        return wl_resource_get_user_data(resource);
  }
+/** Initialize a pre-allocated weston_head
+ *
+ * \param head The head to initialize.
+ *
+ * The head will be safe to attach, detach and release.
+ *
+ * \memberof weston_head
+ * \internal
+ */
+static void
+weston_head_init(struct weston_head *head)
+{
+       /* Add some (in)sane defaults which can be used
+        * for checking if an output was properly configured
+        */
+       memset(head, 0, sizeof *head);
+
+       wl_list_init(&head->output_link);
+       wl_list_init(&head->resource_list);
+}
+
+/** Attach a head to an inactive output
+ *
+ * \param output The output to attach to.
+ * \param head The head that is not yet attached.
+ * \return 0 on success, -1 on failure.
+ *
+ * Attaches the given head to the output. All heads of an output are clones
+ * and share the resolution and timings.
+ *
+ * Cloning heads this way uses less resources than creating an output for
+ * each head, but is not always possible due to environment, driver and 
hardware
+ * limitations.
+ *
+ * On failure, the head remains unattached. Success of this function does not
+ * guarantee the output configuration is actually valid. The final checks are
+ * made on weston_output_enable().
+ *
+ * \memberof weston_output
+ */
+static int
+weston_output_attach_head(struct weston_output *output,
+                         struct weston_head *head)
+{
+       if (output->enabled)
+               return -1;

Would it be reasonable to make !output->enabled an assert()?

+
+       if (!wl_list_empty(&head->output_link))
+               return -1;
+
+       /* XXX: no support for multi-head yet */
+       if (!wl_list_empty(&output->head_list))
+               return -1;
+
+       head->output = output;
+       wl_list_insert(output->head_list.prev, &head->output_link);
+
+       return 0;
+}
+
+/** Detach a head from its output
+ *
+ * \param head The head to detach.
+ *
+ * It is safe to detach a non-attached head.
+ *
+ * \memberof weston_head
+ */
+static void
+weston_head_detach(struct weston_head *head)
+{
+       wl_list_remove(&head->output_link);
+       wl_list_init(&head->output_link);
+       head->output = NULL;
+}
+
+/** Destroy a head
+ *
+ * \param head The head to be released.
+ *
+ * Destroys the head. The caller is responsible for freeing the memory pointed
+ * to by \c head.
+ *
+ * \memberof weston_head
+ * \internal
+ */
+static void
+weston_head_release(struct weston_head *head)
+{
+       weston_head_detach(head);
+}
+
  /** Store monitor make, model and serial number
   *
   * \param head The head to modify.
@@ -4581,8 +4676,9 @@ weston_output_init_geometry(struct weston_output *output, 
int x, int y)
  WL_EXPORT void
  weston_output_move(struct weston_output *output, int x, int y)
  {
-       struct weston_head *head = &output->head;
+       struct weston_head *head;
        struct wl_resource *resource;
+       int ver;
output->move_x = x - output->x;
        output->move_y = y - output->y;
@@ -4598,19 +4694,22 @@ weston_output_move(struct weston_output *output, int x, 
int y)
        wl_signal_emit(&output->compositor->output_moved_signal, output);
/* Notify clients of the change for output position. */
-       wl_resource_for_each(resource, &head->resource_list) {
-               wl_output_send_geometry(resource,
-                                       output->x,
-                                       output->y,
-                                       head->mm_width,
-                                       head->mm_height,
-                                       head->subpixel,
-                                       head->make,
-                                       head->model,
-                                       output->transform);
-
-               if (wl_resource_get_version(resource) >= 
WL_OUTPUT_DONE_SINCE_VERSION)
-                       wl_output_send_done(resource);
+       wl_list_for_each(head, &output->head_list, output_link) {
+               wl_resource_for_each(resource, &head->resource_list) {
+                       wl_output_send_geometry(resource,
+                                               output->x,
+                                               output->y,
+                                               head->mm_width,
+                                               head->mm_height,
+                                               head->subpixel,
+                                               head->make,
+                                               head->model,
+                                               output->transform);
+
+                       ver = wl_resource_get_version(resource);
+                       if (ver >= WL_OUTPUT_DONE_SINCE_VERSION)
+                               wl_output_send_done(resource);
+               }
        }
  }
@@ -4650,11 +4749,11 @@ weston_compositor_add_output(struct weston_compositor *compositor,
        wl_list_insert(compositor->output_list.prev, &output->link);
        output->enabled = true;
- head = &output->head;
-       head->output = output;
-       head->global = wl_global_create(compositor->wl_display,
-                                       &wl_output_interface, 3,
-                                       head, bind_output);
+       wl_list_for_each(head, &output->head_list, output_link) {
+               head->global = wl_global_create(compositor->wl_display,
+                                               &wl_output_interface, 3,
+                                               head, bind_output);
+       }
wl_signal_emit(&compositor->output_created_signal, output); @@ -4747,11 +4846,12 @@ weston_compositor_remove_output(struct weston_output *output)
        wl_signal_emit(&compositor->output_destroyed_signal, output);
        wl_signal_emit(&output->destroy_signal, output);
- head = &output->head;
-       wl_global_destroy(head->global);
-       head->global = NULL;
-       wl_resource_for_each(resource, &head->resource_list) {
-               wl_resource_set_destructor(resource, NULL);
+       wl_list_for_each(head, &output->head_list, output_link) {
+               wl_global_destroy(head->global);
+               head->global = NULL;
+
+               wl_resource_for_each(resource, &head->resource_list)
+                       wl_resource_set_destructor(resource, NULL);
        }
compositor->output_id_pool &= ~(1u << output->id);
@@ -4829,19 +4929,19 @@ weston_output_init(struct weston_output *output,
                   struct weston_compositor *compositor,
                   const char *name)
  {
-       struct weston_head *head = &output->head;
-
        output->compositor = compositor;
        output->destroying = 0;
        output->name = strdup(name);
        wl_list_init(&output->link);
        output->enabled = false;
+ wl_list_init(&output->head_list);
+
+       weston_head_init(&output->head);
+
        /* Add some (in)sane defaults which can be used
         * for checking if an output was properly configured
         */
-       head->mm_width = 0;
-       head->mm_height = 0;
        output->scale = 0;
        /* Can't use -1 on uint32_t and 0 is valid enum value */
        output->transform = UINT32_MAX;
@@ -4915,6 +5015,7 @@ weston_output_enable(struct weston_output *output)
        struct weston_compositor *c = output->compositor;
        struct weston_output *iterator;
        int x = 0, y = 0;
+       int ret;
if (output->enabled) {
                weston_log("Error: attempt to enable an enabled output '%s'\n",
@@ -4948,9 +5049,14 @@ weston_output_enable(struct weston_output *output)
        wl_signal_init(&output->frame_signal);
        wl_signal_init(&output->destroy_signal);
        wl_list_init(&output->animation_list);
-       wl_list_init(&output->head.resource_list);
        wl_list_init(&output->feedback_list);
+ /* XXX: Temporary until all backends are converted. */
+       if (wl_list_empty(&output->head_list)) {
+               ret = weston_output_attach_head(output, &output->head);
+               assert(ret == 0);
+       }
+
        /* Enable the output (set up the crtc or create a
         * window representing the output, set up the
         * renderer, etc)
@@ -5042,6 +5148,8 @@ weston_pending_output_coldplug(struct weston_compositor 
*compositor)
  WL_EXPORT void
  weston_output_release(struct weston_output *output)
  {
+       struct weston_head *head;
+
        output->destroying = 1;
if (output->enabled)
@@ -5050,9 +5158,35 @@ weston_output_release(struct weston_output *output)
        pixman_region32_fini(&output->region);
        pixman_region32_fini(&output->previous_damage);
        wl_list_remove(&output->link);
+
+       while (!wl_list_empty(&output->head_list)) {
+               head = weston_output_get_first_head(output);
+               weston_head_detach(head);
+       }

I feel like I'm missing something here, but... this function looks multi-head aware, but depends on the "hacky" weston_output_get_first_head() function? Should this just be turned into a regular for_each_safe?

It seems like at the end of the series we're left with 4 callers to weston_output_get_first_head()... The cms-colord site seems potentially non-trivial to resolve. What else still needs to be made multi-head aware before the function can be removed?

In any event, everything up to this point is still:
Reviewed-by: Derek Foreman <der...@osg.samsung.com>

+
+       weston_head_release(&output->head);
+
        free(output->name);
  }
+/** When you need a head...
+ *
+ * This function is a hack, used until all code has been converted to become
+ * multi-head aware.
+ *
+ * \param output The weston_output whose head to get.
+ * \return The first head in the output's list.
+ */
+WL_EXPORT struct weston_head *
+weston_output_get_first_head(struct weston_output *output)
+{
+       if (wl_list_empty(&output->head_list))
+               return NULL;
+
+       return container_of(output->head_list.next,
+                           struct weston_head, output_link);
+}
+
  static void
  destroy_viewport(struct wl_resource *resource)
  {
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 5e0a2867..456adcc8 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -154,6 +154,7 @@ enum dpms_enum {
   */
  struct weston_head {
        struct weston_output *output;   /**< the output driving this head */
+       struct wl_list output_link;     /**< in weston_output::head_list */
struct wl_list resource_list; /**< wl_output protocol objects */
        struct wl_global *global;       /**< wl_output global */
@@ -226,7 +227,8 @@ struct weston_output {
        struct weston_mode *original_mode;
        struct wl_list mode_list;
- struct weston_head head;
+       struct weston_head head; /**< head for unconverted backends */
+       struct wl_list head_list; /**< List of driven weston_heads */
void (*start_repaint_loop)(struct weston_output *output);
        int (*repaint)(struct weston_output *output,
@@ -1993,6 +1995,9 @@ weston_pending_output_coldplug(struct weston_compositor 
*compositor);
  struct weston_head *
  weston_head_from_resource(struct wl_resource *resource);
+struct weston_head *
+weston_output_get_first_head(struct weston_output *output);
+
  #ifdef  __cplusplus
  }
  #endif


_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to