Skip to contentSkip to conversation

video patches in ACPI test tree

PATCH New Len Brown 1 participant 11 messages Nov 8, 2008

Subsystems (4)
lore
Share & tools
lore

Len Brown

author reviewer
#0 lore raw

Here are the video patches as refreshed in the ACPI test branch. (there are a couple of merge conflicts, since there are other
patches in the acpi test tree that touch the same files)

I've tested my T61 (Intel Graphics, and thus IGD support) and found that these patches do no harm.

in all cases, I load i915, video and thinkpad_acpi

2.6.27:
Brightness hotkeys display controls in X,
but have no actual effect on brightness
/sys/class/backlight/acpi_video0 has no effect
/sys/class/backlight/acpi_video1 works

2.6.28-rc3+:
Brightness hotkeys display controls in X,
and correctly control brightness
/sys/class/backlight/acpi_video0 works
/sys/class/backlight/acpi_video1 works

2.6.28-rc3+ with these patches:
same as 2.6.28-rc3+, except
/sys/class/backlight/acpi_video1 does not exist.

thanks,
-Len

author reviewer
#1 lore raw

PATCH 1/10 ACPI: video: Ignore devices that aren't present in hardware

From: Thomas Renninger <[email protected]>

This is a reimplemention of commit
0119509c4fbc9adcef1472817fda295334612976
from Matthew Garrett <[email protected]>

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

---
 drivers/acpi/glue.c     |   40 ++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/video.c    |    7 ++++++-
 include/acpi/acpi_bus.h |    2 ++
 3 files changed, 48 insertions(+), 1 deletions(-)
drivers/acpi/glue.c +40 -0
index 24649ad..adec3d1 100644
@@ -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 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;
drivers/acpi/video.c +6 -1
index a29b0cc..6597c2a 100644
@@ -842,11 +842,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.
 	 */
include/acpi/acpi_bus.h +2 -0
index 54a279e..e9f6574 100644
@@ -380,6 +380,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);
Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#2 lore raw

PATCH 2/10 ACPI video: if no ACPI backlight support, use vendor drivers

In reply to Len Brown

From: Thomas Renninger <[email protected]>

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"
Prever vendor driver over ACPI driver for backlight.
"acpi_backlight=video" (default)
Prever ACPI driver over vendor driver for backlight.

---
 Documentation/kernel-parameters.txt |   12 ++
 drivers/acpi/Makefile               |    4 +
 drivers/acpi/scan.c                 |   32 +----
 drivers/acpi/video.c                |   28 ++--
 drivers/acpi/video_detect.c         |  268 +++++++++++++++++++++++++++++++++++
 include/linux/acpi.h                |   44 ++++++
 6 files changed, 345 insertions(+), 43 deletions(-)
 create mode 100644 drivers/acpi/video_detect.c
Documentation/kernel-parameters.txt +12 -0
index c86c074..dd5013f 100644
@@ -198,6 +198,18 @@ 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, prefer vendor specific driver
+			(e.g. thinkpad_acpi, sony_acpi, etc.) instead
+			of the ACPI 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,
drivers/acpi/Makefile +4 -0
index d91c027..c03810a 100644
@@ -46,6 +46,10 @@ obj-$(CONFIG_ACPI_BUTTON)	+= button.o
 obj-$(CONFIG_ACPI_FAN)		+= fan.o
 obj-$(CONFIG_ACPI_DOCK)		+= dock.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_PCI_SLOT)	+= pci_slot.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
drivers/acpi/scan.c +1 -31
index a9dda8e..556b182 100644
@@ -919,36 +919,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
  *
@@ -1031,7 +1001,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;
drivers/acpi/video.c +16 -12
index 6597c2a..2097c39 100644
@@ -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 && max_level > 0) {
 		int result;
@@ -785,18 +786,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;
 }
 
 /*
drivers/acpi/video_detect.c new file +268 -0

268 lines not on this page · raw message

include/linux/acpi.h +44 -0

50 lines not on this page · raw message

Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#3 lore raw

PATCH 3/10 Acer-WMI: fingers off backlight if video.ko is serving this functionality

In reply to Len Brown

From: Thomas Renninger <[email protected]>

---
 drivers/misc/acer-wmi.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
drivers/misc/acer-wmi.c +6 -0
index 0532a2d..94c9f91 100644
@@ -1297,6 +1297,12 @@ static int __init acer_wmi_init(void)
 
 	set_quirks();
 
+	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;
Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#4 lore raw

PATCH 4/10 asus-acpi: fingers off backlight if video.ko is serving this functionality

In reply to Len Brown

From: Thomas Renninger <[email protected]>

---
 drivers/misc/asus-laptop.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
drivers/misc/asus-laptop.c +7 -3
index a9d5228..8fb8b35 100644
@@ -1208,9 +1208,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)
Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#5 lore raw

PATCH 5/10 compal: fingers off backlight if video.ko is serving this functionality

In reply to Len Brown

From: Thomas Renninger <[email protected]>

---
 drivers/misc/compal-laptop.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
drivers/misc/compal-laptop.c +7 -5
index 344b790..11003bb 100644
@@ -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);
+	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;
+		compalbl_device->props.max_brightness = COMPAL_LCD_LEVEL_MAX-1;
+	}
 
 	ret = platform_driver_register(&compal_driver);
 	if (ret)
Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#6 lore raw

PATCH 6/10 eeepc-laptop: fingers off backlight if video.ko is serving this functionality

In reply to Len Brown

From: Thomas Renninger <[email protected]>

---
 drivers/misc/eeepc-laptop.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
drivers/misc/eeepc-laptop.c +9 -3
index 9ef98b2..02fe2b8 100644
@@ -825,9 +825,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;
Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#7 lore raw

PATCH 7/10 fujitsu-laptop: fingers off backlight if video.ko is serving this functionality

In reply to Len Brown

From: Thomas Renninger <[email protected]>

---
 drivers/misc/fujitsu-laptop.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)
drivers/misc/fujitsu-laptop.c +14 -12
index d2cf0bf..1070dc1 100644
@@ -990,16 +990,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)
@@ -1035,7 +1035,8 @@ fail_hotkey:
 
 fail_backlight:
 
-	backlight_device_unregister(fujitsu->bl_device);
+	if (fujitsu->bl_device)
+		backlight_device_unregister(fujitsu->bl_device);
 
 fail_platform_device2:
 
@@ -1062,7 +1063,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);
 
Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#8 lore raw

PATCH 8/10 msi-laptop: fingers off backlight if video.ko is serving this functionality

In reply to Len Brown

From: Thomas Renninger <[email protected]>

---
 drivers/misc/msi-laptop.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)
drivers/misc/msi-laptop.c +10 -6
index de898c6..759763d 100644
@@ -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)
Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#9 lore raw

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

In reply to Len Brown

From: Thomas Renninger <[email protected]>

---
 drivers/misc/sony-laptop.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
drivers/misc/sony-laptop.c +5 -1
index 06f07e1..7bcb810 100644
@@ -1038,7 +1038,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);
Signature · 1 line
1.6.0.3.640.g6331a
author reviewer
#10 lore raw

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

In reply to Len Brown

From: Thomas Renninger <[email protected]>

---
 drivers/misc/thinkpad_acpi.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)
drivers/misc/thinkpad_acpi.c +19 -10
index 4db1cf9..7a4a26b 100644
@@ -4932,16 +4932,25 @@ 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 enabled, even if standard "
+				       "ACPI backlight interface is available\n");
+			}
+		} else {
+			if (brightness_enable > 1) {
+				printk(TPACPI_NOTICE
+				       "Standard ACPI backlight interface not "
+				       "available, thinkpad_acpi native "
+				       "brightness control enabled\n");
+			}
 		}
 	}
 
Signature · 1 line
1.6.0.3.640.g6331a