| Message ID | 1381214401-24672-4-git-send-email-aaron.lu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@intel.com> > Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com> > Tested-by: Yves-Alexis Perez <corsac@debian.org> > Tested-by: Mika Westerberg <mika.westerberg@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. > + return acpi_video_backlight_support(); > +} > +EXPORT_SYMBOL(acpi_video_verify_backlight_support); > + > /* > * Use acpi_backlight=vendor/video to force that backlight switching > * is processed by vendor specific acpi drivers or video.ko driver. Thanks!
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@intel.com> >> Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com> >> Tested-by: Yves-Alexis Perez <corsac@debian.org> >> Tested-by: Mika Westerberg <mika.westerberg@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... Thanks, Aaron >> + return acpi_video_backlight_support(); >> +} >> +EXPORT_SYMBOL(acpi_video_verify_backlight_support); >> + >> /* >> * Use acpi_backlight=vendor/video to force that backlight switching >> * is processed by vendor specific acpi drivers or video.ko driver. > > Thanks! >
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@intel.com> > >> Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com> > >> Tested-by: Yves-Alexis Perez <corsac@debian.org> > >> Tested-by: Mika Westerberg <mika.westerberg@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
On 10/10/2013 08:59 PM, Rafael J. Wysocki wrote: > 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@intel.com> >>>> Tested-by: Igor Gnatenko <i.gnatenko.brain@gmail.com> >>>> Tested-by: Yves-Alexis Perez <corsac@debian.org> >>>> Tested-by: Mika Westerberg <mika.westerberg@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. Sounds good, I'll update in next revision, thanks for the suggestion! -Aaron
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; + return acpi_video_backlight_support(); +} +EXPORT_SYMBOL(acpi_video_verify_backlight_support); + /* * Use acpi_backlight=vendor/video to force that backlight switching * is processed by vendor specific acpi drivers or video.ko driver.