* [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
@ 2014-02-13 15:32 Hans de Goede
2014-02-13 23:47 ` Rafael J. Wysocki
2014-02-14 1:16 ` Aaron Lu
0 siblings, 2 replies; 9+ messages in thread
From: Hans de Goede @ 2014-02-13 15:32 UTC (permalink / raw)
To: Zhang Rui; +Cc: Len Brown, Rafael J. Wysocki, linux-acpi, Hans de Goede
Some devices have duplicate entries in there brightness levels table, ie
on my Dell Latitude E6430 the table looks like this:
[ 3.686060] acpi backlight index 0, val 80
[ 3.686095] acpi backlight index 1, val 50
[ 3.686122] acpi backlight index 2, val 5
[ 3.686147] acpi backlight index 3, val 5
[ 3.686172] acpi backlight index 4, val 5
[ 3.686197] acpi backlight index 5, val 5
[ 3.686223] acpi backlight index 6, val 5
[ 3.686248] acpi backlight index 7, val 5
[ 3.686273] acpi backlight index 8, val 6
[ 3.686332] acpi backlight index 9, val 7
[ 3.686356] acpi backlight index 10, val 8
[ 3.686380] acpi backlight index 11, val 9
etc.
Notice that brightness values 0-5 are all mapped to 5. This means that
if userspace writes any value between 0 and 5 to the brightness sysfs attribute
and then reads it, it will always return 0, which is somewhat unexpected.
This is a problem for ie gnome-settings-daemon, which uses read-modify-write
logic when the users presses the brightness up or down keys. This is done
this way to take brightness changes from other sources into account.
On this specific laptop what happens once the brightness has been set to 0,
is that gsd reads 0, adds 5, writes 5, and on the next brightness up key press
again reads 0, so things get stuck at the lowest brightness setting.
Filtering out the duplicate table entries, makes any write to brightness
read back as the written value as one would expect, fixing this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/video.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index b727d10..ea9d914 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -685,6 +685,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
union acpi_object *o;
struct acpi_video_device_brightness *br = NULL;
int result = -EINVAL;
+ u32 value;
if (!ACPI_SUCCESS(acpi_video_device_lcd_query_levels(device, &obj))) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Could not query available "
@@ -715,7 +716,12 @@ acpi_video_init_brightness(struct acpi_video_device *device)
printk(KERN_ERR PREFIX "Invalid data\n");
continue;
}
- br->levels[count] = (u32) o->integer.value;
+ value = (u32) o->integer.value;
+ /* Skip duplicate entries */
+ if (count > 2 && br->levels[count - 1] == value)
+ continue;
+
+ br->levels[count] = value;
if (br->levels[count] > max_level)
max_level = br->levels[count];
--
1.8.5.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
2014-02-13 15:32 [PATCH] acpi-video: Filter the BCL table for duplicate brightness values Hans de Goede
@ 2014-02-13 23:47 ` Rafael J. Wysocki
2014-02-14 1:16 ` Aaron Lu
1 sibling, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2014-02-13 23:47 UTC (permalink / raw)
To: Hans de Goede, Aaron Lu; +Cc: Zhang Rui, Len Brown, linux-acpi
On Thursday, February 13, 2014 04:32:51 PM Hans de Goede wrote:
> Some devices have duplicate entries in there brightness levels table, ie
> on my Dell Latitude E6430 the table looks like this:
>
> [ 3.686060] acpi backlight index 0, val 80
> [ 3.686095] acpi backlight index 1, val 50
> [ 3.686122] acpi backlight index 2, val 5
> [ 3.686147] acpi backlight index 3, val 5
> [ 3.686172] acpi backlight index 4, val 5
> [ 3.686197] acpi backlight index 5, val 5
> [ 3.686223] acpi backlight index 6, val 5
> [ 3.686248] acpi backlight index 7, val 5
> [ 3.686273] acpi backlight index 8, val 6
> [ 3.686332] acpi backlight index 9, val 7
> [ 3.686356] acpi backlight index 10, val 8
> [ 3.686380] acpi backlight index 11, val 9
> etc.
>
> Notice that brightness values 0-5 are all mapped to 5. This means that
> if userspace writes any value between 0 and 5 to the brightness sysfs attribute
> and then reads it, it will always return 0, which is somewhat unexpected.
>
> This is a problem for ie gnome-settings-daemon, which uses read-modify-write
> logic when the users presses the brightness up or down keys. This is done
> this way to take brightness changes from other sources into account.
>
> On this specific laptop what happens once the brightness has been set to 0,
> is that gsd reads 0, adds 5, writes 5, and on the next brightness up key press
> again reads 0, so things get stuck at the lowest brightness setting.
>
> Filtering out the duplicate table entries, makes any write to brightness
> read back as the written value as one would expect, fixing this.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Aaron, can you please have a look at this?
> ---
> drivers/acpi/video.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index b727d10..ea9d914 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -685,6 +685,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
> union acpi_object *o;
> struct acpi_video_device_brightness *br = NULL;
> int result = -EINVAL;
> + u32 value;
>
> if (!ACPI_SUCCESS(acpi_video_device_lcd_query_levels(device, &obj))) {
> ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Could not query available "
> @@ -715,7 +716,12 @@ acpi_video_init_brightness(struct acpi_video_device *device)
> printk(KERN_ERR PREFIX "Invalid data\n");
> continue;
> }
> - br->levels[count] = (u32) o->integer.value;
> + value = (u32) o->integer.value;
> + /* Skip duplicate entries */
> + if (count > 2 && br->levels[count - 1] == value)
> + continue;
> +
> + br->levels[count] = value;
>
> if (br->levels[count] > max_level)
> max_level = br->levels[count];
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
2014-02-13 15:32 [PATCH] acpi-video: Filter the BCL table for duplicate brightness values Hans de Goede
2014-02-13 23:47 ` Rafael J. Wysocki
@ 2014-02-14 1:16 ` Aaron Lu
2014-02-14 9:05 ` Hans de Goede
1 sibling, 1 reply; 9+ messages in thread
From: Aaron Lu @ 2014-02-14 1:16 UTC (permalink / raw)
To: Hans de Goede, Zhang Rui; +Cc: Len Brown, Rafael J. Wysocki, linux-acpi
On 02/13/2014 11:32 PM, Hans de Goede wrote:
> Some devices have duplicate entries in there brightness levels table, ie
> on my Dell Latitude E6430 the table looks like this:
For reference's purpose, can you please file a bug in kernel bugzilla
under the ACPI/Power-Video category and attach the acpidump there? Thanks.
-Aaron
>
> [ 3.686060] acpi backlight index 0, val 80
> [ 3.686095] acpi backlight index 1, val 50
> [ 3.686122] acpi backlight index 2, val 5
> [ 3.686147] acpi backlight index 3, val 5
> [ 3.686172] acpi backlight index 4, val 5
> [ 3.686197] acpi backlight index 5, val 5
> [ 3.686223] acpi backlight index 6, val 5
> [ 3.686248] acpi backlight index 7, val 5
> [ 3.686273] acpi backlight index 8, val 6
> [ 3.686332] acpi backlight index 9, val 7
> [ 3.686356] acpi backlight index 10, val 8
> [ 3.686380] acpi backlight index 11, val 9
> etc.
>
> Notice that brightness values 0-5 are all mapped to 5. This means that
> if userspace writes any value between 0 and 5 to the brightness sysfs attribute
> and then reads it, it will always return 0, which is somewhat unexpected.
>
> This is a problem for ie gnome-settings-daemon, which uses read-modify-write
> logic when the users presses the brightness up or down keys. This is done
> this way to take brightness changes from other sources into account.
>
> On this specific laptop what happens once the brightness has been set to 0,
> is that gsd reads 0, adds 5, writes 5, and on the next brightness up key press
> again reads 0, so things get stuck at the lowest brightness setting.
>
> Filtering out the duplicate table entries, makes any write to brightness
> read back as the written value as one would expect, fixing this.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/acpi/video.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index b727d10..ea9d914 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -685,6 +685,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
> union acpi_object *o;
> struct acpi_video_device_brightness *br = NULL;
> int result = -EINVAL;
> + u32 value;
>
> if (!ACPI_SUCCESS(acpi_video_device_lcd_query_levels(device, &obj))) {
> ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Could not query available "
> @@ -715,7 +716,12 @@ acpi_video_init_brightness(struct acpi_video_device *device)
> printk(KERN_ERR PREFIX "Invalid data\n");
> continue;
> }
> - br->levels[count] = (u32) o->integer.value;
> + value = (u32) o->integer.value;
> + /* Skip duplicate entries */
> + if (count > 2 && br->levels[count - 1] == value)
> + continue;
> +
> + br->levels[count] = value;
>
> if (br->levels[count] > max_level)
> max_level = br->levels[count];
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
2014-02-14 1:16 ` Aaron Lu
@ 2014-02-14 9:05 ` Hans de Goede
2014-02-14 14:53 ` Aaron Lu
0 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2014-02-14 9:05 UTC (permalink / raw)
To: Aaron Lu, Zhang Rui; +Cc: Len Brown, Rafael J. Wysocki, linux-acpi
Hi,
On 02/14/2014 02:16 AM, Aaron Lu wrote:
> On 02/13/2014 11:32 PM, Hans de Goede wrote:
>> Some devices have duplicate entries in there brightness levels table, ie
>> on my Dell Latitude E6430 the table looks like this:
>
> For reference's purpose, can you please file a bug in kernel bugzilla
> under the ACPI/Power-Video category and attach the acpidump there? Thanks.
Done:
https://bugzilla.kernel.org/show_bug.cgi?id=70571
Regards,
Hans
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
2014-02-14 9:05 ` Hans de Goede
@ 2014-02-14 14:53 ` Aaron Lu
2014-02-15 1:00 ` Rafael J. Wysocki
0 siblings, 1 reply; 9+ messages in thread
From: Aaron Lu @ 2014-02-14 14:53 UTC (permalink / raw)
To: Hans de Goede, Zhang Rui; +Cc: Len Brown, Rafael J. Wysocki, linux-acpi
On 02/14/2014 05:05 PM, Hans de Goede wrote:
> Hi,
>
> On 02/14/2014 02:16 AM, Aaron Lu wrote:
>> On 02/13/2014 11:32 PM, Hans de Goede wrote:
>>> Some devices have duplicate entries in there brightness levels table, ie
>>> on my Dell Latitude E6430 the table looks like this:
>>
>> For reference's purpose, can you please file a bug in kernel bugzilla
>> under the ACPI/Power-Video category and attach the acpidump there? Thanks.
>
> Done:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=70571
Thanks, and the patch looks good to me.
Reviewed-by: Aaron Lu <aaron.lu@intel.com>
-Aaron
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
2014-02-14 14:53 ` Aaron Lu
@ 2014-02-15 1:00 ` Rafael J. Wysocki
2014-02-15 8:49 ` Hans de Goede
2014-02-15 11:46 ` Aaron Lu
0 siblings, 2 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2014-02-15 1:00 UTC (permalink / raw)
To: Aaron Lu; +Cc: Hans de Goede, Zhang Rui, Len Brown, linux-acpi
On Friday, February 14, 2014 10:53:47 PM Aaron Lu wrote:
> On 02/14/2014 05:05 PM, Hans de Goede wrote:
> > Hi,
> >
> > On 02/14/2014 02:16 AM, Aaron Lu wrote:
> >> On 02/13/2014 11:32 PM, Hans de Goede wrote:
> >>> Some devices have duplicate entries in there brightness levels table, ie
> >>> on my Dell Latitude E6430 the table looks like this:
> >>
> >> For reference's purpose, can you please file a bug in kernel bugzilla
> >> under the ACPI/Power-Video category and attach the acpidump there? Thanks.
> >
> > Done:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=70571
>
> Thanks, and the patch looks good to me.
> Reviewed-by: Aaron Lu <aaron.lu@intel.com>
OK, thanks!
Is this stable material? If so, which stable kernels should it go into?
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
2014-02-15 1:00 ` Rafael J. Wysocki
@ 2014-02-15 8:49 ` Hans de Goede
2014-02-15 11:46 ` Aaron Lu
1 sibling, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2014-02-15 8:49 UTC (permalink / raw)
To: Rafael J. Wysocki, Aaron Lu; +Cc: Zhang Rui, Len Brown, linux-acpi
Hi,
On 02/15/2014 02:00 AM, Rafael J. Wysocki wrote:
> On Friday, February 14, 2014 10:53:47 PM Aaron Lu wrote:
>> On 02/14/2014 05:05 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 02/14/2014 02:16 AM, Aaron Lu wrote:
>>>> On 02/13/2014 11:32 PM, Hans de Goede wrote:
>>>>> Some devices have duplicate entries in there brightness levels table, ie
>>>>> on my Dell Latitude E6430 the table looks like this:
>>>>
>>>> For reference's purpose, can you please file a bug in kernel bugzilla
>>>> under the ACPI/Power-Video category and attach the acpidump there? Thanks.
>>>
>>> Done:
>>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=70571
>>
>> Thanks, and the patch looks good to me.
>> Reviewed-by: Aaron Lu <aaron.lu@intel.com>
>
> OK, thanks!
>
> Is this stable material? If so, which stable kernels should it go into?
Not sure if you're asking me or Aaron. I did not think about this before,
but I think adding this to stable makes sense. I'll let Aaron judge
how far it should be backported.
Regards,
Hans
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
2014-02-15 1:00 ` Rafael J. Wysocki
2014-02-15 8:49 ` Hans de Goede
@ 2014-02-15 11:46 ` Aaron Lu
2014-02-15 23:12 ` Rafael J. Wysocki
1 sibling, 1 reply; 9+ messages in thread
From: Aaron Lu @ 2014-02-15 11:46 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Hans de Goede, Zhang Rui, Len Brown, linux-acpi
On Sat, Feb 15, 2014 at 02:00:50AM +0100, Rafael J. Wysocki wrote:
> On Friday, February 14, 2014 10:53:47 PM Aaron Lu wrote:
> > On 02/14/2014 05:05 PM, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 02/14/2014 02:16 AM, Aaron Lu wrote:
> > >> On 02/13/2014 11:32 PM, Hans de Goede wrote:
> > >>> Some devices have duplicate entries in there brightness levels table, ie
> > >>> on my Dell Latitude E6430 the table looks like this:
> > >>
> > >> For reference's purpose, can you please file a bug in kernel bugzilla
> > >> under the ACPI/Power-Video category and attach the acpidump there? Thanks.
> > >
> > > Done:
> > >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=70571
> >
> > Thanks, and the patch looks good to me.
> > Reviewed-by: Aaron Lu <aaron.lu@intel.com>
>
> OK, thanks!
>
> Is this stable material? If so, which stable kernels should it go into?
Yes, all stables I think.
Thanks,
Aaron
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi-video: Filter the BCL table for duplicate brightness values
2014-02-15 11:46 ` Aaron Lu
@ 2014-02-15 23:12 ` Rafael J. Wysocki
0 siblings, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2014-02-15 23:12 UTC (permalink / raw)
To: Aaron Lu; +Cc: Hans de Goede, Zhang Rui, Len Brown, linux-acpi
On Saturday, February 15, 2014 07:46:41 PM Aaron Lu wrote:
> On Sat, Feb 15, 2014 at 02:00:50AM +0100, Rafael J. Wysocki wrote:
> > On Friday, February 14, 2014 10:53:47 PM Aaron Lu wrote:
> > > On 02/14/2014 05:05 PM, Hans de Goede wrote:
> > > > Hi,
> > > >
> > > > On 02/14/2014 02:16 AM, Aaron Lu wrote:
> > > >> On 02/13/2014 11:32 PM, Hans de Goede wrote:
> > > >>> Some devices have duplicate entries in there brightness levels table, ie
> > > >>> on my Dell Latitude E6430 the table looks like this:
> > > >>
> > > >> For reference's purpose, can you please file a bug in kernel bugzilla
> > > >> under the ACPI/Power-Video category and attach the acpidump there? Thanks.
> > > >
> > > > Done:
> > > >
> > > > https://bugzilla.kernel.org/show_bug.cgi?id=70571
> > >
> > > Thanks, and the patch looks good to me.
> > > Reviewed-by: Aaron Lu <aaron.lu@intel.com>
> >
> > OK, thanks!
> >
> > Is this stable material? If so, which stable kernels should it go into?
>
> Yes, all stables I think.
OK, thanks! Queued up for the next PM/ACPI pull request.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-02-15 22:57 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-13 15:32 [PATCH] acpi-video: Filter the BCL table for duplicate brightness values Hans de Goede
2014-02-13 23:47 ` Rafael J. Wysocki
2014-02-14 1:16 ` Aaron Lu
2014-02-14 9:05 ` Hans de Goede
2014-02-14 14:53 ` Aaron Lu
2014-02-15 1:00 ` Rafael J. Wysocki
2014-02-15 8:49 ` Hans de Goede
2014-02-15 11:46 ` Aaron Lu
2014-02-15 23:12 ` Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.