intel: Add a helper for setting backlight without root rights

Once the xserver stops running as root on kms capabable systems, we will need
some other way to access the backlight.

The approach taken in this patch moves most of the heavy lifting to a
helper that runs with root privileges and pipes our requested brightness
into the sysfs backlight interface. Where required, we use pkexec to
launch the helper with the elevated privilege.

v2: Amalgamate much more of the duplicate code.
    Keep the daemon and pipe alive for the lifetime of the backlight interface.
    Provide an alternative for systems without PolicyKit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com> [v1]
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

11 files changed, 560 insertions, 411 deletions

diff --git a/Makefile.am b/Makefile.am
index 71c7698d..6bb48548 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -20,11 +20,7 @@
 
 ACLOCAL_AMFLAGS = ${ACLOCAL_FLAGS} -I m4
 
-SUBDIRS = man xvmc src
-
-if BUILD_TOOLS
-SUBDIRS += tools
-endif
+SUBDIRS = man xvmc src tools
 
 MAINTAINERCLEANFILES = ChangeLog INSTALL
 
diff --git a/configure.ac b/configure.ac
index 4f73ba46..7eb98931 100644
--- a/configure.ac
+++ b/configure.ac
@@ -62,6 +62,23 @@ AC_DISABLE_STATIC
 AC_PROG_LIBTOOL
 AC_SYS_LARGEFILE
 
+# Platform specific settings
+case $host_os in
+  *linux*)
+    backlight_helper=yes
+    ;;
+esac
+
+AC_ARG_ENABLE(backlight-helper,
+              AS_HELP_STRING([--disable-backlight-helper],
+			     [Enable building the backlight helper executable for running X under a normal user [default=auto]]),
+              [backlight_helper="$enableval"],)
+AM_CONDITIONAL(BUILD_BACKLIGHT_HELPER, [test "x$backlight_helper" = "xyes"])
+if test "x$backlight_helper" = "xyes"; then
+	tools_msg="$tools_msg xf86-video-intel-backlight-helper"
+	AC_DEFINE(USE_BACKLIGHT_HELPER, 1, [Enable use of the backlight helper interfaces])
+fi
+
 # Are we in a git checkout?
 dot_git=no
 if test -e .git; then
@@ -218,6 +235,8 @@ if test "x$tools" = "xyes"; then
       AC_CHECK_HEADERS([X11/extensions/XShm.h X11/extensions/shmproto.h X11/extensions/shmstr.h], [], [tools=no],
 		       [#include <X11/Xlibint.h>
 			#include <X11/Xproto.h>])
+
+      tools_msg="$tools_msg intel-virtual-output"
 fi
 
 AM_CONDITIONAL(BUILD_TOOLS, test "x$tools" = "xyes")
@@ -681,6 +700,7 @@ fi
 DRIVER_NAME="intel"
 AC_SUBST([DRIVER_NAME])
 AC_SUBST([moduledir])
+AC_DEFINE_DIR([PREFIX_PATH], prefix, [installation prefix])
 
 AC_CONFIG_FILES([
                 Makefile
@@ -700,6 +720,7 @@ AC_CONFIG_FILES([
                 xvmc/shader/vld/Makefile
 		test/Makefile
 		tools/Makefile
+		tools/org.x.xf86-video-intel.backlight-helper.policy
 ])
 AC_OUTPUT
 
@@ -730,9 +751,7 @@ if test "x$dri_msg" = "x"; then
 	dri_msg=" none"
 fi
 
-if test "x$tools" = "xyes"; then
-	tools_msg=" intel-virtual-output"
-else
+if test "x$tools_msg" = "x"; then
 	tools_msg=" none"
 fi
 
diff --git a/src/Makefile.am b/src/Makefile.am
index b7ccde6c..da751258 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -52,6 +52,8 @@ endif
 NULL:=#
 
 intel_drv_la_SOURCES = \
+	backlight.c \
+	backlight.h \
 	fd.h \
 	fd.c \
 	i915_pciids.h \
diff --git a/src/backlight.c b/src/backlight.c
new file mode 100644
index 00000000..cec0ceb8
--- /dev/null
+++ b/src/backlight.c
@@ -0,0 +1,318 @@
+/***************************************************************************
+
+ Copyright 2014 Intel Corporation.  All Rights Reserved.
+ Copyright 2014 Red Hat, Inc.
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the
+ "Software"), to deal in the Software without restriction, including
+ without limitation the rights to use, copy, modify, merge, publish,
+ distribute, sub license, and/or sell copies of the Software, and to
+ permit persons to whom the Software is furnished to do so, subject to
+ the following conditions:
+
+ The above copyright notice and this permission notice (including the
+ next paragraph) shall be included in all copies or substantial portions
+ of the Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ IN NO EVENT SHALL INTEL, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
+ THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+ **************************************************************************/
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/stat.h>
+
+#include "backlight.h"
+#include "fd.h"
+
+/* Enough for 10 digits of backlight + '\n' + '\0' */
+#define BACKLIGHT_VALUE_LEN 12
+
+/*
+ * Unfortunately this is not as simple as I would like it to be. If selinux is
+ * dropping dbus messages pkexec may block *forever*.
+ *
+ * Backgrounding pkexec by doing System("pkexec ...&") does not work because
+ * that detaches pkexec from its parent at which point its security checks
+ * fail and it refuses to execute the helper.
+ *
+ * So we're left with spawning a helper child which gets levels to set written
+ * to it through a pipe. This turns the blocking forever problem from a hung
+ * machine problem into a simple backlight control not working problem.
+ */
+
+#ifdef __OpenBSD__
+
+#include <dev/wscons/wsconsio.h>
+
+int backlight_set(struct backlight *b, int level)
+{
+	struct wsdisplay_param param;
+
+	if (b->iface == NULL)
+		return;
+
+	if ((unsigned)level > b->max)
+		level = b->max;
+
+	memset(&param, 0, sizeof(param));
+	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+	param.curval = level;
+
+	return ioctl(xf86Info.consoleFd, WSDISPLAYIO_SETPARAM, &param);
+}
+
+int backlight_get(struct backlight *b)
+{
+	struct wsdisplay_param param;
+
+	if (b->iface == NULL)
+		return -1;
+
+	memset(&param, 0, sizeof(param));
+	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+
+	if (ioctl(xf86Info.consoleFd, WSDISPLAYIO_GETPARAM, &param))
+		return -1;
+
+	return param.curval;
+}
+
+int backlight_open(struct backlight *b, char *iface)
+{
+	struct wsdisplay_param param;
+
+	memset(&param, 0, sizeof(param));
+	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+
+	if (ioctl(xf86Info.consoleFd, WSDISPLAYIO_GETPARAM, &param) == -1)
+		return -1;
+
+	b->iface = strdup("wscons");
+	if (b->iface == NULL)
+		return -1;
+
+	b->max = param.max;
+	b->fd = -1;
+
+	return param.curval;
+}
+
+#else
+
+static int
+is_sysfs_fd(int fd)
+{
+	struct stat st;
+	return fstat(fd, &st) == 0 && major(st.st_dev) == 0;
+}
+
+static int
+__backlight_read(const char *iface, const char *file)
+{
+	char buf[1024];
+	int fd, val;
+
+	snprintf(buf, sizeof(buf), "%s/%s/%s", BACKLIGHT_CLASS, iface, file);
+	fd = open(buf, O_RDONLY);
+	if (fd == -1)
+		return -1;
+
+	if (is_sysfs_fd(fd)) {
+		val = read(fd, buf, BACKLIGHT_VALUE_LEN - 1);
+		if (val > 0) {
+			buf[val] = '\0';
+			val = atoi(buf);
+		} else
+			val = -1;
+	} else
+		val = -1;
+	close(fd);
+
+	return val;
+}
+
+int backlight_exists(const char *iface)
+{
+	if (__backlight_read(iface, "brightness") < 0)
+		return 0;
+
+	if (__backlight_read(iface, "max_brightness") <= 0)
+		return 0;
+
+	return 1;
+}
+
+static int __backlight_init(struct backlight *b, char *iface, int fd)
+{
+	b->fd = fd_set_cloexec(fd_set_nonblock(fd));
+	b->iface = iface;
+	return 1;
+}
+
+static int __backlight_direct_init(struct backlight *b, char *iface)
+{
+	char path[1024];
+	int fd;
+
+	snprintf(path, sizeof(path), "%s/%s/brightness", BACKLIGHT_CLASS, iface);
+	fd = open(path, O_RDWR);
+	if (fd < 0)
+		return 0;
+
+	if (!is_sysfs_fd(fd)) {
+		close(fd);
+		return 0;
+	}
+
+	return __backlight_init(b, iface, fd);
+}
+
+static int __backlight_helper_init(struct backlight *b, char *iface)
+{
+#if USE_BACKLIGHT_HELPER
+	struct stat st;
+	char *env[] = { NULL };
+	int use_pkexec = 0;
+	int fds[2];
+
+	/* If system policy is to disallow setuid helpers,
+	 * we fallback to invoking PolicyKit. However, as pkexec
+	 * is quite troublesome and not universally available, we
+	 * still try the old fashioned and simple method first.
+	 * Either way, we have to trust that it is our backlight-helper
+	 * that is run and that we have scrutinised it carefully.
+	 */
+	if (stat(PREFIX_PATH "/libexec/xf86-video-intel-backlight-helper", &st))
+		return 0;
+
+	if ((st.st_mode & (S_IFREG | S_ISUID | S_IXUSR)) != (S_IFREG | S_ISUID | S_IXUSR)) {
+		if (system("pkexec --version"))
+			return 0;
+
+		use_pkexec = 1;
+	}
+
+	if (pipe(fds))
+		return 0;
+
+	switch ((b->pid = fork())) {
+	case 0:
+		close(fds[1]);
+		dup2(fds[0], 0);
+		close(fds[0]);
+		if (use_pkexec) {
+			execlp("pkexec", "pkexec",
+			       PREFIX_PATH "/libexec/xf86-video-intel-backlight-helper",
+			       iface, (char *)0);
+		} else {
+			execle(PREFIX_PATH "/libexec/xf86-video-intel-backlight-helper",
+			       "xf86-video-intel-backlight-helper",
+			       iface, (char *)0, env);
+		}
+		_exit(1);
+		/* unreachable fallthrough */
+	case -1:
+		close(fds[1]);
+		close(fds[0]);
+		return 0;
+
+	default:
+		close(fds[0]);
+		return __backlight_init(b, iface, fds[1]);
+	}
+#else
+	return 0;
+#endif
+}
+
+int backlight_open(struct backlight *b, char *iface)
+{
+	int level;
+
+	if (iface == NULL)
+		return -1;
+
+	b->max = __backlight_read(iface, "max_brightness");
+	if (b->max <= 0)
+		return -1;
+
+	level = __backlight_read(iface, "brightness");
+	if (level < 0)
+		return -1;
+
+	if (!__backlight_direct_init(b, iface) &&
+	    !__backlight_helper_init(b, iface))
+		return -1;
+
+	return level;
+}
+
+int backlight_set(struct backlight *b, int level)
+{
+	char val[BACKLIGHT_VALUE_LEN];
+	int len, ret = 0;
+
+	if (b->iface == NULL)
+		return 0;
+
+	if ((unsigned)level > b->max)
+		level = b->max;
+
+	len = snprintf(val, BACKLIGHT_VALUE_LEN, "%d\n", level);
+	if (write(b->fd, val, len) != len)
+		ret = -1;
+
+	return ret;
+}
+
+int backlight_get(struct backlight *b)
+{
+	int level;
+
+	if (b->iface == NULL)
+		return -1;
+
+	level = __backlight_read(b->iface, "brightness");
+	if (level > b->max)
+		level = b->max;
+	else if (level < 0)
+		level = -1;
+	return level;
+}
+#endif
+
+void backlight_disable(struct backlight *b)
+{
+	if (b->iface == NULL)
+		return;
+
+	if (b->fd != -1)
+		close(b->fd);
+
+	free(b->iface);
+	b->iface = NULL;
+}
+
+void backlight_close(struct backlight *b)
+{
+	backlight_disable(b);
+	if (b->pid)
+		waitpid(b->pid, NULL, 0);
+}
diff --git a/src/backlight.h b/src/backlight.h
new file mode 100644
index 00000000..deecbd00
--- /dev/null
+++ b/src/backlight.h
@@ -0,0 +1,46 @@
+/***************************************************************************
+
+ Copyright 2014 Intel Corporation.  All Rights Reserved.
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the
+ "Software"), to deal in the Software without restriction, including
+ without limitation the rights to use, copy, modify, merge, publish,
+ distribute, sub license, and/or sell copies of the Software, and to
+ permit persons to whom the Software is furnished to do so, subject to
+ the following conditions:
+
+ The above copyright notice and this permission notice (including the
+ next paragraph) shall be included in all copies or substantial portions
+ of the Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ IN NO EVENT SHALL INTEL, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
+ DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
+ THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+ **************************************************************************/
+
+#ifndef BACKLIGHT_H
+#define BACKLIGHT_H
+
+struct backlight {
+	char *iface;
+	int max;
+	int pid, fd;
+};
+
+#define BACKLIGHT_CLASS "/sys/class/backlight"
+
+int backlight_exists(const char *iface);
+
+int backlight_open(struct backlight *backlight, char *iface);
+int backlight_set(struct backlight *backlight, int level);
+int backlight_get(struct backlight *backlight);
+void backlight_disable(struct backlight *backlight);
+void backlight_close(struct backlight *backlight);
+
+#endif /* BACKLIGHT_H */
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 636217aa..83b043c7 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -44,6 +44,7 @@
 #include "sna_reg.h"
 #include "fb/fbpict.h"
 #include "intel_options.h"
+#include "backlight.h"
 
 #include <xf86Crtc.h>
 
@@ -141,9 +142,8 @@ struct sna_output {
 
 	uint32_t dpms_id;
 	int dpms_mode;
-	char *backlight_iface;
+	struct backlight backlight;
 	int backlight_active_level;
-	int backlight_max;
 
 	int num_modes;
 	struct drm_mode_modeinfo *modes;
@@ -186,11 +186,6 @@ static bool sna_mode_wait_for_event(struct sna *sna)
 	return poll(&pfd, 1, -1) == 1;
 }
 
-#define BACKLIGHT_CLASS "/sys/class/backlight"
-
-/* Enough for 10 digits of backlight + '\n' + '\0' */
-#define BACKLIGHT_VALUE_LEN 12
-
 static inline uint32_t fb_id(struct kgem_bo *bo)
 {
 	return bo->delta;
@@ -298,193 +293,40 @@ static void gem_close(int fd, uint32_t handle)
 	(void)drmIoctl(fd, DRM_IOCTL_GEM_CLOSE, &close);
 }
 
-#ifdef __OpenBSD__
-
-#include <dev/wscons/wsconsio.h>
-#include <xf86Priv.h>
+#define BACKLIGHT_NAME             "Backlight"
+#define BACKLIGHT_DEPRECATED_NAME  "BACKLIGHT"
+static Atom backlight_atom, backlight_deprecated_atom;
 
 static void
 sna_output_backlight_set(xf86OutputPtr output, int level)
 {
 	struct sna_output *sna_output = output->driver_private;
-	struct wsdisplay_param param;
-
-	DBG(("%s: level=%d, max=%d\n", __FUNCTION__,
-	     level, sna_output->backlight_max));
-
-	if (!sna_output->backlight_iface)
-		return;
-
-	if ((unsigned)level > sna_output->backlight_max)
-		level = sna_output->backlight_max;
 
-	VG_CLEAR(param);
-	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
-	param.curval = level;
+	DBG(("%s(%s) level=%d, max=%d\n", __FUNCTION__,
+	     output->name, level, sna_output->backlight.max));
 
-	if (ioctl(xf86Info.consoleFd, WSDISPLAYIO_SETPARAM, &param) == -1) {
+	if (backlight_set(&sna_output->backlight, level)) {
 		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
-			   "Failed to set backlight level: %s\n",
-			   strerror(errno));
-	}
-}
-
-static int
-sna_output_backlight_get(xf86OutputPtr output)
-{
-	struct wsdisplay_param param;
-
-	VG_CLEAR(param);
-	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
-
-	if (ioctl(xf86Info.consoleFd, WSDISPLAYIO_GETPARAM, &param) == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
-			   "Failed to get backlight level: %s\n",
-			   strerror(errno));
-		return -1;
-	}
-
-	DBG(("%s: level=%d (max=%d)\n", __FUNCTION__, param.curval, param.max));
-
-	return param.curval;
-}
-
-static void
-sna_output_backlight_init(xf86OutputPtr output)
-{
-	struct sna_output *sna_output = output->driver_private;
-	struct wsdisplay_param param;
-
-	VG_CLEAR(param);
-	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
-
-	if (ioctl(xf86Info.consoleFd, WSDISPLAYIO_GETPARAM, &param) == -1)
-		return;
-
-	DBG(("%s: found 'wscons'\n", __FUNCTION__));
-
-	sna_output->backlight_iface = strdup("wscons");
-	sna_output->backlight_max = param.max;
-	sna_output->backlight_active_level = param.curval;
-}
-
-#else
-
-static void
-sna_output_backlight_set(xf86OutputPtr output, int level)
-{
-	struct sna_output *sna_output = output->driver_private;
-	char path[1024], val[BACKLIGHT_VALUE_LEN];
-	int fd, len, ret;
-
-	DBG(("%s: level=%d, max=%d\n", __FUNCTION__,
-	     level, sna_output->backlight_max));
-
-	if (!sna_output->backlight_iface)
-		return;
-
-	if ((unsigned)level > sna_output->backlight_max)
-		level = sna_output->backlight_max;
-
-	len = snprintf(val, BACKLIGHT_VALUE_LEN, "%d\n", level);
-	sprintf(path, "%s/%s/brightness",
-		BACKLIGHT_CLASS, sna_output->backlight_iface);
-	fd = open(path, O_RDWR);
-	if (fd == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s for backlight "
-			   "control: %s\n", path, strerror(errno));
-		return;
-	}
-
-	ret = write(fd, val, len);
-	if (ret == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "write to %s for backlight "
-			   "control failed: %s\n", path, strerror(errno));
+			   "Failed to set backlight %s for output %s to brightness level %d, disabling\n",
+			   sna_output->backlight.iface, output->name, level);
+		backlight_disable(&sna_output->backlight);
+		if (output->randr_output) {
+			RRDeleteOutputProperty(output->randr_output, backlight_atom);
+			RRDeleteOutputProperty(output->randr_output, backlight_deprecated_atom);
+		}
 	}
-
-	close(fd);
 }
 
 static int
 sna_output_backlight_get(xf86OutputPtr output)
 {
 	struct sna_output *sna_output = output->driver_private;
-	char path[1024], val[BACKLIGHT_VALUE_LEN];
-	int fd, level;
-
-	sprintf(path, "%s/%s/actual_brightness",
-		BACKLIGHT_CLASS, sna_output->backlight_iface);
-	fd = open(path, O_RDONLY);
-	if (fd == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s "
-			   "for backlight control: %s\n", path, strerror(errno));
-		return -1;
-	}
-
-	memset(val, 0, sizeof(val));
-	if (read(fd, val, BACKLIGHT_VALUE_LEN) == -1) {
-		close(fd);
-		return -1;
-	}
-
-	close(fd);
-
-	level = atoi(val);
-	DBG(("%s: level=%d (max=%d)\n",
-	     __FUNCTION__, level, sna_output->backlight_max));
-
-	if (level > sna_output->backlight_max)
-		level = sna_output->backlight_max;
-	else if (level < 0)
-		level = -1;
+	int level = backlight_get(&sna_output->backlight);
+	DBG(("%s(%s) level=%d, max=%d\n", __FUNCTION__,
+	     output->name, level, sna_output->backlight.max));
 	return level;
 }
 
