Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

[Thomas Renninger]

Changes to last version:
  - I found some ugly typos mixing up ands and ors on the acpi_video_support
    bit mask -> fixed.
  - moved video capabilities checking from pci_root.c to the backlight/display
    switching modules. This is important to not search for capabilities where
    no graphics device is found and video.ko or vendor drivers (most through
    dmi modalias) do not load anyway. The real search is only done by the first
    video capable module invoking acpi_video_backlight_support() or
    acpi_video_display_switch_support(). Further calls to these functions
    return a cached value. This change was mainly moving function invokations
    the previous version was already designed in the right way, I somehow
    was fixed to the idea that checking must be done before module load time,
    but this turned out to be wrong.
  - Corrected messages/comments in thinkpad_acpi.c
  - Removed dmi check for Dells
  - Fixed a section mismatch compile message in the fujitsu driver

Most logic is in the second patch where video_detect.c is introduced.
I mean, there could be a typo in one of the vendor specific drivers,
it would be great if the maintainers of each driver could have a quick
look at the code (did I forget to free an already alloced backlight
struct or similar?).

It is highly recommended that these patches are tried out together with
Matthew's/Hong's IGD driver changes.
I do not know whether they are already in the dri tree or in linux-next,
whether they will be accepted for .27 etc. I hope Matthew will answer on
this mail and tell us.
AFAIK on thinkpads they already need an additional patch to work?
Chances are high that other laptop families also need a bit of a special
treatment to get the very new IGD parts to work.
If people test both patches together we could get a matrix like this:

          IGD/video.ko/Vista way   |  vendor specific, XP way
Dell    |           -              |             X
HP      |           -              |             ?
Lenovo  |           X              |             X
MSI     |           -              |             ?
Toshib  |           -              |             -
Fujitsu |           -              |             X
...     |           -              |

Above is more or less guessed, also everybody should tell about its
graphics card.
Intel graphics cards sooner or later must be switched through video.ko
(those and only those are intended by BIOSes to be IGD driven
already or in the near future (-> Matthew's/Hong's patch needed))
On Nvidia or AMD the binary only driver may take over switching
(it doesn't harm to still load and use video.ko then as they disable
 IO space, I expect/hope this will change at some time). Still if you
switch to console backlight can/must be switched through ACPI again
also on these.

Only Vista capable BIOSes are supposed to be switched via video.ko, but
those should sooner or later be switched through it, every Vista capable
BIOS shows generic ACPI backlight functions.


Off topic (a bit) - Toshiba only:
On Toshibas video.ko will not work.
They miss the third generic ACPI backlight function to query the current
brightness level (_BQC).
Zhao Yakui provided a patch ignoring this on module load time.
This resulted in a black screen on startup because of an initial
backlight state of zero.

IMO his patch was the right way and even Linus reverted it, trying again
is worth it:

commit 797de7bdb253624c16144f40b72ec65d63cdcca2
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sat Apr 5 12:14:13 2008 -0700

    Revert "ACPI: Ignore _BQC object when registering backlight device"

Ok, I followed up the bug link, there already is an updated fixed patch in bug:
http://bugzilla.kernel.org/show_bug.cgi?id=10206
attachment is:
http://bugzilla.kernel.org/attachment.cgi?id=16310&action=view

Zhao, could you repost this based on top of these, pls

[PATCH 01/11] ACPI: video: Ignore devices that aren't present in hardware

[Thomas Renninger]

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>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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

[PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Thomas Renninger]

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>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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         |  258 +++++++++++++++++++++++++++++++++++
 include/linux/acpi.h                |   45 ++++++
 7 files changed, 344 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..0533e0d 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..68abba7
--- /dev/null
+++ b/drivers/acpi/video_detect.c
@@ -0,0 +1,258 @@
+/*
+ * 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;
+static bool acpi_video_caps_checked;
+
+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);
+	}
+	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_handle)
+{
+	long caps = 0;
+	struct acpi_device *tmp_dev;
+	acpi_status status;
+
+	if (acpi_video_caps_checked && graphics_handle == NULL)
+		return acpi_video_support;
+
+	if (!graphics_handle) {
+		/* Only do the global walk through all graphics devices once */
+		acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+				    ACPI_UINT32_MAX, find_video,
+				    &caps, NULL);
+		/* There might be boot param flags set already... */
+		acpi_video_support |= caps;
+		acpi_video_caps_checked = 1;
+		/* Add blacklists here. Be careful to use the right *DMI* bits
+		 * to still be able to override logic via boot params, e.g.:
+		 *
+		 *   if (dmi_name_in_vendors("XY")) {
+		 *	acpi_video_support |=
+		 *		ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR;
+		 *	acpi_video_support |=
+		 *		ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
+		 *}
+		 */
+	} else {
+		status = acpi_bus_get_device(graphics_handle, &tmp_dev);
+		if (ACPI_FAILURE(status)) {
+			ACPI_EXCEPTION((AE_INFO, status, "Invalid device"));
+			return 0;
+		}
+		acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_handle,
+				    ACPI_UINT32_MAX, find_video,
+				    &caps, NULL);
+	}
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support %s %s\n",
+			  graphics_handle ? caps : acpi_video_support,
+			  graphics_handle ? "on device " : "in general",
+			  graphics_handle ? acpi_device_bid(tmp_dev) : ""));
+	return caps;
+}
+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

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Matthew Garrett]

There's no need to do the IGD flagging (the drm driver probes it 
properly), but the rest looks good.

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

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Sergio Monteiro Basto]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

Hi, this patches are the a solution on i915 with backlight problems and
others ?

like 

https://bugzilla.redhat.com/show_bug.cgi?id=351661

https://bugzilla.redhat.com/show_bug.cgi?id=389541

https://bugzilla.redhat.com/show_bug.cgi?id=201472

https://bugzilla.redhat.com/show_bug.cgi?id=423931

https://bugs.freedesktop.org/show_bug.cgi?id=13709

https://bugs.freedesktop.org/show_bug.cgi?id=12852

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

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

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


On Wed, 2008-07-16 at 12:16 +0100, Matthew Garrett wrote:
> There's no need to do the IGD flagging (the drm driver probes it 
> properly), but the rest looks good.
> 
-- 
Sérgio M. B.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2192 bytes --]

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Thomas Renninger]

On Wednesday 16 July 2008 15:01:27 Sergio Monteiro Basto wrote:
> Hi, this patches are the a solution on i915 with backlight problems and
> others ?
>
> like
>
> https://bugzilla.redhat.com/show_bug.cgi?id=351661
Here you probably need the IGD parts also.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=389541
Some machines need a switch to console 1 and back to X.
Don't ask me what is missing and what X could do here 
automatically. We have a workaround to just switch the
displays after suspend (not sure whether it's on by default)
but it's a config variable.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=201472
No idea, nothing with backlight, X problem?
>
> https://bugzilla.redhat.com/show_bug.cgi?id=423931
It's a Dell..., is the dcdbas driver involved?
See discussion on my previous posts with Matthew and
the Dell hal bug. Could be related..
>
> https://bugs.freedesktop.org/show_bug.cgi?id=13709
Sounds worth a test.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=12852
This also could be related.

> http://bugzilla.kernel.org/show_bug.cgi?id=10985
unrelated.
>
> http://bugzilla.kernel.org/show_bug.cgi?id=9642
No idea. I have a bug where a lot HP people reported a freeze
when using the LID. They should be lucky that only backlight
is dark :).
Oh wait, _DOS=1 works, _DOS=0 fails -> probably the same
issue. This one is nasty and very critical (a lot HPs freeze), but I did
not have time to look at anything.
>
> http://bugzilla.kernel.org/show_bug.cgi?id=9827
Defintely worth a test with IGD enhancements.


I just had a quick look on each, do not give much on above statements.
If there is any outcome on 9642 (_DOS=1 works, _DOS=0 fails on HPs and LID), 
pls let me know.

        Thomas

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Sergio Monteiro Basto]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]


On Thu, 2008-07-17 at 15:52 +0200, Thomas Renninger wrote:
> I just had a quick look on each, do not give much on above statements.
> If there is any outcome on 9642 (_DOS=1 works, _DOS=0 fails on HPs and
> LID), 
> pls let me know.

9642 , is about my laptop, I will happily try yours patches , and report
back 

thanks, for reply 

-- 
Sérgio M. B.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2192 bytes --]

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Thomas Renninger]

