[PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Thomas Renninger]

Zhang: You showed me once, how IGD detection could be enhance, but I
didn't find it anymore...
---

Check for backlight support via ACPI video.ko otherwise use vendor ACPI 
drivers

If an ACPI graphics device supports backlight brightness functions (cmp. with
latest ACPI spec Appendix B), let the ACPI video driver control backlight and
switch backlight control off in vendor specific ACPI drivers (asus_acpi,
thinkpad_acpi, eeepc, fujitsu_laptop, msi_laptop, sony_laptop, acer-wmi).
An exception is if we find an IGD device. Linux does not have a driver
for this yet. Then do not use video.ko, but allow vendor specific acpi
drivers to switch in the legacy way.

Currently it is possible to load above drivers and let both poke on the
brightness HW registers, the video and vendor specific ACPI drivers.

acpi_vendor=backlight forces video.ko to keep its fingers off backlight 
control
                      even it would find needed functions. The corresponding
                      vendor specific driver be used then.

Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 Documentation/kernel-parameters.txt |    7 +
 drivers/acpi/Makefile               |    5 
 drivers/acpi/pci_root.c             |    6 +
 drivers/acpi/scan.c                 |   32 -----
 drivers/acpi/video.c                |   28 ++---
 drivers/acpi/video_detect.c         |  199 
++++++++++++++++++++++++++++++++++++
 drivers/misc/acer-wmi.c             |    6 +
 drivers/misc/asus-laptop.c          |   10 +
 drivers/misc/eeepc-laptop.c         |   12 +-
 drivers/misc/fujitsu-laptop.c       |   21 +++
 drivers/misc/msi-laptop.c           |   16 +-
 drivers/misc/sony-laptop.c          |    6 -
 drivers/misc/thinkpad_acpi.c        |   66 +++++++++--
 include/linux/acpi.h                |   39 +++++++
 14 files changed, 382 insertions(+), 71 deletions(-)

Index: linux-2.6.25/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.25.orig/Documentation/kernel-parameters.txt
+++ linux-2.6.25/Documentation/kernel-parameters.txt
@@ -196,6 +196,13 @@ and is between 256 and 4096 characters. 
 			that require a timer override, but don't have
 			HPET
 
+	acpi_vendor=	[HW,ACPI]
+			acpi_vendor=backlight
+			acpi_vendor=display_output
+			Tries to force the use of a vendor specific ACPI
+			driver for backlight and display_output
+			switching. You can add both, separated by comma.
+
 	acpi.debug_layer=	[HW,ACPI]
 			Format: <int>
 			Each bit of the <int> indicates an ACPI debug layer,
Index: linux-2.6.25/drivers/acpi/Makefile
===================================================================
--- linux-2.6.25.orig/drivers/acpi/Makefile
+++ linux-2.6.25/drivers/acpi/Makefile
@@ -46,7 +46,12 @@ obj-$(CONFIG_ACPI_BUTTON)	+= button.o
 obj-$(CONFIG_ACPI_FAN)		+= fan.o
 obj-$(CONFIG_ACPI_DOCK)		+= dock.o
 obj-$(CONFIG_ACPI_BAY)		+= bay.o
+
 obj-$(CONFIG_ACPI_VIDEO)	+= video.o
+ifdef CONFIG_ACPI_VIDEO
+obj-y				+= video_detect.o
+endif
+
 obj-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
 obj-$(CONFIG_ACPI_POWER)	+= power.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
Index: linux-2.6.25/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.25.orig/drivers/acpi/pci_root.c
+++ linux-2.6.25/drivers/acpi/pci_root.c
@@ -388,6 +388,12 @@ static int __init acpi_pci_root_init(voi
 	if (acpi_bus_register_driver(&acpi_pci_root_driver) < 0)
 		return -ENODEV;
 
+	/* We must check whether the ACPI graphics device is physically plugged
+	 * in. Therefore this must be called after binding PCI and ACPI devices,
+	 * but before modules are loaded, so that we know which module should
+	 * be responsible depending on what the BIOS provides us.
+	 */
+	acpi_video_get_capabilities();
 	return 0;
 }
 
Index: linux-2.6.25/drivers/acpi/scan.c
===================================================================
--- linux-2.6.25.orig/drivers/acpi/scan.c
+++ linux-2.6.25/drivers/acpi/scan.c
@@ -919,36 +919,6 @@ static void acpi_device_get_busid(struct
 	}
 }
 
-static int
-acpi_video_bus_match(struct acpi_device *device)
-{
-	acpi_handle h_dummy;
-
-	if (!device)
-		return -EINVAL;
-
-	/* Since there is no HID, CID for ACPI Video drivers, we have
-	 * to check well known required nodes for each feature we support.
-	 */
-
-	/* Does this device able to support video switching ? */
-	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) &&
-	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOS", &h_dummy)))
-		return 0;
-
-	/* Does this device able to retrieve a video ROM ? */
-	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_ROM", &h_dummy)))
-		return 0;
-
-	/* Does this device able to configure which video head to be POSTed ? */
-	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_VPO", &h_dummy)) &&
-	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_GPD", &h_dummy)) &&
-	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_SPD", &h_dummy)))
-		return 0;
-
-	return -ENODEV;
-}
-
 /*
  * acpi_bay_match - see if a device is an ejectable driver bay
  *
@@ -1031,7 +1001,7 @@ static void acpi_device_set_id(struct ac
 		   will get autoloaded and the device might still match
 		   against another driver.
 		*/
-		if (ACPI_SUCCESS(acpi_video_bus_match(device)))
+		if (acpi_is_video_device(device))
 			cid_add = ACPI_VIDEO_HID;
 		else if (ACPI_SUCCESS(acpi_bay_match(device)))
 			cid_add = ACPI_BAY_HID;
Index: linux-2.6.25/drivers/misc/acer-wmi.c
===================================================================
--- linux-2.6.25.orig/drivers/misc/acer-wmi.c
+++ linux-2.6.25/drivers/misc/acer-wmi.c
@@ -1100,6 +1100,12 @@ static int __init acer_wmi_init(void)
 		return -ENODEV;
 	}
 
