Re: [patch 28/74] thinkpad-acpi: fix incorrect use of TPACPI_BRGHT_MODE_ECNVRAM

From: Stefan Lippers-Hollmann
Date: Thu Aug 13 2009 - 18:06:52 EST


Hi

On Thursday 13 August 2009, Greg KH wrote:
> 2.6.30-stable review patch. If anyone has any objections, please let us know.
>
> ------------------
>
> From: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
>
> commit 59fe4fe34d7afdf63208124f313be9056feaa2f4 upstream.
>
> HBRV-based default selection of backlight control strategy didn't work
> well, at least the X41 defines it but doesn't use it and I don't think
> it will stop there.
>
> Switch to a white/blacklist. All models that have HBRV defined have
> been included in the list, and initially all ATI GPUs will get
> ECNVRAM, and the Intel GPUs will get UCMS_STEP.
>
> Symptoms of incorrect backlight mode selection are:
>
> 1. Non-working backlight control through sysfs;
>
> 2. Backlight gets reset to the lowest level at every shutdown, reboot
> and when thinkpad-acpi gets unloaded;
>
> This fixes a regression in 2.6.30, bugzilla #13826

As already reported in [1] and confirmed by hmh in [2], this patch breaks
compiling thinkpad_acpi in 2.6.31-rc5.

[...]
CC [M] drivers/platform/x86/hp-wmi.o
CC [M] drivers/platform/x86/sony-laptop.o
CC [M] drivers/platform/x86/thinkpad_acpi.o
drivers/platform/x86/thinkpad_acpi.c:5769: error: array type has incomplete element type
drivers/platform/x86/thinkpad_acpi.c:5771: error: implicit declaration of function 'TPACPI_Q_IBM'
drivers/platform/x86/thinkpad_acpi.c: In function 'brightness_init':
drivers/platform/x86/thinkpad_acpi.c:5799: error: implicit declaration of function 'tpacpi_check_quirks'
drivers/platform/x86/thinkpad_acpi.c:5800: warning: type defaults to 'int' in declaration of 'type name'
drivers/platform/x86/thinkpad_acpi.c:5800: warning: type defaults to 'int' in declaration of 'type name'
drivers/platform/x86/thinkpad_acpi.c:5800: error: size of array 'type name' is negative
make[6]: *** [drivers/platform/x86/thinkpad_acpi.o] Error 1
make[5]: *** [drivers/platform/x86] Error 2
make[4]: *** [drivers/platform] Error 2
make[3]: *** [drivers] Error 2

Regards
Stefan Lippers-Hollmann

[1] From: "Stefan Lippers-Hollmann" <s.L-H@xxxxxx>
Date: Fri, 7 Aug 2009 01:08:10 +0200
Subject: Re: patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree
Message-Id: <200908070108.15095.s.L-H@xxxxxx>
http://lkml.indiana.edu/hypermail/linux/kernel/0908.0/02520.html
[2] From: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
Date: Thu, 6 Aug 2009 20:31:11 -0300
Subject: Re: patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree
Message-ID: <20090806233111.GB14381@xxxxxxxxxxxxxxxxxxxxx>
http://lkml.indiana.edu/hypermail/linux/kernel/0908.0/02526.html

--
> Signed-off-by: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
> Reported-by: Tobias Diedrich <ranma+kernel@xxxxxxxxxxxx>
> Signed-off-by: Len Brown <len.brown@xxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 61 ++++++++++++++++++++++++++---------
> 1 file changed, 47 insertions(+), 14 deletions(-)
>
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -5757,14 +5757,48 @@ static struct backlight_ops ibm_backligh
>
> /* --------------------------------------------------------------------- */
>
> +/*
> + * These are only useful for models that have only one possibility
> + * of GPU. If the BIOS model handles both ATI and Intel, don't use
> + * these quirks.
> + */
> +#define TPACPI_BRGHT_Q_NOEC 0x0001 /* Must NOT use EC HBRV */
> +#define TPACPI_BRGHT_Q_EC 0x0002 /* Should or must use EC HBRV */
> +#define TPACPI_BRGHT_Q_ASK 0x8000 /* Ask for user report */
> +
> +static const struct tpacpi_quirk brightness_quirk_table[] __initconst = {
> + /* Models with ATI GPUs known to require ECNVRAM mode */
> + TPACPI_Q_IBM('1', 'Y', TPACPI_BRGHT_Q_EC), /* T43/p ATI */
> +
> + /* Models with ATI GPUs (waiting confirmation) */
> + TPACPI_Q_IBM('1', 'R', TPACPI_BRGHT_Q_ASK|TPACPI_BRGHT_Q_EC),
> + TPACPI_Q_IBM('1', 'Q', TPACPI_BRGHT_Q_ASK|TPACPI_BRGHT_Q_EC),
> + TPACPI_Q_IBM('7', '6', TPACPI_BRGHT_Q_ASK|TPACPI_BRGHT_Q_EC),
> + TPACPI_Q_IBM('7', '8', TPACPI_BRGHT_Q_ASK|TPACPI_BRGHT_Q_EC),
> +
> + /* Models with Intel Extreme Graphics 2 (waiting confirmation) */
> + TPACPI_Q_IBM('1', 'V', TPACPI_BRGHT_Q_ASK|TPACPI_BRGHT_Q_NOEC),
> + TPACPI_Q_IBM('1', 'W', TPACPI_BRGHT_Q_ASK|TPACPI_BRGHT_Q_NOEC),
> + TPACPI_Q_IBM('1', 'U', TPACPI_BRGHT_Q_ASK|TPACPI_BRGHT_Q_NOEC),
> +
> + /* Models with Intel GMA900 */
> + TPACPI_Q_IBM('7', '0', TPACPI_BRGHT_Q_NOEC), /* T43, R52 */
> + TPACPI_Q_IBM('7', '4', TPACPI_BRGHT_Q_NOEC), /* X41 */
> + TPACPI_Q_IBM('7', '5', TPACPI_BRGHT_Q_NOEC), /* X41 Tablet */
> +};
> +
> static int __init brightness_init(struct ibm_init_struct *iibm)
> {
> int b;
> + unsigned long quirks;
>
> vdbg_printk(TPACPI_DBG_INIT, "initializing brightness subdriver\n");
>
> mutex_init(&brightness_mutex);
>
> + quirks = tpacpi_check_quirks(brightness_quirk_table,
> + ARRAY_SIZE(brightness_quirk_table));
> +
> /*
> * We always attempt to detect acpi support, so as to switch
> * Lenovo Vista BIOS to ACPI brightness mode even if we are not
> @@ -5821,23 +5855,13 @@ static int __init brightness_init(struct
> /* TPACPI_BRGHT_MODE_AUTO not implemented yet, just use default */
> if (brightness_mode == TPACPI_BRGHT_MODE_AUTO ||
> brightness_mode == TPACPI_BRGHT_MODE_MAX) {
> - if (thinkpad_id.vendor == PCI_VENDOR_ID_IBM) {
> - /*
> - * IBM models that define HBRV probably have
> - * EC-based backlight level control
> - */
> - if (acpi_evalf(ec_handle, NULL, "HBRV", "qd"))
> - /* T40-T43, R50-R52, R50e, R51e, X31-X41 */
> - brightness_mode = TPACPI_BRGHT_MODE_ECNVRAM;
> - else
> - /* all other IBM ThinkPads */
> - brightness_mode = TPACPI_BRGHT_MODE_UCMS_STEP;
> - } else
> - /* All Lenovo ThinkPads */
> + if (quirks & TPACPI_BRGHT_Q_EC)
> + brightness_mode = TPACPI_BRGHT_MODE_ECNVRAM;
> + else
> brightness_mode = TPACPI_BRGHT_MODE_UCMS_STEP;
>
> dbg_printk(TPACPI_DBG_BRGHT,
> - "selected brightness_mode=%d\n",
> + "driver auto-selected brightness_mode=%d\n",
> brightness_mode);
> }
>
> @@ -5858,6 +5882,15 @@ static int __init brightness_init(struct
> vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
> "brightness is supported\n");
>
> + if (quirks & TPACPI_BRGHT_Q_ASK) {
> + printk(TPACPI_NOTICE
> + "brightness: will use unverified default: "
> + "brightness_mode=%d\n", brightness_mode);
> + printk(TPACPI_NOTICE
> + "brightness: please report to %s whether it works well "
> + "or not on your ThinkPad\n", TPACPI_MAIL);
> + }
> +
> ibm_backlight_device->props.max_brightness =
> (tp_features.bright_16levels)? 15 : 7;
> ibm_backlight_device->props.brightness = b & TP_EC_BACKLIGHT_LVLMSK;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/