Re: [Regression]: Changing brightness does not work with v3.16-rc4

[Aaron Lu <aaron.lu@intel.com>]

On 07/15/2014 10:05 PM, Hans de Goede wrote:
> Hi,
> 
> On 07/15/2014 02:27 PM, Rafael J. Wysocki wrote:
>> On Tuesday, July 15, 2014 02:06:21 PM Aaron Lu wrote:
>>> On 07/15/2014 03:17 AM, Rafael J. Wysocki wrote:
>>>> On Monday, July 14, 2014 08:56:29 PM Julian Wollrath wrote:
>>>>> Am Mon, 14 Jul 2014 21:10:47 +0200
>>>>> schrieb "Rafael J. Wysocki" <rjw@rjwysocki.net>:
>>>>>
>>>>>> On Monday, July 14, 2014 09:02:34 PM Rafael J. Wysocki wrote:
>>>>>>> On Monday, July 14, 2014 08:34:22 PM Julian Wollrath wrote:
>>>>>>>> Am Mon, 14 Jul 2014 20:21:19 +0200
>>>>>>>> schrieb "Rafael J. Wysocki" <rjw@rjwysocki.net>:
>>>>>>>>
>>>>>>>>> On Saturday, July 12, 2014 06:24:52 PM Julian Wollrath wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>>> changing the brightness on my Thinkpad X230 with Intel
>>>>>>>>>>> HD4000 graphics, stopped working after commit 886129a8eebe
>>>>>>>>>>> (ACPI / video: change acpi-video brightness_switch_enabled
>>>>>>>>>>> default to 0). Changing it back to 1, fixes the problem, if
>>>>>>>>>>> "acpi_osi=!Windows 2012" is also set. Before that commit
>>>>>>>>>>> just setting "acpi_osi=!Windows 2012" was enough to get
>>>>>>>>>>> changing the brightness working.
>>>>>>>>>>
>>>>>>>>>> small correction: It setting "acpi_osi=!Windows 2012" does
>>>>>>>>>> only help when using a kernel with commit 886129a8eebe as
>>>>>>>>>> HEAD, with v3.16-rc4 setting
>>>>>>>>>> video.brightness_switch_enabled=1 and acpi_osi=!Windows 2012
>>>>>>>>>> or just one of them, does not make changing the brightness
>>>>>>>>>> work again.
>>>>>>>>>
>>>>>>>>> And reverting commit 751109aad583 (ACPI / video: Change the
>>>>>>>>> default for video.use_native_backlight to 1) make that last
>>>>>>>>> problem go away, right?
>>>>>>>>>
>>>>>>>>> We are going to rever commit 886129a8eebe for other reasons, so
>>>>>>>>> can you please revert it from 3.16-rc5 on your system and
>>>>>>>>> report back?
>>>>>>>> Yes, with both 751109aad583 and 886129a8eebe reverted, changing
>>>>>>>> the brightness works again.
>>>>>>>
>>>>>>> What if you revert 886129a8eebe alone?
>>>>>>
>>>>>> I mean, what exactly do you need to pass to the kernel command line
>>>>>> in that case to make brightness switching work?
>>>>> I need to pass
>>>>>    acpi_osi="!Windows 2012" video.brightness_switch_enabled=1 video.use_native_backlight=-1
>>>>>
>>>>> If I leave one of these parameters out, the brightness switch stops
>>>>> working.
>>>>
>>>> With 886129a8eebe reverted you should not need video.brightness_switch_enabled=1.
>>>>
>>>> Aaron, what do you think?
>>>
>>> I agree.
>>>
>>> I suppose Julian is not using an standard GUI so he needs the in kernel
>>> processing of the backlight event. And his laptop has a broken ACPI
>>> interface in Win8 mode, so he also needs the "!Windows 2012" option.
>>>
>>> With a standard GUI environment, his system would have only one
>>> interface left: the GPU one and everything should work. But of course,
>>> that may not be his config.
>>
>> Well, do we need to revert 751109aad583 too, then?
> 
> No, just no, clearly using the acpi video interface for backlight control
> is the wrong thing to do on win8 generation laptops, if we don't flip
> the default we're going to end up with a never ending list of quirks
> for models where the acpi-video interface is broken.
> 
> On 07/15/2014 04:00 PM, Aaron Lu wrote:
> 
>  > If we have to consider the following config:
>  > 1 ACPI backlight interface works(or can work with some cmdline option);
> 
> And the BIOS advertises native win8 support.

Right.
So this condition should be:
1 ACPI backlight interface works in Win8 mode;

This means, for Julian's system, the commit 751109aad583 doesn't need to
be reverted as the ACPI backlight interface on his system only works
when the acpi_osi="!Windows 2012" is added. In that case, the
video.use_native_backlight=X doesn't make any difference anymore.

Thanks,
Aaron

> 
>  > 2 Not using a normal GUI(depends on ACPI video module to do the in
>  >    kernel backlight level change on hotkey press).
>  > Then we will need to revert 751109aad583.
> 
> I think the combination of 1 and 2 makes the subset of users
> potentially affected by this so small, that we really should
> continue with video.use_native_backlight=1 as default, the
> other route adding more and more and more and more quirks
> is sheer madness.
> 
> Regards,
> 
> Hans
> 
> 
>