Hi,
it seems a lot of people are complaining about the backlight pull
request of RC2.
This is to tell you how it saved my life:
- Changing brightness from Fn keys does work
- Writing to /sys/class/backlight/intel_backlight/brightness does work
- Changing brightness from gnome brightness settings does work
--> It seems everything is now working (it wasn't before).
--> I appreciate your patch and I am going to be sad if you revert it :-)
My laptop : Asus N56VZ
My /proc/cpuinfo:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1030556/+attachment/3241093/+files/ProcCpuinfo.txt
My dmidecode is below.
Changing brightness from Fn keys didn't work on 3.9 and 3.10 kernels.
I a not sure for previous versions though a patch was made for 3.2 in
ubuntu. See details here:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1030556
There are a lot of bugs opened for this:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1173107
http://askubuntu.com/questions/156708/how-to-get-multimedia-keys-working-at-my-asus-n56vz-ubuntu-12-04-notebook
https://bugs.archlinux.org/task/35320
Regards
Sam
# dmidecode 2.11
# SMBIOS entry point at 0x000f04c0
SMBIOS 2.7 present.
23 structures occupying 1708 bytes.
Table at 0x000EBA70.
Handle 0x0000, DMI type 0, 24 bytes
BIOS Information
Vendor: American Megatrends Inc.
Version: N56VZ.215
Release Date: 11/02/2012
Address: 0xF0000
Runtime Size: 64 kB
ROM Size: 6144 kB
Characteristics:
PCI is supported
BIOS is upgradeable
BIOS shadowing is allowed
Boot from CD is supported
Selectable boot is supported
BIOS ROM is socketed
EDD is supported
5.25"/1.2 MB floppy services are supported (int 13h)
3.5"/720 kB floppy services are supported (int 13h)
3.5"/2.88 MB floppy services are supported (int 13h)
Print screen service is supported (int 5h)
8042 keyboard services are supported (int 9h)
Serial services are supported (int 14h)
Printer services are supported (int 17h)
ACPI is supported
USB legacy is supported
Smart battery is supported
BIOS boot specification is supported
Targeted content distribution is supported
UEFI is supported
BIOS Revision: 4.6
Handle 0x0001, DMI type 1, 27 bytes
System Information
Manufacturer: ASUSTeK COMPUTER INC.
Product Name: N56VZ
Version: 1.0
Serial Number: C6N0AS306834244
UUID: 10017880-B5B7-81E1-3FDF-3085A9061B75
Wake-up Type: Power Switch
SKU Number: ASUS-NotebookSKU
Family: N
Handle 0x0002, DMI type 2, 15 bytes
Base Board Information
Manufacturer: ASUSTeK COMPUTER INC.
Product Name: N56VZ
Version: 1.0
Serial Number: BSN12345678901234567
Asset Tag: ATN12345678901234567
Features:
Board is a hosting board
Board is replaceable
Location In Chassis: MIDDLE
Chassis Handle: 0x0003
Type: Motherboard
Contained Object Handles: 0
Handle 0x0003, DMI type 3, 22 bytes
Chassis Information
Manufacturer: ASUSTeK COMPUTER INC.
Type: Notebook
Lock: Not Present
Version: 1.0
Serial Number: C6N0AS306834244
Asset Tag: No Asset Tag
Boot-up State: Safe
Power Supply State: Safe
Thermal State: Safe
Security Status: None
OEM Information: 0x00000000
Height: Unspecified
Number Of Power Cords: 1
Contained Elements: 0
SKU Number: To be filled by O.E.M.
Handle 0x0004, DMI type 10, 26 bytes
On Board Device 1 Information
Type: Video
Status: Enabled
Description: VGA
On Board Device 2 Information
Type: Ethernet
Status: Enabled
Description: GLAN
On Board Device 3 Information
Type: Ethernet
Status: Enabled
Description: WLAN
On Board Device 4 Information
Type: Sound
Status: Enabled
Description: Audio CODEC
On Board Device 5 Information
Type: SATA Controller
Status: Enabled
Description: SATA Controller
On Board Device 6 Information
Type: Other
Status: Enabled
Description: USB 2.0 Controller
On Board Device 7 Information
Type: Other
Status: Enabled
Description: USB 3.0 Controller
On Board Device 8 Information
Type: Other
Status: Enabled
Description: SMBus Controller
On Board Device 9 Information
Type: Other
Status: Enabled
Description: Card Reader
On Board Device 10 Information
Type: Other
Status: Enabled
Description: Cmos Camera
On Board Device 11 Information
Type: Other
Status: Enabled
Description: Bluetooth
Handle 0x0005, DMI type 11, 5 bytes
OEM Strings
String 1: 6kfb9jsNVeN50
String 2: I76yifUckyZ2n
String 3: W0-kvTpaXZUpn
String 4: 90N9IC442L1875VL355Y
String 5:
String 6:
String 7:
String 8:
String 9:
String 10:
Handle 0x0006, DMI type 12, 5 bytes
System Configuration Options
Option 1: DSN: W54S4QJE
Option 2: DSN:57B1609A5803
Option 3: DSN:3085A9061B75
Option 4: SMI:00B2CA
Handle 0x0007, DMI type 32, 20 bytes
System Boot Information
Status: No errors detected
Handle 0x0008, DMI type 7, 19 bytes
Cache Information
Socket Designation: CPU Internal L2
Configuration: Enabled, Not Socketed, Level 2
Operational Mode: Write Through
Location: Internal
Installed Size: 1024 kB
Maximum Size: 1024 kB
Supported SRAM Types:
Unknown
Installed SRAM Type: Unknown
Speed: Unknown
Error Correction Type: Multi-bit ECC
System Type: Unified
Associativity: 8-way Set-associative
Handle 0x0009, DMI type 7, 19 bytes
Cache Information
Socket Designation: CPU Internal L1
Configuration: Enabled, Not Socketed, Level 1
Operational Mode: Write Through
Location: Internal
Installed Size: 256 kB
Maximum Size: 256 kB
Supported SRAM Types:
Unknown
Installed SRAM Type: Unknown
Speed: Unknown
Error Correction Type: Parity
System Type: Data
Associativity: 8-way Set-associative
Handle 0x000A, DMI type 7, 19 bytes
Cache Information
Socket Designation: CPU Internal L3
Configuration: Enabled, Not Socketed, Level 3
Operational Mode: Write Back
Location: Internal
Installed Size: 6144 kB
Maximum Size: 6144 kB
Supported SRAM Types:
Unknown
Installed SRAM Type: Unknown
Speed: Unknown
Error Correction Type: Multi-bit ECC
System Type: Unified
Associativity: 12-way Set-associative
Handle 0x000B, DMI type 16, 23 bytes
Physical Memory Array
Location: System Board Or Motherboard
Use: System Memory
Error Correction Type: None
Maximum Capacity: 32 GB
Error Information Handle: Not Provided
Number Of Devices: 4
Handle 0x000C, DMI type 4, 42 bytes
Processor Information
Socket Designation: SOCKET 0
Type: Central Processor
Family: Core i7
Manufacturer: Intel(R) Corporation
ID: A9 06 03 00 FF FB EB BF
Signature: Type 0, Family 6, Model 58, Stepping 9
Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)
PSE (Page size extension)
TSC (Time stamp counter)
MSR (Model specific registers)
PAE (Physical address extension)
MCE (Machine check exception)
CX8 (CMPXCHG8 instruction supported)
APIC (On-chip APIC hardware supported)
SEP (Fast system call)
MTRR (Memory type range registers)
PGE (Page global enable)
MCA (Machine check architecture)
CMOV (Conditional move instruction supported)
PAT (Page attribute table)
PSE-36 (36-bit page size extension)
CLFSH (CLFLUSH instruction supported)
DS (Debug store)
ACPI (ACPI supported)
MMX (MMX technology supported)
FXSR (FXSAVE and FXSTOR instructions supported)
SSE (Streaming SIMD extensions)
SSE2 (Streaming SIMD extensions 2)
SS (Self-snoop)
HTT (Multi-threading)
TM (Thermal monitor supported)
PBE (Pending break enabled)
Version: Intel(R) Core(TM) i7-3610QM CPU @ 2.30GHz
Voltage: 2.9 V
External Clock: 100 MHz
Max Speed: 3800 MHz
Current Speed: 2300 MHz
Status: Populated, Enabled
Upgrade: Socket rPGA988B
L1 Cache Handle: 0x0009
L2 Cache Handle: 0x0008
L3 Cache Handle: 0x000A
Serial Number: Not Specified
Asset Tag: Fill By OEM
Part Number: Fill By OEM
Core Count: 4
Core Enabled: 4
Thread Count: 8
Characteristics:
64-bit capable
Handle 0x000D, DMI type 17, 34 bytes
Memory Device
Array Handle: 0x000B
Error Information Handle: Not Provided
Total Width: 64 bits
Data Width: 64 bits
Size: 2048 MB
Form Factor: SODIMM
Set: None
Locator: ChannelA-DIMM0
Bank Locator: BANK 0
Type: DDR3
Type Detail: Synchronous
Speed: 1600 MHz
Manufacturer: Hynix/Hyundai
Serial Number: 196F857E
Asset Tag: 9876543210
Part Number: HMT325S6CFR8C-PB
Rank: 1
Configured Clock Speed: 1600 MHz
Handle 0x000E, DMI type 20, 35 bytes
Memory Device Mapped Address
Starting Address: 0x00000000000
Ending Address: 0x0007FFFFFFF
Range Size: 2 GB
Physical Device Handle: 0x000D
Memory Array Mapped Address Handle: 0x0013
Partition Row Position: Unknown
Interleave Position: 1
Interleaved Data Depth: 2
Handle 0x000F, DMI type 17, 34 bytes
Memory Device
Array Handle: 0x000B
Error Information Handle: Not Provided
Total Width: Unknown
Data Width: Unknown
Size: No Module Installed
Form Factor: DIMM
Set: None
Locator: ChannelA-DIMM1
Bank Locator: BANK 1
Type: Unknown
Type Detail: None
Speed: Unknown
Manufacturer: [Empty]
Serial Number: [Empty]
Asset Tag: 9876543210
Part Number: [Empty]
Rank: Unknown
Configured Clock Speed: Unknown
Handle 0x0010, DMI type 17, 34 bytes
Memory Device
Array Handle: 0x000B
Error Information Handle: Not Provided
Total Width: 64 bits
Data Width: 64 bits
Size: 4096 MB
Form Factor: SODIMM
Set: None
Locator: ChannelB-DIMM0
Bank Locator: BANK 2
Type: DDR3
Type Detail: Synchronous
Speed: 1600 MHz
Manufacturer: Hynix/Hyundai
Serial Number: 0D649EC1
Asset Tag: 9876543210
Part Number: HMT351S6CFR8C-PB
Rank: 2
Configured Clock Speed: 1600 MHz
Handle 0x0011, DMI type 20, 35 bytes
Memory Device Mapped Address
Starting Address: 0x00080000000
Ending Address: 0x0017FFFFFFF
Range Size: 4 GB
Physical Device Handle: 0x0010
Memory Array Mapped Address Handle: 0x0013
Partition Row Position: Unknown
Interleave Position: 2
Interleaved Data Depth: 2
Handle 0x0012, DMI type 17, 34 bytes
Memory Device
Array Handle: 0x000B
Error Information Handle: Not Provided
Total Width: Unknown
Data Width: Unknown
Size: No Module Installed
Form Factor: DIMM
Set: None
Locator: ChannelB-DIMM1
Bank Locator: BANK 3
Type: Unknown
Type Detail: None
Speed: Unknown
Manufacturer: [Empty]
Serial Number: [Empty]
Asset Tag: 9876543210
Part Number: [Empty]
Rank: Unknown
Configured Clock Speed: Unknown
Handle 0x0013, DMI type 19, 31 bytes
Memory Array Mapped Address
Starting Address: 0x00000000000
Ending Address: 0x0017FFFFFFF
Range Size: 6 GB
Physical Array Handle: 0x000B
Partition Width: 4
Handle 0x0018, DMI type 131, 64 bytes
OEM-specific Type
Header and Data:
83 40 18 00 31 00 00 00 00 00 00 00 00 00 00 00
F8 00 59 1E 00 00 00 00 01 20 00 00 00 00 08 00
B8 05 0A 00 00 00 00 00 C8 00 FF FF 00 00 00 00
00 00 00 00 66 00 00 00 76 50 72 6F 00 00 00 00
Handle 0x0019, DMI type 13, 22 bytes
BIOS Language Information
Language Description Format: Long
Installable Languages: 1
en|US|iso8859-1
Currently Installed Language: en|US|iso8859-1
Handle 0x001A, DMI type 127, 4 bytes
End Of Table
On Wednesday, July 24, 2013 08:43:00 PM * SAMÍ * wrote:
> Hi,
>
> it seems a lot of people are complaining about the backlight pull
> request of RC2.
>
> This is to tell you how it saved my life:
> - Changing brightness from Fn keys does work
> - Writing to /sys/class/backlight/intel_backlight/brightness does work
> - Changing brightness from gnome brightness settings does work
> --> It seems everything is now working (it wasn't before).
> --> I appreciate your patch and I am going to be sad if you revert it :-)
Well, there seems to be a number of people who'll be equally unhappy if I don't
do that. Unfortunately for you, things work for them without that patch.
The source of the problem seems to be that the i915's backlight support doesn't
work for all those people, although it should work anyway in principle. So to
me the way to go seems to be to revert the patch for now, fix the i915
backlight interface for them and try to apply it again when we're more
confident about the i915 backlight.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Wed, Jul 24, 2013 at 12:39 PM, Rafael J. Wysocki <[email protected]> wrote:
>
> Well, there seems to be a number of people who'll be equally unhappy if I don't
> do that. Unfortunately for you, things work for them without that patch.
So I think we have to revert the behavior back, but I wonder if we
could keep the logic and have a i915 kernel command line option to
enable it.
We did that for a long time with "drm.modeset" and the
"i915.enable_rc6" mess. We still have the "i915.invert_brightness"
thing, although I don't know who actually uses it. But that option is
itself an indication that the i915 driver still has some backlight
issues.
But if that kind of "revert behavior" isn't easy, we need to just
revert the patches entirely.
Linus
On Wed, Jul 24, 2013 at 10:39 PM, Linus Torvalds <[email protected]> wrote:
> On Wed, Jul 24, 2013 at 12:39 PM, Rafael J. Wysocki <[email protected]> wrote:
>>
>> Well, there seems to be a number of people who'll be equally unhappy if I don't
>> do that. Unfortunately for you, things work for them without that patch.
>
> So I think we have to revert the behavior back, but I wonder if we
> could keep the logic and have a i915 kernel command line option to
> enable it.
>
> We did that for a long time with "drm.modeset" and the
> "i915.enable_rc6" mess. We still have the "i915.invert_brightness"
> thing, although I don't know who actually uses it. But that option is
> itself an indication that the i915 driver still has some backlight
> issues.
invert_brighntess seems to be real, at least we have a pile of quirk
entries for Acer and Packard Bell machines. Generally I'm not too happy
with module options since they tend to get stuck eternally in a limbo
state where everyone in the know sets them to their liking and no one
bothers to fix the underlying.
Otoh this is the backlight and I've essentially given up and opted for
quirk entries :(
> But if that kind of "revert behavior" isn't easy, we need to just
> revert the patches entirely.
I think a i915 module option should be doable, otoh people seem to have a
viable workaround by setting a different acpi os version already.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Wed, Jul 24, 2013 at 2:02 PM, Daniel Vetter <[email protected]> wrote:
>
> I think a i915 module option should be doable, otoh people seem to have a
> viable workaround by setting a different acpi os version already.
At least the original claim was that if you set a non-windows8 acpi os
version, you hit other bugs..
But yeah, if we just do a plain revert, that may be the only option
for the people for whom the current (to-be-reverted) patches made
things work.
Linus
On Wednesday, July 24, 2013 02:05:15 PM Linus Torvalds wrote:
> On Wed, Jul 24, 2013 at 2:02 PM, Daniel Vetter <[email protected]> wrote:
> >
> > I think a i915 module option should be doable, otoh people seem to have a
> > viable workaround by setting a different acpi os version already.
>
> At least the original claim was that if you set a non-windows8 acpi os
> version, you hit other bugs..
>
> But yeah, if we just do a plain revert, that may be the only option
> for the people for whom the current (to-be-reverted) patches made
> things work.
Well, I wonder what about the appended (untested) patch?
It should restore the previous behavior while leaving an option to use the
new stuff if need be.
Rafael
---
drivers/gpu/drm/i915/i915_dma.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 5 +++++
drivers/gpu/drm/i915/i915_drv.h | 1 +
include/acpi/video.h | 16 ++++++----------
4 files changed, 13 insertions(+), 11 deletions(-)
Index: linux-pm/drivers/gpu/drm/i915/i915_dma.c
===================================================================
--- linux-pm.orig/drivers/gpu/drm/i915/i915_dma.c
+++ linux-pm/drivers/gpu/drm/i915/i915_dma.c
@@ -1648,7 +1648,7 @@ int i915_driver_load(struct drm_device *
if (INTEL_INFO(dev)->num_pipes) {
/* Must be done after probing outputs */
intel_opregion_init(dev);
- acpi_video_register_with_quirks();
+ __acpi_video_register(i915_take_over_backlight);
}
if (IS_GEN5(dev))
Index: linux-pm/drivers/gpu/drm/i915/i915_drv.c
===================================================================
--- linux-pm.orig/drivers/gpu/drm/i915/i915_drv.c
+++ linux-pm/drivers/gpu/drm/i915/i915_drv.c
@@ -132,6 +132,11 @@ int i915_enable_ips __read_mostly = 1;
module_param_named(enable_ips, i915_enable_ips, int, 0600);
MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
+bool i915_take_over_backlight __read_mostly = false;
+module_param_named(take_over_backlight, i915_take_over_backlight, bool, 0644);
+MODULE_PARM_DESC(take_over_backlight,
+ "Prevent ACPI backlight from being used (default: false)");
+
static struct drm_driver driver;
extern int intel_agp_enabled;
Index: linux-pm/drivers/gpu/drm/i915/i915_drv.h
===================================================================
--- linux-pm.orig/drivers/gpu/drm/i915/i915_drv.h
+++ linux-pm/drivers/gpu/drm/i915/i915_drv.h
@@ -1542,6 +1542,7 @@ extern int i915_enable_ppgtt __read_most
extern unsigned int i915_preliminary_hw_support __read_mostly;
extern int i915_disable_power_well __read_mostly;
extern int i915_enable_ips __read_mostly;
+extern bool i915_take_over_backlight __read_mostly;
extern int i915_suspend(struct drm_device *dev, pm_message_t state);
extern int i915_resume(struct drm_device *dev);
Index: linux-pm/include/acpi/video.h
===================================================================
--- linux-pm.orig/include/acpi/video.h
+++ linux-pm/include/acpi/video.h
@@ -18,20 +18,11 @@ struct acpi_device;
#if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE)
extern int __acpi_video_register(bool backlight_quirks);
-static inline int acpi_video_register(void)
-{
- return __acpi_video_register(false);
-}
-static inline int acpi_video_register_with_quirks(void)
-{
- return __acpi_video_register(true);
-}
extern void acpi_video_unregister(void);
extern int acpi_video_get_edid(struct acpi_device *device, int type,
int device_id, void **edid);
#else
-static inline int acpi_video_register(void) { return 0; }
-static inline int acpi_video_register_with_quirks(void) { return 0; }
+static inline int __acpi_video_register(bool backlight_quirks) { return 0; }
static inline void acpi_video_unregister(void) { return; }
static inline int acpi_video_get_edid(struct acpi_device *device, int type,
int device_id, void **edid)
@@ -40,4 +31,9 @@ static inline int acpi_video_get_edid(st
}
#endif
+static inline int acpi_video_register(void)
+{
+ return __acpi_video_register(false);
+}
+
#endif
On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> On Wednesday, July 24, 2013 02:05:15 PM Linus Torvalds wrote:
>> On Wed, Jul 24, 2013 at 2:02 PM, Daniel Vetter <[email protected]> wrote:
>> >
>> > I think a i915 module option should be doable, otoh people seem to have a
>> > viable workaround by setting a different acpi os version already.
>>
>> At least the original claim was that if you set a non-windows8 acpi os
>> version, you hit other bugs..
>>
>> But yeah, if we just do a plain revert, that may be the only option
>> for the people for whom the current (to-be-reverted) patches made
>> things work.
>
> Well, I wonder what about the appended (untested) patch?
Rafael, before going there, I've been trying to wrap my (poor, rusty
after vacation) head around
commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
Author: Rafael J. Wysocki <[email protected]>
Date: Thu Jul 18 02:08:06 2013 +0200
ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
and I can't see how it could work.
First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
it's actually set anywhere. Buried deep in the calls from
acpi_video_bus_add(), acpi_video_verify_backlight_support() is used
before acpi_video_backlight_quirks() gets called. (Perhaps if i915 is
reloaded, this goes right as the flags are already set.)
Second, with i915 that has opregion support, __acpi_video_register()
should only ever get called once. Which means the acpi_walk_namespace()
with video_unregister_backlight() should never get called in register.
Please enlighten me.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> > On Wednesday, July 24, 2013 02:05:15 PM Linus Torvalds wrote:
> >> On Wed, Jul 24, 2013 at 2:02 PM, Daniel Vetter <[email protected]> wrote:
> >> >
> >> > I think a i915 module option should be doable, otoh people seem to have a
> >> > viable workaround by setting a different acpi os version already.
> >>
> >> At least the original claim was that if you set a non-windows8 acpi os
> >> version, you hit other bugs..
> >>
> >> But yeah, if we just do a plain revert, that may be the only option
> >> for the people for whom the current (to-be-reverted) patches made
> >> things work.
> >
> > Well, I wonder what about the appended (untested) patch?
>
> Rafael, before going there, I've been trying to wrap my (poor, rusty
> after vacation) head around
>
> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
> Author: Rafael J. Wysocki <[email protected]>
> Date: Thu Jul 18 02:08:06 2013 +0200
>
> ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
>
> and I can't see how it could work.
Well, if it didn't work, people wouldn't see either improvement or breakage
from it, but they do see that, so it evidently works. :-)
> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
> it's actually set anywhere.
Are you sure about that?
acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
in fact is an ACPI driver (the naming sucks, but I didn't invent it). This
means that acpi_video_bus_add() can only be called *after* acpi_video_bus
has been registered with the ACPI subsystem (and the driver core). That
is done by acpi_bus_register_driver() and, guess what?, this happens in
__acpi_video_register(). So clearly, acpi_video_bus_add() *cannot* run before
__acpi_video_register().
> Buried deep in the calls from
> acpi_video_bus_add(), acpi_video_verify_backlight_support() is used
> before acpi_video_backlight_quirks() gets called. (Perhaps if i915 is
> reloaded, this goes right as the flags are already set.)
Please see above.
> Second, with i915 that has opregion support, __acpi_video_register()
> should only ever get called once. Which means the acpi_walk_namespace()
> with video_unregister_backlight() should never get called in register.
>
> Please enlighten me.
Actually, that's correct, so we don't need the whole
video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
be sufficient.
Ah, one more reason to do a full revert. I'm thinking, though, that I'll leave
acpi_video_backlight_quirks() as is so that it can be used by
acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
problems to happen.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
>> > Well, I wonder what about the appended (untested) patch?
>>
>> Rafael, before going there, I've been trying to wrap my (poor, rusty
>> after vacation) head around
>>
>> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
>> Author: Rafael J. Wysocki <[email protected]>
>> Date: Thu Jul 18 02:08:06 2013 +0200
>>
>> ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
>>
>> and I can't see how it could work.
>
> Well, if it didn't work, people wouldn't see either improvement or breakage
> from it, but they do see that, so it evidently works. :-)
I didn't claim it didn't work, just that *I* didn't see how it could. ;)
>> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
>> it's actually set anywhere.
>
> Are you sure about that?
>
> acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
> in fact is an ACPI driver (the naming sucks, but I didn't invent it). This
> means that acpi_video_bus_add() can only be called *after* acpi_video_bus
> has been registered with the ACPI subsystem (and the driver core). That
> is done by acpi_bus_register_driver() and, guess what?, this happens in
> __acpi_video_register(). So clearly, acpi_video_bus_add() *cannot* run before
> __acpi_video_register().
Right. I totally missed the call within the ternary operator. Thanks for
the explanation, and apologies for the noise.
>> Second, with i915 that has opregion support, __acpi_video_register()
>> should only ever get called once. Which means the acpi_walk_namespace()
>> with video_unregister_backlight() should never get called in register.
>>
>> Please enlighten me.
>
> Actually, that's correct, so we don't need the whole
> video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
> be sufficient.
>
> Ah, one more reason to do a full revert. I'm thinking, though, that I'll leave
> acpi_video_backlight_quirks() as is so that it can be used by
> acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
> problems to happen.
I observe that for the regular non-quirk acpi_video_register() call,
acpi_video_backlight_quirks() won't be called during register, but it
will get called later. This might have subtle effects later on, don't
you think?
As to the original problem, and your patch in this thread, what do you
think about having another value in acpi_backlight kernel parameter for
it? Having an i915 module parameter to tell acpi to use or not use
quirks seems odd, since the i915 is not really taking over
anything. It's just passing the info on to acpi.
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
On Thursday, July 25, 2013 03:34:10 PM Jani Nikula wrote:
> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> > On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
> >> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> >> > Well, I wonder what about the appended (untested) patch?
> >>
> >> Rafael, before going there, I've been trying to wrap my (poor, rusty
> >> after vacation) head around
> >>
> >> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
> >> Author: Rafael J. Wysocki <[email protected]>
> >> Date: Thu Jul 18 02:08:06 2013 +0200
> >>
> >> ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
> >>
> >> and I can't see how it could work.
> >
> > Well, if it didn't work, people wouldn't see either improvement or breakage
> > from it, but they do see that, so it evidently works. :-)
>
> I didn't claim it didn't work, just that *I* didn't see how it could. ;)
>
> >> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
> >> it's actually set anywhere.
> >
> > Are you sure about that?
> >
> > acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
> > in fact is an ACPI driver (the naming sucks, but I didn't invent it). This
> > means that acpi_video_bus_add() can only be called *after* acpi_video_bus
> > has been registered with the ACPI subsystem (and the driver core). That
> > is done by acpi_bus_register_driver() and, guess what?, this happens in
> > __acpi_video_register(). So clearly, acpi_video_bus_add() *cannot* run before
> > __acpi_video_register().
>
> Right. I totally missed the call within the ternary operator. Thanks for
> the explanation, and apologies for the noise.
>
> >> Second, with i915 that has opregion support, __acpi_video_register()
> >> should only ever get called once. Which means the acpi_walk_namespace()
> >> with video_unregister_backlight() should never get called in register.
> >>
> >> Please enlighten me.
> >
> > Actually, that's correct, so we don't need the whole
> > video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
> > be sufficient.
> >
> > Ah, one more reason to do a full revert. I'm thinking, though, that I'll leave
> > acpi_video_backlight_quirks() as is so that it can be used by
> > acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
> > problems to happen.
>
> I observe that for the regular non-quirk acpi_video_register() call,
> acpi_video_backlight_quirks() won't be called during register, but it
> will get called later. This might have subtle effects later on, don't
> you think?
Yes, it might, but after dropping ACPI_VIDEO_SKIP_BACKLIGHT it should be OK.
> As to the original problem, and your patch in this thread, what do you
> think about having another value in acpi_backlight kernel parameter for
> it? Having an i915 module parameter to tell acpi to use or not use
> quirks seems odd, since the i915 is not really taking over
> anything. It's just passing the info on to acpi.
I agree, I'm going to send a full revert in a while and we'll think what to
do about all that later.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
Hi Rafael,
did you commit a full revert?
Because I am experiencing quite weird things in rc3.
Do we have a bug opened to discuss about it?
Here is what I can observe:
1) During boot, probably when loading the driver, backlight gets off (or
to a level low enough to make me feel it is off)
2) When I am playing with my Fn+x keys, I am getting a completely full /
completely low brightness with no intermediate steps
3) When I am playing with my Fn+x keys while gnome brightness settings
panel is open, I am recovering intermediate steps but the Fn+x keys
behavior is inverted (the key supposed to lower the brightness make it
increase and vice-versa. Note that the gnome brightness indicator also
gets inverted).
4) Playing with the mouse on gnome brightness settings is working,
except that on the minimum level, backlight gets off
5) Writing to /sys/class/backlight/intel_backlight/brightness works
Regards
On 07/25/2013 02:57 PM, Rafael J. Wysocki wrote:
> On Thursday, July 25, 2013 03:34:10 PM Jani Nikula wrote:
>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
>>> On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
>>>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
>>>>> Well, I wonder what about the appended (untested) patch?
>>>> Rafael, before going there, I've been trying to wrap my (poor, rusty
>>>> after vacation) head around
>>>>
>>>> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
>>>> Author: Rafael J. Wysocki <[email protected]>
>>>> Date: Thu Jul 18 02:08:06 2013 +0200
>>>>
>>>> ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
>>>>
>>>> and I can't see how it could work.
>>> Well, if it didn't work, people wouldn't see either improvement or breakage
>>> from it, but they do see that, so it evidently works. :-)
>> I didn't claim it didn't work, just that *I* didn't see how it could. ;)
>>
>>>> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
>>>> it's actually set anywhere.
>>> Are you sure about that?
>>>
>>> acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
>>> in fact is an ACPI driver (the naming sucks, but I didn't invent it). This
>>> means that acpi_video_bus_add() can only be called *after* acpi_video_bus
>>> has been registered with the ACPI subsystem (and the driver core). That
>>> is done by acpi_bus_register_driver() and, guess what?, this happens in
>>> __acpi_video_register(). So clearly, acpi_video_bus_add() *cannot* run before
>>> __acpi_video_register().
>> Right. I totally missed the call within the ternary operator. Thanks for
>> the explanation, and apologies for the noise.
>>
>>>> Second, with i915 that has opregion support, __acpi_video_register()
>>>> should only ever get called once. Which means the acpi_walk_namespace()
>>>> with video_unregister_backlight() should never get called in register.
>>>>
>>>> Please enlighten me.
>>> Actually, that's correct, so we don't need the whole
>>> video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
>>> be sufficient.
>>>
>>> Ah, one more reason to do a full revert. I'm thinking, though, that I'll leave
>>> acpi_video_backlight_quirks() as is so that it can be used by
>>> acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
>>> problems to happen.
>> I observe that for the regular non-quirk acpi_video_register() call,
>> acpi_video_backlight_quirks() won't be called during register, but it
>> will get called later. This might have subtle effects later on, don't
>> you think?
> Yes, it might, but after dropping ACPI_VIDEO_SKIP_BACKLIGHT it should be OK.
>
>> As to the original problem, and your patch in this thread, what do you
>> think about having another value in acpi_backlight kernel parameter for
>> it? Having an i915 module parameter to tell acpi to use or not use
>> quirks seems odd, since the i915 is not really taking over
>> anything. It's just passing the info on to acpi.
> I agree, I'm going to send a full revert in a while and we'll think what to
> do about all that later.
>
> Thanks,
> Rafael
>
>
On Monday, July 29, 2013 09:36:31 PM * SAMÍ * wrote:
> Hi Rafael,
>
>
> did you commit a full revert?
Yes, but I left acpi_video_backlight_quirks() (in drivers/acpi/video_detect.c)
that is used to decide what to do with _DOS.
Can you please check if making that function always return 'false' makes any
difference?
Rafael
> Because I am experiencing quite weird things in rc3.
> Do we have a bug opened to discuss about it?
>
> Here is what I can observe:
> 1) During boot, probably when loading the driver, backlight gets off (or
> to a level low enough to make me feel it is off)
> 2) When I am playing with my Fn+x keys, I am getting a completely full /
> completely low brightness with no intermediate steps
> 3) When I am playing with my Fn+x keys while gnome brightness settings
> panel is open, I am recovering intermediate steps but the Fn+x keys
> behavior is inverted (the key supposed to lower the brightness make it
> increase and vice-versa. Note that the gnome brightness indicator also
> gets inverted).
> 4) Playing with the mouse on gnome brightness settings is working,
> except that on the minimum level, backlight gets off
> 5) Writing to /sys/class/backlight/intel_backlight/brightness works
>
>
> Regards
>
> On 07/25/2013 02:57 PM, Rafael J. Wysocki wrote:
> > On Thursday, July 25, 2013 03:34:10 PM Jani Nikula wrote:
> >> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> >>> On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
> >>>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> >>>>> Well, I wonder what about the appended (untested) patch?
> >>>> Rafael, before going there, I've been trying to wrap my (poor, rusty
> >>>> after vacation) head around
> >>>>
> >>>> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
> >>>> Author: Rafael J. Wysocki <[email protected]>
> >>>> Date: Thu Jul 18 02:08:06 2013 +0200
> >>>>
> >>>> ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
> >>>>
> >>>> and I can't see how it could work.
> >>> Well, if it didn't work, people wouldn't see either improvement or breakage
> >>> from it, but they do see that, so it evidently works. :-)
> >> I didn't claim it didn't work, just that *I* didn't see how it could. ;)
> >>
> >>>> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
> >>>> it's actually set anywhere.
> >>> Are you sure about that?
> >>>
> >>> acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
> >>> in fact is an ACPI driver (the naming sucks, but I didn't invent it). This
> >>> means that acpi_video_bus_add() can only be called *after* acpi_video_bus
> >>> has been registered with the ACPI subsystem (and the driver core). That
> >>> is done by acpi_bus_register_driver() and, guess what?, this happens in
> >>> __acpi_video_register(). So clearly, acpi_video_bus_add() *cannot* run before
> >>> __acpi_video_register().
> >> Right. I totally missed the call within the ternary operator. Thanks for
> >> the explanation, and apologies for the noise.
> >>
> >>>> Second, with i915 that has opregion support, __acpi_video_register()
> >>>> should only ever get called once. Which means the acpi_walk_namespace()
> >>>> with video_unregister_backlight() should never get called in register.
> >>>>
> >>>> Please enlighten me.
> >>> Actually, that's correct, so we don't need the whole
> >>> video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
> >>> be sufficient.
> >>>
> >>> Ah, one more reason to do a full revert. I'm thinking, though, that I'll leave
> >>> acpi_video_backlight_quirks() as is so that it can be used by
> >>> acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
> >>> problems to happen.
> >> I observe that for the regular non-quirk acpi_video_register() call,
> >> acpi_video_backlight_quirks() won't be called during register, but it
> >> will get called later. This might have subtle effects later on, don't
> >> you think?
> > Yes, it might, but after dropping ACPI_VIDEO_SKIP_BACKLIGHT it should be OK.
> >
> >> As to the original problem, and your patch in this thread, what do you
> >> think about having another value in acpi_backlight kernel parameter for
> >> it? Having an i915 module parameter to tell acpi to use or not use
> >> quirks seems odd, since the i915 is not really taking over
> >> anything. It's just passing the info on to acpi.
> > I agree, I'm going to send a full revert in a while and we'll think what to
> > do about all that later.
> >
> > Thanks,
> > Rafael
> >
> >
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
When I make acpi_video_backlight_quirks() return false:
- the Fn+x keys are not working anymore (remember that they didn't work
in 3.10 nor 3.9)
- At least the backlight remains on at boot.
- Gnome brightness settings do not work anymore. Neither do xbacklight.
- Writing to /sys/class/backlight/intel_backlight/brightness works
Regards,
Sam
On 07/29/2013 10:03 PM, Rafael J. Wysocki wrote:
> On Monday, July 29, 2013 09:36:31 PM * SAMÍ * wrote:
>> Hi Rafael,
>>
>>
>> did you commit a full revert?
> Yes, but I left acpi_video_backlight_quirks() (in drivers/acpi/video_detect.c)
> that is used to decide what to do with _DOS.
>
> Can you please check if making that function always return 'false' makes any
> difference?
>
> Rafael
>
>
>> Because I am experiencing quite weird things in rc3.
>> Do we have a bug opened to discuss about it?
>>
>> Here is what I can observe:
>> 1) During boot, probably when loading the driver, backlight gets off (or
>> to a level low enough to make me feel it is off)
>> 2) When I am playing with my Fn+x keys, I am getting a completely full /
>> completely low brightness with no intermediate steps
>> 3) When I am playing with my Fn+x keys while gnome brightness settings
>> panel is open, I am recovering intermediate steps but the Fn+x keys
>> behavior is inverted (the key supposed to lower the brightness make it
>> increase and vice-versa. Note that the gnome brightness indicator also
>> gets inverted).
>> 4) Playing with the mouse on gnome brightness settings is working,
>> except that on the minimum level, backlight gets off
>> 5) Writing to /sys/class/backlight/intel_backlight/brightness works
>>
>>
>> Regards
>>
>> On 07/25/2013 02:57 PM, Rafael J. Wysocki wrote:
>>> On Thursday, July 25, 2013 03:34:10 PM Jani Nikula wrote:
>>>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
>>>>> On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
>>>>>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
>>>>>>> Well, I wonder what about the appended (untested) patch?
>>>>>> Rafael, before going there, I've been trying to wrap my (poor, rusty
>>>>>> after vacation) head around
>>>>>>
>>>>>> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
>>>>>> Author: Rafael J. Wysocki <[email protected]>
>>>>>> Date: Thu Jul 18 02:08:06 2013 +0200
>>>>>>
>>>>>> ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
>>>>>>
>>>>>> and I can't see how it could work.
>>>>> Well, if it didn't work, people wouldn't see either improvement or breakage
>>>>> from it, but they do see that, so it evidently works. :-)
>>>> I didn't claim it didn't work, just that *I* didn't see how it could. ;)
>>>>
>>>>>> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
>>>>>> it's actually set anywhere.
>>>>> Are you sure about that?
>>>>>
>>>>> acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
>>>>> in fact is an ACPI driver (the naming sucks, but I didn't invent it). This
>>>>> means that acpi_video_bus_add() can only be called *after* acpi_video_bus
>>>>> has been registered with the ACPI subsystem (and the driver core). That
>>>>> is done by acpi_bus_register_driver() and, guess what?, this happens in
>>>>> __acpi_video_register(). So clearly, acpi_video_bus_add() *cannot* run before
>>>>> __acpi_video_register().
>>>> Right. I totally missed the call within the ternary operator. Thanks for
>>>> the explanation, and apologies for the noise.
>>>>
>>>>>> Second, with i915 that has opregion support, __acpi_video_register()
>>>>>> should only ever get called once. Which means the acpi_walk_namespace()
>>>>>> with video_unregister_backlight() should never get called in register.
>>>>>>
>>>>>> Please enlighten me.
>>>>> Actually, that's correct, so we don't need the whole
>>>>> video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
>>>>> be sufficient.
>>>>>
>>>>> Ah, one more reason to do a full revert. I'm thinking, though, that I'll leave
>>>>> acpi_video_backlight_quirks() as is so that it can be used by
>>>>> acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
>>>>> problems to happen.
>>>> I observe that for the regular non-quirk acpi_video_register() call,
>>>> acpi_video_backlight_quirks() won't be called during register, but it
>>>> will get called later. This might have subtle effects later on, don't
>>>> you think?
>>> Yes, it might, but after dropping ACPI_VIDEO_SKIP_BACKLIGHT it should be OK.
>>>
>>>> As to the original problem, and your patch in this thread, what do you
>>>> think about having another value in acpi_backlight kernel parameter for
>>>> it? Having an i915 module parameter to tell acpi to use or not use
>>>> quirks seems odd, since the i915 is not really taking over
>>>> anything. It's just passing the info on to acpi.
>>> I agree, I'm going to send a full revert in a while and we'll think what to
>>> do about all that later.
>>>
>>> Thanks,
>>> Rafael
>>>
>>>
On Monday, July 29, 2013 11:53:59 PM * SAMÍ * wrote:
> When I make acpi_video_backlight_quirks() return false:
> - the Fn+x keys are not working anymore (remember that they didn't work
> in 3.10 nor 3.9)
> - At least the backlight remains on at boot.
> - Gnome brightness settings do not work anymore. Neither do xbacklight.
> - Writing to /sys/class/backlight/intel_backlight/brightness works
Well, you're better off with acpi_video_backlight_quirks() as is, then. :-)
I'm afraid we can't help you by revering anything more at this point.
Please file a bug in the kernel BZ to further track the issues you're
seeing.
Thanks,
Rafael
> On 07/29/2013 10:03 PM, Rafael J. Wysocki wrote:
> > On Monday, July 29, 2013 09:36:31 PM * SAMÍ * wrote:
> >> Hi Rafael,
> >>
> >>
> >> did you commit a full revert?
> > Yes, but I left acpi_video_backlight_quirks() (in drivers/acpi/video_detect.c)
> > that is used to decide what to do with _DOS.
> >
> > Can you please check if making that function always return 'false' makes any
> > difference?
> >
> > Rafael
> >
> >
> >> Because I am experiencing quite weird things in rc3.
> >> Do we have a bug opened to discuss about it?
> >>
> >> Here is what I can observe:
> >> 1) During boot, probably when loading the driver, backlight gets off (or
> >> to a level low enough to make me feel it is off)
> >> 2) When I am playing with my Fn+x keys, I am getting a completely full /
> >> completely low brightness with no intermediate steps
> >> 3) When I am playing with my Fn+x keys while gnome brightness settings
> >> panel is open, I am recovering intermediate steps but the Fn+x keys
> >> behavior is inverted (the key supposed to lower the brightness make it
> >> increase and vice-versa. Note that the gnome brightness indicator also
> >> gets inverted).
> >> 4) Playing with the mouse on gnome brightness settings is working,
> >> except that on the minimum level, backlight gets off
> >> 5) Writing to /sys/class/backlight/intel_backlight/brightness works
> >>
> >>
> >> Regards
> >>
> >> On 07/25/2013 02:57 PM, Rafael J. Wysocki wrote:
> >>> On Thursday, July 25, 2013 03:34:10 PM Jani Nikula wrote:
> >>>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> >>>>> On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
> >>>>>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
> >>>>>>> Well, I wonder what about the appended (untested) patch?
> >>>>>> Rafael, before going there, I've been trying to wrap my (poor, rusty
> >>>>>> after vacation) head around
> >>>>>>
> >>>>>> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
> >>>>>> Author: Rafael J. Wysocki <[email protected]>
> >>>>>> Date: Thu Jul 18 02:08:06 2013 +0200
> >>>>>>
> >>>>>> ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
> >>>>>>
> >>>>>> and I can't see how it could work.
> >>>>> Well, if it didn't work, people wouldn't see either improvement or breakage
> >>>>> from it, but they do see that, so it evidently works. :-)
> >>>> I didn't claim it didn't work, just that *I* didn't see how it could. ;)
> >>>>
> >>>>>> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
> >>>>>> it's actually set anywhere.
> >>>>> Are you sure about that?
> >>>>>
> >>>>> acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
> >>>>> in fact is an ACPI driver (the naming sucks, but I didn't invent it). This
> >>>>> means that acpi_video_bus_add() can only be called *after* acpi_video_bus
> >>>>> has been registered with the ACPI subsystem (and the driver core). That
> >>>>> is done by acpi_bus_register_driver() and, guess what?, this happens in
> >>>>> __acpi_video_register(). So clearly, acpi_video_bus_add() *cannot* run before
> >>>>> __acpi_video_register().
> >>>> Right. I totally missed the call within the ternary operator. Thanks for
> >>>> the explanation, and apologies for the noise.
> >>>>
> >>>>>> Second, with i915 that has opregion support, __acpi_video_register()
> >>>>>> should only ever get called once. Which means the acpi_walk_namespace()
> >>>>>> with video_unregister_backlight() should never get called in register.
> >>>>>>
> >>>>>> Please enlighten me.
> >>>>> Actually, that's correct, so we don't need the whole
> >>>>> video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
> >>>>> be sufficient.
> >>>>>
> >>>>> Ah, one more reason to do a full revert. I'm thinking, though, that I'll leave
> >>>>> acpi_video_backlight_quirks() as is so that it can be used by
> >>>>> acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
> >>>>> problems to happen.
> >>>> I observe that for the regular non-quirk acpi_video_register() call,
> >>>> acpi_video_backlight_quirks() won't be called during register, but it
> >>>> will get called later. This might have subtle effects later on, don't
> >>>> you think?
> >>> Yes, it might, but after dropping ACPI_VIDEO_SKIP_BACKLIGHT it should be OK.
> >>>
> >>>> As to the original problem, and your patch in this thread, what do you
> >>>> think about having another value in acpi_backlight kernel parameter for
> >>>> it? Having an i915 module parameter to tell acpi to use or not use
> >>>> quirks seems odd, since the i915 is not really taking over
> >>>> anything. It's just passing the info on to acpi.
> >>> I agree, I'm going to send a full revert in a while and we'll think what to
> >>> do about all that later.
> >>>
> >>> Thanks,
> >>> Rafael
> >>>
> >>>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On 07/30/2013 03:36 AM, * SAMÍ * wrote:
> Hi Rafael,
>
>
> did you commit a full revert?
> Because I am experiencing quite weird things in rc3.
> Do we have a bug opened to discuss about it?
Yes we have:
https://bugzilla.kernel.org/show_bug.cgi?id=52951
I'll look into this issue.
Thanks,
Aaron
>
> Here is what I can observe:
> 1) During boot, probably when loading the driver, backlight gets off (or
> to a level low enough to make me feel it is off)
> 2) When I am playing with my Fn+x keys, I am getting a completely full /
> completely low brightness with no intermediate steps
> 3) When I am playing with my Fn+x keys while gnome brightness settings
> panel is open, I am recovering intermediate steps but the Fn+x keys
> behavior is inverted (the key supposed to lower the brightness make it
> increase and vice-versa. Note that the gnome brightness indicator also
> gets inverted).
> 4) Playing with the mouse on gnome brightness settings is working,
> except that on the minimum level, backlight gets off
> 5) Writing to /sys/class/backlight/intel_backlight/brightness works
>
>
> Regards
>
> On 07/25/2013 02:57 PM, Rafael J. Wysocki wrote:
>> On Thursday, July 25, 2013 03:34:10 PM Jani Nikula wrote:
>>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
>>>> On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
>>>>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <[email protected]> wrote:
>>>>>> Well, I wonder what about the appended (untested) patch?
>>>>> Rafael, before going there, I've been trying to wrap my (poor, rusty
>>>>> after vacation) head around
>>>>>
>>>>> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
>>>>> Author: Rafael J. Wysocki <[email protected]>
>>>>> Date: Thu Jul 18 02:08:06 2013 +0200
>>>>>
>>>>> ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
>>>>>
>>>>> and I can't see how it could work.
>>>> Well, if it didn't work, people wouldn't see either improvement or breakage
>>>> from it, but they do see that, so it evidently works. :-)
>>> I didn't claim it didn't work, just that *I* didn't see how it could. ;)
>>>
>>>>> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
>>>>> it's actually set anywhere.
>>>> Are you sure about that?
>>>>
>>>> acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
>>>> in fact is an ACPI driver (the naming sucks, but I didn't invent it). This
>>>> means that acpi_video_bus_add() can only be called *after* acpi_video_bus
>>>> has been registered with the ACPI subsystem (and the driver core). That
>>>> is done by acpi_bus_register_driver() and, guess what?, this happens in
>>>> __acpi_video_register(). So clearly, acpi_video_bus_add() *cannot* run before
>>>> __acpi_video_register().
>>> Right. I totally missed the call within the ternary operator. Thanks for
>>> the explanation, and apologies for the noise.
>>>
>>>>> Second, with i915 that has opregion support, __acpi_video_register()
>>>>> should only ever get called once. Which means the acpi_walk_namespace()
>>>>> with video_unregister_backlight() should never get called in register.
>>>>>
>>>>> Please enlighten me.
>>>> Actually, that's correct, so we don't need the whole
>>>> video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
>>>> be sufficient.
>>>>
>>>> Ah, one more reason to do a full revert. I'm thinking, though, that I'll leave
>>>> acpi_video_backlight_quirks() as is so that it can be used by
>>>> acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
>>>> problems to happen.
>>> I observe that for the regular non-quirk acpi_video_register() call,
>>> acpi_video_backlight_quirks() won't be called during register, but it
>>> will get called later. This might have subtle effects later on, don't
>>> you think?
>> Yes, it might, but after dropping ACPI_VIDEO_SKIP_BACKLIGHT it should be OK.
>>
>>> As to the original problem, and your patch in this thread, what do you
>>> think about having another value in acpi_backlight kernel parameter for
>>> it? Having an i915 module parameter to tell acpi to use or not use
>>> quirks seems odd, since the i915 is not really taking over
>>> anything. It's just passing the info on to acpi.
>> I agree, I'm going to send a full revert in a while and we'll think what to
>> do about all that later.
>>
>> Thanks,
>> Rafael
>>
>>
>