On Friday 18 July 2008 05:18:32 am Sergio Monteiro Basto wrote:
> On Thu, 2008-07-17 at 15:52 +0200, Thomas Renninger wrote:
> > I just had a quick look on each, do not give much on above statements.
> > If there is any outcome on 9642 (_DOS=1 works, _DOS=0 fails on HPs and
> > LID),
> > pls let me know.
>
> 9642 , is about my laptop, I will happily try yours patches , and report
> back
Wait..., these patches are unrelated to this one.
I meant: if you come further and find out something, pls tell me.
(Theoretically we could blacklist them easily to not use display output 
switching with the patch, but better still try a bit).
I know about a lot HP and others having problems with _DOS=0 and freezing on 
LID:
https://bugzilla.novell.com/show_bug.cgi?id=378917 (-> a lot duplicates)
https://bugzilla.novell.com/show_bug.cgi?id=290219

        Thomas

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Sergio Monteiro Basto]

[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]


On Fri, 2008-07-18 at 04:54 +0200, Thomas Renninger wrote:
> On Friday 18 July 2008 05:18:32 am Sergio Monteiro Basto wrote:
> > On Thu, 2008-07-17 at 15:52 +0200, Thomas Renninger wrote:
> > > I just had a quick look on each, do not give much on above statements.
> > > If there is any outcome on 9642 (_DOS=1 works, _DOS=0 fails on HPs and
> > > LID),
> > > pls let me know.
> >
> > 9642 , is about my laptop, I will happily try yours patches , and report
> > back
> Wait..., these patches are unrelated to this one.

ok 

> I meant: if you come further and find out something, pls tell me.
> (Theoretically we could blacklist them easily to not use display output 
> switching with the patch, but better still try a bit).
> I know about a lot HP and others having problems with _DOS=0 and freezing on 
> LID:
> https://bugzilla.novell.com/show_bug.cgi?id=378917 (-> a lot duplicates)
> https://bugzilla.novell.com/show_bug.cgi?id=290219

the bug 
http://bugzilla.kernel.org/show_bug.cgi?id=6001 
explain why _DOS is, by default, 0 and not 1. 

Has I write there ,
http://bugzilla.kernel.org/show_bug.cgi?id=6001#c47 
we should back to DOS=1 by default

Seems that you have same opinion that we should set _DOS to 1, by
default. 

I patch the kernel myself , because fix the problem on my compaq
nx6110 , the other Sérgio on bug 6001 , don't reply ...  

Thanks, 
-- 
Sérgio M. B.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2192 bytes --]

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Zhao Yakui]

On Wed, 2008-07-16 at 12:52 +0200, Thomas Renninger wrote:
> 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.

IMO this is OK and reasonable.
> 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>
> Signed-off-by: Thomas Renninger <trenn@stravinsky.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         |  258 +++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h                |   45 ++++++
>  7 files changed, 344 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..0533e0d 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..68abba7
> --- /dev/null
> +++ b/drivers/acpi/video_detect.c
> @@ -0,0 +1,258 @@
> +/*
> + * 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;
> +static bool acpi_video_caps_checked;
> +
> +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 aacpi_is_video_device 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);
> +	}
> +	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_handle)
> +{
> +	long caps = 0;
> +	struct acpi_device *tmp_dev;
> +	acpi_status status;
> +
> +	if (acpi_video_caps_checked && graphics_handle == NULL)
> +		return acpi_video_support;
> +
> +	if (!graphics_handle) {
> +		/* Only do the global walk through all graphics devices once */
> +		acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> +				    ACPI_UINT32_MAX, find_video,
> +				    &caps, NULL);
> +		/* There might be boot param flags set already... */
> +		acpi_video_support |= caps;
> +		acpi_video_caps_checked = 1;
> +		/* Add blacklists here. Be careful to use the right *DMI* bits
> +		 * to still be able to override logic via boot params, e.g.:
> +		 *
> +		 *   if (dmi_name_in_vendors("XY")) {
> +		 *	acpi_video_support |=
> +		 *		ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR;
> +		 *	acpi_video_support |=
> +		 *		ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> +		 *}
> +		 */
> +	} else {
> +		status = acpi_bus_get_device(graphics_handle, &tmp_dev);
> +		if (ACPI_FAILURE(status)) {
> +			ACPI_EXCEPTION((AE_INFO, status, "Invalid device"));
> +			return 0;
> +		}
> +		acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_handle,
> +				    ACPI_UINT32_MAX, find_video,
> +				    &caps, NULL);
> +	}
> +	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support %s %s\n",
> +			  graphics_handle ? caps : acpi_video_support,
> +			  graphics_handle ? "on device " : "in general",
> +			  graphics_handle ? acpi_device_bid(tmp_dev) : ""));
> +	return caps;
> +}
> +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);

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Zhang Rui]

On Wed, 2008-07-16 at 18:52 +0800, Thomas Renninger wrote:

> 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.
> -        */
This comment is wrong.
CID for ACPI video bus device is LNXVIDEO.

thanks,
rui

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

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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

Re: [PATCH 03/11] Acer-WMI: fingers off backlight if video.ko is serving this functionality

[Carlos Corbacho]

On Wednesday 16 July 2008 11:52:25 Thomas Renninger wrote:
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Thomas Renninger <trenn@stravinsky.suse.de>

Acked-by: Carlos Corbacho <carlos@strangeworlds.co.uk>

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



-- 
E-Mail: carlos@strangeworlds.co.uk
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

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

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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 05/11] eeepc-laptop: fingers off backlight if video.ko is serving this functionality

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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 06/11] fujitsu-laptop: fingers off backlight if video.ko is serving this functionality

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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..4f873cb 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

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

[Jonathan Woithe]

I will try to get some time to test this version tonight on the S7020, with
and without the IGD stuff.

Regards
  jonathan

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

[Jonathan Woithe]

Hi Thomas

I managed to test this series of patches overnight.  I applied them to
2.6.26 but in the context I don't *think* that matters a great deal.  I
didn't go down the IDG route since there's no clear indication of what
patches are needed for this.

The practical upshot is that on the S7020 a kernel with just these patches
applied can still control the backlight via the backlight interface provided
by the platform driver.  video.ko still cannot control the backlight but I
wasn't expecting it to for reasons outlined previously.  At least on the
S7020 acpi_video_backlight_support() seems to (correctly) return false,
allowing the fujitsu-laptop module to pick up the backlight control as it's
always done.

I realise that the latest revision of this patch series included some logic
updates in acpi_video_backlight_support() so I would be interested to see
whether the S6410 now works.  I suspect it won't but it's worth a try.

Regards
  jonathan

> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Thomas Renninger <trenn@stravinsky.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..4f873cb 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 07/11] fujitsu-laptop: Fix section mismatch

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.suse.de>
---
 drivers/misc/fujitsu-laptop.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/fujitsu-laptop.c b/drivers/misc/fujitsu-laptop.c
index 4f873cb..754ec38 100644
--- a/drivers/misc/fujitsu-laptop.c
+++ b/drivers/misc/fujitsu-laptop.c
@@ -455,7 +455,7 @@ static int dmi_check_cb_s6410(const struct dmi_system_id *id)
 	return 0;
 }
 
-static struct dmi_system_id __initdata fujitsu_dmi_table[] = {
+static struct dmi_system_id fujitsu_dmi_table[] = {
 	{
 	 .ident = "Fujitsu Siemens",
 	 .matches = {
-- 
1.5.4.5

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

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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 09/11] sony-laptop: fingers off backlight if video.ko is serving this functionality

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.suse.de>
---
 drivers/misc/thinkpad_acpi.c |   77 ++++++++++++++++++++++++++++++++++-------
 1 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index b596929..d3ec19e 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,43 @@ 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 ACPI backlight interface "
+				       "available, but thinkpad_acpi driver "
+				       "will take over control\n");
+			}
+			/* We have an Integrated Graphics Device and BIOS
+			 * won't switch. By default the IGD driver should take
+			 * care about backlight and display output switching.
+			 * People can still run into this code path and let
+			 * thinkpad_acpi take over control by passing:
+			 * acpi_backlight=vendor or acpi_display_output=vendor
+			 * boot params.
+			 */
+			acpi_video_support = acpi_video_get_capabilities(NULL);
+			vdbg_printk(TPACPI_DBG_INIT, "XXXXXk\n");
+
+			if (acpi_video_support & ACPI_VIDEO_IGD) {
+				vdbg_printk(TPACPI_DBG_INIT, "IGD device"
+					    " detected - take over backlight"
+					    " switching\n");
+				tp_features.bright_igdmode = 1;
+			}
+ 		}
 	}
 
 	if (!brightness_enable) {
-- 
1.5.4.5

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Matthew Garrett]

I still don't think we need the IGD code here.

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

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Thomas Renninger]

On Wednesday 16 July 2008 13:20:27 Matthew Garrett wrote:
> I still don't think we need the IGD code here.

But it does not hurt either.
There is so much "ugly work around" code in there..., I promise to send 
something and clean it up when things work out.

We need a pointer to the IGD patches...

       Thomas

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Henrique de Moraes Holschuh]

On Wed, 16 Jul 2008, Thomas Renninger wrote:
> @@ -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);
> +			}
> +

NAK.   Sorry I didn't look it over in detail the last time you posted
this.

As per my golden rule for thinkpad-acpi (always have some way to let the
user disable such stuff, because it WILL break in some thinkpad), these
need to be configurable as a module parameter.

Extending brightness_mode (it is a bitmap) to let one force igd_mode
on and off would solve this detail.

Also, usually I would veto such automatic hotkey handling in the driver,
but since the firmware ALREADY does it on every IBM thinkpad anyway (and
I can't fix it since hotpatching DSDT in code is absolutely forbidden),
the driver has all facilities needed to deal with it.

One question: is there a good reason to actually have the hotkey
handling for IGD mode directly in thinkpad-acpi?  We don't do it for
Vista BIOS, video.ko doesn't do it either (anymore)... and I would make
it configurable for IBM firmware if I could.

Anyway, that means hotkey-driven igd_mode brightness change, if handled
by thinkpad-acpi without going through userspace, HAS to mask off the
relevant hotkeys in hotkey_init and set to KEY_RESERVED the relevant
keymap entries.   The code is there already, but it does mean you
need to factor out brightness_mode autodetection into a function
of its own, and have it run BEFORE hotkey_init and brightness_init
have a chance to run, because both will have to be in sync.

If you'd rather not have all that work, just add minimal support for igd
and the video stuff to thinkpad-acpi so that it doesn't get in the way
of your work, and I will follow up with whatever else enhancements are
needed.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Matthew Garrett]

On Wed, Jul 16, 2008 at 09:40:44AM -0300, Henrique de Moraes Holschuh wrote:

> One question: is there a good reason to actually have the hotkey
> handling for IGD mode directly in thinkpad-acpi?  We don't do it for
> Vista BIOS, video.ko doesn't do it either (anymore)... and I would make
> it configurable for IBM firmware if I could.

No. There's no reason to have IGD handling in thinkpad-acpi at all.

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

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Henrique de Moraes Holschuh]

On Wed, 16 Jul 2008, Matthew Garrett wrote:
> On Wed, Jul 16, 2008 at 09:40:44AM -0300, Henrique de Moraes Holschuh wrote:
> > One question: is there a good reason to actually have the hotkey
> > handling for IGD mode directly in thinkpad-acpi?  We don't do it for
> > Vista BIOS, video.ko doesn't do it either (anymore)... and I would make
> > it configurable for IBM firmware if I could.
> 
> No. There's no reason to have IGD handling in thinkpad-acpi at all.

Then please leave that part out of the final patch.  It will make the
patch a LOT simpler (no need to muck with anything hotkey_*).

But the observation about extending brightness_mode to
force-select/unselect igdmode stands anyway.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Thomas Renninger]

On Wednesday 16 July 2008 15:38:39 Henrique de Moraes Holschuh wrote:
> On Wed, 16 Jul 2008, Matthew Garrett wrote:
> > On Wed, Jul 16, 2008 at 09:40:44AM -0300, Henrique de Moraes Holschuh 
wrote:
> > > One question: is there a good reason to actually have the hotkey
> > > handling for IGD mode directly in thinkpad-acpi?  We don't do it for
> > > Vista BIOS, video.ko doesn't do it either (anymore)... and I would make
> > > it configurable for IBM firmware if I could.
> >
> > No. There's no reason to have IGD handling in thinkpad-acpi at all.
I haven't seen the video driver on a T61 with Intel card doing proper 
backlight switching on the correct device yet.

> Then please leave that part out of the final patch.  It will make the
> patch a LOT simpler (no need to muck with anything hotkey_*).
Ok. Attached.

> But the observation about extending brightness_mode to
> force-select/unselect igdmode stands anyway.
Ok, that's fair enough.

I wonder whether we still need the BCL queries in thinkpad_acpi.c?
Shouldn't (at least in theory) all ThinkPads with generic backlight
functions work with video.ko now?

Maybe now it's a bit too soon, but getting rid of these functions would
be a nice cleanup.

Below one is compile tested...

        Thomas

----
thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

Signed-off-by: Thomas Renninger <trenn@suse.de>

diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index b596929..50da049 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -4750,17 +4750,26 @@ 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 ACPI backlight interface not "
+				       "available, thinkpad_acpi driver "
+				       "will take over control\n");
+			}
+ 		}
 	}
 
 	if (!brightness_enable) {

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Matthew Garrett]

On Wed, Jul 16, 2008 at 05:27:55PM +0200, Thomas Renninger wrote:
> On Wednesday 16 July 2008 15:38:39 Henrique de Moraes Holschuh wrote:
> > On Wed, 16 Jul 2008, Matthew Garrett wrote:
> > > On Wed, Jul 16, 2008 at 09:40:44AM -0300, Henrique de Moraes Holschuh 
> wrote:
> > > > One question: is there a good reason to actually have the hotkey
> > > > handling for IGD mode directly in thinkpad-acpi?  We don't do it for
> > > > Vista BIOS, video.ko doesn't do it either (anymore)... and I would make
> > > > it configurable for IBM firmware if I could.
> > >
> > > No. There's no reason to have IGD handling in thinkpad-acpi at all.
> I haven't seen the video driver on a T61 with Intel card doing proper 
> backlight switching on the correct device yet.

The patch is queued and will be in 2.6.27. It's fine.

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

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Yves-Alexis Perez]

On Wed, Jul 16, 2008 at 04:29:39PM +0100, Matthew Garrett wrote:
> On Wed, Jul 16, 2008 at 05:27:55PM +0200, Thomas Renninger wrote:
> > On Wednesday 16 July 2008 15:38:39 Henrique de Moraes Holschuh wrote:
> > > On Wed, 16 Jul 2008, Matthew Garrett wrote:
> > > > On Wed, Jul 16, 2008 at 09:40:44AM -0300, Henrique de Moraes Holschuh 
> > wrote:
> > > > > One question: is there a good reason to actually have the hotkey
> > > > > handling for IGD mode directly in thinkpad-acpi?  We don't do it for
> > > > > Vista BIOS, video.ko doesn't do it either (anymore)... and I would make
> > > > > it configurable for IBM firmware if I could.
> > > >
> > > > No. There's no reason to have IGD handling in thinkpad-acpi at all.
> > I haven't seen the video driver on a T61 with Intel card doing proper 
> > backlight switching on the correct device yet.
> 
> The patch is queued and will be in 2.6.27. It's fine.

Does this mean that linux-next + the 2 generic patches + (fixed)
thinkpad-acpi one would be enough to test?

Cheers,
-- 
Yves-Alexis

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Andi Kleen]

Matthew Garrett wrote:
> On Wed, Jul 16, 2008 at 05:27:55PM +0200, Thomas Renninger wrote:
>> On Wednesday 16 July 2008 15:38:39 Henrique de Moraes Holschuh wrote:
>>> On Wed, 16 Jul 2008, Matthew Garrett wrote:
>>>> On Wed, Jul 16, 2008 at 09:40:44AM -0300, Henrique de Moraes Holschuh 
>> wrote:
>>>>> One question: is there a good reason to actually have the hotkey
>>>>> handling for IGD mode directly in thinkpad-acpi?  We don't do it for
>>>>> Vista BIOS, video.ko doesn't do it either (anymore)... and I would make
>>>>> it configurable for IBM firmware if I could.
>>>> No. There's no reason to have IGD handling in thinkpad-acpi at all.
>> I haven't seen the video driver on a T61 with Intel card doing proper 
>> backlight switching on the correct device yet.
> 
> The patch is queued and will be in 2.6.27. It's fine.

I admit I have lost track of the state of these patches. If there's an 
agreed subset of patches that should go in post .27 via ACPI please 
resend them for review.

Thanks,

-Andi

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Matthew Garrett]

On Thu, Jul 17, 2008 at 11:10:51AM +0200, Andi Kleen wrote:
> Matthew Garrett wrote:
> >The patch is queued and will be in 2.6.27. It's fine.
> 
> I admit I have lost track of the state of these patches. If there's an 
> agreed subset of patches that should go in post .27 via ACPI please 
> resend them for review.

The opregion brightness control code is part of the DRM tree, not ACPI. 
It ought to be entirely orthogonal to Thomas' patches.

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

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Yves-Alexis Perez]

On Thu, Jul 17, 2008 at 11:46:22AM +0100, Matthew Garrett wrote:
> The opregion brightness control code is part of the DRM tree, not ACPI. 
> It ought to be entirely orthogonal to Thomas' patches.

For an "end user", what does this mean? What patches should we apply,
against which tree, in order to be able to test and validate Thomas's
patches?

Cheers,
-- 
Yves-Alexis

Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

[Matthew Garrett]

On Thu, Jul 17, 2008 at 01:18:08PM +0200, Yves-Alexis Perez wrote:
> On Thu, Jul 17, 2008 at 11:46:22AM +0100, Matthew Garrett wrote:
> > The opregion brightness control code is part of the DRM tree, not ACPI. 
> > It ought to be entirely orthogonal to Thomas' patches.
> 
> For an "end user", what does this mean? What patches should we apply,
> against which tree, in order to be able to test and validate Thomas's
> patches?

Easiest is to wait for the DRM tree to be merged. The patch is against 
DRM master, which doesn't apply directly to Linux - it needs some 
post-processing first.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

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

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <trenn@stravinsky.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: Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

[Thomas Renninger]

Hi Yves,

I hope it's ok for that I add the people to CC again.
Others might have the same question(s).

On Wednesday 16 July 2008 12:59:31 Yves-Alexis Perez wrote:
> On Wed, Jul 16, 2008 at 12:52:22PM +0200, Thomas Renninger wrote:
> > It is highly recommended that these patches are tried out together with
> > Matthew's/Hong's IGD driver changes.
> > I do not know whether they are already in the dri tree or in linux-next,
> > whether they will be accepted for .27 etc. I hope Matthew will answer on
> > this mail and tell us.
> > AFAIK on thinkpads they already need an additional patch to work?
> > Chances are high that other laptop families also need a bit of a special
> > treatment to get the very new IGD parts to work.
> > If people test both patches together we could get a matrix like this:
>
> Hmmh, running on a Thinkpad T61 with intel
There are T61 with Intel and with Nvidia graphics cards.
The first may have (on all recent ThinkPad BIOS they should all have) an IGD 
interface provided by BIOS.
These are the generic ACPI functions plus a bit more on top.

> and a vista-capable bios, 
Since Vista all laptops seem to have the generic ACPI backlight functions 
added. Goal is to use them whenever possible instead of vendor specific ones.

> what does this mean? Which patches do I need, and against which tree?
Matthew has to tell us. I have no idea.

> I guess I at least need to apply those 11 patches against acpi tree,
Yes. Andi's acpi release branch.
It is enough to take the first 2 generic and your laptop model specific one(s) 
if you are lazy.
> but  
> what are the "Matthew's/Hong's IGD driver changes" and the "additional
> patch to work"?
I saw Matthew posting on the dri list. But he said about problems with 750 ms 
delays which he could solve.
Therefore I do not want to point people to wrong patches.
All I know is that Matthew got things working as they should work.

Thanks for the feedback!

       Thomas

Re: Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

[Yves-Alexis Perez]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On mer, 2008-07-16 at 12:52 +0200, Thomas Renninger wrote:
> It is highly recommended that these patches are tried out together
> with
> Matthew's/Hong's IGD driver changes.
> I do not know whether they are already in the dri tree or in
> linux-next,
> whether they will be accepted for .27 etc. I hope Matthew will answer
> on
> this mail and tell us.

Ok, I tried to apply them to linux-next/stable (which should be 2.6.26,
I guess) and it didn't work. Patches apply fine, but the kernel is
unbootable (I guess it's the same error than previously, but didn't yet
rebuild with ACPI_DEBUG). Patch doesn't apply to linux-next/master.
-- 
Yves-Alexis

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

[Yves-Alexis Perez]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

On mer, 2008-07-16 at 12:52 +0200, Thomas Renninger wrote:
> It is highly recommended that these patches are tried out together with
> Matthew's/Hong's IGD driver changes.
> I do not know whether they are already in the dri tree or in linux-next,
> whether they will be accepted for .27 etc. I hope Matthew will answer on
> this mail and tell us.
> AFAIK on thinkpads they already need an additional patch to work?
> Chances are high that other laptop families also need a bit of a special
> treatment to get the very new IGD parts to work.
> If people test both patches together we could get a matrix like this:

What is the status of those patches wrt. 2.6.27-rc1?

Cheers,
-- 
Yves-Alexis

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

[Thomas Renninger]

On Wednesday 30 July 2008 08:29:37 Yves-Alexis Perez wrote:
> On mer, 2008-07-16 at 12:52 +0200, Thomas Renninger wrote:
> > It is highly recommended that these patches are tried out together with
> > Matthew's/Hong's IGD driver changes.
> > I do not know whether they are already in the dri tree or in linux-next,
> > whether they will be accepted for .27 etc. I hope Matthew will answer on
> > this mail and tell us.
> > AFAIK on thinkpads they already need an additional patch to work?
> > Chances are high that other laptop families also need a bit of a special
> > treatment to get the very new IGD parts to work.
> > If people test both patches together we could get a matrix like this:
>
> What is the status of those patches wrt. 2.6.27-rc1?

Matthew and Rui accepted or signed them off AFAIK.
On Peter's system (nokos@gmx.de, fujitsu with IGD support) it returns that
there is no video support which looks wrong.
I contacted him a bit late, after getting pointed to acpidump it still looks
odd/wrong, and I need acpi.debug_level=0x1f log_buf_len=16777216 output to 
hopefully see what it is.

A comment from Andi/Len would be great.
Maybe this could already be sent to linux-next to make testing easier.

      Thomas

PS: and this is version 2, I already sent version 3 which had some cleanups.

Re: Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

[Jonathan Woithe]

Hi Thomas

> > On mer, 2008-07-16 at 12:52 +0200, Thomas Renninger wrote:
> > > It is highly recommended that these patches are tried out together with
> > > Matthew's/Hong's IGD driver changes.
> > > I do not know whether they are already in the dri tree or in linux-next,
> > > whether they will be accepted for .27 etc. I hope Matthew will answer on
> > > this mail and tell us.
> > > AFAIK on thinkpads they already need an additional patch to work?
> > > Chances are high that other laptop families also need a bit of a special
> > > treatment to get the very new IGD parts to work.
> > > If people test both patches together we could get a matrix like this:
> >
> > What is the status of those patches wrt. 2.6.27-rc1?
> 
> Matthew and Rui accepted or signed them off AFAIK.
> On Peter's system (nokos@gmx.de, fujitsu with IGD support) it returns that
> there is no video support which looks wrong.
> I contacted him a bit late, after getting pointed to acpidump it still looks
> odd/wrong, and I need acpi.debug_level=0x1f log_buf_len=16777216 output to 
> hopefully see what it is.
> 
> A comment from Andi/Len would be great.
> Maybe this could already be sent to linux-next to make testing easier.
> :
> PS: and this is version 2, I already sent version 3 which had some cleanups.

I'm trying to sort out the status of this myself.  Is the expectation that
your patches *with* the IGD work from Matthew/Hong should mean that Fujitsus
implementing IGD should not need fujitsu-laptop in order to provide working
software backlight brightness control?  Or shouldn't Matthew/Hong's
patchset be necessary with your patch in theory?

AFAIK even IGD Fujitsus will need fujitsu-laptop to support other hotkeys,
so allowing fujitsu-laptop to co-exist with video.ko handling the backlight
(as your patches allow) is the right thing to do.

Regards
  jonathan

Re: Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

[Zhang Rui]

On Thu, 2008-07-31 at 08:35 +0800, Jonathan Woithe wrote:
> Hi Thomas
> 
> > > On mer, 2008-07-16 at 12:52 +0200, Thomas Renninger wrote:
> > > > It is highly recommended that these patches are tried out
> together with
> > > > Matthew's/Hong's IGD driver changes.
> > > > I do not know whether they are already in the dri tree or in
> linux-next,
> > > > whether they will be accepted for .27 etc. I hope Matthew will
> answer on
> > > > this mail and tell us.
> > > > AFAIK on thinkpads they already need an additional patch to
> work?
> > > > Chances are high that other laptop families also need a bit of a
> special
> > > > treatment to get the very new IGD parts to work.
> > > > If people test both patches together we could get a matrix like
> this:
> > >
> > > What is the status of those patches wrt. 2.6.27-rc1?
> >
> > Matthew and Rui accepted or signed them off AFAIK.
> > On Peter's system (nokos@gmx.de, fujitsu with IGD support) it
> returns that
> > there is no video support which looks wrong.
> > I contacted him a bit late, after getting pointed to acpidump it
> still looks
> > odd/wrong, and I need acpi.debug_level=0x1f log_buf_len=16777216
> output to
> > hopefully see what it is.
> >
> > A comment from Andi/Len would be great.
> > Maybe this could already be sent to linux-next to make testing
> easier.
> > :
> > PS: and this is version 2, I already sent version 3 which had some
> cleanups.
> 
> I'm trying to sort out the status of this myself.  Is the expectation
> that
> your patches *with* the IGD work from Matthew/Hong should mean that
> Fujitsus
> implementing IGD should not need fujitsu-laptop in order to provide
> working
> software backlight brightness control?
I think so.

