LCD brightness capability question

[Danny Kukawka]

On Tuesday 18 April 2006 11:47, Richard Hughes wrote:
> On Tue, 2006-04-18 at 11:15 +0200, Jan Jockusch wrote:
> > Hello Richard,
> >
> > >Can you try the attached patch please. It was created using HEAD, but
> > >should apply to 0.5.7 cleanly.
> >
> > It does. Even overlaying it with the Debian specific patches is no
> > problem (a few lines of offset, that's it).
> >
> > >If it's successful then things should just work, and the laptop_panel
> > >capability should be seen in hal-device-manager and gnome-power-manager.
> >
> > This is too good to be true. Laptop brightness control works like a
> > charm now in gnome-power-control. You're really fast! Thanks a lot! Feel
> > free to commit the patch ;-)
>
> David, good to commit?

No, it's not good to commit. 

I assume he use the generic ACPI video driver (modprobe video) and not a 
special driver e.g. from/for futjisu. 

1.) I can't see where you check if this machine is a) a FujitsuSiemens or/and 
b) this special machine in your patch. If this patch go in, HAL offer set 
brightness for all machines with loaded video module and which have this path 
in /proc/acpi. But this happens also on several (in fact many) other machines 
- on this point we are at:

2.) I see no check if the device really support set brightness. You need to 
check if there are values returned which make sense (e.g. not <not 
supported> )

3.) Also if the device not return <not supported>: this mean not that set 
brightness is supported by this machine. This can also differ in several 
cases from bios version to bios version. And on many (maybe the most) this 
does not work.

4.) Also if a device support set brightness via this path the number of 
brightness levels can differ from '8'.

You see it make no sense to commit this in the CVS.

Cheers,

Danny