[Intel-gfx] What to do with xf86-video-intel backlight control when running Xorg as non root

[Mark Kettenis]

> Date: Thu, 13 Feb 2014 20:37:47 +0100
> From: Hans de Goede <hdegoede at redhat.com>
> 
> Hi,

Hi Hans,

Apologies in advance for jumping into the discussion at a somewhat
random point.

> On 02/13/2014 05:40 PM, Chris Wilson wrote: > On Thu, Feb 13, 2014
> at 04:52:59PM +0100, Hans de Goede wrote: >> Hi All,
> >>
> >> Currently xf86-video-intel is unique in that it is the only video driver
> >> which does backlight control inside the driver rather then letting
> >> something else (ie the desktop environment) deal with it.
> >>
> >> This is a problem when running the xserver without root rights because
> >> writing /sys/class/backlight/foo/brightness requires root rights.
> >>
> >> There are 2 possible (short term) solutions for this:
> >>
> >> 1) Detect that the xserver is not running as root, and don't register the
> >> backlight property on the connectors, let something else deal with it,
> >> as is done or other xf86-video-* drivers already.
> >>
> >> 2) Add a little xf86-video-intel-brightness helper on Linux which the driver
> >> execs (through pkexec) each time it needs to set the brightness.
> >>
> >>
> >> 1) of course is very KISS, so I like. 2) is not that hard either, and
> >> 1) might cause some regressions in cases where ie gsd-brightness-helper
> >> does not do the right thing, where as the intel driver does. OTOH it seems
> >> that the intel video code is mostly there to deal with older kernels, and
> >> rootless xorg will be used with newer kernels only anyways.
> > 
> > Not registering a property that is broken seems like the fundamental first
> > step. Would it be possible to use udev to set the access mode on the
> > backlight properties such that the display controller does have
> > permission to write to those files?
> 
> When we were discussing this at Fosdem Kay Sievers was in the room, and I
> can summarize his response to that suggestion as: *NO*.
> 
> > Otherwise, it seems like we need the
> > proxy in order to keep the xrandr property available to users and
> > prevent those who rely upon it in scripts from seeing regressions.
> 
> Right, that is what I was thinking too, so the question then becomes how
> hard you will scream at me if I add something like this to xf86-video-intel
> linux specific backlight code:
> 
>     if (geteuid() == 0) {
>         /* Old write directly to /sys/class/backlight/... code */
>     {
>     else {
>         /* The & is to avoid the xserver blocking */
>         snprintf(command, sizeof(command), "pkexec %s/libexec/xf86-video-intel-backlight-helper %s %d&",
>                  PREFIX, sna_output->backlight_iface, level);
>         r = system(command);
>         if (r) {
>             /* complain */
>         }
>     }
> 
> If you won't scream too much, and more importantly, if you will accept such
> a patch (including code for the helper), then I'll try to cook up something
> like this tomorrow.

I don't really care as long as this is #ifdef __linux__, but yeah,
that's fairly gross.  Does the Linux community really think it is a
good idea that the xserver starts to depend on policykit.  What will
happen when the next framework for doing things with elevated
priviliges comes along?  And do you realize that you'll end up adding
similar code to many other drivers as well?

You've probably scrolled past the #ifdef __OpenBSD__ code in the
xf86-video-intel driver, and perhaps noticed it is quite a bit
simpler.  On OpenBSD we've had an ioctl-based backlight control
interface for ages.  It works without root priviliges because the
appropriate file descriptor is already open.  (Our xserver is still
setuid root, but it is privilige seperated, and virtually all code
runs in the part that has dropped root priviliges).  It has the major
benefit that we can have an OS-provided tool to control the backlight
that works even when X isn't running.  The backlight policy is
determined by the kernel, and currently rather basic.  For x86 it
decides between some vendor-defined mechanisms, acpi and what the KMS
drivers provide.  And a simple policy seems to cover most of the use
cases.

Conceptually our approach is very similar to having drm provide an
interface for this.  And it would be fairly trivial for us to adapt
such an approach.  But the pkexec or backlightd approach really isn't
compatible with our goals of keeping things simple and providing a
basic X11 environment that is well-integrated into our OS and works
out-of-the-box for most (if not all) people.