Re: [PATCH 1/4] drm/i915: save/restore the legacy backlight control

["Indan Zupancic" <indan@nul.nu>]

On Tue, August 28, 2012 17:15, Daniel Vetter wrote:
> On Tue, Aug 28, 2012 at 04:49:15PM +0200, Indan Zupancic wrote:
>> By the way, saving LBPC only makes sense if it's done before it was
>> set to 0 to disable the panel. I don't know if the current code does
>> the right thing, I haven't looked at it for a while.
>
> I think we can coax it into doing the right thing, see my other mail. If
> your completely sure that lbpc /should/ be handled by the bios across s/r
> I think we can drop this. But tbh I have no idea how this really is
> supposed to work, and unfortunately we're not allowed to cross-check with
> the windows driver codebase :(

Of course I'm not completely sure. But I agree with Chris' reasoning,
why else have two ways of controlling the backlight? To me LBPC gives
the impression of being a system specific thing the OS shouldn't need
to worry about. Partly because it wasn't documented in the old docs at
all, and its address isn't mentioned in the 965 vol3 manual, but also
because it's from around the time that the BIOS used to do a lot more
directly (APM versus ACPI etc.) It also gives a way to lower the max
output voltage (via PWM) to the backlight hardware.

These kind of problems are caused by us not having exactly the same
info as the BIOS/machine makers got. Because that's what guided them
into making the stuff as it is, not how it's supposed to work.

The code could check if LBPC has a reasonable value after resume: Do
nothing if it has and restore the last known good value if it doesn't.

Writing 0 to LBPC before suspend seems unnecessary because the hardware
is going into sleep mode anyway. So the PWM signal should become 0 anyway,
or if it doesn't, the BIOS has a chance to disable the backlight. But I'm
not sure if that's really true.

So to get back to this patch, saving the LBPC value only makes sense if
it's done before it's set to 0. I think saving it at i915_save_display()
time is too late if the panel got disabled. Then the code is always saving
and restoring zeros.

BTW, inverted sense of the PWM register could be related to the polarity
of the PWM signal. That should be controlled with bit 28 in BLC_PWM_CTL2.

Greetings,

Indan