>   Or shouldn't Matthew/Hong's
> patchset be necessary with your patch in theory?
Thomas' patch assigns the responsibility to acpi video driver or
platform drivers when a laptop supports brightness/display switch both
via ACPI video extension and vendor specific control methods.

brightness/display switch via ACPI video driver sometimes doesn't work
even if the ACPI control methods are available, and Matthew/Hong's patch
fixes this problem, on platforms with intel integrated graphics cards.

And now, we should root cause why acpi_video_backlight_support() failed
on this laptop.
> 
> AFAIK even IGD Fujitsus will need fujitsu-laptop to support other
> hotkeys,

yes, this patch set just prevents fujitsu-laptop from the
brightness/display switch hotkeys.

> so allowing fujitsu-laptop to co-exist with video.ko handling the
> backlight
> (as your patches allow) is the right thing to do.

sure, they will co-exist, but I don't think fujitsu-laptop should handle
the backlight stuff, unless it's explicitly stated via the boot
parameter "acpi_backlight=vendor".

thanks,
rui

--
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: Check for ACPI backlight support otherwise use vendor ACPI

[Jonathan Woithe]

> > I'm trying to sort out the status of this myself.  Is the expectation
> > that your patches *with* the IGD work from Matthew/Hong should mean that
> > Fujitsus implementing IGD should not need fujitsu-laptop in order to
> > provide working software backlight brightness control?
> I think so.

Ok.

> > Or shouldn't Matthew/Hong's patchset be necessary with your patch in
> > theory?
> Thomas' patch assigns the responsibility to acpi video driver or
> platform drivers when a laptop supports brightness/display switch both
> via ACPI video extension and vendor specific control methods.
> 
> brightness/display switch via ACPI video driver sometimes doesn't work
> even if the ACPI control methods are available, and Matthew/Hong's patch
> fixes this problem, on platforms with intel integrated graphics cards.

I see.

> And now, we should root cause why acpi_video_backlight_support() failed
> on this laptop.

Agreed.

> > so allowing fujitsu-laptop to co-exist with video.ko handling the
> > backlight (as your patches allow) is the right thing to do.
> 
> sure, they will co-exist, but I don't think fujitsu-laptop should handle
> the backlight stuff, unless it's explicitly stated via the boot
> parameter "acpi_backlight=vendor".

For laptops with both ACPI video extensions and vendor specific control
methods I agree with this.  However, users of laptops which have only vendor
specific control methods (of which the S7020 is one AFAIK) should not have
to give any boot parameters for things to work for them.  In other words,
"acpi_backlight=vendor" should only come into play when the machine has the
ACPI video extensions.  I am assuming that this is the intention since
that's what the current patches appear to do.

Is there a canonical source for Matthew/Hong's patch?  To totally wrap this
up I would like to try the S7020 with Matthew/Hong+Thomas.  I doubt
Matthew/Hong's work is relevant to the S7020 given that it's 3+ years old
now, but it's best to test these things early.

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: Check for ACPI backlight support otherwise use vendor ACPI

[Zhang Rui]

On Thu, 2008-07-31 at 10:51 +0800, Jonathan Woithe wrote:
> > > I'm trying to sort out the status of this myself.  Is the
> expectation
> > > that your patches *with* the IGD work from Matthew/Hong should
> mean that
> > > Fujitsus implementing IGD should not need fujitsu-laptop in order
> to
> > > provide working software backlight brightness control?
> > I think so.
> 
> Ok.
> 
> > > Or shouldn't Matthew/Hong's patchset be necessary with your patch
> in
> > > theory?
> > Thomas' patch assigns the responsibility to acpi video driver or
> > platform drivers when a laptop supports brightness/display switch
> both
> > via ACPI video extension and vendor specific control methods.
> >
> > brightness/display switch via ACPI video driver sometimes doesn't
> work
> > even if the ACPI control methods are available, and Matthew/Hong's
> patch
> > fixes this problem, on platforms with intel integrated graphics
> cards.
> 
> I see.
> 
> > And now, we should root cause why acpi_video_backlight_support()
> failed
> > on this laptop.
> 
> Agreed.
> 
> > > so allowing fujitsu-laptop to co-exist with video.ko handling the
> > > backlight (as your patches allow) is the right thing to do.
> >
> > sure, they will co-exist, but I don't think fujitsu-laptop should
> handle
> > the backlight stuff, unless it's explicitly stated via the boot
> > parameter "acpi_backlight=vendor".
> 
> For laptops with both ACPI video extensions and vendor specific
> control
> methods I agree with this.  However, users of laptops which have only
> vendor
> specific control methods (of which the S7020 is one AFAIK) should not
> have
> to give any boot parameters for things to work for them.

sure.
In this case, acpi_video_backlight_support() returns false,
and vendor driver will handle the backlight switching.

>   In other words,
> "acpi_backlight=vendor" should only come into play when the machine
> has the
> ACPI video extensions.  I am assuming that this is the intention since
> that's what the current patches appear to do.
Yes.
> 
> Is there a canonical source for Matthew/Hong's patch?  To totally wrap
> this
> up I would like to try the S7020 with Matthew/Hong+Thomas.


>   I doubt
> Matthew/Hong's work is relevant to the S7020 given that it's 3+ years
> old
> now, but it's best to test these things early.

Agree.
I think Matthew/Hong's patch should have no effect on your laptops, and
fujistu-laptop should handle the backlight switch as before.

thanks,
rui


--
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: Check for ACPI backlight support otherwise use vendor ACPI drivers - version 2

[nokos]

Hi Thomas

I didi some testing and found whats wrong with the backlight detection
in video_detect.c you should change:

--- video_detect.c 2008-07-31 17:20:59.540059897 +0200
+++ video_detect.c 2008-07-31 17:22:15.752034851 +0200
@@ -45,9 +45,9 @@
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found generic backlight "
 				  "support\n"));
 		*cap |= ACPI_VIDEO_BACKLIGHT;
-		return 0;
+		return -ENODEV;
 	}
-	return -ENODEV;
+	return 0;
 }
 

otherwise it stops walking the subtree at the first device which does
not contain backlight (in my case GFX0 has CRT_ as first child which
does not support backlight). Its ok to stop scanning at the first
positive match since cap then already has ACPI_VIDEO_BACKLIGHT. (ENODEV
is probably not the best error code here ...)

probably you might want to change in acpi_is_video_device:

--- video_detect.c 2008-07-31 17:20:59.540059897 +0200
+++ video_detect.c 2008-07-31 17:22:15.752034851 +0200
@@ -93,8 +93,10 @@
 		video_caps |= ACPI_VIDEO_IGD;
 	}
 
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, device->handle, ACPI_UINT32_MAX,
+        if (video_caps) {
+	        acpi_walk_namespace(ACPI_TYPE_DEVICE, device->handle,
ACPI_UINT32_MAX,
 			    acpi_backlight_cap_match, &video_caps, NULL);
+        }
 
 	return video_caps;
 }

otherwise the whole ACPI namespace is scanned several times and this
restricts the scanning only on those subtrees where the head might be a
video device


BTW. the DRDY detection (for IGD) works ... now I have to find the IGD
patches ... couldn't find them on the list and the version of linux-next
I tried didn't contain anything which looked like it.

Peter

