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

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

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

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

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

Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/asus-laptop.c
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/misc/asus-laptop.c
+++ 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);
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/thinkpad_acpi.c
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/misc/thinkpad_acpi.c
+++ 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 "
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/fujitsu-laptop.c
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/misc/fujitsu-laptop.c
+++ 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);
 
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/acer-wmi.c
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/misc/acer-wmi.c
+++ 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;
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/sony-laptop.c
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/misc/sony-laptop.c
+++ 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 */
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/msi-laptop.c
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/misc/msi-laptop.c
+++ 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)

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

[Thomas Renninger]

On Wed, 2008-04-16 at 21:02 +0200, 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.

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

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

[Henrique de Moraes Holschuh]

On Wed, 16 Apr 2008, Thomas Renninger wrote:
> Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/thinkpad_acpi.c
> ===================================================================
> --- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/misc/thinkpad_acpi.c
> +++ 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 "

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.

-- 
  "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 7/7] Who controls brightness functionality: vendor specific or generic video driver

[Thomas Renninger]

On Wed, 2008-04-16 at 18:01 -0300, Henrique de Moraes Holschuh wrote:
> On Wed, 16 Apr 2008, Thomas Renninger wrote:
> > Index: linux-acpi-2.6_video_native_vs_vendor/drivers/misc/thinkpad_acpi.c
> > ===================================================================
> > --- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/misc/thinkpad_acpi.c
> > +++ 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 "
> 
> 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

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

[Carlos Corbacho]

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 <trenn@suse.de>

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 <carlos@strangeworlds.co.uk>
---

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


diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index dd13a37..0d69bc5 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -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;