v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4; 2 Rename bd_list_head and bd_list_mutex in backlight.c to backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael - patch 1/4.
v4: Remove decleration and stub for acpi_video_unregister_backlight in video.h of patch 2/4 since that function doesn't exist anymore in v3.
v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister_backlight introduced in patch 2/4 as pointed out by Jani Nikula.
v2: v1 has the subject of "Rework ACPI video driver" and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that.
This patchset has four patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/4 for details. Then patch 2/4 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/4 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Patch 4/4 fixes some problems in thinkpad-acpi module.
Technically, patch 2/4 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series.
Aaron Lu (4): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists thinkpad-acpi: fix handle locate for video and query of _BCL
drivers/acpi/internal.h | 4 +- drivers/acpi/video.c | 457 ++++++++++++++++++++--------------- drivers/acpi/video_detect.c | 4 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/linux/backlight.h | 4 + 6 files changed, 326 insertions(+), 205 deletions(-)
Introduce a new API for modules to query if a specific type of backlight device has been registered. This is useful for some backlight device provider module(e.g. ACPI video) to know if a native control interface(e.g. the interface created by i915) is available and then do things accordingly(e.g. avoid register its own on Win8 systems).
Signed-off-by: Aaron Lu aaron.lu@intel.com --- drivers/video/backlight/backlight.c | 31 +++++++++++++++++++++++++++++++ include/linux/backlight.h | 4 ++++ 2 files changed, 35 insertions(+)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c index 94a403a..5d05555 100644 --- a/drivers/video/backlight/backlight.c +++ b/drivers/video/backlight/backlight.c @@ -21,6 +21,9 @@ #include <asm/backlight.h> #endif
+static struct list_head backlight_dev_list; +static struct mutex backlight_dev_list_mutex; + static const char *const backlight_types[] = { [BACKLIGHT_RAW] = "raw", [BACKLIGHT_PLATFORM] = "platform", @@ -349,10 +352,32 @@ struct backlight_device *backlight_device_register(const char *name, mutex_unlock(&pmac_backlight_mutex); #endif
+ mutex_lock(&backlight_dev_list_mutex); + list_add(&new_bd->entry, &backlight_dev_list); + mutex_unlock(&backlight_dev_list_mutex); + return new_bd; } EXPORT_SYMBOL(backlight_device_register);
+bool backlight_device_registered(enum backlight_type type) +{ + bool found = false; + struct backlight_device *bd; + + mutex_lock(&backlight_dev_list_mutex); + list_for_each_entry(bd, &backlight_dev_list, entry) { + if (bd->props.type == type) { + found = true; + break; + } + } + mutex_unlock(&backlight_dev_list_mutex); + + return found; +} +EXPORT_SYMBOL(backlight_device_registered); + /** * backlight_device_unregister - unregisters a backlight device object. * @bd: the backlight device object to be unregistered and freed. @@ -364,6 +389,10 @@ void backlight_device_unregister(struct backlight_device *bd) if (!bd) return;
+ mutex_lock(&backlight_dev_list_mutex); + list_del(&bd->entry); + mutex_unlock(&backlight_dev_list_mutex); + #ifdef CONFIG_PMAC_BACKLIGHT mutex_lock(&pmac_backlight_mutex); if (pmac_backlight == bd) @@ -499,6 +528,8 @@ static int __init backlight_class_init(void)
backlight_class->dev_groups = bl_device_groups; backlight_class->pm = &backlight_class_dev_pm_ops; + INIT_LIST_HEAD(&backlight_dev_list); + mutex_init(&backlight_dev_list_mutex); return 0; }
diff --git a/include/linux/backlight.h b/include/linux/backlight.h index 53b7794..5f9cd96 100644 --- a/include/linux/backlight.h +++ b/include/linux/backlight.h @@ -100,6 +100,9 @@ struct backlight_device { /* The framebuffer notifier block */ struct notifier_block fb_notif;
+ /* list entry of all registered backlight devices */ + struct list_head entry; + struct device dev; };
@@ -123,6 +126,7 @@ extern void devm_backlight_device_unregister(struct device *dev, struct backlight_device *bd); extern void backlight_force_update(struct backlight_device *bd, enum backlight_update_reason reason); +extern bool backlight_device_registered(enum backlight_type type);
#define to_backlight_device(obj) container_of(obj, struct backlight_device, dev)
The backlight control and event delivery functionality provided by ACPI video module is mixed together and registered all during video device enumeration time. As a result, the two functionality are also removed together on module unload time or by the acpi_video_unregister function. The two functionalities are actually independent and one may be useful while the other one may be broken, so it is desirable to seperate the two functionalities such that it is clear and easy to disable one functionality without affecting the other one.
APIs to selectively remove backlight control interface and/or event delivery functionality can be easily added once needed.
Signed-off-by: Aaron Lu aaron.lu@intel.com --- drivers/acpi/video.c | 434 +++++++++++++++++++++++++++++---------------------- 1 file changed, 245 insertions(+), 189 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index aebcf63..3bd1eaa 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -89,6 +89,8 @@ static bool use_bios_initial_backlight = 1; module_param(use_bios_initial_backlight, bool, 0644);
static int register_count; +static struct mutex video_list_lock; +static struct list_head video_bus_head; static int acpi_video_bus_add(struct acpi_device *device); static int acpi_video_bus_remove(struct acpi_device *device); static void acpi_video_bus_notify(struct acpi_device *device, u32 event); @@ -157,6 +159,7 @@ struct acpi_video_bus { struct acpi_video_bus_flags flags; struct list_head video_device_list; struct mutex device_list_lock; /* protects video_device_list */ + struct list_head entry; struct input_dev *input; char phys[32]; /* for input device */ struct notifier_block pm_nb; @@ -884,79 +887,6 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
if (acpi_has_method(device->dev->handle, "_DDC")) device->cap._DDC = 1; - - if (acpi_video_backlight_support()) { - struct backlight_properties props; - struct pci_dev *pdev; - acpi_handle acpi_parent; - struct device *parent = NULL; - int result; - static int count; - char *name; - - result = acpi_video_init_brightness(device); - if (result) - return; - name = kasprintf(GFP_KERNEL, "acpi_video%d", count); - if (!name) - return; - count++; - - acpi_get_parent(device->dev->handle, &acpi_parent); - - pdev = acpi_get_pci_dev(acpi_parent); - if (pdev) { - parent = &pdev->dev; - pci_dev_put(pdev); - } - - memset(&props, 0, sizeof(struct backlight_properties)); - props.type = BACKLIGHT_FIRMWARE; - props.max_brightness = device->brightness->count - 3; - device->backlight = backlight_device_register(name, - parent, - device, - &acpi_backlight_ops, - &props); - kfree(name); - if (IS_ERR(device->backlight)) - return; - - /* - * Save current brightness level in case we have to restore it - * before acpi_video_device_lcd_set_level() is called next time. - */ - device->backlight->props.brightness = - acpi_video_get_brightness(device->backlight); - - device->cooling_dev = thermal_cooling_device_register("LCD", - device->dev, &video_cooling_ops); - if (IS_ERR(device->cooling_dev)) { - /* - * Set cooling_dev to NULL so we don't crash trying to - * free it. - * Also, why the hell we are returning early and - * not attempt to register video output if cooling - * device registration failed? - * -- dtor - */ - device->cooling_dev = NULL; - return; - } - - dev_info(&device->dev->dev, "registered as cooling_device%d\n", - device->cooling_dev->id); - result = sysfs_create_link(&device->dev->dev.kobj, - &device->cooling_dev->device.kobj, - "thermal_cooling"); - if (result) - printk(KERN_ERR PREFIX "Create sysfs link\n"); - result = sysfs_create_link(&device->cooling_dev->device.kobj, - &device->dev->dev.kobj, "device"); - if (result) - printk(KERN_ERR PREFIX "Create sysfs link\n"); - - } }
/* @@ -1143,13 +1073,6 @@ acpi_video_bus_get_one_device(struct acpi_device *device, acpi_video_device_bind(video, data); acpi_video_device_find_cap(data);
- status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, - acpi_video_device_notify, data); - if (ACPI_FAILURE(status)) - dev_err(&device->dev, "Error installing notify handler\n"); - else - data->flags.notify = 1; - mutex_lock(&video->device_list_lock); list_add_tail(&data->entry, &video->video_device_list); mutex_unlock(&video->device_list_lock); @@ -1454,64 +1377,6 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video, return status; }
-static int acpi_video_bus_put_one_device(struct acpi_video_device *device) -{ - acpi_status status; - - if (!device || !device->video) - return -ENOENT; - - if (device->flags.notify) { - status = acpi_remove_notify_handler(device->dev->handle, - ACPI_DEVICE_NOTIFY, acpi_video_device_notify); - if (ACPI_FAILURE(status)) - dev_err(&device->dev->dev, - "Can't remove video notify handler\n"); - } - - if (device->backlight) { - backlight_device_unregister(device->backlight); - device->backlight = NULL; - } - if (device->cooling_dev) { - sysfs_remove_link(&device->dev->dev.kobj, - "thermal_cooling"); - sysfs_remove_link(&device->cooling_dev->device.kobj, - "device"); - thermal_cooling_device_unregister(device->cooling_dev); - device->cooling_dev = NULL; - } - - return 0; -} - -static int acpi_video_bus_put_devices(struct acpi_video_bus *video) -{ - int status; - struct acpi_video_device *dev, *next; - - mutex_lock(&video->device_list_lock); - - list_for_each_entry_safe(dev, next, &video->video_device_list, entry) { - - status = acpi_video_bus_put_one_device(dev); - if (ACPI_FAILURE(status)) - printk(KERN_WARNING PREFIX - "hhuuhhuu bug in acpi video driver.\n"); - - if (dev->brightness) { - kfree(dev->brightness->levels); - kfree(dev->brightness); - } - list_del(&dev->entry); - kfree(dev); - } - - mutex_unlock(&video->device_list_lock); - - return 0; -} - /* acpi_video interface */
/* @@ -1536,7 +1401,7 @@ static void acpi_video_bus_notify(struct acpi_device *device, u32 event) struct input_dev *input; int keycode = 0;
- if (!video) + if (!video || !video->input) return;
input = video->input; @@ -1691,12 +1556,236 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void *context, return AE_OK; }
+static void acpi_video_dev_register_backlight(struct acpi_video_device *device) +{ + if (acpi_video_backlight_support()) { + struct backlight_properties props; + struct pci_dev *pdev; + acpi_handle acpi_parent; + struct device *parent = NULL; + int result; + static int count; + char *name; + + result = acpi_video_init_brightness(device); + if (result) + return; + name = kasprintf(GFP_KERNEL, "acpi_video%d", count); + if (!name) + return; + count++; + + acpi_get_parent(device->dev->handle, &acpi_parent); + + pdev = acpi_get_pci_dev(acpi_parent); + if (pdev) { + parent = &pdev->dev; + pci_dev_put(pdev); + } + + memset(&props, 0, sizeof(struct backlight_properties)); + props.type = BACKLIGHT_FIRMWARE; + props.max_brightness = device->brightness->count - 3; + device->backlight = backlight_device_register(name, + parent, + device, + &acpi_backlight_ops, + &props); + kfree(name); + if (IS_ERR(device->backlight)) + return; + + /* + * Save current brightness level in case we have to restore it + * before acpi_video_device_lcd_set_level() is called next time. + */ + device->backlight->props.brightness = + acpi_video_get_brightness(device->backlight); + + device->cooling_dev = thermal_cooling_device_register("LCD", + device->dev, &video_cooling_ops); + if (IS_ERR(device->cooling_dev)) { + /* + * Set cooling_dev to NULL so we don't crash trying to + * free it. + * Also, why the hell we are returning early and + * not attempt to register video output if cooling + * device registration failed? + * -- dtor + */ + device->cooling_dev = NULL; + return; + } + + dev_info(&device->dev->dev, "registered as cooling_device%d\n", + device->cooling_dev->id); + result = sysfs_create_link(&device->dev->dev.kobj, + &device->cooling_dev->device.kobj, + "thermal_cooling"); + if (result) + printk(KERN_ERR PREFIX "Create sysfs link\n"); + result = sysfs_create_link(&device->cooling_dev->device.kobj, + &device->dev->dev.kobj, "device"); + if (result) + printk(KERN_ERR PREFIX "Create sysfs link\n"); + } +} + +static int acpi_video_bus_register_backlight(struct acpi_video_bus *video) +{ + struct acpi_video_device *dev; + + mutex_lock(&video->device_list_lock); + list_for_each_entry(dev, &video->video_device_list, entry) + acpi_video_dev_register_backlight(dev); + mutex_unlock(&video->device_list_lock); + + video->pm_nb.notifier_call = acpi_video_resume; + video->pm_nb.priority = 0; + return register_pm_notifier(&video->pm_nb); +} + +static void acpi_video_dev_unregister_backlight(struct acpi_video_device *device) +{ + if (device->backlight) { + backlight_device_unregister(device->backlight); + device->backlight = NULL; + } + if (device->brightness) { + kfree(device->brightness->levels); + kfree(device->brightness); + device->brightness = NULL; + } + if (device->cooling_dev) { + sysfs_remove_link(&device->dev->dev.kobj, "thermal_cooling"); + sysfs_remove_link(&device->cooling_dev->device.kobj, "device"); + thermal_cooling_device_unregister(device->cooling_dev); + device->cooling_dev = NULL; + } +} + +static int acpi_video_bus_unregister_backlight(struct acpi_video_bus *video) +{ + struct acpi_video_device *dev; + int error = unregister_pm_notifier(&video->pm_nb); + + mutex_lock(&video->device_list_lock); + list_for_each_entry(dev, &video->video_device_list, entry) + acpi_video_dev_unregister_backlight(dev); + mutex_unlock(&video->device_list_lock); + + return error; +} + +static void acpi_video_dev_add_notify_handler(struct acpi_video_device *device) +{ + acpi_status status; + struct acpi_device *adev = device->dev; + + status = acpi_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY, + acpi_video_device_notify, device); + if (ACPI_FAILURE(status)) + dev_err(&adev->dev, "Error installing notify handler\n"); + else + device->flags.notify = 1; +} + +static int acpi_video_bus_add_notify_handler(struct acpi_video_bus *video) +{ + struct input_dev *input; + struct acpi_video_device *dev; + int error; + + video->input = input = input_allocate_device(); + if (!input) { + error = -ENOMEM; + goto out; + } + + error = acpi_video_bus_start_devices(video); + if (error) + goto err_free_input; + + snprintf(video->phys, sizeof(video->phys), + "%s/video/input0", acpi_device_hid(video->device)); + + input->name = acpi_device_name(video->device); + input->phys = video->phys; + input->id.bustype = BUS_HOST; + input->id.product = 0x06; + input->dev.parent = &video->device->dev; + input->evbit[0] = BIT(EV_KEY); + set_bit(KEY_SWITCHVIDEOMODE, input->keybit); + set_bit(KEY_VIDEO_NEXT, input->keybit); + set_bit(KEY_VIDEO_PREV, input->keybit); + set_bit(KEY_BRIGHTNESS_CYCLE, input->keybit); + set_bit(KEY_BRIGHTNESSUP, input->keybit); + set_bit(KEY_BRIGHTNESSDOWN, input->keybit); + set_bit(KEY_BRIGHTNESS_ZERO, input->keybit); + set_bit(KEY_DISPLAY_OFF, input->keybit); + + error = input_register_device(input); + if (error) + goto err_stop_dev; + + mutex_lock(&video->device_list_lock); + list_for_each_entry(dev, &video->video_device_list, entry) + acpi_video_dev_add_notify_handler(dev); + mutex_unlock(&video->device_list_lock); + + return 0; + +err_stop_dev: + acpi_video_bus_stop_devices(video); +err_free_input: + input_free_device(input); + video->input = NULL; +out: + return error; +} + +static void acpi_video_dev_remove_notify_handler(struct acpi_video_device *dev) +{ + if (dev->flags.notify) { + acpi_remove_notify_handler(dev->dev->handle, ACPI_DEVICE_NOTIFY, + acpi_video_device_notify); + dev->flags.notify = 0; + } +} + +static void acpi_video_bus_remove_notify_handler(struct acpi_video_bus *video) +{ + struct acpi_video_device *dev; + + mutex_lock(&video->device_list_lock); + list_for_each_entry(dev, &video->video_device_list, entry) + acpi_video_dev_remove_notify_handler(dev); + mutex_unlock(&video->device_list_lock); + + acpi_video_bus_stop_devices(video); + input_unregister_device(video->input); + video->input = NULL; +} + +static int acpi_video_bus_put_devices(struct acpi_video_bus *video) +{ + struct acpi_video_device *dev, *next; + + mutex_lock(&video->device_list_lock); + list_for_each_entry_safe(dev, next, &video->video_device_list, entry) { + list_del(&dev->entry); + kfree(dev); + } + mutex_unlock(&video->device_list_lock); + + return 0; +} + static int instance;
static int acpi_video_bus_add(struct acpi_device *device) { struct acpi_video_bus *video; - struct input_dev *input; int error; acpi_status status;
@@ -1748,62 +1837,24 @@ static int acpi_video_bus_add(struct acpi_device *device) if (error) goto err_put_video;
- video->input = input = input_allocate_device(); - if (!input) { - error = -ENOMEM; - goto err_put_video; - } - - error = acpi_video_bus_start_devices(video); - if (error) - goto err_free_input_dev; - - snprintf(video->phys, sizeof(video->phys), - "%s/video/input0", acpi_device_hid(video->device)); - - input->name = acpi_device_name(video->device); - input->phys = video->phys; - input->id.bustype = BUS_HOST; - input->id.product = 0x06; - input->dev.parent = &device->dev; - input->evbit[0] = BIT(EV_KEY); - set_bit(KEY_SWITCHVIDEOMODE, input->keybit); - set_bit(KEY_VIDEO_NEXT, input->keybit); - set_bit(KEY_VIDEO_PREV, input->keybit); - set_bit(KEY_BRIGHTNESS_CYCLE, input->keybit); - set_bit(KEY_BRIGHTNESSUP, input->keybit); - set_bit(KEY_BRIGHTNESSDOWN, input->keybit); - set_bit(KEY_BRIGHTNESS_ZERO, input->keybit); - set_bit(KEY_DISPLAY_OFF, input->keybit); - printk(KERN_INFO PREFIX "%s [%s] (multi-head: %s rom: %s post: %s)\n", ACPI_VIDEO_DEVICE_NAME, acpi_device_bid(device), video->flags.multihead ? "yes" : "no", video->flags.rom ? "yes" : "no", video->flags.post ? "yes" : "no"); + mutex_lock(&video_list_lock); + list_add_tail(&video->entry, &video_bus_head); + mutex_unlock(&video_list_lock);
- video->pm_nb.notifier_call = acpi_video_resume; - video->pm_nb.priority = 0; - error = register_pm_notifier(&video->pm_nb); - if (error) - goto err_stop_video; - - error = input_register_device(input); - if (error) - goto err_unregister_pm_notifier; + acpi_video_bus_register_backlight(video); + acpi_video_bus_add_notify_handler(video);
return 0;
- err_unregister_pm_notifier: - unregister_pm_notifier(&video->pm_nb); - err_stop_video: - acpi_video_bus_stop_devices(video); - err_free_input_dev: - input_free_device(input); - err_put_video: +err_put_video: acpi_video_bus_put_devices(video); kfree(video->attached_array); - err_free_video: +err_free_video: kfree(video); device->driver_data = NULL;
@@ -1820,12 +1871,14 @@ static int acpi_video_bus_remove(struct acpi_device *device)
video = acpi_driver_data(device);
- unregister_pm_notifier(&video->pm_nb); - - acpi_video_bus_stop_devices(video); + acpi_video_bus_remove_notify_handler(video); + acpi_video_bus_unregister_backlight(video); acpi_video_bus_put_devices(video);
- input_unregister_device(video->input); + mutex_lock(&video_list_lock); + list_del(&video->entry); + mutex_unlock(&video_list_lock); + kfree(video->attached_array); kfree(video);
@@ -1874,6 +1927,9 @@ int acpi_video_register(void) return 0; }
+ mutex_init(&video_list_lock); + INIT_LIST_HEAD(&video_bus_head); + result = acpi_bus_register_driver(&acpi_video_bus); if (result < 0) return -ENODEV;
According to Matthew Garrett, "Windows 8 leaves backlight control up to individual graphics drivers rather than making ACPI calls itself. There's plenty of evidence to suggest that the Intel driver for Windows [8] doesn't use the ACPI interface, including the fact that it's broken on a bunch of machines when the OS claims to support Windows 8. The simplest thing to do appears to be to disable the ACPI backlight interface on these systems".
So for Win8 systems, if there is native backlight control interface registered by GPU driver, ACPI video does not need to register its own. Since there are systems that don't work well with this approach, a parameter for video module named use_native_backlight is introduced and has the value of false by default. For users who have a broken ACPI video backlight interface, video.use_native_backlight=1 is needed in kernel cmdline.
Signed-off-by: Aaron Lu aaron.lu@intel.com --- drivers/acpi/internal.h | 4 +--- drivers/acpi/video.c | 25 ++++++++++++++++++++----- drivers/acpi/video_detect.c | 4 ++-- 3 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 20f4233..e9304dc 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -169,9 +169,7 @@ int acpi_create_platform_device(struct acpi_device *adev, Video -------------------------------------------------------------------------- */ #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) -bool acpi_video_backlight_quirks(void); -#else -static inline bool acpi_video_backlight_quirks(void) { return false; } +bool acpi_osi_is_win8(void); #endif
#endif /* _ACPI_INTERNAL_H_ */ diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 3bd1eaa..d020df5 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -88,6 +88,13 @@ module_param(allow_duplicates, bool, 0644); static bool use_bios_initial_backlight = 1; module_param(use_bios_initial_backlight, bool, 0644);
+/* + * For Windows 8 systems: if set ture and the GPU driver has + * registered a backlight interface, skip registering ACPI video's. + */ +static bool use_native_backlight = false; +module_param(use_native_backlight, bool, 0644); + static int register_count; static struct mutex video_list_lock; static struct list_head video_bus_head; @@ -232,6 +239,14 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, static int acpi_video_switch_brightness(struct acpi_video_device *device, int event);
+static bool acpi_video_verify_backlight_support(void) +{ + if (acpi_osi_is_win8() && use_native_backlight && + backlight_device_registered(BACKLIGHT_RAW)) + return false; + return acpi_video_backlight_support(); +} + /* backlight device sysfs support */ static int acpi_video_get_brightness(struct backlight_device *bd) { @@ -1256,8 +1271,8 @@ acpi_video_switch_brightness(struct acpi_video_device *device, int event) unsigned long long level_current, level_next; int result = -EINVAL;
- /* no warning message if acpi_backlight=vendor is used */ - if (!acpi_video_backlight_support()) + /* no warning message if acpi_backlight=vendor or a quirk is used */ + if (!acpi_video_verify_backlight_support()) return 0;
if (!device->brightness) @@ -1386,13 +1401,13 @@ acpi_video_bus_get_devices(struct acpi_video_bus *video, static int acpi_video_bus_start_devices(struct acpi_video_bus *video) { return acpi_video_bus_DOS(video, 0, - acpi_video_backlight_quirks() ? 1 : 0); + acpi_osi_is_win8() ? 1 : 0); }
static int acpi_video_bus_stop_devices(struct acpi_video_bus *video) { return acpi_video_bus_DOS(video, 0, - acpi_video_backlight_quirks() ? 0 : 1); + acpi_osi_is_win8() ? 0 : 1); }
static void acpi_video_bus_notify(struct acpi_device *device, u32 event) @@ -1558,7 +1573,7 @@ acpi_video_bus_match(acpi_handle handle, u32 level, void *context,
static void acpi_video_dev_register_backlight(struct acpi_video_device *device) { - if (acpi_video_backlight_support()) { + if (acpi_video_verify_backlight_support()) { struct backlight_properties props; struct pci_dev *pdev; acpi_handle acpi_parent; diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 940edbf..b639934 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -233,11 +233,11 @@ static void acpi_video_caps_check(void) acpi_video_get_capabilities(NULL); }
-bool acpi_video_backlight_quirks(void) +bool acpi_osi_is_win8(void) { return acpi_gbl_osi_data >= ACPI_OSI_WIN_8; } -EXPORT_SYMBOL(acpi_video_backlight_quirks); +EXPORT_SYMBOL(acpi_osi_is_win8);
/* Promote the vendor interface instead of the generic video module. * This function allow DMI blacklists to be implemented by externals
The tpacpi_acpi_handle_locate function makes use of acpi_get_devices to locate handle for ACPI video by HID, the problem is, ACPI video node doesn't really have HID defined(i.e. no _HID control method is defined for video device), so.. that function would fail. This can be solved by enhancing the callback function for acpi_get_devices, where we can use acpi_device_hid function to check if the ACPI node corresponds to a video controller.
In addition to that, the _BCL control method only exists under a video output device node, not a video controller device node. So to evaluate _BCL, we need the handle of a video output device node, which is child of the located video controller node from tpacpi_acpi_handle_locate.
The two fix are necessary for some Thinkpad models to emit notification on backlight hotkey press as a result of evaluation of _BCL.
Signed-off-by: Aaron Lu aaron.lu@intel.com Tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Acked-by: Henrique de Moraes Holschuh hmh@hmh.eng.br --- drivers/platform/x86/thinkpad_acpi.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 03ca6c1..170f278 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -700,6 +700,14 @@ static void __init drv_acpi_handle_init(const char *name, static acpi_status __init tpacpi_acpi_handle_locate_callback(acpi_handle handle, u32 level, void *context, void **return_value) { + struct acpi_device *dev; + if (!strcmp(context, "video")) { + if (acpi_bus_get_device(handle, &dev)) + return AE_OK; + if (strcmp(ACPI_VIDEO_HID, acpi_device_hid(dev))) + return AE_OK; + } + *(acpi_handle *)return_value = handle;
return AE_CTRL_TERMINATE; @@ -712,10 +720,10 @@ static void __init tpacpi_acpi_handle_locate(const char *name, acpi_status status; acpi_handle device_found;
- BUG_ON(!name || !hid || !handle); + BUG_ON(!name || !handle); vdbg_printk(TPACPI_DBG_INIT, "trying to locate ACPI handle for %s, using HID %s\n", - name, hid); + name, hid ? hid : "NULL");
memset(&device_found, 0, sizeof(device_found)); status = acpi_get_devices(hid, tpacpi_acpi_handle_locate_callback, @@ -6090,19 +6098,28 @@ static int __init tpacpi_query_bcl_levels(acpi_handle handle) { struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; union acpi_object *obj; + struct acpi_device *device, *child; int rc;
- if (ACPI_SUCCESS(acpi_evaluate_object(handle, "_BCL", NULL, &buffer))) { + if (acpi_bus_get_device(handle, &device)) + return 0; + + rc = 0; + list_for_each_entry(child, &device->children, node) { + acpi_status status = acpi_evaluate_object(child->handle, "_BCL", + NULL, &buffer); + if (ACPI_FAILURE(status)) + continue; + obj = (union acpi_object *)buffer.pointer; if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) { pr_err("Unknown _BCL data, please report this to %s\n", - TPACPI_MAIL); + TPACPI_MAIL); rc = 0; } else { rc = obj->package.count; } - } else { - return 0; + break; }
kfree(buffer.pointer); @@ -6118,7 +6135,7 @@ static unsigned int __init tpacpi_check_std_acpi_brightness_support(void) acpi_handle video_device; int bcl_levels = 0;
- tpacpi_acpi_handle_locate("video", ACPI_VIDEO_HID, &video_device); + tpacpi_acpi_handle_locate("video", NULL, &video_device); if (video_device) bcl_levels = tpacpi_query_bcl_levels(video_device);
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
josh
On Fri, 2013-10-11 at 12:42 -0400, Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
josh
I think more better to use this unregister as default and give option to disable it.
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
Thanks!
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel.
josh
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel.
Well, adding a Kconfig option should be simple enough, but I'm not sure I understand the point. You'll still need to rebuild the kernel after changing that option.
On Sat, 2013-10-12 at 00:45 +0200, Rafael J. Wysocki wrote:
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel.
Well, adding a Kconfig option should be simple enough, but I'm not sure I understand the point. You'll still need to rebuild the kernel after changing that option.
I think he meant that we should have Kconfig option alike CONFIG_I915_BACKLIGHT_UNREGISTER which have true or false. If it true - unregister acpi backlight as default. I think so.
On Saturday, October 12, 2013 02:53:38 AM Igor Gnatenko wrote:
On Sat, 2013-10-12 at 00:45 +0200, Rafael J. Wysocki wrote:
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel.
Well, adding a Kconfig option should be simple enough, but I'm not sure I understand the point. You'll still need to rebuild the kernel after changing that option.
I think he meant that we should have Kconfig option alike CONFIG_I915_BACKLIGHT_UNREGISTER which have true or false. If it true - unregister acpi backlight as default. I think so.
That's my understanding too, but I'm not sure about the benefit. The only one seems to be that distros wanting to change the default won't have to carry a patch for that.
Thanks!
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel.
If we are to use a Kconfig option, why don't we use one instead of rather than in addition to a command line option? Say, we have CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like the previous version of the Aaron's patchset (the one without video.use_native_backlight)?
Opinions?
On sam., 2013-10-12 at 01:27 +0200, Rafael J. Wysocki wrote:
If we are to use a Kconfig option, why don't we use one instead of rather than in addition to a command line option? Say, we have CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like the previous version of the Aaron's patchset (the one without video.use_native_backlight)?
Doesn't this mean user will have to rebuild distro kernel in order to test behavior?
Regards,
On Saturday, October 12, 2013 07:54:06 AM Yves-Alexis Perez wrote:
On sam., 2013-10-12 at 01:27 +0200, Rafael J. Wysocki wrote:
If we are to use a Kconfig option, why don't we use one instead of rather than in addition to a command line option? Say, we have CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like the previous version of the Aaron's patchset (the one without video.use_native_backlight)?
Doesn't this mean user will have to rebuild distro kernel in order to test behavior?
Yes, it does, which is a good point too.
Thanks!
On Fri, Oct 11, 2013 at 4:27 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel.
If we are to use a Kconfig option, why don't we use one instead of rather than in addition to a command line option? Say, we have CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like the previous version of the Aaron's patchset (the one without video.use_native_backlight)?
Opinions?
If you only have a config option, users can't override the distro settings. If you simply have a config option for the default value, the distros can set it without having to carry a patch (the primary benefit), but users can still override that without having to rebuild a kernel.
josh
On Sat, Oct 12, 2013 at 04:44:30AM -0700, Josh Boyer wrote:
On Fri, Oct 11, 2013 at 4:27 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel.
If we are to use a Kconfig option, why don't we use one instead of rather than in addition to a command line option? Say, we have CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like the previous version of the Aaron's patchset (the one without video.use_native_backlight)?
Opinions?
If you only have a config option, users can't override the distro settings. If you simply have a config option for the default value, the distros can set it without having to carry a patch (the primary benefit), but users can still override that without having to rebuild a kernel.
It sounds like the blacklist and the default value of the parameter would be inherently tied together, i.e. the blacklist essentially overrides the default value for specific machines. So when the config option were flipped from its default the blacklist wouldn't work anymore, and you'd need a second blacklist for machines which require video.use_native_backlight=n. I doubt anyone wants to see that happen, so I think we have to pick one value or the other for the default and make it configurable only via the command line.
Seth
On Saturday, October 12, 2013 08:34:32 AM Seth Forshee wrote:
On Sat, Oct 12, 2013 at 04:44:30AM -0700, Josh Boyer wrote:
On Fri, Oct 11, 2013 at 4:27 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 06:01:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 6:10 PM, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote: > v5: > 1 Introduce video.use_native_backlight module parameter and set its > value to false by default as suggested by Rafael. For Win8 systems > which have broken ACPI video backlight control, the parameter can be > set to 1 in kernel cmdline to skip registering ACPI video's backlight > interface. Due to this change, the acpi_video_verify_backlight_support > is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
If nothing else, can you add a config option for the default so distros can use that to decide which way to default it and then work on fixing the remaining users that have troubles?
The current plan is to create a blacklist of systems where that option should be set. We actually already have one, but it is at the _OSI() level, which is overkill in my view and may affect things beyond backlight. Along with that we will debug systems where setting that option (to true) causes problems to happen, so that we'll be able to drop it going forward (hopefully).
Of course, distro kernels may always change the default to true if they want.
They can, but they'd need to either patch the kernel to do so, or code it in userspace bootloader configs. Having a config option they can set to change the default makes it reasonable and contained within the kernel.
If we are to use a Kconfig option, why don't we use one instead of rather than in addition to a command line option? Say, we have CONFIG_ACPI_VIDEO_WIN8_WORKAROUND and if that is set, the code will work like the previous version of the Aaron's patchset (the one without video.use_native_backlight)?
Opinions?
If you only have a config option, users can't override the distro settings. If you simply have a config option for the default value, the distros can set it without having to carry a patch (the primary benefit), but users can still override that without having to rebuild a kernel.
It sounds like the blacklist and the default value of the parameter would be inherently tied together, i.e. the blacklist essentially overrides the default value for specific machines. So when the config option were flipped from its default the blacklist wouldn't work anymore, and you'd need a second blacklist for machines which require video.use_native_backlight=n. I doubt anyone wants to see that happen, so I think we have to pick one value or the other for the default and make it configurable only via the command line.
Good point.
Thanks!
On sam., 2013-10-12 at 00:10 +0200, Rafael J. Wysocki wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
Well, for some users, the regression actually happened when support for Win8 OSI call was introduced.
Regards,
On Fri, Oct 11, 2013 at 11:00 PM, Yves-Alexis Perez corsac@debian.org wrote:
On sam., 2013-10-12 at 00:10 +0200, Rafael J. Wysocki wrote:
On Friday, October 11, 2013 12:42:43 PM Josh Boyer wrote:
On Fri, Oct 11, 2013 at 9:27 AM, Aaron Lu aaron.lu@intel.com wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4;
That's a fairly untenable position for distro kernels to be in. They now have to ask every user that reports an issue with the backlight to try setting that option on the command line. While I appreciate the setting breaks things for some people, doesn't the Win8 issue impact far more people? Shouldn't it be defaulted to true?
Well, we have a rule in the kernel not to introduce regressions for users even if they are minority.
Well, for some users, the regression actually happened when support for Win8 OSI call was introduced.
Yes, this is true. It's probably one of the more common bug reports we get in this area. Kernels prior to that have working backlight, kernels after that don't.
josh
On Friday, October 11, 2013 09:27:42 PM Aaron Lu wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4; 2 Rename bd_list_head and bd_list_mutex in backlight.c to backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael
- patch 1/4.
v4: Remove decleration and stub for acpi_video_unregister_backlight in video.h of patch 2/4 since that function doesn't exist anymore in v3.
v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister_backlight introduced in patch 2/4 as pointed out by Jani Nikula.
v2: v1 has the subject of "Rework ACPI video driver" and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that.
This patchset has four patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/4 for details. Then patch 2/4 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/4 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Patch 4/4 fixes some problems in thinkpad-acpi module.
Technically, patch 2/4 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series.
Aaron Lu (4): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists thinkpad-acpi: fix handle locate for video and query of _BCL
drivers/acpi/internal.h | 4 +- drivers/acpi/video.c | 457 ++++++++++++++++++++--------------- drivers/acpi/video_detect.c | 4 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/linux/backlight.h | 4 + 6 files changed, 326 insertions(+), 205 deletions(-)
I've added this series to my queue for 3.13.
Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default.
Hopefully, in the future we'll be able to fix the problems causing video.use_native_backlight=1 to fail of the systems where it fails and then we'll be able to make that the default behavior and drop the option altogether.
Thanks!
On 10/16/2013 07:33 AM, Rafael J. Wysocki wrote:
On Friday, October 11, 2013 09:27:42 PM Aaron Lu wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4; 2 Rename bd_list_head and bd_list_mutex in backlight.c to backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael
- patch 1/4.
v4: Remove decleration and stub for acpi_video_unregister_backlight in video.h of patch 2/4 since that function doesn't exist anymore in v3.
v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister_backlight introduced in patch 2/4 as pointed out by Jani Nikula.
v2: v1 has the subject of "Rework ACPI video driver" and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that.
This patchset has four patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/4 for details. Then patch 2/4 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/4 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Patch 4/4 fixes some problems in thinkpad-acpi module.
Technically, patch 2/4 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series.
Aaron Lu (4): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists thinkpad-acpi: fix handle locate for video and query of _BCL
drivers/acpi/internal.h | 4 +- drivers/acpi/video.c | 457 ++++++++++++++++++++--------------- drivers/acpi/video_detect.c | 4 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/linux/backlight.h | 4 + 6 files changed, 326 insertions(+), 205 deletions(-)
I've added this series to my queue for 3.13.
Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default.
Hopefully, in the future we'll be able to fix the problems causing video.use_native_backlight=1 to fail of the systems where it fails and then we'll be able to make that the default behavior and drop the option altogether.
I've prepared a patch(at the end of the mail) to set use_native_backlight by default for some systems. There are 3 systems currently that I'm kind of sure that should be added:
The ThinkPad T430s and X230 is: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reported-by: Igor Gnatenko i.gnatenko.brain@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
The Lenovo Yoga is: Reported-by: Lennart Poettering lennart@poettering.net Reference: https://lkml.org/lkml/2013/10/13/178
From: Aaron Lu aaron.lu@intel.com Subject: [PATCH] ACPI / video: Add systems that should favor native backlight interface
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be clear, the ThinkPad T430s/X230 and Lenovo Yoga 13 are added here.
Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reported-by: Lennart Poettering lennart@poettering.net Reported-by: Igor Gnatenko i.gnatenko.brain@gmail.com Signed-off-by: Aaron Lu aaron.lu@intel.com --- drivers/acpi/video.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index d020df5..9a80a94 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -412,6 +412,12 @@ static int video_ignore_initial_backlight(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{ + use_native_backlight = true; + return 0; +} + static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -504,6 +510,30 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion m4 Notebook PC"), }, }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad T430s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X230", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Lenovo Yoga 13", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), + }, + }, {} };
On Thu, 2013-10-24 at 16:13 +0800, Aaron Lu wrote:
On 10/16/2013 07:33 AM, Rafael J. Wysocki wrote:
On Friday, October 11, 2013 09:27:42 PM Aaron Lu wrote:
v5: 1 Introduce video.use_native_backlight module parameter and set its value to false by default as suggested by Rafael. For Win8 systems which have broken ACPI video backlight control, the parameter can be set to 1 in kernel cmdline to skip registering ACPI video's backlight interface. Due to this change, the acpi_video_verify_backlight_support is moved from video_detect.c to video.c - patch 3/4; 2 Rename bd_list_head and bd_list_mutex in backlight.c to backlight_dev_list and backlight_dev_list_mutex as suggested by Rafael
- patch 1/4.
v4: Remove decleration and stub for acpi_video_unregister_backlight in video.h of patch 2/4 since that function doesn't exist anymore in v3.
v3: 1 Add a new patch 4/4 to fix some problems in thinkpad-acpi module; 2 Remove unnecessary function acpi_video_unregister_backlight introduced in patch 2/4 as pointed out by Jani Nikula.
v2: v1 has the subject of "Rework ACPI video driver" and is posted here: http://lkml.org/lkml/2013/9/9/74 Since the objective is really to fix Win8 backlight issues, I changed the subject in this version, sorry about that.
This patchset has four patches, the first introduced a new API named backlight_device_registered in backlight layer that can be used for backlight interface provider module to check if a specific type backlight interface has been registered, see changelog for patch 1/4 for details. Then patch 2/4 does the cleanup to sepeate the backlight control and event delivery functionality in the ACPI video module and patch 3/4 solves some Win8 backlight control problems by avoiding register ACPI video's backlight interface if: 1 Kernel cmdline option acpi_backlight=video is not given; 2 This is a Win8 system; 3 Native backlight control interface exists. Patch 4/4 fixes some problems in thinkpad-acpi module.
Technically, patch 2/4 is not required to fix the issue here. So if you think it is not necessary, I can remove it from the series.
Aaron Lu (4): backlight: introduce backlight_device_registered ACPI / video: seperate backlight control and event interface ACPI / video: Do not register backlight if win8 and native interface exists thinkpad-acpi: fix handle locate for video and query of _BCL
drivers/acpi/internal.h | 4 +- drivers/acpi/video.c | 457 ++++++++++++++++++++--------------- drivers/acpi/video_detect.c | 4 +- drivers/platform/x86/thinkpad_acpi.c | 31 ++- drivers/video/backlight/backlight.c | 31 +++ include/linux/backlight.h | 4 + 6 files changed, 326 insertions(+), 205 deletions(-)
I've added this series to my queue for 3.13.
Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default.
Hopefully, in the future we'll be able to fix the problems causing video.use_native_backlight=1 to fail of the systems where it fails and then we'll be able to make that the default behavior and drop the option altogether.
I've prepared a patch(at the end of the mail) to set use_native_backlight by default for some systems. There are 3 systems currently that I'm kind of sure that should be added:
The ThinkPad T430s and X230 is: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reported-by: Igor Gnatenko i.gnatenko.brain@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
The Lenovo Yoga is: Reported-by: Lennart Poettering lennart@poettering.net Reference: https://lkml.org/lkml/2013/10/13/178
From: Aaron Lu aaron.lu@intel.com Subject: [PATCH] ACPI / video: Add systems that should favor native backlight interface
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be clear, the ThinkPad T430s/X230 and Lenovo Yoga 13 are added here.
Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reported-by: Lennart Poettering lennart@poettering.net Reported-by: Igor Gnatenko i.gnatenko.brain@gmail.com Signed-off-by: Aaron Lu aaron.lu@intel.com
drivers/acpi/video.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index d020df5..9a80a94 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -412,6 +412,12 @@ static int video_ignore_initial_backlight(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{
- use_native_backlight = true;
- return 0;
+}
static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -504,6 +510,30 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion m4 Notebook PC"), }, },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad T430s",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad X230",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Lenovo Yoga 13",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),},- }, {}
};
Aaron, add this notebook to list. I've CC'ed owner. And I've tested this patch on my TP X230 (add as Reported-and-Tested me please) + { + .callback = video_set_use_native_backlight, + .ident = "Dell Inspiron 7520", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"), + }, + },
On 10/25/2013 02:35 PM, Igor Gnatenko wrote:
Aaron, add this notebook to list. I've CC'ed owner. And I've tested this patch on my TP X230 (add as Reported-and-Tested me please)
- {
.callback = video_set_use_native_backlight,.ident = "Dell Inspiron 7520",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),},- },
Thanks Igor, updated patch follows:
From: Aaron Lu aaron.lu@intel.com Subject: [PATCH] ACPI / video: Add systems that should favour native backlight interface
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be specific, the ThinkPad T430s/X230, Lenovo Yoga 13 and Dell Inspiron 7520 are added here.
Thinkpad T430s: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Thinkpad X230: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Lenovo Yoga 13: Reported-by: Lennart Poettering lennart@poettering.net Reported-and-tested-by: Kevin Smith thirdwiggin@gmail.com Dell Inspiron 7520: Reported-by: Renat Ibragimov ibragimovrinat@mail.ru
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Signed-off-by: Aaron Lu aaron.lu@intel.com --- drivers/acpi/video.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index d020df5..0a1b030 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -412,6 +412,12 @@ static int video_ignore_initial_backlight(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{ + use_native_backlight = true; + return 0; +} + static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -504,6 +510,38 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion m4 Notebook PC"), }, }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad T430s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X230", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Lenovo Yoga 13", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Dell Inspiron 7520", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"), + }, + }, {} };
On Mon, 28 Oct 2013, Aaron Lu aaron.lu@intel.com wrote:
On 10/25/2013 02:35 PM, Igor Gnatenko wrote:
Aaron, add this notebook to list. I've CC'ed owner. And I've tested this patch on my TP X230 (add as Reported-and-Tested me please)
- {
.callback = video_set_use_native_backlight,.ident = "Dell Inspiron 7520",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),},- },
Thanks Igor, updated patch follows:
From: Aaron Lu aaron.lu@intel.com Subject: [PATCH] ACPI / video: Add systems that should favour native backlight interface
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be specific, the ThinkPad T430s/X230, Lenovo Yoga 13 and Dell Inspiron 7520 are added here.
Thinkpad T430s: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Thinkpad X230: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Lenovo Yoga 13: Reported-by: Lennart Poettering lennart@poettering.net Reported-and-tested-by: Kevin Smith thirdwiggin@gmail.com Dell Inspiron 7520: Reported-by: Renat Ibragimov ibragimovrinat@mail.ru
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Signed-off-by: Aaron Lu aaron.lu@intel.com
drivers/acpi/video.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index d020df5..0a1b030 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -412,6 +412,12 @@ static int video_ignore_initial_backlight(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{
- use_native_backlight = true;
- return 0;
+}
Hi Aaron, it might be beneficial to make use_native_backlight support three values: force true, force false, and use platform default based on DMI.
BR, Jani.
static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -504,6 +510,38 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion m4 Notebook PC"), }, },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad T430s",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad X230",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Lenovo Yoga 13",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Dell Inspiron 7520",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),},- }, {}
};
-- 1.8.4.39.ga0d3f10
On 10/28/2013 04:09 PM, Jani Nikula wrote:
On Mon, 28 Oct 2013, Aaron Lu aaron.lu@intel.com wrote:
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{
- use_native_backlight = true;
- return 0;
+}
Hi Aaron, it might be beneficial to make use_native_backlight support three values: force true, force false, and use platform default based on DMI.
Makes sense. I modified the patch a little bit so that if user has specified the cmdline option use_native_backlight=0/1, it will always take effect no matter if the system is in DMI table or not.
From: Aaron Lu aaron.lu@intel.com Subject: [PATCH] ACPI / video: Add systems that should favor native backlight interface
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be specific, the ThinkPad T430s/X230, Lenovo Yoga 13 and Dell Inspiron 7520 are added here.
Note that the user specified kernel cmdline option will always have the highest priority, i.e. if use_native_backlight=0 is specified and the system is in the DMI table, the video module will not skip register backlight interface for it.
Thinkpad T430s: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Thinkpad X230: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Lenovo Yoga 13: Reported-by: Lennart Poettering lennart@poettering.net Reported-and-tested-by: Kevin Smith thirdwiggin@gmail.com Dell Inspiron 7520: Reported-by: Rinat Ibragimov ibragimovrinat@mail.ru
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Signed-off-by: Aaron Lu aaron.lu@intel.com --- drivers/acpi/video.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 38c3a28..41bd4b4 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -89,11 +89,12 @@ static bool use_bios_initial_backlight = 1; module_param(use_bios_initial_backlight, bool, 0644);
/* - * For Windows 8 systems: if set ture and the GPU driver has - * registered a backlight interface, skip registering ACPI video's. + * For Windows 8 systems: used to decide if video module + * should skip registering backlight interface of its own. */ -static bool use_native_backlight = false; -module_param(use_native_backlight, bool, 0644); +static int use_native_backlight_param = -1; +module_param_named(use_native_backlight, use_native_backlight_param, int, 0444); +static bool use_native_backlight_dmi = false;
static int register_count; static struct mutex video_list_lock; @@ -239,9 +240,17 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, static int acpi_video_switch_brightness(struct acpi_video_device *device, int event);
+static bool acpi_video_use_native_backlight(void) +{ + if (use_native_backlight_param != -1) + return !!use_native_backlight_param; + else + return use_native_backlight_dmi; +} + static bool acpi_video_verify_backlight_support(void) { - if (acpi_osi_is_win8() && use_native_backlight && + if (acpi_osi_is_win8() && acpi_video_use_native_backlight() && backlight_device_registered(BACKLIGHT_RAW)) return false; return acpi_video_backlight_support(); @@ -412,6 +421,12 @@ static int video_ignore_initial_backlight(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{ + use_native_backlight_dmi = true; + return 0; +} + static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -512,6 +527,38 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "HP 250 G1 Notebook PC"), }, }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad T430s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X230", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Lenovo Yoga 13", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Dell Inspiron 7520", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"), + }, + }, {} };
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be specific, the ThinkPad T430s/X230, Lenovo Yoga 13, Dell Inspiron 7520 and Acer Aspire 5733Z are added here, if they appear in some other DMI table before, they are removed from there.
Note that the user specified kernel cmdline option will always have the highest priority, i.e. if use_native_backlight=0 is specified and the system is in the DMI table, the video module will not skip registering backlight interface for it.
Thinkpad T430s: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Thinkpad X230: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Lenovo Yoga 13: Reported-by: Lennart Poettering lennart@poettering.net Reported-and-tested-by: Kevin Smith thirdwiggin@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 Dell Inspiron 7520: Reported-by: Rinat Ibragimov ibragimovrinat@mail.ru Acer Aspire 5733Z: Reported-by: sov.info@mail.ru Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941
Signed-off-by: Aaron Lu aaron.lu@intel.com --- v2: Add Acer Aspire 5733Z, see bug #62941; Remove Inspiron 7520 from acpi_osi_dmi_table and Yoga 13 from video_detect_dmi_table.
Based on top of Rafael's linux-next branch + the following patch: [PATCH] ACPI / video: clean up DMI table for initial black screen problem http://marc.info/?l=linux-acpi&m=138448583006432&w=2
drivers/acpi/blacklist.c | 8 ------ drivers/acpi/video.c | 65 +++++++++++++++++++++++++++++++++++++++++---- drivers/acpi/video_detect.c | 8 ------ 3 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index 078c4f7fe2dd..2b6a76b6d59a 100644 --- a/drivers/acpi/blacklist.c +++ b/drivers/acpi/blacklist.c @@ -261,14 +261,6 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = { }, { .callback = dmi_disable_osi_win8, - .ident = "Dell Inspiron 15R SE", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), - DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7520"), - }, - }, - { - .callback = dmi_disable_osi_win8, .ident = "ThinkPad Edge E530", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 4ccb89e5c4ad..49abe0348b03 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -89,11 +89,12 @@ static bool use_bios_initial_backlight = 1; module_param(use_bios_initial_backlight, bool, 0644);
/* - * For Windows 8 systems: if set ture and the GPU driver has - * registered a backlight interface, skip registering ACPI video's. + * For Windows 8 systems: used to decide if video module + * should skip registering backlight interface of its own. */ -static bool use_native_backlight = false; -module_param(use_native_backlight, bool, 0644); +static int use_native_backlight_param = -1; +module_param_named(use_native_backlight, use_native_backlight_param, int, 0444); +static bool use_native_backlight_dmi = false;
static int register_count; static struct mutex video_list_lock; @@ -239,9 +240,17 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, static int acpi_video_switch_brightness(struct acpi_video_device *device, int event);
+static bool acpi_video_use_native_backlight(void) +{ + if (use_native_backlight_param != -1) + return !!use_native_backlight_param; + else + return use_native_backlight_dmi; +} + static bool acpi_video_verify_backlight_support(void) { - if (acpi_osi_is_win8() && use_native_backlight && + if (acpi_osi_is_win8() && acpi_video_use_native_backlight() && backlight_device_registered(BACKLIGHT_RAW)) return false; return acpi_video_backlight_support(); @@ -412,6 +421,12 @@ static int video_ignore_initial_backlight(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{ + use_native_backlight_dmi = true; + return 0; +} + static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -456,6 +471,46 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"), }, }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad T430s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X230", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Lenovo Yoga 13", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Dell Inspiron 7520", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Acer Aspire 5733Z", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5733Z"), + }, + }, {} };
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 84875fd4c74f..b6399343de51 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -168,14 +168,6 @@ static struct dmi_system_id video_detect_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"), }, }, - { - .callback = video_detect_force_vendor, - .ident = "Lenovo Yoga 13", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), - }, - }, { }, };
On Fri, 2013-11-15 at 14:07 +0800, Aaron Lu wrote:
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be specific, the ThinkPad T430s/X230, Lenovo Yoga 13, Dell Inspiron 7520 and Acer Aspire 5733Z are added here, if they appear in some other DMI table before, they are removed from there.
Note that the user specified kernel cmdline option will always have the highest priority, i.e. if use_native_backlight=0 is specified and the system is in the DMI table, the video module will not skip registering backlight interface for it.
Thinkpad T430s: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Thinkpad X230: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Lenovo Yoga 13: Reported-by: Lennart Poettering lennart@poettering.net Reported-and-tested-by: Kevin Smith thirdwiggin@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 Dell Inspiron 7520: Reported-by: Rinat Ibragimov ibragimovrinat@mail.ru Acer Aspire 5733Z: Reported-by: sov.info@mail.ru Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941
Signed-off-by: Aaron Lu aaron.lu@intel.com
v2: Add Acer Aspire 5733Z, see bug #62941; Remove Inspiron 7520 from acpi_osi_dmi_table and Yoga 13 from video_detect_dmi_table.
Based on top of Rafael's linux-next branch + the following patch: [PATCH] ACPI / video: clean up DMI table for initial black screen problem http://marc.info/?l=linux-acpi&m=138448583006432&w=2
drivers/acpi/blacklist.c | 8 ------ drivers/acpi/video.c | 65 +++++++++++++++++++++++++++++++++++++++++---- drivers/acpi/video_detect.c | 8 ------ 3 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index 078c4f7fe2dd..2b6a76b6d59a 100644 --- a/drivers/acpi/blacklist.c +++ b/drivers/acpi/blacklist.c @@ -261,14 +261,6 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = { }, { .callback = dmi_disable_osi_win8,
- .ident = "Dell Inspiron 15R SE",
- .matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7520"),},- },
- {
- .callback = dmi_disable_osi_win8, .ident = "ThinkPad Edge E530", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 4ccb89e5c4ad..49abe0348b03 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -89,11 +89,12 @@ static bool use_bios_initial_backlight = 1; module_param(use_bios_initial_backlight, bool, 0644);
/*
- For Windows 8 systems: if set ture and the GPU driver has
- registered a backlight interface, skip registering ACPI video's.
- For Windows 8 systems: used to decide if video module
*/
- should skip registering backlight interface of its own.
-static bool use_native_backlight = false; -module_param(use_native_backlight, bool, 0644); +static int use_native_backlight_param = -1; +module_param_named(use_native_backlight, use_native_backlight_param, int, 0444); +static bool use_native_backlight_dmi = false;
static int register_count; static struct mutex video_list_lock; @@ -239,9 +240,17 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, static int acpi_video_switch_brightness(struct acpi_video_device *device, int event);
+static bool acpi_video_use_native_backlight(void) +{
- if (use_native_backlight_param != -1)
return !!use_native_backlight_param;- else
return use_native_backlight_dmi;+}
static bool acpi_video_verify_backlight_support(void) {
- if (acpi_osi_is_win8() && use_native_backlight &&
- if (acpi_osi_is_win8() && acpi_video_use_native_backlight() && backlight_device_registered(BACKLIGHT_RAW)) return false; return acpi_video_backlight_support();
@@ -412,6 +421,12 @@ static int video_ignore_initial_backlight(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{
- use_native_backlight_dmi = true;
- return 0;
+}
static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -456,6 +471,46 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"), }, },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad T430s",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad X230",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Lenovo Yoga 13",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Dell Inspiron 7520",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Acer Aspire 5733Z",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Acer"),DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5733Z"),},- }, {}
};
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 84875fd4c74f..b6399343de51 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -168,14 +168,6 @@ static struct dmi_system_id video_detect_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"), }, },
- {
- .callback = video_detect_force_vendor,
- .ident = "Lenovo Yoga 13",
- .matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),},- }, { },
};
Any news here? If no - I think we need re-send patch as new..
On 11/21/2013 04:56 AM, Igor Gnatenko wrote:
Any news here? If no - I think we need re-send patch as new..
Since the v2 patch can't apply cleanly on top of pm's -next tree, I think it's worth a re-send, so here it comes.
--- Subject: [PATCH] ACPI / video: Add systems that should favor native backlight interface From: Aaron Lu aaron.lu@intel.com Date: Thu, 21 Nov 2013 11:24:48 +0800
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be specific, the ThinkPad T430s/X230, Lenovo Yoga 13, Dell Inspiron 7520 and Acer Aspire 5733Z are added here, if they appear in some other DMI table before, they are removed from there.
Note that the user specified kernel cmdline option will always have the highest priority, i.e. if use_native_backlight=0 is specified and the system is in the DMI table, the video module will not skip registering backlight interface for it.
Thinkpad T430s: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Thinkpad X230: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Lenovo Yoga 13: Reported-by: Lennart Poettering lennart@poettering.net Reported-and-tested-by: Kevin Smith thirdwiggin@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 Dell Inspiron 7520: Reported-by: Rinat Ibragimov ibragimovrinat@mail.ru Acer Aspire 5733Z: Reported-by: sov.info@mail.ru Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941
Signed-off-by: Aaron Lu aaron.lu@intel.com --- drivers/acpi/blacklist.c | 8 ------ drivers/acpi/video.c | 65 +++++++++++++++++++++++++++++++++++++++++---- drivers/acpi/video_detect.c | 8 ------ 3 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index 078c4f7fe2dd..2b6a76b6d59a 100644 --- a/drivers/acpi/blacklist.c +++ b/drivers/acpi/blacklist.c @@ -261,14 +261,6 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = { }, { .callback = dmi_disable_osi_win8, - .ident = "Dell Inspiron 15R SE", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), - DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7520"), - }, - }, - { - .callback = dmi_disable_osi_win8, .ident = "ThinkPad Edge E530", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 995e91bcb97b..7dc6071a04b6 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -82,11 +82,12 @@ static bool allow_duplicates; module_param(allow_duplicates, bool, 0644);
/* - * For Windows 8 systems: if set ture and the GPU driver has - * registered a backlight interface, skip registering ACPI video's. + * For Windows 8 systems: used to decide if video module + * should skip registering backlight interface of its own. */ -static bool use_native_backlight = false; -module_param(use_native_backlight, bool, 0644); +static int use_native_backlight_param = -1; +module_param_named(use_native_backlight, use_native_backlight_param, int, 0444); +static bool use_native_backlight_dmi = false;
static int register_count; static struct mutex video_list_lock; @@ -232,9 +233,17 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, static int acpi_video_switch_brightness(struct acpi_video_device *device, int event);
+static bool acpi_video_use_native_backlight(void) +{ + if (use_native_backlight_param != -1) + return use_native_backlight_param; + else + return use_native_backlight_dmi; +} + static bool acpi_video_verify_backlight_support(void) { - if (acpi_osi_is_win8() && use_native_backlight && + if (acpi_osi_is_win8() && acpi_video_use_native_backlight() && backlight_device_registered(BACKLIGHT_RAW)) return false; return acpi_video_backlight_support(); @@ -399,6 +408,12 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{ + use_native_backlight_dmi = true; + return 0; +} + static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -443,6 +458,46 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"), }, }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad T430s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X230", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Lenovo Yoga 13", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Dell Inspiron 7520", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"), + }, + }, + { + .callback = video_set_use_native_backlight, + .ident = "Acer Aspire 5733Z", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5733Z"), + }, + }, {} };
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 84875fd4c74f..b6399343de51 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -168,14 +168,6 @@ static struct dmi_system_id video_detect_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"), }, }, - { - .callback = video_detect_force_vendor, - .ident = "Lenovo Yoga 13", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"), - }, - }, { }, };
Hi,
please add some updates ;)
On Thu, 2013-11-21 at 13:29 +0800, Aaron Lu wrote:
On 11/21/2013 04:56 AM, Igor Gnatenko wrote:
Any news here? If no - I think we need re-send patch as new..
Since the v2 patch can't apply cleanly on top of pm's -next tree, I think it's worth a re-send, so here it comes.
Subject: [PATCH] ACPI / video: Add systems that should favor native backlight interface From: Aaron Lu aaron.lu@intel.com Date: Thu, 21 Nov 2013 11:24:48 +0800
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be specific, the ThinkPad T430s/X230, Lenovo Yoga 13, Dell Inspiron 7520 and Acer Aspire 5733Z are added here, if they appear in some other DMI table before, they are removed from there.
Note that the user specified kernel cmdline option will always have the highest priority, i.e. if use_native_backlight=0 is specified and the system is in the DMI table, the video module will not skip registering backlight interface for it.
Thinkpad T430s: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Thinkpad X230: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
ThinkPad X1 Carbon: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
Lenovo Yoga 13: Reported-by: Lennart Poettering lennart@poettering.net Reported-and-tested-by: Kevin Smith thirdwiggin@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 Dell Inspiron 7520: Reported-by: Rinat Ibragimov ibragimovrinat@mail.ru Acer Aspire 5733Z: Reported-by: sov.info@mail.ru Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941
HP ProBook 4340s: Reported-and-tested-by: Vladimir Sherenkov a_12300@mail.ru Reference: http://redmine.russianfedora.pro/issues/1258
Signed-off-by: Aaron Lu aaron.lu@intel.com
drivers/acpi/blacklist.c | 8 ------ drivers/acpi/video.c | 65 +++++++++++++++++++++++++++++++++++++++++---- drivers/acpi/video_detect.c | 8 ------ 3 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index 078c4f7fe2dd..2b6a76b6d59a 100644 --- a/drivers/acpi/blacklist.c +++ b/drivers/acpi/blacklist.c @@ -261,14 +261,6 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = { }, { .callback = dmi_disable_osi_win8,
- .ident = "Dell Inspiron 15R SE",
- .matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7520"),},- },
- {
- .callback = dmi_disable_osi_win8, .ident = "ThinkPad Edge E530", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 995e91bcb97b..7dc6071a04b6 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -82,11 +82,12 @@ static bool allow_duplicates; module_param(allow_duplicates, bool, 0644);
/*
- For Windows 8 systems: if set ture and the GPU driver has
- registered a backlight interface, skip registering ACPI video's.
- For Windows 8 systems: used to decide if video module
*/
- should skip registering backlight interface of its own.
-static bool use_native_backlight = false; -module_param(use_native_backlight, bool, 0644); +static int use_native_backlight_param = -1; +module_param_named(use_native_backlight, use_native_backlight_param, int, 0444); +static bool use_native_backlight_dmi = false;
static int register_count; static struct mutex video_list_lock; @@ -232,9 +233,17 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, static int acpi_video_switch_brightness(struct acpi_video_device *device, int event);
+static bool acpi_video_use_native_backlight(void) +{
- if (use_native_backlight_param != -1)
return use_native_backlight_param;- else
return use_native_backlight_dmi;+}
static bool acpi_video_verify_backlight_support(void) {
- if (acpi_osi_is_win8() && use_native_backlight &&
- if (acpi_osi_is_win8() && acpi_video_use_native_backlight() && backlight_device_registered(BACKLIGHT_RAW)) return false; return acpi_video_backlight_support();
@@ -399,6 +408,12 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{
- use_native_backlight_dmi = true;
- return 0;
+}
static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -443,6 +458,46 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"), }, },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad T430s",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad X230",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Lenovo Yoga 13",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Dell Inspiron 7520",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Acer Aspire 5733Z",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Acer"),DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5733Z"),},- }, {}
};
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 84875fd4c74f..b6399343de51 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -168,14 +168,6 @@ static struct dmi_system_id video_detect_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"), }, },
- {
- .callback = video_detect_force_vendor,
- .ident = "Lenovo Yoga 13",
- .matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),},- }, { },
};
drivers/acpi/video.c
+ { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X1 Carbon", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X1 Carbon"), + }, + },
+ { + .callback = video_set_use_native_backlight, + .ident = "HP ProBook 4340s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"), + DMI_MATCH(DMI_PRODUCT_VERSION, "HP ProBook 4340s"), + }, + },
Hi,
add some updates to the patch ;)
On Thu, 2013-11-21 at 13:29 +0800, Aaron Lu wrote:
On 11/21/2013 04:56 AM, Igor Gnatenko wrote:
Any news here? If no - I think we need re-send patch as new..
Since the v2 patch can't apply cleanly on top of pm's -next tree, I think it's worth a re-send, so here it comes.
Subject: [PATCH] ACPI / video: Add systems that should favor native backlight interface From: Aaron Lu aaron.lu@intel.com Date: Thu, 21 Nov 2013 11:24:48 +0800
Some system's ACPI video backlight control interface is broken and the native backlight control interface should be used by default. This patch sets the use_native_backlight parameter to true for those systems so that video backlight control interface will not be created. To be specific, the ThinkPad T430s/X230, Lenovo Yoga 13, Dell Inspiron 7520 and Acer Aspire 5733Z are added here, if they appear in some other DMI table before, they are removed from there.
Note that the user specified kernel cmdline option will always have the highest priority, i.e. if use_native_backlight=0 is specified and the system is in the DMI table, the video module will not skip registering backlight interface for it.
Thinkpad T430s: Reported-by: Theodore Tso tytso@mit.edu Reported-and-tested-by: Peter Weber bugs@ttyhoney.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231 Thinkpad X230: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=51231
ThinkPad X1 Carbon: Reported-and-tested-by: Igor Gnatenko i.gnatenko.brain@gmail.com
Lenovo Yoga 13: Reported-by: Lennart Poettering lennart@poettering.net Reported-and-tested-by: Kevin Smith thirdwiggin@gmail.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=63811 Dell Inspiron 7520: Reported-by: Rinat Ibragimov ibragimovrinat@mail.ru Acer Aspire 5733Z: Reported-by: sov.info@mail.ru Reference: https://bugzilla.kernel.org/show_bug.cgi?id=62941
HP ProBook 4340s: Reported-and-tested-by: Vladimir Sherenkov a_12300@mail.ru Reference: http://redmine.russianfedora.pro/issues/1258
Signed-off-by: Aaron Lu aaron.lu@intel.com
drivers/acpi/blacklist.c | 8 ------ drivers/acpi/video.c | 65 +++++++++++++++++++++++++++++++++++++++++---- drivers/acpi/video_detect.c | 8 ------ 3 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index 078c4f7fe2dd..2b6a76b6d59a 100644 --- a/drivers/acpi/blacklist.c +++ b/drivers/acpi/blacklist.c @@ -261,14 +261,6 @@ static struct dmi_system_id acpi_osi_dmi_table[] __initdata = { }, { .callback = dmi_disable_osi_win8,
- .ident = "Dell Inspiron 15R SE",
- .matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7520"),},- },
- {
- .callback = dmi_disable_osi_win8, .ident = "ThinkPad Edge E530", .matches = { DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 995e91bcb97b..7dc6071a04b6 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -82,11 +82,12 @@ static bool allow_duplicates; module_param(allow_duplicates, bool, 0644);
/*
- For Windows 8 systems: if set ture and the GPU driver has
- registered a backlight interface, skip registering ACPI video's.
- For Windows 8 systems: used to decide if video module
*/
- should skip registering backlight interface of its own.
-static bool use_native_backlight = false; -module_param(use_native_backlight, bool, 0644); +static int use_native_backlight_param = -1; +module_param_named(use_native_backlight, use_native_backlight_param, int, 0444); +static bool use_native_backlight_dmi = false;
static int register_count; static struct mutex video_list_lock; @@ -232,9 +233,17 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, static int acpi_video_switch_brightness(struct acpi_video_device *device, int event);
+static bool acpi_video_use_native_backlight(void) +{
- if (use_native_backlight_param != -1)
return use_native_backlight_param;- else
return use_native_backlight_dmi;+}
static bool acpi_video_verify_backlight_support(void) {
- if (acpi_osi_is_win8() && use_native_backlight &&
- if (acpi_osi_is_win8() && acpi_video_use_native_backlight() && backlight_device_registered(BACKLIGHT_RAW)) return false; return acpi_video_backlight_support();
@@ -399,6 +408,12 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d) return 0; }
+static int __init video_set_use_native_backlight(const struct dmi_system_id *d) +{
- use_native_backlight_dmi = true;
- return 0;
+}
static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -443,6 +458,46 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"), }, },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad T430s",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T430s"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "ThinkPad X230",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X230"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Lenovo Yoga 13",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Dell Inspiron 7520",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),DMI_MATCH(DMI_PRODUCT_VERSION, "Inspiron 7520"),},- },
- {
.callback = video_set_use_native_backlight,.ident = "Acer Aspire 5733Z",.matches = {DMI_MATCH(DMI_SYS_VENDOR, "Acer"),DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 5733Z"),},- }, {}
};
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 84875fd4c74f..b6399343de51 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -168,14 +168,6 @@ static struct dmi_system_id video_detect_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"), }, },
- {
- .callback = video_detect_force_vendor,
- .ident = "Lenovo Yoga 13",
- .matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),},- }, { },
};
drivers/acpi/video.c
+ { + .callback = video_set_use_native_backlight, + .ident = "ThinkPad X1 Carbon", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X1 Carbon"), + }, + },
+ { + .callback = video_set_use_native_backlight, + .ident = "HP ProBook 4340s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"), + DMI_MATCH(DMI_PRODUCT_VERSION, "HP ProBook 4340s"), + }, + },
On Wed, 2013-10-16 at 01:33 +0200, Rafael J. Wysocki wrote:
Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default.
I'd still really prefer not to add such a list, because it almost inevitably means that we'll never fix this problem properly.
On Wednesday, October 30, 2013 12:40:13 AM Matthew Garrett wrote:
On Wed, 2013-10-16 at 01:33 +0200, Rafael J. Wysocki wrote:
Since the next step will be to introduce a list of systems that need video.use_native_backlight=1 *and* don't break in that configuration, I don't see much point adding another Kconfig option for the default.
I'd still really prefer not to add such a list, because it almost inevitably means that we'll never fix this problem properly.
We have this list in blacklist.c anyway.
Thanks!
dri-devel@lists.freedesktop.org
-
Aaron Lu -
Igor Gnatenko -
Jani Nikula -
Josh Boyer -
Matthew Garrett -
Rafael J. Wysocki -
Seth Forshee -
Yves-Alexis Perez