Re: patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree

Re: patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree

[Stefan Lippers-Hollmann]

Hi

On Friday 07 August 2009, gregkh@suse.de wrote:
> This is a note to let you know that we have just queued up the patch titled
> 
>     Subject: thinkpad-acpi: fix incorrect use of TPACPI_BRGHT_MODE_ECNVRAM
> 
> to the 2.6.30-stable tree.  Its filename is
> 
>     thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch
> 
> A git repo of this tree can be found at 
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> 
> From 59fe4fe34d7afdf63208124f313be9056feaa2f4 Mon Sep 17 00:00:00 2001
> From: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Date: Sat, 1 Aug 2009 12:04:20 -0300
> Subject: thinkpad-acpi: fix incorrect use of TPACPI_BRGHT_MODE_ECNVRAM

This patch fails to compile if applied to 2.6.30 + current stable queue.

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[4]: *** Waiting for unfinished jobs....      

It seems to depend on the new quirk handling, introduced in:

commit 7d95a3d564901e88ed42810f054e579874151999
Author: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Date:   Sat May 30 13:25:06 2009 -0300

    thinkpad-acpi: add quirklist engine

    Add a quirklist engine suitable for matching ThinkPad firmware,
    and change the code to use it.

    Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
    Signed-off-by: Len Brown <len.brown@intel.com>

Regards
	Stefan Lippers-Hollmann

-- 
> From: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> 
> 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
> 
> Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> Reported-by: Tobias Diedrich <ranma+kernel@tdiedrich.de>
> Signed-off-by: Len Brown <len.brown@intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> ---
>  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;

Re: patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree

[Henrique de Moraes Holschuh]

On Fri, 07 Aug 2009, Stefan Lippers-Hollmann wrote:
> This patch fails to compile if applied to 2.6.30 + current stable queue.
[...]
> It seems to depend on the new quirk handling, introduced in:
> commit 7d95a3d564901e88ed42810f054e579874151999

Good catch.  And I _will_ need that quirk handling stuff to keep 2.6.30 in
shape :-(

The dependencies for the "thinkpad-acpi: fix incorrect use of
TPACPI_BRGHT_MODE_ECNVRAM" patch are:

commit 050df107c408a3df048524b3783a5fc6d4dccfdb
Author: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Date:   Sat May 30 13:25:05 2009 -0300

    thinkpad-acpi: store fw version with strict checking

    Extend the thinkpad model and firmware identification data with the
    release serial number for the BIOS and firmware (when available), as
    that is easier to parse and compare than the version strings.

    We're going to greatly extend the use of the ThinkPad DMI data through
    quirk lists, so it is best to be quite strict and make sure what we
    get from DMI is exactly what we expect, otherwise quirk matching may
    result in quite insane things.

    IBM (and Lenovo, at least for the ThinkPad line) uses this schema for
    firmware versioning and model:

    Firmware model: Two digits, [0-9A-Z]

    Firmware version: AABBCCDD, where
      AA = firmware model, see above
      BB = "ET" for BIOS, "HT" for EC
      CC = release version, two digits, [0-9A-Z],
           "00" < "09" < "0A" < "10" < "A0" < "ZZ"
      DD = "WW"

    Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
    Signed-off-by: Len Brown <len.brown@intel.com>

And, as identified by Stefan,

commit 7d95a3d564901e88ed42810f054e579874151999
Author: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Date:   Sat May 30 13:25:06 2009 -0300

    thinkpad-acpi: add quirklist engine

    Add a quirklist engine suitable for matching ThinkPad firmware,
    and change the code to use it.

    Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
    Signed-off-by: Len Brown <len.brown@intel.com>

I cherry picked the commits on the order below, on top of v2.6.30.4 and it
compiled properly (and visual inspection of the result shows it is fine):

050df107c408a3df048524b3783a5fc6d4dccfdb
7d95a3d564901e88ed42810f054e579874151999
59fe4fe34d7afdf63208124f313be9056feaa2f4

Now, commit 7d95a3d564901e88ed42810f054e579874151999 could be reduced a bit
for -stable if you guys prefer (only the first hunk is really necessary),
but that would just make things more complicated later on if I find myself
in need to patch anything in the fan control subdriver for 2.6.30.y
(unlikely, but still...)

-- 
  "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: [stable] patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree

[Greg KH]

On Thu, Aug 06, 2009 at 08:31:11PM -0300, Henrique de Moraes Holschuh wrote:
> On Fri, 07 Aug 2009, Stefan Lippers-Hollmann wrote:
> > This patch fails to compile if applied to 2.6.30 + current stable queue.
> [...]
> > It seems to depend on the new quirk handling, introduced in:
> > commit 7d95a3d564901e88ed42810f054e579874151999
> 
> Good catch.  And I _will_ need that quirk handling stuff to keep 2.6.30 in
> shape :-(

Oops, sorry for missing this.

> The dependencies for the "thinkpad-acpi: fix incorrect use of
> TPACPI_BRGHT_MODE_ECNVRAM" patch are:

Due to the need to add a new feature just to get these quirks to work,
I'll just drop this single patch, which should keep everything building
properly, right?

thanks,

greg k-h

Re: [stable] patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree

[Stefan Lippers-Hollmann]

Hi

On Friday 14 August 2009, Greg KH wrote:
> On Thu, Aug 06, 2009 at 08:31:11PM -0300, Henrique de Moraes Holschuh wrote:
> > On Fri, 07 Aug 2009, Stefan Lippers-Hollmann wrote:
> > > This patch fails to compile if applied to 2.6.30 + current stable queue.
> > [...]
> > > It seems to depend on the new quirk handling, introduced in:
> > > commit 7d95a3d564901e88ed42810f054e579874151999
> > 
> > Good catch.  And I _will_ need that quirk handling stuff to keep 2.6.30 in
> > shape :-(
> 
> Oops, sorry for missing this.
> 
> > The dependencies for the "thinkpad-acpi: fix incorrect use of
> > TPACPI_BRGHT_MODE_ECNVRAM" patch are:
> 
> Due to the need to add a new feature just to get these quirks to work,
> I'll just drop this single patch, which should keep everything building
> properly, right?

Yes, dropping this patch
thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch
results in a buildable tree again.

> thanks,
> 
> greg k-h

Regards
	Stefan Lippers-Hollmann

Re: [stable] patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree

[Greg KH]

On Fri, Aug 14, 2009 at 01:02:45AM +0200, Stefan Lippers-Hollmann wrote:
> Hi
> 
> On Friday 14 August 2009, Greg KH wrote:
> > On Thu, Aug 06, 2009 at 08:31:11PM -0300, Henrique de Moraes Holschuh wrote:
> > > On Fri, 07 Aug 2009, Stefan Lippers-Hollmann wrote:
> > > > This patch fails to compile if applied to 2.6.30 + current stable queue.
> > > [...]
> > > > It seems to depend on the new quirk handling, introduced in:
> > > > commit 7d95a3d564901e88ed42810f054e579874151999
> > > 
> > > Good catch.  And I _will_ need that quirk handling stuff to keep 2.6.30 in
> > > shape :-(
> > 
> > Oops, sorry for missing this.
> > 
> > > The dependencies for the "thinkpad-acpi: fix incorrect use of
> > > TPACPI_BRGHT_MODE_ECNVRAM" patch are:
> > 
> > Due to the need to add a new feature just to get these quirks to work,
> > I'll just drop this single patch, which should keep everything building
> > properly, right?
> 
> Yes, dropping this patch
> thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch
> results in a buildable tree again.

Thanks again so much for verifying this, I really appreciate it.

greg k-h

Re: [stable] patch thinkpad-acpi-fix-incorrect-use-of-tpacpi_brght_mode_ecnvram.patch added to 2.6.30-stable tree

[Henrique de Moraes Holschuh]

On Thu, 13 Aug 2009, Greg KH wrote:
> On Thu, Aug 06, 2009 at 08:31:11PM -0300, Henrique de Moraes Holschuh wrote:
> > On Fri, 07 Aug 2009, Stefan Lippers-Hollmann wrote:
> > > This patch fails to compile if applied to 2.6.30 + current stable queue.
> > [...]
> > > It seems to depend on the new quirk handling, introduced in:
> > > commit 7d95a3d564901e88ed42810f054e579874151999
> > 
> > Good catch.  And I _will_ need that quirk handling stuff to keep 2.6.30 in
> > shape :-(
> 
> Oops, sorry for missing this.
> 
> > The dependencies for the "thinkpad-acpi: fix incorrect use of
> > TPACPI_BRGHT_MODE_ECNVRAM" patch are:
> 
> Due to the need to add a new feature just to get these quirks to work,
> I'll just drop this single patch, which should keep everything building
> properly, right?

Right.  I will hack something and submit it to the next stable.

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