linux-next 20160512 - ACPI issue with screen brightness

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

linux-next 20160512 - ACPI issue with screen brightness

Valdis.Kletnieks
next-20160512 sets the screen brightness to about 40%-ish or so, rather
than the 100% intensity I want.

Dell Latitude E6530 laptop.

git bisect tells me:

059500940defe285222d3b189b366dfe7f299cae is the first bad commit
commit 059500940defe285222d3b189b366dfe7f299cae
Author: Aaron Lu <[hidden email]>
Date:   Wed Apr 27 20:45:04 2016 +0800

    ACPI/video: export acpi_video_get_levels

    The acpi_video_get_levels is useful for other drivers, i.e. the
    to-be-added int3406 thermal driver, so export it.

    Signed-off-by: Aaron Lu <[hidden email]>
    Signed-off-by: Rafael J. Wysocki <[hidden email]>

:040000 040000 24f4f5abe8beda2fa219dee7549faacc2f63e29f dfbe3a3bb4f58d82d5be768263d91172276fed98 M      drivers
:040000 040000 603ffbf36e76717fe4f4ecb0418c9dbc59c76e25 a559bcd81c949a6471ffd045866df9b4e3fa4406 M      include


but I've stared at the code and don't see what would do this....


attachment0 (866 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Rafael J. Wysocki-5
On Tue, May 17, 2016 at 8:41 PM, Valdis Kletnieks
<[hidden email]> wrote:

> next-20160512 sets the screen brightness to about 40%-ish or so, rather
> than the 100% intensity I want.
>
> Dell Latitude E6530 laptop.
>
> git bisect tells me:
>
> 059500940defe285222d3b189b366dfe7f299cae is the first bad commit
> commit 059500940defe285222d3b189b366dfe7f299cae
> Author: Aaron Lu <[hidden email]>
> Date:   Wed Apr 27 20:45:04 2016 +0800
>
>     ACPI/video: export acpi_video_get_levels
>
>     The acpi_video_get_levels is useful for other drivers, i.e. the
>     to-be-added int3406 thermal driver, so export it.
>
>     Signed-off-by: Aaron Lu <[hidden email]>
>     Signed-off-by: Rafael J. Wysocki <[hidden email]>
>
> :040000 040000 24f4f5abe8beda2fa219dee7549faacc2f63e29f dfbe3a3bb4f58d82d5be768263d91172276fed98 M      drivers
> :040000 040000 603ffbf36e76717fe4f4ecb0418c9dbc59c76e25 a559bcd81c949a6471ffd045866df9b4e3fa4406 M      include
>
>
> but I've stared at the code and don't see what would do this....

Please try to check out the acpi-video branch from linux-pm.git and
see if the problem is present then.  If it is, please revert all of
the top-most commits up to and including the above one and see if the
problem goes away.
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Valdis.Kletnieks
On Tue, 17 May 2016 22:50:11 +0200, "Rafael J. Wysocki" said:

> On Tue, May 17, 2016 at 8:41 PM, Valdis Kletnieks <[hidden email]> wrote:
> > next-20160512 sets the screen brightness to about 40%-ish or so, rather
> > than the 100% intensity I want.
> >
> > Dell Latitude E6530 laptop.
> >
> > git bisect tells me:
> >
> > 059500940defe285222d3b189b366dfe7f299cae is the first bad commit
> > commit 059500940defe285222d3b189b366dfe7f299cae
> > Author: Aaron Lu <[hidden email]>
> > Date:   Wed Apr 27 20:45:04 2016 +0800
> >
> >     ACPI/video: export acpi_video_get_levels
> >
> >     The acpi_video_get_levels is useful for other drivers, i.e. the
> >     to-be-added int3406 thermal driver, so export it.
> >
> >     Signed-off-by: Aaron Lu <[hidden email]>
> >     Signed-off-by: Rafael J. Wysocki <[hidden email]>
> >
> > :040000 040000 24f4f5abe8beda2fa219dee7549faacc2f63e29fdfbe3a3bb4f58d82d5be768263d91172276fed98 M      drivers
> > :040000 040000 603ffbf36e76717fe4f4ecb0418c9dbc59c76e25a559bcd81c949a6471ffd045866df9b4e3fa4406 M      include
> >
> >
> > but I've stared at the code and don't see what would do this....
>
> Please try to check out the acpi-video branch from linux-pm.git and
> see if the problem is present then.  If it is, please revert all of
> the top-most commits up to and including the above one and see if the
> problem goes away.
Put this one in the "things that go bump in the night" pile - the problem
doesn't manifest on next-20160519, even though the commit I bisected to
is in the tree for today, and I don't see any obvious smoking guns to
have fixed it in the past week's worth of 'git log'....


attachment0 (866 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Valdis.Kletnieks
On Thu, 19 May 2016 18:53:17 -0400, [hidden email] said:

> > > next-20160512 sets the screen brightness to about 40%-ish or so, rather
> > > than the 100% intensity I want.

> Put this one in the "things that go bump in the night" pile - the problem
> doesn't manifest on next-20160519, even though the commit I bisected to
> is in the tree for today, and I don't see any obvious smoking guns to
> have fixed it in the past week's worth of 'git log'....

Actually, put it in "things that go bump in the night but need a stake driven
through them"- it looks like I booted the wrong kernel while testing, and in
fact next-20160519 *is* still broken.  However, after:

git revert -n e4f35c1339f0cfcf38d3f63dd6fea2b070399263
git revert -n 059500940defe285222d3b189b366dfe7f299cae

Things work again.

attachment0 (866 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Aaron Lu
On 05/20/2016 11:05 AM, [hidden email] wrote:
> On Thu, 19 May 2016 18:53:17 -0400, [hidden email] said:
>
>>>> next-20160512 sets the screen brightness to about 40%-ish or so, rather
>>>> than the 100% intensity I want.

Do you mean after boot, the screen brightness is now 40% instead of the
previous 100%? Are you using a GUI? If so, please boot into console mode
to see if this is still the case as GUI sometimes will change backlight
levels so better isolate its impact when testing.

And please list your backlight interfaces:
# ls /sys/class/backlight
I assume it is acpi_video0 that is controlling your backlight levels,
can you please check if it is still working well?
# cd /sys/class/backlight/acpi_video0
# cat max_brightness
XXX
# echo a_value_smaller_or_euqal_to_XXX > brightness

BTW, do you see any error messages in your dmesg?
Probably you can file a new bug at https://bugzilla.kernel.org under the
ACPI/Power-Video category and attach your dmesg/acpidump there, thanks.
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Valdis.Kletnieks
On Fri, 20 May 2016 13:45:30 +0800, Aaron Lu said:
> On 05/20/2016 11:05 AM, [hidden email] wrote:
> > On Thu, 19 May 2016 18:53:17 -0400, [hidden email] said:
> >
> >>>> next-20160512 sets the screen brightness to about 40%-ish or so, rather
> >>>> than the 100% intensity I want.
>
> Do you mean after boot, the screen brightness is now 40% instead of the
> previous 100%? Are you using a GUI?

Nope, even the very first line of output from initramfs is dim, and if I then
reboot and go into the BIOS settings, the screen intensity is at 40%. While
it's rebooting, the Dell bios splash will start off bright and then suddenly
dim down.

With the patches reverted:

[/sys/class/backlight/acpi_video0] grep . *bright*
actual_brightness:95
brightness:95
max_brightness:95

And the weird part inside the kernel - on a kernel that has the problem,
/sys/class/backlight is *empty* - only '.' and '..' entries.

No, I don't understand why the acpi_video0 entry isn't created when that
commit is in place.



attachment0 (866 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Aaron Lu
On Fri, May 20, 2016 at 03:02:08AM -0400, [hidden email] wrote:

> On Fri, 20 May 2016 13:45:30 +0800, Aaron Lu said:
> > On 05/20/2016 11:05 AM, [hidden email] wrote:
> > > On Thu, 19 May 2016 18:53:17 -0400, [hidden email] said:
> > >
> > >>>> next-20160512 sets the screen brightness to about 40%-ish or so, rather
> > >>>> than the 100% intensity I want.
> >
> > Do you mean after boot, the screen brightness is now 40% instead of the
> > previous 100%? Are you using a GUI?
>
> Nope, even the very first line of output from initramfs is dim, and if I then
> reboot and go into the BIOS settings, the screen intensity is at 40%. While
> it's rebooting, the Dell bios splash will start off bright and then suddenly
> dim down.
>
> With the patches reverted:
>
> [/sys/class/backlight/acpi_video0] grep . *bright*
> actual_brightness:95
> brightness:95
> max_brightness:95
>
> And the weird part inside the kernel - on a kernel that has the problem,
> /sys/class/backlight is *empty* - only '.' and '..' entries.
>
> No, I don't understand why the acpi_video0 entry isn't created when that
> commit is in place.

The commit probably makes acpi_video_init_brightness bail out for some
reason, adding debug prints under those "if (result)" in
acpi_video_init_brightness should help to identify where it goes wrong.

And your acpidump please, thanks.
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Aaron Lu
On Fri, May 20, 2016 at 03:17:20PM +0800, Aaron Lu wrote:

> On Fri, May 20, 2016 at 03:02:08AM -0400, [hidden email] wrote:
> > On Fri, 20 May 2016 13:45:30 +0800, Aaron Lu said:
> > > On 05/20/2016 11:05 AM, [hidden email] wrote:
> > > > On Thu, 19 May 2016 18:53:17 -0400, [hidden email] said:
> > > >
> > > >>>> next-20160512 sets the screen brightness to about 40%-ish or so, rather
> > > >>>> than the 100% intensity I want.
> > >
> > > Do you mean after boot, the screen brightness is now 40% instead of the
> > > previous 100%? Are you using a GUI?
> >
> > Nope, even the very first line of output from initramfs is dim, and if I then
> > reboot and go into the BIOS settings, the screen intensity is at 40%. While
> > it's rebooting, the Dell bios splash will start off bright and then suddenly
> > dim down.
> >
> > With the patches reverted:
> >
> > [/sys/class/backlight/acpi_video0] grep . *bright*
> > actual_brightness:95
> > brightness:95
> > max_brightness:95
> >
> > And the weird part inside the kernel - on a kernel that has the problem,
> > /sys/class/backlight is *empty* - only '.' and '..' entries.
> >
> > No, I don't understand why the acpi_video0 entry isn't created when that
> > commit is in place.
>
> The commit probably makes acpi_video_init_brightness bail out for some
> reason, adding debug prints under those "if (result)" in
> acpi_video_init_brightness should help to identify where it goes wrong.

Like the below one:

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 3d5b8a099351..69b321580fa3 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -323,8 +323,10 @@ acpi_video_device_lcd_query_levels(acpi_handle handle,
  *levels = NULL;
 
  status = acpi_evaluate_object(handle, "_BCL", NULL, &buffer);
- if (!ACPI_SUCCESS(status))
+ if (!ACPI_SUCCESS(status)) {
+ pr_err("acpi_evaluate_BCL failed, %d\n", status);
  return status;
+ }
  obj = (union acpi_object *)buffer.pointer;
  if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) {
  printk(KERN_ERR PREFIX "Invalid _BCL data\n");
@@ -765,13 +767,13 @@ int acpi_video_get_levels(struct acpi_device *device,
 
  if (!ACPI_SUCCESS(acpi_video_device_lcd_query_levels(device->handle,
  &obj))) {
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Could not query available "
- "LCD brightness level\n"));
+ pr_err("Could not query available LCD brightness level\n");
  result = -ENODEV;
  goto out;
  }
 
  if (obj->package.count < 2) {
+ pr_err("_BCL count smaller than 2, %d\n", obj->package.count);
  result = -EINVAL;
  goto out;
  }
@@ -786,6 +788,7 @@ int acpi_video_get_levels(struct acpi_device *device,
  br->levels = kmalloc((obj->package.count + 2) * sizeof *(br->levels),
  GFP_KERNEL);
  if (!br->levels) {
+ pr_err("kmalloc for br->levels failed\n");
  result = -ENOMEM;
  goto out_free;
  }
@@ -870,8 +873,10 @@ acpi_video_init_brightness(struct acpi_video_device *device)
  int result = -EINVAL;
 
  result = acpi_video_get_levels(device->dev, &br);
- if (result)
+ if (result) {
+ pr_err("acpi_video_get_levels failed, %d\n", result);
  return result;
+ }
  device->brightness = br;
 
  /* _BQC uses INDEX while _BCL uses VALUE in some laptops */
@@ -882,12 +887,16 @@ acpi_video_init_brightness(struct acpi_video_device *device)
 
  result = acpi_video_device_lcd_get_level_current(device,
  &level_old, true);
- if (result)
+ if (result) {
+ pr_err("acpi_video_device_lcd_get_level_current failed, %d\n", result);
  goto out_free_levels;
+ }
 
  result = acpi_video_bqc_quirk(device, max_level, level_old);
- if (result)
+ if (result) {
+ pr_err("acpi_video_bqc_quirk failed, %d\n", result);
  goto out_free_levels;
+ }
  /*
  * cap._BQC may get cleared due to _BQC is found to be broken
  * in acpi_video_bqc_quirk, so check again here.
@@ -910,11 +919,12 @@ acpi_video_init_brightness(struct acpi_video_device *device)
 
 set_level:
  result = acpi_video_device_lcd_set_level(device, level);
- if (result)
+ if (result) {
+ pr_err("acpi_video_device_lcd_set_level failed, %d\n", result);
  goto out_free_levels;
+ }
 
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-  "found %d brightness levels\n", br->count - 2));
+ pr_info("found %d brightness levels\n", br->count - 2);
  return 0;
 
 out_free_levels:
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Valdis.Kletnieks
In reply to this post by Aaron Lu
On Fri, 20 May 2016 15:17:20 +0800, Aaron Lu said:

> On Fri, May 20, 2016 at 03:02:08AM -0400, [hidden email] wrote:
> > With the patches reverted:
> >
> > [/sys/class/backlight/acpi_video0] grep . *bright*
> > actual_brightness:95
> > brightness:95
> > max_brightness:95
> >
> > And the weird part inside the kernel - on a kernel that has the problem,
> > /sys/class/backlight is *empty* - only '.' and '..' entries.
> >
> > No, I don't understand why the acpi_video0 entry isn't created when that
> > commit is in place.
>
> The commit probably makes acpi_video_init_brightness bail out for some
> reason, adding debug prints under those "if (result)" in
> acpi_video_init_brightness should help to identify where it goes wrong.
>
> And your acpidump please, thanks.
The dmesg output:

[    1.966400] ACPI: Power Button [PWRF]
[    1.969759] ACPI: Video Device [VID] (multi-head: yes  rom: yes  post: no)
[    1.969829] acpi_evaluate_BCL failed, 4097
[    1.969866] acpi_evaluate_BCL failed, 4097
[    1.969898] acpi_evaluate_BCL failed, 4097
[    1.969930] acpi_evaluate_BCL failed, 4097
[    1.969961] acpi_evaluate_BCL failed, 4097
[    1.969994] acpi_evaluate_BCL failed, 4097
[    1.970045] acpi_evaluate_BCL failed, 4097
[    1.970077] acpi_evaluate_BCL failed, 4097
[    1.970621] acpi_evaluate_BCL failed, 5
[    1.970652] Could not query available LCD brightness level
[    1.970692] acpi_video_get_levels failed, -19
[    1.973516] ACPI Error: Current brightness invalid (20160422/video-368)
[    1.973528] acpi_video_bqc_quirk failed, -22
[    1.973567] acpi_evaluate_BCL failed, 5
[    1.973597] Could not query available LCD brightness level
[    1.973636] acpi_video_get_levels failed, -19
[    1.973672] acpi_evaluate_BCL failed, 5
[    1.973701] Could not query available LCD brightness level
[    1.973741] acpi_video_get_levels failed, -19
[    1.973777] acpi_evaluate_BCL failed, 5
[    1.973807] Could not query available LCD brightness level
[    1.973847] acpi_video_get_levels failed, -19
[    1.973882] acpi_evaluate_BCL failed, 5
[    1.973911] Could not query available LCD brightness level
[    1.973951] acpi_video_get_levels failed, -19
[    1.973986] acpi_evaluate_BCL failed, 5
[    1.974439] Could not query available LCD brightness level
[    1.974479] acpi_video_get_levels failed, -19
[    1.974529] acpi_evaluate_BCL failed, 5
[    1.974565] Could not query available LCD brightness level
[    1.974603] acpi_video_get_levels failed, -19
[    1.977783] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:14/LNXVIDEO:00/input/input4
[    1.980879] thermal LNXTHERM:00: registered as thermal_zone0
[    1.980884] ACPI: Thermal Zone [THM] (64 C)

acpidump is attached...

acpidump.e6530.a17 (276K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: linux-next 20160512 - ACPI issue with screen brightness

Aaron Lu
On Fri, May 20, 2016 at 08:15:12PM -0400, [hidden email] wrote:

> On Fri, 20 May 2016 15:17:20 +0800, Aaron Lu said:
> > On Fri, May 20, 2016 at 03:02:08AM -0400, [hidden email] wrote:
> > > With the patches reverted:
> > >
> > > [/sys/class/backlight/acpi_video0] grep . *bright*
> > > actual_brightness:95
> > > brightness:95
> > > max_brightness:95
> > >
> > > And the weird part inside the kernel - on a kernel that has the problem,
> > > /sys/class/backlight is *empty* - only '.' and '..' entries.
> > >
> > > No, I don't understand why the acpi_video0 entry isn't created when that
> > > commit is in place.
> >
> > The commit probably makes acpi_video_init_brightness bail out for some
> > reason, adding debug prints under those "if (result)" in
> > acpi_video_init_brightness should help to identify where it goes wrong.
> >
> > And your acpidump please, thanks.
>
> The dmesg output:
>
> [    1.966400] ACPI: Power Button [PWRF]
> [    1.969759] ACPI: Video Device [VID] (multi-head: yes  rom: yes  post: no)
> [    1.969829] acpi_evaluate_BCL failed, 4097
> [    1.969866] acpi_evaluate_BCL failed, 4097
> [    1.969898] acpi_evaluate_BCL failed, 4097
> [    1.969930] acpi_evaluate_BCL failed, 4097
> [    1.969961] acpi_evaluate_BCL failed, 4097
> [    1.969994] acpi_evaluate_BCL failed, 4097
> [    1.970045] acpi_evaluate_BCL failed, 4097
> [    1.970077] acpi_evaluate_BCL failed, 4097

My bad, I see the problem here: the acpi_video_device_lcd_query_levels
called in acpi_video_run_bcl_for_osi didn't do the proper conversion.
The acpi_handle is a "void *" so there is no warnings...

> [    1.970621] acpi_evaluate_BCL failed, 5

This error messages means there is no _BCL but the acpidump shows that
there is.

Anyway, please apply this patch on top of the existing one and attach
dmesg, thanks, this can at least fix the above 4097 errors and let's see
if things start to change:

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 69b321580fa3..bdef49074372 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -765,6 +765,9 @@ int acpi_video_get_levels(struct acpi_device *device,
  int result = 0;
  u32 value;
 
+ pr_info("%s for device %s, handle %p\n", __func__, dev_name(&device->dev),
+ device->handle);
+
  if (!ACPI_SUCCESS(acpi_video_device_lcd_query_levels(device->handle,
  &obj))) {
  pr_err("Could not query available LCD brightness level\n");
@@ -1747,7 +1750,8 @@ static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
 
  mutex_lock(&video->device_list_lock);
  list_for_each_entry(dev, &video->video_device_list, entry) {
- if (!acpi_video_device_lcd_query_levels(dev, &levels))
+ if (!acpi_video_device_lcd_query_levels(dev->dev->handle,
+ &levels))
  kfree(levels);
  }
  mutex_unlock(&video->device_list_lock);

-Aaron

> [    1.970652] Could not query available LCD brightness level
> [    1.970692] acpi_video_get_levels failed, -19
> [    1.973516] ACPI Error: Current brightness invalid (20160422/video-368)
> [    1.973528] acpi_video_bqc_quirk failed, -22
> [    1.973567] acpi_evaluate_BCL failed, 5
> [    1.973597] Could not query available LCD brightness level
> [    1.973636] acpi_video_get_levels failed, -19
> [    1.973672] acpi_evaluate_BCL failed, 5
> [    1.973701] Could not query available LCD brightness level
> [    1.973741] acpi_video_get_levels failed, -19
> [    1.973777] acpi_evaluate_BCL failed, 5
> [    1.973807] Could not query available LCD brightness level
> [    1.973847] acpi_video_get_levels failed, -19
> [    1.973882] acpi_evaluate_BCL failed, 5
> [    1.973911] Could not query available LCD brightness level
> [    1.973951] acpi_video_get_levels failed, -19
> [    1.973986] acpi_evaluate_BCL failed, 5
> [    1.974439] Could not query available LCD brightness level
> [    1.974479] acpi_video_get_levels failed, -19
> [    1.974529] acpi_evaluate_BCL failed, 5
> [    1.974565] Could not query available LCD brightness level
> [    1.974603] acpi_video_get_levels failed, -19
> [    1.977783] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:14/LNXVIDEO:00/input/input4
> [    1.980879] thermal LNXTHERM:00: registered as thermal_zone0
> [    1.980884] ACPI: Thermal Zone [THM] (64 C)
Reply | Threaded
Open this post in threaded view
|

[PATCH] ACPI / Thermal / video: fix max_level incorrect value

Aaron Lu
On Sat, May 21, 2016 at 11:29:33AM +0800, Aaron Lu wrote:
> My bad, I see the problem here: the acpi_video_device_lcd_query_levels
> called in acpi_video_run_bcl_for_osi didn't do the proper conversion.
> The acpi_handle is a "void *" so there is no warnings...

I think I have found the problem, please give the patch a test, thanks.

From: Aaron Lu <[hidden email]>
Date: Sat, 21 May 2016 15:30:46 +0800
Subject: [PATCH] ACPI / Thermal / video: fix max_level incorrect value

commit 059500940def("ACPI/video: export acpi_video_get_levels")
mistakenly dropped the correct value of max_level and that caused the
set_level function following failed and the acpi_video backlight interface
didn't get created. Fix this by passing back the correct max_level value.

While at it, also fix the param used in acpi_video_device_lcd_query_levels
where acpi_handle is expected but acpi_video_device is passed.

Reported-by: Valdis Kletnieks <[hidden email]>
Signed-off-by: Aaron Lu <[hidden email]>
---
 drivers/acpi/acpi_video.c                         | 9 ++++++---
 drivers/thermal/int340x_thermal/int3406_thermal.c | 2 +-
 include/acpi/video.h                              | 6 ++++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 3d5b8a099351..c1d138e128cb 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -754,7 +754,8 @@ static int acpi_video_bqc_quirk(struct acpi_video_device *device,
 }
 
 int acpi_video_get_levels(struct acpi_device *device,
-  struct acpi_video_device_brightness **dev_br)
+  struct acpi_video_device_brightness **dev_br,
+  int *pmax_level)
 {
  union acpi_object *obj = NULL;
  int i, max_level = 0, count = 0, level_ac_battery = 0;
@@ -841,6 +842,8 @@ int acpi_video_get_levels(struct acpi_device *device,
 
  br->count = count;
  *dev_br = br;
+ if (pmax_level)
+ *pmax_level = max_level;
 
 out:
  kfree(obj);
@@ -869,7 +872,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
  struct acpi_video_device_brightness *br = NULL;
  int result = -EINVAL;
 
- result = acpi_video_get_levels(device->dev, &br);
+ result = acpi_video_get_levels(device->dev, &br, &max_level);
  if (result)
  return result;
  device->brightness = br;
@@ -1737,7 +1740,7 @@ static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
 
  mutex_lock(&video->device_list_lock);
  list_for_each_entry(dev, &video->video_device_list, entry) {
- if (!acpi_video_device_lcd_query_levels(dev, &levels))
+ if (!acpi_video_device_lcd_query_levels(dev->dev->handle, &levels))
  kfree(levels);
  }
  mutex_unlock(&video->device_list_lock);
diff --git a/drivers/thermal/int340x_thermal/int3406_thermal.c b/drivers/thermal/int340x_thermal/int3406_thermal.c
index 13d431cbd29e..a578cd257db4 100644
--- a/drivers/thermal/int340x_thermal/int3406_thermal.c
+++ b/drivers/thermal/int340x_thermal/int3406_thermal.c
@@ -177,7 +177,7 @@ static int int3406_thermal_probe(struct platform_device *pdev)
  return -ENODEV;
  d->raw_bd = bd;
 
- ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br);
+ ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br, NULL);
  if (ret)
  return ret;
 
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 70a41f742037..5731ccb42585 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -51,7 +51,8 @@ extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type);
  */
 extern bool acpi_video_handles_brightness_key_presses(void);
 extern int acpi_video_get_levels(struct acpi_device *device,
- struct acpi_video_device_brightness **dev_br);
+ struct acpi_video_device_brightness **dev_br,
+ int *pmax_level);
 #else
 static inline int acpi_video_register(void) { return 0; }
 static inline void acpi_video_unregister(void) { return; }
@@ -72,7 +73,8 @@ static inline bool acpi_video_handles_brightness_key_presses(void)
  return false;
 }
 static inline int acpi_video_get_levels(struct acpi_device *device,
- struct acpi_video_device_brightness **dev_br)
+ struct acpi_video_device_brightness **dev_br,
+ int *pmax_level)
 {
  return -ENODEV;
 }
--
2.5.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ACPI / Thermal / video: fix max_level incorrect value

Aaron Lu
Valdis, can you please give the patch a try? Thanks.

-Aaron

On Sat, May 21, 2016 at 03:55:00PM +0800, Aaron Lu wrote:

> I think I have found the problem, please give the patch a test, thanks.
>
> From: Aaron Lu <[hidden email]>
> Date: Sat, 21 May 2016 15:30:46 +0800
> Subject: [PATCH] ACPI / Thermal / video: fix max_level incorrect value
>
> commit 059500940def("ACPI/video: export acpi_video_get_levels")
> mistakenly dropped the correct value of max_level and that caused the
> set_level function following failed and the acpi_video backlight interface
> didn't get created. Fix this by passing back the correct max_level value.
>
> While at it, also fix the param used in acpi_video_device_lcd_query_levels
> where acpi_handle is expected but acpi_video_device is passed.
>
> Reported-by: Valdis Kletnieks <[hidden email]>
> Signed-off-by: Aaron Lu <[hidden email]>
> ---
>  drivers/acpi/acpi_video.c                         | 9 ++++++---
>  drivers/thermal/int340x_thermal/int3406_thermal.c | 2 +-
>  include/acpi/video.h                              | 6 ++++--
>  3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 3d5b8a099351..c1d138e128cb 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -754,7 +754,8 @@ static int acpi_video_bqc_quirk(struct acpi_video_device *device,
>  }
>  
>  int acpi_video_get_levels(struct acpi_device *device,
> -  struct acpi_video_device_brightness **dev_br)
> +  struct acpi_video_device_brightness **dev_br,
> +  int *pmax_level)
>  {
>   union acpi_object *obj = NULL;
>   int i, max_level = 0, count = 0, level_ac_battery = 0;
> @@ -841,6 +842,8 @@ int acpi_video_get_levels(struct acpi_device *device,
>  
>   br->count = count;
>   *dev_br = br;
> + if (pmax_level)
> + *pmax_level = max_level;
>  
>  out:
>   kfree(obj);
> @@ -869,7 +872,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
>   struct acpi_video_device_brightness *br = NULL;
>   int result = -EINVAL;
>  
> - result = acpi_video_get_levels(device->dev, &br);
> + result = acpi_video_get_levels(device->dev, &br, &max_level);
>   if (result)
>   return result;
>   device->brightness = br;
> @@ -1737,7 +1740,7 @@ static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
>  
>   mutex_lock(&video->device_list_lock);
>   list_for_each_entry(dev, &video->video_device_list, entry) {
> - if (!acpi_video_device_lcd_query_levels(dev, &levels))
> + if (!acpi_video_device_lcd_query_levels(dev->dev->handle, &levels))
>   kfree(levels);
>   }
>   mutex_unlock(&video->device_list_lock);
> diff --git a/drivers/thermal/int340x_thermal/int3406_thermal.c b/drivers/thermal/int340x_thermal/int3406_thermal.c
> index 13d431cbd29e..a578cd257db4 100644
> --- a/drivers/thermal/int340x_thermal/int3406_thermal.c
> +++ b/drivers/thermal/int340x_thermal/int3406_thermal.c
> @@ -177,7 +177,7 @@ static int int3406_thermal_probe(struct platform_device *pdev)
>   return -ENODEV;
>   d->raw_bd = bd;
>  
> - ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br);
> + ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br, NULL);
>   if (ret)
>   return ret;
>  
> diff --git a/include/acpi/video.h b/include/acpi/video.h
> index 70a41f742037..5731ccb42585 100644
> --- a/include/acpi/video.h
> +++ b/include/acpi/video.h
> @@ -51,7 +51,8 @@ extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type);
>   */
>  extern bool acpi_video_handles_brightness_key_presses(void);
>  extern int acpi_video_get_levels(struct acpi_device *device,
> - struct acpi_video_device_brightness **dev_br);
> + struct acpi_video_device_brightness **dev_br,
> + int *pmax_level);
>  #else
>  static inline int acpi_video_register(void) { return 0; }
>  static inline void acpi_video_unregister(void) { return; }
> @@ -72,7 +73,8 @@ static inline bool acpi_video_handles_brightness_key_presses(void)
>   return false;
>  }
>  static inline int acpi_video_get_levels(struct acpi_device *device,
> - struct acpi_video_device_brightness **dev_br)
> + struct acpi_video_device_brightness **dev_br,
> + int *pmax_level)
>  {
>   return -ENODEV;
>  }
> --
> 2.5.5
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ACPI / Thermal / video: fix max_level incorrect value

