Re: [PATCH] ACPI: Disable Windows 8 compatibility for some Lenovo ThinkPads

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

On 04/02/2013 09:00 PM, Seth Forshee wrote:
> On Tue, Apr 02, 2013 at 05:08:23PM +0800, Aaron Lu wrote:
>> On 04/01/2013 09:03 PM, Seth Forshee wrote:
>>> On Mon, Apr 01, 2013 at 09:53:36AM +0800, Aaron Lu wrote:
>>>> On 02/12/2013 12:21 AM, Seth Forshee wrote:
>>>>> The AML implementation for brightness control on several ThinkPads
>>>>> contains a workaround to meet a Windows 8 requirement of 101 brightness
>>>>> levels [1]. The implementation is flawed, as only 16 of the brighness
>>>>> values reported by _BCL affect a change in brightness. _BCM silently
>>>>> discards the rest of the values. Disabling Windows 8 compatibility on
>>>>> these machines reverts them to the old behavior, making _BCL only report
>>>>> the 16 brightness levels which actually work. Add a quirk to do this
>>>>> along with a dmi callback to disable Win8 compatibility.
>>>>
>>>> If we disable the _BQC(i.e. set cap._BQC=0) for these systems, will the
>>>> problem go away? If so, I think perhaps we can put these systems into a
>>>> _BQC quirk table and set cap._BQC=0 for them.
>>>
>>> That helps a little, but we're still left with only 16 of the 101
>>> brightness levels causing any change in brightness. The firmware isn't
>>> rounding the "bad" values or anything like that; it just silently
>>> ignores them.
>>
>> OK, and if GUI tool like g-s-d decides to go some more steps when
>> adjusting backlight level, it may always choose the non-functional
>> values. Hmm, doesn't seem to be an usable way then.
> 
> Exactly.
> 
>> I really wondered, how Windows handled this, it should have the same
>> problem, unless they are not using the acpi video interface?
> 
> I can only guess.
> 
> I think I remember reading that Windows 8 does smooth backlight
> transitions, so it may well hit every intermediate brightness value.
> Lenovo could also be supplying a driver which rounds values to the
> nearest working value or uses some other interface or something else.

Right, thanks for the hint.

> 
>>> I submitted a second set of patches [1] which writes all intermediate
>>> values between the old and new brightness values and disables _BQC for
>>> these machines (empirically rather than using a quirk table), though no
>>> one seems to be interested in reviewing them.
>>
>> Suppose we are at level 100, and the user sets the target level to be
>> 20, then we will need to call _BCM 80 times?
> 
> Yes. And on machines where _BQC actually returns legitimate values it
> will get called 80 times as well. On these Lenovo's we'd quickly
> determine that _BQC doesn't work and stop trying to use it however.

OK, but we still have to call it 80 times, right? From smooth point of
view, this might be a good thing, but I'm not sure if such change of
behaviour just for this problem is desired.

And I just took a look at patch 3, it seems to affect the normal code
path too, so we are changing the behaviour of a brightness level change
for some specific systems.

I really do not want these quirk code to pollute normal code path, it
made code difficult to understand and complicated. And all systems
suffer from this change, not just the affected ones.

I hope we can use quirk to differentiate them.

Thanks,
Aaron