[PATCH weston 05/12] weston: Port fbdev backend to new output handling API

Pekka Paalanen ppaalanen at gmail.com
Tue Aug 16 12:33:32 UTC 2016


On Sun, 14 Aug 2016 17:28:14 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:

> This is a complete port of the fbdev backend that uses
> recently added output handling API for output
> configuration.
> 
> It is required that the scale and transform values are
> set using the previously added functionality.
> 
> After everything has been set, output needs to be
> enabled manually using weston_output_enable().
> 
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
>  compositor/main.c            | 30 ++++++++++-----
>  libweston/compositor-fbdev.c | 87 +++++++++++++++++++++++++++-----------------
>  libweston/compositor-fbdev.h |  2 -
>  3 files changed, 74 insertions(+), 45 deletions(-)
> 
> diff --git a/compositor/main.c b/compositor/main.c
> index b0d21ea..2675f38 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -1296,13 +1296,26 @@ load_rdp_backend(struct weston_compositor *c,
>  	return ret;
>  }
>  
> +static void
> +fbdev_backend_output_configure(struct wl_listener *listener, void *data)
> +{
> +	struct weston_output *output = data;
> +	struct weston_config *wc = wet_get_config(output->compositor);
> +	struct weston_config_section *section;
> +
> +	section = weston_config_get_section(wc, "output", "name", "fbdev");
> +
> +	wet_output_set_transform(output, section, WL_OUTPUT_TRANSFORM_NORMAL, UINT32_MAX);
> +	weston_output_set_scale(output, 1);
> +
> +	weston_output_enable(output);
> +}
> +
>  static int
>  load_fbdev_backend(struct weston_compositor *c,
>  		      int *argc, char **argv, struct weston_config *wc)
>  {
>  	struct weston_fbdev_backend_config config = {{ 0, }};
> -	struct weston_config_section *section;
> -	char *s = NULL;
>  	int ret = 0;
>  
>  	const struct weston_option fbdev_options[] = {
> @@ -1315,12 +1328,6 @@ load_fbdev_backend(struct weston_compositor *c,
>  	if (!config.device)
>  		config.device = strdup("/dev/fb0");
>  
> -	section = weston_config_get_section(wc, "output", "name", "fbdev");
> -	weston_config_section_get_string(section, "transform", &s, "normal");
> -	if (weston_parse_transform(s, &config.output_transform) < 0)
> -		weston_log("Invalid transform \"%s\" for output fbdev\n", s);
> -	free(s);
> -
>  	config.base.struct_version = WESTON_FBDEV_BACKEND_CONFIG_VERSION;
>  	config.base.struct_size = sizeof(struct weston_fbdev_backend_config);
>  	config.configure_device = configure_input_device;
> @@ -1329,8 +1336,13 @@ load_fbdev_backend(struct weston_compositor *c,
>  	ret = weston_compositor_load_backend(c, WESTON_BACKEND_FBDEV,
>  					     &config.base);
>  
> -	free(config.device);
> +	if (ret < 0)
> +		goto out;
> +
> +	wet_set_pending_output_handler(c, fbdev_backend_output_configure);
>  
> +out:
> +	free(config.device);
>  	return ret;
>  }
>  

All the above looks ok.


> diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
> index 852acc0..afa1e7a 100644
> --- a/libweston/compositor-fbdev.c
> +++ b/libweston/compositor-fbdev.c
> @@ -426,12 +426,56 @@ static void fbdev_output_destroy(struct weston_output *base);
>  static void fbdev_output_disable(struct weston_output *base);
>  
>  static int
> +fbdev_output_enable(struct weston_output *base)
> +{
> +	struct fbdev_output *output = to_fbdev_output(base);
> +	struct fbdev_backend *backend = to_fbdev_backend(base->compositor);
> +	int fb_fd;
> +	struct wl_event_loop *loop;
> +
> +	/* Create the frame buffer. */
> +	fb_fd = fbdev_frame_buffer_open(output, output->device, &output->fb_info);
> +	if (fb_fd < 0) {
> +		weston_log("Creating frame buffer failed.\n");
> +		return -1;
> +	}
> +
> +	if (fbdev_frame_buffer_map(output, fb_fd) < 0) {
> +		weston_log("Mapping frame buffer failed.\n");
> +		return -1;
> +	}
> +
> +	output->base.start_repaint_loop = fbdev_output_start_repaint_loop;
> +	output->base.repaint = fbdev_output_repaint;
> +
> +	if (pixman_renderer_output_create(&output->base) < 0)
> +		goto out_hw_surface;
> +
> +	loop = wl_display_get_event_loop(backend->compositor->wl_display);
> +	output->finish_frame_timer =
> +		wl_event_loop_add_timer(loop, finish_frame_handler, output);
> +
> +	weston_log("fbdev output %d×%d px\n",
> +	           output->mode.width, output->mode.height);
> +	weston_log_continue(STAMP_SPACE "guessing %d Hz and 96 dpi\n",
> +	                    output->mode.refresh / 1000);
> +
> +	return 0;
> +
> +out_hw_surface:
> +	pixman_image_unref(output->hw_surface);
> +	output->hw_surface = NULL;
> +	fbdev_frame_buffer_destroy(output);
> +
> +	return -1;
> +}
> +
> +static int
>  fbdev_output_create(struct fbdev_backend *backend,
>                      const char *device)
>  {
>  	struct fbdev_output *output;
>  	int fb_fd;
> -	struct wl_event_loop *loop;
>  
>  	weston_log("Creating fbdev output.\n");
>  
> @@ -449,14 +493,12 @@ fbdev_output_create(struct fbdev_backend *backend,
>  		goto out_free;
>  	}
>  
> -	if (fbdev_frame_buffer_map(output, fb_fd) < 0) {
> -		weston_log("Mapping frame buffer failed.\n");
> -		goto out_free;
> -	}
> -
> -	output->base.start_repaint_loop = fbdev_output_start_repaint_loop;
> -	output->base.repaint = fbdev_output_repaint;
> +	output->base.name = strdup("fbdev");
>  	output->base.destroy = fbdev_output_destroy;
> +	output->base.disable = NULL;

Ok, we can't really implement a proper disable as in turn off.

If someone tries to disable an enabled output, we should fail an assert
the very least, since that's just not supported. The common code does
not catch that either, IIRC.

The existing fbdev_output_disable() serves a different purpose: it
handles the fbdev releasing when the user switches VT away from weston.
You can keep it like that, I am not sure we care enough to actually
support on-the-fly disabling of a fbdev output via
weston_output_disable(). Not yet, anyway.

> +	output->base.enable = fbdev_output_enable;
> +
> +	weston_output_init_pending(&output->base, backend->compositor);

I think everything below this point should be moved into
fbdev_output_enable(). That would avoid the re-opening of the device
and overwriting fb_info.

It's not like we promise pending outputs to have any mode info.

>  
>  	/* only one static mode in list */
>  	output->mode.flags =
> @@ -471,35 +513,14 @@ fbdev_output_create(struct fbdev_backend *backend,
>  	output->base.subpixel = WL_OUTPUT_SUBPIXEL_UNKNOWN;
>  	output->base.make = "unknown";
>  	output->base.model = output->fb_info.id;
> -	output->base.name = strdup("fbdev");
>  
> -	weston_output_init(&output->base, backend->compositor,
> -	                   0, 0, output->fb_info.width_mm,
> -	                   output->fb_info.height_mm,
> -	                   backend->output_transform,
> -			   1);
> +	output->base.mm_width = output->fb_info.width_mm;
> +	output->base.mm_height = output->fb_info.height_mm;
>  
> -	if (pixman_renderer_output_create(&output->base) < 0)
> -		goto out_hw_surface;
> -
> -	loop = wl_display_get_event_loop(backend->compositor->wl_display);
> -	output->finish_frame_timer =
> -		wl_event_loop_add_timer(loop, finish_frame_handler, output);
> -
> -	weston_compositor_add_output(backend->compositor, &output->base);
> -
> -	weston_log("fbdev output %d×%d px\n",
> -	           output->mode.width, output->mode.height);
> -	weston_log_continue(STAMP_SPACE "guessing %d Hz and 96 dpi\n",
> -	                    output->mode.refresh / 1000);
> +	close(fb_fd);
>  
>  	return 0;
>  
> -out_hw_surface:
> -	pixman_image_unref(output->hw_surface);
> -	output->hw_surface = NULL;
> -	weston_output_destroy(&output->base);
> -	fbdev_frame_buffer_destroy(output);
>  out_free:
>  	free(output->device);
>  	free(output);
> @@ -721,7 +742,6 @@ fbdev_backend_create(struct weston_compositor *compositor,
>  	backend->base.restore = fbdev_restore;
>  
>  	backend->prev_state = WESTON_COMPOSITOR_ACTIVE;
> -	backend->output_transform = param->output_transform;
>  
>  	weston_setup_vt_switch_bindings(compositor);
>  
> @@ -757,7 +777,6 @@ config_init_to_defaults(struct weston_fbdev_backend_config *config)
>  	 * udev, rather than passing a device node in as a parameter. */
>  	config->tty = 0; /* default to current tty */
>  	config->device = "/dev/fb0"; /* default frame buffer */
> -	config->output_transform = WL_OUTPUT_TRANSFORM_NORMAL;
>  }
>  
>  WL_EXPORT int
> diff --git a/libweston/compositor-fbdev.h b/libweston/compositor-fbdev.h
> index 7b182c7..0fd06c8 100644
> --- a/libweston/compositor-fbdev.h
> +++ b/libweston/compositor-fbdev.h
> @@ -44,8 +44,6 @@ struct weston_fbdev_backend_config {
>  	int tty;
>  	char *device;
>  
> -	uint32_t output_transform;
> -
>  	/** Callback used to configure input devices.
>  	 *
>  	 * This function will be called by the backend when a new input device


However, fbdev-backend still works after this patch, and Quentin is for
it, so I'll count it as landable as is. The changes I wanted are minor
and ok as follow-ups.

VT switching doesn't seem to work right, but that was already broken.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160816/c59d1dca/attachment.sig>


More information about the wayland-devel mailing list