brightness control on thinkpad t61p

brightness control on thinkpad t61p

[Andrew Morton]

When I fire up the latest Linus tree on my thinkpad t61p I get:

thinkpad_acpi: ThinkPad ACPI Extras v0.17
thinkpad_acpi: http://ibm-acpi.sf.net/
thinkpad_acpi: ThinkPad BIOS 7LET44WW (1.14 ), EC 7KHT22WW-1.06
thinkpad_acpi: Lenovo ThinkPad T61p
thinkpad_acpi: radio switch found; radios are enabled
ACPI: Lid Switch [LID]
input: Sleep Button (CM) as /class/input/input6
ACPI: Sleep Button (CM) [SLPB]
thinkpad_acpi: standard ACPI backlight interface available, not loading native one...



and /sys/class/backlight/thinkpad_screen/brightness is no longer present, so
my `brightness' script no longer works.  That, my friends, is a regression.

Here's my script:

(
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

which rather shows how pathetic we are in this area.  Ho hum.


So I go hunting around and find

	/sys/class/backlight/acpi_video0/
and
	/sys/class/backlight/acpi_video1/

Why are there two of them?


Both of these have a `brightness' entry which has contents of 100.  When I set
that to 10, the screen's brightness is not reduced.

So as far as I can tell, we have lost the ability to alter the brightness of
the screen on this machine.

Re: brightness control on thinkpad t61p

[Rafael J. Wysocki]

On Sunday, 23 of December 2007, Andrew Morton wrote:
> 
> When I fire up the latest Linus tree on my thinkpad t61p I get:
> 
> thinkpad_acpi: ThinkPad ACPI Extras v0.17
> thinkpad_acpi: http://ibm-acpi.sf.net/
> thinkpad_acpi: ThinkPad BIOS 7LET44WW (1.14 ), EC 7KHT22WW-1.06
> thinkpad_acpi: Lenovo ThinkPad T61p
> thinkpad_acpi: radio switch found; radios are enabled
> ACPI: Lid Switch [LID]
> input: Sleep Button (CM) as /class/input/input6
> ACPI: Sleep Button (CM) [SLPB]
> thinkpad_acpi: standard ACPI backlight interface available, not loading native one...
> 
> 
> 
> and /sys/class/backlight/thinkpad_screen/brightness is no longer present, so
> my `brightness' script no longer works.  That, my friends, is a regression.
> 
> Here's my script:
> 
> (
> 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
> 
> which rather shows how pathetic we are in this area.  Ho hum.
> 
> 
> So I go hunting around and find
> 
> 	/sys/class/backlight/acpi_video0/
> and
> 	/sys/class/backlight/acpi_video1/
> 
> Why are there two of them?
> 
> 
> Both of these have a `brightness' entry which has contents of 100.  When I set
> that to 10, the screen's brightness is not reduced.
> 
> So as far as I can tell, we have lost the ability to alter the brightness of
> the screen on this machine.

FYI, I have created a bugzilla entry for this issue at:
http://bugzilla.kernel.org/show_bug.cgi?id=9625
and added it to the list of known recent regressions.

Thanks,
Rafael

Re: brightness control on thinkpad t61p

[Matthew Garrett]

On Sun, Dec 23, 2007 at 12:00:57AM -0800, Andrew Morton wrote:

> and /sys/class/backlight/thinkpad_screen/brightness is no longer present, so
> my `brightness' script no longer works.  That, my friends, is a regression.

The thinkpad_acpi brightness interface simply doesn't work correctly on 
recent Thinkpads. The ACPI interface does, but the currently exported ui 
to it approximates some sort of horrific loss. I'll try to fix that in 
the next week or so.

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

Re: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Sun, 23 Dec 2007, Andrew Morton wrote:
> When I fire up the latest Linus tree on my thinkpad t61p I get:
> 
> thinkpad_acpi: ThinkPad ACPI Extras v0.17
> thinkpad_acpi: http://ibm-acpi.sf.net/
> thinkpad_acpi: ThinkPad BIOS 7LET44WW (1.14 ), EC 7KHT22WW-1.06
> thinkpad_acpi: Lenovo ThinkPad T61p
> thinkpad_acpi: radio switch found; radios are enabled
> ACPI: Lid Switch [LID]
> input: Sleep Button (CM) as /class/input/input6
> ACPI: Sleep Button (CM) [SLPB]
> thinkpad_acpi: standard ACPI backlight interface available, not loading native one...
> 
> and /sys/class/backlight/thinkpad_screen/brightness is no longer present, so
> my `brightness' script no longer works.  That, my friends, is a regression.

For thinkpad-acpi at least, it is not a regression. 2.6.23/0.16 did NOT
support your thinkpad (it will pretend to work, but it won't work 100% right
as it only supports 8 levels of backlight control).  2.6.24-rc4/0.17 added
support for it (16-level brightness), but also added the automatic detection
of ACPI generic video support.

You can ask thinkpad-acpi for the backlight interface using the
"brightness_enable=1" parameter, if you'd rather use it instead of the
generic ACPI video driver.  I don't know if you can ask video to not enable
backlight control, though.

> So I go hunting around and find
> 
> 	/sys/class/backlight/acpi_video0/
> and
> 	/sys/class/backlight/acpi_video1/
> 
> Why are there two of them?

Two nodes in the ACPI tree (AGP and PCI), and the ACPI drivers don't
differentiate a deactivated node from a working node yet.  There are some
tentative patches flying around to fix it, AFAIK.

> Both of these have a `brightness' entry which has contents of 100.  When I set
> that to 10, the screen's brightness is not reduced.

One of them should work.  Maybe X.org is doing something?

-- 
  "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: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Mon, 24 Dec 2007, Matthew Garrett wrote:
> On Sun, Dec 23, 2007 at 12:00:57AM -0800, Andrew Morton wrote:
> 
> > and /sys/class/backlight/thinkpad_screen/brightness is no longer present, so
> > my `brightness' script no longer works.  That, my friends, is a regression.
> 
> The thinkpad_acpi brightness interface simply doesn't work correctly on 
> recent Thinkpads. The ACPI interface does, but the currently exported ui 

Yes, it does since 2.6.24-rc4.  And since MUCH earlier in the out-of-tree
develompent versions.

What usually causes weird breakage is to have more than one of (Ubuntu
hackage in HAL calling back into thinkpad-acpi, X.org, ACPI video) trying to
respond to brightness up/down.  And you *do* *have* to use the latest Lenovo
BIOSes, too. 

Also, apparently now X.org knows how to tell the BIOS to not react to
backlight changes, so you have to configure it properly (I don't know much
about it), too.

Matthew, btw, Dmitri approved the _NOTIFY keycodes, which need to be part of
the solution re. backlight control in thinkpads.  Are you intending to push
a patch implementing them?  As I said before, I am okay with that solution,
as long as Dmitri accepted it, and I will use _NOTIFY keycodes in
thinkpad-acpi when they become available.

-- 
  "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: brightness control on thinkpad t61p

[Andrew Morton]

On Mon, 24 Dec 2007 07:34:06 +0000 Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Sun, Dec 23, 2007 at 12:00:57AM -0800, Andrew Morton wrote:
> 
> > and /sys/class/backlight/thinkpad_screen/brightness is no longer present, so
> > my `brightness' script no longer works.  That, my friends, is a regression.
> 
> The thinkpad_acpi brightness interface simply doesn't work correctly on 
> recent Thinkpads. The ACPI interface does, but the currently exported ui 
> to it approximates some sort of horrific loss. I'll try to fix that in 
> the next week or so.

Vice versa for me.  The thinkpad_acpi driver in 2.6.23 _does_ work,
but its interface vanished in 2.6.24-rc5 and the acpi driver does not
work.

Re: brightness control on thinkpad t61p

[Andrew Morton]

On Mon, 24 Dec 2007 15:08:37 -0200 Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> On Sun, 23 Dec 2007, Andrew Morton wrote:
> > When I fire up the latest Linus tree on my thinkpad t61p I get:
> > 
> > thinkpad_acpi: ThinkPad ACPI Extras v0.17
> > thinkpad_acpi: http://ibm-acpi.sf.net/
> > thinkpad_acpi: ThinkPad BIOS 7LET44WW (1.14 ), EC 7KHT22WW-1.06
> > thinkpad_acpi: Lenovo ThinkPad T61p
> > thinkpad_acpi: radio switch found; radios are enabled
> > ACPI: Lid Switch [LID]
> > input: Sleep Button (CM) as /class/input/input6
> > ACPI: Sleep Button (CM) [SLPB]
> > thinkpad_acpi: standard ACPI backlight interface available, not loading native one...
> > 
> > and /sys/class/backlight/thinkpad_screen/brightness is no longer present, so
> > my `brightness' script no longer works.  That, my friends, is a regression.
> 
> For thinkpad-acpi at least, it is not a regression. 2.6.23/0.16 did NOT
> support your thinkpad (it will pretend to work, but it won't work 100% right
> as it only supports 8 levels of backlight control).

