From patchwork Fri Jul 18 12:32:51 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 4583091 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 3E1B29F37C for ; Fri, 18 Jul 2014 12:33:29 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4339C2018E for ; Fri, 18 Jul 2014 12:33:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3D1EB20160 for ; Fri, 18 Jul 2014 12:33:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762099AbaGRMdL (ORCPT ); Fri, 18 Jul 2014 08:33:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3467 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761556AbaGRMdI (ORCPT ); Fri, 18 Jul 2014 08:33:08 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s6ICWtVd016128 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 18 Jul 2014 08:32:56 -0400 Received: from shalem.localdomain.com (vpn1-6-200.ams2.redhat.com [10.36.6.200]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s6ICWqoP007860; Fri, 18 Jul 2014 08:32:54 -0400 From: Hans de Goede To: "Rafael J. Wysocki" , Linus Torvalds , =?UTF-8?q?Bj=C3=B8rn=20Mork?= Cc: linux-acpi@vger.kernel.org, Hans de Goede Subject: [PATCH v2] acpi-video: Fix backlight taking 2 steps on a brightness up/down keypress Date: Fri, 18 Jul 2014 14:32:51 +0200 Message-Id: <1405686771-15483-2-git-send-email-hdegoede@redhat.com> In-Reply-To: <1405686771-15483-1-git-send-email-hdegoede@redhat.com> References: <1405686771-15483-1-git-send-email-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 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.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 From: Linus Torvalds In various scenarious userspace will respond to brightness up/down keypresses by increasing/decreasing the backlight brightness itself. If the kernel then also changes the brightness this results in the brightness having changed 2 steps for a single keypress which is undesirable. See e.g. : https://bugs.launchpad.net/gnome-settings-daemon/+bug/527157 http://askubuntu.com/questions/173921/why-does-my-thinkpad-brightness-control-skip-steps This commit delays responding to brightness up/down keypresses by 100 ms and if userspace in that time responds by changing the backlight itself, cancels the kernels own handling of these keypresses, fixing the 2 steps issue. [hdegoede@redhat.com: Move the delayed_work struct into struct acpi_video_device instead of having it as a global] [hdegoede@redhat.com: Keep brightness_switch_enabled as a boolean and always delay the keypress handling] Tested-by: Hans de Goede Tested-by: Bjørn Mork Signed-off-by: Hans de Goede --- Changes in v2: -Keep brightness_switch_enabled as a boolean and always delay the handling --- drivers/acpi/video.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1a450c9..18c0e69 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -204,6 +204,8 @@ struct acpi_video_device { struct acpi_video_device_flags flags; struct acpi_video_device_cap cap; struct list_head entry; + struct delayed_work switch_brightness_work; + int switch_brightness_event; struct acpi_video_bus *video; struct acpi_device *dev; struct acpi_video_device_brightness *brightness; @@ -230,8 +232,7 @@ static int acpi_video_device_lcd_get_level_current( unsigned long long *level, bool raw); static int acpi_video_get_next_level(struct acpi_video_device *device, u32 level_current, u32 event); -static int acpi_video_switch_brightness(struct acpi_video_device *device, - int event); +static void acpi_video_switch_brightness(struct work_struct *work); static bool acpi_video_use_native_backlight(void) { @@ -275,6 +276,7 @@ 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(&vd->switch_brightness_work); return acpi_video_device_lcd_set_level(vd, vd->brightness->levels[request_level]); } @@ -1252,6 +1254,8 @@ acpi_video_bus_get_one_device(struct acpi_device *device, data->device_id = device_id; data->video = video; data->dev = device; + INIT_DELAYED_WORK(&data->switch_brightness_work, + acpi_video_switch_brightness); attribute = acpi_video_get_device_attr(video, device_id); @@ -1474,15 +1478,18 @@ acpi_video_get_next_level(struct acpi_video_device *device, } } -static int -acpi_video_switch_brightness(struct acpi_video_device *device, int event) +static void +acpi_video_switch_brightness(struct work_struct *work) { + struct acpi_video_device *device = container_of(to_delayed_work(work), + struct acpi_video_device, switch_brightness_work); unsigned long long level_current, level_next; + int event = device->switch_brightness_event; int result = -EINVAL; /* no warning message if acpi_backlight=vendor or a quirk is used */ if (!acpi_video_verify_backlight_support()) - return 0; + return; if (!device->brightness) goto out; @@ -1504,8 +1511,6 @@ acpi_video_switch_brightness(struct acpi_video_device *device, int event) out: if (result) printk(KERN_ERR PREFIX "Failed to switch the brightness\n"); - - return result; } int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, @@ -1673,6 +1678,16 @@ 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; + + video_device->switch_brightness_event = event; + schedule_delayed_work(&video_device->switch_brightness_work, HZ / 10); +} + static void acpi_video_device_notify(acpi_handle handle, u32 event, void *data) { struct acpi_video_device *video_device = data; @@ -1690,28 +1705,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: