[PATCH weston v5 14/36] libweston: new head-based output management API

Derek Foreman derekf at osg.samsung.com
Fri Feb 2 21:00:09 UTC 2018


On 2017-12-14 05:40 AM, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> Introduce the API for users (compositors) to create an output from a
> head, attach and detach heads, and destroy outputs created this way.
> This also adds the backend-facing API to libweston.
> 
> In the new API design, a backend creates heads, and the compositor
> chooses one or more heads (clone mode) to be driven by an output.
> In the future backends will be converted to not create outputs directly
> but only in the new create_output hook.
> 
> The user subscribes to a heads_changed hook and arranges heads into
> outputs from there.
> 
> Adding the API this way will allow frontends (main.c) and backends to be
> converted one by one. This adds compatiblity paths in
> weston_compositor_create_output_with_head() and weston_output_destroy()
> so that frontends can be converted first to call these, and then
> backends can be converted one by one to the new design. Afterwards, the
> compatibility paths will be removed along with weston_output::head.
> 
> Currently heads can be added to a disabled output only. This is less
> than ideal for clone mode hotplug and should be improved on later.
> 
> v4: Remove the wl_output global on head detach if output is enabled.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>   libweston/compositor.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++---
>   libweston/compositor.h |  78 +++++++++++++++++++++
>   2 files changed, 256 insertions(+), 9 deletions(-)
> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 1d436522..a43ee277 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -4424,7 +4424,7 @@ weston_head_from_resource(struct wl_resource *resource)
>    * \memberof weston_head
>    * \internal
>    */
> -static void
> +WL_EXPORT void
>   weston_head_init(struct weston_head *head, const char *name)
>   {
>   	/* Add some (in)sane defaults which can be used
> @@ -4483,7 +4483,7 @@ weston_compositor_schedule_heads_changed(struct weston_compositor *compositor)
>    * \memberof weston_compositor
>    * \internal
>    */
> -static void
> +WL_EXPORT void
>   weston_compositor_add_head(struct weston_compositor *compositor,
>   			   struct weston_head *head)
>   {
> @@ -4564,6 +4564,52 @@ weston_compositor_iterate_heads(struct weston_compositor *compositor,
>   	return container_of(node, struct weston_head, compositor_link);
>   }
>   
> +/** Iterate over attached heads
> + *
> + * \param output The output whose heads to iterate.
> + * \param item The iterator, or NULL for start.
> + * \return The next attached head in the list.
> + *
> + * Returns all heads currently attached to the output.
> + *
> + * You can iterate over all heads as follows:
> + * \code
> + * struct weston_head *head = NULL;
> + *
> + * while ((head = weston_output_iterate_heads(output, head))) {
> + * 	...
> + * }
> + * \endcode
> + *
> + *  If you cause \c iter to be removed from the list, you cannot use it to
> + * continue iterating. Removing any other item is safe.
> + *
> + * \memberof weston_compositor
> + */
> +WL_EXPORT struct weston_head *
> +weston_output_iterate_heads(struct weston_output *output,
> +			    struct weston_head *iter)
> +{
> +	struct wl_list *list = &output->head_list;
> +	struct wl_list *node;
> +
> +	assert(output);
> +	assert(!iter || iter->output == output);
> +
> +	if (iter)
> +		node = iter->output_link.next;
> +	else
> +		node = list->next;
> +
> +	assert(node);
> +	assert(!iter || node != &iter->output_link);
> +
> +	if (node == list)
> +		return NULL;
> +
> +	return container_of(node, struct weston_head, output_link);
> +}
> +
>   /** Attach a head to an inactive output
>    *
>    * \param output The output to attach to.
> @@ -4583,7 +4629,7 @@ weston_compositor_iterate_heads(struct weston_compositor *compositor,
>    *
>    * \memberof weston_output
>    */
> -static int
> +WL_EXPORT int
>   weston_output_attach_head(struct weston_output *output,
>   			  struct weston_head *head)
>   {
> @@ -4593,9 +4639,13 @@ weston_output_attach_head(struct weston_output *output,
>   	if (!wl_list_empty(&head->output_link))
>   		return -1;
>   
> -	/* XXX: no support for multi-head yet */
> -	if (!wl_list_empty(&output->head_list))
> +	if (output->attach_head) {
> +		if (output->attach_head(output, head) < 0)
> +			return -1;
> +	} else if (!wl_list_empty(&output->head_list)) {
> +		/* No support for clones in the legacy path. */
>   		return -1;
> +	}
>   
>   	head->output = output;
>   	wl_list_insert(output->head_list.prev, &head->output_link);
> @@ -4609,14 +4659,33 @@ weston_output_attach_head(struct weston_output *output,
>    *
>    * It is safe to detach a non-attached head.
>    *
> + * If the head is attached to an enabled output and the output will be left
> + * with no heads, the output will be disabled.
> + *
>    * \memberof weston_head
> + * \sa weston_output_disable
>    */
> -static void
> +WL_EXPORT void
>   weston_head_detach(struct weston_head *head)
>   {
> +	struct weston_output *output = head->output;
> +
>   	wl_list_remove(&head->output_link);
>   	wl_list_init(&head->output_link);
>   	head->output = NULL;
> +
> +	if (!output)
> +		return;
> +
> +	if (output->detach_head)
> +		output->detach_head(output, head);
> +
> +	if (output->enabled) {
> +		weston_head_remove_global(head);
> +
> +		if (wl_list_empty(&output->head_list))
> +			weston_output_disable(output);
> +	}
>   }
>   
>   /** Destroy a head
> @@ -4629,7 +4698,7 @@ weston_head_detach(struct weston_head *head)
>    * \memberof weston_head
>    * \internal
>    */
> -static void
> +WL_EXPORT void
>   weston_head_release(struct weston_head *head)
>   {
>   	weston_head_detach(head);
> @@ -4799,6 +4868,31 @@ weston_head_is_enabled(struct weston_head *head)
>   	return head->output->enabled;
>   }
>   
> +/** Get the name of a head
> + *
> + * \param head The head to query.
> + * \return The head's name, not NULL.
> + *
> + * The name depends on the backend. The DRM backend uses connector names,
> + * other backends may use hardcoded names or user-given names.
> + */
> +WL_EXPORT const char *
> +weston_head_get_name(struct weston_head *head)
> +{
> +	return head->name;
> +}
> +
> +/** Get the output the head is attached to
> + *
> + * \param head The head to query.
> + * \return The output the head is attached to, or NULL if detached.
> + */
> +WL_EXPORT struct weston_output *
> +weston_head_get_output(struct weston_head *head)
> +{
> +	return head->output;
> +}
> +
>   /* Move other outputs when one is resized so the space remains contiguous. */
>   static void
>   weston_compositor_reflow_outputs(struct weston_compositor *compositor,
> @@ -5170,8 +5264,11 @@ weston_output_init(struct weston_output *output,
>   	wl_list_init(&output->head_list);
>   
>   	weston_head_init(&output->head, name);
> -	weston_head_set_connection_status(&output->head, true);
> -	weston_compositor_add_head(compositor, &output->head);
> +	output->head.allocator_output = output;
> +	if (!compositor->backend->create_output) {
> +		weston_head_set_connection_status(&output->head, true);
> +		weston_compositor_add_head(compositor, &output->head);
> +	}
>   
>   	/* Add some (in)sane defaults which can be used
>   	 * for checking if an output was properly configured
> @@ -5408,6 +5505,78 @@ weston_output_release(struct weston_output *output)
>   	free(output->name);
>   }
>   
> +/** Create an output for an unused head
> + *
> + * \param compositor The compositor.
> + * \param head The head to attach to the output.
> + * \return A new \c weston_output, or NULL on failure.
> + *
> + * This creates a new weston_output that starts with the given head attached.
> + * The output inherits the name of the head. The head must not be already
> + * attached to another output.
> + *
> + * An output must be configured before it can be enabled.
> + *
> + * \memberof weston_compositor
> + */
> +WL_EXPORT struct weston_output *
> +weston_compositor_create_output_with_head(struct weston_compositor *compositor,
> +					  struct weston_head *head)
> +{
> +	struct weston_output *output;
> +
> +	if (head->allocator_output) {
> +		/* XXX: compatibility path to be removed after all converted */
> +		output = head->allocator_output;
> +	} else {
> +		assert(compositor->backend->create_output);
> +		output = compositor->backend->create_output(compositor,
> +							    head->name);
> +	}
> +
> +	if (!output)
> +		return NULL;
> +
> +	if (weston_output_attach_head(output, head) < 0) {
> +		if (!head->allocator_output)
> +			output->destroy(output);
> +
> +		return NULL;
> +	}
> +
> +	return output;
> +}
> +
> +/** Destroy an output
> + *
> + * \param output The output to destroy.
> + *
> + * The heads attached to the given output are detached and become unused again.
> + *
> + * It is not necessary to explicitly destroy all outputs at compositor exit.
> + * weston_compositor_destroy() will automatically destroy any remaining
> + * outputs.
> + *
> + * \memberof weston_output
> + */
> +WL_EXPORT void
> +weston_output_destroy(struct weston_output *output)
> +{
> +	struct weston_head *head;
> +
> +	/* XXX: compatibility path to be removed after all converted */
> +	head = weston_output_get_first_head(output);

This took me a couple of reads...  if I understand correctly, the full 
list of outputs is going to be torn down in the backend 
output->destroy()?  And we hit that path if we don't have the legacy 
allocator_output.

I think the code all looks sound and the API looks reasonable to me, but 
I have to concede that I don't know the output handling path enough to 
understand weston_compositor_reflow_output's setting up of the 
allocator_output.

Acked-by: Derek Foreman <derekf at osg.samsung.com>

Thanks,
Derek

> +	if (head->allocator_output) {
> +		/* The old design: backend is responsible for destroying the
> +		 * output, so just undo create_output_with_head()
> +		 */
> +		weston_head_detach(head);
> +		return;
> +	}
> +
> +	output->destroy(output);
> +}
> +
>   /** When you need a head...
>    *
>    * This function is a hack, used until all code has been converted to become
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index f7b7050c..f9d034c3 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -172,6 +172,8 @@ struct weston_head {
>   
>   	char *name;			/**< head name, e.g. connector name */
>   	bool connected;			/**< is physically connected */
> +
> +	struct weston_output *allocator_output;	/**< XXX: to be removed */
>   };
>   
>   struct weston_output {
> @@ -263,6 +265,33 @@ struct weston_output {
>   
>   	int (*enable)(struct weston_output *output);
>   	int (*disable)(struct weston_output *output);
> +
> +	/** Attach a head in the backend
> +	 *
> +	 * @param output The output to attach to.
> +	 * @param head The head to attach.
> +	 * @return 0 on success, -1 on failure.
> +	 *
> +	 * Do anything necessary to account for a new head being attached to
> +	 * the output, and check any conditions possible. On failure, both
> +	 * the head and the output must be left as before the call.
> +	 *
> +	 * Libweston core will add the head to the head_list after a successful
> +	 * call.
> +	 */
> +	int (*attach_head)(struct weston_output *output,
> +			   struct weston_head *head);
> +
> +	/** Detach a head in the backend
> +	 *
> +	 * @param output The output to detach from.
> +	 * @param head The head to detach.
> +	 *
> +	 * Do any clean-up necessary to detach this head from the output.
> +	 * The head has already been removed from the output's head_list.
> +	 */
> +	void (*detach_head)(struct weston_output *output,
> +			    struct weston_head *head);
>   };
>   
>   enum weston_pointer_motion_mask {
> @@ -878,6 +907,21 @@ struct weston_backend {
>   	 */
>   	void (*repaint_flush)(struct weston_compositor *compositor,
>   			      void *repaint_data);
> +
> +	/** Allocate a new output
> +	 *
> +	 * @param compositor The compositor.
> +	 * @param name Name for the new output.
> +	 *
> +	 * Allocates a new output structure that embeds a weston_output,
> +	 * initializes it, and returns the pointer to the weston_output
> +	 * member.
> +	 *
> +	 * Must set weston_output members @c destroy, @c enable and @c disable.
> +	 */
> +	struct weston_output *
> +	(*create_output)(struct weston_compositor *compositor,
> +			 const char *name);
>   };
>   
>   struct weston_desktop_xwayland;
> @@ -1961,6 +2005,16 @@ int
>   weston_compositor_load_xwayland(struct weston_compositor *compositor);
>   
>   void
> +weston_head_init(struct weston_head *head, const char *name);
> +
> +void
> +weston_head_release(struct weston_head *head);
> +
> +void
> +weston_compositor_add_head(struct weston_compositor *compositor,
> +			   struct weston_head *head);
> +
> +void
>   weston_head_set_monitor_strings(struct weston_head *head,
>   				const char *make,
>   				const char *model,
> @@ -1986,6 +2040,15 @@ weston_head_is_connected(struct weston_head *head);
>   bool
>   weston_head_is_enabled(struct weston_head *head);
>   
> +const char *
> +weston_head_get_name(struct weston_head *head);
> +
> +struct weston_output *
> +weston_head_get_output(struct weston_head *head);
> +
> +void
> +weston_head_detach(struct weston_head *head);
> +
>   struct weston_head *
>   weston_compositor_iterate_heads(struct weston_compositor *compositor,
>   				struct weston_head *iter);
> @@ -1994,6 +2057,21 @@ void
>   weston_compositor_set_heads_changed_cb(struct weston_compositor *compositor,
>   				       weston_heads_changed_func_t cb);
>   
> +struct weston_output *
> +weston_compositor_create_output_with_head(struct weston_compositor *compositor,
> +					  struct weston_head *head);
> +
> +void
> +weston_output_destroy(struct weston_output *output);
> +
> +int
> +weston_output_attach_head(struct weston_output *output,
> +			  struct weston_head *head);
> +
> +struct weston_head *
> +weston_output_iterate_heads(struct weston_output *output,
> +			    struct weston_head *iter);
> +
>   void
>   weston_output_set_scale(struct weston_output *output,
>   			int32_t scale);
> 



More information about the wayland-devel mailing list