Am Mittwoch, den 30.07.2008, 11:06 +0200 schrieb Thomas Renninger:
> On Wednesday 30 July 2008 08:29:37 Yves-Alexis Perez wrote:
> > On mer, 2008-07-16 at 12:52 +0200, Thomas Renninger wrote:
> > > It is highly recommended that these patches are tried out together with
> > > Matthew's/Hong's IGD driver changes.
> > > I do not know whether they are already in the dri tree or in linux-next,
> > > whether they will be accepted for .27 etc. I hope Matthew will answer on
> > > this mail and tell us.
> > > AFAIK on thinkpads they already need an additional patch to work?
> > > Chances are high that other laptop families also need a bit of a special
> > > treatment to get the very new IGD parts to work.
> > > If people test both patches together we could get a matrix like this:
> >
> > What is the status of those patches wrt. 2.6.27-rc1?
> 
> Matthew and Rui accepted or signed them off AFAIK.
> On Peter's system (nokos@gmx.de, fujitsu with IGD support) it returns that
> there is no video support which looks wrong.
> I contacted him a bit late, after getting pointed to acpidump it still looks
> odd/wrong, and I need acpi.debug_level=0x1f log_buf_len=16777216 output to 
> hopefully see what it is.
> 
> A comment from Andi/Len would be great.
> Maybe this could already be sent to linux-next to make testing easier.
> 
>       Thomas
> 
> PS: and this is version 2, I already sent version 3 which had some cleanups.

Re: Check for ACPI backlight support otherwise use vendor ACPI

[Jonathan Woithe]

Hi gyys

> I didi some testing and found whats wrong with the backlight detection
> in video_detect.c you should change:
> 
> --- video_detect.c 2008-07-31 17:20:59.540059897 +0200
> +++ video_detect.c 2008-07-31 17:22:15.752034851 +0200
> @@ -45,9 +45,9 @@
>  		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found generic backlight "
>  				  "support\n"));
>  		*cap |= ACPI_VIDEO_BACKLIGHT;
> -		return 0;
> +		return -ENODEV;
>  	}
> -	return -ENODEV;
> +	return 0;
>  }
>  
> 
> otherwise it stops walking the subtree at the first device which does
> not contain backlight (in my case GFX0 has CRT_ as first child which
> does not support backlight).

Makes sense to me (although admittedly I haven't looked deeply into it). 
Are there likely to be any obscure side effects on other hardware which
for some reason assumes backlight support is on the first device scanned?

I'll try to find some time over the weekend to make sure this causes no
ill-effects on the S7020.  It shouldn't, but you never know.

> BTW. the DRDY detection (for IGD) works ... now I have to find the IGD
> patches ... couldn't find them on the list and the version of linux-next
> I tried didn't contain anything which looked like it.

Unfortunately I don't know of a definitive source for these either.

Regards
  jonathan

Re: Check for ACPI backlight support otherwise use vendor ACPIdrivers - version 2

[Zhang Rui]

On Thu, 2008-07-31 at 23:32 +0800, nokos@gmx.net wrote:
> Hi Thomas
> 
> I didi some testing and found whats wrong with the backlight detection
> in video_detect.c you should change:
> 
> --- video_detect.c 2008-07-31 17:20:59.540059897 +0200
> +++ video_detect.c 2008-07-31 17:22:15.752034851 +0200
> @@ -45,9 +45,9 @@
>                 ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found generic
> backlight "
>                                   "support\n"));
>                 *cap |= ACPI_VIDEO_BACKLIGHT;
> -               return 0;
> +               return -ENODEV;
>         }
> -       return -ENODEV;
> +       return 0;
>  }
> 
Yes, that's the problem. 

In the original patch,
acpi_walk_namespace(ACPI_TYPE_DEVICE, device->handle, ACPI_UINT32_MAX,
                     acpi_backlight_cap_match, &video_caps, NULL)
in acpi_is_video_device() returns if the first video device doesn't
support backlight control.

> 
> otherwise it stops walking the subtree at the first device which does
> not contain backlight (in my case GFX0 has CRT_ as first child which
> does not support backlight). Its ok to stop scanning at the first
> positive match since cap then already has ACPI_VIDEO_BACKLIGHT.
> (ENODEV
> is probably not the best error code here ...)
> 
> probably you might want to change in acpi_is_video_device:
> 
> --- video_detect.c 2008-07-31 17:20:59.540059897 +0200
> +++ video_detect.c 2008-07-31 17:22:15.752034851 +0200
> @@ -93,8 +93,10 @@
>                 video_caps |= ACPI_VIDEO_IGD;
>         }
> 
> -       acpi_walk_namespace(ACPI_TYPE_DEVICE, device->handle,
> ACPI_UINT32_MAX,
> +        if (video_caps) {
> +               acpi_walk_namespace(ACPI_TYPE_DEVICE, device->handle,
> ACPI_UINT32_MAX,
>                             acpi_backlight_cap_match, &video_caps,
> NULL);
> +        }
> 
>         return video_caps;
>  }
No.
For a video bus device, video_caps can never be 0 here.

> otherwise the whole ACPI namespace is scanned several times and this
> restricts the scanning only on those subtrees where the head might be
> a
> video device
> 
> 
> BTW. the DRDY detection (for IGD) works ... now I have to find the IGD
> patches ... couldn't find them on the list and the version of
> linux-next
> I tried didn't contain anything which looked like it.
You can get them here.
http://cgit.freedesktop.org/mesa/drm/diff/?id=dfd441cf964f20e4a761cb8490d7cd82cf32e7b9

don't know if this is the latest version, cc Matthew and Jessie.

thanks,
rui


--
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: Check for ACPI backlight support otherwise use vendor ACPIdrivers - version 2

[nokos]

Am Freitag, den 01.08.2008, 09:28 +0800 schrieb Zhang Rui:
> On Thu, 2008-07-31 at 23:32 +0800, nokos@gmx.net wrote:
> > Hi Thomas
> > 
> > probably you might want to change in acpi_is_video_device:
> > 
> > --- video_detect.c 2008-07-31 17:20:59.540059897 +0200
> > +++ video_detect.c 2008-07-31 17:22:15.752034851 +0200
> > @@ -93,8 +93,10 @@
> >                 video_caps |= ACPI_VIDEO_IGD;
> >         }
> > 
> > -       acpi_walk_namespace(ACPI_TYPE_DEVICE, device->handle,
> > ACPI_UINT32_MAX,
> > +        if (video_caps) {
> > +               acpi_walk_namespace(ACPI_TYPE_DEVICE, device->handle,
> > ACPI_UINT32_MAX,
> >                             acpi_backlight_cap_match, &video_caps,
> > NULL);
> > +        }
> > 
> >         return video_caps;
> >  }
> No.
> For a video bus device, video_caps can never be 0 here.
> 

Exactly, but acpi_is_video_device is called for every possible device in
the system, so there are a lot of subtree scans for devices which are
definitely no video devices (in my case 100 devices get scanned and
about 400 checks for backlight_cap) which produce a lot of useless logs
with debug_level=0x1f. With the if (video_caps) the namespace wlking is
restricted to devices where there is a chance of being a video device. 

> > otherwise the whole ACPI namespace is scanned several times and this
> > restricts the scanning only on those subtrees where the head might be
> > a
> > video device
> > 
> > 
> > BTW. the DRDY detection (for IGD) works ... now I have to find the IGD
> > patches ... couldn't find them on the list and the version of
> > linux-next
> > I tried didn't contain anything which looked like it.
> You can get them here.
> http://cgit.freedesktop.org/mesa/drm/diff/?id=dfd441cf964f20e4a761cb8490d7cd82cf32e7b9

thanks
Peter

> don't know if this is the latest version, cc Matthew and Jessie.
> 
> thanks,
> rui
> 
> 
> --
> 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: Check for ACPI backlight support otherwise use vendor ACPIdrivers - version 2

[nokos]

Am Freitag, den 01.08.2008, 09:28 +0800 schrieb Zhang Rui:
> > 
> > BTW. the DRDY detection (for IGD) works ... now I have to find the IGD
> > patches ... couldn't find them on the list and the version of
> > linux-next
> > I tried didn't contain anything which looked like it.
> You can get them here.
> http://cgit.freedesktop.org/mesa/drm/diff/?id=dfd441cf964f20e4a761cb8490d7cd82cf32e7b9
> 
> don't know if this is the latest version, cc Matthew and Jessie.

I tried, but was quite entirely unsuccessful in applying the patch to a
recent linux-next so for now the only solution for me is (since Thomas
patch disables a functional fujitsu-laptop backlight control in favor of
a non-functional acpi_video backlight control) to blacklist:

--- video_detect.c      2008-07-31 17:20:59.540059897 +0200
+++ video_detect.c      2008-08-01 10:42:06.817046615 +0200
@@ -157,6 +159,11 @@ long acpi_video_get_capabilities(acpi_ha
                 *              ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
                 *}
                 */