+	if (!acpi_video_backlight_support() && has_cap(ACER_CAP_BRIGHTNESS)) {
+		interface->capability &= ~ACER_CAP_BRIGHTNESS;
+		printk(ACER_INFO "Brightness must be controlled by "
+		       "generic video driver\n");
+	}
+
 	if (platform_driver_register(&acer_platform_driver)) {
 		printk(ACER_ERR "Unable to register platform driver.\n");
 		goto error_platform_register;
Index: linux-2.6.25/drivers/misc/asus-laptop.c
===================================================================
--- linux-2.6.25.orig/drivers/misc/asus-laptop.c
+++ linux-2.6.25/drivers/misc/asus-laptop.c
@@ -1207,9 +1207,13 @@ static int __init asus_laptop_init(void)
 
 	dev = acpi_get_physical_device(hotk->device->handle);
 
-	result = asus_backlight_init(dev);
-	if (result)
-		goto fail_backlight;
+	if (!acpi_video_backlight_support()) {
+		result = asus_backlight_init(dev);
+		if (result)
+			goto fail_backlight;
+	} else
+		printk(ASUS_INFO "Brightness ignored, must be controlled by "
+		       "ACPI video driver\n");
 
 	result = asus_led_init(dev);
 	if (result)
Index: linux-2.6.25/drivers/misc/eeepc-laptop.c
===================================================================
--- linux-2.6.25.orig/drivers/misc/eeepc-laptop.c
+++ linux-2.6.25/drivers/misc/eeepc-laptop.c
@@ -625,9 +625,15 @@ static int __init eeepc_laptop_init(void
 		return -ENODEV;
 	}
 	dev = acpi_get_physical_device(ehotk->device->handle);
-	result = eeepc_backlight_init(dev);
-	if (result)
-		goto fail_backlight;
+
+	if (!acpi_video_backlight_support()) {
+		result = eeepc_backlight_init(dev);
+		if (result)
+			goto fail_backlight;
+	} else
+		printk(EEEPC_INFO "Backlight controlled by ACPI video "
+		       "driver\n");
+
 	result = eeepc_hwmon_init(dev);
 	if (result)
 		goto fail_hwmon;
Index: linux-2.6.25/drivers/misc/fujitsu-laptop.c
===================================================================
--- linux-2.6.25.orig/drivers/misc/fujitsu-laptop.c
+++ linux-2.6.25/drivers/misc/fujitsu-laptop.c
@@ -279,7 +279,20 @@ static int __init fujitsu_init(void)
 	if (IS_ERR(fujitsu->bl_device))
 		return PTR_ERR(fujitsu->bl_device);
 
-	fujitsu->bl_device->props.max_brightness = FUJITSU_LCD_N_LEVELS - 1;
+	if (acpi_video_backlight_support()) {
+		printk(KERN_INFO "Brightness ignored, must be controlled by "
+		       "ACPI video driver\n");
+	} else {
+		fujitsu->bl_device =
+			backlight_device_register("fujitsu-laptop", NULL, NULL,
+						  &fujitsubl_ops);
+		if (IS_ERR(fujitsu->bl_device))
+			return PTR_ERR(fujitsu->bl_device);
+
+		fujitsu->bl_device->props.max_brightness =
+			FUJITSU_LCD_N_LEVELS - 1;
+	}
+
 	ret = platform_driver_register(&fujitsupf_driver);
 	if (ret)
 		goto fail_backlight;
@@ -321,7 +334,8 @@ static int __init fujitsu_init(void)
 
       fail_backlight:
 
-	backlight_device_unregister(fujitsu->bl_device);
+	if (fujitsu->bl_device)
+		backlight_device_unregister(fujitsu->bl_device);
 
       fail_acpi:
 
@@ -336,7 +350,8 @@ static void __exit fujitsu_cleanup(void)
 			   &fujitsupf_attribute_group);
 	platform_device_unregister(fujitsu->pf_device);
 	platform_driver_unregister(&fujitsupf_driver);
-	backlight_device_unregister(fujitsu->bl_device);
+	if (fujitsu->bl_device)
+		backlight_device_unregister(fujitsu->bl_device);
 
 	acpi_bus_unregister_driver(&acpi_fujitsu_driver);
 
Index: linux-2.6.25/drivers/misc/msi-laptop.c
===================================================================
--- linux-2.6.25.orig/drivers/misc/msi-laptop.c
+++ linux-2.6.25/drivers/misc/msi-laptop.c
@@ -347,12 +347,16 @@ static int __init msi_init(void)
 
 	/* Register backlight stuff */
 
-	msibl_device = backlight_device_register("msi-laptop-bl", NULL, NULL,
-						&msibl_ops);
-	if (IS_ERR(msibl_device))
-		return PTR_ERR(msibl_device);
-
-	msibl_device->props.max_brightness = MSI_LCD_LEVEL_MAX-1;
+	if (acpi_video_backlight_support()) {
+		printk(KERN_INFO "MSI: Brightness ignored, must be controlled "
+		       "by ACPI video driver\n");
+	} else {
+		msibl_device = backlight_device_register("msi-laptop-bl", NULL,
+							 NULL, &msibl_ops);
+		if (IS_ERR(msibl_device))
+			return PTR_ERR(msibl_device);
+		msibl_device->props.max_brightness = MSI_LCD_LEVEL_MAX-1;
+	}
 
 	ret = platform_driver_register(&msipf_driver);
 	if (ret)
Index: linux-2.6.25/drivers/misc/sony-laptop.c
===================================================================
--- linux-2.6.25.orig/drivers/misc/sony-laptop.c
+++ linux-2.6.25/drivers/misc/sony-laptop.c
@@ -1037,7 +1037,11 @@ static int sony_nc_add(struct acpi_devic
 		goto outinput;
 	}
 
