On Thu, 30 Aug 2012 11:29:11 +0200 "Indan Zupancic" <indan at nul.nu> wrote: > On Tue, August 28, 2012 16:55, Daniel Vetter wrote: > > On Tue, Aug 28, 2012 at 04:39:34PM +0200, Indan Zupancic wrote: > >> Some backlight problems on GEN4 can be solved by not fiddling with the > >> backlight. The current code sets the backlight to 0 to disable the panel > >> (last year anyway, maybe the code changed), but on gen >= 4 it can do the > >> same by clearing bit 31 in BLC_PWM_CLT2. That way the original backlight > >> value doesn't need to be saved anywhere. > > > > A while back I've improved the backlight code to properly switch the pipe > > that controls the pwm and also to properly toggle the enable bit for > > gen4+, see the new intel_panel_enable_backlight functions. Would it be > > correct in your opinion to simply ditch the call to > > intel_panel_actually_set_backlight for gen4+ unconditionally? Same for > > intel_panel_disable_backlight obviously? > > Yes, that seems the whole point of having the PWM disable bit. > > I would also ditch the backlight_enabled state tracking, as it's > unnecessary and incorrect because the enable/disable calls aren't > balanced. > > I'll update my kernel to the latest git code and try out how the > current code works and if not touching LBPC at all works for gen 2 > hardware. If it doesn't then LBPC needs to be saved/restored in > i915_suspend.c after all. > > It would be nice if backlight control worked with ASLE disabled, > that would get rid of all complexity, including combination mode. > Or alternatively, if disabling combination mode at boot works then > we can get rid of the complicated brightness setting code to cope > with it. Did we bottom out on this discussion? Reading through the thread I don't see any firm conclusions, and we definitely still have bugs open that may be resolved by this incorrect patchset... -- Jesse Barnes, Intel Open Source Technology Center