[PATCH] Rationalise ACPI backlight implementation

[PATCH] Rationalise ACPI backlight implementation

[Matthew Garrett]

The sysfs backlight class provides no mechanism for querying the 
acceptable brightness for a backlight. The ACPI spec states that values 
are only valid if they are reported as available by the firmware. Since 
we can't provide that information to userspace, instead collapse the 
range to the number of actual values that can be set.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

---

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 521645e..12b2adb 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -296,18 +296,26 @@ static int acpi_video_device_set_state(struct acpi_video_device *device, int sta
 static int acpi_video_get_brightness(struct backlight_device *bd)
 {
 	unsigned long cur_level;
+	int i;
 	struct acpi_video_device *vd =
 		(struct acpi_video_device *)bl_get_data(bd);
 	acpi_video_device_lcd_get_level_current(vd, &cur_level);
-	return (int) cur_level;
+	for (i=2; i<vd->brightness->count; i++) {
+		if (vd->brightness->levels[i] == cur_level)
+			/* The first two entries are special - see page 575
+			   of the ACPI spec 3.0 */
+			return i-2;
+	}
+	return 0;
 }
 
 static int acpi_video_set_brightness(struct backlight_device *bd)
 {
-	int request_level = bd->props.brightness;
+	int request_level = bd->props.brightness+2;
 	struct acpi_video_device *vd =
 		(struct acpi_video_device *)bl_get_data(bd);
-	acpi_video_device_lcd_set_level(vd, request_level);
+	acpi_video_device_lcd_set_level(vd, 
+					vd->brightness->levels[request_level]);
 	return 0;
 }
 
@@ -656,7 +664,6 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
 	kfree(obj);
 
 	if (device->cap._BCL && device->cap._BCM && device->cap._BQC && max_level > 0){
-		unsigned long tmp;
 		static int count = 0;
 		char *name;
 		name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
@@ -664,11 +671,10 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
 			return;
 
 		sprintf(name, "acpi_video%d", count++);
-		acpi_video_device_lcd_get_level_current(device, &tmp);
 		device->backlight = backlight_device_register(name,
 			NULL, device, &acpi_backlight_ops);
-		device->backlight->props.max_brightness = max_level;
-		device->backlight->props.brightness = (int)tmp;
+		device->backlight->props.max_brightness = device->brightness->count-3;
+		device->backlight->props.brightness = acpi_video_get_brightness(device->backlight);
 		backlight_update_status(device->backlight);
 
 		kfree(name);

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

Re: [PATCH] Rationalise ACPI backlight implementation

[Matthew Garrett]

Len, I've had no feedback on this - the backlight maintainer thinks it's 
the right way to go, so I'd like to get it queued for .25 at least.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Rationalise ACPI backlight implementation

[Zhang Rui]

On Wed, 2007-12-26 at 02:03 +0000, Matthew Garrett wrote:
> The sysfs backlight class provides no mechanism for querying the 
> acceptable brightness for a backlight. The ACPI spec states that values 
> are only valid if they are reported as available by the firmware. Since 
> we can't provide that information to userspace, instead collapse the 
> range to the number of actual values that can be set.
> 
There is another patch which round the bd->props.brightness to the
nearest but larger acceptable brightness level.
Please refer to http://bugzilla.kernel.org/show_bug.cgi?id=9277

I have no obvious objection on either of these two proposals.
But one thing to mention is that
both of these two patches is written on the assumption that the
brightness levels listed in _BCL method are in ascending order, while
this is not stated in the ACPI spec.
Is this a problem?

Thanks,
Rui

> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> 
> ---
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 521645e..12b2adb 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -296,18 +296,26 @@ static int acpi_video_device_set_state(struct acpi_video_device *device, int sta
>  static int acpi_video_get_brightness(struct backlight_device *bd)
>  {
>  	unsigned long cur_level;
> +	int i;
>  	struct acpi_video_device *vd =
>  		(struct acpi_video_device *)bl_get_data(bd);
>  	acpi_video_device_lcd_get_level_current(vd, &cur_level);
> -	return (int) cur_level;
> +	for (i=2; i<vd->brightness->count; i++) {
> +		if (vd->brightness->levels[i] == cur_level)
> +			/* The first two entries are special - see page 575
> +			   of the ACPI spec 3.0 */
> +			return i-2;
> +	}
> +	return 0;
>  }
>  
>  static int acpi_video_set_brightness(struct backlight_device *bd)
>  {
> -	int request_level = bd->props.brightness;
> +	int request_level = bd->props.brightness+2;
>  	struct acpi_video_device *vd =
>  		(struct acpi_video_device *)bl_get_data(bd);
> -	acpi_video_device_lcd_set_level(vd, request_level);
> +	acpi_video_device_lcd_set_level(vd, 
> +					vd->brightness->levels[request_level]);
>  	return 0;
>  }
>  
> @@ -656,7 +664,6 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
>  	kfree(obj);
>  
>  	if (device->cap._BCL && device->cap._BCM && device->cap._BQC && max_level > 0){
> -		unsigned long tmp;
>  		static int count = 0;
>  		char *name;
>  		name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> @@ -664,11 +671,10 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
>  			return;
>  
>  		sprintf(name, "acpi_video%d", count++);
> -		acpi_video_device_lcd_get_level_current(device, &tmp);
>  		device->backlight = backlight_device_register(name,
>  			NULL, device, &acpi_backlight_ops);
> -		device->backlight->props.max_brightness = max_level;
> -		device->backlight->props.brightness = (int)tmp;
> +		device->backlight->props.max_brightness = device->brightness->count-3;
> +		device->backlight->props.brightness = acpi_video_get_brightness(device->backlight);
>  		backlight_update_status(device->backlight);
>  
>  		kfree(name);
> 

Re: [PATCH] Rationalise ACPI backlight implementation

[Matthew Garrett]

On Tue, Jan 22, 2008 at 04:33:29PM +0800, Zhang Rui wrote:

> I have no obvious objection on either of these two proposals.
> But one thing to mention is that
> both of these two patches is written on the assumption that the
> brightness levels listed in _BCL method are in ascending order, while
> this is not stated in the ACPI spec.
> Is this a problem?

The driver already makes that assumption, and it's implicit in the spec.

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

Re: [PATCH] Rationalise ACPI backlight implementation

[Henrique de Moraes Holschuh]

On Tue, 22 Jan 2008, Matthew Garrett wrote:
> On Tue, Jan 22, 2008 at 04:33:29PM +0800, Zhang Rui wrote:
> > I have no obvious objection on either of these two proposals.
> > But one thing to mention is that
> > both of these two patches is written on the assumption that the
> > brightness levels listed in _BCL method are in ascending order, while
> > this is not stated in the ACPI spec.
> > Is this a problem?
> 
> The driver already makes that assumption, and it's implicit in the spec.

Actually, no, it is not implicit.  The spec clearly states that the
brightness values will be cycled through the ones given by that list.  It is
implicit that brightness up actions will cycle through the list in order,
and brightness down will cycle through the list in reverse order.

If a vendor where to decide to place the most used brightness values in the
beginning of the list, and the rest of it later, he would be allowed to do
so.  Nobody was weird enough to do it yet though, AFAIK.

-- 
  "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] Rationalise ACPI backlight implementation

[Len Brown]

On Tuesday 25 December 2007 21:03, Matthew Garrett wrote:
> The sysfs backlight class provides no mechanism for querying the 
> acceptable brightness for a backlight. The ACPI spec states that values 
> are only valid if they are reported as available by the firmware. Since 
> we can't provide that information to userspace, instead collapse the 
> range to the number of actual values that can be set.
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

I wish we did this in the first place.
But doing it now is an API change -- since
with the old way 100 always meant 100% brightness, yes?

so my concern is that if we change what "10" means, somebody like akpm
with an existing script gets grumpy.

-Len

> ---
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 521645e..12b2adb 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -296,18 +296,26 @@ static int acpi_video_device_set_state(struct acpi_video_device *device, int sta
>  static int acpi_video_get_brightness(struct backlight_device *bd)
>  {
>  	unsigned long cur_level;
> +	int i;
>  	struct acpi_video_device *vd =
>  		(struct acpi_video_device *)bl_get_data(bd);
>  	acpi_video_device_lcd_get_level_current(vd, &cur_level);
> -	return (int) cur_level;
> +	for (i=2; i<vd->brightness->count; i++) {
> +		if (vd->brightness->levels[i] == cur_level)
> +			/* The first two entries are special - see page 575
> +			   of the ACPI spec 3.0 */
> +			return i-2;
> +	}
> +	return 0;
>  }
>  
>  static int acpi_video_set_brightness(struct backlight_device *bd)
>  {
> -	int request_level = bd->props.brightness;
> +	int request_level = bd->props.brightness+2;
>  	struct acpi_video_device *vd =
>  		(struct acpi_video_device *)bl_get_data(bd);
> -	acpi_video_device_lcd_set_level(vd, request_level);
> +	acpi_video_device_lcd_set_level(vd, 
> +					vd->brightness->levels[request_level]);
>  	return 0;
>  }
>  
> @@ -656,7 +664,6 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
>  	kfree(obj);
>  
>  	if (device->cap._BCL && device->cap._BCM && device->cap._BQC && max_level > 0){
> -		unsigned long tmp;
>  		static int count = 0;
>  		char *name;
>  		name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> @@ -664,11 +671,10 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
>  			return;
>  
>  		sprintf(name, "acpi_video%d", count++);
> -		acpi_video_device_lcd_get_level_current(device, &tmp);
>  		device->backlight = backlight_device_register(name,
>  			NULL, device, &acpi_backlight_ops);
> -		device->backlight->props.max_brightness = max_level;
> -		device->backlight->props.brightness = (int)tmp;
> +		device->backlight->props.max_brightness = device->brightness->count-3;
> +		device->backlight->props.brightness = acpi_video_get_brightness(device->backlight);
>  		backlight_update_status(device->backlight);
>  
>  		kfree(name);
> 

Re: [PATCH] Rationalise ACPI backlight implementation

[Matthew Garrett]

On Thu, Jan 24, 2008 at 04:44:48PM -0500, Len Brown wrote:
> On Tuesday 25 December 2007 21:03, Matthew Garrett wrote:
> > The sysfs backlight class provides no mechanism for querying the 
> > acceptable brightness for a backlight. The ACPI spec states that values 
> > are only valid if they are reported as available by the firmware. Since 
> > we can't provide that information to userspace, instead collapse the 
> > range to the number of actual values that can be set.
> > 
> > Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> 
> I wish we did this in the first place.
> But doing it now is an API change -- since
> with the old way 100 always meant 100% brightness, yes?

Yes.

> so my concern is that if we change what "10" means, somebody like akpm
> with an existing script gets grumpy.

I don't buy that argument. Assuming that the values will always result 
in an identical brightness is making incorrect assumptions about the 
API. For example, consider the case where a bug means that only half the 
hardware's available brightness levels are exposed. Fixing that bug 
would change the meaning of the numbers, but it's not an API change in 
any real way. Anyone using the backlight class should check the 
maximum_brightness field before deciding what range of values to use.

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

Re: [PATCH] Rationalise ACPI backlight implementation

[Andrew Morton]

> On Thu, 24 Jan 2008 16:44:48 -0500 Len Brown <lenb@kernel.org> wrote:
> On Tuesday 25 December 2007 21:03, Matthew Garrett wrote:
> > The sysfs backlight class provides no mechanism for querying the 
> > acceptable brightness for a backlight. The ACPI spec states that values 
> > are only valid if they are reported as available by the firmware. Since 
> > we can't provide that information to userspace, instead collapse the 
> > range to the number of actual values that can be set.
> > 
> > Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> 
> I wish we did this in the first place.
> But doing it now is an API change -- since
> with the old way 100 always meant 100% brightness, yes?
> 
> so my concern is that if we change what "10" means, somebody like akpm
> with an existing script gets grumpy.

It takes more than that to make me grumpy.  I've been very grumpy lately.

- Create a new /sys node with a new name which has the new semantics.

- Deprecate the old /sys entry by emitting an angry printk when someone
  uses it.

- Wait 12 months

- Kill the old one.

Re: [PATCH] Rationalise ACPI backlight implementation

[Matthew Garrett]

On Sat, Jan 26, 2008 at 10:00:45PM -0800, Andrew Morton wrote:
> - Create a new /sys node with a new name which has the new semantics.

The semantics are the same as they always have been - values between 0 
and max_brightness are valid values. If you've made assumptions about 
what max_brightness is, then that's a bug in the userspace application 
rather than a change in the semantics of the interface.

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

Re: [PATCH] Rationalise ACPI backlight implementation

[Andrew Morton]

On Mon, 28 Jan 2008 01:25:50 +0000 Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Sat, Jan 26, 2008 at 10:00:45PM -0800, Andrew Morton wrote:
> > - Create a new /sys node with a new name which has the new semantics.
> 
> The semantics are the same as they always have been - values between 0 
> and max_brightness are valid values. If you've made assumptions about 
> what max_brightness is, then that's a bug in the userspace application 
> rather than a change in the semantics of the interface.
> 

WTH?  My (utterly comedic chase-crap-around-the-tree) brightness script
was:

(
0 sh -c "echo $1 > /proc/acpi/sony/brightness"
0 sh -c "echo $1 > /proc/acpi/sony/brightness_default"
0 sh -c "echo $1 > /sys/class/backlight/sony/brightness"
0 sh -c "echo $1 > /sys/class/backlight/thinkpad_screen/brightness"
) 2>/dev/null

And yes, I had an rc.local command which assumed that 7 (later 8) is max
brightness.

You cannot seriously tell me that if we are to change this range from 0-8
up to 0-100 then this is not a backwards-incompatible change in
semantics.

My /sys/class/backlight/ directory is presently empty.  Rather than trying
to find out why, I think I'll just collapse in laughter.

Guys, sort it out, please.

Re: [PATCH] Rationalise ACPI backlight implementation

[Matthew Garrett]

On Sun, Jan 27, 2008 at 09:10:13PM -0800, Andrew Morton wrote:

> You cannot seriously tell me that if we are to change this range from 0-8
> up to 0-100 then this is not a backwards-incompatible change in
> semantics.

We're talking about changing 0-100 to 0-something sane, because the 
current driver is quite clearly broken. And yes, I'm perfectly happy to 
say that - the reason that max_brightness is exported is to allow 
userspace to determine what range of values is acceptable, and if 
userspace ignores that then things are going to break for it at some 
point.

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

Re: [PATCH] Rationalise ACPI backlight implementation

[Henrique de Moraes Holschuh]

On Sun, 27 Jan 2008, Andrew Morton wrote:
> 0 sh -c "echo $1 > /sys/class/backlight/thinkpad_screen/brightness"
> ) 2>/dev/null
> 
> And yes, I had an rc.local command which assumed that 7 (later 8) is max
> brightness.

You have to use 15 (or 16? I forget) on T61, X61, R61, X60(I think)... for
maximum brightness :-p   And the interface to get maximum brightness has
always been to cat max_brightness > brightness, AFAIK.

> My /sys/class/backlight/ directory is presently empty.  Rather than trying
> to find out why, I think I'll just collapse in laughter.

Well, I can tell you why it is empty on the thinkpad-acpi side.  It defaults
to brightness_enable=0 on Lenovo 16-brightness level boxes, where the right
thing to do is to control brightness using the X.org drivers or through the
ACPI brightness driver.

But I have no idea why it is empty re. the ACPI video driver.

> Guys, sort it out, please.

I am reading this ML with eyes fully open.  When a sensible (read: any)
consensus is found, I will follow it.

-- 
  "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] Rationalise ACPI backlight implementation

[Len Brown]

On Monday 28 January 2008 00:10, Andrew Morton wrote:
> On Mon, 28 Jan 2008 01:25:50 +0000 Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> 
> > On Sat, Jan 26, 2008 at 10:00:45PM -0800, Andrew Morton wrote:
> > > - Create a new /sys node with a new name which has the new semantics.
> > 
> > The semantics are the same as they always have been - values between 0 
> > and max_brightness are valid values. If you've made assumptions about 
> > what max_brightness is, then that's a bug in the userspace application 
> > rather than a change in the semantics of the interface.
> > 
> 
> WTH?  My (utterly comedic chase-crap-around-the-tree) brightness script
> was:
> 
> (
> 0 sh -c "echo $1 > /proc/acpi/sony/brightness"
> 0 sh -c "echo $1 > /proc/acpi/sony/brightness_default"
> 0 sh -c "echo $1 > /sys/class/backlight/sony/brightness"
> 0 sh -c "echo $1 > /sys/class/backlight/thinkpad_screen/brightness"
> ) 2>/dev/null
> 
> And yes, I had an rc.local command which assumed that 7 (later 8) is max
> brightness.
> 
> You cannot seriously tell me that if we are to change this range from 0-8
> up to 0-100 then this is not a backwards-incompatible change in
> semantics.
> 
> My /sys/class/backlight/ directory is presently empty.  Rather than trying
> to find out why, I think I'll just collapse in laughter.
> 
> Guys, sort it out, please.

I think that Matthew got it right.

The generic API is unchanged, brightness goes from 0 to max_brighness.

What he did was repair systems that use the acpi video driver
(which none of akpm's examples above do) such that the generic
API works for that case the same as it does with all other video drivers.

Andrew,
You might check if CONFIG_ACPI_VIDEO=m is set and you can load the "video" module.
While the sony may be non-standard and not load, your thinkpad may work.

thanks,
-Len

Re: [PATCH] Rationalise ACPI backlight implementation

[Henrique de Moraes Holschuh]

On Fri, 01 Feb 2008, Len Brown wrote:
> You might check if CONFIG_ACPI_VIDEO=m is set and you can load the "video" module.
> While the sony may be non-standard and not load, your thinkpad may work.

It will work except under new X.org, which disables BIOS backlight
functionality in order to do it directly, at least on Intel GPUs (and
probably also on ATI GPUs.  nVidia is no-man's-land).

And I have been told that X.org can do 256 backlight levels on Intel
hardware *including* switching the backlight off, instead of just the 16
levels Lenovo chose to export through ACPI.   Looks like someone should
add a kernel framebuffer device for those Intels :)

We really need to solve the userspace mess, though.

-- 
  "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] Rationalise ACPI backlight implementation

[Romano Giannetti]

On Sat, 2008-02-02 at 09:30 -0200, Henrique de Moraes Holschuh wrote:
> On Fri, 01 Feb 2008, Len Brown wrote:
> > You might check if CONFIG_ACPI_VIDEO=m is set and you can load the "video" module.
> > While the sony may be non-standard and not load, your thinkpad may work.
[...]
> 
> We really need to solve the userspace mess, though.
> 

I do not know if this is relevant... but for example, with a i915 hw
here (toshiba laptop):

#cat max_brightness 
100

#echo 50 > brightness ; cat actual_brightness 
0
#echo 60 > brightness ; cat actual_brightness 
0
#echo 70 > brightness ; cat actual_brightness 
70
#echo 80 > brightness ; cat actual_brightness 
0
#echo 90 > brightness ; cat actual_brightness 
0
#echo 100 > brightness ; cat actual_brightness 
100

#uname -a
Linux rukbat 2.6.24 #13 SMP PREEMPT Fri Feb 1 13:12:23 CET 2008 i686 GNU/Linux

which is at least strange (and probably broken). actual_brightness is
real, when it says 0 backlight is off. You can imagine what happens
using the backlight up/down control... flashing lights! 

Romano


-- 
Sorry for the disclaimer --- ¡I cannot stop it!



--
La presente comunicación tiene carácter confidencial y es para el exclusivo uso del destinatario indicado en la misma. Si Ud. no es el destinatario indicado, le informamos que cualquier forma de distribución, reproducción o uso de esta comunicación y/o de la información contenida en la misma están estrictamente prohibidos por la ley. Si Ud. ha recibido esta comunicación por error, por favor, notifíquelo inmediatamente al remitente contestando a este mensaje y proceda a continuación a destruirlo. Gracias por su colaboración.

This communication contains confidential information. It is for the exclusive use of the intended addressee. If you are not the intended addressee, please note that any form of distribution, copying or use of this communication or the information in it is strictly prohibited by law. If you have received this communication in error, please immediately notify the sender by reply e-mail and destroy this message. Thank you for your cooperation. 

Re: [PATCH] Rationalise ACPI backlight implementation

[Len Brown]

Applied.

thanks,
-Len

On Tuesday 25 December 2007 21:03, Matthew Garrett wrote:
> The sysfs backlight class provides no mechanism for querying the 
> acceptable brightness for a backlight. The ACPI spec states that values 
> are only valid if they are reported as available by the firmware. Since 
> we can't provide that information to userspace, instead collapse the 
> range to the number of actual values that can be set.
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> 
> ---
> 
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 521645e..12b2adb 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -296,18 +296,26 @@ static int acpi_video_device_set_state(struct acpi_video_device *device, int sta
>  static int acpi_video_get_brightness(struct backlight_device *bd)
>  {
>  	unsigned long cur_level;
> +	int i;
>  	struct acpi_video_device *vd =
>  		(struct acpi_video_device *)bl_get_data(bd);
>  	acpi_video_device_lcd_get_level_current(vd, &cur_level);
> -	return (int) cur_level;
> +	for (i=2; i<vd->brightness->count; i++) {
> +		if (vd->brightness->levels[i] == cur_level)
> +			/* The first two entries are special - see page 575
> +			   of the ACPI spec 3.0 */
> +			return i-2;
> +	}
> +	return 0;
>  }
>  
>  static int acpi_video_set_brightness(struct backlight_device *bd)
>  {
> -	int request_level = bd->props.brightness;
> +	int request_level = bd->props.brightness+2;
>  	struct acpi_video_device *vd =
>  		(struct acpi_video_device *)bl_get_data(bd);
> -	acpi_video_device_lcd_set_level(vd, request_level);
> +	acpi_video_device_lcd_set_level(vd, 
> +					vd->brightness->levels[request_level]);
>  	return 0;
>  }
>  
> @@ -656,7 +664,6 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
>  	kfree(obj);
>  
>  	if (device->cap._BCL && device->cap._BCM && device->cap._BQC && max_level > 0){
> -		unsigned long tmp;
>  		static int count = 0;
>  		char *name;
>  		name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> @@ -664,11 +671,10 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
>  			return;
>  
>  		sprintf(name, "acpi_video%d", count++);
> -		acpi_video_device_lcd_get_level_current(device, &tmp);
>  		device->backlight = backlight_device_register(name,
>  			NULL, device, &acpi_backlight_ops);
> -		device->backlight->props.max_brightness = max_level;
> -		device->backlight->props.brightness = (int)tmp;
> +		device->backlight->props.max_brightness = device->brightness->count-3;
> +		device->backlight->props.brightness = acpi_video_get_brightness(device->backlight);
>  		backlight_update_status(device->backlight);
>  
>  		kfree(name);
>