[Intel-gfx] [PATCH v4 3/4] ACPI / video: Do not register backlight if win8 and native interface exists
Rafael J. Wysocki
rjw at sisk.pl
Thu Oct 10 14:59:02 CEST 2013
On Thursday, October 10, 2013 09:02:55 AM Aaron Lu wrote:
> On 10/10/2013 08:29 AM, Rafael J. Wysocki wrote:
> > On Tuesday, October 08, 2013 02:40:00 PM Aaron Lu wrote:
> >> According to Matthew Garrett, "Windows 8 leaves backlight control up
> >> to individual graphics drivers rather than making ACPI calls itself.
> >> There's plenty of evidence to suggest that the Intel driver for
> >> Windows [8] doesn't use the ACPI interface, including the fact that
> >> it's broken on a bunch of machines when the OS claims to support
> >> Windows 8. The simplest thing to do appears to be to disable the
> >> ACPI backlight interface on these systems".
> >>
> >> So for Win8 systems, if there is native backlight control interface
> >> registered by GPU driver, ACPI video will not register its own. For
> >> users who prefer to keep ACPI video's backlight interface, the existing
> >> kernel cmdline option acpi_backlight=video can be used.
> >>
> >> Signed-off-by: Aaron Lu <aaron.lu at intel.com>
> >> Tested-by: Igor Gnatenko <i.gnatenko.brain at gmail.com>
> >> Tested-by: Yves-Alexis Perez <corsac at debian.org>
> >> Tested-by: Mika Westerberg <mika.westerberg at linux.intel.com>
> >> ---
> >> drivers/acpi/internal.h | 5 ++---
> >> drivers/acpi/video.c | 10 +++++-----
> >> drivers/acpi/video_detect.c | 14 ++++++++++++--
> >> 3 files changed, 19 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> >> index 20f4233..453ae8d 100644
> >> --- a/drivers/acpi/internal.h
> >> +++ b/drivers/acpi/internal.h
> >> @@ -169,9 +169,8 @@ int acpi_create_platform_device(struct acpi_device *adev,
> >> Video
> >> -------------------------------------------------------------------------- */
> >> #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
> >> -bool acpi_video_backlight_quirks(void);
> >> -#else
> >> -static inline bool acpi_video_backlight_quirks(void) { return false; }
> >> +bool acpi_osi_is_win8(void);
> >> +bool acpi_video_verify_backlight_support(void);
> >> #endif
> >>
> >> #endif /* _ACPI_INTERNAL_H_ */
> >> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> >> index 3bd1eaa..343db59 100644
> >> --- a/drivers/acpi/video.c
> >> +++ b/drivers/acpi/video.c
> >> @@ -1256,8 +1256,8 @@ acpi_video_switch_brightness(struct acpi_video_device *device, int event)
> >> unsigned long long level_current, level_next;
> >> int result = -EINVAL;
> >>
> >> - /* no warning message if acpi_backlight=vendor is used */
> >> - if (!acpi_video_backlight_support())
> >> + /* no warning message if acpi_backlight=vendor or a quirk is used */
> >> + if (!acpi_video_verify_backlight_support())
> >> return 0;
> >>
> >> if (!device->brightness)
> >> @@ -1386,13 +1386,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video,
> >> static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
> >> {
> >> return acpi_video_bus_DOS(video, 0,
> >> - acpi_video_backlight_quirks() ? 1 : 0);
> >> + acpi_osi_is_win8() ? 1 : 0);
> >> }
> >>
> >> static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
> >> {
> >> return acpi_video_bus_DOS(video, 0,
> >> - acpi_video_backlight_quirks() ? 0 : 1);
> >> + acpi_osi_is_win8() ? 0 : 1);
> >> }
> >>
> >> static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
> >> @@ -1558,7 +1558,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void *context,
> >>
> >> static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
> >> {
> >> - if (acpi_video_backlight_support()) {
> >> + if (acpi_video_verify_backlight_support()) {
> >> struct backlight_properties props;
> >> struct pci_dev *pdev;
> >> acpi_handle acpi_parent;
> >> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> >> index 940edbf..23d7d26 100644
> >> --- a/drivers/acpi/video_detect.c
> >> +++ b/drivers/acpi/video_detect.c
> >> @@ -37,6 +37,7 @@
> >> #include <linux/acpi.h>
> >> #include <linux/dmi.h>
> >> #include <linux/pci.h>
> >> +#include <linux/backlight.h>
> >>
> >> #include "internal.h"
> >>
> >> @@ -233,11 +234,11 @@ static void acpi_video_caps_check(void)
> >> acpi_video_get_capabilities(NULL);
> >> }
> >>
> >> -bool acpi_video_backlight_quirks(void)
> >> +bool acpi_osi_is_win8(void)
> >> {
> >> return acpi_gbl_osi_data >= ACPI_OSI_WIN_8;
> >> }
> >> -EXPORT_SYMBOL(acpi_video_backlight_quirks);
> >> +EXPORT_SYMBOL(acpi_osi_is_win8);
> >>
> >> /* Promote the vendor interface instead of the generic video module.
> >> * This function allow DMI blacklists to be implemented by externals
> >> @@ -283,6 +284,15 @@ int acpi_video_backlight_support(void)
> >> }
> >> EXPORT_SYMBOL(acpi_video_backlight_support);
> >>
> >> +bool acpi_video_verify_backlight_support(void)
> >> +{
> >> + if (!(acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO) &&
> >> + acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
> >> + return false;
> >
> > If I'm not mistaken, this will introduce a regression for the people who have
> > problems with the native i915 backlight on Win8-compatible systems. I'd prefer
> > to avoid that at this point.
> >
>
> OK, I see.
>
> Then I'm afraid a new kernel command line option is needed, something
> like video.use_native_backlight and set it to false by default, then
> for people who need to avoid the ACPI video backlight interface, they
> can add video.use_native_backlight=true to kernel cmdline.
>
> One thing I need to mention is, with the new cmdline option, users will
> need to manually add a kernel cmdline option to make backlight work on
> their systems, while they can already make backlight work by modifying
> xorg.conf to specify using intel_backlight interface, so it doesn't seem
> this patchset will be very useful then...
Except if we add a (black)list of systems where that option will be 'true'
by default instead of the _OSI blacklist we have today.
Also we can switch the default during development cycles to get an idea
about how many systems are affected and maybe we can find a way to fix them,
in which case we can simply drop the option.
Thanks,
Rafael
More information about the Intel-gfx
mailing list