-static int
-sna_output_backlight_get_max(xf86OutputPtr output)
-{
-	struct sna_output *sna_output = output->driver_private;
-	char path[1024], val[BACKLIGHT_VALUE_LEN];
-	struct stat st;
-	int fd, max = 0;
-
-	/* We are used as an initial check to see if we can
-	 * control the backlight, so first test if we can set values.
-	 */
-	sprintf(path, "%s/%s/brightness",
-		BACKLIGHT_CLASS, sna_output->backlight_iface);
-	if (access(path, R_OK | W_OK))
-		return -1;
-
-	if (stat(path, &st))
-		return -1;
-
-	if (major(st.st_dev)) /* is this a kernel psuedo filesystem? */
-		return -1;
-
-	sprintf(path, "%s/%s/max_brightness",
-		BACKLIGHT_CLASS, sna_output->backlight_iface);
-	fd = open(path, O_RDONLY);
-	if (fd == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s "
-			   "for backlight control: %s\n", path, strerror(errno));
-		return -1;
-	}
-
-	memset(val, 0, sizeof(val));
-	if (read(fd, val, BACKLIGHT_VALUE_LEN) == -1) {
-		close(fd);
-		return -1;
-	}
-
-	close(fd);
-
-	max = atoi(val);
-	if (max <= 0)
-		max = -1;
-	return max;
-}
-
 enum {
 	PLATFORM,
 	FIRMWARE,
@@ -495,21 +337,16 @@ enum {
 static char *
 has_user_backlight_override(xf86OutputPtr output)
 {
-	struct sna_output *sna_output = output->driver_private;
 	struct sna *sna = to_sna(output->scrn);
 	const char *str;
-	int max;
 
 	str = xf86GetOptValString(sna->Options, OPTION_BACKLIGHT);
 	if (str == NULL)
 		return NULL;
 
-	sna_output->backlight_iface = (char *)str;
-	max = sna_output_backlight_get_max(output);
-	sna_output->backlight_iface = NULL;
-	if (max <= 0) {
+	if (!backlight_exists(str)) {
 		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
-			   "unrecognised backlight control interface '%s'\n",
+			   "Unrecognised backlight control interface '%s'\n",
 			   str);
 		return NULL;
 	}
@@ -520,7 +357,6 @@ has_user_backlight_override(xf86OutputPtr output)
 static char *
 has_device_backlight(xf86OutputPtr output, int *best_type)
 {
-	struct sna_output *sna_output = output->driver_private;
 	struct sna *sna = to_sna(output->scrn);
 	struct pci_device *pci;
 	char path[1024];
@@ -576,12 +412,8 @@ has_device_backlight(xf86OutputPtr output, int *best_type)
 
 		if (v < *best_type) {
 			char *copy;
-			int max;
 
-			sna_output->backlight_iface = de->d_name;
-			max = sna_output_backlight_get_max(output);
-			sna_output->backlight_iface = NULL;
-			if (max <= 0)
+			if (!backlight_exists(de->d_name))
 				continue;
 
 			copy = strdup(de->d_name);
@@ -615,7 +447,6 @@ has_backlight(xf86OutputPtr output, int *best_type)
 		"acpi_video0",
 		"intel_backlight",
 	};
-	struct sna_output *sna_output = output->driver_private;
 	char *best_iface = NULL;
 	DIR *dir;
 	struct dirent *de;
@@ -669,14 +500,10 @@ has_backlight(xf86OutputPtr output, int *best_type)
 
 		if (v < *best_type) {
 			char *copy;
-			int max;
 
 			/* XXX detect right backlight for multi-GPU/panels */
 
-			sna_output->backlight_iface = de->d_name;
-			max = sna_output_backlight_get_max(output);
-			sna_output->backlight_iface = NULL;
-			if (max <= 0)
+			if (!backlight_exists(de->d_name))
 				continue;
 
 			copy = strdup(de->d_name);
@@ -713,12 +540,15 @@ sna_output_backlight_init(xf86OutputPtr output)
 	if (best_iface)
 		goto done;
 
-	return;
+	best_type = PLATFORM;
+	best_iface = NULL;
 
 done:
-	sna_output->backlight_iface = best_iface;
-	sna_output->backlight_max = sna_output_backlight_get_max(output);
-	sna_output->backlight_active_level = sna_output_backlight_get(output);
+	sna_output->backlight_active_level =
+		backlight_open(&sna_output->backlight, best_iface);
+	if (sna_output->backlight_active_level < 0)
+		return;
+
 	switch (best_type) {
 	case INT_MAX: best_iface = (char *)"user"; from = X_CONFIG; break;
 	case FIRMWARE: best_iface = (char *)"firmware"; break;
@@ -727,10 +557,9 @@ done:
 	default: best_iface = (char *)"unknown"; break;
 	}
 	xf86DrvMsg(output->scrn->scrnIndex, from,
-		   "found backlight control interface %s (type '%s')\n",
-		   sna_output->backlight_iface, best_iface);
+		   "Found backlight control interface %s (type '%s') for output %s\n",
+		   sna_output->backlight.iface, best_iface, output->name);
 }
-#endif
 
 static char *canonical_kmode_name(const struct drm_mode_modeinfo *kmode)
 {
@@ -2585,7 +2414,7 @@ sna_output_destroy(xf86OutputPtr output)
 	free(sna_output->prop_ids);
 	free(sna_output->prop_values);
 
-	free(sna_output->backlight_iface);
+	backlight_close(&sna_output->backlight);
 
 	free(sna_output);
 	output->driver_private = NULL;
@@ -2596,7 +2425,7 @@ sna_output_dpms_backlight(xf86OutputPtr output, int oldmode, int mode)
 {
 	struct sna_output *sna_output = output->driver_private;
 
-	if (!sna_output->backlight_iface)
+	if (!sna_output->backlight.iface)
 		return;
 
 	DBG(("%s(%s) -- %d -> %d\n", __FUNCTION__, output->name, oldmode, mode));
@@ -2700,10 +2529,6 @@ sna_output_create_ranged_atom(xf86OutputPtr output, Atom *atom,
 			   "RRChangeOutputProperty error, %d\n", err);
 }
 
-#define BACKLIGHT_NAME             "Backlight"
-#define BACKLIGHT_DEPRECATED_NAME  "BACKLIGHT"
-static Atom backlight_atom, backlight_deprecated_atom;
-
 static void
 sna_output_create_resources(xf86OutputPtr output)
 {
@@ -2775,20 +2600,20 @@ sna_output_create_resources(xf86OutputPtr output)
 		}
 	}
 
-	if (sna_output->backlight_iface) {
+	if (sna_output->backlight.iface) {
 		/* Set up the backlight property, which takes effect
 		 * immediately and accepts values only within the
 		 * backlight_range.
 		 */
 		sna_output_create_ranged_atom(output, &backlight_atom,
 					      BACKLIGHT_NAME, 0,
-					      sna_output->backlight_max,
+					      sna_output->backlight.max,
 					      sna_output->backlight_active_level,
 					      FALSE);
 		sna_output_create_ranged_atom(output,
 					      &backlight_deprecated_atom,
 					      BACKLIGHT_DEPRECATED_NAME, 0,
-					      sna_output->backlight_max,
+					      sna_output->backlight.max,
 					      sna_output->backlight_active_level,
 					      FALSE);
 	}
@@ -2813,8 +2638,8 @@ sna_output_set_property(xf86OutputPtr output, Atom property,
 
 		val = *(INT32 *)value->data;
 		DBG(("%s: setting backlight to %d (max=%d)\n",
-		     __FUNCTION__, (int)val, sna_output->backlight_max));
-		if (val < 0 || val > sna_output->backlight_max)
+		     __FUNCTION__, (int)val, sna_output->backlight.max));
+		if (val < 0 || val > sna_output->backlight.max)
 			return FALSE;
 
 		if (sna_output->dpms_mode == DPMSModeOn)
@@ -2880,7 +2705,7 @@ sna_output_get_property(xf86OutputPtr output, Atom property)
 	if (property == backlight_atom || property == backlight_deprecated_atom) {
 		INT32 val;
 
-		if (!sna_output->backlight_iface)
+		if (!sna_output->backlight.iface)
 			return FALSE;
 
 		val = sna_output_backlight_get(output);
@@ -2890,7 +2715,6 @@ sna_output_get_property(xf86OutputPtr output, Atom property)
 		err = RRChangeOutputProperty(output->randr_output, property,
 					     XA_INTEGER, 32, PropModeReplace, 1, &val,
 					     FALSE, FALSE);
-
 		if (err != 0) {
 			xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
 				   "RRChangeOutputProperty error, %d\n", err);
diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
index e1b29201..c7c2d7c9 100644
--- a/src/uxa/intel_display.c
+++ b/src/uxa/intel_display.c
@@ -43,6 +43,7 @@
 #include "intel.h"
 #include "intel_bufmgr.h"
 #include "intel_options.h"
+#include "backlight.h"
 #include "xf86drm.h"
 #include "xf86drmMode.h"
 #include "X11/Xatom.h"
@@ -120,9 +121,8 @@ struct intel_output {
 	int panel_vdisplay;
 
 	int dpms_mode;
-	const char *backlight_iface;
+	struct backlight backlight;
 	int backlight_active_level;
-	int backlight_max;
 	xf86OutputPtr output;
 	struct list link;
 };
@@ -139,68 +139,6 @@ crtc_id(struct intel_crtc *crtc)
 	return crtc->mode_crtc->crtc_id;
 }
 
-#ifdef __OpenBSD__
-
-#include <dev/wscons/wsconsio.h>
-#include "xf86Priv.h"
-
-static void
-intel_output_backlight_set(xf86OutputPtr output, int level)
-{
-	struct intel_output *intel_output = output->driver_private;
-	struct wsdisplay_param param;
-
-	if (level > intel_output->backlight_max)
-		level = intel_output->backlight_max;
-	if (! intel_output->backlight_iface || level < 0)
-		return;
-
-	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
-	param.curval = level;
-	if (ioctl(xf86Info.consoleFd, WSDISPLAYIO_SETPARAM, &param) == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
-			   "Failed to set backlight level: %s\n",
-			   strerror(errno));
-	}
-}
-
-static int
-intel_output_backlight_get(xf86OutputPtr output)
-{
-	struct wsdisplay_param param;
-
-	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
-	if (ioctl(xf86Info.consoleFd, WSDISPLAYIO_GETPARAM, &param) == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
-			   "Failed to get backlight level: %s\n",
-			   strerror(errno));
-		return -1;
-	}
-
-	return param.curval;
-}
-
-static void
-intel_output_backlight_init(xf86OutputPtr output)
-{
-	struct intel_output *intel_output = output->driver_private;
-	struct wsdisplay_param param;
-
-	param.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
-	if (ioctl(xf86Info.consoleFd, WSDISPLAYIO_GETPARAM, &param) == -1) {
-		intel_output->backlight_iface = NULL;
-		return;
-	}
-
-	intel_output->backlight_iface = "wscons";
-	intel_output->backlight_max = param.max;
-	intel_output->backlight_active_level = param.curval;
-}
-
-#else
-
-#define BACKLIGHT_CLASS "/sys/class/backlight"
-
 /*
  * List of available kernel interfaces in priority order
  */
@@ -220,135 +158,50 @@ static const char *backlight_interfaces[] = {
 	"intel_backlight",
 	NULL,
 };
-/*
- * Must be long enough for BACKLIGHT_CLASS + '/' + longest in above table +
- * '/' + "max_backlight"
- */
-#define BACKLIGHT_PATH_LEN 80
-/* Enough for 10 digits of backlight + '\n' + '\0' */
-#define BACKLIGHT_VALUE_LEN 12
 
 static void
 intel_output_backlight_set(xf86OutputPtr output, int level)
 {
 	struct intel_output *intel_output = output->driver_private;
-	char path[BACKLIGHT_PATH_LEN], val[BACKLIGHT_VALUE_LEN];
-	int fd, len, ret;
-
-	if (level > intel_output->backlight_max)
-		level = intel_output->backlight_max;
-	if (! intel_output->backlight_iface || level < 0)
-		return;
-
-	len = snprintf(val, BACKLIGHT_VALUE_LEN, "%d\n", level);
-	sprintf(path, "%s/%s/brightness",
-		BACKLIGHT_CLASS, intel_output->backlight_iface);
-	fd = open(path, O_RDWR);
-	if (fd == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s for backlight "
-			   "control: %s\n", path, strerror(errno));
-		return;
-	}
-
-	ret = write(fd, val, len);
-	if (ret == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "write to %s for backlight "
-			   "control failed: %s\n", path, strerror(errno));
+	if (backlight_set(&intel_output->backlight, level) < 0) {
+		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR,
+			   "failed to set backlight %s to brightness level %d, disabling\n",
+			   intel_output->backlight.iface, level);
+		backlight_disable(&intel_output->backlight);
 	}
