On Wed, 2015-03-11 at 16:05 +0200, Pekka Paalanen wrote: > On Tue, 10 Mar 2015 11:34:45 +0900 > Ryo Munakata <ryomnk...@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.
.o/
> > Signed-off-by: Ryo Munakata <ryomnk...@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;
Why break here? Why not break where you actually assign to fbdev, just
below?
> > +
> > + 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?
Yes, it is. Although I suspect the original intention was also to
support outputting to all the framebuffer devices simultaneously. That’s
a separate issue; grabbing the first framebuffer from udev is a good
start.
Philip
signature.asc
Description: This is a digitally signed message part
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
