Bug 75001 - [945gm] ACPI Brightness adjustment is broken in 945GM with kernel 3.14-rc2
Summary: [945gm] ACPI Brightness adjustment is broken in 945GM with kernel 3.14-rc2
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: XOrg git
Hardware: x86 (IA32) Linux (All)
: medium normal
Assignee: Jani Nikula
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-14 19:31 UTC by Luis Ortega
Modified: 2017-07-24 22:55 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
dmesg output 3.14-rc2 with drm.debug (78.31 KB, text/plain)
2014-02-14 19:31 UTC, Luis Ortega
no flags Details
Bundle of attachments (30.10 KB, application/zip)
2014-02-14 19:39 UTC, Luis Ortega
no flags Details
drm/i915: use legacy combination mode also for i945gm (1.43 KB, patch)
2014-02-24 15:30 UTC, Jani Nikula
no flags Details | Splinter Review

Description Luis Ortega 2014-02-14 19:31:41 UTC
Created attachment 94098 [details]
dmesg output 3.14-rc2 with drm.debug

First reported in the Linux kernel mailing list.

Laptop Model: Asus EeePC 1000H
Graphics: Intel 945GM

Testing the kernel version 3.14-rc2 I noticed adjusting the screen brightness didn't work. The problem is already present in 3.14-rc1 whereas the 3.13 branch works fine.

I first bisected the issue down to the following commit:
bc0bb9fd1c7810407ab810d204bbaecb255fddde drm/i915: remove QUIRK_NO_PCH_PWM_ENABLE

However, given the concern that the bisection might be bogus I replayed it up to the last 6 bisect steps. The issue was present in this bisect session and this time I tracked it down to the commit:
b35684b8fa94e04f55fd38bf672b737741d2f9e2 drm/i915: do full backlight setup at enable time

Hopefully this makes more sense.

Per request of the developers I've tested changing the brightness manually through the sysfs. Here are the results.

ls -l /sys/class/backlight:
lrwxrwxrwx 1 root root 0 Feb 14 21:00 eeepc -> ../../devices/platform/eeepc/backlight/eeepc
lrwxrwxrwx 1 root root 0 Feb 14 20:59 intel_backlight -> ../../devices/pci0000:00/0000:00:02.0/drm/card0/card0-LVDS-1/intel_backlight


Linux 3.13.3: change the brightness manually:
Changing /sys/class/backlight/eeepc/brightness changes brightness and updates the value.
changing /sys/class/backlight/intel_backlight/brightness changes brightness but does NOT update the value. Always reports 156.

Linux 3.14-rc2: change the brightness manually:
changing /sys/class/backlight/eeepc/brightness does NOT change brightness but updates the value.
changing /sys/class/backlight/intel_backlight/brightness changes brightness and updates the value.


Finally I attach the following files:
dmesg.log
Comment 1 Luis Ortega 2014-02-14 19:39:33 UTC
Created attachment 94099 [details]
Bundle of attachments

Bundle of the following files:

dmesg.log: dmesg output in 3.14-rc2 with drm.debug=0xe
lspci.txt: output of lspci -nn
Xorg.0.log: last session in 3.14-rc2 with drm.debug=0xe
reg_dump_3.13_good.log: output of intel_reg_dump in 3.13.3
reg_dump_3.14_bad.log: output of intel_reg_dumo in 3.14-rc2

PS: Crappy Bugzilla is crappy.
Comment 2 Chris Wilson 2014-02-14 19:43:50 UTC
good (3.13):
BLC_PWM_CTL:  0x01390138
BLC_PWM_CTL2: 0x000000

bad (3.14):
BLC_PWM_CTL:  0x013800c8
BLC_PWM_CTL2: 0x00000000
Comment 3 Chris Wilson 2014-02-14 19:50:14 UTC
Notice the legacy backlight combination bit set - that seems to be being used by the ACPI firmware to control the backlight.
Comment 4 Jani Nikula 2014-02-24 07:31:00 UTC
Thanks for the report, the bisect result makes more sense now. And thanks Chris for digging out the crucial bits of info for me! :)
Comment 5 Jani Nikula 2014-02-24 15:27:37 UTC
(In reply to comment #3)
> Notice the legacy backlight combination bit set - that seems to be being
> used by the ACPI firmware to control the backlight.

My scarce gen3 specs do not say anything about bit 16 of BLC_PWM_CTL. Bits 31:17 are similar to gen2, which does have bit 16 specified as legacy mode.

Clearly we should interpret bit 16 as legacy mode also for I945GM, but how about other gen3 hardware? *scratches head*
Comment 6 Jani Nikula 2014-02-24 15:30:48 UTC
Created attachment 94662 [details] [review]
drm/i915: use legacy combination mode also for i945gm

In the mean time, please try the attached patch. Thanks.
Comment 7 Luis Ortega 2014-02-24 22:52:18 UTC
This patch corrects the issue. The Fn keys work as expected and I can again change the brightness. In more detail:

su -c "echo N > /sys/class/backlight/eeepc/brightness" changes the brightness and catting the file reports the new value.

su -c "echo N > /sys/class/backlight/intel_backlight/brightness" changes the brightness and catting the file reports the new value.

BTW, I've noticed that the values reported by /sys/class/backlight/intel_backlight/brightness are much higher than previously. Booting in 3.13 and 3.14-rc2 I get the value 156, which equals that of max_brightness though I actually have my screen dimmed. With your patch I get the value 7800 and a max_brightness of 39780.
Comment 8 Jani Nikula 2014-02-25 08:46:38 UTC
(In reply to comment #7)
> This patch corrects the issue. The Fn keys work as expected and I can again
> change the brightness. In more detail:
> 
> su -c "echo N > /sys/class/backlight/eeepc/brightness" changes the
> brightness and catting the file reports the new value.
> 
> su -c "echo N > /sys/class/backlight/intel_backlight/brightness" changes the
> brightness and catting the file reports the new value.
> 
> BTW, I've noticed that the values reported by
> /sys/class/backlight/intel_backlight/brightness are much higher than
> previously. Booting in 3.13 and 3.14-rc2 I get the value 156, which equals
> that of max_brightness though I actually have my screen dimmed. With your
> patch I get the value 7800 and a max_brightness of 39780.

Hmm, I can't recall us intentionally fixing that, but apparently we have. :)

Thanks for testing.
Comment 9 Jani Nikula 2014-02-27 11:01:00 UTC
Pushed to -fixes. Thanks for the report.

commit dfa89d2d2d0df5d72e40c12af8a2df46887e5388
Author: Jani Nikula <jani.nikula@intel.com>
Date:   Tue Feb 25 13:11:47 2014 +0200

    drm/i915: use backlight legacy combination mode also for i915gm/i945gm


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.