On Sunday, August 04, 2013 01:42:49 AM Felipe Contreras wrote: > 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. Yes, it would. > 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. If we're going to fix it in 3.12, it's good to discuss it now, which is why I'm askig about that. > 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. So, let's assume that the one-liner goes into 3.11 and work further with that assumption. How would you address the sythetic case (on top of the one-liner)? Rafael -- 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
