[PATCH 1/9] Check for ACPI backlight support otherwise use vendor ACPI drivers

[PATCH 1/9] Check for ACPI backlight support otherwise use vendor ACPI drivers

[trenn]

From: Thomas Renninger <trenn@suse.de>

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

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

This patch provides the basic support to check for BIOS capabilities before
driver loading time. Driver specific modifications are in separate follow up
patches.

acpi_backlight=vendor/video
      boot params forces video.ko or vendor specific drivers 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 |   13 ++
 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         |  250 +++++++++++++++++++++++++++++++++++
 include/linux/acpi.h                |   45 +++++++
 7 files changed, 336 insertions(+), 43 deletions(-)
 create mode 100644 drivers/acpi/video_detect.c

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 6acfe8e..56392f8 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -191,6 +191,19 @@ and is between 256 and 4096 characters. It is defined in the file
 			that require a timer override, but don't have
 			HPET
 
+	acpi_backlight=	[HW,ACPI]
+			acpi_backlight=vendor
+			acpi_backlight=video
+			If set to vendor, it enforces the use of a
+			vendor specific ACPI driver for backlight switching
+			(e.g. thinkpad_acpi, sony_acpi, etc.) instead
+			of the video.ko driver.
+
+	acpi_display_output=	[HW,ACPI]
+			acpi_display_output=vendor
+			acpi_display_output=video
+			See above.
+
 	acpi.debug_layer=	[HW,ACPI]
 			Format: <int>
 			Each bit of the <int> indicates an ACPI debug layer,
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 4efbe59..d91dc80 100644
--- a/drivers/acpi/Makefile
+++ b/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
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index c3fed31..0a6d5e0 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -388,6 +388,12 @@ static int __init acpi_pci_root_init(void)
 	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(NULL);
 	return 0;
 }
 
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5b049cd..fb8e2df 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -931,36 +931,6 @@ static void acpi_device_get_busid(struct acpi_device *device,
 	}
 }
 
-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
  *
@@ -1043,7 +1013,7 @@ static void acpi_device_set_id(struct acpi_device *device,
 		   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;
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 767d9b9..5949278 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -739,7 +739,8 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
 		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(struct acpi_video_device *device)
 			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;
+			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;
 }
 
 /*
diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
new file mode 100644
index 0000000..ac006d2
--- /dev/null
+++ b/drivers/acpi/video_detect.c
@@ -0,0 +1,250 @@
+/*
+ * 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>
+#include <linux/dmi.h>
+
+ACPI_MODULE_NAME("video");
+#define ACPI_VIDEO_COMPONENT		0x08000000
+#define _COMPONENT		ACPI_VIDEO_COMPONENT
+
+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;
+
+	/* IGD detection is not perfect. It should use the same method as done
+	 * to identify an IGD device in the dri parts or video.ko
+	*/
+	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},
+	};
+	if (acpi_bus_get_device(handle, &acpi_dev))
+		return AE_OK;
+
+	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;
+}
+
+/* Returns the video capabilities of a specific ACPI graphics device
+ *
+ * if NULL is passed as argument all ACPI devices are enumerated and
+ * all graphics capabilities of physically present devices are
+ * summerized and returned. This is cached and done only once.
+ */
+long acpi_video_get_capabilities(acpi_handle graphics_dev_handle)
+{
+	/* already detected video capabilities as long as grahpics cards
+	 * hotplugging is not an issue checking once is ok.
+	 */
+	if (acpi_video_support >= 0 && graphics_dev_handle == NULL)
+		return acpi_video_support;
+
+	acpi_video_support = 0;
+
+	if (!graphics_dev_handle) {
+		acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+				    ACPI_UINT32_MAX, find_video,
+				    &acpi_video_support, NULL);
+		/* Dells have their own way: they trigger DMI via userspace
+		 * and dcdbas driver
+		 *
+		 * Add more dmi blacklisting here if necessary. Use _DMI_ as
+		 * done below to ensure boot params override this
+		 */
+		if (dmi_name_in_vendors("Dell")) {
+			acpi_video_support &=
+				ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR;
+			acpi_video_support &=
+				ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
+		}
+	} else
+		acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_dev_handle,
+				    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
+ *
+ */
+int acpi_video_backlight_support(void)
+{
+
+	/* 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;
+}
+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;
+	else if (acpi_video_support & ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VIDEO)
+		return 1;
+
+	if (acpi_video_support & ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR)
+		return 0;
+	else if (acpi_video_support & ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VIDEO)
+		return 1;
+
+	return acpi_video_support & ACPI_VIDEO_OUTPUT_SWITCHING;
+}
+EXPORT_SYMBOL(acpi_video_display_switch_support);
+
+/* Use acpi_display_output=vendor/video or acpi_backlight=vendor/video
+ * To force that backlight or display output switching is processed by vendor
+ * specific acpi drivers or video.ko driver.
+ */
+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_OUTPUT_SWITCHING_FORCE_VIDEO;
+	}
+	return 1;
+}
+__setup("acpi_backlight=", acpi_backlight);
+
+int __init acpi_display_output(char *str)
+{
+	if (str == NULL || *str == '\0')
+		return 1;
+	else {
+		if (!strcmp("vendor", str))
+			acpi_video_support &=
+				ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VENDOR;
+		if (!strcmp("video", str))
+			acpi_video_support &=
+				ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VIDEO;
+	}
+	return 1;
+}
+__setup("acpi_display_output=", acpi_display_output);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 41f7ce7..7e4e5bc 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -201,6 +201,51 @@ extern bool wmi_has_guid(const char *guid);
 
 #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_BACKLIGHT_FORCE_VIDEO		64
+#define ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VENDOR	128
+#define ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VIDEO		256
+#define ACPI_VIDEO_BACKLIGHT_DMI_VENDOR			512
+#define ACPI_VIDEO_BACKLIGHT_DMI_VIDEO			1024
+#define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR		2048
+#define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VIDEO		4096
+
+#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(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(acpi_handle graphics_dev_handle)
+{
+	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);
-- 
1.5.4.5

[PATCH 2/9] Acer-WMI: fingers off backlight if video.ko is serving this functionality

[trenn]

From: Thomas Renninger <trenn@suse.de>

---
 drivers/misc/acer-wmi.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index e7a3fe5..a843ac2 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -1218,6 +1218,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;
-- 
1.5.4.5

[PATCH 3/9] asus-acpi: fingers off backlight if video.ko is serving this functionality

[trenn]

From: Thomas Renninger <trenn@suse.de>

---
 drivers/misc/asus-laptop.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/asus-laptop.c b/drivers/misc/asus-laptop.c
index 7c6dfd0..a9a823b 100644
--- a/drivers/misc/asus-laptop.c
+++ b/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)
-- 
1.5.4.5

[PATCH 4/9] eeepc-laptop: fingers off backlight if video.ko is serving this functionality

[trenn]

From: Thomas Renninger <trenn@suse.de>

---
 drivers/misc/eeepc-laptop.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/eeepc-laptop.c b/drivers/misc/eeepc-laptop.c
index 9e8d79e..0d891a6 100644
--- a/drivers/misc/eeepc-laptop.c
+++ b/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;
-- 
1.5.4.5

[PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko is serving this functionality

[trenn]

From: Thomas Renninger <trenn@suse.de>

---
 drivers/misc/fujitsu-laptop.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/misc/fujitsu-laptop.c b/drivers/misc/fujitsu-laptop.c
index 3601224..7306776 100644
--- a/drivers/misc/fujitsu-laptop.c
+++ b/drivers/misc/fujitsu-laptop.c
@@ -963,16 +963,16 @@ static int __init fujitsu_init(void)
 
 	/* Register backlight stuff */
 
-	fujitsu->bl_device =
-	    backlight_device_register("fujitsu-laptop", NULL, NULL,
-				      &fujitsubl_ops);
-	if (IS_ERR(fujitsu->bl_device))
-		return PTR_ERR(fujitsu->bl_device);
-
-	max_brightness = fujitsu->max_brightness;
-
-	fujitsu->bl_device->props.max_brightness = max_brightness - 1;
-	fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
+	if (!acpi_video_backlight_support()) {
+		fujitsu->bl_device =
+			backlight_device_register("fujitsu-laptop", NULL, NULL,
+						  &fujitsubl_ops);
+		if (IS_ERR(fujitsu->bl_device))
+			return PTR_ERR(fujitsu->bl_device);
+		max_brightness = fujitsu->max_brightness;
+		fujitsu->bl_device->props.max_brightness = max_brightness - 1;
+		fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
+	}
 
 	ret = platform_driver_register(&fujitsupf_driver);
 	if (ret)
@@ -1008,7 +1008,8 @@ fail_hotkey:
 
 fail_backlight:
 
-	backlight_device_unregister(fujitsu->bl_device);
+	if (fujitsu->bl_device)
+		backlight_device_unregister(fujitsu->bl_device);
 
 fail_platform_device2:
 
@@ -1035,7 +1036,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);
 
-- 
1.5.4.5

[PATCH 6/9] msi-laptop: fingers off backlight if video.ko is serving this functionality

[trenn]

From: Thomas Renninger <trenn@suse.de>

---
 drivers/misc/msi-laptop.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/msi-laptop.c b/drivers/misc/msi-laptop.c
index de898c6..759763d 100644
--- a/drivers/misc/msi-laptop.c
+++ b/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)
-- 
1.5.4.5

[PATCH 7/9] sony-laptop: fingers off backlight if video.ko is serving this functionality

[trenn]

From: Thomas Renninger <trenn@suse.de>

---
 drivers/misc/sony-laptop.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/sony-laptop.c b/drivers/misc/sony-laptop.c
index 00e48e2..32675cb 100644
--- a/drivers/misc/sony-laptop.c
+++ b/drivers/misc/sony-laptop.c
@@ -1037,7 +1037,11 @@ static int sony_nc_add(struct acpi_device *device)
 		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);
-- 
1.5.4.5

[PATCH 8/9] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[trenn]

From: Thomas Renninger <trenn@suse.de>

---
 drivers/misc/thinkpad_acpi.c |   72 ++++++++++++++++++++++++++++++++++-------
 1 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index b596929..ba9934d 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/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_struct *ibm, u32 event)
 		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_backlight_data = {
 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,17 +4775,38 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
 	 */
 	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 and
+			 * BIOS won't switch, we have to do it ourselves
+			 * as long as no IGD driver exists
+			 *
+			 * This special case currently is the only part making
+			 * use of acpi_video_support flags and the reason why
+			 * they get exported via acpi.h.
+			*/
+			acpi_video_support = acpi_video_get_capabilities(NULL);
+			if (acpi_video_support & ACPI_VIDEO_IGD)
+				tp_features.bright_igdmode = 1;
+ 		}
 	}
 
 	if (!brightness_enable) {
-- 
1.5.4.5

[PATCH 9/9] compal: fingers off backlight if video.ko is serving this functionality

[trenn]

From: Thomas Renninger <trenn@suse.de>

---
 drivers/misc/compal-laptop.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/compal-laptop.c b/drivers/misc/compal-laptop.c
index 971e59a..f03cfbf 100644
--- a/drivers/misc/compal-laptop.c
+++ b/drivers/misc/compal-laptop.c
@@ -326,12 +326,14 @@ static int __init compal_init(void)
 
 	/* Register backlight stuff */
 
-	compalbl_device = backlight_device_register("compal-laptop", NULL, NULL,
-						&compalbl_ops);
-	if (IS_ERR(compalbl_device))
-		return PTR_ERR(compalbl_device);
-
-	compalbl_device->props.max_brightness = COMPAL_LCD_LEVEL_MAX-1;
+	if (!acpi_video_backlight_support()) {
+		compalbl_device = backlight_device_register("compal-laptop", NULL, NULL,
+							    &compalbl_ops);
+		if (IS_ERR(compalbl_device))
+			return PTR_ERR(compalbl_device);
+		
+		compalbl_device->props.max_brightness = COMPAL_LCD_LEVEL_MAX-1;
+	}
 
 	ret = platform_driver_register(&compal_driver);
 	if (ret)
-- 
1.5.4.5

Re: [PATCH 0/9] Check whether ACPI graphics device is physically present

[Thomas Renninger]

This one is needed also.
Matthew made me a bit nervous about it,but to detect video capabilities it is 
necessary to know whether the ACPI graphics device is really there.
The IGD problems should be solved now and this can be added, right? IMO it 
could have been added before also because the other patches took care about 
the reported regression.

Other patches depend on this one and won't compile without this function.

All patches are against:
git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-acpi-2.6.git release

       Thomas
----
Check whether ACPI graphics device is physically present

This is a reimplemention of commit
0119509c4fbc9adcef1472817fda295334612976
from Matthew Garrett <mjg59@srcf.ucam.org>

This patch got removed because of a regression: ThinkPads with a
Intel graphics card and an Integrated Graphics Device BIOS implementation
stopped working.
In fact, they only worked because the ACPI device of the discrete, the
wrong one, got used (via int10). So ACPI functions were poking on the wrong
hardware used which is a sever bug.
The next patch provides support for above ThinkPads to be able to
switch brightness via the legacy thinkpad_acpi driver and automatically
detect when to use it.

Original commit message from Matthew Garrett:
    Vendors often ship machines with a choice of integrated or discrete
    graphics, and use the same DSDT for both. As a result, the ACPI video
    module will locate devices that may not exist on this specific platform.
    Attempt to determine whether the device exists or not, and abort the
    device creation if it doesn't.

http://bugzilla.kernel.org/show_bug.cgi?id=9614

Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 drivers/acpi/glue.c     |   40 ++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/video.c    |    7 ++++++-
 include/acpi/acpi_bus.h |    2 ++
 3 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 9b227d4..a76ef8f 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -140,6 +140,46 @@ struct device *acpi_get_physical_device(acpi_handle 
handle)
 
 EXPORT_SYMBOL(acpi_get_physical_device);
 
+/* ToDo: When a PCI bridge is found, return the PCI device behind the bridge
+ *       This should work in general, but did not on a Lenovo T61 for the
+ *	 graphics card. But this must be fixed when the PCI device is
+ *       bound and the kernel device struct is attached to the acpi device
+ * Note: A success call will increase reference count by one
+ *       Do call put_device(dev) on the returned device then
+ */
+struct device *acpi_get_physical_pci_device(acpi_handle handle)
+{
+	struct device *dev;
+	long device_id;
+	acpi_status status;
+
+	status =
+		acpi_evaluate_integer(handle, "_ADR", NULL, &device_id);
+
+	if (ACPI_FAILURE(status))
+		return NULL;
+
+	/* We need to attempt to determine whether the _ADR refers to a
+	   PCI device or not. There's no terribly good way to do this,
+	   so the best we can hope for is to assume that there'll never
+	   be a device in the host bridge */
+	if (device_id >= 0x10000) {
+		/* It looks like a PCI device. Does it exist? */
+		dev = acpi_get_physical_device(handle);
+	} else {
+		/* It doesn't look like a PCI device. Does its parent
+		   exist? */
+		acpi_handle phandle;
+		if (acpi_get_parent(handle, &phandle))
+			return NULL;
+		dev = acpi_get_physical_device(phandle);
+	}
+	if (!dev)
+		return NULL;
+	return dev;
+}
+EXPORT_SYMBOL(acpi_get_physical_pci_device);
+
 static int acpi_bind_one(struct device *dev, acpi_handle handle)
 {
 	struct acpi_device *acpi_dev;
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 0da8f55..767d9b9 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -833,11 +833,16 @@ static void acpi_video_bus_find_cap(struct 
acpi_video_bus *video)
 static int acpi_video_bus_check(struct acpi_video_bus *video)
 {
 	acpi_status status = -ENOENT;
-
+	struct device *dev;
 
 	if (!video)
 		return -EINVAL;
 
+	dev = acpi_get_physical_pci_device(video->device->handle);
+	if (!dev)
+		return -ENODEV;
+	put_device(dev);
+
 	/* Since there is no HID, CID and so on for VGA driver, we have
 	 * to check well known required nodes.
 	 */
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index db90a74..d4243a4 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -370,6 +370,8 @@ struct acpi_bus_type {
 int register_acpi_bus_type(struct acpi_bus_type *);
 int unregister_acpi_bus_type(struct acpi_bus_type *);
 struct device *acpi_get_physical_device(acpi_handle);
+struct device *acpi_get_physical_pci_device(acpi_handle);
+
 /* helper */
 acpi_handle acpi_get_child(acpi_handle, acpi_integer);
 acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
-- 
1.5.4.5

Re: [PATCH 0/9] Check whether ACPI graphics device is physically present

[Matthew Garrett]

On Thu, Jul 10, 2008 at 09:06:24PM +0200, Thomas Renninger wrote:
> This one is needed also.
> Matthew made me a bit nervous about it,but to detect video capabilities it is 
> necessary to know whether the ACPI graphics device is really there.
> The IGD problems should be solved now and this can be added, right? IMO it 
> could have been added before also because the other patches took care about 
> the reported regression.

The patch is good, but the comment still indicates that you want to 
force control over to thinkpad_acpi. We're not going to do that, so 
either resend it without that inference or I can resend it when I get 
back home next week.

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

Re: [PATCH 0/9] Check whether ACPI graphics device is physically present

[Thomas Renninger]

On Friday 11 July 2008 11:06:52 Matthew Garrett wrote:
> On Thu, Jul 10, 2008 at 09:06:24PM +0200, Thomas Renninger wrote:
> > This one is needed also.
> > Matthew made me a bit nervous about it,but to detect video capabilities
> > it is necessary to know whether the ACPI graphics device is really there.
> > The IGD problems should be solved now and this can be added, right? IMO
> > it could have been added before also because the other patches took care
> > about the reported regression.
>
> The patch is good, but the comment still indicates that you want to
> force control over to thinkpad_acpi. We're not going to do that, so
> either resend it without that inference or I can resend it when I get
> back home next week.

Yep, sorry for the long delay, something else seem to not work anymore since 
my last changes.

I will repost soon.

      Thomas

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko is serving this functionality

[nokos]

Hi there,

on the S6410 this prevents brightness control from working correctly. 
the ACPI BIOS provides non-functional _BCM/_BQC so the video.ko is
registering backlight functions but does not work. Unfortunately not
loading video.ko isn't a solution since with
bc45b1d39a925b56796bebf8a397a0491489d85c the brightness keys are only
reported through video.ko.



Am Donnerstag, den 10.07.2008, 16:31 +0200 schrieb trenn@suse.de: 
> From: Thomas Renninger <trenn@suse.de>
> 
> ---
>  drivers/misc/fujitsu-laptop.c |   26 ++++++++++++++------------
>  1 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/misc/fujitsu-laptop.c b/drivers/misc/fujitsu-laptop.c
> index 3601224..7306776 100644
> --- a/drivers/misc/fujitsu-laptop.c
> +++ b/drivers/misc/fujitsu-laptop.c
> @@ -963,16 +963,16 @@ static int __init fujitsu_init(void)
>  
>  	/* Register backlight stuff */
>  
> -	fujitsu->bl_device =
> -	    backlight_device_register("fujitsu-laptop", NULL, NULL,
> -				      &fujitsubl_ops);
> -	if (IS_ERR(fujitsu->bl_device))
> -		return PTR_ERR(fujitsu->bl_device);
> -
> -	max_brightness = fujitsu->max_brightness;
> -
> -	fujitsu->bl_device->props.max_brightness = max_brightness - 1;
> -	fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
> +	if (!acpi_video_backlight_support()) {
> +		fujitsu->bl_device =
> +			backlight_device_register("fujitsu-laptop", NULL, NULL,
> +						  &fujitsubl_ops);
> +		if (IS_ERR(fujitsu->bl_device))
> +			return PTR_ERR(fujitsu->bl_device);
> +		max_brightness = fujitsu->max_brightness;
> +		fujitsu->bl_device->props.max_brightness = max_brightness - 1;
> +		fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
> +	}
>  
>  	ret = platform_driver_register(&fujitsupf_driver);
>  	if (ret)
> @@ -1008,7 +1008,8 @@ fail_hotkey:
>  
>  fail_backlight:
>  
> -	backlight_device_unregister(fujitsu->bl_device);
> +	if (fujitsu->bl_device)
> +		backlight_device_unregister(fujitsu->bl_device);
>  
>  fail_platform_device2:
>  
> @@ -1035,7 +1036,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);
>  

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko is serving this functionality

[Thomas Renninger]

On Tuesday 15 July 2008 11:38:20 nokos@gmx.net wrote:
> Hi there,
>
> on the S6410 this prevents brightness control from working correctly.
> the ACPI BIOS provides non-functional _BCM/_BQC so the video.ko is
> registering backlight functions but does not work. Unfortunately not
> loading video.ko isn't a solution since with
> bc45b1d39a925b56796bebf8a397a0491489d85c the brightness keys are only
> reported through video.ko.
Is this an Intel graphics card?
Then you should also try Matthew's/Hong's IGD extensions.
It is likely that everything works out with video.ko then.
If not, with a patch I sent recently you should switch off Windows osi strings 
with:
acpi_osi=windows_false   (preferred)
or (already works)
acpi_osi="!Windows 2006"

If it still does not work, then there is a driver missing which was  a 
proprietary one on XP and which has not been fully re-engeeniered for Linux.

Using ACPI video events and pass them to the Fujitsu driver to change 
backlight (is this how it works? Is this done through userspace?), mixes up 
everything.
Either you have video.ko or fujitsu_acpi.ko or you are in trouble of double 
touching the HW.

         Thomas

> Am Donnerstag, den 10.07.2008, 16:31 +0200 schrieb trenn@suse.de:
> > From: Thomas Renninger <trenn@suse.de>
> >
> > ---
> >  drivers/misc/fujitsu-laptop.c |   26 ++++++++++++++------------
> >  1 files changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/misc/fujitsu-laptop.c
> > b/drivers/misc/fujitsu-laptop.c index 3601224..7306776 100644
> > --- a/drivers/misc/fujitsu-laptop.c
> > +++ b/drivers/misc/fujitsu-laptop.c
> > @@ -963,16 +963,16 @@ static int __init fujitsu_init(void)
> >
> >  	/* Register backlight stuff */
> >
> > -	fujitsu->bl_device =
> > -	    backlight_device_register("fujitsu-laptop", NULL, NULL,
> > -				      &fujitsubl_ops);
> > -	if (IS_ERR(fujitsu->bl_device))
> > -		return PTR_ERR(fujitsu->bl_device);
> > -
> > -	max_brightness = fujitsu->max_brightness;
> > -
> > -	fujitsu->bl_device->props.max_brightness = max_brightness - 1;
> > -	fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
> > +	if (!acpi_video_backlight_support()) {
> > +		fujitsu->bl_device =
> > +			backlight_device_register("fujitsu-laptop", NULL, NULL,
> > +						  &fujitsubl_ops);
> > +		if (IS_ERR(fujitsu->bl_device))
> > +			return PTR_ERR(fujitsu->bl_device);
> > +		max_brightness = fujitsu->max_brightness;
> > +		fujitsu->bl_device->props.max_brightness = max_brightness - 1;
> > +		fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
> > +	}
> >
> >  	ret = platform_driver_register(&fujitsupf_driver);
> >  	if (ret)
> > @@ -1008,7 +1008,8 @@ fail_hotkey:
> >
> >  fail_backlight:
> >
> > -	backlight_device_unregister(fujitsu->bl_device);
> > +	if (fujitsu->bl_device)
> > +		backlight_device_unregister(fujitsu->bl_device);
> >
> >  fail_platform_device2:
> >
> > @@ -1035,7 +1036,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);

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko is serving this functionality

[nokos]

Am Dienstag, den 15.07.2008, 14:46 +0200 schrieb Thomas Renninger:
> On Tuesday 15 July 2008 11:38:20 nokos@gmx.net wrote:
> > Hi there,
> >
> > on the S6410 this prevents brightness control from working correctly.
> > the ACPI BIOS provides non-functional _BCM/_BQC so the video.ko is
> > registering backlight functions but does not work. Unfortunately not
> > loading video.ko isn't a solution since with
> > bc45b1d39a925b56796bebf8a397a0491489d85c the brightness keys are only
> > reported through video.ko.
> Is this an Intel graphics card?
> Then you should also try Matthew's/Hong's IGD extensions.

where can I find those?

> It is likely that everything works out with video.ko then.
> If not, with a patch I sent recently you should switch off Windows osi strings 
> with:
> acpi_osi=windows_false   (preferred)
> or (already works)
> acpi_osi="!Windows 2006"

disabling windows_osi disables backlight through video.ko and everything
works fine with fujitsu_laptop (also brightness-buttons since ACPI then
generates events on the fujitsu device) but some other ACPI stuff is
disabled too (extended battery status etc...)

> If it still does not work, then there is a driver missing which was  a 
> proprietary one on XP and which has not been fully re-engeeniered for Linux.
> 
> Using ACPI video events and pass them to the Fujitsu driver to change 
> backlight (is this how it works? Is this done through userspace?), mixes up 
> everything.

without acpi_osi="!Windows 2006" the buttons just produce ACPI video
events (recieved by video.ko) and hal does the adjustment through
fujitsu_laptop.ko
with acpi_osi="!Windows 2006" ACPI does the adjustment itself and no
direct event is generated (fujitsu_laptop gets an "something changed,
find out what" event)

> Either you have video.ko or fujitsu_acpi.ko or you are in trouble of double 
> touching the HW.

The Problem is that video.ko only touches an otherwise unused ACPI
register but does not change the brightness... (you can just store a
number between 0 and 7 there)
Perhaps there is a way to disable the dysfunctional backlight stuff in 
video.ko but retain the ACPI video event handling?!

Peter

>          Thomas
> 
> > Am Donnerstag, den 10.07.2008, 16:31 +0200 schrieb trenn@suse.de:
> > > From: Thomas Renninger <trenn@suse.de>
> > >
> > > ---
> > >  drivers/misc/fujitsu-laptop.c |   26 ++++++++++++++------------
> > >  1 files changed, 14 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/misc/fujitsu-laptop.c
> > > b/drivers/misc/fujitsu-laptop.c index 3601224..7306776 100644
> > > --- a/drivers/misc/fujitsu-laptop.c
> > > +++ b/drivers/misc/fujitsu-laptop.c
> > > @@ -963,16 +963,16 @@ static int __init fujitsu_init(void)
> > >
> > >  	/* Register backlight stuff */
> > >
> > > -	fujitsu->bl_device =
> > > -	    backlight_device_register("fujitsu-laptop", NULL, NULL,
> > > -				      &fujitsubl_ops);
> > > -	if (IS_ERR(fujitsu->bl_device))
> > > -		return PTR_ERR(fujitsu->bl_device);
> > > -
> > > -	max_brightness = fujitsu->max_brightness;
> > > -
> > > -	fujitsu->bl_device->props.max_brightness = max_brightness - 1;
> > > -	fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
> > > +	if (!acpi_video_backlight_support()) {
> > > +		fujitsu->bl_device =
> > > +			backlight_device_register("fujitsu-laptop", NULL, NULL,
> > > +						  &fujitsubl_ops);
> > > +		if (IS_ERR(fujitsu->bl_device))
> > > +			return PTR_ERR(fujitsu->bl_device);
> > > +		max_brightness = fujitsu->max_brightness;
> > > +		fujitsu->bl_device->props.max_brightness = max_brightness - 1;
> > > +		fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
> > > +	}
> > >
> > >  	ret = platform_driver_register(&fujitsupf_driver);
> > >  	if (ret)
> > > @@ -1008,7 +1008,8 @@ fail_hotkey:
> > >
> > >  fail_backlight:
> > >
> > > -	backlight_device_unregister(fujitsu->bl_device);
> > > +	if (fujitsu->bl_device)
> > > +		backlight_device_unregister(fujitsu->bl_device);
> > >
> > >  fail_platform_device2:
> > >
> > > @@ -1035,7 +1036,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);
> 
> 
> --
> 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 5/9] fujitsu-laptop: fingers off backlight if video.ko is serving this functionality

[Jonathan Woithe]

> On Tuesday 15 July 2008 11:38:20 nokos@gmx.net wrote:
> > on the S6410 this prevents brightness control from working correctly.
> > the ACPI BIOS provides non-functional _BCM/_BQC so the video.ko is
> > registering backlight functions but does not work. Unfortunately not
> > loading video.ko isn't a solution since with
> > bc45b1d39a925b56796bebf8a397a0491489d85c the brightness keys are only
> > reported through video.ko.
> Is this an Intel graphics card?

It probably is.  Certainly the S7020 uses an intel chipset.  Unfortunately I
haven't had a chance to test this patch on my S7020 yet and won't have time
until at least tomorrow night.

> Then you should also try Matthew's/Hong's IGD extensions.
> It is likely that everything works out with video.ko then.

I'll track these down and give them a go.

> Using ACPI video events and pass them to the Fujitsu driver to change 
> backlight (is this how it works? Is this done through userspace?), mixes up 
> everything.

I don't fully understand this statement but from what I can tell this is not
how it works.  Whatever video.ko does, it doesn't work out the box on
certain Fujitsu laptops (S6410, S7020 confirmed) - the relevant backlight
controls provided by video.ko are either non-functional or give "no such
device" errors.  The Fujitsu driver hooks into the methods which are present
on these laptops through the custom Fujitsu FUJ02B1 device (SBLL/GBLL or
SBL2/GBLS depending on model) and uses these to provide a standard backlight
class interface.  Userspace is not involved here, nor are ACPI video events
utilised.  There's certainly no passing of ACPI video events into the
Fujitsu driver.

The pressing of the LCD brightness buttons are passed to userspace as ACPI
events but the actual changing of the brightness is handled entirely by the
kernel - the keypress events are for notification only.

> Either you have video.ko or fujitsu_acpi.ko or you are in trouble of double 
> touching the HW.

With both video.ko and fujitsu-laptop.ko loaded I don't think the hardware
can be double touched unless I'm missing something.  The reason why
fujitsu-laptop includes backlight control is because video.ko's methods
don't have any effect on the hardware.  If video.ko fails to control the
backlight then it's clearly not touching the same hardware as
fujitsu-laptop.

The reason why things go pear-shaped with
bc45b1d39a925b56796bebf8a397a0491489d85c when video.ko isn't loaded is
probably due to the code added to fujitsu-laptop by this patch.  Since
(non-functional) _BCM/_BQC are apparently present in this laptop, the new
code makes fujitsu-laptop think that ACPI video will handle the backlight
and so fujitsu-laptop doesn't set up its backlight control system.  We're
then right back where we started - video.ko "handles" the backlight but it
doesn't work with this hardware because the "standard" methods don't affect
the LCD brightness on these laptops.

Perhaps the IGD extensions will magically make video.ko drive the backlight
correctly with these laptops - I will have to test this myself.  However, as
things stand at the moment it seems that this patch causes a regression for
Fujitsu S-series owners.

Regards
  jonathan

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko

[Jonathan Woithe]

> > It is likely that everything works out with video.ko then.
> > If not, with a patch I sent recently you should switch off Windows osi strings 
> > with:
> > acpi_osi=windows_false   (preferred)
> > or (already works)
> > acpi_osi="!Windows 2006"
> 
> disabling windows_osi disables backlight through video.ko and everything
> works fine with fujitsu_laptop (also brightness-buttons since ACPI then
> generates events on the fujitsu device) but some other ACPI stuff is
> disabled too (extended battery status etc...)

As per my previous email, the fact that this works is probably because in
this instance acpi_video_backlight_support() returns false and allows
fujitsu-laptop to set up its backlight control functions.

> > If it still does not work, then there is a driver missing which was  a 
> > proprietary one on XP and which has not been fully re-engeeniered for Linux.
> > 
> > Using ACPI video events and pass them to the Fujitsu driver to change 
> > backlight (is this how it works? Is this done through userspace?), mixes up 
> > everything.
> 
> without acpi_osi="!Windows 2006" the buttons just produce ACPI video
> events (recieved by video.ko) and hal does the adjustment through
> fujitsu_laptop.ko

I guess video.ko receives them because they are ACPI video events.  However,
as previously determined it doesn't do anything useful with them.  I'm a
little surprised that the physical brightness adjustment comes from hal
though.  On my S7020 the brightness changes brought about by the brightness
buttons are seemingly done in hardware - the buttons have an effect even
when fujitsu-laptop isn't loaded.  The brightness control functions in
fujitsu-laptop were added primarily to allow *software* control of the
brightness - something which definitely doesn't work without fujitsu-laptop.
The generation of ACPI video brightness keypress events was added to
fujitsu-laptop so userspace could keep track of changes instigated by the
hardware buttons.

> The Problem is that video.ko only touches an otherwise unused ACPI
> register but does not change the brightness... (you can just store a
> number between 0 and 7 there)

Yes, which is why I'm fairly sure there's no risk of double-touching the
hardware.  video.ko and fujitsu-laptop.ko are driving different hardware.

> Perhaps there is a way to disable the dysfunctional backlight stuff in 
> video.ko but retain the ACPI video event handling?!

The problem with doing this is that it's only the platform-specific drivers
which have the knowledge as to whether this is necessary or not.  video.ko
would therefore have to query each loaded platform-specific module which
seems messy to me.  If video.ko was loaded before the relevant
platform-specific module it gets even messier.  I guess we could add a
video.ko function like acpi_video_backlight_disable() which platform modules
could call if they choose to handle the backlight.  However, this seems
suboptimal to since it means video.ko sets up a backlight interface only to
have to tear it down again soon after.  I guess it's no worse than setting
up a backlight interface which never gets used though.

The fundamental problem as I see at this point it is that the new
acpi_video_backlight_support() function returns true on Fujitsu S-series
laptops because the ACPI environment looks "right" from video.ko's point of
view.  However, it's clear that the desired registers/methods - although
present - are completely non-functional in these laptops so in reality
video.ko cannot handle the backlight in this case.  Unless video.ko
hooks into the Fujitsu-specific registers (which defeats the modularity of
hardware-specific modules) this won't change.

To date I haven't had a chance to look at the whole IGD thing so I have no
idea what effect (if any) those patches are likely to have on this hardware. 
I will try to get to that in the next few days.  Maybe the IGD extensions
will allow video.ko to correctly drive the backlight on these laptops but
until this has been determined the best way forward is probably to omit the
acpi_video_backlight_support() tests from fujitsu-laptop (in effect this
means drop this part of the patch series since that's all it does).  This is
on the basis that acpi_video_backlight_support() doesn't currently take this
Fujitsu hardware into account when it formulates its return value, and
therefore its return value doesn't currently make sense on this platform.

Regards
  jonathan

> From: Thomas Renninger <trenn@suse.de>
>
> ---
>  drivers/misc/fujitsu-laptop.c |   26 ++++++++++++++------------
>  1 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/misc/fujitsu-laptop.c
> b/drivers/misc/fujitsu-laptop.c index 3601224..7306776 100644
> --- a/drivers/misc/fujitsu-laptop.c
> +++ b/drivers/misc/fujitsu-laptop.c
> @@ -963,16 +963,16 @@ static int __init fujitsu_init(void)
>
>  	/* Register backlight stuff */
>
> -	fujitsu->bl_device =
> -	    backlight_device_register("fujitsu-laptop", NULL, NULL,
> -				      &fujitsubl_ops);
> -	if (IS_ERR(fujitsu->bl_device))
> -		return PTR_ERR(fujitsu->bl_device);
> -
> -	max_brightness = fujitsu->max_brightness;
> -
> -	fujitsu->bl_device->props.max_brightness = max_brightness - 1;
> -	fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
> +	if (!acpi_video_backlight_support()) {
> +		fujitsu->bl_device =
> +			backlight_device_register("fujitsu-laptop", NULL, NULL,
> +						  &fujitsubl_ops);
> +		if (IS_ERR(fujitsu->bl_device))
> +			return PTR_ERR(fujitsu->bl_device);
> +		max_brightness = fujitsu->max_brightness;
> +		fujitsu->bl_device->props.max_brightness = max_brightness - 1;
> +		fujitsu->bl_device->props.brightness = fujitsu->brightness_level;
> +	}
>
>  	ret = platform_driver_register(&fujitsupf_driver);
>  	if (ret)
> @@ -1008,7 +1008,8 @@ fail_hotkey:
>
>  fail_backlight:
>
> -	backlight_device_unregister(fujitsu->bl_device);
> +	if (fujitsu->bl_device)
> +		backlight_device_unregister(fujitsu->bl_device);
>
>  fail_platform_device2:
>
> @@ -1035,7 +1036,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);

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko is serving this functionality

[Zhang Rui]

On Tue, 2008-07-15 at 14:46 +0200, Thomas Renninger wrote:
> On Tuesday 15 July 2008 11:38:20 nokos@gmx.net wrote:
> > Hi there,
> >
> > on the S6410 this prevents brightness control from working correctly.
> > the ACPI BIOS provides non-functional _BCM/_BQC so the video.ko is
> > registering backlight functions but does not work. Unfortunately not
> > loading video.ko isn't a solution since with
> > bc45b1d39a925b56796bebf8a397a0491489d85c the brightness keys are only
> > reported through video.ko.
> Is this an Intel graphics card?
> Then you should also try Matthew's/Hong's IGD extensions.
> It is likely that everything works out with video.ko then.
> If not, with a patch I sent recently you should switch off Windows osi strings 
> with:
> acpi_osi=windows_false   (preferred)
> or (already works)
> acpi_osi="!Windows 2006"
> 
> If it still does not work, then there is a driver missing which was  a 
> proprietary one on XP and which has not been fully re-engeeniered for Linux.
> 
> Using ACPI video events and pass them to the Fujitsu driver to change 
> backlight (is this how it works? Is this done through userspace?), mixes up 
> everything.
No. The platform drivers have nothing to do with ACPI Video driver, they
have platform specific events/control methods.
Maybe you mixed it up with the IGD stuff.
In IGD mode, ACPI video works together with graphics driver,
And in this case, ACPI video exports ACPI video events, graphics driver
gets the events (acpi_notifier_chain) and do the actual work. The ACPI
control method (_BCM) is not used any more so that only the graphics
driver touches the hardware. :)

