From patchwork Mon Jul 14 16:59:51 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 4548111 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id DF9789F2F4 for ; Mon, 14 Jul 2014 16:59:56 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 100F120158 for ; Mon, 14 Jul 2014 16:59:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 37D2A20131 for ; Mon, 14 Jul 2014 16:59:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756181AbaGNQ7y (ORCPT ); Mon, 14 Jul 2014 12:59:54 -0400 Received: from mail-vc0-f171.google.com ([209.85.220.171]:44136 "EHLO mail-vc0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755920AbaGNQ7w (ORCPT ); Mon, 14 Jul 2014 12:59:52 -0400 Received: by mail-vc0-f171.google.com with SMTP id id10so7959109vcb.30 for ; Mon, 14 Jul 2014 09:59:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=c2xSLTs4dmy7KFqjAa8/p4b2TbkuEpGtEFRjMhVuPF4=; b=KXE2DgeG8mf6QfP6S0zY8mDmilOQ2ebcUNTv717Rl3zdJ59Fsj5eGO20GuGw8Jbq6G u5OHr/p59tB+/gukd20vkjrKEBll4PD5U22eoqsBL8WtzCDZ5ABdHTRybl3a+wg/8a5B zej/+nWfnk0vt3V6/v8fxWY4YDbo92UUKU69XDmGIlPR9TqYhjguG+vqMrOkbD5ZWHEg NJz2tqtadRh76FXYPgGgiSGce3fZALbGXv2HjvxAazSlDdsPFHNER0n9+T/FVUXyxjym PrjokuYygXX1xZymehw+4Tl41Ms2Y8jhw0OADkisfD2RjjkW71szfCSvwbtzwOvHM3yd XKNA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=c2xSLTs4dmy7KFqjAa8/p4b2TbkuEpGtEFRjMhVuPF4=; b=F6PDCK+nWlM/eenyR8wGsebmjt7Sfw/yY8wdUporCIICVh4Qw/HzhSACvfsG4nLR2Q NVps/GnG+fQIWZzfnRlRfo5P1CyAW8sNbKefFXZaoC/K4xkcdMRWl0lEuVCTaZiUtqTd Meh4mnbiw1eTl8lFjqScE9tY1/NE+Sgy/OCs0= MIME-Version: 1.0 X-Received: by 10.220.163.3 with SMTP id y3mr4359986vcx.7.1405357191930; Mon, 14 Jul 2014 09:59:51 -0700 (PDT) Received: by 10.221.58.141 with HTTP; Mon, 14 Jul 2014 09:59:51 -0700 (PDT) In-Reply-To: References: <87pph8kse7.fsf@nemi.mork.no> <53C3D7C3.7090505@redhat.com> Date: Mon, 14 Jul 2014 09:59:51 -0700 X-Google-Sender-Auth: r1TfFpL7gKVVe5aGUzpvj_L-qd8 Message-ID: Subject: Re: [BISECTED 3.16-rc REGREGRESSION] backlight control stopped working From: Linus Torvalds To: Hans de Goede Cc: =?UTF-8?Q?Bj=C3=B8rn_Mork?= , Linux Kernel Mailing List , Linux ACPI , "Rafael J. Wysocki" Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Jul 14, 2014 at 9:24 AM, Linus Torvalds 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: