Brightness change on notebook that follow ACPI specification (par. B.7)

[Dan Lukes]

On 03/31/10 01:32, Jung-uk Kim:
>>    --- 1 ---------------
>> /* events */
>> #define	VID_NOTIFY_SWITCHED	0x80
>> #define	VID_NOTIFY_REPROBE	0x81
>> #define	VID_NOTIFY_CYCLE_BRN	0x85
>> #define	VID_NOTIFY_INC_BRN	0x86
>> #define	VID_NOTIFY_DEC_BRN	0x87
>> #define	VID_NOTIFY_ZERO_BRN	0x88
>>
>> The events 0x80 and 0x81 are "Display Devices" device specific
>> notifications (according ACPI specification B.5), but events
>> 0x85-0x88 are "Output devices" device specific notification (ACPI
>> spec. B.7). The common name VID_NOTIFY_* for values that come from
>> different domains is confusing. Note that future versions of ACPI
>> specification may overlaps (there may be defined 0x80 event for
>> Output device or event 0x85 event for Display device).
>
> I didn't think it was necessary because ACPI will not define
> overlapping events, AFAIK.

Of course. Language problem. I'm speaking about the meaning of the 
event. We are speaking about range of "device specific" events and they 
are different meaning for every type of devices.

>> Handling of event 0x85 refuse do anything when there are less than
>> four levels. I see no reason for such limitation - we can safely
>> cycle trougth three or even trougth two levels as well.
>
> B.6.2 _BCL (Query List of Brightness Control Levels Supported)
>
> The first number in the package is the level of the panel when full
> power is connected to the machine. The second number in the package is
> the level of the panel when the machine is on batteries. All other
> numbers are treated as a list of levels OSPM will cycle through when
> the user toggles (via a keystroke) the brightness level of the
> display.
>
> Please note the last sentence.  I know a lot of BIOSes do not follow
> the rule but I wanted to implement it as close as possible.

There is no "only" word in mentioned sequence. They are list of levels 
the OSPM will cycle through when ..., but sentence doesn't say that the 
levels come from such list only. It's sounds like your over-assumption 
of text rather than "close as possible" implementation. Or, in the world 
of mathematics symbols, '=>' operator is not '<=>'.

I interpret whole _BCL list as list of levels. Yes, first two levels 
have additional meaning, but it doesn't mean they lost the basic attribute.

You should look into description of event, e.g. table B-7 which sounds 
very clear to me. It doesn't distinquish between first two levels in the 
list and other levels in the list.

But I'm not native english speaker, so it's hard to dispute about exact 
meaning of the specification text and meaning of wording in the 
sentences for me.

In the fact, I can say for sure only I see nothing in the specification, 
that can be interpret as "with only three levels in _BCL you can't cycle 
trough it".

>> It advance, the 0x85 handling assume the _BCL list is sorted and
>> ignores vo_levels[0] and vo_levels[1] (note that handling of
>> 0x86/0x87 just few lines bellow doesn't assume sorted levels nor
>> ignore levels [0] and [1]).
>
> It was intentional, i.e., I didn't want to implement increase/decrease
> via cycle up/down or to miss the first two levels.  The spec. was
> little vague about those cases and I abused it. :-)

I'm sayed the actual implementation looks to me like not to be optimal 
approach. Nothing more...

>> The 0x88 handling is duplicate implementation of
>> acpi_video_vo_check_level instead just calling the already
>> implemented function.
>
> Again, it was intentional, i.e., to mimic the style of other cases in
> the switch statement and to avoid an unnecessary assertion.

So you implemented the functionality you already have just few lines 
above because it looks more uniform when printed on paper ? Well, it's 
not my way, but it's your code and it works, so it's between you and 
comitter - and because you are comitter, then it's between you and you ;-)

							Dan



P.S. Don't forget the english is not my native language. It doesn't 
affect my interpertation of the specification text only but such 
disputation as well.