thanks,
rui

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko

[Zhang Rui]

On Wed, 2008-07-16 at 09:25 +0930, Jonathan Woithe wrote:
> > > It is likely that everything works out with video.ko then.
> > > If not, with a patch I sent recently you should switch off Windows osi strings 
> > > with:
> > > acpi_osi=windows_false   (preferred)
> > > or (already works)
> > > acpi_osi="!Windows 2006"
> > 
> > disabling windows_osi disables backlight through video.ko and everything
> > works fine with fujitsu_laptop (also brightness-buttons since ACPI then
> > generates events on the fujitsu device) but some other ACPI stuff is
> > disabled too (extended battery status etc...)
> 
> As per my previous email, the fact that this works is probably because in
> this instance acpi_video_backlight_support() returns false and allows
> fujitsu-laptop to set up its backlight control functions.

Peter,
can you attach the acpidump output please?
you can use the latest pmtools at
http://www.lesswatts.org/patches/linux_acpi/

> 
> > > If it still does not work, then there is a driver missing which was  a 
> > > proprietary one on XP and which has not been fully re-engeeniered for Linux.
> > > 
> > > Using ACPI video events and pass them to the Fujitsu driver to change 
> > > backlight (is this how it works? Is this done through userspace?), mixes up 
> > > everything.
> > 
> > without acpi_osi="!Windows 2006" the buttons just produce ACPI video
> > events (recieved by video.ko) and hal does the adjustment through
> > fujitsu_laptop.ko
> 
> I guess video.ko receives them because they are ACPI video events.  However,
> as previously determined it doesn't do anything useful with them.  I'm a
> little surprised that the physical brightness adjustment comes from hal
> though.  On my S7020 the brightness changes brought about by the brightness
> buttons are seemingly done in hardware - the buttons have an effect even
> when fujitsu-laptop isn't loaded.  The brightness control functions in
> fujitsu-laptop were added primarily to allow *software* control of the
> brightness - something which definitely doesn't work without fujitsu-laptop.
> The generation of ACPI video brightness keypress events was added to
> fujitsu-laptop so userspace could keep track of changes instigated by the
> hardware buttons.
> 
> > The Problem is that video.ko only touches an otherwise unused ACPI
> > register but does not change the brightness... (you can just store a
> > number between 0 and 7 there)
> 
> Yes, which is why I'm fairly sure there's no risk of double-touching the
> hardware.  video.ko and fujitsu-laptop.ko are driving different hardware.
> 
> > Perhaps there is a way to disable the dysfunctional backlight stuff in 
> > video.ko but retain the ACPI video event handling?!
> 
> The problem with doing this is that it's only the platform-specific drivers
> which have the knowledge as to whether this is necessary or not.  video.ko
> would therefore have to query each loaded platform-specific module which
> seems messy to me.  If video.ko was loaded before the relevant
> platform-specific module it gets even messier.  I guess we could add a
> video.ko function like acpi_video_backlight_disable() which platform modules
> could call if they choose to handle the backlight.  However, this seems
> suboptimal to since it means video.ko sets up a backlight interface only to
> have to tear it down again soon after.  I guess it's no worse than setting
> up a backlight interface which never gets used though.
> 
> The fundamental problem as I see at this point it is that the new
> acpi_video_backlight_support() function returns true on Fujitsu S-series
> laptops because the ACPI environment looks "right" from video.ko's point of
> view.  However, it's clear that the desired registers/methods - although
> present - are completely non-functional in these laptops so in reality
> video.ko cannot handle the backlight in this case.  Unless video.ko
> hooks into the Fujitsu-specific registers (which defeats the modularity of
> hardware-specific modules) this won't change.

