[PATCH 00/32] acpi-video: Rewrite backlight interface selection logic

[PATCH 00/32] acpi-video: Rewrite backlight interface selection logic

[Hans de Goede]

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

[PATCH 01/32] apple-gmux: Stop using acpi_video_dmi_demote_vendor()

[Hans de Goede]

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();
 }
-- 
2.4.2

[PATCH 02/32] acpi-video-detect: Remove the unused acpi_video_dmi_demote_vendor() function

[Hans de Goede]

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;
-- 
2.4.2

[PATCH 03/32] acpi-video-detect: Make acpi_video_get_capabilities a private function

[Hans de Goede]

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);
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/32] acpi-video-detect: Move acpi_is_video_device() to acpi/scan.c

[Hans de Goede]

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)
 {
 }
-- 
2.4.2

[PATCH 05/32] acpi-video-detect: Move acpi_osi_is_win8 to osl.c

[Hans de Goede]

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);
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/32] acpi-video-detect: video: Make video_detect code part of the video module

[Hans de Goede]

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
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Hans de Goede]

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
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Aaron Lu]

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;
>  	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
> -- 
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Hans de Goede]

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;
>>   	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
>> --
>> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Aaron Lu]

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:
> 
> 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.

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;
>>>   	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
>>> --
>>> 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

Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Pali Rohár]

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.

-- 
Pali Rohár
pali.rohar@gmail.com
--
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

Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Hans de Goede]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Pali Rohár]

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

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Jani Nikula]

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

-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 07/32] acpi-video-detect: Rewrite backlight interface selection logic

[Hans de Goede]

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

------------------------------------------------------------------------------
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

[PATCH 08/32] acpi-video-detect: Unregister acpi_video backlight when dmi quirks are added

[Hans de Goede]

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);
 
-- 
2.4.2

[PATCH 09/32] drm: i915: Port to new backlight interface selection API

[Hans de Goede]

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;
 	}
-- 
2.4.2

[PATCH 10/32] acpi-video: Port to new backlight interface selection API

[Hans de Goede]

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;
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/32] acpi-video: Move backlight notifier to video_detect.c

[Hans de Goede]

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);
+}
-- 
2.4.2

[PATCH 12/32] acpi-video: Move dmi_check_system from module_init to acpi_video_register

[Hans de Goede]

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;
 
-- 
2.4.2

[PATCH 13/32] acpi-video: Fix acpi_video _register vs _unregister_backlight race

[Hans de Goede]

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(&register_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(&register_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(&register_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(&register_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(&register_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(&register_count_mutex);
 }
 EXPORT_SYMBOL(acpi_video_unregister_backlight);
 
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/32] acer-wmi: Port to new backlight interface selection API

[Hans de Goede]

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) {
-- 
2.4.2

Re: [PATCH 14/32] acer-wmi: Port to new backlight interface selection API

[joeyli]

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
> 

[PATCH 15/32] apple-gmux: Port to new backlight interface selection API

[Hans de Goede]

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;
-- 
2.4.2

[PATCH 16/32] asus-laptop: Port to new backlight interface selection API

[Hans de Goede]

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)
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 17/32] asus-wmi: Port to new backlight interface selection API

[Hans de Goede]

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);
-- 
2.4.2

[PATCH 18/32] compal-laptop: Port to new backlight interface selection API

[Hans de Goede]

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;
-- 
2.4.2

[PATCH 19/32] dell-laptop: Port to new backlight interface selection API

[Hans de Goede]

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
 
-- 
2.4.2

Re: [PATCH 19/32] dell-laptop: Port to new backlight interface selection API

[Pali Rohár]

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?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH 19/32] dell-laptop: Port to new backlight interface selection API

[Hans de Goede]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 19/32] dell-laptop: Port to new backlight interface selection API

[Pali Rohár]

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:
> 
> 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

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.

-- 
Pali Rohár
pali.rohar@gmail.com

[PATCH 20/32] dell-wmi: Port to new backlight interface selection API

[Hans de Goede]

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)
-- 
2.4.2

Re: [PATCH 20/32] dell-wmi: Port to new backlight interface selection API

[Pali Rohár]

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.

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH 20/32] dell-wmi: Port to new backlight interface selection API

[Hans de Goede]

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

Re: [PATCH 20/32] dell-wmi: Port to new backlight interface selection API

[Pali Rohár]

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.

-- 
Pali Rohár
pali.rohar@gmail.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 21/32] eeepc-laptop: Port to new backlight interface selection API

[Hans de Goede]

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);
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 22/32] fujitsu-laptop: Port to new backlight interface selection API

[Hans de Goede]

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
-- 
2.4.2

Re: [PATCH 22/32] fujitsu-laptop: Port to new backlight interface selection API

[Jonathan Woithe]

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
> 
> --

[PATCH 23/32] ideapad-laptop: Port to new backlight interface selection API

[Hans de Goede]

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;
-- 
2.4.2

[PATCH 24/32] intel-oaktrail: Port to new backlight interface selection API

[Hans de Goede]

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) {
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 25/32] msi-laptop: Port to new backlight interface selection API

[Hans de Goede]

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;
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 26/32] msi-wmi: Port to new backlight interface selection API

[Hans de Goede]

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");
-- 
2.4.2

[PATCH 27/32] samsung-laptop: Port to new backlight interface selection API

[Hans de Goede]

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;
-- 
2.4.2

[PATCH 28/32] sony-laptop: Port to new backlight interface selection API

[Hans de Goede]

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) {
-- 
2.4.2

Re: [PATCH 28/32] sony-laptop: Port to new backlight interface selection API

[Mattia Dongili]

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
> 
-- 
mattia
:wq!

Re: [PATCH 28/32] sony-laptop: Port to new backlight interface selection API

[Hans de Goede]

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

[PATCH 29/32] thinkpad-acpi: Port to new backlight interface selection API

[Hans de Goede]

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");
-- 
2.4.2

Re: [PATCH 29/32] thinkpad-acpi: Port to new backlight interface selection API

[Henrique de Moraes Holschuh]

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) {
>  		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");
> -- 
> 2.4.2

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 30/32] toshiba-acpi: Port to new backlight interface selection API

[Hans de Goede]

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)
-- 
2.4.2

[PATCH 31/32] acpi-video-detect: Remove old API

[Hans de Goede]

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);
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 32/32] acpi-video: Make acpi_video_unregister_backlight() private

[Hans de Goede]

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(&register_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)
 {
-- 
2.4.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel