[PATCH weston v5 06/36] libweston: introduce weston_output::head_list
Derek Foreman
derekf at osg.samsung.com
Thu Feb 1 21:36:55 UTC 2018
On 2017-12-14 05:40 AM, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at 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.paalanen at 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 <derekf at 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
>
More information about the wayland-devel
mailing list