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

[Mattia Dongili <malattia@linux.it>]

On Tue, Jun 07, 2011 at 01:26:06AM +0200, Marco Chiappero wrote:
> Il 07/06/2011 00:24, Mattia Dongili ha scritto:
> >>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?
> 
> Potentially many, user keys, power saving options, proximity
> sensors, als, etc. and any other user defined (eg. low backlight
> setting when compiling software).
> 
> >A system should have
> >only one application trying to control the backlight, having more than
> >one is asking for trouble in the first place.
> 
> But it is not when using sony-acpid.

How do you coordinate different policies, one trying to increase and one
trying to decrease brightness?
Or even, say you have two sony-acpid-like applications working with
different policies, how do you solve that? Create another backlight
sysfs file that overrules the other two?
Do you see where I'm going? I think you're trying to slove a problem at
the bottom of the stack when it should be solved at the top.

> >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.
> 
> Again another wrong assumption, IMO: if the user enables the
> autodimming feature everything should follow this rule. You are
> saying is that if you echo to sysfs, you really want to mess things
> up... really hard to believe.

possibly, I guess I just can't make my mind up about the fact that you
could set a backlight value via /sys/class/backlight/ that is not what
is going to applied.

> >>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.
> 
> Frankly, I cannot see how, especially right now... We have some
> hardware designed in a certain way that allows to be used only in
> that way, small changes are possible, but I really doubt that what
> you are asking for is possible.

I don't see how presenting illumination data and correction factors and
giving a way to change lid backlight is such an unreasonable request.
Why do you think is not possible with als chips you find on vaios?

...
> >the backlight change request is initiated in userspace already,
> 
> Right, but how can you be sure when you have a backlight device that
> can be accessed by anything?

I'm not following you here. Sure about what? Also, is als_backlight only
accessible to a few privileged ones?

> >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?
> 
> Please specify "interaction/integration part". There is almost
> nothing to be removed if you want code that makes sense and
> something working, if you specify a particular function I'll try to
> remove it, if it is feasible, otherwise be more specific, please.

I'd just like to see just the ALS driver in one patch, all the
managed/backlight stuff in another.

PS: to tell you the truth, I think the way Sony implemented all this
autodimming idea is just a clever workaround with limitations and
constraints they find in Windows where I guess they couldn't strip off
some of the standard acpi components. It has nothing to do with
hardware.
-- 
mattia
:wq!