+               if (dmi_name_in_vendors("FUJITSUSIEMENS") && 
+                                dmi_name_in_vendors("LIFEBOOKS6410")) {
+                       acpi_video_support |=
+                               ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
+               }
        } else {
                status = acpi_bus_get_device(graphics_handle, &tmp_dev);
                if (ACPI_FAILURE(status)) {


Peter

Check for ACPI backlight support otherwise use vendor ACPI drivers - version 3

[Thomas Renninger]

Minor cleanup changes to version 2:

   - Remove all IGD stuff
   - beautify bit mask defines

[PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Thomas Renninger]

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/scan.c                 |   32 +----
 drivers/acpi/video.c                |   28 ++--
 drivers/acpi/video_detect.c         |  267 +++++++++++++++++++++++++++++++++++
 include/linux/acpi.h                |   44 ++++++
 6 files changed, 346 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/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..0533e0d 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..e90a5dc
--- /dev/null
+++ b/drivers/acpi/video_detect.c
@@ -0,0 +1,267 @@
+/*
+ *  Copyright (C) 2008       SuSE Linux Products GmbH
+ *                           Thomas Renninger <trenn@suse.de>
+ *
+ *  May be copied or modified under the terms of the GNU General Public License
+ *
+ * video_detect.c:
+ * Provides acpi_is_video_device() for early scanning of ACPI devices in scan.c
+ * There a Linux specific (Spec does not provide a HID for video devices) is
+ * 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. The check for general
+ * video capabilities will be triggered by the first caller of
+ * acpi_video_get_capabilities(NULL); which will happen when the first
+ * backlight (or display output) switching supporting driver calls:
+ * acpi_video_backlight_support();
+ *
+ * Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix B)
+ * are available, video.ko should be used to handle the device.
+ *
+ * Otherwise vendor specific drivers like thinkpad_acpi, asus_acpi,
+ * sony_acpi,... can take care about backlight brightness and display output
+ * switching.
+ *
+ * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
+ * this file will not be compiled, acpi_video_get_capabilities() and
+ * acpi_video_backlight_support() will always return 0 and vendor specific
+ * drivers always can handle backlight.
+ *
+ */
+
+#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;
+static bool acpi_video_caps_checked;
+
+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;
+
+	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);
+	}
+	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_handle)
+{
+	long caps = 0;
+	struct acpi_device *tmp_dev;
+	acpi_status status;
+
+	if (acpi_video_caps_checked && graphics_handle == NULL)
+		return acpi_video_support;
+
+	if (!graphics_handle) {
+		/* Only do the global walk through all graphics devices once */
+		acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+				    ACPI_UINT32_MAX, find_video,
+				    &caps, NULL);
+		/* There might be boot param flags set already... */
+		acpi_video_support |= caps;
+		acpi_video_caps_checked = 1;
+		/* Add blacklists here. Be careful to use the right *DMI* bits
+		 * to still be able to override logic via boot params, e.g.:
+		 *
+		 *   if (dmi_name_in_vendors("XY")) {
+		 *	acpi_video_support |=
+		 *		ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR;
+		 *	acpi_video_support |=
+		 *		ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
+		 *}
+		 */
+	} else {
+		status = acpi_bus_get_device(graphics_handle, &tmp_dev);
+		if (ACPI_FAILURE(status)) {
+			ACPI_EXCEPTION((AE_INFO, status, "Invalid device"));
+			return 0;
+		}
+		acpi_walk_namespace(ACPI_TYPE_DEVICE, graphics_handle,
+				    ACPI_UINT32_MAX, find_video,
+				    &caps, NULL);
+	}
+	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "We have 0x%lX video support %s %s\n",
+			  graphics_handle ? caps : acpi_video_support,
+			  graphics_handle ? "on device " : "in general",
+			  graphics_handle ? acpi_device_bid(tmp_dev) : ""));
+	return caps;
+}
+EXPORT_SYMBOL(acpi_video_get_capabilities);
+
+/* Returns true if video.ko can do backlight switching
+ *
+ */
+int acpi_video_backlight_support(void)
+{
+	/* We must check whether the ACPI graphics device is physically plugged
+	 * in. Therefore this must be called after binding PCI and ACPI devices
+	 */
+	if (!acpi_video_caps_checked)
+		acpi_video_get_capabilities(NULL);
+
+	/* 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.
+ *
+ */
+int acpi_video_display_switch_support(void)
+{
+	if (!acpi_video_caps_checked)
+		acpi_video_get_capabilities(NULL);
+
+	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..63aff1a 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -201,6 +201,50 @@ extern bool wmi_has_guid(const char *guid);
 
 #endif	/* CONFIG_ACPI_WMI */
 
+#define ACPI_VIDEO_OUTPUT_SWITCHING			0x0001
+#define ACPI_VIDEO_DEVICE_POSTING			0x0002
+#define ACPI_VIDEO_ROM_AVAILABLE			0x0004
+#define ACPI_VIDEO_BACKLIGHT				0x0008
+#define ACPI_VIDEO_BACKLIGHT_FORCE_VENDOR		0x0010
+#define ACPI_VIDEO_BACKLIGHT_FORCE_VIDEO		0x0020
+#define ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VENDOR	0x0040
+#define ACPI_VIDEO_OUTPUT_SWITCHING_FORCE_VIDEO		0x0080
+#define ACPI_VIDEO_BACKLIGHT_DMI_VENDOR			0x0100
+#define ACPI_VIDEO_BACKLIGHT_DMI_VIDEO			0x0200
+#define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR		0x0400
+#define ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VIDEO		0x0800
+
+#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

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Zhang Rui]

On Thu, 2008-07-17 at 19:32 +0200, Thomas Renninger wrote:
> 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/scan.c                 |   32 +----
>  drivers/acpi/video.c                |   28 ++--
>  drivers/acpi/video_detect.c         |  267 +++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h                |   44 ++++++
>  6 files changed, 346 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/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..0533e0d 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..e90a5dc
> --- /dev/null
> +++ b/drivers/acpi/video_detect.c
> @@ -0,0 +1,267 @@
> +/*
> + *  Copyright (C) 2008       SuSE Linux Products GmbH
> + *                           Thomas Renninger <trenn@suse.de>
> + *
> + *  May be copied or modified under the terms of the GNU General Public License
> + *
> + * video_detect.c:
> + * Provides acpi_is_video_device() for early scanning of ACPI devices in scan.c
> + * There a Linux specific (Spec does not provide a HID for video devices) is
> + * 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. The check for general
> + * video capabilities will be triggered by the first caller of
> + * acpi_video_get_capabilities(NULL); which will happen when the first
> + * backlight (or display output) switching supporting driver calls:
> + * acpi_video_backlight_support();
> + *
> + * Depending on whether ACPI graphics extensions (cmp. ACPI spec Appendix B)
> + * are available, video.ko should be used to handle the device.
> + *
> + * Otherwise vendor specific drivers like thinkpad_acpi, asus_acpi,
> + * sony_acpi,... can take care about backlight brightness and display output
> + * switching.
> + *
> + * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a module (m)
> + * this file will not be compiled, acpi_video_get_capabilities() and
> + * acpi_video_backlight_support() will always return 0 and vendor specific
> + * drivers always can handle backlight.
> + *
> + */
> +
> +#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;
> +static bool acpi_video_caps_checked;
> +
> +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;
> +
> +	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);
> +	}
> +	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_handle)
> +{
> +	long caps = 0;
> +	struct acpi_device *tmp_dev;
> +	acpi_status status;
> +
> +	if (acpi_video_caps_checked && graphics_handle == NULL)
> +		return acpi_video_support;
> +
> +	if (!graphics_handle) {
> +		/* Only do the global walk through all graphics devices once */
> +		acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> +				    ACPI_UINT32_MAX, find_video,
> +				    &caps, NULL);
> +		/* There might be boot param flags set already... */
> +		acpi_video_support |= caps;
> +		acpi_video_caps_checked = 1;
> +		/* Add blacklists here. Be careful to use the right *DMI* bits
> +		 * to still be able to override logic via boot params, e.g.:
> +		 *
> +		 *   if (dmi_name_in_vendors("XY")) {
> +		 *	acpi_video_support |=
> +		 *		ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR;
> +		 *	acpi_video_support |=
> +		 *		ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> +		 *}
> +		 */
so this is for the thinkpad laptops which patch 01 breaks, right?
why don't you add the thinkpad dmi entries directly?
If we don't blacklist them, this patch set still causes regressions on
these laptops, unless the IGD OpRegion patches are upstream.

thanks,
rui

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Thomas Renninger]