-
-	close(fd);
 }
 
 static int
 intel_output_backlight_get(xf86OutputPtr output)
 {
 	struct intel_output *intel_output = output->driver_private;
-	char path[BACKLIGHT_PATH_LEN], val[BACKLIGHT_VALUE_LEN];
-	int fd, level;
-
-	sprintf(path, "%s/%s/actual_brightness",
-		BACKLIGHT_CLASS, intel_output->backlight_iface);
-	fd = open(path, O_RDONLY);
-	if (fd == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s "
-			   "for backlight control: %s\n", path, strerror(errno));
-		return -1;
-	}
-
-	memset(val, 0, sizeof(val));
-	if (read(fd, val, BACKLIGHT_VALUE_LEN) == -1) {
-		close(fd);
-		return -1;
-	}
-
-	close(fd);
-
-	level = atoi(val);
-	if (level > intel_output->backlight_max)
-		level = intel_output->backlight_max;
-	if (level < 0)
-		level = -1;
-	return level;
-}
-
-static int
-intel_output_backlight_get_max(xf86OutputPtr output)
-{
-	struct intel_output *intel_output = output->driver_private;
-	char path[BACKLIGHT_PATH_LEN], val[BACKLIGHT_VALUE_LEN];
-	int fd, max = 0;
-
-	/* We are used as an initial check to see if we can
-	 * control the backlight, so first test if we can set values.
-	 */
-	sprintf(path, "%s/%s/brightness",
-		BACKLIGHT_CLASS, intel_output->backlight_iface);
-	if (access(path, R_OK | W_OK))
-		return -1;
-
-	sprintf(path, "%s/%s/max_brightness",
-		BACKLIGHT_CLASS, intel_output->backlight_iface);
-	fd = open(path, O_RDONLY);
-	if (fd == -1) {
-		xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s "
-			   "for backlight control: %s\n", path, strerror(errno));
-		return -1;
-	}
-
-	memset(val, 0, sizeof(val));
-	if (read(fd, val, BACKLIGHT_VALUE_LEN) == -1) {
-		close(fd);
-		return -1;
-	}
-
-	close(fd);
-
-	max = atoi(val);
-	if (max <= 0)
-		max = -1;
-	return max;
+	return backlight_get(&intel_output->backlight);
 }
 
 static void
 intel_output_backlight_init(xf86OutputPtr output)
 {
+#ifdef __OpenBSD__
+	intel_output->backlight_active_level =
+		backlight_init(&intel_output->backlight, NULL);
+	if (intel_output->backlight_active_level != -1) {
+		xf86DrvMsg(output->scrn->scrnIndex, X_PROBED,
+			   "found backlight control interface\n");
+	}
+#else
 	struct intel_output *intel_output = output->driver_private;
 	intel_screen_private *intel = intel_get_screen_private(output->scrn);
-	char path[BACKLIGHT_PATH_LEN];
-	struct stat buf;
 	char *str;
 	int i;
 
 	str = xf86GetOptValString(intel->Options, OPTION_BACKLIGHT);
 	if (str != NULL) {
-		sprintf(path, "%s/%s", BACKLIGHT_CLASS, str);
-		if (!stat(path, &buf)) {
-			intel_output->backlight_iface = str;
-			intel_output->backlight_max = intel_output_backlight_get_max(output);
-			if (intel_output->backlight_max > 0) {
-				intel_output->backlight_active_level = intel_output_backlight_get(output);
+		if (backlight_exists(str)) {
+			intel_output->backlight_active_level =
+				backlight_open(&intel_output->backlight, strdup(str));
+			if (intel_output->backlight_active_level != -1) {
 				xf86DrvMsg(output->scrn->scrnIndex, X_CONFIG,
-					   "found backlight control interface %s\n", path);
+					   "found backlight control interface %s\n", str);
 				return;
 			}
 		}
@@ -357,22 +210,18 @@ intel_output_backlight_init(xf86OutputPtr output)
 	}
 
 	for (i = 0; backlight_interfaces[i] != NULL; i++) {
-		sprintf(path, "%s/%s", BACKLIGHT_CLASS, backlight_interfaces[i]);
-		if (!stat(path, &buf)) {
-			intel_output->backlight_iface = backlight_interfaces[i];
-			intel_output->backlight_max = intel_output_backlight_get_max(output);
-			if (intel_output->backlight_max > 0) {
-				intel_output->backlight_active_level = intel_output_backlight_get(output);
+		if (backlight_exists(backlight_interfaces[i])) {
+			intel_output->backlight_active_level =
+				backlight_open(&intel_output->backlight, strdup(backlight_interfaces[i]));
+			if (intel_output->backlight_active_level != -1) {
 				xf86DrvMsg(output->scrn->scrnIndex, X_PROBED,
-					   "found backlight control interface %s\n", path);
+					   "found backlight control interface %s\n", backlight_interfaces[i]);
 				return;
 			}
 		}
 	}
-	intel_output->backlight_iface = NULL;
-}
-
 #endif
+}
 
 static void
 mode_from_kmode(ScrnInfoPtr scrn,
@@ -1083,6 +932,7 @@ intel_output_destroy(xf86OutputPtr output)
 	intel_output->mode_output = NULL;
 
 	list_del(&intel_output->link);
+	backlight_close(&intel_output->backlight);
 	free(intel_output);
 
 	output->driver_private = NULL;
@@ -1093,7 +943,7 @@ intel_output_dpms_backlight(xf86OutputPtr output, int oldmode, int mode)
 {
 	struct intel_output *intel_output = output->driver_private;
 
-	if (!intel_output->backlight_iface)
+	if (!intel_output->backlight.iface)
 		return;
 
 	if (mode == DPMSModeOn) {
@@ -1286,20 +1136,20 @@ intel_output_create_resources(xf86OutputPtr output)
 		}
 	}
 
-	if (intel_output->backlight_iface) {
+	if (intel_output->backlight.iface) {
 		/* Set up the backlight property, which takes effect
 		 * immediately and accepts values only within the
 		 * backlight_range.
 		 */
 		intel_output_create_ranged_atom(output, &backlight_atom,
 					BACKLIGHT_NAME, 0,
-					intel_output->backlight_max,
+					intel_output->backlight.max,
 					intel_output->backlight_active_level,
 					FALSE);
 		intel_output_create_ranged_atom(output,
 					&backlight_deprecated_atom,
 					BACKLIGHT_DEPRECATED_NAME, 0,
-					intel_output->backlight_max,
+					intel_output->backlight.max,
 					intel_output->backlight_active_level,
 					FALSE);
 	}
@@ -1323,7 +1173,7 @@ intel_output_set_property(xf86OutputPtr output, Atom property,
 		}
 
 		val = *(INT32 *)value->data;
-		if (val < 0 || val > intel_output->backlight_max)
+		if (val < 0 || val > intel_output->backlight.max)
 			return FALSE;
 
 		if (intel_output->dpms_mode == DPMSModeOn)
@@ -1389,7 +1239,7 @@ intel_output_get_property(xf86OutputPtr output, Atom property)
 	if (property == backlight_atom || property == backlight_deprecated_atom) {
 		INT32 val;
 
-		if (! intel_output->backlight_iface)
+		if (!intel_output->backlight.iface)
 			return FALSE;
 
 		val = intel_output_backlight_get(output);
diff --git a/tools/.gitignore b/tools/.gitignore
index 72150ff9..36868c69 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -1 +1,3 @@
 intel-virtual-output
+xf86-video-intel-backlight-helper
+org.x.xf86-video-intel.backlight-helper.policy
diff --git a/tools/Makefile.am b/tools/Makefile.am
index d294e2ad..a5667f34 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -20,23 +20,45 @@
 
 AM_CFLAGS = \
 	@CWARNFLAGS@ \
-	$(TOOL_CFLAGS) \
 	@NOWARNFLAGS@ \
 	$(NULL)
 
 drivermandir = $(DRIVER_MAN_DIR)
+backlight_helperdir = $(prefix)/libexec
+policydir = $(datarootdir)/polkit-1/actions
 
+if BUILD_TOOLS
 bin_PROGRAMS = intel-virtual-output
 driverman_DATA = intel-virtual-output.$(DRIVER_MAN_SUFFIX)
+endif
+
+if BUILD_BACKLIGHT_HELPER
+backlight_helper_PROGRAMS = xf86-video-intel-backlight-helper
+nodist_policy_DATA = org.x.xf86-video-intel.backlight-helper.policy
+
+backlight_helper = $(backlight_helperdir)/xf86-video-intel-backlight-helper
+install-data-hook:
+	-chown root $(backlight_helper) && chmod u+s $(backlight_helper)
+endif
 
+intel_virtual_output_CFLAGS = \
+	@CWARNFLAGS@ \
+	$(TOOL_CFLAGS) \
+	@NOWARNFLAGS@ \
+	$(NULL)
 intel_virtual_output_SOURCES = \
 	virtual.c \
 	$(NULL)
+intel_virtual_output_LDADD = \
+	$(TOOL_LIBS) \
+	$(NULL)
 
-intel_virtual_output_LDADD = $(TOOL_LIBS)
+xf86_video_intel_backlight_helper_SOURCES = \
+	backlight_helper.c \
+	$(NULL)
 
-EXTRA_DIST = intel-virtual-output.man
-CLEANFILES = $(driverman_DATA)
+EXTRA_DIST = intel-virtual-output.man org.x.xf86-video-intel.backlight-helper.policy.in
+CLEANFILES = $(driverman_DATA) $(nodist_policy_DATA)
 
 # String replacements in MAN_SUBSTS now come from xorg-macros.m4 via configure
 SUFFIXES = .$(DRIVER_MAN_SUFFIX) .man
diff --git a/tools/backlight_helper.c b/tools/backlight_helper.c
new file mode 100644
index 00000000..fc16fcea
--- /dev/null
+++ b/tools/backlight_helper.c
@@ -0,0 +1,51 @@
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
+
+int main(int argc, char *argv[])
+{
+	struct stat st;
+	char buf[1024], *b = buf;
+	int len, fd;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s <iface>\n", argv[0]);
+		return 1;
+	}
+
+	snprintf(buf, sizeof(buf), "/sys/class/backlight/%s/brightness", argv[1]);
+	fd = open(buf, O_RDWR);
+	if (fd < 0 || fstat(fd, &st) || major(st.st_dev)) {
+		fprintf(stderr, "Cannot access backlight interface '%s'\n", argv[1]);
+		return 1;
+	}
+
+	while ((len = read(0, b, sizeof(buf) - (b - buf) - 1)) > 0) {
+		len += b - buf;
+		buf[len] = '\0';
+
+		b = buf;
+		do {
+			char *end = strchr(b, '\n');
+			if (end == NULL)
+				break;
+
+			++end;
+			if (write(fd, b, end - b) != end - b) {
+				fprintf(stderr, "Failed to update backlight interface '%s'\n", argv[1]);
+				return 2;
+			}
+
+			b = end;
+		} while (1);
+
+		memmove(buf, b, len = buf + len - b);
+		b = buf + len;
+	}
+
+	return 0;
+}
diff --git a/tools/org.x.xf86-video-intel.backlight-helper.policy.in b/tools/org.x.xf86-video-intel.backlight-helper.policy.in
new file mode 100644
index 00000000..37e96226
--- /dev/null
+++ b/tools/org.x.xf86-video-intel.backlight-helper.policy.in
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE policyconfig PUBLIC
+ "-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/PolicyKit/1.0/policyconfig.dtd">
+<policyconfig>
+  <vendor>The X.Org project</vendor>
+  <vendor_url>https://01.org/linuxgraphics/community/xf86-video-intel</vendor_url>
+  <icon_name>brightness</icon_name>
+  <action id="org.x.xf86-video-intel.backlight-helper">
+    <description>Modify lcd panel brightness</description>
+    <message>Authentication is required to modify the lcd panel brightness</message>
+    <defaults>
+      <allow_any>no</allow_any>
+      <allow_inactive>no</allow_inactive>
+      <allow_active>yes</allow_active>
+    </defaults>
+    <annotate key="org.freedesktop.policykit.exec.path">@prefix@/libexec/xf86-video-intel-backlight-helper</annotate>
+  </action>
+</policyconfig>