Well it may have been partially working, but it worked.

>  2.6.24-rc4/0.17 added
> support for it (16-level brightness), but also added the automatic detection
> of ACPI generic video support.

Doesn't work.

> You can ask thinkpad-acpi for the backlight interface using the
> "brightness_enable=1" parameter, if you'd rather use it instead of the
> generic ACPI video driver.  I don't know if you can ask video to not enable
> backlight control, though.

I shouldn't have to add some module parameter to get previously-working
stuff to work again.

> > So I go hunting around and find
> > 
> > 	/sys/class/backlight/acpi_video0/
> > and
> > 	/sys/class/backlight/acpi_video1/
> > 
> > Why are there two of them?
> 
> Two nodes in the ACPI tree (AGP and PCI), and the ACPI drivers don't
> differentiate a deactivated node from a working node yet.  There are some
> tentative patches flying around to fix it, AFAIK.
> 
> > Both of these have a `brightness' entry which has contents of 100.  When I set
> > that to 10, the screen's brightness is not reduced.
> 
> One of them should work.  Maybe X.org is doing something?

Dunno.

Re: brightness control on thinkpad t61p

[Matthew Garrett]

On Wed, Dec 26, 2007 at 02:10:27PM -0800, Andrew Morton wrote:

> Vice versa for me.  The thinkpad_acpi driver in 2.6.23 _does_ work,
> but its interface vanished in 2.6.24-rc5 and the acpi driver does not
> work.

As I said, the interface for the ACPI backlight driver is confusing. 10 
will only work if the firmware supports it - have you tried other 
values?

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

Re: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Wed, 26 Dec 2007, Andrew Morton wrote:
> Vice versa for me.  The thinkpad_acpi driver in 2.6.23 _does_ work,
> but its interface vanished in 2.6.24-rc5 and the acpi driver does not
> work.

As I said, give thinkpad-acpi the brightness_enable=1 parameter, and it will
export its backlight interface, regardless of the presence of a generic ACPI
backlight interface in the AML...

-- 
  "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: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Wed, 26 Dec 2007, Matthew Garrett wrote:
> On Wed, Dec 26, 2007 at 02:10:27PM -0800, Andrew Morton wrote:
> 
> > Vice versa for me.  The thinkpad_acpi driver in 2.6.23 _does_ work,
> > but its interface vanished in 2.6.24-rc5 and the acpi driver does not
> > work.
> 
> As I said, the interface for the ACPI backlight driver is confusing. 10 
> will only work if the firmware supports it - have you tried other 
> values?

Not on a thinkpad.  The AML code is not that stupid, it rounds down to the
nearest supported value.   It takes a brightness percentage (0 to 100), maps
it through a table to get the visual brightness -> hardware brightness curve
right (it is non-linear), then does what thinkpad-acpi does to set backlight
brightness.

-- 
  "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: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Wed, 26 Dec 2007, Andrew Morton wrote:
> > For thinkpad-acpi at least, it is not a regression. 2.6.23/0.16 did NOT
> > support your thinkpad (it will pretend to work, but it won't work 100% right
> > as it only supports 8 levels of backlight control).
> 
> Well it may have been partially working, but it worked.

See below...

> >  2.6.24-rc4/0.17 added
> > support for it (16-level brightness), but also added the automatic detection
> > of ACPI generic video support.
> 
> Doesn't work.

Needs fixing, then.  THAT is the way to go in the future, including for
thinkpads.

> > You can ask thinkpad-acpi for the backlight interface using the
> > "brightness_enable=1" parameter, if you'd rather use it instead of the
> > generic ACPI video driver.  I don't know if you can ask video to not enable
> > backlight control, though.
> 
> I shouldn't have to add some module parameter to get previously-working
> stuff to work again.

It was not working properly, and it was unsupported.  Things that work
half-way by accident are NOT considered to be supported in my book.

While I don't mind at all to leave it enabled when there are other drivers
also taking care of brightness, I am afraid it will cause an even worse
mess, since ACPI video will attempt to control the backlight level as
well.  So, I have disabled the thinkpad-specific interface when the presence
of the generic interface (driven by ACPI video) is detected.

> > > Both of these have a `brightness' entry which has contents of 100.
> > > When I set that to 10, the screen's brightness is not reduced.
> > 
> > One of them should work.  Maybe X.org is doing something?
> 
> Dunno.

Really up-to-date X.org can completely disable the BIOS backlight handling
by changing some registers in the GPUs, at least for Radeons (and probably
Intel GPUs).  I believe there are ways to control it through xbacklight
options, but I don't have that X.org yet, so I can't tell what to try.

-- 
  "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: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Thu, 27 Dec 2007, Henrique de Moraes Holschuh wrote:
> On Wed, 26 Dec 2007, Matthew Garrett wrote:
> > On Wed, Dec 26, 2007 at 02:10:27PM -0800, Andrew Morton wrote:
> > 
> > > Vice versa for me.  The thinkpad_acpi driver in 2.6.23 _does_ work,
> > > but its interface vanished in 2.6.24-rc5 and the acpi driver does not
> > > work.
> > 
> > As I said, the interface for the ACPI backlight driver is confusing. 10 
> > will only work if the firmware supports it - have you tried other 
> > values?
> 
> Not on a thinkpad.  The AML code is not that stupid, it rounds down to the
> nearest supported value.   It takes a brightness percentage (0 to 100), maps
> it through a table to get the visual brightness -> hardware brightness curve
> right (it is non-linear), then does what thinkpad-acpi does to set backlight
> brightness.

Bah, I spoke too soon.  Latest round of BIOSes seems to have broken this,
either that or I completely misunderstood the older AML code.

Rounding to the nearest supplied _BCL value before we call _BCM apparently
will be needed on thinkpads as well.

Oh, just another datapoint for Andrew: your thinkpad backlight firmware
behaves differently before and after something calls _BCL.  And I believe it
also behaves differently before Linux boots (my guess is that it changes
behaviour when ACPI inits), so it is getting more and more complicated to
debug this thing without direct access to one such thinkpad.

I heavily recommend calling the ACPI _BCL method (e.g. by loading ACPI video
and unloading it later if you don't want it), so that at least you have a
small chance of things working consistently :) The AML code is supposed to
just issue normal ACPI events and leave the backlight hardware alone after
you call _BCL.

But if X.org is messing with the BIOS scratch registers, not even ACPI
video.c will work, as the SMBIOS crap the thinkpad AML code calls to
implement BQC/BCQ and BCM will be disabled.  In that case, X.org has to be
able to modify the hardware directly (through xbacklight) in order to get
anything done.

I am seriously considering a call to _BCM in thinkpad-acpi, to make sure we
get a more-or-less sane thinkpad behaviour on these newer Lenovo BIOSes,
even if the user doesn't want to use ACPI video.c for some reason.

-- 
  "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: brightness control on thinkpad t61p

[Matthew Garrett]

On Sun, Jan 06, 2008 at 11:36:23PM -0200, Henrique de Moraes Holschuh wrote:

> Bah, I spoke too soon.  Latest round of BIOSes seems to have broken this,
> either that or I completely misunderstood the older AML code.
> 
> Rounding to the nearest supplied _BCL value before we call _BCM apparently
> will be needed on thinkpads as well.

