Hi,
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.
In case you're wondering what pkexec is, it stands for policy-kit-exec, it
is a little helper which will ask policykit if it is ok to run argv[1] as
root (for which there needs to be policy-file saying so), and if it is ok,
it will execute argv[1 .. x] as root (in a sanitized environment).
This will more or less give us the "display controller permission" you
were suggesting before without trying to get udev / logind to manage acls
on sysfs files.
To be specific we would need a policy file with aprox. this in there:
<policyconfig>
<action id="org.x.xf86-video-intel.backlight-helper>
<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
</action>
</policyconfig>
Regards,
Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
