On 03/15/2013 06:10 PM, Danny Baumann wrote:
> In particular, this fixes brightness control initialization for all
> devices that return index values from _BQC and don't happen to have the
> initial index set by the BIOS in their _BCL table. One example for that
> is the Dell Inspiron 15R SE (model number 7520).
>
> What happened for those devices is that acpi_init_brightness queried the
> initial brightness by calling acpi_video_device_lcd_get_level_current.
> This called _BQC, which returned e.g. 13. As _BQC_use_index isn't
> determined at this point (and thus has its initial value of 0), the
> index isn't converted into the actual level. As '13' isn't present in
> the _BCL list, *level is later overwritten with brightness->curr, which
> was initialized to max_level (100) before. Later in
> acpi_init_brightness, level_old (with the value 100) is used as an index
> into the _BCL table, which causes a value outside of the actual table to
> be used as input into acpi_video_device_lcd_set_level(). Depending on
> the (undefined) value of that location, this call will fail, causing the
> brightness control for the device in question not to be enabled.
>
> Fix that by returning the raw value returned by the _BQC call in the
> initialization case.
>
> Signed-off-by: Danny Baumann <dannybaumann@xxxxxx>
> ---
> drivers/acpi/video.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 313f959..9c33871 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -222,7 +222,7 @@ static int acpi_video_device_lcd_set_level(struct acpi_video_device *device,
> int level);
> static int acpi_video_device_lcd_get_level_current(
> struct acpi_video_device *device,
> - unsigned long long *level, int init);
> + unsigned long long *level, int raw);
Why not bool?
Thanks,
Aaron
> static int acpi_video_get_next_level(struct acpi_video_device *device,
> u32 level_current, u32 event);
> static int acpi_video_switch_brightness(struct acpi_video_device *device,
> @@ -452,7 +452,7 @@ static struct dmi_system_id video_dmi_table[] __initdata = {
>
> static int
> acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
> - unsigned long long *level, int init)
> + unsigned long long *level, int raw)
> {
> acpi_status status = AE_OK;
> int i;
> @@ -463,6 +463,15 @@ acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
> status = acpi_evaluate_integer(device->dev->handle, buf,
> NULL, level);
> if (ACPI_SUCCESS(status)) {
> + if (raw) {
> + /*
> + * Caller has indicated he wants the raw
> + * value returned by _BQC, so don't furtherly
> + * mess with the value.
> + */
> + return 0;
> + }
> +
> if (device->brightness->flags._BQC_use_index) {
> if (device->brightness->flags._BCL_reversed)
> *level = device->brightness->count
> @@ -476,16 +485,14 @@ acpi_video_device_lcd_get_level_current(struct acpi_video_device *device,
> device->brightness->curr = *level;
> return 0;
> }
> - if (!init) {
> - /*
> - * BQC returned an invalid level.
> - * Stop using it.
> - */
> - ACPI_WARNING((AE_INFO,
> - "%s returned an invalid level",
> - buf));
> - device->cap._BQC = device->cap._BCQ = 0;
> - }
> + /*
> + * BQC returned an invalid level.
> + * Stop using it.
> + */
> + ACPI_WARNING((AE_INFO,
> + "%s returned an invalid level",
> + buf));
> + device->cap._BQC = device->cap._BCQ = 0;
> } else {
> /* Fixme:
> * should we return an error or ignore this failure?
> @@ -714,7 +721,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
> if (result)
> goto out_free_levels;
>
> - result = acpi_video_device_lcd_get_level_current(device, &level, 0);
> + result = acpi_video_device_lcd_get_level_current(device, &level, 1);
> if (result)
> goto out_free_levels;
>
>
--
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
