Bug 60677 - Backlight regression in ASUS Zenbok Prime UX31A
Summary: Backlight regression in ASUS Zenbok Prime UX31A
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Video (show other bugs)
Hardware: x86-64 Linux
: P1 high
Assignee: Aaron Lu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-01 18:46 UTC by Felipe Contreras
Modified: 2013-09-05 05:55 UTC (History)
2 users (show)

See Also:
Kernel Version: 3.11-rc3
Subsystem:
Regression: No
Bisected commit-id:


Attachments
acpidump (430.33 KB, application/octet-stream)
2013-08-01 18:46 UTC, Felipe Contreras
Details

Description Felipe Contreras 2013-08-01 18:46:45 UTC
Created attachment 107075 [details]
acpidump

Commit efaa14c (ACPI / video: no automatic brightness changes by win8-compatible firmware) fixed the backlight controls when in win8 OSI mode, but introduced multiple issues.

1. The first boot afterwards happens with level 0 (the old level is not used)

2. Level 0 now turns the screen off, which didn't happen before (v3.6; the last time the backlight driver worked correctly)

3. Each time the backlight level is changed through X, a flash happens (level 100%, level 0%, and then the desired level). It works fine through sysfs though.
Comment 1 Felipe Contreras 2013-08-01 23:42:06 UTC
Here's a patch to fix #3.

http://article.gmane.org/gmane.linux.kernel/1536959

Now #1 is less of an issue, because it now boots with 100% instead of 0.
Comment 2 Aaron Lu 2013-08-02 00:49:03 UTC
(In reply to Felipe Contreras from comment #0)
> Created attachment 107075 [details]
> acpidump
> 
> Commit efaa14c (ACPI / video: no automatic brightness changes by
> win8-compatible firmware) fixed the backlight controls when in win8 OSI
> mode, but introduced multiple issues.

Are you using intel_backlight or acpi_video? From your comment #1, it seems you are using acpi_video interface. Does intel_backlight have any problem?

> 
> 1. The first boot afterwards happens with level 0 (the old level is not used)

The black screen happens when GUI starts?

> 
> 2. Level 0 now turns the screen off, which didn't happen before (v3.6; the
> last time the backlight driver worked correctly)

So we are talking about acpi_video interface, right?

> 
> 3. Each time the backlight level is changed through X, a flash happens
> (level 100%, level 0%, and then the desired level). It works fine through
> sysfs though.

