Re: [PATCH] fix backlight brightness on intel LVDS panel after reopening lid

[Indan Zupancic]

On Tue, February 22, 2011 22:04, Jesse Barnes wrote:
> On Sat, 19 Feb 2011 15:07:49 -0800
> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>> On Sat, Feb 19, 2011 at 4:26 AM, Alex Riesen <raa.lkml@xxxxxxxxx> wrote:
>> > On Sat, Feb 19, 2011 at 13:11, Alex Riesen <raa.lkml@xxxxxxxxx> wrote:
>> >>> Lastly, could you verify that my patch at https://lkml.org/lkml/2011/2/16/447
>> fixes
>> >>> it for you too? (Make sure you're at max brightness before rebooting.)
>> >>
>> >> I'll try it now.
>> >>
>> >
>> > I can confirm that it does fix backlight in my case (Dell XPS 1330,
>> > LVDS panel, GM965/GL960).
>> >
>> > Tested-by: Alex Riesen <raa.lkml@xxxxxxxxx>
>>
>> Guys, should I apply this, or will I get it through somebody's pull?
>
> I'm worried that removing combo mode will break some working setups,
> but if it's seen a lot of testing and is ok, then I'm fine with it, as
> it definitely simplifies things.

This all seems new code added in 2.6.37, it wasn't there before. The working
setups stopped working when that code was added. The only reason it may work
for some gen 2 and gen 3 hardware is because of a random value of the max
brightness (bit 16).  The buggy code seems to be written in a haste without
any testing done. It's so off from the official documentation that I suspect
that the windows driver was used as reference, but its code was misinterpreted.

I grepped the userspace driver source, and LBPC is defined there for 810,
but not used anywhere either.

This patch should be added to stable kernel 2.6.37.2, because it messes
up the LPBC register, which some laptops store between boots.

Quoting https://bugzilla.kernel.org/show_bug.cgi?id=23472#c57

- Checking bit 16 in BLC_PWM_CTL is wrong, it has no special meaning.

- If LBPC == 0xff, it should be ignored and it's not in combination mode.
  (This is for gen 3).

- Gen 2 documentation doesn't mention LBPC or combination mode at all.
  Gen 3 does, but doesn't tell what the register value is or how to use it,
  it just mentions it.

- The calculations are rubbish, resulting in bogus LBPC values, and
  depending on how lucky you are, it writes different values for the
  registers after a restore.

All this code is new and causes problems, while everything worked before
just fine, when the driver didn't do anything special.

So it seems a bit like voodoo programming, because nothing the driver did
followed the official Intel documentation.

Now, there may be real reasons for some of the code, but I propose we add
them one at a time when people show up with problems without the weird code
added. That way the reason for any weirdness is also documented.

Greetings,

Indan


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