Re: [BISECTED 3.16-rc REGREGRESSION] backlight control stopped working

[Linus Torvalds]

On Mon, Jul 14, 2014 at 9:24 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> BjÃrn, what's your setup? Is this perhaps solvable some other way?

For example, I wonder if we could fix the "dual brightness change"
problem automatically by making a new option for
'brightness_switch_enabled'.

Currently, there are two cases:

 - enabled: do the actual brightness change _and_ send the input
report keycode for a brightness change

 - disabled: just send the keycode, excpecting the desktop software to
handle it.

and maybe we could have a new case (and make *that* the default):

 - delayed: send the keycode, and set up a delayed timer (say, one
tenth of a second) to do the actual brightness change. And if a
brightness change from user mode comes in during that delay, we cancel
the kernel-induced pending change.

Something like the very hacky attached patch that is COMPLETELY UNTESTED.

My point being that I think we can get this right *without* some
stupid "user has to specify the behavior of their desktop application
and ACPI implementation" crap. Especially since it's entirely possible
that there are different behaviors for the same machine (ie the user
session may act differently from the login screen, which will act
differently from the text virtual terminal).

I really don't expect my patch to work as-is, it is really meant more
as an illustration of an approach that might work. There may well be
many other complications (ie how does this interact with the whole
"use_native_backlight" thing and user space possibly accessing *other*
backlight controls). But I have the feeling that this should be
solvable without breaking old setups or causing problems on newer
ones.

                Linus
 drivers/acpi/video.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 071c1dfb93f3..9c4afface8e7 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -68,8 +68,8 @@ MODULE_AUTHOR("Bruno Ducrot");
 MODULE_DESCRIPTION("ACPI Video Driver");
 MODULE_LICENSE("GPL");
 
-static bool brightness_switch_enabled;
-module_param(brightness_switch_enabled, bool, 0644);
+static int brightness_switch_enabled = -1;
+module_param(brightness_switch_enabled, int, 0644);
 
 /*
  * By default, we don't allow duplicate ACPI video bus devices
@@ -270,11 +270,21 @@ static int acpi_video_get_brightness(struct backlight_device *bd)
 	return 0;
 }
 
+static u32 acpi_brightness_event;
+static struct acpi_video_device *acpi_brightness_device;
+static void acpi_video_set_brighness_delayed(struct work_struct *work)
+{
+	acpi_video_switch_brightness(acpi_brightness_device, acpi_brightness_event);
+}
+
+static DECLARE_DELAYED_WORK(acpi_brightness_work, acpi_video_set_brighness_delayed);
+
 static int acpi_video_set_brightness(struct backlight_device *bd)
 {
 	int request_level = bd->props.brightness + 2;
 	struct acpi_video_device *vd = bl_get_data(bd);
 
+	cancel_delayed_work(&acpi_brightness_work);
 	return acpi_video_device_lcd_set_level(vd,
 				vd->brightness->levels[request_level]);
 }
@@ -1601,6 +1611,19 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
 	return;
 }
 
+static void brightness_switch_event(struct acpi_video_device *video_device, u32 event)
+{
+	if (!brightness_switch_enabled)
+		return;
+	if (brightness_switch_enabled > 0) {
+		acpi_video_switch_brightness(video_device, event);
+		return;
+	}
+	acpi_brightness_device = video_device;
+	acpi_brightness_event = event;
+	schedule_delayed_work(&acpi_brightness_work, HZ/10);
+}
+
 static void acpi_video_device_notify(acpi_handle handle, u32 event, void *data)
 {
 	struct acpi_video_device *video_device = data;
@@ -1618,28 +1641,23 @@ static void acpi_video_device_notify(acpi_handle handle, u32 event, void *data)
 
 	switch (event) {
 	case ACPI_VIDEO_NOTIFY_CYCLE_BRIGHTNESS:	/* Cycle brightness */
-		if (brightness_switch_enabled)
-			acpi_video_switch_brightness(video_device, event);
+		brightness_switch_event(video_device, event);
 		keycode = KEY_BRIGHTNESS_CYCLE;
 		break;
 	case ACPI_VIDEO_NOTIFY_INC_BRIGHTNESS:	/* Increase brightness */
-		if (brightness_switch_enabled)
-			acpi_video_switch_brightness(video_device, event);
+		brightness_switch_event(video_device, event);
 		keycode = KEY_BRIGHTNESSUP;
 		break;
 	case ACPI_VIDEO_NOTIFY_DEC_BRIGHTNESS:	/* Decrease brightness */
-		if (brightness_switch_enabled)
-			acpi_video_switch_brightness(video_device, event);
+		brightness_switch_event(video_device, event);
 		keycode = KEY_BRIGHTNESSDOWN;
 		break;
 	case ACPI_VIDEO_NOTIFY_ZERO_BRIGHTNESS:	/* zero brightness */
-		if (brightness_switch_enabled)
-			acpi_video_switch_brightness(video_device, event);
+		brightness_switch_event(video_device, event);
 		keycode = KEY_BRIGHTNESS_ZERO;
 		break;
 	case ACPI_VIDEO_NOTIFY_DISPLAY_OFF:	/* display device off */
-		if (brightness_switch_enabled)
-			acpi_video_switch_brightness(video_device, event);
+		brightness_switch_event(video_device, event);
 		keycode = KEY_DISPLAY_OFF;
 		break;
 	default: