[PATCH] compositor: add dpms and backlight support

Kristian Hoegsberg hoegsberg at gmail.com
Wed Feb 29 12:47:01 PST 2012


On Wed, Feb 29, 2012 at 10:45:35AM -0800, Jesse Barnes wrote:
> 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

Maybe not, I don't see a problem in having them there though.

>   - 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)

Right, that would be better.  The keyboard binding can just increase
in steps of 25 or so.  And can we just start from 0?

Another issue I'd like to see fixed is that we let the key presses
through to the application.  We will need something like what Scott
did with the zoom_key_grab code, only a little different, so we can
handle the brightness up/down keys that don't use a modifier.

> Looks good otherwise.  With the above addressed either in a revised
> patch or patch on top:

Yeah, looks good to me, committed and pushed.  I decided to pull your
branch of libbacklight into weston (just libbacklight.[ch]).  It's 300
lines of code in one C file, so as long as we're the only user and we
need your branch anyway, I'd rather just keep it inside weston.

Kristian

> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> 
> Thanks,
> Jesse
> 
> On Wed, 29 Feb 2012 19:53:50 +0200
> Tiago Vignatti <tiago.vignatti at 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.vignatti at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list