Hi All,
Here is the long promised rewrite / cleanup of the acpi-video (and platform/x86) backlight interface selection logic.
Reviewers do not be alarmed by the large number of patches, the gist of the series is in patches 6-13, the earlier ones are a few simple prepararion patches, and the later ones all port over drivers to the new API one at a time.
I've had to jump through some hoops to make it possible to still build the acpi_video code as a module. One downside of this is that the kernel commandline parameters have a changed prefix now. If people do not like this, an alternative would to change CONFIG_ACPI_VIDEO from a tristate to a bool, iow either build it in, or do not build it at all.
Regards,
Hans
acpi_video_dmi_demote_vendor() is going away as part of the cleanup of the code for determinging which backlight class driver(s) to register.
The call to acpi_video_dmi_demote_vendor() was meant to undo the call to acpi_video_dmi_promote_vendor() when the gmux device is removed, this is questionable though since the promote call sets a flag, not a counter, so the demote call may undo a promoto done elsewhere. Moreover in practice this is a nop since the gmux device is never removed, and the flag is only checked when acpi/video.ko gets loaded, so even if the user manually removes apple-gmux the demote call is still a nop as video.ko will already have loaded by this time.
Also note that none of the other users of acpi_video_dmi_promote_vendor() use acpi_video_dmi_demote_vendor().
If we ever encounter a system with a gmux where the acpi-video interface should be used, then the proper fix would be to dmi-blacklist the gmux driver on that system.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/apple-gmux.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c index 45032ce..a7f6412 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -645,7 +645,6 @@ static void gmux_remove(struct pnp_dev *pnp) apple_gmux_data = NULL; kfree(gmux_data);
- acpi_video_dmi_demote_vendor(); acpi_video_register(); apple_bl_register(); }
Remove the now unused acpi_video_dmi_demote_vendor() function, this was never a proper counter part of acpi_video_dmi_promote_vendor() since the calls to acpi_video_dmi_promote_vendor() are not counted.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video_detect.c | 9 --------- include/linux/acpi.h | 5 ----- 2 files changed, 14 deletions(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index c42feb2..6ca1f27 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -258,15 +258,6 @@ void acpi_video_dmi_promote_vendor(void) } EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
-/* To be called when a driver who previously promoted the vendor - * interface */ -void acpi_video_dmi_demote_vendor(void) -{ - acpi_video_caps_check(); - acpi_video_support &= ~ACPI_VIDEO_BACKLIGHT_DMI_VENDOR; -} -EXPORT_SYMBOL(acpi_video_dmi_demote_vendor); - /* Returns true if video.ko can do backlight switching */ int acpi_video_backlight_support(void) { diff --git a/include/linux/acpi.h b/include/linux/acpi.h index e4da5e3..01bffd3 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -248,7 +248,6 @@ extern bool wmi_has_guid(const char *guid); extern long acpi_video_get_capabilities(acpi_handle graphics_dev_handle); extern long acpi_is_video_device(acpi_handle handle); extern void acpi_video_dmi_promote_vendor(void); -extern void acpi_video_dmi_demote_vendor(void); extern int acpi_video_backlight_support(void); extern int acpi_video_display_switch_support(void);
@@ -268,10 +267,6 @@ static inline void acpi_video_dmi_promote_vendor(void) { }
-static inline void acpi_video_dmi_demote_vendor(void) -{ -} - static inline int acpi_video_backlight_support(void) { return 0;
acpi_video_get_capabilities() is only used inside video_detect.c so make it static. While at it also remove the prototype for the non existent acpi_video_display_switch_support function from acpi.h
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video_detect.c | 3 +-- include/linux/acpi.h | 12 ------------ 2 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 6ca1f27..68c45c3 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -184,7 +184,7 @@ static struct dmi_system_id video_detect_dmi_table[] = { * all graphics capabilities of physically present devices are * summarized and returned. This is cached and done only once. */ -long acpi_video_get_capabilities(acpi_handle graphics_handle) +static long acpi_video_get_capabilities(acpi_handle graphics_handle) { long caps = 0; struct acpi_device *tmp_dev; @@ -227,7 +227,6 @@ long acpi_video_get_capabilities(acpi_handle graphics_handle) graphics_handle ? acpi_device_bid(tmp_dev) : "")); return caps; } -EXPORT_SYMBOL(acpi_video_get_capabilities);
static void acpi_video_caps_check(void) { diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 01bffd3..88c92a03 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -245,19 +245,12 @@ extern bool wmi_has_guid(const char *guid);
#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
-extern long acpi_video_get_capabilities(acpi_handle graphics_dev_handle); extern long acpi_is_video_device(acpi_handle handle); extern void acpi_video_dmi_promote_vendor(void); extern int acpi_video_backlight_support(void); -extern int acpi_video_display_switch_support(void);
#else
-static inline long acpi_video_get_capabilities(acpi_handle graphics_dev_handle) -{ - return 0; -} - static inline long acpi_is_video_device(acpi_handle handle) { return 0; @@ -272,11 +265,6 @@ static inline int acpi_video_backlight_support(void) return 0; }
-static inline int acpi_video_display_switch_support(void) -{ - return 0; -} - #endif /* defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) */
extern int acpi_blacklisted(void);
This allows video_detect.c to be build as a module, this is a preparation patch for the backlight interface selection logic cleanup.
Note this commit also causes acpi_is_video_device() to always be build indepedent of CONFIG_ACPI_VIDEO, as there is no reason to make its building depend on CONFIG_ACPI_VIDEO.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/scan.c | 56 ++++++++++++++++++++++++++++++++++++++++++ drivers/acpi/video_detect.c | 60 --------------------------------------------- include/linux/acpi.h | 8 ++---- 3 files changed, 58 insertions(+), 66 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 03141aa..811b40d 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1947,6 +1947,62 @@ bool acpi_dock_match(acpi_handle handle) return acpi_has_method(handle, "_DCK"); }
+static acpi_status +acpi_backlight_cap_match(acpi_handle handle, u32 level, void *context, + void **return_value) +{ + long *cap = context; + + if (acpi_has_method(handle, "_BCM") && + acpi_has_method(handle, "_BCL")) { + ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found generic backlight " + "support\n")); + *cap |= ACPI_VIDEO_BACKLIGHT; + if (!acpi_has_method(handle, "_BQC")) + printk(KERN_WARNING FW_BUG PREFIX "No _BQC method, " + "cannot determine initial brightness\n"); + /* We have backlight support, no need to scan further */ + return AE_CTRL_TERMINATE; + } + return 0; +} + +/* Returns true if the ACPI object is a video device which can be + * handled by video.ko. + * The device will get a Linux specific CID added in scan.c to + * identify the device as an ACPI graphics device + * Be aware that the graphics device may not be physically present + * Use acpi_video_get_capabilities() to detect general ACPI video + * capabilities of present cards + */ +long acpi_is_video_device(acpi_handle handle) +{ + long video_caps = 0; + + /* Is this device able to support video switching ? */ + if (acpi_has_method(handle, "_DOD") || acpi_has_method(handle, "_DOS")) + video_caps |= ACPI_VIDEO_OUTPUT_SWITCHING; + + /* Is this device able to retrieve a video ROM ? */ + if (acpi_has_method(handle, "_ROM")) + video_caps |= ACPI_VIDEO_ROM_AVAILABLE; + + /* Is this device able to configure which video head to be POSTed ? */ + if (acpi_has_method(handle, "_VPO") && + acpi_has_method(handle, "_GPD") && + acpi_has_method(handle, "_SPD")) + video_caps |= ACPI_VIDEO_DEVICE_POSTING; + + /* Only check for backlight functionality if one of the above hit. */ + if (video_caps) + acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, + ACPI_UINT32_MAX, acpi_backlight_cap_match, NULL, + &video_caps, NULL); + + return video_caps; +} +EXPORT_SYMBOL(acpi_is_video_device); + const char *acpi_device_hid(struct acpi_device *device) { struct acpi_hardware_id *hid; diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 68c45c3..aad5cf4 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -5,10 +5,6 @@ * May be copied or modified under the terms of the GNU General Public License * * video_detect.c: - * Provides acpi_is_video_device() for early scanning of ACPI devices in scan.c - * There a Linux specific (Spec does not provide a HID for video devices) is - * assigned - * * After PCI devices are glued with ACPI devices * acpi_get_pci_dev() can be called to identify ACPI graphics * devices for which a real graphics card is plugged in @@ -47,62 +43,6 @@ static long acpi_video_support; static bool acpi_video_caps_checked;
static acpi_status -acpi_backlight_cap_match(acpi_handle handle, u32 level, void *context, - void **return_value) -{ - long *cap = context; - - if (acpi_has_method(handle, "_BCM") && - acpi_has_method(handle, "_BCL")) { - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found generic backlight " - "support\n")); - *cap |= ACPI_VIDEO_BACKLIGHT; - if (!acpi_has_method(handle, "_BQC")) - printk(KERN_WARNING FW_BUG PREFIX "No _BQC method, " - "cannot determine initial brightness\n"); - /* We have backlight support, no need to scan further */ - return AE_CTRL_TERMINATE; - } - return 0; -} - -/* Returns true if the ACPI object is a video device which can be - * handled by video.ko. - * The device will get a Linux specific CID added in scan.c to - * identify the device as an ACPI graphics device - * Be aware that the graphics device may not be physically present - * Use acpi_video_get_capabilities() to detect general ACPI video - * capabilities of present cards - */ -long acpi_is_video_device(acpi_handle handle) -{ - long video_caps = 0; - - /* Is this device able to support video switching ? */ - if (acpi_has_method(handle, "_DOD") || acpi_has_method(handle, "_DOS")) - video_caps |= ACPI_VIDEO_OUTPUT_SWITCHING; - - /* Is this device able to retrieve a video ROM ? */ - if (acpi_has_method(handle, "_ROM")) - video_caps |= ACPI_VIDEO_ROM_AVAILABLE; - - /* Is this device able to configure which video head to be POSTed ? */ - if (acpi_has_method(handle, "_VPO") && - acpi_has_method(handle, "_GPD") && - acpi_has_method(handle, "_SPD")) - video_caps |= ACPI_VIDEO_DEVICE_POSTING; - - /* Only check for backlight functionality if one of the above hit. */ - if (video_caps) - acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, - ACPI_UINT32_MAX, acpi_backlight_cap_match, NULL, - &video_caps, NULL); - - return video_caps; -} -EXPORT_SYMBOL(acpi_is_video_device); - -static acpi_status find_video(acpi_handle handle, u32 lvl, void *context, void **rv) { long *cap = context; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 88c92a03..7cb3b0b 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -243,19 +243,15 @@ extern bool wmi_has_guid(const char *guid); #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR 0x0400 #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VIDEO 0x0800
+extern long acpi_is_video_device(acpi_handle handle); + #if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
-extern long acpi_is_video_device(acpi_handle handle); extern void acpi_video_dmi_promote_vendor(void); extern int acpi_video_backlight_support(void);
#else
-static inline long acpi_is_video_device(acpi_handle handle) -{ - return 0; -} - static inline void acpi_video_dmi_promote_vendor(void) { }
acpi_osi_is_win8 needs access to acpi_gbl_osi_data which is not exported, so move it to osl.c. Alternatively we could export acpi_gbl_osi_data but that seems undesirable.
This allows video_detect.c to be build as a module, besides that acpi_osi_is_win8() is something which does not really belong in video_detect.c in the first place.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/internal.h | 7 ------- drivers/acpi/osl.c | 6 ++++++ drivers/acpi/video_detect.c | 6 ------ include/linux/acpi.h | 1 + 4 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index ba4a61e..c370676 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -182,13 +182,6 @@ static inline void suspend_nvs_restore(void) {} #endif
/*-------------------------------------------------------------------------- - Video - -------------------------------------------------------------------------- */ -#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) -bool acpi_osi_is_win8(void); -#endif - -/*-------------------------------------------------------------------------- Device properties -------------------------------------------------------------------------- */ void acpi_init_properties(struct acpi_device *adev); diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 7ccba39..b906deb 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -1684,6 +1684,12 @@ int acpi_resources_are_enforced(void) } EXPORT_SYMBOL(acpi_resources_are_enforced);
+bool acpi_osi_is_win8(void) +{ + return acpi_gbl_osi_data >= ACPI_OSI_WIN_8; +} +EXPORT_SYMBOL(acpi_osi_is_win8); + /* * Deallocate the memory for a spinlock. */ diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index aad5cf4..026376d 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -178,12 +178,6 @@ static void acpi_video_caps_check(void) acpi_video_get_capabilities(NULL); }
-bool acpi_osi_is_win8(void) -{ - return acpi_gbl_osi_data >= ACPI_OSI_WIN_8; -} -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 * platform drivers instead of putting a big blacklist in video_detect.c diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 7cb3b0b..913a1c1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -266,6 +266,7 @@ static inline int acpi_video_backlight_support(void) extern int acpi_blacklisted(void); extern void acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d); extern void acpi_osi_setup(char *str); +extern bool acpi_osi_is_win8(void);
#ifdef CONFIG_ACPI_NUMA int acpi_get_node(acpi_handle handle);
This is a preparation patch for the backlight interface selection logic cleanup, there are 2 reasons to not always build the video_detect code into the kernel:
1) In order for the video_detect.c to also deal with / select native backlight interfaces on win8 systems, instead of doing this in video.c where it does not belong, video_detect.c needs to call into the backlight class code. Which cannot be done if it is builtin and the blacklight class is not.
2) Currently all the platform/x86 drivers which have quirks to prefer the vendor driver over acpi-video call acpi_video_unregister_backlight() to remove the acpi-video backlight interface, this logic really belongs in video_detect.c, which will cause video_detect.c to depend on symbols of video.c and video.c already depends on video_detect.c symbols, so they really need to be a single module.
Note that as a side-effect this commit also changes the acpi_backlight cmdline option to acpi_video.backlight and the prefix for other acpi-video module parameters from video.xxx to acpi_video.xxx, which actually is a better prefix. This may require people relying on these options to change there kernel cmdline, which is undesirable, OTOH people should really not be relying on these options, and instead report bugs for us to fix when they need them.
This commit also makes changes to drivers/platform/x86/Kconfig to ensure that drivers which use acpi_video_backlight_support() from video_detect.c, will not be built-in when acpi_video is not built in. This also changes some "select" uses to "depends on" to avoid dependency loops.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/Makefile | 7 +++---- drivers/acpi/video.c | 2 +- drivers/acpi/video_detect.c | 40 +++++++++++++++++----------------------- drivers/platform/x86/Kconfig | 21 ++++++++++++++++----- 4 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 8a063e2..1008d9d 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -52,9 +52,6 @@ acpi-$(CONFIG_X86) += acpi_cmos_rtc.o acpi-$(CONFIG_DEBUG_FS) += debugfs.o acpi-$(CONFIG_ACPI_NUMA) += numa.o acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o -ifdef CONFIG_ACPI_VIDEO -acpi-y += video_detect.o -endif acpi-y += acpi_lpat.o acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
@@ -66,7 +63,7 @@ obj-$(CONFIG_ACPI_IPMI) += acpi_ipmi.o obj-$(CONFIG_ACPI_AC) += ac.o obj-$(CONFIG_ACPI_BUTTON) += button.o obj-$(CONFIG_ACPI_FAN) += fan.o -obj-$(CONFIG_ACPI_VIDEO) += video.o +obj-$(CONFIG_ACPI_VIDEO) += acpi_video.o obj-$(CONFIG_ACPI_PCI_SLOT) += pci_slot.o obj-$(CONFIG_ACPI_PROCESSOR) += processor.o obj-y += container.o @@ -95,3 +92,5 @@ obj-$(CONFIG_ACPI_EXTLOG) += acpi_extlog.o obj-$(CONFIG_PMIC_OPREGION) += pmic/intel_pmic.o obj-$(CONFIG_CRC_PMIC_OPREGION) += pmic/intel_pmic_crc.o obj-$(CONFIG_XPOWER_PMIC_OPREGION) += pmic/intel_pmic_xpower.o + +acpi_video-objs += video_detect.o video.o diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 518f0e1..6249968 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -43,7 +43,7 @@ #include <acpi/video.h> #include <asm/uaccess.h>
-#include "internal.h" +#define PREFIX "ACPI: "
#define ACPI_VIDEO_BUS_NAME "Video Bus" #define ACPI_VIDEO_DEVICE_NAME "Video Device" diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 026376d..0bc8b98 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -32,16 +32,28 @@ #include <linux/export.h> #include <linux/acpi.h> #include <linux/dmi.h> +#include <linux/module.h> #include <linux/pci.h>
-#include "internal.h" - ACPI_MODULE_NAME("video"); #define _COMPONENT ACPI_VIDEO_COMPONENT
static long acpi_video_support; static bool acpi_video_caps_checked;
+static char acpi_backlight_str[16]; +module_param_string(backlight, acpi_backlight_str, 16, 0444); +MODULE_PARM_DESC(backlight, + "Select which backlight interface to use [vendor|video]"); + +static void acpi_video_parse_cmdline(void) +{ + if (!strcmp("vendor", acpi_backlight_str)) + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR; + if (!strcmp("video", acpi_backlight_str)) + acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO; +} + static acpi_status find_video(acpi_handle handle, u32 lvl, void *context, void **rv) { @@ -174,8 +186,10 @@ static void acpi_video_caps_check(void) * We must check whether the ACPI graphics device is physically plugged * in. Therefore this must be called after binding PCI and ACPI devices */ - if (!acpi_video_caps_checked) + if (!acpi_video_caps_checked) { + acpi_video_parse_cmdline(); acpi_video_get_capabilities(NULL); + } }
/* Promote the vendor interface instead of the generic video module. @@ -212,23 +226,3 @@ int acpi_video_backlight_support(void) return acpi_video_support & ACPI_VIDEO_BACKLIGHT; } EXPORT_SYMBOL(acpi_video_backlight_support); - -/* - * Use acpi_backlight=vendor/video to force that backlight switching - * is processed by vendor specific acpi drivers or video.ko driver. - */ -static int __init acpi_backlight(char *str) -{ - if (str == NULL || *str == '\0') - return 1; - else { - if (!strcmp("vendor", str)) - acpi_video_support |= - ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR; - if (!strcmp("video", str)) - acpi_video_support |= - ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO; - } - return 1; -} -__setup("acpi_backlight=", acpi_backlight); diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index f9f205c..909133c 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -71,9 +71,10 @@ config ASUS_LAPTOP depends on ACPI select LEDS_CLASS select NEW_LEDS - select BACKLIGHT_CLASS_DEVICE + depends on BACKLIGHT_CLASS_DEVICE depends on INPUT depends on RFKILL || RFKILL = n + depends on ACPI_VIDEO || ACPI_VIDEO = n select INPUT_SPARSEKMAP select INPUT_POLLDEV ---help--- @@ -95,6 +96,7 @@ config DELL_LAPTOP depends on X86 depends on DCDBAS depends on BACKLIGHT_CLASS_DEVICE + depends on ACPI_VIDEO || ACPI_VIDEO = n depends on RFKILL || RFKILL = n depends on SERIO_I8042 select POWER_SUPPLY @@ -109,6 +111,7 @@ config DELL_WMI tristate "Dell WMI extras" depends on ACPI_WMI depends on INPUT + depends on ACPI_VIDEO || ACPI_VIDEO = n select INPUT_SPARSEKMAP ---help--- Say Y here if you want to support WMI-based hotkeys on Dell laptops. @@ -144,6 +147,7 @@ config FUJITSU_LAPTOP depends on ACPI depends on INPUT depends on BACKLIGHT_CLASS_DEVICE + depends on ACPI_VIDEO || ACPI_VIDEO = n depends on LEDS_CLASS || LEDS_CLASS=n ---help--- This is a driver for laptops built by Fujitsu: @@ -247,6 +251,7 @@ config MSI_LAPTOP tristate "MSI Laptop Extras" depends on ACPI depends on BACKLIGHT_CLASS_DEVICE + depends on ACPI_VIDEO || ACPI_VIDEO = n depends on RFKILL depends on INPUT && SERIO_I8042 select INPUT_SPARSEKMAP @@ -280,6 +285,7 @@ config COMPAL_LAPTOP tristate "Compal (and others) Laptop Extras" depends on ACPI depends on BACKLIGHT_CLASS_DEVICE + depends on ACPI_VIDEO || ACPI_VIDEO = n depends on RFKILL depends on HWMON depends on POWER_SUPPLY @@ -296,7 +302,8 @@ config COMPAL_LAPTOP config SONY_LAPTOP tristate "Sony Laptop Extras" depends on ACPI - select BACKLIGHT_CLASS_DEVICE + depends on ACPI_VIDEO || ACPI_VIDEO = n + depends on BACKLIGHT_CLASS_DEVICE depends on INPUT depends on RFKILL ---help--- @@ -321,6 +328,7 @@ config IDEAPAD_LAPTOP depends on RFKILL && INPUT depends on SERIO_I8042 depends on BACKLIGHT_CLASS_DEVICE + depends on ACPI_VIDEO || ACPI_VIDEO = n select INPUT_SPARSEKMAP help This is a driver for Lenovo IdeaPad netbooks contains drivers for @@ -331,8 +339,8 @@ config THINKPAD_ACPI depends on ACPI depends on INPUT depends on RFKILL || RFKILL = n - select BACKLIGHT_LCD_SUPPORT - select BACKLIGHT_CLASS_DEVICE + depends on ACPI_VIDEO || ACPI_VIDEO = n + depends on BACKLIGHT_CLASS_DEVICE select HWMON select NVRAM select NEW_LEDS @@ -500,8 +508,9 @@ config EEEPC_LAPTOP depends on ACPI depends on INPUT depends on RFKILL || RFKILL = n + depends on ACPI_VIDEO || ACPI_VIDEO = n depends on HOTPLUG_PCI - select BACKLIGHT_CLASS_DEVICE + depends on BACKLIGHT_CLASS_DEVICE select HWMON select LEDS_CLASS select NEW_LEDS @@ -587,6 +596,7 @@ config MSI_WMI depends on ACPI_WMI depends on INPUT depends on BACKLIGHT_CLASS_DEVICE + depends on ACPI_VIDEO || ACPI_VIDEO = n select INPUT_SPARSEKMAP help Say Y here if you want to support WMI-based hotkeys on MSI laptops. @@ -824,6 +834,7 @@ config MXM_WMI config INTEL_OAKTRAIL tristate "Intel Oaktrail Platform Extras" depends on ACPI + depends on ACPI_VIDEO || ACPI_VIDEO = n depends on RFKILL && BACKLIGHT_CLASS_DEVICE && ACPI ---help--- Intel Oaktrail platform need this driver to provide interfaces to
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video_detect.c | 172 +++++++++++++++++++++----------------------- include/acpi/video.h | 17 +++++ 2 files changed, 100 insertions(+), 89 deletions(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 0bc8b98..01c8c46 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -1,4 +1,6 @@ /* + * Copyright (C) 2015 Red Hat Inc. + * Hans de Goede hdegoede@redhat.com * Copyright (C) 2008 SuSE Linux Products GmbH * Thomas Renninger trenn@suse.de * @@ -9,28 +11,24 @@ * acpi_get_pci_dev() can be called to identify ACPI graphics * devices for which a real graphics card is plugged in * - * Now acpi_video_get_capabilities() can be called to check which - * capabilities the graphics cards plugged in support. The check for general - * video capabilities will be triggered by the first caller of - * acpi_video_get_capabilities(NULL); which will happen when the first - * backlight switching supporting driver calls: - * acpi_video_backlight_support(); - * * Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix B) * are available, video.ko should be used to handle the device. * * Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop, * sony_acpi,... can take care about backlight brightness. * - * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m) - * this file will not be compiled, acpi_video_get_capabilities() and - * acpi_video_backlight_support() will always return 0 and vendor specific - * drivers always can handle backlight. + * Backlight drivers can use acpi_video_get_backlight_type() to determine + * which driver should handle the backlight. * + * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m) + * this file will not be compiled and acpi_video_get_backlight_type() will + * always return acpi_backlight_vendor. */
+#include <acpi/video.h> #include <linux/export.h> #include <linux/acpi.h> +#include <linux/backlight.h> #include <linux/dmi.h> #include <linux/module.h> #include <linux/pci.h> @@ -38,20 +36,24 @@ ACPI_MODULE_NAME("video"); #define _COMPONENT ACPI_VIDEO_COMPONENT
-static long acpi_video_support; -static bool acpi_video_caps_checked; +static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef; +static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
static char acpi_backlight_str[16]; module_param_string(backlight, acpi_backlight_str, 16, 0444); MODULE_PARM_DESC(backlight, - "Select which backlight interface to use [vendor|video]"); + "Select which backlight interface to use [vendor|video|native]");
static void acpi_video_parse_cmdline(void) { if (!strcmp("vendor", acpi_backlight_str)) - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR; + acpi_backlight_cmdline = acpi_backlight_vendor; if (!strcmp("video", acpi_backlight_str)) - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO; + acpi_backlight_cmdline = acpi_backlight_video; + if (!strcmp("native", acpi_backlight_str)) + acpi_backlight_cmdline = acpi_backlight_native; + if (!strcmp("none", acpi_backlight_str)) + acpi_backlight_cmdline = acpi_backlight_none; }
static acpi_status @@ -82,7 +84,7 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv) * buggy */ static int video_detect_force_vendor(const struct dmi_system_id *d) { - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR; + acpi_backlight_dmi = acpi_backlight_vendor; return 0; }
@@ -130,99 +132,91 @@ static struct dmi_system_id video_detect_dmi_table[] = { };
/* - * Returns the video capabilities of a specific ACPI graphics device + * Determine which type of backlight interface to use on this system, + * First check cmdline, then dmi quirks, then do autodetect. + * + * The autodetect order is: + * 1) Is the acpi-video backlight interface supported -> + * no, use a vendor interface + * 2) Is this a win8 "ready" BIOS and do we have a native interface -> + * yes, use a native interface + * 3) Else use the acpi-video interface * - * if NULL is passed as argument all ACPI devices are enumerated and - * all graphics capabilities of physically present devices are - * summarized and returned. This is cached and done only once. + * Arguably the native on win8 check should be done first, but that would + * be a behavior change, which may causes issues. */ -static long acpi_video_get_capabilities(acpi_handle graphics_handle) +enum acpi_backlight_type acpi_video_get_backlight_type(void) { - long caps = 0; - struct acpi_device *tmp_dev; - acpi_status status; - - if (acpi_video_caps_checked && graphics_handle == NULL) - return acpi_video_support; - - if (!graphics_handle) { - /* Only do the global walk through all graphics devices once */ - acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, - ACPI_UINT32_MAX, find_video, NULL, - &caps, NULL); - /* There might be boot param flags set already... */ - acpi_video_support |= caps; - acpi_video_caps_checked = 1; - /* Add blacklists here. Be careful to use the right *DMI* bits - * to still be able to override logic via boot params, e.g.: - * - * if (dmi_name_in_vendors("XY")) { - * acpi_video_support |= - * ACPI_VIDEO_BACKLIGHT_DMI_VENDOR; - *} - */ + static DEFINE_MUTEX(init_mutex); + static bool init_done; + static long video_caps;
+ /* Parse cmdline, dmi and acpi only once */ + mutex_lock(&init_mutex); + if (!init_done) { + acpi_video_parse_cmdline(); dmi_check_system(video_detect_dmi_table); - } else { - status = acpi_bus_get_device(graphics_handle, &tmp_dev); - if (ACPI_FAILURE(status)) { - ACPI_EXCEPTION((AE_INFO, status, "Invalid device")); - return 0; - } - acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_handle, + acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, find_video, NULL, - &caps, NULL); + &video_caps, NULL); + init_done = true; } - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support %s %s\n", - graphics_handle ? caps : acpi_video_support, - graphics_handle ? "on device " : "in general", - graphics_handle ? acpi_device_bid(tmp_dev) : "")); - return caps; + mutex_unlock(&init_mutex); + + if (acpi_backlight_cmdline != acpi_backlight_undef) + return acpi_backlight_cmdline; + + if (acpi_backlight_dmi != acpi_backlight_undef) + return acpi_backlight_dmi; + + if (!(video_caps & ACPI_VIDEO_BACKLIGHT)) + return acpi_backlight_vendor; + + if (acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW)) + return acpi_backlight_native; + + return acpi_backlight_video; } +EXPORT_SYMBOL(acpi_video_get_backlight_type);
-static void acpi_video_caps_check(void) +/* + * Set the preferred backlight interface type based on DMI info. + * This function allows DMI blacklists to be implemented by external + * platform drivers instead of putting a big blacklist in video_detect.c + */ +void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) { - /* - * We must check whether the ACPI graphics device is physically plugged - * in. Therefore this must be called after binding PCI and ACPI devices - */ - if (!acpi_video_caps_checked) { - acpi_video_parse_cmdline(); - acpi_video_get_capabilities(NULL); - } + acpi_backlight_dmi = type; } +EXPORT_SYMBOL(acpi_video_set_dmi_backlight_type);
-/* Promote the vendor interface instead of the generic video module. - * This function allow DMI blacklists to be implemented by externals - * platform drivers instead of putting a big blacklist in video_detect.c +/* + * Compatiblity function, this is going away as soon as all drivers are + * converted to acpi_video_set_dmi_backlight_type(). + * + * Promote the vendor interface instead of the generic video module. * After calling this function you will probably want to call * acpi_video_unregister() to make sure the video module is not loaded */ void acpi_video_dmi_promote_vendor(void) { - acpi_video_caps_check(); - acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR; + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); } EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
-/* Returns true if video.ko can do backlight switching */ +/* + * Compatiblity function, this is going away as soon as all drivers are + * converted to acpi_video_get_backlight_type(). + * + * Returns true if video.ko can do backlight switching. + */ int acpi_video_backlight_support(void) { - acpi_video_caps_check(); - - /* First check for boot param -> highest prio */ - if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR) - return 0; - else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO) - return 1; - - /* Then check for DMI blacklist -> second highest prio */ - if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VENDOR) - return 0; - else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VIDEO) - return 1; - - /* Then go the default way */ - return acpi_video_support & ACPI_VIDEO_BACKLIGHT; + /* + * This is done this way since vendor drivers call this to see + * if they should load, and we do not want them to load for both + * the acpi_backlight_video and acpi_backlight_native cases. + */ + return acpi_video_get_backlight_type() != acpi_backlight_vendor; } EXPORT_SYMBOL(acpi_video_backlight_support); diff --git a/include/acpi/video.h b/include/acpi/video.h index 843ef1a..01b5cc7 100644 --- a/include/acpi/video.h +++ b/include/acpi/video.h @@ -16,6 +16,14 @@ struct acpi_device; #define ACPI_VIDEO_DISPLAY_LEGACY_PANEL 0x0110 #define ACPI_VIDEO_DISPLAY_LEGACY_TV 0x0200
+enum acpi_backlight_type { + acpi_backlight_undef = -1, + acpi_backlight_none = 0, + acpi_backlight_video, + acpi_backlight_vendor, + acpi_backlight_native, +}; + #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE) extern int acpi_video_register(void); extern void acpi_video_unregister(void); @@ -23,6 +31,8 @@ extern void acpi_video_unregister_backlight(void); extern int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, void **edid); extern bool acpi_video_verify_backlight_support(void); +extern enum acpi_backlight_type acpi_video_get_backlight_type(void); +extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type); #else static inline int acpi_video_register(void) { return 0; } static inline void acpi_video_unregister(void) { return; } @@ -33,6 +43,13 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type, return -ENODEV; } static inline bool acpi_video_verify_backlight_support(void) { return false; } +static inline enum acpi_backlight_type acpi_video_get_backlight_type(void) +{ + return acpi_backlight_vendor; +} +static void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) +{ +} #endif
#endif
On Wed, Jun 10, 2015 at 03:01:07PM +0200, Hans de Goede wrote:
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
I like backlight=[firmware|platform|raw|none] better, but that would require to put the selection/quirk logic to backlight core. What do you think?
Regards, Aaron
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/acpi/video_detect.c | 172 +++++++++++++++++++++----------------------- include/acpi/video.h | 17 +++++ 2 files changed, 100 insertions(+), 89 deletions(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 0bc8b98..01c8c46 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -1,4 +1,6 @@ /*
- Copyright (C) 2015 Red Hat Inc.
Hans de Goede <hdegoede@redhat.com>- Copyright (C) 2008 SuSE Linux Products GmbH
Thomas Renninger <trenn@suse.de>@@ -9,28 +11,24 @@
- acpi_get_pci_dev() can be called to identify ACPI graphics
- devices for which a real graphics card is plugged in
- Now acpi_video_get_capabilities() can be called to check which
- capabilities the graphics cards plugged in support. The check for general
- video capabilities will be triggered by the first caller of
- acpi_video_get_capabilities(NULL); which will happen when the first
- backlight switching supporting driver calls:
- acpi_video_backlight_support();
- Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix B)
- are available, video.ko should be used to handle the device.
- Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop,
- sony_acpi,... can take care about backlight brightness.
- If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
- this file will not be compiled, acpi_video_get_capabilities() and
- acpi_video_backlight_support() will always return 0 and vendor specific
- drivers always can handle backlight.
- Backlight drivers can use acpi_video_get_backlight_type() to determine
- which driver should handle the backlight.
- If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
- this file will not be compiled and acpi_video_get_backlight_type() will
*/
- always return acpi_backlight_vendor.
+#include <acpi/video.h> #include <linux/export.h> #include <linux/acpi.h> +#include <linux/backlight.h> #include <linux/dmi.h> #include <linux/module.h> #include <linux/pci.h> @@ -38,20 +36,24 @@ ACPI_MODULE_NAME("video"); #define _COMPONENT ACPI_VIDEO_COMPONENT
-static long acpi_video_support; -static bool acpi_video_caps_checked; +static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef; +static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
static char acpi_backlight_str[16]; module_param_string(backlight, acpi_backlight_str, 16, 0444); MODULE_PARM_DESC(backlight,
- "Select which backlight interface to use [vendor|video]");
- "Select which backlight interface to use [vendor|video|native]");
static void acpi_video_parse_cmdline(void) { if (!strcmp("vendor", acpi_backlight_str))
acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
acpi_backlight_cmdline = acpi_backlight_vendor;
acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO;
acpi_backlight_cmdline = acpi_backlight_video;- if (!strcmp("native", acpi_backlight_str))
acpi_backlight_cmdline = acpi_backlight_native;- if (!strcmp("none", acpi_backlight_str))
acpi_backlight_cmdline = acpi_backlight_none;}
static acpi_status @@ -82,7 +84,7 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
- buggy */
static int video_detect_force_vendor(const struct dmi_system_id *d) {
- acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
- acpi_backlight_dmi = acpi_backlight_vendor; return 0;
}
@@ -130,99 +132,91 @@ static struct dmi_system_id video_detect_dmi_table[] = { };
/*
- Returns the video capabilities of a specific ACPI graphics device
- Determine which type of backlight interface to use on this system,
- First check cmdline, then dmi quirks, then do autodetect.
- The autodetect order is:
- Is the acpi-video backlight interface supported ->
- no, use a vendor interface
- Is this a win8 "ready" BIOS and do we have a native interface ->
- yes, use a native interface
- Else use the acpi-video interface
- if NULL is passed as argument all ACPI devices are enumerated and
- all graphics capabilities of physically present devices are
- summarized and returned. This is cached and done only once.
- Arguably the native on win8 check should be done first, but that would
*/
- be a behavior change, which may causes issues.
-static long acpi_video_get_capabilities(acpi_handle graphics_handle) +enum acpi_backlight_type acpi_video_get_backlight_type(void) {
- long caps = 0;
- struct acpi_device *tmp_dev;
- acpi_status status;
- if (acpi_video_caps_checked && graphics_handle == NULL)
return acpi_video_support;- if (!graphics_handle) {
/* Only do the global walk through all graphics devices once */acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,ACPI_UINT32_MAX, find_video, NULL,&caps, NULL);/* There might be boot param flags set already... */acpi_video_support |= caps;acpi_video_caps_checked = 1;/* Add blacklists here. Be careful to use the right *DMI* bits* to still be able to override logic via boot params, e.g.:** if (dmi_name_in_vendors("XY")) {* acpi_video_support |=* ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;*}*/
static DEFINE_MUTEX(init_mutex);
static bool init_done;
static long video_caps;
/* Parse cmdline, dmi and acpi only once */
mutex_lock(&init_mutex);
if (!init_done) {
acpi_video_parse_cmdline();dmi_check_system(video_detect_dmi_table);
- } else {
status = acpi_bus_get_device(graphics_handle, &tmp_dev);if (ACPI_FAILURE(status)) {ACPI_EXCEPTION((AE_INFO, status, "Invalid device"));return 0;}acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_handle,
acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, find_video, NULL,
&caps, NULL);
&video_caps, NULL);init_done = true;
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support %s %s\n",
graphics_handle ? caps : acpi_video_support,graphics_handle ? "on device " : "in general",graphics_handle ? acpi_device_bid(tmp_dev) : ""));- return caps;
- mutex_unlock(&init_mutex);
- if (acpi_backlight_cmdline != acpi_backlight_undef)
return acpi_backlight_cmdline;- if (acpi_backlight_dmi != acpi_backlight_undef)
return acpi_backlight_dmi;- if (!(video_caps & ACPI_VIDEO_BACKLIGHT))
return acpi_backlight_vendor;- if (acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
return acpi_backlight_native;- return acpi_backlight_video;
} +EXPORT_SYMBOL(acpi_video_get_backlight_type);
-static void acpi_video_caps_check(void) +/*
- Set the preferred backlight interface type based on DMI info.
- This function allows DMI blacklists to be implemented by external
- platform drivers instead of putting a big blacklist in video_detect.c
- */
+void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) {
- /*
* We must check whether the ACPI graphics device is physically plugged* in. Therefore this must be called after binding PCI and ACPI devices*/- if (!acpi_video_caps_checked) {
acpi_video_parse_cmdline();acpi_video_get_capabilities(NULL);- }
- acpi_backlight_dmi = type;
} +EXPORT_SYMBOL(acpi_video_set_dmi_backlight_type);
-/* Promote the vendor interface instead of the generic video module.
- This function allow DMI blacklists to be implemented by externals
- platform drivers instead of putting a big blacklist in video_detect.c
+/*
- Compatiblity function, this is going away as soon as all drivers are
- converted to acpi_video_set_dmi_backlight_type().
*/
- Promote the vendor interface instead of the generic video module.
- After calling this function you will probably want to call
- acpi_video_unregister() to make sure the video module is not loaded
void acpi_video_dmi_promote_vendor(void) {
- acpi_video_caps_check();
- acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
- acpi_video_set_dmi_backlight_type(acpi_backlight_vendor);
} EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
-/* Returns true if video.ko can do backlight switching */ +/*
- Compatiblity function, this is going away as soon as all drivers are
- converted to acpi_video_get_backlight_type().
- Returns true if video.ko can do backlight switching.
- */
int acpi_video_backlight_support(void) {
- acpi_video_caps_check();
- /* First check for boot param -> highest prio */
- if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR)
return 0;- else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO)
return 1;- /* Then check for DMI blacklist -> second highest prio */
- if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VENDOR)
return 0;- else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VIDEO)
return 1;- /* Then go the default way */
- return acpi_video_support & ACPI_VIDEO_BACKLIGHT;
- /*
* This is done this way since vendor drivers call this to see* if they should load, and we do not want them to load for both* the acpi_backlight_video and acpi_backlight_native cases.*/- return acpi_video_get_backlight_type() != acpi_backlight_vendor;
} EXPORT_SYMBOL(acpi_video_backlight_support); diff --git a/include/acpi/video.h b/include/acpi/video.h index 843ef1a..01b5cc7 100644 --- a/include/acpi/video.h +++ b/include/acpi/video.h @@ -16,6 +16,14 @@ struct acpi_device; #define ACPI_VIDEO_DISPLAY_LEGACY_PANEL 0x0110 #define ACPI_VIDEO_DISPLAY_LEGACY_TV 0x0200
+enum acpi_backlight_type {
- acpi_backlight_undef = -1,
- acpi_backlight_none = 0,
- acpi_backlight_video,
- acpi_backlight_vendor,
- acpi_backlight_native,
+};
#if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE) extern int acpi_video_register(void); extern void acpi_video_unregister(void); @@ -23,6 +31,8 @@ extern void acpi_video_unregister_backlight(void); extern int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, void **edid); extern bool acpi_video_verify_backlight_support(void); +extern enum acpi_backlight_type acpi_video_get_backlight_type(void); +extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type); #else static inline int acpi_video_register(void) { return 0; } static inline void acpi_video_unregister(void) { return; } @@ -33,6 +43,13 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type, return -ENODEV; } static inline bool acpi_video_verify_backlight_support(void) { return false; } +static inline enum acpi_backlight_type acpi_video_get_backlight_type(void) +{
- return acpi_backlight_vendor;
+} +static void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) +{ +} #endif
#endif
2.4.2
-- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
On 11-06-15 11:00, Aaron Lu wrote:
On Wed, Jun 10, 2015 at 03:01:07PM +0200, Hans de Goede wrote:
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
I like backlight=[firmware|platform|raw|none] better, but that would require to put the selection/quirk logic to backlight core. What do you think?
I think you are asking 3 separate questions:
1) Should the backlight selection logic maybe be moved to the backlight class code ?
My answer to that one is: no it should not as it is x86/acpi specific.
2) What would be a good kernel cmdline option name for this?
Given the answer to one I think it is good to keep acpi in the name space of the cmdline option. Also note that moving the code to the backlight class code does not magically give us a backlight= cmdline option. None module-name prefixed cmdline options are only available to code which is always builtin, and the backlight class code can be build as a module, so putting the cmdline option in the backlight class will give us: backlight.backlight=
So all in all I think acpi_video.backlight= is actually pretty ok.
3) What would be a good kernel cmdline option values for this?
I agree that the old vendor/video/native value labels are not necessarily the best labels for this, but I would like to keep them as those are what people are used to. Ideally I would like to have just kept acpi_backlight=[video|vendor|native|none] but that is not possible without making all of the acpi_video code always builtin.
Regards,
Hans
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/acpi/video_detect.c | 172 +++++++++++++++++++++----------------------- include/acpi/video.h | 17 +++++ 2 files changed, 100 insertions(+), 89 deletions(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 0bc8b98..01c8c46 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -1,4 +1,6 @@ /*
- Copyright (C) 2015 Red Hat Inc.
Hans de Goede <hdegoede@redhat.com>- Copyright (C) 2008 SuSE Linux Products GmbH
Thomas Renninger <trenn@suse.de>@@ -9,28 +11,24 @@
- acpi_get_pci_dev() can be called to identify ACPI graphics
- devices for which a real graphics card is plugged in
- Now acpi_video_get_capabilities() can be called to check which
- capabilities the graphics cards plugged in support. The check for general
- video capabilities will be triggered by the first caller of
- acpi_video_get_capabilities(NULL); which will happen when the first
- backlight switching supporting driver calls:
- acpi_video_backlight_support();
- Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix B)
- are available, video.ko should be used to handle the device.
- Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop,
- sony_acpi,... can take care about backlight brightness.
- If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
- this file will not be compiled, acpi_video_get_capabilities() and
- acpi_video_backlight_support() will always return 0 and vendor specific
- drivers always can handle backlight.
- Backlight drivers can use acpi_video_get_backlight_type() to determine
- which driver should handle the backlight.
- If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
- this file will not be compiled and acpi_video_get_backlight_type() will
*/
- always return acpi_backlight_vendor.
+#include <acpi/video.h> #include <linux/export.h> #include <linux/acpi.h> +#include <linux/backlight.h> #include <linux/dmi.h> #include <linux/module.h> #include <linux/pci.h> @@ -38,20 +36,24 @@ ACPI_MODULE_NAME("video"); #define _COMPONENT ACPI_VIDEO_COMPONENT
-static long acpi_video_support; -static bool acpi_video_caps_checked; +static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef; +static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
static char acpi_backlight_str[16]; module_param_string(backlight, acpi_backlight_str, 16, 0444); MODULE_PARM_DESC(backlight,
- "Select which backlight interface to use [vendor|video]");
"Select which backlight interface to use [vendor|video|native]");
static void acpi_video_parse_cmdline(void) { if (!strcmp("vendor", acpi_backlight_str))
acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
acpi_backlight_cmdline = acpi_backlight_vendor;
acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO;
acpi_backlight_cmdline = acpi_backlight_video;if (!strcmp("native", acpi_backlight_str))
acpi_backlight_cmdline = acpi_backlight_native;if (!strcmp("none", acpi_backlight_str))
acpi_backlight_cmdline = acpi_backlight_none;}
static acpi_status
@@ -82,7 +84,7 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
- buggy */
static int video_detect_force_vendor(const struct dmi_system_id *d) {
- acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
- acpi_backlight_dmi = acpi_backlight_vendor; return 0; }
@@ -130,99 +132,91 @@ static struct dmi_system_id video_detect_dmi_table[] = { };
/*
- Returns the video capabilities of a specific ACPI graphics device
- Determine which type of backlight interface to use on this system,
- First check cmdline, then dmi quirks, then do autodetect.
- The autodetect order is:
- Is the acpi-video backlight interface supported ->
- no, use a vendor interface
- Is this a win8 "ready" BIOS and do we have a native interface ->
- yes, use a native interface
- Else use the acpi-video interface
- if NULL is passed as argument all ACPI devices are enumerated and
- all graphics capabilities of physically present devices are
- summarized and returned. This is cached and done only once.
- Arguably the native on win8 check should be done first, but that would
*/
- be a behavior change, which may causes issues.
-static long acpi_video_get_capabilities(acpi_handle graphics_handle) +enum acpi_backlight_type acpi_video_get_backlight_type(void) {
- long caps = 0;
- struct acpi_device *tmp_dev;
- acpi_status status;
- if (acpi_video_caps_checked && graphics_handle == NULL)
return acpi_video_support;- if (!graphics_handle) {
/* Only do the global walk through all graphics devices once */acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,ACPI_UINT32_MAX, find_video, NULL,&caps, NULL);/* There might be boot param flags set already... */acpi_video_support |= caps;acpi_video_caps_checked = 1;/* Add blacklists here. Be careful to use the right *DMI* bits* to still be able to override logic via boot params, e.g.:** if (dmi_name_in_vendors("XY")) {* acpi_video_support |=* ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;*}*/
static DEFINE_MUTEX(init_mutex);
static bool init_done;
static long video_caps;
/* Parse cmdline, dmi and acpi only once */
mutex_lock(&init_mutex);
if (!init_done) {
acpi_video_parse_cmdline();dmi_check_system(video_detect_dmi_table);
- } else {
status = acpi_bus_get_device(graphics_handle, &tmp_dev);if (ACPI_FAILURE(status)) {ACPI_EXCEPTION((AE_INFO, status, "Invalid device"));return 0;}acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_handle,
acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, find_video, NULL,
&caps, NULL);
&video_caps, NULL);init_done = true;
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support %s %s\n",
graphics_handle ? caps : acpi_video_support,graphics_handle ? "on device " : "in general",graphics_handle ? acpi_device_bid(tmp_dev) : ""));- return caps;
- mutex_unlock(&init_mutex);
- if (acpi_backlight_cmdline != acpi_backlight_undef)
return acpi_backlight_cmdline;- if (acpi_backlight_dmi != acpi_backlight_undef)
return acpi_backlight_dmi;- if (!(video_caps & ACPI_VIDEO_BACKLIGHT))
return acpi_backlight_vendor;- if (acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
return acpi_backlight_native;- return acpi_backlight_video; }
+EXPORT_SYMBOL(acpi_video_get_backlight_type);
-static void acpi_video_caps_check(void) +/*
- Set the preferred backlight interface type based on DMI info.
- This function allows DMI blacklists to be implemented by external
- platform drivers instead of putting a big blacklist in video_detect.c
- */
+void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) {
- /*
* We must check whether the ACPI graphics device is physically plugged* in. Therefore this must be called after binding PCI and ACPI devices*/- if (!acpi_video_caps_checked) {
acpi_video_parse_cmdline();acpi_video_get_capabilities(NULL);- }
- acpi_backlight_dmi = type; }
+EXPORT_SYMBOL(acpi_video_set_dmi_backlight_type);
-/* Promote the vendor interface instead of the generic video module.
- This function allow DMI blacklists to be implemented by externals
- platform drivers instead of putting a big blacklist in video_detect.c
+/*
- Compatiblity function, this is going away as soon as all drivers are
- converted to acpi_video_set_dmi_backlight_type().
*/ void acpi_video_dmi_promote_vendor(void) {
- Promote the vendor interface instead of the generic video module.
- After calling this function you will probably want to call
- acpi_video_unregister() to make sure the video module is not loaded
- acpi_video_caps_check();
- acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
- acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); } EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
-/* Returns true if video.ko can do backlight switching */ +/*
- Compatiblity function, this is going away as soon as all drivers are
- converted to acpi_video_get_backlight_type().
- Returns true if video.ko can do backlight switching.
- */ int acpi_video_backlight_support(void) {
- acpi_video_caps_check();
- /* First check for boot param -> highest prio */
- if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR)
return 0;- else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO)
return 1;- /* Then check for DMI blacklist -> second highest prio */
- if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VENDOR)
return 0;- else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VIDEO)
return 1;- /* Then go the default way */
- return acpi_video_support & ACPI_VIDEO_BACKLIGHT;
- /*
* This is done this way since vendor drivers call this to see* if they should load, and we do not want them to load for both* the acpi_backlight_video and acpi_backlight_native cases.*/- return acpi_video_get_backlight_type() != acpi_backlight_vendor; } EXPORT_SYMBOL(acpi_video_backlight_support);
diff --git a/include/acpi/video.h b/include/acpi/video.h index 843ef1a..01b5cc7 100644 --- a/include/acpi/video.h +++ b/include/acpi/video.h @@ -16,6 +16,14 @@ struct acpi_device; #define ACPI_VIDEO_DISPLAY_LEGACY_PANEL 0x0110 #define ACPI_VIDEO_DISPLAY_LEGACY_TV 0x0200
+enum acpi_backlight_type {
- acpi_backlight_undef = -1,
- acpi_backlight_none = 0,
- acpi_backlight_video,
- acpi_backlight_vendor,
- acpi_backlight_native,
+};
- #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE) extern int acpi_video_register(void); extern void acpi_video_unregister(void);
@@ -23,6 +31,8 @@ extern void acpi_video_unregister_backlight(void); extern int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, void **edid); extern bool acpi_video_verify_backlight_support(void); +extern enum acpi_backlight_type acpi_video_get_backlight_type(void); +extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type); #else static inline int acpi_video_register(void) { return 0; } static inline void acpi_video_unregister(void) { return; } @@ -33,6 +43,13 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type, return -ENODEV; } static inline bool acpi_video_verify_backlight_support(void) { return false; } +static inline enum acpi_backlight_type acpi_video_get_backlight_type(void) +{
- return acpi_backlight_vendor;
+} +static void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) +{ +} #endif
#endif
2.4.2
-- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/11/2015 05:19 PM, Hans de Goede wrote:
Hi,
On 11-06-15 11:00, Aaron Lu wrote:
On Wed, Jun 10, 2015 at 03:01:07PM +0200, Hans de Goede wrote:
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
I like backlight=[firmware|platform|raw|none] better, but that would require to put the selection/quirk logic to backlight core. What do you think?
I think you are asking 3 separate questions:
- Should the backlight selection logic maybe be moved to the backlight
class code ?
My answer to that one is: no it should not as it is x86/acpi specific.
- What would be a good kernel cmdline option name for this?
Given the answer to one I think it is good to keep acpi in the name space of the cmdline option. Also note that moving the code to the backlight class code does not magically give us a backlight= cmdline option. None module-name prefixed cmdline options are only available to code which is always builtin, and the backlight class code can be build as a module, so putting the cmdline option in the backlight class will give us: backlight.backlight=
So all in all I think acpi_video.backlight= is actually pretty ok.
- What would be a good kernel cmdline option values for this?
I agree that the old vendor/video/native value labels are not necessarily the best labels for this, but I would like to keep them as those are what people are used to. Ideally I would like to have just kept acpi_backlight=[video|vendor|native|none] but that is not possible without making all of the acpi_video code always builtin.
OK, I don't have other comments, thanks for doing this.
Regards, Aaron
Regards,
Hans
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/acpi/video_detect.c | 172 +++++++++++++++++++++----------------------- include/acpi/video.h | 17 +++++ 2 files changed, 100 insertions(+), 89 deletions(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 0bc8b98..01c8c46 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -1,4 +1,6 @@ /*
- Copyright (C) 2015 Red Hat Inc.
Hans de Goede <hdegoede@redhat.com>- Copyright (C) 2008 SuSE Linux Products GmbH
Thomas Renninger <trenn@suse.de>@@ -9,28 +11,24 @@
- acpi_get_pci_dev() can be called to identify ACPI graphics
- devices for which a real graphics card is plugged in
- Now acpi_video_get_capabilities() can be called to check which
- capabilities the graphics cards plugged in support. The check for general
- video capabilities will be triggered by the first caller of
- acpi_video_get_capabilities(NULL); which will happen when the first
- backlight switching supporting driver calls:
- acpi_video_backlight_support();
- Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix B)
- are available, video.ko should be used to handle the device.
- Otherwise vendor specific drivers like thinkpad_acpi, asus-laptop,
- sony_acpi,... can take care about backlight brightness.
- If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
- this file will not be compiled, acpi_video_get_capabilities() and
- acpi_video_backlight_support() will always return 0 and vendor specific
- drivers always can handle backlight.
- Backlight drivers can use acpi_video_get_backlight_type() to determine
- which driver should handle the backlight.
- If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
- this file will not be compiled and acpi_video_get_backlight_type() will
*/
- always return acpi_backlight_vendor.
+#include <acpi/video.h> #include <linux/export.h> #include <linux/acpi.h> +#include <linux/backlight.h> #include <linux/dmi.h> #include <linux/module.h> #include <linux/pci.h> @@ -38,20 +36,24 @@ ACPI_MODULE_NAME("video"); #define _COMPONENT ACPI_VIDEO_COMPONENT
-static long acpi_video_support; -static bool acpi_video_caps_checked; +static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef; +static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
static char acpi_backlight_str[16]; module_param_string(backlight, acpi_backlight_str, 16, 0444); MODULE_PARM_DESC(backlight,
- "Select which backlight interface to use [vendor|video]");
"Select which backlight interface to use [vendor|video|native]");
static void acpi_video_parse_cmdline(void) { if (!strcmp("vendor", acpi_backlight_str))
acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
acpi_backlight_cmdline = acpi_backlight_vendor;
acpi_video_support |= ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO;
acpi_backlight_cmdline = acpi_backlight_video;if (!strcmp("native", acpi_backlight_str))
acpi_backlight_cmdline = acpi_backlight_native;if (!strcmp("none", acpi_backlight_str))
acpi_backlight_cmdline = acpi_backlight_none;}
static acpi_status
@@ -82,7 +84,7 @@ find_video(acpi_handle handle, u32 lvl, void *context, void **rv)
- buggy */
static int video_detect_force_vendor(const struct dmi_system_id *d) {
- acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
- acpi_backlight_dmi = acpi_backlight_vendor; return 0; }
@@ -130,99 +132,91 @@ static struct dmi_system_id video_detect_dmi_table[] = { };
/*
- Returns the video capabilities of a specific ACPI graphics device
- Determine which type of backlight interface to use on this system,
- First check cmdline, then dmi quirks, then do autodetect.
- The autodetect order is:
- Is the acpi-video backlight interface supported ->
- no, use a vendor interface
- Is this a win8 "ready" BIOS and do we have a native interface ->
- yes, use a native interface
- Else use the acpi-video interface
- if NULL is passed as argument all ACPI devices are enumerated and
- all graphics capabilities of physically present devices are
- summarized and returned. This is cached and done only once.
- Arguably the native on win8 check should be done first, but that would
*/
- be a behavior change, which may causes issues.
-static long acpi_video_get_capabilities(acpi_handle graphics_handle) +enum acpi_backlight_type acpi_video_get_backlight_type(void) {
- long caps = 0;
- struct acpi_device *tmp_dev;
- acpi_status status;
- if (acpi_video_caps_checked && graphics_handle == NULL)
return acpi_video_support;- if (!graphics_handle) {
/* Only do the global walk through all graphics devices once */acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,ACPI_UINT32_MAX, find_video, NULL,&caps, NULL);/* There might be boot param flags set already... */acpi_video_support |= caps;acpi_video_caps_checked = 1;/* Add blacklists here. Be careful to use the right *DMI* bits* to still be able to override logic via boot params, e.g.:** if (dmi_name_in_vendors("XY")) {* acpi_video_support |=* ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;*}*/
static DEFINE_MUTEX(init_mutex);
static bool init_done;
static long video_caps;
/* Parse cmdline, dmi and acpi only once */
mutex_lock(&init_mutex);
if (!init_done) {
acpi_video_parse_cmdline();dmi_check_system(video_detect_dmi_table);
- } else {
status = acpi_bus_get_device(graphics_handle, &tmp_dev);if (ACPI_FAILURE(status)) {ACPI_EXCEPTION((AE_INFO, status, "Invalid device"));return 0;}acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_handle,
acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, find_video, NULL,
&caps, NULL);
&video_caps, NULL);init_done = true;
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support %s %s\n",
graphics_handle ? caps : acpi_video_support,graphics_handle ? "on device " : "in general",graphics_handle ? acpi_device_bid(tmp_dev) : ""));- return caps;
- mutex_unlock(&init_mutex);
- if (acpi_backlight_cmdline != acpi_backlight_undef)
return acpi_backlight_cmdline;- if (acpi_backlight_dmi != acpi_backlight_undef)
return acpi_backlight_dmi;- if (!(video_caps & ACPI_VIDEO_BACKLIGHT))
return acpi_backlight_vendor;- if (acpi_osi_is_win8() && backlight_device_registered(BACKLIGHT_RAW))
return acpi_backlight_native;- return acpi_backlight_video; }
+EXPORT_SYMBOL(acpi_video_get_backlight_type);
-static void acpi_video_caps_check(void) +/*
- Set the preferred backlight interface type based on DMI info.
- This function allows DMI blacklists to be implemented by external
- platform drivers instead of putting a big blacklist in video_detect.c
- */
+void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) {
- /*
* We must check whether the ACPI graphics device is physically plugged* in. Therefore this must be called after binding PCI and ACPI devices*/- if (!acpi_video_caps_checked) {
acpi_video_parse_cmdline();acpi_video_get_capabilities(NULL);- }
- acpi_backlight_dmi = type; }
+EXPORT_SYMBOL(acpi_video_set_dmi_backlight_type);
-/* Promote the vendor interface instead of the generic video module.
- This function allow DMI blacklists to be implemented by externals
- platform drivers instead of putting a big blacklist in video_detect.c
+/*
- Compatiblity function, this is going away as soon as all drivers are
- converted to acpi_video_set_dmi_backlight_type().
*/ void acpi_video_dmi_promote_vendor(void) {
- Promote the vendor interface instead of the generic video module.
- After calling this function you will probably want to call
- acpi_video_unregister() to make sure the video module is not loaded
- acpi_video_caps_check();
- acpi_video_support |= ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
- acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); } EXPORT_SYMBOL(acpi_video_dmi_promote_vendor);
-/* Returns true if video.ko can do backlight switching */ +/*
- Compatiblity function, this is going away as soon as all drivers are
- converted to acpi_video_get_backlight_type().
- Returns true if video.ko can do backlight switching.
- */ int acpi_video_backlight_support(void) {
- acpi_video_caps_check();
- /* First check for boot param -> highest prio */
- if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR)
return 0;- else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO)
return 1;- /* Then check for DMI blacklist -> second highest prio */
- if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VENDOR)
return 0;- else if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_DMI_VIDEO)
return 1;- /* Then go the default way */
- return acpi_video_support & ACPI_VIDEO_BACKLIGHT;
- /*
* This is done this way since vendor drivers call this to see* if they should load, and we do not want them to load for both* the acpi_backlight_video and acpi_backlight_native cases.*/- return acpi_video_get_backlight_type() != acpi_backlight_vendor; } EXPORT_SYMBOL(acpi_video_backlight_support);
diff --git a/include/acpi/video.h b/include/acpi/video.h index 843ef1a..01b5cc7 100644 --- a/include/acpi/video.h +++ b/include/acpi/video.h @@ -16,6 +16,14 @@ struct acpi_device; #define ACPI_VIDEO_DISPLAY_LEGACY_PANEL 0x0110 #define ACPI_VIDEO_DISPLAY_LEGACY_TV 0x0200
+enum acpi_backlight_type {
- acpi_backlight_undef = -1,
- acpi_backlight_none = 0,
- acpi_backlight_video,
- acpi_backlight_vendor,
- acpi_backlight_native,
+};
- #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE) extern int acpi_video_register(void); extern void acpi_video_unregister(void);
@@ -23,6 +31,8 @@ extern void acpi_video_unregister_backlight(void); extern int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, void **edid); extern bool acpi_video_verify_backlight_support(void); +extern enum acpi_backlight_type acpi_video_get_backlight_type(void); +extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type); #else static inline int acpi_video_register(void) { return 0; } static inline void acpi_video_unregister(void) { return; } @@ -33,6 +43,13 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type, return -ENODEV; } static inline bool acpi_video_verify_backlight_support(void) { return false; } +static inline enum acpi_backlight_type acpi_video_get_backlight_type(void) +{
- return acpi_backlight_vendor;
+} +static void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) +{ +} #endif
#endif
2.4.2
-- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 10 June 2015 15:01:07 Hans de Goede wrote:
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Hello,
lot of people are using acpi_backlight parameter in kernel cmdline to fix some of problems. I would like to see this parameter still working and to not break existing configuration. E.g acpi_backlight=vendor to use dell-laptop.ko or thinkpad_acpi.ko backlight.
It is still nightmare to get laptop panel backlight working on different (broken) laptops and people learnt to try to use acpi_backlight parameter for quit/hot fixing these problems. Upgrading kernel (if you remove acpi_backlight parameter) just break it again.
Hi,
On 11-06-15 11:19, Pali Rohár wrote:
On Wednesday 10 June 2015 15:01:07 Hans de Goede wrote:
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Hello,
lot of people are using acpi_backlight parameter in kernel cmdline to fix some of problems. I would like to see this parameter still working and to not break existing configuration. E.g acpi_backlight=vendor to use dell-laptop.ko or thinkpad_acpi.ko backlight.
I would have like to keep acpi_backlight= for that exact same reason, but that is not possible while keeping acpi_video as a module.
Thinking more about this, this is not strictly true, we could make some other (core) part of the acpi code use __setup to get the acpi_backlight= value and have that part export the value for use in the acpi_video module. This is not really pretty, but I think it may be the best way to solve this.
It is still nightmare to get laptop panel backlight working on different (broken) laptops
Well one reason it is a nightmare is because there are too many backlight drivers fighting for control and there is not an easy way to tell the kernel only register this one, this is exactly what this patch-set is trying to address :) People may still need to use a cmdline option but now there is only one option to play with.
and people learnt to try to use acpi_backlight parameter for quit/hot fixing these problems.
People who need to pass a kernel commandline option really should report a bug once they have figured out what option to use.
Fedora users are getting pretty good at this as the Fedora kernel maintainers punt all such bug reports to me and I properly deal with them verifying the users solution is sane and then submitting a patch with a dmi based quirk for the users laptop upstream.
Upgrading kernel (if you remove acpi_backlight parameter) just break it again.
I think that is actually (partially) a good thing, as said people who rely on cmdline workarounds should file bugs, so that we can add a quirk. Had the users done so, the quirk would be long in place and the changing of the cmdline option name would not be an issue for them.
I realize that this knife cuts both ways, and that this will cause unhappy users, as said if it had been possible to not change the cmdline option name in a clean way I would have done so.
If everyone agrees with the solution I've outlined above, we can actually make it so as to not break things for users who's setup relies on acpi_backlight=foo
Regards,
Hans
On Thursday 11 June 2015 11:29:24 Hans de Goede wrote:
Hi,
On 11-06-15 11:19, Pali Rohár wrote:
On Wednesday 10 June 2015 15:01:07 Hans de Goede wrote:
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Hello,
lot of people are using acpi_backlight parameter in kernel cmdline to fix some of problems. I would like to see this parameter still working and to not break existing configuration. E.g acpi_backlight=vendor to use dell-laptop.ko or thinkpad_acpi.ko backlight.
I would have like to keep acpi_backlight= for that exact same reason, but that is not possible while keeping acpi_video as a module.
Thinking more about this, this is not strictly true, we could make some other (core) part of the acpi code use __setup to get the acpi_backlight= value and have that part export the value for use in the acpi_video module. This is not really pretty, but I think it may be the best way to solve this.
This solution should work. It is not nice, but current mess of backlight drivers, acpi, etc, is not nice too. It will help users who already know (or read on internet/forums/discussions) acpi_backlight= parameter...
It is still nightmare to get laptop panel backlight working on different (broken) laptops
Well one reason it is a nightmare is because there are too many backlight drivers fighting for control and there is not an easy way to tell the kernel only register this one, this is exactly what this patch-set is trying to address :) People may still need to use a cmdline option but now there is only one option to play with.
It is not only that. Sometimes there are three possible ways how to control backlight (vendor driver, acpi driver, gpu driver). And all those driver can work, but some of them works "better".
E.g in my case on Dell Latitude E6440 all three drivers can register backlight interface and export it to userspace via sysfs.
But the best option for *me* is to use one from dell-laptop.ko because it has good granularity of levels for controlling backlight. Intel one has too many levels and does not fit some linear distribution (which I would like to see). ACPI video.ko sometimes does not work correctly with KDE4 setting -- sometimes I need to press brightness key more times to take effect. I do not know if this is problem of KDE or ACPI or whatever. For me for my current desktop & kernel setup is the best solution to use dell-laptop.ko with acpi_backlight=vendor param. I do not have to debug or change anything, just add one param to cmdline.
And I think other users (without programming knowledge) who need immediately working laptop will use same option as me.
and people learnt to try to use acpi_backlight parameter for quit/hot fixing these problems.
People who need to pass a kernel commandline option really should report a bug once they have figured out what option to use.
This is good assumption, but do not forget that there are people who:
1) do not have time to trying find out where to report bug and how 2) do not know how to report bug 3) even do not know that they can do something (like for MS systems)
Fedora users are getting pretty good at this as the Fedora kernel maintainers punt all such bug reports to me and I properly deal with them verifying the users solution is sane and then submitting a patch with a dmi based quirk for the users laptop upstream.
Upgrading kernel (if you remove acpi_backlight parameter) just break it again.
I think that is actually (partially) a good thing, as said people who rely on cmdline workarounds should file bugs, so that we can add a quirk. Had the users done so, the quirk would be long in place and the changing of the cmdline option name would not be an issue for them.
I realize that this knife cuts both ways, and that this will cause unhappy users, as said if it had been possible to not change the cmdline option name in a clean way I would have done so.
If everyone agrees with the solution I've outlined above, we can actually make it so as to not break things for users who's setup relies on acpi_backlight=foo
Regards,
Hans
On Thu, 11 Jun 2015, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 11-06-15 11:19, Pali Rohár wrote:
On Wednesday 10 June 2015 15:01:07 Hans de Goede wrote:
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Hello,
lot of people are using acpi_backlight parameter in kernel cmdline to fix some of problems. I would like to see this parameter still working and to not break existing configuration. E.g acpi_backlight=vendor to use dell-laptop.ko or thinkpad_acpi.ko backlight.
I would have like to keep acpi_backlight= for that exact same reason, but that is not possible while keeping acpi_video as a module.
Thinking more about this, this is not strictly true, we could make some other (core) part of the acpi code use __setup to get the acpi_backlight= value and have that part export the value for use in the acpi_video module. This is not really pretty, but I think it may be the best way to solve this.
It just might be a good idea to try to not change the backlight related parameters all the time... See [1]. I probably forgot a few.
BR, Jani.
[1] http://nikula.org/~jani/backlight/#index11h2
It is still nightmare to get laptop panel backlight working on different (broken) laptops
Well one reason it is a nightmare is because there are too many backlight drivers fighting for control and there is not an easy way to tell the kernel only register this one, this is exactly what this patch-set is trying to address :) People may still need to use a cmdline option but now there is only one option to play with.
and people learnt to try to use acpi_backlight parameter for quit/hot fixing these problems.
People who need to pass a kernel commandline option really should report a bug once they have figured out what option to use.
Fedora users are getting pretty good at this as the Fedora kernel maintainers punt all such bug reports to me and I properly deal with them verifying the users solution is sane and then submitting a patch with a dmi based quirk for the users laptop upstream.
Upgrading kernel (if you remove acpi_backlight parameter) just break it again.
I think that is actually (partially) a good thing, as said people who rely on cmdline workarounds should file bugs, so that we can add a quirk. Had the users done so, the quirk would be long in place and the changing of the cmdline option name would not be an issue for them.
I realize that this knife cuts both ways, and that this will cause unhappy users, as said if it had been possible to not change the cmdline option name in a clean way I would have done so.
If everyone agrees with the solution I've outlined above, we can actually make it so as to not break things for users who's setup relies on acpi_backlight=foo
Regards,
Hans _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi,
On 11-06-15 14:28, Jani Nikula wrote:
On Thu, 11 Jun 2015, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 11-06-15 11:19, Pali Rohár wrote:
On Wednesday 10 June 2015 15:01:07 Hans de Goede wrote:
Currently we have 2 kernel commandline options + dmi-quirks in 3 places all interacting (in interesting ways) to select which which backlight interface to use. On the commandline we've acpi_backlight=[video|vendor] and video.use_native_backlight=[0|1]. DMI quirks we have in acpi/video-detect.c, acpi/video.c and drivers/platform/x86/*.c .
This commit is the first step to cleaning this up, replacing the 2 cmdline options with just acpi_video.backlight=[video|vendor|native|none], and adding a new API to video_detect.c to reflect this.
Follow up commits will also move other related code, like unregistering the acpi_video backlight interface if it was registered before other drivers which take priority over it are loaded, to video_detect.c where this logic really belongs.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Hello,
lot of people are using acpi_backlight parameter in kernel cmdline to fix some of problems. I would like to see this parameter still working and to not break existing configuration. E.g acpi_backlight=vendor to use dell-laptop.ko or thinkpad_acpi.ko backlight.
I would have like to keep acpi_backlight= for that exact same reason, but that is not possible while keeping acpi_video as a module.
Thinking more about this, this is not strictly true, we could make some other (core) part of the acpi code use __setup to get the acpi_backlight= value and have that part export the value for use in the acpi_video module. This is not really pretty, but I think it may be the best way to solve this.
It just might be a good idea to try to not change the backlight related parameters all the time... See [1]. I probably forgot a few.
Right, so as said I'm fine with adding the above work-around to the next version of the patch-set to keep acpi_backlight=[vendor|video] working as before.
Regards,
Hans
Make acpi_video_set_dmi_backlight_type() call acpi_video_unregister_backlight() when the new dmi quirk results in the desired backlight interface being of a type other then acpi_backlight_video.
This avoid the need for the second if in the following construction which is currently found in many platform/x86 drivers:
if (prefer_vendor_quirk) acpi_video_dmi_promote_vendor();
if (!acpi_video_backlight_support()) acpi_video_unregister_backlight()
This second if-block will be removed from the platform drivers as part of their conversion to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video_detect.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 01c8c46..98368cf 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -187,6 +187,9 @@ EXPORT_SYMBOL(acpi_video_get_backlight_type); void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) { acpi_backlight_dmi = type; + /* Remove acpi-video backlight interface if it is no longer desired */ + if (acpi_video_get_backlight_type() != acpi_backlight_video) + acpi_video_unregister_backlight(); } EXPORT_SYMBOL(acpi_video_set_dmi_backlight_type);
This results in a nice cleanup, as we can replace the complicated logic from should_ignore_backlight_request() with a simple check for the type being native.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/gpu/drm/i915/intel_opregion.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index 71e87ab..4813374 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -396,16 +396,6 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state) return -EINVAL; }
-/* - * If the vendor backlight interface is not in use and ACPI backlight interface - * is broken, do not bother processing backlight change requests from firmware. - */ -static bool should_ignore_backlight_request(void) -{ - return acpi_video_backlight_support() && - !acpi_video_verify_backlight_support(); -} - static u32 asle_set_backlight(struct drm_device *dev, u32 bclp) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -414,7 +404,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
- if (should_ignore_backlight_request()) { + if (acpi_video_get_backlight_type() == acpi_backlight_native) { DRM_DEBUG_KMS("opregion backlight request ignored\n"); return 0; }
Most of the patch is moving the dmi quirks for forcing use of the acpi-video / the native backlight interface to video_detect.c.
What remains is a nice cleanup.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video.c | 178 +++----------------------------------------- drivers/acpi/video_detect.c | 130 ++++++++++++++++++++++++++++++++ include/acpi/video.h | 2 - 3 files changed, 141 insertions(+), 169 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 6249968..1bbe9b1 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -78,20 +78,6 @@ module_param(brightness_switch_enabled, bool, 0644); static bool allow_duplicates; module_param(allow_duplicates, bool, 0644);
-/* - * For Windows 8 systems: used to decide if video module - * should skip registering backlight interface of its own. - */ -enum { - NATIVE_BACKLIGHT_NOT_SET = -1, - NATIVE_BACKLIGHT_OFF, - NATIVE_BACKLIGHT_ON, -}; - -static int use_native_backlight_param = NATIVE_BACKLIGHT_NOT_SET; -module_param_named(use_native_backlight, use_native_backlight_param, int, 0444); -static int use_native_backlight_dmi = NATIVE_BACKLIGHT_NOT_SET; - static int register_count; static struct mutex video_list_lock; static struct list_head video_bus_head; @@ -241,24 +227,6 @@ static int acpi_video_get_next_level(struct acpi_video_device *device, u32 level_current, u32 event); static void acpi_video_switch_brightness(struct work_struct *work);
-static bool acpi_video_use_native_backlight(void) -{ - if (use_native_backlight_param != NATIVE_BACKLIGHT_NOT_SET) - return use_native_backlight_param; - else if (use_native_backlight_dmi != NATIVE_BACKLIGHT_NOT_SET) - return use_native_backlight_dmi; - return acpi_osi_is_win8(); -} - -bool acpi_video_verify_backlight_support(void) -{ - if (acpi_video_use_native_backlight() && - backlight_device_registered(BACKLIGHT_RAW)) - return false; - return acpi_video_backlight_support(); -} -EXPORT_SYMBOL_GPL(acpi_video_verify_backlight_support); - /* backlight device sysfs support */ static int acpi_video_get_brightness(struct backlight_device *bd) { @@ -419,18 +387,6 @@ static int __init video_set_bqc_offset(const struct dmi_system_id *d) return 0; }
-static int __init video_disable_native_backlight(const struct dmi_system_id *d) -{ - use_native_backlight_dmi = NATIVE_BACKLIGHT_OFF; - return 0; -} - -static int __init video_enable_native_backlight(const struct dmi_system_id *d) -{ - use_native_backlight_dmi = NATIVE_BACKLIGHT_ON; - return 0; -} - static struct dmi_system_id video_dmi_table[] __initdata = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 @@ -475,123 +431,6 @@ static struct dmi_system_id video_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 7720"), }, }, - - /* - * These models have a working acpi_video backlight control, and using - * native backlight causes a regression where backlight does not work - * when userspace is not handling brightness key events. Disable - * native_backlight on these to fix this: - * https://bugzilla.kernel.org/show_bug.cgi?id=81691 - */ - { - .callback = video_disable_native_backlight, - .ident = "ThinkPad T420", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T420"), - }, - }, - { - .callback = video_disable_native_backlight, - .ident = "ThinkPad T520", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T520"), - }, - }, - { - .callback = video_disable_native_backlight, - .ident = "ThinkPad X201s", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201s"), - }, - }, - - /* The native backlight controls do not work on some older machines */ - { - /* https://bugs.freedesktop.org/show_bug.cgi?id=81515 */ - .callback = video_disable_native_backlight, - .ident = "HP ENVY 15 Notebook", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"), - DMI_MATCH(DMI_PRODUCT_NAME, "HP ENVY 15 Notebook PC"), - }, - }, - - { - .callback = video_disable_native_backlight, - .ident = "SAMSUNG 870Z5E/880Z5E/680Z5E", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), - DMI_MATCH(DMI_PRODUCT_NAME, "870Z5E/880Z5E/680Z5E"), - }, - }, - { - .callback = video_disable_native_backlight, - .ident = "SAMSUNG 370R4E/370R4V/370R5E/3570RE/370R5V", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), - DMI_MATCH(DMI_PRODUCT_NAME, "370R4E/370R4V/370R5E/3570RE/370R5V"), - }, - }, - { - /* https://bugzilla.redhat.com/show_bug.cgi?id=1186097 */ - .callback = video_disable_native_backlight, - .ident = "SAMSUNG 3570R/370R/470R/450R/510R/4450RV", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), - DMI_MATCH(DMI_PRODUCT_NAME, "3570R/370R/470R/450R/510R/4450RV"), - }, - }, - { - /* https://bugzilla.redhat.com/show_bug.cgi?id=1094948 */ - .callback = video_disable_native_backlight, - .ident = "SAMSUNG 730U3E/740U3E", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), - DMI_MATCH(DMI_PRODUCT_NAME, "730U3E/740U3E"), - }, - }, - { - /* https://bugs.freedesktop.org/show_bug.cgi?id=87286 */ - .callback = video_disable_native_backlight, - .ident = "SAMSUNG 900X3C/900X3D/900X3E/900X4C/900X4D", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), - DMI_MATCH(DMI_PRODUCT_NAME, "900X3C/900X3D/900X3E/900X4C/900X4D"), - }, - }, - - { - /* https://bugzilla.redhat.com/show_bug.cgi?id=1163574 */ - .callback = video_disable_native_backlight, - .ident = "Dell XPS15 L521X", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), - DMI_MATCH(DMI_PRODUCT_NAME, "XPS L521X"), - }, - }, - - /* Non win8 machines which need native backlight nevertheless */ - { - /* https://bugzilla.redhat.com/show_bug.cgi?id=1187004 */ - .callback = video_enable_native_backlight, - .ident = "Lenovo Ideapad Z570", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), - DMI_MATCH(DMI_PRODUCT_NAME, "102434U"), - }, - }, - { - /* https://bugzilla.redhat.com/show_bug.cgi?id=1217249 */ - .callback = video_enable_native_backlight, - .ident = "Apple MacBook Pro 12,1", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."), - DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro12,1"), - }, - }, {} };
@@ -1391,7 +1230,7 @@ acpi_video_switch_brightness(struct work_struct *work) int result = -EINVAL;
/* no warning message if acpi_backlight=vendor or a quirk is used */ - if (!acpi_video_verify_backlight_support()) + if (!device->backlight) return;
if (!device->brightness) @@ -1738,8 +1577,10 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) &acpi_backlight_ops, &props); kfree(name); - if (IS_ERR(device->backlight)) + if (IS_ERR(device->backlight)) { + device->backlight = NULL; return; + }
/* * Save current brightness level in case we have to restore it @@ -1796,7 +1637,7 @@ static int acpi_video_bus_register_backlight(struct acpi_video_bus *video)
acpi_video_run_bcl_for_osi(video);
- if (!acpi_video_verify_backlight_support()) + if (acpi_video_get_backlight_type() != acpi_backlight_video) return 0;
mutex_lock(&video->device_list_lock); @@ -1945,20 +1786,23 @@ static int acpi_video_backlight_notify(struct notifier_block *nb, { struct backlight_device *backlight = bd; struct acpi_video_bus *video; + enum acpi_backlight_type type;
- /* acpi_video_verify_backlight_support only cares about raw devices */ + /* A raw bl (un)registering may change native <-> video */ if (backlight->props.type != BACKLIGHT_RAW) return NOTIFY_DONE;
video = container_of(nb, struct acpi_video_bus, backlight_nb); + type = acpi_video_get_backlight_type();
switch (val) { case BACKLIGHT_REGISTERED: - if (!acpi_video_verify_backlight_support()) + if (type != acpi_backlight_video) acpi_video_bus_unregister_backlight(video); break; case BACKLIGHT_UNREGISTERED: - acpi_video_bus_register_backlight(video); + if (type == acpi_backlight_video) + acpi_video_bus_register_backlight(video); break; }
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 98368cf..c6b5951 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -88,6 +88,18 @@ static int video_detect_force_vendor(const struct dmi_system_id *d) return 0; }
+static int video_detect_force_video(const struct dmi_system_id *d) +{ + acpi_backlight_dmi = acpi_backlight_video; + return 0; +} + +static int video_detect_force_native(const struct dmi_system_id *d) +{ + acpi_backlight_dmi = acpi_backlight_native; + return 0; +} + static struct dmi_system_id video_detect_dmi_table[] = { /* On Samsung X360, the BIOS will set a flag (VDRV) if generic * ACPI backlight device is used. This flag will definitively break @@ -128,6 +140,124 @@ static struct dmi_system_id video_detect_dmi_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 5737"), }, }, + + /* + * These models have a working acpi_video backlight control, and using + * native backlight causes a regression where backlight does not work + * when userspace is not handling brightness key events. Disable + * native_backlight on these to fix this: + * https://bugzilla.kernel.org/show_bug.cgi?id=81691 + */ + { + .callback = video_detect_force_video, + .ident = "ThinkPad T420", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T420"), + }, + }, + { + .callback = video_detect_force_video, + .ident = "ThinkPad T520", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T520"), + }, + }, + { + .callback = video_detect_force_video, + .ident = "ThinkPad X201s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X201s"), + }, + }, + + /* The native backlight controls do not work on some older machines */ + { + /* https://bugs.freedesktop.org/show_bug.cgi?id=81515 */ + .callback = video_detect_force_video, + .ident = "HP ENVY 15 Notebook", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"), + DMI_MATCH(DMI_PRODUCT_NAME, "HP ENVY 15 Notebook PC"), + }, + }, + { + .callback = video_detect_force_video, + .ident = "SAMSUNG 870Z5E/880Z5E/680Z5E", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), + DMI_MATCH(DMI_PRODUCT_NAME, "870Z5E/880Z5E/680Z5E"), + }, + }, + { + .callback = video_detect_force_video, + .ident = "SAMSUNG 370R4E/370R4V/370R5E/3570RE/370R5V", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), + DMI_MATCH(DMI_PRODUCT_NAME, + "370R4E/370R4V/370R5E/3570RE/370R5V"), + }, + }, + { + /* https://bugzilla.redhat.com/show_bug.cgi?id=1186097 */ + .callback = video_detect_force_video, + .ident = "SAMSUNG 3570R/370R/470R/450R/510R/4450RV", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), + DMI_MATCH(DMI_PRODUCT_NAME, + "3570R/370R/470R/450R/510R/4450RV"), + }, + }, + { + /* https://bugzilla.redhat.com/show_bug.cgi?id=1094948 */ + .callback = video_detect_force_video, + .ident = "SAMSUNG 730U3E/740U3E", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), + DMI_MATCH(DMI_PRODUCT_NAME, "730U3E/740U3E"), + }, + }, + { + /* https://bugs.freedesktop.org/show_bug.cgi?id=87286 */ + .callback = video_detect_force_video, + .ident = "SAMSUNG 900X3C/900X3D/900X3E/900X4C/900X4D", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."), + DMI_MATCH(DMI_PRODUCT_NAME, + "900X3C/900X3D/900X3E/900X4C/900X4D"), + }, + }, + { + /* https://bugzilla.redhat.com/show_bug.cgi?id=1163574 */ + .callback = video_detect_force_video, + .ident = "Dell XPS15 L521X", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "XPS L521X"), + }, + }, + + /* Non win8 machines which need native backlight nevertheless */ + { + /* https://bugzilla.redhat.com/show_bug.cgi?id=1187004 */ + .callback = video_detect_force_native, + .ident = "Lenovo Ideapad Z570", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "102434U"), + }, + }, + { + /* https://bugzilla.redhat.com/show_bug.cgi?id=1217249 */ + .callback = video_detect_force_native, + .ident = "Apple MacBook Pro 12,1", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro12,1"), + }, + }, { }, };
diff --git a/include/acpi/video.h b/include/acpi/video.h index 01b5cc7..47c7ad6 100644 --- a/include/acpi/video.h +++ b/include/acpi/video.h @@ -30,7 +30,6 @@ extern void acpi_video_unregister(void); extern void acpi_video_unregister_backlight(void); extern int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, void **edid); -extern bool acpi_video_verify_backlight_support(void); extern enum acpi_backlight_type acpi_video_get_backlight_type(void); extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type); #else @@ -42,7 +41,6 @@ static inline int acpi_video_get_edid(struct acpi_device *device, int type, { return -ENODEV; } -static inline bool acpi_video_verify_backlight_support(void) { return false; } static inline enum acpi_backlight_type acpi_video_get_backlight_type(void) { return acpi_backlight_vendor;
Move the unregistering of the acpi backlight interface on registering of a native backlight from video.c to video_detect.c where it belongs.
Note this removes support for re-registering the acpi backlight interface when the native interface goes away. In practice this never happens and it needlessly complicates the code.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video.c | 59 ++------------------------------------------- drivers/acpi/video_detect.c | 27 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 57 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index 1bbe9b1..f5ffac9 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -84,6 +84,7 @@ 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); +void acpi_video_detect_exit(void);
static const struct acpi_device_id video_device_ids[] = { {ACPI_VIDEO_HID, 0}, @@ -143,7 +144,6 @@ struct acpi_video_enumerated_device { struct acpi_video_bus { struct acpi_device *device; bool backlight_registered; - bool backlight_notifier_registered; u8 dos_setting; struct acpi_video_enumerated_device *attached_array; u8 attached_count; @@ -156,7 +156,6 @@ struct acpi_video_bus { struct input_dev *input; char phys[32]; /* for input device */ struct notifier_block pm_nb; - struct notifier_block backlight_nb; };
struct acpi_video_device_flags { @@ -1781,59 +1780,6 @@ static void acpi_video_bus_remove_notify_handler(struct acpi_video_bus *video) video->input = NULL; }
-static int acpi_video_backlight_notify(struct notifier_block *nb, - unsigned long val, void *bd) -{ - struct backlight_device *backlight = bd; - struct acpi_video_bus *video; - enum acpi_backlight_type type; - - /* A raw bl (un)registering may change native <-> video */ - if (backlight->props.type != BACKLIGHT_RAW) - return NOTIFY_DONE; - - video = container_of(nb, struct acpi_video_bus, backlight_nb); - type = acpi_video_get_backlight_type(); - - switch (val) { - case BACKLIGHT_REGISTERED: - if (type != acpi_backlight_video) - acpi_video_bus_unregister_backlight(video); - break; - case BACKLIGHT_UNREGISTERED: - if (type == acpi_backlight_video) - acpi_video_bus_register_backlight(video); - break; - } - - return NOTIFY_OK; -} - -static int acpi_video_bus_add_backlight_notify_handler( - struct acpi_video_bus *video) -{ - int error; - - video->backlight_nb.notifier_call = acpi_video_backlight_notify; - video->backlight_nb.priority = 0; - error = backlight_register_notifier(&video->backlight_nb); - if (error == 0) - video->backlight_notifier_registered = true; - - return error; -} - -static int acpi_video_bus_remove_backlight_notify_handler( - struct acpi_video_bus *video) -{ - if (!video->backlight_notifier_registered) - return 0; - - video->backlight_notifier_registered = false; - - return backlight_unregister_notifier(&video->backlight_nb); -} - static int acpi_video_bus_put_devices(struct acpi_video_bus *video) { struct acpi_video_device *dev, *next; @@ -1915,7 +1861,6 @@ static int acpi_video_bus_add(struct acpi_device *device)
acpi_video_bus_register_backlight(video); acpi_video_bus_add_notify_handler(video); - acpi_video_bus_add_backlight_notify_handler(video);
return 0;
@@ -1939,7 +1884,6 @@ static int acpi_video_bus_remove(struct acpi_device *device)
video = acpi_driver_data(device);
- acpi_video_bus_remove_backlight_notify_handler(video); acpi_video_bus_remove_notify_handler(video); acpi_video_bus_unregister_backlight(video); acpi_video_bus_put_devices(video); @@ -2075,6 +2019,7 @@ static int __init acpi_video_init(void)
static void __exit acpi_video_exit(void) { + acpi_video_detect_exit(); acpi_video_unregister();
return; diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index c6b5951..4989ba6 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -36,6 +36,9 @@ ACPI_MODULE_NAME("video"); #define _COMPONENT ACPI_VIDEO_COMPONENT
+static bool backlight_notifier_registered; +static struct notifier_block backlight_nb; + static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef; static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef;
@@ -261,6 +264,20 @@ static struct dmi_system_id video_detect_dmi_table[] = { { }, };
+static int acpi_video_backlight_notify(struct notifier_block *nb, + unsigned long val, void *bd) +{ + struct backlight_device *backlight = bd; + + /* A raw bl registering may change video -> native */ + if (backlight->props.type == BACKLIGHT_RAW && + val == BACKLIGHT_REGISTERED && + acpi_video_get_backlight_type() != acpi_backlight_video) + acpi_video_unregister_backlight(); + + return NOTIFY_OK; +} + /* * Determine which type of backlight interface to use on this system, * First check cmdline, then dmi quirks, then do autodetect. @@ -289,6 +306,10 @@ enum acpi_backlight_type acpi_video_get_backlight_type(void) acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, find_video, NULL, &video_caps, NULL); + backlight_nb.notifier_call = acpi_video_backlight_notify; + backlight_nb.priority = 0; + if (backlight_register_notifier(&backlight_nb) == 0) + backlight_notifier_registered = true; init_done = true; } mutex_unlock(&init_mutex); @@ -353,3 +374,9 @@ int acpi_video_backlight_support(void) return acpi_video_get_backlight_type() != acpi_backlight_vendor; } EXPORT_SYMBOL(acpi_video_backlight_support); + +void __exit acpi_video_detect_exit(void) +{ + if (backlight_notifier_registered) + backlight_unregister_notifier(&backlight_nb); +}
When builtin there is no guarantee in which order module_init functions are run, so acpi_video_register() may get called from the i915 driver (if it is also builtin) before acpi_video_init() gets called, resulting in the dmi quirks not yet being parsed.
This commit moves the dmi_check_system() call to acpi_video_register(), so that we can be sure the dmi quirks have always been applied before probing.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index f5ffac9..b8053a8 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -380,13 +380,13 @@ acpi_video_device_lcd_set_level(struct acpi_video_device *device, int level) */
static int bqc_offset_aml_bug_workaround; -static int __init video_set_bqc_offset(const struct dmi_system_id *d) +static int video_set_bqc_offset(const struct dmi_system_id *d) { bqc_offset_aml_bug_workaround = 9; return 0; }
-static struct dmi_system_id video_dmi_table[] __initdata = { +static struct dmi_system_id video_dmi_table[] = { /* * Broken _BQC workaround http://bugzilla.kernel.org/show_bug.cgi?id=13121 */ @@ -1944,6 +1944,8 @@ int acpi_video_register(void) mutex_init(&video_list_lock); INIT_LIST_HEAD(&video_bus_head);
+ dmi_check_system(video_dmi_table); + ret = acpi_bus_register_driver(&acpi_video_bus); if (ret) return ret; @@ -2009,8 +2011,6 @@ static int __init acpi_video_init(void) if (acpi_disabled) return 0;
- dmi_check_system(video_dmi_table); - if (intel_opregion_present()) return 0;
It is possible for a native backlight driver to load while acpi_video_register is running, which may lead to acpi_video_unregister_backlight being called while acpi_video_register is running and the 2 racing against eachother.
The register_count variable protects against this, but not in a thread safe manner, this commit adds locking to make this thread safe.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video.c | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index b8053a8..fcc7c2e 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -79,6 +79,7 @@ static bool allow_duplicates; module_param(allow_duplicates, bool, 0644);
static int register_count; +static DEFINE_MUTEX(register_count_mutex); static struct mutex video_list_lock; static struct list_head video_bus_head; static int acpi_video_bus_add(struct acpi_device *device); @@ -1931,14 +1932,15 @@ static int __init intel_opregion_present(void)
int acpi_video_register(void) { - int ret; + int ret = 0;
+ mutex_lock(®ister_count_mutex); if (register_count) { /* * if the function of acpi_video_register is already called, * don't register the acpi_vide_bus again and return no error. */ - return 0; + goto leave; }
mutex_init(&video_list_lock); @@ -1948,7 +1950,7 @@ int acpi_video_register(void)
ret = acpi_bus_register_driver(&acpi_video_bus); if (ret) - return ret; + goto leave;
/* * When the acpi_video_bus is loaded successfully, increase @@ -1956,24 +1958,20 @@ int acpi_video_register(void) */ register_count = 1;
- return 0; +leave: + mutex_unlock(®ister_count_mutex); + return ret; } EXPORT_SYMBOL(acpi_video_register);
void acpi_video_unregister(void) { - if (!register_count) { - /* - * If the acpi video bus is already unloaded, don't - * unload it again and return directly. - */ - return; + mutex_lock(®ister_count_mutex); + if (register_count) { + acpi_bus_unregister_driver(&acpi_video_bus); + register_count = 0; } - acpi_bus_unregister_driver(&acpi_video_bus); - - register_count = 0; - - return; + mutex_unlock(®ister_count_mutex); } EXPORT_SYMBOL(acpi_video_unregister);
@@ -1981,13 +1979,14 @@ void acpi_video_unregister_backlight(void) { struct acpi_video_bus *video;
- if (!register_count) - return; - - mutex_lock(&video_list_lock); - list_for_each_entry(video, &video_bus_head, entry) - acpi_video_bus_unregister_backlight(video); - mutex_unlock(&video_list_lock); + mutex_lock(®ister_count_mutex); + if (register_count) { + mutex_lock(&video_list_lock); + list_for_each_entry(video, &video_bus_head, entry) + acpi_video_bus_unregister_backlight(video); + mutex_unlock(&video_list_lock); + } + mutex_unlock(®ister_count_mutex); } EXPORT_SYMBOL(acpi_video_unregister_backlight);
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/acer-wmi.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c index 3ac29a1..f6b280d 100644 --- a/drivers/platform/x86/acer-wmi.c +++ b/drivers/platform/x86/acer-wmi.c @@ -2246,14 +2246,10 @@ static int __init acer_wmi_init(void) set_quirks();
if (dmi_check_system(video_vendor_dmi_table)) - acpi_video_dmi_promote_vendor(); - if (acpi_video_backlight_support()) { + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); + + if (acpi_video_get_backlight_type() != acpi_backlight_vendor) interface->capability &= ~ACER_CAP_BRIGHTNESS; - pr_info("Brightness must be controlled by acpi video driver\n"); - } else { - pr_info("Disabling ACPI video driver\n"); - acpi_video_unregister_backlight(); - }
if (wmi_has_guid(WMID_GUID3)) { if (ec_raw_mode) {
On Wed, Jun 10, 2015 at 03:01:14PM +0200, Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Reviewed-by: Lee, Chun-Yi jlee@suse.com
Thanks a lot! Joey Lee
drivers/platform/x86/acer-wmi.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c index 3ac29a1..f6b280d 100644 --- a/drivers/platform/x86/acer-wmi.c +++ b/drivers/platform/x86/acer-wmi.c @@ -2246,14 +2246,10 @@ static int __init acer_wmi_init(void) set_quirks();
if (dmi_check_system(video_vendor_dmi_table))
acpi_video_dmi_promote_vendor();- if (acpi_video_backlight_support()) {
acpi_video_set_dmi_backlight_type(acpi_backlight_vendor);- if (acpi_video_get_backlight_type() != acpi_backlight_vendor) interface->capability &= ~ACER_CAP_BRIGHTNESS;
pr_info("Brightness must be controlled by acpi video driver\n");} else {
pr_info("Disabling ACPI video driver\n");acpi_video_unregister_backlight();}
if (wmi_has_guid(WMID_GUID3)) { if (ec_raw_mode) {
-- 2.4.2
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/apple-gmux.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c index a7f6412..0dec3f5 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -550,8 +550,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id) * backlight control and supports more levels than other options. * Disable the other backlight choices. */ - acpi_video_dmi_promote_vendor(); - acpi_video_unregister_backlight(); + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); apple_bl_unregister();
gmux_data->power_state = VGA_SWITCHEROO_ON;
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/asus-laptop.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/asus-laptop.c b/drivers/platform/x86/asus-laptop.c index 46b2746..58d29c4 100644 --- a/drivers/platform/x86/asus-laptop.c +++ b/drivers/platform/x86/asus-laptop.c @@ -54,6 +54,7 @@ #include <linux/slab.h> #include <linux/dmi.h> #include <linux/acpi.h> +#include <acpi/video.h>
#define ASUS_LAPTOP_VERSION "0.42"
@@ -1884,12 +1885,11 @@ static int asus_acpi_add(struct acpi_device *device) if (result) goto fail_platform;
- if (!acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { result = asus_backlight_init(asus); if (result) goto fail_backlight; - } else - pr_info("Backlight controlled by ACPI video driver\n"); + }
result = asus_input_init(asus); if (result)
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/asus-wmi.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 945145d..6f8558f 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -1364,7 +1364,7 @@ static void asus_wmi_notify(u32 value, void *context) code = ASUS_WMI_BRN_DOWN;
if (code == ASUS_WMI_BRN_DOWN || code == ASUS_WMI_BRN_UP) { - if (!acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { asus_wmi_backlight_notify(asus, orig_code); goto exit; } @@ -1772,17 +1772,16 @@ static int asus_wmi_add(struct platform_device *pdev) stop this from showing up */ chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE); if (chassis_type && !strcmp(chassis_type, "3")) - acpi_video_dmi_promote_vendor(); + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); + if (asus->driver->quirks->wmi_backlight_power) - acpi_video_dmi_promote_vendor(); - if (!acpi_video_backlight_support()) { - pr_info("Disabling ACPI video driver\n"); - acpi_video_unregister_backlight(); + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); + + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { err = asus_wmi_backlight_init(asus); if (err && err != -ENODEV) goto fail_backlight; - } else - pr_info("Backlight controlled by ACPI video driver\n"); + }
status = wmi_install_notify_handler(asus->driver->event_guid, asus_wmi_notify, asus);
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/compal-laptop.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/compal-laptop.c b/drivers/platform/x86/compal-laptop.c index b4e9447..f2706d2 100644 --- a/drivers/platform/x86/compal-laptop.c +++ b/drivers/platform/x86/compal-laptop.c @@ -82,7 +82,7 @@ #include <linux/hwmon-sysfs.h> #include <linux/power_supply.h> #include <linux/fb.h> - +#include <acpi/video.h>
/* ======= */ /* Defines */ @@ -959,7 +959,7 @@ static int __init compal_init(void) return -ENODEV; }
- if (!acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { struct backlight_properties props; memset(&props, 0, sizeof(struct backlight_properties)); props.type = BACKLIGHT_PLATFORM;
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/dell-laptop.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index d688d80..db2e031 100644 --- a/drivers/platform/x86/dell-laptop.c +++ b/drivers/platform/x86/dell-laptop.c @@ -31,6 +31,7 @@ #include <linux/slab.h> #include <linux/debugfs.h> #include <linux/seq_file.h> +#include <acpi/video.h> #include "../../firmware/dcdbas.h"
#define BRIGHTNESS_TOKEN 0x7d @@ -1921,10 +1922,7 @@ static int __init dell_init(void) &dell_debugfs_fops);
#ifdef CONFIG_ACPI - /* In the event of an ACPI backlight being available, don't - * register the platform controller. - */ - if (acpi_video_backlight_support()) + if (acpi_video_get_backlight_type() != acpi_backlight_vendor) return 0; #endif
On Wednesday 10 June 2015 15:01:19 Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/platform/x86/dell-laptop.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index d688d80..db2e031 100644 --- a/drivers/platform/x86/dell-laptop.c +++ b/drivers/platform/x86/dell-laptop.c @@ -31,6 +31,7 @@ #include <linux/slab.h> #include <linux/debugfs.h> #include <linux/seq_file.h> +#include <acpi/video.h> #include "../../firmware/dcdbas.h"
#define BRIGHTNESS_TOKEN 0x7d @@ -1921,10 +1922,7 @@ static int __init dell_init(void) &dell_debugfs_fops);
#ifdef CONFIG_ACPI
- /* In the event of an ACPI backlight being available, don't
* register the platform controller.*/- if (acpi_video_backlight_support())
- if (acpi_video_get_backlight_type() != acpi_backlight_vendor) return 0;
#endif
Now I'm thinking... Is this correct condition? And do we need it? This Dell backlight interface works even if ACPI or intel i915 driver controls backlight. Why we should prevent dell-laptop.ko to register Dell backlight interface if ACPI video already register one? And why not prevent when intel i915 driver register another?
Hi,
On 11-06-15 13:47, Pali Rohár wrote:
On Wednesday 10 June 2015 15:01:19 Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/platform/x86/dell-laptop.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index d688d80..db2e031 100644 --- a/drivers/platform/x86/dell-laptop.c +++ b/drivers/platform/x86/dell-laptop.c @@ -31,6 +31,7 @@ #include <linux/slab.h> #include <linux/debugfs.h> #include <linux/seq_file.h> +#include <acpi/video.h> #include "../../firmware/dcdbas.h"
#define BRIGHTNESS_TOKEN 0x7d @@ -1921,10 +1922,7 @@ static int __init dell_init(void) &dell_debugfs_fops);
#ifdef CONFIG_ACPI
- /* In the event of an ACPI backlight being available, don't
* register the platform controller.*/- if (acpi_video_backlight_support())
- if (acpi_video_get_backlight_type() != acpi_backlight_vendor) return 0; #endif
Now I'm thinking... Is this correct condition? And do we need it? This Dell backlight interface works even if ACPI or intel i915 driver controls backlight. Why we should prevent dell-laptop.ko to register Dell backlight interface if ACPI video already register one? And why not prevent when intel i915 driver register another?
3 things:
1) This is mostly a cleanup series, it does not make any behavioral changes (other then renaming the kernelcmdline option and we've already discussed fixing that).
2) All 3 drivers ultimately bit-bang the same hardware, we really should have only one driver banging the hardware, so if (acpi_video_get_backlight_type() != acpi_backlight_vendor) return 0;
Definitely is the right thing todo here.
3) You're right that the intel_backlight still registering even if acpi_video_get_backlight_type() != acpi_backlight_native is weird, but that is how it was before this series, and userspace prefers firmware type backlight sysfs entries over platform /raw ones so it will automatically pick the right one. We should consider not registering the backlight sysfs class interface from the intel code when acpi_video_get_backlight_type() != acpi_backlight_native but that is something for later...
Regards,
Hans
On Thursday 11 June 2015 14:29:24 Hans de Goede wrote:
Hi,
On 11-06-15 13:47, Pali Rohár wrote:
On Wednesday 10 June 2015 15:01:19 Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/platform/x86/dell-laptop.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index d688d80..db2e031 100644 --- a/drivers/platform/x86/dell-laptop.c +++ b/drivers/platform/x86/dell-laptop.c @@ -31,6 +31,7 @@ #include <linux/slab.h> #include <linux/debugfs.h> #include <linux/seq_file.h> +#include <acpi/video.h> #include "../../firmware/dcdbas.h"
#define BRIGHTNESS_TOKEN 0x7d @@ -1921,10 +1922,7 @@ static int __init dell_init(void) &dell_debugfs_fops);
#ifdef CONFIG_ACPI
- /* In the event of an ACPI backlight being available, don't
* register the platform controller.*/- if (acpi_video_backlight_support())
- if (acpi_video_get_backlight_type() != acpi_backlight_vendor) return 0;
#endif
Now I'm thinking... Is this correct condition? And do we need it? This Dell backlight interface works even if ACPI or intel i915 driver controls backlight. Why we should prevent dell-laptop.ko to register Dell backlight interface if ACPI video already register one? And why not prevent when intel i915 driver register another?
3 things:
- This is mostly a cleanup series, it does not make any behavioral
changes (other then renaming the kernelcmdline option and we've already discussed fixing that).
- All 3 drivers ultimately bit-bang the same hardware, we really should
have only one driver banging the hardware, so if (acpi_video_get_backlight_type() != acpi_backlight_vendor) return 0;
Definitely is the right thing todo here.
- You're right that the intel_backlight still registering even if
acpi_video_get_backlight_type() != acpi_backlight_native is weird, but that is how it was before this series, and userspace prefers firmware type backlight sysfs entries over platform /raw ones so it will automatically pick the right one. We should consider not registering the backlight sysfs class interface from the intel code when acpi_video_get_backlight_type() != acpi_backlight_native but that is something for later...
Regards,
Hans
Ok. This patch series is just refactor/cleanup which does not change functional behavior. I'm ok with this dell-laptop change, so you can add my Acked-By.
Once you will create new patches for backlight (maybe for intel i915), please CC me, so I can discuss about them too.
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/dell-wmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 6512a06..f2d77fe 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -35,6 +35,7 @@ #include <linux/acpi.h> #include <linux/string.h> #include <linux/dmi.h> +#include <acpi/video.h>
MODULE_AUTHOR("Matthew Garrett mjg@redhat.com"); MODULE_DESCRIPTION("Dell laptop WMI hotkeys driver"); @@ -397,7 +398,7 @@ static int __init dell_wmi_init(void) }
dmi_walk(find_hk_type, NULL); - acpi_video = acpi_video_backlight_support(); + acpi_video = acpi_video_get_backlight_type() != acpi_backlight_vendor;
err = dell_wmi_input_setup(); if (err)
On Wednesday 10 June 2015 15:01:20 Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/platform/x86/dell-wmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 6512a06..f2d77fe 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -35,6 +35,7 @@ #include <linux/acpi.h> #include <linux/string.h> #include <linux/dmi.h> +#include <acpi/video.h>
MODULE_AUTHOR("Matthew Garrett mjg@redhat.com"); MODULE_DESCRIPTION("Dell laptop WMI hotkeys driver"); @@ -397,7 +398,7 @@ static int __init dell_wmi_init(void) }
dmi_walk(find_hk_type, NULL);
- acpi_video = acpi_video_backlight_support();
acpi_video = acpi_video_get_backlight_type() != acpi_backlight_vendor;
err = dell_wmi_input_setup(); if (err)
Hello,
to make sure that nothing will be broken I will try to explain current code. Variable acpi_video is global boolean (for this module) and when is set to true it is expected that ACPI generate brightness up/down key events via ACPI input device. When is acpi_video boolean variable is set to false then ACPI input device should not send brightness up/down keys to userspace. And dell-wmi.ko driver send those two keys via dell-wmi input device.
So please check that your change does not change above behaviour. Maybe it would make sense to rename "acpi_video" variable to something better.
Hi,
On 11-06-15 13:43, Pali Rohár wrote:
On Wednesday 10 June 2015 15:01:20 Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/platform/x86/dell-wmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 6512a06..f2d77fe 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -35,6 +35,7 @@ #include <linux/acpi.h> #include <linux/string.h> #include <linux/dmi.h> +#include <acpi/video.h>
MODULE_AUTHOR("Matthew Garrett mjg@redhat.com"); MODULE_DESCRIPTION("Dell laptop WMI hotkeys driver"); @@ -397,7 +398,7 @@ static int __init dell_wmi_init(void) }
dmi_walk(find_hk_type, NULL);
- acpi_video = acpi_video_backlight_support();
acpi_video = acpi_video_get_backlight_type() != acpi_backlight_vendor;
err = dell_wmi_input_setup(); if (err)
Hello,
to make sure that nothing will be broken I will try to explain current code. Variable acpi_video is global boolean (for this module) and when is set to true it is expected that ACPI generate brightness up/down key events via ACPI input device. When is acpi_video boolean variable is set to false then ACPI input device should not send brightness up/down keys to userspace. And dell-wmi.ko driver send those two keys via dell-wmi input device.
I know.
So please check that your change does not change above behaviour.
I've already checked this.
Maybe it would make sense to rename "acpi_video" variable to something better.
There is another driver which has a similar construction. I believe that it would be good to add an acpi_video_handles_key_presses() helper to the acpi-video code which can be used for this instead of abusing acpi_video_get_backlight_type() for this. I've put this on my todo list to do as a further cleanup once the initial series is merged.
Regards,
Hans
On Thursday 11 June 2015 14:19:11 Hans de Goede wrote:
Hi,
On 11-06-15 13:43, Pali Rohár wrote:
On Wednesday 10 June 2015 15:01:20 Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/platform/x86/dell-wmi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c index 6512a06..f2d77fe 100644 --- a/drivers/platform/x86/dell-wmi.c +++ b/drivers/platform/x86/dell-wmi.c @@ -35,6 +35,7 @@ #include <linux/acpi.h> #include <linux/string.h> #include <linux/dmi.h> +#include <acpi/video.h>
MODULE_AUTHOR("Matthew Garrett mjg@redhat.com"); MODULE_DESCRIPTION("Dell laptop WMI hotkeys driver"); @@ -397,7 +398,7 @@ static int __init dell_wmi_init(void) }
dmi_walk(find_hk_type, NULL);
- acpi_video = acpi_video_backlight_support();
acpi_video = acpi_video_get_backlight_type() != acpi_backlight_vendor;
err = dell_wmi_input_setup(); if (err)
Hello,
to make sure that nothing will be broken I will try to explain current code. Variable acpi_video is global boolean (for this module) and when is set to true it is expected that ACPI generate brightness up/down key events via ACPI input device. When is acpi_video boolean variable is set to false then ACPI input device should not send brightness up/down keys to userspace. And dell-wmi.ko driver send those two keys via dell-wmi input device.
I know.
So please check that your change does not change above behaviour.
I've already checked this.
Ok, then this patch is fine for me. Add my Acked-by or Reviewed-by.
Maybe it would make sense to rename "acpi_video" variable to something better.
There is another driver which has a similar construction. I believe that it would be good to add an acpi_video_handles_key_presses() helper to the acpi-video code which can be used for this instead of abusing acpi_video_get_backlight_type() for this. I've put this on my todo list to do as a further cleanup once the initial series is merged.
Regards,
Hans
Ok, thanks! acpi_video_handles_key_presses() has really better name.
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/eeepc-laptop.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c index 844c209..8cdf315 100644 --- a/drivers/platform/x86/eeepc-laptop.c +++ b/drivers/platform/x86/eeepc-laptop.c @@ -37,6 +37,7 @@ #include <linux/pci_hotplug.h> #include <linux/leds.h> #include <linux/dmi.h> +#include <acpi/video.h>
#define EEEPC_LAPTOP_VERSION "0.1" #define EEEPC_LAPTOP_NAME "Eee PC Hotkey Driver" @@ -1433,12 +1434,10 @@ static int eeepc_acpi_add(struct acpi_device *device) if (result) goto fail_platform;
- if (!acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { result = eeepc_backlight_init(eeepc); if (result) goto fail_backlight; - } else { - pr_info("Backlight controlled by ACPI video driver\n"); }
result = eeepc_input_init(eeepc);
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/fujitsu-laptop.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c index 2a9afa2..1c62caf 100644 --- a/drivers/platform/x86/fujitsu-laptop.c +++ b/drivers/platform/x86/fujitsu-laptop.c @@ -72,6 +72,7 @@ #if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE) #include <linux/leds.h> #endif +#include <acpi/video.h>
#define FUJITSU_DRIVER_VERSION "0.6.0"
@@ -1099,7 +1100,7 @@ static int __init fujitsu_init(void)
/* Register backlight stuff */
- if (!acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { struct backlight_properties props;
memset(&props, 0, sizeof(struct backlight_properties)); @@ -1137,8 +1138,7 @@ static int __init fujitsu_init(void) }
/* Sync backlight power status (needs FUJ02E3 device, hence deferred) */ - - if (!acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { if (call_fext_func(FUNC_BACKLIGHT, 0x2, 0x4, 0x0) == 3) fujitsu->bl_device->props.power = FB_BLANK_POWERDOWN; else
On Wed, Jun 10, 2015 at 03:01:22PM +0200, Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
Acked-by: Jonathan Woithe jwoithe@just42.net
Signed-off-by: Hans de Goede hdegoede@redhat.com
drivers/platform/x86/fujitsu-laptop.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c index 2a9afa2..1c62caf 100644 --- a/drivers/platform/x86/fujitsu-laptop.c +++ b/drivers/platform/x86/fujitsu-laptop.c @@ -72,6 +72,7 @@ #if defined(CONFIG_LEDS_CLASS) || defined(CONFIG_LEDS_CLASS_MODULE) #include <linux/leds.h> #endif +#include <acpi/video.h>
#define FUJITSU_DRIVER_VERSION "0.6.0"
@@ -1099,7 +1100,7 @@ static int __init fujitsu_init(void)
/* Register backlight stuff */
- if (!acpi_video_backlight_support()) {
if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { struct backlight_properties props;
memset(&props, 0, sizeof(struct backlight_properties));
@@ -1137,8 +1138,7 @@ static int __init fujitsu_init(void) }
/* Sync backlight power status (needs FUJ02E3 device, hence deferred) */
- if (!acpi_video_backlight_support()) {
- if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { if (call_fext_func(FUNC_BACKLIGHT, 0x2, 0x4, 0x0) == 3) fujitsu->bl_device->props.power = FB_BLANK_POWERDOWN; else
-- 2.4.2
--
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/ideapad-laptop.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c index b496db8..bea0228 100644 --- a/drivers/platform/x86/ideapad-laptop.c +++ b/drivers/platform/x86/ideapad-laptop.c @@ -38,6 +38,7 @@ #include <linux/i8042.h> #include <linux/dmi.h> #include <linux/device.h> +#include <acpi/video.h>
#define IDEAPAD_RFKILL_DEV_NUM (3)
@@ -903,7 +904,7 @@ static int ideapad_acpi_add(struct platform_device *pdev) ideapad_sync_rfk_state(priv); ideapad_sync_touchpad_state(priv);
- if (!acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { ret = ideapad_backlight_init(priv); if (ret && ret != -ENODEV) goto backlight_failed;
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/intel_oaktrail.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/intel_oaktrail.c b/drivers/platform/x86/intel_oaktrail.c index 8037c8b..6aa33c4 100644 --- a/drivers/platform/x86/intel_oaktrail.c +++ b/drivers/platform/x86/intel_oaktrail.c @@ -50,6 +50,7 @@ #include <linux/platform_device.h> #include <linux/dmi.h> #include <linux/rfkill.h> +#include <acpi/video.h>
#define DRIVER_NAME "intel_oaktrail" #define DRIVER_VERSION "0.4ac1" @@ -343,13 +344,11 @@ static int __init oaktrail_init(void) goto err_device_add; }
- if (!acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { ret = oaktrail_backlight_init(); if (ret) goto err_backlight; - - } else - pr_info("Backlight controlled by ACPI video driver\n"); + }
ret = oaktrail_rfkill_init(); if (ret) {
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/msi-laptop.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c index 0859877..4231770 100644 --- a/drivers/platform/x86/msi-laptop.c +++ b/drivers/platform/x86/msi-laptop.c @@ -64,6 +64,7 @@ #include <linux/i8042.h> #include <linux/input.h> #include <linux/input/sparse-keymap.h> +#include <acpi/video.h>
#define MSI_DRIVER_VERSION "0.5"
@@ -1069,9 +1070,8 @@ static int __init msi_init(void)
/* Register backlight stuff */
- if (!quirks->old_ec_model || acpi_video_backlight_support()) { - pr_info("Brightness ignored, must be controlled by ACPI video driver\n"); - } else { + if (quirks->old_ec_model || + acpi_video_get_backlight_type() == acpi_backlight_vendor) { struct backlight_properties props; memset(&props, 0, sizeof(struct backlight_properties)); props.type = BACKLIGHT_PLATFORM;
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/msi-wmi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c index 6d2bac0..978e6d6 100644 --- a/drivers/platform/x86/msi-wmi.c +++ b/drivers/platform/x86/msi-wmi.c @@ -29,6 +29,7 @@ #include <linux/backlight.h> #include <linux/slab.h> #include <linux/module.h> +#include <acpi/video.h>
MODULE_AUTHOR("Thomas Renninger trenn@suse.de"); MODULE_DESCRIPTION("MSI laptop WMI hotkeys driver"); @@ -320,7 +321,8 @@ static int __init msi_wmi_init(void) break; }
- if (wmi_has_guid(MSIWMI_BIOS_GUID) && !acpi_video_backlight_support()) { + if (wmi_has_guid(MSIWMI_BIOS_GUID) && + acpi_video_get_backlight_type() == acpi_backlight_vendor) { err = msi_wmi_backlight_setup(); if (err) { pr_err("Unable to setup backlight device\n");
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/samsung-laptop.c | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c index 0df03e2..8c146e2 100644 --- a/drivers/platform/x86/samsung-laptop.c +++ b/drivers/platform/x86/samsung-laptop.c @@ -1720,27 +1720,14 @@ static int __init samsung_init(void) samsung->handle_backlight = true; samsung->quirks = quirks;
- #ifdef CONFIG_ACPI if (samsung->quirks->broken_acpi_video) - acpi_video_dmi_promote_vendor(); - - /* Don't handle backlight here if the acpi video already handle it */ - if (acpi_video_backlight_support()) { - samsung->handle_backlight = false; - } else if (samsung->quirks->broken_acpi_video) { - pr_info("Disabling ACPI video driver\n"); - acpi_video_unregister_backlight(); - } + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); + if (samsung->quirks->use_native_backlight) + acpi_video_set_dmi_backlight_type(acpi_backlight_native);
- if (samsung->quirks->use_native_backlight) { - pr_info("Using native backlight driver\n"); - /* Tell acpi-video to not handle the backlight */ - acpi_video_dmi_promote_vendor(); - acpi_video_unregister_backlight(); - /* And also do not handle it ourselves */ + if (acpi_video_get_backlight_type() != acpi_backlight_vendor) samsung->handle_backlight = false; - } #endif
ret = samsung_platform_init(samsung); @@ -1751,12 +1738,6 @@ static int __init samsung_init(void) if (ret) goto error_sabi;
-#ifdef CONFIG_ACPI - /* Only log that if we are really on a sabi platform */ - if (acpi_video_backlight_support()) - pr_info("Backlight controlled by ACPI video driver\n"); -#endif - ret = samsung_sysfs_init(samsung); if (ret) goto error_sysfs;
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/sony-laptop.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c index e51c1e7..6207be6 100644 --- a/drivers/platform/x86/sony-laptop.c +++ b/drivers/platform/x86/sony-laptop.c @@ -69,6 +69,7 @@ #include <linux/miscdevice.h> #endif #include <asm/uaccess.h> +#include <acpi/video.h>
#define dprintk(fmt, ...) \ do { \ @@ -3199,11 +3200,8 @@ static int sony_nc_add(struct acpi_device *device) }
/* setup input devices and helper fifo */ - if (acpi_video_backlight_support()) { - pr_info("brightness ignored, must be controlled by ACPI video driver\n"); - } else { + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) sony_nc_backlight_setup(); - }
/* create sony_pf sysfs attributes related to the SNC device */ for (item = sony_nc_values; item->name; ++item) {
On Wed, Jun 10, 2015 at 03:01:28PM +0200, Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Mattia Dongili malattia@linux.it
drivers/platform/x86/sony-laptop.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c index e51c1e7..6207be6 100644 --- a/drivers/platform/x86/sony-laptop.c +++ b/drivers/platform/x86/sony-laptop.c @@ -69,6 +69,7 @@ #include <linux/miscdevice.h> #endif #include <asm/uaccess.h> +#include <acpi/video.h>
#define dprintk(fmt, ...) \ do { \ @@ -3199,11 +3200,8 @@ static int sony_nc_add(struct acpi_device *device) }
/* setup input devices and helper fifo */
- if (acpi_video_backlight_support()) {
The comment above is really misplaced. I wouldn'd mind if you got rid of that too.
pr_info("brightness ignored, must be controlled by ACPI video driver\n");- } else {
- if (acpi_video_get_backlight_type() == acpi_backlight_vendor) sony_nc_backlight_setup();
}
/* create sony_pf sysfs attributes related to the SNC device */ for (item = sony_nc_values; item->name; ++item) {
-- 2.4.2
Hi,
On 10-06-15 23:30, Mattia Dongili wrote:
On Wed, Jun 10, 2015 at 03:01:28PM +0200, Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
This commit also removes various obsolete pr_xxx messages related to backlight interface selection. These are obsolete because they assume there is only a vendor or acpi backlight driver and no other choice. Also they are not necessary, if the user wants to know which backlight interfaces are registered a simple "ls /sys/class/backlight" suffices.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Mattia Dongili malattia@linux.it
drivers/platform/x86/sony-laptop.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c index e51c1e7..6207be6 100644 --- a/drivers/platform/x86/sony-laptop.c +++ b/drivers/platform/x86/sony-laptop.c @@ -69,6 +69,7 @@ #include <linux/miscdevice.h> #endif #include <asm/uaccess.h> +#include <acpi/video.h>
#define dprintk(fmt, ...) \ do { \ @@ -3199,11 +3200,8 @@ static int sony_nc_add(struct acpi_device *device) }
/* setup input devices and helper fifo */
- if (acpi_video_backlight_support()) {
The comment above is really misplaced. I wouldn'd mind if you got rid of that too.
I've removed it and squashed the removal of it into this patch. I plan to do a v2 of this patch-set later today, that will have this change.
Regards,
Hans
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/thinkpad_acpi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 28f3281..33e488c 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -83,6 +83,7 @@ #include <sound/control.h> #include <sound/initval.h> #include <asm/uaccess.h> +#include <acpi/video.h>
/* ThinkPad CMOS commands */ #define TP_CMOS_VOLUME_DOWN 0 @@ -3487,7 +3488,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) /* Do not issue duplicate brightness change events to * userspace. tpacpi_detect_brightness_capabilities() must have * been called before this point */ - if (acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() != acpi_backlight_vendor) { pr_info("This ThinkPad has standard ACPI backlight " "brightness control, supported by the ACPI " "video driver\n"); @@ -6491,7 +6492,7 @@ static int __init brightness_init(struct ibm_init_struct *iibm) return 1; }
- if (acpi_video_backlight_support()) { + if (acpi_video_get_backlight_type() != acpi_backlight_vendor) { if (brightness_enable > 1) { pr_info("Standard ACPI backlight interface " "available, not loading native one\n");
On Wed, Jun 10, 2015, at 10:01, Hans de Goede wrote:
Port the backlight selection logic to the new backlight interface selection API.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Henrique de Moraes Holschuh hmh@hmh.eng.br
drivers/platform/x86/thinkpad_acpi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index 28f3281..33e488c 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -83,6 +83,7 @@ #include <sound/control.h> #include <sound/initval.h> #include <asm/uaccess.h> +#include <acpi/video.h>
/* ThinkPad CMOS commands */ #define TP_CMOS_VOLUME_DOWN 0 @@ -3487,7 +3488,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) /* Do not issue duplicate brightness change events to * userspace. tpacpi_detect_brightness_capabilities() must have * been called before this point */
if (acpi_video_backlight_support()) {
if (acpi_video_get_backlight_type() != acpi_backlight_vendor) {@@ -6491,7 +6492,7 @@ static int __init brightness_init(struct ibm_init_struct *iibm) return 1; }
if (acpi_video_backlight_support()) {
if (acpi_video_get_backlight_type() != acpi_backlight_vendor) {-- 2.4.2
Port the backlight selection logic to the new backlight interface selection API.
Also fix a compiler warning about bt_present not being initialized.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/platform/x86/toshiba_acpi.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c index 9956b990..c484d47 100644 --- a/drivers/platform/x86/toshiba_acpi.c +++ b/drivers/platform/x86/toshiba_acpi.c @@ -2640,14 +2640,11 @@ static int toshiba_acpi_setup_backlight(struct toshiba_acpi_dev *dev) */ if (dev->tr_backlight_supported || dmi_check_system(toshiba_vendor_backlight_dmi)) - acpi_video_dmi_promote_vendor(); + acpi_video_set_dmi_backlight_type(acpi_backlight_vendor);
- if (acpi_video_backlight_support()) + if (acpi_video_get_backlight_type() != acpi_backlight_vendor) return 0;
- /* acpi-video may have loaded before we called dmi_promote_vendor() */ - acpi_video_unregister_backlight(); - memset(&props, 0, sizeof(props)); props.type = BACKLIGHT_PLATFORM; props.max_brightness = HCI_LCD_BRIGHTNESS_LEVELS - 1; @@ -2733,7 +2730,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev) const char *hci_method; u32 special_functions; u32 dummy; - bool bt_present; + bool bt_present = false; int ret = 0;
if (toshiba_acpi)
Remove the old backlight interface selection API now that all drivers have been ported to the new API.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video_detect.c | 31 ------------------------------- include/linux/acpi.h | 19 ------------------- 2 files changed, 50 deletions(-)
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 4989ba6..d5e6f1c 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -344,37 +344,6 @@ void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type) } EXPORT_SYMBOL(acpi_video_set_dmi_backlight_type);
-/* - * Compatiblity function, this is going away as soon as all drivers are - * converted to acpi_video_set_dmi_backlight_type(). - * - * Promote the vendor interface instead of the generic video module. - * After calling this function you will probably want to call - * acpi_video_unregister() to make sure the video module is not loaded - */ -void acpi_video_dmi_promote_vendor(void) -{ - acpi_video_set_dmi_backlight_type(acpi_backlight_vendor); -} -EXPORT_SYMBOL(acpi_video_dmi_promote_vendor); - -/* - * Compatiblity function, this is going away as soon as all drivers are - * converted to acpi_video_get_backlight_type(). - * - * Returns true if video.ko can do backlight switching. - */ -int acpi_video_backlight_support(void) -{ - /* - * This is done this way since vendor drivers call this to see - * if they should load, and we do not want them to load for both - * the acpi_backlight_video and acpi_backlight_native cases. - */ - return acpi_video_get_backlight_type() != acpi_backlight_vendor; -} -EXPORT_SYMBOL(acpi_video_backlight_support); - void __exit acpi_video_detect_exit(void) { if (backlight_notifier_registered) diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 913a1c1..948c8a4 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -244,25 +244,6 @@ extern bool wmi_has_guid(const char *guid); #define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VIDEO 0x0800
extern long acpi_is_video_device(acpi_handle handle); - -#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) - -extern void acpi_video_dmi_promote_vendor(void); -extern int acpi_video_backlight_support(void); - -#else - -static inline void acpi_video_dmi_promote_vendor(void) -{ -} - -static inline int acpi_video_backlight_support(void) -{ - return 0; -} - -#endif /* defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) */ - extern int acpi_blacklisted(void); extern void acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d); extern void acpi_osi_setup(char *str);
acpi_video_unregister_backlight() is now only used by video_detect.c which is part of the same acpi_video module as video.c, make acpi_video_unregister_backlight() private to this module.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- drivers/acpi/video.c | 1 - drivers/acpi/video_detect.c | 2 ++ include/acpi/video.h | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c index fcc7c2e..79062ff 100644 --- a/drivers/acpi/video.c +++ b/drivers/acpi/video.c @@ -1988,7 +1988,6 @@ void acpi_video_unregister_backlight(void) } mutex_unlock(®ister_count_mutex); } -EXPORT_SYMBOL(acpi_video_unregister_backlight);
/* * This is kind of nasty. Hardware using Intel chipsets may require diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index d5e6f1c..001d8c3 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -36,6 +36,8 @@ ACPI_MODULE_NAME("video"); #define _COMPONENT ACPI_VIDEO_COMPONENT
+void acpi_video_unregister_backlight(void); + static bool backlight_notifier_registered; static struct notifier_block backlight_nb;
diff --git a/include/acpi/video.h b/include/acpi/video.h index 47c7ad6..a7d7f10 100644 --- a/include/acpi/video.h +++ b/include/acpi/video.h @@ -27,7 +27,6 @@ enum acpi_backlight_type { #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE) extern int acpi_video_register(void); extern void acpi_video_unregister(void); -extern void acpi_video_unregister_backlight(void); extern int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, void **edid); extern enum acpi_backlight_type acpi_video_get_backlight_type(void); @@ -35,7 +34,6 @@ extern void acpi_video_set_dmi_backlight_type(enum acpi_backlight_type type); #else static inline int acpi_video_register(void) { return 0; } static inline void acpi_video_unregister(void) { return; } -static inline void acpi_video_unregister_backlight(void) { return; } static inline int acpi_video_get_edid(struct acpi_device *device, int type, int device_id, void **edid) {
dri-devel@lists.freedesktop.org
-
Aaron Lu -
Hans de Goede -
Henrique de Moraes Holschuh -
Jani Nikula -
joeyli -
Jonathan Woithe -
Mattia Dongili -
Pali Rohár