Re: [PATCH 10/11] thinkpad_acpi: fingers off backlight if video.ko is serving this functionality

Henrique de Moraes Holschuh
2008-07-16T12:40:48Z

On Wed, 16 Jul 2008, Thomas Renninger wrote:
> @@ -2337,6 +2341,28 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
> case 1:
> /* 0x1000-0x1FFF: key presses */
> scancode = hkey & 0xfff;
> + if (tp_features.bright_igdmode) {
> + /* ToDo:
> + * Is there an already defined key?
> + */
> + if (hkey == 0x1011) {
> + if (ibm_backlight_device->
> + props.brightness > 0) {
> + ibm_backlight_device->
> + props.brightness--;
> + }
> + } else if (hkey == 0x1010) {
> + if (ibm_backlight_device->
> + props.brightness <
> + ibm_backlight_device->
> + props.max_brightness) {
> + ibm_backlight_device->
> + props.brightness++;
> + }
> + }
> + brightness_update_status(ibm_backlight_device);
> + }
> +

NAK. Sorry I didn't look it over in detail the last time you posted
this.

As per my golden rule for thinkpad-acpi (always have some way to let the
user disable such stuff, because it WILL break in some thinkpad), these
need to be configurable as a module parameter.

Extending brightness_mode (it is a bitmap) to let one force igd_mode
on and off would solve this detail.

Also, usually I would veto such automatic hotkey handling in the driver,
but since the firmware ALREADY does it on every IBM thinkpad anyway (and
I can't fix it since hotpatching DSDT in code is absolutely forbidden),
the driver has all facilities needed to deal with it.

One question: is there a good reason to actually have the hotkey
handling for IGD mode directly in thinkpad-acpi? We don't do it for
Vista BIOS, video.ko doesn't do it either (anymore)... and I would make
it configurable for IBM firmware if I could.

Anyway, that means hotkey-driven igd_mode brightness change, if handled
by thinkpad-acpi without going through userspace, HAS to mask off the
relevant hotkeys in hotkey_init and set to KEY_RESERVED the relevant
keymap entries. The code is there already, but it does mean you
need to factor out brightness_mode autodetection into a function
of its own, and have it run BEFORE hotkey_init and brightness_init
have a chance to run, because both will have to be in sync.

If you'd rather not have all that work, just add minimal support for igd
and the video stuff to thinkpad-acpi so that it doesn't get in the way
of your work, and I will follow up with whatever else enhancements are
needed.

--
"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