Since v3.7 the acpi backlight driver doesn't work at all on this machine
because presumably the ACPI code contains stub code when Windows 8 OSI is
reported.
The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
backlight driver, however, on this machine it turns the backlight completely
off when it reaches level 0%, after which the user might have a lot trouble
trying to bring it back.
This patch fixes both regressions by blacklisting the win8 OSI, so we are back
to v3.6 behavior, and it should remain that way until the intel backlight
driver is fixed.
Since v3.7, users have been forced to fix the initial regression by modifying
the boot arguments [1].
[1] https://wiki.archlinux.org/index.php/ASUS_Zenbook_Prime_UX31A
Signed-off-by: Felipe Contreras <[email protected]>
---
drivers/acpi/blacklist.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
index cb96296..a404127 100644
--- a/drivers/acpi/blacklist.c
+++ b/drivers/acpi/blacklist.c
@@ -192,6 +192,12 @@ static int __init dmi_disable_osi_win7(const struct dmi_system_id *d)
acpi_osi_setup("!Windows 2009");
return 0;
}
+static int __init dmi_disable_osi_win8(const struct dmi_system_id *d)
+{
+ printk(KERN_NOTICE PREFIX "DMI detected: %s\n", d->ident);
+ acpi_osi_setup("!Windows 2012");
+ return 0;
+}
static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
{
@@ -267,6 +273,14 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
DMI_MATCH(DMI_PRODUCT_NAME, "Satellite P305D"),
},
},
+ {
+ .callback = dmi_disable_osi_win8,
+ .ident = "ASUS Zenbook Prime UX31A",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "UX31A"),
+ },
+ },
/*
* BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
--
1.8.3.4
On Monday, July 29, 2013 02:20:58 PM Felipe Contreras wrote:
> Since v3.7 the acpi backlight driver doesn't work at all on this machine
> because presumably the ACPI code contains stub code when Windows 8 OSI is
> reported.
>
> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
> backlight driver, however, on this machine it turns the backlight completely
> off when it reaches level 0%, after which the user might have a lot trouble
> trying to bring it back.
>
> This patch fixes both regressions by blacklisting the win8 OSI, so we are back
> to v3.6 behavior, and it should remain that way until the intel backlight
> driver is fixed.
Well, it really is a workaround, so perhaps it would be fair to call it this
way?
> Since v3.7, users have been forced to fix the initial regression by modifying
> the boot arguments [1].
>
> [1] https://wiki.archlinux.org/index.php/ASUS_Zenbook_Prime_UX31A
>
> Signed-off-by: Felipe Contreras <[email protected]>
I don't really feel this is the right approach, but then I don't care enough
to fight with you. :-)
I'll queue up this patch for 3.12.
Thanks,
Rafael
> ---
> drivers/acpi/blacklist.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
> index cb96296..a404127 100644
> --- a/drivers/acpi/blacklist.c
> +++ b/drivers/acpi/blacklist.c
> @@ -192,6 +192,12 @@ static int __init dmi_disable_osi_win7(const struct dmi_system_id *d)
> acpi_osi_setup("!Windows 2009");
> return 0;
> }
> +static int __init dmi_disable_osi_win8(const struct dmi_system_id *d)
> +{
> + printk(KERN_NOTICE PREFIX "DMI detected: %s\n", d->ident);
> + acpi_osi_setup("!Windows 2012");
> + return 0;
> +}
>
> static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
> {
> @@ -267,6 +273,14 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
> DMI_MATCH(DMI_PRODUCT_NAME, "Satellite P305D"),
> },
> },
> + {
> + .callback = dmi_disable_osi_win8,
> + .ident = "ASUS Zenbook Prime UX31A",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "UX31A"),
> + },
> + },
>
> /*
> * BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Mon, Jul 29, 2013 at 3:17 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Monday, July 29, 2013 02:20:58 PM Felipe Contreras wrote:
>> Since v3.7 the acpi backlight driver doesn't work at all on this machine
>> because presumably the ACPI code contains stub code when Windows 8 OSI is
>> reported.
>>
>> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
>> backlight driver, however, on this machine it turns the backlight completely
>> off when it reaches level 0%, after which the user might have a lot trouble
>> trying to bring it back.
>>
>> This patch fixes both regressions by blacklisting the win8 OSI, so we are back
>> to v3.6 behavior, and it should remain that way until the intel backlight
>> driver is fixed.
>
> Well, it really is a workaround, so perhaps it would be fair to call it this
> way?
Maybe, but to me, and to most users, the important thing is that it
fixes a regression.
>> Since v3.7, users have been forced to fix the initial regression by modifying
>> the boot arguments [1].
>>
>> [1] https://wiki.archlinux.org/index.php/ASUS_Zenbook_Prime_UX31A
>>
>> Signed-off-by: Felipe Contreras <[email protected]>
>
> I don't really feel this is the right approach, but then I don't care enough
> to fight with you. :-)
It's not the right approach, I don't think I (or anybody) argued
otherwise, but unless the Intel driver is fixed before 3.11, it's the
only sane approach.
--
Felipe Contreras
On Monday, July 29, 2013 03:22:56 PM Felipe Contreras wrote:
> On Mon, Jul 29, 2013 at 3:17 PM, Rafael J. Wysocki <[email protected]> wrote:
> > On Monday, July 29, 2013 02:20:58 PM Felipe Contreras wrote:
> >> Since v3.7 the acpi backlight driver doesn't work at all on this machine
> >> because presumably the ACPI code contains stub code when Windows 8 OSI is
> >> reported.
> >>
> >> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
> >> backlight driver, however, on this machine it turns the backlight completely
> >> off when it reaches level 0%, after which the user might have a lot trouble
> >> trying to bring it back.
> >>
> >> This patch fixes both regressions by blacklisting the win8 OSI, so we are back
> >> to v3.6 behavior, and it should remain that way until the intel backlight
> >> driver is fixed.
> >
> > Well, it really is a workaround, so perhaps it would be fair to call it this
> > way?
>
> Maybe, but to me, and to most users, the important thing is that it
> fixes a regression.
>
> >> Since v3.7, users have been forced to fix the initial regression by modifying
> >> the boot arguments [1].
> >>
> >> [1] https://wiki.archlinux.org/index.php/ASUS_Zenbook_Prime_UX31A
> >>
> >> Signed-off-by: Felipe Contreras <[email protected]>
> >
> > I don't really feel this is the right approach, but then I don't care enough
> > to fight with you. :-)
>
> It's not the right approach, I don't think I (or anybody) argued
> otherwise, but unless the Intel driver is fixed before 3.11, it's the
> only sane approach.
Oh, you're *so* sure about that ...
What I'm actually thinking is that applying this workaround will reduce the
incentive to fix intel_backlight for the users in question, so it's not like
there are no drawbacks.
But as I said, I'm not going to fight over this.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Mon, Jul 29, 2013 at 3:47 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Monday, July 29, 2013 03:22:56 PM Felipe Contreras wrote:
>> On Mon, Jul 29, 2013 at 3:17 PM, Rafael J. Wysocki <[email protected]> wrote:
>> > On Monday, July 29, 2013 02:20:58 PM Felipe Contreras wrote:
>> >> Since v3.7 the acpi backlight driver doesn't work at all on this machine
>> >> because presumably the ACPI code contains stub code when Windows 8 OSI is
>> >> reported.
>> >>
>> >> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
>> >> backlight driver, however, on this machine it turns the backlight completely
>> >> off when it reaches level 0%, after which the user might have a lot trouble
>> >> trying to bring it back.
>> >>
>> >> This patch fixes both regressions by blacklisting the win8 OSI, so we are back
>> >> to v3.6 behavior, and it should remain that way until the intel backlight
>> >> driver is fixed.
>> >
>> > Well, it really is a workaround, so perhaps it would be fair to call it this
>> > way?
>>
>> Maybe, but to me, and to most users, the important thing is that it
>> fixes a regression.
>>
>> >> Since v3.7, users have been forced to fix the initial regression by modifying
>> >> the boot arguments [1].
>> >>
>> >> [1] https://wiki.archlinux.org/index.php/ASUS_Zenbook_Prime_UX31A
>> >>
>> >> Signed-off-by: Felipe Contreras <[email protected]>
>> >
>> > I don't really feel this is the right approach, but then I don't care enough
>> > to fight with you. :-)
>>
>> It's not the right approach, I don't think I (or anybody) argued
>> otherwise, but unless the Intel driver is fixed before 3.11, it's the
>> only sane approach.
>
> Oh, you're *so* sure about that ...
Yeah, I have heard Linus say many many times that Linux should never
ever break user-experience, that's the number one rule.
http://www.youtube.com/watch?v=kzKzUBLEzwk&t=1m58s
--
Felipe Contreras
On 07/30/2013 03:20 AM, Felipe Contreras wrote:
> Since v3.7 the acpi backlight driver doesn't work at all on this machine
> because presumably the ACPI code contains stub code when Windows 8 OSI is
> reported.
>
> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
> backlight driver, however, on this machine it turns the backlight completely
> off when it reaches level 0%, after which the user might have a lot trouble
> trying to bring it back.
What do you mean by a lot of trouble? If you press hotkey to increase
backlight brightness level, does it work? If so, the screen should not
be black any more, it's not that user has to blindly enter some command
to get out of the black screen.
And I'm not sure if this is a bug of intel_backlight(setting a low level
makes the screen almost off), I see different models with different
vendors having this behavior. If this is deemed a bug, then I'm afraid
intel_backlight interface is useless for a lot of systems...perhaps we
can only say, intel_backlight's interpretation of low levels are
different with ACPI video's, and that's probably why its type is named
as raw :-)
-Aaron
>
> This patch fixes both regressions by blacklisting the win8 OSI, so we are back
> to v3.6 behavior, and it should remain that way until the intel backlight
> driver is fixed.
>
> Since v3.7, users have been forced to fix the initial regression by modifying
> the boot arguments [1].
>
> [1] https://wiki.archlinux.org/index.php/ASUS_Zenbook_Prime_UX31A
>
> Signed-off-by: Felipe Contreras <[email protected]>
> ---
> drivers/acpi/blacklist.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
> index cb96296..a404127 100644
> --- a/drivers/acpi/blacklist.c
> +++ b/drivers/acpi/blacklist.c
> @@ -192,6 +192,12 @@ static int __init dmi_disable_osi_win7(const struct dmi_system_id *d)
> acpi_osi_setup("!Windows 2009");
> return 0;
> }
> +static int __init dmi_disable_osi_win8(const struct dmi_system_id *d)
> +{
> + printk(KERN_NOTICE PREFIX "DMI detected: %s\n", d->ident);
> + acpi_osi_setup("!Windows 2012");
> + return 0;
> +}
>
> static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
> {
> @@ -267,6 +273,14 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = {
> DMI_MATCH(DMI_PRODUCT_NAME, "Satellite P305D"),
> },
> },
> + {
> + .callback = dmi_disable_osi_win8,
> + .ident = "ASUS Zenbook Prime UX31A",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "UX31A"),
> + },
> + },
>
> /*
> * BIOS invocation of _OSI(Linux) is almost always a BIOS bug.
>
On Mon, Jul 29, 2013 at 10:11 PM, Aaron Lu <[email protected]> wrote:
> On 07/30/2013 03:20 AM, Felipe Contreras wrote:
>> Since v3.7 the acpi backlight driver doesn't work at all on this machine
>> because presumably the ACPI code contains stub code when Windows 8 OSI is
>> reported.
>>
>> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
>> backlight driver, however, on this machine it turns the backlight completely
>> off when it reaches level 0%, after which the user might have a lot trouble
>> trying to bring it back.
>
> What do you mean by a lot of trouble? If you press hotkey to increase
> backlight brightness level, does it work?
I guess so, *if* there is indeed a user-space power manager handling
that, *and* the keyboard has such keys, *or* the user has set custom
hotkeys.
> If so, the screen should not
> be black any more, it's not that user has to blindly enter some command
> to get out of the black screen.
>
> And I'm not sure if this is a bug of intel_backlight(setting a low level
> makes the screen almost off), I see different models with different
> vendors having this behavior.
I mean, the screen is completely off, I cannot see absolutely
anything. I don't see this behavior with the ACPI backlight driver,
nor do I see that in Windows 7.
> If this is deemed a bug, then I'm afraid
> intel_backlight interface is useless for a lot of systems...perhaps we
> can only say, intel_backlight's interpretation of low levels are
> different with ACPI video's, and that's probably why its type is named
> as raw :-)
Well, a bug is defined as unexpected behavior -- as a user, if I'm
changing the brightness of the screen, I certainly don't expect the
screen to turn off, and I think that's the expectation from most
people. It's the first time I see something like that.
--
Felipe Contreras
On 07/30/2013 11:44 AM, Felipe Contreras wrote:
> On Mon, Jul 29, 2013 at 10:11 PM, Aaron Lu <[email protected]> wrote:
>> On 07/30/2013 03:20 AM, Felipe Contreras wrote:
>>> Since v3.7 the acpi backlight driver doesn't work at all on this machine
>>> because presumably the ACPI code contains stub code when Windows 8 OSI is
>>> reported.
>>>
>>> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
>>> backlight driver, however, on this machine it turns the backlight completely
>>> off when it reaches level 0%, after which the user might have a lot trouble
>>> trying to bring it back.
>>
>> What do you mean by a lot of trouble? If you press hotkey to increase
>> backlight brightness level, does it work?
>
> I guess so, *if* there is indeed a user-space power manager handling
> that, *and* the keyboard has such keys, *or* the user has set custom
> hotkeys.
Right, for a GUI environment this may not be a big problem(user uses Fn
key to decrease brightness level and then hit the black screen, it's
natural he will use Fn key to increase brightness level).
>
>> If so, the screen should not
>> be black any more, it's not that user has to blindly enter some command
>> to get out of the black screen.
>>
>> And I'm not sure if this is a bug of intel_backlight(setting a low level
>> makes the screen almost off), I see different models with different
>> vendors having this behavior.
>
> I mean, the screen is completely off, I cannot see absolutely
> anything. I don't see this behavior with the ACPI backlight driver,
> nor do I see that in Windows 7.
>
>> If this is deemed a bug, then I'm afraid
>> intel_backlight interface is useless for a lot of systems...perhaps we
>> can only say, intel_backlight's interpretation of low levels are
>> different with ACPI video's, and that's probably why its type is named
>> as raw :-)
>
> Well, a bug is defined as unexpected behavior -- as a user, if I'm
> changing the brightness of the screen, I certainly don't expect the
> screen to turn off, and I think that's the expectation from most
> people. It's the first time I see something like that.
I agree this is kind of un-expected. At the same time, this seems to be
the normal behavior for intel_backlight. I don't know what the correct
thing to do here if this is something we want to avoid - modify intel
backlight handling code not to set too low value or change the user
space tool not to set a too low value if they are interacting with a
raw type interface. Neither of them sounds cool... Or, users may get
used to it, I for example, don't find this to be very annoying, but
maybe I'm already used to it.
-Aaron
On 07/30/2013 01:51 PM, Aaron Lu wrote:
> On 07/30/2013 11:44 AM, Felipe Contreras wrote:
>> On Mon, Jul 29, 2013 at 10:11 PM, Aaron Lu <[email protected]> wrote:
>>> On 07/30/2013 03:20 AM, Felipe Contreras wrote:
>>>> Since v3.7 the acpi backlight driver doesn't work at all on this machine
>>>> because presumably the ACPI code contains stub code when Windows 8 OSI is
>>>> reported.
>>>>
>>>> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
>>>> backlight driver, however, on this machine it turns the backlight completely
>>>> off when it reaches level 0%, after which the user might have a lot trouble
>>>> trying to bring it back.
>>>
>>> What do you mean by a lot of trouble? If you press hotkey to increase
>>> backlight brightness level, does it work?
>>
>> I guess so, *if* there is indeed a user-space power manager handling
>> that, *and* the keyboard has such keys, *or* the user has set custom
>> hotkeys.
>
> Right, for a GUI environment this may not be a big problem(user uses Fn
> key to decrease brightness level and then hit the black screen, it's
> natural he will use Fn key to increase brightness level).
>
>>
>>> If so, the screen should not
>>> be black any more, it's not that user has to blindly enter some command
>>> to get out of the black screen.
>>>
>>> And I'm not sure if this is a bug of intel_backlight(setting a low level
>>> makes the screen almost off), I see different models with different
>>> vendors having this behavior.
>>
>> I mean, the screen is completely off, I cannot see absolutely
>> anything. I don't see this behavior with the ACPI backlight driver,
>> nor do I see that in Windows 7.
>>
>>> If this is deemed a bug, then I'm afraid
>>> intel_backlight interface is useless for a lot of systems...perhaps we
>>> can only say, intel_backlight's interpretation of low levels are
>>> different with ACPI video's, and that's probably why its type is named
>>> as raw :-)
>>
>> Well, a bug is defined as unexpected behavior -- as a user, if I'm
>> changing the brightness of the screen, I certainly don't expect the
>> screen to turn off, and I think that's the expectation from most
>> people. It's the first time I see something like that.
>
> I agree this is kind of un-expected. At the same time, this seems to be
> the normal behavior for intel_backlight. I don't know what the correct
> thing to do here if this is something we want to avoid - modify intel
> backlight handling code not to set too low value or change the user
> space tool not to set a too low value if they are interacting with a
> raw type interface. Neither of them sounds cool... Or, users may get
> used to it, I for example, don't find this to be very annoying, but
> maybe I'm already used to it.
BTW, for the complete screen off problem, I don't see there is anything
wrong with it from code's point of view. It's not that there is an error
in code or this is a broken hardware that caused the screen off when
setting a very low or 0 brightness level, it is simply the expected
behavior of what this interface can provide. It can really set the
brightness level to minimum(zero) or maximum. Don't get me wrong, I
didn't mean this is a good user experience, I don't know that. I just
don't think this is a program bug, and I don't know if this should be
fixed or not - obviously this interface did what it is asked to do,
correctly.
Thanks,
Aaron
On Tuesday, July 30, 2013 01:57:55 PM Aaron Lu wrote:
> On 07/30/2013 01:51 PM, Aaron Lu wrote:
> > On 07/30/2013 11:44 AM, Felipe Contreras wrote:
> >> On Mon, Jul 29, 2013 at 10:11 PM, Aaron Lu <[email protected]> wrote:
> >>> On 07/30/2013 03:20 AM, Felipe Contreras wrote:
> >>>> Since v3.7 the acpi backlight driver doesn't work at all on this machine
> >>>> because presumably the ACPI code contains stub code when Windows 8 OSI is
> >>>> reported.
> >>>>
> >>>> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
> >>>> backlight driver, however, on this machine it turns the backlight completely
> >>>> off when it reaches level 0%, after which the user might have a lot trouble
> >>>> trying to bring it back.
> >>>
> >>> What do you mean by a lot of trouble? If you press hotkey to increase
> >>> backlight brightness level, does it work?
> >>
> >> I guess so, *if* there is indeed a user-space power manager handling
> >> that, *and* the keyboard has such keys, *or* the user has set custom
> >> hotkeys.
> >
> > Right, for a GUI environment this may not be a big problem(user uses Fn
> > key to decrease brightness level and then hit the black screen, it's
> > natural he will use Fn key to increase brightness level).
> >
> >>
> >>> If so, the screen should not
> >>> be black any more, it's not that user has to blindly enter some command
> >>> to get out of the black screen.
> >>>
> >>> And I'm not sure if this is a bug of intel_backlight(setting a low level
> >>> makes the screen almost off), I see different models with different
> >>> vendors having this behavior.
> >>
> >> I mean, the screen is completely off, I cannot see absolutely
> >> anything. I don't see this behavior with the ACPI backlight driver,
> >> nor do I see that in Windows 7.
> >>
> >>> If this is deemed a bug, then I'm afraid
> >>> intel_backlight interface is useless for a lot of systems...perhaps we
> >>> can only say, intel_backlight's interpretation of low levels are
> >>> different with ACPI video's, and that's probably why its type is named
> >>> as raw :-)
> >>
> >> Well, a bug is defined as unexpected behavior -- as a user, if I'm
> >> changing the brightness of the screen, I certainly don't expect the
> >> screen to turn off, and I think that's the expectation from most
> >> people. It's the first time I see something like that.
> >
> > I agree this is kind of un-expected. At the same time, this seems to be
> > the normal behavior for intel_backlight. I don't know what the correct
> > thing to do here if this is something we want to avoid - modify intel
> > backlight handling code not to set too low value or change the user
> > space tool not to set a too low value if they are interacting with a
> > raw type interface. Neither of them sounds cool... Or, users may get
> > used to it, I for example, don't find this to be very annoying, but
> > maybe I'm already used to it.
>
> BTW, for the complete screen off problem, I don't see there is anything
> wrong with it from code's point of view. It's not that there is an error
> in code or this is a broken hardware that caused the screen off when
> setting a very low or 0 brightness level, it is simply the expected
> behavior of what this interface can provide. It can really set the
> brightness level to minimum(zero) or maximum. Don't get me wrong, I
> didn't mean this is a good user experience, I don't know that. I just
> don't think this is a program bug, and I don't know if this should be
> fixed or not - obviously this interface did what it is asked to do,
> correctly.
Precisely, user space asks for 0 and the kernel delivers.
And there are reasons why 0 should be "screen off", like power management
(when you have a policy to dim the screen completely after a period of
inactivity, for example).
So in my opinion, if that's a problem for anyone, it has to be addressed in
user space and if there are any vendors who try to address *that* in their ACPI
tables, that's one more reason to avoid using ACPI for backlight control.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Tue, Jul 30, 2013 at 8:42 AM, Rafael J. Wysocki <[email protected]> wrote:
> On Tuesday, July 30, 2013 01:57:55 PM Aaron Lu wrote:
>> On 07/30/2013 01:51 PM, Aaron Lu wrote:
>> > On 07/30/2013 11:44 AM, Felipe Contreras wrote:
>> >> On Mon, Jul 29, 2013 at 10:11 PM, Aaron Lu <[email protected]> wrote:
>> >>> On 07/30/2013 03:20 AM, Felipe Contreras wrote:
>> >>>> Since v3.7 the acpi backlight driver doesn't work at all on this machine
>> >>>> because presumably the ACPI code contains stub code when Windows 8 OSI is
>> >>>> reported.
>> >>>>
>> >>>> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
>> >>>> backlight driver, however, on this machine it turns the backlight completely
>> >>>> off when it reaches level 0%, after which the user might have a lot trouble
>> >>>> trying to bring it back.
>> >>>
>> >>> What do you mean by a lot of trouble? If you press hotkey to increase
>> >>> backlight brightness level, does it work?
>> >>
>> >> I guess so, *if* there is indeed a user-space power manager handling
>> >> that, *and* the keyboard has such keys, *or* the user has set custom
>> >> hotkeys.
>> >
>> > Right, for a GUI environment this may not be a big problem(user uses Fn
>> > key to decrease brightness level and then hit the black screen, it's
>> > natural he will use Fn key to increase brightness level).
>> >
>> >>
>> >>> If so, the screen should not
>> >>> be black any more, it's not that user has to blindly enter some command
>> >>> to get out of the black screen.
>> >>>
>> >>> And I'm not sure if this is a bug of intel_backlight(setting a low level
>> >>> makes the screen almost off), I see different models with different
>> >>> vendors having this behavior.
>> >>
>> >> I mean, the screen is completely off, I cannot see absolutely
>> >> anything. I don't see this behavior with the ACPI backlight driver,
>> >> nor do I see that in Windows 7.
>> >>
>> >>> If this is deemed a bug, then I'm afraid
>> >>> intel_backlight interface is useless for a lot of systems...perhaps we
>> >>> can only say, intel_backlight's interpretation of low levels are
>> >>> different with ACPI video's, and that's probably why its type is named
>> >>> as raw :-)
>> >>
>> >> Well, a bug is defined as unexpected behavior -- as a user, if I'm
>> >> changing the brightness of the screen, I certainly don't expect the
>> >> screen to turn off, and I think that's the expectation from most
>> >> people. It's the first time I see something like that.
>> >
>> > I agree this is kind of un-expected. At the same time, this seems to be
>> > the normal behavior for intel_backlight. I don't know what the correct
>> > thing to do here if this is something we want to avoid - modify intel
>> > backlight handling code not to set too low value or change the user
>> > space tool not to set a too low value if they are interacting with a
>> > raw type interface. Neither of them sounds cool... Or, users may get
>> > used to it, I for example, don't find this to be very annoying, but
>> > maybe I'm already used to it.
>>
>> BTW, for the complete screen off problem, I don't see there is anything
>> wrong with it from code's point of view. It's not that there is an error
>> in code or this is a broken hardware that caused the screen off when
>> setting a very low or 0 brightness level, it is simply the expected
>> behavior of what this interface can provide. It can really set the
>> brightness level to minimum(zero) or maximum. Don't get me wrong, I
>> didn't mean this is a good user experience, I don't know that. I just
>> don't think this is a program bug, and I don't know if this should be
>> fixed or not - obviously this interface did what it is asked to do,
>> correctly.
>
> Precisely, user space asks for 0 and the kernel delivers.
>
> And there are reasons why 0 should be "screen off", like power management
> (when you have a policy to dim the screen completely after a period of
> inactivity, for example).
There is another interface the turn the screen off.
If 0 turns the screen off with the intel driver, 0 should turn the
screen off with the ACPI driver, having inconsistent behavior
depending on which driver is used is a bug.
If 0 did not turn off the screen in v3.10, 0 should not turn off the
screen in v3.11, to do so would be a *regression*.
> So in my opinion, if that's a problem for anyone, it has to be addressed in
> user space and if there are any vendors who try to address *that* in their ACPI
> tables, that's one more reason to avoid using ACPI for backlight control.
If you think it's the user-space responsibility to deal with kernel
bugs, I think it's only a matter of time before you receive one of
these [1].
"If a change results in user programs breaking, it's a bug in the
kernel. We never EVER blame the user programs. How hard can this be to
understand?"
"WE DO NOT BREAK USERSPACE!"
[1] https://lkml.org/lkml/2012/12/23/75
--
Felipe Contreras
On Tuesday, July 30, 2013 03:59:26 PM Felipe Contreras wrote:
> On Tue, Jul 30, 2013 at 8:42 AM, Rafael J. Wysocki <[email protected]> wrote:
> > On Tuesday, July 30, 2013 01:57:55 PM Aaron Lu wrote:
> >> On 07/30/2013 01:51 PM, Aaron Lu wrote:
> >> > On 07/30/2013 11:44 AM, Felipe Contreras wrote:
> >> >> On Mon, Jul 29, 2013 at 10:11 PM, Aaron Lu <[email protected]> wrote:
> >> >>> On 07/30/2013 03:20 AM, Felipe Contreras wrote:
> >> >>>> Since v3.7 the acpi backlight driver doesn't work at all on this machine
> >> >>>> because presumably the ACPI code contains stub code when Windows 8 OSI is
> >> >>>> reported.
> >> >>>>
> >> >>>> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
> >> >>>> backlight driver, however, on this machine it turns the backlight completely
> >> >>>> off when it reaches level 0%, after which the user might have a lot trouble
> >> >>>> trying to bring it back.
> >> >>>
> >> >>> What do you mean by a lot of trouble? If you press hotkey to increase
> >> >>> backlight brightness level, does it work?
> >> >>
> >> >> I guess so, *if* there is indeed a user-space power manager handling
> >> >> that, *and* the keyboard has such keys, *or* the user has set custom
> >> >> hotkeys.
> >> >
> >> > Right, for a GUI environment this may not be a big problem(user uses Fn
> >> > key to decrease brightness level and then hit the black screen, it's
> >> > natural he will use Fn key to increase brightness level).
> >> >
> >> >>
> >> >>> If so, the screen should not
> >> >>> be black any more, it's not that user has to blindly enter some command
> >> >>> to get out of the black screen.
> >> >>>
> >> >>> And I'm not sure if this is a bug of intel_backlight(setting a low level
> >> >>> makes the screen almost off), I see different models with different
> >> >>> vendors having this behavior.
> >> >>
> >> >> I mean, the screen is completely off, I cannot see absolutely
> >> >> anything. I don't see this behavior with the ACPI backlight driver,
> >> >> nor do I see that in Windows 7.
> >> >>
> >> >>> If this is deemed a bug, then I'm afraid
> >> >>> intel_backlight interface is useless for a lot of systems...perhaps we
> >> >>> can only say, intel_backlight's interpretation of low levels are
> >> >>> different with ACPI video's, and that's probably why its type is named
> >> >>> as raw :-)
> >> >>
> >> >> Well, a bug is defined as unexpected behavior -- as a user, if I'm
> >> >> changing the brightness of the screen, I certainly don't expect the
> >> >> screen to turn off, and I think that's the expectation from most
> >> >> people. It's the first time I see something like that.
> >> >
> >> > I agree this is kind of un-expected. At the same time, this seems to be
> >> > the normal behavior for intel_backlight. I don't know what the correct
> >> > thing to do here if this is something we want to avoid - modify intel
> >> > backlight handling code not to set too low value or change the user
> >> > space tool not to set a too low value if they are interacting with a
> >> > raw type interface. Neither of them sounds cool... Or, users may get
> >> > used to it, I for example, don't find this to be very annoying, but
> >> > maybe I'm already used to it.
> >>
> >> BTW, for the complete screen off problem, I don't see there is anything
> >> wrong with it from code's point of view. It's not that there is an error
> >> in code or this is a broken hardware that caused the screen off when
> >> setting a very low or 0 brightness level, it is simply the expected
> >> behavior of what this interface can provide. It can really set the
> >> brightness level to minimum(zero) or maximum. Don't get me wrong, I
> >> didn't mean this is a good user experience, I don't know that. I just
> >> don't think this is a program bug, and I don't know if this should be
> >> fixed or not - obviously this interface did what it is asked to do,
> >> correctly.
> >
> > Precisely, user space asks for 0 and the kernel delivers.
> >
> > And there are reasons why 0 should be "screen off", like power management
> > (when you have a policy to dim the screen completely after a period of
> > inactivity, for example).
>
> There is another interface the turn the screen off.
>
> If 0 turns the screen off with the intel driver, 0 should turn the
> screen off with the ACPI driver, having inconsistent behavior
> depending on which driver is used is a bug.
The ACPI driver simply exposes and interface to interact with the AML methods
in the BIOS directly.
Yes, this is a mistake and shouldn't be designed this way.
However, incidentally, this makes backlight control work on your machine.
Anyway, we need all backlight drivers to work consistently and don't tempt me
to rip the ACPI driver entirely from the kernel for what it's worth.
Yes, that will break backlight on your system and *then* you can complain to
Linus if you wish.
This is the last message I'm sending in this thread.
Kind regards,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Tue, Jul 30, 2013 at 6:13 PM, Rafael J. Wysocki <[email protected]> wrote:
>> If 0 turns the screen off with the intel driver, 0 should turn the
>> screen off with the ACPI driver, having inconsistent behavior
>> depending on which driver is used is a bug.
>
> The ACPI driver simply exposes and interface to interact with the AML methods
> in the BIOS directly.
No, the ACPI driver is exposing a backlight interface, which has a
defined stable API.
Documentation/ABI/stable/sysfs-class-backlight
Yes, the interface doesn't define what should happen at 0, that is a
bug in the interface definition.
*How* it achieves that is an implementation detail.
> Yes, this is a mistake and shouldn't be designed this way.
>
> However, incidentally, this makes backlight control work on your machine.
>
> Anyway, we need all backlight drivers to work consistently and don't tempt me
> to rip the ACPI driver entirely from the kernel for what it's worth.
Yes, they should work consistently, and go ahead, rip the ACPI driver,
*then* you'll see many more people complaining about the Linux kernel
breaking user-space, which should never happen. Mistakes happen, but
if you do this willingly and knowingly, I think there would be
repercussions for you.
> Yes, that will break backlight on your system and *then* you can complain to
> Linus if you wish.
It is already broken in v3.11-rc3, in fact I just booted that to try
it out and it booted with the screen completely black (fortunately I
knew exactly what to type to change that).
Apparently this commit also needs to be reverted: efaa14c (ACPI /
video: no automatic brightness changes by win8-compatible firmware).
In this machine it makes the backlight work again (without
acpi_osi="!Windows 2012"), but by doing so the ACPI driver also turns
off the screen completely at level 0. Also, each time I change the
backlight level from X, the screen blinks as if going 100%, 0%, and
then the desired level.
For this particular machine simply applying the attached patch would
solve all those regressions, but who knows in other machines, I think
it's safer to revert efaa14c.
--
Felipe Contreras
On 07/31/2013 08:11 AM, Felipe Contreras wrote:
> On Tue, Jul 30, 2013 at 6:13 PM, Rafael J. Wysocki <[email protected]> wrote:
>
>>> If 0 turns the screen off with the intel driver, 0 should turn the
>>> screen off with the ACPI driver, having inconsistent behavior
>>> depending on which driver is used is a bug.
>>
>> The ACPI driver simply exposes and interface to interact with the AML methods
>> in the BIOS directly.
>
> No, the ACPI driver is exposing a backlight interface, which has a
> defined stable API.
>
> Documentation/ABI/stable/sysfs-class-backlight
>
> Yes, the interface doesn't define what should happen at 0, that is a
> bug in the interface definition.
>
> *How* it achieves that is an implementation detail.
>
>> Yes, this is a mistake and shouldn't be designed this way.
>>
>> However, incidentally, this makes backlight control work on your machine.
>>
>> Anyway, we need all backlight drivers to work consistently and don't tempt me
>> to rip the ACPI driver entirely from the kernel for what it's worth.
>
> Yes, they should work consistently, and go ahead, rip the ACPI driver,
> *then* you'll see many more people complaining about the Linux kernel
> breaking user-space, which should never happen. Mistakes happen, but
> if you do this willingly and knowingly, I think there would be
> repercussions for you.
>
>> Yes, that will break backlight on your system and *then* you can complain to
>> Linus if you wish.
>
> It is already broken in v3.11-rc3, in fact I just booted that to try
> it out and it booted with the screen completely black (fortunately I
> knew exactly what to type to change that).
That is bad, can you please file a bug for that? I'll need to take a
look at your ACPI tables, thanks.
>
> Apparently this commit also needs to be reverted: efaa14c (ACPI /
> video: no automatic brightness changes by win8-compatible firmware).
> In this machine it makes the backlight work again (without
> acpi_osi="!Windows 2012"), but by doing so the ACPI driver also turns
> off the screen completely at level 0. Also, each time I change the
So with rc3 and commit efaa14c reverted, when you set level 0 to ACPI
video's backlight interface, the screen will be off now? And this is not
the case in 3.10, right?
> backlight level from X, the screen blinks as if going 100%, 0%, and
> then the desired level.
Please attach acpidump output to the to be opened bugzilla page, thanks.
-Aaron
>
> For this particular machine simply applying the attached patch would
> solve all those regressions, but who knows in other machines, I think
> it's safer to revert efaa14c.
>
On 07/31/2013 04:59 AM, Felipe Contreras wrote:
> On Tue, Jul 30, 2013 at 8:42 AM, Rafael J. Wysocki <[email protected]> wrote:
>> On Tuesday, July 30, 2013 01:57:55 PM Aaron Lu wrote:
>>> On 07/30/2013 01:51 PM, Aaron Lu wrote:
>>>> On 07/30/2013 11:44 AM, Felipe Contreras wrote:
>>>>> On Mon, Jul 29, 2013 at 10:11 PM, Aaron Lu <[email protected]> wrote:
>>>>>> On 07/30/2013 03:20 AM, Felipe Contreras wrote:
>>>>>>> Since v3.7 the acpi backlight driver doesn't work at all on this machine
>>>>>>> because presumably the ACPI code contains stub code when Windows 8 OSI is
>>>>>>> reported.
>>>>>>>
>>>>>>> The commit ea45ea7 (in v3.11-rc2) tried to fix this problem by using the intel
>>>>>>> backlight driver, however, on this machine it turns the backlight completely
>>>>>>> off when it reaches level 0%, after which the user might have a lot trouble
>>>>>>> trying to bring it back.
>>>>>>
>>>>>> What do you mean by a lot of trouble? If you press hotkey to increase
>>>>>> backlight brightness level, does it work?
>>>>>
>>>>> I guess so, *if* there is indeed a user-space power manager handling
>>>>> that, *and* the keyboard has such keys, *or* the user has set custom
>>>>> hotkeys.
>>>>
>>>> Right, for a GUI environment this may not be a big problem(user uses Fn
>>>> key to decrease brightness level and then hit the black screen, it's
>>>> natural he will use Fn key to increase brightness level).
>>>>
>>>>>
>>>>>> If so, the screen should not
>>>>>> be black any more, it's not that user has to blindly enter some command
>>>>>> to get out of the black screen.
>>>>>>
>>>>>> And I'm not sure if this is a bug of intel_backlight(setting a low level
>>>>>> makes the screen almost off), I see different models with different
>>>>>> vendors having this behavior.
>>>>>
>>>>> I mean, the screen is completely off, I cannot see absolutely
>>>>> anything. I don't see this behavior with the ACPI backlight driver,
>>>>> nor do I see that in Windows 7.
>>>>>
>>>>>> If this is deemed a bug, then I'm afraid
>>>>>> intel_backlight interface is useless for a lot of systems...perhaps we
>>>>>> can only say, intel_backlight's interpretation of low levels are
>>>>>> different with ACPI video's, and that's probably why its type is named
>>>>>> as raw :-)
>>>>>
>>>>> Well, a bug is defined as unexpected behavior -- as a user, if I'm
>>>>> changing the brightness of the screen, I certainly don't expect the
>>>>> screen to turn off, and I think that's the expectation from most
>>>>> people. It's the first time I see something like that.
>>>>
>>>> I agree this is kind of un-expected. At the same time, this seems to be
>>>> the normal behavior for intel_backlight. I don't know what the correct
>>>> thing to do here if this is something we want to avoid - modify intel
>>>> backlight handling code not to set too low value or change the user
>>>> space tool not to set a too low value if they are interacting with a
>>>> raw type interface. Neither of them sounds cool... Or, users may get
>>>> used to it, I for example, don't find this to be very annoying, but
>>>> maybe I'm already used to it.
>>>
>>> BTW, for the complete screen off problem, I don't see there is anything
>>> wrong with it from code's point of view. It's not that there is an error
>>> in code or this is a broken hardware that caused the screen off when
>>> setting a very low or 0 brightness level, it is simply the expected
>>> behavior of what this interface can provide. It can really set the
>>> brightness level to minimum(zero) or maximum. Don't get me wrong, I
>>> didn't mean this is a good user experience, I don't know that. I just
>>> don't think this is a program bug, and I don't know if this should be
>>> fixed or not - obviously this interface did what it is asked to do,
>>> correctly.
>>
>> Precisely, user space asks for 0 and the kernel delivers.
>>
>> And there are reasons why 0 should be "screen off", like power management
>> (when you have a policy to dim the screen completely after a period of
>> inactivity, for example).
>
> There is another interface the turn the screen off.
>
> If 0 turns the screen off with the intel driver, 0 should turn the
> screen off with the ACPI driver, having inconsistent behavior
> depending on which driver is used is a bug.
I'm not sure of this. Remember the days when we switch the hard disk
driver from IDE to SCSI? The block device name changed from hdx to sdx.
Is this a bug?
>
> If 0 did not turn off the screen in v3.10, 0 should not turn off the
> screen in v3.11, to do so would be a *regression*.
That depends on how you see it. I believe 0 also turns off the screen in
v3.10 if we are talking about the same driver(intel_backlight).
-Aaron
>
>> So in my opinion, if that's a problem for anyone, it has to be addressed in
>> user space and if there are any vendors who try to address *that* in their ACPI
>> tables, that's one more reason to avoid using ACPI for backlight control.
>
> If you think it's the user-space responsibility to deal with kernel
> bugs, I think it's only a matter of time before you receive one of
> these [1].
>
> "If a change results in user programs breaking, it's a bug in the
> kernel. We never EVER blame the user programs. How hard can this be to
> understand?"
>
> "WE DO NOT BREAK USERSPACE!"
>
> [1] https://lkml.org/lkml/2012/12/23/75
>
On Tue, Jul 30, 2013 at 8:36 PM, Aaron Lu <[email protected]> wrote:
> On 07/31/2013 08:11 AM, Felipe Contreras wrote:
>> On Tue, Jul 30, 2013 at 6:13 PM, Rafael J. Wysocki <[email protected]> wrote:
>>
>>>> If 0 turns the screen off with the intel driver, 0 should turn the
>>>> screen off with the ACPI driver, having inconsistent behavior
>>>> depending on which driver is used is a bug.
>>>
>>> The ACPI driver simply exposes and interface to interact with the AML methods
>>> in the BIOS directly.
>>
>> No, the ACPI driver is exposing a backlight interface, which has a
>> defined stable API.
>>
>> Documentation/ABI/stable/sysfs-class-backlight
>>
>> Yes, the interface doesn't define what should happen at 0, that is a
>> bug in the interface definition.
>>
>> *How* it achieves that is an implementation detail.
>>
>>> Yes, this is a mistake and shouldn't be designed this way.
>>>
>>> However, incidentally, this makes backlight control work on your machine.
>>>
>>> Anyway, we need all backlight drivers to work consistently and don't tempt me
>>> to rip the ACPI driver entirely from the kernel for what it's worth.
>>
>> Yes, they should work consistently, and go ahead, rip the ACPI driver,
>> *then* you'll see many more people complaining about the Linux kernel
>> breaking user-space, which should never happen. Mistakes happen, but
>> if you do this willingly and knowingly, I think there would be
>> repercussions for you.
>>
>>> Yes, that will break backlight on your system and *then* you can complain to
>>> Linus if you wish.
>>
>> It is already broken in v3.11-rc3, in fact I just booted that to try
>> it out and it booted with the screen completely black (fortunately I
>> knew exactly what to type to change that).
>
> That is bad, can you please file a bug for that? I'll need to take a
> look at your ACPI tables, thanks.
File a bug where?
>> Apparently this commit also needs to be reverted: efaa14c (ACPI /
>> video: no automatic brightness changes by win8-compatible firmware).
>> In this machine it makes the backlight work again (without
>> acpi_osi="!Windows 2012"), but by doing so the ACPI driver also turns
>> off the screen completely at level 0. Also, each time I change the
>
> So with rc3 and commit efaa14c reverted, when you set level 0 to ACPI
> video's backlight interface, the screen will be off now? And this is not
> the case in 3.10, right?
No, setting level 0 turns it off if efaa14c is *not* reverted. In 3.10
0 doesn't turn it off.
>> backlight level from X, the screen blinks as if going 100%, 0%, and
>> then the desired level.
>
> Please attach acpidump output to the to be opened bugzilla page, thanks.
I looked at the code in the DCDT, it appears to me that they store
different levels depending on the OSI version, so I don't think the
problem is in the ACPI driver. Yet, the issue remains there.
--
Felipe Contreras
On Tue, Jul 30, 2013 at 8:59 PM, Aaron Lu <[email protected]> wrote:
> On 07/31/2013 04:59 AM, Felipe Contreras wrote:
>> There is another interface the turn the screen off.
>>
>> If 0 turns the screen off with the intel driver, 0 should turn the
>> screen off with the ACPI driver, having inconsistent behavior
>> depending on which driver is used is a bug.
>
> I'm not sure of this. Remember the days when we switch the hard disk
> driver from IDE to SCSI? The block device name changed from hdx to sdx.
> Is this a bug?
The name might have changed, but the way the interface worked did not;
both hdx and sdx worked exactly the same.
--
Felipe Contreras
On 07/31/2013 10:07 AM, Felipe Contreras wrote:
> On Tue, Jul 30, 2013 at 8:36 PM, Aaron Lu <[email protected]> wrote:
>> On 07/31/2013 08:11 AM, Felipe Contreras wrote:
>>> On Tue, Jul 30, 2013 at 6:13 PM, Rafael J. Wysocki <[email protected]> wrote:
>>>
>>>>> If 0 turns the screen off with the intel driver, 0 should turn the
>>>>> screen off with the ACPI driver, having inconsistent behavior
>>>>> depending on which driver is used is a bug.
>>>>
>>>> The ACPI driver simply exposes and interface to interact with the AML methods
>>>> in the BIOS directly.
>>>
>>> No, the ACPI driver is exposing a backlight interface, which has a
>>> defined stable API.
>>>
>>> Documentation/ABI/stable/sysfs-class-backlight
>>>
>>> Yes, the interface doesn't define what should happen at 0, that is a
>>> bug in the interface definition.
>>>
>>> *How* it achieves that is an implementation detail.
>>>
>>>> Yes, this is a mistake and shouldn't be designed this way.
>>>>
>>>> However, incidentally, this makes backlight control work on your machine.
>>>>
>>>> Anyway, we need all backlight drivers to work consistently and don't tempt me
>>>> to rip the ACPI driver entirely from the kernel for what it's worth.
>>>
>>> Yes, they should work consistently, and go ahead, rip the ACPI driver,
>>> *then* you'll see many more people complaining about the Linux kernel
>>> breaking user-space, which should never happen. Mistakes happen, but
>>> if you do this willingly and knowingly, I think there would be
>>> repercussions for you.
>>>
>>>> Yes, that will break backlight on your system and *then* you can complain to
>>>> Linus if you wish.
>>>
>>> It is already broken in v3.11-rc3, in fact I just booted that to try
>>> it out and it booted with the screen completely black (fortunately I
>>> knew exactly what to type to change that).
>>
>> That is bad, can you please file a bug for that? I'll need to take a
>> look at your ACPI tables, thanks.
>
> File a bug where?
https://bugzilla.kernel.org/
For component, please choose ACPI->Power_Video.
>
>>> Apparently this commit also needs to be reverted: efaa14c (ACPI /
>>> video: no automatic brightness changes by win8-compatible firmware).
>>> In this machine it makes the backlight work again (without
>>> acpi_osi="!Windows 2012"), but by doing so the ACPI driver also turns
>>> off the screen completely at level 0. Also, each time I change the
>>
>> So with rc3 and commit efaa14c reverted, when you set level 0 to ACPI
>> video's backlight interface, the screen will be off now? And this is not
>> the case in 3.10, right?
>
> No, setting level 0 turns it off if efaa14c is *not* reverted. In 3.10
> 0 doesn't turn it off.
AFAIK, Win8 firmware will check a flag to decide what to do on hotkey
press and backlight brightness level change. Common behavior is:
If that flag is set:
- on hotkey press, send out event;
- on brightness level change, use GPU driver's interface to change
backlight level following operation region spec.
If that flag is not set:
- on hotkey press, adjust the brightness level without sending out
event;
- on brightness level change, use EC to change the brightness level.
The said commit sets the flag, so it seems with the flag set now,
it is possible the firmware interface will also give a screen off
result. But since we want to have notification event, we will have to
set that flag I think.
>
>>> backlight level from X, the screen blinks as if going 100%, 0%, and
>>> then the desired level.
>>
>> Please attach acpidump output to the to be opened bugzilla page, thanks.
>
> I looked at the code in the DCDT, it appears to me that they store
> different levels depending on the OSI version, so I don't think the
> problem is in the ACPI driver. Yet, the issue remains there.
It's good to know what is the problem, so I would like to see the table.
Most of the bugs I solved in ACPI video driver's backlight control are
caused by firmware, so yes, it's very unlikely a bug of the ACPI video
driver itself.
-Aaron
On Tue, Jul 30, 2013 at 07:11:06PM -0500, Felipe Contreras wrote:
> No, the ACPI driver is exposing a backlight interface, which has a
> defined stable API.
>From the ACPI spec:
'The OEM may define the number 0 as "Zero brightness" that can mean to
turn off the lighting (e.g. LCD panel backlight) in the device.'
There's no mechanism for an OS to know whether or not a firmware
implementation will actually turn the backlight off at 0, so there's no
way the OS can define the lowest backlight state as anything other than
"May or may not turn the screen off". If your userspace application
depends on specific numbers having specific meanings, your userspace
application is broken. Don't ascribe meanings to arbitrary values.
--
Matthew Garrett | [email protected]
On Wed, Jul 31, 2013 at 12:14 AM, Matthew Garrett <[email protected]> wrote:
> On Tue, Jul 30, 2013 at 07:11:06PM -0500, Felipe Contreras wrote:
>
>> No, the ACPI driver is exposing a backlight interface, which has a
>> defined stable API.
>
> From the ACPI spec:
>
> 'The OEM may define the number 0 as "Zero brightness" that can mean to
> turn off the lighting (e.g. LCD panel backlight) in the device.'
>
> There's no mechanism for an OS to know whether or not a firmware
> implementation will actually turn the backlight off at 0, so there's no
> way the OS can define the lowest backlight state as anything other than
> "May or may not turn the screen off".
Yes there is; quirks.
--
Felipe Contreras
On Wed, Jul 31, 2013 at 06:32:47AM -0500, Felipe Contreras wrote:
> On Wed, Jul 31, 2013 at 12:14 AM, Matthew Garrett <[email protected]> wrote:
> > There's no mechanism for an OS to know whether or not a firmware
> > implementation will actually turn the backlight off at 0, so there's no
> > way the OS can define the lowest backlight state as anything other than
> > "May or may not turn the screen off".
>
> Yes there is; quirks.
We aren't going to maintain a quirk list in order to support a guarantee
that was never made. "0" as a backlight level has potentially meant
"screen off" since the interface was first introduced.
--
Matthew Garrett | [email protected]
On Wed, Jul 31, 2013 at 9:00 AM, Matthew Garrett <[email protected]> wrote:
> On Wed, Jul 31, 2013 at 06:32:47AM -0500, Felipe Contreras wrote:
>> On Wed, Jul 31, 2013 at 12:14 AM, Matthew Garrett <[email protected]> wrote:
>> > There's no mechanism for an OS to know whether or not a firmware
>> > implementation will actually turn the backlight off at 0, so there's no
>> > way the OS can define the lowest backlight state as anything other than
>> > "May or may not turn the screen off".
>>
>> Yes there is; quirks.
>
> We aren't going to maintain a quirk list in order to support a guarantee
> that was never made. "0" as a backlight level has potentially meant
> "screen off" since the interface was first introduced.
That doesn't change the fact that you were wrong, and there *is*
actually a way. The fact that you don't want to go there doesn't mean
it's not there.
Here's another: device tree.
There are ways to provide a consistent backlight interface to user-space.
--
Felipe Contreras
On Wed, Jul 31, 2013 at 12:46:04PM -0500, Felipe Contreras wrote:
> That doesn't change the fact that you were wrong, and there *is*
> actually a way. The fact that you don't want to go there doesn't mean
> it's not there.
A quirk list will be incomplete, and as such there's no way to guarantee
whether or not a value of 0 will turn off the backlight. This is why the
interface doesn't make that guarantee, and why any userspace that
depends upon that behaviour is behaving incorrectly.
> Here's another: device tree.
There's no functional distinction between device tree and a quirk list
on x86 - they're both static data sources provided by something other
than the system firmware. As a result, they will both be incomplete.
> There are ways to provide a consistent backlight interface to user-space.
No, there aren't. What you *can* do is propose to change the ABI
description for the sysfs backlight interface such that any system where
0 turns off the backlight is considered buggy, but that won't make those
systems vanish. A good interface doesn't promise things it can't
guarantee, so I'd expect that any such proposal would be rejected.
--
Matthew Garrett | [email protected]
On Wed, Jul 31, 2013 at 12:52 PM, Matthew Garrett <[email protected]> wrote:
> On Wed, Jul 31, 2013 at 12:46:04PM -0500, Felipe Contreras wrote:
>
>> That doesn't change the fact that you were wrong, and there *is*
>> actually a way. The fact that you don't want to go there doesn't mean
>> it's not there.
>
> A quirk list will be incomplete, and as such there's no way to guarantee
> whether or not a value of 0 will turn off the backlight. This is why the
> interface doesn't make that guarantee, and why any userspace that
> depends upon that behaviour is behaving incorrectly.
There's no "guarantee" of anything. There is no "guarantee" that your
computer won't freeze when you boot Linux.
>> Here's another: device tree.
>
> There's no functional distinction between device tree and a quirk list
> on x86 - they're both static data sources provided by something other
> than the system firmware. As a result, they will both be incomplete.
So? If something can't be perfect that means we shouldn't even try?
If we can make the software behave consistently for 99% of the
machines out there instead of only 90%, that's better.
>> There are ways to provide a consistent backlight interface to user-space.
>
> No, there aren't.
Yes there are. Not perfectly, nothing is ever perfect, but there are ways.
--
Felipe Contreras
On Wed, Jul 31, 2013 at 01:07:57PM -0500, Felipe Contreras wrote:
> If we can make the software behave consistently for 99% of the
> machines out there instead of only 90%, that's better.
If we can't make an interface 100% consistent, we shouldn't pretend that
the interface is 100% consistent. We can't, and so we don't. Setting a
backlight value of 0 may turn the screen off, and userspace needs to
deal with that.
--
Matthew Garrett | [email protected]
On Wed, Jul 31, 2013 at 1:47 PM, Matthew Garrett <[email protected]> wrote:
> On Wed, Jul 31, 2013 at 01:07:57PM -0500, Felipe Contreras wrote:
>> If we can make the software behave consistently for 99% of the
>> machines out there instead of only 90%, that's better.
>
> If we can't make an interface 100% consistent, we shouldn't pretend that
> the interface is 100% consistent. We can't, and so we don't. Setting a
> backlight value of 0 may turn the screen off, and userspace needs to
> deal with that.
This is insanity; we can never guarantee 100% of anything.
Better is better. And 99% is better than 90%
% git grep quirks | wc -l
1585
Moreover, Linux already does quirks, and when there are quirks it
means there's no 100% guarantee of the thing working as it should;
hence the need for quirks, which is never complete, never 100%.
Anyway, screw the users, right? All you care about is that the code
looks good to you.
If we care about the users, we would provide a consistent interface,
where 0 means the same thing on all the backlight drivers. If all we
can do is provide this consistency 99% of the time through quirks,
that is the way to go.
--
Felipe Contreras
On Thu, Aug 01, 2013 at 12:37:04PM -0500, Felipe Contreras wrote:
> On Wed, Jul 31, 2013 at 1:47 PM, Matthew Garrett <[email protected]> wrote:
> > If we can't make an interface 100% consistent, we shouldn't pretend that
> > the interface is 100% consistent. We can't, and so we don't. Setting a
> > backlight value of 0 may turn the screen off, and userspace needs to
> > deal with that.
>
> This is insanity; we can never guarantee 100% of anything.
>
> Better is better. And 99% is better than 90%
An interface that describes reality is better than one that doesn't. But
hey, feel free to disagree and post a patch for the ABI docs.
--
Matthew Garrett | [email protected]
On Thu, Aug 1, 2013 at 12:42 PM, Matthew Garrett <[email protected]> wrote:
> On Thu, Aug 01, 2013 at 12:37:04PM -0500, Felipe Contreras wrote:
>> On Wed, Jul 31, 2013 at 1:47 PM, Matthew Garrett <[email protected]> wrote:
>> > If we can't make an interface 100% consistent, we shouldn't pretend that
>> > the interface is 100% consistent. We can't, and so we don't. Setting a
>> > backlight value of 0 may turn the screen off, and userspace needs to
>> > deal with that.
>>
>> This is insanity; we can never guarantee 100% of anything.
>>
>> Better is better. And 99% is better than 90%
>
> An interface that describes reality is better than one that doesn't. But
> hey, feel free to disagree and post a patch for the ABI docs.
An interface that is useful to the user is better than one that is not.
--
Felipe Contreras
On Thu, Aug 01, 2013 at 12:50:18PM -0500, Felipe Contreras wrote:
> On Thu, Aug 1, 2013 at 12:42 PM, Matthew Garrett <[email protected]> wrote:
> > An interface that describes reality is better than one that doesn't. But
> > hey, feel free to disagree and post a patch for the ABI docs.
>
> An interface that is useful to the user is better than one that is not.
The interface is useful. There are plenty of machines out there that
disable the backlight at minimum brightness setting (see every Apple,
for example), and assuming otherwise has always been incorrect. But,
like I said, send the patch.
--
Matthew Garrett | [email protected]
On Thu, Aug 1, 2013 at 1:01 PM, Matthew Garrett <[email protected]> wrote:
> On Thu, Aug 01, 2013 at 12:50:18PM -0500, Felipe Contreras wrote:
>> On Thu, Aug 1, 2013 at 12:42 PM, Matthew Garrett <[email protected]> wrote:
>> > An interface that describes reality is better than one that doesn't. But
>> > hey, feel free to disagree and post a patch for the ABI docs.
>>
>> An interface that is useful to the user is better than one that is not.
>
> The interface is useful.
Not 100% useful.
If you are going to claim that 99% true is not true, then useful but
inconsistent is not useful.
I think it is useful, and I think it can be consistent if we want it to.
> There are plenty of machines out there that
> disable the backlight at minimum brightness setting (see every Apple,
> for example), and assuming otherwise has always been incorrect. But,
> like I said, send the patch.
The key word in "every Apple" is *every*; what level 0 does in every
apple is consistent. If 0 should turn off the screen, it should do so
on all Linux machines.
I will investigate and prepare an update to the documentation and the
quirks to do so.
--
Felipe Contreras
On Tue, Jul 30, 2013 at 9:22 PM, Aaron Lu <[email protected]> wrote:
> On 07/31/2013 10:07 AM, Felipe Contreras wrote:
>> On Tue, Jul 30, 2013 at 8:36 PM, Aaron Lu <[email protected]> wrote:
>>> On 07/31/2013 08:11 AM, Felipe Contreras wrote:
>>>> On Tue, Jul 30, 2013 at 6:13 PM, Rafael J. Wysocki <[email protected]> wrote:
>>>>> Yes, that will break backlight on your system and *then* you can complain to
>>>>> Linus if you wish.
>>>>
>>>> It is already broken in v3.11-rc3, in fact I just booted that to try
>>>> it out and it booted with the screen completely black (fortunately I
>>>> knew exactly what to type to change that).
>>>
>>> That is bad, can you please file a bug for that? I'll need to take a
>>> look at your ACPI tables, thanks.
>>
>> File a bug where?
>
> https://bugzilla.kernel.org/
> For component, please choose ACPI->Power_Video.
Here you go:
https://bugzilla.kernel.org/show_bug.cgi?id=60677
>>>> Apparently this commit also needs to be reverted: efaa14c (ACPI /
>>>> video: no automatic brightness changes by win8-compatible firmware).
>>>> In this machine it makes the backlight work again (without
>>>> acpi_osi="!Windows 2012"), but by doing so the ACPI driver also turns
>>>> off the screen completely at level 0. Also, each time I change the
>>>
>>> So with rc3 and commit efaa14c reverted, when you set level 0 to ACPI
>>> video's backlight interface, the screen will be off now? And this is not
>>> the case in 3.10, right?
>>
>> No, setting level 0 turns it off if efaa14c is *not* reverted. In 3.10
>> 0 doesn't turn it off.
>
> AFAIK, Win8 firmware will check a flag to decide what to do on hotkey
> press and backlight brightness level change. Common behavior is:
> If that flag is set:
> - on hotkey press, send out event;
> - on brightness level change, use GPU driver's interface to change
> backlight level following operation region spec.
> If that flag is not set:
> - on hotkey press, adjust the brightness level without sending out
> event;
> - on brightness level change, use EC to change the brightness level.
>
> The said commit sets the flag, so it seems with the flag set now,
> it is possible the firmware interface will also give a screen off
> result. But since we want to have notification event, we will have to
> set that flag I think.
Probably, but changing the brightness should still work even without
the flag, right? It doesn't work here.
Setting the flag makes it work, but not properly.
>>> Please attach acpidump output to the to be opened bugzilla page, thanks.
>>
>> I looked at the code in the DCDT, it appears to me that they store
>> different levels depending on the OSI version, so I don't think the
>> problem is in the ACPI driver. Yet, the issue remains there.
>
> It's good to know what is the problem, so I would like to see the table.
> Most of the bugs I solved in ACPI video driver's backlight control are
> caused by firmware, so yes, it's very unlikely a bug of the ACPI video
> driver itself.
I hope you meant the output of this:
https://aur.archlinux.org/packages/acpidump/
--
Felipe Contreras
On Tue, Jul 30, 2013 at 7:11 PM, Felipe Contreras
<[email protected]> wrote:
> It is already broken in v3.11-rc3, in fact I just booted that to try
> it out and it booted with the screen completely black (fortunately I
> knew exactly what to type to change that).
>
> Also, each time I change the
> backlight level from X, the screen blinks as if going 100%, 0%, and
> then the desired level.
This one is a bug in the ACPI driver. Here's the fix:
http://article.gmane.org/gmane.linux.kernel/1536959
Now instead of booting with 0% (off), it boots with 100%, which is
still not ideal, but way better.
--
Felipe Contreras