-	if (ACPI_SUCCESS(acpi_get_handle(sony_nc_acpi_handle, "GBRT", &handle))) {
+	if (!acpi_video_backlight_support()) {
+		printk(KERN_INFO DRV_PFX "Sony: Brightness ignored, must be "
+		       "controlled by ACPI video driver\n");
+	} else if (ACPI_SUCCESS(acpi_get_handle(sony_nc_acpi_handle, "GBRT",
+						&handle))) {
 		sony_backlight_device = backlight_device_register("sony", NULL,
 								  NULL,
 								  &sony_backlight_ops);
Index: linux-2.6.25/drivers/misc/thinkpad_acpi.c
===================================================================
--- linux-2.6.25.orig/drivers/misc/thinkpad_acpi.c
+++ linux-2.6.25/drivers/misc/thinkpad_acpi.c
@@ -233,6 +233,7 @@ static struct {
 	u32 light_status:1;
 	u32 bright_16levels:1;
 	u32 bright_acpimode:1;
+	u32 bright_igdmode:1;
 	u32 wan:1;
 	u32 fan_ctrl_status_undef:1;
 	u32 input_device_registered:1;
@@ -2299,6 +2300,9 @@ err_exit:
 	return (res < 0)? res : 1;
 }
 
+static struct backlight_device *ibm_backlight_device;
+static int brightness_update_status(struct backlight_device *bd);
+
 static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 {
 	u32 hkey;
@@ -2337,6 +2341,28 @@ static void hotkey_notify(struct ibm_str
 		case 1:
 			/* 0x1000-0x1FFF: key presses */
 			scancode = hkey & 0xfff;
+			if (tp_features.bright_igdmode) {
+				/* ToDo:
+				 * Is there an already defined key?
+				 */
+				if (hkey == 0x1011) {
+					if (ibm_backlight_device->
+					    props.brightness > 0) {
+						ibm_backlight_device->
+							props.brightness--;
+					}
+				} else if (hkey == 0x1010) {
+					if (ibm_backlight_device->
+					    props.brightness <
+					    ibm_backlight_device->
+					    props.max_brightness) {
+						ibm_backlight_device->
+							props.brightness++;
+					}
+				}
+				brightness_update_status(ibm_backlight_device);
+			}
+
 			if (scancode > 0 && scancode < 0x21) {
 				scancode--;
 				if (!(hotkey_source_mask & (1 << scancode))) {
@@ -4599,7 +4625,6 @@ enum {
 	TP_EC_BACKLIGHT_MAPSW = 0x20,
 };
 
-static struct backlight_device *ibm_backlight_device;
 static int brightness_mode;
 static unsigned int brightness_enable = 2; /* 2 = auto, 0 = no, 1 = yes */
 
@@ -4738,7 +4763,7 @@ static struct backlight_ops ibm_backligh
 static int __init brightness_init(struct ibm_init_struct *iibm)
 {
 	int b;
-
+	long acpi_video_support;
 	vdbg_printk(TPACPI_DBG_INIT, "initializing brightness subdriver\n");
 
 	mutex_init(&brightness_mutex);
@@ -4750,16 +4775,33 @@ static int __init brightness_init(struct
 	 */
 	b = tpacpi_check_std_acpi_brightness_support();
 	if (b > 0) {
-		if (thinkpad_id.vendor == PCI_VENDOR_ID_LENOVO) {
-			printk(TPACPI_NOTICE
-			       "Lenovo BIOS switched to ACPI backlight "
-			       "control mode\n");
-		}
-		if (brightness_enable > 1) {
-			printk(TPACPI_NOTICE
-			       "standard ACPI backlight interface "
-			       "available, not loading native one...\n");
-			return 1;
+
+		if (acpi_video_backlight_support()) {
+			if (brightness_enable > 1) {
+				printk(TPACPI_NOTICE
+				       "standard ACPI backlight interface "
+				       "available, not loading native one.\n");
+				return 1;
+			} else if (brightness_enable == 1) {
+				printk(TPACPI_NOTICE
+				       "Backlight control force, even standard "
+				       "ACPI backlight interface available\n");
+			}
+		} else {
+			if (brightness_enable > 1) {
+				printk(TPACPI_NOTICE
+				       "Standard, but not supported ACPI "
+				       "backlight interface available,"
+				       " probably an IGD device - handled in"
+				       " legacy way\n");
+			}
+			/* We have an Integrated Graphics Device
+			 * BIOS won't switch, we have to do it ourselves
+			 * as long as no IGD driver exists
+			*/
+			acpi_video_support = acpi_video_get_capabilities();
+			if (acpi_video_support & ACPI_VIDEO_IGD)
+				tp_features.bright_igdmode = 1;
 		}
 	}
 
Index: linux-2.6.25/include/linux/acpi.h
===================================================================
--- linux-2.6.25.orig/include/linux/acpi.h
+++ linux-2.6.25/include/linux/acpi.h
@@ -201,6 +201,45 @@ extern bool wmi_has_guid(const char *gui
 
 #endif	/* CONFIG_ACPI_WMI */
 
+#define ACPI_VIDEO_OUTPUT_SWITCHING			1
+#define ACPI_VIDEO_DEVICE_POSTING			2
+#define ACPI_VIDEO_ROM_AVAILABLE			4
+#define ACPI_VIDEO_BACKLIGHT				8
+#define ACPI_VIDEO_IGD					16
+#define ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR		32
+#define ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VENDOR	64
+
+#if defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE)
+
+extern long acpi_video_get_capabilities(void);
+extern long acpi_is_video_device(struct acpi_device *device);
+extern int acpi_video_backlight_support(void);
+extern int acpi_video_display_switch_support(void);
+
+#else
+
+static inline long acpi_video_get_capabilities(void)
+{
+	return 0;
+}
+
+static inline long acpi_is_video_device(struct acpi_device *device)
+{
+	return 0;
+}
+
+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);
 #ifdef CONFIG_DMI
 extern void acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d);
Index: linux-2.6.25/drivers/acpi/video_detect.c
===================================================================
--- /dev/null
+++ linux-2.6.25/drivers/acpi/video_detect.c
@@ -0,0 +1,199 @@
+/*
+ * 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
+ * assinged
+ *
+ * After PCI devices are glued with ACPI devices
+ * acpi_get_physical_pci_device() 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.
+ *
+ * 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_acpi,
+ * sony_acpi,... can take care about backlight brightness and display output
+ * switching.
+ *
+ * Copyright 2008   Thomas Renninger <trenn@suse.de>
+ */
+
+/* If video.ko is not selected, we do not need to protect vendor acpi drivers 
*/
+
+#include <linux/acpi.h>
+
+ACPI_MODULE_NAME("video");
+#define ACPI_VIDEO_COMPONENT		0x08000000
+#define _COMPONENT		ACPI_VIDEO_COMPONENT
+
+/* video/backlight support */
+#define ACPI_VIDEO_OUTPUT_SWITCHING			1
+#define ACPI_VIDEO_DEVICE_POSTING			2
+#define ACPI_VIDEO_ROM_AVAILABLE			4
+#define ACPI_VIDEO_BACKLIGHT				8
+#define ACPI_VIDEO_IGD					16
+#define ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR		32
+#define ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VENDOR	64
+
+static long acpi_video_support = -1;
+
+static acpi_status
+acpi_backlight_cap_match(acpi_handle handle, u32 level, void *context,
+			  void **retyurn_value)
+{
+	long *cap = context;
+	acpi_handle h_dummy;
+
+	if (ACPI_SUCCESS(acpi_get_handle(handle, "_BCM", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(handle, "_BCL", &h_dummy))) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found generic backlight "
+				  "support\n"));
+		*cap |= ACPI_VIDEO_BACKLIGHT;
+		return 0;
+	}
+	return -ENODEV;
+}
+
+/* Returns true if the device 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(struct acpi_device *device)
+{
+	acpi_handle h_dummy;
+	long video_caps = 0;
+
+	if (!device)
+		return 0;
+
+	/* Since there is no HID, CID for ACPI Video drivers, we have
+	 * to check well known required nodes for each feature we support.
+	 */
+
+	/* Does this device able to support video switching ? */
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOD", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_DOS", &h_dummy)))
+		video_caps |= ACPI_VIDEO_OUTPUT_SWITCHING;
+
+	/* Does this device able to retrieve a video ROM ? */
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_ROM", &h_dummy)))
+		video_caps |= ACPI_VIDEO_ROM_AVAILABLE;
+
+	/* Does this device able to configure which video head to be POSTed ? */
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "_VPO", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_GPD", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "_SPD", &h_dummy)))
+		video_caps |= ACPI_VIDEO_DEVICE_POSTING;
+
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "DRDY", &h_dummy))) {
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found IGD device\n"));
+		video_caps |= ACPI_VIDEO_IGD;
+	}
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, device->handle, ACPI_UINT32_MAX,
+			    acpi_backlight_cap_match, &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;
+	struct device *dev;
+	struct acpi_device *acpi_dev;
+
+	const struct acpi_device_id video_ids[] = {
+		{ACPI_VIDEO_HID, 0},
+		{"", 0},
+	};
+	acpi_bus_get_device(handle, &acpi_dev);
+
+	if (!acpi_match_device_ids(acpi_dev, video_ids)) {
+		dev = acpi_get_physical_pci_device(handle);
+		if (!dev)
+			return AE_OK;
+		put_device(dev);
+		*cap |= acpi_is_video_device(acpi_dev);
+		printk(KERN_INFO "We have 0x%lX vid support\n", *cap);
+	}
+	return AE_OK;
+}
+
+long acpi_video_get_capabilities(void)
+{
+	/* already detected video capabilities as long as grahpics cards
+	 * hotplugging is not an issue check once should be ok.
+	 */
+	if (acpi_video_support >= 0)
+		return acpi_video_support;
+	acpi_video_support = 0;
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX, find_video, &acpi_video_support, NULL);
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support\n",
+			  acpi_video_support));
+	return acpi_video_support;
+}
+EXPORT_SYMBOL(acpi_video_get_capabilities);
+
+/* Returns true if video.ko can do backlight switching
+ *
+ * ToDo: Remove the IGD check as soon as there is a working driver
+ * for it available
+ */
+int acpi_video_backlight_support(void)
+{
+
+	if (acpi_video_support & ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR)
+		return 0;
+
+	return (acpi_video_support & ACPI_VIDEO_BACKLIGHT) &&
+		!(acpi_video_support & ACPI_VIDEO_IGD);
+}
+EXPORT_SYMBOL(acpi_video_backlight_support);
+
+/* Returns true if video.ko can do display output switching.
+ * This does not work well/at all with binary graphics drivers
+ * which disable system io ranges and do it on their own.
+ *
+ * It should work well when we have an IGD driver for Intel
+ * graphics cards.
+ */
+int acpi_video_display_switch_support(void)
+{
+
+	if (acpi_video_support & ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VENDOR)
+		return 0;
+	return (acpi_video_support & ACPI_VIDEO_OUTPUT_SWITCHING) &&
+		!(acpi_video_support & ACPI_VIDEO_IGD);
+}
+EXPORT_SYMBOL(acpi_video_display_switch_support);
+
+/* Use acpi_video_vendor=backlight,display_output
+ * To force that backlight or display output switching is processed by vendor
+ * specific acpi drivers instead of the ACPI video.ko driver
+ */
+int __init acpi_vendor(char *str)
+{
+	if (str == NULL || *str == '\0')
+		return 1;
+	else {
+		if (!strcmp("backlight", str))
+			acpi_video_support &=
+				ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR;
+		if (!strcmp("display_output", str))
+			acpi_video_support &=
+				ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VENDOR;
+	}
+	return 1;
+}
+__setup("acpi_vendor=", acpi_vendor);
Index: linux-2.6.25/drivers/acpi/video.c
===================================================================
--- linux-2.6.25.orig/drivers/acpi/video.c
+++ linux-2.6.25/drivers/acpi/video.c
@@ -739,7 +739,8 @@ static void acpi_video_device_find_cap(s
 		device->cap._DSS = 1;
 	}
 
