2006-09-30 17:05:50

by Alessandro Guido

[permalink] [raw]
Subject: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

Make the sony_acpi use the backlight subsystem to adjust brightness value
instead of using the /proc/sony/brightness file.
(Other settings will still have a /proc/sony/... entry)

Signed-off-by: Alessandro Guido <[email protected]>
---
drivers/acpi/Kconfig | 1
drivers/acpi/sony_acpi.c | 59 +++++++++++++++++++++++++++++++++++++----------
2 files changed, 48 insertions(+), 12 deletions(-)

Index: linux-2.6.18/drivers/acpi/Kconfig
===================================================================
--- linux-2.6.18.orig/drivers/acpi/Kconfig
+++ linux-2.6.18/drivers/acpi/Kconfig
@@ -258,6 +258,7 @@ config ACPI_TOSHIBA
config ACPI_SONY
tristate "Sony Laptop Extras"
depends on X86 && ACPI
+ select BACKLIGHT_CLASS_DEVICE
default m
---help---
This mini-driver drives the ACPI SNC device present in the
Index: linux-2.6.18/drivers/acpi/sony_acpi.c
===================================================================
--- linux-2.6.18.orig/drivers/acpi/sony_acpi.c
+++ linux-2.6.18/drivers/acpi/sony_acpi.c
@@ -27,13 +27,19 @@
#include <linux/moduleparam.h>
#include <linux/init.h>
#include <linux/types.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
#include <acpi/acpi_drivers.h>
#include <acpi/acpi_bus.h>
#include <asm/uaccess.h>

#define ACPI_SNC_CLASS "sony"
#define ACPI_SNC_HID "SNY5001"
-#define ACPI_SNC_DRIVER_NAME "ACPI Sony Notebook Control Driver v0.2"
+#define ACPI_SNC_DRIVER_NAME "ACPI Sony Notebook Control Driver v0.3"
+
+/* the device uses 1-based values, while the backlight subsystem uses
+ 0-based values */
+#define SONY_MAX_BRIGHTNESS 8

#define LOG_PFX KERN_WARNING "sony_acpi: "

@@ -49,6 +55,16 @@ MODULE_PARM_DESC(debug, "set this to 1 (
static acpi_handle sony_acpi_handle;
static struct proc_dir_entry *sony_acpi_dir;

+static int sony_backlight_update_status(struct backlight_device *bd);
+static int sony_backlight_get_brightness(struct backlight_device *bd);
+static struct backlight_device *sony_backlight_device;
+static struct backlight_properties sony_backlight_properties = {
+ .owner = THIS_MODULE,
+ .update_status = sony_backlight_update_status,
+ .get_brightness = sony_backlight_get_brightness,
+ .max_brightness = SONY_MAX_BRIGHTNESS - 1,
+};
+
static struct sony_acpi_value {
char *name; /* name of the entry */
struct proc_dir_entry *proc; /* /proc entry */
@@ -61,19 +77,11 @@ static struct sony_acpi_value {
int debug; /* active only in debug mode ? */
} sony_acpi_values[] = {
{
- .name = "brightness",
- .acpiget = "GBRT",
- .acpiset = "SBRT",
- .min = 1,
- .max = 8,
- .debug = 0,
- },
- {
.name = "brightness_default",
.acpiget = "GPBR",
.acpiset = "SPBR",
.min = 1,
- .max = 8,
+ .max = SONY_MAX_BRIGHTNESS,
.debug = 0,
},
{
@@ -276,6 +284,7 @@ static int sony_acpi_add(struct acpi_dev
{
acpi_status status;
int result;
+ acpi_handle handle;
struct sony_acpi_value *item;

sony_acpi_handle = device->handle;
@@ -303,9 +312,15 @@ static int sony_acpi_add(struct acpi_dev
}
}

- for (item = sony_acpi_values; item->name; ++item) {
- acpi_handle handle;
+ if (ACPI_SUCCESS(acpi_get_handle(sony_acpi_handle, "GBRT", &handle))) {
+ sony_backlight_device = backlight_device_register("sony", NULL,
+ &sony_backlight_properties);
+ if (IS_ERR(sony_backlight_device)) {
+ printk(LOG_PFX "unable to register backlight device\n");
+ }
+ }

+ for (item = sony_acpi_values; item->name; ++item) {
if (!debug && item->debug)
continue;

@@ -358,6 +373,9 @@ static int sony_acpi_remove(struct acpi_
acpi_status status;
struct sony_acpi_value *item;

+ if (sony_backlight_device)
+ backlight_device_unregister(sony_backlight_device);
+
if (debug) {
status = acpi_remove_notify_handler(sony_acpi_handle,
ACPI_DEVICE_NOTIFY,
@@ -375,6 +393,23 @@ static int sony_acpi_remove(struct acpi_
return 0;
}

+static int sony_backlight_update_status(struct backlight_device *bd)
+{
+ return acpi_callsetfunc(sony_acpi_handle, "SBRT",
+ bd->props->brightness + 1,
+ NULL);
+}
+
+static int sony_backlight_get_brightness(struct backlight_device *bd)
+{
+ int value;
+
+ if (acpi_callgetfunc(sony_acpi_handle, "GBRT", &value))
+ return 0;
+ /* brightness levels are 1-based, while backlight ones are 0-based */
+ return value - 1;
+}
+
static struct acpi_driver sony_acpi_driver = {
.name = ACPI_SNC_DRIVER_NAME,
.class = ACPI_SNC_CLASS,


2006-09-30 17:12:36

by Alessandro Guido

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Sat, 30 Sep 2006 19:08:10 +0200
Alessandro Guido <[email protected]> wrote:

> Make the sony_acpi use the backlight subsystem to adjust brightness value
> instead of using the /proc/sony/brightness file.
> (Other settings will still have a /proc/sony/... entry)

I meant /proc/acpi/sony/brightness and /proc/acpi/sony/...

sorry

2006-09-30 17:35:25

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver


>> Make the sony_acpi use the backlight subsystem to adjust brightness value
>> instead of using the /proc/sony/brightness file.
>> (Other settings will still have a /proc/sony/... entry)
>
>I meant /proc/acpi/sony/brightness and /proc/acpi/sony/...

Hm spicctrl needs to be updated then. (Or maybe not if it does not use
/proc)


Jan Engelhardt
--

2006-10-02 00:19:28

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Sat, 30 Sep 2006 19:08:10 +0200
Alessandro Guido <[email protected]> wrote:

> Make the sony_acpi use the backlight subsystem to adjust brightness value
> instead of using the /proc/sony/brightness file.
> (Other settings will still have a /proc/sony/... entry)

umm, OK, but now how do I adjust my screen brightness? ;)

I assume that cute userspace applications for controlling backlight
brightness via the generic backlight driver either exist or are in
progress? What is the status of that?

Thanks.

2006-10-02 00:39:07

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Sun, Oct 01, 2006 at 05:19:12PM -0700, Andrew Morton wrote:
> On Sat, 30 Sep 2006 19:08:10 +0200
> Alessandro Guido <[email protected]> wrote:
>
> > Make the sony_acpi use the backlight subsystem to adjust brightness value
> > instead of using the /proc/sony/brightness file.
> > (Other settings will still have a /proc/sony/... entry)
>
> umm, OK, but now how do I adjust my screen brightness? ;)
>
> I assume that cute userspace applications for controlling backlight
> brightness via the generic backlight driver either exist or are in
> progress? What is the status of that?

For Dell laptops, the dellLcdBrightness app is included in the
libsmbios-bin package (http://linux.dell.com/libsmbios/main/ and
http://linux.dell.com/libsmbios/main/yum.html for the yum repo). It's
entirely userspace.

--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2006-10-02 00:48:22

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Sun, Oct 01, 2006 at 07:39:08PM -0500, Matt Domsch wrote:
> On Sun, Oct 01, 2006 at 05:19:12PM -0700, Andrew Morton wrote:
> > umm, OK, but now how do I adjust my screen brightness? ;)
> >
> > I assume that cute userspace applications for controlling backlight
> > brightness via the generic backlight driver either exist or are in
> > progress? What is the status of that?
>
> For Dell laptops, the dellLcdBrightness app is included in the
> libsmbios-bin package (http://linux.dell.com/libsmbios/main/ and
> http://linux.dell.com/libsmbios/main/yum.html for the yum repo). It's
> entirely userspace.

Which is to say, if a system-agnostic userspace app exists, please
advise, as we'd like to incorporate dellLcdBrightness functionality
into it.

Thanks,
Matt

--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2006-10-02 10:30:40

by Holger Macht

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Sun 01. Oct - 17:19:12, Andrew Morton wrote:
> On Sat, 30 Sep 2006 19:08:10 +0200
> Alessandro Guido <[email protected]> wrote:
>
> > Make the sony_acpi use the backlight subsystem to adjust brightness value
> > instead of using the /proc/sony/brightness file.
> > (Other settings will still have a /proc/sony/... entry)
>
> umm, OK, but now how do I adjust my screen brightness? ;)
>
> I assume that cute userspace applications for controlling backlight
> brightness via the generic backlight driver either exist or are in
> progress? What is the status of that?

Most applications use HAL as a backend for display brightness these
days. HAL still supports the old interface in /proc for the different
brightness drivers, though, but the conversion to the new interface is on
my TODO and I will do it this week. So userspace should have fixed this
soon, so go ahead ;-)

Btw, there's a patchset from Matthew Garrett [1] sent some time ago which
also converts the asus_acpi, ibm_acpi and the toshiba_acpi drivers.

Regards,
Holger

[1] http://lkml.org/lkml/2006/4/18/28

2006-10-02 11:23:28

by Alessandro Guido

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Sun, 1 Oct 2006 17:19:12 -0700
Andrew Morton <[email protected]> wrote:
>
> umm, OK, but now how do I adjust my screen brightness? ;)
>
> I assume that cute userspace applications for controlling backlight
> brightness via the generic backlight driver either exist or are in
> progress? What is the status of that?
>

I use this tool: http://www.xs4all.nl/~bsamwel/laptop_mode/tools/
that automagically fires up whenever needed

$ cat /etc/laptop-mode/batt-start/brightness
#!/bin/sh
echo -n CHOOSE_A_LOW_VALUE > /sys/class/backlight/sony/brightness

and

$ cat /etc/laptop-mode/batt-stop/brightness
#!/bin/sh
let val="`</proc/acpi/sony/brightness_default` - 1"
echo -n "$val" > /sys/class/backlight/sony/brightness

But I seldom use my laptop in battery mode.

> I assume that cute userspace applications for controlling backlight
> brightness via the generic backlight driver either exist or are in
> progress? What is the status of that?
>

I think the new gnome-power-manager does it, but I'm not sure since I use Xfce.

> Thanks.

Thank you.

2006-10-06 14:51:40

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

Hi!

> > I assume that cute userspace applications for controlling backlight
> > brightness via the generic backlight driver either exist or are in
> > progress? What is the status of that?
>
> For Dell laptops, the dellLcdBrightness app is included in the
> libsmbios-bin package (http://linux.dell.com/libsmbios/main/ and
> http://linux.dell.com/libsmbios/main/yum.html for the yum repo). It's
> entirely userspace.

Please move it into the kernel where it belongs, and use lcd
brightness subsystem like everyone else.
Pavel
--
Thanks for all the (sleeping) penguins.

2006-10-06 21:18:30

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Thu, Oct 05, 2006 at 10:36:57AM +0000, Pavel Machek wrote:
> Hi!
>
> > > I assume that cute userspace applications for controlling backlight
> > > brightness via the generic backlight driver either exist or are in
> > > progress? What is the status of that?
> >
> > For Dell laptops, the dellLcdBrightness app is included in the
> > libsmbios-bin package (http://linux.dell.com/libsmbios/main/ and
> > http://linux.dell.com/libsmbios/main/yum.html for the yum repo). It's
> > entirely userspace.
>
> Please move it into the kernel where it belongs, and use lcd
> brightness subsystem like everyone else.

We've been through this before.
http://marc.theaimsgroup.com/?l=linux-kernel&m=114067198323596&w=2

In addition, the SMI call used to change the backlight level *may*
require (if configured by the sysadmin in BIOS), a password be
entered.

This begs for a common userspace app that can grok libsmbios and
kernel interfaces both, and use the appropriate method on each, rather
than just putting it all in the kernel.

Thanks,
Matt

--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2006-10-06 22:45:20

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

Hi!

> > Please move it into the kernel where it belongs, and use lcd
> > brightness subsystem like everyone else.
>
> We've been through this before.
> http://marc.theaimsgroup.com/?l=linux-kernel&m=114067198323596&w=2
>
> In addition, the SMI call used to change the backlight level *may*
> require (if configured by the sysadmin in BIOS), a password be
> entered.

This is crazy, password-protected backlight level?

Can you make sure this crazyness does not infect newer models?

> This begs for a common userspace app that can grok libsmbios and
> kernel interfaces both, and use the appropriate method on each, rather
> than just putting it all in the kernel.

Kernel is expected to provide hardware abstraction. libsmbios will
mean person able to change backlight will be able to do lots of
strange stuff...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-10-10 14:32:52

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

> > Please move it into the kernel where it belongs, and use lcd
> > brightness subsystem like everyone else.
>
> We've been through this before.
> http://marc.theaimsgroup.com/?l=linux-kernel&m=114067198323596&w=2
>
> In addition, the SMI call used to change the backlight level *may*
> require (if configured by the sysadmin in BIOS), a password be
> entered.
>
> This begs for a common userspace app that can grok libsmbios and
> kernel interfaces both, and use the appropriate method on each, rather
> than just putting it all in the kernel

>From my understanding, a cute userspace App shouldn't have this kind
of logic:
if (is DELL )
invoke libsmbios
if (is foo)
invoke libfoo,
if (is bar)
invoke libbar,
....
else
operate on /sys/class/backlight/ ,.,..

It should be:
just write/read file in /sys/class/backlight ,....

Right?

Thanks,
Luming



2006-10-10 14:55:25

by Richard Hughes

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tue, 2006-10-10 at 22:32 +0800, Yu Luming wrote:
> >From my understanding, a cute userspace App shouldn't have this kind
> of logic:
> if (is DELL )
> invoke libsmbios
> if (is foo)
> invoke libfoo,
> if (is bar)
> invoke libbar,
> ....
> else
> operate on /sys/class/backlight/ ,.,..

This is what HAL has at the moment[1]. And it's hell to maintain, but
works for a lot of users.

> It should be:
> just write/read file in /sys/class/backlight ,....

That would make things much easier IMO.

Richard.

[1]
http://gitweb.freedesktop.org/?p=hal.git;a=blob;h=3ff9284be440a7197b0de9b5f0234761c3397cb1;hb=dbffafacbf7b9143d82547b9eabe61d1a5b8fffc;f=tools/linux/hal-system-lcd-get-brightness-linux

2006-10-10 15:17:31

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

> I use this tool: http://www.xs4all.nl/~bsamwel/laptop_mode/tools/
> that automagically fires up whenever needed
>
> $ cat /etc/laptop-mode/batt-start/brightness
> #!/bin/sh
> echo -n CHOOSE_A_LOW_VALUE > /sys/class/backlight/sony/brightness

Also, we need to make hot-key events have similar handling code .
For example, Fn+F5 and Fn+F6 are brightness down and up key on my sony laptop.
There is a driver called sonypi.c can map Fn+F5/F6 to KEY_FN_F5/F6. But I
think It should be mapped to KEY_BRIGHTNESSDOWN/UP (linux/input.h)
Although, sonypi.c is NOT so clean, but , if it can report right event to
input layer for all sony laptop(it works for me), and all related functions
can be controlled through generic sysfs interface, then I would say sony has
the best hot-key solution I have even seen so far for linux.

Thanks,
Luming.



2006-10-10 15:22:14

by Richard Hughes

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tue, 2006-10-10 at 23:17 +0800, Yu Luming wrote:
> It should be mapped to KEY_BRIGHTNESSDOWN/UP (linux/input.h)

Completely agree. Then userspace (like gnome-power-manager or
powersaved) can change the brightness based on policy and user-lockdown.

Richard.


2006-10-10 16:10:15

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tue, Oct 10, 2006 at 03:47:26PM +0100, Richard Hughes wrote:
> On Tue, 2006-10-10 at 22:32 +0800, Yu Luming wrote:
> > >From my understanding, a cute userspace App shouldn't have this kind
> > of logic:
> > if (is DELL )
> > invoke libsmbios
> > if (is foo)
> > invoke libfoo,
> > if (is bar)
> > invoke libbar,
> > ....
> > else
> > operate on /sys/class/backlight/ ,.,..
>
> This is what HAL has at the moment[1]. And it's hell to maintain, but
> works for a lot of users.

This is slightly different. This shows that there are a number of
slightly different kernel implementations:

/proc/acpi/toshiba/lcd
/proc/acpi/asus/brn
/proc/acpi/pcc/brightness
/proc/acpi/ibm/brightness
/proc/acpi/sony/brightness
/proc/omnibook/lcd

which is indeed nasty. I'd agree all in-kernel solutions should use
the same kernel<->user interface. I'd also expect the kernel to have
a generic ACPI driver that exports the _BCL and _BCM method
implementations via that same interface, so that systems providing
that will "just work". drivers/acpi/video.c currently exports this
via /proc/acpi/video/$DEVICE/brightness, which isn't the same as
/sys/class/backlight. :-(

There's also at least one more userspace option for the sonypi using
spiictrl. This is where I expected libsmbios to plug in also, as a
fallback to the ACPI _BCL/_BCM methods above.

Thanks,
Matt

--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2006-10-10 21:23:55

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tue, Oct 10, 2006 at 11:17:23PM +0800, Yu Luming wrote:

> Also, we need to make hot-key events have similar handling code .
> For example, Fn+F5 and Fn+F6 are brightness down and up key on my sony laptop.
> There is a driver called sonypi.c can map Fn+F5/F6 to KEY_FN_F5/F6. But I
> think It should be mapped to KEY_BRIGHTNESSDOWN/UP (linux/input.h)
> Although, sonypi.c is NOT so clean, but , if it can report right event to
> input layer for all sony laptop(it works for me), and all related functions
> can be controlled through generic sysfs interface, then I would say sony has
> the best hot-key solution I have even seen so far for linux.

It would have to be DMI-based to some extent - not all Sonys use the
same keys for the same purpose. Misery ensues.

--
Matthew Garrett | [email protected]

2006-10-10 21:26:30

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tue, Oct 10, 2006 at 10:32:46PM +0800, Yu Luming wrote:

> >From my understanding, a cute userspace App shouldn't have this kind
> of logic:

(snip switching on hardware type)

> It should be:
> just write/read file in /sys/class/backlight ,....

Yup, but to do that on Dell hardware is basically impossible. It'd be
nice if they implemented the ACPI video extension properly for future
hardware.

--
Matthew Garrett | [email protected]

2006-10-11 03:02:32

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tue, Oct 10, 2006 at 10:26:15PM +0100, Matthew Garrett wrote:
> On Tue, Oct 10, 2006 at 10:32:46PM +0800, Yu Luming wrote:
>
> > >From my understanding, a cute userspace App shouldn't have this kind
> > of logic:
>
> (snip switching on hardware type)
>
> > It should be:
> > just write/read file in /sys/class/backlight ,....
>
> Yup, but to do that on Dell hardware is basically impossible. It'd be
> nice if they implemented the ACPI video extension properly for future
> hardware.

Yes, our BIOS teams are looking to do exactly that. I wouldn't expect
such a change to be propogated back to earlier released systems though.

--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2006-10-11 03:16:49

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tue, Oct 10, 2006 at 10:02:32PM -0500, Matt Domsch wrote:

> Yes, our BIOS teams are looking to do exactly that. I wouldn't expect
> such a change to be propogated back to earlier released systems though.

That's great news. I accept that it's impractical to port back to
earlier hardware, but it means it'll stop being a problem eventually.

(Entirely unrelatedly, have you got any idea why several D-series
Latitudes with Intel graphics switch off the backlight on lid close and
never switch it back on again? It happens under Windows if you boot in
safe mode, so it's not Linux specific...)
--
Matthew Garrett | [email protected]

2006-10-11 03:20:30

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tuesday 10 October 2006 17:23, Matthew Garrett wrote:
> On Tue, Oct 10, 2006 at 11:17:23PM +0800, Yu Luming wrote:
>
> > Also, we need to make hot-key events have similar handling code .
> > For example, Fn+F5 and Fn+F6 are brightness down and up key on my sony laptop.
> > There is a driver called sonypi.c can map Fn+F5/F6 to KEY_FN_F5/F6. But I
> > think It should be mapped to KEY_BRIGHTNESSDOWN/UP (linux/input.h)
> > Although, sonypi.c is NOT so clean, but , if it can report right event to
> > input layer for all sony laptop(it works for me), and all related functions
> > can be controlled through generic sysfs interface, then I would say sony has
> > the best hot-key solution I have even seen so far for linux.
>
> It would have to be DMI-based to some extent - not all Sonys use the
> same keys for the same purpose. Misery ensues.
>

Then we need to add keymap table to the sonypi's input device so that
keymap can be changed from userspace.

--
Dmitry

2006-10-11 06:59:29

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Tue, 2006-10-10 at 22:26 +0100, Matthew Garrett wrote:
> On Tue, Oct 10, 2006 at 10:32:46PM +0800, Yu Luming wrote:
>
> > >From my understanding, a cute userspace App shouldn't have this kind
> > of logic:
>
> (snip switching on hardware type)
>
> > It should be:
> > just write/read file in /sys/class/backlight ,....
>
> Yup, but to do that on Dell hardware is basically impossible. It'd be
> nice if they implemented the ACPI video extension properly for future
> hardware.

it'd also be nice if the linux-ready firmware developer kit had a test
for this, so that we can offer 1) a way to test this to the bios guys
and 2) encourage adding/note the lack easily


>
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com

2006-10-11 07:04:22

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Wed, Oct 11, 2006 at 08:59:04AM +0200, Arjan van de Ven wrote:

> it'd also be nice if the linux-ready firmware developer kit had a test
> for this, so that we can offer 1) a way to test this to the bios guys
> and 2) encourage adding/note the lack easily

Sure. Reading /proc/acpi/video/*/*/info should tell you whether a device
is an LCD or not. The brightness file should then contain a list of
available brightnesses, and writing one into there should change the
screen value. There's a patch somewhere that ports this to the
/sys/class/backlight infrastructure, but I don't think it's applied yet.

I'd write a test up for you, but I don't actually seem to have any
hardware that implements this properly. Tch.
--
Matthew Garrett | [email protected]

2006-10-11 08:04:43

by Ismail Dönmez

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

11 Eki 2006 Çar 10:04 tarihinde, Matthew Garrett şunları yazmıştı:
> On Wed, Oct 11, 2006 at 08:59:04AM +0200, Arjan van de Ven wrote:
> > it'd also be nice if the linux-ready firmware developer kit had a test
> > for this, so that we can offer 1) a way to test this to the bios guys
> > and 2) encourage adding/note the lack easily
>
> Sure. Reading /proc/acpi/video/*/*/info should tell you whether a device
> is an LCD or not.

On my Sony Vaio with latest linux-2.6 git kernel it says its a CRT :

[~]> cat /proc/acpi/video/*/*/info
device_id: 0x0100
type: CRT
known by bios: no
device_id: 0x0320
type: UNKNOWN
known by bios: no
device_id: 0x0410
type: UNKNOWN
known by bios: no
device_id: 0x0240
type: UNKNOWN
known by bios: no
device_id: 0x0100
type: CRT
known by bios: no
device_id: 0x0111
type: UNKNOWN
known by bios: no
device_id: 0x0118
type: UNKNOWN
known by bios: no
device_id: 0x0200
type: TVOUT
known by bios: no

So I don't think its reliable.

Regards,
ismail

2006-10-11 08:12:59

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Wed, Oct 11, 2006 at 11:04:48AM +0300, Ismail Donmez wrote:

> device_id: 0x0410
> type: UNKNOWN
> known by bios: no

This one's an LCD, according to the spec. The id spec was changed
slightly in 3.0 of the acpi spec (in a backwards compatible way) - I'd
guess that the acpi video driver just hasn't been updated.

--
Matthew Garrett | [email protected]

2006-10-11 16:29:01

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Wednesday 11 October 2006 00:10, Matt Domsch wrote:
> On Tue, Oct 10, 2006 at 03:47:26PM +0100, Richard Hughes wrote:
> > On Tue, 2006-10-10 at 22:32 +0800, Yu Luming wrote:
> > > >From my understanding, a cute userspace App shouldn't have this kind
> > >
> > > of logic:
> > > if (is DELL )
> > > invoke libsmbios
> > > if (is foo)
> > > invoke libfoo,
> > > if (is bar)
> > > invoke libbar,
> > > ....
> > > else
> > > operate on /sys/class/backlight/ ,.,..
> >
> > This is what HAL has at the moment[1]. And it's hell to maintain, but
> > works for a lot of users.
>
> This is slightly different. This shows that there are a number of
> slightly different kernel implementations:
>
> /proc/acpi/toshiba/lcd
> /proc/acpi/asus/brn
> /proc/acpi/pcc/brightness
> /proc/acpi/ibm/brightness
> /proc/acpi/sony/brightness
> /proc/omnibook/lcd
>
> which is indeed nasty. I'd agree all in-kernel solutions should use
> the same kernel<->user interface. I'd also expect the kernel to have

Yes, we all seem to agree we need to throw away /proc/acpi just after we
have solid sysfs interface for acpi.

> a generic ACPI driver that exports the _BCL and _BCM method
> implementations via that same interface, so that systems providing
> that will "just work". drivers/acpi/video.c currently exports this
> via /proc/acpi/video/$DEVICE/brightness, which isn't the same as
> /sys/class/backlight. :-(

Yes, I'm working on acpi video driver transition , and have posted a patch to
user backlight for acpi video driver.
http://marc.theaimsgroup.com/?l=linux-acpi&m=115574087203605&w=2

>
> There's also at least one more userspace option for the sonypi using
> spiictrl. This is where I expected libsmbios to plug in also, as a
> fallback to the ACPI _BCL/_BCM methods above.

I think spiictrl would be thrown away just after we have solid sysfs support.

Thanks,
Luming

2006-10-11 16:31:26

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Wednesday 11 October 2006 16:04, Ismail Donmez wrote:
> 11 Eki 2006 Çar 10:04 tarihinde, Matthew Garrett şunları yazmıştı:
> > On Wed, Oct 11, 2006 at 08:59:04AM +0200, Arjan van de Ven wrote:
> > > it'd also be nice if the linux-ready firmware developer kit had a test
> > > for this, so that we can offer 1) a way to test this to the bios guys
> > > and 2) encourage adding/note the lack easily
> >
> > Sure. Reading /proc/acpi/video/*/*/info should tell you whether a device
> > is an LCD or not.
>
> On my Sony Vaio with latest linux-2.6 git kernel it says its a CRT :
>
> [~]> cat /proc/acpi/video/*/*/info
> device_id: 0x0100
> type: CRT
> known by bios: no
> device_id: 0x0320
> type: UNKNOWN
> known by bios: no
> device_id: 0x0410
> type: UNKNOWN
> known by bios: no
> device_id: 0x0240
> type: UNKNOWN
> known by bios: no
> device_id: 0x0100
> type: CRT
> known by bios: no
> device_id: 0x0111
> type: UNKNOWN
> known by bios: no
> device_id: 0x0118
> type: UNKNOWN
> known by bios: no
> device_id: 0x0200
> type: TVOUT
> known by bios: no
>
> So I don't think its reliable.
Please open a bug on bugzilla.kernel.org,
and post acpidump output.

Thanks,
Luming

2006-10-11 16:37:26

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

> Yes, our BIOS teams are looking to do exactly that. I wouldn't expect
> such a change to be propogated back to earlier released systems though.
Wow! This is a really good news! Please just notify me when it ready.
Because, I'd like to have one. :-)

Thanks,
Luming

2006-10-11 16:45:23

by Ismail Dönmez

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

11 Eki 2006 Çar 19:31 tarihinde, Yu Luming şunları yazmıştı:
[...]
> Please open a bug on bugzilla.kernel.org,
> and post acpidump output.

Done, http://bugzilla.kernel.org/show_bug.cgi?id=7349

Regards,
ismail

2006-10-11 16:49:57

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

> > It would have to be DMI-based to some extent - not all Sonys use the
> > same keys for the same purpose. Misery ensues.
>
> Then we need to add keymap table to the sonypi's input device so that
> keymap can be changed from userspace.
If some key is physically broken, I agree configurable keymap is the only
solution. But, I don't see any other benefit of doing so, if we expect
platform specific driver report meaningful key code to input layer.

Thanks,
Luming

2006-10-11 19:08:47

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On 10/11/06, Yu Luming <[email protected]> wrote:
> > > It would have to be DMI-based to some extent - not all Sonys use the
> > > same keys for the same purpose. Misery ensues.
> >
> > Then we need to add keymap table to the sonypi's input device so that
> > keymap can be changed from userspace.
> If some key is physically broken, I agree configurable keymap is the only
> solution. But, I don't see any other benefit of doing so, if we expect
> platform specific driver report meaningful key code to input layer.
>

As Matthew said different Sony models use different mapping. DMI-based
keymap solution requires kernel upgrade every time new model is out
whereas configurable keymap can be loaded easily form userspace. Also
user might want to remap keys to do different stuff. For example I
never change brigtness on my laptop now that I found settings that I
like. So I could map one key to start kmail and another one to build
kernel for example ;)

--
Dmitry

2006-10-16 17:47:28

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

> > a generic ACPI driver that exports the _BCL and _BCM method
> > implementations via that same interface, so that systems providing
> > that will "just work". drivers/acpi/video.c currently exports this
> > via /proc/acpi/video/$DEVICE/brightness, which isn't the same as
> > /sys/class/backlight. :-(
>
> Yes, I'm working on acpi video driver transition , and have posted a patch
> to user backlight for acpi video driver.
> http://marc.theaimsgroup.com/?l=linux-acpi&m=115574087203605&w=2

Just updated the backlight and output sysfs support for ACPI Video driver on
bugzilla. If you are interested this, please take a look at
http://bugzilla.kernel.org/show_bug.cgi?id=5749#c18

signed-off-by [email protected]

[patch 1/3] vidoe sysfs support: Add dev argument for baclight sys dev
[patch 2/3] Add display output class support
[patch 3/3] backlight and output sysfs support for acpi video driver

Thanks,
Luming

2006-10-25 07:07:25

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

Hi!

> > > a generic ACPI driver that exports the _BCL and _BCM method
> > > implementations via that same interface, so that systems providing
> > > that will "just work". drivers/acpi/video.c currently exports this
> > > via /proc/acpi/video/$DEVICE/brightness, which isn't the same as
> > > /sys/class/backlight. :-(
> >
> > Yes, I'm working on acpi video driver transition , and have posted a patch
> > to user backlight for acpi video driver.
> > http://marc.theaimsgroup.com/?l=linux-acpi&m=115574087203605&w=2
>
> Just updated the backlight and output sysfs support for ACPI Video driver on
> bugzilla. If you are interested this, please take a look at
> http://bugzilla.kernel.org/show_bug.cgi?id=5749#c18

> [patch 1/3] vidoe sysfs support: Add dev argument for baclight sys dev

Two typos in one line :-).

> [patch 2/3] Add display output class support

I guess this needs Documentation/ so we can tell if user<->kernel
interface is sane..

> [patch 3/3] backlight and output sysfs support for acpi video driver

Some whitespace is not okay there...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-10-25 17:26:35

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

On Wednesday 25 October 2006 15:07, Pavel Machek wrote:
> Hi!
>
> > > > a generic ACPI driver that exports the _BCL and _BCM method
> > > > implementations via that same interface, so that systems providing
> > > > that will "just work". drivers/acpi/video.c currently exports this
> > > > via /proc/acpi/video/$DEVICE/brightness, which isn't the same as
> > > > /sys/class/backlight. :-(
> > >
> > > Yes, I'm working on acpi video driver transition , and have posted a
> > > patch to user backlight for acpi video driver.
> > > http://marc.theaimsgroup.com/?l=linux-acpi&m=115574087203605&w=2
> >
> > Just updated the backlight and output sysfs support for ACPI Video driver
> > on bugzilla. If you are interested this, please take a look at
> > http://bugzilla.kernel.org/show_bug.cgi?id=5749#c18
> >
> > [patch 1/3] vidoe sysfs support: Add dev argument for baclight sys dev
>
> Two typos in one line :-).
>
> > [patch 2/3] Add display output class support
>
> I guess this needs Documentation/ so we can tell if user<->kernel
> interface is sane..
>
> > [patch 3/3] backlight and output sysfs support for acpi video driver
>
> Some whitespace is not okay there...
> Pavel

updated version.
[patch 1/4] video sysfs support: Add dev argument for backlight sys dev
drivers/video/backlight/backlight.c | 3 ++-
include/linux/backlight.h | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)

[patch 2/4] Add display output class support
drivers/video/output.c | 110
+++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/output.h | 23 ++++++++++
2 files changed, 133 insertions(+)

[patch 3/4] backlight and output sysfs support for acpi video driver
acpi/Kconfig | 2
acpi/video.c | 257
+++++++++++++++++++++++++++++++++++++++++++++++++++++----
video/Kconfig | 8 +
video/Makefile | 1
4 files changed, 252 insertions(+), 16 deletions(-)

[patch 4/4] Add output class document
video-output.txt | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)

Thanks,
Luming


Attachments:
(No filename) (2.00 kB)
1-fix.patch (1.51 kB)
2-fix.patch (3.62 kB)
4-fix.patch (1.19 kB)
3-fix.patch (14.65 kB)
Download all attachments

2006-10-29 17:51:19

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

Hi!

> > Some whitespace is not okay there...
> > Pavel
>
> updated version.

> index 27597c5..1a18cdb 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -190,7 +190,7 @@ static int fb_notifier_callback(struct n
> * Creates and registers new backlight class_device. Returns either an
> * ERR_PTR() or a pointer to the newly allocated device.
> */
> -struct backlight_device *backlight_device_register(const char *name, void *devdata,
> +struct backlight_device *backlight_device_register(const char *name,struct device *dev, void *devdata,
> struct

80-columns, and fix the whitespace, please.

> --- /dev/null
> +++ b/drivers/video/output.c
> @@ -0,0 +1,110 @@
> +/*
> + * Video output switch support
> + */

I guess this one needs copyright/GPL.

> + struct output_device *new_dev;
> + int ret_code = 0;

Indentation is wrong where...

> + new_dev = (struct output_device *) kzalloc( sizeof(struct
output_device), GFP_KERNEL);

Cast should not be needed.

> + strlcpy(new_dev->class_dev.class_id, name, KOBJ_NAME_LEN);
> + class_set_devdata(&new_dev->class_dev, devdata);
> + ret_code = class_device_register(&new_dev->class_dev);
> + if (ret_code){

") {", please.

> + kfree (new_dev);

..and no space between kfree and its arguments.


> @@ -0,0 +1,27 @@
> +The output sysfs class driver is to provide video output abstract layer that can be used to hook platform specific methods to enable/disable video output device through common sysfs interface.
> +

80-columns, please. And some title would be nice.

> @@ -141,11 +144,11 @@ struct acpi_video_device_cap {
> u8 _ADR:1; /*Return the unique ID */
> u8 _BCL:1; /*Query list of brightness control levels supported */
> u8 _BCM:1; /*Set the brightness level */
> + u8 _BQC:1; /*Get current brightness level */
> u8 _DDC:1; /*Return the EDID for this device */
> u8 _DCS:1; /*Return status of output device */
> u8 _DGS:1; /*Query graphics state */
> u8 _DSS:1; /*Device state set */

It is nicer to have space between /* and comment.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2006-10-30 15:49:25

by Luming Yu

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-mm2] acpi: add backlight support to the sony_acpi driver

updated version attached.

signed-off-by [email protected]

[patch 1/6] video sysfs support: Add dev argument for backlight sys dev
drivers/video/backlight/backlight.c | 7 +++++--
include/linux/backlight.h | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)

patch 2/6] Add display output class support
drivers/video/output.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/output.h | 42 +++++++++++++++
2 files changed, 171 insertions(+)

[patch 3/6] backlight and output sysfs support for acpi video driver
acpi/Kconfig | 2
acpi/video.c | 257 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
video/Kconfig | 9 +
video/Makefile | 1
4 files changed, 253 insertions(+), 16 deletions(-)

[patch 4/6] Add output class document
video-output.txt | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

[patch 5/6] fix comments style
video.c | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)

[patch 6/6] fix compile time warning
video.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)


On 10/30/06, Pavel Machek <[email protected]> wrote:
> Hi!
>
> > > Some whitespace is not okay there...
> > > Pavel
> >
> > updated version.
>
> > index 27597c5..1a18cdb 100644
> > --- a/drivers/video/backlight/backlight.c
> > +++ b/drivers/video/backlight/backlight.c
> > @@ -190,7 +190,7 @@ static int fb_notifier_callback(struct n
> > * Creates and registers new backlight class_device. Returns either an
> > * ERR_PTR() or a pointer to the newly allocated device.
> > */
> > -struct backlight_device *backlight_device_register(const char *name, void *devdata,
> > +struct backlight_device *backlight_device_register(const char *name,struct device *dev, void *devdata,
> > struct
>
> 80-columns, and fix the whitespace, please.
>
> > --- /dev/null
> > +++ b/drivers/video/output.c
> > @@ -0,0 +1,110 @@
> > +/*
> > + * Video output switch support
> > + */
>
> I guess this one needs copyright/GPL.
>
> > + struct output_device *new_dev;
> > + int ret_code = 0;
>
> Indentation is wrong where...
>
> > + new_dev = (struct output_device *) kzalloc( sizeof(struct
> output_device), GFP_KERNEL);
>
> Cast should not be needed.
>
> > + strlcpy(new_dev->class_dev.class_id, name, KOBJ_NAME_LEN);
> > + class_set_devdata(&new_dev->class_dev, devdata);
> > + ret_code = class_device_register(&new_dev->class_dev);
> > + if (ret_code){
>
> ") {", please.
>
> > + kfree (new_dev);
>
> ..and no space between kfree and its arguments.
>
>
> > @@ -0,0 +1,27 @@
> > +The output sysfs class driver is to provide video output abstract layer that can be used to hook platform specific methods to enable/disable video output device through common sysfs interface.
> > +
>
> 80-columns, please. And some title would be nice.
>
> > @@ -141,11 +144,11 @@ struct acpi_video_device_cap {
> > u8 _ADR:1; /*Return the unique ID */
> > u8 _BCL:1; /*Query list of brightness control levels supported */
> > u8 _BCM:1; /*Set the brightness level */
> > + u8 _BQC:1; /*Get current brightness level */
> > u8 _DDC:1; /*Return the EDID for this device */
> > u8 _DCS:1; /*Return status of output device */
> > u8 _DGS:1; /*Query graphics state */
> > u8 _DSS:1; /*Device state set */
>
> It is nicer to have space between /* and comment.
> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>


Attachments:
(No filename) (3.80 kB)
1-fix.patch (1.58 kB)
2-fix.patch (5.52 kB)
3-fix.patch (14.80 kB)
4-fix.patch (1.29 kB)
5-fix.patch (2.67 kB)
6-fix.patch (826.00 B)
Download all attachments