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
