2012-06-10 16:37:54

by Carlos R. Mafra

[permalink] [raw]
Subject: 3.5.0-rc2 bisected regression: Can't change brightness on Vaio laptop

Commit ea9f8856bd6d4 ("ACPI video: Harden video bus adding") breaks the
backlight controls on my Vaio laptop.

After this patch the folder /sys/class/backlight becomes empty and
I can't change the lcd brightness anymore.

This regression is present in v3.4.2 as well as in the current
v3.5.0-rc2.

I tried to manually revert that commit from v3.5.0-rc2, and the resulting
patch below fixes the issue for me.

My config is here:
http://www.damtp.cam.ac.uk/user/crm66/config-3.5-rc1

and the dmesg of v3.5.0-rc2 plus the patch below is here:
http://www.damtp.cam.ac.uk/user/crm66/dmesg_v3.5-rc2+patch.txt

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index a576575..4612b1c 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -548,27 +548,27 @@ acpi_video_device_EDID(struct acpi_video_device *device,
* 1. The system BIOS should NOT automatically control the brightness
* level of the LCD when the power changes from AC to DC.
* Return Value:
- * -EINVAL wrong arg.
+ * -1 wrong arg.
*/

static int
acpi_video_bus_DOS(struct acpi_video_bus *video, int bios_flag, int lcd_flag)
{
- acpi_status status;
+ u64 status = 0;
union acpi_object arg0 = { ACPI_TYPE_INTEGER };
struct acpi_object_list args = { 1, &arg0 };


- if (bios_flag < 0 || bios_flag > 3 || lcd_flag < 0 || lcd_flag > 1)
- return -EINVAL;
+ if (bios_flag < 0 || bios_flag > 3 || lcd_flag < 0 || lcd_flag > 1) {
+ status = -1;
+ goto Failed;
+ }
arg0.integer.value = (lcd_flag << 2) | bios_flag;
video->dos_setting = arg0.integer.value;
- status = acpi_evaluate_object(video->device->handle, "_DOS",
- &args, NULL);
- if (ACPI_FAILURE(status))
- return -EIO;
+ acpi_evaluate_object(video->device->handle, "_DOS", &args, NULL);

- return 0;
+ Failed:
+ return status;
}

/*
@@ -1343,17 +1343,15 @@ static int
acpi_video_bus_get_devices(struct acpi_video_bus *video,
struct acpi_device *device)
{
- int status;
+ int status = 0;
struct acpi_device *dev;

- status = acpi_video_device_enumerate(video);
- if (status)
- return status;
+ acpi_video_device_enumerate(video);

list_for_each_entry(dev, &device->children, node) {

status = acpi_video_bus_get_one_device(dev, video);
- if (status) {
+ if (ACPI_FAILURE(status)) {
printk(KERN_WARNING PREFIX
"Can't attach device\n");
continue;
@@ -1655,9 +1653,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
mutex_init(&video->device_list_lock);
INIT_LIST_HEAD(&video->video_device_list);

- error = acpi_video_bus_get_devices(video, device);
- if (error)
- goto err_free_video;
+ acpi_video_bus_get_devices(video, device);
+ acpi_video_bus_start_devices(video);

video->input = input = input_allocate_device();
if (!input) {
@@ -1695,13 +1692,9 @@ static int acpi_video_bus_add(struct acpi_device *device)

video->pm_nb.notifier_call = acpi_video_resume;
video->pm_nb.priority = 0;
- error = register_pm_notifier(&video->pm_nb);
- if (error)
- goto err_stop_video;

- error = input_register_device(input);
- if (error)
- goto err_unregister_pm_notifier;
+ register_pm_notifier(&video->pm_nb);
+ input_register_device(input);

return 0;


2012-06-14 22:26:51

by Igor Murzov

[permalink] [raw]
Subject: Re: 3.5.0-rc2 bisected regression: Can't change brightness on Vaio laptop

Please follow the discussion here:
https://bugzilla.kernel.org/show_bug.cgi?id=43168


> Commit ea9f8856bd6d4 ("ACPI video: Harden video bus adding") breaks the
> backlight controls on my Vaio laptop.
>
> After this patch the folder /sys/class/backlight becomes empty and
> I can't change the lcd brightness anymore.
>
> This regression is present in v3.4.2 as well as in the current
> v3.5.0-rc2.
>
> I tried to manually revert that commit from v3.5.0-rc2, and the resulting
> patch below fixes the issue for me.
>
> My config is here:
> http://www.damtp.cam.ac.uk/user/crm66/config-3.5-rc1
>
> and the dmesg of v3.5.0-rc2 plus the patch below is here:
> http://www.damtp.cam.ac.uk/user/crm66/dmesg_v3.5-rc2+patch.txt
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index a576575..4612b1c 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -548,27 +548,27 @@ acpi_video_device_EDID(struct acpi_video_device *device,
> * 1. The system BIOS should NOT automatically control the brightness
> * level of the LCD when the power changes from AC to DC.
> * Return Value:
> - * -EINVAL wrong arg.
> + * -1 wrong arg.
> */
>
> static int
> acpi_video_bus_DOS(struct acpi_video_bus *video, int bios_flag, int lcd_flag)
> {
> - acpi_status status;
> + u64 status = 0;
> union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> struct acpi_object_list args = { 1, &arg0 };
>
>
> - if (bios_flag < 0 || bios_flag > 3 || lcd_flag < 0 || lcd_flag > 1)
> - return -EINVAL;
> + if (bios_flag < 0 || bios_flag > 3 || lcd_flag < 0 || lcd_flag > 1) {
> + status = -1;
> + goto Failed;
> + }
> arg0.integer.value = (lcd_flag << 2) | bios_flag;
> video->dos_setting = arg0.integer.value;
> - status = acpi_evaluate_object(video->device->handle, "_DOS",
> - &args, NULL);
> - if (ACPI_FAILURE(status))
> - return -EIO;
> + acpi_evaluate_object(video->device->handle, "_DOS", &args, NULL);
>
> - return 0;
> + Failed:
> + return status;
> }
>
> /*
> @@ -1343,17 +1343,15 @@ static int
> acpi_video_bus_get_devices(struct acpi_video_bus *video,
> struct acpi_device *device)
> {
> - int status;
> + int status = 0;
> struct acpi_device *dev;
>
> - status = acpi_video_device_enumerate(video);
> - if (status)
> - return status;
> + acpi_video_device_enumerate(video);
>
> list_for_each_entry(dev, &device->children, node) {
>
> status = acpi_video_bus_get_one_device(dev, video);
> - if (status) {
> + if (ACPI_FAILURE(status)) {
> printk(KERN_WARNING PREFIX
> "Can't attach device\n");
> continue;
> @@ -1655,9 +1653,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
> mutex_init(&video->device_list_lock);
> INIT_LIST_HEAD(&video->video_device_list);
>
> - error = acpi_video_bus_get_devices(video, device);
> - if (error)
> - goto err_free_video;
> + acpi_video_bus_get_devices(video, device);
> + acpi_video_bus_start_devices(video);
>
> video->input = input = input_allocate_device();
> if (!input) {
> @@ -1695,13 +1692,9 @@ static int acpi_video_bus_add(struct acpi_device *device)
>
> video->pm_nb.notifier_call = acpi_video_resume;
> video->pm_nb.priority = 0;
> - error = register_pm_notifier(&video->pm_nb);
> - if (error)
> - goto err_stop_video;
>
> - error = input_register_device(input);
> - if (error)
> - goto err_unregister_pm_notifier;
> + register_pm_notifier(&video->pm_nb);
> + input_register_device(input);
>
> return 0;
>
>

2012-06-15 00:16:21

by Carlos R. Mafra

[permalink] [raw]
Subject: Re: 3.5.0-rc2 bisected regression: Can't change brightness on Vaio laptop

On Fri, 15 Jun 2012 at 2:26:41 +0400, Igor Murzov wrote:
> Please follow the discussion here:
> https://bugzilla.kernel.org/show_bug.cgi?id=43168

I tested the patch from Zhang Rui on comment #15 and it fixes
the issue in the latest mainline.


> > Commit ea9f8856bd6d4 ("ACPI video: Harden video bus adding") breaks the
> > backlight controls on my Vaio laptop.
> >
> > After this patch the folder /sys/class/backlight becomes empty and
> > I can't change the lcd brightness anymore.
> >
> > This regression is present in v3.4.2 as well as in the current
> > v3.5.0-rc2.
> >
> > I tried to manually revert that commit from v3.5.0-rc2, and the resulting
> > patch below fixes the issue for me.
> >
> > My config is here:
> > http://www.damtp.cam.ac.uk/user/crm66/config-3.5-rc1
> >
> > and the dmesg of v3.5.0-rc2 plus the patch below is here:
> > http://www.damtp.cam.ac.uk/user/crm66/dmesg_v3.5-rc2+patch.txt
> >
> > diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> > index a576575..4612b1c 100644
> > --- a/drivers/acpi/video.c
> > +++ b/drivers/acpi/video.c
> > @@ -548,27 +548,27 @@ acpi_video_device_EDID(struct acpi_video_device *device,
> > * 1. The system BIOS should NOT automatically control the brightness
> > * level of the LCD when the power changes from AC to DC.
> > * Return Value:
> > - * -EINVAL wrong arg.
> > + * -1 wrong arg.
> > */
> >
> > static int
> > acpi_video_bus_DOS(struct acpi_video_bus *video, int bios_flag, int lcd_flag)
> > {
> > - acpi_status status;
> > + u64 status = 0;
> > union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> > struct acpi_object_list args = { 1, &arg0 };
> >
> >
> > - if (bios_flag < 0 || bios_flag > 3 || lcd_flag < 0 || lcd_flag > 1)
> > - return -EINVAL;
> > + if (bios_flag < 0 || bios_flag > 3 || lcd_flag < 0 || lcd_flag > 1) {
> > + status = -1;
> > + goto Failed;
> > + }
> > arg0.integer.value = (lcd_flag << 2) | bios_flag;
> > video->dos_setting = arg0.integer.value;
> > - status = acpi_evaluate_object(video->device->handle, "_DOS",
> > - &args, NULL);
> > - if (ACPI_FAILURE(status))
> > - return -EIO;
> > + acpi_evaluate_object(video->device->handle, "_DOS", &args, NULL);
> >
> > - return 0;
> > + Failed:
> > + return status;
> > }
> >
> > /*
> > @@ -1343,17 +1343,15 @@ static int
> > acpi_video_bus_get_devices(struct acpi_video_bus *video,
> > struct acpi_device *device)
> > {
> > - int status;
> > + int status = 0;
> > struct acpi_device *dev;
> >
> > - status = acpi_video_device_enumerate(video);
> > - if (status)
> > - return status;
> > + acpi_video_device_enumerate(video);
> >
> > list_for_each_entry(dev, &device->children, node) {
> >
> > status = acpi_video_bus_get_one_device(dev, video);
> > - if (status) {
> > + if (ACPI_FAILURE(status)) {
> > printk(KERN_WARNING PREFIX
> > "Can't attach device\n");
> > continue;
> > @@ -1655,9 +1653,8 @@ static int acpi_video_bus_add(struct acpi_device *device)
> > mutex_init(&video->device_list_lock);
> > INIT_LIST_HEAD(&video->video_device_list);
> >
> > - error = acpi_video_bus_get_devices(video, device);
> > - if (error)
> > - goto err_free_video;
> > + acpi_video_bus_get_devices(video, device);
> > + acpi_video_bus_start_devices(video);
> >
> > video->input = input = input_allocate_device();
> > if (!input) {
> > @@ -1695,13 +1692,9 @@ static int acpi_video_bus_add(struct acpi_device *device)
> >
> > video->pm_nb.notifier_call = acpi_video_resume;
> > video->pm_nb.priority = 0;
> > - error = register_pm_notifier(&video->pm_nb);
> > - if (error)
> > - goto err_stop_video;
> >
> > - error = input_register_device(input);
> > - if (error)
> > - goto err_unregister_pm_notifier;
> > + register_pm_notifier(&video->pm_nb);
> > + input_register_device(input);
> >
> > return 0;
> >
> >

2012-06-21 03:00:39

by Linus Torvalds

[permalink] [raw]
Subject: Re: 3.5.0-rc2 bisected regression: Can't change brightness on Vaio laptop

Len,
I assume this regression fix is in some ACPI tree pending?

Linus

On Thu, Jun 14, 2012 at 5:16 PM, Carlos R. Mafra <[email protected]> wrote:
> On Fri, 15 Jun 2012 at ?2:26:41 +0400, Igor Murzov wrote:
>> Please follow the discussion here:
>> https://bugzilla.kernel.org/show_bug.cgi?id=43168
>
> I tested the patch from Zhang Rui on comment #15 and it fixes
> the issue in the latest mainline.