IMO, In this case, we should add a dmi check in video_detect.c and set
acpi_backlight=vendor for this laptop, both before acpi video and
fujitsu laptop driver is loaded.

thanks,
rui

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko

[Jonathan Woithe]

> > > Perhaps there is a way to disable the dysfunctional backlight stuff in 
> > > video.ko but retain the ACPI video event handling?!
> > 
> > The problem with doing this is that it's only the platform-specific drivers
> > which have the knowledge as to whether this is necessary or not.  video.ko
> > would therefore have to query each loaded platform-specific module which
> > seems messy to me.  If video.ko was loaded before the relevant
> > platform-specific module it gets even messier.  I guess we could add a
> > video.ko function like acpi_video_backlight_disable() which platform modules
> > could call if they choose to handle the backlight.  However, this seems
> > suboptimal to since it means video.ko sets up a backlight interface only to
> > have to tear it down again soon after.  I guess it's no worse than setting
> > up a backlight interface which never gets used though.
> > 
> > The fundamental problem as I see at this point it is that the new
> > acpi_video_backlight_support() function returns true on Fujitsu S-series
> > laptops because the ACPI environment looks "right" from video.ko's point of
> > view.  However, it's clear that the desired registers/methods - although
> > present - are completely non-functional in these laptops so in reality
> > video.ko cannot handle the backlight in this case.  Unless video.ko
> > hooks into the Fujitsu-specific registers (which defeats the modularity of
> > hardware-specific modules) this won't change.
> 
> IMO, In this case, we should add a dmi check in video_detect.c and set
> acpi_backlight=vendor for this laptop, both before acpi video and
> fujitsu laptop driver is loaded.

