Re: [PATCH 23/25] sony-laptop: add ALS support

[Mattia Dongili <malattia@linux.it>]

On Mon, Jun 06, 2011 at 03:51:46PM +0200, Marco Chiappero wrote:
> Il 06/06/2011 15:08, Mattia Dongili ha scritto:
> >i.e. would it work if you always enable managed mode to receive the
> >ambient light events and just let userpace react for automatic dimming?
> >Also, userspace will intercept key-presses and scale the user
> >requested brightness change to an appropriate value using the current
> >lux values.
> 
> sony-acpid is already doing this without listening for key events
> (we can do that but we miss any other different backlight correction
> otherwise).

how many sources of backlight changes do you have? A system should have
only one application trying to control the backlight, having more than
one is asking for trouble in the first place.
Also, if I echo a number to /sys/... directly I would expect that to be
applied no matter what and without any magic correction going on.

> >At this point you could have a generic software (modulo the not yet
> >standardized als interface) handling any laptop that has these simpler
> >features.
> 
> The small big issue is that not every notebook use the same LCD
> panel, so, in theory, different calculations are required (and this
> also the reason why a model specific parameter list is also provided
> by every Vaio notebook too). A single suboptimal formula for
> everybody is not likely to provide satisfactory results.

this is something else that should be thought of presenting in a unified
way to userspace. The entire idea of platform drivers is to enable and
unify how platform specific features are handled. Having each one of
them diverge to its own custom interface defeats the whole purpose.

> >the question was why sending a "user requested to change
> >brightness"-event instead of setting the brightness. The calculation
> >should be done in userspace
> 
> ...taking into account both the current
> user/power_saving_sofware/etc. backlight correction level and the
> ambient light level. We need to forward both informations to
> userspace otherwise no calculation is possible.

the backlight change request is initiated in userspace already, why do
you need to tell userspace that it generated a backlight change request?

So again, can you split the patch in two so that we have the driver in
one patch and the interaction/integration part in another?

Thanks
-- 
mattia
:wq!