Skip to contentSkip to conversation

Who controls brightness functionality: vendor specific or generic video driver

PATCH In review Thomas Renninger 3 participants 5 messages Apr 16, 2008 – Apr 19, 2008

Subsystems (2)
ACPI Char and misc drivers
lore
Share & tools
lore

Thomas Renninger

author
#0 lore raw

PATCH 7/7 Who controls brightness functionality: vendor specific or generic video driver

Who controls brightness functionality: vendor specific or generic video driver

Depending on the capablity of ACPI graphics devices and whether they implement all necessary functions so that the generic ACPI video driver can control them, decide at driver load time whether a vendor specific device driver or the ACPI video driver should control the brighntess functionality.

Signed-off-by Thomas Renninger <[email protected]>
---
 drivers/misc/acer-wmi.c       |   15 +++++++++++----
 drivers/misc/asus-laptop.c    |    6 ++++++
 drivers/misc/fujitsu-laptop.c |   27 +++++++++++++++++++--------
 drivers/misc/msi-laptop.c     |   16 +++++++++++-----
 drivers/misc/sony-laptop.c    |   14 +++++++++-----
 drivers/misc/thinkpad_acpi.c  |    7 +++++++
 6 files changed, 63 insertions(+), 22 deletions(-)
drivers/misc/asus-laptop.c +6 -0
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/asus-laptop.c
===================================================================
@@ -1105,6 +1105,12 @@ static int asus_backlight_init(struct de
 {
 	struct backlight_device *bd;
 
+	if (acpi_video_support & ACPI_VIDEO_BRIGHTNESS &&
+	    !(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC)) {
+		printk(ASUS_INFO "Brightness ignored, must be controlled by "
+		       "ACPI video driver\n");
+		return -ENODEV;
+	}
 	if (brightness_set_handle && lcd_switch_handle) {
 		bd = backlight_device_register(ASUS_HOTK_FILE, dev,
 					       NULL, &asusbl_ops);
drivers/misc/thinkpad_acpi.c +7 -0
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/thinkpad_acpi.c
===================================================================
@@ -4381,6 +4381,13 @@ static int __init brightness_init(struct
 			   "module parameter\n");
 		return 1;
 	} else if (brightness_enable > 1) {
+		if ((acpi_video_support & ACPI_VIDEO_BRIGHTNESS) &&
+		    !(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC)) {
+			printk(TPACPI_INFO "Brightness ignored, must be "
+			       "controlled by ACPI video driver\n");
+			return 1;
+		}
+		/* Above, generic approach should make this check obsolete */
 		if (brightness_check_std_acpi_support()) {
 			printk(TPACPI_NOTICE
 			       "standard ACPI backlight interface "
drivers/misc/fujitsu-laptop.c +19 -8
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/fujitsu-laptop.c
===================================================================
@@ -273,13 +273,21 @@ 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);
+	if (acpi_video_support & ACPI_VIDEO_BRIGHTNESS &&
+	    !(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC)) {
+		printk(KERN_INFO "Brightness ignored, must be controlled by "
+		       "ACPI video driver\n");
+	} else {
+		fujitsu->bl_device =
+			backlight_device_register("fujitsu-laptop", NULL, NULL,
+						  &fujitsubl_ops);
+		if (IS_ERR(fujitsu->bl_device))
+			return PTR_ERR(fujitsu->bl_device);
+
+		fujitsu->bl_device->props.max_brightness =
+			FUJITSU_LCD_N_LEVELS - 1;
+	}
 
-	fujitsu->bl_device->props.max_brightness = FUJITSU_LCD_N_LEVELS - 1;
 	ret = platform_driver_register(&fujitsupf_driver);
 	if (ret)
 		goto fail_backlight;

@@ -321,7 +329,8 @@ static int __init fujitsu_init(void)
 
       fail_backlight:
 
-	backlight_device_unregister(fujitsu->bl_device);
+	if (fujitsu->bl_device)
+		backlight_device_unregister(fujitsu->bl_device);
 
       fail_acpi:
 

@@ -336,9 +345,11 @@ 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);
 
-	acpi_bus_unregister_driver(&acpi_fujitsu_driver);
+	if (fujitsu->bl_device)
+		acpi_bus_unregister_driver(&acpi_fujitsu_driver);
 
 	kfree(fujitsu);
drivers/misc/acer-wmi.c +11 -4
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/acer-wmi.c
===================================================================
@@ -837,7 +837,8 @@ static int __devinit acer_backlight_init
 
 static void acer_backlight_exit(void)
 {
-	backlight_device_unregister(acer_backlight_device);
+	if (acer_backlight_device)
+		backlight_device_unregister(acer_backlight_device);
 }
 
 /*

@@ -907,9 +908,15 @@ static int __devinit acer_platform_probe
 	}
 
 	if (has_cap(ACER_CAP_BRIGHTNESS)) {
-		err = acer_backlight_init(&device->dev);
-		if (err)
-			goto error_brightness;
+		if (acpi_video_support & ACPI_VIDEO_BRIGHTNESS &&
+		    !(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC))
+			printk(ACER_INFO "Brightness must be controlled by "
+			       "generic video driver\n");
+		else {
+			err = acer_backlight_init(&device->dev);
+			if (err)
+				goto error_brightness;
+		}
 	}
 
 	return 0;
drivers/misc/sony-laptop.c +9 -5
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/sony-laptop.c
===================================================================
@@ -1037,7 +1037,12 @@ static int sony_nc_add(struct acpi_devic
 		goto outinput;
 	}
 
-	if (ACPI_SUCCESS(acpi_get_handle(sony_nc_acpi_handle, "GBRT", &handle))) {
+	if (acpi_video_support & ACPI_VIDEO_BRIGHTNESS &&
+	    !(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC)) {
+		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);

@@ -1047,12 +1052,11 @@ static int sony_nc_add(struct acpi_devic
 			sony_backlight_device = NULL;
 		} else {
 			sony_backlight_device->props.brightness =
-			    sony_backlight_get_brightness
-			    (sony_backlight_device);
+				sony_backlight_get_brightness
+				(sony_backlight_device);
 			sony_backlight_device->props.max_brightness =
-			    SONY_MAX_BRIGHTNESS - 1;
+				SONY_MAX_BRIGHTNESS - 1;
 		}
-
 	}
 
 	/* initialize models with specific requirements */
drivers/misc/msi-laptop.c +11 -5
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/msi-laptop.c
===================================================================
@@ -347,12 +347,18 @@ 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);
+	if ((acpi_video_support & ACPI_VIDEO_BRIGHTNESS) &&
+	    !(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC)) {
+		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;
+		msibl_device->props.max_brightness = MSI_LCD_LEVEL_MAX-1;
+	}
 
 	ret = platform_driver_register(&msipf_driver);
 	if (ret)
author
#1 lore raw
Quoted 6 lines · Thomas Renninger

Who controls brightness functionality: vendor specific or generic video driver

Depending on the capablity of ACPI graphics devices and whether they implement all necessary functions so that the generic ACPI video driver can control them, decide at driver load time whether a vendor specific device driver or the ACPI video driver should control the brighntess functionality.

This probably clashes with other patches.
I also forgot the eepc driver (and others?) which might also interfere with the video driver?

Please review and if you like it tell me how this could get integrated in the easiest way...

Thomas

#2 lore raw
In reply to Thomas Renninger
Quoted 8 lines · Thomas Renninger

Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/thinkpad_acpi.c
===================================================================

drivers/misc/thinkpad_acpi.c +0 -0
@@ -4381,6 +4381,13 @@ static int __init brightness_init(struct
 			   "module parameter\n");
 		return 1;
 	} else if (brightness_enable > 1) {

drivers/misc/thinkpad_acpi.c static int __init brightness_init(struct · line 4393

Quoted 10 lines
drivers/misc/thinkpad_acpi.c +7 -0
+		if ((acpi_video_support & ACPI_VIDEO_BRIGHTNESS) &&
+		    !(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC)) {
+			printk(TPACPI_INFO "Brightness ignored, must be "
+			       "controlled by ACPI video driver\n");
+			return 1;
+		}
+		/* Above, generic approach should make this check obsolete */
 		if (brightness_check_std_acpi_support()) {
 			printk(TPACPI_NOTICE
 			       "standard ACPI backlight interface "

Ack. But I am sure I will have to change this hunk a bit on thinkpad-acpi 0.20. This is not a big issue, and if you prefer, I can handle adding the above support for thinkpad-acpi 0.20 since it is likely going to be merged late due to its dependency on some stuff for the LED class that needs to be in mainline first...

Otherwise, merging it now is also fine for acpi-test, and I will resend the thinkpad-acpi queue adjusted accordingly.

Signature · 4 lines
  "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
author
#3 lore raw
Quoted 17 lines · Henrique de Moraes Holschuh
Quoted 16 lines · Thomas Renninger

Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/thinkpad_acpi.c
===================================================================

drivers/misc/thinkpad_acpi.c +7 -0
@@ -4381,6 +4381,13 @@ static int __init brightness_init(struct
 			   "module parameter\n");
 		return 1;
 	} else if (brightness_enable > 1) {
+		if ((acpi_video_support & ACPI_VIDEO_BRIGHTNESS) &&
+		    !(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC)) {
+			printk(TPACPI_INFO "Brightness ignored, must be "
+			       "controlled by ACPI video driver\n");
+			return 1;
+		}
+		/* Above, generic approach should make this check obsolete */
 		if (brightness_check_std_acpi_support()) {
Quoted 8 lines

drivers/misc/thinkpad_acpi.c static int __init brightness_init(struct · line 4393

Quoted 2 lines
drivers/misc/thinkpad_acpi.c +0 -0
 			printk(TPACPI_NOTICE
 			       "standard ACPI backlight interface "

Ack. But I am sure I will have to change this hunk a bit on thinkpad-acpi 0.20. This is not a big issue, and if you prefer, I can handle adding the above support for thinkpad-acpi 0.20 since it is likely going to be merged late due to its dependency on some stuff for the LED class that needs to be in mainline first...

I need to post another version, I hope to find some time over the week-end.
I also need to go through and disable output switching also where supported.

Thanks,

Thomas

#4 lore raw
In reply to Thomas Renninger
Quoted 10 lines · On Wednesday 16 April 2008 20:02:52 Thomas Renninger wrote:

Who controls brightness functionality: vendor specific or generic video driver

Depending on the capablity of ACPI graphics devices and whether they implement all necessary functions so that the generic ACPI video driver can control them, decide at driver load time whether a vendor specific device driver or the ACPI video driver should control the brighntess functionality.

Signed-off-by Thomas Renninger <[email protected]>

NACK the acer-wmi bit, use the following patch instead (feel free to fold it into your superpatch though with my sign-off).

-Carlos
--
acer-wmi: Use ACPI video driver instead of WMI where possible

If the generic ACPI video driver can control the video, then we should hand off control to it, rather than try to handle it ourselves.

Signed-off-by Carlos Corbacho <[email protected]>

---

 drivers/misc/acer-wmi.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
drivers/misc/acer-wmi.c +8 -0
index dd13a37..0d69bc5 100644
@@ -1100,6 +1100,14 @@ static int __init acer_wmi_init(void)
 		return -ENODEV;
 	}
 
+	if (acpi_video_support & ACPI_VIDEO_BRIGHTNESS &&
+		!(acpi_video_support & ACPI_VIDEO_FORCE_VENDOR_SPECIFIC) &&
+		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;