Looks pretty good, thanks. For future patches can you include a version and short changelog of what's changed from the last patch? Just makes things easier to track (even for the patch author, in my experience).
Few notes on this: - do we need all the dpms values? on and off seem to be all that most displays actually use reasonably - a range of 1-10 isn't enough for really smooth fade effects, 1-255 would be better (though clearly not all hw supports it) Looks good otherwise. With the above addressed either in a revised patch or patch on top: Reviewed-by: Jesse Barnes <jbar...@virtuousgeek.org> Thanks, Jesse On Wed, 29 Feb 2012 19:53:50 +0200 Tiago Vignatti <tiago.vigna...@intel.com> wrote: > DPMS kicks in only when wscreensaver is launched, in the moment that > shell call lock() for the second time. Backlight control internals > are managed by libbacklight: > > http://cgit.freedesktop.org/~vignatti/libbacklight/ > > Signed-off-by: Tiago Vignatti <tiago.vigna...@intel.com> > --- > > Thanks Jesse and Kristian for the comments. I've updated the patch > addressing most of the issues. > > configure.ac | 2 +- > src/compositor-drm.c | 114 > ++++++++++++++++++++++++++++++++++++++++++---- > src/compositor-openwfd.c | 2 + src/compositor-wayland.c | 2 + > src/compositor-x11.c | 2 + > src/compositor.c | 12 +++++ > src/compositor.h | 13 +++++ > src/shell.c | 47 ++++++++++++++++++- > 8 files changed, 182 insertions(+), 12 deletions(-) > > diff --git a/configure.ac b/configure.ac > index ba42a04..6b54bb3 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -73,7 +73,7 @@ AC_ARG_ENABLE(drm-compositor, > [ --enable-drm-compositor],, AM_CONDITIONAL(ENABLE_DRM_COMPOSITOR, > test x$enable_drm_compositor = xyes) if test x$enable_drm_compositor > = xyes; then AC_DEFINE([BUILD_DRM_COMPOSITOR], [1], [Build the DRM > compositor]) > - PKG_CHECK_MODULES(DRM_COMPOSITOR, [libudev >= 136 libdrm >= 2.4.30 > gbm]) > + PKG_CHECK_MODULES(DRM_COMPOSITOR, [libbacklight libudev >= 136 > libdrm >= 2.4.30 gbm]) fi > > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c > index 38ff02b..cfaad15 100644 > --- a/src/compositor-drm.c > +++ b/src/compositor-drm.c > @@ -35,6 +35,7 @@ > #include <drm_fourcc.h> > > #include <gbm.h> > +#include <libbacklight.h> > > #include "compositor.h" > #include "evdev.h" > @@ -86,6 +87,7 @@ struct drm_output { > struct wl_listener scanout_buffer_destroy_listener; > struct wl_buffer *pending_scanout_buffer; > struct wl_listener pending_scanout_buffer_destroy_listener; > + struct backlight *backlight; > }; > > /* > @@ -697,6 +699,9 @@ drm_output_destroy(struct weston_output > *output_base) drmModeCrtcPtr origcrtc = output->original_crtc; > int i; > > + if (output->backlight) > + backlight_destroy(output->backlight); > + > /* Turn off hardware cursor */ > drm_output_set_cursor(&output->base, NULL); > > @@ -907,11 +912,92 @@ sprite_handle_pending_buffer_destroy(struct > wl_listener *listener, sprite->pending_surface = NULL; > } > > +/* returns a value between 1-10 range, where higher is brighter */ > +static uint32_t > +drm_get_backlight(struct drm_output *output) > +{ > + long brightness, max_brightness, norm; > + > + brightness = backlight_get_brightness(output->backlight); > + max_brightness = > backlight_get_max_brightness(output->backlight); + > + /* convert it on a scale of 1 to 10 */ > + norm = 1 + ((brightness) * 9)/(max_brightness); > + > + return (uint32_t) norm; > +} > + > +/* values accepted are between 1-10 range */ > +static void > +drm_set_backlight(struct weston_output *output_base, uint32_t value) > +{ > + struct drm_output *output = (struct drm_output *) > output_base; > + long max_brightness, new_brightness; > + > + if (!output->backlight) > + return; > + > + if (value < 1 || value > 10) > + return; > + > + max_brightness = > backlight_get_max_brightness(output->backlight); + > + /* get denormalized value */ > + new_brightness = ((value - 1) * (max_brightness)) / 9; > + > + backlight_set_brightness(output->backlight, new_brightness); > +} > + > +static drmModePropertyPtr > +drm_get_prop(int fd, drmModeConnectorPtr connector, const char *name) > +{ > + drmModePropertyPtr props; > + int i; > + > + for (i = 0; i < connector->count_props; i++) { > + props = drmModeGetProperty(fd, connector->props[i]); > + if (!props) > + continue; > + > + if (!strcmp(props->name, name)) > + return props; > + > + drmModeFreeProperty(props); > + } > + > + return NULL; > +} > + > +static void > +drm_set_dpms(struct weston_output *output_base, enum dpms_enum level) > +{ > + struct drm_output *output = (struct drm_output *) > output_base; > + struct weston_compositor *ec = output_base->compositor; > + struct drm_compositor *c = (struct drm_compositor *) ec; > + drmModeConnectorPtr connector; > + drmModePropertyPtr prop; > + > + connector = drmModeGetConnector(c->drm.fd, > output->connector_id); > + if (!connector) > + return; > + > + prop = drm_get_prop(c->drm.fd, connector, "DPMS"); > + if (!prop) { > + drmModeFreeConnector(connector); > + return; > + } > + > + drmModeConnectorSetProperty(c->drm.fd, > connector->connector_id, > + prop->prop_id, level); > + drmModeFreeProperty(prop); > + drmModeFreeConnector(connector); > +} > + > static int > create_output_for_connector(struct drm_compositor *ec, > drmModeRes *resources, > drmModeConnector *connector, > - int x, int y) > + int x, int y, struct udev_device > *drm_device) { > struct drm_output *output; > struct drm_mode *drm_mode, *next; > @@ -1026,6 +1112,13 @@ create_output_for_connector(struct > drm_compositor *ec, goto err_fb; > } > > + output->backlight = backlight_init(drm_device, > + > connector->connector_type); > + if (output->backlight) { > + output->base.set_backlight = drm_set_backlight; > + output->base.backlight_current = > drm_get_backlight(output); > + } > + > weston_output_init(&output->base, &ec->base, x, y, > connector->mmWidth, connector->mmHeight, 0); > > @@ -1040,6 +1133,7 @@ create_output_for_connector(struct > drm_compositor *ec, output->base.repaint = drm_output_repaint; > output->base.destroy = drm_output_destroy; > output->base.assign_planes = drm_assign_planes; > + output->base.set_dpms = drm_set_dpms; > > return 0; > > @@ -1148,7 +1242,8 @@ destroy_sprites(struct drm_compositor > *compositor) } > > static int > -create_outputs(struct drm_compositor *ec, int option_connector) > +create_outputs(struct drm_compositor *ec, int option_connector, > + struct udev_device *drm_device) > { > drmModeConnector *connector; > drmModeRes *resources; > @@ -1178,7 +1273,8 @@ create_outputs(struct drm_compositor *ec, int > option_connector) (option_connector == 0 || > connector->connector_id == option_connector)) { > if (create_output_for_connector(ec, > resources, > - connector, > x, y) < 0) { > + connector, > x, y, > + drm_device) > < 0) { drmModeFreeConnector(connector); > continue; > } > @@ -1202,7 +1298,7 @@ create_outputs(struct drm_compositor *ec, int > option_connector) } > > static void > -update_outputs(struct drm_compositor *ec) > +update_outputs(struct drm_compositor *ec, struct udev_device > *drm_device) { > drmModeConnector *connector; > drmModeRes *resources; > @@ -1245,7 +1341,8 @@ update_outputs(struct drm_compositor *ec) > x = 0; > y = 0; > create_output_for_connector(ec, resources, > - connector, x, y); > + connector, x, y, > + drm_device); > printf("connector %d connected\n", > connector_id); > } > @@ -1301,7 +1398,7 @@ udev_drm_event(int fd, uint32_t mask, void > *data) event = udev_monitor_receive_device(ec->udev_monitor); > > if (udev_event_is_hotplug(event)) > - update_outputs(ec); > + update_outputs(ec, event); > > udev_device_unref(event); > > @@ -1469,8 +1566,6 @@ drm_compositor_create(struct wl_display > *display, return NULL; > } > > - udev_device_unref(drm_device); > - > ec->base.destroy = drm_destroy; > > ec->base.focus = 1; > @@ -1487,11 +1582,12 @@ drm_compositor_create(struct wl_display > *display, wl_list_init(&ec->sprite_list); > create_sprites(ec); > > - if (create_outputs(ec, connector) < 0) { > + if (create_outputs(ec, connector, drm_device) < 0) { > fprintf(stderr, "failed to create output for %s\n", > path); return NULL; > } > > + udev_device_unref(drm_device); > udev_enumerate_unref(e); > path = NULL; > > diff --git a/src/compositor-openwfd.c b/src/compositor-openwfd.c > index 8dce304..eae1e66 100644 > --- a/src/compositor-openwfd.c > +++ b/src/compositor-openwfd.c > @@ -407,6 +407,8 @@ create_output_for_port(struct wfd_compositor *ec, > output->base.set_hardware_cursor = wfd_output_set_cursor; > output->base.destroy = wfd_output_destroy; > output->base.assign_planes = NULL; > + output->base.set_backlight = NULL; > + output->base.set_dpms = NULL; > > wl_list_insert(ec->base.output_list.prev, > &output->base.link); > diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c > index a42e76f..f8b5a32 100644 > --- a/src/compositor-wayland.c > +++ b/src/compositor-wayland.c > @@ -435,6 +435,8 @@ wayland_compositor_create_output(struct > wayland_compositor *c, output->base.repaint = wayland_output_repaint; > output->base.destroy = wayland_output_destroy; > output->base.assign_planes = NULL; > + output->base.set_backlight = NULL; > + output->base.set_dpms = NULL; > > wl_list_insert(c->base.output_list.prev, &output->base.link); > > diff --git a/src/compositor-x11.c b/src/compositor-x11.c > index 7243329..53998d2 100644 > --- a/src/compositor-x11.c > +++ b/src/compositor-x11.c > @@ -441,6 +441,8 @@ x11_compositor_create_output(struct > x11_compositor *c, int x, int y, output->base.repaint = > x11_output_repaint; output->base.destroy = x11_output_destroy; > output->base.assign_planes = NULL; > + output->base.set_backlight = NULL; > + output->base.set_dpms = NULL; > > wl_list_insert(c->base.output_list.prev, &output->base.link); > > diff --git a/src/compositor.c b/src/compositor.c > index 74c40da..8f09550 100644 > --- a/src/compositor.c > +++ b/src/compositor.c > @@ -1386,12 +1386,23 @@ weston_compositor_wake(struct > weston_compositor *compositor) compositor->idle_time * 1000); > } > > +static void > +weston_compositor_dpms_on(struct weston_compositor *compositor) > +{ > + struct weston_output *output; > + > + wl_list_for_each(output, &compositor->output_list, link) > + if (output->set_dpms) > + output->set_dpms(output, WESTON_DPMS_ON); > +} > + > WL_EXPORT void > weston_compositor_activity(struct weston_compositor *compositor) > { > if (compositor->state == WESTON_COMPOSITOR_ACTIVE) { > weston_compositor_wake(compositor); > } else { > + weston_compositor_dpms_on(compositor); > compositor->shell->unlock(compositor->shell); > } > } > @@ -2513,6 +2524,7 @@ int main(int argc, char *argv[]) > exit(EXIT_FAILURE); > } > > + weston_compositor_dpms_on(ec); > weston_compositor_wake(ec); > if (setjmp(segv_jmp_buf) == 0) > wl_display_run(display); > diff --git a/src/compositor.h b/src/compositor.h > index dd9cb20..8c57e4f 100644 > --- a/src/compositor.h > +++ b/src/compositor.h > @@ -62,6 +62,14 @@ struct weston_output_zoom { > float trans_x, trans_y; > }; > > +/* bit compatible with drm definitions. */ > +enum dpms_enum { > + WESTON_DPMS_ON, > + WESTON_DPMS_STANDBY, > + WESTON_DPMS_SUSPEND, > + WESTON_DPMS_OFF > +}; > + > struct weston_output { > struct wl_list link; > struct weston_compositor *compositor; > @@ -86,6 +94,11 @@ struct weston_output { > void (*repaint)(struct weston_output *output); > void (*destroy)(struct weston_output *output); > void (*assign_planes)(struct weston_output *output); > + > + /* backlight values are on 1-10 range, where higher is > brighter */ > + uint32_t backlight_current; > + void (*set_backlight)(struct weston_output *output, uint32_t > value); > + void (*set_dpms)(struct weston_output *output, enum > dpms_enum level); }; > > struct weston_input_device { > diff --git a/src/shell.c b/src/shell.c > index a28302b..01e5cf6 100644 > --- a/src/shell.c > +++ b/src/shell.c > @@ -1311,10 +1311,16 @@ lock(struct weston_shell *base) > struct weston_surface *tmp; > struct weston_input_device *device; > struct shell_surface *shsurf; > + struct weston_output *output; > uint32_t time; > > - if (shell->locked) > + if (shell->locked) { > + wl_list_for_each(output, > &shell->compositor->output_list, link) > + /* TODO: find a way to jump to other DPMS > levels */ > + if (output->set_dpms) > + output->set_dpms(output, > WESTON_DPMS_STANDBY); return; > + } > > shell->locked = true; > > @@ -1834,6 +1840,34 @@ switcher_binding(struct wl_input_device > *device, uint32_t time, } > > static void > +backlight_binding(struct wl_input_device *device, uint32_t time, > + uint32_t key, uint32_t button, uint32_t state, > void *data) +{ > + struct weston_compositor *compositor = data; > + struct weston_output *output; > + > + /* TODO: we're limiting to simple use cases, where we assume > just > + * control on the primary display. We'd have to extend later > if we > + * ever get support for setting backlights on random desktop > LCD > + * panels though */ > + output = get_default_output(compositor); > + if (!output) > + return; > + > + if (!output->set_backlight) > + return; > + > + if ((key == KEY_F9 || key == KEY_BRIGHTNESSDOWN) && > + output->backlight_current > 1) > + output->backlight_current--; > + else if ((key == KEY_F10 || key == KEY_BRIGHTNESSUP) && > + output->backlight_current < 10) > + output->backlight_current++; > + > + output->set_backlight(output, output->backlight_current); > +} > + > +static void > shell_destroy(struct weston_shell *base) > { > struct wl_shell *shell = container_of(base, struct wl_shell, > shell); @@ -1905,10 +1939,19 @@ shell_init(struct weston_compositor > *ec) weston_compositor_add_binding(ec, 0, BTN_LEFT, > MODIFIER_SUPER | MODIFIER_ALT, > rotate_binding, NULL); > - > weston_compositor_add_binding(ec, KEY_TAB, 0, MODIFIER_SUPER, > switcher_binding, ec); > > + /* brightness */ > + weston_compositor_add_binding(ec, KEY_F9, 0, MODIFIER_CTRL, > + backlight_binding, ec); > + weston_compositor_add_binding(ec, KEY_BRIGHTNESSDOWN, 0, 0, > + backlight_binding, ec); > + weston_compositor_add_binding(ec, KEY_F10, 0, MODIFIER_CTRL, > + backlight_binding, ec); > + weston_compositor_add_binding(ec, KEY_BRIGHTNESSUP, 0, 0, > + backlight_binding, ec); > + > ec->shell = &shell->shell; > > return 0; _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel