linux-kernel - Re: [PATCH] acpi: video: improve quirk check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMP44s0+VKH_vTRiXvhKz8gGUwAa+_Hgf_dN3UddYE1Z7DLPAA@mail.gmail.com>
Date:	Sun, 4 Aug 2013 01:42:49 -0500
From:	Felipe Contreras <felipe.contreras@...il.com>
To:	Aaron Lu <aaron.lwe@...il.com>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>, linux-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org, Len Brown <lenb@...nel.org>,
	Zhang Rui <rui.zhang@...el.com>
Subject: Re: [PATCH] acpi: video: improve quirk check

On Sat, Aug 3, 2013 at 8:18 PM, Aaron Lu <aaron.lwe@...il.com> wrote:
> On 08/03/2013 07:34 PM, Rafael J. Wysocki wrote:
>> On Saturday, August 03, 2013 04:14:04 PM Aaron Lu wrote:
>>> On 08/03/2013 07:47 AM, Rafael J. Wysocki wrote:
>>>> On Friday, August 02, 2013 02:37:09 PM Felipe Contreras wrote:
>>>>> If the _BCL package is descending, the first level (br->levels[2]) will
>>>>> be 0, and if the number of levels matches the number of steps, we might
>>>>> confuse a returned level to mean the index.
>>>>>
>>>>> For example:
>>>>>
>>>>>   current_level = max_level = 100
>>>>>   test_level = 0
>>>>>   returned level = 100
>>>>>
>>>>> In this case 100 means the level, not the index, and _BCM failed. But if
>>>>> the _BCL package is descending, the index of level 0 is also 100, so we
>>>>> assume _BQC is indexed, when it's not.
>>>>>
>>>>> This causes all _BQC calls to return bogus values causing weird behavior
>>>>> from the user's perspective. For example: xbacklight -set 10; xbacklight
>>>>> -set 20; would flash to 90% and then slowly down to the desired level
>>>>> (20).
>>>>>
>>>>> The solution is simple; test anything other than the first level (e.g.
>>>>> 1).
>>>>>
>>>>> Signed-off-by: Felipe Contreras <felipe.contreras@...il.com>
>>>>
>>>> Looks reasonable.
>>>>
>>>> Aaron, what do you think?
>>>
>>> Yes, the patch is correct, but I still prefer my own version :-)
>>> https://github.com/aaronlu/linux/commit/0a3d2c5b59caf80ae5bb1ca1fda0f7bf448b38c9
>>>
>>> In case you want to take mine and mine needs refresh, please let me know
>>> and I can do the re-base, thanks.
>>
>> Well, I prefer simpler, unless there's a good reason to use more complicated.
>>
>> Why exactly do you think your version is better?
>
> As explained here:
> https://lkml.org/lkml/2013/8/2/81
> https://lkml.org/lkml/2013/8/2/112
>
> And for the demo broken _BQC, mine patch will disable _BQC while still
> make the backlight work, and this patch here is testing the max
> brightness level and may fail.

Yes, but that problem can *only* happen in such a simplified _BCL,
which is very very unlikely. Still, it would make sense to fix the
code for that case.

However, we can fix the problem first for the real known cases with my
simple one-liner patch that can even be merged for v3.11, and *later*
fix the issue for the synthetic unlikely case.

Personally I think there are better ways to fix the code for the
synthetic case than what you patch does, which will also make _BQC
work. That can be discussed later though, the one-liner is simple, and
it works.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/