Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8

Daniel Vetter
2013-06-15T18:29:42Z

Hi all,

So to me it looks like the discussion is going in circles a bit, hence let
me drop my maintainer-opinion here:

1. Matthew's patch series here looks reasonable, and if it fixes a bunch
of systems (which it seems to) it has my Ack and imo should go in. If acpi
maintainers can smash their Ack onto the acpi parts I'd very much like to
merge this into drm-intel-next, that should give us the most coverage for
intel systems.

Len, Rafael, are you ok with the acpi part of this and merging it through
drm-intel-next?

2. Imo the current amount of quirking we expose to users (we have kernel
options to disable the acpi interface, blacklist platform modules, all
backlights can be tested through sysfs and on top of that xf86-video-intel
has an xorg.conf to select the backlight used by the driver). I haven't
spotted a compelling reason in this thread to add another one, what we
have seems to be good enough to debug backligh issues.

3. Also, adding yet another backlight quirk mechanism isn't gonna
magically fix broken systems.

We _really_ should strive to make this just work and not offer the angry
user another roll of duct-tape for free.

4. The currently established priority selection for backlights of platform
> firmware > raw seems to be good enough. Note that the explicit list in
xf86-vidoe-intel is only used as a fallback for really old kernels which
do not expose this information. We could probably rip it out.

5. We've recently looked at opregion again and couldn't spot a hint.
Unfortnately it looks like both noodling better information out of Intel
and trying to publish an updated opregion spec seem to be losing games :(
We'll keep on trying though.

Aside at the end: If the gnome tool indeed has its own backlight code and
doesn't just use that as a fallback if the xrandr backligh property isn't
available, then that's just a serious bug in gnome and should be fixed
asap. But imo that's not something we should try to (nor do I see any way
how to) work around in the kernel.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch