Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-07-17 at 13:38 +0200, Rafael J. Wysocki wrote:
> On Wednesday, July 17, 2013 09:16:38 AM Igor Gnatenko wrote:
> > On Wed, 2013-07-17 at 00:01 +0200, Rafael J. Wysocki wrote:
> > > On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote:
> > > > On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote:
> > > > > Hmm. I found regression in user-space. In GNOME (maybe and other DEs)
> > > > > we no longer see switch status of backlight.
> > > >
> > > > Yeah, I can duplicate that. Rafael, we have to call
> > > > acpi_video_init_brightness() even if we're not going to initialise the
> > > > backlight - Thinkpads seem to use this as the trigger for enabling ACPI
> > > > notifications rather than handling it in firmware. This seems to do the
> > > > job:
> > >
> > > Igor, does this additional patch from Matthew help?
> > Yes. With this patch I have backlight switch indicator on my ThinkPad X230.
>
> OK, thanks for the confirmation.
>
> Can you please also check if applying the appended patch on top of the
> Matthew's
> one changes anything (ie. things still work)?
Yes. I've tested and not found regressions in indicator or in switcher. Good
work.
>
> Rafael
>
>
> ---
Tested-by: Igor Gnatenko
> drivers/acpi/video.c |5 +
> 1 file changed, 5 insertions(+)
>
> Index: linux-pm/drivers/acpi/video.c
> ===
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -957,6 +957,11 @@ static void acpi_video_device_find_cap(s
> if (result)
> printk(KERN_ERR PREFIX "Create sysfs link\n");
>
> + } else {
> + /* Remove the brightness object. */
> + kfree(device->brightness->levels);
> + kfree(device->brightness);
> + device->brightness = NULL;
> }
> }
>
>
>
--
Igor Gnatenko
Fedora release 19 (Schrödinger’s Cat)
Linux 3.11.0-0.rc0.git7.1.fc20.x86_64
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wednesday, July 17, 2013 09:16:38 AM Igor Gnatenko wrote:
> On Wed, 2013-07-17 at 00:01 +0200, Rafael J. Wysocki wrote:
> > On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote:
> > > On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote:
> > > > Hmm. I found regression in user-space. In GNOME (maybe and other DEs)
> > > > we no longer see switch status of backlight.
> > >
> > > Yeah, I can duplicate that. Rafael, we have to call
> > > acpi_video_init_brightness() even if we're not going to initialise the
> > > backlight - Thinkpads seem to use this as the trigger for enabling ACPI
> > > notifications rather than handling it in firmware. This seems to do the
> > > job:
> >
> > Igor, does this additional patch from Matthew help?
> Yes. With this patch I have backlight switch indicator on my ThinkPad X230.
OK, thanks for the confirmation.
Can you please also check if applying the appended patch on top of the Matthew's
one changes anything (ie. things still work)?
Rafael
---
drivers/acpi/video.c |5 +
1 file changed, 5 insertions(+)
Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -957,6 +957,11 @@ static void acpi_video_device_find_cap(s
if (result)
printk(KERN_ERR PREFIX "Create sysfs link\n");
+ } else {
+ /* Remove the brightness object. */
+ kfree(device->brightness->levels);
+ kfree(device->brightness);
+ device->brightness = NULL;
}
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Wed, 2013-07-17 at 00:01 +0200, Rafael J. Wysocki wrote:
> On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote:
> > On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote:
> > > Hmm. I found regression in user-space. In GNOME (maybe and other DEs) we
> > > no longer see switch status of backlight.
> >
> > Yeah, I can duplicate that. Rafael, we have to call
> > acpi_video_init_brightness() even if we're not going to initialise the
> > backlight - Thinkpads seem to use this as the trigger for enabling ACPI
> > notifications rather than handling it in firmware. This seems to do the
> > job:
>
> Igor, does this additional patch from Matthew help?
Yes. With this patch I have backlight switch indicator on my ThinkPad X230.
>
> Rafael
>
>
Tested-by: Igor Gnatenko
> > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> > index 01b1a25..71865f7 100644
> > --- a/drivers/acpi/video.c
> > +++ b/drivers/acpi/video.c
> > @@ -900,6 +900,9 @@ static void acpi_video_device_find_cap(struct
> > acpi_video_device *device)
> > device->cap._DDC = 1;
> > }
> >
> > + if (acpi_video_init_brightness(device))
> > + return;
> > +
> > if (acpi_video_verify_backlight_support()) {
> > struct backlight_properties props;
> > struct pci_dev *pdev;
> > @@ -909,9 +912,6 @@ static void acpi_video_device_find_cap(struct
> > acpi_video_device *device)
> > static int count = 0;
> > char *name;
> >
> > - result = acpi_video_init_brightness(device);
> > - if (result)
> > - return;
> > name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
> > if (!name)
> > return;
> >
> >
> > --
> > Matthew Garrett | mj...@srcf.ucam.org
> >
--
Igor Gnatenko
Fedora release 19 (Schrödinger’s Cat)
Linux 3.11.0-0.rc0.git7.1.fc20.x86_64
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tuesday, July 16, 2013 05:08:16 PM Matthew Garrett wrote:
> On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote:
> > Hmm. I found regression in user-space. In GNOME (maybe and other DEs) we no
> > longer see switch status of backlight.
>
> Yeah, I can duplicate that. Rafael, we have to call
> acpi_video_init_brightness() even if we're not going to initialise the
> backlight - Thinkpads seem to use this as the trigger for enabling ACPI
> notifications rather than handling it in firmware. This seems to do the
> job:
Igor, does this additional patch from Matthew help?
Rafael
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 01b1a25..71865f7 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -900,6 +900,9 @@ static void acpi_video_device_find_cap(struct
> acpi_video_device *device)
> device->cap._DDC = 1;
> }
>
> + if (acpi_video_init_brightness(device))
> + return;
> +
> if (acpi_video_verify_backlight_support()) {
> struct backlight_properties props;
> struct pci_dev *pdev;
> @@ -909,9 +912,6 @@ static void acpi_video_device_find_cap(struct
> acpi_video_device *device)
> static int count = 0;
> char *name;
>
> - result = acpi_video_init_brightness(device);
> - if (result)
> - return;
> name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
> if (!name)
> return;
>
>
> --
> Matthew Garrett | mj...@srcf.ucam.org
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-07-16 at 17:32 +0400, Igor Gnatenko wrote:
> Hmm. I found regression in user-space. In GNOME (maybe and other DEs) we no
> longer see switch status of backlight.
Yeah, I can duplicate that. Rafael, we have to call
acpi_video_init_brightness() even if we're not going to initialise the
backlight - Thinkpads seem to use this as the trigger for enabling ACPI
notifications rather than handling it in firmware. This seems to do the
job:
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 01b1a25..71865f7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -900,6 +900,9 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
device->cap._DDC = 1;
}
+ if (acpi_video_init_brightness(device))
+ return;
+
if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
@@ -909,9 +912,6 @@ static void acpi_video_device_find_cap(struct
acpi_video_device *device)
static int count = 0;
char *name;
- result = acpi_video_init_brightness(device);
- if (result)
- return;
name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
if (!name)
return;
--
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
Hmm. I found regression in user-space. In GNOME (maybe and other DEs) we no longer see switch status of backlight. -- Igor Gnatenko Fedora release 19 (Schrödinger’s Cat) Linux 3.11.0-0.rc0.git7.1.fc20.x86_64 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tuesday, July 16, 2013 11:24:05 AM Aaron Lu wrote: > On 07/15/2013 07:42 PM, Rafael J. Wysocki wrote: > > On Monday, July 15, 2013 10:36:15 AM Aaron Lu wrote: > >> On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote: > >>> From: Rafael J. Wysocki > >>> > >>> 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". > >>> > >>> There's a problem with that approach, however, because simply > >>> avoiding to register the ACPI backlight interface if the firmware > >>> calls _OSI for Windows 8 may not work in the following situations: > >>> (1) The ACPI backlight interface actually works on the given system > >>> and the i915 driver is not loaded (e.g. another graphics driver > >>> is used). > >>> (2) The ACPI backlight interface doesn't work on the given system, > >>> but there is a vendor platform driver that will register its > >>> own, equally broken, backlight interface if not prevented from > >>> doing so by the ACPI subsystem. > >>> Therefore we need to allow the ACPI backlight interface to be > >>> registered until the i915 driver is loaded which then will unregister > >>> it if the firmware has called _OSI for Windows 8 (or will register > >>> the ACPI video driver without backlight support if not already > >>> present). > >>> > >>> For this reason, introduce an alternative function for registering > >>> ACPI video, acpi_video_register_with_quirks(), that will check > >>> whether or not the ACPI video driver has already been registered > >>> and whether or not the backlight Windows 8 quirk has to be applied. > >>> If the quirk has to be applied, it will block the ACPI backlight > >>> support and either unregister the backlight interface if the ACPI > >>> video driver has already been registered, or register the ACPI > >>> video driver without the backlight interface otherwise. Make > >>> the i915 driver use acpi_video_register_with_quirks() instead of > >>> acpi_video_register() in i915_driver_load(). > >>> > >>> This change is based on earlier patches from Matthew Garrett, > >>> Chun-Yi Lee and Seth Forshee and Aaron Lu's comments. > >>> > >>> Signed-off-by: Rafael J. Wysocki > >> > >> Reviewed-by: Aaron Lu > >> > >> BTW, I also tested on a Toshiba laptop Z830 where its AML code > >> claims support of win8, the result is as expected: ACPI video > >> interface is removed, i915 Xorg driver picks intel_backlight. > >> > >> Thanks for the fix. > > > > Cool, thanks for testing this! > > > > Can you please also ask bug reporters in the BZ entires related to this to > > test > > it too? > > Sure. > > To be clear, I actually tested the patch in your linux-next branch, > which turned out to be a little different in that you have fixed the > problem Igor raised here. > > I'll ask reporters to test on a stable tree with the following two > patches on top: > https://patchwork.kernel.org/patch/2812951/ (expose OSI version) > https://patchwork.kernel.org/patch/2827793/ (your updated patch) > or your linux-next branch, whichever they like. OK Thanks, Rafael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Tue, 2013-07-16 at 01:53 +0200, Rafael J. Wysocki wrote:
> On Monday, July 15, 2013 05:06:09 PM Igor Gnatenko wrote:
> > On Sat, 2013-07-13 at 02:46 +0200, Rafael J. Wysocki wrote:
>
> [...]
>
> >
> > I can't build it. Where did I go wrong?
>
> Probably nowhere, you tried to build the ACPI video driver as a module, that's
> all. And you need to apply https://patchwork.kernel.org/patch/2812951/
> before.
>
> Please try the appended version (on top of
> https://patchwork.kernel.org/patch/2812951/).
>
> Thanks,
> Rafael
>
Tested-by: Igor Gnatenko
>
> ---
> From: Rafael J. Wysocki
> Subject: ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
>
> 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".
>
> There's a problem with that approach, however, because simply
> avoiding to register the ACPI backlight interface if the firmware
> calls _OSI for Windows 8 may not work in the following situations:
> (1) The ACPI backlight interface actually works on the given system
> and the i915 driver is not loaded (e.g. another graphics driver
> is used).
> (2) The ACPI backlight interface doesn't work on the given system,
> but there is a vendor platform driver that will register its
> own, equally broken, backlight interface if not prevented from
> doing so by the ACPI subsystem.
> Therefore we need to allow the ACPI backlight interface to be
> registered until the i915 driver is loaded which then will unregister
> it if the firmware has called _OSI for Windows 8 (or will register
> the ACPI video driver without backlight support if not already
> present).
>
> For this reason, introduce an alternative function for registering
> ACPI video, acpi_video_register_with_quirks(), that will check
> whether or not the ACPI video driver has already been registered
> and whether or not the backlight Windows 8 quirk has to be applied.
> If the quirk has to be applied, it will block the ACPI backlight
> support and either unregister the backlight interface if the ACPI
> video driver has already been registered, or register the ACPI
> video driver without the backlight interface otherwise. Make
> the i915 driver use acpi_video_register_with_quirks() instead of
> acpi_video_register() in i915_driver_load().
>
> This change is based on earlier patches from Matthew Garrett,
> Chun-Yi Lee and Seth Forshee and Aaron Lu's comments.
>
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/acpi/internal.h | 11 ++
> drivers/acpi/video.c| 65
>
> drivers/acpi/video_detect.c | 21
> drivers/gpu/drm/i915/i915_dma.c |2 -
> include/acpi/video.h| 11 ++
> include/linux/acpi.h|1
> 6 files changed, 103 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/acpi/video.c
> ===
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -44,6 +44,8 @@
> #include
> #include
>
> +#include "internal.h"
> +
> #define PREFIX "ACPI: "
>
> #define ACPI_VIDEO_BUS_NAME "Video Bus"
> @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s
> device->cap._DDC = 1;
> }
>
> - if (acpi_video_backlight_support()) {
> + if (acpi_video_verify_backlight_support()) {
> struct backlight_properties props;
> struct pci_dev *pdev;
> acpi_handle acpi_parent;
> @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct
> return 0;
> }
>
> +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
> + void *context, void **rv)
> +{
> + struct acpi_device *acpi_dev;
> + struct acpi_video_bus *video;
> + struct acpi_video_device *dev, *next;
> +
> + if (acpi_bus_get_device(handle, &acpi_dev))
> + return AE_OK;
> +
> + if (acpi_match_device_ids(acpi_dev, video_device_ids))
> + return AE_OK;
> +
> + video = acpi_driver_data(acpi_dev);
> + if (!video)
> + return AE_OK;
> +
> + acpi_video_bus_stop_devices(video);
> + mutex_lock(&video->device_list_lock);
> + list_for_each_entry_safe(dev, next, &video->video_device_list, entry) {
> + if (dev->backlight) {
> + backlight_device_unregister(dev->backlight);
> + dev->backlight = NULL;
> + kfree(dev->brightness->levels);
> + kfree(dev->br
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 07/15/2013 07:42 PM, Rafael J. Wysocki wrote:
> On Monday, July 15, 2013 10:36:15 AM Aaron Lu wrote:
>> On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki
>>>
>>> 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".
>>>
>>> There's a problem with that approach, however, because simply
>>> avoiding to register the ACPI backlight interface if the firmware
>>> calls _OSI for Windows 8 may not work in the following situations:
>>> (1) The ACPI backlight interface actually works on the given system
>>> and the i915 driver is not loaded (e.g. another graphics driver
>>> is used).
>>> (2) The ACPI backlight interface doesn't work on the given system,
>>> but there is a vendor platform driver that will register its
>>> own, equally broken, backlight interface if not prevented from
>>> doing so by the ACPI subsystem.
>>> Therefore we need to allow the ACPI backlight interface to be
>>> registered until the i915 driver is loaded which then will unregister
>>> it if the firmware has called _OSI for Windows 8 (or will register
>>> the ACPI video driver without backlight support if not already
>>> present).
>>>
>>> For this reason, introduce an alternative function for registering
>>> ACPI video, acpi_video_register_with_quirks(), that will check
>>> whether or not the ACPI video driver has already been registered
>>> and whether or not the backlight Windows 8 quirk has to be applied.
>>> If the quirk has to be applied, it will block the ACPI backlight
>>> support and either unregister the backlight interface if the ACPI
>>> video driver has already been registered, or register the ACPI
>>> video driver without the backlight interface otherwise. Make
>>> the i915 driver use acpi_video_register_with_quirks() instead of
>>> acpi_video_register() in i915_driver_load().
>>>
>>> This change is based on earlier patches from Matthew Garrett,
>>> Chun-Yi Lee and Seth Forshee and Aaron Lu's comments.
>>>
>>> Signed-off-by: Rafael J. Wysocki
>>
>> Reviewed-by: Aaron Lu
>>
>> BTW, I also tested on a Toshiba laptop Z830 where its AML code
>> claims support of win8, the result is as expected: ACPI video
>> interface is removed, i915 Xorg driver picks intel_backlight.
>>
>> Thanks for the fix.
>
> Cool, thanks for testing this!
>
> Can you please also ask bug reporters in the BZ entires related to this to
> test
> it too?
Sure.
To be clear, I actually tested the patch in your linux-next branch,
which turned out to be a little different in that you have fixed the
problem Igor raised here.
I'll ask reporters to test on a stable tree with the following two
patches on top:
https://patchwork.kernel.org/patch/2812951/ (expose OSI version)
https://patchwork.kernel.org/patch/2827793/ (your updated patch)
or your linux-next branch, whichever they like.
-Aaron
>
> Rafael
>
>
>>> ---
>>> drivers/acpi/internal.h | 11 ++
>>> drivers/acpi/video.c| 65
>>>
>>> drivers/acpi/video_detect.c | 21
>>> drivers/gpu/drm/i915/i915_dma.c |2 -
>>> include/acpi/video.h| 11 ++
>>> include/linux/acpi.h|1
>>> 6 files changed, 103 insertions(+), 8 deletions(-)
>>>
>>> Index: linux-pm/drivers/acpi/video.c
>>> ===
>>> --- linux-pm.orig/drivers/acpi/video.c
>>> +++ linux-pm/drivers/acpi/video.c
>>> @@ -44,6 +44,8 @@
>>> #include
>>> #include
>>>
>>> +#include "internal.h"
>>> +
>>> #define PREFIX "ACPI: "
>>>
>>> #define ACPI_VIDEO_BUS_NAME"Video Bus"
>>> @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s
>>> device->cap._DDC = 1;
>>> }
>>>
>>> - if (acpi_video_backlight_support()) {
>>> + if (acpi_video_verify_backlight_support()) {
>>> struct backlight_properties props;
>>> struct pci_dev *pdev;
>>> acpi_handle acpi_parent;
>>> @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct
>>> return 0;
>>> }
>>>
>>> +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
>>> + void *context, void **rv)
>>> +{
>>> + struct acpi_device *acpi_dev;
>>> + struct acpi_video_bus *video;
>>> + struct acpi_video_device *dev, *next;
>>> +
>>> + if (acpi_bus_get_device(handle, &acpi_dev))
>>> + return AE_OK;
>>> +
>>> + if (acpi_match_device_ids(acpi_dev, video_device_ids))
>>> + return AE_OK;
>>> +
>>> + video =
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Monday, July 15, 2013 05:06:09 PM Igor Gnatenko wrote:
> On Sat, 2013-07-13 at 02:46 +0200, Rafael J. Wysocki wrote:
[...]
>
> I can't build it. Where did I go wrong?
Probably nowhere, you tried to build the ACPI video driver as a module, that's
all. And you need to apply https://patchwork.kernel.org/patch/2812951/
before.
Please try the appended version (on top of
https://patchwork.kernel.org/patch/2812951/).
Thanks,
Rafael
---
From: Rafael J. Wysocki
Subject: ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
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".
There's a problem with that approach, however, because simply
avoiding to register the ACPI backlight interface if the firmware
calls _OSI for Windows 8 may not work in the following situations:
(1) The ACPI backlight interface actually works on the given system
and the i915 driver is not loaded (e.g. another graphics driver
is used).
(2) The ACPI backlight interface doesn't work on the given system,
but there is a vendor platform driver that will register its
own, equally broken, backlight interface if not prevented from
doing so by the ACPI subsystem.
Therefore we need to allow the ACPI backlight interface to be
registered until the i915 driver is loaded which then will unregister
it if the firmware has called _OSI for Windows 8 (or will register
the ACPI video driver without backlight support if not already
present).
For this reason, introduce an alternative function for registering
ACPI video, acpi_video_register_with_quirks(), that will check
whether or not the ACPI video driver has already been registered
and whether or not the backlight Windows 8 quirk has to be applied.
If the quirk has to be applied, it will block the ACPI backlight
support and either unregister the backlight interface if the ACPI
video driver has already been registered, or register the ACPI
video driver without the backlight interface otherwise. Make
the i915 driver use acpi_video_register_with_quirks() instead of
acpi_video_register() in i915_driver_load().
This change is based on earlier patches from Matthew Garrett,
Chun-Yi Lee and Seth Forshee and Aaron Lu's comments.
Signed-off-by: Rafael J. Wysocki
---
drivers/acpi/internal.h | 11 ++
drivers/acpi/video.c| 65
drivers/acpi/video_detect.c | 21
drivers/gpu/drm/i915/i915_dma.c |2 -
include/acpi/video.h| 11 ++
include/linux/acpi.h|1
6 files changed, 103 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -44,6 +44,8 @@
#include
#include
+#include "internal.h"
+
#define PREFIX "ACPI: "
#define ACPI_VIDEO_BUS_NAME"Video Bus"
@@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s
device->cap._DDC = 1;
}
- if (acpi_video_backlight_support()) {
+ if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
@@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct
return 0;
}
+static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
+ void *context, void **rv)
+{
+ struct acpi_device *acpi_dev;
+ struct acpi_video_bus *video;
+ struct acpi_video_device *dev, *next;
+
+ if (acpi_bus_get_device(handle, &acpi_dev))
+ return AE_OK;
+
+ if (acpi_match_device_ids(acpi_dev, video_device_ids))
+ return AE_OK;
+
+ video = acpi_driver_data(acpi_dev);
+ if (!video)
+ return AE_OK;
+
+ acpi_video_bus_stop_devices(video);
+ mutex_lock(&video->device_list_lock);
+ list_for_each_entry_safe(dev, next, &video->video_device_list, entry) {
+ if (dev->backlight) {
+ backlight_device_unregister(dev->backlight);
+ dev->backlight = NULL;
+ kfree(dev->brightness->levels);
+ kfree(dev->brightness);
+ }
+ if (dev->cooling_dev) {
+ sysfs_remove_link(&dev->dev->dev.kobj,
+ "thermal_cooling");
+ sysfs_remove_link(&dev->cooling_dev->device.kobj,
+
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Sat, 2013-07-13 at 02:46 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> 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".
>
> There's a problem with that approach, however, because simply
> avoiding to register the ACPI backlight interface if the firmware
> calls _OSI for Windows 8 may not work in the following situations:
> (1) The ACPI backlight interface actually works on the given system
> and the i915 driver is not loaded (e.g. another graphics driver
> is used).
> (2) The ACPI backlight interface doesn't work on the given system,
> but there is a vendor platform driver that will register its
> own, equally broken, backlight interface if not prevented from
> doing so by the ACPI subsystem.
> Therefore we need to allow the ACPI backlight interface to be
> registered until the i915 driver is loaded which then will unregister
> it if the firmware has called _OSI for Windows 8 (or will register
> the ACPI video driver without backlight support if not already
> present).
>
> For this reason, introduce an alternative function for registering
> ACPI video, acpi_video_register_with_quirks(), that will check
> whether or not the ACPI video driver has already been registered
> and whether or not the backlight Windows 8 quirk has to be applied.
> If the quirk has to be applied, it will block the ACPI backlight
> support and either unregister the backlight interface if the ACPI
> video driver has already been registered, or register the ACPI
> video driver without the backlight interface otherwise. Make
> the i915 driver use acpi_video_register_with_quirks() instead of
> acpi_video_register() in i915_driver_load().
>
> This change is based on earlier patches from Matthew Garrett,
> Chun-Yi Lee and Seth Forshee and Aaron Lu's comments.
>
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/acpi/internal.h | 11 ++
> drivers/acpi/video.c| 65
>
> drivers/acpi/video_detect.c | 21
> drivers/gpu/drm/i915/i915_dma.c |2 -
> include/acpi/video.h| 11 ++
> include/linux/acpi.h|1
> 6 files changed, 103 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/acpi/video.c
> ===
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -44,6 +44,8 @@
> #include
> #include
>
> +#include "internal.h"
> +
> #define PREFIX "ACPI: "
>
> #define ACPI_VIDEO_BUS_NAME "Video Bus"
> @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s
> device->cap._DDC = 1;
> }
>
> - if (acpi_video_backlight_support()) {
> + if (acpi_video_verify_backlight_support()) {
> struct backlight_properties props;
> struct pci_dev *pdev;
> acpi_handle acpi_parent;
> @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct
> return 0;
> }
>
> +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
> + void *context, void **rv)
> +{
> + struct acpi_device *acpi_dev;
> + struct acpi_video_bus *video;
> + struct acpi_video_device *dev, *next;
> +
> + if (acpi_bus_get_device(handle, &acpi_dev))
> + return AE_OK;
> +
> + if (acpi_match_device_ids(acpi_dev, video_device_ids))
> + return AE_OK;
> +
> + video = acpi_driver_data(acpi_dev);
> + if (!video)
> + return AE_OK;
> +
> + acpi_video_bus_stop_devices(video);
> + mutex_lock(&video->device_list_lock);
> + list_for_each_entry_safe(dev, next, &video->video_device_list, entry) {
> + if (dev->backlight) {
> + backlight_device_unregister(dev->backlight);
> + dev->backlight = NULL;
> + kfree(dev->brightness->levels);
> + kfree(dev->brightness);
> + }
> + if (dev->cooling_dev) {
> + sysfs_remove_link(&dev->dev->dev.kobj,
> + "thermal_cooling");
> + sysfs_remove_link(&dev->cooling_dev->device.kobj,
> + "device");
> + thermal_cooling_device_unregister(dev->cooling_dev);
> + dev->cooling_dev = NULL;
> + }
> + }
> + mutex_unlock(&video->device_list_lock);
> + acpi_video_bus_start_devices(video);
> + return AE_OK;
>
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On Monday, July 15, 2013 10:36:15 AM Aaron Lu wrote:
> On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki
> >
> > 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".
> >
> > There's a problem with that approach, however, because simply
> > avoiding to register the ACPI backlight interface if the firmware
> > calls _OSI for Windows 8 may not work in the following situations:
> > (1) The ACPI backlight interface actually works on the given system
> > and the i915 driver is not loaded (e.g. another graphics driver
> > is used).
> > (2) The ACPI backlight interface doesn't work on the given system,
> > but there is a vendor platform driver that will register its
> > own, equally broken, backlight interface if not prevented from
> > doing so by the ACPI subsystem.
> > Therefore we need to allow the ACPI backlight interface to be
> > registered until the i915 driver is loaded which then will unregister
> > it if the firmware has called _OSI for Windows 8 (or will register
> > the ACPI video driver without backlight support if not already
> > present).
> >
> > For this reason, introduce an alternative function for registering
> > ACPI video, acpi_video_register_with_quirks(), that will check
> > whether or not the ACPI video driver has already been registered
> > and whether or not the backlight Windows 8 quirk has to be applied.
> > If the quirk has to be applied, it will block the ACPI backlight
> > support and either unregister the backlight interface if the ACPI
> > video driver has already been registered, or register the ACPI
> > video driver without the backlight interface otherwise. Make
> > the i915 driver use acpi_video_register_with_quirks() instead of
> > acpi_video_register() in i915_driver_load().
> >
> > This change is based on earlier patches from Matthew Garrett,
> > Chun-Yi Lee and Seth Forshee and Aaron Lu's comments.
> >
> > Signed-off-by: Rafael J. Wysocki
>
> Reviewed-by: Aaron Lu
>
> BTW, I also tested on a Toshiba laptop Z830 where its AML code
> claims support of win8, the result is as expected: ACPI video
> interface is removed, i915 Xorg driver picks intel_backlight.
>
> Thanks for the fix.
Cool, thanks for testing this!
Can you please also ask bug reporters in the BZ entires related to this to test
it too?
Rafael
> > ---
> > drivers/acpi/internal.h | 11 ++
> > drivers/acpi/video.c| 65
> >
> > drivers/acpi/video_detect.c | 21
> > drivers/gpu/drm/i915/i915_dma.c |2 -
> > include/acpi/video.h| 11 ++
> > include/linux/acpi.h|1
> > 6 files changed, 103 insertions(+), 8 deletions(-)
> >
> > Index: linux-pm/drivers/acpi/video.c
> > ===
> > --- linux-pm.orig/drivers/acpi/video.c
> > +++ linux-pm/drivers/acpi/video.c
> > @@ -44,6 +44,8 @@
> > #include
> > #include
> >
> > +#include "internal.h"
> > +
> > #define PREFIX "ACPI: "
> >
> > #define ACPI_VIDEO_BUS_NAME"Video Bus"
> > @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s
> > device->cap._DDC = 1;
> > }
> >
> > - if (acpi_video_backlight_support()) {
> > + if (acpi_video_verify_backlight_support()) {
> > struct backlight_properties props;
> > struct pci_dev *pdev;
> > acpi_handle acpi_parent;
> > @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct
> > return 0;
> > }
> >
> > +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
> > + void *context, void **rv)
> > +{
> > + struct acpi_device *acpi_dev;
> > + struct acpi_video_bus *video;
> > + struct acpi_video_device *dev, *next;
> > +
> > + if (acpi_bus_get_device(handle, &acpi_dev))
> > + return AE_OK;
> > +
> > + if (acpi_match_device_ids(acpi_dev, video_device_ids))
> > + return AE_OK;
> > +
> > + video = acpi_driver_data(acpi_dev);
> > + if (!video)
> > + return AE_OK;
> > +
> > + acpi_video_bus_stop_devices(video);
> > + mutex_lock(&video->device_list_lock);
> > + list_for_each_entry_safe(dev, next, &video->video_device_list, entry) {
> > + if (dev->backlight) {
> > + backlight_device_unregister(dev->backlight);
> > + dev->backlight = NULL;
> > + kfree(dev->brightness->levels);
> > + kfree(dev->brightness);
>
Re: [Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> 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".
>
> There's a problem with that approach, however, because simply
> avoiding to register the ACPI backlight interface if the firmware
> calls _OSI for Windows 8 may not work in the following situations:
> (1) The ACPI backlight interface actually works on the given system
> and the i915 driver is not loaded (e.g. another graphics driver
> is used).
> (2) The ACPI backlight interface doesn't work on the given system,
> but there is a vendor platform driver that will register its
> own, equally broken, backlight interface if not prevented from
> doing so by the ACPI subsystem.
> Therefore we need to allow the ACPI backlight interface to be
> registered until the i915 driver is loaded which then will unregister
> it if the firmware has called _OSI for Windows 8 (or will register
> the ACPI video driver without backlight support if not already
> present).
>
> For this reason, introduce an alternative function for registering
> ACPI video, acpi_video_register_with_quirks(), that will check
> whether or not the ACPI video driver has already been registered
> and whether or not the backlight Windows 8 quirk has to be applied.
> If the quirk has to be applied, it will block the ACPI backlight
> support and either unregister the backlight interface if the ACPI
> video driver has already been registered, or register the ACPI
> video driver without the backlight interface otherwise. Make
> the i915 driver use acpi_video_register_with_quirks() instead of
> acpi_video_register() in i915_driver_load().
>
> This change is based on earlier patches from Matthew Garrett,
> Chun-Yi Lee and Seth Forshee and Aaron Lu's comments.
>
> Signed-off-by: Rafael J. Wysocki
Reviewed-by: Aaron Lu
BTW, I also tested on a Toshiba laptop Z830 where its AML code
claims support of win8, the result is as expected: ACPI video
interface is removed, i915 Xorg driver picks intel_backlight.
Thanks for the fix.
-Aaron
> ---
> drivers/acpi/internal.h | 11 ++
> drivers/acpi/video.c| 65
>
> drivers/acpi/video_detect.c | 21
> drivers/gpu/drm/i915/i915_dma.c |2 -
> include/acpi/video.h| 11 ++
> include/linux/acpi.h|1
> 6 files changed, 103 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/acpi/video.c
> ===
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -44,6 +44,8 @@
> #include
> #include
>
> +#include "internal.h"
> +
> #define PREFIX "ACPI: "
>
> #define ACPI_VIDEO_BUS_NAME "Video Bus"
> @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s
> device->cap._DDC = 1;
> }
>
> - if (acpi_video_backlight_support()) {
> + if (acpi_video_verify_backlight_support()) {
> struct backlight_properties props;
> struct pci_dev *pdev;
> acpi_handle acpi_parent;
> @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct
> return 0;
> }
>
> +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
> + void *context, void **rv)
> +{
> + struct acpi_device *acpi_dev;
> + struct acpi_video_bus *video;
> + struct acpi_video_device *dev, *next;
> +
> + if (acpi_bus_get_device(handle, &acpi_dev))
> + return AE_OK;
> +
> + if (acpi_match_device_ids(acpi_dev, video_device_ids))
> + return AE_OK;
> +
> + video = acpi_driver_data(acpi_dev);
> + if (!video)
> + return AE_OK;
> +
> + acpi_video_bus_stop_devices(video);
> + mutex_lock(&video->device_list_lock);
> + list_for_each_entry_safe(dev, next, &video->video_device_list, entry) {
> + if (dev->backlight) {
> + backlight_device_unregister(dev->backlight);
> + dev->backlight = NULL;
> + kfree(dev->brightness->levels);
> + kfree(dev->brightness);
> + }
> + if (dev->cooling_dev) {
> + sysfs_remove_link(&dev->dev->dev.kobj,
> + "thermal_cooling");
> + sysfs_remove_link(&dev->cooling_dev->device.kobj,
> + "device");
> + thermal_cooling_
[Intel-gfx] [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
From: Rafael J. Wysocki
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".
There's a problem with that approach, however, because simply
avoiding to register the ACPI backlight interface if the firmware
calls _OSI for Windows 8 may not work in the following situations:
(1) The ACPI backlight interface actually works on the given system
and the i915 driver is not loaded (e.g. another graphics driver
is used).
(2) The ACPI backlight interface doesn't work on the given system,
but there is a vendor platform driver that will register its
own, equally broken, backlight interface if not prevented from
doing so by the ACPI subsystem.
Therefore we need to allow the ACPI backlight interface to be
registered until the i915 driver is loaded which then will unregister
it if the firmware has called _OSI for Windows 8 (or will register
the ACPI video driver without backlight support if not already
present).
For this reason, introduce an alternative function for registering
ACPI video, acpi_video_register_with_quirks(), that will check
whether or not the ACPI video driver has already been registered
and whether or not the backlight Windows 8 quirk has to be applied.
If the quirk has to be applied, it will block the ACPI backlight
support and either unregister the backlight interface if the ACPI
video driver has already been registered, or register the ACPI
video driver without the backlight interface otherwise. Make
the i915 driver use acpi_video_register_with_quirks() instead of
acpi_video_register() in i915_driver_load().
This change is based on earlier patches from Matthew Garrett,
Chun-Yi Lee and Seth Forshee and Aaron Lu's comments.
Signed-off-by: Rafael J. Wysocki
---
drivers/acpi/internal.h | 11 ++
drivers/acpi/video.c| 65
drivers/acpi/video_detect.c | 21
drivers/gpu/drm/i915/i915_dma.c |2 -
include/acpi/video.h| 11 ++
include/linux/acpi.h|1
6 files changed, 103 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/acpi/video.c
===
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -44,6 +44,8 @@
#include
#include
+#include "internal.h"
+
#define PREFIX "ACPI: "
#define ACPI_VIDEO_BUS_NAME"Video Bus"
@@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s
device->cap._DDC = 1;
}
- if (acpi_video_backlight_support()) {
+ if (acpi_video_verify_backlight_support()) {
struct backlight_properties props;
struct pci_dev *pdev;
acpi_handle acpi_parent;
@@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct
return 0;
}
+static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl,
+ void *context, void **rv)
+{
+ struct acpi_device *acpi_dev;
+ struct acpi_video_bus *video;
+ struct acpi_video_device *dev, *next;
+
+ if (acpi_bus_get_device(handle, &acpi_dev))
+ return AE_OK;
+
+ if (acpi_match_device_ids(acpi_dev, video_device_ids))
+ return AE_OK;
+
+ video = acpi_driver_data(acpi_dev);
+ if (!video)
+ return AE_OK;
+
+ acpi_video_bus_stop_devices(video);
+ mutex_lock(&video->device_list_lock);
+ list_for_each_entry_safe(dev, next, &video->video_device_list, entry) {
+ if (dev->backlight) {
+ backlight_device_unregister(dev->backlight);
+ dev->backlight = NULL;
+ kfree(dev->brightness->levels);
+ kfree(dev->brightness);
+ }
+ if (dev->cooling_dev) {
+ sysfs_remove_link(&dev->dev->dev.kobj,
+ "thermal_cooling");
+ sysfs_remove_link(&dev->cooling_dev->device.kobj,
+ "device");
+ thermal_cooling_device_unregister(dev->cooling_dev);
+ dev->cooling_dev = NULL;
+ }
+ }
+ mutex_unlock(&video->device_list_lock);
+ acpi_video_bus_start_devices(video);
+ return AE_OK;
+}
+
static int __init is_i740(struct pci_dev *dev)
{
if (dev->device == 0x00D1)
@@ -1885,14 +1927,25 @@ static int __init intel_opregion_present
return opregion;
}
-int acpi_video_register(