That could work.  Keeping in mind that I'm yet to test the S7020 against
this patch though, what if it turns out that this problem affects most
Fujitsu laptops?  One wouldn't want dozens of model-specific checks in
video_detect.c, but I guess it may be possible to come up with a
sufficiently general DMI check that matched all such laptops.

Regards
  jonathan

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko

[nokos]

Am Mittwoch, den 16.07.2008, 10:12 +0800 schrieb Zhang Rui:
> On Wed, 2008-07-16 at 09:25 +0930, Jonathan Woithe wrote:
> > > > It is likely that everything works out with video.ko then.
> > > > If not, with a patch I sent recently you should switch off Windows osi strings 
> > > > with:
> > > > acpi_osi=windows_false   (preferred)
> > > > or (already works)
> > > > acpi_osi="!Windows 2006"
> > > 
> > > disabling windows_osi disables backlight through video.ko and everything
> > > works fine with fujitsu_laptop (also brightness-buttons since ACPI then
> > > generates events on the fujitsu device) but some other ACPI stuff is
> > > disabled too (extended battery status etc...)
> > 
> > As per my previous email, the fact that this works is probably because in
> > this instance acpi_video_backlight_support() returns false and allows
> > fujitsu-laptop to set up its backlight control functions.
> 
> Peter,
> can you attach the acpidump output please?
> you can use the latest pmtools at
> http://www.lesswatts.org/patches/linux_acpi/

already in http://bugzilla.kernel.org/show_bug.cgi?id=10454

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko

[nokos]

Am Mittwoch, den 16.07.2008, 09:25 +0930 schrieb Jonathan Woithe:
> > > It is likely that everything works out with video.ko then.
> > > If not, with a patch I sent recently you should switch off Windows osi strings 
> > > with:
> > > acpi_osi=windows_false   (preferred)
> > > or (already works)
> > > acpi_osi="!Windows 2006"
> > 
> > disabling windows_osi disables backlight through video.ko and everything
> > works fine with fujitsu_laptop (also brightness-buttons since ACPI then
> > generates events on the fujitsu device) but some other ACPI stuff is
> > disabled too (extended battery status etc...)
> 
> As per my previous email, the fact that this works is probably because in
> this instance acpi_video_backlight_support() returns false and allows
> fujitsu-laptop to set up its backlight control functions.
> 
> > > If it still does not work, then there is a driver missing which was  a 
> > > proprietary one on XP and which has not been fully re-engeeniered for Linux.
> > > 
> > > Using ACPI video events and pass them to the Fujitsu driver to change 
> > > backlight (is this how it works? Is this done through userspace?), mixes up 
> > > everything.
> > 
> > without acpi_osi="!Windows 2006" the buttons just produce ACPI video
> > events (recieved by video.ko) and hal does the adjustment through
> > fujitsu_laptop.ko
> 
> I guess video.ko receives them because they are ACPI video events.  However,
> as previously determined it doesn't do anything useful with them.  I'm a
> little surprised that the physical brightness adjustment comes from hal
> though.  On my S7020 the brightness changes brought about by the brightness
> buttons are seemingly done in hardware - the buttons have an effect even
> when fujitsu-laptop isn't loaded.  The brightness control functions in
> fujitsu-laptop were added primarily to allow *software* control of the
> brightness - something which definitely doesn't work without fujitsu-laptop.
> The generation of ACPI video brightness keypress events was added to
> fujitsu-laptop so userspace could keep track of changes instigated by the
> hardware buttons.

This behavior seems to be a recent change to the ACPI BIOS
implementation (its in the VIST table, and only takes effect if
osi("Windows 2006") returns true) ... its only my guess thats its hal
who is doing it (with debug="0xffff" on fujitsu_laptop I can verify that
somebody writes to /sys/class/backlight/fujitsu-laptop/brightness and
since it also happens without running windowing environment I suspect
hal). On the S6410 the acpi_osi="!Windows 2006" case probably resembled
the S7020 behavior.

> > The Problem is that video.ko only touches an otherwise unused ACPI
> > register but does not change the brightness... (you can just store a
> > number between 0 and 7 there)
> 
> Yes, which is why I'm fairly sure there's no risk of double-touching the
> hardware.  video.ko and fujitsu-laptop.ko are driving different hardware.
> 
> > Perhaps there is a way to disable the dysfunctional backlight stuff in 
> > video.ko but retain the ACPI video event handling?!
> 
> The problem with doing this is that it's only the platform-specific drivers
> which have the knowledge as to whether this is necessary or not.  video.ko
> would therefore have to query each loaded platform-specific module which
> seems messy to me.  If video.ko was loaded before the relevant
> platform-specific module it gets even messier.  I guess we could add a
> video.ko function like acpi_video_backlight_disable() which platform modules
> could call if they choose to handle the backlight.  However, this seems
> suboptimal to since it means video.ko sets up a backlight interface only to
> have to tear it down again soon after.  I guess it's no worse than setting
> up a backlight interface which never gets used though.

perhaps this can be done with a DMI based blacklist in video.ko, which
dis/enables the different parts of the module (backlight control, video
events ...)

Peter

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko

[Jonathan Woithe]

Hi Peter

> Am Mittwoch, den 16.07.2008, 09:25 +0930 schrieb Jonathan Woithe:
> > > > It is likely that everything works out with video.ko then.
> > > > If not, with a patch I sent recently you should switch off Windows osi strings 
> > > > with:
> > > > acpi_osi=windows_false   (preferred)
> > > > or (already works)
> > > > acpi_osi="!Windows 2006"
> > > 
> > > disabling windows_osi disables backlight through video.ko and everything
> > > works fine with fujitsu_laptop (also brightness-buttons since ACPI then
> > > generates events on the fujitsu device) but some other ACPI stuff is
> > > disabled too (extended battery status etc...)
> > 
> > As per my previous email, the fact that this works is probably because in
> > this instance acpi_video_backlight_support() returns false and allows
> > fujitsu-laptop to set up its backlight control functions.
:
> This behavior seems to be a recent change to the ACPI BIOS
> implementation (its in the VIST table, and only takes effect if
> osi("Windows 2006") returns true) ... its only my guess thats its hal
> who is doing it (with debug="0xffff" on fujitsu_laptop I can verify that
> somebody writes to /sys/class/backlight/fujitsu-laptop/brightness and
> since it also happens without running windowing environment I suspect
> hal). On the S6410 the acpi_osi="!Windows 2006" case probably resembled
> the S7020 behavior.

Could you please re-test the S6410 with Thomas' latest patch revision?  I
tested the S7020 overnight and it seems to work fine with this revision but
I suspect the S6410 will still have trouble.  There were some logic fixes in
this latest version though so a retest is probably worth it.

Regards
  jonathan

Re: [PATCH 5/9] fujitsu-laptop: fingers off backlight if video.ko

[nokos]

Hi Jonathan

Am Freitag, den 18.07.2008, 09:37 +0930 schrieb Jonathan Woithe: 
> Hi Peter
> 
> > Am Mittwoch, den 16.07.2008, 09:25 +0930 schrieb Jonathan Woithe:
> > > > > It is likely that everything works out with video.ko then.
> > > > > If not, with a patch I sent recently you should switch off Windows osi strings 
> > > > > with:
> > > > > acpi_osi=windows_false   (preferred)
> > > > > or (already works)
> > > > > acpi_osi="!Windows 2006"
> > > > 
> > > > disabling windows_osi disables backlight through video.ko and everything
> > > > works fine with fujitsu_laptop (also brightness-buttons since ACPI then
> > > > generates events on the fujitsu device) but some other ACPI stuff is
> > > > disabled too (extended battery status etc...)
> > > 
> > > As per my previous email, the fact that this works is probably because in
> > > this instance acpi_video_backlight_support() returns false and allows
> > > fujitsu-laptop to set up its backlight control functions.
> :
> > This behavior seems to be a recent change to the ACPI BIOS
> > implementation (its in the VIST table, and only takes effect if
> > osi("Windows 2006") returns true) ... its only my guess thats its hal
> > who is doing it (with debug="0xffff" on fujitsu_laptop I can verify that
> > somebody writes to /sys/class/backlight/fujitsu-laptop/brightness and
> > since it also happens without running windowing environment I suspect
> > hal). On the S6410 the acpi_osi="!Windows 2006" case probably resembled
> > the S7020 behavior.
> 
> Could you please re-test the S6410 with Thomas' latest patch revision?  I
> tested the S7020 overnight and it seems to work fine with this revision but
> I suspect the S6410 will still have trouble.  There were some logic fixes in
> this latest version though so a retest is probably worth it.

I have tested the lastest patch revision and now it works ... 
acpi_video_backlight_support() returns false and only the fujitsu driver
takes care of manipulating the brightness. Brightness buttons get reported by
video.ko. I don't exactly know if its only the new patchset since I also used 
a fresh pull of the kernel as basis.
Interestingly with acpi debug i get

