2009-10-21 07:56:13

by Pavel Machek

[permalink] [raw]
Subject: spitz backlight: fix brightness limiting


On spitz (& similar) machines, if battery is running low, backlight
needs to be limited to lower step. Unfortunately, current code uses &=
for limiting, turning backlight off completely for some backlight
settings. Fix that.

Signed-off-by: Pavel Machek <[email protected]>

--- linux-rc/drivers/video/backlight/corgi_lcd.c 2009-10-18 18:11:36.000000000 +0200
+++ linux-rc/drivers/video/backlight/corgi_lcd.c 2009-10-16 02:10:13.000000000 +0200
@@ -433,8 +434,9 @@

if (corgibl_flags & CORGIBL_SUSPENDED)
intensity = 0;
- if (corgibl_flags & CORGIBL_BATTLOW)
- intensity &= lcd->limit_mask;
+
+ if ((corgibl_flags & CORGIBL_BATTLOW) && intensity > lcd->limit_mask)
+ intensity = lcd->limit_mask;

return corgi_bl_set_intensity(lcd, intensity);
}


--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


2009-10-21 20:20:51

by Eric Miao

[permalink] [raw]
Subject: Re: spitz backlight: fix brightness limiting

It's a little bit weird it's called 'limit_mask' when I first converted it
to a spi driver. There must be some reasons, Richard, you've got
any ideas?

On Tue, Oct 20, 2009 at 5:37 AM, Pavel Machek <[email protected]> wrote:
>
> On spitz (& similar) machines, if battery is running low, backlight
> needs to be limited to lower step. Unfortunately, current code uses &=
> for limiting, turning backlight off completely for some backlight
> settings. Fix that.
>
> Signed-off-by: Pavel Machek <[email protected]>
>
> --- linux-rc/drivers/video/backlight/corgi_lcd.c        2009-10-18 18:11:36.000000000 +0200
> +++ linux-rc/drivers/video/backlight/corgi_lcd.c        2009-10-16 02:10:13.000000000 +0200
> @@ -433,8 +434,9 @@
>
>        if (corgibl_flags & CORGIBL_SUSPENDED)
>                intensity = 0;
> -       if (corgibl_flags & CORGIBL_BATTLOW)
> -               intensity &= lcd->limit_mask;
> +
> +       if ((corgibl_flags & CORGIBL_BATTLOW) && intensity > lcd->limit_mask)
> +               intensity = lcd->limit_mask;
>
>        return corgi_bl_set_intensity(lcd, intensity);
>  }
>
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>

2009-10-22 07:05:30

by Pavel Machek

[permalink] [raw]
Subject: Re: spitz backlight: fix brightness limiting

On Thu 2009-10-22 04:20:34, Eric Miao wrote:
> It's a little bit weird it's called 'limit_mask' when I first converted it
> to a spi driver. There must be some reasons, Richard, you've got
> any ideas?

Yes, the code was "interesting"...

I can certainly do the rename, but maybe we can fix the issue now and
do futher cleanups later?
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-10-22 13:59:05

by Eric Miao

[permalink] [raw]
Subject: Re: spitz backlight: fix brightness limiting

On Thu, Oct 22, 2009 at 3:05 PM, Pavel Machek <[email protected]> wrote:
> On Thu 2009-10-22 04:20:34, Eric Miao wrote:
>> It's a little bit weird it's called 'limit_mask' when I first converted it
>> to a spi driver. There must be some reasons, Richard, you've got
>> any ideas?
>
> Yes, the code was "interesting"...
>
> I can certainly do the rename, but maybe we can fix the issue now and
> do futher cleanups later?

Fair enough, I'm fine with that - I'd like this to go through Richard's
backlight tree if possible, though.

2009-10-22 22:47:27

by Richard Purdie

[permalink] [raw]
Subject: Re: spitz backlight: fix brightness limiting

On Thu, 2009-10-22 at 04:20 +0800, Eric Miao wrote:
> It's a little bit weird it's called 'limit_mask' when I first converted it
> to a spi driver. There must be some reasons, Richard, you've got
> any ideas?

Its a historical artifact that once worked through some bitfield magic
but probably stopped working when we increased the range of values the
backlight could accept and we didn't notice the limiting problem.

I'll happily take a patch to fix that.

Cheers,

Richard

2009-10-23 05:20:56

by Pavel Machek

[permalink] [raw]
Subject: Re: spitz backlight: fix brightness limiting

On Thu 2009-10-22 23:43:19, Richard Purdie wrote:
> On Thu, 2009-10-22 at 04:20 +0800, Eric Miao wrote:
> > It's a little bit weird it's called 'limit_mask' when I first converted it
> > to a spi driver. There must be some reasons, Richard, you've got
> > any ideas?
>
> Its a historical artifact that once worked through some bitfield magic
> but probably stopped working when we increased the range of values the
> backlight could accept and we didn't notice the limiting problem.
>
> I'll happily take a patch to fix that.

Patch is available in the parent post. For now, I just fix it so that
it does not turn backlight off when it should not. I'll submit
limit_mask -> limit as separate patch, ok?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-10-23 07:57:15

by Eric Miao

[permalink] [raw]
Subject: Re: spitz backlight: fix brightness limiting

On Fri, Oct 23, 2009 at 1:20 PM, Pavel Machek <[email protected]> wrote:
> On Thu 2009-10-22 23:43:19, Richard Purdie wrote:
>> On Thu, 2009-10-22 at 04:20 +0800, Eric Miao wrote:
>> > It's a little bit weird it's called 'limit_mask' when I first converted it
>> > to a spi driver. There must be some reasons, Richard, you've got
>> > any ideas?
>>
>> Its a historical artifact that once worked through some bitfield magic
>> but probably stopped working when we increased the range of values the
>> backlight could accept and we didn't notice the limiting problem.
>>
>> I'll happily take a patch to fix that.
>
> Patch is available in the parent post. For now, I just fix it so that
> it does not turn backlight off when it should not. I'll submit
> limit_mask -> limit as separate patch, ok?

I'm fine with that. Feel free to.