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

[Aaron Lu <aaron.lu@intel.com>]

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: