On Sat, Aug 3, 2013 at 8:18 PM, Aaron Lu <aaron.lwe@xxxxxxxxx> 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@xxxxxxxxx> >>>> >>>> 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-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
