From patchwork Fri Feb 14 23:02:37 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 3654921 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 9C20F9F1EE for ; Fri, 14 Feb 2014 23:02:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 21393201BF for ; Fri, 14 Feb 2014 23:02:57 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 2CDB0201BB for ; Fri, 14 Feb 2014 23:02:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 88BB4FA6A1; Fri, 14 Feb 2014 15:02:53 -0800 (PST) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by gabe.freedesktop.org (Postfix) with ESMTP id C69B3FA67B for ; Fri, 14 Feb 2014 15:02:42 -0800 (PST) Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1EN2gFW028921 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 14 Feb 2014 18:02:42 -0500 Received: from shalem.localdomain.com (vpn1-5-47.ams2.redhat.com [10.36.5.47]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s1EN2d7b010760; Fri, 14 Feb 2014 18:02:41 -0500 From: Hans de Goede To: intel-gfx@lists.freedesktop.org Date: Sat, 15 Feb 2014 00:02:37 +0100 Message-Id: <1392418957-12889-3-git-send-email-hdegoede@redhat.com> In-Reply-To: <1392418957-12889-1-git-send-email-hdegoede@redhat.com> References: <1392418957-12889-1-git-send-email-hdegoede@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 Cc: peter.hutterer@redhat.com Subject: [Intel-gfx] [PATCH 3/3] xf86-video-intel: Add a helper for setting backlight without root rights X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD,UNPARSEABLE_RELAY,URIBL_BLACK autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 leaves most of the heavy lifting (wrt doing everything suid root safe) to pkexec, as is done in ie gnome-settings-daemon, which controls the backlight directly on ati and nouveau cards. This commit adds src/backlight.h and src/backlight.c as a place to share common backlight code, in the future most of the duplicate backlight code inside src/sna/sna_display.c and src/uxa/intel_display.c should be moved there. Signed-off-by: Hans de Goede --- Makefile.am | 4 + backlight_helper/Makefile.am | 37 ++++++ backlight_helper/backlight_helper.c | 37 ++++++ ...g.x.xf86-video-intel.backlight-helper.policy.in | 19 +++ configure.ac | 11 ++ src/Makefile.am | 2 + src/backlight.c | 142 +++++++++++++++++++++ src/backlight.h | 22 ++++ src/sna/sna_display.c | 17 +-- src/uxa/intel_display.c | 17 +-- 10 files changed, 292 insertions(+), 16 deletions(-) create mode 100644 backlight_helper/Makefile.am create mode 100644 backlight_helper/backlight_helper.c create mode 100644 backlight_helper/org.x.xf86-video-intel.backlight-helper.policy.in create mode 100644 src/backlight.c create mode 100644 src/backlight.h diff --git a/Makefile.am b/Makefile.am index 71c7698..d6b97b6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -26,6 +26,10 @@ if BUILD_TOOLS SUBDIRS += tools endif +if BUILD_BACKLIGHT_HELPER +SUBDIRS += backlight_helper +endif + MAINTAINERCLEANFILES = ChangeLog INSTALL if HAVE_X11 diff --git a/backlight_helper/Makefile.am b/backlight_helper/Makefile.am new file mode 100644 index 0000000..a4a5050 --- /dev/null +++ b/backlight_helper/Makefile.am @@ -0,0 +1,37 @@ +# Copyright 2005 Adam Jackson. +# +# 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 +# on 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 +# ADAM JACKSON 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. + +AM_CFLAGS = \ + @CWARNFLAGS@ \ + @NOWARNFLAGS@ \ + $(NULL) + +backlight_helperdir = $(prefix)/libexec +policydir = $(datarootdir)/polkit-1/actions + +backlight_helper_PROGRAMS = xf86-video-intel-backlight-helper +nodist_policy_DATA = org.x.xf86-video-intel.backlight-helper.policy + +xf86_video_intel_backlight_helper_SOURCES = \ + backlight_helper.c \ + $(NULL) + +EXTRA_DIST = org.x.xf86-video-intel.backlight-helper.policy.in +DISTCLEANFILES = $(nodist_policy_DATA) diff --git a/backlight_helper/backlight_helper.c b/backlight_helper/backlight_helper.c new file mode 100644 index 0000000..c681aae --- /dev/null +++ b/backlight_helper/backlight_helper.c @@ -0,0 +1,37 @@ +#include +#include +#include +#include + +#define BUF_SIZE 1024 + +int main(int argc, char *argv[]) +{ + char buf[BUF_SIZE]; + FILE *f; + int r; + + if (argc != 3) { + fprintf(stderr, "Error %s expects exactly 2 parameters\n", + argv[0]); + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + + snprintf(buf, BUF_SIZE, "/sys/class/backlight/%s/brightness", argv[1]); + f = fopen(buf, "r+"); + if (!f) { + fprintf(stderr, "Error opening %s: %s\n", buf, strerror(errno)); + return 1; + } + + r = fprintf(f, "%s\n", argv[2]); + if (r < 0) { + fprintf(stderr, "Error writing %s: %s\n", buf, strerror(errno)); + return 1; + } + + fclose(f); + + return 0; +} diff --git a/backlight_helper/org.x.xf86-video-intel.backlight-helper.policy.in b/backlight_helper/org.x.xf86-video-intel.backlight-helper.policy.in new file mode 100644 index 0000000..37e9622 --- /dev/null +++ b/backlight_helper/org.x.xf86-video-intel.backlight-helper.policy.in @@ -0,0 +1,19 @@ + + + + The X.Org project + https://01.org/linuxgraphics/community/xf86-video-intel + brightness + + Modify lcd panel brightness + Authentication is required to modify the lcd panel brightness + + no + no + yes + + @prefix@/libexec/xf86-video-intel-backlight-helper + + diff --git a/configure.ac b/configure.ac index 4f73ba4..6bc80de 100644 --- a/configure.ac +++ b/configure.ac @@ -62,6 +62,14 @@ AC_DISABLE_STATIC AC_PROG_LIBTOOL AC_SYS_LARGEFILE +# Platform specific settings +case $host_os in + *linux*) + backlight_helper=yes + ;; +esac +AM_CONDITIONAL(BUILD_BACKLIGHT_HELPER, [test "x$backlight_helper" = "xyes"]) + # Are we in a git checkout? dot_git=no if test -e .git; then @@ -681,6 +689,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 +709,8 @@ AC_CONFIG_FILES([ xvmc/shader/vld/Makefile test/Makefile tools/Makefile + backlight_helper/Makefile + backlight_helper/org.x.xf86-video-intel.backlight-helper.policy ]) AC_OUTPUT diff --git a/src/Makefile.am b/src/Makefile.am index e87f030..842d0b2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -59,6 +59,8 @@ intel_drv_la_SOURCES = \ intel_driver.h \ intel_options.c \ intel_module.c \ + backlight.c \ + backlight.h \ compat-api.h \ $(NULL) diff --git a/src/backlight.c b/src/backlight.c new file mode 100644 index 0000000..a98b6e6 --- /dev/null +++ b/src/backlight.c @@ -0,0 +1,142 @@ +/*************************************************************************** + + Copyright 2013 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. + + **************************************************************************/ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "intel_driver.h" +#include "backlight.h" + +#define READ_PIPE 0 +#define WRITE_PIPE 1 + +#ifdef __linux__ /* The helper is for linux only */ + +/* + * 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. + */ + +static void backlight_helper_main(struct backlight_helper *helper) +{ + char cmd[1024]; + int r, level; + + while (1) { + r = read(helper->fds[READ_PIPE], &level, sizeof(level)); + if (r != sizeof(level)) + break; + snprintf(cmd, sizeof(cmd), + "pkexec %s/libexec/xf86-video-intel-backlight-helper %s %d", + PREFIX_PATH, helper->iface, level); + System(cmd); + } + close(helper->fds[READ_PIPE]); + _exit(0); +} + +static int backlight_helper_init(struct backlight_helper *helper, + xf86OutputPtr output, const char *iface) +{ + int r; + + if (helper->child_created) + return 0; + + helper->iface = iface; + + r = pipe(helper->fds); + if (r) { + xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, + "backlight pipe error: %s\n", strerror(errno)); + return r; + } + intel_fd_set_cloexec(helper->fds[READ_PIPE]); + intel_fd_set_cloexec(helper->fds[WRITE_PIPE]); + intel_fd_set_nonblock(helper->fds[WRITE_PIPE]); + + helper->pid = fork(); + switch (helper->pid) { + case -1: + close(helper->fds[READ_PIPE]); + close(helper->fds[WRITE_PIPE]); + xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, + "backlight fork error: %s\n", strerror(errno)); + return -1; + case 0: + close(helper->fds[WRITE_PIPE]); + backlight_helper_main(helper); + break; + default: + close(helper->fds[READ_PIPE]); + helper->child_created = 1; + } + return 0; +} + +void backlight_helper_set(struct backlight_helper *helper, + xf86OutputPtr output, const char *iface, int level) +{ + int r; + + r = backlight_helper_init(helper, output, iface); + if (r) + return; + + r = write(helper->fds[WRITE_PIPE], &level, sizeof(level)); + if (r != sizeof(level)) + xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, + "backlight write error: %s\n", strerror(errno)); +} + +void backlight_helper_cleanup(struct backlight_helper *helper) +{ + if (!helper->child_created) + return; + + close(helper->fds[WRITE_PIPE]); + waitpid(helper->pid, NULL, 0); +} + +#endif /* The helper is for linux only */ diff --git a/src/backlight.h b/src/backlight.h new file mode 100644 index 0000000..dd036c4 --- /dev/null +++ b/src/backlight.h @@ -0,0 +1,22 @@ +#ifndef BACKLIGHT_H +#define BACKLIGHT_H + +struct backlight_helper { + int child_created; + int fds[2]; + int pid; + const char *iface; +}; + +#ifdef __linux__ + +void backlight_helper_set(struct backlight_helper *helper, + xf86OutputPtr output, const char *iface, int level); +void backlight_helper_cleanup(struct backlight_helper *helper); + +#else + +#define backlight_helper_cleanup(helper) + +#endif +#endif diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c index 521d4ef..559c8fa 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 @@ -144,6 +145,7 @@ struct sna_output { char *backlight_iface; int backlight_active_level; int backlight_max; + struct backlight_helper backlight_helper; int num_modes; struct drm_mode_modeinfo *modes; @@ -391,6 +393,12 @@ sna_output_backlight_set(xf86OutputPtr output, int level) BACKLIGHT_CLASS, sna_output->backlight_iface); fd = open(path, O_RDWR); if (fd == -1) { + /* If this is an access error, invoke the helper instead */ + if (errno == EACCES) { + backlight_helper_set(&sna_output->backlight_helper, + output, sna_output->backlight_iface, level); + return; + } xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s for backlight " "control: %s\n", path, strerror(errno)); return; @@ -447,14 +455,6 @@ sna_output_backlight_get_max(xf86OutputPtr output) char path[1024], 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, sna_output->backlight_iface); - if (access(path, R_OK | W_OK)) - return -1; - sprintf(path, "%s/%s/max_brightness", BACKLIGHT_CLASS, sna_output->backlight_iface); fd = open(path, O_RDONLY); @@ -2539,6 +2539,7 @@ sna_output_destroy(xf86OutputPtr output) free(sna_output->prop_ids); free(sna_output->prop_values); + backlight_helper_cleanup(&sna_output->backlight_helper); free(sna_output->backlight_iface); free(sna_output); diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c index e1b2920..41b391e 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" @@ -123,6 +124,7 @@ struct intel_output { const char *backlight_iface; int backlight_active_level; int backlight_max; + struct backlight_helper backlight_helper; xf86OutputPtr output; struct list link; }; @@ -245,6 +247,12 @@ intel_output_backlight_set(xf86OutputPtr output, int level) BACKLIGHT_CLASS, intel_output->backlight_iface); fd = open(path, O_RDWR); if (fd == -1) { + /* If this is an access error, invoke the helper instead */ + if (errno == EACCES) { + backlight_helper_set(&intel_output->backlight_helper, + output, intel_output->backlight_iface, level); + return; + } xf86DrvMsg(output->scrn->scrnIndex, X_ERROR, "failed to open %s for backlight " "control: %s\n", path, strerror(errno)); return; @@ -298,14 +306,6 @@ intel_output_backlight_get_max(xf86OutputPtr output) 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); @@ -1083,6 +1083,7 @@ intel_output_destroy(xf86OutputPtr output) intel_output->mode_output = NULL; list_del(&intel_output->link); + backlight_helper_cleanup(&intel_output->backlight_helper); free(intel_output); output->driver_private = NULL;