I'll prepare a debug patch to see what happened, thanks.
Comment 3 Felipe Contreras 2013-08-02 04:30:31 UTC
(In reply to Aaron Lu from comment #2)
> (In reply to Felipe Contreras from comment #0)
> > Created attachment 107075 [details]
> > acpidump
> > 
> > Commit efaa14c (ACPI / video: no automatic brightness changes by
> > win8-compatible firmware) fixed the backlight controls when in win8 OSI
> > mode, but introduced multiple issues.
> 
> Are you using intel_backlight or acpi_video? From your comment #1, it seems
> you are using acpi_video interface.

Since this is 3.11-rc3; acpi_video.

>  Does intel_backlight have any problem?

#2
 
> > 1. The first boot afterwards happens with level 0 (the old level is not
> used)
> 
> The black screen happens when GUI starts?

It's always off, before and after GUI starts.

> > 3. Each time the backlight level is changed through X, a flash happens
> > (level 100%, level 0%, and then the desired level). It works fine through
> > sysfs though.
> 
> I'll prepare a debug patch to see what happened, thanks.

What happens is xbacklight tries to change the levels smoothly: 10, 11, 12... etc. But instead of starting from the actual level (e.g. 10%) it starts from the opposite (e.g. 90%).

I also noticed that 'xbacklight -get' cycled between the values:

% xbacklight -set 10
% xbacklight -get -> changes it to 90
% xbacklight -get -> changes it back to 10

The patch above fixes this issue.
Comment 4 Aaron Lu 2013-08-02 04:38:29 UTC
(In reply to Felipe Contreras from comment #3)
> (In reply to Aaron Lu from comment #2)
> > (In reply to Felipe Contreras from comment #0)
> > > Created attachment 107075 [details]
> > > acpidump
> > > 
> > > Commit efaa14c (ACPI / video: no automatic brightness changes by
> > > win8-compatible firmware) fixed the backlight controls when in win8 OSI
> > > mode, but introduced multiple issues.
> > 
> > Are you using intel_backlight or acpi_video? From your comment #1, it seems
> > you are using acpi_video interface.
> 
> Since this is 3.11-rc3; acpi_video.
> 
> >  Does intel_backlight have any problem?
> 
> #2

Please file a bug to Drivers/DRM-intel for this intel_backlight problem, if you insist this is a bug.
Comment 5 Aaron Lu 2013-08-03 14:34:17 UTC
With the patch you have proposed:
http://www.spinics.net/lists/linux-acpi/msg45157.html

Is there any problem left?
Comment 6 Felipe Contreras 2013-08-03 20:39:42 UTC
(In reply to Aaron Lu from comment #5)
> With the patch you have proposed:
> http://www.spinics.net/lists/linux-acpi/msg45157.html
> 
> Is there any problem left?

No. But _BQC would be disabled, which is still not ideal.

We need to figure out why the initial _BCMs fail. If there are other machines that fail _BCMs win win8 mode, the indexed _BQC would not get detected, so they would experience exactly the same problem I did.
Comment 7 Aaron Lu 2013-08-05 01:35:40 UTC
(In reply to Felipe Contreras from comment #6)
> (In reply to Aaron Lu from comment #5)
> > With the patch you have proposed:
> > http://www.spinics.net/lists/linux-acpi/msg45157.html
> > 
> > Is there any problem left?
> 
> No. But _BQC would be disabled, which is still not ideal.

So you think _BQC here is working and it's _BCM that failed. Can you please try to set maximum level through _BCM, see if the screen goes to the brightest level? Then set the lowest level through _BCM, does the screen goes to the darkest level? These can be done through acpi_video's sysfs interface.

> 
> We need to figure out why the initial _BCMs fail. If there are other
> machines that fail _BCMs win win8 mode, the indexed _BQC would not get
> detected, so they would experience exactly the same problem I did.

If it is _BCM that failed instead of _BQC, then I agree.
Comment 8 Felipe Contreras 2013-08-05 01:45:31 UTC
(In reply to Aaron Lu from comment #7)
> (In reply to Felipe Contreras from comment #6)
> > (In reply to Aaron Lu from comment #5)
> > > With the patch you have proposed:
> > > http://www.spinics.net/lists/linux-acpi/msg45157.html
> > > 
> > > Is there any problem left?
> > 
> > No. But _BQC would be disabled, which is still not ideal.
> 
> So you think _BQC here is working and it's _BCM that failed. Can you please
> try to set maximum level through _BCM, see if the screen goes to the
> brightest level? Then set the lowest level through _BCM, does the screen
> goes to the darkest level? These can be done through acpi_video's sysfs
> interface.

And how would I do that before acpi_video_bus_start_devices()? That's what I mean by initial _BCMs.
Comment 9 Aaron Lu 2013-08-22 07:51:44 UTC
I suppose problem #1 and #3 are now solved, right?
Comment 10 Felipe Contreras 2013-08-22 14:37:10 UTC
(In reply to Aaron Lu from comment #9)
> I suppose problem #1 and #3 are now solved, right?

Yes, but there's a 1.1: instead of booting with 0%, it boots with 100%.
Comment 11 Aaron Lu 2013-08-23 00:42:16 UTC
(In reply to Felipe Contreras from comment #10)
> (In reply to Aaron Lu from comment #9)
> > I suppose problem #1 and #3 are now solved, right?
> 
> Yes, but there's a 1.1: instead of booting with 0%, it boots with 100%.

Do you mean it should boot with brightness level 0, i.e. dark screen?

The current logic of video module is, if _BQC is broken, it will set the brightness level to maximum.
Comment 12 Felipe Contreras 2013-08-23 01:59:32 UTC
(In reply to Aaron Lu from comment #11)
> (In reply to Felipe Contreras from comment #10)
> > (In reply to Aaron Lu from comment #9)
> > > I suppose problem #1 and #3 are now solved, right?
> > 
> > Yes, but there's a 1.1: instead of booting with 0%, it boots with 100%.
> 
> Do you mean it should boot with brightness level 0, i.e. dark screen?

It should boot with the previous brightness level.

> The current logic of video module is, if _BQC is broken, it will set the
> brightness level to maximum.

_BQC is not broken.
Comment 13 Aaron Lu 2013-08-23 02:09:58 UTC
Can you please show me the reason why _BQC is not broken?
Comment 14 Aaron Lu 2013-08-23 02:23:09 UTC
BTW, I wrote something about the problem of _BQC the other day:
https://gist.github.com/aaronlu/6314920
It would be good if you can take a look and let me know what do you think.
Comment 15 Felipe Contreras 2013-08-23 03:26:42 UTC
(In reply to Aaron Lu from comment #13)
> Can you please show me the reason why _BQC is not broken?

--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -531,6 +531,8 @@ acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
                status = acpi_evaluate_integer(device->dev->handle, buf,
                                                NULL, level);
                if (ACPI_SUCCESS(status)) {
+                       pr_info("acpi: video: test: %s %llu", buf, *level);
+
                        if (raw) {
                                /*
                                 * Caller has indicated he wants the raw
@@ -707,9 +709,6 @@ static int acpi_video_bqc_quirk(struct acpi_video_device *device,
                        if (br->levels[level + 2] == test_level)
                                br->flags._BQC_use_index = 1;
                }
-
-               if (!br->flags._BQC_use_index)
-                       device->cap._BQC = device->cap._BCQ = 0;
        }
 
        return 0;

% xbacklight -set 40
% xbacklight -get
% xbacklight -set 30
% xbacklight -get
% xbacklight -set 20
% xbacklight -get

acpi: video: test: _BQC 100
acpi: video: test: _BQC 100

acpi: video: test: _BQC 40
acpi: video: test: _BQC 40

acpi: video: test: _BQC 29
acpi: video: test: _BQC 28

acpi: video: test: _BQC 20
acpi: video: test: _BQC 20

_BQC works.
Comment 16 Felipe Contreras 2013-08-23 03:31:55 UTC
(In reply to Aaron Lu from comment #14)
> BTW, I wrote something about the problem of _BQC the other day:
> https://gist.github.com/aaronlu/6314920
> It would be good if you can take a look and let me know what do you think.

Yeah, the same happens in this machine, but the solution is simple: acpi_osi="!Windows 2012".
Comment 17 Aaron Lu 2013-08-23 05:35:24 UTC
(In reply to Felipe Contreras from comment #15)
> (In reply to Aaron Lu from comment #13)
> > Can you please show me the reason why _BQC is not broken?
> 
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -531,6 +531,8 @@ acpi_video_device_lcd_get_level_current(struct
> acpi_video_device *device,
>                 status = acpi_evaluate_integer(device->dev->handle, buf,
>                                                 NULL, level);
>                 if (ACPI_SUCCESS(status)) {
> +                       pr_info("acpi: video: test: %s %llu", buf, *level);
> +
>                         if (raw) {
>                                 /*
>                                  * Caller has indicated he wants the raw
> @@ -707,9 +709,6 @@ static int acpi_video_bqc_quirk(struct acpi_video_device
> *device,
>                         if (br->levels[level + 2] == test_level)
>                                 br->flags._BQC_use_index = 1;
>                 }
> -
> -               if (!br->flags._BQC_use_index)
> -                       device->cap._BQC = device->cap._BCQ = 0;
>         }
>  
>         return 0;
> 
> % xbacklight -set 40
> % xbacklight -get
> % xbacklight -set 30
> % xbacklight -get
> % xbacklight -set 20
> % xbacklight -get
> 
> acpi: video: test: _BQC 100
> acpi: video: test: _BQC 100
> 
> acpi: video: test: _BQC 40
> acpi: video: test: _BQC 40
> 
> acpi: video: test: _BQC 29
> acpi: video: test: _BQC 28
> 
> acpi: video: test: _BQC 20
> acpi: video: test: _BQC 20
> 
> _BQC works.

It doesn't for all values.

(In reply to Felipe Contreras from comment #16)
> (In reply to Aaron Lu from comment #14)
> > BTW, I wrote something about the problem of _BQC the other day:
> > https://gist.github.com/aaronlu/6314920
> > It would be good if you can take a look and let me know what do you think.
> 
> Yeah, the same happens in this machine, but the solution is simple:
> acpi_osi="!Windows 2012".

Your patch:
commit cb7a386c6c25e85c2710cdb1a498a73794cda660
Author: Felipe Contreras <felipe.contreras@gmail.com>
Date:   Mon Jul 29 14:20:58 2013 -0500

    ACPI: blacklist win8 OSI for ASUS Zenbook Prime UX31A

has entered Rafael's linux-next tree.
Comment 18 Felipe Contreras 2013-08-23 06:18:51 UTC
(In reply to Aaron Lu from comment #17)

> > _BQC works.
> 
> It doesn't for all values.

Yes it does. The fact that it's imprecise doesn't mean it doesn't work.

> (In reply to Felipe Contreras from comment #16)
> > (In reply to Aaron Lu from comment #14)
> > > BTW, I wrote something about the problem of _BQC the other day:
> > > https://gist.github.com/aaronlu/6314920
> > > It would be good if you can take a look and let me know what do you
> think.
> > 
> > Yeah, the same happens in this machine, but the solution is simple:
> > acpi_osi="!Windows 2012".
> 
> Your patch:
> commit cb7a386c6c25e85c2710cdb1a498a73794cda660
> Author: Felipe Contreras <felipe.contreras@gmail.com>
> Date:   Mon Jul 29 14:20:58 2013 -0500
> 
>     ACPI: blacklist win8 OSI for ASUS Zenbook Prime UX31A
> 
> has entered Rafael's linux-next tree.

That's good, now it needs to land in the mainline, and we need to do the same for other laptops.
Comment 19 Aaron Lu 2013-09-05 05:55:02 UTC
Patch:
commit cb7a386c6c25e85c2710cdb1a498a73794cda660
Author: Felipe Contreras <felipe.contreras@gmail.com>
Date:   Mon Jul 29 14:20:58 2013 -0500

    ACPI: blacklist win8 OSI for ASUS Zenbook Prime UX31A

has entered Linus' tree.

Note You need to log in before you can comment on or make changes to this bug.