We should just stop exposing the 0-100 range, and instead map it into a 
contiguous (smaller) range. I've posted a patch to do that.

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

Re: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Mon, 07 Jan 2008, Matthew Garrett wrote:
> On Sun, Jan 06, 2008 at 11:36:23PM -0200, Henrique de Moraes Holschuh wrote:
> > Bah, I spoke too soon.  Latest round of BIOSes seems to have broken this,
> > either that or I completely misunderstood the older AML code.
> > 
> > Rounding to the nearest supplied _BCL value before we call _BCM apparently
> > will be needed on thinkpads as well.
> 
> We should just stop exposing the 0-100 range, and instead map it into a 
> contiguous (smaller) range. I've posted a patch to do that.

Should we?  Why?  We lose information doing that.  Instead of a nice linear
0-100% brightness scale, you are now back to an 8 or 16-level non-linear
brightness scale.

0 to 100% is hardware agnostic.  You know where the middle backlight level
is.  You know where the one quarter, and three quarter levels are.  You
don't know anything about the brightness level anymore, after you compress
it to an array index.

It is a step backwards IMHO.

-- 
  "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: brightness control on thinkpad t61p

[Matthew Garrett]

On Mon, Jan 07, 2008 at 10:32:46PM -0200, Henrique de Moraes Holschuh wrote:

> Should we?  Why?  We lose information doing that.  Instead of a nice linear
> 0-100% brightness scale, you are now back to an 8 or 16-level non-linear
> brightness scale.

The spec makes no guarantees about what that 0-100 range means - it may 
be brightness, power consumption or bonghits. It's really not guaranteed 
to be meaningful, and the backlight class doesn't provide that guarantee 
either.

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

Re: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Tue, 08 Jan 2008, Matthew Garrett wrote:
> On Mon, Jan 07, 2008 at 10:32:46PM -0200, Henrique de Moraes Holschuh wrote:
> 
> > Should we?  Why?  We lose information doing that.  Instead of a nice linear
> > 0-100% brightness scale, you are now back to an 8 or 16-level non-linear
> > brightness scale.
> 
> The spec makes no guarantees about what that 0-100 range means - it may 
> be brightness, power consumption or bonghits. It's really not guaranteed 
> to be meaningful, and the backlight class doesn't provide that guarantee 
> either.

True.  But it is still a step backwards for firmware that do better.

And, frankly, the backlight class is so spartan, I don't consider missing
features in it anything but a "time to fix it" issue.  It is just like the
LED class, it is too spartan to actually do all we need it to in some cases.

Is there a *real* technical reason why we can't just round to the nearest?
That will work well with any firmware, and it will not remove functionality
from any box, while at the same time plugging the current issues nicely.

-- 
  "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: brightness control on thinkpad t61p

[Matthew Garrett]

On Tue, Jan 08, 2008 at 10:06:53AM -0200, Henrique de Moraes Holschuh wrote:

> Is there a *real* technical reason why we can't just round to the nearest?
> That will work well with any firmware, and it will not remove functionality
> from any box, while at the same time plugging the current issues nicely.

Yes. Software makes the assumption that writing a value larger than the 
current value will result in the value increasing, which isn't 
necessarily true if you round it.

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

Re: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Tue, 08 Jan 2008, Matthew Garrett wrote:
> On Tue, Jan 08, 2008 at 10:06:53AM -0200, Henrique de Moraes Holschuh wrote:
> > Is there a *real* technical reason why we can't just round to the nearest?
> > That will work well with any firmware, and it will not remove functionality
> > from any box, while at the same time plugging the current issues nicely.
> 
> Yes. Software makes the assumption that writing a value larger than the 
> current value will result in the value increasing, which isn't 
> necessarily true if you round it.

I dare say that such an assumption is broken for the backlight class, there
is a reason why we have actual_brightness and brightness separate, and it is
just that one AFAIK.  I am cc'ing the backlight class maintainer to get his
opinion on the matter.

That doesn't mean we shouldn't improve the backlight class to give it
something like _BCL does as an *optional* attribute (but with extra explicit
guarantees, such as the one that it will list values sorted from lowest to
highest, etc), but it does mean that anything assuming that adding one to a
brightness level in the backlight class will INCREASE brightness is just
plain *broken*.  If actual_brightness did not change, then the backlight
brightness level did not change either.

Heck, back to the video.c driver, you shouldn't even assume _BCL gives you a
sorted list after the second element.  I admit it takes an special kind of
weird firmware to screw with _BCL like that, but the ACPI spec doesn't
mandate it, it just says that the OSPM will cycle through the values
returned by _BCL, but nowhere does it define HOW one cycles through those...

-- 
  "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: brightness control on thinkpad t61p

[Matthew Garrett]

On Tue, Jan 08, 2008 at 10:48:20AM -0200, Henrique de Moraes Holschuh wrote:

> Heck, back to the video.c driver, you shouldn't even assume _BCL gives you a
> sorted list after the second element.  I admit it takes an special kind of
> weird firmware to screw with _BCL like that, but the ACPI spec doesn't
> mandate it, it just says that the OSPM will cycle through the values
> returned by _BCL, but nowhere does it define HOW one cycles through those...

Sorting the list is hardly a problem - we can clearly do it if 
necessary.

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

Re: brightness control on thinkpad t61p

[Richard Purdie]

On Tue, 2008-01-08 at 10:48 -0200, Henrique de Moraes Holschuh wrote:
> On Tue, 08 Jan 2008, Matthew Garrett wrote:
> > On Tue, Jan 08, 2008 at 10:06:53AM -0200, Henrique de Moraes Holschuh wrote:
> > > Is there a *real* technical reason why we can't just round to the nearest?
> > > That will work well with any firmware, and it will not remove functionality
> > > from any box, while at the same time plugging the current issues nicely.
> > 
> > Yes. Software makes the assumption that writing a value larger than the 
> > current value will result in the value increasing, which isn't 
> > necessarily true if you round it.
> 
> I dare say that such an assumption is broken for the backlight class, there
> is a reason why we have actual_brightness and brightness separate, and it is
> just that one AFAIK.  I am cc'ing the backlight class maintainer to get his
> opinion on the matter.

The reason actual_brightness and brightness are two separate things are
because some hardware exists that will let you request one brightness
but choose one that is different itself. There are also some devices
that have backlight limiting in software for use in emergency low
battery situations (the battery can get low enough that the device
reboots on high backlight but is otherwise usable).

I did't get enough context above but I went through the archives and it
seems this is about linearising backlight values.

You can't just export an interface of 0-100 and hope things will work
since you lose the information about which values do something and which
don't.

I agree something which tells us what those brightness values mean could
be useful though, a kind of brightness value <-> relative brightness
table. The framebuffer layer has something like this already from before
the backlight core was abstracted/standardised. I'd look at that as a
first port of call and try to see if it could be integrated into the
backlight class somehow.

Cheers,

Richard

Re: brightness control on thinkpad t61p

[Matthew Garrett]

On Tue, Jan 08, 2008 at 03:45:02PM +0000, Richard Purdie wrote:

> I did't get enough context above but I went through the archives and it
> seems this is about linearising backlight values.

Indeed. The ACPI spec provides a range of 0-100, without specifying what 
this actually means (it gives brightness and power consumption as two 
different examples). Implementations are only required to support a 
subset of these, with the others being ignored. The current hook into 
the backlight class exports this range but provides no means for an 
application to determine which values are valid - I'd prefer to just 
flatten the range to remove the holes. Given the lack of standardisation 
in the real meaning of the values, I don't think exporting the 0-100 
range buys us anything.

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

Re: brightness control on thinkpad t61p

[Richard Purdie]

On Tue, 2008-01-08 at 15:54 +0000, Matthew Garrett wrote:
> On Tue, Jan 08, 2008 at 03:45:02PM +0000, Richard Purdie wrote:
> 
> > I did't get enough context above but I went through the archives and it
> > seems this is about linearising backlight values.
> 
> Indeed. The ACPI spec provides a range of 0-100, without specifying what 
> this actually means (it gives brightness and power consumption as two 
> different examples). Implementations are only required to support a 
> subset of these, with the others being ignored. The current hook into 
> the backlight class exports this range but provides no means for an 
> application to determine which values are valid - I'd prefer to just 
> flatten the range to remove the holes. Given the lack of standardisation 
> in the real meaning of the values, I don't think exporting the 0-100 
> range buys us anything.

I agree with that. 0-100 actually breaks a useful and valid way the
class gets used in the "brightness + 1" case...

Cheers,

Richard

Re: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Tue, 08 Jan 2008, Richard Purdie wrote:
> On Tue, 2008-01-08 at 15:54 +0000, Matthew Garrett wrote:
> > On Tue, Jan 08, 2008 at 03:45:02PM +0000, Richard Purdie wrote:
> > 
> > > I did't get enough context above but I went through the archives and it
> > > seems this is about linearising backlight values.
> > 
> > Indeed. The ACPI spec provides a range of 0-100, without specifying what 
> > this actually means (it gives brightness and power consumption as two 
> > different examples). Implementations are only required to support a 
> > subset of these, with the others being ignored. The current hook into 
> > the backlight class exports this range but provides no means for an 
> > application to determine which values are valid - I'd prefer to just 
> > flatten the range to remove the holes. Given the lack of standardisation 
> > in the real meaning of the values, I don't think exporting the 0-100 
> > range buys us anything.
> 
> I agree with that. 0-100 actually breaks a useful and valid way the
> class gets used in the "brightness + 1" case...

So be it, then.  But my request that we get a way to add the information we
are losing to the backlight class still stands.

-- 
  "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: brightness control on thinkpad t61p

[Richard Purdie]

On Tue, 2008-01-08 at 14:49 -0200, Henrique de Moraes Holschuh wrote:
> On Tue, 08 Jan 2008, Richard Purdie wrote:
> > On Tue, 2008-01-08 at 15:54 +0000, Matthew Garrett wrote:
> > > On Tue, Jan 08, 2008 at 03:45:02PM +0000, Richard Purdie wrote:
> > > 
> > > > I did't get enough context above but I went through the archives and it
> > > > seems this is about linearising backlight values.
> > > 
> > > Indeed. The ACPI spec provides a range of 0-100, without specifying what 
> > > this actually means (it gives brightness and power consumption as two 
> > > different examples). Implementations are only required to support a 
> > > subset of these, with the others being ignored. The current hook into 
> > > the backlight class exports this range but provides no means for an 
> > > application to determine which values are valid - I'd prefer to just 
> > > flatten the range to remove the holes. Given the lack of standardisation 
> > > in the real meaning of the values, I don't think exporting the 0-100 
> > > range buys us anything.
> > 
> > I agree with that. 0-100 actually breaks a useful and valid way the
> > class gets used in the "brightness + 1" case...
> 
> So be it, then.  But my request that we get a way to add the information we
> are losing to the backlight class still stands.

The thing is does this 0-100 value have a use to userspace? Its
extremely device specific and can't be exported from the class in a
generic way suggesting it should really be something exported by the
driver itself. You still have to ask whether userspace actually needs
this value though?

Kernel debugging is the one use I can see. For that you'd probably be
better off with a pr_debug() though. There are hundreds of other
'useful' numbers the kernel has but doesn't export.

Cheers,

Richard

Re: brightness control on thinkpad t61p

[Henrique de Moraes Holschuh]

On Tue, 08 Jan 2008, Richard Purdie wrote:
> > So be it, then.  But my request that we get a way to add the information we
> > are losing to the backlight class still stands.
> 
> The thing is does this 0-100 value have a use to userspace? Its

You can request something like "middle brightness please" (although whether
you get middle power comsumption or middle brightness is undetermined in the
ACPI spec).

> extremely device specific and can't be exported from the class in a
> generic way suggesting it should really be something exported by the
> driver itself. You still have to ask whether userspace actually needs
> this value though?

To just cycle brightness up and down, it certainly doesn't need 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