[PATCH weston] compositor-fbdev: enumerate available fbdevs using udev

[Pekka Paalanen]

On Tue, 10 Mar 2015 11:34:45 +0900
Ryo Munakata <ryomnktml at gmail.com> wrote:

> This was a TODO:
> "Ideally, available frame buffers should be enumerated
> using udev, rather than passing a device node in as a
> parameter."

Hi,

I'm CC'ing the person responsible for the TODO-comment.

> 
> Signed-off-by: Ryo Munakata <ryomnktml at gmail.com>
> ---
>  src/compositor-fbdev.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 52 insertions(+), 3 deletions(-)
> 
> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> index eedf252..c0e6985 100644
> --- a/src/compositor-fbdev.c
> +++ b/src/compositor-fbdev.c
> @@ -46,6 +46,8 @@
>  #include "gl-renderer.h"
>  #include "presentation_timing-server-protocol.h"
>  
> +#define FBDEV_MAJOR_NUMBER (29)
> +
>  struct fbdev_compositor {
>  	struct weston_compositor base;
>  	uint32_t prev_state;
> @@ -690,6 +692,9 @@ fbdev_output_destroy(struct weston_output *base)
>  		gl_renderer->output_destroy(base);
>  	}
>  
> +	if (output->device)
> +		free((char *)output->device);

No need to cast. Also no need for 'if' because free(NULL) is perfectly
legal.

> +
>  	/* Remove the output. */
>  	weston_output_destroy(&output->base);
>  
> @@ -748,6 +753,7 @@ fbdev_output_reenable(struct fbdev_compositor *compositor,
>  		 * the frame buffer X/Y resolution (such as the shadow buffer)
>  		 * are re-initialised. */
>  		device = output->device;
> +		output->device = NULL;

Why NULLing it here? Seem like this would make repeated reenable to
fail, no?

>  		fbdev_output_destroy(base);
>  		fbdev_output_create(compositor, device);
>  
> @@ -795,6 +801,7 @@ fbdev_compositor_destroy(struct weston_compositor *base)
>  	struct fbdev_compositor *compositor = to_fbdev_compositor(base);
>  
>  	udev_input_destroy(&compositor->input);
> +	udev_unref(compositor->udev);
>  
>  	/* Destroy the output. */
>  	weston_compositor_shutdown(&compositor->base);
> @@ -861,6 +868,47 @@ switch_vt_binding(struct weston_seat *seat, uint32_t time, uint32_t key, void *d
>  	weston_launcher_activate_vt(compositor->launcher, key - KEY_F1 + 1);
>  }
>  
> +static char *
> +udev_get_fbdev(struct udev *udev)
> +{
> +	char *fbdev = NULL;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *list, *entry;
> +
> +	if (!(enumerate = udev_enumerate_new(udev)))
> +		goto err;
> +
> +	udev_enumerate_add_match_subsystem(enumerate, "graphics");
> +	udev_enumerate_scan_devices(enumerate);
> +
> +	list = udev_enumerate_get_list_entry(enumerate);
> +	udev_list_entry_foreach(entry, list) {
> +		const char *devnode;
> +		const char *syspath = udev_list_entry_get_name(entry);
> +		struct udev_device *device;
> +
> +		if (fbdev)
> +			break;
> +
> +		if (!(device = udev_device_new_from_syspath(udev, syspath)))
> +			continue;
> +
> +		devnode = udev_device_get_devnode(device);
> +		if (devnode) {
> +			dev_t devnum = udev_device_get_devnum(device);
> +
> +			if (major(devnum) == FBDEV_MAJOR_NUMBER)
> +				fbdev = strdup(devnode);
> +		}
> +
> +		udev_device_unref(device);
> +	}
> +
> +	udev_enumerate_unref(enumerate);
> +err:
> +	return fbdev ? fbdev : strdup("/dev/fb0");
> +}
> +
>  static struct weston_compositor *
>  fbdev_compositor_create(struct wl_display *display, int *argc, char *argv[],
>                          struct weston_config *config,
> @@ -933,6 +981,9 @@ fbdev_compositor_create(struct wl_display *display, int *argc, char *argv[],
>  		}
>  	}
>  
> +	if (!param->device)
> +		param->device = udev_get_fbdev(compositor->udev);
> +
>  	if (fbdev_output_create(compositor, param->device) < 0)
>  		goto out_pixman;
>  
> @@ -962,11 +1013,9 @@ WL_EXPORT struct weston_compositor *
>  backend_init(struct wl_display *display, int *argc, char *argv[],
>  	     struct weston_config *config)
>  {
> -	/* TODO: Ideally, available frame buffers should be enumerated using
> -	 * udev, rather than passing a device node in as a parameter. */
>  	struct fbdev_parameters param = {
>  		.tty = 0, /* default to current tty */
> -		.device = "/dev/fb0", /* default frame buffer */
> +		.device = NULL,
>  		.use_gl = 0,
>  	};
>  

Philip, is this what you had in mind?


Thanks,
pq