Jul 21 12:16:27 kernel: nsutils-0454 [00] ns_build_internal_name: Returning [ffff88012d37a4b0] (rel) "_BCL"
Jul 21 12:16:27 kernel: nsaccess-0406 [00] ns_lookup             : Searching relative to prefix scope [LCD_] (ffff88012fc16000)
Jul 21 12:16:27 kernel: nsaccess-0516 [00] ns_lookup             : Simple Pathname (1 segment, Flags=2)
Jul 21 12:16:27 kernel: nsdump-0087 [00] ns_print_pathname     : [_BCL]
Jul 21 12:16:27 kernel: nssearch-0109 [00] ns_search_one_scope   : Searching \_SB_.PCI0.GFX0.LCD_ (ffff88012fc16000) For [_BCL] (Untyped)
Jul 21 12:16:27 kernel: nssearch-0144 [00] ns_search_one_scope   : Name [_BCL] (Method) ffff88012fc4e6c0 found in scope [LCD_] ffff88012fc16000
Jul 21 12:16:27 kernel: nsutils-0454 [00] ns_build_internal_name: Returning [ffff88012d37a4b0] (rel) "_BCM"
Jul 21 12:16:27 kernel: nsaccess-0406 [00] ns_lookup             : Searching relative to prefix scope [LCD_] (ffff88012fc16000)
Jul 21 12:16:27 kernel: nsaccess-0516 [00] ns_lookup             : Simple Pathname (1 segment, Flags=2)
Jul 21 12:16:27 kernel: nsdump-0087 [00] ns_print_pathname     : [_BCM]
Jul 21 12:16:27 kernel: nssearch-0109 [00] ns_search_one_scope   : Searching \_SB_.PCI0.GFX0.LCD_ (ffff88012fc16000) For [_BCM] (Untyped)
Jul 21 12:16:27 kernel: nssearch-0144 [00] ns_search_one_scope   : Name [_BCM] (Method) ffff88012fc4e6e0 found in scope [LCD_] ffff88012fc16000

so acpi somehow finds BCL, BCM etc. but nevertheless does video_detect report no backlight capability.

Peter

> Regards
>   jonathan
> --
> 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 5/9] fujitsu-laptop: fingers off backlight if video.ko

[Thomas Renninger]

On Thursday 24 July 2008 02:12:47 nokos@gmx.net wrote:
> Hi Jonathan
>
> Am Freitag, den 18.07.2008, 09:37 +0930 schrieb Jonathan Woithe:
> > Hi Peter
> >
> > > Am Mittwoch, den 16.07.2008, 09:25 +0930 schrieb Jonathan Woithe:
> > > > > > It is likely that everything works out with video.ko then.
> > > > > > If not, with a patch I sent recently you should switch off
> > > > > > Windows osi strings with:
> > > > > > acpi_osi=windows_false   (preferred)
> > > > > > or (already works)
> > > > > > acpi_osi="!Windows 2006"
> > > > >
> > > > > disabling windows_osi disables backlight through video.ko and
> > > > > everything works fine with fujitsu_laptop (also brightness-buttons
> > > > > since ACPI then generates events on the fujitsu device) but some
> > > > > other ACPI stuff is disabled too (extended battery status etc...)
> > > >
> > > > As per my previous email, the fact that this works is probably
> > > > because in this instance acpi_video_backlight_support() returns false
> > > > and allows fujitsu-laptop to set up its backlight control functions.
> > >
> > > This behavior seems to be a recent change to the ACPI BIOS
> > > implementation (its in the VIST table, and only takes effect if
> > > osi("Windows 2006") returns true) ... its only my guess thats its hal
> > > who is doing it (with debug="0xffff" on fujitsu_laptop I can verify
> > > that somebody writes to /sys/class/backlight/fujitsu-laptop/brightness
> > > and since it also happens without running windowing environment I
> > > suspect hal). On the S6410 the acpi_osi="!Windows 2006" case probably
> > > resembled the S7020 behavior.
> >
> > Could you please re-test the S6410 with Thomas' latest patch revision?  I
> > tested the S7020 overnight and it seems to work fine with this revision
> > but I suspect the S6410 will still have trouble.  There were some logic
> > fixes in this latest version though so a retest is probably worth it.
>
> I have tested the lastest patch revision and now it works ...
> acpi_video_backlight_support() returns false and only the fujitsu driver
> takes care of manipulating the brightness. Brightness buttons get reported
> by video.ko. I don't exactly know if its only the new patchset since I also
> used a fresh pull of the kernel as basis.
> Interestingly with acpi debug i get
>
> Jul 21 12:16:27 kernel: nsutils-0454 [00] ns_build_internal_name: Returning
> [ffff88012d37a4b0] (rel) "_BCL" Jul 21 12:16:27 kernel: nsaccess-0406 [00]
> ns_lookup             : Searching relative to prefix scope [LCD_]
> (ffff88012fc16000) Jul 21 12:16:27 kernel: nsaccess-0516 [00] ns_lookup    
>         : Simple Pathname (1 segment, Flags=2) Jul 21 12:16:27 kernel:
> nsdump-0087 [00] ns_print_pathname     : [_BCL] Jul 21 12:16:27 kernel:
> nssearch-0109 [00] ns_search_one_scope   : Searching \_SB_.PCI0.GFX0.LCD_
> (ffff88012fc16000) For [_BCL] (Untyped) Jul 21 12:16:27 kernel:
> nssearch-0144 [00] ns_search_one_scope   : Name [_BCL] (Method)
> ffff88012fc4e6c0 found in scope [LCD_] ffff88012fc16000 Jul 21 12:16:27
> kernel: nsutils-0454 [00] ns_build_internal_name: Returning
> [ffff88012d37a4b0] (rel) "_BCM" Jul 21 12:16:27 kernel: nsaccess-0406 [00]
> ns_lookup             : Searching relative to prefix scope [LCD_]
> (ffff88012fc16000) Jul 21 12:16:27 kernel: nsaccess-0516 [00] ns_lookup    
>         : Simple Pathname (1 segment, Flags=2) Jul 21 12:16:27 kernel:
> nsdump-0087 [00] ns_print_pathname     : [_BCM] Jul 21 12:16:27 kernel:
> nssearch-0109 [00] ns_search_one_scope   : Searching \_SB_.PCI0.GFX0.LCD_
> (ffff88012fc16000) For [_BCM] (Untyped) Jul 21 12:16:27 kernel:
> nssearch-0144 [00] ns_search_one_scope   : Name [_BCM] (Method)
> ffff88012fc4e6e0 found in scope [LCD_] ffff88012fc16000
>
> so acpi somehow finds BCL, BCM etc. but nevertheless does video_detect
> report no backlight capability.

That's odd, with the latest patcheset video.ko should be used.
After checking acpidump: Your machine has IGD support and should work with 
Matthew's latest patches (and might break with them if you still load fujitsu 
and video driver without my patches).

Maybe you had not used the very latest version of my patches, there I ripped 
out the IGD check. Can you double check the subject of the initial mail PATCH 
0/X].
Sorry, I only sent the last, 3rd version to Andi and the list:
Check for ACPI backlight support otherwise use vendor ACPI drivers - version 3

But you should really try linux-next or 2.6.27-rcX with only video.ko driver 
loaded and my patchset. This could be the first Fujitsu with IGD 
running/tested on Linux.

Hopefully my patches also find their way upstream, else Matthew's parts 
probably will break with vendor acpi drivers.

        Thomas