[PATCH weston v5 01/36] libweston: introduce weston_head

Derek Foreman derekf at osg.samsung.com
Thu Feb 1 19:59:24 UTC 2018


On 2017-12-14 05:40 AM, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> In order to support clone modes, libweston needs the concept of a head
> that is separate from weston_output. While weston_output manages buffers
> and the repaint state machine, weston_head will represent a single
> monitor. In the future it will be possible to have a single
> weston_output drive one or more weston_heads for a clone mode that
> shares the framebuffers between all cloned heads.
> 
> All the fields that are obviously properties of the monitor are moved
> from weston_output into weston_head.
> 
> As moving the fields requires one to touch all the backends for all the
> assingments, introduce setter functions for them while we are here. The
> setters are identical to the old assignments, for now.
> 
> As a temporary measure, weston_output embeds a single head. Also the
> ugly casts in weston_head_set_monitor_strings() will be removed by a
> follow-up patch.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>   compositor/cms-colord.c         |  32 ++++++------
>   libweston/compositor-drm.c      |  15 +++---
>   libweston/compositor-fbdev.c    |  12 +++--
>   libweston/compositor-headless.c |   8 +--
>   libweston/compositor-rdp.c      |   8 +--
>   libweston/compositor-wayland.c  |  18 ++++---
>   libweston/compositor-x11.c      |  13 +++--
>   libweston/compositor.c          | 105 +++++++++++++++++++++++++++++++++++-----
>   libweston/compositor.h          |  38 +++++++++++++--
>   9 files changed, 184 insertions(+), 65 deletions(-)
> 
> diff --git a/compositor/cms-colord.c b/compositor/cms-colord.c
> index 0daa2a7e..f421773b 100644
> --- a/compositor/cms-colord.c
> +++ b/compositor/cms-colord.c
> @@ -102,22 +102,23 @@ edid_value_valid(const char *str)
>   static gchar *
>   get_output_id(struct cms_colord *cms, struct weston_output *o)
>   {
> +	struct weston_head *head = &o->head;
>   	const gchar *tmp;
>   	GString *device_id;
>   
>   	/* see https://github.com/hughsie/colord/blob/master/doc/device-and-profile-naming-spec.txt
>   	 * for format and allowed values */
>   	device_id = g_string_new("xrandr");
> -	if (edid_value_valid(o->make)) {
> -		tmp = g_hash_table_lookup(cms->pnp_ids, o->make);
> +	if (edid_value_valid(head->make)) {
> +		tmp = g_hash_table_lookup(cms->pnp_ids, head->make);
>   		if (tmp == NULL)
> -			tmp = o->make;
> +			tmp = head->make;
>   		g_string_append_printf(device_id, "-%s", tmp);
>   	}
> -	if (edid_value_valid(o->model))
> -		g_string_append_printf(device_id, "-%s", o->model);
> -	if (edid_value_valid(o->serial_number))
> -		g_string_append_printf(device_id, "-%s", o->serial_number);
> +	if (edid_value_valid(head->model))
> +		g_string_append_printf(device_id, "-%s", head->model);
> +	if (edid_value_valid(head->serial_number))
> +		g_string_append_printf(device_id, "-%s", head->serial_number);
>   
>   	/* no EDID data, so use fallback */
>   	if (strcmp(device_id->str, "xrandr") == 0)
> @@ -230,6 +231,7 @@ colord_notifier_output_destroy(struct wl_listener *listener, void *data)
>   static void
>   colord_output_created(struct cms_colord *cms, struct weston_output *o)
>   {
> +	struct weston_head *head = &o->head;
>   	CdDevice *device;
>   	const gchar *tmp;
>   	gchar *device_id;
> @@ -251,25 +253,25 @@ colord_output_created(struct cms_colord *cms, struct weston_output *o)
>   	g_hash_table_insert (device_props,
>   			     g_strdup(CD_DEVICE_PROPERTY_COLORSPACE),
>   			     g_strdup(cd_colorspace_to_string(CD_COLORSPACE_RGB)));
> -	if (edid_value_valid(o->make)) {
> -		tmp = g_hash_table_lookup(cms->pnp_ids, o->make);
> +	if (edid_value_valid(head->make)) {
> +		tmp = g_hash_table_lookup(cms->pnp_ids, head->make);
>   		if (tmp == NULL)
> -			tmp = o->make;
> +			tmp = head->make;
>   		g_hash_table_insert (device_props,
>   				     g_strdup(CD_DEVICE_PROPERTY_VENDOR),
>   				     g_strdup(tmp));
>   	}
> -	if (edid_value_valid(o->model)) {
> +	if (edid_value_valid(head->model)) {
>   		g_hash_table_insert (device_props,
>   				     g_strdup(CD_DEVICE_PROPERTY_MODEL),
> -				     g_strdup(o->model));
> +				     g_strdup(head->model));
>   	}
> -	if (edid_value_valid(o->serial_number)) {
> +	if (edid_value_valid(head->serial_number)) {
>   		g_hash_table_insert (device_props,
>   				     g_strdup(CD_DEVICE_PROPERTY_SERIAL),
> -				     g_strdup(o->serial_number));
> +				     g_strdup(head->serial_number));
>   	}
> -	if (o->connection_internal) {
> +	if (head->connection_internal) {
>   		g_hash_table_insert (device_props,
>   				     g_strdup (CD_DEVICE_PROPERTY_EMBEDDED),
>   				     NULL);
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index b77209c4..b312f5d4 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -3339,6 +3339,7 @@ create_output_for_connector(struct drm_backend *b,
>   			    struct udev_device *drm_device)
>   {
>   	struct drm_output *output;
> +	struct weston_head *head;
>   	drmModeObjectPropertiesPtr props;
>   	struct drm_mode *drm_mode;
>   	char *name;
> @@ -3391,19 +3392,19 @@ create_output_for_connector(struct drm_backend *b,
>   	}
>   	drm_property_info_populate(b, connector_props, output->props_conn,
>   				   WDRM_CONNECTOR__COUNT, props);
> +	head = &output->base.head;
>   	find_and_parse_output_edid(b, output, props,
>   				   &make, &model, &serial_number);
> -	output->base.make = (char *)make;
> -	output->base.model = (char *)model;
> -	output->base.serial_number = (char *)serial_number;
> -	output->base.subpixel = drm_subpixel_to_wayland(output->connector->subpixel);
> +	weston_head_set_monitor_strings(head, make, model, serial_number);
> +	weston_head_set_subpixel(head,
> +		drm_subpixel_to_wayland(output->connector->subpixel));
>   
>   	if (output->connector->connector_type == DRM_MODE_CONNECTOR_LVDS ||
>   	    output->connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> -		output->base.connection_internal = true;
> +		weston_head_set_internal(head);
>   
> -	output->base.mm_width = output->connector->mmWidth;
> -	output->base.mm_height = output->connector->mmHeight;
> +	weston_head_set_physical_size(head, output->connector->mmWidth,
> +				      output->connector->mmHeight);
>   
>   	drmModeFreeObjectProperties(props);
>   
> diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
> index fbab634b..c71d7eb5 100644
> --- a/libweston/compositor-fbdev.c
> +++ b/libweston/compositor-fbdev.c
> @@ -499,6 +499,7 @@ fbdev_output_create(struct fbdev_backend *backend,
>                       const char *device)
>   {
>   	struct fbdev_output *output;
> +	struct weston_head *head;
>   	int fb_fd;
>   
>   	weston_log("Creating fbdev output.\n");
> @@ -532,12 +533,13 @@ fbdev_output_create(struct fbdev_backend *backend,
>   	wl_list_insert(&output->base.mode_list, &output->mode.link);
>   
>   	output->base.current_mode = &output->mode;
> -	output->base.subpixel = WL_OUTPUT_SUBPIXEL_UNKNOWN;
> -	output->base.make = "unknown";
> -	output->base.model = output->fb_info.id;
>   
> -	output->base.mm_width = output->fb_info.width_mm;
> -	output->base.mm_height = output->fb_info.height_mm;
> +	head = &output->base.head;
> +	weston_head_set_monitor_strings(head, "unknown", output->fb_info.id,
> +					NULL);
> +	weston_head_set_subpixel(head, WL_OUTPUT_SUBPIXEL_UNKNOWN);
> +	weston_head_set_physical_size(head, output->fb_info.width_mm,
> +				      output->fb_info.height_mm);
>   
>   	close(fb_fd);
>   
> diff --git a/libweston/compositor-headless.c b/libweston/compositor-headless.c
> index 2f01b64a..8fbb83ff 100644
> --- a/libweston/compositor-headless.c
> +++ b/libweston/compositor-headless.c
> @@ -185,6 +185,7 @@ headless_output_set_size(struct weston_output *base,
>   			 int width, int height)
>   {
>   	struct headless_output *output = to_headless_output(base);
> +	struct weston_head *head = &output->base.head;
>   	int output_width, output_height;
>   
>   	/* We can only be called once. */
> @@ -204,12 +205,11 @@ headless_output_set_size(struct weston_output *base,
>   	wl_list_insert(&output->base.mode_list, &output->mode.link);
>   
>   	output->base.current_mode = &output->mode;
> -	output->base.make = "weston";
> -	output->base.model = "headless";
> +
> +	weston_head_set_monitor_strings(head, "weston", "headless", NULL);
>   
>   	/* XXX: Calculate proper size. */
> -	output->base.mm_width = width;
> -	output->base.mm_height = height;
> +	weston_head_set_physical_size(head, width, height);
>   
>   	output->base.start_repaint_loop = headless_output_start_repaint_loop;
>   	output->base.repaint = headless_output_repaint;
> diff --git a/libweston/compositor-rdp.c b/libweston/compositor-rdp.c
> index 98286737..ffad52a6 100644
> --- a/libweston/compositor-rdp.c
> +++ b/libweston/compositor-rdp.c
> @@ -482,6 +482,7 @@ rdp_output_set_size(struct weston_output *base,
>   		    int width, int height)
>   {
>   	struct rdp_output *output = to_rdp_output(base);
> +	struct weston_head *head = &output->base.head;
>   	struct weston_mode *currentMode;
>   	struct weston_mode initMode;
>   
> @@ -500,12 +501,11 @@ rdp_output_set_size(struct weston_output *base,
>   		return -1;
>   
>   	output->base.current_mode = output->base.native_mode = currentMode;
> -	output->base.make = "weston";
> -	output->base.model = "rdp";
> +
> +	weston_head_set_monitor_strings(head, "weston", "rdp", NULL);
>   
>   	/* XXX: Calculate proper size. */
> -	output->base.mm_width = width;
> -	output->base.mm_height = height;
> +	weston_head_set_physical_size(head, width, height);
>   
>   	output->base.start_repaint_loop = rdp_output_start_repaint_loop;
>   	output->base.repaint = rdp_output_repaint;
> diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
> index 3bdfb03e..040c40b9 100644
> --- a/libweston/compositor-wayland.c
> +++ b/libweston/compositor-wayland.c
> @@ -1313,6 +1313,7 @@ static int
>   wayland_output_set_size(struct weston_output *base, int width, int height)
>   {
>   	struct wayland_output *output = to_wayland_output(base);
> +	struct weston_head *head = &output->base.head;
>   	int output_width, output_height;
>   
>   	/* We can only be called once. */
> @@ -1345,12 +1346,11 @@ wayland_output_set_size(struct weston_output *base, int width, int height)
>   	wl_list_insert(&output->base.mode_list, &output->mode.link);
>   
>   	output->base.current_mode = &output->mode;
> -	output->base.make = "wayland";
> -	output->base.model = "none";
> +
> +	weston_head_set_monitor_strings(head, "wayland", "none", NULL);
>   
>   	/* XXX: Calculate proper size. */
> -	output->base.mm_width = width;
> -	output->base.mm_height = height;
> +	weston_head_set_physical_size(head, width, height);
>   
>   	return 0;
>   }
> @@ -1383,10 +1383,12 @@ wayland_output_create_for_parent_output(struct wayland_backend *b,
>   
>   	output->parent.output = poutput->global;
>   
> -	output->base.make = poutput->physical.make;
> -	output->base.model = poutput->physical.model;
> -	output->base.mm_width = poutput->physical.width;
> -	output->base.mm_height = poutput->physical.height;
> +	weston_head_set_monitor_strings(&output->base.head,
> +					poutput->physical.make,
> +					poutput->physical.model, NULL);
> +	weston_head_set_physical_size(&output->base.head,
> +				      poutput->physical.width,
> +				      poutput->physical.height);
>   
>   	wl_list_insert_list(&output->base.mode_list, &poutput->mode_list);
>   	wl_list_init(&poutput->mode_list);
> diff --git a/libweston/compositor-x11.c b/libweston/compositor-x11.c
> index fd948540..b800132a 100644
> --- a/libweston/compositor-x11.c
> +++ b/libweston/compositor-x11.c
> @@ -1061,6 +1061,8 @@ x11_output_set_size(struct weston_output *base, int width, int height)
>   {
>   	struct x11_output *output = to_x11_output(base);
>   	struct x11_backend *b = to_x11_backend(base->compositor);
> +	struct weston_head *head = &output->base.head;
> +	xcb_screen_t *scrn = b->screen;
>   	int output_width, output_height;
>   
>   	/* We can only be called once. */
> @@ -1095,16 +1097,13 @@ x11_output_set_size(struct weston_output *base, int width, int height)
>   	wl_list_insert(&output->base.mode_list, &output->mode.link);
>   
>   	output->base.current_mode = &output->mode;
> -	output->base.make = "weston-X11";
> -	output->base.model = "none";
> -
>   	output->base.native_mode = &output->native;
>   	output->base.native_scale = output->base.scale;
>   
> -	output->base.mm_width = width * b->screen->width_in_millimeters /
> -		b->screen->width_in_pixels;
> -	output->base.mm_height = height * b->screen->height_in_millimeters /
> -		b->screen->height_in_pixels;
> +	weston_head_set_monitor_strings(head, "weston-X11", "none", NULL);
> +	weston_head_set_physical_size(head,
> +		width * scrn->width_in_millimeters / scrn->width_in_pixels,
> +		height * scrn->height_in_millimeters / scrn->height_in_pixels);
>   
>   	return 0;
>   }
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 7d7a17ed..04ac65d2 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -4313,6 +4313,7 @@ bind_output(struct wl_client *client,
>   	struct weston_output *output = data;
>   	struct weston_mode *mode;
>   	struct wl_resource *resource;
> +	struct weston_head *head = &output->head;
>   
>   	resource = wl_resource_create(client, &wl_output_interface,
>   				      version, id);
> @@ -4327,10 +4328,10 @@ bind_output(struct wl_client *client,
>   	wl_output_send_geometry(resource,
>   				output->x,
>   				output->y,
> -				output->mm_width,
> -				output->mm_height,
> -				output->subpixel,
> -				output->make, output->model,
> +				head->mm_width,
> +				head->mm_height,
> +				head->subpixel,
> +				head->make, head->model,
>   				output->transform);
>   	if (version >= WL_OUTPUT_SCALE_SINCE_VERSION)
>   		wl_output_send_scale(resource,
> @@ -4363,6 +4364,85 @@ weston_output_from_resource(struct wl_resource *resource)
>   	return wl_resource_get_user_data(resource);
>   }
>   
> +/** Store monitor make, model and serial number
> + *
> + * \param head The head to modify.
> + * \param make The monitor make. If EDID is available, the PNP ID. Otherwise
> + * any string, or NULL for none.
> + * \param model The monitor model or name, or a made-up string, or NULL for
> + * none.
> + * \param serialno The monitor serial number, a made-up string, or NULL for
> + * none.
> + *
> + * \memberof weston_head
> + * \internal
> + */
> +WL_EXPORT void
> +weston_head_set_monitor_strings(struct weston_head *head,
> +				const char *make,
> +				const char *model,
> +				const char *serialno)
> +{
> +	head->make = (char *)make;
> +	head->model = (char *)model;
> +	head->serial_number = (char *)serialno;
> +}
> +
> +/** Store physical image size
> + *
> + * \param head The head to modify.
> + * \param mm_width Image area width in millimeters.
> + * \param mm_height Image area height in millimeters.
> + *
> + * \memberof weston_head
> + * \internal
> + */
> +WL_EXPORT void
> +weston_head_set_physical_size(struct weston_head *head,
> +			      int32_t mm_width, int32_t mm_height)
> +{
> +	head->mm_width = mm_width;
> +	head->mm_height = mm_height;
> +}
> +
> +/** Store monitor sub-pixel layout
> + *
> + * \param head The head to modify.
> + * \param sp Sub-pixel layout. The possible values are:
> + * - WL_OUTPUT_SUBPIXEL_UNKNOWN,
> + * - WL_OUTPUT_SUBPIXEL_NONE,
> + * - WL_OUTPUT_SUBPIXEL_HORIZONTAL_RGB,
> + * - WL_OUTPUT_SUBPIXEL_HORIZONTAL_BGR,
> + * - WL_OUTPUT_SUBPIXEL_VERTICAL_RGB,
> + * - WL_OUTPUT_SUBPIXEL_VERTICAL_BGR
> + *
> + * \memberof weston_head
> + * \internal
> + */
> +WL_EXPORT void
> +weston_head_set_subpixel(struct weston_head *head,
> +			 enum wl_output_subpixel sp)
> +{
> +	head->subpixel = sp;
> +}
> +
> +/** Mark the monitor as internal
> + *
> + * This is used for embedded screens, like laptop panels.
> + *
> + * \param head The head to mark as internal.
> + *
> + * By default a head is external. The type is often inferred from the physical
> + * connector type.
> + *
> + * \memberof weston_head
> + * \internal
> + */
> +WL_EXPORT void
> +weston_head_set_internal(struct weston_head *head)
> +{
> +	head->connection_internal = true;
> +}
>   
>   /* Move other outputs when one is resized so the space remains contiguous. */
>   static void
> @@ -4479,6 +4559,7 @@ 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 wl_resource *resource;
>   
>   	output->move_x = x - output->x;
> @@ -4499,11 +4580,11 @@ weston_output_move(struct weston_output *output, int x, int y)
>   		wl_output_send_geometry(resource,
>   					output->x,
>   					output->y,
> -					output->mm_width,
> -					output->mm_height,
> -					output->subpixel,
> -					output->make,
> -					output->model,
> +					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)
> @@ -4721,6 +4802,8 @@ 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);
> @@ -4730,8 +4813,8 @@ weston_output_init(struct weston_output *output,
>   	/* Add some (in)sane defaults which can be used
>   	 * for checking if an output was properly configured
>   	 */
> -	output->mm_width = 0;
> -	output->mm_height = 0;
> +	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;
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index dffcba89..b1bed44c 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -147,6 +147,21 @@ enum dpms_enum {
>   	WESTON_DPMS_OFF
>   };
>   
> +/** Represents a monitor
> + *
> + * This object represents a monitor (hardware backends like DRM) or a window
> + * (windowed nested backends).
> + */
> +struct weston_head {
> +	int32_t mm_width;		/**< physical image width in mm */
> +	int32_t mm_height;		/**< physical image height in mm */
> +	char *make;			/**< monitor manufacturer (PNP ID) */
> +	char *model;			/**< monitor model */
> +	char *serial_number;		/**< monitor serial */
> +	uint32_t subpixel;		/**< enum wl_output_subpixel */
> +	bool connection_internal;	/**< embedded monitor (e.g. laptop) */
> +};
> +
>   struct weston_output {
>   	uint32_t id;
>   	char *name;
> @@ -165,7 +180,6 @@ struct weston_output {
>   
>   	struct wl_list animation_list;
>   	int32_t x, y, width, height;
> -	int32_t mm_width, mm_height;
>   
>   	/** Output area in global coordinates, simple rect */
>   	pixman_region32_t region;
> @@ -199,8 +213,6 @@ struct weston_output {
>   	int destroying;
>   	struct wl_list feedback_list;
>   
> -	char *make, *model, *serial_number;
> -	uint32_t subpixel;
>   	uint32_t transform;
>   	int32_t native_scale;
>   	int32_t current_scale;
> @@ -211,6 +223,8 @@ struct weston_output {
>   	struct weston_mode *original_mode;
>   	struct wl_list mode_list;
>   
> +	struct weston_head head;
> +
>   	void (*start_repaint_loop)(struct weston_output *output);
>   	int (*repaint)(struct weston_output *output,
>   			pixman_region32_t *damage,
> @@ -224,7 +238,6 @@ struct weston_output {
>   	void (*set_backlight)(struct weston_output *output, uint32_t value);
>   	void (*set_dpms)(struct weston_output *output, enum dpms_enum level);
>   
> -	bool connection_internal;
>   	uint16_t gamma_size;
>   	void (*set_gamma)(struct weston_output *output,
>   			  uint16_t size,
> @@ -1932,6 +1945,23 @@ int
>   weston_compositor_load_xwayland(struct weston_compositor *compositor);
>   
>   void
> +weston_head_set_monitor_strings(struct weston_head *head,
> +				const char *make,
> +				const char *model,
> +				const char *serialno);

I was going to complain about the apparently silly trivial helper 
functions, but their importance becomes obvious later in the series.

Reviewed-by: Derek Foreman <derekf at osg.samsung.com>
(though I don't want to see any of it land until we can get as far as 
removing the embedded weston_head in weston_output...)

Thanks,
Derek

> +
> +void
> +weston_head_set_physical_size(struct weston_head *head,
> +			      int32_t mm_width, int32_t mm_height);
> +
> +void
> +weston_head_set_subpixel(struct weston_head *head,
> +			 enum wl_output_subpixel sp);
> +
> +void
> +weston_head_set_internal(struct weston_head *head);
> +
> +void
>   weston_output_set_scale(struct weston_output *output,
>   			int32_t scale);
>   
> 



More information about the wayland-devel mailing list