-	max_level = acpi_video_init_brightness(device);
+	if (acpi_video_backlight_support())
+		max_level = acpi_video_init_brightness(device);
 
 	if (device->cap._BCL && device->cap._BCM && device->cap._BQC && max_level > 
0){
 		int result;
@@ -776,18 +777,21 @@ static void acpi_video_device_find_cap(s
 			printk(KERN_ERR PREFIX "Create sysfs link\n");
 
 	}
-	if (device->cap._DCS && device->cap._DSS){
-		static int count = 0;
-		char *name;
-		name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
-		if (!name)
-			return;
-		sprintf(name, "acpi_video%d", count++);
-		device->output_dev = video_output_register(name,
-				NULL, device, &acpi_output_properties);
-		kfree(name);
+
+	if (acpi_video_display_switch_support()) {
+
+		if (device->cap._DCS && device->cap._DSS){
+			static int count = 0;
+			char *name;
+			name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
+			if (!name)
+				return;
+			sprintf(name, "acpi_video%d", count++);
+			device->output_dev = video_output_register(name,
+					NULL, device, &acpi_output_properties);
+			kfree(name);
+		}
 	}
-	return;
 }
 
 /*

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Thu, Jul 03, 2008 at 06:08:59PM +0200, Thomas Renninger wrote:

> +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "DRDY", &h_dummy))) {
> +		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found IGD device\n"));
> +		video_caps |= ACPI_VIDEO_IGD;
> +	}

In principle, it's possible for a vendor to implement opregion but not 
actually use it until the tche flags are set. If anyone's done that then 
you've just broken backlight control on their system.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Thu, Jul 03, 2008 at 05:21:52PM +0100, Matthew Garrett wrote:
> On Thu, Jul 03, 2008 at 06:08:59PM +0200, Thomas Renninger wrote:
> 
> > +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "DRDY", &h_dummy))) {
> > +		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found IGD device\n"));
> > +		video_caps |= ACPI_VIDEO_IGD;
> > +	}
> 
> In principle, it's possible for a vendor to implement opregion but not 
> actually use it until the tche flags are set. If anyone's done that then 
> you've just broken backlight control on their system.

I've just had it confirmed that recent Panasonics behave this way. I 
suspect the same's true of many 965 systems.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Thomas Renninger]

On Thursday 03 July 2008 18:21:52 Matthew Garrett wrote:
> On Thu, Jul 03, 2008 at 06:08:59PM +0200, Thomas Renninger wrote:
> > +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "DRDY", &h_dummy))) {
> > +		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found IGD device\n"));
> > +		video_caps |= ACPI_VIDEO_IGD;
> > +	}
>
> In principle, it's possible for a vendor to implement opregion but not
> actually use it until the tche flags are set. If anyone's done that then
> you've just broken backlight control on their system.

Who should set that flags?
The dri driver is going to set that if he can serve IGD parts, right?
I am going to figure out whether your DRI parts, video.ko only or vendor 
specific acpi drivers are going to serve backlight switching.

Currently this patch makes the vendor specific e.g. thinkpad_acpi to serve 
backlight brightness switching if a IGD device is found. Because currently 
there is no IGD support in the kernel.

What is needed is code that runs before module load time and checks for BIOS 
support and then decides whether video.ko or thinkpad_acpi, sony_acpi, 
whatever_can_do_backlight.ko is going to do it. This is in fact what this 
whole patch is about.

You should export a function intel_opregion_init()? so that built in kernel 
parts can check for IGD backlight support, instead of the lines above (part 
of my patch). If this succeeds, my patch will tell vendor specfic drivers to 
keep fingers off backlight switching.

In the dri parts you directly map to the memory instead of using ACPI Field 
names. While a specification might state that the ACPI name should be TCHE, 
it should be checked before module load time the same way as you do it in the 
dri parts.


    Thomas

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Wed, Jul 09, 2008 at 04:49:49PM +0200, Thomas Renninger wrote:
> On Thursday 03 July 2008 18:21:52 Matthew Garrett wrote:
> > On Thu, Jul 03, 2008 at 06:08:59PM +0200, Thomas Renninger wrote:
> > > +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "DRDY", &h_dummy))) {
> > > +		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found IGD device\n"));
> > > +		video_caps |= ACPI_VIDEO_IGD;
> > > +	}
> >
> > In principle, it's possible for a vendor to implement opregion but not
> > actually use it until the tche flags are set. If anyone's done that then
> > you've just broken backlight control on their system.
> 
> Who should set that flags?
> The dri driver is going to set that if he can serve IGD parts, right?

No. The opregion spec requires you to use video.ko. 

> Currently this patch makes the vendor specific e.g. thinkpad_acpi to serve 
> backlight brightness switching if a IGD device is found. Because currently 
> there is no IGD support in the kernel.

Yes, but that's *wrong*. There are many machines that support opregion 
but do not require its use - they'll behave using the old ACPI mechanism 
until you switch them to the new mode. Except right now if you switch 
them to the new mode then they'll stop working, because the DRI code 
isn't merged yet.

> You should export a function intel_opregion_init()? so that built in kernel 
> parts can check for IGD backlight support, instead of the lines above (part 
> of my patch). If this succeeds, my patch will tell vendor specfic drivers to 
> keep fingers off backlight switching.

No. Please read the opregion spec. The way it works is as follows:

1) Firmware exports an ACPI video class
2) OS checks whether the hardware supports the opregion ACPI video 
handling (this is done by looking at tables in a region of memory 
pointed at by a PCI configuration register)
3) OS sets opregion flags
4) OS calls the ACPI backlight set function
5) Firmware writes desired brightness to an opregion mailbox and 
triggers an interrupt
6) OS sets the brightness

For this to work, the ACPI video driver needs to work as it currently 
does. It is not correct for it to disable itself on IGD systems.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Thomas Renninger]

...
> For this to work, the ACPI video driver needs to work as it currently
> does. It is not correct for it to disable itself on IGD systems.
Right, now I got it. It is only the T61/Lenovo you must have the opregion 
driver ready or you better fallback to thinkpad_acpi...
Is that correct?
I will add a dmi check for Lenovo in video_detect.c.
That would be:
  - If ACPI Appendix B video brightness functions are found
  |     -> use video.ko
  |     |
  |     -> If it's a ThinkPad, check for Opregion BIOS support
  |           -> Use video.ko if IGD parts are implemented (and the one
  |           |   ThinkPad specific func you had the 750ms delay is available)
  |           -> otherwise go back to thinkpad_acpi
  |
  ->  Use vendor specific driver

FWIW, the IGD part to switch to vendor specific drivers can be deleted anyway 
if you say the IGD parts are working with your upcoming patches.

This would then be to use video.ko when detecting general brightness functions 
and otherwise let vendor specific drivers do it. No IGD check is necessary at 
all anymore.
The check for IGD was because ThinkPads did not work with video.ko then and 
this blocked the "check for physical graphics device" patch fixing a lot 
others.

Please also provide an OSI string with your video opregion code parts, so that 
BIOS can check whether the kernel is capable of serving IGD devices. Just 
choose a random string you like, e.g. let OSI("Opregion Video Support") 
return true.

    Thomas

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Wed, Jul 09, 2008 at 06:00:23PM +0200, Thomas Renninger wrote:
> ...
> > For this to work, the ACPI video driver needs to work as it currently
> > does. It is not correct for it to disable itself on IGD systems.
> Right, now I got it. It is only the T61/Lenovo you must have the opregion 
> driver ready or you better fallback to thinkpad_acpi...
> Is that correct?
> I will add a dmi check for Lenovo in video_detect.c.
> That would be:
>   - If ACPI Appendix B video brightness functions are found
>   |     -> use video.ko
>   |     |
>   |     -> If it's a ThinkPad, check for Opregion BIOS support
>   |           -> Use video.ko if IGD parts are implemented (and the one
>   |           |   ThinkPad specific func you had the 750ms delay is available)
>   |           -> otherwise go back to thinkpad_acpi

This is unnecessary logic. Let's just follow the spec. There's no need 
to use thinkpad_acpi here.

> Please also provide an OSI string with your video opregion code parts, so that 
> BIOS can check whether the kernel is capable of serving IGD devices. Just 
> choose a random string you like, e.g. let OSI("Opregion Video Support") 
> return true.

No. The DRM can be (and usually is) built as a module and OSI strings 
are going to be checked at ACPI init time. This can't be made to work 
correctly. Vendors can choose whether to use the opregion or old-style 
support based on whether the driver has enabled the support.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Thomas Renninger]

On Thursday 10 July 2008 12:07:09 Matthew Garrett wrote:
> On Wed, Jul 09, 2008 at 06:00:23PM +0200, Thomas Renninger wrote:
> > ...
> >
> > > For this to work, the ACPI video driver needs to work as it currently
> > > does. It is not correct for it to disable itself on IGD systems.
> >
> > Right, now I got it. It is only the T61/Lenovo you must have the opregion
> > driver ready or you better fallback to thinkpad_acpi...
> > Is that correct?
> > I will add a dmi check for Lenovo in video_detect.c.
> > That would be:
> >   - If ACPI Appendix B video brightness functions are found
> >
> >   |     -> use video.ko
> >   |
> >   |     -> If it's a ThinkPad, check for Opregion BIOS support
> >   |           -> Use video.ko if IGD parts are implemented (and the one
> >   |
> >   |           |   ThinkPad specific func you had the 750ms delay is
> >   |           | available)
> >   |
> >   |           -> otherwise go back to thinkpad_acpi
>
> This is unnecessary logic. Let's just follow the spec. There's no need
> to use thinkpad_acpi here.
That is what will be the next version.
But two days ago, there was no IGD working driver and ThinkPads did not work 
with an IGD graphics device and video.ko without your dri extensions. In fact 
this was the "regression" (in fact it never worked) that blocked the "do not 
let the video driver poke on graphics devices for which no graphics card is 
plugged in" (Patch 1/2).

So there was a need for this.
In fact, my next version will still use thinkpad_acpi by default.
But this one will be based on a dmi blacklist. You mentioned ThinkPad BIOSes 
which are missing a specific function and cannot work with IGD? Please adjust 
the blacklist then to match those.

> > Please also provide an OSI string with your video opregion code parts, so
> > that BIOS can check whether the kernel is capable of serving IGD devices.
> > Just choose a random string you like, e.g. let OSI("Opregion Video
> > Support") return true.
>
> No. The DRM can be (and usually is) built as a module and OSI strings
> are going to be checked at ACPI init time. This can't be made to work
> correctly. Vendors can choose whether to use the opregion or old-style
> support based on whether the driver has enabled the support.
This has nothing to do with OS.
Just give vendors/BIOS developers the possibility to check whether the OS is 
capable of Opregion video support.
This is not necessary as long as Linux returns true for Windows strings.
There we cannot differ for  individual OSI features anyway.
But if we do not do it correct now, we will close the door to be spec 
compatible forever.

Thanks,

      Thomas

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Thu, Jul 10, 2008 at 01:00:34PM +0200, Thomas Renninger wrote:
> On Thursday 10 July 2008 12:07:09 Matthew Garrett wrote:
> > This is unnecessary logic. Let's just follow the spec. There's no need
> > to use thinkpad_acpi here.
> That is what will be the next version.
> But two days ago, there was no IGD working driver and ThinkPads did not work 
> with an IGD graphics device and video.ko without your dri extensions. In fact 
> this was the "regression" (in fact it never worked) that blocked the "do not 
> let the video driver poke on graphics devices for which no graphics card is 
> plugged in" (Patch 1/2).

And now there is, so...

> So there was a need for this.
> In fact, my next version will still use thinkpad_acpi by default.
> But this one will be based on a dmi blacklist. You mentioned ThinkPad BIOSes 
> which are missing a specific function and cannot work with IGD? Please adjust 
> the blacklist then to match those.

No I didn't. Please don't do this - we have all the code needed to do it 
properly, so there's no need to use the thinkpad_acpi driver for 
backlight control on this hardware.

> > No. The DRM can be (and usually is) built as a module and OSI strings
> > are going to be checked at ACPI init time. This can't be made to work
> > correctly. Vendors can choose whether to use the opregion or old-style
> > support based on whether the driver has enabled the support.
> This has nothing to do with OS.
> Just give vendors/BIOS developers the possibility to check whether the OS is 
> capable of Opregion video support.

We can't. There's no way of telling whether the OS is capable of 
opregion video support until the drm module has been loaded, and any 
BIOS is going to have done its OSI checks at boot time.

> This is not necessary as long as Linux returns true for Windows strings.
> There we cannot differ for  individual OSI features anyway.
> But if we do not do it correct now, we will close the door to be spec 
> compatible forever.

The spec doesn't require an OSI string for this. Firmware that wants to 
know whether the OS is able to respond to opregion requests should do so 
by checking the specced opregion flags that the OS will set when it 
loads the driver.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Thomas Renninger]

On Thursday 10 July 2008 13:09:41 Matthew Garrett wrote:
> On Thu, Jul 10, 2008 at 01:00:34PM +0200, Thomas Renninger wrote:
> > On Thursday 10 July 2008 12:07:09 Matthew Garrett wrote:
> > > This is unnecessary logic. Let's just follow the spec. There's no need
> > > to use thinkpad_acpi here.
> >
> > That is what will be the next version.
> > But two days ago, there was no IGD working driver and ThinkPads did not
> > work with an IGD graphics device and video.ko without your dri
> > extensions. In fact this was the "regression" (in fact it never worked)
> > that blocked the "do not let the video driver poke on graphics devices
> > for which no graphics card is plugged in" (Patch 1/2).
>
> And now there is, so...
>
> > So there was a need for this.
> > In fact, my next version will still use thinkpad_acpi by default.
> > But this one will be based on a dmi blacklist. You mentioned ThinkPad
> > BIOSes which are missing a specific function and cannot work with IGD?
> > Please adjust the blacklist then to match those.
>
> No I didn't. Please don't do this - we have all the code needed to do it
> properly, so there's no need to use the thinkpad_acpi driver for
> backlight control on this hardware.
Ok.
But haven't you said there are ThinkPad BIOSes missing a specific ACPI part 
and therefore you had the delay?
If you could give me a dmidecode output, I like to add it.
It would be great to have an example in the blacklist, then things are much 
easier for others...

> > > No. The DRM can be (and usually is) built as a module and OSI strings
> > > are going to be checked at ACPI init time. This can't be made to work
> > > correctly. Vendors can choose whether to use the opregion or old-style
> > > support based on whether the driver has enabled the support.
> >
> > This has nothing to do with OS.
> > Just give vendors/BIOS developers the possibility to check whether the OS
> > is capable of Opregion video support.
>
> We can't. There's no way of telling whether the OS is capable of
> opregion video support until the drm module has been loaded, and any
> BIOS is going to have done its OSI checks at boot time.
>
> > This is not necessary as long as Linux returns true for Windows strings.
> > There we cannot differ for  individual OSI features anyway.
> > But if we do not do it correct now, we will close the door to be spec
> > compatible forever.
>
> The spec doesn't require an OSI string for this. Firmware that wants to
> know whether the OS is able to respond to opregion requests should do so
> by checking the specced opregion flags that the OS will set when it
> loads the driver.
Ok.

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Thu, Jul 10, 2008 at 01:15:18PM +0200, Thomas Renninger wrote:
> On Thursday 10 July 2008 13:09:41 Matthew Garrett wrote:
> > No I didn't. Please don't do this - we have all the code needed to do it
> > properly, so there's no need to use the thinkpad_acpi driver for
> > backlight control on this hardware.
> Ok.
> But haven't you said there are ThinkPad BIOSes missing a specific ACPI part 
> and therefore you had the delay?
> If you could give me a dmidecode output, I like to add it.
> It would be great to have an example in the blacklist, then things are much 
> easier for others...

No, I said that on Thinkpads with a PWM method, the PWM method has to be 
called in order to avoid the delay. There's no need for a blacklist.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Thomas Renninger]

On Thursday 10 July 2008 13:19:00 Matthew Garrett wrote:
> On Thu, Jul 10, 2008 at 01:15:18PM +0200, Thomas Renninger wrote:
> > On Thursday 10 July 2008 13:09:41 Matthew Garrett wrote:
> > > No I didn't. Please don't do this - we have all the code needed to do
> > > it properly, so there's no need to use the thinkpad_acpi driver for
> > > backlight control on this hardware.
> >
> > Ok.
> > But haven't you said there are ThinkPad BIOSes missing a specific ACPI
> > part and therefore you had the delay?
> > If you could give me a dmidecode output, I like to add it.
> > It would be great to have an example in the blacklist, then things are
> > much easier for others...
>
> No, I said that on Thinkpads with a PWM method, the PWM method has to be
> called in order to avoid the delay. There's no need for a blacklist.

Ok.
I just got a report that Dells must not use the video driver:
https://bugzilla.novell.com/show_bug.cgi?id=404627

They have a really ugly dcdbas driver which issues SMIs triggered from 
userspace.
Hmm, to do it correctly, I have to match against the alias of the dcdbas 
driver:
MODULE_ALIAS("dmi:*:[bs]vnD[Ee][Ll][Ll]*:*");
but matching for Dell should be enough.

Also not nice is that the video driver probably should not be loaded at all in 
this case.
Hmm, maybe I should check for !video_backlight_support 
&& !video_display_output_support and then do not load the video driver at 
all, it shouldn't have any functionality then, not sure...

   Thomas

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Thu, Jul 10, 2008 at 01:36:04PM +0200, Thomas Renninger wrote:
> On Thursday 10 July 2008 13:19:00 Matthew Garrett wrote:
> > No, I said that on Thinkpads with a PWM method, the PWM method has to be
> > called in order to avoid the delay. There's no need for a blacklist.
> 
> Ok.
> I just got a report that Dells must not use the video driver:
> https://bugzilla.novell.com/show_bug.cgi?id=404627
> 
> They have a really ugly dcdbas driver which issues SMIs triggered from 
> userspace.

If the ACPI video driver has bound, then using the dcdbas mechanism for 
backlight control is incorrect. I wasn't aware that any Dells actually 
implemented that. The correct thing is for userspace to stop using 
dcdbas if a real backlight control is available, not to cripple the 
ACPI video driver.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Thomas Renninger]

On Thursday 10 July 2008 13:53:17 Matthew Garrett wrote:
> On Thu, Jul 10, 2008 at 01:36:04PM +0200, Thomas Renninger wrote:
> > On Thursday 10 July 2008 13:19:00 Matthew Garrett wrote:
> > > No, I said that on Thinkpads with a PWM method, the PWM method has to
> > > be called in order to avoid the delay. There's no need for a blacklist.
> >
> > Ok.
> > I just got a report that Dells must not use the video driver:
> > https://bugzilla.novell.com/show_bug.cgi?id=404627
> >
> > They have a really ugly dcdbas driver which issues SMIs triggered from
> > userspace.
>
> If the ACPI video driver has bound, then using the dcdbas mechanism for
> backlight control is incorrect. I wasn't aware that any Dells actually
> implemented that. The correct thing is for userspace to stop using
> dcdbas if a real backlight control is available, not to cripple the
> ACPI video driver.

Yes, I agree.
Anyway, given the fact that video.ko was rather broken all the time, a 
reasonable solution for now is to exclude Dells from using it.

The dcdbas driver writers (I expect Dell itself has its finger on this one) 
should get informed and solution will be found. video_detect.c probably will 
help here.

        Thomas

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Thu, Jul 10, 2008 at 02:19:04PM +0200, Thomas Renninger wrote:
> On Thursday 10 July 2008 13:53:17 Matthew Garrett wrote:
> > If the ACPI video driver has bound, then using the dcdbas mechanism for
> > backlight control is incorrect. I wasn't aware that any Dells actually
> > implemented that. The correct thing is for userspace to stop using
> > dcdbas if a real backlight control is available, not to cripple the
> > ACPI video driver.
> 
> Yes, I agree.
> Anyway, given the fact that video.ko was rather broken all the time, a 
> reasonable solution for now is to exclude Dells from using it.

No, the problem was that the backlight was simultaneously being altered 
by two pieces of code. kpowersave is doing the backlight control via hal 
(I assume), and hal should simply not provide the Dell backlight control 
on systems that have ACPI video backlight control. There's no need to 
have this policy in the kernel.

> The dcdbas driver writers (I expect Dell itself has its finger on this one) 
> should get informed and solution will be found. video_detect.c probably will 
> help here.

It's a userspace problem and a straightforward userspace solution.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Thomas Renninger]

On Thursday 10 July 2008 14:24:54 Matthew Garrett wrote:
> On Thu, Jul 10, 2008 at 02:19:04PM +0200, Thomas Renninger wrote:
> > On Thursday 10 July 2008 13:53:17 Matthew Garrett wrote:
> > > If the ACPI video driver has bound, then using the dcdbas mechanism for
> > > backlight control is incorrect. I wasn't aware that any Dells actually
> > > implemented that. The correct thing is for userspace to stop using
> > > dcdbas if a real backlight control is available, not to cripple the
> > > ACPI video driver.
> >
> > Yes, I agree.
> > Anyway, given the fact that video.ko was rather broken all the time, a
> > reasonable solution for now is to exclude Dells from using it.
>
> No, the problem was that the backlight was simultaneously being altered
> by two pieces of code. kpowersave is doing the backlight control via hal
> (I assume), and hal should simply not provide the Dell backlight control
> on systems that have ACPI video backlight control. There's no need to
> have this policy in the kernel.
Do you know Dells working with the video.ko driver?

If you tell me video.ko, best with an IGD device and without one is working 
there, it can be removed. Even then talking with dcdbas developers how to 
inform their user space app first is a good idea.

As Dell is cooking their own soup here and the dcdbas driver was reported to 
work correctly with ACPI brightness functions in BIOS it is ok to blacklist 
Dells here until the first test reports are coming in telling us that 
video.ko is actually working correctly there.
Testing will be easy via boot param.

I will not risk again that this whole bunch of our patches will be reverted 
on -rc6 again, because a Dell user is reporting a backlight regression.
We then have the same situation we had when the "check for physical device was 
removed": The implementation is wrong but worked. The implementation is 
right, but does not work on a specific machine -> regression -> revert.

      Thomas

Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett]

On Thu, Jul 10, 2008 at 02:48:12PM +0200, Thomas Renninger wrote:
> On Thursday 10 July 2008 14:24:54 Matthew Garrett wrote:
> > On Thu, Jul 10, 2008 at 02:19:04PM +0200, Thomas Renninger wrote:
> > > Yes, I agree.
> > > Anyway, given the fact that video.ko was rather broken all the time, a
> > > reasonable solution for now is to exclude Dells from using it.
> >
> > No, the problem was that the backlight was simultaneously being altered
> > by two pieces of code. kpowersave is doing the backlight control via hal
> > (I assume), and hal should simply not provide the Dell backlight control
> > on systems that have ACPI video backlight control. There's no need to
> > have this policy in the kernel.
> Do you know Dells working with the video.ko driver?

Presumably the Inspiron 640M, since otherwise there's no way that that 
bug could trigger.

> If you tell me video.ko, best with an IGD device and without one is working 
> there, it can be removed. Even then talking with dcdbas developers how to 
> inform their user space app first is a good idea.

The userspace app that needs fixing is hal. I'm already working on that.

> As Dell is cooking their own soup here and the dcdbas driver was reported to 
> work correctly with ACPI brightness functions in BIOS it is ok to blacklist 
> Dells here until the first test reports are coming in telling us that 
> video.ko is actually working correctly there.
> Testing will be easy via boot param.

No, really, let's just fix the problem properly.

> I will not risk again that this whole bunch of our patches will be reverted 
> on -rc6 again, because a Dell user is reporting a backlight regression.
> We then have the same situation we had when the "check for physical device was 
> removed": The implementation is wrong but worked. The implementation is 
> right, but does not work on a specific machine -> regression -> revert.

I've no idea at all why you think this is a kernel issue.

-- 
Matthew Garrett | mjg59@srcf.ucam.org