Hi, >> +static unsigned long long
+acpi_video_bqc_value_to_level(struct acpi_video_device *device, + unsigned long long bqc_value) +{ + unsigned long long level; + + if (device->brightness->flags._BQC_use_index) { + if (device->brightness->flags._BCL_reversed) + bqc_value = device->brightness->count - 3 - bqc_value; + + level = device->brightness->levels[bqc_value + 2];I don't understand this, what does the +2 have to do here? _BQC returned us an index, and then we should just convert it to level, why +2? The only explanation would be, for BIOS tables that return _BQC as index, they are indexing from the 3rd entry. Is it the case? If so, I think we need to put a comment here.
Yes, that's the case. The old code did the same thing:
- if (device->brightness->flags._BQC_use_index) { - if (device->brightness->flags._BCL_reversed) - *level = device->brightness->count - - 3 - (*level); - *level = device->brightness->levels[*level + 2]; + *level = acpi_video_bqc_value_to_level(device, *level);
That's also the reason for the -3 instead of -1 in the BCL_reversed case. I can add a comment, though.
+ if (use_bios_initial_backlight) { + if (!br->flags._BQC_use_index) { + /* + * Set the backlight to the initial state. + * On some buggy laptops, _BQC returns an uninitialized + * value when invoked for the first time, i.e. + * level_old is invalid. Set the backlight to max_level + * in this case. + */ for (i = 2; i < br->count; i++) if (level_old == br->levels[i]) { level = level_old; break; } + } else { + level = acpi_video_bqc_value_to_level(device, + level_old);What about we convert the value to level first? if (use_bios_initial_backlight) { level = acpi_video_bqc_value_to_level(device, level_old); /* * Set the backlight to the initial state. * On some buggy laptops, _BQC returns an uninitialized * value when invoked for the first time, i.e. * level_old is invalid(no matter it is a level, or an * index.) Set the backlight to max_level in this case. */ for (i = 2; i < br->count; i++) if (level == br->levels[i]) break; if (i == br->count) level = max_level; }
That works as well, and looks cleaner to me as well. I'll change that. Regards, Danny -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
