Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1402531 > unrolled thread

linux-next 20160512 - ACPI issue with screen brightness

Back to article view | Back to linux.kernel

Contents

  linux-next 20160512 - ACPI issue with screen brightness Valdis Kletnieks <Valdis.Kletnieks@vt.edu> - 2016-05-17 20:50 +0200
    Re: linux-next 20160512 - ACPI issue with screen brightness "Rafael J. Wysocki" <rafael@kernel.org> - 2016-05-17 23:00 +0200
      Re: linux-next 20160512 - ACPI issue with screen brightness Valdis.Kletnieks@vt.edu - 2016-05-20 01:00 +0200
        Re: linux-next 20160512 - ACPI issue with screen brightness Valdis.Kletnieks@vt.edu - 2016-05-20 05:10 +0200
          Re: linux-next 20160512 - ACPI issue with screen brightness Aaron Lu <aaron.lu@intel.com> - 2016-05-20 07:50 +0200
            Re: linux-next 20160512 - ACPI issue with screen brightness Valdis.Kletnieks@vt.edu - 2016-05-20 09:10 +0200
              Re: linux-next 20160512 - ACPI issue with screen brightness Aaron Lu <aaron.lu@intel.com> - 2016-05-20 09:20 +0200
                Re: linux-next 20160512 - ACPI issue with screen brightness Aaron Lu <aaron.lu@intel.com> - 2016-05-20 09:40 +0200
                Re: linux-next 20160512 - ACPI issue with screen brightness Aaron Lu <aaron.lu@intel.com> - 2016-05-21 05:30 +0200
                  [PATCH] ACPI / Thermal / video: fix max_level incorrect value Aaron Lu <aaron.lu@intel.com> - 2016-05-21 10:00 +0200
                    Re: [PATCH] ACPI / Thermal / video: fix max_level incorrect value Aaron Lu <aaron.lu@intel.com> - 2016-05-25 07:20 +0200
                      Re: [PATCH] ACPI / Thermal / video: fix max_level incorrect value Valdis.Kletnieks@vt.edu - 2016-05-26 03:50 +0200
                        Re: [PATCH] ACPI / Thermal / video: fix max_level incorrect value Aaron Lu <aaron.lu@intel.com> - 2016-05-26 07:20 +0200
                          Re: [PATCH] ACPI / Thermal / video: fix max_level incorrect value "Zhang, Rui" <rui.zhang@intel.com> - 2016-05-27 02:50 +0200

#1402531 — linux-next 20160512 - ACPI issue with screen brightness

[Multipart message — attachments visible in raw view] — view raw

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 <aaron.lu@intel.com>
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 <aaron.lu@intel.com>
    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

: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....

[toc] | [next] | [standalone]

#1402594

On Tue, May 17, 2016 at 8:41 PM, Valdis Kletnieks
<Valdis.Kletnieks@vt.edu> 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 <aaron.lu@intel.com>
> 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 <aaron.lu@intel.com>
>     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> :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.

[toc] | [prev] | [next] | [standalone]

#1404008

[Multipart message — attachments visible in raw view] — view raw

On Tue, 17 May 2016 22:50:11 +0200, "Rafael J. Wysocki" said:
> On Tue, May 17, 2016 at 8:41 PM, Valdis Kletnieks <Valdis.Kletnieks@vt.edu> 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 <aaron.lu@intel.com>
> > 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 <aaron.lu@intel.com>
> >     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > :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'....

[toc] | [prev] | [next] | [standalone]

#1404102

[Multipart message — attachments visible in raw view] — view raw

On Thu, 19 May 2016 18:53:17 -0400, Valdis.Kletnieks@vt.edu 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.

[toc] | [prev] | [next] | [standalone]

#1404143

On 05/20/2016 11:05 AM, Valdis.Kletnieks@vt.edu wrote:
> On Thu, 19 May 2016 18:53:17 -0400, Valdis.Kletnieks@vt.edu 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.

[toc] | [prev] | [next] | [standalone]

#1404181

[Multipart message — attachments visible in raw view] — view raw

On Fri, 20 May 2016 13:45:30 +0800, Aaron Lu said:
> On 05/20/2016 11:05 AM, Valdis.Kletnieks@vt.edu wrote:
> > On Thu, 19 May 2016 18:53:17 -0400, Valdis.Kletnieks@vt.edu 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.

[toc] | [prev] | [next] | [standalone]

#1404190

On Fri, May 20, 2016 at 03:02:08AM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 20 May 2016 13:45:30 +0800, Aaron Lu said:
> > On 05/20/2016 11:05 AM, Valdis.Kletnieks@vt.edu wrote:
> > > On Thu, 19 May 2016 18:53:17 -0400, Valdis.Kletnieks@vt.edu 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.

[toc] | [prev] | [next] | [standalone]

#1404194

On Fri, May 20, 2016 at 03:17:20PM +0800, Aaron Lu wrote:
> On Fri, May 20, 2016 at 03:02:08AM -0400, Valdis.Kletnieks@vt.edu wrote:
> > On Fri, 20 May 2016 13:45:30 +0800, Aaron Lu said:
> > > On 05/20/2016 11:05 AM, Valdis.Kletnieks@vt.edu wrote:
> > > > On Thu, 19 May 2016 18:53:17 -0400, Valdis.Kletnieks@vt.edu 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:

[toc] | [prev] | [next] | [standalone]

#1404797

On Fri, May 20, 2016 at 08:15:12PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 20 May 2016 15:17:20 +0800, Aaron Lu said:
> > On Fri, May 20, 2016 at 03:02:08AM -0400, Valdis.Kletnieks@vt.edu 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)

[toc] | [prev] | [next] | [standalone]

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

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 <aaron.lu@intel.com>
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 <valdis.kletnieks@vt.edu>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 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

[toc] | [prev] | [next] | [standalone]

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

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 <aaron.lu@intel.com>
> 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 <valdis.kletnieks@vt.edu>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> ---
>  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
> 

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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 <valdis.kletnieks@vt.edu>
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > ---
> >  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

[toc] | [prev] | [next] | [standalone]

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

On 05/26/2016 09:49 AM, Valdis.Kletnieks@vt.edu 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 <aaron.lu@intel.com>
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 <valdis.kletnieks@vt.edu>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 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

[toc] | [prev] | [next] | [standalone]

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

On Thu, 2016-05-26 at 13:15 +0800, Aaron Lu wrote:
> On 05/26/2016 09:49 AM, Valdis.Kletnieks@vt.edu 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 <aaron.lu@intel.com>
> 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 <valdis.kletnieks@vt.edu>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>

Acked-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  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;
>  }

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web