Valdis.Kletnieks
On Wed, 25 May 2016 13:15:26 +0800, Aaron Lu said:
> Valdis, can you please give the patch a try? Thanks.

Sorry, had a few days where actual work commitments and other
things got in the way... I tested this patch against next-20160524,
and can report that the problem is fixed, so feel free to
stick a "Tested-By:" on it....

> > Reported-by: Valdis Kletnieks <[hidden email]>
> > Signed-off-by: Aaron Lu <[hidden email]>
> > ---
> >  drivers/acpi/acpi_video.c                         | 9 ++++++---
> >  drivers/thermal/int340x_thermal/int3406_thermal.c | 2 +-
> >  include/acpi/video.h                              | 6 ++++--
> >  3 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> > index 3d5b8a099351..c1d138e128cb 100644
> > --- a/drivers/acpi/acpi_video.c
> > +++ b/drivers/acpi/acpi_video.c


attachment0 (866 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ACPI / Thermal / video: fix max_level incorrect value

Aaron Lu
On 05/26/2016 09:49 AM, [hidden email] wrote:
> On Wed, 25 May 2016 13:15:26 +0800, Aaron Lu said:
>> Valdis, can you please give the patch a try? Thanks.
>
> Sorry, had a few days where actual work commitments and other
> things got in the way... I tested this patch against next-20160524,
> and can report that the problem is fixed, so feel free to
> stick a "Tested-By:" on it....

Thanks a lot for the confirm, here is the updated patch with your
tested-by tag added.

From: Aaron Lu <[hidden email]>
Date: Sat, 21 May 2016 15:30:46 +0800
Subject: [PATCH] ACPI / Thermal / video: fix max_level incorrect value

commit 059500940def("ACPI/video: export acpi_video_get_levels")
mistakenly dropped the correct value of max_level and that caused the
set_level function following failed and the acpi_video backlight interface
didn't get created. Fix this by passing back the correct max_level value.

While at it, also fix the param used in acpi_video_device_lcd_query_levels
where acpi_handle is expected but acpi_video_device is passed.

Reported-and-tested-by: Valdis Kletnieks <[hidden email]>
Signed-off-by: Aaron Lu <[hidden email]>
---
 drivers/acpi/acpi_video.c                         | 9 ++++++---
 drivers/thermal/int340x_thermal/int3406_thermal.c | 2 +-
 include/acpi/video.h                              | 6 ++++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 3d5b8a099351..c1d138e128cb 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -754,7 +754,8 @@ static int acpi_video_bqc_quirk(struct acpi_video_device *device,
 }
 
 int acpi_video_get_levels(struct acpi_device *device,
-  struct acpi_video_device_brightness **dev_br)
+  struct acpi_video_device_brightness **dev_br,
+  int *pmax_level)
 {
  union acpi_object *obj = NULL;
  int i, max_level = 0, count = 0, level_ac_battery = 0;
@@ -841,6 +842,8 @@ int acpi_video_get_levels(struct acpi_device *device,
 
  br->count = count;
  *dev_br = br;
+ if (pmax_level)
+ *pmax_level = max_level;
 
 out:
  kfree(obj);
@@ -869,7 +872,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
  struct acpi_video_device_brightness *br = NULL;
  int result = -EINVAL;
 
- result = acpi_video_get_levels(device->dev, &br);
+ result = acpi_video_get_levels(device->dev, &br, &max_level);
  if (result)
  return result;
  device->brightness = br;
@@ -1737,7 +1740,7 @@ static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
 
  mutex_lock(&video->device_list_lock);
  list_for_each_entry(dev, &video->video_device_list, entry) {
- if (!acpi_video_device_lcd_query_levels(dev, &levels))
+ if (!acpi_video_device_lcd_query_levels(dev->dev->handle, &levels))
  kfree(levels);
  }
  mutex_unlock(&video->device_list_lock);
diff --git a/drivers/thermal/int340x_thermal/int3406_thermal.c b/drivers/thermal/int340x_thermal/int3406_thermal.c
index 13d431cbd29e..a578cd257db4 100644
--- a/drivers/thermal/int340x_thermal/int3406_thermal.c
+++ b/drivers/thermal/int340x_thermal/int3406_thermal.c
@@ -177,7 +177,7 @@ static int int3406_thermal_probe(struct platform_device *pdev)
  return -ENODEV;
  d->raw_bd = bd;
 
- ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br);
+ ret = acpi_video_get_levels(ACPI_COMPANION(&pdev->dev), &d->br, NULL);
  if (ret)
  return ret;
 
diff --git a/include/acpi/video.h b/include/acpi/video.h
index 70a41f742037..5731ccb42585 100644
--- a/include/acpi/video.h
+++ b/include/acpi/video.h
@@ -51,7 +51,8 @@ extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type);
  */
 extern bool acpi_video_handles_brightness_key_presses(void);
 extern int acpi_video_get_levels(struct acpi_device *device,
- struct acpi_video_device_brightness **dev_br);
+ struct acpi_video_device_brightness **dev_br,
+ int *pmax_level);
 #else
 static inline int acpi_video_register(void) { return 0; }
 static inline void acpi_video_unregister(void) { return; }
@@ -72,7 +73,8 @@ static inline bool acpi_video_handles_brightness_key_presses(void)
  return false;
 }
 static inline int acpi_video_get_levels(struct acpi_device *device,
- struct acpi_video_device_brightness **dev_br)
+ struct acpi_video_device_brightness **dev_br,
+ int *pmax_level)
 {
  return -ENODEV;
 }
--
2.5.5