On Wednesday 23 July 2008 05:35:39 Zhang Rui wrote:
> On Thu, 2008-07-17 at 19:32 +0200, Thomas Renninger wrote:
> > 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/scan.c                 |   32 +----
> >  drivers/acpi/video.c                |   28 ++--
> >  drivers/acpi/video_detect.c         |  267
> > +++++++++++++++++++++++++++++++++++ include/linux/acpi.h                |
> >   44 ++++++
> >  6 files changed, 346 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/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..0533e0d 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..e90a5dc
> > --- /dev/null
> > +++ b/drivers/acpi/video_detect.c
> > @@ -0,0 +1,267 @@
> > +/*
> > + *  Copyright (C) 2008       SuSE Linux Products GmbH
> > + *                           Thomas Renninger <trenn@suse.de>
> > + *
> > + *  May be copied or modified under the terms of the GNU General Public
> > License + *
> > + * video_detect.c:
> > + * Provides acpi_is_video_device() for early scanning of ACPI devices in
> > scan.c + * There a Linux specific (Spec does not provide a HID for video
> > devices) is + * 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. The check for
> > general + * video capabilities will be triggered by the first caller of
> > + * acpi_video_get_capabilities(NULL); which will happen when the first
> > + * backlight (or display output) switching supporting driver calls:
> > + * acpi_video_backlight_support();
> > + *
> > + * Depending on whether ACPI graphics extensions (cmp. ACPI spec
> > Appendix B) + * are available, video.ko should be used to handle the
> > device. + *
> > + * Otherwise vendor specific drivers like thinkpad_acpi, asus_acpi,
> > + * sony_acpi,... can take care about backlight brightness and display
> > output + * switching.
> > + *
> > + * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor as a
> > module (m) + * this file will not be compiled,
> > acpi_video_get_capabilities() and + * acpi_video_backlight_support() will
> > always return 0 and vendor specific + * drivers always can handle
> > backlight.
> > + *
> > + */
> > +
> > +#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;
> > +static bool acpi_video_caps_checked;
> > +
> > +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;
> > +
> > +	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);
> > +	}
> > +	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_handle)
> > +{
> > +	long caps = 0;
> > +	struct acpi_device *tmp_dev;
> > +	acpi_status status;
> > +
> > +	if (acpi_video_caps_checked && graphics_handle == NULL)
> > +		return acpi_video_support;
> > +
> > +	if (!graphics_handle) {
> > +		/* Only do the global walk through all graphics devices once */
> > +		acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> > +				    ACPI_UINT32_MAX, find_video,
> > +				    &caps, NULL);
> > +		/* There might be boot param flags set already... */
> > +		acpi_video_support |= caps;
> > +		acpi_video_caps_checked = 1;
> > +		/* Add blacklists here. Be careful to use the right *DMI* bits
> > +		 * to still be able to override logic via boot params, e.g.:
> > +		 *
> > +		 *   if (dmi_name_in_vendors("XY")) {
> > +		 *	acpi_video_support |=
> > +		 *		ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR;
> > +		 *	acpi_video_support |=
> > +		 *		ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> > +		 *}
> > +		 */
>
> so this is for the thinkpad laptops which patch 01 breaks, right?
> why don't you add the thinkpad dmi entries directly?
> If we don't blacklist them, this patch set still causes regressions on
> these laptops, unless the IGD OpRegion patches are upstream.

Because Matthew told us the IGD OpRegion patches will be upstream for 2.6.27.
While this makes it a bit difficult to test these, it is a good idea though to 
try to do it right in first place.

So if there are still problems, people can test with boot params. If we are 
not able to fix things during 2.6.27-rcX so that they work properly with 
video.ko, we can still add a dmi blacklist for specific models (should be 
some general "panasonic" string or something like that, most vendors should 
have similar general ACPI brightness implementation as this is quite young).

Andi, these patches are needed by Matthews IGD OpRegion driver, or video.ko 
will still register for two graphics devices even there is only one.
Do you plan do push them?

       Thomas

Re: [PATCH 02/11] Check for ACPI backlight support otherwise use vendor ACPI drivers

[Zhang Rui]

On Wed, 2008-07-23 at 17:49 +0800, Thomas Renninger wrote:
> On Wednesday 23 July 2008 05:35:39 Zhang Rui wrote:
> > On Thu, 2008-07-17 at 19:32 +0200, Thomas Renninger wrote:
> > > 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/scan.c                 |   32 +----
> > >  drivers/acpi/video.c                |   28 ++--
> > >  drivers/acpi/video_detect.c         |  267
> > > +++++++++++++++++++++++++++++++++++ include/linux/acpi.h
> |
> > >   44 ++++++
> > >  6 files changed, 346 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/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..0533e0d 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..e90a5dc
> > > --- /dev/null
> > > +++ b/drivers/acpi/video_detect.c
> > > @@ -0,0 +1,267 @@
> > > +/*
> > > + *  Copyright (C) 2008       SuSE Linux Products GmbH
> > > + *                           Thomas Renninger <trenn@suse.de>
> > > + *
> > > + *  May be copied or modified under the terms of the GNU General
> Public
> > > License + *
> > > + * video_detect.c:
> > > + * Provides acpi_is_video_device() for early scanning of ACPI
> devices in
> > > scan.c + * There a Linux specific (Spec does not provide a HID for
> video
> > > devices) is + * 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. The check
> for
> > > general + * video capabilities will be triggered by the first
> caller of
> > > + * acpi_video_get_capabilities(NULL); which will happen when the
> first
> > > + * backlight (or display output) switching supporting driver
> calls:
> > > + * acpi_video_backlight_support();
> > > + *
> > > + * Depending on whether ACPI graphics extensions (cmp. ACPI spec
> > > Appendix B) + * are available, video.ko should be used to handle
> the
> > > device. + *
> > > + * Otherwise vendor specific drivers like thinkpad_acpi,
> asus_acpi,
> > > + * sony_acpi,... can take care about backlight brightness and
> display
> > > output + * switching.
> > > + *
> > > + * If CONFIG_ACPI_VIDEO is neither set as "compiled in" (y) nor
> as a
> > > module (m) + * this file will not be compiled,
> > > acpi_video_get_capabilities() and + *
> acpi_video_backlight_support() will
> > > always return 0 and vendor specific + * drivers always can handle
> > > backlight.
> > > + *
> > > + */
> > > +
> > > +#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;
> > > +static bool acpi_video_caps_checked;
> > > +
> > > +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;
> > > +
> > > +   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);
> > > +   }
> > > +   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_handle)
> > > +{
> > > +   long caps = 0;
> > > +   struct acpi_device *tmp_dev;
> > > +   acpi_status status;
> > > +
> > > +   if (acpi_video_caps_checked && graphics_handle == NULL)
> > > +           return acpi_video_support;
> > > +
> > > +   if (!graphics_handle) {
> > > +           /* Only do the global walk through all graphics
> devices once */
> > > +           acpi_walk_namespace(ACPI_TYPE_DEVICE,
> ACPI_ROOT_OBJECT,
> > > +                               ACPI_UINT32_MAX, find_video,
> > > +                               &caps, NULL);
> > > +           /* There might be boot param flags set already... */
> > > +           acpi_video_support |= caps;
> > > +           acpi_video_caps_checked = 1;
> > > +           /* Add blacklists here. Be careful to use the right
> *DMI* bits
> > > +            * to still be able to override logic via boot params,
> e.g.:
> > > +            *
> > > +            *   if (dmi_name_in_vendors("XY")) {
> > > +            *      acpi_video_support |=
> > > +            *
> ACPI_VIDEO_OUTPUT_SWITCHING_DMI_VENDOR;
> > > +            *      acpi_video_support |=
> > > +            *              ACPI_VIDEO_BACKLIGHT_DMI_VENDOR;
> > > +            *}
> > > +            */
> >
> > so this is for the thinkpad laptops which patch 01 breaks, right?
> > why don't you add the thinkpad dmi entries directly?
> > If we don't blacklist them, this patch set still causes regressions
> on
> > these laptops, unless the IGD OpRegion patches are upstream.
> 
> Because Matthew told us the IGD OpRegion patches will be upstream for
> 2.6.27.
> While this makes it a bit difficult to test these, it is a good idea
> though to
> try to do it right in first place.
> 
> So if there are still problems, people can test with boot params. If
> we are
> not able to fix things during 2.6.27-rcX so that they work properly
> with
> video.ko, we can still add a dmi blacklist for specific models (should
> be
> some general "panasonic" string or something like that, most vendors
> should
> have similar general ACPI brightness implementation as this is quite
> young).

I see. Then I'm okay with the whole patch series.

Acked-by: Zhang Rui <rui.zhang@intel.com>

thanks,
rui
> 
> Andi, these patches are needed by Matthews IGD OpRegion driver, or
> video.ko
> will still register for two graphics devices even there is only one.
> Do you plan do push them?
> 
>        Thomas
> 
>